[v15] Add PWM support for IPQ chipsets

[PATCH V15 2/4] dt-bindings: pwm: add IPQ6018 binding

Posted by Devi Priya 2 years, 2 months ago

DT binding for the PWM block in Qualcomm IPQ6018 SoC.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
---
v15:

  No change

v14:

  Picked up the R-b tag

v13:

  Updated the file name to match the compatible
  
  Sorted the properties and updated the order in the required field

  Dropped the syscon node from examples

v12:

  Picked up the R-b tag

v11:

  No change

v10:

  No change

v9:

  Add 'ranges' property to example (Rob)

  Drop label in example (Rob)

v8:

  Add size cell to 'reg' (Rob)

v7:

  Use 'reg' instead of 'offset' (Rob)

  Drop 'clock-names' and 'assigned-clock*' (Bjorn)

  Use single cell address/size in example node (Bjorn)

  Move '#pwm-cells' lower in example node (Bjorn)

  List 'reg' as required

v6:

  Device node is child of TCSR; remove phandle (Rob Herring)

  Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)

v5: Use qcom,pwm-regs for phandle instead of direct regs (Bjorn
    Andersson, Kathiravan T)

v4: Update the binding example node as well (Rob Herring's bot)

v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)

v2: Make #pwm-cells const (Rob Herring)

 .../bindings/pwm/qcom,ipq6018-pwm.yaml        | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml

diff --git a/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml b/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
new file mode 100644
index 000000000000..6d0d7ed271f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/qcom,ipq6018-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm IPQ6018 PWM controller
+
+maintainers:
+  - Baruch Siach <baruch@tkos.co.il>
+
+properties:
+  compatible:
+    const: qcom,ipq6018-pwm
+
+  reg:
+    description: Offset of PWM register in the TCSR block.
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - "#pwm-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
+
+    pwm: pwm@a010 {
+        compatible = "qcom,ipq6018-pwm";
+        reg = <0xa010 0x20>;
+        clocks = <&gcc GCC_ADSS_PWM_CLK>;
+        assigned-clocks = <&gcc GCC_ADSS_PWM_CLK>;
+        assigned-clock-rates = <100000000>;
+        #pwm-cells = <2>;
+    };
-- 
2.34.1

Re: [PATCH V15 2/4] dt-bindings: pwm: add IPQ6018 binding

Posted by Uwe Kleine-König 2 years, 2 months ago

Hello,

On Thu, Oct 05, 2023 at 09:35:48PM +0530, Devi Priya wrote:
> DT binding for the PWM block in Qualcomm IPQ6018 SoC.
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> ---
> v15:
> 
>   No change
> 
> v14:
> 
>   Picked up the R-b tag
> 
> v13:
> 
>   Updated the file name to match the compatible
>   
>   Sorted the properties and updated the order in the required field
> 
>   Dropped the syscon node from examples
> 
> v12:
> 
>   Picked up the R-b tag
> 
> v11:
> 
>   No change
> 
> v10:
> 
>   No change
> 
> v9:
> 
>   Add 'ranges' property to example (Rob)
> 
>   Drop label in example (Rob)
> 
> v8:
> 
>   Add size cell to 'reg' (Rob)
> 
> v7:
> 
>   Use 'reg' instead of 'offset' (Rob)
> 
>   Drop 'clock-names' and 'assigned-clock*' (Bjorn)
> 
>   Use single cell address/size in example node (Bjorn)
> 
>   Move '#pwm-cells' lower in example node (Bjorn)
> 
>   List 'reg' as required
> 
> v6:
> 
>   Device node is child of TCSR; remove phandle (Rob Herring)
> 
>   Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)
> 
> v5: Use qcom,pwm-regs for phandle instead of direct regs (Bjorn
>     Andersson, Kathiravan T)
> 
> v4: Update the binding example node as well (Rob Herring's bot)
> 
> v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
> 
> v2: Make #pwm-cells const (Rob Herring)
> 
>  .../bindings/pwm/qcom,ipq6018-pwm.yaml        | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml b/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
> new file mode 100644
> index 000000000000..6d0d7ed271f7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/qcom,ipq6018-pwm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm IPQ6018 PWM controller
> +
> +maintainers:
> +  - Baruch Siach <baruch@tkos.co.il>

Not being very fluent in dt and binding yaml I wonder if adding

	allOf:
	  - $ref: pwm.yaml#

would be beneficial?!

> +properties:
> +  compatible:
> +    const: qcom,ipq6018-pwm
> +
> +  reg:
> +    description: Offset of PWM register in the TCSR block.
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  "#pwm-cells":
> +    const: 2

The driver only supports normal polarity. Is this a shortcoming of the
driver, or is the hardware incapable to do that, too?

If it's only the former I'd want #pwm-cells = <3> here. For ease of use
I'd not oppose if you pick #pwm-cells = <3> even if the hardware can
only do normal polarity.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - "#pwm-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
> +
> +    pwm: pwm@a010 {
> +        compatible = "qcom,ipq6018-pwm";
> +        reg = <0xa010 0x20>;
> +        clocks = <&gcc GCC_ADSS_PWM_CLK>;
> +        assigned-clocks = <&gcc GCC_ADSS_PWM_CLK>;
> +        assigned-clock-rates = <100000000>;
> +        #pwm-cells = <2>;
> +    };

Best regards
Uwe

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

Re: [PATCH V15 2/4] dt-bindings: pwm: add IPQ6018 binding

Posted by Rob Herring 2 years, 2 months ago

On Wed, Oct 18, 2023 at 3:46 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> Hello,
>
> On Thu, Oct 05, 2023 at 09:35:48PM +0530, Devi Priya wrote:
> > DT binding for the PWM block in Qualcomm IPQ6018 SoC.
> >
> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
> > Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
> > Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> > ---
> > v15:
> >
> >   No change
> >
> > v14:
> >
> >   Picked up the R-b tag
> >
> > v13:
> >
> >   Updated the file name to match the compatible
> >
> >   Sorted the properties and updated the order in the required field
> >
> >   Dropped the syscon node from examples
> >
> > v12:
> >
> >   Picked up the R-b tag
> >
> > v11:
> >
> >   No change
> >
> > v10:
> >
> >   No change
> >
> > v9:
> >
> >   Add 'ranges' property to example (Rob)
> >
> >   Drop label in example (Rob)
> >
> > v8:
> >
> >   Add size cell to 'reg' (Rob)
> >
> > v7:
> >
> >   Use 'reg' instead of 'offset' (Rob)
> >
> >   Drop 'clock-names' and 'assigned-clock*' (Bjorn)
> >
> >   Use single cell address/size in example node (Bjorn)
> >
> >   Move '#pwm-cells' lower in example node (Bjorn)
> >
> >   List 'reg' as required
> >
> > v6:
> >
> >   Device node is child of TCSR; remove phandle (Rob Herring)
> >
> >   Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)
> >
> > v5: Use qcom,pwm-regs for phandle instead of direct regs (Bjorn
> >     Andersson, Kathiravan T)
> >
> > v4: Update the binding example node as well (Rob Herring's bot)
> >
> > v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
> >
> > v2: Make #pwm-cells const (Rob Herring)
> >
> >  .../bindings/pwm/qcom,ipq6018-pwm.yaml        | 45 +++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml b/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
> > new file mode 100644
> > index 000000000000..6d0d7ed271f7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
> > @@ -0,0 +1,45 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pwm/qcom,ipq6018-pwm.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm IPQ6018 PWM controller
> > +
> > +maintainers:
> > +  - Baruch Siach <baruch@tkos.co.il>
>
> Not being very fluent in dt and binding yaml I wonder if adding
>
>         allOf:
>           - $ref: pwm.yaml#
>
> would be beneficial?!

Not really because the only thing you pick up is #pwm-cells, but
that's still needed here since that varies by binding. A reference
generally becomes useful when there are child nodes (e.g. a bus
binding) or multiple properties.

> > +properties:
> > +  compatible:
> > +    const: qcom,ipq6018-pwm
> > +
> > +  reg:
> > +    description: Offset of PWM register in the TCSR block.
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  "#pwm-cells":
> > +    const: 2
>
> The driver only supports normal polarity. Is this a shortcoming of the
> driver, or is the hardware incapable to do that, too?
>
> If it's only the former I'd want #pwm-cells = <3> here. For ease of use
> I'd not oppose if you pick #pwm-cells = <3> even if the hardware can
> only do normal polarity.

Devi, Can we get an answer here soon.

The MFD part has been applied and it references this schema causing
warnings. So this needs to land or MFD schema reverted.

Rob

Re: [PATCH V15 2/4] dt-bindings: pwm: add IPQ6018 binding

Posted by Uwe Kleine-König 2 years, 2 months ago

Hello,

On Fri, Oct 20, 2023 at 10:14:48AM -0500, Rob Herring wrote:
> On Wed, Oct 18, 2023 at 3:46 PM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > On Thu, Oct 05, 2023 at 09:35:48PM +0530, Devi Priya wrote:
> > > +  "#pwm-cells":
> > > +    const: 2
> >
> > The driver only supports normal polarity. Is this a shortcoming of the
> > driver, or is the hardware incapable to do that, too?
> >
> > If it's only the former I'd want #pwm-cells = <3> here. For ease of use
> > I'd not oppose if you pick #pwm-cells = <3> even if the hardware can
> > only do normal polarity.
> 
> Devi, Can we get an answer here soon.
> 
> The MFD part has been applied and it references this schema causing
> warnings. So this needs to land or MFD schema reverted.

Or the reference to the pwm stuff deleted from the mfd binding?

Best regards
Uwe

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

Re: [PATCH V15 2/4] dt-bindings: pwm: add IPQ6018 binding

Posted by Kathiravan Thirumoorthy 2 years, 2 months ago

On 10/20/2023 8:44 PM, Rob Herring wrote:
> On Wed, Oct 18, 2023 at 3:46 PM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
>> Hello,
>>
>> On Thu, Oct 05, 2023 at 09:35:48PM +0530, Devi Priya wrote:
>>> DT binding for the PWM block in Qualcomm IPQ6018 SoC.
>>>
>>> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
>>> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
>>> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
>>> ---
>>> v15:
>>>
>>>    No change
>>>
>>> v14:
>>>
>>>    Picked up the R-b tag
>>>
>>> v13:
>>>
>>>    Updated the file name to match the compatible
>>>
>>>    Sorted the properties and updated the order in the required field
>>>
>>>    Dropped the syscon node from examples
>>>
>>> v12:
>>>
>>>    Picked up the R-b tag
>>>
>>> v11:
>>>
>>>    No change
>>>
>>> v10:
>>>
>>>    No change
>>>
>>> v9:
>>>
>>>    Add 'ranges' property to example (Rob)
>>>
>>>    Drop label in example (Rob)
>>>
>>> v8:
>>>
>>>    Add size cell to 'reg' (Rob)
>>>
>>> v7:
>>>
>>>    Use 'reg' instead of 'offset' (Rob)
>>>
>>>    Drop 'clock-names' and 'assigned-clock*' (Bjorn)
>>>
>>>    Use single cell address/size in example node (Bjorn)
>>>
>>>    Move '#pwm-cells' lower in example node (Bjorn)
>>>
>>>    List 'reg' as required
>>>
>>> v6:
>>>
>>>    Device node is child of TCSR; remove phandle (Rob Herring)
>>>
>>>    Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)
>>>
>>> v5: Use qcom,pwm-regs for phandle instead of direct regs (Bjorn
>>>      Andersson, Kathiravan T)
>>>
>>> v4: Update the binding example node as well (Rob Herring's bot)
>>>
>>> v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
>>>
>>> v2: Make #pwm-cells const (Rob Herring)
>>>
>>>   .../bindings/pwm/qcom,ipq6018-pwm.yaml        | 45 +++++++++++++++++++
>>>   1 file changed, 45 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml b/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
>>> new file mode 100644
>>> index 000000000000..6d0d7ed271f7
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
>>> @@ -0,0 +1,45 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/pwm/qcom,ipq6018-pwm.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm IPQ6018 PWM controller
>>> +
>>> +maintainers:
>>> +  - Baruch Siach <baruch@tkos.co.il>
>> Not being very fluent in dt and binding yaml I wonder if adding
>>
>>          allOf:
>>            - $ref: pwm.yaml#
>>
>> would be beneficial?!
> Not really because the only thing you pick up is #pwm-cells, but
> that's still needed here since that varies by binding. A reference
> generally becomes useful when there are child nodes (e.g. a bus
> binding) or multiple properties.
>
>>> +properties:
>>> +  compatible:
>>> +    const: qcom,ipq6018-pwm
>>> +
>>> +  reg:
>>> +    description: Offset of PWM register in the TCSR block.
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  "#pwm-cells":
>>> +    const: 2
>> The driver only supports normal polarity. Is this a shortcoming of the
>> driver, or is the hardware incapable to do that, too?
>>
>> If it's only the former I'd want #pwm-cells = <3> here. For ease of use
>> I'd not oppose if you pick #pwm-cells = <3> even if the hardware can
>> only do normal polarity.
> Devi, Can we get an answer here soon.


Rob, Devi is on vacation for couple of weeks. I don't have much idea on 
this as of now, how can I help here, if needed?


>
> The MFD part has been applied and it references this schema causing
> warnings. So this needs to land or MFD schema reverted.
>
> Rob

[PATCH V15 1/4] pwm: driver for qualcomm ipq6018 pwm block
[PATCH V15 2/4] dt-bindings: pwm: add IPQ6018 binding
[PATCH V15 3/4] dt-bindings: mfd: qcom,tcsr: Add simple-mfd support for IPQ6018
[PATCH V15 4/4] arm64: dts: qcom: ipq6018: add pwm node