| Message ID | 20240106142654.1262758-2-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: pwm: rockchip: Allow "interrupts" prooperty | expand |
Hello, not sure who will pick this up. Whoever it does, please do Subject ~= s/proop/prop/ on application. Best regards, Uwe
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@pengutronix.de> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
Hi, 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@pengutronix.de> > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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@gmail.com/#r> https://lore.kernel.org/linux-rockchip/ed3df2c8-ffb5-1723-0ed7-3a2721972852@gmail.com/ [PATCH 1/1] dt-bindings: pwm: rockchip: Add description for rk3588 <https://lore.kernel.org/linux-rockchip/20220901135523.52151-1-sebastian.reichel@collabora.com/#r> https://lore.kernel.org/linux-rockchip/66b5b616-ae9f-a1aa-e2b5-450f570cfcdd@gmail.com/ [PATCH v1 03/11] dt-bindings: pwm: rockchip: add rockchip,rk3128-pwm <https://lore.kernel.org/linux-rockchip/f5dd0ee4-d97e-d878-ffde-c06e9b233e38@gmail.com/> https://lore.kernel.org/linux-rockchip/946d8ac2-6ff2-093a-ad3c-aa755e00d1dd@arm.com/ 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. Johan > Best regards, > Krzysztof > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hello, 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@pengutronix.de> > > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > 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@gmail.com/#r> > > https://lore.kernel.org/linux-rockchip/ed3df2c8-ffb5-1723-0ed7-3a2721972852@gmail.com/ > > [PATCH 1/1] dt-bindings: pwm: rockchip: Add description for rk3588 <https://lore.kernel.org/linux-rockchip/20220901135523.52151-1-sebastian.reichel@collabora.com/#r> > > https://lore.kernel.org/linux-rockchip/66b5b616-ae9f-a1aa-e2b5-450f570cfcdd@gmail.com/ > > [PATCH v1 03/11] dt-bindings: pwm: rockchip: add rockchip,rk3128-pwm <https://lore.kernel.org/linux-rockchip/f5dd0ee4-d97e-d878-ffde-c06e9b233e38@gmail.com/> > > https://lore.kernel.org/linux-rockchip/946d8ac2-6ff2-093a-ad3c-aa755e00d1dd@arm.com/ > > > 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. Best regards Uwe
On 07/01/2024 00:25, Uwe Kleine-König wrote: > Hello, > > 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@pengutronix.de> >> >>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> >> 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@gmail.com/#r> >> >> https://lore.kernel.org/linux-rockchip/ed3df2c8-ffb5-1723-0ed7-3a2721972852@gmail.com/ >> >> [PATCH 1/1] dt-bindings: pwm: rockchip: Add description for rk3588 <https://lore.kernel.org/linux-rockchip/20220901135523.52151-1-sebastian.reichel@collabora.com/#r> >> >> https://lore.kernel.org/linux-rockchip/66b5b616-ae9f-a1aa-e2b5-450f570cfcdd@gmail.com/ >> >> [PATCH v1 03/11] dt-bindings: pwm: rockchip: add rockchip,rk3128-pwm <https://lore.kernel.org/linux-rockchip/f5dd0ee4-d97e-d878-ffde-c06e9b233e38@gmail.com/> >> >> https://lore.kernel.org/linux-rockchip/946d8ac2-6ff2-093a-ad3c-aa755e00d1dd@arm.com/ >> >> >> 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? Best regards, Krzysztof
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@pengutronix.de> > >> > >>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> > >> 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@gmail.com/#r> > >> > >> https://lore.kernel.org/linux-rockchip/ed3df2c8-ffb5-1723-0ed7-3a2721972852@gmail.com/ > >> > >> [PATCH 1/1] dt-bindings: pwm: rockchip: Add description for rk3588 <https://lore.kernel.org/linux-rockchip/20220901135523.52151-1-sebastian.reichel@collabora.com/#r> > >> > >> https://lore.kernel.org/linux-rockchip/66b5b616-ae9f-a1aa-e2b5-450f570cfcdd@gmail.com/ > >> > >> [PATCH v1 03/11] dt-bindings: pwm: rockchip: add rockchip,rk3128-pwm <https://lore.kernel.org/linux-rockchip/f5dd0ee4-d97e-d878-ffde-c06e9b233e38@gmail.com/> > >> > >> https://lore.kernel.org/linux-rockchip/946d8ac2-6ff2-093a-ad3c-aa755e00d1dd@arm.com/ > >> > >> > >> 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. Best regards Uwe
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@pengutronix.de> >>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>> 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@gmail.com/#r> >>>> >>>> https://lore.kernel.org/linux-rockchip/ed3df2c8-ffb5-1723-0ed7-3a2721972852@gmail.com/ >>>> >>>> [PATCH 1/1] dt-bindings: pwm: rockchip: Add description for rk3588 <https://lore.kernel.org/linux-rockchip/20220901135523.52151-1-sebastian.reichel@collabora.com/#r> >>>> >>>> https://lore.kernel.org/linux-rockchip/66b5b616-ae9f-a1aa-e2b5-450f570cfcdd@gmail.com/ >>>> >>>> [PATCH v1 03/11] dt-bindings: pwm: rockchip: add rockchip,rk3128-pwm <https://lore.kernel.org/linux-rockchip/f5dd0ee4-d97e-d878-ffde-c06e9b233e38@gmail.com/> >>>> >>>> https://lore.kernel.org/linux-rockchip/946d8ac2-6ff2-093a-ad3c-aa755e00d1dd@arm.com/ >>>> >>>> >>>> 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 >
Hello Johan, On Wed, Jan 24, 2024 at 10:40:15AM +0100, Johan Jonker wrote: > 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. After some more feedback on irc, I sent a patch removing the interrupts property. However I failed to Cc: you on it. This first patch catched your attention even without you being explicitly Cc:d, so I hoped the new patch also hit your inbox. Find it at https://lore.kernel.org/linux-pwm/20240129113205.2453029-2-u.kleine-koenig@pengutronix.de and tell me if you want a bounce. Best regards Uwe
diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml b/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml index 65bfb492b3a4..031a448aa2b3 100644 --- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml +++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml @@ -45,6 +45,9 @@ properties: clock-names: maxItems: 2 + interrupts: + maxItems: 1 + "#pwm-cells": enum: [2, 3] description:
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@pengutronix.de> --- Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml | 3 +++ 1 file changed, 3 insertions(+) base-commit: d73f444d06fb8a42a5c0623453f3ea1fe9880229