diff mbox series

[v2,01/11] dt-bindings: clock: Add StarFive JH7110 System-Top-Group clock and reset generator

Message ID 20230221083323.302471-2-xingyu.wu@starfivetech.com
State New
Headers
Series Add new partial clock and reset drivers for StarFive JH7110 |

Commit Message

Xingyu Wu Feb. 21, 2023, 8:33 a.m. UTC 
  Add bindings for the System-Top-Group clock and reset generator (STGCRG)
on the JH7110 RISC-V SoC by StarFive Ltd.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 .../clock/starfive,jh7110-stgcrg.yaml         | 82 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 .../dt-bindings/clock/starfive,jh7110-crg.h   | 34 ++++++++
 .../dt-bindings/reset/starfive,jh7110-crg.h   | 28 +++++++
 4 files changed, 145 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml

Comments

Krzysztof Kozlowski Feb. 21, 2023, 11:25 a.m. UTC | #1 
On 21/02/2023 09:33, Xingyu Wu wrote:
> Add bindings for the System-Top-Group clock and reset generator (STGCRG)
> on the JH7110 RISC-V SoC by StarFive Ltd.
> 
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>


> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 93eb504c3b21..2e70c9f21989 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19914,6 +19914,7 @@ F:	arch/riscv/boot/dts/starfive/
>  STARFIVE JH71X0 CLOCK DRIVERS
>  M:	Emil Renner Berthing <kernel@esmil.dk>
>  M:	Hal Feng <hal.feng@starfivetech.com>
> +M:	Xingyu Wu <xingyu.wu@starfivetech.com>

No improvements here. You add here new bindings for one device and then
- without explanation - add yourself to all Starfive clock bindings.
Either explain it or drop it or move it to separate patch.

You already got comment for this.

Best regards,
Krzysztof
Xingyu Wu Feb. 21, 2023, 1:01 p.m. UTC | #2 
On 2023/2/21 19:25, Krzysztof Kozlowski wrote:
> On 21/02/2023 09:33, Xingyu Wu wrote:
>> Add bindings for the System-Top-Group clock and reset generator (STGCRG)
>> on the JH7110 RISC-V SoC by StarFive Ltd.
>> 
>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> 
> 
>> +    };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 93eb504c3b21..2e70c9f21989 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19914,6 +19914,7 @@ F:	arch/riscv/boot/dts/starfive/
>>  STARFIVE JH71X0 CLOCK DRIVERS
>>  M:	Emil Renner Berthing <kernel@esmil.dk>
>>  M:	Hal Feng <hal.feng@starfivetech.com>
>> +M:	Xingyu Wu <xingyu.wu@starfivetech.com>
> 
> No improvements here. You add here new bindings for one device and then
> - without explanation - add yourself to all Starfive clock bindings.
> Either explain it or drop it or move it to separate patch.
> 
> You already got comment for this.
> 

Sorry, I didn't understand what you meant before. Now my understanding is that, 
If I improvements JH71X0 driver no JH7110 driver, I could add this here. Right?

Is it OK if I do it this way to move it to separate patch like this?:
+STARFIVE JH7110 STG CLOCK DRIVERS
+M:	Xingyu Wu <xingyu.wu@starfivetech.com>


Best Regards,
Xingyu Wu
Krzysztof Kozlowski Feb. 21, 2023, 1:37 p.m. UTC | #3 
On 21/02/2023 14:01, Xingyu Wu wrote:
> On 2023/2/21 19:25, Krzysztof Kozlowski wrote:
>> On 21/02/2023 09:33, Xingyu Wu wrote:
>>> Add bindings for the System-Top-Group clock and reset generator (STGCRG)
>>> on the JH7110 RISC-V SoC by StarFive Ltd.
>>>
>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>
>>
>>> +    };
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 93eb504c3b21..2e70c9f21989 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -19914,6 +19914,7 @@ F:	arch/riscv/boot/dts/starfive/
>>>  STARFIVE JH71X0 CLOCK DRIVERS
>>>  M:	Emil Renner Berthing <kernel@esmil.dk>
>>>  M:	Hal Feng <hal.feng@starfivetech.com>
>>> +M:	Xingyu Wu <xingyu.wu@starfivetech.com>
>>
>> No improvements here. You add here new bindings for one device and then
>> - without explanation - add yourself to all Starfive clock bindings.
>> Either explain it or drop it or move it to separate patch.
>>
>> You already got comment for this.
>>
> 
> Sorry, I didn't understand what you meant before. Now my understanding is that, 
> If I improvements JH71X0 driver no JH7110 driver, I could add this here. Right?
> 
> Is it OK if I do it this way to move it to separate patch like this?:
> +STARFIVE JH7110 STG CLOCK DRIVERS
> +M:	Xingyu Wu <xingyu.wu@starfivetech.com>

If you want to be the maintainer of all drivers, add separate commit for
this, so this is obvious. Or at least explain this change in commit msg.

Best regards,
Krzysztof
Xingyu Wu Feb. 22, 2023, 1:48 a.m. UTC | #4 
On 2023/2/21 21:37, Krzysztof Kozlowski wrote:
> On 21/02/2023 14:01, Xingyu Wu wrote:
>> On 2023/2/21 19:25, Krzysztof Kozlowski wrote:
>>> On 21/02/2023 09:33, Xingyu Wu wrote:
>>>> Add bindings for the System-Top-Group clock and reset generator (STGCRG)
>>>> on the JH7110 RISC-V SoC by StarFive Ltd.
>>>>
>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>
>>>
>>>> +    };
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 93eb504c3b21..2e70c9f21989 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -19914,6 +19914,7 @@ F:	arch/riscv/boot/dts/starfive/
>>>>  STARFIVE JH71X0 CLOCK DRIVERS
>>>>  M:	Emil Renner Berthing <kernel@esmil.dk>
>>>>  M:	Hal Feng <hal.feng@starfivetech.com>
>>>> +M:	Xingyu Wu <xingyu.wu@starfivetech.com>
>>>
>>> No improvements here. You add here new bindings for one device and then
>>> - without explanation - add yourself to all Starfive clock bindings.
>>> Either explain it or drop it or move it to separate patch.
>>>
>>> You already got comment for this.
>>>
>> 
>> Sorry, I didn't understand what you meant before. Now my understanding is that, 
>> If I improvements JH71X0 driver no JH7110 driver, I could add this here. Right?
>> 
>> Is it OK if I do it this way to move it to separate patch like this?:
>> +STARFIVE JH7110 STG CLOCK DRIVERS
>> +M:	Xingyu Wu <xingyu.wu@starfivetech.com>
> 
> If you want to be the maintainer of all drivers, add separate commit for
> this, so this is obvious. Or at least explain this change in commit msg.
> 

OK, got it. Thanks.

Best regard,
Xingyu Wu
Emil Renner Berthing March 2, 2023, 3:05 p.m. UTC | #5 
On Tue, 21 Feb 2023 at 09:37, Xingyu Wu <xingyu.wu@starfivetech.com> wrote:
> Add bindings for the System-Top-Group clock and reset generator (STGCRG)
> on the JH7110 RISC-V SoC by StarFive Ltd.
>
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> ---
>  .../clock/starfive,jh7110-stgcrg.yaml         | 82 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  .../dt-bindings/clock/starfive,jh7110-crg.h   | 34 ++++++++
>  .../dt-bindings/reset/starfive,jh7110-crg.h   | 28 +++++++
>  4 files changed, 145 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml
>
> diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml
> new file mode 100644
> index 000000000000..b64ccd84200a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/starfive,jh7110-stgcrg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 System-Top-Group Clock and Reset Generator
> +
> +maintainers:
> +  - Xingyu Wu <xingyu.wu@starfivetech.com>
> +
> +properties:
> +  compatible:
> +    const: starfive,jh7110-stgcrg
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Main Oscillator (24 MHz)
> +      - description: HIFI4 core
> +      - description: STG AXI/AHB
> +      - description: USB (125 MHz)
> +      - description: CPU Bus
> +      - description: HIFI4 Axi
> +      - description: NOC STG Bus
> +      - description: APB Bus
> +
> +  clock-names:
> +    items:
> +      - const: osc
> +      - const: hifi4_core
> +      - const: stg_axiahb
> +      - const: usb_125m
> +      - const: cpu_bus
> +      - const: hifi4_axi
> +      - const: nocstg_bus
> +      - const: apb_bus
> +
> +  '#clock-cells':
> +    const: 1
> +    description:
> +      See <dt-bindings/clock/starfive,jh7110-crg.h> for valid indices.
> +
> +  '#reset-cells':
> +    const: 1
> +    description:
> +      See <dt-bindings/reset/starfive,jh7110-crg.h> for valid indices.
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/starfive,jh7110-crg.h>
> +
> +    stgcrg: clock-controller@10230000 {
> +        compatible = "starfive,jh7110-stgcrg";
> +        reg = <0x10230000 0x10000>;
> +        clocks = <&osc>,
> +                 <&syscrg JH7110_SYSCLK_HIFI4_CORE>,
> +                 <&syscrg JH7110_SYSCLK_STG_AXIAHB>,
> +                 <&syscrg JH7110_SYSCLK_USB_125M>,
> +                 <&syscrg JH7110_SYSCLK_CPU_BUS>,
> +                 <&syscrg JH7110_SYSCLK_HIFI4_AXI>,
> +                 <&syscrg JH7110_SYSCLK_NOCSTG_BUS>,
> +                 <&syscrg JH7110_SYSCLK_APB_BUS>;
> +        clock-names = "osc", "hifi4_core",
> +                      "stg_axiahb", "usb_125m",
> +                      "cpu_bus", "hifi4_axi",
> +                      "nocstg_bus", "apb_bus";
> +        #clock-cells = <1>;
> +        #reset-cells = <1>;
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 93eb504c3b21..2e70c9f21989 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19914,6 +19914,7 @@ F:      arch/riscv/boot/dts/starfive/
>  STARFIVE JH71X0 CLOCK DRIVERS
>  M:     Emil Renner Berthing <kernel@esmil.dk>
>  M:     Hal Feng <hal.feng@starfivetech.com>
> +M:     Xingyu Wu <xingyu.wu@starfivetech.com>
>  S:     Maintained
>  F:     Documentation/devicetree/bindings/clock/starfive,jh71*.yaml
>  F:     drivers/clk/starfive/clk-starfive-jh71*
> diff --git a/include/dt-bindings/clock/starfive,jh7110-crg.h b/include/dt-bindings/clock/starfive,jh7110-crg.h
> index 5e4f21ca0642..5ac8a4d90a7a 100644
> --- a/include/dt-bindings/clock/starfive,jh7110-crg.h
> +++ b/include/dt-bindings/clock/starfive,jh7110-crg.h
> @@ -1,6 +1,7 @@
>  /* SPDX-License-Identifier: GPL-2.0 OR MIT */
>  /*
>   * Copyright 2022 Emil Renner Berthing <kernel@esmil.dk>
> + * Copyright 2022 StarFive Technology Co., Ltd.
>   */
>
>  #ifndef __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__
> @@ -222,4 +223,37 @@
>
>  #define JH7110_AONCLK_END                      14

Hi Xingyu,

The clock and reset names below have been shortened from the very long
names in the documentation. I see you've come to the same shortened
names as I used in the first STGCRG driver I pushed, which is great,
but I find it highly unlikely to have happened without looking at /
copying my code like you did for the SYSCRG and AONCRG drivers Hal has
posted. Unfortunately the commit message above doesn't reflect that,
so please add a
Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>

I do have some updated suggestions for short names below though:

> +/* STGCRG clocks */
> +#define JH7110_STGCLK_HIFI4_CLK_CORE           0
> +#define JH7110_STGCLK_USB0_APB                 1
> +#define JH7110_STGCLK_USB0_UTMI_APB            2 unli
> +#define JH7110_STGCLK_USB0_AXI                 3
> +#define JH7110_STGCLK_USB0_LPM                 4
> +#define JH7110_STGCLK_USB0_STB                 5
> +#define JH7110_STGCLK_USB0_APP_125             6
> +#define JH7110_STGCLK_USB0_REFCLK              7
> +#define JH7110_STGCLK_PCIE0_AXI_MST0           8
> +#define JH7110_STGCLK_PCIE0_APB                        9
> +#define JH7110_STGCLK_PCIE0_TL                 10
> +#define JH7110_STGCLK_PCIE1_AXI_MST0           11
> +#define JH7110_STGCLK_PCIE1_APB                        12
> +#define JH7110_STGCLK_PCIE1_TL                 13
> +#define JH7110_STGCLK_PCIE01_SLV_DEC_MAINCLK   14

Does PCIE01 here mean that the clock is used by both pcie0 and pcie1?
If so then maybe just call it JH7110_PCIE_SLV_MAIN

> +#define JH7110_STGCLK_SEC_HCLK                 15

For other clocks I think "hclk" means ahb clock, so maybe JH7110_STGCLK_SEC_AHB

> +#define JH7110_STGCLK_SEC_MISCAHB              16

I find something like JH7110_STGCLK_SEC_MISC_AHB a little easier to read.

> +#define JH7110_STGCLK_GRP0_MAIN                        17
> +#define JH7110_STGCLK_GRP0_BUS                 18
> +#define JH7110_STGCLK_GRP0_STG                 19
> +#define JH7110_STGCLK_GRP1_MAIN                        20
> +#define JH7110_STGCLK_GRP1_BUS                 21
> +#define JH7110_STGCLK_GRP1_STG                 22
> +#define JH7110_STGCLK_GRP1_HIFI                        23
> +#define JH7110_STGCLK_E2_RTC                   24
> +#define JH7110_STGCLK_E2_CORE                  25
> +#define JH7110_STGCLK_E2_DBG                   26
> +#define JH7110_STGCLK_DMA1P_AXI                        27
> +#define JH7110_STGCLK_DMA1P_AHB                        28
> +
> +#define JH7110_STGCLK_END                      29
> +
>  #endif /* __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__ */
> diff --git a/include/dt-bindings/reset/starfive,jh7110-crg.h b/include/dt-bindings/reset/starfive,jh7110-crg.h
> index d78e38690ceb..4a865ded78b8 100644
> --- a/include/dt-bindings/reset/starfive,jh7110-crg.h
> +++ b/include/dt-bindings/reset/starfive,jh7110-crg.h
> @@ -1,6 +1,7 @@
>  /* SPDX-License-Identifier: GPL-2.0 OR MIT */
>  /*
>   * Copyright (C) 2022 Emil Renner Berthing <kernel@esmil.dk>
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>   */
>
>  #ifndef __DT_BINDINGS_RESET_STARFIVE_JH7110_CRG_H__
> @@ -151,4 +152,31 @@
>
>  #define JH7110_AONRST_END                      8
>
> +/* STGCRG resets */
> +#define JH7110_STGRST_SYSCON                   0
> +#define JH7110_STGRST_HIFI4_CORE               1
> +#define JH7110_STGRST_HIFI4_AXI                        2
> +#define JH7110_STGRST_SEC_TOP_HRESETN          3

JH7110_STGRST_SEC_AHB to match the clock above.

> +#define JH7110_STGRST_E24_CORE                 4
> +#define JH7110_STGRST_DMA1P_AXI                        5
> +#define JH7110_STGRST_DMA1P_AHB                        6
> +#define JH7110_STGRST_USB0_AXI                 7
> +#define JH7110_STGRST_USB0_APB                 8
> +#define JH7110_STGRST_USB0_UTMI_APB            9
> +#define JH7110_STGRST_USB0_PWRUP               10
> +#define JH7110_STGRST_PCIE0_AXI_MST0           11
> +#define JH7110_STGRST_PCIE0_AXI_SLV0           12
> +#define JH7110_STGRST_PCIE0_AXI_SLV            13
> +#define JH7110_STGRST_PCIE0_BRG                        14
> +#define JH7110_STGRST_PCIE0_CORE               15
> +#define JH7110_STGRST_PCIE0_APB                        16
> +#define JH7110_STGRST_PCIE1_AXI_MST0           17
> +#define JH7110_STGRST_PCIE1_AXI_SLV0           18
> +#define JH7110_STGRST_PCIE1_AXI_SLV            19
> +#define JH7110_STGRST_PCIE1_BRG                        20
> +#define JH7110_STGRST_PCIE1_CORE               21
> +#define JH7110_STGRST_PCIE1_APB                        22
> +
> +#define JH7110_STGRST_END                      23
> +
>  #endif /* __DT_BINDINGS_RESET_STARFIVE_JH7110_CRG_H__ */
> --
> 2.25.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Xingyu Wu March 3, 2023, 3:14 a.m. UTC | #6 
On 2023/3/2 23:05, Emil Renner Berthing wrote:
> On Tue, 21 Feb 2023 at 09:37, Xingyu Wu <xingyu.wu@starfivetech.com> wrote:
>> Add bindings for the System-Top-Group clock and reset generator (STGCRG)
>> on the JH7110 RISC-V SoC by StarFive Ltd.
>>
>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>> ---
>>  .../clock/starfive,jh7110-stgcrg.yaml         | 82 +++++++++++++++++++
>>  MAINTAINERS                                   |  1 +
>>  .../dt-bindings/clock/starfive,jh7110-crg.h   | 34 ++++++++
>>  .../dt-bindings/reset/starfive,jh7110-crg.h   | 28 +++++++
>>  4 files changed, 145 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml
>> new file mode 100644
>> index 000000000000..b64ccd84200a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml
>> @@ -0,0 +1,82 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/starfive,jh7110-stgcrg.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive JH7110 System-Top-Group Clock and Reset Generator
>> +
>> +maintainers:
>> +  - Xingyu Wu <xingyu.wu@starfivetech.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: starfive,jh7110-stgcrg
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: Main Oscillator (24 MHz)
>> +      - description: HIFI4 core
>> +      - description: STG AXI/AHB
>> +      - description: USB (125 MHz)
>> +      - description: CPU Bus
>> +      - description: HIFI4 Axi
>> +      - description: NOC STG Bus
>> +      - description: APB Bus
>> +
>> +  clock-names:
>> +    items:
>> +      - const: osc
>> +      - const: hifi4_core
>> +      - const: stg_axiahb
>> +      - const: usb_125m
>> +      - const: cpu_bus
>> +      - const: hifi4_axi
>> +      - const: nocstg_bus
>> +      - const: apb_bus
>> +
>> +  '#clock-cells':
>> +    const: 1
>> +    description:
>> +      See <dt-bindings/clock/starfive,jh7110-crg.h> for valid indices.
>> +
>> +  '#reset-cells':
>> +    const: 1
>> +    description:
>> +      See <dt-bindings/reset/starfive,jh7110-crg.h> for valid indices.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - '#clock-cells'
>> +  - '#reset-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/starfive,jh7110-crg.h>
>> +
>> +    stgcrg: clock-controller@10230000 {
>> +        compatible = "starfive,jh7110-stgcrg";
>> +        reg = <0x10230000 0x10000>;
>> +        clocks = <&osc>,
>> +                 <&syscrg JH7110_SYSCLK_HIFI4_CORE>,
>> +                 <&syscrg JH7110_SYSCLK_STG_AXIAHB>,
>> +                 <&syscrg JH7110_SYSCLK_USB_125M>,
>> +                 <&syscrg JH7110_SYSCLK_CPU_BUS>,
>> +                 <&syscrg JH7110_SYSCLK_HIFI4_AXI>,
>> +                 <&syscrg JH7110_SYSCLK_NOCSTG_BUS>,
>> +                 <&syscrg JH7110_SYSCLK_APB_BUS>;
>> +        clock-names = "osc", "hifi4_core",
>> +                      "stg_axiahb", "usb_125m",
>> +                      "cpu_bus", "hifi4_axi",
>> +                      "nocstg_bus", "apb_bus";
>> +        #clock-cells = <1>;
>> +        #reset-cells = <1>;
>> +    };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 93eb504c3b21..2e70c9f21989 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19914,6 +19914,7 @@ F:      arch/riscv/boot/dts/starfive/
>>  STARFIVE JH71X0 CLOCK DRIVERS
>>  M:     Emil Renner Berthing <kernel@esmil.dk>
>>  M:     Hal Feng <hal.feng@starfivetech.com>
>> +M:     Xingyu Wu <xingyu.wu@starfivetech.com>
>>  S:     Maintained
>>  F:     Documentation/devicetree/bindings/clock/starfive,jh71*.yaml
>>  F:     drivers/clk/starfive/clk-starfive-jh71*
>> diff --git a/include/dt-bindings/clock/starfive,jh7110-crg.h b/include/dt-bindings/clock/starfive,jh7110-crg.h
>> index 5e4f21ca0642..5ac8a4d90a7a 100644
>> --- a/include/dt-bindings/clock/starfive,jh7110-crg.h
>> +++ b/include/dt-bindings/clock/starfive,jh7110-crg.h
>> @@ -1,6 +1,7 @@
>>  /* SPDX-License-Identifier: GPL-2.0 OR MIT */
>>  /*
>>   * Copyright 2022 Emil Renner Berthing <kernel@esmil.dk>
>> + * Copyright 2022 StarFive Technology Co., Ltd.
>>   */
>>
>>  #ifndef __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__
>> @@ -222,4 +223,37 @@
>>
>>  #define JH7110_AONCLK_END                      14
> 
> Hi Xingyu,
> 
> The clock and reset names below have been shortened from the very long
> names in the documentation. I see you've come to the same shortened
> names as I used in the first STGCRG driver I pushed, which is great,
> but I find it highly unlikely to have happened without looking at /
> copying my code like you did for the SYSCRG and AONCRG drivers Hal has
> posted. Unfortunately the commit message above doesn't reflect that,
> so please add a
> Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>

Thanks. But these STG/ISP/VOUT drivers are transplanted from 7110-SDK wrote
by myself one year ago and followed 7100 clock drivers framework and structure
at that time. And I haven't seen your STGCRG driver before. I improved these
to follow these SYSCRG and AONCRG drivers' framework. So you could look like copying
your code but these are still little different like these clock and reset name.

But I don't know if I am right. I follow your code framework to write it and
I should add 'Co-developed-by' and 'Signed-off-by' to reflect that. It that right?

> 
> I do have some updated suggestions for short names below though:
> 
>> +/* STGCRG clocks */
>> +#define JH7110_STGCLK_HIFI4_CLK_CORE           0
>> +#define JH7110_STGCLK_USB0_APB                 1
>> +#define JH7110_STGCLK_USB0_UTMI_APB            2 unli
>> +#define JH7110_STGCLK_USB0_AXI                 3
>> +#define JH7110_STGCLK_USB0_LPM                 4
>> +#define JH7110_STGCLK_USB0_STB                 5
>> +#define JH7110_STGCLK_USB0_APP_125             6
>> +#define JH7110_STGCLK_USB0_REFCLK              7
>> +#define JH7110_STGCLK_PCIE0_AXI_MST0           8
>> +#define JH7110_STGCLK_PCIE0_APB                        9
>> +#define JH7110_STGCLK_PCIE0_TL                 10
>> +#define JH7110_STGCLK_PCIE1_AXI_MST0           11
>> +#define JH7110_STGCLK_PCIE1_APB                        12
>> +#define JH7110_STGCLK_PCIE1_TL                 13
>> +#define JH7110_STGCLK_PCIE01_SLV_DEC_MAINCLK   14
> 
> Does PCIE01 here mean that the clock is used by both pcie0 and pcie1?
> If so then maybe just call it JH7110_PCIE_SLV_MAIN

Yes, it is used by both pcie0 and pcie1. Will modify it.

> 
>> +#define JH7110_STGCLK_SEC_HCLK                 15
> 
> For other clocks I think "hclk" means ahb clock, so maybe JH7110_STGCLK_SEC_AHB

Will modify it.

> 
>> +#define JH7110_STGCLK_SEC_MISCAHB              16
> 
> I find something like JH7110_STGCLK_SEC_MISC_AHB a little easier to read.

Will modify it.

> 
>> +#define JH7110_STGCLK_GRP0_MAIN                        17
>> +#define JH7110_STGCLK_GRP0_BUS                 18
>> +#define JH7110_STGCLK_GRP0_STG                 19
>> +#define JH7110_STGCLK_GRP1_MAIN                        20
>> +#define JH7110_STGCLK_GRP1_BUS                 21
>> +#define JH7110_STGCLK_GRP1_STG                 22
>> +#define JH7110_STGCLK_GRP1_HIFI                        23
>> +#define JH7110_STGCLK_E2_RTC                   24
>> +#define JH7110_STGCLK_E2_CORE                  25
>> +#define JH7110_STGCLK_E2_DBG                   26
>> +#define JH7110_STGCLK_DMA1P_AXI                        27
>> +#define JH7110_STGCLK_DMA1P_AHB                        28
>> +
>> +#define JH7110_STGCLK_END                      29
>> +
>>  #endif /* __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__ */
>> diff --git a/include/dt-bindings/reset/starfive,jh7110-crg.h b/include/dt-bindings/reset/starfive,jh7110-crg.h
>> index d78e38690ceb..4a865ded78b8 100644
>> --- a/include/dt-bindings/reset/starfive,jh7110-crg.h
>> +++ b/include/dt-bindings/reset/starfive,jh7110-crg.h
>> @@ -1,6 +1,7 @@
>>  /* SPDX-License-Identifier: GPL-2.0 OR MIT */
>>  /*
>>   * Copyright (C) 2022 Emil Renner Berthing <kernel@esmil.dk>
>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>>   */
>>
>>  #ifndef __DT_BINDINGS_RESET_STARFIVE_JH7110_CRG_H__
>> @@ -151,4 +152,31 @@
>>
>>  #define JH7110_AONRST_END                      8
>>
>> +/* STGCRG resets */
>> +#define JH7110_STGRST_SYSCON                   0
>> +#define JH7110_STGRST_HIFI4_CORE               1
>> +#define JH7110_STGRST_HIFI4_AXI                        2
>> +#define JH7110_STGRST_SEC_TOP_HRESETN          3
> 
> JH7110_STGRST_SEC_AHB to match the clock above.

Will modify it.

> 
>> +#define JH7110_STGRST_E24_CORE                 4
>> +#define JH7110_STGRST_DMA1P_AXI                        5
>> +#define JH7110_STGRST_DMA1P_AHB                        6
>> +#define JH7110_STGRST_USB0_AXI                 7
>> +#define JH7110_STGRST_USB0_APB                 8
>> +#define JH7110_STGRST_USB0_UTMI_APB            9
>> +#define JH7110_STGRST_USB0_PWRUP               10
>> +#define JH7110_STGRST_PCIE0_AXI_MST0           11
>> +#define JH7110_STGRST_PCIE0_AXI_SLV0           12
>> +#define JH7110_STGRST_PCIE0_AXI_SLV            13
>> +#define JH7110_STGRST_PCIE0_BRG                        14
>> +#define JH7110_STGRST_PCIE0_CORE               15
>> +#define JH7110_STGRST_PCIE0_APB                        16
>> +#define JH7110_STGRST_PCIE1_AXI_MST0           17
>> +#define JH7110_STGRST_PCIE1_AXI_SLV0           18
>> +#define JH7110_STGRST_PCIE1_AXI_SLV            19
>> +#define JH7110_STGRST_PCIE1_BRG                        20
>> +#define JH7110_STGRST_PCIE1_CORE               21
>> +#define JH7110_STGRST_PCIE1_APB                        22
>> +
>> +#define JH7110_STGRST_END                      23
>> +
>>  #endif /* __DT_BINDINGS_RESET_STARFIVE_JH7110_CRG_H__ */
>> --
>> 2.25.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Best regards
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml
new file mode 100644
index 000000000000..b64ccd84200a
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml
@@ -0,0 +1,82 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/starfive,jh7110-stgcrg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 System-Top-Group Clock and Reset Generator
+
+maintainers:
+  - Xingyu Wu <xingyu.wu@starfivetech.com>
+
+properties:
+  compatible:
+    const: starfive,jh7110-stgcrg
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Main Oscillator (24 MHz)
+      - description: HIFI4 core
+      - description: STG AXI/AHB
+      - description: USB (125 MHz)
+      - description: CPU Bus
+      - description: HIFI4 Axi
+      - description: NOC STG Bus
+      - description: APB Bus
+
+  clock-names:
+    items:
+      - const: osc
+      - const: hifi4_core
+      - const: stg_axiahb
+      - const: usb_125m
+      - const: cpu_bus
+      - const: hifi4_axi
+      - const: nocstg_bus
+      - const: apb_bus
+
+  '#clock-cells':
+    const: 1
+    description:
+      See <dt-bindings/clock/starfive,jh7110-crg.h> for valid indices.
+
+  '#reset-cells':
+    const: 1
+    description:
+      See <dt-bindings/reset/starfive,jh7110-crg.h> for valid indices.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+  - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/starfive,jh7110-crg.h>
+
+    stgcrg: clock-controller@10230000 {
+        compatible = "starfive,jh7110-stgcrg";
+        reg = <0x10230000 0x10000>;
+        clocks = <&osc>,
+                 <&syscrg JH7110_SYSCLK_HIFI4_CORE>,
+                 <&syscrg JH7110_SYSCLK_STG_AXIAHB>,
+                 <&syscrg JH7110_SYSCLK_USB_125M>,
+                 <&syscrg JH7110_SYSCLK_CPU_BUS>,
+                 <&syscrg JH7110_SYSCLK_HIFI4_AXI>,
+                 <&syscrg JH7110_SYSCLK_NOCSTG_BUS>,
+                 <&syscrg JH7110_SYSCLK_APB_BUS>;
+        clock-names = "osc", "hifi4_core",
+                      "stg_axiahb", "usb_125m",
+                      "cpu_bus", "hifi4_axi",
+                      "nocstg_bus", "apb_bus";
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 93eb504c3b21..2e70c9f21989 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19914,6 +19914,7 @@  F:	arch/riscv/boot/dts/starfive/
 STARFIVE JH71X0 CLOCK DRIVERS
 M:	Emil Renner Berthing <kernel@esmil.dk>
 M:	Hal Feng <hal.feng@starfivetech.com>
+M:	Xingyu Wu <xingyu.wu@starfivetech.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/clock/starfive,jh71*.yaml
 F:	drivers/clk/starfive/clk-starfive-jh71*
diff --git a/include/dt-bindings/clock/starfive,jh7110-crg.h b/include/dt-bindings/clock/starfive,jh7110-crg.h
index 5e4f21ca0642..5ac8a4d90a7a 100644
--- a/include/dt-bindings/clock/starfive,jh7110-crg.h
+++ b/include/dt-bindings/clock/starfive,jh7110-crg.h
@@ -1,6 +1,7 @@ 
 /* SPDX-License-Identifier: GPL-2.0 OR MIT */
 /*
  * Copyright 2022 Emil Renner Berthing <kernel@esmil.dk>
+ * Copyright 2022 StarFive Technology Co., Ltd.
  */
 
 #ifndef __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__
@@ -222,4 +223,37 @@ 
 
 #define JH7110_AONCLK_END			14
 
+/* STGCRG clocks */
+#define JH7110_STGCLK_HIFI4_CLK_CORE		0
+#define JH7110_STGCLK_USB0_APB			1
+#define JH7110_STGCLK_USB0_UTMI_APB		2
+#define JH7110_STGCLK_USB0_AXI			3
+#define JH7110_STGCLK_USB0_LPM			4
+#define JH7110_STGCLK_USB0_STB			5
+#define JH7110_STGCLK_USB0_APP_125		6
+#define JH7110_STGCLK_USB0_REFCLK		7
+#define JH7110_STGCLK_PCIE0_AXI_MST0		8
+#define JH7110_STGCLK_PCIE0_APB			9
+#define JH7110_STGCLK_PCIE0_TL			10
+#define JH7110_STGCLK_PCIE1_AXI_MST0		11
+#define JH7110_STGCLK_PCIE1_APB			12
+#define JH7110_STGCLK_PCIE1_TL			13
+#define JH7110_STGCLK_PCIE01_SLV_DEC_MAINCLK	14
+#define JH7110_STGCLK_SEC_HCLK			15
+#define JH7110_STGCLK_SEC_MISCAHB		16
+#define JH7110_STGCLK_GRP0_MAIN			17
+#define JH7110_STGCLK_GRP0_BUS			18
+#define JH7110_STGCLK_GRP0_STG			19
+#define JH7110_STGCLK_GRP1_MAIN			20
+#define JH7110_STGCLK_GRP1_BUS			21
+#define JH7110_STGCLK_GRP1_STG			22
+#define JH7110_STGCLK_GRP1_HIFI			23
+#define JH7110_STGCLK_E2_RTC			24
+#define JH7110_STGCLK_E2_CORE			25
+#define JH7110_STGCLK_E2_DBG			26
+#define JH7110_STGCLK_DMA1P_AXI			27
+#define JH7110_STGCLK_DMA1P_AHB			28
+
+#define JH7110_STGCLK_END			29
+
 #endif /* __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__ */
diff --git a/include/dt-bindings/reset/starfive,jh7110-crg.h b/include/dt-bindings/reset/starfive,jh7110-crg.h
index d78e38690ceb..4a865ded78b8 100644
--- a/include/dt-bindings/reset/starfive,jh7110-crg.h
+++ b/include/dt-bindings/reset/starfive,jh7110-crg.h
@@ -1,6 +1,7 @@ 
 /* SPDX-License-Identifier: GPL-2.0 OR MIT */
 /*
  * Copyright (C) 2022 Emil Renner Berthing <kernel@esmil.dk>
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
  */
 
 #ifndef __DT_BINDINGS_RESET_STARFIVE_JH7110_CRG_H__
@@ -151,4 +152,31 @@ 
 
 #define JH7110_AONRST_END			8
 
+/* STGCRG resets */
+#define JH7110_STGRST_SYSCON			0
+#define JH7110_STGRST_HIFI4_CORE		1
+#define JH7110_STGRST_HIFI4_AXI			2
+#define JH7110_STGRST_SEC_TOP_HRESETN		3
+#define JH7110_STGRST_E24_CORE			4
+#define JH7110_STGRST_DMA1P_AXI			5
+#define JH7110_STGRST_DMA1P_AHB			6
+#define JH7110_STGRST_USB0_AXI			7
+#define JH7110_STGRST_USB0_APB			8
+#define JH7110_STGRST_USB0_UTMI_APB		9
+#define JH7110_STGRST_USB0_PWRUP		10
+#define JH7110_STGRST_PCIE0_AXI_MST0		11
+#define JH7110_STGRST_PCIE0_AXI_SLV0		12
+#define JH7110_STGRST_PCIE0_AXI_SLV		13
+#define JH7110_STGRST_PCIE0_BRG			14
+#define JH7110_STGRST_PCIE0_CORE		15
+#define JH7110_STGRST_PCIE0_APB			16
+#define JH7110_STGRST_PCIE1_AXI_MST0		17
+#define JH7110_STGRST_PCIE1_AXI_SLV0		18
+#define JH7110_STGRST_PCIE1_AXI_SLV		19
+#define JH7110_STGRST_PCIE1_BRG			20
+#define JH7110_STGRST_PCIE1_CORE		21
+#define JH7110_STGRST_PCIE1_APB			22
+
+#define JH7110_STGRST_END			23
+
 #endif /* __DT_BINDINGS_RESET_STARFIVE_JH7110_CRG_H__ */