linux-kernel - Re: [PATCH v2 3/6] ARM: dts: rockchip: remove interrupts properties from pwm nodes rv1108.dtsi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGb2v65+A402jCPVRJdDBdxqAEYOJmFTkKB4LJCvnW89hXb8QA@mail.gmail.com>
Date:   Mon, 12 Apr 2021 18:33:03 +0800
From:   Chen-Yu Tsai <wens@...nel.org>
To:     Johan Jonker <jbx6244@...il.com>
Cc:     Heiko Stübner <heiko@...ech.de>,
        Rob Herring <robh+dt@...nel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>, Lee Jones <lee.jones@...aro.org>,
        linux-pwm@...r.kernel.org, devicetree <devicetree@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 3/6] ARM: dts: rockchip: remove interrupts properties
 from pwm nodes rv1108.dtsi

On Mon, Apr 12, 2021 at 6:03 PM Johan Jonker <jbx6244@...il.com> wrote:
>
> On 4/12/21 5:15 AM, Chen-Yu Tsai wrote:
> > On Sun, Apr 11, 2021 at 9:11 PM Johan Jonker <jbx6244@...il.com> wrote:
> >>
> >> A test with the command below gives this error:
> >>
> >> /arch/arm/boot/dts/rv1108-evb.dt.yaml:
> >> pwm@...80000: 'interrupts' does not match any of the regexes:
> >> 'pinctrl-[0-9]+'
> >>
> >> "interrupts" is an undocumented property, so remove them
> >> from pwm nodes in rv1108.dtsi.
> >>
> >> make ARCH=arm dtbs_check
> >> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
> >>
> >> Signed-off-by: Johan Jonker <jbx6244@...il.com>
> >
> > Given that the interrupts were specified, meaning they are wired up in hardware,
> > shouldn't the solution be to add the interrupts property to the binding instead?
> >
> > After all, the device tree describes the actual hardware, not just what the
> > implementations need.
> >
> > ChenYu
> >
>
> Hi,
>
> The question of what to do with it was asked in version 1, but no answer
> was given, so I made a proposal.
> The device tree description should be complete, but also as lean as
> possible. If someone manages to sneak in undocumented properties without
> reason then the ultimate consequence should be removal I think.
>
> Not sure about the (missing?) rv1108 TRM, but for rk3328 the interrupt
> is used for:
>
> PWM_INTSTS 0x0040 W 0x00000000 Interrupt Status Register
>   Channel Interrupt Polarity Flag
>     This bit is used in capture mode in order to identify the
>     transition of the input waveform when interrupt is generated.
>   Channel Interrupt Status
>     Interrupt generated
>
> PWM_INT_EN 0x0044 W 0x00000000 Interrupt Enable Register
>   Channel Interrupt Enable
>
> Is there any current realistic use/setup for it to convince rob+dt this
> should be added to pwm-rockchip.yaml?

Well, the PWM core has capture support, and pwm-sti implements it with
interrupt support, so I guess there's at least a legitimate case for
adding that to the binding. Whether someone has an actual use case for
it and adds code to implement it is another story.

> The rk3328 interrupt rkpwm_int seems shared between channels, but only
> included to pwm3. What is the proper way for that?

I guess the bigger question is why was the PWM controller split into
four device nodes, instead of just one encompassing the whole block.
Now we'd have to introduce a new binding to support capture mode and
interrupts.

In that case I agree with dropping the interrupts for now, as it just
won't fit. But I would add this additional information to the commit
message.


Regards
ChenYu