Re: [PATCH 3/4] riscv: dts: sophgo: add reset generator for Sophgo CV1800 series SoC

[Chen Wang <unicorn_wang@outlook.com>]

On 2025/2/10 9:20, Yixun Lan wrote:
> Hi Chen:
>
> On 08:55 Mon 10 Feb     , Chen Wang wrote:
>> On 2025/2/9 20:29, Inochi Amaoto wrote:
>>> Add reset generator node for all CV18XX series SoC.
>>>
>>> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
>>> ---
>>>    arch/riscv/boot/dts/sophgo/cv18xx-reset.h | 98 +++++++++++++++++++++++
>> Why it's not include/dt-bindings/reset/sophgo,cv18xx-reset.h?
>>
> then first need to answer if it's really a binding, there are some disucssions
> I personally favor Inochi's way here
>
> https://lore.kernel.org/linux-devicetree/c7e243e3-3f61-4d63-8727-3837838bdfcc@linaro.org
> https://lore.kernel.org/linux-devicetree/c088e01c-0714-82be-8347-6140daf56640@linaro.org/

Well, it's fine if you don't want to define it as binding just because 
the ids are not continuous. In fact, for simple-reset, I think there is 
no big problem to define it as bindings.

Actually, in the first link upon, I see Krzysztof also said 
"simple-reset is an exception."

I recall we ever discussed this when Jisheng submitted the first version 
of reset driver for cv18xx. FYI. See 
https://lore.kernel.org/linux-riscv/42ad3f60-2755-4d3b-a766-8e4404f76a7c@linaro.org/.

>> Regards,
>>
>> Chen
>>
>>>    arch/riscv/boot/dts/sophgo/cv18xx.dtsi    |  7 ++
>>>    2 files changed, 105 insertions(+)
>>>    create mode 100644 arch/riscv/boot/dts/sophgo/cv18xx-reset.h
>>>
>>> diff --git a/arch/riscv/boot/dts/sophgo/cv18xx-reset.h b/arch/riscv/boot/dts/sophgo/cv18xx-reset.h
>>> new file mode 100644
>>> index 000000000000..3d9aa9ec7e90
>>> --- /dev/null
>>> +++ b/arch/riscv/boot/dts/sophgo/cv18xx-reset.h
>>> @@ -0,0 +1,98 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>>> +/*
>>> + * Copyright (C) 2025 Inochi Amaoto <inochiama@outlook.com>
>>> + */
>>> +
>>> +#ifndef _SOPHGO_CV18XX_RESET
>>> +#define _SOPHGO_CV18XX_RESET
>>> +
>>> +#define RST_DDR				2
>>> +#define RST_H264C			3
>>> +#define RST_JPEG			4
>>> +#define RST_H265C			5
>>> +#define RST_VIPSYS			6
>>> +#define RST_TDMA			7
>>> +#define RST_TPU				8
>>> +#define RST_TPUSYS			9
>>> +#define RST_USB				11
>>> +#define RST_ETH0			12
>>> +#define RST_ETH1			13
>>> +#define RST_NAND			14
>>> +#define RST_EMMC			15
>>> +#define RST_SD0				16
>>> +#define RST_SDMA			18
>>> +#define RST_I2S0			19
>>> +#define RST_I2S1			20
>>> +#define RST_I2S2			21
>>> +#define RST_I2S3			22
>>> +#define RST_UART0			23
>>> +#define RST_UART1			24
>>> +#define RST_UART2			25
>>> +#define RST_UART3			26
>>> +#define RST_I2C0			27
>>> +#define RST_I2C1			28
>>> +#define RST_I2C2			29
>>> +#define RST_I2C3			30
>>> +#define RST_I2C4			31
>>> +#define RST_PWM0			32
>>> +#define RST_PWM1			33
>>> +#define RST_PWM2			34
>>> +#define RST_PWM3			35
>>> +#define RST_SPI0			40
>>> +#define RST_SPI1			41
>>> +#define RST_SPI2			42
>>> +#define RST_SPI3			43
>>> +#define RST_GPIO0			44
>>> +#define RST_GPIO1			45
>>> +#define RST_GPIO2			46
>>> +#define RST_EFUSE			47
>>> +#define RST_WDT				48
>>> +#define RST_AHB_ROM			49
>>> +#define RST_SPIC			50
>>> +#define RST_TEMPSEN			51
>>> +#define RST_SARADC			52
>>> +#define RST_COMBO_PHY0			58
>>> +#define RST_SPI_NAND			61
>>> +#define RST_SE				62
>>> +#define RST_UART4			74
>>> +#define RST_GPIO3			75
>>> +#define RST_SYSTEM			76
>>> +#define RST_TIMER			77
>>> +#define RST_TIMER0			78
>>> +#define RST_TIMER1			79
>>> +#define RST_TIMER2			80
>>> +#define RST_TIMER3			81
>>> +#define RST_TIMER4			82
>>> +#define RST_TIMER5			83
>>> +#define RST_TIMER6			84
>>> +#define RST_TIMER7			85
>>> +#define RST_WGN0			86
>>> +#define RST_WGN1			87
>>> +#define RST_WGN2			88
>>> +#define RST_KEYSCAN			89
>>> +#define RST_AUDDAC			91
>>> +#define RST_AUDDAC_APB			92
>>> +#define RST_AUDADC			93
>>> +#define RST_VCSYS			95
>>> +#define RST_ETHPHY			96
>>> +#define RST_ETHPHY_APB			97
>>> +#define RST_AUDSRC			98
>>> +#define RST_VIP_CAM0			99
>>> +#define RST_WDT1			100
>>> +#define RST_WDT2			101
>>> +#define RST_AUTOCLEAR_CPUCORE0		128
>>> +#define RST_AUTOCLEAR_CPUCORE1		129
>>> +#define RST_AUTOCLEAR_CPUCORE2		130
>>> +#define RST_AUTOCLEAR_CPUCORE3		131
>>> +#define RST_AUTOCLEAR_CPUSYS0		132
>>> +#define RST_AUTOCLEAR_CPUSYS1		133
>>> +#define RST_AUTOCLEAR_CPUSYS2		134
>>> +#define RST_CPUCORE0			160
>>> +#define RST_CPUCORE1			161
>>> +#define RST_CPUCORE2			162
>>> +#define RST_CPUCORE3			163
>>> +#define RST_CPUSYS0			164
>>> +#define RST_CPUSYS1			165
>>> +#define RST_CPUSYS2			166
>>> +
>>> +#endif /* _SOPHGO_CV18XX_RESET */
>>> diff --git a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
>>> index c18822ec849f..9aa28ade73a4 100644
>>> --- a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
>>> +++ b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
>>> @@ -7,6 +7,7 @@
>>>    #include <dt-bindings/clock/sophgo,cv1800.h>
>>>    #include <dt-bindings/gpio/gpio.h>
>>>    #include <dt-bindings/interrupt-controller/irq.h>
>>> +#include "cv18xx-reset.h"
>>>    
>>>    / {
>>>    	#address-cells = <1>;
>>> @@ -61,6 +62,12 @@ clk: clock-controller@3002000 {
>>>    			#clock-cells = <1>;
>>>    		};
>>>    
>>> +		rst: reset-controller@3003000 {
>>> +			compatible = "sophgo,cv1800b-reset";
>>> +			reg = <0x3003000 0x1000>;
>>> +			#reset-cells = <1>;
>>> +		};
>>> +
>>>    		gpio0: gpio@3020000 {
>>>    			compatible = "snps,dw-apb-gpio";
>>>    			reg = <0x3020000 0x1000>;