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

[Maxime Ripard <maxime@cerno.tech>]

[-- Attachment #1: Type: text/plain, Size: 4141 bytes --]

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@sholland.org>
> ---
>  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

maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]