Add Microchip ZL3073x support

[PATCH 16/28] dt-bindings: dpll: Add support for Microchip Azurite chip family

Posted by Ivan Vecera 10 months, 1 week ago

This adds DT bindings schema for Microchip Azurite DPLL chip family.
These bindings are used by zl3073x driver and specifies this device
that can be connected either to I2C or SPI bus.

The schema inherits existing dpll-device and dpll-pin schemas.

Reviewed-by: Michal Schmidt <mschmidt@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 .../bindings/dpll/microchip,zl3073x.yaml      | 74 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml

diff --git a/Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml b/Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml
new file mode 100644
index 0000000000000..38a6cc00bc026
--- /dev/null
+++ b/Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dpll/microchip,zl3073x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip Azurite DPLL device
+
+maintainers:
+  - Ivan Vecera <ivecera@redhat.com>
+
+properties:
+  compatible:
+    enum:
+      - microchip,zl3073x-i2c
+      - microchip,zl3073x-spi
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/dpll/dpll-device.yaml
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      dpll@70 {
+        compatible = "microchip,zl3073x-i2c";
+        #address-cells = <0>;
+        #size-cells = <0>;
+        reg = <0x70>;
+        status = "okay";
+
+        num-dplls = <2>;
+        dpll-types = "pps", "eec";
+
+        input-pins {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          pin@0 { /* REF0P */
+            reg = <0>;
+
+            label = "Input 0";
+            type = "ext";
+            supported-frequencies = /bits/ 64 <1 1000>;
+          };
+        };
+
+        output-pins {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          pin@3 { /* OUT1N */
+            reg = <3>;
+
+            label = "Output 1";
+            type = "gnss";
+            esync-control;
+            supported-frequencies = /bits/ 64 <1 10000>;
+          };
+        };
+      };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index eaf2576a9b746..ec86bec05c40c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16001,6 +16001,7 @@ M:	Ivan Vecera <ivecera@redhat.com>
 M:	Prathosh Satish <Prathosh.Satish@microchip.com>
 L:	netdev@vger.kernel.org
 S:	Supported
+F:	Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml
 F:	drivers/dpll/dpll_zl3073x*
 F:	drivers/mfd/zl3073x*
 F:	include/linux/mfd/zl3073x.h
-- 
2.48.1

Re: [PATCH 16/28] dt-bindings: dpll: Add support for Microchip Azurite chip family

Posted by Krzysztof Kozlowski 10 months, 1 week ago

On 07/04/2025 19:31, Ivan Vecera wrote:
> This adds DT bindings schema for Microchip Azurite DPLL chip family.

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> These bindings are used by zl3073x driver and specifies this device
> that can be connected either to I2C or SPI bus.

Bindings are for hardware, not driver. Explain the hardware.

> 
> The schema inherits existing dpll-device and dpll-pin schemas.
> 

Do not explain what schema does - we see  it. Explain the hardware which
we do not see here, because we (or to be precise: I) know nothing about.

> Reviewed-by: Michal Schmidt <mschmidt@redhat.com>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>  .../bindings/dpll/microchip,zl3073x.yaml      | 74 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml b/Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml
> new file mode 100644
> index 0000000000000..38a6cc00bc026
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml
> @@ -0,0 +1,74 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dpll/microchip,zl3073x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip Azurite DPLL device
> +
> +maintainers:
> +  - Ivan Vecera <ivecera@redhat.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,zl3073x-i2c
> +      - microchip,zl3073x-spi

1. No, you do not get two compatibles. Only one.
2. What is 'x'? Wildcard? If so, drop and use specific compatibles.


> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: /schemas/dpll/dpll-device.yaml
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      dpll@70 {
> +        compatible = "microchip,zl3073x-i2c";

> +        #address-cells = <0>;
> +        #size-cells = <0>;

Again, why do you need them if you are not using these two?

> +        reg = <0x70>;
> +        status = "okay";

Drop

Also, follow DTS coding style and order properties properly.

> +
> +        num-dplls = <2>;
> +        dpll-types = "pps", "eec";
> +




Best regards,
Krzysztof

Re: [PATCH 16/28] dt-bindings: dpll: Add support for Microchip Azurite chip family

Posted by Ivan Vecera 10 months ago


On 07. 04. 25 8:04 odp., Krzysztof Kozlowski wrote:
> On 07/04/2025 19:31, Ivan Vecera wrote:
>> This adds DT bindings schema for Microchip Azurite DPLL chip family.
> 
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> 
>> These bindings are used by zl3073x driver and specifies this device
>> that can be connected either to I2C or SPI bus.
> 
> Bindings are for hardware, not driver. Explain the hardware.

OK

>>
>> The schema inherits existing dpll-device and dpll-pin schemas.
>>
> 
> Do not explain what schema does - we see  it. Explain the hardware which
> we do not see here, because we (or to be precise: I) know nothing about.

OK

>> Reviewed-by: Michal Schmidt <mschmidt@redhat.com>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> ---
>>   .../bindings/dpll/microchip,zl3073x.yaml      | 74 +++++++++++++++++++
>>   MAINTAINERS                                   |  1 +
>>   2 files changed, 75 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml b/Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml
>> new file mode 100644
>> index 0000000000000..38a6cc00bc026
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dpll/microchip,zl3073x.yaml
>> @@ -0,0 +1,74 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/dpll/microchip,zl3073x.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Microchip Azurite DPLL device
>> +
>> +maintainers:
>> +  - Ivan Vecera <ivecera@redhat.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - microchip,zl3073x-i2c
>> +      - microchip,zl3073x-spi
> 
> 1. No, you do not get two compatibles. Only one.

Will split to two files, one for i2c and one for spi.

> 2. What is 'x'? Wildcard? If so, drop and use specific compatibles.

Microchip refers to the ZL3073x as a family of compatible DPLL chips 
with the same features. There is no need to introduce separate 
compatible string for each of them.

>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +allOf:
>> +  - $ref: /schemas/dpll/dpll-device.yaml
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    i2c {
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +
>> +      dpll@70 {
>> +        compatible = "microchip,zl3073x-i2c";
> 
>> +        #address-cells = <0>;
>> +        #size-cells = <0>;
> 
> Again, why do you need them if you are not using these two?

The dpll-device.yaml defines them as required. Shouldn't they be 
specified explicitly?

>> +        reg = <0x70>;
>> +        status = "okay";
> 
> Drop

OK

> Also, follow DTS coding style and order properties properly.
> 
>> +
>> +        num-dplls = <2>;
>> +        dpll-types = "pps", "eec";
>> +

Ack
> Best regards,
> Krzysztof
>

Re: [PATCH 16/28] dt-bindings: dpll: Add support for Microchip Azurite chip family

Posted by Krzysztof Kozlowski 10 months ago

On 09/04/2025 09:19, Ivan Vecera wrote:
>>> +
>>> +maintainers:
>>> +  - Ivan Vecera <ivecera@redhat.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - microchip,zl3073x-i2c
>>> +      - microchip,zl3073x-spi
>>
>> 1. No, you do not get two compatibles. Only one.
> 
> Will split to two files, one for i2c and one for spi.

No. One device, one compatible.

> 
>> 2. What is 'x'? Wildcard? If so, drop and use specific compatibles.
> 
> Microchip refers to the ZL3073x as a family of compatible DPLL chips 
> with the same features. There is no need to introduce separate 
> compatible string for each of them.

So a wildcard, thus drop. Use full product names. Google search gives me
no products for ZL3073x but gives me ZL30735.

> 
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +allOf:
>>> +  - $ref: /schemas/dpll/dpll-device.yaml
>>> +
>>> +unevaluatedProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    i2c {
>>> +      #address-cells = <1>;
>>> +      #size-cells = <0>;
>>> +
>>> +      dpll@70 {
>>> +        compatible = "microchip,zl3073x-i2c";
>>
>>> +        #address-cells = <0>;
>>> +        #size-cells = <0>;
>>
>> Again, why do you need them if you are not using these two?
> 
> The dpll-device.yaml defines them as required. Shouldn't they be 
> specified explicitly?

But you do not use them. Where is any child node?

> 
>>> +        reg = <0x70>;
>>> +        status = "okay";
>>
>> Drop
Best regards,
Krzysztof

Re: [PATCH 16/28] dt-bindings: dpll: Add support for Microchip Azurite chip family

Posted by Ivan Vecera 10 months ago


On 10. 04. 25 9:01 dop., Krzysztof Kozlowski wrote:
> On 09/04/2025 09:19, Ivan Vecera wrote:
>>>> +
>>>> +maintainers:
>>>> +  - Ivan Vecera <ivecera@redhat.com>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - microchip,zl3073x-i2c
>>>> +      - microchip,zl3073x-spi
>>>
>>> 1. No, you do not get two compatibles. Only one.
>>
>> Will split to two files, one for i2c and one for spi.
> 
> No. One device, one compatible.

OK, get it now. I thought that I need to have separate compatible for 
each bus access type.

>>> 2. What is 'x'? Wildcard? If so, drop and use specific compatibles.
>>
>> Microchip refers to the ZL3073x as a family of compatible DPLL chips
>> with the same features. There is no need to introduce separate
>> compatible string for each of them.
> 
> So a wildcard, thus drop. Use full product names. Google search gives me
> no products for ZL3073x but gives me ZL30735.

I will use more appropriate microchip,azurite compatible.

>>
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +
>>>> +allOf:
>>>> +  - $ref: /schemas/dpll/dpll-device.yaml
>>>> +
>>>> +unevaluatedProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    i2c {
>>>> +      #address-cells = <1>;
>>>> +      #size-cells = <0>;
>>>> +
>>>> +      dpll@70 {
>>>> +        compatible = "microchip,zl3073x-i2c";
>>>
>>>> +        #address-cells = <0>;
>>>> +        #size-cells = <0>;
>>>
>>> Again, why do you need them if you are not using these two?
>>
>> The dpll-device.yaml defines them as required. Shouldn't they be
>> specified explicitly?
> 
> But you do not use them. Where is any child node?

I though I have to specify this due to existence of 'input-pins' and 
'output-pins' in the example.

>>
>>>> +        reg = <0x70>;
>>>> +        status = "okay";
>>>
>>> Drop
> Best regards,
> Krzysztof
> 

Thanks,
Ivan

Re: [PATCH 16/28] dt-bindings: dpll: Add support for Microchip Azurite chip family

Posted by Krzysztof Kozlowski 10 months ago

On 10/04/2025 12:28, Ivan Vecera wrote:
> 
>>>> 2. What is 'x'? Wildcard? If so, drop and use specific compatibles.
>>>
>>> Microchip refers to the ZL3073x as a family of compatible DPLL chips
>>> with the same features. There is no need to introduce separate
>>> compatible string for each of them.
>>
>> So a wildcard, thus drop. Use full product names. Google search gives me
>> no products for ZL3073x but gives me ZL30735.
> 
> I will use more appropriate microchip,azurite compatible.

Hm? What/who gave such hint? Please read writing bindings or any other
guide/speech about it. If that's a zl30735 then use "zl30735" as device
part. If you have more devices, use fallbacks. See writing bindings.

> 
>>>
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +required:
>>>>> +  - compatible
>>>>> +  - reg
>>>>> +
>>>>> +allOf:
>>>>> +  - $ref: /schemas/dpll/dpll-device.yaml
>>>>> +
>>>>> +unevaluatedProperties: false
>>>>> +
>>>>> +examples:
>>>>> +  - |
>>>>> +    i2c {
>>>>> +      #address-cells = <1>;
>>>>> +      #size-cells = <0>;
>>>>> +
>>>>> +      dpll@70 {
>>>>> +        compatible = "microchip,zl3073x-i2c";
>>>>
>>>>> +        #address-cells = <0>;
>>>>> +        #size-cells = <0>;
>>>>
>>>> Again, why do you need them if you are not using these two?
>>>
>>> The dpll-device.yaml defines them as required. Shouldn't they be
>>> specified explicitly?
>>
>> But you do not use them. Where is any child node?
> 
> I though I have to specify this due to existence of 'input-pins' and 
> 'output-pins' in the example.

They do not have addressing, so no need for cells.

Best regards,
Krzysztof

Re: [PATCH 16/28] dt-bindings: dpll: Add support for Microchip Azurite chip family

Posted by Ivan Vecera 10 months ago


On 10. 04. 25 2:19 odp., Krzysztof Kozlowski wrote:
> On 10/04/2025 12:28, Ivan Vecera wrote:
>>
>>>>> 2. What is 'x'? Wildcard? If so, drop and use specific compatibles.
>>>>
>>>> Microchip refers to the ZL3073x as a family of compatible DPLL chips
>>>> with the same features. There is no need to introduce separate
>>>> compatible string for each of them.
>>>
>>> So a wildcard, thus drop. Use full product names. Google search gives me
>>> no products for ZL3073x but gives me ZL30735.
>>
>> I will use more appropriate microchip,azurite compatible.
> 
> Hm? What/who gave such hint? Please read writing bindings or any other
> guide/speech about it. If that's a zl30735 then use "zl30735" as device
> part. If you have more devices, use fallbacks. See writing bindings.
> 
Something like this:

1)
properties:
   compatible:
     enum:
       - microchip,zl30731
       - microchip,zl30732
       - microchip,zl30732
       - microchip,zl80032
       - microchip,zl80732

or

2)
properties:
   compatible:
     items:
       - enum:
           - microchip,zl30731
           - microchip,zl30732
           - microchip,zl30732
           - microchip,zl80032
           - microchip,zl80732
       - const: microchip,azurite

If 1) what should be the filename ?

Thanks for patience.

Ivan

>>
>>>>
>>>>>> +
>>>>>> +  reg:
>>>>>> +    maxItems: 1
>>>>>> +
>>>>>> +required:
>>>>>> +  - compatible
>>>>>> +  - reg
>>>>>> +
>>>>>> +allOf:
>>>>>> +  - $ref: /schemas/dpll/dpll-device.yaml
>>>>>> +
>>>>>> +unevaluatedProperties: false
>>>>>> +
>>>>>> +examples:
>>>>>> +  - |
>>>>>> +    i2c {
>>>>>> +      #address-cells = <1>;
>>>>>> +      #size-cells = <0>;
>>>>>> +
>>>>>> +      dpll@70 {
>>>>>> +        compatible = "microchip,zl3073x-i2c";
>>>>>
>>>>>> +        #address-cells = <0>;
>>>>>> +        #size-cells = <0>;
>>>>>
>>>>> Again, why do you need them if you are not using these two?
>>>>
>>>> The dpll-device.yaml defines them as required. Shouldn't they be
>>>> specified explicitly?
>>>
>>> But you do not use them. Where is any child node?
>>
>> I though I have to specify this due to existence of 'input-pins' and
>> 'output-pins' in the example.
> 
> They do not have addressing, so no need for cells.

Thanks for explanation.

> 
> Best regards,
> Krzysztof
>