| Message ID | 20221118010627.70576-10-hal.feng@starfivetech.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,01/14] clk: starfive: Factor out common JH7100 and JH7110 code | expand |
On 18/11/2022 02:06, Hal Feng wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > Add bindings for the system clock and reset generator (SYSCRG) on the > JH7110 RISC-V SoC by StarFive Ltd. > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > Signed-off-by: Hal Feng <hal.feng@starfivetech.com> Binding headers are coming with the file bringing bindings for the device, so you need to squash patches. > --- > .../clock/starfive,jh7110-syscrg.yaml | 80 +++++++++++++++++++ > MAINTAINERS | 2 +- > 2 files changed, 81 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > > diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > new file mode 100644 > index 000000000000..a8cafbc0afe2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > @@ -0,0 +1,80 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/starfive,jh7110-syscrg.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: StarFive JH7110 System Clock and Reset Generator > + > +maintainers: > + - Emil Renner Berthing <kernel@esmil.dk> > + > +properties: > + compatible: > + const: starfive,jh7110-syscrg > + > + reg: > + maxItems: 1 > + > + clocks: > + items: > + - description: Main Oscillator (24 MHz) > + - description: RMII reference clock > + - description: RGMII RX clock > + - description: I2S TX bit clock > + - description: I2S TX left/right clock > + - description: I2S RX bit clock > + - description: I2S RX left/right clock > + - description: TDM > + - description: mclk > + > + clock-names: > + items: > + - const: osc > + - const: gmac1_rmii_refin > + - const: gmac1_rgmii_rxin > + - const: i2stx_bclk_ext > + - const: i2stx_lrck_ext > + - const: i2srx_bclk_ext > + - const: i2srx_lrck_ext > + - const: tdm_ext > + - const: mclk_ext > + > + '#clock-cells': > + const: 1 > + description: > + See <dt-bindings/clock/starfive-jh7110.h> for valid indices. Fix filename. > + > + '#reset-cells': > + const: 1 > + description: > + See <dt-bindings/reset/starfive-jh7110.h> for valid indices. Fix filename. > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - '#clock-cells' > + - '#reset-cells' > + Best regards, Krzysztof
On Sat, 19 Nov 2022 00:50:41 +0800, Emil Renner Berthing wrote: > On Fri, 18 Nov 2022 at 02:06, Hal Feng <hal.feng@starfivetech.com> wrote: > > diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > > new file mode 100644 > > index 000000000000..a8cafbc0afe2 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > > @@ -0,0 +1,80 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/clock/starfive,jh7110-syscrg.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: StarFive JH7110 System Clock and Reset Generator > > + > > +maintainers: > > + - Emil Renner Berthing <kernel@esmil.dk> > > + > > +properties: > > + compatible: > > + const: starfive,jh7110-syscrg > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: Main Oscillator (24 MHz) > > + - description: RMII reference clock > > + - description: RGMII RX clock > > + - description: I2S TX bit clock > > + - description: I2S TX left/right clock > > + - description: I2S RX bit clock > > + - description: I2S RX left/right clock > > + - description: TDM > > + - description: mclk > > Maybe you could ask your colleagues for a better description of these clocks. OK, I will improve the description. Best regards, Hal
On Wed, 30 Nov 2022 12:48:30 +0100, Krzysztof Kozlowski wrote: > On 30/11/2022 10:47, Hal Feng wrote: >> On Fri, 25 Nov 2022 14:41:12 +0800, Hal Feng wrote: >>> On Mon, 21 Nov 2022 09:47:08 +0100, Krzysztof Kozlowski wrote: >>>> On 18/11/2022 02:06, Hal Feng wrote: >>>>> From: Emil Renner Berthing <kernel@esmil.dk> >>>>> >>>>> Add bindings for the system clock and reset generator (SYSCRG) on the >>>>> JH7110 RISC-V SoC by StarFive Ltd. >>>>> >>>>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >>>>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com> >>>> >>>> Binding headers are coming with the file bringing bindings for the >>>> device, so you need to squash patches. >>> >>> As we discussed in patch 7, could I merge patch 7, 8, 9, 10 and add the >>> following files in one commit? >>> >>> include/dt-bindings/clock/starfive,jh7110-crg.h >>> include/dt-bindings/reset/starfive,jh7110-crg.h >>> Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >>> Documentation/devicetree/bindings/clock/starfive,jh7110-aoncrg.yaml >> >> Hi, Krzysztof, >> >> Could you please give me some suggestions? > > You can keep aon and sys split. First add one of them with their own > headers. Then add second with their own defines. You mean split patch 7 and patch 8 into sys part and aon part respectively? There are totally five regions (sys/aon/stg/isp/vout) for clocks and resets in JH7110. If we do that, there will be 5 headers for JH7110 in either clock or reset directory finally. Is that OK if there are too many headers for just one SoC? Best regards, Hal
On 30/11/2022 19:05, Hal Feng wrote: > On Wed, 30 Nov 2022 16:19:06 +0100, Krzysztof Kozlowski wrote: >> On 30/11/2022 16:12, Hal Feng wrote: >>> On Wed, 30 Nov 2022 12:48:30 +0100, Krzysztof Kozlowski wrote: >>>> On 30/11/2022 10:47, Hal Feng wrote: >>>>> On Fri, 25 Nov 2022 14:41:12 +0800, Hal Feng wrote: >>>>>> On Mon, 21 Nov 2022 09:47:08 +0100, Krzysztof Kozlowski wrote: >>>>>>> On 18/11/2022 02:06, Hal Feng wrote: >>>>>>>> From: Emil Renner Berthing <kernel@esmil.dk> >>>>>>>> >>>>>>>> Add bindings for the system clock and reset generator (SYSCRG) on the >>>>>>>> JH7110 RISC-V SoC by StarFive Ltd. >>>>>>>> >>>>>>>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >>>>>>>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com> >>>>>>> >>>>>>> Binding headers are coming with the file bringing bindings for the >>>>>>> device, so you need to squash patches. >>>>>> >>>>>> As we discussed in patch 7, could I merge patch 7, 8, 9, 10 and add the >>>>>> following files in one commit? >>>>>> >>>>>> include/dt-bindings/clock/starfive,jh7110-crg.h >>>>>> include/dt-bindings/reset/starfive,jh7110-crg.h >>>>>> Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >>>>>> Documentation/devicetree/bindings/clock/starfive,jh7110-aoncrg.yaml >>>>> >>>>> Hi, Krzysztof, >>>>> >>>>> Could you please give me some suggestions? >>>> >>>> You can keep aon and sys split. First add one of them with their own >>>> headers. Then add second with their own defines. >>> >>> You mean split patch 7 and patch 8 into sys part and aon part >>> respectively? There are totally five regions (sys/aon/stg/isp/vout) >>> for clocks and resets in JH7110. If we do that, there will be 5 >>> headers for JH7110 in either clock or reset directory finally. Is >>> that OK if there are too many headers for just one SoC? >> >> >> Sorry, I lost the track of what patches you have. The comment was - >> bindings include both the doc and headers. You want to split some, some >> merge, sorry, no clue. I did not propose splitting headers... > > It's ok. The problem was that the header > > include/dt-bindings/clock/starfive,jh7110-crg.h > > was used in both > > Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > > and > > Documentation/devicetree/bindings/clock/starfive,jh7110-aoncrg.yaml. > > The same for include/dt-bindings/reset/starfive,jh7110-crg.h. > So should I add these four files in one patch? No. I think I wrote proposed flow of patches: 1. syscrg bindings with header 2. aoncrg bindings with changes to header Why do you need to merge anything? Best regards, Krzysztof
On Thu, 1 Dec 2022 11:21:04 +0100, Krzysztof Kozlowski wrote: > On 30/11/2022 19:05, Hal Feng wrote: >> On Wed, 30 Nov 2022 16:19:06 +0100, Krzysztof Kozlowski wrote: >>> On 30/11/2022 16:12, Hal Feng wrote: >>>> On Wed, 30 Nov 2022 12:48:30 +0100, Krzysztof Kozlowski wrote: >>>>> On 30/11/2022 10:47, Hal Feng wrote: >>>>>> On Fri, 25 Nov 2022 14:41:12 +0800, Hal Feng wrote: >>>>>>> On Mon, 21 Nov 2022 09:47:08 +0100, Krzysztof Kozlowski wrote: >>>>>>>> On 18/11/2022 02:06, Hal Feng wrote: >>>>>>>>> From: Emil Renner Berthing <kernel@esmil.dk> >>>>>>>>> >>>>>>>>> Add bindings for the system clock and reset generator (SYSCRG) on the >>>>>>>>> JH7110 RISC-V SoC by StarFive Ltd. >>>>>>>>> >>>>>>>>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >>>>>>>>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com> >>>>>>>> >>>>>>>> Binding headers are coming with the file bringing bindings for the >>>>>>>> device, so you need to squash patches. >>>>>>> >>>>>>> As we discussed in patch 7, could I merge patch 7, 8, 9, 10 and add the >>>>>>> following files in one commit? >>>>>>> >>>>>>> include/dt-bindings/clock/starfive,jh7110-crg.h >>>>>>> include/dt-bindings/reset/starfive,jh7110-crg.h >>>>>>> Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >>>>>>> Documentation/devicetree/bindings/clock/starfive,jh7110-aoncrg.yaml >>>>>> >>>>>> Hi, Krzysztof, >>>>>> >>>>>> Could you please give me some suggestions? >>>>> >>>>> You can keep aon and sys split. First add one of them with their own >>>>> headers. Then add second with their own defines. >>>> >>>> You mean split patch 7 and patch 8 into sys part and aon part >>>> respectively? There are totally five regions (sys/aon/stg/isp/vout) >>>> for clocks and resets in JH7110. If we do that, there will be 5 >>>> headers for JH7110 in either clock or reset directory finally. Is >>>> that OK if there are too many headers for just one SoC? >>> >>> >>> Sorry, I lost the track of what patches you have. The comment was - >>> bindings include both the doc and headers. You want to split some, some >>> merge, sorry, no clue. I did not propose splitting headers... >> >> It's ok. The problem was that the header >> >> include/dt-bindings/clock/starfive,jh7110-crg.h >> >> was used in both >> >> Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> >> and >> >> Documentation/devicetree/bindings/clock/starfive,jh7110-aoncrg.yaml. >> >> The same for include/dt-bindings/reset/starfive,jh7110-crg.h. >> So should I add these four files in one patch? > > No. I think I wrote proposed flow of patches: > 1. syscrg bindings with header > 2. aoncrg bindings with changes to header Great. Got it. Thanks a lot! Best regards, Hal > > Why do you need to merge anything?
diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml new file mode 100644 index 000000000000..a8cafbc0afe2 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml @@ -0,0 +1,80 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/starfive,jh7110-syscrg.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive JH7110 System Clock and Reset Generator + +maintainers: + - Emil Renner Berthing <kernel@esmil.dk> + +properties: + compatible: + const: starfive,jh7110-syscrg + + reg: + maxItems: 1 + + clocks: + items: + - description: Main Oscillator (24 MHz) + - description: RMII reference clock + - description: RGMII RX clock + - description: I2S TX bit clock + - description: I2S TX left/right clock + - description: I2S RX bit clock + - description: I2S RX left/right clock + - description: TDM + - description: mclk + + clock-names: + items: + - const: osc + - const: gmac1_rmii_refin + - const: gmac1_rgmii_rxin + - const: i2stx_bclk_ext + - const: i2stx_lrck_ext + - const: i2srx_bclk_ext + - const: i2srx_lrck_ext + - const: tdm_ext + - const: mclk_ext + + '#clock-cells': + const: 1 + description: + See <dt-bindings/clock/starfive-jh7110.h> for valid indices. + + '#reset-cells': + const: 1 + description: + See <dt-bindings/reset/starfive-jh7110.h> for valid indices. + +required: + - compatible + - reg + - clocks + - clock-names + - '#clock-cells' + - '#reset-cells' + +additionalProperties: false + +examples: + - | + clock-controller@13020000 { + compatible = "starfive,jh7110-syscrg"; + reg = <0x13020000 0x10000>; + clocks = <&osc>, <&gmac1_rmii_refin>, + <&gmac1_rgmii_rxin>, + <&i2stx_bclk_ext>, <&i2stx_lrck_ext>, + <&i2srx_bclk_ext>, <&i2srx_lrck_ext>, + <&tdm_ext>, <&mclk_ext>; + clock-names = "osc", "gmac1_rmii_refin", + "gmac1_rgmii_rxin", + "i2stx_bclk_ext", "i2stx_lrck_ext", + "i2srx_bclk_ext", "i2srx_lrck_ext", + "tdm_ext", "mclk_ext"; + #clock-cells = <1>; + #reset-cells = <1>; + }; diff --git a/MAINTAINERS b/MAINTAINERS index eeab26f5597c..ec6647e2772f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -19602,7 +19602,7 @@ STARFIVE CLOCK DRIVERS M: Emil Renner Berthing <kernel@esmil.dk> M: Hal Feng <hal.feng@starfivetech.com> S: Maintained -F: Documentation/devicetree/bindings/clock/starfive,jh7100-*.yaml +F: Documentation/devicetree/bindings/clock/starfive* F: drivers/clk/starfive/ F: include/dt-bindings/clock/starfive*