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

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

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

Devi Priya Sept. 29, 2023, 7:55 a.m. UTC | #1
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 | #2
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 | #3
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 | #4
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
>
  

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>;
+        };
+    };