On 1/24/24 09:26, Uwe Kleine-König wrote: > Hello Johan, > > On Sun, Jan 07, 2024 at 11:30:14AM +0100, Krzysztof Kozlowski wrote: >> On 07/01/2024 00:25, Uwe Kleine-König wrote: >>> On Sat, Jan 06, 2024 at 10:25:10PM +0100, Johan Jonker wrote: >>>> On 1/6/24 18:10, Krzysztof Kozlowski wrote: >>>>> On 06/01/2024 15:26, Uwe Kleine-König wrote: >>>>>> This fixes the dtbs_check error >>>>>> >>>>>> arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dtb: pwm@10280030: 'interrupts' does not match any of the regexes: 'pinctrl-[0-9]+' >>>>>> from schema $id: http://devicetree.org/schemas/pwm/pwm-rockchip.yaml# >>>>>> >>>>>> in several device trees. >>>>>> >>>>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> >>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> >>>> NAK >>>> >>>> There's a reason why this isn't implemented before: >>>> >>>> [RFC PATCH v1 1/2] dt-bindings: pwm: rockchip: add interrupts property <https://lore.kernel.org/linux-rockchip/ed3df2c8-ffb5-1723-0ed7-3a2721972852@xxxxxxxxx/#r> >>>> >>>> https://lore.kernel.org/linux-rockchip/ed3df2c8-ffb5-1723-0ed7-3a2721972852@xxxxxxxxx/ >>>> >>>> [PATCH 1/1] dt-bindings: pwm: rockchip: Add description for rk3588 <https://lore.kernel.org/linux-rockchip/20220901135523.52151-1-sebastian.reichel@xxxxxxxxxxxxx/#r> >>>> >>>> https://lore.kernel.org/linux-rockchip/66b5b616-ae9f-a1aa-e2b5-450f570cfcdd@xxxxxxxxx/ >>>> >>>> [PATCH v1 03/11] dt-bindings: pwm: rockchip: add rockchip,rk3128-pwm <https://lore.kernel.org/linux-rockchip/f5dd0ee4-d97e-d878-ffde-c06e9b233e38@xxxxxxxxx/> >>>> >>>> https://lore.kernel.org/linux-rockchip/946d8ac2-6ff2-093a-ad3c-aa755e00d1dd@xxxxxxx/ >>>> >>>> >>>> On how to correctly model the DT with common interrupts , PWM and one shot as a sort of MFD etc there's no consensus yet. >>>> >>>> Leaf it as it is till someone made a working driver demo, so that the coder is free to model a DT solution that fits to him/her. >>> Having the warnings until this happens is bad though. If describing the >>> irqs in the schema is considered wrong, we should remove the interrupts >>> properties from the device tree sources. >> I think the previous thread mixes bindings with driver. Does the >> hardware have interrupt? Yes? Add it to the bindings. No? Don't add it. >> >> However Johan's reply is saying something about driver, so how is it >> related? > Following Krzysztof's argumentation I'm inclined to apply the patch > despite Johan's objection as the irqs are already described in the > device trees and not having them in the binding only adds warnings to > the dt checks. The interrupts are common interrupts for a group of 4 PWM channels! The name PWM channels is somewhat confusing. It has various modes not related to PWM >From Rockchip RK3368 TRM V2.0.pdf: Continuous Mode: No interrupts. One-shot Mode: an interrupt is asserted to inform that the operation has been finished Reference mode: asserts an interrupt when the polarity of the input waveform changes. Also IR === PWM_INTSTS0x00040 W0x00000000 Interrupt Status Register PWM_INT_EN0x00044 W0x00000000 Interrupt Enable Register PWM_FIFO_CTRL0x00050 W0x00000000 PWM Channel 3 FIFO Mode Control Register PWM_FIFO_INTSTS0x00054 W0x00000000 FIFO Interrupts Status Register === pwm0: pwm@ff680000 { compatible = "rockchip,rk3368-pwm", "rockchip,rk3288-pwm"; reg = <0x0 0xff680000 0x0 0x10>; }; pwm1: pwm@ff680010 { compatible = "rockchip,rk3368-pwm", "rockchip,rk3288-pwm"; reg = <0x0 0xff680010 0x0 0x10>; }; pwm2: pwm@ff680020 { compatible = "rockchip,rk3368-pwm", "rockchip,rk3288-pwm"; reg = <0x0 0xff680020 0x0 0x10>; }; pwm3: pwm@ff680030 { compatible = "rockchip,rk3368-pwm", "rockchip,rk3288-pwm"; reg = <0x0 0xff680030 0x0 0x10>; }; === The interrupt registers are located outside the PWM range and have nothing to do with the PWM driver. Adding them to a PWM binding is just bogus. They are a left over from the manufacturer tree that use them in a IR detection driver. Heiko keeps them because someone outside mainline kernel might use them? They should be removed and remodeled in a new sort of MFD node that handles all operating 3 modes. Johan > > Best regards > Uwe >
