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@xxxxxxxxxxxxxxxx> > --- > .../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@xxxxxxxxxxxxxxxx> > + > +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