Devicetree
 help / color / mirror / Atom feed
* [PATCH 0/4] clk: add Siflower SF21 topcrm support
@ 2026-05-17 14:12 Chuanhong Guo
  2026-05-17 14:12 ` [PATCH 1/4] riscv: add Siflower RISC-V SoC family Kconfig support Chuanhong Guo
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Chuanhong Guo @ 2026-05-17 14:12 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-riscv, linux-kernel, linux-clk, devicetree, Chuanhong Guo

Siflower SF21A6826 and SF21H8898 are RISC-V chips with quad-core
T-Head C908 for home routers and gateways.
This series adds the initial RISC-V Kconfig entry for Siflower SoCs and
support for the toplevel clock and reset module on Siflower SF21 socs.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
Chuanhong Guo (4):
      riscv: add Siflower RISC-V SoC family Kconfig support
      dt-bindings: clock: add binding header for sf21-topcrm
      dt-bindings: clock: add doc for Siflower sf21-topcrm
      clk: add support for siflower sf21-topcrm

 .../bindings/clock/siflower,sf21-topcrm.yaml       |   69 ++
 arch/riscv/Kconfig.socs                            |    7 +
 drivers/clk/Kconfig                                |    1 +
 drivers/clk/Makefile                               |    1 +
 drivers/clk/siflower/Kconfig                       |   22 +
 drivers/clk/siflower/Makefile                      |    1 +
 drivers/clk/siflower/clk-sf21-topcrm.c             | 1053 ++++++++++++++++++++
 include/dt-bindings/clock/siflower,sf21-topcrm.h   |   63 ++
 8 files changed, 1217 insertions(+)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260515-sf21-topcrm-1856c05b6138

Best regards,
-- 
Chuanhong Guo <gch981213@gmail.com>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] riscv: add Siflower RISC-V SoC family Kconfig support
  2026-05-17 14:12 [PATCH 0/4] clk: add Siflower SF21 topcrm support Chuanhong Guo
@ 2026-05-17 14:12 ` Chuanhong Guo
  2026-05-17 14:31   ` sashiko-bot
  2026-05-17 14:12 ` [PATCH 2/4] dt-bindings: clock: add binding header for sf21-topcrm Chuanhong Guo
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Chuanhong Guo @ 2026-05-17 14:12 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-riscv, linux-kernel, linux-clk, devicetree, Chuanhong Guo

Siflower RISC-V SoCs, including SF21A6826 and SF21H8898, are RISC-V
chips with T-Head C908 cores for home routers and gateways. Add a
Kconfig entry named ARCH_SIFLOWER for them.
Notably these chips uses ARM PL011 for UART. ARM_AMBA is selected
for its driver.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 arch/riscv/Kconfig.socs | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index c174ac0ec46b..9996591cd9db 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -37,6 +37,13 @@ config ARCH_SIFIVE
 	help
 	  This enables support for SiFive SoC platform hardware.
 
+config ARCH_SIFLOWER
+	bool "Siflower RISC-V SoCs"
+	select ARM_AMBA if TTY
+	select ERRATA_THEAD
+	help
+	  This enables support for Siflower RISC-V SoC platform hardware.
+
 config ARCH_SOPHGO
 	bool "Sophgo SoCs"
 	help

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] dt-bindings: clock: add binding header for sf21-topcrm
  2026-05-17 14:12 [PATCH 0/4] clk: add Siflower SF21 topcrm support Chuanhong Guo
  2026-05-17 14:12 ` [PATCH 1/4] riscv: add Siflower RISC-V SoC family Kconfig support Chuanhong Guo
@ 2026-05-17 14:12 ` Chuanhong Guo
  2026-05-17 14:36   ` sashiko-bot
  2026-05-17 20:47   ` Conor Dooley
  2026-05-17 14:12 ` [PATCH 3/4] dt-bindings: clock: add doc for Siflower sf21-topcrm Chuanhong Guo
  2026-05-17 14:12 ` [PATCH 4/4] clk: add support for siflower sf21-topcrm Chuanhong Guo
  3 siblings, 2 replies; 13+ messages in thread
From: Chuanhong Guo @ 2026-05-17 14:12 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-riscv, linux-kernel, linux-clk, devicetree, Chuanhong Guo

Add the device tree binding header for Siflower SF21A6826/SF21H8898
toplevel clock and reset module. The header covers both clock and
reset IDs provided by the block.
CLK_ETH_REF_P is a clock name that exists in the vendor datasheet.
This clock connects directly to CLK_PCIEPLL_FOUT2 and there's no
clock gate/mux in between. An alias is created for this clock
to make available clock names align with the datasheet.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 include/dt-bindings/clock/siflower,sf21-topcrm.h | 63 ++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/include/dt-bindings/clock/siflower,sf21-topcrm.h b/include/dt-bindings/clock/siflower,sf21-topcrm.h
new file mode 100644
index 000000000000..3690b3452501
--- /dev/null
+++ b/include/dt-bindings/clock/siflower,sf21-topcrm.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
+
+#ifndef _DT_BINDINGS_CLK_SIFLOWER_SF21_TOPCRM_H
+#define _DT_BINDINGS_CLK_SIFLOWER_SF21_TOPCRM_H
+
+#define SF21_CLK_CMNPLL_VCO		0
+#define SF21_CLK_CMNPLL_POSTDIV		1
+
+#define SF21_CLK_DDRPLL_POSTDIV		2
+
+#define SF21_CLK_PCIEPLL_VCO		3
+#define SF21_CLK_PCIEPLL_FOUT0		4
+#define SF21_CLK_PCIEPLL_FOUT1		5
+#define SF21_CLK_PCIEPLL_FOUT2		6
+#define SF21_CLK_ETH_REF_P		SF21_CLK_PCIEPLL_FOUT2
+#define SF21_CLK_PCIEPLL_FOUT3		7
+
+#define SF21_CLK_CPU			8
+#define SF21_CLK_PIC			9
+#define SF21_CLK_AXI			10
+#define SF21_CLK_AHB			11
+#define SF21_CLK_APB			12
+#define SF21_CLK_UART			13
+#define SF21_CLK_IRAM			14
+#define SF21_CLK_NPU			15
+#define SF21_CLK_DDRPHY_REF		16
+#define SF21_CLK_DDR_BYPASS		17
+#define SF21_CLK_ETHTSU			18
+#define SF21_CLK_GMAC_BYP_REF		19
+#define SF21_CLK_USB			20
+#define SF21_CLK_USBPHY			21
+#define SF21_CLK_SERDES_CSR		22
+#define SF21_CLK_CRYPT_CSR		23
+#define SF21_CLK_CRYPT_APP		24
+#define SF21_CLK_IROM			25
+#define SF21_CLK_BOOT			26
+#define SF21_CLK_PVT			27
+#define SF21_CLK_PLL_TEST		28
+#define SF21_CLK_PCIE_REFN		29
+#define SF21_CLK_PCIE_REFP		30
+#define SF21_CLK_MAX			31
+
+#define SF21_RESET_GIC			0
+#define SF21_RESET_AXI			1
+#define SF21_RESET_AHB			2
+#define SF21_RESET_APB			3
+#define SF21_RESET_IRAM			4
+#define SF21_RESET_NPU			5
+#define SF21_RESET_DDR_CTL		6
+#define SF21_RESET_DDR_PHY		7
+#define SF21_RESET_DDR_PWR_OK_IN	8
+#define SF21_RESET_DDR_CTL_APB		9
+#define SF21_RESET_DDR_PHY_APB		10
+#define SF21_RESET_USB			11
+#define SF21_RESET_PVT			12
+#define SF21_RESET_SERDES_CSR		13
+#define SF21_RESET_CRYPT_CSR		14
+#define SF21_RESET_CRYPT_APP		15
+#define SF21_RESET_NPU2DDR_ASYNCBRIDGE	16
+#define SF21_RESET_IROM			17
+#define SF21_RESET_MAX			18
+
+#endif

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] dt-bindings: clock: add doc for Siflower sf21-topcrm
  2026-05-17 14:12 [PATCH 0/4] clk: add Siflower SF21 topcrm support Chuanhong Guo
  2026-05-17 14:12 ` [PATCH 1/4] riscv: add Siflower RISC-V SoC family Kconfig support Chuanhong Guo
  2026-05-17 14:12 ` [PATCH 2/4] dt-bindings: clock: add binding header for sf21-topcrm Chuanhong Guo
@ 2026-05-17 14:12 ` Chuanhong Guo
  2026-05-17 15:35   ` Rob Herring (Arm)
  2026-05-17 20:50   ` Conor Dooley
  2026-05-17 14:12 ` [PATCH 4/4] clk: add support for siflower sf21-topcrm Chuanhong Guo
  3 siblings, 2 replies; 13+ messages in thread
From: Chuanhong Guo @ 2026-05-17 14:12 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-riscv, linux-kernel, linux-clk, devicetree, Chuanhong Guo

Add a binding doc for the top clock and reset module found on Siflower
SF21 SoCs. This block provides the main PLLs, high-level clock
controls, and some reset lines.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 .../bindings/clock/siflower,sf21-topcrm.yaml       | 69 ++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/siflower,sf21-topcrm.yaml b/Documentation/devicetree/bindings/clock/siflower,sf21-topcrm.yaml
new file mode 100644
index 000000000000..a013d48841f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/siflower,sf21-topcrm.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/siflower,sf21-topcrm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Siflower SF21 toplevel clock and reset module
+
+maintainers:
+  - Chuanhong Guo <gch981213@gmail.com>
+
+description: |
+  The toplevel clock and reset module on Siflower SF21 SoCs manages
+  the main PLLs, high-level clock muxes/dividers/gates, and some
+  reset lines.
+  Available clocks and resets are defined in:
+  include/dt-bindings/clock/siflower,sf21-topcrm.h
+
+properties:
+  compatible:
+    const: siflower,sf21-topcrm
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: xin25m
+
+  "#clock-cells":
+    const: 1
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - "#clock-cells"
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/siflower,sf21-topcrm.h>
+    / {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        xin25m: clock-25000000 {
+            compatible = "fixed-clock";
+            #clock-cells = <0>;
+            clock-frequency = <25000000>;
+        };
+
+        clock-controller@ce00400 {
+            compatible = "siflower,sf21-topcrm";
+            reg = <0x0ce00400 0x400>;
+            clocks = <&xin25m>;
+            clock-names = "xin25m";
+            #clock-cells = <1>;
+            #reset-cells = <1>;
+        };
+    };

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] clk: add support for siflower sf21-topcrm
  2026-05-17 14:12 [PATCH 0/4] clk: add Siflower SF21 topcrm support Chuanhong Guo
                   ` (2 preceding siblings ...)
  2026-05-17 14:12 ` [PATCH 3/4] dt-bindings: clock: add doc for Siflower sf21-topcrm Chuanhong Guo
@ 2026-05-17 14:12 ` Chuanhong Guo
  2026-05-17 15:09   ` sashiko-bot
  3 siblings, 1 reply; 13+ messages in thread
From: Chuanhong Guo @ 2026-05-17 14:12 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-riscv, linux-kernel, linux-clk, devicetree, Chuanhong Guo

This commit adds a driver for the Toplevel clock and reset controller
found on Siflower SF21A6826/SF21H8898 SoCs.
This block contains control for 3 PLLs, several clock mux/gate/divider
blocks, and a reset register for on-chip peripherals.

There are also two registers for enabling PCIE clock output in this
block. They aren't covered by this patch because I can't test those
without a PCIE driver. These will be added with the PCIE driver
patchset later after I get that working.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 drivers/clk/Kconfig                    |    1 +
 drivers/clk/Makefile                   |    1 +
 drivers/clk/siflower/Kconfig           |   22 +
 drivers/clk/siflower/Makefile          |    1 +
 drivers/clk/siflower/clk-sf21-topcrm.c | 1053 ++++++++++++++++++++++++++++++++
 5 files changed, 1078 insertions(+)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index b2efbe9f6acb..8098e38d5f59 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -523,6 +523,7 @@ source "drivers/clk/renesas/Kconfig"
 source "drivers/clk/rockchip/Kconfig"
 source "drivers/clk/samsung/Kconfig"
 source "drivers/clk/sifive/Kconfig"
+source "drivers/clk/siflower/Kconfig"
 source "drivers/clk/socfpga/Kconfig"
 source "drivers/clk/sophgo/Kconfig"
 source "drivers/clk/spacemit/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index a3e2862ebd7e..7492942b7fad 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -144,6 +144,7 @@ obj-y					+= renesas/
 obj-$(CONFIG_ARCH_ROCKCHIP)		+= rockchip/
 obj-$(CONFIG_COMMON_CLK_SAMSUNG)	+= samsung/
 obj-$(CONFIG_CLK_SIFIVE)		+= sifive/
+obj-$(CONFIG_CLK_SIFLOWER)		+= siflower/
 obj-y					+= socfpga/
 obj-y					+= sophgo/
 obj-y					+= spacemit/
diff --git a/drivers/clk/siflower/Kconfig b/drivers/clk/siflower/Kconfig
new file mode 100644
index 000000000000..03cbfbdbdb8d
--- /dev/null
+++ b/drivers/clk/siflower/Kconfig
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: GPL-2.0
+
+menuconfig CLK_SIFLOWER
+	bool "Clock driver for Siflower SoCs"
+	depends on ARCH_SIFLOWER || COMPILE_TEST
+	default ARCH_SIFLOWER
+	help
+	  Clock drivers for Siflower Linux-capable SoCs.
+
+if CLK_SIFLOWER
+
+config CLK_SF21_TOPCRM
+	tristate "Clock driver for Siflower SF21 toplevel clock & reset module"
+	depends on ARCH_SIFLOWER || COMPILE_TEST
+	default ARCH_SIFLOWER
+	select RESET_CONTROLLER
+	help
+	  Supports the toplevel clock and reset module in Siflower SF21 SoCs.
+	  If this kernel is meant to run on Siflower SF21A6826 or SF21H8898,
+	  enable this driver.
+
+endif
diff --git a/drivers/clk/siflower/Makefile b/drivers/clk/siflower/Makefile
new file mode 100644
index 000000000000..952a470a4308
--- /dev/null
+++ b/drivers/clk/siflower/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_CLK_SF21_TOPCRM) += clk-sf21-topcrm.o
diff --git a/drivers/clk/siflower/clk-sf21-topcrm.c b/drivers/clk/siflower/clk-sf21-topcrm.c
new file mode 100644
index 000000000000..7d4c5e370d6d
--- /dev/null
+++ b/drivers/clk/siflower/clk-sf21-topcrm.c
@@ -0,0 +1,1053 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/clk-provider.h>
+#include <linux/bitfield.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/rational.h>
+#include <linux/module.h>
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+#include <linux/platform_device.h>
+#include <dt-bindings/clock/siflower,sf21-topcrm.h>
+
+struct sf_clk_common {
+	void __iomem	*base;
+	/* Serializes register RMW sequences shared by clocks and resets. */
+	spinlock_t	*lock;
+	struct clk_hw	hw;
+};
+
+static inline struct sf_clk_common *hw_to_sf_clk_common(struct clk_hw *hw)
+{
+	return container_of(hw, struct sf_clk_common, hw);
+}
+
+static inline u32 sf_readl(struct sf_clk_common *priv, u32 reg)
+{
+	return readl(priv->base + reg);
+}
+
+static inline void sf_writel(struct sf_clk_common *priv, u32 reg, u32 val)
+{
+	return writel(val, priv->base + reg);
+}
+
+static inline void sf_rmw(struct sf_clk_common *priv, u32 reg, u32 clr, u32 set)
+{
+	u32 val;
+
+	val = sf_readl(priv, reg);
+	val &= ~clr;
+	val |= set;
+	sf_writel(priv, reg, val);
+}
+
+#define PLL_CMN_CFG1		0x0
+#define  PLL_CMN_BYPASS		BIT(27)
+#define  PLL_CMN_PD		BIT(26)
+#define  PLL_CMN_FBDIV		GENMASK(25, 14)
+#define  PLL_CMN_FBDIV_BITS	(25 - 14 + 1)
+#define  PLL_CMN_POSTDIV_PD	BIT(13)
+#define  PLL_CMN_VCO_PD		BIT(12)
+#define  PLL_CMN_POSTDIV1	GENMASK(11, 9)
+#define  PLL_CMN_POSTDIV2	GENMASK(8, 6)
+#define  PLL_CMN_REFDIV		GENMASK(5, 0)
+#define  PLL_CMN_REFDIV_BITS	6
+
+#define PLL_CMN_LOCK		0xc8
+#define PLL_DDR_LOCK		0xcc
+#define PLL_PCIE_LOCK		0xd4
+
+#define CFG_LOAD		0x100
+#define  CFG_LOAD_PCIE_PLL	BIT(4)
+#define  CFG_LOAD_DDR_PLL	BIT(2)
+#define  CFG_LOAD_CMN_PLL	BIT(1)
+#define  CFG_LOAD_DIV		BIT(0)
+
+#define PLL_LOCK_TIMEOUT_US	1000
+
+static unsigned long sf21_cmnpll_vco_recalc_rate(struct clk_hw *hw,
+						 unsigned long parent_rate)
+{
+	struct sf_clk_common *priv = hw_to_sf_clk_common(hw);
+	u32 cfg = sf_readl(priv, PLL_CMN_CFG1);
+	unsigned long refdiv = FIELD_GET(PLL_CMN_REFDIV, cfg);
+	unsigned long fbdiv = FIELD_GET(PLL_CMN_FBDIV, cfg);
+
+	if (!refdiv || !fbdiv)
+		return 0;
+
+	return (parent_rate / refdiv) * fbdiv;
+}
+
+static int sf21_cmnpll_vco_determine_rate(struct clk_hw *hw,
+					  struct clk_rate_request *req)
+{
+	unsigned long fbdiv, refdiv;
+
+	rational_best_approximation(req->rate, req->best_parent_rate,
+				    BIT(PLL_CMN_FBDIV_BITS) - 1,
+				    BIT(PLL_CMN_REFDIV_BITS) - 1, &fbdiv,
+				    &refdiv);
+	if (!refdiv || !fbdiv)
+		return -EINVAL;
+
+	req->rate = (req->best_parent_rate / refdiv) * fbdiv;
+
+	return 0;
+}
+
+static int sf21_cmnpll_vco_set_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long parent_rate)
+{
+	struct sf_clk_common *priv = hw_to_sf_clk_common(hw);
+	unsigned long flags;
+	unsigned long fbdiv, refdiv;
+	u32 val;
+	int ret;
+
+	rational_best_approximation(rate, parent_rate,
+				    BIT(PLL_CMN_FBDIV_BITS) - 1,
+				    BIT(PLL_CMN_REFDIV_BITS) - 1, &fbdiv,
+				    &refdiv);
+	if (!refdiv || !fbdiv)
+		return -EINVAL;
+
+	spin_lock_irqsave(priv->lock, flags);
+
+	sf_rmw(priv, PLL_CMN_CFG1, PLL_CMN_REFDIV | PLL_CMN_FBDIV | PLL_CMN_PD,
+	       FIELD_PREP(PLL_CMN_REFDIV, refdiv) |
+		       FIELD_PREP(PLL_CMN_FBDIV, fbdiv));
+	sf_writel(priv, CFG_LOAD, CFG_LOAD_CMN_PLL);
+	sf_writel(priv, CFG_LOAD, 0);
+
+	ret = readl_poll_timeout_atomic(priv->base + PLL_CMN_LOCK, val, val & 1,
+					0, PLL_LOCK_TIMEOUT_US);
+	if (ret)
+		goto out_unlock;
+
+out_unlock:
+	spin_unlock_irqrestore(priv->lock, flags);
+	return ret;
+}
+
+static const struct clk_ops sf21_cmnpll_vco_ops = {
+	.recalc_rate = sf21_cmnpll_vco_recalc_rate,
+	.determine_rate = sf21_cmnpll_vco_determine_rate,
+	.set_rate = sf21_cmnpll_vco_set_rate,
+};
+
+static struct sf_clk_common cmnpll_vco = {
+	.hw.init = CLK_HW_INIT_FW_NAME("cmnpll_vco", "xin25m",
+				       &sf21_cmnpll_vco_ops, 0),
+};
+
+static unsigned long sf21_dualdiv_round_rate(unsigned long rate,
+					     unsigned long parent_rate,
+					     unsigned int range,
+					     unsigned int *diva,
+					     unsigned int *divb)
+{
+	unsigned int div = DIV_ROUND_CLOSEST(parent_rate, rate);
+	unsigned int best_diff, da, db, cur_div, cur_diff;
+
+	if (div <= 1) {
+		*diva = 1;
+		*divb = 1;
+		return parent_rate;
+	}
+
+	best_diff = div - 1;
+	*diva = 1;
+	*divb = 1;
+
+	for (da = 1; da <= range; da++) {
+		db = DIV_ROUND_CLOSEST(div, da);
+		if (db > da)
+			db = da;
+
+		cur_div = da * db;
+		if (div > cur_div)
+			cur_diff = div - cur_div;
+		else
+			cur_diff = cur_div - div;
+
+		if (cur_diff < best_diff) {
+			best_diff = cur_diff;
+			*diva = da;
+			*divb = db;
+		}
+		if (cur_diff == 0)
+			break;
+	}
+
+	return parent_rate / *diva / *divb;
+}
+
+static int sf21_cmnpll_postdiv_determine_rate(struct clk_hw *hw,
+					      struct clk_rate_request *req)
+{
+	unsigned int diva, divb;
+
+	if (!req->rate)
+		return -EINVAL;
+
+	req->rate = sf21_dualdiv_round_rate(req->rate, req->best_parent_rate,
+					    7, &diva, &divb);
+
+	return 0;
+}
+
+static int sf21_cmnpll_postdiv_set_rate(struct clk_hw *hw,
+					unsigned long rate,
+					unsigned long parent_rate)
+{
+	struct sf_clk_common *priv = hw_to_sf_clk_common(hw);
+	unsigned int diva, divb;
+	unsigned long flags;
+
+	if (!rate)
+		return -EINVAL;
+
+	sf21_dualdiv_round_rate(rate, parent_rate, 7, &diva, &divb);
+
+	spin_lock_irqsave(priv->lock, flags);
+	sf_rmw(priv, PLL_CMN_CFG1, PLL_CMN_POSTDIV1 | PLL_CMN_POSTDIV2,
+	       FIELD_PREP(PLL_CMN_POSTDIV1, diva) |
+		       FIELD_PREP(PLL_CMN_POSTDIV2, divb));
+	sf_writel(priv, CFG_LOAD, CFG_LOAD_CMN_PLL);
+	sf_writel(priv, CFG_LOAD, 0);
+	spin_unlock_irqrestore(priv->lock, flags);
+	return 0;
+}
+
+static unsigned long
+sf21_cmnpll_postdiv_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct sf_clk_common *priv = hw_to_sf_clk_common(hw);
+	u32 cfg = sf_readl(priv, PLL_CMN_CFG1);
+	unsigned long div1 = FIELD_GET(PLL_CMN_POSTDIV1, cfg);
+	unsigned long div2 = FIELD_GET(PLL_CMN_POSTDIV2, cfg);
+
+	if (!div1 || !div2)
+		return 0;
+
+	return parent_rate / div1 / div2;
+}
+
+static const struct clk_ops sf21_cmnpll_postdiv_ops = {
+	.recalc_rate = sf21_cmnpll_postdiv_recalc_rate,
+	.determine_rate = sf21_cmnpll_postdiv_determine_rate,
+	.set_rate = sf21_cmnpll_postdiv_set_rate,
+};
+
+static struct sf_clk_common cmnpll_postdiv = {
+	.hw.init = CLK_HW_INIT_HW("cmnpll_postdiv", &cmnpll_vco.hw,
+				  &sf21_cmnpll_postdiv_ops, 0),
+};
+
+#define PLL_DDR_CFG1		0x18
+#define  PLL_DDR_BYPASS		BIT(23)
+#define  PLL_DDR_PLLEN		BIT(22)
+#define  PLL_DDR_4PHASEEN	BIT(21)
+#define  PLL_DDR_POSTDIVEN	BIT(20)
+#define  PLL_DDR_DSMEN		BIT(19)
+#define  PLL_DDR_DACEN		BIT(18)
+#define  PLL_DDR_DSKEWCALBYP	BIT(17)
+#define  PLL_DDR_DSKEWCALCNT	GENMASK(16, 14)
+#define  PLL_DDR_DSKEWCALEN	BIT(13)
+#define  PLL_DDR_DSKEWCALIN	GENMASK(12, 1)
+#define  PLL_DDR_DSKEWFASTCAL	BIT(0)
+
+#define PLL_DDR_CFG2		0x1c
+#define  PLL_DDR_POSTDIV1	GENMASK(29, 27)
+#define  PLL_DDR_POSTDIV2	GENMASK(26, 24)
+#define  PLL_DDR_FRAC		GENMASK(23, 0)
+
+#define PLL_DDR_CFG3		0x20
+#define  PLL_DDR_FBDIV		GENMASK(17, 6)
+#define  PLL_DDR_REFDIV		GENMASK(5, 0)
+
+static unsigned long
+sf21_ddrpll_postdiv_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct sf_clk_common *priv = hw_to_sf_clk_common(hw);
+	u32 cfg2 = sf_readl(priv, PLL_DDR_CFG2);
+	u32 postdiv1 = FIELD_GET(PLL_DDR_POSTDIV1, cfg2);
+	u32 postdiv2 = FIELD_GET(PLL_DDR_POSTDIV2, cfg2);
+	u32 cfg3 = sf_readl(priv, PLL_DDR_CFG3);
+	u32 fbdiv = FIELD_GET(PLL_DDR_FBDIV, cfg3);
+	u32 refdiv = FIELD_GET(PLL_DDR_REFDIV, cfg3);
+
+	if (!refdiv || !fbdiv || !postdiv1 || !postdiv2)
+		return 0;
+
+	return (parent_rate / refdiv) * fbdiv / postdiv1 / postdiv2;
+}
+
+static const struct clk_ops sf21_ddrpll_postdiv_ops = {
+	.recalc_rate = sf21_ddrpll_postdiv_recalc_rate,
+};
+
+static struct sf_clk_common ddrpll_postdiv = {
+	.hw.init = CLK_HW_INIT_FW_NAME("ddrpll_postdiv", "xin25m",
+				       &sf21_ddrpll_postdiv_ops, 0),
+};
+
+#define PLL_PCIE_CFG1		0x4c
+#define  PLL_PCIE_PLLEN		BIT(31)
+#define  PLL_PCIE_POSTDIV0PRE	BIT(30)
+#define  PLL_PCIE_REFDIV	GENMASK(29, 24)
+#define  PLL_PCIE_FRAC		GENMASK(23, 0)
+
+#define PLL_PCIE_CFG2		0x50
+#define  PLL_PCIE_FOUTEN(i)	BIT(28 + (i))
+#define  PLL_PCIE_BYPASS(i)	BIT(24 + (i))
+#define  PLL_PCIE_PDIVA_OFFS(i)	(21 - 6 * (i))
+#define  PLL_PCIE_PDIVB_OFFS(i)	(18 - 6 * (i))
+#define  PLL_PCIE_PDIV_MASK	GENMASK(2, 0)
+
+#define PLL_PCIE_CFG3		0x54
+#define  PLL_PCIE_DSKEWFASTCAL	BIT(31)
+#define  PLL_PCIE_DACEN		BIT(30)
+#define  PLL_PCIE_DSMEN		BIT(29)
+#define  PLL_PCIE_DSKEWCALEN	BIT(28)
+#define  PLL_PCIE_DSKEWCALBYP	BIT(27)
+#define  PLL_PCIE_DSKEWCALCNT	GENMASK(26, 24)
+#define  PLL_PCIE_DSKEWCALIN	GENMASK(23, 12)
+#define  PLL_PCIE_FBDIV		GENMASK(11, 0)
+
+static unsigned long
+sf21_pciepll_vco_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	struct sf_clk_common *priv = hw_to_sf_clk_common(hw);
+	u32 cfg1 = sf_readl(priv, PLL_PCIE_CFG1);
+	unsigned long refdiv = FIELD_GET(PLL_PCIE_REFDIV, cfg1);
+	u32 cfg3 = sf_readl(priv, PLL_PCIE_CFG3);
+	unsigned long fbdiv = FIELD_GET(PLL_PCIE_FBDIV, cfg3);
+
+	if (!refdiv || !fbdiv)
+		return 0;
+
+	return (parent_rate / refdiv) * fbdiv / 4;
+}
+
+static int sf21_pciepll_vco_enable(struct clk_hw *hw)
+{
+	struct sf_clk_common *priv = hw_to_sf_clk_common(hw);
+	unsigned long flags;
+	u32 val;
+	int ret;
+
+	spin_lock_irqsave(priv->lock, flags);
+	sf_rmw(priv, PLL_PCIE_CFG1, 0, PLL_PCIE_PLLEN);
+	sf_writel(priv, CFG_LOAD, CFG_LOAD_PCIE_PLL);
+	sf_writel(priv, CFG_LOAD, 0);
+	ret = readl_poll_timeout_atomic(priv->base + PLL_PCIE_LOCK, val, val & 1,
+					0, PLL_LOCK_TIMEOUT_US);
+	spin_unlock_irqrestore(priv->lock, flags);
+	return ret;
+}
+
+static void sf21_pciepll_vco_disable(struct clk_hw *hw)
+{
+	struct sf_clk_common *priv = hw_to_sf_clk_common(hw);
+	unsigned long flags;
+
+	spin_lock_irqsave(priv->lock, flags);
+	sf_rmw(priv, PLL_PCIE_CFG1, PLL_PCIE_PLLEN, 0);
+	sf_writel(priv, CFG_LOAD, CFG_LOAD_PCIE_PLL);
+	sf_writel(priv, CFG_LOAD, 0);
+	spin_unlock_irqrestore(priv->lock, flags);
+}
+
+static int sf21_pciepll_vco_is_enabled(struct clk_hw *hw)
+{
+	struct sf_clk_common *priv = hw_to_sf_clk_common(hw);
+
+	return !!(sf_readl(priv, PLL_PCIE_CFG1) & PLL_PCIE_PLLEN);
+}
+
+static const struct clk_ops sf21_pciepll_vco_ops = {
+	.enable = sf21_pciepll_vco_enable,
+	.disable = sf21_pciepll_vco_disable,
+	.is_enabled = sf21_pciepll_vco_is_enabled,
+	.recalc_rate = sf21_pciepll_vco_recalc_rate,
+};
+
+static struct sf_clk_common pciepll_vco = {
+	.hw.init = CLK_HW_INIT_FW_NAME("pciepll_vco", "xin25m",
+				       &sf21_pciepll_vco_ops,
+				       CLK_SET_RATE_GATE),
+};
+
+struct sf21_pciepll_fout {
+	struct sf_clk_common common;
+	u8 index;
+};
+
+static int sf21_pciepll_fout_enable(struct clk_hw *hw)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_pciepll_fout *priv =
+		container_of(cmn_priv, struct sf21_pciepll_fout, common);
+	unsigned long flags;
+
+	spin_lock_irqsave(cmn_priv->lock, flags);
+	sf_rmw(cmn_priv, PLL_PCIE_CFG2, 0, PLL_PCIE_FOUTEN(priv->index));
+	sf_writel(cmn_priv, CFG_LOAD, CFG_LOAD_PCIE_PLL);
+	sf_writel(cmn_priv, CFG_LOAD, 0);
+	spin_unlock_irqrestore(cmn_priv->lock, flags);
+	return 0;
+}
+
+static void sf21_pciepll_fout_disable(struct clk_hw *hw)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_pciepll_fout *priv =
+		container_of(cmn_priv, struct sf21_pciepll_fout, common);
+	unsigned long flags;
+
+	spin_lock_irqsave(cmn_priv->lock, flags);
+	sf_rmw(cmn_priv, PLL_PCIE_CFG2, PLL_PCIE_FOUTEN(priv->index), 0);
+	sf_writel(cmn_priv, CFG_LOAD, CFG_LOAD_PCIE_PLL);
+	sf_writel(cmn_priv, CFG_LOAD, 0);
+	spin_unlock_irqrestore(cmn_priv->lock, flags);
+}
+
+static int sf21_pciepll_fout_is_enabled(struct clk_hw *hw)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_pciepll_fout *priv =
+		container_of(cmn_priv, struct sf21_pciepll_fout, common);
+
+	return !!(sf_readl(cmn_priv, PLL_PCIE_CFG2) &
+		  PLL_PCIE_FOUTEN(priv->index));
+}
+
+static int sf21_pciepll_fout_determine_rate(struct clk_hw *hw,
+					    struct clk_rate_request *req)
+{
+	unsigned int diva, divb;
+
+	if (!req->rate)
+		return -EINVAL;
+
+	req->rate = sf21_dualdiv_round_rate(req->rate, req->best_parent_rate,
+					    8, &diva, &divb);
+
+	return 0;
+}
+
+static int sf21_pciepll_fout_set_rate(struct clk_hw *hw,
+				      unsigned long rate,
+				      unsigned long parent_rate)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_pciepll_fout *priv =
+		container_of(cmn_priv, struct sf21_pciepll_fout, common);
+	unsigned int diva, divb;
+	unsigned long flags;
+
+	if (!rate)
+		return -EINVAL;
+
+	sf21_dualdiv_round_rate(rate, parent_rate, 8, &diva, &divb);
+
+	spin_lock_irqsave(cmn_priv->lock, flags);
+	sf_rmw(cmn_priv, PLL_PCIE_CFG2,
+	       (PLL_PCIE_PDIV_MASK << PLL_PCIE_PDIVA_OFFS(priv->index)) |
+		       (PLL_PCIE_PDIV_MASK << PLL_PCIE_PDIVB_OFFS(priv->index)),
+	       ((diva - 1) << PLL_PCIE_PDIVA_OFFS(priv->index)) |
+		       ((divb - 1) << PLL_PCIE_PDIVB_OFFS(priv->index)));
+	sf_writel(cmn_priv, CFG_LOAD, CFG_LOAD_PCIE_PLL);
+	sf_writel(cmn_priv, CFG_LOAD, 0);
+	spin_unlock_irqrestore(cmn_priv->lock, flags);
+	return 0;
+}
+
+static unsigned long
+sf21_pciepll_fout_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_pciepll_fout *priv =
+		container_of(cmn_priv, struct sf21_pciepll_fout, common);
+	int idx = priv->index;
+	u32 cfg2 = sf_readl(cmn_priv, PLL_PCIE_CFG2);
+	ulong pdiva = (cfg2 >> PLL_PCIE_PDIVA_OFFS(idx)) & PLL_PCIE_PDIV_MASK;
+	ulong pdivb = (cfg2 >> PLL_PCIE_PDIVB_OFFS(idx)) & PLL_PCIE_PDIV_MASK;
+
+	return parent_rate / (pdiva + 1) / (pdivb + 1);
+}
+
+static const struct clk_ops sf21_pciepll_fout_ops = {
+	.enable = sf21_pciepll_fout_enable,
+	.disable = sf21_pciepll_fout_disable,
+	.is_enabled = sf21_pciepll_fout_is_enabled,
+	.recalc_rate = sf21_pciepll_fout_recalc_rate,
+	.determine_rate = sf21_pciepll_fout_determine_rate,
+	.set_rate = sf21_pciepll_fout_set_rate,
+};
+
+#define SF21_PCIEPLL_FOUT(_name, _idx, _flags)			\
+	struct sf21_pciepll_fout _name = {				\
+		.common.hw.init = CLK_HW_INIT_HW(#_name,		\
+						 &pciepll_vco.hw,	\
+						 &sf21_pciepll_fout_ops,\
+						 _flags),		\
+		.index = _idx,						\
+	}
+
+static SF21_PCIEPLL_FOUT(pciepll_fout0, 0, 0);
+static SF21_PCIEPLL_FOUT(pciepll_fout1, 1, 0);
+static SF21_PCIEPLL_FOUT(pciepll_fout2, 2, 0);
+static SF21_PCIEPLL_FOUT(pciepll_fout3, 3, 0);
+
+struct sf21_clk_muxdiv {
+	struct sf_clk_common common;
+	u16 en;
+	u8 mux_reg;
+	u8 mux_offs;
+	u8 div_reg;
+	u8 div_offs;
+};
+
+#define CRM_CLK_SEL(_x)		((_x) * 4 + 0x80)
+#define  CLK_SEL1_PLL_TEST	GENMASK(6, 4)
+#define CRM_CLK_EN		0x8c
+#define CRM_CLK_DIV(_x)		((_x) * 4 + 0x94)
+#define  CRM_CLK_DIV_MASK	GENMASK(7, 0)
+
+static unsigned long sf21_muxdiv_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_clk_muxdiv *priv =
+		container_of(cmn_priv, struct sf21_clk_muxdiv, common);
+	ulong div_reg = CRM_CLK_DIV(priv->div_reg);
+	u16 div_offs = priv->div_offs;
+	u16 div_val = (sf_readl(cmn_priv, div_reg) >> div_offs) &
+		      CRM_CLK_DIV_MASK;
+	div_val += 1;
+	return parent_rate / div_val;
+}
+
+static int sf21_muxdiv_determine_rate(struct clk_hw *hw,
+				      struct clk_rate_request *req)
+{
+	unsigned int div;
+
+	if (!req->rate)
+		return -EINVAL;
+
+	div = DIV_ROUND_CLOSEST(req->best_parent_rate, req->rate);
+	if (!div)
+		div = 1;
+	else if (div > CRM_CLK_DIV_MASK + 1)
+		div = CRM_CLK_DIV_MASK + 1;
+
+	req->rate = req->best_parent_rate / div;
+	return 0;
+}
+
+static int sf21_muxdiv_set_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_rate)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_clk_muxdiv *priv =
+		container_of(cmn_priv, struct sf21_clk_muxdiv, common);
+	ulong div_reg = CRM_CLK_DIV(priv->div_reg);
+	u16 div_offs = priv->div_offs;
+	unsigned long flags;
+	unsigned int div;
+
+	if (!rate)
+		return -EINVAL;
+
+	div = DIV_ROUND_CLOSEST(parent_rate, rate);
+	if (div < 1)
+		div = 1;
+	else if (div > CRM_CLK_DIV_MASK + 1)
+		div = CRM_CLK_DIV_MASK + 1;
+	div -= 1;
+
+	spin_lock_irqsave(cmn_priv->lock, flags);
+	sf_rmw(cmn_priv, div_reg, CRM_CLK_DIV_MASK << div_offs,
+	       div << div_offs);
+	sf_writel(cmn_priv, CFG_LOAD, CFG_LOAD_DIV);
+	sf_writel(cmn_priv, CFG_LOAD, 0);
+	spin_unlock_irqrestore(cmn_priv->lock, flags);
+	return 0;
+}
+
+static int sf21_muxdiv_enable(struct clk_hw *hw)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_clk_muxdiv *priv =
+		container_of(cmn_priv, struct sf21_clk_muxdiv, common);
+	unsigned long flags;
+
+	spin_lock_irqsave(cmn_priv->lock, flags);
+	sf_rmw(cmn_priv, CRM_CLK_EN, 0, BIT(priv->en));
+	/*
+	 * Clock divider value load only happens when the clock is running.
+	 * Pulse the CFG_LOAD_DIV so that set_rate() which happened
+	 * before enable() is applied.
+	 */
+	sf_writel(cmn_priv, CFG_LOAD, CFG_LOAD_DIV);
+	sf_writel(cmn_priv, CFG_LOAD, 0);
+	spin_unlock_irqrestore(cmn_priv->lock, flags);
+	return 0;
+}
+
+static void sf21_muxdiv_disable(struct clk_hw *hw)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_clk_muxdiv *priv =
+		container_of(cmn_priv, struct sf21_clk_muxdiv, common);
+	unsigned long flags;
+
+	spin_lock_irqsave(cmn_priv->lock, flags);
+	sf_rmw(cmn_priv, CRM_CLK_EN, BIT(priv->en), 0);
+	spin_unlock_irqrestore(cmn_priv->lock, flags);
+}
+
+static int sf21_muxdiv_is_enabled(struct clk_hw *hw)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_clk_muxdiv *priv =
+		container_of(cmn_priv, struct sf21_clk_muxdiv, common);
+	u32 reg_val = sf_readl(cmn_priv, CRM_CLK_EN);
+
+	return reg_val & (BIT(priv->en)) ? 1 : 0;
+}
+
+static u8 sf21_muxdiv_get_parent(struct clk_hw *hw)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_clk_muxdiv *priv =
+		container_of(cmn_priv, struct sf21_clk_muxdiv, common);
+	ulong mux_reg = CRM_CLK_SEL(priv->mux_reg);
+	u16 mux_offs = priv->mux_offs;
+	u32 reg_val = sf_readl(cmn_priv, mux_reg);
+
+	return reg_val & BIT(mux_offs) ? 1 : 0;
+}
+
+static int sf21_muxdiv_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	struct sf21_clk_muxdiv *priv =
+		container_of(cmn_priv, struct sf21_clk_muxdiv, common);
+	ulong mux_reg = CRM_CLK_SEL(priv->mux_reg);
+	u16 mux_offs = priv->mux_offs;
+	unsigned long flags;
+
+	spin_lock_irqsave(cmn_priv->lock, flags);
+	if (index)
+		sf_rmw(cmn_priv, mux_reg, 0, BIT(mux_offs));
+	else
+		sf_rmw(cmn_priv, mux_reg, BIT(mux_offs), 0);
+
+	spin_unlock_irqrestore(cmn_priv->lock, flags);
+	return 0;
+}
+
+static const struct clk_ops sf21_clk_muxdiv_ops = {
+	.enable = sf21_muxdiv_enable,
+	.disable = sf21_muxdiv_disable,
+	.is_enabled = sf21_muxdiv_is_enabled,
+	.recalc_rate = sf21_muxdiv_recalc_rate,
+	.determine_rate = sf21_muxdiv_determine_rate,
+	.set_rate = sf21_muxdiv_set_rate,
+	.get_parent = sf21_muxdiv_get_parent,
+	.set_parent = sf21_muxdiv_set_parent,
+};
+
+#define SF21_MUXDIV(_name, _parents, _mux_reg, _mux_offs, _div_reg,	\
+		    _div_offs, _en, _flags)				\
+	struct sf21_clk_muxdiv _name = {				\
+		.common.hw.init = CLK_HW_INIT_PARENTS_HW(		\
+		#_name, _parents, &sf21_clk_muxdiv_ops, _flags),	\
+		.en = _en,						\
+		.mux_reg = _mux_reg,					\
+		.mux_offs = _mux_offs,					\
+		.div_reg = _div_reg,					\
+		.div_offs = _div_offs,					\
+	}
+
+static const struct clk_hw *clk_periph_parents[] = {
+	&cmnpll_postdiv.hw,
+	&ddrpll_postdiv.hw,
+};
+
+static const struct clk_hw *clk_ddr_parents[] = {
+	&ddrpll_postdiv.hw,
+	&cmnpll_postdiv.hw,
+};
+
+static const struct clk_hw *clk_gmac_usb_parents[] = {
+	&cmnpll_vco.hw,
+	&ddrpll_postdiv.hw,
+};
+
+static SF21_MUXDIV(muxdiv_cpu, clk_periph_parents, 0, 1, 0, 0, 0,
+		   CLK_IGNORE_UNUSED);
+static SF21_MUXDIV(muxdiv_pic, clk_periph_parents, 0, 3, 3, 16, 1,
+		   CLK_IGNORE_UNUSED);
+static SF21_MUXDIV(muxdiv_axi, clk_periph_parents, 0, 5, 0, 8, 2,
+		   CLK_IS_CRITICAL);
+static SF21_MUXDIV(muxdiv_ahb, clk_periph_parents, 0, 7, 0, 16, 3,
+		   CLK_IS_CRITICAL);
+static SF21_MUXDIV(muxdiv_apb, clk_periph_parents, 0, 9, 0, 24, 4,
+		   CLK_IS_CRITICAL);
+static SF21_MUXDIV(muxdiv_uart, clk_periph_parents, 0, 11, 1, 0, 5, 0);
+static SF21_MUXDIV(muxdiv_iram, clk_periph_parents, 0, 13, 1, 8, 6, 0);
+static SF21_MUXDIV(muxdiv_npu, clk_periph_parents, 0, 17, 1, 24, 8, 0);
+static SF21_MUXDIV(muxdiv_ddrphy, clk_ddr_parents, 0, 19, 2, 0, 9,
+		   CLK_IS_CRITICAL);
+static SF21_MUXDIV(muxdiv_ddr_bypass, clk_ddr_parents, 0, 21, 3, 0, 10,
+		   CLK_IS_CRITICAL);
+static SF21_MUXDIV(muxdiv_ethtsu, clk_periph_parents, 0, 25, 2, 16, 12,
+		   0);
+static SF21_MUXDIV(muxdiv_gmac_byp_ref, clk_gmac_usb_parents, 0, 27, 2,
+		   24, 13, 0);
+static SF21_MUXDIV(muxdiv_usb, clk_gmac_usb_parents, 1, 1, 1, 16, 24, 0);
+static SF21_MUXDIV(muxdiv_usbphy, clk_gmac_usb_parents, 1, 3, 2, 8, 25,
+		   0);
+static SF21_MUXDIV(muxdiv_serdes_csr, clk_periph_parents, 1, 15, 5, 0,
+		   20, 0);
+static SF21_MUXDIV(muxdiv_crypt_csr, clk_periph_parents, 1, 17, 5, 8,
+		   21, 0);
+static SF21_MUXDIV(muxdiv_crypt_app, clk_periph_parents, 1, 19, 5, 16,
+		   22, 0);
+static SF21_MUXDIV(muxdiv_irom, clk_periph_parents, 1, 21, 5, 24, 23,
+		   CLK_IS_CRITICAL);
+
+static int sf21_mux_determine_rate(struct clk_hw *hw,
+				   struct clk_rate_request *req)
+{
+	req->rate = req->best_parent_rate;
+	return 0;
+}
+
+static const struct clk_ops sf21_clk_mux_ops = {
+	.get_parent = sf21_muxdiv_get_parent,
+	.set_parent = sf21_muxdiv_set_parent,
+	.determine_rate = sf21_mux_determine_rate,
+};
+
+#define SF21_MUX(_name, _parents, _mux_reg, _mux_offs, _flags)	\
+	struct sf21_clk_muxdiv _name = {				\
+		.common.hw.init = CLK_HW_INIT_PARENTS_DATA(		\
+			#_name, _parents, &sf21_clk_mux_ops, _flags),	\
+		.en = 0,						\
+		.mux_reg = _mux_reg,					\
+		.mux_offs = _mux_offs,					\
+		.div_reg = 0,						\
+		.div_offs = 0,						\
+	}
+
+static const struct clk_parent_data clk_boot_parents[] = {
+	{ .hw = &muxdiv_irom.common.hw },
+	{ .fw_name = "xin25m" },
+};
+
+static SF21_MUX(mux_boot, clk_boot_parents, 0, 30, CLK_IS_CRITICAL);
+
+static const struct clk_ops sf21_clk_div_ops = {
+	.recalc_rate = sf21_muxdiv_recalc_rate,
+	.determine_rate = sf21_muxdiv_determine_rate,
+	.set_rate = sf21_muxdiv_set_rate,
+};
+
+#define SF21_DIV(_name, _parent, _div_reg, _div_offs, _flags)		\
+	struct sf21_clk_muxdiv _name = {				\
+		.common.hw.init = CLK_HW_INIT_FW_NAME(			\
+			#_name, _parent, &sf21_clk_div_ops, _flags),	\
+		.en = 0,						\
+		.mux_reg = 0,						\
+		.mux_offs = 0,						\
+		.div_reg = _div_reg,					\
+		.div_offs = _div_offs,					\
+	}
+
+static SF21_DIV(div_pvt, "xin25m", 3, 8, 0);
+
+static const struct clk_hw *clk_pll_test_parents[] = {
+	&cmnpll_postdiv.hw,
+	&ddrpll_postdiv.hw,
+	&pciepll_fout3.common.hw,
+};
+
+static u8 sf21_pll_test_get_parent(struct clk_hw *hw)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	u32 reg_val = sf_readl(cmn_priv, CRM_CLK_SEL(1));
+	u8 parent = FIELD_GET(CLK_SEL1_PLL_TEST, reg_val);
+
+	if (parent >= ARRAY_SIZE(clk_pll_test_parents))
+		return 0;
+
+	return parent;
+}
+
+static int sf21_pll_test_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct sf_clk_common *cmn_priv = hw_to_sf_clk_common(hw);
+	unsigned long flags;
+
+	if (index >= ARRAY_SIZE(clk_pll_test_parents))
+		return -EINVAL;
+
+	spin_lock_irqsave(cmn_priv->lock, flags);
+	sf_rmw(cmn_priv, CRM_CLK_SEL(1), CLK_SEL1_PLL_TEST,
+	       FIELD_PREP(CLK_SEL1_PLL_TEST, index));
+	spin_unlock_irqrestore(cmn_priv->lock, flags);
+	return 0;
+}
+
+static const struct clk_ops sf21_clk_pll_test_ops = {
+	.recalc_rate = sf21_muxdiv_recalc_rate,
+	.determine_rate = sf21_muxdiv_determine_rate,
+	.set_rate = sf21_muxdiv_set_rate,
+	.get_parent = sf21_pll_test_get_parent,
+	.set_parent = sf21_pll_test_set_parent,
+};
+
+static struct sf21_clk_muxdiv muxdiv_pll_test = {
+	.common.hw.init = CLK_HW_INIT_PARENTS_HW("muxdiv_pll_test",
+						 clk_pll_test_parents,
+						 &sf21_clk_pll_test_ops, 0),
+	.en = 0,
+	.mux_reg = 0,
+	.mux_offs = 0,
+	.div_reg = 3,
+	.div_offs = 24,
+};
+
+static const struct clk_ops sf21_clk_gate_ops = {
+	.enable = sf21_muxdiv_enable,
+	.disable = sf21_muxdiv_disable,
+	.is_enabled = sf21_muxdiv_is_enabled,
+};
+
+#define SF21_GATE(_name, _parents, _en, _flags)				\
+	struct sf21_clk_muxdiv _name = {				\
+		.common.hw.init = CLK_HW_INIT_PARENTS_HW(		\
+			#_name, _parents, &sf21_clk_gate_ops, _flags),	\
+		.en = _en,						\
+		.mux_reg = 0,						\
+		.mux_offs = 0,						\
+		.div_reg = 0,						\
+		.div_offs = 0,						\
+	}
+
+static const struct clk_hw *clk_pcie_parents[] = {
+	&pciepll_fout1.common.hw,
+};
+
+static SF21_GATE(pcie_refclk_n, clk_pcie_parents, 15, 0);
+static SF21_GATE(pcie_refclk_p, clk_pcie_parents, 16, 0);
+
+static struct clk_hw_onecell_data sf21_hw_clks = {
+	.num = SF21_CLK_MAX,
+	.hws = {
+		[SF21_CLK_CMNPLL_VCO] = &cmnpll_vco.hw,
+		[SF21_CLK_CMNPLL_POSTDIV] = &cmnpll_postdiv.hw,
+		[SF21_CLK_DDRPLL_POSTDIV] = &ddrpll_postdiv.hw,
+		[SF21_CLK_PCIEPLL_VCO] = &pciepll_vco.hw,
+		[SF21_CLK_PCIEPLL_FOUT0] = &pciepll_fout0.common.hw,
+		[SF21_CLK_PCIEPLL_FOUT1] = &pciepll_fout1.common.hw,
+		[SF21_CLK_PCIEPLL_FOUT2] = &pciepll_fout2.common.hw,
+		[SF21_CLK_PCIEPLL_FOUT3] = &pciepll_fout3.common.hw,
+		[SF21_CLK_CPU] = &muxdiv_cpu.common.hw,
+		[SF21_CLK_PIC] = &muxdiv_pic.common.hw,
+		[SF21_CLK_AXI] = &muxdiv_axi.common.hw,
+		[SF21_CLK_AHB] = &muxdiv_ahb.common.hw,
+		[SF21_CLK_APB] = &muxdiv_apb.common.hw,
+		[SF21_CLK_UART] = &muxdiv_uart.common.hw,
+		[SF21_CLK_IRAM] = &muxdiv_iram.common.hw,
+		[SF21_CLK_NPU] = &muxdiv_npu.common.hw,
+		[SF21_CLK_DDRPHY_REF] = &muxdiv_ddrphy.common.hw,
+		[SF21_CLK_DDR_BYPASS] = &muxdiv_ddr_bypass.common.hw,
+		[SF21_CLK_ETHTSU] = &muxdiv_ethtsu.common.hw,
+		[SF21_CLK_GMAC_BYP_REF] = &muxdiv_gmac_byp_ref.common.hw,
+		[SF21_CLK_USB] = &muxdiv_usb.common.hw,
+		[SF21_CLK_USBPHY] = &muxdiv_usbphy.common.hw,
+		[SF21_CLK_SERDES_CSR] = &muxdiv_serdes_csr.common.hw,
+		[SF21_CLK_CRYPT_CSR] = &muxdiv_crypt_csr.common.hw,
+		[SF21_CLK_CRYPT_APP] = &muxdiv_crypt_app.common.hw,
+		[SF21_CLK_IROM] = &muxdiv_irom.common.hw,
+		[SF21_CLK_BOOT] = &mux_boot.common.hw,
+		[SF21_CLK_PVT] = &div_pvt.common.hw,
+		[SF21_CLK_PLL_TEST] = &muxdiv_pll_test.common.hw,
+		[SF21_CLK_PCIE_REFN] = &pcie_refclk_n.common.hw,
+		[SF21_CLK_PCIE_REFP] = &pcie_refclk_p.common.hw,
+	}
+};
+
+struct sf21_clk_ctrl {
+	void __iomem *base;
+	/* Serializes register RMW sequences shared by clocks and resets. */
+	spinlock_t lock;
+	struct reset_controller_dev rcdev;
+	const u32 *reset_bits;
+	unsigned int nr_resets;
+};
+
+#define SF21_SOFT_RESET		0xc0
+
+static const u32 sf21_topcrm_reset_bits[] = {
+	[SF21_RESET_GIC]			= BIT(1),
+	[SF21_RESET_AXI]			= BIT(2),
+	[SF21_RESET_AHB]			= BIT(3),
+	[SF21_RESET_APB]			= BIT(4),
+	[SF21_RESET_IRAM]			= BIT(5),
+	[SF21_RESET_NPU]			= BIT(7),
+	[SF21_RESET_DDR_CTL]			= BIT(8),
+	[SF21_RESET_DDR_PHY]			= BIT(9),
+	[SF21_RESET_DDR_PWR_OK_IN]		= BIT(10),
+	[SF21_RESET_DDR_CTL_APB]		= BIT(11),
+	[SF21_RESET_DDR_PHY_APB]		= BIT(12),
+	[SF21_RESET_USB]			= BIT(19),
+	[SF21_RESET_PVT]			= BIT(23),
+	[SF21_RESET_SERDES_CSR]			= BIT(24),
+	[SF21_RESET_CRYPT_CSR]			= BIT(28),
+	[SF21_RESET_CRYPT_APP]			= BIT(29),
+	[SF21_RESET_NPU2DDR_ASYNCBRIDGE]	= BIT(30),
+	[SF21_RESET_IROM]			= BIT(31),
+};
+
+static inline struct sf21_clk_ctrl *
+rcdev_to_sf21_topcrm(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct sf21_clk_ctrl, rcdev);
+}
+
+static int sf21_topcrm_reset_update(struct reset_controller_dev *rcdev,
+				    unsigned long id, bool assert)
+{
+	struct sf21_clk_ctrl *ctrl = rcdev_to_sf21_topcrm(rcdev);
+	u32 bit = ctrl->reset_bits[id];
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(&ctrl->lock, flags);
+	reg = readl(ctrl->base + SF21_SOFT_RESET);
+	if (assert)
+		reg &= ~bit;
+	else
+		reg |= bit;
+	writel(reg, ctrl->base + SF21_SOFT_RESET);
+	spin_unlock_irqrestore(&ctrl->lock, flags);
+
+	return 0;
+}
+
+static int sf21_topcrm_reset_assert(struct reset_controller_dev *rcdev,
+				    unsigned long id)
+{
+	return sf21_topcrm_reset_update(rcdev, id, true);
+}
+
+static int sf21_topcrm_reset_deassert(struct reset_controller_dev *rcdev,
+				      unsigned long id)
+{
+	return sf21_topcrm_reset_update(rcdev, id, false);
+}
+
+static int sf21_topcrm_reset_status(struct reset_controller_dev *rcdev,
+				    unsigned long id)
+{
+	struct sf21_clk_ctrl *ctrl = rcdev_to_sf21_topcrm(rcdev);
+	u32 bit = ctrl->reset_bits[id];
+
+	return !(readl(ctrl->base + SF21_SOFT_RESET) & bit);
+}
+
+static const struct reset_control_ops sf21_topcrm_reset_ops = {
+	.assert = sf21_topcrm_reset_assert,
+	.deassert = sf21_topcrm_reset_deassert,
+	.status = sf21_topcrm_reset_status,
+};
+
+static int sf21_topcrm_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct sf21_clk_ctrl *ctrl;
+	int i, ret;
+
+	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	ctrl->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ctrl->base))
+		return dev_err_probe(dev, PTR_ERR(ctrl->base),
+				     "failed to map resources\n");
+
+	spin_lock_init(&ctrl->lock);
+
+	for (i = 0; i < sf21_hw_clks.num; i++) {
+		struct clk_hw *hw = sf21_hw_clks.hws[i];
+		struct sf_clk_common *common;
+
+		if (!hw)
+			continue;
+		common = hw_to_sf_clk_common(hw);
+		common->base = ctrl->base;
+		common->lock = &ctrl->lock;
+		ret = devm_clk_hw_register(dev, hw);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to register clock %d\n",
+					     i);
+	}
+
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+					  &sf21_hw_clks);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to add hw provider\n");
+
+	ctrl->reset_bits = sf21_topcrm_reset_bits;
+	ctrl->nr_resets = ARRAY_SIZE(sf21_topcrm_reset_bits);
+	ctrl->rcdev.owner = THIS_MODULE;
+	ctrl->rcdev.nr_resets = ctrl->nr_resets;
+	ctrl->rcdev.ops = &sf21_topcrm_reset_ops;
+	ctrl->rcdev.of_node = dev->of_node;
+	ctrl->rcdev.of_reset_n_cells = 1;
+
+	ret = devm_reset_controller_register(dev, &ctrl->rcdev);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to register reset controller\n");
+
+	return 0;
+}
+
+static const struct of_device_id sf21_topcrm_dt_ids[] = {
+	{ .compatible = "siflower,sf21-topcrm" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, sf21_topcrm_dt_ids);
+
+static struct platform_driver sf21_topcrm_driver = {
+	.probe	= sf21_topcrm_probe,
+	.driver	= {
+		.name			= "sf21-topcrm",
+		.suppress_bind_attrs	= true,
+		.of_match_table		= sf21_topcrm_dt_ids,
+	},
+};
+module_platform_driver(sf21_topcrm_driver);
+
+MODULE_AUTHOR("Chuanhong Guo <gch981213@gmail.com>");
+MODULE_DESCRIPTION("driver for Siflower SF21 top clock and reset module");
+MODULE_LICENSE("GPL");

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] riscv: add Siflower RISC-V SoC family Kconfig support
  2026-05-17 14:12 ` [PATCH 1/4] riscv: add Siflower RISC-V SoC family Kconfig support Chuanhong Guo
@ 2026-05-17 14:31   ` sashiko-bot
  2026-05-17 20:46     ` Conor Dooley
  0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-05-17 14:31 UTC (permalink / raw)
  To: Chuanhong Guo; +Cc: robh, conor+dt, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Conditionally selecting the `ARM_AMBA` hardware bus based on the `TTY` software subsystem.
--

commit 49824e33ae15603df7f899c7df0a4ab9b5f20eed
Author: Chuanhong Guo <gch981213@gmail.com>

riscv: add Siflower RISC-V SoC family Kconfig support

This adds Kconfig support for the Siflower RISC-V SoCs, including the
SF21A6826 and SF21H8898 chips. It defines ARCH_SIFLOWER, selects the
relevant T-Head errata, and conditionally selects ARM_AMBA for the UART.

> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index c174ac0ec46b4..9996591cd9dbf 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
[ ... ]
> @@ -37,6 +37,13 @@ config ARCH_SIFIVE
>  	help
>  	  This enables support for SiFive SoC platform hardware.
>  
> +config ARCH_SIFLOWER
> +	bool "Siflower RISC-V SoCs"
> +	select ARM_AMBA if TTY

Should ARM_AMBA be selected conditionally based on TTY?

ARM_AMBA models the presence of a physical hardware bus, which exists
regardless of whether the TTY software subsystem is enabled. Even if the
PL011 UART is currently the only AMBA device on this platform, tying the
hardware bus infrastructure to a software abstraction might cause issues if
other AMBA devices are added later and TTY is disabled.

Would it be better to select ARM_AMBA unconditionally, similar to how other
architectures handle it?

> +	select ERRATA_THEAD
> +	help
> +	  This enables support for Siflower RISC-V SoC platform hardware.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517-sf21-topcrm-v1-0-438f2e0513ff@gmail.com?part=1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] dt-bindings: clock: add binding header for sf21-topcrm
  2026-05-17 14:12 ` [PATCH 2/4] dt-bindings: clock: add binding header for sf21-topcrm Chuanhong Guo
@ 2026-05-17 14:36   ` sashiko-bot
  2026-05-17 20:47   ` Conor Dooley
  1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-05-17 14:36 UTC (permalink / raw)
  To: Chuanhong Guo; +Cc: conor+dt, robh, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The dt-bindings header file exposes software-specific `MAX` macros (`SF21_CLK_MAX` and `SF21_RESET_MAX`).
--

commit b39e2fdfa1953205f67f3e13ae23a09e43917c11
Author: Chuanhong Guo <gch981213@gmail.com>

dt-bindings: clock: add binding header for sf21-topcrm

Add the device tree binding header for Siflower SF21A6826/SF21H8898
toplevel clock and reset module. The header covers both clock and
reset IDs provided by the block.

> diff --git a/include/dt-bindings/clock/siflower,sf21-topcrm.h b/include/dt-bindings/clock/siflower,sf21-topcrm.h
> new file mode 100644
> index 0000000000000..3690b3452501e
> --- /dev/null
> +++ b/include/dt-bindings/clock/siflower,sf21-topcrm.h
[ ... ]
> +#define SF21_CLK_PCIE_REFN		29
> +#define SF21_CLK_PCIE_REFP		30
> +#define SF21_CLK_MAX			31

Should this MAX macro be exposed in the dt-bindings header?

Device tree binding headers define the hardware ABI, and the total number
of clocks is an internal software implementation detail. Changing this
value when new clocks are added in the future breaks ABI compatibility for
components compiled against the older header.

Could this macro be defined internally within the driver source code instead?

> +
> +#define SF21_RESET_GIC			0
[ ... ]
> +#define SF21_RESET_NPU2DDR_ASYNCBRIDGE	16
> +#define SF21_RESET_IROM			17
> +#define SF21_RESET_MAX			18

Similarly, should SF21_RESET_MAX be moved to the driver source code to
avoid breaking ABI compatibility if new resets are added?

> +
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517-sf21-topcrm-v1-0-438f2e0513ff@gmail.com?part=2

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] clk: add support for siflower sf21-topcrm
  2026-05-17 14:12 ` [PATCH 4/4] clk: add support for siflower sf21-topcrm Chuanhong Guo
@ 2026-05-17 15:09   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-05-17 15:09 UTC (permalink / raw)
  To: Chuanhong Guo; +Cc: conor+dt, robh, devicetree

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Static initialization of the flexible array in `sf21_hw_clks` creates a critical out-of-bounds memory access vulnerability.
- [High] Lock nesting violation on PREEMPT_RT due to `spinlock_t` usage inside `.enable`/`.disable` callbacks.
- [High] Global static clock definition structures are modified with per-device dynamically allocated pointers.
- [High] Missing `select RATIONAL` in Kconfig leads to linker errors.
- [Medium] Integer truncation in PLL rate calculations causes precision loss.
- [Medium] Silent acceptance of invalid or unsupported soft reset IDs.
--

commit 83e3e7ec9410a85a7d5912f8b3485089ca10bdce
Author: Chuanhong Guo <gch981213@gmail.com>

clk: add support for siflower sf21-topcrm

This commit adds a driver for the toplevel clock and reset controller
found on Siflower SF21A6826/SF21H8898 SoCs. It includes support for
multiple PLLs, mux/gate/divider blocks, and a reset register.

> diff --git a/drivers/clk/siflower/Kconfig b/drivers/clk/siflower/Kconfig
> new file mode 100644
> index 0000000000000..03cbfbdbdb8db
> --- /dev/null
> +++ b/drivers/clk/siflower/Kconfig
> @@ -0,0 +1,22 @@
[ ... ]
> +config CLK_SF21_TOPCRM
> +	tristate "Clock driver for Siflower SF21 toplevel clock & reset module"
> +	depends on ARCH_SIFLOWER || COMPILE_TEST
> +	default ARCH_SIFLOWER
> +	select RESET_CONTROLLER

Should this select RATIONAL? The driver calls rational_best_approximation()
in rate calculations, which could lead to linker errors if RATIONAL is not
enabled during a COMPILE_TEST build.

[ ... ]
> diff --git a/drivers/clk/siflower/clk-sf21-topcrm.c b/drivers/clk/siflower/clk-sf21-topcrm.c
> new file mode 100644
> index 0000000000000..7d4c5e370d6d2
> --- /dev/null
> +++ b/drivers/clk/siflower/clk-sf21-topcrm.c
> @@ -0,0 +1,1053 @@
[ ... ]
> +struct sf_clk_common {
> +	void __iomem	*base;
> +	/* Serializes register RMW sequences shared by clocks and resets. */
> +	spinlock_t	*lock;

Could this lock nesting cause issues on PREEMPT_RT?

The Common Clock Framework invokes .enable and .disable callbacks while
holding its own enable_lock, which is a raw_spinlock_t. Acquiring a
spinlock_t inside a raw spinlock critical section is a lock nesting
violation and can lead to a scheduling-while-atomic panic on PREEMPT_RT
kernels.

Does this hardware lock need to be a raw_spinlock_t?

> +	struct clk_hw	hw;
> +};
[ ... ]
> +static unsigned long sf21_cmnpll_vco_recalc_rate(struct clk_hw *hw,
> +						 unsigned long parent_rate)
> +{
> +	struct sf_clk_common *priv = hw_to_sf_clk_common(hw);
> +	u32 cfg = sf_readl(priv, PLL_CMN_CFG1);
> +	unsigned long refdiv = FIELD_GET(PLL_CMN_REFDIV, cfg);
> +	unsigned long fbdiv = FIELD_GET(PLL_CMN_FBDIV, cfg);
> +
> +	if (!refdiv || !fbdiv)
> +		return 0;
> +
> +	return (parent_rate / refdiv) * fbdiv;

Could this integer division truncate the remainder and cause precision
loss in the reported frequency?

This pattern also appears in other rate calculation functions like
sf21_pciepll_vco_recalc_rate and sf21_ddrpll_postdiv_recalc_rate. Would it
be better to use 64-bit math such as div_u64((u64)parent_rate * fbdiv,
refdiv) or mult_frac()?

> +}
[ ... ]
> +static struct clk_hw_onecell_data sf21_hw_clks = {
> +	.num = SF21_CLK_MAX,
> +	.hws = {
> +		[SF21_CLK_CMNPLL_VCO] = &cmnpll_vco.hw,
[ ... ]
> +		[SF21_CLK_PCIE_REFN] = &pcie_refclk_n.common.hw,
> +		[SF21_CLK_PCIE_REFP] = &pcie_refclk_p.common.hw,
> +	}
> +};

If SF21_CLK_MAX is larger than the number of items explicitly initialized
in this array, could this lead to an out-of-bounds memory read?

When statically initialized, the compiler only allocates space up to the
highest explicitly initialized index (SF21_CLK_PCIE_REFP). If the loop in
the probe function reads up to sf21_hw_clks.num, it could read past the end
of the allocated structure.

[ ... ]
> +struct sf21_clk_ctrl {
> +	void __iomem *base;
> +	/* Serializes register RMW sequences shared by clocks and resets. */
> +	spinlock_t lock;
> +	struct reset_controller_dev rcdev;
> +	const u32 *reset_bits;
> +	unsigned int nr_resets;
> +};
[ ... ]
> +static int sf21_topcrm_reset_update(struct reset_controller_dev *rcdev,
> +				    unsigned long id, bool assert)
> +{
> +	struct sf21_clk_ctrl *ctrl = rcdev_to_sf21_topcrm(rcdev);
> +	u32 bit = ctrl->reset_bits[id];

Can this accept invalid or unsupported reset IDs? Since the
sf21_topcrm_reset_bits array has unpopulated gaps, bit will evaluate to 0
for unsupported IDs.

If this happens, the code blindly performs reg &= ~0 or reg |= 0 and
returns success without throwing an error. Should it verify that the bit
is valid and return -EINVAL if not?

> +	unsigned long flags;
> +	u32 reg;
> +
> +	spin_lock_irqsave(&ctrl->lock, flags);
> +	reg = readl(ctrl->base + SF21_SOFT_RESET);
> +	if (assert)
> +		reg &= ~bit;
> +	else
> +		reg |= bit;
> +	writel(reg, ctrl->base + SF21_SOFT_RESET);
> +	spin_unlock_irqrestore(&ctrl->lock, flags);
> +
> +	return 0;
> +}
[ ... ]
> +static int sf21_topcrm_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct sf21_clk_ctrl *ctrl;
> +	int i, ret;
> +
> +	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
> +	if (!ctrl)
> +		return -ENOMEM;
> +
> +	ctrl->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(ctrl->base))
> +		return dev_err_probe(dev, PTR_ERR(ctrl->base),
> +				     "failed to map resources\n");
> +
> +	spin_lock_init(&ctrl->lock);
> +
> +	for (i = 0; i < sf21_hw_clks.num; i++) {
> +		struct clk_hw *hw = sf21_hw_clks.hws[i];
> +		struct sf_clk_common *common;
> +
> +		if (!hw)
> +			continue;
> +		common = hw_to_sf_clk_common(hw);
> +		common->base = ctrl->base;
> +		common->lock = &ctrl->lock;

Could modifying global static clock definitions with per-device pointers
lead to a use-after-free?

If probe defers or fails later, ctrl is freed by devres, leaving the
global clock objects pointing to dangling memory. Subsequent probes could
then encounter corrupted pointers.

> +		ret = devm_clk_hw_register(dev, hw);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "failed to register clock %d\n",
> +					     i);
> +	}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517-sf21-topcrm-v1-0-438f2e0513ff@gmail.com?part=4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] dt-bindings: clock: add doc for Siflower sf21-topcrm
  2026-05-17 14:12 ` [PATCH 3/4] dt-bindings: clock: add doc for Siflower sf21-topcrm Chuanhong Guo
@ 2026-05-17 15:35   ` Rob Herring (Arm)
  2026-05-17 20:50   ` Conor Dooley
  1 sibling, 0 replies; 13+ messages in thread
From: Rob Herring (Arm) @ 2026-05-17 15:35 UTC (permalink / raw)
  To: Chuanhong Guo
  Cc: Palmer Dabbelt, Conor Dooley, Michael Turquette, Paul Walmsley,
	Alexandre Ghiti, linux-riscv, Philipp Zabel, Stephen Boyd,
	linux-clk, devicetree, Krzysztof Kozlowski, linux-kernel,
	Brian Masney, Albert Ou


On Sun, 17 May 2026 22:12:57 +0800, Chuanhong Guo wrote:
> Add a binding doc for the top clock and reset module found on Siflower
> SF21 SoCs. This block provides the main PLLs, high-level clock
> controls, and some reset lines.
> 
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> ---
>  .../bindings/clock/siflower,sf21-topcrm.yaml       | 69 ++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/siflower,sf21-topcrm.example.dtb: /: 'compatible' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/siflower,sf21-topcrm.example.dtb: /: 'model' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260517-sf21-topcrm-v1-3-438f2e0513ff@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] riscv: add Siflower RISC-V SoC family Kconfig support
  2026-05-17 14:31   ` sashiko-bot
@ 2026-05-17 20:46     ` Conor Dooley
  0 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2026-05-17 20:46 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Chuanhong Guo, robh, conor+dt, devicetree

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

On Sun, May 17, 2026 at 02:31:52PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Conditionally selecting the `ARM_AMBA` hardware bus based on the `TTY` software subsystem.
> --
> 
> commit 49824e33ae15603df7f899c7df0a4ab9b5f20eed
> Author: Chuanhong Guo <gch981213@gmail.com>
> 
> riscv: add Siflower RISC-V SoC family Kconfig support
> 
> This adds Kconfig support for the Siflower RISC-V SoCs, including the
> SF21A6826 and SF21H8898 chips. It defines ARCH_SIFLOWER, selects the
> relevant T-Head errata, and conditionally selects ARM_AMBA for the UART.
> 
> > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > index c174ac0ec46b4..9996591cd9dbf 100644
> > --- a/arch/riscv/Kconfig.socs
> > +++ b/arch/riscv/Kconfig.socs
> [ ... ]
> > @@ -37,6 +37,13 @@ config ARCH_SIFIVE
> >  	help
> >  	  This enables support for SiFive SoC platform hardware.
> >  
> > +config ARCH_SIFLOWER
> > +	bool "Siflower RISC-V SoCs"
> > +	select ARM_AMBA if TTY
> 
> Should ARM_AMBA be selected conditionally based on TTY?
> 
> ARM_AMBA models the presence of a physical hardware bus, which exists
> regardless of whether the TTY software subsystem is enabled. Even if the

There's no point having the driver built if there's no driver built for
devices on the bus, I think is the angle here.

> PL011 UART is currently the only AMBA device on this platform, tying the
> hardware bus infrastructure to a software abstraction might cause issues if
> other AMBA devices are added later and TTY is disabled.
> 
> Would it be better to select ARM_AMBA unconditionally, similar to how other
> architectures handle it?
> 
> > +	select ERRATA_THEAD
> > +	help
> > +	  This enables support for Siflower RISC-V SoC platform hardware.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260517-sf21-topcrm-v1-0-438f2e0513ff@gmail.com?part=1

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] dt-bindings: clock: add binding header for sf21-topcrm
  2026-05-17 14:12 ` [PATCH 2/4] dt-bindings: clock: add binding header for sf21-topcrm Chuanhong Guo
  2026-05-17 14:36   ` sashiko-bot
@ 2026-05-17 20:47   ` Conor Dooley
  2026-05-17 20:51     ` Conor Dooley
  1 sibling, 1 reply; 13+ messages in thread
From: Conor Dooley @ 2026-05-17 20:47 UTC (permalink / raw)
  To: Chuanhong Guo
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-riscv,
	linux-kernel, linux-clk, devicetree

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

On Sun, May 17, 2026 at 10:12:56PM +0800, Chuanhong Guo wrote:
> +#define SF21_CLK_PCIE_REFP		30
> +#define SF21_CLK_MAX			31

> +#define SF21_RESET_IROM			17
> +#define SF21_RESET_MAX			18


Having _MAX is not permitted, but is also really confusing that "max"
appears to be max+1, so actually represents the count not the max?

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] dt-bindings: clock: add doc for Siflower sf21-topcrm
  2026-05-17 14:12 ` [PATCH 3/4] dt-bindings: clock: add doc for Siflower sf21-topcrm Chuanhong Guo
  2026-05-17 15:35   ` Rob Herring (Arm)
@ 2026-05-17 20:50   ` Conor Dooley
  1 sibling, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2026-05-17 20:50 UTC (permalink / raw)
  To: Chuanhong Guo
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-riscv,
	linux-kernel, linux-clk, devicetree

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

On Sun, May 17, 2026 at 10:12:57PM +0800, Chuanhong Guo wrote:
> Add a binding doc for the top clock and reset module found on Siflower
> SF21 SoCs. This block provides the main PLLs, high-level clock
> controls, and some reset lines.
> 
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> ---
>  .../bindings/clock/siflower,sf21-topcrm.yaml       | 69 ++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/siflower,sf21-topcrm.yaml b/Documentation/devicetree/bindings/clock/siflower,sf21-topcrm.yaml
> new file mode 100644
> index 000000000000..a013d48841f4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/siflower,sf21-topcrm.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/siflower,sf21-topcrm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Siflower SF21 toplevel clock and reset module
> +
> +maintainers:
> +  - Chuanhong Guo <gch981213@gmail.com>
> +
> +description: |
> +  The toplevel clock and reset module on Siflower SF21 SoCs manages
> +  the main PLLs, high-level clock muxes/dividers/gates, and some
> +  reset lines.
> +  Available clocks and resets are defined in:
> +  include/dt-bindings/clock/siflower,sf21-topcrm.h
> +
> +properties:
> +  compatible:
> +    const: siflower,sf21-topcrm
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: xin25m

Is a 25 MHz reference required on this SoC? If not, name here is
obviously problematic if it can be something else.
Not much value in clock-names anyway when you only have 1.

> +
> +  "#clock-cells":
> +    const: 1
> +
> +  "#reset-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - "#clock-cells"
> +  - "#reset-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/siflower,sf21-topcrm.h>
> +    / {

Replace this / with "soc".

> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        xin25m: clock-25000000 {
> +            compatible = "fixed-clock";
> +            #clock-cells = <0>;
> +            clock-frequency = <25000000>;
> +        };

Delete this node, it's not needed in the example. The tooling will fill
it in.

Also, please test your bindings since this doesn't pass.

pw-bot: changes-requested

Thanks,
Conor.

> +
> +        clock-controller@ce00400 {
> +            compatible = "siflower,sf21-topcrm";
> +            reg = <0x0ce00400 0x400>;
> +            clocks = <&xin25m>;
> +            clock-names = "xin25m";
> +            #clock-cells = <1>;
> +            #reset-cells = <1>;
> +        };
> +    };
> 
> -- 
> 2.54.0
> 

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] dt-bindings: clock: add binding header for sf21-topcrm
  2026-05-17 20:47   ` Conor Dooley
@ 2026-05-17 20:51     ` Conor Dooley
  0 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2026-05-17 20:51 UTC (permalink / raw)
  To: Chuanhong Guo
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-riscv,
	linux-kernel, linux-clk, devicetree

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

On Sun, May 17, 2026 at 09:47:26PM +0100, Conor Dooley wrote:
> On Sun, May 17, 2026 at 10:12:56PM +0800, Chuanhong Guo wrote:
> > +#define SF21_CLK_PCIE_REFP		30
> > +#define SF21_CLK_MAX			31
> 
> > +#define SF21_RESET_IROM			17
> > +#define SF21_RESET_MAX			18
> 
> 
> Having _MAX is not permitted, but is also really confusing that "max"
> appears to be max+1, so actually represents the count not the max?

To be clear, if you need a define like this, put it in the driver.
Bindings having a "_MAX" or "NUM_CLKS" doesn't make sense, and half the
time this number ends up changing anyway.

Also, squash this with the patch adding the clock binding.

Cheers,
Conor.



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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-05-17 20:52 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-17 14:12 [PATCH 0/4] clk: add Siflower SF21 topcrm support Chuanhong Guo
2026-05-17 14:12 ` [PATCH 1/4] riscv: add Siflower RISC-V SoC family Kconfig support Chuanhong Guo
2026-05-17 14:31   ` sashiko-bot
2026-05-17 20:46     ` Conor Dooley
2026-05-17 14:12 ` [PATCH 2/4] dt-bindings: clock: add binding header for sf21-topcrm Chuanhong Guo
2026-05-17 14:36   ` sashiko-bot
2026-05-17 20:47   ` Conor Dooley
2026-05-17 20:51     ` Conor Dooley
2026-05-17 14:12 ` [PATCH 3/4] dt-bindings: clock: add doc for Siflower sf21-topcrm Chuanhong Guo
2026-05-17 15:35   ` Rob Herring (Arm)
2026-05-17 20:50   ` Conor Dooley
2026-05-17 14:12 ` [PATCH 4/4] clk: add support for siflower sf21-topcrm Chuanhong Guo
2026-05-17 15:09   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox