diff mbox series

[v2,1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

Message ID 20230309165529.223052-2-biju.das.jz@bp.renesas.com (mailing list archive)
State Changes Requested, archived
Headers show
Series Add Versa3 clock generator support | expand

Commit Message

Biju Das March 9, 2023, 4:55 p.m. UTC
Document Renesas versa3 clock generator(5P35023) bindings.

The 5P35023 is a VersaClock programmable clock generator and
is designed for low-power, consumer, and high-performance PCI
Express applications. The 5P35023 device is a three PLL
architecture design, and each PLL is individually programmable
and allowing for up to 6 unique frequency outputs.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
RFC->v2:
 * Renamed the filename to match with compatible
 * Added maintainers entry after title
 * Removed the wrapping for the link to data sheet.
 * Removed reg description
 * Removed clock names
 * Replaced minItems->maxItems in renesas,settings property
 * Dropped assigned-clocks, assigned-clock-rates
 * Dropped renesas,clock-divider-read-only and renesas,clock-flags
 * Drooped clock handle part from example
 * Dropped reg from example.
 * Dropped consumer example
---
 .../bindings/clock/renesas,5p35023.yaml       | 90 +++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,5p35023.yaml

Comments

Krzysztof Kozlowski March 10, 2023, 8:47 a.m. UTC | #1
On 09/03/2023 17:55, Biju Das wrote:
> Document Renesas versa3 clock generator(5P35023) bindings.
> 
> The 5P35023 is a VersaClock programmable clock generator and
> is designed for low-power, consumer, and high-performance PCI
> Express applications. The 5P35023 device is a three PLL
> architecture design, and each PLL is individually programmable
> and allowing for up to 6 unique frequency outputs.
> 

Thank you for your patch. There is something to discuss/improve.

> +description: |
> +  The 5P35023 is a VersaClock programmable clock generator and
> +  is designed for low-power, consumer, and high-performance PCI
> +  express applications. The 5P35023 device is a three PLL
> +  architecture design, and each PLL is individually programmable
> +  and allowing for up to 6 unique frequency outputs.
> +
> +  An internal OTP memory allows the user to store the configuration
> +  in the device. After power up, the user can change the device register
> +  settings through the I2C interface when I2C mode is selected.
> +
> +  The driver can read a full register map from the DT, and will use that
> +  register map to initialize the attached part (via I2C) when the system
> +  boots. Any configuration not supported by the common clock framework
> +  must be done via the full register map, including optimized settings.
> +
> +  Link to datasheet: |

| is not correct here

> +  https://www.renesas.com/us/en/products/clocks-timing/clock-generation/programmable-clocks/5p35023-versaclock-3s-programmable-clock-generator
> +
> +properties:
> +  compatible:
> +    enum:
> +      - renesas,5p35023
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  renesas,settings:
> +    description: Optional, complete register map of the device.
> +      Optimized settings for the device must be provided in full
> +      and are written during initialization.
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    maxItems: 37
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#clock-cells'
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +

Stray blank line, drop.

> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;

With both above:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Biju Das March 10, 2023, 9:02 a.m. UTC | #2
Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH v2 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> On 09/03/2023 17:55, Biju Das wrote:
> > Document Renesas versa3 clock generator(5P35023) bindings.
> >
> > The 5P35023 is a VersaClock programmable clock generator and is
> > designed for low-power, consumer, and high-performance PCI Express
> > applications. The 5P35023 device is a three PLL architecture design,
> > and each PLL is individually programmable and allowing for up to 6
> > unique frequency outputs.
> >
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> > +description: |
> > +  The 5P35023 is a VersaClock programmable clock generator and
> > +  is designed for low-power, consumer, and high-performance PCI
> > +  express applications. The 5P35023 device is a three PLL
> > +  architecture design, and each PLL is individually programmable
> > +  and allowing for up to 6 unique frequency outputs.
> > +
> > +  An internal OTP memory allows the user to store the configuration
> > + in the device. After power up, the user can change the device
> > + register  settings through the I2C interface when I2C mode is selected.
> > +
> > +  The driver can read a full register map from the DT, and will use
> > + that  register map to initialize the attached part (via I2C) when
> > + the system  boots. Any configuration not supported by the common
> > + clock framework  must be done via the full register map, including
> optimized settings.
> > +
> > +  Link to datasheet: |
> 
> | is not correct here

OK will remove.

> 
> > +
> > + https://www.renesas.com/us/en/products/clocks-timing/clock-generatio
> > + n/programmable-clocks/5p35023-versaclock-3s-programmable-clock-gener
> > + ator
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,5p35023
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  '#clock-cells':
> > +    const: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  renesas,settings:
> > +    description: Optional, complete register map of the device.
> > +      Optimized settings for the device must be provided in full
> > +      and are written during initialization.
> > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > +    maxItems: 37
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - '#clock-cells'
> > +  - clocks
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +
> 
> Stray blank line, drop.

Agreed. Will send next version based on feedback for the driver patch and reviews
from others.

Cheers,
Biju
> 
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> 
> With both above:
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Best regards,
> Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
new file mode 100644
index 000000000000..ee4afc2ea67b
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
@@ -0,0 +1,90 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/renesas,5p35023.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas 5p35023 VersaClock 3 programmable I2C clock generator
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  The 5P35023 is a VersaClock programmable clock generator and
+  is designed for low-power, consumer, and high-performance PCI
+  express applications. The 5P35023 device is a three PLL
+  architecture design, and each PLL is individually programmable
+  and allowing for up to 6 unique frequency outputs.
+
+  An internal OTP memory allows the user to store the configuration
+  in the device. After power up, the user can change the device register
+  settings through the I2C interface when I2C mode is selected.
+
+  The driver can read a full register map from the DT, and will use that
+  register map to initialize the attached part (via I2C) when the system
+  boots. Any configuration not supported by the common clock framework
+  must be done via the full register map, including optimized settings.
+
+  Link to datasheet: |
+  https://www.renesas.com/us/en/products/clocks-timing/clock-generation/programmable-clocks/5p35023-versaclock-3s-programmable-clock-generator
+
+properties:
+  compatible:
+    enum:
+      - renesas,5p35023
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+  clocks:
+    maxItems: 1
+
+  renesas,settings:
+    description: Optional, complete register map of the device.
+      Optimized settings for the device must be provided in full
+      and are written during initialization.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    maxItems: 37
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        versa3: clock-generator@68 {
+            compatible = "renesas,5p35023";
+            reg = <0x68>;
+            #clock-cells = <1>;
+
+            clocks = <&x1_x2>;
+
+            renesas,settings = [
+                80 00 11 19 4c 02 23 7f 83 19 08 a9 5f 25 24 bf
+                00 14 7a e1 00 00 00 00 01 55 59 bb 3f 30 90 b6
+                80 b0 45 c4 95
+            ];
+
+            assigned-clocks = <&versa3 0>,
+                              <&versa3 1>,
+                              <&versa3 2>,
+                              <&versa3 3>,
+                              <&versa3 4>,
+                              <&versa3 5>;
+            assigned-clock-rates = <12288000>, <25000000>,
+                                   <12000000>, <11289600>,
+                                   <11289600>, <24000000>;
+        };
+    };