On 20/02/2023 14:13, 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. > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > --- > .../bindings/clock/renesas,versaclock3.yaml | 135 ++++++++++++++++++ > 1 file changed, 135 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml > > diff --git a/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml b/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml > new file mode 100644 > index 000000000000..f45b8da73ec3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml Filename usually is based on the compatible. Why these two are so different? > @@ -0,0 +1,135 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/renesas,versaclock3.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Renesas VersaClock 3 programmable I2C clock generators > + > +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 I think link should not be wrapped. Start in next line and just make it long. While touching this, please keep the same order of entries as in example-schema, so maintainers go after title. > + > +maintainers: > + - Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > + > +properties: > + compatible: > + enum: > + - renesas,5p35023 > + > + reg: > + description: I2C device address Drop description, it's obvious. > + maxItems: 1 > + > + '#clock-cells': > + const: 1 > + > + clock-names: > + oneOf: > + - items: > + - const: x1 > + - items: > + - const: clkin This should be specific, not one or another. Why do you have two entirely different clock inputs? (and if this stays, then just items: - enum: []) > + > + 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 > + minItems: 37 maxItems instead... but I am not sure that we allow register settings in DT in general. > + > + assigned-clocks: > + minItems: 6 Drop. > + > + assigned-clock-rates: > + minItems: 6 Drop. > + > + renesas,clock-divider-read-only: > + description: Flag for setting divider in read only mode. Flag? Then type: boolean. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 5 This is broken... > + > + renesas,clock-flags: > + description: Flags used in common clock frame work for configuring > + clk outputs. See include/linux/clk-provider.h These are not bindings, so why do you non-bindings constants as bindings? They can change anytime. Choose one: 1. Drop entire property, 2. Make it a proper binding property, so an ABI and explain why this is DT specific. None of clock providers have to do it, so you need here good explanation. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 6 maxItems instead > + > +required: > + - compatible > + - reg > + - '#clock-cells' > + - clocks > + - clock-names > + > +additionalProperties: false > + > +examples: > + - | > + /* 24MHz crystal */ > + x1_x2: xtal { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <24000000>; > + }; Drop this part, obvious. > + > + i2c@0 { > + reg = <0x0 0x100>; Just i2c { and drop reg > + #address-cells = <1>; > + #size-cells = <0>; > + > + versa3: clock-generator@68 { > + compatible = "renesas,5p35023"; > + reg = <0x68>; > + #clock-cells = <1>; > + > + clocks = <&x1_x2>; > + clock-names = "x1"; > + > + 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>; > + renesas,clock-divider-read-only = <1>, <1>, <1>, <1>, <1>; > + renesas,clock-flags = <2176>, <2176>, <2176>, <2052>, > + <2176>, <2048>; > + }; > + }; > + > + /* Consumer referencing the versa 3 */ > + consumer { > + /* ... */ > + clocks = <&versa3 3>; > + /* ... */ Drop consumer. Do you see it anywhere else? Best regards, Krzysztof
