[PATCH] dt-bindings: pwm: rockchip: Allow "interrupts" prooperty

[PATCH] dt-bindings: pwm: rockchip: Allow "interrupts" prooperty

[Uwe Kleine-König]

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(+)

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:

base-commit: d73f444d06fb8a42a5c0623453f3ea1fe9880229
-- 
2.43.0

Re: [PATCH] dt-bindings: pwm: rockchip: Allow "interrupts" prooperty

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 298 bytes --]

Hello,

not sure who will pick this up. Whoever it does, please do

Subject ~= s/proop/prop/

on application.

Best regards,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] dt-bindings: pwm: rockchip: Allow "interrupts" prooperty

[Krzysztof Kozlowski]

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

Re: [PATCH] dt-bindings: pwm: rockchip: Allow "interrupts" prooperty

[Johan Jonker]

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

Re: [PATCH] dt-bindings: pwm: rockchip: Allow "interrupts" prooperty

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2107 bytes --]

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] dt-bindings: pwm: rockchip: Allow "interrupts" prooperty

[Krzysztof Kozlowski]

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

Re: [PATCH] dt-bindings: pwm: rockchip: Allow "interrupts" prooperty

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2789 bytes --]

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] dt-bindings: pwm: rockchip: Allow "interrupts" prooperty

[Johan Jonker]

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
>

Re: [PATCH] dt-bindings: pwm: rockchip: Allow "interrupts" prooperty

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]