Re: [PATCH 1/4] clk: sunxi-ng: h6-r: Add R_APB2_RSB clock and reset

[Samuel Holland]

On 12/14/20 8:57 AM, Maxime Ripard wrote:
> Hi Samuel,
> 
> On Sun, Dec 13, 2020 at 05:55:03PM -0600, Samuel Holland wrote:
>> While no information about the H6 RSB controller is included in the
>> datasheet or manual, the vendor BSP and power management blob both
>> reference the RSB clock parent and register address. These values were
>> verified by experimentation.
>>
>> Since this clock/reset are added late, the specifier is added at the end
>> to maintain the existing DT binding. The code is kept in register order.
>>
>> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx>
>> ---
>>  drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c      | 5 +++++
>>  drivers/clk/sunxi-ng/ccu-sun50i-h6-r.h      | 2 +-
>>  include/dt-bindings/clock/sun50i-h6-r-ccu.h | 1 +
>>  include/dt-bindings/reset/sun50i-h6-r-ccu.h | 1 +
>>  4 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c b/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
>> index 50f8d1bc7046..56e351b513f3 100644
>> --- a/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
>> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
>> @@ -91,6 +91,8 @@ static SUNXI_CCU_GATE(r_apb2_uart_clk,	"r-apb2-uart",	"r-apb2",
>>  		      0x18c, BIT(0), 0);
>>  static SUNXI_CCU_GATE(r_apb2_i2c_clk,	"r-apb2-i2c",	"r-apb2",
>>  		      0x19c, BIT(0), 0);
>> +static SUNXI_CCU_GATE(r_apb2_rsb_clk,	"r-apb2-rsb",	"r-apb2",
>> +		      0x1bc, BIT(0), 0);
>>  static SUNXI_CCU_GATE(r_apb1_ir_clk,	"r-apb1-ir",	"r-apb1",
>>  		      0x1cc, BIT(0), 0);
>>  static SUNXI_CCU_GATE(r_apb1_w1_clk,	"r-apb1-w1",	"r-apb1",
>> @@ -130,6 +132,7 @@ static struct ccu_common *sun50i_h6_r_ccu_clks[] = {
>>  	&r_apb1_pwm_clk.common,
>>  	&r_apb2_uart_clk.common,
>>  	&r_apb2_i2c_clk.common,
>> +	&r_apb2_rsb_clk.common,
>>  	&r_apb1_ir_clk.common,
>>  	&r_apb1_w1_clk.common,
>>  	&ir_clk.common,
>> @@ -147,6 +150,7 @@ static struct clk_hw_onecell_data sun50i_h6_r_hw_clks = {
>>  		[CLK_R_APB1_PWM]	= &r_apb1_pwm_clk.common.hw,
>>  		[CLK_R_APB2_UART]	= &r_apb2_uart_clk.common.hw,
>>  		[CLK_R_APB2_I2C]	= &r_apb2_i2c_clk.common.hw,
>> +		[CLK_R_APB2_RSB]	= &r_apb2_rsb_clk.common.hw,
>>  		[CLK_R_APB1_IR]		= &r_apb1_ir_clk.common.hw,
>>  		[CLK_R_APB1_W1]		= &r_apb1_w1_clk.common.hw,
>>  		[CLK_IR]		= &ir_clk.common.hw,
>> @@ -161,6 +165,7 @@ static struct ccu_reset_map sun50i_h6_r_ccu_resets[] = {
>>  	[RST_R_APB1_PWM]	=  { 0x13c, BIT(16) },
>>  	[RST_R_APB2_UART]	=  { 0x18c, BIT(16) },
>>  	[RST_R_APB2_I2C]	=  { 0x19c, BIT(16) },
>> +	[RST_R_APB2_RSB]	=  { 0x1bc, BIT(16) },
>>  	[RST_R_APB1_IR]		=  { 0x1cc, BIT(16) },
>>  	[RST_R_APB1_W1]		=  { 0x1ec, BIT(16) },
>>  };
>> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.h b/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.h
>> index 782117dc0b28..7e290b840803 100644
>> --- a/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.h
>> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.h
>> @@ -14,6 +14,6 @@
>>  
>>  #define CLK_R_APB2	3
>>  
>> -#define CLK_NUMBER	(CLK_W1 + 1)
>> +#define CLK_NUMBER	(CLK_R_APB2_RSB + 1)
>>  
>>  #endif /* _CCU_SUN50I_H6_R_H */
>> diff --git a/include/dt-bindings/clock/sun50i-h6-r-ccu.h b/include/dt-bindings/clock/sun50i-h6-r-ccu.h
>> index 76136132a13e..f46ec03848ca 100644
>> --- a/include/dt-bindings/clock/sun50i-h6-r-ccu.h
>> +++ b/include/dt-bindings/clock/sun50i-h6-r-ccu.h
>> @@ -15,6 +15,7 @@
>>  #define CLK_R_APB1_PWM		6
>>  #define CLK_R_APB2_UART		7
>>  #define CLK_R_APB2_I2C		8
>> +#define CLK_R_APB2_RSB		13
>>  #define CLK_R_APB1_IR		9
>>  #define CLK_R_APB1_W1		10
>>  
>> diff --git a/include/dt-bindings/reset/sun50i-h6-r-ccu.h b/include/dt-bindings/reset/sun50i-h6-r-ccu.h
>> index 01c84dba49a4..6fe199a7969d 100644
>> --- a/include/dt-bindings/reset/sun50i-h6-r-ccu.h
>> +++ b/include/dt-bindings/reset/sun50i-h6-r-ccu.h
>> @@ -11,6 +11,7 @@
>>  #define RST_R_APB1_PWM		2
>>  #define RST_R_APB2_UART		3
>>  #define RST_R_APB2_I2C		4
>> +#define RST_R_APB2_RSB		7
>>  #define RST_R_APB1_IR		5
>>  #define RST_R_APB1_W1		6
> 
> I think for the clock and reset binding, we'll want to sort by number.
> It's fairly easy to miss otherwise and if we end up adding another one
> it wouldn't be far fetched to assume the same indices would be used

I think GCC would complain about the duplicate array initialization in
the driver, but I can move them for v2.

Cheers,
Samuel