2022-09-14 21:48:28

by Alex Helms

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: clock: Add bindings for Renesas ProXO

Add dt bindings for the Renesas ProXO oscillator.

Signed-off-by: Alex Helms <[email protected]>
---
.../bindings/clock/renesas,proxo.yaml | 50 +++++++++++++++++++
MAINTAINERS | 5 ++
2 files changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/renesas,proxo.yaml

diff --git a/Documentation/devicetree/bindings/clock/renesas,proxo.yaml b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml
new file mode 100644
index 000000000..2ae131431
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/renesas,versaclock7.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas ProXO Oscillator Device Tree Bindings
+
+maintainers:
+ - Alex Helms <[email protected]>
+
+description: |
+ Renesas ProXO is a family of programmable ultra-low phase noise
+ quartz-based oscillators.
+
+properties:
+ '#clock-cells':
+ const: 0
+
+ compatible:
+ enum:
+ - renesas,proxo-xp
+
+ reg:
+ maxItems: 1
+
+ renesas,xtal:
+ description: Internal crystal frequency, default is 50000000 (50MHz)
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+ - '#clock-cells'
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c@0 {
+ reg = <0x0 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ proxo: proxo@55 {
+ compatible = "renesas,proxo-xp";
+ reg = <0x55>;
+ #clock-cells = <0>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 350102355..d52a8a5d2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16080,6 +16080,11 @@ S: Supported
F: Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml
F: drivers/iio/adc/rzg2l_adc.c

+RENESAS PROXO CLOCK DRIVER
+M: Alex Helms <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/clock/renesas,proxo.yaml
+
RESET CONTROLLER FRAMEWORK
M: Philipp Zabel <[email protected]>
S: Maintained
--
2.30.2


2022-09-15 09:42:22

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: clock: Add bindings for Renesas ProXO

On Wed, 14 Sep 2022 14:18:08 -0700, Alex Helms wrote:
> Add dt bindings for the Renesas ProXO oscillator.
>
> Signed-off-by: Alex Helms <[email protected]>
> ---
> .../bindings/clock/renesas,proxo.yaml | 50 +++++++++++++++++++
> MAINTAINERS | 5 ++
> 2 files changed, 55 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/clock/renesas,proxo.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/clock/renesas,proxo.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2022-09-15 12:28:49

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: clock: Add bindings for Renesas ProXO

On Wed, Sep 14, 2022 at 02:18:08PM -0700, Alex Helms wrote:
> Add dt bindings for the Renesas ProXO oscillator.
>
> Signed-off-by: Alex Helms <[email protected]>
> ---
> .../bindings/clock/renesas,proxo.yaml | 50 +++++++++++++++++++
> MAINTAINERS | 5 ++
> 2 files changed, 55 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>
> diff --git a/Documentation/devicetree/bindings/clock/renesas,proxo.yaml b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml
> new file mode 100644
> index 000000000..2ae131431
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/renesas,versaclock7.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas ProXO Oscillator Device Tree Bindings
> +
> +maintainers:
> + - Alex Helms <[email protected]>
> +
> +description: |

Don't need '|' if no formatting to preserve.

> + Renesas ProXO is a family of programmable ultra-low phase noise
> + quartz-based oscillators.

Are they all the same from a s/w standpoint. If not you need specific
compatibles for the differences.

> +
> +properties:
> + '#clock-cells':
> + const: 0
> +
> + compatible:
> + enum:
> + - renesas,proxo-xp
> +
> + reg:
> + maxItems: 1
> +
> + renesas,xtal:
> + description: Internal crystal frequency, default is 50000000 (50MHz)
> + $ref: /schemas/types.yaml#/definitions/uint32

Use 'clock-frequency'.

> +
> +required:
> + - '#clock-cells'
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c@0 {

i2c {

> + reg = <0x0 0x100>;

Drop

> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + proxo: proxo@55 {

clock-controller@55

> + compatible = "renesas,proxo-xp";
> + reg = <0x55>;
> + #clock-cells = <0>;
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 350102355..d52a8a5d2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16080,6 +16080,11 @@ S: Supported
> F: Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml
> F: drivers/iio/adc/rzg2l_adc.c
>
> +RENESAS PROXO CLOCK DRIVER
> +M: Alex Helms <[email protected]>
> +S: Maintained
> +F: Documentation/devicetree/bindings/clock/renesas,proxo.yaml
> +
> RESET CONTROLLER FRAMEWORK
> M: Philipp Zabel <[email protected]>
> S: Maintained
> --
> 2.30.2
>
>

2022-09-22 18:16:38

by Alex Helms

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: clock: Add bindings for Renesas ProXO

On 9/15/2022 5:08 AM, Rob Herring wrote:
> On Wed, Sep 14, 2022 at 02:18:08PM -0700, Alex Helms wrote:
>> Add dt bindings for the Renesas ProXO oscillator.
>>
>> Signed-off-by: Alex Helms <[email protected]>
>> ---
>> .../bindings/clock/renesas,proxo.yaml | 50 +++++++++++++++++++
>> MAINTAINERS | 5 ++
>> 2 files changed, 55 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/clock/renesas,proxo.yaml b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>> new file mode 100644
>> index 000000000..2ae131431
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>> @@ -0,0 +1,50 @@
>> +# 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%2Cversaclock7.yaml%23&amp;data=05%7C01%7Calexander.helms.jy%40renesas.com%7C34ce960b2542459b61d408da97130b50%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637988405289843575%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=izvR%2FXbUNVXknaODSnMmcetPnn%2Ff3Jk%2F6RuBdZ7uSa0%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%7C34ce960b2542459b61d408da97130b50%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637988405289843575%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=dQ7nZtgfTE5DrXB1iavJuOhSWmGl4tYI1m6iZpkPAhA%3D&amp;reserved=0
>> +
>> +title: Renesas ProXO Oscillator Device Tree Bindings
>> +
>> +maintainers:
>> + - Alex Helms <[email protected]>
>> +
>> +description: |
>
> Don't need '|' if no formatting to preserve.
>
>> + Renesas ProXO is a family of programmable ultra-low phase noise
>> + quartz-based oscillators.
>
> Are they all the same from a s/w standpoint. If not you need specific
> compatibles for the differences.

At this time only the XP variant is supported and this is reflected in
the "compatible" string.

>
>> +
>> +properties:
>> + '#clock-cells':
>> + const: 0
>> +
>> + compatible:
>> + enum:
>> + - renesas,proxo-xp
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + renesas,xtal:
>> + description: Internal crystal frequency, default is 50000000 (50MHz)
>> + $ref: /schemas/types.yaml#/definitions/uint32
>
> Use 'clock-frequency'.

I think "crystal-frequency" would make more sense. The parts come
programmed to a factory clock frequency which is different than the
internal crystal frequency.

>
>> +
>> +required:
>> + - '#clock-cells'
>> + - compatible
>> + - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + i2c@0 {
>
> i2c {
>
>> + reg = <0x0 0x100>;
>
> Drop
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + proxo: proxo@55 {
>
> clock-controller@55
>
>> + compatible = "renesas,proxo-xp";
>> + reg = <0x55>;
>> + #clock-cells = <0>;
>> + };
>> + };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 350102355..d52a8a5d2 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -16080,6 +16080,11 @@ S: Supported
>> F: Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml
>> F: drivers/iio/adc/rzg2l_adc.c
>>
>> +RENESAS PROXO CLOCK DRIVER
>> +M: Alex Helms <[email protected]>
>> +S: Maintained
>> +F: Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>> +
>> RESET CONTROLLER FRAMEWORK
>> M: Philipp Zabel <[email protected]>
>> S: Maintained
>> --
>> 2.30.2
>>
>>

2022-10-29 00:01:28

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: clock: Add bindings for Renesas ProXO

On 22/09/2022 14:06, Alex Helms wrote:
> On 9/15/2022 5:08 AM, Rob Herring wrote:
>> On Wed, Sep 14, 2022 at 02:18:08PM -0700, Alex Helms wrote:
>>> Add dt bindings for the Renesas ProXO oscillator.
>>>
>>> Signed-off-by: Alex Helms <[email protected]>

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

>>> ---
>>> .../bindings/clock/renesas,proxo.yaml | 50 +++++++++++++++++++
>>> MAINTAINERS | 5 ++
>>> 2 files changed, 55 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/renesas,proxo.yaml b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>>> new file mode 100644
>>> index 000000000..2ae131431
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>>> @@ -0,0 +1,50 @@
>>> +# 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%2Cversaclock7.yaml%23&amp;data=05%7C01%7Calexander.helms.jy%40renesas.com%7C34ce960b2542459b61d408da97130b50%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637988405289843575%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=izvR%2FXbUNVXknaODSnMmcetPnn%2Ff3Jk%2F6RuBdZ7uSa0%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%7C34ce960b2542459b61d408da97130b50%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637988405289843575%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=dQ7nZtgfTE5DrXB1iavJuOhSWmGl4tYI1m6iZpkPAhA%3D&amp;reserved=0
>>> +
>>> +title: Renesas ProXO Oscillator Device Tree Bindings
>>> +
>>> +maintainers:
>>> + - Alex Helms <[email protected]>
>>> +
>>> +description: |
>>
>> Don't need '|' if no formatting to preserve.
>>
>>> + Renesas ProXO is a family of programmable ultra-low phase noise
>>> + quartz-based oscillators.
>>
>> Are they all the same from a s/w standpoint. If not you need specific
>> compatibles for the differences.
>
> At this time only the XP variant is supported and this is reflected in
> the "compatible" string.

You should document everything, if possible, not what is supported by
some version of some system/driver/firmware.

>
>>
>>> +
>>> +properties:
>>> + '#clock-cells':
>>> + const: 0
>>> +
>>> + compatible:
>>> + enum:
>>> + - renesas,proxo-xp
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + renesas,xtal:
>>> + description: Internal crystal frequency, default is 50000000 (50MHz)
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>
>> Use 'clock-frequency'.
>
> I think "crystal-frequency" would make more sense. The parts come
> programmed to a factory clock frequency which is different than the
> internal crystal frequency.

We have a standard property and it should be used, when applicable.
Don't encode here frequency for something else than the driving clock.

Best regards,
Krzysztof