[PATCH v9 0/3] Add support for AST2700 clk driver

[PATCH v9 0/3] Add support for AST2700 clk driver

[Ryan Chen]

This patch series is add clk driver for AST2700.

AST2700 is the 8th generation of Integrated Remote Management Processor
introduced by ASPEED Technology Inc. Which is Board Management controller
(BMC) SoC family. AST2700 have two SoC connected, one is SoC0, another
is SoC1, it has it's own scu, this driver inlcude SCU0 and SCU1 driver.

v9:
-aspeed,ast2700-scu.h: no change.
add more clear commit description.
-clk-ast2700.c:
add inlcude bitfield.h
remove redundant clk_parent_data soc0_mpll_div8/soc0_ahb/uart13clk/
uart14clk/uart15clk/uart16clk/soc1_ahb/d_clk_sels

v8:
-aspeed,ast2700-scu.h: remove no use soc0 clock, add new clock
-clk-ast2700.c: remove include <linux/auxiliary_bus.h>,
include <linux/clk-provider.h>, include <linux/of_address.h>
-clk-ast2700.c: add include <linux/mod_devicetable.h>
-clk-ast2700.c: modify include <soc/aspeed/reset-aspeed.h> order before
dt-bindings
-clk-ast2700.c: modify define to be tabbed out space
-clk-ast2700.c: add union struct for each clk type
	union {
		struct ast2700_clk_fixed_factor_data factor;
		struct ast2700_clk_fixed_rate_data rate;
		struct ast2700_clk_gate_data gate;
		struct ast2700_clk_div_data div;
		struct ast2700_clk_pll_data pll;
		struct ast2700_clk_mux_data mux;
	} data;
-clk-ast2700.c: modify clk_data = device_get_match_data(dev);
-clk-ast2700.c: modify builtin_platform_driver_probe to 
arch_initcall(clk_ast2700_init)
-clk-ast2700.c: ast2700_clk_hw_register_hpll explain: scu010[4:2],
scu010[4:2] = 010, hpll force 1.8Ghz
scu010[4:2] = 011, hpll force 1.7Ghz
scu010[4:2] = 110, hpll force 1.2Ghz
scu010[4:2] = 111, hpll force 800Mhz
others depend on hpll parameter register setting.

v7:
-reset-aspeed.h: fix declare static inline aspeed_reset_controller_register
if the function is not used.

v6:
-patch-2: add reset-aspeed.h
-reset-aspeed: add include cleanup.h for guard()
-reset-aspeed: change ids name clk_aspeed to reset_aspeed
-reset-aspeed: move aspeed_reset_controller_register,
aspeed_reset_adev_release, aspeed_reset_unregister_adev from clk-ast2700.c
-reset-aspeed: drop base check, since it check in clk-ast2700.c
-clk-ast2700: sync each gate name from *clk to *clk-gate name.
-clk-ast2700: add CLK_GATE_ASPEED to diff clk_hw_register_gate and
ast2700_clk_hw_register_gate.

v5:
-patch-2 Kconfig: add select AUXILIARY_BUS
-reset-aspeed: #define to_aspeed_reset(p) turn into static inline function.
-reset-aspeed: modify spin_lock_irqsave to guard(spinlock_irqsave)
-reset-aspeed: remove unnecessary parentheses.
-clk-ast2700: use <linux/units.h> and refrain from define clk

v4:
-yaml: keep size-cells=<1>.
-merge clk,reset dt binding header with yaml the same patch.
-rename clk,reset dt binding header to aspeed,ast2700-scu.h
-reset-aspeed: update tables tabs sapces to consistent spaces.
-reset-aspeed: remove no use dev_set_drvdata.
-clk-ast2700: modify reset_name to const int scu in struct clk_data.
-clk-ast2700: use scu number in clk_data generate reset_name for reset
 driver register.
-clk-ast2700: fix pll number mix up scu0,scu1.
-clk-ast2700: update dt-binding clock include file.

v3:
-yaml: v2 missing send yaml patch, v3 add.
-yaml: drop 64bits address example.
-yaml: add discription about soc0 and soc1
-dt-bindings: remove (), *_NUMS, reserved.
-dt-bindings: remove dulipated define number.
-dt-bindings: merge clk and reset to be one patch.
-reset-aspeed: add auxiliary device for reset driver.
-clk-ast2700: modify reset to be auxiliary add.
-clk-ast2700: modify to be platform driver.
-clk-ast2700: modify each clk to const clk array.

v2:
-yaml: drop 64bits address example.
-yaml: add discription about soc0 and soc1
-dt-bindings: remove (), *_NUMS, reserved.
-dt-bindings: remove dulipated define number
-clk-ast2700: drop WARN_ON, weird comment.

Ryan Chen (3):
  dt-binding: clock: ast2700: modify soc0/1 clock define
  reset: aspeed: register AST2700 reset auxiliary bus device
  clk: aspeed: add AST2700 clock driver

 drivers/clk/Kconfig                           |    8 +
 drivers/clk/Makefile                          |    1 +
 drivers/clk/clk-ast2700.c                     | 1120 +++++++++++++++++
 drivers/reset/Kconfig                         |    7 +
 drivers/reset/Makefile                        |    1 +
 drivers/reset/reset-aspeed.c                  |  302 +++++
 .../dt-bindings/clock/aspeed,ast2700-scu.h    |    7 +-
 include/soc/aspeed/reset-aspeed.h             |   21 +
 8 files changed, 1464 insertions(+), 3 deletions(-)
 create mode 100644 drivers/clk/clk-ast2700.c
 create mode 100644 drivers/reset/reset-aspeed.c
 create mode 100644 include/soc/aspeed/reset-aspeed.h

-- 
2.34.1

[PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock define

[Ryan Chen]

-remove redundant SOC0_CLK_UART_DIV13:
SOC0_CLK_UART_DIV13 is not use at clk-ast2700.c, the clock
source tree is uart clk src -> uart_div_table -> uart clk.

-Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX:
modify clock tree implement.
older CLK_AHB use mpll_div_ahb/hpll_div_ahb to be ahb clock source.
mpll->mpll_div_ahb
                  -> clk_ahb
hpll->hpll_div_ahb

new use SOC0_CLK_AHBMUX for more understand clock source divide tree.
mpll->
      ahb_mux -> div_table -> clk_ahb
hpll->

-new add clock:
 SOC0_CLK_MPHYSRC: UFS MPHY clock source.
 SOC0_CLK_U2PHY_REFCLKSRC: USB2.0 phy clock reference source.
 SOC1_CLK_I3C: I3C clock source.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
 include/dt-bindings/clock/aspeed,ast2700-scu.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/dt-bindings/clock/aspeed,ast2700-scu.h b/include/dt-bindings/clock/aspeed,ast2700-scu.h
index 63021af3caf5..c7389530629d 100644
--- a/include/dt-bindings/clock/aspeed,ast2700-scu.h
+++ b/include/dt-bindings/clock/aspeed,ast2700-scu.h
@@ -13,18 +13,17 @@
 #define SCU0_CLK_24M		1
 #define SCU0_CLK_192M		2
 #define SCU0_CLK_UART		3
-#define SCU0_CLK_UART_DIV13	3
 #define SCU0_CLK_PSP		4
 #define SCU0_CLK_HPLL		5
 #define SCU0_CLK_HPLL_DIV2	6
 #define SCU0_CLK_HPLL_DIV4	7
-#define SCU0_CLK_HPLL_DIV_AHB	8
+#define SCU0_CLK_AHBMUX		8
 #define SCU0_CLK_DPLL		9
 #define SCU0_CLK_MPLL		10
 #define SCU0_CLK_MPLL_DIV2	11
 #define SCU0_CLK_MPLL_DIV4	12
 #define SCU0_CLK_MPLL_DIV8	13
-#define SCU0_CLK_MPLL_DIV_AHB	14
+#define SCU0_CLK_MPHYSRC	14
 #define SCU0_CLK_D0		15
 #define SCU0_CLK_D1		16
 #define SCU0_CLK_CRT0		17
@@ -68,6 +67,7 @@
 #define SCU0_CLK_GATE_UFSCLK	53
 #define SCU0_CLK_GATE_EMMCCLK	54
 #define SCU0_CLK_GATE_RVAS1CLK	55
+#define SCU0_CLK_U2PHY_REFCLKSRC 56
 
 /* SOC1 clk */
 #define SCU1_CLKIN		0
@@ -160,4 +160,5 @@
 #define SCU1_CLK_GATE_PORTDUSB2CLK	85
 #define SCU1_CLK_GATE_LTPI1TXCLK	86
 
+#define SCU1_CLK_I3C	87
 #endif
-- 
2.34.1

[PATCH v9 2/3] reset: aspeed: register AST2700 reset auxiliary bus device

[Ryan Chen]

The AST2700 reset driver is registered as an auxiliary device
due to reset and clock controller share the same register region.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/reset/Kconfig             |   7 +
 drivers/reset/Makefile            |   1 +
 drivers/reset/reset-aspeed.c      | 302 ++++++++++++++++++++++++++++++
 include/soc/aspeed/reset-aspeed.h |  21 +++
 4 files changed, 331 insertions(+)
 create mode 100644 drivers/reset/reset-aspeed.c
 create mode 100644 include/soc/aspeed/reset-aspeed.h

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 5b3abb6db248..7e1ba795eb23 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -22,6 +22,13 @@ config RESET_A10SR
 	  This option enables support for the external reset functions for
 	  peripheral PHYs on the Altera Arria10 System Resource Chip.
 
+config RESET_ASPEED
+	tristate "ASPEED Reset Driver"
+	depends on ARCH_ASPEED || COMPILE_TEST
+	select AUXILIARY_BUS
+	help
+	  This enables the reset controller driver for AST2700.
+
 config RESET_ATH79
 	bool "AR71xx Reset Driver" if COMPILE_TEST
 	default ATH79
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 677c4d1e2632..25af31f673a4 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -6,6 +6,7 @@ obj-y += starfive/
 obj-y += sti/
 obj-y += tegra/
 obj-$(CONFIG_RESET_A10SR) += reset-a10sr.o
+obj-$(CONFIG_RESET_ASPEED) += reset-aspeed.o
 obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
 obj-$(CONFIG_RESET_AXS10X) += reset-axs10x.o
 obj-$(CONFIG_RESET_BCM6345) += reset-bcm6345.o
diff --git a/drivers/reset/reset-aspeed.c b/drivers/reset/reset-aspeed.c
new file mode 100644
index 000000000000..2dc8478af7b0
--- /dev/null
+++ b/drivers/reset/reset-aspeed.c
@@ -0,0 +1,302 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 ASPEED Technology Inc.
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/reset/aspeed,ast2700-scu.h>
+#include <soc/aspeed/reset-aspeed.h>
+
+#define SCU0_RESET_CTRL1 0x200
+#define SCU0_RESET_CTRL2 0x220
+#define SCU1_RESET_CTRL1 0x200
+#define SCU1_RESET_CTRL2 0x220
+#define SCU1_PCIE3_CTRL 0x908
+
+struct ast2700_reset_signal {
+	bool dedicated_clr; /* dedicated reset clr offset */
+	u32 offset, bit;
+};
+
+struct aspeed_reset_info {
+	unsigned int nr_resets;
+	const struct ast2700_reset_signal *signal;
+};
+
+struct aspeed_reset {
+	struct reset_controller_dev rcdev;
+	struct aspeed_reset_info *info;
+	spinlock_t lock; /* Protect read-modify-write cycle */
+	void __iomem *base;
+};
+
+static const struct ast2700_reset_signal ast2700_reset0_signals[] = {
+	[SCU0_RESET_SDRAM] = { true, SCU0_RESET_CTRL1, BIT(0) },
+	[SCU0_RESET_DDRPHY] = { true, SCU0_RESET_CTRL1, BIT(1) },
+	[SCU0_RESET_RSA] = { true, SCU0_RESET_CTRL1, BIT(2) },
+	[SCU0_RESET_SHA3] = { true, SCU0_RESET_CTRL1, BIT(3) },
+	[SCU0_RESET_HACE] = { true, SCU0_RESET_CTRL1, BIT(4) },
+	[SCU0_RESET_SOC] = { true, SCU0_RESET_CTRL1, BIT(5) },
+	[SCU0_RESET_VIDEO] = { true, SCU0_RESET_CTRL1, BIT(6) },
+	[SCU0_RESET_2D] = { true, SCU0_RESET_CTRL1, BIT(7) },
+	[SCU0_RESET_PCIS] = { true, SCU0_RESET_CTRL1, BIT(8) },
+	[SCU0_RESET_RVAS0] = { true, SCU0_RESET_CTRL1, BIT(9) },
+	[SCU0_RESET_RVAS1] = { true, SCU0_RESET_CTRL1, BIT(10) },
+	[SCU0_RESET_SM3] = { true, SCU0_RESET_CTRL1, BIT(11) },
+	[SCU0_RESET_SM4] = { true, SCU0_RESET_CTRL1, BIT(12) },
+	[SCU0_RESET_CRT0] = { true, SCU0_RESET_CTRL1, BIT(13) },
+	[SCU0_RESET_ECC] = { true, SCU0_RESET_CTRL1, BIT(14) },
+	[SCU0_RESET_DP_PCI] = { true, SCU0_RESET_CTRL1, BIT(15) },
+	[SCU0_RESET_UFS] = { true, SCU0_RESET_CTRL1, BIT(16) },
+	[SCU0_RESET_EMMC] = { true, SCU0_RESET_CTRL1, BIT(17) },
+	[SCU0_RESET_PCIE1RST] = { true, SCU0_RESET_CTRL1, BIT(18) },
+	[SCU0_RESET_PCIE1RSTOE] = { true, SCU0_RESET_CTRL1, BIT(19) },
+	[SCU0_RESET_PCIE0RST] = { true, SCU0_RESET_CTRL1, BIT(20) },
+	[SCU0_RESET_PCIE0RSTOE] = { true, SCU0_RESET_CTRL1, BIT(21) },
+	[SCU0_RESET_JTAG] = { true, SCU0_RESET_CTRL1, BIT(22) },
+	[SCU0_RESET_MCTP0] = { true, SCU0_RESET_CTRL1, BIT(23) },
+	[SCU0_RESET_MCTP1] = { true, SCU0_RESET_CTRL1, BIT(24) },
+	[SCU0_RESET_XDMA0] = { true, SCU0_RESET_CTRL1, BIT(25) },
+	[SCU0_RESET_XDMA1] = { true, SCU0_RESET_CTRL1, BIT(26) },
+	[SCU0_RESET_H2X1] = { true, SCU0_RESET_CTRL1, BIT(27) },
+	[SCU0_RESET_DP] = { true, SCU0_RESET_CTRL1, BIT(28) },
+	[SCU0_RESET_DP_MCU] = { true, SCU0_RESET_CTRL1, BIT(29) },
+	[SCU0_RESET_SSP] = { true, SCU0_RESET_CTRL1, BIT(30) },
+	[SCU0_RESET_H2X0] = { true, SCU0_RESET_CTRL1, BIT(31) },
+	[SCU0_RESET_PORTA_VHUB] = { true, SCU0_RESET_CTRL2, BIT(0) },
+	[SCU0_RESET_PORTA_PHY3] = { true, SCU0_RESET_CTRL2, BIT(1) },
+	[SCU0_RESET_PORTA_XHCI] = { true, SCU0_RESET_CTRL2, BIT(2) },
+	[SCU0_RESET_PORTB_VHUB] = { true, SCU0_RESET_CTRL2, BIT(3) },
+	[SCU0_RESET_PORTB_PHY3] = { true, SCU0_RESET_CTRL2, BIT(4) },
+	[SCU0_RESET_PORTB_XHCI] = { true, SCU0_RESET_CTRL2, BIT(5) },
+	[SCU0_RESET_PORTA_VHUB_EHCI] = { true, SCU0_RESET_CTRL2, BIT(6) },
+	[SCU0_RESET_PORTB_VHUB_EHCI] = { true, SCU0_RESET_CTRL2, BIT(7) },
+	[SCU0_RESET_UHCI] = { true, SCU0_RESET_CTRL2, BIT(8) },
+	[SCU0_RESET_TSP] = { true, SCU0_RESET_CTRL2, BIT(9) },
+	[SCU0_RESET_E2M0] = { true, SCU0_RESET_CTRL2, BIT(10) },
+	[SCU0_RESET_E2M1] = { true, SCU0_RESET_CTRL2, BIT(11) },
+	[SCU0_RESET_VLINK] = { true, SCU0_RESET_CTRL2, BIT(12) },
+};
+
+static const struct ast2700_reset_signal ast2700_reset1_signals[] = {
+	[SCU1_RESET_LPC0] = { true, SCU1_RESET_CTRL1, BIT(0) },
+	[SCU1_RESET_LPC1] = { true, SCU1_RESET_CTRL1, BIT(1) },
+	[SCU1_RESET_MII] = { true, SCU1_RESET_CTRL1, BIT(2) },
+	[SCU1_RESET_PECI] = { true, SCU1_RESET_CTRL1, BIT(3) },
+	[SCU1_RESET_PWM] = { true, SCU1_RESET_CTRL1, BIT(4) },
+	[SCU1_RESET_MAC0] = { true, SCU1_RESET_CTRL1, BIT(5) },
+	[SCU1_RESET_MAC1] = { true, SCU1_RESET_CTRL1, BIT(6) },
+	[SCU1_RESET_MAC2] = { true, SCU1_RESET_CTRL1, BIT(7) },
+	[SCU1_RESET_ADC] = { true, SCU1_RESET_CTRL1, BIT(8) },
+	[SCU1_RESET_SD] = { true, SCU1_RESET_CTRL1, BIT(9) },
+	[SCU1_RESET_ESPI0] = { true, SCU1_RESET_CTRL1, BIT(10) },
+	[SCU1_RESET_ESPI1] = { true, SCU1_RESET_CTRL1, BIT(11) },
+	[SCU1_RESET_JTAG1] = { true, SCU1_RESET_CTRL1, BIT(12) },
+	[SCU1_RESET_SPI0] = { true, SCU1_RESET_CTRL1, BIT(13) },
+	[SCU1_RESET_SPI1] = { true, SCU1_RESET_CTRL1, BIT(14) },
+	[SCU1_RESET_SPI2] = { true, SCU1_RESET_CTRL1, BIT(15) },
+	[SCU1_RESET_I3C0] = { true, SCU1_RESET_CTRL1, BIT(16) },
+	[SCU1_RESET_I3C1] = { true, SCU1_RESET_CTRL1, BIT(17) },
+	[SCU1_RESET_I3C2] = { true, SCU1_RESET_CTRL1, BIT(18) },
+	[SCU1_RESET_I3C3] = { true, SCU1_RESET_CTRL1, BIT(19) },
+	[SCU1_RESET_I3C4] = { true, SCU1_RESET_CTRL1, BIT(20) },
+	[SCU1_RESET_I3C5] = { true, SCU1_RESET_CTRL1, BIT(21) },
+	[SCU1_RESET_I3C6] = { true, SCU1_RESET_CTRL1, BIT(22) },
+	[SCU1_RESET_I3C7] = { true, SCU1_RESET_CTRL1, BIT(23) },
+	[SCU1_RESET_I3C8] = { true, SCU1_RESET_CTRL1, BIT(24) },
+	[SCU1_RESET_I3C9] = { true, SCU1_RESET_CTRL1, BIT(25) },
+	[SCU1_RESET_I3C10] = { true, SCU1_RESET_CTRL1, BIT(26) },
+	[SCU1_RESET_I3C11] = { true, SCU1_RESET_CTRL1, BIT(27) },
+	[SCU1_RESET_I3C12] = { true, SCU1_RESET_CTRL1, BIT(28) },
+	[SCU1_RESET_I3C13] = { true, SCU1_RESET_CTRL1, BIT(29) },
+	[SCU1_RESET_I3C14] = { true, SCU1_RESET_CTRL1, BIT(30) },
+	[SCU1_RESET_I3C15] = { true, SCU1_RESET_CTRL1, BIT(31) },
+	[SCU1_RESET_MCU0] = { true, SCU1_RESET_CTRL2, BIT(0) },
+	[SCU1_RESET_MCU1] = { true, SCU1_RESET_CTRL2, BIT(1) },
+	[SCU1_RESET_H2A_SPI1] = { true, SCU1_RESET_CTRL2, BIT(2) },
+	[SCU1_RESET_H2A_SPI2] = { true, SCU1_RESET_CTRL2, BIT(3) },
+	[SCU1_RESET_UART0] = { true, SCU1_RESET_CTRL2, BIT(4) },
+	[SCU1_RESET_UART1] = { true, SCU1_RESET_CTRL2, BIT(5) },
+	[SCU1_RESET_UART2] = { true, SCU1_RESET_CTRL2, BIT(6) },
+	[SCU1_RESET_UART3] = { true, SCU1_RESET_CTRL2, BIT(7) },
+	[SCU1_RESET_I2C_FILTER] = { true, SCU1_RESET_CTRL2, BIT(8) },
+	[SCU1_RESET_CALIPTRA] = { true, SCU1_RESET_CTRL2, BIT(9) },
+	[SCU1_RESET_XDMA] = { true, SCU1_RESET_CTRL2, BIT(10) },
+	[SCU1_RESET_FSI] = { true, SCU1_RESET_CTRL2, BIT(12) },
+	[SCU1_RESET_CAN] = { true, SCU1_RESET_CTRL2, BIT(13) },
+	[SCU1_RESET_MCTP] = { true, SCU1_RESET_CTRL2, BIT(14) },
+	[SCU1_RESET_I2C] = { true, SCU1_RESET_CTRL2, BIT(15) },
+	[SCU1_RESET_UART6] = { true, SCU1_RESET_CTRL2, BIT(16) },
+	[SCU1_RESET_UART7] = { true, SCU1_RESET_CTRL2, BIT(17) },
+	[SCU1_RESET_UART8] = { true, SCU1_RESET_CTRL2, BIT(18) },
+	[SCU1_RESET_UART9] = { true, SCU1_RESET_CTRL2, BIT(19) },
+	[SCU1_RESET_LTPI0] = { true, SCU1_RESET_CTRL2, BIT(20) },
+	[SCU1_RESET_VGAL] = { true, SCU1_RESET_CTRL2, BIT(21) },
+	[SCU1_RESET_LTPI1] = { true, SCU1_RESET_CTRL2, BIT(22) },
+	[SCU1_RESET_ACE] = { true, SCU1_RESET_CTRL2, BIT(23) },
+	[SCU1_RESET_E2M] = { true, SCU1_RESET_CTRL2, BIT(24) },
+	[SCU1_RESET_UHCI] = { true, SCU1_RESET_CTRL2, BIT(25) },
+	[SCU1_RESET_PORTC_USB2UART] = { true, SCU1_RESET_CTRL2, BIT(26) },
+	[SCU1_RESET_PORTC_VHUB_EHCI] = { true, SCU1_RESET_CTRL2, BIT(27) },
+	[SCU1_RESET_PORTD_USB2UART] = { true, SCU1_RESET_CTRL2, BIT(28) },
+	[SCU1_RESET_PORTD_VHUB_EHCI] = { true, SCU1_RESET_CTRL2, BIT(29) },
+	[SCU1_RESET_H2X] = { true, SCU1_RESET_CTRL2, BIT(30) },
+	[SCU1_RESET_I3CDMA] = { true, SCU1_RESET_CTRL2, BIT(31) },
+	[SCU1_RESET_PCIE2RST] = { false, SCU1_PCIE3_CTRL, BIT(0) },
+};
+
+static inline struct aspeed_reset *to_aspeed_reset(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct aspeed_reset, rcdev);
+}
+
+static int aspeed_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct aspeed_reset *rc = to_aspeed_reset(rcdev);
+	void __iomem *reg_offset = rc->base + rc->info->signal[id].offset;
+
+	if (rc->info->signal[id].dedicated_clr) {
+		writel(rc->info->signal[id].bit, reg_offset);
+	} else {
+		guard(spinlock_irqsave)(&rc->lock);
+		writel(readl(reg_offset) & ~rc->info->signal[id].bit, reg_offset);
+	}
+
+	return 0;
+}
+
+static int aspeed_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct aspeed_reset *rc = to_aspeed_reset(rcdev);
+	void __iomem *reg_offset = rc->base + rc->info->signal[id].offset;
+
+	if (rc->info->signal[id].dedicated_clr) {
+		writel(rc->info->signal[id].bit, reg_offset + 0x04);
+	} else {
+		guard(spinlock_irqsave)(&rc->lock);
+		writel(readl(reg_offset) | rc->info->signal[id].bit, reg_offset);
+	}
+
+	return 0;
+}
+
+static int aspeed_reset_status(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct aspeed_reset *rc = to_aspeed_reset(rcdev);
+	void __iomem *reg_offset = rc->base + rc->info->signal[id].offset;
+
+	return (readl(reg_offset) & rc->info->signal[id].bit) ? 1 : 0;
+}
+
+static const struct reset_control_ops aspeed_reset_ops = {
+	.assert = aspeed_reset_assert,
+	.deassert = aspeed_reset_deassert,
+	.status = aspeed_reset_status,
+};
+
+static int aspeed_reset_probe(struct auxiliary_device *adev,
+			      const struct auxiliary_device_id *id)
+{
+	struct aspeed_reset *reset;
+	struct device *dev = &adev->dev;
+
+	reset = devm_kzalloc(dev, sizeof(*reset), GFP_KERNEL);
+	if (!reset)
+		return -ENOMEM;
+
+	spin_lock_init(&reset->lock);
+
+	reset->info = (struct aspeed_reset_info *)id->driver_data;
+	reset->rcdev.owner = THIS_MODULE;
+	reset->rcdev.nr_resets = reset->info->nr_resets;
+	reset->rcdev.ops = &aspeed_reset_ops;
+	reset->rcdev.of_node = dev->parent->of_node;
+	reset->rcdev.dev = dev;
+	reset->rcdev.of_reset_n_cells = 1;
+	reset->base = (void __iomem *)adev->dev.platform_data;
+
+	return devm_reset_controller_register(dev, &reset->rcdev);
+}
+
+static void aspeed_reset_unregister_adev(void *_adev)
+{
+	struct auxiliary_device *adev = _adev;
+
+	auxiliary_device_delete(adev);
+	auxiliary_device_uninit(adev);
+}
+
+static void aspeed_reset_adev_release(struct device *dev)
+{
+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+	kfree(adev);
+}
+
+int aspeed_reset_controller_register(struct device *clk_dev, void __iomem *base,
+				     const char *adev_name)
+{
+	struct auxiliary_device *adev;
+	int ret;
+
+	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+	if (!adev)
+		return -ENOMEM;
+
+	adev->name = adev_name;
+	adev->dev.parent = clk_dev;
+	adev->dev.release = aspeed_reset_adev_release;
+	adev->id = 666u;
+
+	ret = auxiliary_device_init(adev);
+	if (ret) {
+		kfree(adev);
+		return ret;
+	}
+
+	ret = auxiliary_device_add(adev);
+	if (ret) {
+		auxiliary_device_uninit(adev);
+		return ret;
+	}
+
+	adev->dev.platform_data = (__force void *)base;
+
+	return devm_add_action_or_reset(clk_dev, aspeed_reset_unregister_adev, adev);
+}
+EXPORT_SYMBOL_GPL(aspeed_reset_controller_register);
+
+static const struct aspeed_reset_info ast2700_reset0_info = {
+	.nr_resets = ARRAY_SIZE(ast2700_reset0_signals),
+	.signal = ast2700_reset0_signals,
+};
+
+static const struct aspeed_reset_info ast2700_reset1_info = {
+	.nr_resets = ARRAY_SIZE(ast2700_reset1_signals),
+	.signal = ast2700_reset1_signals,
+};
+
+static const struct auxiliary_device_id aspeed_reset_ids[] = {
+	{ .name = "reset_aspeed.reset0", .driver_data = (kernel_ulong_t)&ast2700_reset0_info },
+	{ .name = "reset_aspeed.reset1", .driver_data = (kernel_ulong_t)&ast2700_reset1_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(auxiliary, aspeed_reset_ids);
+
+static struct auxiliary_driver aspeed_reset_driver = {
+	.probe		= aspeed_reset_probe,
+	.id_table	= aspeed_reset_ids,
+};
+
+module_auxiliary_driver(aspeed_reset_driver);
+
+MODULE_AUTHOR("Ryan Chen <ryan_chen@aspeedtech.com>");
+MODULE_DESCRIPTION("ASPEED SoC Reset Controller Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/soc/aspeed/reset-aspeed.h b/include/soc/aspeed/reset-aspeed.h
new file mode 100644
index 000000000000..51a4d13fbe3d
--- /dev/null
+++ b/include/soc/aspeed/reset-aspeed.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2024 ASPEED Technology Inc.
+ * Author: Ryan Chen <ryan_chen@aspeedtech.com>
+ */
+
+#ifndef __RESET_ASPEED_H__
+#define __RESET_ASPEED_H__
+
+#if IS_ENABLED(CONFIG_RESET_ASPEED)
+int aspeed_reset_controller_register(struct device *clk_dev, void __iomem *base,
+				     const char *adev_name);
+#else
+static inline int aspeed_reset_controller_register(struct device *clk_dev, void __iomem *base,
+				     const char *adev_name)
+{
+	return -ENODEV;
+}
+#endif /* if IS_ENABLED(CONFIG_RESET_ASPEED) */
+
+#endif /* __RESET_ASPEED_H__ */
-- 
2.34.1

[PATCH v9 3/3] clk: aspeed: add AST2700 clock driver

[Ryan Chen]

Add AST2700 clock controller driver and also use axiliary
device framework register the reset controller driver.
Due to clock and reset using the same register region.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
 drivers/clk/Kconfig       |    8 +
 drivers/clk/Makefile      |    1 +
 drivers/clk/clk-ast2700.c | 1120 +++++++++++++++++++++++++++++++++++++
 3 files changed, 1129 insertions(+)
 create mode 100644 drivers/clk/clk-ast2700.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 713573b6c86c..285618c9b5ca 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -288,6 +288,14 @@ config COMMON_CLK_ASPEED
 	  The G4 and G5 series, including the ast2400 and ast2500, are supported
 	  by this driver.
 
+config COMMON_CLK_AST2700
+	bool "Clock driver for AST2700 SoC"
+	depends on ARCH_ASPEED || COMPILE_TEST
+	help
+	  This driver provides support for clock on AST2700 SoC.
+	  The driver is responsible for managing the various clocks required
+	  by the peripherals and cores within the AST2700.
+
 config COMMON_CLK_S2MPS11
 	tristate "Clock driver for S2MPS1X/S5M8767 MFD"
 	depends on MFD_SEC_CORE || COMPILE_TEST
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index bf4bd45adc3a..c084e50bc662 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_COMMON_CLK_FSL_SAI)	+= clk-fsl-sai.o
 obj-$(CONFIG_COMMON_CLK_GEMINI)		+= clk-gemini.o
 obj-$(CONFIG_COMMON_CLK_ASPEED)		+= clk-aspeed.o
 obj-$(CONFIG_MACH_ASPEED_G6)		+= clk-ast2600.o
+obj-$(CONFIG_COMMON_CLK_AST2700)	+= clk-ast2700.o
 obj-$(CONFIG_ARCH_HIGHBANK)		+= clk-highbank.o
 obj-$(CONFIG_CLK_HSDK)			+= clk-hsdk-pll.o
 obj-$(CONFIG_COMMON_CLK_K210)		+= clk-k210.o
diff --git a/drivers/clk/clk-ast2700.c b/drivers/clk/clk-ast2700.c
new file mode 100644
index 000000000000..974aa27cfd33
--- /dev/null
+++ b/drivers/clk/clk-ast2700.c
@@ -0,0 +1,1120 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2024 ASPEED Technology Inc.
+ * Author: Ryan Chen <ryan_chen@aspeedtech.com>
+ */
+#include <linux/bitfield.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/units.h>
+
+#include <soc/aspeed/reset-aspeed.h>
+#include <dt-bindings/clock/aspeed,ast2700-scu.h>
+
+#define SCU_CLK_12MHZ	(12 * HZ_PER_MHZ)
+#define SCU_CLK_24MHZ	(24 * HZ_PER_MHZ)
+#define SCU_CLK_25MHZ	(25 * HZ_PER_MHZ)
+#define SCU_CLK_192MHZ	(192 * HZ_PER_MHZ)
+
+/* SOC0 */
+#define SCU0_HWSTRAP1		0x010
+#define SCU0_CLK_STOP		0x240
+#define SCU0_CLK_SEL1		0x280
+#define SCU0_CLK_SEL2		0x284
+#define GET_USB_REFCLK_DIV(x)	((GENMASK(23, 20) & (x)) >> 20)
+#define UART_DIV13_EN		BIT(30)
+#define SCU0_HPLL_PARAM		0x300
+#define SCU0_DPLL_PARAM		0x308
+#define SCU0_MPLL_PARAM		0x310
+#define SCU0_D0CLK_PARAM	0x320
+#define SCU0_D1CLK_PARAM	0x330
+#define SCU0_CRT0CLK_PARAM	0x340
+#define SCU0_CRT1CLK_PARAM	0x350
+#define SCU0_MPHYCLK_PARAM	0x360
+
+/* SOC1 */
+#define SCU1_REVISION_ID	0x0
+#define REVISION_ID		GENMASK(23, 16)
+#define SCU1_CLK_STOP		0x240
+#define SCU1_CLK_STOP2		0x260
+#define SCU1_CLK_SEL1		0x280
+#define SCU1_CLK_SEL2		0x284
+#define SCU1_CLK_I3C_DIV_MASK	GENMASK(25, 23)
+#define SCU1_CLK_I3C_DIV(n)	((n) - 1)
+#define UXCLK_MASK		GENMASK(1, 0)
+#define HUXCLK_MASK		GENMASK(4, 3)
+#define SCU1_HPLL_PARAM		0x300
+#define SCU1_APLL_PARAM		0x310
+#define SCU1_DPLL_PARAM		0x320
+#define SCU1_UXCLK_CTRL		0x330
+#define SCU1_HUXCLK_CTRL	0x334
+#define SCU1_MAC12_CLK_DLY	0x390
+#define SCU1_MAC12_CLK_DLY_100M	0x394
+#define SCU1_MAC12_CLK_DLY_10M	0x398
+
+enum ast2700_clk_type {
+	CLK_MUX,
+	CLK_PLL,
+	CLK_HPLL,
+	CLK_GATE,
+	CLK_MISC,
+	CLK_FIXED,
+	DCLK_FIXED,
+	CLK_DIVIDER,
+	CLK_UART_PLL,
+	CLK_FIXED_FACTOR,
+	CLK_GATE_ASPEED,
+};
+
+struct ast2700_clk_fixed_factor_data {
+	const struct clk_parent_data *parent;
+	unsigned int mult;
+	unsigned int div;
+};
+
+struct ast2700_clk_gate_data {
+	const struct clk_parent_data *parent;
+	u32 flags;
+	u32 reg;
+	u8 bit;
+};
+
+struct ast2700_clk_mux_data {
+	const struct clk_parent_data *parents;
+	unsigned int num_parents;
+	u8 bit_shift;
+	u8 bit_width;
+	u32 reg;
+};
+
+struct ast2700_clk_div_data {
+	const struct clk_div_table *div_table;
+	const struct clk_parent_data *parent;
+	u8 bit_shift;
+	u8 bit_width;
+	u32 reg;
+};
+
+struct ast2700_clk_pll_data {
+	const struct clk_parent_data *parent;
+	u32 reg;
+};
+
+struct ast2700_clk_fixed_rate_data {
+	unsigned long fixed_rate;
+};
+
+struct ast2700_clk_info {
+	const char *name;
+	u8 clk_idx;
+	u32 reg;
+	u32 type;
+	union {
+		struct ast2700_clk_fixed_factor_data factor;
+		struct ast2700_clk_fixed_rate_data rate;
+		struct ast2700_clk_gate_data gate;
+		struct ast2700_clk_div_data div;
+		struct ast2700_clk_pll_data pll;
+		struct ast2700_clk_mux_data mux;
+	} data;
+};
+
+struct ast2700_clk_data {
+	struct ast2700_clk_info const *clk_info;
+	unsigned int nr_clks;
+	const int scu;
+};
+
+struct ast2700_clk_ctrl {
+	const struct ast2700_clk_data *clk_data;
+	struct device *dev;
+	void __iomem *base;
+	spinlock_t lock; /* clk lock */
+};
+
+static const struct clk_div_table ast2700_rgmii_div_table[] = {
+	{ 0x0, 4 },
+	{ 0x1, 4 },
+	{ 0x2, 6 },
+	{ 0x3, 8 },
+	{ 0x4, 10 },
+	{ 0x5, 12 },
+	{ 0x6, 14 },
+	{ 0x7, 16 },
+	{ 0 }
+};
+
+static const struct clk_div_table ast2700_rmii_div_table[] = {
+	{ 0x0, 8 },
+	{ 0x1, 8 },
+	{ 0x2, 12 },
+	{ 0x3, 16 },
+	{ 0x4, 20 },
+	{ 0x5, 24 },
+	{ 0x6, 28 },
+	{ 0x7, 32 },
+	{ 0 }
+};
+
+static const struct clk_div_table ast2700_clk_div_table[] = {
+	{ 0x0, 2 },
+	{ 0x1, 2 },
+	{ 0x2, 3 },
+	{ 0x3, 4 },
+	{ 0x4, 5 },
+	{ 0x5, 6 },
+	{ 0x6, 7 },
+	{ 0x7, 8 },
+	{ 0 }
+};
+
+static const struct clk_div_table ast2700_clk_div_table2[] = {
+	{ 0x0, 2 },
+	{ 0x1, 4 },
+	{ 0x2, 6 },
+	{ 0x3, 8 },
+	{ 0x4, 10 },
+	{ 0x5, 12 },
+	{ 0x6, 14 },
+	{ 0x7, 16 },
+	{ 0 }
+};
+
+static const struct clk_div_table ast2700_hclk_div_table[] = {
+	{ 0x0, 6 },
+	{ 0x1, 5 },
+	{ 0x2, 4 },
+	{ 0x3, 7 },
+	{ 0 }
+};
+
+static const struct clk_div_table ast2700_clk_uart_div_table[] = {
+	{ 0x0, 1 },
+	{ 0x1, 13 },
+	{ 0 }
+};
+
+static const struct clk_parent_data soc0_clkin[] = {
+	{ .fw_name = "soc0-clkin", .name = "soc0-clkin" },
+};
+
+static const struct clk_parent_data pspclk[] = {
+	{ .fw_name = "pspclk", .name = "pspclk" },
+};
+
+static const struct clk_parent_data mphysrc[] = {
+	{ .fw_name = "mphysrc", .name = "mphysrc" },
+};
+
+static const struct clk_parent_data u2phy_refclksrc[] = {
+	{ .fw_name = "u2phy_refclksrc", .name = "u2phy_refclksrc" },
+};
+
+static const struct clk_parent_data soc0_hpll[] = {
+	{ .fw_name = "soc0-hpll", .name = "soc0-hpll" },
+};
+
+static const struct clk_parent_data soc0_mpll[] = {
+	{ .fw_name = "soc0-mpll", .name = "soc0-mpll" },
+};
+
+static const struct clk_parent_data axi0clk[] = {
+	{ .fw_name = "axi0clk", .name = "axi0clk" },
+};
+
+static const struct clk_parent_data soc0_ahbmux[] = {
+	{ .fw_name = "soc0-ahbmux", .name = "soc0-ahbmux" },
+};
+
+static const struct clk_parent_data soc0_uartclk[] = {
+	{ .fw_name = "soc0-uartclk", .name = "soc0-uartclk" },
+};
+
+static const struct clk_parent_data emmcclk[] = {
+	{ .fw_name = "emmcclk", .name = "emmcclk" },
+};
+
+static const struct clk_parent_data emmcsrc_mux[] = {
+	{ .fw_name = "emmcsrc-mux", .name = "emmcsrc-mux" },
+};
+
+static const struct clk_parent_data soc1_clkin[] = {
+	{ .fw_name = "soc1-clkin", .name = "soc1-clkin" },
+};
+
+static const struct clk_parent_data soc1_hpll[] = {
+	{ .fw_name = "soc1-hpll", .name = "soc1-hpll" },
+};
+
+static const struct clk_parent_data soc1_apll[] = {
+	{ .fw_name = "soc1-apll", .name = "soc1-apll" },
+};
+
+static const struct clk_parent_data sdclk[] = {
+	{ .fw_name = "sdclk", .name = "sdclk" },
+};
+
+static const struct clk_parent_data sdclk_mux[] = {
+	{ .fw_name = "sdclk-mux", .name = "sdclk-mux" },
+};
+
+static const struct clk_parent_data huartxclk[] = {
+	{ .fw_name = "huartxclk", .name = "huartxclk" },
+};
+
+static const struct clk_parent_data uxclk[] = {
+	{ .fw_name = "uxclk", .name = "uxclk" },
+};
+
+static const struct clk_parent_data huxclk[] = {
+	{ .fw_name = "huxclk", .name = "huxclk" },
+};
+
+static const struct clk_parent_data uart0clk[] = {
+	{ .fw_name = "uart0clk", .name = "uart0clk" },
+};
+
+static const struct clk_parent_data uart1clk[] = {
+	{ .fw_name = "uart1clk", .name = "uart1clk" },
+};
+
+static const struct clk_parent_data uart2clk[] = {
+	{ .fw_name = "uart2clk", .name = "uart2clk" },
+};
+
+static const struct clk_parent_data uart3clk[] = {
+	{ .fw_name = "uart3clk", .name = "uart3clk" },
+};
+
+static const struct clk_parent_data uart5clk[] = {
+	{ .fw_name = "uart5clk", .name = "uart5clk" },
+};
+
+static const struct clk_parent_data uart4clk[] = {
+	{ .fw_name = "uart4clk", .name = "uart4clk" },
+};
+
+static const struct clk_parent_data uart6clk[] = {
+	{ .fw_name = "uart6clk", .name = "uart6clk" },
+};
+
+static const struct clk_parent_data uart7clk[] = {
+	{ .fw_name = "uart7clk", .name = "uart7clk" },
+};
+
+static const struct clk_parent_data uart8clk[] = {
+	{ .fw_name = "uart8clk", .name = "uart8clk" },
+};
+
+static const struct clk_parent_data uart9clk[] = {
+	{ .fw_name = "uart9clk", .name = "uart9clk" },
+};
+
+static const struct clk_parent_data uart10clk[] = {
+	{ .fw_name = "uart10clk", .name = "uart10clk" },
+};
+
+static const struct clk_parent_data uart11clk[] = {
+	{ .fw_name = "uart11clk", .name = "uart11clk" },
+};
+
+static const struct clk_parent_data uart12clk[] = {
+	{ .fw_name = "uart12clk", .name = "uart12clk" },
+};
+
+static const struct clk_parent_data soc1_i3c[] = {
+	{ .fw_name = "soc1-i3c", .name = "soc1-i3c" },
+};
+
+static const struct clk_parent_data canclk[] = {
+	{ .fw_name = "canclk", .name = "canclk" },
+};
+
+static const struct clk_parent_data rmii[] = {
+	{ .fw_name = "rmii", .name = "rmii" },
+};
+
+static const struct clk_parent_data hclk_clk_sels[] = {
+	{ .fw_name = "soc0-hpll", .name = "soc0-hpll" },
+	{ .fw_name = "soc0-mpll", .name = "soc0-mpll" },
+};
+
+static const struct clk_parent_data mhpll_clk_sels[] = {
+	{ .fw_name = "soc0-mpll", .name = "soc0-mpll" },
+	{ .fw_name = "soc0-hpll", .name = "soc0-hpll" },
+};
+
+static const struct clk_parent_data mphy_clk_sels[] = {
+	{ .fw_name = "soc0-mpll", .name = "soc0-mpll" },
+	{ .fw_name = "soc0-hpll", .name = "soc0-hpll" },
+	{ .fw_name = "soc0-dpll", .name = "soc0-dpll" },
+	{ .fw_name = "soc0-clk192Mhz", .name = "soc0-clk192Mhz" },
+};
+
+static const struct clk_parent_data psp_clk_sels[] = {
+	{ .fw_name = "soc0-mpll", .name = "soc0-mpll" },
+	{ .fw_name = "soc0-hpll", .name = "soc0-hpll" },
+	{ .fw_name = "soc0-hpll", .name = "soc0-hpll" },
+	{ .fw_name = "soc0-hpll", .name = "soc0-hpll" },
+	{ .fw_name = "soc0-mpll_div2", .name = "soc0-mpll_div2" },
+	{ .fw_name = "soc0-hpll_div2", .name = "soc0-hpll_div2" },
+	{ .fw_name = "soc0-hpll", .name = "soc0-hpll" },
+	{ .fw_name = "soc0-hpll", .name = "soc0-hpll" },
+};
+
+static const struct clk_parent_data uart_clk_sels[] = {
+	{ .fw_name = "soc0-clk24Mhz", .name = "soc0-clk24Mhz" },
+	{ .fw_name = "soc0-clk192Mhz", .name = "soc0-clk192Mhz" },
+};
+
+static const struct clk_parent_data emmc_clk_sels[] = {
+	{ .fw_name = "soc0-mpll_div4", .name = "soc0-mpll_div4" },
+	{ .fw_name = "soc0-hpll_div4", .name = "soc0-hpll_div4" },
+};
+
+static const struct clk_parent_data sdio_clk_sels[] = {
+	{ .fw_name = "soc1-hpll", .name = "soc1-hpll" },
+	{ .fw_name = "soc1-apll", .name = "soc1-apll" },
+};
+
+static const struct clk_parent_data ux_clk_sels[] = {
+	{ .fw_name = "soc1-apll_div4", .name = "soc1-apll_div4" },
+	{ .fw_name = "soc1-apll_div2", .name = "soc1-apll_div2" },
+	{ .fw_name = "soc1-apll", .name = "soc1-apll" },
+	{ .fw_name = "soc1-hpll", .name = "soc1-hpll" },
+};
+
+static const struct clk_parent_data uartx_clk_sels[] = {
+	{ .fw_name = "uartxclk", .name = "uartxclk" },
+	{ .fw_name = "huartxclk", .name = "huartxclk" },
+};
+
+#define FIXED_CLK(_id, _name, _rate) \
+	[_id] = { \
+		.type = CLK_FIXED, \
+		.name = _name, \
+		.data = { .rate = { .fixed_rate = _rate, } }, \
+	}
+
+#define PLL_CLK(_id, _type, _name, _parent, _reg) \
+	[_id] = { \
+		.type = _type, \
+		.name = _name, \
+		.data = { .pll = { .parent = _parent, .reg = _reg, } }, \
+	}
+
+#define MUX_CLK(_id, _name, _parents, _num_parents, _reg, _shift, _width) \
+	[_id] = { \
+		.type = CLK_MUX, \
+		.name = _name, \
+		.data = { \
+			.mux = { \
+				.parents = _parents, \
+				.num_parents = _num_parents, \
+				.reg = _reg, \
+				.bit_shift = _shift, \
+				.bit_width = _width, \
+			}, \
+		}, \
+	}
+
+#define DIVIDER_CLK(_id, _name, _parent, _reg, _shift, _width, _div_table) \
+	[_id] = { \
+		.type = CLK_DIVIDER, \
+		.name = _name, \
+		.data = { \
+			.div = { \
+				.parent = _parent, \
+				.reg = _reg, \
+				.bit_shift = _shift, \
+				.bit_width = _width, \
+				.div_table = _div_table, \
+			}, \
+		}, \
+	}
+
+#define FIXED_FACTOR_CLK(_id, _name, _parent, _mult, _div) \
+	[_id] = { \
+		.type = CLK_FIXED_FACTOR, \
+		.name = _name, \
+		.data = { .factor = { .parent = _parent, .mult = _mult, .div = _div, } }, \
+	}
+
+#define GATE_CLK(_id, _type, _name, _parent, _reg, _bit, _flags) \
+	[_id] = { \
+		.type = _type, \
+		.name = _name, \
+		.data = { \
+			.gate = { \
+				.parent = _parent, \
+				.reg = _reg, \
+				.bit = _bit, \
+				.flags = _flags, \
+			}, \
+		}, \
+	}
+
+static const struct ast2700_clk_info ast2700_scu0_clk_info[] __initconst = {
+	FIXED_CLK(SCU0_CLKIN, "soc0-clkin", SCU_CLK_25MHZ),
+	FIXED_CLK(SCU0_CLK_24M, "soc0-clk24Mhz", SCU_CLK_24MHZ),
+	FIXED_CLK(SCU0_CLK_192M, "soc0-clk192Mhz", SCU_CLK_192MHZ),
+	FIXED_CLK(SCU0_CLK_U2PHY_CLK12M, "u2phy_clk12m", SCU_CLK_12MHZ),
+	PLL_CLK(SCU0_CLK_HPLL, CLK_HPLL, "soc0-hpll", soc0_clkin, SCU0_HPLL_PARAM),
+	PLL_CLK(SCU0_CLK_DPLL, CLK_PLL, "soc0-dpll", soc0_clkin, SCU0_DPLL_PARAM),
+	PLL_CLK(SCU0_CLK_MPLL, CLK_PLL, "soc0-mpll", soc0_clkin, SCU0_MPLL_PARAM),
+	PLL_CLK(SCU0_CLK_D0, DCLK_FIXED, "d0clk", NULL, SCU0_D0CLK_PARAM),
+	PLL_CLK(SCU0_CLK_D1, DCLK_FIXED, "d1clk", NULL, SCU0_D1CLK_PARAM),
+	PLL_CLK(SCU0_CLK_CRT0, DCLK_FIXED, "crt0clk", NULL, SCU0_CRT0CLK_PARAM),
+	PLL_CLK(SCU0_CLK_CRT1, DCLK_FIXED, "crt1clk", NULL, SCU0_CRT1CLK_PARAM),
+	PLL_CLK(SCU0_CLK_MPHY, CLK_MISC, "mphyclk", mphysrc, SCU0_MPHYCLK_PARAM),
+	PLL_CLK(SCU0_CLK_U2PHY_REFCLK, CLK_MISC, "u2phy_refclk", u2phy_refclksrc, SCU0_CLK_SEL2),
+	FIXED_FACTOR_CLK(SCU0_CLK_HPLL_DIV2, "soc0-hpll_div2", soc0_hpll, 1, 2),
+	FIXED_FACTOR_CLK(SCU0_CLK_HPLL_DIV4, "soc0-hpll_div4", soc0_hpll, 1, 4),
+	FIXED_FACTOR_CLK(SCU0_CLK_MPLL_DIV2, "soc0-mpll_div2", soc0_mpll, 1, 2),
+	FIXED_FACTOR_CLK(SCU0_CLK_MPLL_DIV4, "soc0-mpll_div4", soc0_mpll, 1, 4),
+	FIXED_FACTOR_CLK(SCU0_CLK_MPLL_DIV8, "soc0-mpll_div8", soc0_mpll, 1, 8),
+	FIXED_FACTOR_CLK(SCU0_CLK_AXI0, "axi0clk", pspclk, 1, 2),
+	FIXED_FACTOR_CLK(SCU0_CLK_AXI1, "axi1clk", soc0_mpll, 1, 4),
+	DIVIDER_CLK(SCU0_CLK_AHB, "soc0-ahb", soc0_ahbmux,
+		    SCU0_HWSTRAP1, 5, 2, ast2700_hclk_div_table),
+	DIVIDER_CLK(SCU0_CLK_EMMC, "emmcclk", emmcsrc_mux,
+		    SCU0_CLK_SEL1, 12, 3, ast2700_clk_div_table2),
+	DIVIDER_CLK(SCU0_CLK_APB, "soc0-apb", axi0clk,
+		    SCU0_CLK_SEL1, 23, 3, ast2700_clk_div_table2),
+	DIVIDER_CLK(SCU0_CLK_UART4, "uart4clk", soc0_uartclk,
+		    SCU0_CLK_SEL2, 30, 1, ast2700_clk_uart_div_table),
+	MUX_CLK(SCU0_CLK_PSP, "pspclk", psp_clk_sels, ARRAY_SIZE(psp_clk_sels),
+		SCU0_HWSTRAP1, 2, 3),
+	MUX_CLK(SCU0_CLK_AHBMUX, "soc0-ahbmux", hclk_clk_sels, ARRAY_SIZE(hclk_clk_sels),
+		SCU0_HWSTRAP1, 7, 1),
+	MUX_CLK(SCU0_CLK_EMMCMUX, "emmcsrc-mux", emmc_clk_sels, ARRAY_SIZE(emmc_clk_sels),
+		SCU0_CLK_SEL1, 11, 1),
+	MUX_CLK(SCU0_CLK_MPHYSRC, "mphysrc", mphy_clk_sels, ARRAY_SIZE(mphy_clk_sels),
+		SCU0_CLK_SEL2, 18, 2),
+	MUX_CLK(SCU0_CLK_U2PHY_REFCLKSRC, "u2phy_refclksrc", mhpll_clk_sels,
+		ARRAY_SIZE(mhpll_clk_sels), SCU0_CLK_SEL2, 23, 1),
+	MUX_CLK(SCU0_CLK_UART, "soc0-uartclk", uart_clk_sels, ARRAY_SIZE(uart_clk_sels),
+		SCU0_CLK_SEL2, 14, 1),
+	GATE_CLK(SCU0_CLK_GATE_MCLK, CLK_GATE_ASPEED, "mclk-gate", soc0_mpll,
+		 SCU0_CLK_STOP, 0, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_ECLK, CLK_GATE_ASPEED, "eclk-gate", NULL, SCU0_CLK_STOP, 1, 0),
+	GATE_CLK(SCU0_CLK_GATE_2DCLK, CLK_GATE_ASPEED, "gclk-gate", NULL, SCU0_CLK_STOP, 2, 0),
+	GATE_CLK(SCU0_CLK_GATE_VCLK, CLK_GATE_ASPEED, "vclk-gate", NULL, SCU0_CLK_STOP, 3, 0),
+	GATE_CLK(SCU0_CLK_GATE_BCLK, CLK_GATE_ASPEED, "bclk-gate", NULL,
+		 SCU0_CLK_STOP, 4, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_VGA0CLK,  CLK_GATE_ASPEED, "vga0clk-gate", NULL,
+		 SCU0_CLK_STOP, 5, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_REFCLK,  CLK_GATE_ASPEED, "soc0-refclk-gate", soc0_clkin,
+		 SCU0_CLK_STOP, 6, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_PORTBUSB2CLK, CLK_GATE_ASPEED, "portb-usb2clk-gate", NULL,
+		 SCU0_CLK_STOP, 7, 0),
+	GATE_CLK(SCU0_CLK_GATE_UHCICLK, CLK_GATE_ASPEED, "uhciclk-gate", NULL, SCU0_CLK_STOP, 9, 0),
+	GATE_CLK(SCU0_CLK_GATE_VGA1CLK, CLK_GATE_ASPEED, "vga1clk-gate", NULL,
+		 SCU0_CLK_STOP, 10, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_DDRPHYCLK, CLK_GATE_ASPEED, "ddrphy-gate", NULL,
+		 SCU0_CLK_STOP, 11, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_E2M0CLK, CLK_GATE_ASPEED, "e2m0clk-gate", NULL,
+		 SCU0_CLK_STOP, 12, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_HACCLK, CLK_GATE_ASPEED, "hacclk-gate", NULL, SCU0_CLK_STOP, 13, 0),
+	GATE_CLK(SCU0_CLK_GATE_PORTAUSB2CLK, CLK_GATE_ASPEED, "porta-usb2clk-gate", NULL,
+		 SCU0_CLK_STOP, 14, 0),
+	GATE_CLK(SCU0_CLK_GATE_UART4CLK, CLK_GATE_ASPEED, "uart4clk-gate", uart4clk,
+		 SCU0_CLK_STOP, 15, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_SLICLK, CLK_GATE_ASPEED, "soc0-sliclk-gate", NULL,
+		 SCU0_CLK_STOP, 16, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_DACCLK, CLK_GATE_ASPEED, "dacclk-gate", NULL,
+		 SCU0_CLK_STOP, 17, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_DP, CLK_GATE_ASPEED, "dpclk-gate", NULL,
+		 SCU0_CLK_STOP, 18, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_E2M1CLK, CLK_GATE_ASPEED, "e2m1clk-gate", NULL,
+		 SCU0_CLK_STOP, 19, CLK_IS_CRITICAL),
+	GATE_CLK(SCU0_CLK_GATE_CRT0CLK, CLK_GATE_ASPEED, "crt0clk-gate", NULL,
+		 SCU0_CLK_STOP, 20, 0),
+	GATE_CLK(SCU0_CLK_GATE_CRT1CLK, CLK_GATE_ASPEED, "crt1clk-gate", NULL,
+		 SCU0_CLK_STOP, 21, 0),
+	GATE_CLK(SCU0_CLK_GATE_ECDSACLK, CLK_GATE_ASPEED, "eccclk-gate", NULL,
+		 SCU0_CLK_STOP, 23, 0),
+	GATE_CLK(SCU0_CLK_GATE_RSACLK, CLK_GATE_ASPEED, "rsaclk-gate", NULL,
+		 SCU0_CLK_STOP, 24, 0),
+	GATE_CLK(SCU0_CLK_GATE_RVAS0CLK, CLK_GATE_ASPEED, "rvas0clk-gate", NULL,
+		 SCU0_CLK_STOP, 25, 0),
+	GATE_CLK(SCU0_CLK_GATE_UFSCLK, CLK_GATE_ASPEED, "ufsclk-gate", NULL,
+		 SCU0_CLK_STOP, 26, 0),
+	GATE_CLK(SCU0_CLK_GATE_EMMCCLK, CLK_GATE_ASPEED, "emmcclk-gate", emmcclk,
+		 SCU0_CLK_STOP, 27, 0),
+	GATE_CLK(SCU0_CLK_GATE_RVAS1CLK, CLK_GATE_ASPEED, "rvas1clk-gate", NULL,
+		 SCU0_CLK_STOP, 28, 0),
+};
+
+static const struct ast2700_clk_info ast2700_scu1_clk_info[] __initconst = {
+	FIXED_CLK(SCU1_CLKIN, "soc1-clkin", SCU_CLK_25MHZ),
+	PLL_CLK(SCU1_CLK_HPLL, CLK_PLL, "soc1-hpll", soc1_clkin, SCU1_HPLL_PARAM),
+	PLL_CLK(SCU1_CLK_APLL, CLK_PLL, "soc1-apll", soc1_clkin, SCU1_APLL_PARAM),
+	PLL_CLK(SCU1_CLK_DPLL, CLK_PLL, "soc1-dpll", soc1_clkin, SCU1_DPLL_PARAM),
+	PLL_CLK(SCU1_CLK_UARTX, CLK_UART_PLL, "uartxclk", uxclk, SCU1_UXCLK_CTRL),
+	PLL_CLK(SCU1_CLK_HUARTX, CLK_UART_PLL, "huartxclk", huxclk, SCU1_HUXCLK_CTRL),
+	FIXED_FACTOR_CLK(SCU1_CLK_APLL_DIV2, "soc1-apll_div2", soc1_apll, 1, 2),
+	FIXED_FACTOR_CLK(SCU1_CLK_APLL_DIV4, "soc1-apll_div4", soc1_apll, 1, 4),
+	FIXED_FACTOR_CLK(SCU1_CLK_UART13, "uart13clk", huartxclk, 1, 1),
+	FIXED_FACTOR_CLK(SCU1_CLK_UART14, "uart14clk", huartxclk, 1, 1),
+	FIXED_FACTOR_CLK(SCU1_CLK_CAN, "canclk", soc1_apll, 1, 10),
+	DIVIDER_CLK(SCU1_CLK_SDCLK, "sdclk", sdclk_mux,
+		    SCU1_CLK_SEL1, 14, 3, ast2700_clk_div_table),
+	DIVIDER_CLK(SCU1_CLK_APB, "soc1-apb", soc1_hpll,
+		    SCU1_CLK_SEL1, 18, 3, ast2700_clk_div_table2),
+	DIVIDER_CLK(SCU1_CLK_RMII, "rmii", soc1_hpll,
+		    SCU1_CLK_SEL1, 21, 3, ast2700_rmii_div_table),
+	DIVIDER_CLK(SCU1_CLK_RGMII, "rgmii", soc1_hpll,
+		    SCU1_CLK_SEL1, 25, 3, ast2700_rgmii_div_table),
+	DIVIDER_CLK(SCU1_CLK_MACHCLK, "machclk", soc1_hpll,
+		    SCU1_CLK_SEL1, 29, 3, ast2700_clk_div_table),
+	DIVIDER_CLK(SCU1_CLK_APLL_DIVN, "soc1-apll_divn", soc1_apll,
+		    SCU1_CLK_SEL2, 8, 3, ast2700_clk_div_table),
+	DIVIDER_CLK(SCU1_CLK_AHB, "soc1-ahb", soc1_hpll,
+		    SCU1_CLK_SEL2, 20, 3, ast2700_clk_div_table),
+	DIVIDER_CLK(SCU1_CLK_I3C, "soc1-i3c", soc1_hpll,
+		    SCU1_CLK_SEL2, 23, 3, ast2700_clk_div_table),
+	MUX_CLK(SCU1_CLK_UART0, "uart0clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 0, 1),
+	MUX_CLK(SCU1_CLK_UART1, "uart1clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 1, 1),
+	MUX_CLK(SCU1_CLK_UART2, "uart2clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 2, 1),
+	MUX_CLK(SCU1_CLK_UART3, "uart3clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 3, 1),
+	MUX_CLK(SCU1_CLK_UART5, "uart5clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 5, 1),
+	MUX_CLK(SCU1_CLK_UART6, "uart6clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 6, 1),
+	MUX_CLK(SCU1_CLK_UART7, "uart7clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 7, 1),
+	MUX_CLK(SCU1_CLK_UART8, "uart8clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 8, 1),
+	MUX_CLK(SCU1_CLK_UART9, "uart9clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 9, 1),
+	MUX_CLK(SCU1_CLK_UART10, "uart10clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 10, 1),
+	MUX_CLK(SCU1_CLK_UART11, "uart11clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 11, 1),
+	MUX_CLK(SCU1_CLK_UART12, "uart12clk", uartx_clk_sels, ARRAY_SIZE(uartx_clk_sels),
+		SCU1_CLK_SEL1, 12, 1),
+	MUX_CLK(SCU1_CLK_SDMUX, "sdclk-mux", sdio_clk_sels, ARRAY_SIZE(sdio_clk_sels),
+		SCU1_CLK_SEL1, 13, 1),
+	MUX_CLK(SCU1_CLK_UXCLK, "uxclk", ux_clk_sels, ARRAY_SIZE(ux_clk_sels),
+		SCU1_CLK_SEL2, 0, 2),
+	MUX_CLK(SCU1_CLK_HUXCLK, "huxclk", ux_clk_sels, ARRAY_SIZE(ux_clk_sels),
+		SCU1_CLK_SEL2, 3, 2),
+	GATE_CLK(SCU1_CLK_MAC0RCLK, CLK_GATE, "mac0rclk-gate", rmii, SCU1_MAC12_CLK_DLY, 29, 0),
+	GATE_CLK(SCU1_CLK_MAC1RCLK, CLK_GATE, "mac1rclk-gate", rmii, SCU1_MAC12_CLK_DLY, 30, 0),
+	GATE_CLK(SCU1_CLK_GATE_LCLK0, CLK_GATE_ASPEED, "lclk0-gate", NULL,
+		 SCU1_CLK_STOP, 0, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_LCLK1, CLK_GATE_ASPEED, "lclk1-gate", NULL,
+		 SCU1_CLK_STOP, 1, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_ESPI0CLK, CLK_GATE_ASPEED, "espi0clk-gate", NULL,
+		 SCU1_CLK_STOP, 2, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_ESPI1CLK, CLK_GATE_ASPEED, "espi1clk-gate", NULL,
+		 SCU1_CLK_STOP, 3, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_SDCLK, CLK_GATE_ASPEED, "sdclk-gate", sdclk,
+		 SCU1_CLK_STOP, 4, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_IPEREFCLK, CLK_GATE_ASPEED, "soc1-iperefclk-gate", NULL,
+		 SCU1_CLK_STOP, 5, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_REFCLK, CLK_GATE_ASPEED, "soc1-refclk-gate", NULL,
+		 SCU1_CLK_STOP, 6, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_LPCHCLK, CLK_GATE_ASPEED, "lpchclk-gate", NULL,
+		 SCU1_CLK_STOP, 7, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_MAC0CLK, CLK_GATE_ASPEED, "mac0clk-gate", NULL,
+		 SCU1_CLK_STOP, 8, 0),
+	GATE_CLK(SCU1_CLK_GATE_MAC1CLK, CLK_GATE_ASPEED, "mac1clk-gate", NULL,
+		 SCU1_CLK_STOP, 9, 0),
+	GATE_CLK(SCU1_CLK_GATE_MAC2CLK, CLK_GATE_ASPEED, "mac2clk-gate", NULL,
+		 SCU1_CLK_STOP, 10, 0),
+	GATE_CLK(SCU1_CLK_GATE_UART0CLK, CLK_GATE_ASPEED, "uart0clk-gate", uart0clk,
+		 SCU1_CLK_STOP, 11, 0),
+	GATE_CLK(SCU1_CLK_GATE_UART1CLK, CLK_GATE_ASPEED, "uart1clk-gate", uart1clk,
+		 SCU1_CLK_STOP, 12, 0),
+	GATE_CLK(SCU1_CLK_GATE_UART2CLK, CLK_GATE_ASPEED, "uart2clk-gate", uart2clk,
+		 SCU1_CLK_STOP, 13, 0),
+	GATE_CLK(SCU1_CLK_GATE_UART3CLK, CLK_GATE_ASPEED, "uart3clk-gate", uart3clk,
+		 SCU1_CLK_STOP, 14, 0),
+	GATE_CLK(SCU1_CLK_GATE_I2CCLK, CLK_GATE_ASPEED, "i2cclk-gate", NULL, SCU1_CLK_STOP, 15, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C0CLK, CLK_GATE_ASPEED, "i3c0clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 16, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C1CLK, CLK_GATE_ASPEED, "i3c1clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 17, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C2CLK, CLK_GATE_ASPEED, "i3c2clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 18, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C3CLK, CLK_GATE_ASPEED, "i3c3clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 19, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C4CLK, CLK_GATE_ASPEED, "i3c4clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 20, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C5CLK, CLK_GATE_ASPEED, "i3c5clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 21, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C6CLK, CLK_GATE_ASPEED, "i3c6clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 22, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C7CLK, CLK_GATE_ASPEED, "i3c7clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 23, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C8CLK, CLK_GATE_ASPEED, "i3c8clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 24, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C9CLK, CLK_GATE_ASPEED, "i3c9clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 25, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C10CLK, CLK_GATE_ASPEED, "i3c10clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 26, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C11CLK, CLK_GATE_ASPEED, "i3c11clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 27, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C12CLK, CLK_GATE_ASPEED, "i3c12clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 28, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C13CLK, CLK_GATE_ASPEED, "i3c13clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 29, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C14CLK, CLK_GATE_ASPEED, "i3c14clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 30, 0),
+	GATE_CLK(SCU1_CLK_GATE_I3C15CLK, CLK_GATE_ASPEED, "i3c15clk-gate", soc1_i3c,
+		 SCU1_CLK_STOP, 31, 0),
+	GATE_CLK(SCU1_CLK_GATE_UART5CLK, CLK_GATE_ASPEED, "uart5clk-gate", uart5clk,
+		 SCU1_CLK_STOP2, 0, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_UART6CLK, CLK_GATE_ASPEED, "uart6clk-gate", uart6clk,
+		 SCU1_CLK_STOP2, 1, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_UART7CLK, CLK_GATE_ASPEED, "uart7clk-gate", uart7clk,
+		 SCU1_CLK_STOP2, 2, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_UART8CLK, CLK_GATE_ASPEED, "uart8clk-gate", uart8clk,
+		 SCU1_CLK_STOP2, 3, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_UART9CLK, CLK_GATE_ASPEED, "uart9clk-gate", uart9clk,
+		 SCU1_CLK_STOP2, 4, 0),
+	GATE_CLK(SCU1_CLK_GATE_UART10CLK, CLK_GATE_ASPEED, "uart10clk-gate", uart10clk,
+		 SCU1_CLK_STOP2, 5, 0),
+	GATE_CLK(SCU1_CLK_GATE_UART11CLK, CLK_GATE_ASPEED, "uart11clk-gate", uart11clk,
+		 SCU1_CLK_STOP2, 6, 0),
+	GATE_CLK(SCU1_CLK_GATE_UART12CLK, CLK_GATE_ASPEED, "uart12clk-gate", uart12clk,
+		 SCU1_CLK_STOP2, 7, 0),
+	GATE_CLK(SCU1_CLK_GATE_FSICLK, CLK_GATE_ASPEED, "fsiclk-gate", NULL, SCU1_CLK_STOP2, 8, 0),
+	GATE_CLK(SCU1_CLK_GATE_LTPIPHYCLK, CLK_GATE_ASPEED, "ltpiphyclk-gate", NULL,
+		 SCU1_CLK_STOP2, 9, 0),
+	GATE_CLK(SCU1_CLK_GATE_LTPICLK, CLK_GATE_ASPEED, "ltpiclk-gate", NULL,
+		 SCU1_CLK_STOP2, 10, 0),
+	GATE_CLK(SCU1_CLK_GATE_VGALCLK, CLK_GATE_ASPEED, "vgalclk-gate", NULL,
+		 SCU1_CLK_STOP2, 11, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_UHCICLK, CLK_GATE_ASPEED, "usbuartclk-gate", NULL,
+		 SCU1_CLK_STOP2, 12, 0),
+	GATE_CLK(SCU1_CLK_GATE_CANCLK, CLK_GATE_ASPEED, "canclk-gate", canclk,
+		 SCU1_CLK_STOP2, 13, 0),
+	GATE_CLK(SCU1_CLK_GATE_PCICLK, CLK_GATE_ASPEED, "pciclk-gate", NULL,
+		 SCU1_CLK_STOP2, 14, 0),
+	GATE_CLK(SCU1_CLK_GATE_SLICLK, CLK_GATE_ASPEED, "soc1-sliclk-gate", NULL,
+		 SCU1_CLK_STOP2, 15, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_E2MCLK, CLK_GATE_ASPEED, "soc1-e2m-gate", NULL,
+		 SCU1_CLK_STOP2, 16, CLK_IS_CRITICAL),
+	GATE_CLK(SCU1_CLK_GATE_PORTCUSB2CLK, CLK_GATE_ASPEED, "portcusb2-gate", NULL,
+		 SCU1_CLK_STOP2, 17, 0),
+	GATE_CLK(SCU1_CLK_GATE_PORTDUSB2CLK, CLK_GATE_ASPEED, "portdusb2-gate", NULL,
+		 SCU1_CLK_STOP2, 18, 0),
+	GATE_CLK(SCU1_CLK_GATE_LTPI1TXCLK, CLK_GATE_ASPEED, "ltp1tx-gate", NULL,
+		 SCU1_CLK_STOP2, 19, 0),
+};
+
+static struct clk_hw *ast2700_clk_hw_register_hpll(void __iomem *reg,
+						   const char *name, const char *parent_name,
+						   struct ast2700_clk_ctrl *clk_ctrl)
+{
+	unsigned int mult, div;
+	u32 val;
+
+	val = readl(clk_ctrl->base + SCU0_HWSTRAP1);
+	if ((readl(clk_ctrl->base) & REVISION_ID) && (val & BIT(3))) {
+		switch ((val & GENMASK(4, 2)) >> 2) {
+		case 2:
+			return devm_clk_hw_register_fixed_rate(clk_ctrl->dev, name, NULL,
+							       0, 1800 * HZ_PER_MHZ);
+		case 3:
+			return devm_clk_hw_register_fixed_rate(clk_ctrl->dev, name, NULL,
+							       0, 1700 * HZ_PER_MHZ);
+		case 6:
+			return devm_clk_hw_register_fixed_rate(clk_ctrl->dev, name, NULL,
+							       0, 1200 * HZ_PER_MHZ);
+		case 7:
+			return devm_clk_hw_register_fixed_rate(clk_ctrl->dev, name, NULL,
+							       0, 800 * HZ_PER_MHZ);
+		default:
+			return ERR_PTR(-EINVAL);
+		}
+	} else if ((val & GENMASK(3, 2)) != 0) {
+		switch ((val & GENMASK(3, 2)) >> 2) {
+		case 1:
+			return devm_clk_hw_register_fixed_rate(clk_ctrl->dev, name, NULL,
+							       0, 1900 * HZ_PER_MHZ);
+		case 2:
+			return devm_clk_hw_register_fixed_rate(clk_ctrl->dev, name, NULL,
+							       0, 1800 * HZ_PER_MHZ);
+		case 3:
+			return devm_clk_hw_register_fixed_rate(clk_ctrl->dev, name, NULL,
+							       0, 1700 * HZ_PER_MHZ);
+		default:
+			return ERR_PTR(-EINVAL);
+		}
+	} else {
+		val = readl(reg);
+
+		if (val & BIT(24)) {
+			/* Pass through mode */
+			mult = 1;
+			div = 1;
+		} else {
+			u32 m = val & 0x1fff;
+			u32 n = (val >> 13) & 0x3f;
+			u32 p = (val >> 19) & 0xf;
+
+			mult = (m + 1) / (2 * (n + 1));
+			div = (p + 1);
+		}
+	}
+
+	return devm_clk_hw_register_fixed_factor(clk_ctrl->dev, name, parent_name, 0, mult, div);
+}
+
+static struct clk_hw *ast2700_clk_hw_register_pll(int clk_idx, void __iomem *reg,
+						  const char *name, const char *parent_name,
+						  struct ast2700_clk_ctrl *clk_ctrl)
+{
+	int scu = clk_ctrl->clk_data->scu;
+	unsigned int mult, div;
+	u32 val = readl(reg);
+
+	if (val & BIT(24)) {
+		/* Pass through mode */
+		mult = 1;
+		div = 1;
+	} else {
+		u32 m = val & 0x1fff;
+		u32 n = (val >> 13) & 0x3f;
+		u32 p = (val >> 19) & 0xf;
+
+		if (scu) {
+			mult = (m + 1) / (n + 1);
+			div = (p + 1);
+		} else {
+			if (clk_idx == SCU0_CLK_MPLL) {
+				mult = m / (n + 1);
+				div = (p + 1);
+			} else {
+				mult = (m + 1) / (2 * (n + 1));
+				div = (p + 1);
+			}
+		}
+	}
+
+	return devm_clk_hw_register_fixed_factor(clk_ctrl->dev, name, parent_name, 0, mult, div);
+}
+
+static struct clk_hw *ast2700_clk_hw_register_dclk(void __iomem *reg, const char *name,
+						   struct ast2700_clk_ctrl *clk_ctrl)
+{
+	unsigned int mult, div, r, n;
+	u32 xdclk;
+	u32 val;
+
+	val = readl(clk_ctrl->base + 0x284);
+	if (val & BIT(29))
+		xdclk = 800 * HZ_PER_MHZ;
+	else
+		xdclk = 1000 * HZ_PER_MHZ;
+
+	val = readl(reg);
+	r = val & GENMASK(15, 0);
+	n = (val >> 16) & GENMASK(15, 0);
+	mult = r;
+	div = 2 * n;
+
+	return devm_clk_hw_register_fixed_rate(clk_ctrl->dev, name, NULL, 0, (xdclk * mult) / div);
+}
+
+static struct clk_hw *ast2700_clk_hw_register_uartpll(void __iomem *reg,
+						      const char *name, const char *parent_name,
+						      struct ast2700_clk_ctrl *clk_ctrl)
+{
+	unsigned int mult, div;
+	u32 val = readl(reg);
+	u32 r = val & 0xff;
+	u32 n = (val >> 8) & 0x3ff;
+
+	mult = r;
+	div = n * 2;
+
+	return devm_clk_hw_register_fixed_factor(clk_ctrl->dev, name,
+						 parent_name, 0, mult, div);
+}
+
+static struct clk_hw *ast2700_clk_hw_register_misc(int clk_idx, void __iomem *reg,
+						   const char *name, const char *parent_name,
+						   struct ast2700_clk_ctrl *clk_ctrl)
+{
+	u32 div = 0;
+
+	if (clk_idx == SCU0_CLK_MPHY) {
+		div = readl(reg) + 1;
+	} else if (clk_idx == SCU0_CLK_U2PHY_REFCLK) {
+		if (readl(clk_ctrl->base) & REVISION_ID)
+			div = (GET_USB_REFCLK_DIV(readl(reg)) + 1) << 4;
+		else
+			div = (GET_USB_REFCLK_DIV(readl(reg)) + 1) << 1;
+	} else {
+		return ERR_PTR(-EINVAL);
+	}
+
+	return devm_clk_hw_register_fixed_factor(clk_ctrl->dev, name,
+						   parent_name, 0, 1, div);
+}
+
+static int ast2700_clk_is_enabled(struct clk_hw *hw)
+{
+	struct clk_gate *gate = to_clk_gate(hw);
+	u32 clk = BIT(gate->bit_idx);
+	u32 reg;
+
+	reg = readl(gate->reg);
+
+	return !(reg & clk);
+}
+
+static int ast2700_clk_enable(struct clk_hw *hw)
+{
+	struct clk_gate *gate = to_clk_gate(hw);
+	u32 clk = BIT(gate->bit_idx);
+
+	if (readl(gate->reg) & clk)
+		writel(clk, gate->reg + 0x04);
+
+	return 0;
+}
+
+static void ast2700_clk_disable(struct clk_hw *hw)
+{
+	struct clk_gate *gate = to_clk_gate(hw);
+	u32 clk = BIT(gate->bit_idx);
+
+	/* Clock is set to enable, so use write to set register */
+	writel(clk, gate->reg);
+}
+
+static const struct clk_ops ast2700_clk_gate_ops = {
+	.enable = ast2700_clk_enable,
+	.disable = ast2700_clk_disable,
+	.is_enabled = ast2700_clk_is_enabled,
+};
+
+static struct clk_hw *ast2700_clk_hw_register_gate(struct device *dev, const char *name,
+						   const struct clk_parent_data	*parent,
+						   void __iomem *reg, u8 clock_idx,
+						   unsigned long clk_gate_flags, spinlock_t *lock)
+{
+	struct clk_gate *gate;
+	struct clk_hw *hw;
+	struct clk_init_data init;
+	int ret = -EINVAL;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &ast2700_clk_gate_ops;
+	init.flags = clk_gate_flags;
+	init.parent_names = parent ? &parent->name : NULL;
+	init.num_parents = parent ? 1 : 0;
+
+	gate->reg = reg;
+	gate->bit_idx = clock_idx;
+	gate->flags = 0;
+	gate->lock = lock;
+	gate->hw.init = &init;
+
+	hw = &gate->hw;
+	ret = clk_hw_register(dev, hw);
+	if (ret) {
+		kfree(gate);
+		hw = ERR_PTR(ret);
+	}
+
+	return hw;
+}
+
+static void ast2700_soc1_configure_i3c_clk(struct ast2700_clk_ctrl *clk_ctrl)
+{
+	if (readl(clk_ctrl->base + SCU1_REVISION_ID) & REVISION_ID)
+		/* I3C 250MHz = HPLL/4 */
+		writel((readl(clk_ctrl->base + SCU1_CLK_SEL2) &
+			~SCU1_CLK_I3C_DIV_MASK) |
+			       FIELD_PREP(SCU1_CLK_I3C_DIV_MASK,
+					  SCU1_CLK_I3C_DIV(4)),
+		       clk_ctrl->base + SCU1_CLK_SEL2);
+}
+
+static int ast2700_soc_clk_probe(struct platform_device *pdev)
+{
+	const struct ast2700_clk_data *clk_data;
+	struct ast2700_clk_ctrl *clk_ctrl;
+	struct clk_hw_onecell_data *clk_hw_data;
+	struct device *dev = &pdev->dev;
+	void __iomem *clk_base;
+	struct clk_hw **hws;
+	char *reset_name;
+	int ret;
+	int i;
+
+	clk_ctrl = devm_kzalloc(dev, sizeof(*clk_ctrl), GFP_KERNEL);
+	if (!clk_ctrl)
+		return -ENOMEM;
+	clk_ctrl->dev = dev;
+	dev_set_drvdata(&pdev->dev, clk_ctrl);
+
+	spin_lock_init(&clk_ctrl->lock);
+
+	clk_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(clk_base))
+		return PTR_ERR(clk_base);
+
+	clk_ctrl->base = clk_base;
+
+	clk_data = device_get_match_data(dev);
+	if (!clk_data)
+		return -ENODEV;
+
+	clk_ctrl->clk_data = clk_data;
+	reset_name = devm_kasprintf(dev, GFP_KERNEL, "reset%d", clk_data->scu);
+
+	clk_hw_data = devm_kzalloc(dev, struct_size(clk_hw_data, hws, clk_data->nr_clks),
+				   GFP_KERNEL);
+	if (!clk_hw_data)
+		return -ENOMEM;
+
+	clk_hw_data->num = clk_data->nr_clks;
+	hws = clk_hw_data->hws;
+
+	if (clk_data->scu)
+		ast2700_soc1_configure_i3c_clk(clk_ctrl);
+
+	for (i = 0; i < clk_data->nr_clks; i++) {
+		const struct ast2700_clk_info *clk = &clk_data->clk_info[i];
+		void __iomem *reg;
+
+		if (clk->type == CLK_FIXED) {
+			const struct ast2700_clk_fixed_rate_data *fixed_rate = &clk->data.rate;
+
+			hws[i] = devm_clk_hw_register_fixed_rate(dev, clk->name, NULL, 0,
+								 fixed_rate->fixed_rate);
+		} else if (clk->type == CLK_FIXED_FACTOR) {
+			const struct ast2700_clk_fixed_factor_data *factor = &clk->data.factor;
+
+			hws[i] = devm_clk_hw_register_fixed_factor(dev, clk->name,
+								   factor->parent->name,
+								   0, factor->mult, factor->div);
+		} else if (clk->type == DCLK_FIXED) {
+			const struct ast2700_clk_pll_data *pll = &clk->data.pll;
+
+			reg = clk_ctrl->base + pll->reg;
+			hws[i] = ast2700_clk_hw_register_dclk(reg, clk->name, clk_ctrl);
+		} else if (clk->type == CLK_HPLL) {
+			const struct ast2700_clk_pll_data *pll = &clk->data.pll;
+
+			reg = clk_ctrl->base + pll->reg;
+			hws[i] = ast2700_clk_hw_register_hpll(reg, clk->name,
+							      pll->parent->name, clk_ctrl);
+		} else if (clk->type == CLK_PLL) {
+			const struct ast2700_clk_pll_data *pll = &clk->data.pll;
+
+			reg = clk_ctrl->base + pll->reg;
+			hws[i] = ast2700_clk_hw_register_pll(i, reg, clk->name,
+							     pll->parent->name, clk_ctrl);
+		} else if (clk->type == CLK_UART_PLL) {
+			const struct ast2700_clk_pll_data *pll = &clk->data.pll;
+
+			reg = clk_ctrl->base + pll->reg;
+			hws[i] = ast2700_clk_hw_register_uartpll(reg, clk->name,
+								 pll->parent->name, clk_ctrl);
+		} else if (clk->type == CLK_MUX) {
+			const struct ast2700_clk_mux_data *mux = &clk->data.mux;
+
+			reg = clk_ctrl->base + mux->reg;
+			hws[i] = devm_clk_hw_register_mux_parent_data_table(dev, clk->name,
+									    mux->parents,
+									    mux->num_parents, 0,
+									    reg, mux->bit_shift,
+									    mux->bit_width, 0,
+									    NULL, &clk_ctrl->lock);
+		} else if (clk->type == CLK_MISC) {
+			const struct ast2700_clk_pll_data *misc = &clk->data.pll;
+
+			reg = clk_ctrl->base + misc->reg;
+			hws[i] = ast2700_clk_hw_register_misc(i, reg, clk->name,
+							      misc->parent->name, clk_ctrl);
+		} else if (clk->type == CLK_DIVIDER) {
+			const struct ast2700_clk_div_data *div = &clk->data.div;
+
+			reg = clk_ctrl->base + div->reg;
+			hws[i] = devm_clk_hw_register_divider_table(dev, clk->name,
+								    div->parent->name, 0,
+								    reg, div->bit_shift,
+								    div->bit_width, 0,
+								    div->div_table,
+								    &clk_ctrl->lock);
+		} else if (clk->type == CLK_GATE_ASPEED) {
+			const struct ast2700_clk_gate_data *gate = &clk->data.gate;
+
+			reg = clk_ctrl->base + gate->reg;
+			hws[i] = ast2700_clk_hw_register_gate(dev, clk->name, gate->parent,
+							      reg, gate->bit, gate->flags, 0);
+
+		} else {
+			const struct ast2700_clk_gate_data *gate = &clk->data.gate;
+
+			reg = clk_ctrl->base + gate->reg;
+			hws[i] = devm_clk_hw_register_gate_parent_data(dev, clk->name, gate->parent,
+								       0, reg, clk->clk_idx, 0,
+								       &clk_ctrl->lock);
+		}
+
+		if (IS_ERR(hws[i]))
+			return PTR_ERR(hws[i]);
+	}
+
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_hw_data);
+	if (ret)
+		return ret;
+
+	return aspeed_reset_controller_register(dev, clk_base, reset_name);
+}
+
+static const struct ast2700_clk_data ast2700_clk0_data = {
+	.scu = 0,
+	.nr_clks = ARRAY_SIZE(ast2700_scu0_clk_info),
+	.clk_info = ast2700_scu0_clk_info,
+};
+
+static const struct ast2700_clk_data ast2700_clk1_data = {
+	.scu = 1,
+	.nr_clks = ARRAY_SIZE(ast2700_scu1_clk_info),
+	.clk_info = ast2700_scu1_clk_info,
+};
+
+static const struct of_device_id ast2700_scu_match[] = {
+	{ .compatible = "aspeed,ast2700-scu0", .data = &ast2700_clk0_data },
+	{ .compatible = "aspeed,ast2700-scu1", .data = &ast2700_clk1_data },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, ast2700_scu_match);
+
+static struct platform_driver ast2700_scu_driver = {
+	.probe = ast2700_soc_clk_probe,
+	.driver = {
+		.name = "clk-ast2700",
+		.of_match_table = ast2700_scu_match,
+	},
+};
+
+static int __init clk_ast2700_init(void)
+{
+	return platform_driver_register(&ast2700_scu_driver);
+}
+arch_initcall(clk_ast2700_init);
-- 
2.34.1

Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock define

[Krzysztof Kozlowski]

On 24/02/2025 10:55, Ryan Chen wrote:
> -remove redundant SOC0_CLK_UART_DIV13:
> SOC0_CLK_UART_DIV13 is not use at clk-ast2700.c, the clock
> source tree is uart clk src -> uart_div_table -> uart clk.
> 
> -Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX:
> modify clock tree implement.
> older CLK_AHB use mpll_div_ahb/hpll_div_ahb to be ahb clock source.
> mpll->mpll_div_ahb
>                   -> clk_ahb
> hpll->hpll_div_ahb


I can barely understand it and from the pieces I got, it does not
explain need for ABI break.



Best regards,
Krzysztof

Re: [PATCH v9 2/3] reset: aspeed: register AST2700 reset auxiliary bus device

[Krzysztof Kozlowski]

On 24/02/2025 10:55, Ryan Chen wrote:
> +
> +static void aspeed_reset_unregister_adev(void *_adev)
> +{
> +	struct auxiliary_device *adev = _adev;
> +
> +	auxiliary_device_delete(adev);
> +	auxiliary_device_uninit(adev);
> +}
> +
> +static void aspeed_reset_adev_release(struct device *dev)
> +{
> +	struct auxiliary_device *adev = to_auxiliary_dev(dev);
> +
> +	kfree(adev);
> +}
> +

Every exported function *must* have kerneldoc.

> +int aspeed_reset_controller_register(struct device *clk_dev, void __iomem *base,
> +				     const char *adev_name)
> +{
> +	struct auxiliary_device *adev;
> +	int ret;
> +
> +	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> +	if (!adev)
> +		return -ENOMEM;
> +
> +	adev->name = adev_name;
> +	adev->dev.parent = clk_dev;
> +	adev->dev.release = aspeed_reset_adev_release;
> +	adev->id = 666u;
> +
> +	ret = auxiliary_device_init(adev);
> +	if (ret) {
> +		kfree(adev);
> +		return ret;
> +	}
> +
> +	ret = auxiliary_device_add(adev);
> +	if (ret) {
> +		auxiliary_device_uninit(adev);
> +		return ret;
> +	}
> +
> +	adev->dev.platform_data = (__force void *)base;
> +
> +	return devm_add_action_or_reset(clk_dev, aspeed_reset_unregister_adev, adev);
> +}
> +EXPORT_SYMBOL_GPL(aspeed_reset_controller_register);

No, you cannot export functions without users. There is no single user
of this, so this is not justified at all.




Best regards,
Krzysztof

Re: [PATCH v9 2/3] reset: aspeed: register AST2700 reset auxiliary bus device

[Krzysztof Kozlowski]

On 24/02/2025 11:12, Krzysztof Kozlowski wrote:
> On 24/02/2025 10:55, Ryan Chen wrote:
>> +
>> +static void aspeed_reset_unregister_adev(void *_adev)
>> +{
>> +	struct auxiliary_device *adev = _adev;
>> +
>> +	auxiliary_device_delete(adev);
>> +	auxiliary_device_uninit(adev);
>> +}
>> +
>> +static void aspeed_reset_adev_release(struct device *dev)
>> +{
>> +	struct auxiliary_device *adev = to_auxiliary_dev(dev);
>> +
>> +	kfree(adev);
>> +}
>> +
> 
> Every exported function *must* have kerneldoc.
> 
>> +int aspeed_reset_controller_register(struct device *clk_dev, void __iomem *base,
>> +				     const char *adev_name)
>> +{
>> +	struct auxiliary_device *adev;
>> +	int ret;
>> +
>> +	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
>> +	if (!adev)
>> +		return -ENOMEM;
>> +
>> +	adev->name = adev_name;
>> +	adev->dev.parent = clk_dev;
>> +	adev->dev.release = aspeed_reset_adev_release;
>> +	adev->id = 666u;
>> +
>> +	ret = auxiliary_device_init(adev);
>> +	if (ret) {
>> +		kfree(adev);
>> +		return ret;
>> +	}
>> +
>> +	ret = auxiliary_device_add(adev);
>> +	if (ret) {
>> +		auxiliary_device_uninit(adev);
>> +		return ret;
>> +	}
>> +
>> +	adev->dev.platform_data = (__force void *)base;
>> +
>> +	return devm_add_action_or_reset(clk_dev, aspeed_reset_unregister_adev, adev);
>> +}
>> +EXPORT_SYMBOL_GPL(aspeed_reset_controller_register);
> 
> No, you cannot export functions without users. There is no single user
> of this, so this is not justified at all.
My mistake, I missed patch #3 which uses it.

I don't get why do you need to export this in the first place, instead
of putting it in the clock driver, as usually expected. Handling child
creation is logically the task of the device having children, the
parent. Not the child.

Best regards,
Krzysztof

Re: [PATCH v9 2/3] reset: aspeed: register AST2700 reset auxiliary bus device

[Philipp Zabel]

On Mo, 2025-02-24 at 11:15 +0100, Krzysztof Kozlowski wrote:
> On 24/02/2025 11:12, Krzysztof Kozlowski wrote:
> > On 24/02/2025 10:55, Ryan Chen wrote:
> > > +
> > > +static void aspeed_reset_unregister_adev(void *_adev)
> > > +{
> > > +	struct auxiliary_device *adev = _adev;
> > > +
> > > +	auxiliary_device_delete(adev);
> > > +	auxiliary_device_uninit(adev);
> > > +}
> > > +
> > > +static void aspeed_reset_adev_release(struct device *dev)
> > > +{
> > > +	struct auxiliary_device *adev = to_auxiliary_dev(dev);
> > > +
> > > +	kfree(adev);
> > > +}
> > > +
> > 
> > Every exported function *must* have kerneldoc.
> > 
> > > +int aspeed_reset_controller_register(struct device *clk_dev, void __iomem *base,
> > > +				     const char *adev_name)
> > > +{
> > > +	struct auxiliary_device *adev;
> > > +	int ret;
> > > +
> > > +	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> > > +	if (!adev)
> > > +		return -ENOMEM;
> > > +
> > > +	adev->name = adev_name;
> > > +	adev->dev.parent = clk_dev;
> > > +	adev->dev.release = aspeed_reset_adev_release;
> > > +	adev->id = 666u;
> > > +
> > > +	ret = auxiliary_device_init(adev);
> > > +	if (ret) {
> > > +		kfree(adev);
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = auxiliary_device_add(adev);
> > > +	if (ret) {
> > > +		auxiliary_device_uninit(adev);
> > > +		return ret;
> > > +	}
> > > +
> > > +	adev->dev.platform_data = (__force void *)base;
> > > +
> > > +	return devm_add_action_or_reset(clk_dev, aspeed_reset_unregister_adev, adev);
> > > +}
> > > +EXPORT_SYMBOL_GPL(aspeed_reset_controller_register);
> > 
> > No, you cannot export functions without users. There is no single user
> > of this, so this is not justified at all.
> My mistake, I missed patch #3 which uses it.
> 
> I don't get why do you need to export this in the first place, instead
> of putting it in the clock driver, as usually expected. Handling child
> creation is logically the task of the device having children, the
> parent. Not the child.

Also, consider basing this on top of:

https://lore.kernel.org/all/20250218-aux-device-create-helper-v4-0-c3d7dfdea2e6@baylibre.com/

regards
Philipp

RE: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock define

[Ryan Chen]

> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock
> define
> 
> On 24/02/2025 10:55, Ryan Chen wrote:
> > -remove redundant SOC0_CLK_UART_DIV13:
> > SOC0_CLK_UART_DIV13 is not use at clk-ast2700.c, the clock source tree
> > is uart clk src -> uart_div_table -> uart clk.
> >
> > -Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX:
> > modify clock tree implement.
> > older CLK_AHB use mpll_div_ahb/hpll_div_ahb to be ahb clock source.
> > mpll->mpll_div_ahb
> >                   -> clk_ahb
> > hpll->hpll_div_ahb
> 
> 
> I can barely understand it and from the pieces I got, it does not explain need
> for ABI break.
> 

#1. SCU0_CLK_UART_DIV13 is redundant, it does not impact ABI break
#2. Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX
Older implement where `mpll_div_ahb` and `hpll_div_ahb` were **hardcoded dividers** for AHB.
In **the new approach (v8)**, I refactored the clock tree to clock tree.
It should be ABI-safe change

Or you want to keep original SOC0_CLK_HPLL_DIV_AHB define and then add SOC0_CLK_AHBMUX.
To be 1st patch, then 2n patch remove redundant SOC0_CLK_HPLL_DIV_AHB?


> 
> Best regards,
> Krzysztof

RE: [PATCH v9 2/3] reset: aspeed: register AST2700 reset auxiliary bus device

[Ryan Chen]

> Subject: Re: [PATCH v9 2/3] reset: aspeed: register AST2700 reset auxiliary bus
> device
> 
> On Mo, 2025-02-24 at 11:15 +0100, Krzysztof Kozlowski wrote:
> > On 24/02/2025 11:12, Krzysztof Kozlowski wrote:
> > > On 24/02/2025 10:55, Ryan Chen wrote:
> > > > +
> > > > +static void aspeed_reset_unregister_adev(void *_adev) {
> > > > +	struct auxiliary_device *adev = _adev;
> > > > +
> > > > +	auxiliary_device_delete(adev);
> > > > +	auxiliary_device_uninit(adev);
> > > > +}
> > > > +
> > > > +static void aspeed_reset_adev_release(struct device *dev) {
> > > > +	struct auxiliary_device *adev = to_auxiliary_dev(dev);
> > > > +
> > > > +	kfree(adev);
> > > > +}
> > > > +
> > >
> > > Every exported function *must* have kerneldoc.
> > >
> > > > +int aspeed_reset_controller_register(struct device *clk_dev, void
> __iomem *base,
> > > > +				     const char *adev_name)
> > > > +{
> > > > +	struct auxiliary_device *adev;
> > > > +	int ret;
> > > > +
> > > > +	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> > > > +	if (!adev)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	adev->name = adev_name;
> > > > +	adev->dev.parent = clk_dev;
> > > > +	adev->dev.release = aspeed_reset_adev_release;
> > > > +	adev->id = 666u;
> > > > +
> > > > +	ret = auxiliary_device_init(adev);
> > > > +	if (ret) {
> > > > +		kfree(adev);
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	ret = auxiliary_device_add(adev);
> > > > +	if (ret) {
> > > > +		auxiliary_device_uninit(adev);
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	adev->dev.platform_data = (__force void *)base;
> > > > +
> > > > +	return devm_add_action_or_reset(clk_dev,
> > > > +aspeed_reset_unregister_adev, adev); }
> > > > +EXPORT_SYMBOL_GPL(aspeed_reset_controller_register);
> > >
> > > No, you cannot export functions without users. There is no single
> > > user of this, so this is not justified at all.
> > My mistake, I missed patch #3 which uses it.
> >
> > I don't get why do you need to export this in the first place, instead
> > of putting it in the clock driver, as usually expected. Handling child
> > creation is logically the task of the device having children, the
> > parent. Not the child.
> 
> Also, consider basing this on top of:
> 
> https://lore.kernel.org/all/20250218-aux-device-create-helper-v4-0-c3d7dfdea
> 2e6@baylibre.com/

Hello, Base on this series, I will use adev = devm_auxiliary_device_create instead addev_alloc, adev_releas.
But it still have EXPORT_SYMBOL_GPL(aspeed_reset_controller_register);
Am I right?

> regards
> Philipp

Re: [PATCH v9 2/3] reset: aspeed: register AST2700 reset auxiliary bus device

[Krzysztof Kozlowski]

On 25/02/2025 03:20, Ryan Chen wrote:
>>>>> +
>>>>> +	return devm_add_action_or_reset(clk_dev,
>>>>> +aspeed_reset_unregister_adev, adev); }
>>>>> +EXPORT_SYMBOL_GPL(aspeed_reset_controller_register);
>>>>
>>>> No, you cannot export functions without users. There is no single
>>>> user of this, so this is not justified at all.
>>> My mistake, I missed patch #3 which uses it.
>>>
>>> I don't get why do you need to export this in the first place, instead
>>> of putting it in the clock driver, as usually expected. Handling child
>>> creation is logically the task of the device having children, the
>>> parent. Not the child.
>>
>> Also, consider basing this on top of:
>>
>> https://lore.kernel.org/all/20250218-aux-device-create-helper-v4-0-c3d7dfdea
>> 2e6@baylibre.com/
> 
> Hello, Base on this series, I will use adev = devm_auxiliary_device_create instead addev_alloc, adev_releas.
> But it still have EXPORT_SYMBOL_GPL(aspeed_reset_controller_register);
> Am I right?


Why? It looks like you skipped entirely my message. It is the same over
and over - respond to people comments...



Best regards,
Krzysztof

Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock define

[Krzysztof Kozlowski]

On 25/02/2025 01:41, Ryan Chen wrote:
>> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock
>> define
>>
>> On 24/02/2025 10:55, Ryan Chen wrote:
>>> -remove redundant SOC0_CLK_UART_DIV13:
>>> SOC0_CLK_UART_DIV13 is not use at clk-ast2700.c, the clock source tree
>>> is uart clk src -> uart_div_table -> uart clk.
>>>
>>> -Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX:
>>> modify clock tree implement.
>>> older CLK_AHB use mpll_div_ahb/hpll_div_ahb to be ahb clock source.
>>> mpll->mpll_div_ahb
>>>                   -> clk_ahb
>>> hpll->hpll_div_ahb
>>
>>
>> I can barely understand it and from the pieces I got, it does not explain need
>> for ABI break.
>>
> 
> #1. SCU0_CLK_UART_DIV13 is redundant, it does not impact ABI break

You did not explain how it does not impact. Clock was exported, there
was a user and now there is no clock. User stops working. ABI break.

> #2. Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX
> Older implement where `mpll_div_ahb` and `hpll_div_ahb` were **hardcoded dividers** for AHB.
> In **the new approach (v8)**, I refactored the clock tree to clock tree.

I still cannot parse sentences like "refactoring A to A". It's
meaningless to me.

> It should be ABI-safe change

No, you do not understand the ABI. You removed a clock ID, that's the
ABI change.

Otherwise explain how this is not changing ABI.


> 
> Or you want to keep original SOC0_CLK_HPLL_DIV_AHB define and then add SOC0_CLK_AHBMUX.
> To be 1st patch, then 2n patch remove redundant SOC0_CLK_HPLL_DIV_AHB?

If you break the ABI you need to clearly explain why. We have long
conversations and you still did not say why.


Best regards,
Krzysztof

RE: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock define

[Ryan Chen]

> >> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1
> >> clock define
> >>
> >> On 24/02/2025 10:55, Ryan Chen wrote:
> >>> -remove redundant SOC0_CLK_UART_DIV13:
> >>> SOC0_CLK_UART_DIV13 is not use at clk-ast2700.c, the clock source
> >>> tree is uart clk src -> uart_div_table -> uart clk.
> >>>
> >>> -Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX:
> >>> modify clock tree implement.
> >>> older CLK_AHB use mpll_div_ahb/hpll_div_ahb to be ahb clock source.
> >>> mpll->mpll_div_ahb
> >>>                   -> clk_ahb
> >>> hpll->hpll_div_ahb
> >>
> >>
> >> I can barely understand it and from the pieces I got, it does not
> >> explain need for ABI break.
> >>
> >
> > #1. SCU0_CLK_UART_DIV13 is redundant, it does not impact ABI break
> 
> You did not explain how it does not impact. Clock was exported, there was a
> user and now there is no clock. User stops working. ABI break.
> 

Sorry, SCU0_CLK_UART_DIV13 was defined, but was never referenced in any upstream device trees.
Since there is no in-tree usage of `SCU0_CLK_UART_DIV13`, its removal does not cause an ABI break.

> > #2. Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX Older
> implement
> > where `mpll_div_ahb` and `hpll_div_ahb` were **hardcoded dividers** for
> AHB.
> > In **the new approach (v8)**, I refactored the clock tree to clock tree.
> 
> I still cannot parse sentences like "refactoring A to A". It's meaningless to me.
> 
> > It should be ABI-safe change
> 
> No, you do not understand the ABI. You removed a clock ID, that's the ABI
> change.
> 
> Otherwise explain how this is not changing ABI.
> 
> 
> >
> > Or you want to keep original SOC0_CLK_HPLL_DIV_AHB define and then add
> SOC0_CLK_AHBMUX.
> > To be 1st patch, then 2n patch remove redundant
> SOC0_CLK_HPLL_DIV_AHB?
> 
> If you break the ABI you need to clearly explain why. We have long
> conversations and you still did not say why.
> 
Sorry, my point will be following steps to avoid potential ABI issues, 
I can modify the patch series as follows:
1. **Patch 1:** Add `SOC0_CLK_AHBMUX` without removing `SOC0_CLK_HPLL_DIV_AHB`.
2. **Patch 2:** Finally remove `SOC0_CLK_HPLL_DIV_AHB`.

Let me know if you prefer this approach.


> 
> Best regards,
> Krzysztof

Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock define

[Krzysztof Kozlowski]

On 25/02/2025 10:49, Ryan Chen wrote:
>>>> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1
>>>> clock define
>>>>
>>>> On 24/02/2025 10:55, Ryan Chen wrote:
>>>>> -remove redundant SOC0_CLK_UART_DIV13:
>>>>> SOC0_CLK_UART_DIV13 is not use at clk-ast2700.c, the clock source
>>>>> tree is uart clk src -> uart_div_table -> uart clk.
>>>>>
>>>>> -Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX:
>>>>> modify clock tree implement.
>>>>> older CLK_AHB use mpll_div_ahb/hpll_div_ahb to be ahb clock source.
>>>>> mpll->mpll_div_ahb
>>>>>                   -> clk_ahb
>>>>> hpll->hpll_div_ahb
>>>>
>>>>
>>>> I can barely understand it and from the pieces I got, it does not
>>>> explain need for ABI break.
>>>>
>>>
>>> #1. SCU0_CLK_UART_DIV13 is redundant, it does not impact ABI break
>>
>> You did not explain how it does not impact. Clock was exported, there was a
>> user and now there is no clock. User stops working. ABI break.
>>
> 
> Sorry, SCU0_CLK_UART_DIV13 was defined, but was never referenced in any upstream device trees.


That's incomplete definition of ABI

> Since there is no in-tree usage of `SCU0_CLK_UART_DIV13`, its removal does not cause an ABI break.


You ignored out of tree users. Please read carefully ABI docs.


> 
>>> #2. Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX Older
>> implement
>>> where `mpll_div_ahb` and `hpll_div_ahb` were **hardcoded dividers** for
>> AHB.
>>> In **the new approach (v8)**, I refactored the clock tree to clock tree.
>>
>> I still cannot parse sentences like "refactoring A to A". It's meaningless to me.
>>
>>> It should be ABI-safe change
>>
>> No, you do not understand the ABI. You removed a clock ID, that's the ABI
>> change.
>>
>> Otherwise explain how this is not changing ABI.
>>
>>
>>>
>>> Or you want to keep original SOC0_CLK_HPLL_DIV_AHB define and then add
>> SOC0_CLK_AHBMUX.
>>> To be 1st patch, then 2n patch remove redundant
>> SOC0_CLK_HPLL_DIV_AHB?
>>
>> If you break the ABI you need to clearly explain why. We have long
>> conversations and you still did not say why.
>>
> Sorry, my point will be following steps to avoid potential ABI issues, 
> I can modify the patch series as follows:
> 1. **Patch 1:** Add `SOC0_CLK_AHBMUX` without removing `SOC0_CLK_HPLL_DIV_AHB`.
> 2. **Patch 2:** Finally remove `SOC0_CLK_HPLL_DIV_AHB`.


I do not understand what changed here. You remove exported clock which
is ABI, so how is this answering my question.

You keep dodging my questions. Here I asked "why". I do not see any
answer why.

Best regards,
Krzysztof

RE: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock define

[Ryan Chen]

> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock
> define
> 
> On 25/02/2025 10:49, Ryan Chen wrote:
> >>>> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify
> >>>> soc0/1 clock define
> >>>>
> >>>> On 24/02/2025 10:55, Ryan Chen wrote:
> >>>>> -remove redundant SOC0_CLK_UART_DIV13:
> >>>>> SOC0_CLK_UART_DIV13 is not use at clk-ast2700.c, the clock source
> >>>>> tree is uart clk src -> uart_div_table -> uart clk.
> >>>>>
> >>>>> -Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX:
> >>>>> modify clock tree implement.
> >>>>> older CLK_AHB use mpll_div_ahb/hpll_div_ahb to be ahb clock source.
> >>>>> mpll->mpll_div_ahb
> >>>>>                   -> clk_ahb
> >>>>> hpll->hpll_div_ahb
> >>>>
> >>>>
> >>>> I can barely understand it and from the pieces I got, it does not
> >>>> explain need for ABI break.
> >>>>
> >>>
> >>> #1. SCU0_CLK_UART_DIV13 is redundant, it does not impact ABI break
> >>
> >> You did not explain how it does not impact. Clock was exported, there
> >> was a user and now there is no clock. User stops working. ABI break.
> >>
> >
> > Sorry, SCU0_CLK_UART_DIV13 was defined, but was never referenced in any
> upstream device trees.
> 
> 
> That's incomplete definition of ABI
> 
> > Since there is no in-tree usage of `SCU0_CLK_UART_DIV13`, its removal does
> not cause an ABI break.
> 
> 
> You ignored out of tree users. Please read carefully ABI docs.
> 
> 
> >
> >>> #2. Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX Older
> >> implement
> >>> where `mpll_div_ahb` and `hpll_div_ahb` were **hardcoded dividers**
> >>> for
> >> AHB.
> >>> In **the new approach (v8)**, I refactored the clock tree to clock tree.
> >>
> >> I still cannot parse sentences like "refactoring A to A". It's meaningless to
> me.
> >>
> >>> It should be ABI-safe change
> >>
> >> No, you do not understand the ABI. You removed a clock ID, that's the
> >> ABI change.
> >>
> >> Otherwise explain how this is not changing ABI.
> >>
> >>
> >>>
> >>> Or you want to keep original SOC0_CLK_HPLL_DIV_AHB define and then
> >>> add
> >> SOC0_CLK_AHBMUX.
> >>> To be 1st patch, then 2n patch remove redundant
> >> SOC0_CLK_HPLL_DIV_AHB?
> >>
> >> If you break the ABI you need to clearly explain why. We have long
> >> conversations and you still did not say why.
> >>
> > Sorry, my point will be following steps to avoid potential ABI issues,
> > I can modify the patch series as follows:
> > 1. **Patch 1:** Add `SOC0_CLK_AHBMUX` without removing
> `SOC0_CLK_HPLL_DIV_AHB`.
> > 2. **Patch 2:** Finally remove `SOC0_CLK_HPLL_DIV_AHB`.
> 
> 
> I do not understand what changed here. You remove exported clock which is
> ABI, so how is this answering my question.
> 
> You keep dodging my questions. Here I asked "why". I do not see any answer
> why.

Apology, I can't catch your point. But I still need your guideline.

**Regarding `SCU0_CLK_UART_DIV13`:**
   - This clock ID was originally defined but was never used in any in-tree device trees. (i2c-ast2700.c v1 ~ v9)
   - So there should not have ABI-break am I correct?

**Regarding `SOC0_CLK_HPLL_DIV_AHB` → `SOC0_CLK_AHBMUX`:**
   - The previous implementation used dividers (`mpll_div_ahb`, `hpll_div_ahb`) for AHB clock selection. (i2c-ast2700.c v1 ~ v8)
mpll->mpll_div_ahb
                  -> clk_ahb
hpll->hpll_div_ahb
   - The new approach use `SOC0_CLK_AHBMUX`, which AHB clock sources via a mux. (i2c-ast2700.c v9)
mpll->
      ahb_mux -> div_table -> clk_ahb
hpll->
   - And since i2c-ast2700.c (v8) is not applied, so there should also no one use this ABI. Am I correct?

If this is not acceptable, my next patch will keep SCU0_CLK_UART_DIV13/SCU0_CLK_HPLL_DIV_AHB define.
But add new SCU0_CLK_AHBMUX define for avoid your point ABI break. Is this acceptable?  

Any more clear guideline will help more clear your through. Thank your patience. 

> 
> Best regards,
> Krzysztof

Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock define

[Krzysztof Kozlowski]

On 26/02/2025 06:10, Ryan Chen wrote:
>> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock
>> define
>>
>> On 25/02/2025 10:49, Ryan Chen wrote:
>>>>>> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify
>>>>>> soc0/1 clock define
>>>>>>
>>>>>> On 24/02/2025 10:55, Ryan Chen wrote:
>>>>>>> -remove redundant SOC0_CLK_UART_DIV13:
>>>>>>> SOC0_CLK_UART_DIV13 is not use at clk-ast2700.c, the clock source
>>>>>>> tree is uart clk src -> uart_div_table -> uart clk.
>>>>>>>
>>>>>>> -Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX:
>>>>>>> modify clock tree implement.
>>>>>>> older CLK_AHB use mpll_div_ahb/hpll_div_ahb to be ahb clock source.
>>>>>>> mpll->mpll_div_ahb
>>>>>>>                   -> clk_ahb
>>>>>>> hpll->hpll_div_ahb
>>>>>>
>>>>>>
>>>>>> I can barely understand it and from the pieces I got, it does not
>>>>>> explain need for ABI break.
>>>>>>
>>>>>
>>>>> #1. SCU0_CLK_UART_DIV13 is redundant, it does not impact ABI break
>>>>
>>>> You did not explain how it does not impact. Clock was exported, there
>>>> was a user and now there is no clock. User stops working. ABI break.
>>>>
>>>
>>> Sorry, SCU0_CLK_UART_DIV13 was defined, but was never referenced in any
>> upstream device trees.
>>
>>
>> That's incomplete definition of ABI
>>
>>> Since there is no in-tree usage of `SCU0_CLK_UART_DIV13`, its removal does
>> not cause an ABI break.
>>
>>
>> You ignored out of tree users. Please read carefully ABI docs.
>>
>>
>>>
>>>>> #2. Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX Older
>>>> implement
>>>>> where `mpll_div_ahb` and `hpll_div_ahb` were **hardcoded dividers**
>>>>> for
>>>> AHB.
>>>>> In **the new approach (v8)**, I refactored the clock tree to clock tree.
>>>>
>>>> I still cannot parse sentences like "refactoring A to A". It's meaningless to
>> me.
>>>>
>>>>> It should be ABI-safe change
>>>>
>>>> No, you do not understand the ABI. You removed a clock ID, that's the
>>>> ABI change.
>>>>
>>>> Otherwise explain how this is not changing ABI.
>>>>
>>>>
>>>>>
>>>>> Or you want to keep original SOC0_CLK_HPLL_DIV_AHB define and then
>>>>> add
>>>> SOC0_CLK_AHBMUX.
>>>>> To be 1st patch, then 2n patch remove redundant
>>>> SOC0_CLK_HPLL_DIV_AHB?
>>>>
>>>> If you break the ABI you need to clearly explain why. We have long
>>>> conversations and you still did not say why.
>>>>
>>> Sorry, my point will be following steps to avoid potential ABI issues,
>>> I can modify the patch series as follows:
>>> 1. **Patch 1:** Add `SOC0_CLK_AHBMUX` without removing
>> `SOC0_CLK_HPLL_DIV_AHB`.
>>> 2. **Patch 2:** Finally remove `SOC0_CLK_HPLL_DIV_AHB`.
>>
>>
>> I do not understand what changed here. You remove exported clock which is
>> ABI, so how is this answering my question.
>>
>> You keep dodging my questions. Here I asked "why". I do not see any answer
>> why.
> 
> Apology, I can't catch your point. But I still need your guideline.
> 
> **Regarding `SCU0_CLK_UART_DIV13`:**
>    - This clock ID was originally defined but was never used in any in-tree device trees. (i2c-ast2700.c v1 ~ v9)
>    - So there should not have ABI-break am I correct?


No, because there are other users of bindings. All forks, out of tree
DTS, other projects etc. You defined ABI for them.

> 
> **Regarding `SOC0_CLK_HPLL_DIV_AHB` → `SOC0_CLK_AHBMUX`:**
>    - The previous implementation used dividers (`mpll_div_ahb`, `hpll_div_ahb`) for AHB clock selection. (i2c-ast2700.c v1 ~ v8)
> mpll->mpll_div_ahb
>                   -> clk_ahb
> hpll->hpll_div_ahb
>    - The new approach use `SOC0_CLK_AHBMUX`, which AHB clock sources via a mux. (i2c-ast2700.c v9)
> mpll->
>       ahb_mux -> div_table -> clk_ahb
> hpll->

Your formatting is one of the problems I have troubles understanding it.
Above is not wrapped or wrapped oddly. You keep using bold * but double
**, which is not a standard markup. Please switch to standard mailing
list formatting - proper wrapping, only text mode and use client which
actually can parse and create that.

What I don't understand is how clocks could change in hardware. You
described implementation, but the clock IDS do not describe
implementation but the the mapping to real hardware clocks. So how
SOC0_CLK_HPLL_DIV_AHB clock disappeared from hardware?


>    - And since i2c-ast2700.c (v8) is not applied, so there should also no one use this ABI. Am I correct?

If binding was not applied, then what are you changing here?

Does it mean header described clock which was in the hardware but its
handling was not yet implemented in the driver?


> 
> If this is not acceptable, my next patch will keep SCU0_CLK_UART_DIV13/SCU0_CLK_HPLL_DIV_AHB define.

Maybe my last message was not clear, so: you need to explain why you are
breaking ABI and/or explain the ABI impact in the commit msg.

> But add new SCU0_CLK_AHBMUX define for avoid your point ABI break. Is this acceptable?  


Best regards,
Krzysztof

RE: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock define

[Ryan Chen]

> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock
> define
> 
> On 26/02/2025 06:10, Ryan Chen wrote:
> >> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1
> >> clock define
> >>
> >> On 25/02/2025 10:49, Ryan Chen wrote:
> >>>>>> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify
> >>>>>> soc0/1 clock define
> >>>>>>
> >>>>>> On 24/02/2025 10:55, Ryan Chen wrote:
> >>>>>>> -remove redundant SOC0_CLK_UART_DIV13:
> >>>>>>> SOC0_CLK_UART_DIV13 is not use at clk-ast2700.c, the clock
> >>>>>>> source tree is uart clk src -> uart_div_table -> uart clk.
> >>>>>>>
> >>>>>>> -Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX:
> >>>>>>> modify clock tree implement.
> >>>>>>> older CLK_AHB use mpll_div_ahb/hpll_div_ahb to be ahb clock
> source.
> >>>>>>> mpll->mpll_div_ahb
> >>>>>>>                   -> clk_ahb
> >>>>>>> hpll->hpll_div_ahb
> >>>>>>
> >>>>>>
> >>>>>> I can barely understand it and from the pieces I got, it does not
> >>>>>> explain need for ABI break.
> >>>>>>
> >>>>>
> >>>>> #1. SCU0_CLK_UART_DIV13 is redundant, it does not impact ABI break
> >>>>
> >>>> You did not explain how it does not impact. Clock was exported,
> >>>> there was a user and now there is no clock. User stops working. ABI
> break.
> >>>>
> >>>
> >>> Sorry, SCU0_CLK_UART_DIV13 was defined, but was never referenced in
> >>> any
> >> upstream device trees.
> >>
> >>
> >> That's incomplete definition of ABI
> >>
> >>> Since there is no in-tree usage of `SCU0_CLK_UART_DIV13`, its
> >>> removal does
> >> not cause an ABI break.
> >>
> >>
> >> You ignored out of tree users. Please read carefully ABI docs.
> >>
> >>
> >>>
> >>>>> #2. Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX Older
> >>>> implement
> >>>>> where `mpll_div_ahb` and `hpll_div_ahb` were **hardcoded
> >>>>> dividers** for
> >>>> AHB.
> >>>>> In **the new approach (v8)**, I refactored the clock tree to clock tree.
> >>>>
> >>>> I still cannot parse sentences like "refactoring A to A". It's
> >>>> meaningless to
> >> me.
> >>>>
> >>>>> It should be ABI-safe change
> >>>>
> >>>> No, you do not understand the ABI. You removed a clock ID, that's
> >>>> the ABI change.
> >>>>
> >>>> Otherwise explain how this is not changing ABI.
> >>>>
> >>>>
> >>>>>
> >>>>> Or you want to keep original SOC0_CLK_HPLL_DIV_AHB define and
> then
> >>>>> add
> >>>> SOC0_CLK_AHBMUX.
> >>>>> To be 1st patch, then 2n patch remove redundant
> >>>> SOC0_CLK_HPLL_DIV_AHB?
> >>>>
> >>>> If you break the ABI you need to clearly explain why. We have long
> >>>> conversations and you still did not say why.
> >>>>
> >>> Sorry, my point will be following steps to avoid potential ABI
> >>> issues, I can modify the patch series as follows:
> >>> 1. **Patch 1:** Add `SOC0_CLK_AHBMUX` without removing
> >> `SOC0_CLK_HPLL_DIV_AHB`.
> >>> 2. **Patch 2:** Finally remove `SOC0_CLK_HPLL_DIV_AHB`.
> >>
> >>
> >> I do not understand what changed here. You remove exported clock
> >> which is ABI, so how is this answering my question.
> >>
> >> You keep dodging my questions. Here I asked "why". I do not see any
> >> answer why.
> >
> > Apology, I can't catch your point. But I still need your guideline.
> >
> > **Regarding `SCU0_CLK_UART_DIV13`:**
> >    - This clock ID was originally defined but was never used in any in-tree
> device trees. (i2c-ast2700.c v1 ~ v9)
> >    - So there should not have ABI-break am I correct?
> 
> 
> No, because there are other users of bindings. All forks, out of tree DTS, other
> projects etc. You defined ABI for them.
> 
> >
> > **Regarding `SOC0_CLK_HPLL_DIV_AHB` → `SOC0_CLK_AHBMUX`:**
> >    - The previous implementation used dividers (`mpll_div_ahb`,
> > `hpll_div_ahb`) for AHB clock selection. (i2c-ast2700.c v1 ~ v8)
> > mpll->mpll_div_ahb
> >                   -> clk_ahb
> > hpll->hpll_div_ahb
> >    - The new approach use `SOC0_CLK_AHBMUX`, which AHB clock sources
> > via a mux. (i2c-ast2700.c v9)
> > mpll->
> >       ahb_mux -> div_table -> clk_ahb
> > hpll->
> 
> Your formatting is one of the problems I have troubles understanding it.
> Above is not wrapped or wrapped oddly. You keep using bold * but double **,
> which is not a standard markup. Please switch to standard mailing list
> formatting - proper wrapping, only text mode and use client which actually can
> parse and create that.
> 
> What I don't understand is how clocks could change in hardware. You described
> implementation, but the clock IDS do not describe implementation but the the
> mapping to real hardware clocks. So how SOC0_CLK_HPLL_DIV_AHB clock
> disappeared from hardware?

Sorry, It's not disappeared, it is replaced by implement.

- The previous implementation hardcoded `mpll_div_ahb` and `hpll_div_ahb` as the only AHB clock sources:
     mpll -> mpll_div_ahb 
						-> clk_ahb
     hpll -> hpll_div_ahb> 

- The new approach introduces `SOC0_CLK_AHBMUX`, clear clock tree selection of AHB clock sources via a mux:
 Than through div_table to get ahb clk.
     mpll ->
       		-> ahb_mux -> div_table -> clk_ahb
     hpll ->

> 
> >    - And since i2c-ast2700.c (v8) is not applied, so there should also no one
> use this ABI. Am I correct?
> 
> If binding was not applied, then what are you changing here?

My point is only i2c-ast2700.c (v8) used, which not be applied in Linux. 
And i2c-ast2700.c (v9) no need anymore.

> 
> Does it mean header described clock which was in the hardware but its
> handling was not yet implemented in the driver?
> 
> 
> >
> > If this is not acceptable, my next patch will keep
> SCU0_CLK_UART_DIV13/SCU0_CLK_HPLL_DIV_AHB define.
> 
> Maybe my last message was not clear, so: you need to explain why you are
> breaking ABI and/or explain the ABI impact in the commit msg.
Understood your concern about this.
I think add new SCU0_CLK_AHBMUX to avoid ABI impact will be the better way.

Thank your guideline. 
> 
> > But add new SCU0_CLK_AHBMUX define for avoid your point ABI break. Is
> this acceptable?
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v9 3/3] clk: aspeed: add AST2700 clock driver

[kernel test robot]

Hi Ryan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.14-rc5 next-20250306]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ryan-Chen/dt-binding-clock-ast2700-modify-soc0-1-clock-define/20250224-175830
base:   linus/master
patch link:    https://lore.kernel.org/r/20250224095506.2047064-4-ryan_chen%40aspeedtech.com
patch subject: [PATCH v9 3/3] clk: aspeed: add AST2700 clock driver
config: powerpc64-randconfig-r121-20250306 (https://download.01.org/0day-ci/archive/20250307/202503070117.mMjnpop8-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250307/202503070117.mMjnpop8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503070117.mMjnpop8-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/clk/clk-ast2700.c:1066:92: sparse: sparse: Using plain integer as NULL pointer

vim +1066 drivers/clk/clk-ast2700.c

   952	
   953	static int ast2700_soc_clk_probe(struct platform_device *pdev)
   954	{
   955		const struct ast2700_clk_data *clk_data;
   956		struct ast2700_clk_ctrl *clk_ctrl;
   957		struct clk_hw_onecell_data *clk_hw_data;
   958		struct device *dev = &pdev->dev;
   959		void __iomem *clk_base;
   960		struct clk_hw **hws;
   961		char *reset_name;
   962		int ret;
   963		int i;
   964	
   965		clk_ctrl = devm_kzalloc(dev, sizeof(*clk_ctrl), GFP_KERNEL);
   966		if (!clk_ctrl)
   967			return -ENOMEM;
   968		clk_ctrl->dev = dev;
   969		dev_set_drvdata(&pdev->dev, clk_ctrl);
   970	
   971		spin_lock_init(&clk_ctrl->lock);
   972	
   973		clk_base = devm_platform_ioremap_resource(pdev, 0);
   974		if (IS_ERR(clk_base))
   975			return PTR_ERR(clk_base);
   976	
   977		clk_ctrl->base = clk_base;
   978	
   979		clk_data = device_get_match_data(dev);
   980		if (!clk_data)
   981			return -ENODEV;
   982	
   983		clk_ctrl->clk_data = clk_data;
   984		reset_name = devm_kasprintf(dev, GFP_KERNEL, "reset%d", clk_data->scu);
   985	
   986		clk_hw_data = devm_kzalloc(dev, struct_size(clk_hw_data, hws, clk_data->nr_clks),
   987					   GFP_KERNEL);
   988		if (!clk_hw_data)
   989			return -ENOMEM;
   990	
   991		clk_hw_data->num = clk_data->nr_clks;
   992		hws = clk_hw_data->hws;
   993	
   994		if (clk_data->scu)
   995			ast2700_soc1_configure_i3c_clk(clk_ctrl);
   996	
   997		for (i = 0; i < clk_data->nr_clks; i++) {
   998			const struct ast2700_clk_info *clk = &clk_data->clk_info[i];
   999			void __iomem *reg;
  1000	
  1001			if (clk->type == CLK_FIXED) {
  1002				const struct ast2700_clk_fixed_rate_data *fixed_rate = &clk->data.rate;
  1003	
  1004				hws[i] = devm_clk_hw_register_fixed_rate(dev, clk->name, NULL, 0,
  1005									 fixed_rate->fixed_rate);
  1006			} else if (clk->type == CLK_FIXED_FACTOR) {
  1007				const struct ast2700_clk_fixed_factor_data *factor = &clk->data.factor;
  1008	
  1009				hws[i] = devm_clk_hw_register_fixed_factor(dev, clk->name,
  1010									   factor->parent->name,
  1011									   0, factor->mult, factor->div);
  1012			} else if (clk->type == DCLK_FIXED) {
  1013				const struct ast2700_clk_pll_data *pll = &clk->data.pll;
  1014	
  1015				reg = clk_ctrl->base + pll->reg;
  1016				hws[i] = ast2700_clk_hw_register_dclk(reg, clk->name, clk_ctrl);
  1017			} else if (clk->type == CLK_HPLL) {
  1018				const struct ast2700_clk_pll_data *pll = &clk->data.pll;
  1019	
  1020				reg = clk_ctrl->base + pll->reg;
  1021				hws[i] = ast2700_clk_hw_register_hpll(reg, clk->name,
  1022								      pll->parent->name, clk_ctrl);
  1023			} else if (clk->type == CLK_PLL) {
  1024				const struct ast2700_clk_pll_data *pll = &clk->data.pll;
  1025	
  1026				reg = clk_ctrl->base + pll->reg;
  1027				hws[i] = ast2700_clk_hw_register_pll(i, reg, clk->name,
  1028								     pll->parent->name, clk_ctrl);
  1029			} else if (clk->type == CLK_UART_PLL) {
  1030				const struct ast2700_clk_pll_data *pll = &clk->data.pll;
  1031	
  1032				reg = clk_ctrl->base + pll->reg;
  1033				hws[i] = ast2700_clk_hw_register_uartpll(reg, clk->name,
  1034									 pll->parent->name, clk_ctrl);
  1035			} else if (clk->type == CLK_MUX) {
  1036				const struct ast2700_clk_mux_data *mux = &clk->data.mux;
  1037	
  1038				reg = clk_ctrl->base + mux->reg;
  1039				hws[i] = devm_clk_hw_register_mux_parent_data_table(dev, clk->name,
  1040										    mux->parents,
  1041										    mux->num_parents, 0,
  1042										    reg, mux->bit_shift,
  1043										    mux->bit_width, 0,
  1044										    NULL, &clk_ctrl->lock);
  1045			} else if (clk->type == CLK_MISC) {
  1046				const struct ast2700_clk_pll_data *misc = &clk->data.pll;
  1047	
  1048				reg = clk_ctrl->base + misc->reg;
  1049				hws[i] = ast2700_clk_hw_register_misc(i, reg, clk->name,
  1050								      misc->parent->name, clk_ctrl);
  1051			} else if (clk->type == CLK_DIVIDER) {
  1052				const struct ast2700_clk_div_data *div = &clk->data.div;
  1053	
  1054				reg = clk_ctrl->base + div->reg;
  1055				hws[i] = devm_clk_hw_register_divider_table(dev, clk->name,
  1056									    div->parent->name, 0,
  1057									    reg, div->bit_shift,
  1058									    div->bit_width, 0,
  1059									    div->div_table,
  1060									    &clk_ctrl->lock);
  1061			} else if (clk->type == CLK_GATE_ASPEED) {
  1062				const struct ast2700_clk_gate_data *gate = &clk->data.gate;
  1063	
  1064				reg = clk_ctrl->base + gate->reg;
  1065				hws[i] = ast2700_clk_hw_register_gate(dev, clk->name, gate->parent,
> 1066								      reg, gate->bit, gate->flags, 0);
  1067	
  1068			} else {
  1069				const struct ast2700_clk_gate_data *gate = &clk->data.gate;
  1070	
  1071				reg = clk_ctrl->base + gate->reg;
  1072				hws[i] = devm_clk_hw_register_gate_parent_data(dev, clk->name, gate->parent,
  1073									       0, reg, clk->clk_idx, 0,
  1074									       &clk_ctrl->lock);
  1075			}
  1076	
  1077			if (IS_ERR(hws[i]))
  1078				return PTR_ERR(hws[i]);
  1079		}
  1080	
  1081		ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_hw_data);
  1082		if (ret)
  1083			return ret;
  1084	
  1085		return aspeed_reset_controller_register(dev, clk_base, reset_name);
  1086	}
  1087	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki