[PATCH 1/1] dt-bindings: pwm: rockchip: Add description for rk3588

[Sebastian Reichel]

Hi,

On Wed, Sep 28, 2022 at 08:29:35PM +0200, Johan Jonker wrote:
> On 9/13/22 16:16, Sebastian Reichel wrote:
> > On Tue, Sep 13, 2022 at 11:12:02AM +0200, Uwe Kleine-König wrote:
> >> Hello Rob,
> >>
> >> On Thu, Sep 01, 2022 at 02:14:55PM -0500, Rob Herring wrote:
> >>> On Thu, 01 Sep 2022 15:55:23 +0200, Sebastian Reichel wrote:
> >>>> Add "rockchip,rk3588-pwm" compatible string for PWM nodes found
> >>>> on a rk3588 platform.
> >>>>
> >>>> Signed-off-by: Sebastian Reichel <sebastian.reichel at collabora.com>
> >>>> ---
> >>>> No driver changes required.
> >>>> ---
> >>>>  Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml | 1 +
> >>>>  1 file changed, 1 insertion(+)
> >>>>
> >>>
> >>> Running 'make dtbs_check' with the schema in this patch gives the
> >>> following warnings. Consider if they are expected or the schema is
> >>> incorrect. These may not be new warnings.
> >>>
> >>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> >>> This will change in the future.
> >>
> >> Is this a list of *new* warnings, or is the report (somewhat) orthogonal
> >> to the actual change and you just used the opportunity that someone
> >> touched the pwm-rockchip binding to point out that there is some cleanup
> >> to do?
> >>
> >>> Full log is available here: https://patchwork.ozlabs.org/patch/
> >>
> >> Hm, that gives me a 404.
> > 
> 
> > This is an existing problem with the rv1108 binding.
> > The rk3588 does not have pwm interrupts.
> 
> Hi,
> 
> Could you recheck?
> 
> From Rockchip RK3588 Datasheet V0.1-20210727.pdf:

Indeed. Looks like I missed the PWM interrupts because the list
of interrupts is in two columns in the datasheet and I only looked
through the first one assuming it was one long line. I should have
checked this more carefully. Sorry about that.

> ===
> ARM: dts: rk3288: add the interrupts property for PWM 
> https://github.com/rockchip-linux/kernel/commit/16b7b284618d1652e694f6286f575ce82f5f03e5
> 
> Comment:
> At the moment, we can find the remotectl pwm is needed on box.
> We can add the property for all PWMs. AFAIK, the pwm driver don't use it
> but the drivers/input/remotectl/rockchip_pwm_remotectl.c
> ===

rk3288 != rk3588. That's a different SoC. The downstream kernel I
used is this one and does not describe the interrupts:

https://github.com/radxa/kernel/tree/stable-5.10-rock5/arch/arm64/boot/dts/rockchip

I suppose we have these options:

1. Remove the 'interrupts' property from any upstream rockchip DT,
   since they are not used by SW? They can always be properly
   introduced without breaking backwards compatibility.

2. Describe 'interrupts' in the DT binding as optional with
   description to shut up the warning and otherwise ignore the
   problem until somebody has code to use the interrupts.

3. Same as 2, but also add the interrupts for rk356x and rk3588
   even though they are not used at the moment.

4. Describe 'interrupts' as mandatory in the DT binding for any
   rockchip SoC that has them and add them in DT. Considering
   normal PWM usage does not need them at all that seems wrong
   to me.

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-rockchip/attachments/20220929/c18227e7/attachment.sig>