On 4/12/21 12:33 PM, Chen-Yu Tsai wrote: > On Mon, Apr 12, 2021 at 6:03 PM Johan Jonker <jbx6244@xxxxxxxxx> wrote: >> >> On 4/12/21 5:15 AM, Chen-Yu Tsai wrote: >>> On Sun, Apr 11, 2021 at 9:11 PM Johan Jonker <jbx6244@xxxxxxxxx> wrote: >>>> >>>> A test with the command below gives this error: >>>> >>>> /arch/arm/boot/dts/rv1108-evb.dt.yaml: >>>> pwm@10280000: '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@xxxxxxxxx> >>> >>> 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? Found: pwm3 combined with ir uses a irq. Keep that as it is for now. https://github.com/rockchip-linux/kernel/blob/develop-4.19/drivers/input/remotectl/rockchip_pwm_remotectl.c > > 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. Will wait with adding "interrupts" to pwm-rockchip.yaml till someone makes a solution for the whole block. Convert only current document/binding to reduce notifications. For Heiko: patch 3 + 5 can go in the garbage bin: [PATCH v2 3/6] ARM: dts: rockchip: remove interrupts properties from pwm nodes rv1108.dtsi [PATCH v2 5/6] arm64: dts: rockchip: remove interrupts properties from pwm nodes rk3328.dtsi Johan > > > Regards > ChenYu >
