diff mbox series

[1/2] dtbindings: clock: Add bindings for Renesas PhiClock

Message ID 20221115192625.9410-2-alexander.helms.jy@renesas.com
State New
Headers show
Series [1/2] dtbindings: clock: Add bindings for Renesas PhiClock | expand

Commit Message

Alex Helms Nov. 15, 2022, 7:26 p.m. UTC
Add dt bindings for the Renesas PhiClock clock generator.

Signed-off-by: Alex Helms <alexander.helms.jy@renesas.com>
---
 .../bindings/clock/renesas,phiclock.yaml      | 81 +++++++++++++++++++
 MAINTAINERS                                   |  5 ++
 2 files changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,phiclock.yaml

Comments

Alex Helms Nov. 16, 2022, 8:11 p.m. UTC | #1
On 11/16/2022 1:20 AM, Krzysztof Kozlowski wrote:
> On 15/11/2022 20:26, Alex Helms wrote:
>> Add dt bindings for the Renesas PhiClock clock generator.
>>
> 
> Subject: drop second, redundant "bindings"
> 
>> Signed-off-by: Alex Helms <alexander.helms.jy@renesas.com>
>> ---
>>  .../bindings/clock/renesas,phiclock.yaml      | 81 +++++++++++++++++++
>>  MAINTAINERS                                   |  5 ++
>>  2 files changed, 86 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,phiclock.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/clock/renesas,phiclock.yaml b/Documentation/devicetree/bindings/clock/renesas,phiclock.yaml
>> new file mode 100644
>> index 000000000..2b36534d3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/renesas,phiclock.yaml
> 
> Filename based on compatible.
> 

As Geert mentioned in the other thread, this is a family of products but
the others cannot be added now.

>> @@ -0,0 +1,81 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fclock%2Frenesas%2Cphiclock.yaml%23&amp;data=05%7C01%7Calexander.helms.jy%40renesas.com%7C9c13a32848f3434e217108dac7ab69f6%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638041836281252737%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=J6kNqua%2FJf0c8HczRM8gU8%2Fm%2BhX6gSF2fqnf2n3wSbI%3D&amp;reserved=0
>> +$schema: https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Calexander.helms.jy%40renesas.com%7C9c13a32848f3434e217108dac7ab69f6%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638041836281252737%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ofZbC2sBnTpJR3KKzPVqhsFy28r4JjbJpaVGSuHKx38%3D&amp;reserved=0
>> +
>> +title: Renesas PhiClock Clock Generator Device Tree Bindings
> 
> Drop "Device Tree Bindings"
> 
>> +
>> +maintainers:
>> +  - Alex Helms <alexander.helms.jy@renesas.com>
>> +
>> +description: |
>> +  The Renesas PhiClock is a programmable I2C clock generator that provides
>> +  1 reference output and 2 clock outputs.
>> +
>> +  The driver supports spread spectrum but only if all configurations use the
> 
> Driver as in Linux driver? Drop entire paragraph. Bindings are about
> hardware, not driver.
> 
>> +  same spread spectrum parameters. If your configuration uses spread spectrum,
>> +  you must include renesas,ss-amount-percent, renesas,ss-modulation-hz, and
>> +  renesas,ss-direction in the device tree.
>> +
>> +properties:
> 
> compatible goes always first. Start your schema from example-schema.yaml.
> 
>> +  '#clock-cells':
>> +    const: 1
>> +
>> +  clock-names:
>> +    items:
>> +      - const: xin-clkin
> 
> Just "xin" or entirely drop.

The pin name on the datasheet is "xin-clkin" and as the name implies it
can be a crystal or clock input. If the name were different it could be
confusing.

> 
>> +
>> +  clocks:
>> +    const: 1
>> +
>> +  compatible:
>> +    enum:
>> +      - renesas,9fgv1006
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  renesas,ss-amount-percent:
>> +    description: Spread spectrum absolute amount as hundredths of a percent, e.g. 150 is 1.50%.
> 
> What? If this is percent then it cannot be hundreds of percent. Percent
> is percent. Use appropriate units.
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdevicetree-org%2Fdt-schema%2Fblob%2Fmain%2Fdtschema%2Fschemas%2Fproperty-units.yaml&amp;data=05%7C01%7Calexander.helms.jy%40renesas.com%7C9c13a32848f3434e217108dac7ab69f6%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638041836281252737%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=6MULpJhPyyjWSo1SvPCrz6KidE1VEtiiNYk1O5wS1vI%3D&amp;reserved=0
> 

Values like 0.5% or 2.5% must be representable which is why this
property is an integer of hundredths of percent. How else would you
represent a non-integer percent?

>> +    minimum: 0
>> +    maximum: 500
>> +
>> +  renesas,ss-modulation-hz:
>> +    description: Spread spectrum modulation rate in Hz
>> +    minimum: 30000
>> +    maximum: 63000
>> +
>> +  renesas,ss-direction:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description: Spread spectrum direction
>> +    enum: [ down, center ]
>> +
>> +required:
>> +  - clock-names
>> +  - '#clock-cells'
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    ref25: ref25m {
>> +      compatible = "fixed-clock";
>> +      #clock-cells = <0>;
>> +      clock-frequency = <25000000>;
>> +    };
> 
> Drop, it's obvious, isn't it?
> 

I disagree, this may be obvious to someone familiar with how clocks in
the device tree works but not long ago it was entirely new to me and
examples like these in the dt schemas were very helpful in getting the
device up and running. There are several other bindings that define
external crystals and reference clocks in this way.

>> +
> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/renesas,phiclock.yaml b/Documentation/devicetree/bindings/clock/renesas,phiclock.yaml
new file mode 100644
index 000000000..2b36534d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,phiclock.yaml
@@ -0,0 +1,81 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/renesas,phiclock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas PhiClock Clock Generator Device Tree Bindings
+
+maintainers:
+  - Alex Helms <alexander.helms.jy@renesas.com>
+
+description: |
+  The Renesas PhiClock is a programmable I2C clock generator that provides
+  1 reference output and 2 clock outputs.
+
+  The driver supports spread spectrum but only if all configurations use the
+  same spread spectrum parameters. If your configuration uses spread spectrum,
+  you must include renesas,ss-amount-percent, renesas,ss-modulation-hz, and
+  renesas,ss-direction in the device tree.
+
+properties:
+  '#clock-cells':
+    const: 1
+
+  clock-names:
+    items:
+      - const: xin-clkin
+
+  clocks:
+    const: 1
+
+  compatible:
+    enum:
+      - renesas,9fgv1006
+
+  reg:
+    maxItems: 1
+
+  renesas,ss-amount-percent:
+    description: Spread spectrum absolute amount as hundredths of a percent, e.g. 150 is 1.50%.
+    minimum: 0
+    maximum: 500
+
+  renesas,ss-modulation-hz:
+    description: Spread spectrum modulation rate in Hz
+    minimum: 30000
+    maximum: 63000
+
+  renesas,ss-direction:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: Spread spectrum direction
+    enum: [ down, center ]
+
+required:
+  - clock-names
+  - '#clock-cells'
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    ref25: ref25m {
+      compatible = "fixed-clock";
+      #clock-cells = <0>;
+      clock-frequency = <25000000>;
+    };
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      phiclock: clock-controller@68 {
+        compatible = "renesas,9fgv1006";
+        reg = <0x68>;
+        #clock-cells = <1>;
+        clocks = <&ref25>;
+        clock-names = "xin-clkin";
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 256f03904..7eabe930b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17641,6 +17641,11 @@  S:	Maintained
 F:	Documentation/devicetree/bindings/clock/renesas,versaclock7.yaml
 F:	drivers/clk/clk-versaclock7.c
 
+RENESAS PHICLOCK CLOCK DRIVER
+M:	Alex Helms <alexander.helms.jy@renesas.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/clock/renesas,phiclock.yaml
+
 RESET CONTROLLER FRAMEWORK
 M:	Philipp Zabel <p.zabel@pengutronix.de>
 S:	Maintained