| Message ID | 20230221141147.303642-2-xingyu.wu@starfivetech.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Add PLL clocks driver for StarFive JH7110 | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
On 21/02/2023 15:11, Xingyu Wu wrote: > Add bindings for the PLL clock generator on the JH7110 RISC-V SoC. Just one sentence... not explaining anything around ABI break. Nope. > > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > --- > .../bindings/clock/starfive,jh7110-pll.yaml | 45 +++++++++++++++++++ > .../dt-bindings/clock/starfive,jh7110-crg.h | 12 ++--- > 2 files changed, 52 insertions(+), 5 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml > > diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml > new file mode 100644 > index 000000000000..8fd18e6c2e9b > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml > @@ -0,0 +1,45 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/starfive,jh7110-pll.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: StarFive JH7110 PLL Clock Generator > + > +maintainers: > + - Xingyu Wu <xingyu.wu@starfivetech.com> > + > +properties: > + compatible: > + const: starfive,jh7110-pll > + > + clocks: > + maxItems: 1 > + - description: Main Oscillator (24 MHz) Does not look like you tested the bindings. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). > + > + '#clock-cells': > + const: 1 > + description: > + See <dt-bindings/clock/starfive,jh7110-crg.h> for valid indices. > + > + starfive,sysreg: > + $ref: /schemas/types.yaml#/definitions/phandle-array That's not how the property is modeled... look at other code. > + description: > + the phandle to System Register Controller syscon node. > + > +required: > + - compatible > + - clocks > + - '#clock-cells' > + - starfive,sysreg > + > +additionalProperties: false > + > +examples: > + - | > + pllclk: pll-clock-controller { > + compatible = "starfive,jh7110-pll"; > + clocks = <&osc>; > + #clock-cells = <1>; > + starfive,sysreg = <&sys_syscon>; > + }; > diff --git a/include/dt-bindings/clock/starfive,jh7110-crg.h b/include/dt-bindings/clock/starfive,jh7110-crg.h > index 5e4f21ca0642..086a6ddcf380 100644 > --- a/include/dt-bindings/clock/starfive,jh7110-crg.h > +++ b/include/dt-bindings/clock/starfive,jh7110-crg.h > @@ -6,6 +6,12 @@ > #ifndef __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__ > #define __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__ > > +/* PLL clocks */ > +#define JH7110_CLK_PLL0_OUT 0 > +#define JH7110_CLK_PLL1_OUT 1 > +#define JH7110_CLK_PLL2_OUT 2 > +#define JH7110_PLLCLK_END 3 > + > /* SYSCRG clocks */ > #define JH7110_SYSCLK_CPU_ROOT 0 > #define JH7110_SYSCLK_CPU_CORE 1 > @@ -198,11 +204,7 @@ > #define JH7110_SYSCLK_TDM_TDM_INV 188 > #define JH7110_SYSCLK_JTAG_CERTIFICATION_TRNG 189 > > -#define JH7110_SYSCLK_PLL0_OUT 190 > -#define JH7110_SYSCLK_PLL1_OUT 191 > -#define JH7110_SYSCLK_PLL2_OUT 192 > - NAK. ABI break. You have entire commit msg to explain this and avoid questions from reviewers. Best regards, Krzysztof
On 2023/2/22 17:11, Krzysztof Kozlowski wrote: > On 21/02/2023 15:11, Xingyu Wu wrote: >> Add bindings for the PLL clock generator on the JH7110 RISC-V SoC. > > Just one sentence... not explaining anything around ABI break. Nope. > >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> --- >> .../bindings/clock/starfive,jh7110-pll.yaml | 45 +++++++++++++++++++ >> .../dt-bindings/clock/starfive,jh7110-crg.h | 12 ++--- >> 2 files changed, 52 insertions(+), 5 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml >> >> diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml >> new file mode 100644 >> index 000000000000..8fd18e6c2e9b >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml >> @@ -0,0 +1,45 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/clock/starfive,jh7110-pll.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: StarFive JH7110 PLL Clock Generator >> + >> +maintainers: >> + - Xingyu Wu <xingyu.wu@starfivetech.com> >> + >> +properties: >> + compatible: >> + const: starfive,jh7110-pll >> + >> + clocks: >> + maxItems: 1 >> + - description: Main Oscillator (24 MHz) > > Does not look like you tested the bindings. Please run `make > dt_binding_check` (see > Documentation/devicetree/bindings/writing-schema.rst for instructions). Remove the ' - ' before 'description' and pass the dt_binding_check. > >> + >> + '#clock-cells': >> + const: 1 >> + description: >> + See <dt-bindings/clock/starfive,jh7110-crg.h> for valid indices. >> + >> + starfive,sysreg: >> + $ref: /schemas/types.yaml#/definitions/phandle-array > > That's not how the property is modeled... look at other code. Remove the '-array'. > >> + description: >> + the phandle to System Register Controller syscon node. >> + >> +required: >> + - compatible >> + - clocks >> + - '#clock-cells' >> + - starfive,sysreg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + pllclk: pll-clock-controller { >> + compatible = "starfive,jh7110-pll"; >> + clocks = <&osc>; >> + #clock-cells = <1>; >> + starfive,sysreg = <&sys_syscon>; >> + }; >> diff --git a/include/dt-bindings/clock/starfive,jh7110-crg.h b/include/dt-bindings/clock/starfive,jh7110-crg.h >> index 5e4f21ca0642..086a6ddcf380 100644 >> --- a/include/dt-bindings/clock/starfive,jh7110-crg.h >> +++ b/include/dt-bindings/clock/starfive,jh7110-crg.h >> @@ -6,6 +6,12 @@ >> #ifndef __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__ >> #define __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__ >> >> +/* PLL clocks */ >> +#define JH7110_CLK_PLL0_OUT 0 >> +#define JH7110_CLK_PLL1_OUT 1 >> +#define JH7110_CLK_PLL2_OUT 2 >> +#define JH7110_PLLCLK_END 3 >> + >> /* SYSCRG clocks */ >> #define JH7110_SYSCLK_CPU_ROOT 0 >> #define JH7110_SYSCLK_CPU_CORE 1 >> @@ -198,11 +204,7 @@ >> #define JH7110_SYSCLK_TDM_TDM_INV 188 >> #define JH7110_SYSCLK_JTAG_CERTIFICATION_TRNG 189 >> >> -#define JH7110_SYSCLK_PLL0_OUT 190 >> -#define JH7110_SYSCLK_PLL1_OUT 191 >> -#define JH7110_SYSCLK_PLL2_OUT 192 >> - > > > NAK. ABI break. You have entire commit msg to explain this and avoid > questions from reviewers. > Discussed with Hal.Feng to delete this part and move it to driver in his next patch series. Best regards, Xingyu Wu
diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml new file mode 100644 index 000000000000..8fd18e6c2e9b --- /dev/null +++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml @@ -0,0 +1,45 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/starfive,jh7110-pll.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive JH7110 PLL Clock Generator + +maintainers: + - Xingyu Wu <xingyu.wu@starfivetech.com> + +properties: + compatible: + const: starfive,jh7110-pll + + clocks: + maxItems: 1 + - description: Main Oscillator (24 MHz) + + '#clock-cells': + const: 1 + description: + See <dt-bindings/clock/starfive,jh7110-crg.h> for valid indices. + + starfive,sysreg: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: + the phandle to System Register Controller syscon node. + +required: + - compatible + - clocks + - '#clock-cells' + - starfive,sysreg + +additionalProperties: false + +examples: + - | + pllclk: pll-clock-controller { + compatible = "starfive,jh7110-pll"; + clocks = <&osc>; + #clock-cells = <1>; + starfive,sysreg = <&sys_syscon>; + }; diff --git a/include/dt-bindings/clock/starfive,jh7110-crg.h b/include/dt-bindings/clock/starfive,jh7110-crg.h index 5e4f21ca0642..086a6ddcf380 100644 --- a/include/dt-bindings/clock/starfive,jh7110-crg.h +++ b/include/dt-bindings/clock/starfive,jh7110-crg.h @@ -6,6 +6,12 @@ #ifndef __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__ #define __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__ +/* PLL clocks */ +#define JH7110_CLK_PLL0_OUT 0 +#define JH7110_CLK_PLL1_OUT 1 +#define JH7110_CLK_PLL2_OUT 2 +#define JH7110_PLLCLK_END 3 + /* SYSCRG clocks */ #define JH7110_SYSCLK_CPU_ROOT 0 #define JH7110_SYSCLK_CPU_CORE 1 @@ -198,11 +204,7 @@ #define JH7110_SYSCLK_TDM_TDM_INV 188 #define JH7110_SYSCLK_JTAG_CERTIFICATION_TRNG 189 -#define JH7110_SYSCLK_PLL0_OUT 190 -#define JH7110_SYSCLK_PLL1_OUT 191 -#define JH7110_SYSCLK_PLL2_OUT 192 - -#define JH7110_SYSCLK_END 193 +#define JH7110_SYSCLK_END 190 /* AONCRG clocks */ #define JH7110_AONCLK_OSC_DIV4 0
Add bindings for the PLL clock generator on the JH7110 RISC-V SoC. Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> --- .../bindings/clock/starfive,jh7110-pll.yaml | 45 +++++++++++++++++++ .../dt-bindings/clock/starfive,jh7110-crg.h | 12 ++--- 2 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml