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

  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
  

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.

> @@ -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

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.

> +
> +  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://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

> +    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?

> +

Best regards,
Krzysztof
  

Hi Krzysztof,

On Wed, Nov 16, 2022 at 9:20 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> 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.

Looks like there are more of them, so I guess the family name is fine?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
  

On 16/11/2022 09:34, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Wed, Nov 16, 2022 at 9:20 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> 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.
> 
> Looks like there are more of them, so I guess the family name is fine?
>

If you are sure, then yes. Can we add them now? If not, it's also fine.

Best regards,
Krzysztof
  

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
>
  

On 16/11/2022 21:11, Alex Helms wrote:
>>> +  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&data=05%7C01%7Calexander.helms.jy%40renesas.com%7C9c13a32848f3434e217108dac7ab69f6%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638041836281252737%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6MULpJhPyyjWSo1SvPCrz6KidE1VEtiiNYk1O5wS1vI%3D&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?

With an appropriate unit.

> 
>>> +    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.

It is obvious because it is the same for every device being a consumer
of external clock. There is no point to duplicate non-device related
examples in every device binding.

Best regards,
Krzysztof
  

On Thu, Nov 17, 2022 at 8:39 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 16/11/2022 21:11, Alex Helms wrote:
> >>> +  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?
>
> With an appropriate unit.

Krzysztof really means the property should be named e.g.
"renesas,ss-amount-centipercent" instead.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
  

On 17/11/2022 09:11, Geert Uytterhoeven wrote:
>>>>> +
>>>>> +  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&data=05%7C01%7Calexander.helms.jy%40renesas.com%7C9c13a32848f3434e217108dac7ab69f6%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638041836281252737%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6MULpJhPyyjWSo1SvPCrz6KidE1VEtiiNYk1O5wS1vI%3D&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?
>>
>> With an appropriate unit.
> 
> Krzysztof really means the property should be named e.g.
> "renesas,ss-amount-centipercent" instead.
> 

I provided an URL to document with units, so I was hoping Alex will take
a bit of effort to open it. Hundredths of percent sounds like basis
point... If it is not basis point, maybe it could be made such?

Best regards,
Krzysztof
  

Hi Krzysztof,

On Thu, Nov 17, 2022 at 11:41 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 17/11/2022 09:11, Geert Uytterhoeven wrote:
> >>>>> +  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?
> >>
> >> With an appropriate unit.
> >
> > Krzysztof really means the property should be named e.g.
> > "renesas,ss-amount-centipercent" instead.
>
> I provided an URL to document with units, so I was hoping Alex will take
> a bit of effort to open it. Hundredths of percent sounds like basis
> point... If it is not basis point, maybe it could be made such?

Thank you, I didn't know about basis points. TIL ;-)

(and I didn't dare to click on the safelink-converted quoted link)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds