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

[Samuel Holland]

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 <sam...@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

--
2.26.2

[Maxime Ripard]

Hi Samuel,

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

[Samuel Holland]

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

Cheers,
Samuel

[André Przywara]

I agree here. Admittedly this whole approach is really fragile. I ended
up with some tiny tool to check for consecutive numbers, reporting any
outliers (some gaps are legit). Of course the "-r" versions of the CCU
are not really a big issue, but with the 100+ clocks in the main CCU
this is a problem. Also it becomes ABI, so is hard to fix.

I guess there is no nice kernel CPP hack to re-use enums as preprocessor
symbols?

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

It doesn't with -Wall, you need -Wextra for that. I actually had a
subtle bug in my H616 CCU patch due to a double numbering.

Cheers,
Andre