diff mbox series

[V12,2/3] dt-bindings: pwm: add IPQ6018 binding

Message ID 20230925065915.3467964-3-quic_devipriy@quicinc.com
State Changes Requested
Headers show
Series Add PWM support for IPQ chipsets | expand

Commit Message

Devi Priya Sept. 25, 2023, 6:59 a.m. UTC
DT binding for the PWM block in Qualcomm IPQ6018 SoC.

Reviewed-by: Bjorn Andersson <bjorn.andersson@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>
---
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)

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

Comments

Krzysztof Kozlowski Sept. 25, 2023, 7:11 a.m. UTC | #1
On 25/09/2023 08:59, Devi Priya wrote:
> DT binding for the PWM block in Qualcomm IPQ6018 SoC.
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@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>

...

> diff --git a/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
> new file mode 100644
> index 000000000000..857086ad539e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml

Filename matching compatible, so qcom,ipq6018-pwm.yaml

> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/ipq-pwm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm IPQ6018 PWM controller
> +
> +maintainers:
> +  - Baruch Siach <baruch@tkos.co.il>
> +
> +properties:
> +  "#pwm-cells":
> +    const: 2
> +
> +  compatible:
> +    const: qcom,ipq6018-pwm

compatible is always the first property.

> +
> +  reg:
> +    description: Offset of PWM register in the TCSR block.
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - "#pwm-cells"

And this order must be the same as in properties.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
> +
> +    syscon@1937000 {
> +        compatible = "qcom,tcsr-ipq6018", "syscon", "simple-mfd";
> +        reg = <0x01937000 0x21000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges = <0 0x1937000 0x21000>;

Drop this node, not related. The parent binding could have full example,
on the other hand. Additionally, I have doubts that you really tested
the parent binding.

> +
> +        pwm: pwm@a010 {
> +            compatible = "qcom,ipq6018-pwm";

Best regards,
Krzysztof
Rob Herring (Arm) Sept. 25, 2023, 7:26 a.m. UTC | #2
On Mon, 25 Sep 2023 12:29:14 +0530, Devi Priya wrote:
> DT binding for the PWM block in Qualcomm IPQ6018 SoC.
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@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>
> ---
> 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)
> 
>  .../devicetree/bindings/pwm/ipq-pwm.yaml      | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pwm/ipq-pwm.example.dtb: syscon@1937000: compatible: ['qcom,tcsr-ipq6018', 'syscon', 'simple-mfd'] is too long
	from schema $id: http://devicetree.org/schemas/mfd/qcom,tcsr.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pwm/ipq-pwm.example.dtb: syscon@1937000: '#address-cells', '#size-cells', 'pwm@a010', 'ranges' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/mfd/qcom,tcsr.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230925065915.3467964-3-quic_devipriy@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Devi Priya Sept. 29, 2023, 7:55 a.m. UTC | #3
On 9/25/2023 12:41 PM, Krzysztof Kozlowski wrote:
> On 25/09/2023 08:59, Devi Priya wrote:
>> DT binding for the PWM block in Qualcomm IPQ6018 SoC.
>>
>> Reviewed-by: Bjorn Andersson <bjorn.andersson@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>
> 
> ...
> 
>> diff --git a/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
>> new file mode 100644
>> index 000000000000..857086ad539e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
> 
> Filename matching compatible, so qcom,ipq6018-pwm.yaml
okay
> 
>> @@ -0,0 +1,53 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pwm/ipq-pwm.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm IPQ6018 PWM controller
>> +
>> +maintainers:
>> +  - Baruch Siach <baruch@tkos.co.il>
>> +
>> +properties:
>> +  "#pwm-cells":
>> +    const: 2
>> +
>> +  compatible:
>> +    const: qcom,ipq6018-pwm
> 
> compatible is always the first property.
okay
> 
>> +
>> +  reg:
>> +    description: Offset of PWM register in the TCSR block.
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - "#pwm-cells"
> 
> And this order must be the same as in properties.
okay
> 
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
>> +
>> +    syscon@1937000 {
>> +        compatible = "qcom,tcsr-ipq6018", "syscon", "simple-mfd";
>> +        reg = <0x01937000 0x21000>;
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +        ranges = <0 0x1937000 0x21000>;
> 
> Drop this node, not related. The parent binding could have full example,
> on the other hand. Additionally, I have doubts that you really tested
> the parent binding.
Sure, will drop the syscon node

Thanks,
Devi Priya
> 
>> +
>> +        pwm: pwm@a010 {
>> +            compatible = "qcom,ipq6018-pwm";
> 
> Best regards,
> Krzysztof
>
Devi Priya Sept. 29, 2023, 8:56 a.m. UTC | #4
On 9/29/2023 1:25 PM, Devi Priya wrote:
> 
> 
> On 9/25/2023 12:41 PM, Krzysztof Kozlowski wrote:
>> On 25/09/2023 08:59, Devi Priya wrote:
>>> DT binding for the PWM block in Qualcomm IPQ6018 SoC.
>>>
>>> Reviewed-by: Bjorn Andersson <bjorn.andersson@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>
>>
>> ...
>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml 
>>> b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
>>> new file mode 100644
>>> index 000000000000..857086ad539e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
>>
>> Filename matching compatible, so qcom,ipq6018-pwm.yaml
> okay
We would have other ipq compatibles (ipq9574 & ipq5332) being added to
the binding in the upcoming series.
So, shall we rename the binding to qcom,ipq-pwm.yaml

Thanks,
Devi Priya
>>
>>> @@ -0,0 +1,53 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/pwm/ipq-pwm.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm IPQ6018 PWM controller
>>> +
>>> +maintainers:
>>> +  - Baruch Siach <baruch@tkos.co.il>
>>> +
>>> +properties:
>>> +  "#pwm-cells":
>>> +    const: 2
>>> +
>>> +  compatible:
>>> +    const: qcom,ipq6018-pwm
>>
>> compatible is always the first property.
> okay
>>
>>> +
>>> +  reg:
>>> +    description: Offset of PWM register in the TCSR block.
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - clocks
>>> +  - "#pwm-cells"
>>
>> And this order must be the same as in properties.
> okay
>>
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
>>> +
>>> +    syscon@1937000 {
>>> +        compatible = "qcom,tcsr-ipq6018", "syscon", "simple-mfd";
>>> +        reg = <0x01937000 0x21000>;
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +        ranges = <0 0x1937000 0x21000>;
>>
>> Drop this node, not related. The parent binding could have full example,
>> on the other hand. Additionally, I have doubts that you really tested
>> the parent binding.
> Sure, will drop the syscon node
> 
> Thanks,
> Devi Priya
>>
>>> +
>>> +        pwm: pwm@a010 {
>>> +            compatible = "qcom,ipq6018-pwm";
>>
>> Best regards,
>> Krzysztof
>>
Krzysztof Kozlowski Sept. 30, 2023, 3:32 p.m. UTC | #5
On 29/09/2023 10:56, Devi Priya wrote:

>>>> diff --git a/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml 
>>>> b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
>>>> new file mode 100644
>>>> index 000000000000..857086ad539e
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
>>>
>>> Filename matching compatible, so qcom,ipq6018-pwm.yaml
>> okay
> We would have other ipq compatibles (ipq9574 & ipq5332) being added to
> the binding in the upcoming series.
> So, shall we rename the binding to qcom,ipq-pwm.yaml

I prefer not.

Best regards,
Krzysztof
Devi Priya Sept. 30, 2023, 5:12 p.m. UTC | #6
On 9/30/2023 9:02 PM, Krzysztof Kozlowski wrote:
> On 29/09/2023 10:56, Devi Priya wrote:
> 
>>>>> diff --git a/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
>>>>> b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..857086ad539e
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
>>>>
>>>> Filename matching compatible, so qcom,ipq6018-pwm.yaml
>>> okay
>> We would have other ipq compatibles (ipq9574 & ipq5332) being added to
>> the binding in the upcoming series.
>> So, shall we rename the binding to qcom,ipq-pwm.yaml
> 
> I prefer not.
> 
Sure, okay

Thanks,
Devi Priya
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
new file mode 100644
index 000000000000..857086ad539e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/ipq-pwm.yaml
@@ -0,0 +1,53 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/ipq-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm IPQ6018 PWM controller
+
+maintainers:
+  - Baruch Siach <baruch@tkos.co.il>
+
+properties:
+  "#pwm-cells":
+    const: 2
+
+  compatible:
+    const: qcom,ipq6018-pwm
+
+  reg:
+    description: Offset of PWM register in the TCSR block.
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - "#pwm-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
+
+    syscon@1937000 {
+        compatible = "qcom,tcsr-ipq6018", "syscon", "simple-mfd";
+        reg = <0x01937000 0x21000>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges = <0 0x1937000 0x21000>;
+
+        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>;
+        };
+    };