| Message ID | 20241229-b4-k230-clk-v1-1-221a917e80ed@zohomail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: canaan: Add support for K230-Canmv clock | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
On Sun, 29 Dec 2024 21:21:08 +0800, Xukai Wang wrote: > This patch adds the Device Tree binding for the clock controller > on Canaan k230. The binding defines the new clocks available and > the required properties to configure them correctly. > > Signed-off-by: Xukai Wang <kingxukai@zohomail.com> > --- > .../devicetree/bindings/clock/canaan,k230-clk.yaml | 41 ++++++++++++++++++ > include/dt-bindings/clock/k230-clk.h | 49 ++++++++++++++++++++++ > 2 files changed, 90 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/canaan,k230-clk.example.dtb: clock-controller@91102000: clocks: 1 was expected from schema $id: http://devicetree.org/schemas/clock/canaan,k230-clk.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241229-b4-k230-clk-v1-1-221a917e80ed@zohomail.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.
On 2024/12/29 21:21, Xukai Wang wrote: > This patch adds the Device Tree binding for the clock controller > on Canaan k230. The binding defines the new clocks available and > the required properties to configure them correctly. > > Signed-off-by: Xukai Wang <kingxukai@zohomail.com> > --- > .../devicetree/bindings/clock/canaan,k230-clk.yaml | 41 ++++++++++++++++++ > include/dt-bindings/clock/k230-clk.h | 49 ++++++++++++++++++++++ > 2 files changed, 90 insertions(+) > > diff --git a/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..ffd4e0b052455bf3dcedd9355d93764119df3d68 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml > @@ -0,0 +1,41 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/canaan,k230-clk.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Canaan Kendryte K230 Clock > + > +maintainers: > + - Xukai Wang <kingxukai@zohomail.com> > + > +properties: > + compatible: > + const: canaan,k230-clk > + > + clocks: > + const: 1 `maxItems: 1` instead of `const: 1` > +
On Sun, Dec 29, 2024 at 09:21:08PM +0800, Xukai Wang wrote: > This patch adds the Device Tree binding for the clock controller > on Canaan k230. The binding defines the new clocks available and > the required properties to configure them correctly. > > Signed-off-by: Xukai Wang <kingxukai@zohomail.com> > --- > .../devicetree/bindings/clock/canaan,k230-clk.yaml | 41 ++++++++++++++++++ > include/dt-bindings/clock/k230-clk.h | 49 ++++++++++++++++++++++ > 2 files changed, 90 insertions(+) Please run scripts/checkpatch.pl and fix reported warnings. After that, run also 'scripts/checkpatch.pl --strict' and (probably) fix more warnings. Some warnings can be ignored, especially from --strict run, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. > > diff --git a/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..ffd4e0b052455bf3dcedd9355d93764119df3d68 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml > @@ -0,0 +1,41 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/canaan,k230-clk.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Canaan Kendryte K230 Clock > + > +maintainers: > + - Xukai Wang <kingxukai@zohomail.com> > + > +properties: > + compatible: > + const: canaan,k230-clk > + > + clocks: > + const: 1 > + > + reg: > + maxItems: 2 > + minItems: 1 List and describe the items instead. > + > + '#clock-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + - clocks > + - '#clock-cells' > + > +additionalProperties: false > + > +examples: > + - | > + clock-controller@91102000 { > + compatible = "canaan,k230-clk"; > + reg = <0x91102000 0x1000>, <0x91100000 0x1000>; > + #clock-cells = <1>; > + clocks = <&osc24m>; > + }; > diff --git a/include/dt-bindings/clock/k230-clk.h b/include/dt-bindings/clock/k230-clk.h > new file mode 100644 > index 0000000000000000000000000000000000000000..31d1f82fbcff654072ef1a8985a884377d801e72 > --- /dev/null > +++ b/include/dt-bindings/clock/k230-clk.h > @@ -0,0 +1,49 @@ > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > +/* > + * Kendryte Canaan K230 Clock Drivers > + * > + * Author: Xukai Wang <kingxukai@zohomail.com> > + */ > + > +#ifndef CLOCK_K230_CLK_H > +#define CLOCK_K230_CLK_H > + > +/* Kendryte K230 SoC clock identifiers (arbitrary values). */ > +#define K230_CPU0_SRC 0 Drop the indentation after '#define' Best regards, Krzysztof
On 2024/12/30 15:54, Krzysztof Kozlowski wrote: > On Sun, Dec 29, 2024 at 09:21:08PM +0800, Xukai Wang wrote: >> This patch adds the Device Tree binding for the clock controller >> on Canaan k230. The binding defines the new clocks available and >> the required properties to configure them correctly. >> >> Signed-off-by: Xukai Wang <kingxukai@zohomail.com> >> --- >> .../devicetree/bindings/clock/canaan,k230-clk.yaml | 41 ++++++++++++++++++ >> include/dt-bindings/clock/k230-clk.h | 49 ++++++++++++++++++++++ >> 2 files changed, 90 insertions(+) > Please run scripts/checkpatch.pl and fix reported warnings. After that, > run also 'scripts/checkpatch.pl --strict' and (probably) fix more > warnings. Some warnings can be ignored, especially from --strict run, > but the code here looks like it needs a fix. Feel free to get in touch > if the warning is not clear. Apologies for forgetting to use `--strict` with `checkpatch.pl`. I will run it and address the warnings accordingly before next submission. >> diff --git a/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml >> new file mode 100644 >> index 0000000000000000000000000000000000000000..ffd4e0b052455bf3dcedd9355d93764119df3d68 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml >> @@ -0,0 +1,41 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/clock/canaan,k230-clk.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Canaan Kendryte K230 Clock >> + >> +maintainers: >> + - Xukai Wang <kingxukai@zohomail.com> >> + >> +properties: >> + compatible: >> + const: canaan,k230-clk >> + >> + clocks: >> + const: 1 >> + >> + reg: >> + maxItems: 2 >> + minItems: 1 > List and describe the items instead. OK, thank you for the suggestion. I've addressed the feedback and listed the items under reg with detailed descriptions. Here's the update version: reg: description: | The `reg` property specifies the base address and size of the device's registers. - The first address corresponds to the base address of the PLL control registers. - The second address corresponds to the base address of the sysclk control registers. minItems: 1 items: - description: Base address and size of the PLL control registers. - description: Base address and size of the sysclk control registers. Does this content look appropriate? >> + >> + '#clock-cells': >> + const: 1 >> + >> +required: >> + - compatible >> + - reg >> + - clocks >> + - '#clock-cells' >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + clock-controller@91102000 { >> + compatible = "canaan,k230-clk"; >> + reg = <0x91102000 0x1000>, <0x91100000 0x1000>; >> + #clock-cells = <1>; >> + clocks = <&osc24m>; >> + }; >> diff --git a/include/dt-bindings/clock/k230-clk.h b/include/dt-bindings/clock/k230-clk.h >> new file mode 100644 >> index 0000000000000000000000000000000000000000..31d1f82fbcff654072ef1a8985a884377d801e72 >> --- /dev/null >> +++ b/include/dt-bindings/clock/k230-clk.h >> @@ -0,0 +1,49 @@ >> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ >> +/* >> + * Kendryte Canaan K230 Clock Drivers >> + * >> + * Author: Xukai Wang <kingxukai@zohomail.com> >> + */ >> + >> +#ifndef CLOCK_K230_CLK_H >> +#define CLOCK_K230_CLK_H >> + >> +/* Kendryte K230 SoC clock identifiers (arbitrary values). */ >> +#define K230_CPU0_SRC 0 > Drop the indentation after '#define' Thank you for your feedback. I have removed the indentation after #define. > Best regards, > Krzysztof >
On 04/01/2025 10:23, Xukai Wang wrote: > > Here's the update version: > > reg: > description: | > The `reg` property specifies the base address and size of the > device's registers. > - The first address corresponds to the base address of the PLL > control registers. > - The second address corresponds to the base address of the sysclk > control registers. No, drop all these. You duplicate schema. > minItems: 1 Why? > items: > - description: Base address and size of the PLL control registers. > - description: Base address and size of the sysclk control registers. Drop redundant "Base address and size of the". This cannot be anything else in this context. > > Does this content look appropriate? > >>> + >>> + '#clock-cells': >>> + const: 1 >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - clocks >>> + - '#clock-cells' >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + clock-controller@91102000 { >>> + compatible = "canaan,k230-clk"; >>> + reg = <0x91102000 0x1000>, <0x91100000 0x1000>; >>> + #clock-cells = <1>; >>> + clocks = <&osc24m>; >>> + }; >>> diff --git a/include/dt-bindings/clock/k230-clk.h b/include/dt-bindings/clock/k230-clk.h I missed one thing - filename is supposed to be the same as binding filename. Best regards, Krzysztof
On 2025/1/4 18:40, Krzysztof Kozlowski wrote: > On 04/01/2025 10:23, Xukai Wang wrote: >> Here's the update version: >> >> reg: >> description: | >> The `reg` property specifies the base address and size of the >> device's registers. >> - The first address corresponds to the base address of the PLL >> control registers. >> - The second address corresponds to the base address of the sysclk >> control registers. > No, drop all these. You duplicate schema. > >> minItems: 1 > Why? > >> items: >> - description: Base address and size of the PLL control registers. >> - description: Base address and size of the sysclk control registers. > Drop redundant "Base address and size of the". This cannot be anything > else in this context. Thank you for point these out, I’ve removed the redundant descriptions, and here's the updated version: reg: items: - description: PLL control registers. - description: Sysclk control registers. Does this content look appropriate? >>>> + >>>> + '#clock-cells': >>>> + const: 1 >>>> + >>>> +required: >>>> + - compatible >>>> + - reg >>>> + - clocks >>>> + - '#clock-cells' >>>> + >>>> +additionalProperties: false >>>> + >>>> +examples: >>>> + - | >>>> + clock-controller@91102000 { >>>> + compatible = "canaan,k230-clk"; >>>> + reg = <0x91102000 0x1000>, <0x91100000 0x1000>; >>>> + #clock-cells = <1>; >>>> + clocks = <&osc24m>; >>>> + }; >>>> diff --git a/include/dt-bindings/clock/k230-clk.h b/include/dt-bindings/clock/k230-clk.h > I missed one thing - filename is supposed to be the same as binding > filename. OK, I will rename the file `k230-clk.h` to `canaan,k230-clk.h` to maintain consistency with the binding filename. > > Best regards, > Krzysztof
On 04/01/2025 13:09, Xukai Wang wrote: >> Why? >> >>> items: >>> - description: Base address and size of the PLL control registers. >>> - description: Base address and size of the sysclk control registers. >> Drop redundant "Base address and size of the". This cannot be anything >> else in this context. > > Thank you for point these out, I’ve removed the redundant descriptions, > and here's the updated version: > > reg: > items: > - description: PLL control registers. > - description: Sysclk control registers. > > Does this content look appropriate? Yes, thank you. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml new file mode 100644 index 0000000000000000000000000000000000000000..ffd4e0b052455bf3dcedd9355d93764119df3d68 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/canaan,k230-clk.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Canaan Kendryte K230 Clock + +maintainers: + - Xukai Wang <kingxukai@zohomail.com> + +properties: + compatible: + const: canaan,k230-clk + + clocks: + const: 1 + + reg: + maxItems: 2 + minItems: 1 + + '#clock-cells': + const: 1 + +required: + - compatible + - reg + - clocks + - '#clock-cells' + +additionalProperties: false + +examples: + - | + clock-controller@91102000 { + compatible = "canaan,k230-clk"; + reg = <0x91102000 0x1000>, <0x91100000 0x1000>; + #clock-cells = <1>; + clocks = <&osc24m>; + }; diff --git a/include/dt-bindings/clock/k230-clk.h b/include/dt-bindings/clock/k230-clk.h new file mode 100644 index 0000000000000000000000000000000000000000..31d1f82fbcff654072ef1a8985a884377d801e72 --- /dev/null +++ b/include/dt-bindings/clock/k230-clk.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ +/* + * Kendryte Canaan K230 Clock Drivers + * + * Author: Xukai Wang <kingxukai@zohomail.com> + */ + +#ifndef CLOCK_K230_CLK_H +#define CLOCK_K230_CLK_H + +/* Kendryte K230 SoC clock identifiers (arbitrary values). */ +#define K230_CPU0_SRC 0 +#define K230_CPU0_ACLK 1 +#define K230_CPU0_PLIC 2 +#define K230_CPU0_NOC_DDRCP4 3 +#define K230_CPU0_PCLK 4 +#define K230_PMU_PCLK 5 +#define K230_HS_HCLK_HIGN_SRC 6 +#define K230_HS_HCLK_HIGN_GATE 7 +#define K230_HS_HCLK_SRC 8 +#define K230_HS_SD0_HS_AHB_GAT 9 +#define K230_HS_SD1_HS_AHB_GAT 10 +#define K230_HS_SSI1_HS_AHB_GA 11 +#define K230_HS_SSI2_HS_AHB_GA 12 +#define K230_HS_USB0_HS_AHB_GA 13 +#define K230_HS_USB1_HS_AHB_GA 14 +#define K230_HS_SSI0_AXI15 15 +#define K230_HS_SSI1 16 +#define K230_HS_SSI2 17 +#define K230_HS_QSPI_AXI_SRC 18 +#define K230_HS_SSI1_ACLK_GATE 19 +#define K230_HS_SSI2_ACLK_GATE 20 +#define K230_HS_SD_CARD_SRC 21 +#define K230_HS_SD0_CARD_TX 22 +#define K230_HS_SD1_CARD_TX 23 +#define K230_HS_SD_AXI_SRC 24 +#define K230_HS_SD0_AXI_GATE 25 +#define K230_HS_SD1_AXI_GATE 26 +#define K230_HS_SD0_BASE_GATE 27 +#define K230_HS_SD1_BASE_GATE 28 +#define K230_HS_OSPI_SRC 29 +#define K230_HS_USB_REF_50M 30 +#define K230_HS_SD_TIMER_SRC 31 +#define K230_HS_SD0_TIMER_GATE 32 +#define K230_HS_SD1_TIMER_GATE 33 +#define K230_HS_USB0_REFERENCE 34 +#define K230_HS_USB1_REFERENCE 35 + +#endif /* CLOCK_K230_CLK_H */
This patch adds the Device Tree binding for the clock controller on Canaan k230. The binding defines the new clocks available and the required properties to configure them correctly. Signed-off-by: Xukai Wang <kingxukai@zohomail.com> --- .../devicetree/bindings/clock/canaan,k230-clk.yaml | 41 ++++++++++++++++++ include/dt-bindings/clock/k230-clk.h | 49 ++++++++++++++++++++++ 2 files changed, 90 insertions(+)