| Message ID | 20240108135421.684263-2-tmaimon77@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Introduce Nuvoton Arbel NPCM8XX BMC SoC | expand |
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.
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
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 >
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
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
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.
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
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>;
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(+)