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

From: Samuel Holland
Date: Mon Dec 14 2020 - 22:27:07 EST


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