1. Patchew
  2. linux
  3. [PATCH v22 0/8] Introduce Nuvoton Arbel NPCM8XX BMC SoC
  4. View patch

[PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property

Tomer Maimon posted 8 patches 2 years, 1 month ago
There is a newer version of this series
[PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Posted by Tomer Maimon 2 years, 1 month ago 
The NPCM8XX clock driver uses 25Mhz external clock, therefor adding
refclk property.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 .../bindings/clock/nuvoton,npcm845-clk.yaml      | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
index b901ca13cd25..0b642bfce292 100644
--- a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
+++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
@@ -21,6 +21,14 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    items:
+      - description: 25Mhz referance clock
+
+  clock-names:
+    items:
+      - const: refclk
+
   '#clock-cells':
     const: 1
     description:
@@ -30,12 +38,20 @@ properties:
 required:
   - compatible
   - reg
+  - clocks
+  - clock-names
   - '#clock-cells'
 
 additionalProperties: false
 
 examples:
   - |
+    refclk: refclk-25mhz {
+        compatible = "fixed-clock";
+        #clock-cells = <0>;
+        clock-frequency = <25000000>;
+    }; 
+  
     ahb {
         #address-cells = <2>;
         #size-cells = <2>;
-- 
2.34.1
Re: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Posted by Rob Herring 2 years ago 
On Mon, Jan 08, 2024 at 03:54:14PM +0200, Tomer Maimon wrote:
> The NPCM8XX clock driver uses 25Mhz external clock, therefor adding

therefore

> refclk property.

'refclk' is not a property.

> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../bindings/clock/nuvoton,npcm845-clk.yaml      | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> index b901ca13cd25..0b642bfce292 100644
> --- a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> +++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> @@ -21,6 +21,14 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  clocks:
> +    items:
> +      - description: 25Mhz referance clock

reference

> +
> +  clock-names:
> +    items:
> +      - const: refclk
> +
>    '#clock-cells':
>      const: 1
>      description:
> @@ -30,12 +38,20 @@ properties:
>  required:
>    - compatible
>    - reg
> +  - clocks
> +  - clock-names

New required properties are an ABI break. That's fine if you explain why 
that's okay in the commit msg.


>    - '#clock-cells'
>  
>  additionalProperties: false
>  
>  examples:
>    - |
> +    refclk: refclk-25mhz {
> +        compatible = "fixed-clock";
> +        #clock-cells = <0>;
> +        clock-frequency = <25000000>;
> +    }; 

Examples don't need to show providers.

> +  
>      ahb {
>          #address-cells = <2>;
>          #size-cells = <2>;
> -- 
> 2.34.1
>
Re: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Posted by Tomer Maimon 2 years ago 
Hi Rob,

Thanks for your comment.

On Tue, 9 Jan 2024 at 19:08, Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Jan 08, 2024 at 03:54:14PM +0200, Tomer Maimon wrote:
> > The NPCM8XX clock driver uses 25Mhz external clock, therefor adding
>
> therefore
>
> > refclk property.
>
> 'refclk' is not a property.
>
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >  .../bindings/clock/nuvoton,npcm845-clk.yaml      | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> > index b901ca13cd25..0b642bfce292 100644
> > --- a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> > +++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> > @@ -21,6 +21,14 @@ properties:
> >    reg:
> >      maxItems: 1
> >
> > +  clocks:
> > +    items:
> > +      - description: 25Mhz referance clock
>
> reference
>
> > +
> > +  clock-names:
> > +    items:
> > +      - const: refclk
> > +
> >    '#clock-cells':
> >      const: 1
> >      description:
> > @@ -30,12 +38,20 @@ properties:
> >  required:
> >    - compatible
> >    - reg
> > +  - clocks
> > +  - clock-names
>
> New required properties are an ABI break. That's fine if you explain why
> that's okay in the commit msg.
What do you mean?
Could I add the new required properties to the required list?
>
>
> >    - '#clock-cells'
> >
> >  additionalProperties: false
> >
> >  examples:
> >    - |
> > +    refclk: refclk-25mhz {
> > +        compatible = "fixed-clock";
> > +        #clock-cells = <0>;
> > +        clock-frequency = <25000000>;
> > +    };
>
> Examples don't need to show providers.
>
> > +
> >      ahb {
> >          #address-cells = <2>;
> >          #size-cells = <2>;
> > --
> > 2.34.1
> >

Best regards,

Tomer
Re: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Posted by Krzysztof Kozlowski 2 years ago 
On 10/01/2024 14:47, Tomer Maimon wrote:
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: refclk
>>> +
>>>    '#clock-cells':
>>>      const: 1
>>>      description:
>>> @@ -30,12 +38,20 @@ properties:
>>>  required:
>>>    - compatible
>>>    - reg
>>> +  - clocks
>>> +  - clock-names
>>
>> New required properties are an ABI break. That's fine if you explain why
>> that's okay in the commit msg.
> What do you mean?

I think it was clear. Which part is not clear?

> Could I add the new required properties to the required list?

You just did, didn't you? And received feedback that you are breaking
the ABI.

Best regards,
Krzysztof
Re: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Posted by Stephen Boyd 2 years ago 
Quoting Krzysztof Kozlowski (2024-01-10 12:54:14)
> On 10/01/2024 14:47, Tomer Maimon wrote:
> >>> +
> >>> +  clock-names:
> >>> +    items:
> >>> +      - const: refclk
> >>> +
> >>>    '#clock-cells':
> >>>      const: 1
> >>>      description:
> >>> @@ -30,12 +38,20 @@ properties:
> >>>  required:
> >>>    - compatible
> >>>    - reg
> >>> +  - clocks
> >>> +  - clock-names
> >>
> >> New required properties are an ABI break. That's fine if you explain why
> >> that's okay in the commit msg.
> > What do you mean?
> 
> I think it was clear. Which part is not clear?
> 
> > Could I add the new required properties to the required list?
> 
> You just did, didn't you? And received feedback that you are breaking
> the ABI.
> 

It's fine to break the ABI as long as the commit message explains that
the driver isn't merged yet.
Re: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Posted by Tomer Maimon 2 years ago 
Hi Stephen,

On Wed, 10 Jan 2024 at 23:46, Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Krzysztof Kozlowski (2024-01-10 12:54:14)
> > On 10/01/2024 14:47, Tomer Maimon wrote:
> > >>> +
> > >>> +  clock-names:
> > >>> +    items:
> > >>> +      - const: refclk
> > >>> +
> > >>>    '#clock-cells':
> > >>>      const: 1
> > >>>      description:
> > >>> @@ -30,12 +38,20 @@ properties:
> > >>>  required:
> > >>>    - compatible
> > >>>    - reg
> > >>> +  - clocks
> > >>> +  - clock-names
> > >>
> > >> New required properties are an ABI break. That's fine if you explain why
> > >> that's okay in the commit msg.
> > > What do you mean?
> >
> > I think it was clear. Which part is not clear?
> >
> > > Could I add the new required properties to the required list?
> >
> > You just did, didn't you? And received feedback that you are breaking
> > the ABI.
> >
>
> It's fine to break the ABI as long as the commit message explains that
> the driver isn't merged yet.

Thanks for your clarification.

Best regards,

Tomer
Re: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Posted by Rob Herring 2 years, 1 month ago 
On Mon, 08 Jan 2024 15:54:14 +0200, Tomer Maimon wrote:
> The NPCM8XX clock driver uses 25Mhz external clock, therefor adding
> refclk property.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../bindings/clock/nuvoton,npcm845-clk.yaml      | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 

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:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.example.dtb: clock-controller@f0801000: 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.example.dtb: clock-controller@f0801000: 'clock-names' is a required property
	from schema $id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240108135421.684263-2-tmaimon77@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Posted by Tomer Maimon 2 years ago 
Hi Rob,

Thanks for your comment.

On Mon, 8 Jan 2024 at 23:09, Rob Herring <robh@kernel.org> wrote:
>
>
> On Mon, 08 Jan 2024 15:54:14 +0200, Tomer Maimon wrote:
> > The NPCM8XX clock driver uses 25Mhz external clock, therefor adding
> > refclk property.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >  .../bindings/clock/nuvoton,npcm845-clk.yaml      | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
>
> 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:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.example.dtb: clock-controller@f0801000: 'clocks' is a required property
>         from schema $id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.example.dtb: clock-controller@f0801000: 'clock-names' is a required property
>         from schema $id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240108135421.684263-2-tmaimon77@gmail.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> 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 after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>

probably I missed adding the clock and clock-names to the example
node, will be fixed next version

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.