[v2,1/4] dt-bindings: clock: sophgo: Add clock controller of CV1800 series SoC

Message ID IA1PR20MB49535CCEBCC36C864B949CF5BB85A@IA1PR20MB4953.namprd20.prod.outlook.com
State New
Headers
Series riscv: sophgo: add clock support for Sophgo CV1800 SoCs |

Commit Message

Inochi Amaoto Dec. 5, 2023, 11:55 a.m. UTC
  Add definition for the clock controller of the CV1800 series SoC.

Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf
---
 .../bindings/clock/sophgo,cv1800-clk.yaml     |  53 ++++++
 include/dt-bindings/clock/sophgo,cv1800.h     | 174 ++++++++++++++++++
 2 files changed, 227 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
 create mode 100644 include/dt-bindings/clock/sophgo,cv1800.h

--
2.43.0
  

Comments

Conor Dooley Dec. 5, 2023, 4:32 p.m. UTC | #1
On Tue, Dec 05, 2023 at 07:55:50PM +0800, Inochi Amaoto wrote:
> Add definition for the clock controller of the CV1800 series SoC.
> 
> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf
> ---
>  .../bindings/clock/sophgo,cv1800-clk.yaml     |  53 ++++++
>  include/dt-bindings/clock/sophgo,cv1800.h     | 174 ++++++++++++++++++
>  2 files changed, 227 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>  create mode 100644 include/dt-bindings/clock/sophgo,cv1800.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
> new file mode 100644
> index 000000000000..388be5bfa163
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/sophgo,cv1800-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sophgo CV1800 Series Clock Controller
> +
> +maintainers:
> +  - Inochi Amaoto <inochiama@outlook.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - sophgo,cv1800-clk
> +      - sophgo,cv1810-clk

I'm not reading 1000s of lines of driver code to figure it out, what
differs in the programming model for these two devices? You should
mention in your commit message why the cv1810 has an incompatible
programming model if you are adding multiple devices in one commit
message.

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Oscillator (25 MHz)

This could just be "maxItems: 1".

> +
> +  clock-names:
> +    items:
> +      - const: osc

You have one clock, why do you need a name?

Otherwise, this looks okay, thanks.
  
Inochi Amaoto Dec. 6, 2023, 12:39 a.m. UTC | #2
>On Tue, Dec 05, 2023 at 07:55:50PM +0800, Inochi Amaoto wrote:
>> Add definition for the clock controller of the CV1800 series SoC.
>>
>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf
>> ---
>>  .../bindings/clock/sophgo,cv1800-clk.yaml     |  53 ++++++
>>  include/dt-bindings/clock/sophgo,cv1800.h     | 174 ++++++++++++++++++
>>  2 files changed, 227 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>>  create mode 100644 include/dt-bindings/clock/sophgo,cv1800.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>> new file mode 100644
>> index 000000000000..388be5bfa163
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>> @@ -0,0 +1,53 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/sophgo,cv1800-clk.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Sophgo CV1800 Series Clock Controller
>> +
>> +maintainers:
>> +  - Inochi Amaoto <inochiama@outlook.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - sophgo,cv1800-clk
>> +      - sophgo,cv1810-clk
>
>I'm not reading 1000s of lines of driver code to figure it out, what
>differs in the programming model for these two devices?

In fact, they have no different in the programming model. The only
different between them is that cv1810 have one extra clock.

>You should
>mention in your commit message why the cv1810 has an incompatible
>programming model if you are adding multiple devices in one commit
>message.
>

OK, I will

>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: Oscillator (25 MHz)
>
>This could just be "maxItems: 1".
>

OK, thanks.

>> +
>> +  clock-names:
>> +    items:
>> +      - const: osc
>
>You have one clock, why do you need a name?

I am not pretty familiar with this. I just wrote this binding by
referencing others. Maybe use "maxItems: 1" is just fine?

>
>Otherwise, this looks okay, thanks.
>
>
  
Inochi Amaoto Dec. 6, 2023, 2:04 a.m. UTC | #3
>
>> On Tue, Dec 05, 2023 at 07:55:50PM +0800, Inochi Amaoto wrote:
>>> Add definition for the clock controller of the CV1800 series SoC.
>>>
>>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>>> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf
>>> ---
>>>  .../bindings/clock/sophgo,cv1800-clk.yaml     |  53 ++++++
>>>  include/dt-bindings/clock/sophgo,cv1800.h     | 174 ++++++++++++++++++
>>>  2 files changed, 227 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>>>  create mode 100644 include/dt-bindings/clock/sophgo,cv1800.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>>> new file mode 100644
>>> index 000000000000..388be5bfa163
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>>> @@ -0,0 +1,53 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/sophgo,cv1800-clk.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Sophgo CV1800 Series Clock Controller
>>> +
>>> +maintainers:
>>> +  - Inochi Amaoto <inochiama@outlook.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - sophgo,cv1800-clk
>>> +      - sophgo,cv1810-clk
>>
>> I'm not reading 1000s of lines of driver code to figure it out, what
>> differs in the programming model for these two devices?
>
>In fact, they have no different in the programming model. The only
>different between them is that cv1810 have one extra clock.
>
>> You should
>> mention in your commit message why the cv1810 has an incompatible
>> programming model if you are adding multiple devices in one commit
>> message.
>>
>
>OK, I will
>
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    items:
>>> +      - description: Oscillator (25 MHz)
>>
>> This could just be "maxItems: 1".
>>
>
>OK, thanks.
>


>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: osc
>>
>> You have one clock, why do you need a name?
>
>I am not pretty familiar with this. I just wrote this binding by
>referencing others. Maybe use "maxItems: 1" is just fine?
>

I have referenced this name in the clk_parent_data as the global clock
parent. Removing this will cause driver broken.

>>
>> Otherwise, this looks okay, thanks.
>>
>>
>
  
Chen Wang Dec. 6, 2023, 2:08 a.m. UTC | #4
On 2023/12/5 19:55, Inochi Amaoto wrote:
> Add definition for the clock controller of the CV1800 series SoC.
>
> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf
> ---
>   .../bindings/clock/sophgo,cv1800-clk.yaml     |  53 ++++++
With more and more sophgo products coming, I suggest add one subfolder 
"sophgo" for sophgo clock drivers, and this is what I am doing for 
sg2042, you can have a look of my patch.
>   include/dt-bindings/clock/sophgo,cv1800.h     | 174 ++++++++++++++++++
>   2 files changed, 227 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>   create mode 100644 include/dt-bindings/clock/sophgo,cv1800.h
>
> diff --git a/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
> new file mode 100644
> index 000000000000..388be5bfa163
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/sophgo,cv1800-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sophgo CV1800 Series Clock Controller
> +
> +maintainers:
> +  - Inochi Amaoto <inochiama@outlook.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - sophgo,cv1800-clk
> +      - sophgo,cv1810-clk
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Oscillator (25 MHz)
> +
> +  clock-names:
> +    items:
> +      - const: osc
> +
> +  "#clock-cells":
> +    const: 1
> +    description:
> +      See <dt-bindings/clock/sophgo,cv1800.h> for valid indices.
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - "#clock-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    clock-controller@3002000 {
> +        compatible = "sophgo,cv1800-clk";
> +        reg = <0x03002000 0x1000>;
> +        clocks = <&osc>;
> +        clock-names = "osc";
> +        #clock-cells = <1>;
> +    };
> +
> +...
> diff --git a/include/dt-bindings/clock/sophgo,cv1800.h b/include/dt-bindings/clock/sophgo,cv1800.h
> new file mode 100644
> index 000000000000..a71ad2a93153
> --- /dev/null
> +++ b/include/dt-bindings/clock/sophgo,cv1800.h
> @@ -0,0 +1,174 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Copyright (C) 2023 Sophgo Ltd.
> + */
> +
> +#ifndef __DT_BINDINGS_SOPHGO_CV1800_CLK_H__
> +#define __DT_BINDINGS_SOPHGO_CV1800_CLK_H__
> +
> +#define CLK_MPLL			0
> +#define CLK_TPLL			1
> +#define CLK_FPLL			2
> +#define CLK_MIPIMPLL			3
> +#define CLK_A0PLL			4
> +#define CLK_DISPPLL			5
> +#define CLK_CAM0PLL			6
> +#define CLK_CAM1PLL			7
> +
> +#define CLK_MIPIMPLL_D3			8
> +#define CLK_CAM0PLL_D2			9
> +#define CLK_CAM0PLL_D3			10
> +
> +#define CLK_TPU				11
> +#define CLK_TPU_FAB			12
> +#define CLK_AHB_ROM			13
> +#define CLK_DDR_AXI_REG			14
> +#define CLK_RTC_25M			15
> +#define CLK_SRC_RTC_SYS_0		16
> +#define CLK_TEMPSEN			17
> +#define CLK_SARADC			18
> +#define CLK_EFUSE			19
> +#define CLK_APB_EFUSE			20
> +#define CLK_DEBUG			21
> +#define CLK_AP_DEBUG			22
> +#define CLK_XTAL_MISC			23
> +#define CLK_AXI4_EMMC			24
> +#define CLK_EMMC			25
> +#define CLK_EMMC_100K			26
> +#define CLK_AXI4_SD0			27
> +#define CLK_SD0				28
> +#define CLK_SD0_100K			29
> +#define CLK_AXI4_SD1			30
> +#define CLK_SD1				31
> +#define CLK_SD1_100K			32
> +#define CLK_SPI_NAND			33
> +#define CLK_ETH0_500M			34
> +#define CLK_AXI4_ETH0			35
> +#define CLK_ETH1_500M			36
> +#define CLK_AXI4_ETH1			37
> +#define CLK_APB_GPIO			38
> +#define CLK_APB_GPIO_INTR		39
> +#define CLK_GPIO_DB			40
> +#define CLK_AHB_SF			41
> +#define CLK_AHB_SF1			42
> +#define CLK_A24M			43
> +#define CLK_AUDSRC			44
> +#define CLK_APB_AUDSRC			45
> +#define CLK_SDMA_AXI			46
> +#define CLK_SDMA_AUD0			47
> +#define CLK_SDMA_AUD1			48
> +#define CLK_SDMA_AUD2			49
> +#define CLK_SDMA_AUD3			50
> +#define CLK_I2C				51
> +#define CLK_APB_I2C			52
> +#define CLK_APB_I2C0			53
> +#define CLK_APB_I2C1			54
> +#define CLK_APB_I2C2			55
> +#define CLK_APB_I2C3			56
> +#define CLK_APB_I2C4			57
> +#define CLK_APB_WDT			58
> +#define CLK_PWM_SRC			59
> +#define CLK_PWM				60
> +#define CLK_SPI				61
> +#define CLK_APB_SPI0			62
> +#define CLK_APB_SPI1			63
> +#define CLK_APB_SPI2			64
> +#define CLK_APB_SPI3			65
> +#define CLK_1M				66
> +#define CLK_CAM0_200			67
> +#define CLK_PM				68
> +#define CLK_TIMER0			69
> +#define CLK_TIMER1			70
> +#define CLK_TIMER2			71
> +#define CLK_TIMER3			72
> +#define CLK_TIMER4			73
> +#define CLK_TIMER5			74
> +#define CLK_TIMER6			75
> +#define CLK_TIMER7			76
> +#define CLK_UART0			77
> +#define CLK_APB_UART0			78
> +#define CLK_UART1			79
> +#define CLK_APB_UART1			80
> +#define CLK_UART2			81
> +#define CLK_APB_UART2			82
> +#define CLK_UART3			83
> +#define CLK_APB_UART3			84
> +#define CLK_UART4			85
> +#define CLK_APB_UART4			86
> +#define CLK_APB_I2S0			87
> +#define CLK_APB_I2S1			88
> +#define CLK_APB_I2S2			89
> +#define CLK_APB_I2S3			90
> +#define CLK_AXI4_USB			91
> +#define CLK_APB_USB			92
> +#define CLK_USB_125M			93
> +#define CLK_USB_33K			94
> +#define CLK_USB_12M			95
> +#define CLK_AXI4			96
> +#define CLK_AXI6			97
> +#define CLK_DSI_ESC			98
> +#define CLK_AXI_VIP			99
> +#define CLK_SRC_VIP_SYS_0		100
> +#define CLK_SRC_VIP_SYS_1		101
> +#define CLK_SRC_VIP_SYS_2		102
> +#define CLK_SRC_VIP_SYS_3		103
> +#define CLK_SRC_VIP_SYS_4		104
> +#define CLK_CSI_BE_VIP			105
> +#define CLK_CSI_MAC0_VIP		106
> +#define CLK_CSI_MAC1_VIP		107
> +#define CLK_CSI_MAC2_VIP		108
> +#define CLK_CSI0_RX_VIP			109
> +#define CLK_CSI1_RX_VIP			110
> +#define CLK_ISP_TOP_VIP			111
> +#define CLK_IMG_D_VIP			112
> +#define CLK_IMG_V_VIP			113
> +#define CLK_SC_TOP_VIP			114
> +#define CLK_SC_D_VIP			115
> +#define CLK_SC_V1_VIP			116
> +#define CLK_SC_V2_VIP			117
> +#define CLK_SC_V3_VIP			118
> +#define CLK_DWA_VIP			119
> +#define CLK_BT_VIP			120
> +#define CLK_DISP_SRC_VIP		121
> +#define CLK_DISP_VIP			122
> +#define CLK_DSI_MAC_VIP			123
> +#define CLK_LVDS0_VIP			124
> +#define CLK_LVDS1_VIP			125
> +#define CLK_PAD_VI_VIP			126
> +#define CLK_PAD_VI1_VIP			127
> +#define CLK_PAD_VI2_VIP			128
> +#define CLK_CFG_REG_VIP			129
> +#define CLK_VIP_IP0			130
> +#define CLK_VIP_IP1			131
> +#define CLK_VIP_IP2			132
> +#define CLK_VIP_IP3			133
> +#define CLK_IVE_VIP			134
> +#define CLK_RAW_VIP			135
> +#define CLK_OSDC_VIP			136
> +#define CLK_CAM0_VIP			137
> +#define CLK_AXI_VIDEO_CODEC		138
> +#define CLK_VC_SRC0			139
> +#define CLK_VC_SRC1			140
> +#define CLK_VC_SRC2			141
> +#define CLK_H264C			142
> +#define CLK_APB_H264C			143
> +#define CLK_H265C			144
> +#define CLK_APB_H265C			145
> +#define CLK_JPEG			146
> +#define CLK_APB_JPEG			147
> +#define CLK_CAM0			148
> +#define CLK_CAM1			149
> +#define CLK_WGN				150
> +#define CLK_WGN0			151
> +#define CLK_WGN1			152
> +#define CLK_WGN2			153
> +#define CLK_KEYSCAN			154
> +#define CLK_CFG_REG_VC			155
> +#define CLK_C906_0			156
> +#define CLK_C906_1			157
> +#define CLK_A53				158
> +#define CLK_CPU_AXI0			159
> +#define CLK_CPU_GIC			160
> +#define CLK_XTAL_AP			161
> +
> +#endif /* __DT_BINDINGS_SOPHGO_CV1800_CLK_H__ */
> --
> 2.43.0
>
  
Inochi Amaoto Dec. 6, 2023, 3:50 a.m. UTC | #5
>On 2023/12/5 19:55, Inochi Amaoto wrote:
>> Add definition for the clock controller of the CV1800 series SoC.
>>
>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf
>> ---
>>   .../bindings/clock/sophgo,cv1800-clk.yaml     |  53 ++++++
>With more and more sophgo products coming, I suggest add one subfolder "sophgo" for sophgo clock drivers, and this is what I am doing for sg2042, you can have a look of my patch.

Can you share some more convincing reasons? Otherwise, I suggest you may
change your file path. I have not noticed anyone doing this except sifive.
And I still have some doubt on this.

>>   include/dt-bindings/clock/sophgo,cv1800.h     | 174 ++++++++++++++++++
>>   2 files changed, 227 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>>   create mode 100644 include/dt-bindings/clock/sophgo,cv1800.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>> new file mode 100644
>> index 000000000000..388be5bfa163
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
>> @@ -0,0 +1,53 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/sophgo,cv1800-clk.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Sophgo CV1800 Series Clock Controller
>> +
>> +maintainers:
>> +  - Inochi Amaoto <inochiama@outlook.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - sophgo,cv1800-clk
>> +      - sophgo,cv1810-clk
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: Oscillator (25 MHz)
>> +
>> +  clock-names:
>> +    items:
>> +      - const: osc
>> +
>> +  "#clock-cells":
>> +    const: 1
>> +    description:
>> +      See <dt-bindings/clock/sophgo,cv1800.h> for valid indices.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - "#clock-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    clock-controller@3002000 {
>> +        compatible = "sophgo,cv1800-clk";
>> +        reg = <0x03002000 0x1000>;
>> +        clocks = <&osc>;
>> +        clock-names = "osc";
>> +        #clock-cells = <1>;
>> +    };
>> +
>> +...
>> diff --git a/include/dt-bindings/clock/sophgo,cv1800.h b/include/dt-bindings/clock/sophgo,cv1800.h
>> new file mode 100644
>> index 000000000000..a71ad2a93153
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/sophgo,cv1800.h
>> @@ -0,0 +1,174 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
>> +/*
>> + * Copyright (C) 2023 Sophgo Ltd.
>> + */
>> +
>> +#ifndef __DT_BINDINGS_SOPHGO_CV1800_CLK_H__
>> +#define __DT_BINDINGS_SOPHGO_CV1800_CLK_H__
>> +
>> +#define CLK_MPLL            0
>> +#define CLK_TPLL            1
>> +#define CLK_FPLL            2
>> +#define CLK_MIPIMPLL            3
>> +#define CLK_A0PLL            4
>> +#define CLK_DISPPLL            5
>> +#define CLK_CAM0PLL            6
>> +#define CLK_CAM1PLL            7
>> +
>> +#define CLK_MIPIMPLL_D3            8
>> +#define CLK_CAM0PLL_D2            9
>> +#define CLK_CAM0PLL_D3            10
>> +
>> +#define CLK_TPU                11
>> +#define CLK_TPU_FAB            12
>> +#define CLK_AHB_ROM            13
>> +#define CLK_DDR_AXI_REG            14
>> +#define CLK_RTC_25M            15
>> +#define CLK_SRC_RTC_SYS_0        16
>> +#define CLK_TEMPSEN            17
>> +#define CLK_SARADC            18
>> +#define CLK_EFUSE            19
>> +#define CLK_APB_EFUSE            20
>> +#define CLK_DEBUG            21
>> +#define CLK_AP_DEBUG            22
>> +#define CLK_XTAL_MISC            23
>> +#define CLK_AXI4_EMMC            24
>> +#define CLK_EMMC            25
>> +#define CLK_EMMC_100K            26
>> +#define CLK_AXI4_SD0            27
>> +#define CLK_SD0                28
>> +#define CLK_SD0_100K            29
>> +#define CLK_AXI4_SD1            30
>> +#define CLK_SD1                31
>> +#define CLK_SD1_100K            32
>> +#define CLK_SPI_NAND            33
>> +#define CLK_ETH0_500M            34
>> +#define CLK_AXI4_ETH0            35
>> +#define CLK_ETH1_500M            36
>> +#define CLK_AXI4_ETH1            37
>> +#define CLK_APB_GPIO            38
>> +#define CLK_APB_GPIO_INTR        39
>> +#define CLK_GPIO_DB            40
>> +#define CLK_AHB_SF            41
>> +#define CLK_AHB_SF1            42
>> +#define CLK_A24M            43
>> +#define CLK_AUDSRC            44
>> +#define CLK_APB_AUDSRC            45
>> +#define CLK_SDMA_AXI            46
>> +#define CLK_SDMA_AUD0            47
>> +#define CLK_SDMA_AUD1            48
>> +#define CLK_SDMA_AUD2            49
>> +#define CLK_SDMA_AUD3            50
>> +#define CLK_I2C                51
>> +#define CLK_APB_I2C            52
>> +#define CLK_APB_I2C0            53
>> +#define CLK_APB_I2C1            54
>> +#define CLK_APB_I2C2            55
>> +#define CLK_APB_I2C3            56
>> +#define CLK_APB_I2C4            57
>> +#define CLK_APB_WDT            58
>> +#define CLK_PWM_SRC            59
>> +#define CLK_PWM                60
>> +#define CLK_SPI                61
>> +#define CLK_APB_SPI0            62
>> +#define CLK_APB_SPI1            63
>> +#define CLK_APB_SPI2            64
>> +#define CLK_APB_SPI3            65
>> +#define CLK_1M                66
>> +#define CLK_CAM0_200            67
>> +#define CLK_PM                68
>> +#define CLK_TIMER0            69
>> +#define CLK_TIMER1            70
>> +#define CLK_TIMER2            71
>> +#define CLK_TIMER3            72
>> +#define CLK_TIMER4            73
>> +#define CLK_TIMER5            74
>> +#define CLK_TIMER6            75
>> +#define CLK_TIMER7            76
>> +#define CLK_UART0            77
>> +#define CLK_APB_UART0            78
>> +#define CLK_UART1            79
>> +#define CLK_APB_UART1            80
>> +#define CLK_UART2            81
>> +#define CLK_APB_UART2            82
>> +#define CLK_UART3            83
>> +#define CLK_APB_UART3            84
>> +#define CLK_UART4            85
>> +#define CLK_APB_UART4            86
>> +#define CLK_APB_I2S0            87
>> +#define CLK_APB_I2S1            88
>> +#define CLK_APB_I2S2            89
>> +#define CLK_APB_I2S3            90
>> +#define CLK_AXI4_USB            91
>> +#define CLK_APB_USB            92
>> +#define CLK_USB_125M            93
>> +#define CLK_USB_33K            94
>> +#define CLK_USB_12M            95
>> +#define CLK_AXI4            96
>> +#define CLK_AXI6            97
>> +#define CLK_DSI_ESC            98
>> +#define CLK_AXI_VIP            99
>> +#define CLK_SRC_VIP_SYS_0        100
>> +#define CLK_SRC_VIP_SYS_1        101
>> +#define CLK_SRC_VIP_SYS_2        102
>> +#define CLK_SRC_VIP_SYS_3        103
>> +#define CLK_SRC_VIP_SYS_4        104
>> +#define CLK_CSI_BE_VIP            105
>> +#define CLK_CSI_MAC0_VIP        106
>> +#define CLK_CSI_MAC1_VIP        107
>> +#define CLK_CSI_MAC2_VIP        108
>> +#define CLK_CSI0_RX_VIP            109
>> +#define CLK_CSI1_RX_VIP            110
>> +#define CLK_ISP_TOP_VIP            111
>> +#define CLK_IMG_D_VIP            112
>> +#define CLK_IMG_V_VIP            113
>> +#define CLK_SC_TOP_VIP            114
>> +#define CLK_SC_D_VIP            115
>> +#define CLK_SC_V1_VIP            116
>> +#define CLK_SC_V2_VIP            117
>> +#define CLK_SC_V3_VIP            118
>> +#define CLK_DWA_VIP            119
>> +#define CLK_BT_VIP            120
>> +#define CLK_DISP_SRC_VIP        121
>> +#define CLK_DISP_VIP            122
>> +#define CLK_DSI_MAC_VIP            123
>> +#define CLK_LVDS0_VIP            124
>> +#define CLK_LVDS1_VIP            125
>> +#define CLK_PAD_VI_VIP            126
>> +#define CLK_PAD_VI1_VIP            127
>> +#define CLK_PAD_VI2_VIP            128
>> +#define CLK_CFG_REG_VIP            129
>> +#define CLK_VIP_IP0            130
>> +#define CLK_VIP_IP1            131
>> +#define CLK_VIP_IP2            132
>> +#define CLK_VIP_IP3            133
>> +#define CLK_IVE_VIP            134
>> +#define CLK_RAW_VIP            135
>> +#define CLK_OSDC_VIP            136
>> +#define CLK_CAM0_VIP            137
>> +#define CLK_AXI_VIDEO_CODEC        138
>> +#define CLK_VC_SRC0            139
>> +#define CLK_VC_SRC1            140
>> +#define CLK_VC_SRC2            141
>> +#define CLK_H264C            142
>> +#define CLK_APB_H264C            143
>> +#define CLK_H265C            144
>> +#define CLK_APB_H265C            145
>> +#define CLK_JPEG            146
>> +#define CLK_APB_JPEG            147
>> +#define CLK_CAM0            148
>> +#define CLK_CAM1            149
>> +#define CLK_WGN                150
>> +#define CLK_WGN0            151
>> +#define CLK_WGN1            152
>> +#define CLK_WGN2            153
>> +#define CLK_KEYSCAN            154
>> +#define CLK_CFG_REG_VC            155
>> +#define CLK_C906_0            156
>> +#define CLK_C906_1            157
>> +#define CLK_A53                158
>> +#define CLK_CPU_AXI0            159
>> +#define CLK_CPU_GIC            160
>> +#define CLK_XTAL_AP            161
>> +
>> +#endif /* __DT_BINDINGS_SOPHGO_CV1800_CLK_H__ */
>> --
>> 2.43.0
>>
>
  
Krzysztof Kozlowski Dec. 6, 2023, 10:28 a.m. UTC | #6
On 06/12/2023 03:04, Inochi Amaoto wrote:
>>>> +
>>>> +  clock-names:
>>>> +    items:
>>>> +      - const: osc
>>>
>>> You have one clock, why do you need a name?
>>
>> I am not pretty familiar with this. I just wrote this binding by
>> referencing others. Maybe use "maxItems: 1" is just fine?

No, just drop clock-names and get clocks by indices in the driver.

>>
> 
> I have referenced this name in the clk_parent_data as the global clock
> parent. Removing this will cause driver broken.

It's not related.

Best regards,
Krzysztof
  
Inochi Amaoto Dec. 6, 2023, 10:58 a.m. UTC | #7
>On 06/12/2023 03:04, Inochi Amaoto wrote:
>>>>> +
>>>>> +  clock-names:
>>>>> +    items:
>>>>> +      - const: osc
>>>>
>>>> You have one clock, why do you need a name?
>>>
>>> I am not pretty familiar with this. I just wrote this binding by
>>> referencing others. Maybe use "maxItems: 1" is just fine?
>
>No, just drop clock-names and get clocks by indices in the driver.
>

OK, thanks, I will remove it.

>>>
>>
>> I have referenced this name in the clk_parent_data as the global clock
>> parent. Removing this will cause driver broken.
>
>It's not related.
>
>Best regards,
>Krzysztof
>
>
  

Patch

diff --git a/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
new file mode 100644
index 000000000000..388be5bfa163
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/sophgo,cv1800-clk.yaml
@@ -0,0 +1,53 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/sophgo,cv1800-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo CV1800 Series Clock Controller
+
+maintainers:
+  - Inochi Amaoto <inochiama@outlook.com>
+
+properties:
+  compatible:
+    enum:
+      - sophgo,cv1800-clk
+      - sophgo,cv1810-clk
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Oscillator (25 MHz)
+
+  clock-names:
+    items:
+      - const: osc
+
+  "#clock-cells":
+    const: 1
+    description:
+      See <dt-bindings/clock/sophgo,cv1800.h> for valid indices.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-controller@3002000 {
+        compatible = "sophgo,cv1800-clk";
+        reg = <0x03002000 0x1000>;
+        clocks = <&osc>;
+        clock-names = "osc";
+        #clock-cells = <1>;
+    };
+
+...
diff --git a/include/dt-bindings/clock/sophgo,cv1800.h b/include/dt-bindings/clock/sophgo,cv1800.h
new file mode 100644
index 000000000000..a71ad2a93153
--- /dev/null
+++ b/include/dt-bindings/clock/sophgo,cv1800.h
@@ -0,0 +1,174 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright (C) 2023 Sophgo Ltd.
+ */
+
+#ifndef __DT_BINDINGS_SOPHGO_CV1800_CLK_H__
+#define __DT_BINDINGS_SOPHGO_CV1800_CLK_H__
+
+#define CLK_MPLL			0
+#define CLK_TPLL			1
+#define CLK_FPLL			2
+#define CLK_MIPIMPLL			3
+#define CLK_A0PLL			4
+#define CLK_DISPPLL			5
+#define CLK_CAM0PLL			6
+#define CLK_CAM1PLL			7
+
+#define CLK_MIPIMPLL_D3			8
+#define CLK_CAM0PLL_D2			9
+#define CLK_CAM0PLL_D3			10
+
+#define CLK_TPU				11
+#define CLK_TPU_FAB			12
+#define CLK_AHB_ROM			13
+#define CLK_DDR_AXI_REG			14
+#define CLK_RTC_25M			15
+#define CLK_SRC_RTC_SYS_0		16
+#define CLK_TEMPSEN			17
+#define CLK_SARADC			18
+#define CLK_EFUSE			19
+#define CLK_APB_EFUSE			20
+#define CLK_DEBUG			21
+#define CLK_AP_DEBUG			22
+#define CLK_XTAL_MISC			23
+#define CLK_AXI4_EMMC			24
+#define CLK_EMMC			25
+#define CLK_EMMC_100K			26
+#define CLK_AXI4_SD0			27
+#define CLK_SD0				28
+#define CLK_SD0_100K			29
+#define CLK_AXI4_SD1			30
+#define CLK_SD1				31
+#define CLK_SD1_100K			32
+#define CLK_SPI_NAND			33
+#define CLK_ETH0_500M			34
+#define CLK_AXI4_ETH0			35
+#define CLK_ETH1_500M			36
+#define CLK_AXI4_ETH1			37
+#define CLK_APB_GPIO			38
+#define CLK_APB_GPIO_INTR		39
+#define CLK_GPIO_DB			40
+#define CLK_AHB_SF			41
+#define CLK_AHB_SF1			42
+#define CLK_A24M			43
+#define CLK_AUDSRC			44
+#define CLK_APB_AUDSRC			45
+#define CLK_SDMA_AXI			46
+#define CLK_SDMA_AUD0			47
+#define CLK_SDMA_AUD1			48
+#define CLK_SDMA_AUD2			49
+#define CLK_SDMA_AUD3			50
+#define CLK_I2C				51
+#define CLK_APB_I2C			52
+#define CLK_APB_I2C0			53
+#define CLK_APB_I2C1			54
+#define CLK_APB_I2C2			55
+#define CLK_APB_I2C3			56
+#define CLK_APB_I2C4			57
+#define CLK_APB_WDT			58
+#define CLK_PWM_SRC			59
+#define CLK_PWM				60
+#define CLK_SPI				61
+#define CLK_APB_SPI0			62
+#define CLK_APB_SPI1			63
+#define CLK_APB_SPI2			64
+#define CLK_APB_SPI3			65
+#define CLK_1M				66
+#define CLK_CAM0_200			67
+#define CLK_PM				68
+#define CLK_TIMER0			69
+#define CLK_TIMER1			70
+#define CLK_TIMER2			71
+#define CLK_TIMER3			72
+#define CLK_TIMER4			73
+#define CLK_TIMER5			74
+#define CLK_TIMER6			75
+#define CLK_TIMER7			76
+#define CLK_UART0			77
+#define CLK_APB_UART0			78
+#define CLK_UART1			79
+#define CLK_APB_UART1			80
+#define CLK_UART2			81
+#define CLK_APB_UART2			82
+#define CLK_UART3			83
+#define CLK_APB_UART3			84
+#define CLK_UART4			85
+#define CLK_APB_UART4			86
+#define CLK_APB_I2S0			87
+#define CLK_APB_I2S1			88
+#define CLK_APB_I2S2			89
+#define CLK_APB_I2S3			90
+#define CLK_AXI4_USB			91
+#define CLK_APB_USB			92
+#define CLK_USB_125M			93
+#define CLK_USB_33K			94
+#define CLK_USB_12M			95
+#define CLK_AXI4			96
+#define CLK_AXI6			97
+#define CLK_DSI_ESC			98
+#define CLK_AXI_VIP			99
+#define CLK_SRC_VIP_SYS_0		100
+#define CLK_SRC_VIP_SYS_1		101
+#define CLK_SRC_VIP_SYS_2		102
+#define CLK_SRC_VIP_SYS_3		103
+#define CLK_SRC_VIP_SYS_4		104
+#define CLK_CSI_BE_VIP			105
+#define CLK_CSI_MAC0_VIP		106
+#define CLK_CSI_MAC1_VIP		107
+#define CLK_CSI_MAC2_VIP		108
+#define CLK_CSI0_RX_VIP			109
+#define CLK_CSI1_RX_VIP			110
+#define CLK_ISP_TOP_VIP			111
+#define CLK_IMG_D_VIP			112
+#define CLK_IMG_V_VIP			113
+#define CLK_SC_TOP_VIP			114
+#define CLK_SC_D_VIP			115
+#define CLK_SC_V1_VIP			116
+#define CLK_SC_V2_VIP			117
+#define CLK_SC_V3_VIP			118
+#define CLK_DWA_VIP			119
+#define CLK_BT_VIP			120
+#define CLK_DISP_SRC_VIP		121
+#define CLK_DISP_VIP			122
+#define CLK_DSI_MAC_VIP			123
+#define CLK_LVDS0_VIP			124
+#define CLK_LVDS1_VIP			125
+#define CLK_PAD_VI_VIP			126
+#define CLK_PAD_VI1_VIP			127
+#define CLK_PAD_VI2_VIP			128
+#define CLK_CFG_REG_VIP			129
+#define CLK_VIP_IP0			130
+#define CLK_VIP_IP1			131
+#define CLK_VIP_IP2			132
+#define CLK_VIP_IP3			133
+#define CLK_IVE_VIP			134
+#define CLK_RAW_VIP			135
+#define CLK_OSDC_VIP			136
+#define CLK_CAM0_VIP			137
+#define CLK_AXI_VIDEO_CODEC		138
+#define CLK_VC_SRC0			139
+#define CLK_VC_SRC1			140
+#define CLK_VC_SRC2			141
+#define CLK_H264C			142
+#define CLK_APB_H264C			143
+#define CLK_H265C			144
+#define CLK_APB_H265C			145
+#define CLK_JPEG			146
+#define CLK_APB_JPEG			147
+#define CLK_CAM0			148
+#define CLK_CAM1			149
+#define CLK_WGN				150
+#define CLK_WGN0			151
+#define CLK_WGN1			152
+#define CLK_WGN2			153
+#define CLK_KEYSCAN			154
+#define CLK_CFG_REG_VC			155
+#define CLK_C906_0			156
+#define CLK_C906_1			157
+#define CLK_A53				158
+#define CLK_CPU_AXI0			159
+#define CLK_CPU_GIC			160
+#define CLK_XTAL_AP			161
+
+#endif /* __DT_BINDINGS_SOPHGO_CV1800_CLK_H__ */