| Message ID | 20240106142654.1262758-2-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: pwm: rockchip: Allow "interrupts" prooperty | expand |
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
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