[PATCH v4 0/3] riscv: canaan: Add support for K230-Canmv clock

[PATCH v4 0/3] riscv: canaan: Add support for K230-Canmv clock

[Xukai Wang]

This patch series adds clock controller support for the Canaan Kendryte
K230 SoC. The K230 SoC includes an external 24MHz OSC and 4 internal
PLLs, with the controller managing these sources and their derived clocks.

The clock tree and hardware-specific definition can be found in the
vendor's DTS [1],
and this series is based on the K230 initial series [2].

Link: https://github.com/kendryte/k230_sdk/blob/main/src/little/linux/arch/riscv/boot/dts/kendryte/clock_provider.dtsi [1]
Link: https://lore.kernel.org/linux-clk/tencent_F76EB8D731C521C18D5D7C4F8229DAA58E08@qq.com/ [2]

Co-developed-by: Troy Mitchell <TroyMitchell988@gmail.com>
Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
Signed-off-by: Xukai Wang <kingxukai@zohomail.com>

---
Changes in v4:
- Remove redundant onecell_get callback and add_provider function
for pll_divs.
- Modify the base-commit in cover letter.
- Link to v3: https://lore.kernel.org/r/20250203-b4-k230-clk-v3-0-362c79124572@zohomail.com

Changes in v3:
- Reorder the defination and declaration in drivers code.
- Reorder the properties in dts node.
- Replace global variable `k230_sysclk` with dynamic memory allocation.
- Rename the macro K230_NUM_CLKS to K230_CLK_NUM.
- Use dev_err_probe for error handling.
- Remove unused includes.
- Link to v2: https://lore.kernel.org/r/20250108-b4-k230-clk-v2-0-27b30a2ca52d@zohomail.com

Changes in v2:
- Add items and description.
- Rename k230-clk.h to canaan,k230-clk.h
- Link to v1: https://lore.kernel.org/r/20241229-b4-k230-clk-v1-0-221a917e80ed@zohomail.com

---
Xukai Wang (3):
      dt-bindings: clock: Add bindings for Canaan K230 clock controller
      clk: canaan: Add clock driver for Canaan K230
      riscv: dts: canaan: Add clock initial support for K230

 .../devicetree/bindings/clock/canaan,k230-clk.yaml |   43 +
 arch/riscv/boot/dts/canaan/k230.dtsi               |   32 +
 drivers/clk/Kconfig                                |    6 +
 drivers/clk/Makefile                               |    1 +
 drivers/clk/clk-k230.c                             | 1347 ++++++++++++++++++++
 include/dt-bindings/clock/canaan,k230-clk.h        |   49 +
 6 files changed, 1478 insertions(+)
---
base-commit: 7fdb24bbac37ce692346d60b89f5aa29844b8b88
change-id: 20241206-b4-k230-clk-925f33fed6c2

Best regards,
-- 
Xukai Wang <kingxukai@zohomail.com>

[PATCH v4 1/3] dt-bindings: clock: Add bindings for Canaan K230 clock controller

[Xukai Wang]

This patch adds the Device Tree binding for the clock controller
on Canaan k230. The binding defines the new clocks available and
the required properties to configure them correctly.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Xukai Wang <kingxukai@zohomail.com>
---
 .../devicetree/bindings/clock/canaan,k230-clk.yaml | 43 +++++++++++++++++++
 include/dt-bindings/clock/canaan,k230-clk.h        | 49 ++++++++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..d7220fa30e4699a68fa5279c04abc63c1905fa4a
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/canaan,k230-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Canaan Kendryte K230 Clock
+
+maintainers:
+  - Xukai Wang <kingxukai@zohomail.com>
+
+properties:
+  compatible:
+    const: canaan,k230-clk
+
+  reg:
+    items:
+      - description: PLL control registers.
+      - description: Sysclk control registers.
+
+  clocks:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-controller@91102000 {
+        compatible = "canaan,k230-clk";
+        reg = <0x91102000 0x1000>,
+              <0x91100000 0x1000>;
+        clocks = <&osc24m>;
+        #clock-cells = <1>;
+    };
diff --git a/include/dt-bindings/clock/canaan,k230-clk.h b/include/dt-bindings/clock/canaan,k230-clk.h
new file mode 100644
index 0000000000000000000000000000000000000000..47d966fda5771615dad8ade64eeec42a9b27696e
--- /dev/null
+++ b/include/dt-bindings/clock/canaan,k230-clk.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Kendryte Canaan K230 Clock Drivers
+ *
+ * Author: Xukai Wang <kingxukai@zohomail.com>
+ */
+
+#ifndef CLOCK_K230_CLK_H
+#define CLOCK_K230_CLK_H
+
+/* Kendryte K230 SoC clock identifiers (arbitrary values). */
+#define K230_CPU0_SRC			0
+#define K230_CPU0_ACLK			1
+#define K230_CPU0_PLIC			2
+#define K230_CPU0_NOC_DDRCP4		3
+#define K230_CPU0_PCLK			4
+#define K230_PMU_PCLK			5
+#define K230_HS_HCLK_HIGH_SRC		6
+#define K230_HS_HCLK_HIGH_GATE		7
+#define K230_HS_HCLK_SRC		8
+#define K230_HS_SD0_HS_AHB_GAT		9
+#define K230_HS_SD1_HS_AHB_GAT		10
+#define K230_HS_SSI1_HS_AHB_GA		11
+#define K230_HS_SSI2_HS_AHB_GA		12
+#define K230_HS_USB0_HS_AHB_GA		13
+#define K230_HS_USB1_HS_AHB_GA		14
+#define K230_HS_SSI0_AXI15		15
+#define K230_HS_SSI1			16
+#define K230_HS_SSI2			17
+#define K230_HS_QSPI_AXI_SRC		18
+#define K230_HS_SSI1_ACLK_GATE		19
+#define K230_HS_SSI2_ACLK_GATE		20
+#define K230_HS_SD_CARD_SRC		21
+#define K230_HS_SD0_CARD_TX		22
+#define K230_HS_SD1_CARD_TX		23
+#define K230_HS_SD_AXI_SRC		24
+#define K230_HS_SD0_AXI_GATE		25
+#define K230_HS_SD1_AXI_GATE		26
+#define K230_HS_SD0_BASE_GATE		27
+#define K230_HS_SD1_BASE_GATE		28
+#define K230_HS_OSPI_SRC		29
+#define K230_HS_USB_REF_50M		30
+#define K230_HS_SD_TIMER_SRC		31
+#define K230_HS_SD0_TIMER_GATE		32
+#define K230_HS_SD1_TIMER_GATE		33
+#define K230_HS_USB0_REFERENCE		34
+#define K230_HS_USB1_REFERENCE		35
+
+#endif /* CLOCK_K230_CLK_H */

-- 
2.34.1

Re: [PATCH v4 1/3] dt-bindings: clock: Add bindings for Canaan K230 clock controller

[Stephen Boyd]

Quoting Xukai Wang (2025-02-17 06:45:16)
> diff --git a/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..d7220fa30e4699a68fa5279c04abc63c1905fa4a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/canaan,k230-clk.yaml
> @@ -0,0 +1,43 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/canaan,k230-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Canaan Kendryte K230 Clock
> +
> +maintainers:
> +  - Xukai Wang <kingxukai@zohomail.com>

Is this missing a description of the device?

> +
> +properties:
> +  compatible:
> +    const: canaan,k230-clk
> +
> +  reg:
> +    items:
> +      - description: PLL control registers.
> +      - description: Sysclk control registers.
> +
> +  clocks:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    clock-controller@91102000 {
> +        compatible = "canaan,k230-clk";
> +        reg = <0x91102000 0x1000>,

Is there a reason why the PLL range comes first? What's at 0x91101000?
More clk hardware?

> +              <0x91100000 0x1000>;
> +        clocks = <&osc24m>;
> +        #clock-cells = <1>;
> +    };

Re: [PATCH v4 1/3] dt-bindings: clock: Add bindings for Canaan K230 clock controller

[Xukai Wang]

On 2025/2/19 05:51, Stephen Boyd wrote:
> Quoting Xukai Wang (2025-02-17 06:45:16)
>> +$id: http://devicetree.org/schemas/clock/canaan,k230-clk.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Canaan Kendryte K230 Clock
>> +
>> +maintainers:
>> +  - Xukai Wang <kingxukai@zohomail.com>
> Is this missing a description of the device?

Alright. Here's the added description:

The K230 clock controller manages the PLLs and system clocks (sysclk),
generating clocks for different domains. The internal PLLs are derived
from the external osc24M, and all sysclk signals use either the PLLs'
dividers or directly the osc24M source.

Not all macros defined in include/dt-bindings/clock/canaan,k230-clk.h
are available for use in clock consumer nodes yet.

Does this looks appropriate?

>> +        reg = <0x91102000 0x1000>,
> Is there a reason why the PLL range comes first?
As I thought that the PLL is the base for other system clocks (sysclk),
so I placed it first.
> What's at 0x91101000? More clk hardware?

Regarding 0x91101000, there is no clk hardware at that address; it is
related to the Reset Management Unit (RMU).

You can refer to the K230 Technical Reference Manual [1] on page 11 for
its Address Space mapping.

Thanks for your time.

Link:
https://kendryte-download.canaan-creative.com/developer/k230/HDK/K230%E7%A1%AC%E4%BB%B6%E6%96%87%E6%A1%A3/K230_Technical_Reference_Manual_V0.3.1_20241118.pdf
[1]

>> +              <0x91100000 0x1000>;

[PATCH v4 2/3] clk: canaan: Add clock driver for Canaan K230

[Xukai Wang]

This patch provides basic support for the K230 clock, which does not
cover all clocks.

The clock tree of the K230 SoC consists of OSC24M, PLLs and sysclk.

Co-developed-by: Troy Mitchell <TroyMitchell988@gmail.com>
Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
Signed-off-by: Xukai Wang <kingxukai@zohomail.com>
---
 drivers/clk/Kconfig    |    6 +
 drivers/clk/Makefile   |    1 +
 drivers/clk/clk-k230.c | 1347 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1354 insertions(+)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 299bc678ed1b9fcd9110bb8c5937a1bd1ea60e23..f63355ab8adeeee90d29d1a975607f5dc0a58bd6 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -464,6 +464,12 @@ config COMMON_CLK_K210
 	help
 	  Support for the Canaan Kendryte K210 RISC-V SoC clocks.
 
+config COMMON_CLK_K230
+	bool "Clock driver for the Canaan Kendryte K230 SoC"
+	depends on OF && (ARCH_CANAAN || COMPILE_TEST)
+        help
+          Support for the Canaan Kendryte K230 RISC-V SoC clocks.
+
 config COMMON_CLK_SP7021
 	tristate "Clock driver for Sunplus SP7021 SoC"
 	depends on SOC_SP7021 || COMPILE_TEST
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index fb8878a5d7d93da6bec487460cdf63f1f764a431..5df50b1e14c701ed38397bfb257db26e8dd278b8 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_MACH_ASPEED_G6)		+= clk-ast2600.o
 obj-$(CONFIG_ARCH_HIGHBANK)		+= clk-highbank.o
 obj-$(CONFIG_CLK_HSDK)			+= clk-hsdk-pll.o
 obj-$(CONFIG_COMMON_CLK_K210)		+= clk-k210.o
+obj-$(CONFIG_COMMON_CLK_K230)		+= clk-k230.o
 obj-$(CONFIG_LMK04832)			+= clk-lmk04832.o
 obj-$(CONFIG_COMMON_CLK_LAN966X)	+= clk-lan966x.o
 obj-$(CONFIG_COMMON_CLK_LOCHNAGAR)	+= clk-lochnagar.o
diff --git a/drivers/clk/clk-k230.c b/drivers/clk/clk-k230.c
new file mode 100644
index 0000000000000000000000000000000000000000..017ecad9ec47ed751f75fe5197aa119ae35e05f9
--- /dev/null
+++ b/drivers/clk/clk-k230.c
@@ -0,0 +1,1347 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Kendryte Canaan K230 Clock Drivers
+ *
+ * Author: Xukai Wang <kingxukai@zohomail.com>
+ * Author: Troy Mitchell <troymitchell988@gmail.com>
+ */
+
+#include <dt-bindings/clock/canaan,k230-clk.h>
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+/* PLL control register bits. */
+#define K230_PLL_BYPASS_ENABLE				BIT(19)
+#define K230_PLL_GATE_ENABLE				BIT(2)
+#define K230_PLL_GATE_WRITE_ENABLE			BIT(18)
+#define K230_PLL_OD_SHIFT				24
+#define K230_PLL_OD_MASK				0xF
+#define K230_PLL_R_SHIFT				16
+#define K230_PLL_R_MASK					0x3F
+#define K230_PLL_F_SHIFT				0
+#define K230_PLL_F_MASK					0x1FFFF
+#define K230_PLL0_OFFSET_BASE				0x00
+#define K230_PLL1_OFFSET_BASE				0x10
+#define K230_PLL2_OFFSET_BASE				0x20
+#define K230_PLL3_OFFSET_BASE				0x30
+#define K230_PLL_DIV_REG_OFFSET				0x00
+#define K230_PLL_BYPASS_REG_OFFSET			0x04
+#define K230_PLL_GATE_REG_OFFSET			0x08
+#define K230_PLL_LOCK_REG_OFFSET			0x0C
+
+/* PLL lock register bits.  */
+#define K230_PLL_STATUS_MASK				BIT(0)
+
+/* K230 CLK registers offset */
+#define K230_CLK_AUDIO_CLKDIV_OFFSET			0x34
+#define K230_CLK_PDM_CLKDIV_OFFSET			0x40
+#define K230_CLK_CODEC_ADC_MCLKDIV_OFFSET		0x38
+#define K230_CLK_CODEC_DAC_MCLKDIV_OFFSET		0x3c
+
+/* K230 CLK OPS. */
+#define K230_CLK_OPS_GATE				\
+	.enable		= k230_clk_enable,		\
+	.disable	= k230_clk_disable,		\
+	.is_enabled	= k230_clk_is_enabled
+
+#define K230_CLK_OPS_RATE				\
+	.set_rate	= k230_clk_set_rate,		\
+	.round_rate	= k230_clk_round_rate,		\
+	.recalc_rate	= k230_clk_get_rate
+
+#define K230_CLK_OPS_MUX				\
+	.set_parent	= k230_clk_set_parent,		\
+	.get_parent	= k230_clk_get_parent,		\
+	.determine_rate	= clk_hw_determine_rate_no_reparent
+
+#define K230_CLK_OPS_ID_NONE				0
+#define K230_CLK_OPS_ID_GATE_ONLY			1
+#define K230_CLK_OPS_ID_RATE_ONLY			2
+#define K230_CLK_OPS_ID_RATE_GATE			3
+#define K230_CLK_OPS_ID_MUX_ONLY			4
+#define K230_CLK_OPS_ID_MUX_GATE			5
+#define K230_CLK_OPS_ID_MUX_RATE			6
+#define K230_CLK_OPS_ID_ALL				7
+#define K230_CLK_OPS_ID_NUM				8
+
+/* K230 CLK MACROS */
+#define K230_CLK_MAX_PARENT_NUM				6
+
+#define K230_GATE_FORMAT(_reg, _bit, _reverse, _have_gate)			\
+	.gate_reg_off = (_reg),							\
+	.gate_bit_enable = (_bit),						\
+	.gate_bit_reverse = (_reverse),						\
+	.have_gate = (_have_gate)
+
+#define K230_RATE_FORMAT(_mul_min, _mul_max, _mul_shift, _mul_mask,		\
+			_div_min, _div_max, _div_shift, _div_mask,		\
+			_reg, _bit, _method, _reg_c, _bit_c,			\
+			_mul_min_c, _mul_max_c, _mul_shift_c, _mul_mask_c,	\
+			_have_rate, _have_rate_c)				\
+	.rate_mul_min = (_mul_min),						\
+	.rate_mul_max = (_mul_max),						\
+	.rate_mul_shift = (_mul_shift),						\
+	.rate_mul_mask = (_mul_mask),						\
+	.rate_mul_min_c = (_mul_min_c),						\
+	.rate_mul_max_c = (_mul_max_c),						\
+	.rate_mul_shift_c = (_mul_shift_c),					\
+	.rate_mul_mask_c = (_mul_mask_c),					\
+	.rate_div_min = (_div_min),						\
+	.rate_div_max = (_div_max),						\
+	.rate_div_shift = (_div_shift),						\
+	.rate_div_mask = (_div_mask),						\
+	.rate_reg_off = (_reg),							\
+	.rate_reg_off_c = (_reg_c),						\
+	.rate_write_enable_bit = (_bit),					\
+	.rate_write_enable_bit_c = (_bit_c),					\
+	.method = (_method),							\
+	.have_rate = (_have_rate),						\
+	.have_rate_c = (_have_rate_c)
+
+#define K230_MUX_FORMAT(_reg, _shift, _mask, _have_mux)				\
+	.mux_reg_off = (_reg),							\
+	.mux_reg_shift = (_shift),						\
+	.mux_reg_mask = (_mask),						\
+	.have_mux = (_have_mux)
+
+#define K230_GATE_FORMAT_ZERO	K230_GATE_FORMAT(0, 0, 0, false)
+#define K230_RATE_FORMAT_ZERO	K230_RATE_FORMAT(0, 0, 0, 0, 0, 0,		\
+						0, 0, 0, 0, 0, 0,		\
+						0, 0, 0, 0, 0, false, false)
+#define K230_MUX_FORMAT_ZERO	K230_MUX_FORMAT(0, 0, 0, false)
+
+struct k230_sysclk;
+
+/* K230 PLLs. */
+enum k230_pll_id {
+	K230_PLL0, K230_PLL1, K230_PLL2, K230_PLL3, K230_PLL_NUM
+};
+
+struct k230_pll {
+	enum k230_pll_id id;
+	struct k230_sysclk *ksc;
+	void __iomem *div, *bypass, *gate, *lock;
+	struct clk_hw hw;
+};
+
+#define to_k230_pll(_hw)	container_of(_hw, struct k230_pll, hw)
+
+struct k230_pll_cfg {
+	u32 reg;
+	enum k230_pll_id pll_id;
+	const char *name;
+};
+
+/* K230 PLL_DIVS. */
+struct k230_pll_div {
+	struct k230_sysclk *ksc;
+	struct clk_hw *hw;
+};
+
+struct k230_pll_div_cfg {
+	const char *parent_name, *name;
+	int div;
+};
+
+enum k230_pll_div_id {
+	K230_PLL0_DIV2,
+	K230_PLL0_DIV3,
+	K230_PLL0_DIV4,
+	K230_PLL1_DIV2,
+	K230_PLL1_DIV3,
+	K230_PLL1_DIV4,
+	K230_PLL2_DIV2,
+	K230_PLL2_DIV3,
+	K230_PLL2_DIV4,
+	K230_PLL3_DIV2,
+	K230_PLL3_DIV3,
+	K230_PLL3_DIV4,
+	K230_PLL_DIV_NUM
+};
+
+enum k230_clk_div_type {
+	K230_MUL,
+	K230_DIV,
+	K230_MUL_DIV,
+};
+
+/* K230 CLKS. */
+struct k230_clk {
+	int id;
+	struct k230_sysclk *ksc;
+	struct clk_hw hw;
+};
+
+#define to_k230_clk(_hw)	container_of(_hw, struct k230_clk, hw)
+
+enum k230_clk_parent_type {
+	K230_OSC24M,
+	K230_PLL,
+	K230_PLL_DIV,
+	K230_CLK_COMPOSITE,
+};
+
+struct k230_clk_parent {
+	enum k230_clk_parent_type type;
+	union {
+		enum k230_pll_div_id pll_div_id;
+		enum k230_pll_id pll_id;
+		int clk_id;
+	};
+};
+
+struct k230_clk_cfg {
+	/* attr */
+	const char *name;
+	/* 0-read & write; 1-read only */
+	bool read_only;
+	int num_parent;
+	struct k230_clk_parent parent[K230_CLK_MAX_PARENT_NUM];
+	bool status;
+	int flags;
+
+	/* rate reg */
+	u32 rate_reg_off;
+	u32 rate_reg_off_c;
+	void __iomem *rate_reg;
+	void __iomem *rate_reg_c;
+	/* rate info*/
+	u32 rate_write_enable_bit;
+	u32 rate_write_enable_bit_c;
+	enum k230_clk_div_type method;
+	bool have_rate;
+	bool have_rate_c;
+	/* rate mul */
+	u32 rate_mul_min;
+	u32 rate_mul_max;
+	u32 rate_mul_shift;
+	u32 rate_mul_mask;
+	/* rate mul-changable */
+	u32 rate_mul_min_c;
+	u32 rate_mul_max_c;
+	u32 rate_mul_shift_c;
+	u32 rate_mul_mask_c;
+	/* rate div */
+	u32 rate_div_min;
+	u32 rate_div_max;
+	u32 rate_div_shift;
+	u32 rate_div_mask;
+
+	/* gate reg */
+	u32 gate_reg_off;
+	void __iomem *gate_reg;
+	/* gate info*/
+	bool have_gate;
+	u32 gate_bit_enable;
+	u32 gate_bit_reverse;
+
+	/* mux reg */
+	u32 mux_reg_off;
+	void __iomem *mux_reg;
+	/* mux info */
+	bool have_mux;
+	u32 mux_reg_shift;
+	u32 mux_reg_mask;
+};
+
+/* K230 SYSCLK. */
+struct k230_sysclk {
+	struct platform_device *pdev;
+	void __iomem	       *pll_regs, *regs;
+	spinlock_t	       pll_lock, clk_lock;
+	struct k230_pll	       *plls;
+	struct k230_clk	       *clks;
+	struct k230_pll_div    *dclks;
+};
+
+static const struct k230_pll_cfg k230_pll_cfgs[] = {
+	[K230_PLL0] = {
+		.reg = K230_PLL0_OFFSET_BASE,
+		.pll_id = K230_PLL0,
+		.name = "pll0",
+	},
+	[K230_PLL1] = {
+		.reg = K230_PLL1_OFFSET_BASE,
+		.pll_id = K230_PLL1,
+		.name = "pll1",
+	},
+	[K230_PLL2] = {
+		.reg = K230_PLL2_OFFSET_BASE,
+		.pll_id = K230_PLL2,
+		.name = "pll2",
+	},
+	[K230_PLL3] = {
+		.reg = K230_PLL3_OFFSET_BASE,
+		.pll_id = K230_PLL3,
+		.name = "pll3",
+	},
+};
+
+static const struct k230_pll_div_cfg k230_pll_div_cfgs[] = {
+	[K230_PLL0_DIV2] = { "pll0", "pll0_div2", 2},
+	[K230_PLL0_DIV3] = { "pll0", "pll0_div3", 3},
+	[K230_PLL0_DIV4] = { "pll0", "pll0_div4", 4},
+	[K230_PLL1_DIV2] = { "pll1", "pll1_div2", 2},
+	[K230_PLL1_DIV3] = { "pll1", "pll1_div3", 3},
+	[K230_PLL1_DIV4] = { "pll1", "pll1_div4", 4},
+	[K230_PLL2_DIV2] = { "pll2", "pll2_div2", 2},
+	[K230_PLL2_DIV3] = { "pll2", "pll2_div3", 3},
+	[K230_PLL2_DIV4] = { "pll2", "pll2_div4", 4},
+	[K230_PLL3_DIV2] = { "pll3", "pll3_div2", 2},
+	[K230_PLL3_DIV3] = { "pll3", "pll3_div3", 3},
+	[K230_PLL3_DIV4] = { "pll3", "pll3_div4", 4},
+};
+
+static struct k230_clk_cfg k230_clk_cfgs[] = {
+	[K230_CPU0_SRC] = {
+		.name = "cpu0_src",
+		.read_only = false,
+		.flags = 0,
+		.status = true,
+		.num_parent = 1,
+		.parent[0] = {
+			.type = K230_PLL_DIV,
+			.pll_div_id = K230_PLL0_DIV2,
+		},
+		K230_RATE_FORMAT(1, 16, 0, 0,
+				 16, 16, 1, 0xF,
+				 0x0, 31, K230_MUL, 0, 0,
+				 0, 0, 0, 0,
+				 true, false),
+		K230_GATE_FORMAT(0, 0, 0, true),
+		K230_MUX_FORMAT_ZERO,
+	},
+	[K230_CPU0_ACLK] = {
+		.name = "cpu0_aclk",
+		.read_only = false,
+		.flags = 0,
+		.status = true,
+		.num_parent = 1,
+		.parent[0] = {
+			.type = K230_CLK_COMPOSITE,
+			.clk_id = K230_CPU0_SRC,
+		},
+		K230_RATE_FORMAT(1, 1, 0, 0,
+				 1, 8, 7, 0x7,
+				 0x0, 31, K230_MUL, 0, 0,
+				 0, 0, 0, 0,
+				 true, false),
+		K230_GATE_FORMAT_ZERO,
+		K230_MUX_FORMAT_ZERO,
+	},
+	[K230_CPU0_PLIC] = {
+		.name = "cpu0_plic",
+		.read_only = false,
+		.flags = 0,
+		.status = true,
+		.num_parent = 1,
+		.parent[0] = {
+			.type = K230_CLK_COMPOSITE,
+			.clk_id = K230_CPU0_SRC,
+		},
+		K230_RATE_FORMAT(1, 1, 0, 0,
+				 1, 8, 10, 0x7,
+				 0x0, 31, K230_DIV, 0, 0,
+				 0, 0, 0, 0,
+				 true, false),
+		K230_GATE_FORMAT(0, 9, 0, true),
+		K230_MUX_FORMAT_ZERO,
+	},
+	[K230_CPU0_NOC_DDRCP4] = {
+		.name = "cpu0_noc_ddrcp4",
+		.read_only = false,
+		.flags = 0,
+		.status = true,
+		.num_parent = 1,
+		.parent[0] = {
+			.type = K230_CLK_COMPOSITE,
+			.clk_id = K230_CPU0_SRC,
+		},
+		K230_RATE_FORMAT_ZERO,
+		K230_GATE_FORMAT(0x60, 7, 0, true),
+		K230_MUX_FORMAT_ZERO,
+	},
+	[K230_CPU0_PCLK] = {
+		.name = "cpu0_pclk",
+		.read_only = false,
+		.flags = 0,
+		.status = true,
+		.num_parent = 1,
+		.parent[0] = {
+			.type = K230_PLL_DIV,
+			.pll_div_id = K230_PLL0_DIV4,
+		},
+		K230_RATE_FORMAT(1, 1, 0, 0,
+				 1, 8, 15, 0x7,
+				 0x0, 31, K230_DIV, 0, 0,
+				 0, 0, 0, 0,
+				 true, false),
+		K230_GATE_FORMAT(0, 13, 0, true),
+		K230_MUX_FORMAT_ZERO,
+	},
+	[K230_PMU_PCLK] = {
+		.name = "pmu_pclk",
+		.read_only = false,
+		.flags = 0,
+		.status = true,
+		.num_parent = 1,
+		.parent[0] = {
+			.type = K230_OSC24M,
+		},
+		K230_RATE_FORMAT_ZERO,
+		K230_GATE_FORMAT(0x10, 0, 0, true),
+		K230_MUX_FORMAT_ZERO,
+	},
+	[K230_HS_OSPI_SRC] = {
+		.name = "hs_ospi_src",
+		.read_only = false,
+		.flags = 0,
+		.status = true,
+		.num_parent = 2,
+		.parent[0] = {
+			.type = K230_PLL_DIV,
+			.pll_div_id = K230_PLL0_DIV2,
+		},
+		.parent[1] = {
+			.type = K230_PLL_DIV,
+			.pll_div_id = K230_PLL2_DIV4,
+		},
+		K230_RATE_FORMAT_ZERO,
+		K230_GATE_FORMAT(0x18, 24, 0, true),
+		K230_MUX_FORMAT(0x20, 18, 0x1, true),
+	},
+};
+
+#define K230_CLK_NUM	ARRAY_SIZE(k230_clk_cfgs)
+
+static void k230_init_pll(void __iomem *regs, enum k230_pll_id pll_id,
+			  struct k230_pll *pll)
+{
+	void __iomem *base;
+
+	pll->id = pll_id;
+	base = regs + k230_pll_cfgs[pll_id].reg;
+	pll->div = base + K230_PLL_DIV_REG_OFFSET;
+	pll->bypass = base + K230_PLL_BYPASS_REG_OFFSET;
+	pll->gate = base + K230_PLL_GATE_REG_OFFSET;
+	pll->lock = base + K230_PLL_LOCK_REG_OFFSET;
+}
+
+static int k230_pll_prepare(struct clk_hw *hw)
+{
+	struct k230_pll *pll = to_k230_pll(hw);
+	u32 reg;
+
+	/* wait for PLL lock until it reachs lock status */
+	return readl_poll_timeout(pll->lock, reg,
+				  (reg & K230_PLL_STATUS_MASK) == K230_PLL_STATUS_MASK,
+				  400, 0);
+}
+
+static bool k230_pll_hw_is_enabled(struct k230_pll *pll)
+{
+	return (readl(pll->gate) & K230_PLL_GATE_ENABLE) == K230_PLL_GATE_ENABLE;
+}
+
+static void k230_pll_enable_hw(void __iomem *regs, struct k230_pll *pll)
+{
+	u32 reg;
+
+	if (k230_pll_hw_is_enabled(pll))
+		return;
+
+	/* Set PLL factors */
+	reg = readl(pll->gate);
+	reg |= (K230_PLL_GATE_ENABLE | K230_PLL_GATE_WRITE_ENABLE);
+	writel(reg, pll->gate);
+}
+
+static int k230_pll_enable(struct clk_hw *hw)
+{
+	struct k230_pll *pll = to_k230_pll(hw);
+	struct k230_sysclk *ksc = pll->ksc;
+
+	guard(spinlock)(&ksc->pll_lock);
+	k230_pll_enable_hw(ksc->regs, pll);
+
+	return 0;
+}
+
+static void k230_pll_disable(struct clk_hw *hw)
+{
+	struct k230_pll *pll = to_k230_pll(hw);
+	struct k230_sysclk *ksc = pll->ksc;
+	u32 reg;
+
+	guard(spinlock)(&ksc->pll_lock);
+	reg = readl(pll->gate);
+
+	reg &= ~(K230_PLL_GATE_ENABLE);
+	reg |= (K230_PLL_GATE_WRITE_ENABLE);
+
+	writel(reg, pll->gate);
+}
+
+static int k230_pll_is_enabled(struct clk_hw *hw)
+{
+	return k230_pll_hw_is_enabled(to_k230_pll(hw));
+}
+
+static int k230_pll_init(struct clk_hw *hw)
+{
+	if (k230_pll_is_enabled(hw))
+		return clk_prepare_enable(hw->clk);
+
+	return 0;
+}
+
+static unsigned long k230_pll_get_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	struct k230_pll *pll = to_k230_pll(hw);
+	struct k230_sysclk *ksc = pll->ksc;
+	u32 reg;
+	u32 r, f, od;
+
+	reg = readl(pll->bypass);
+	if (reg & K230_PLL_BYPASS_ENABLE)
+		return parent_rate;
+
+	reg = readl(pll->lock);
+	if (!(reg & (K230_PLL_STATUS_MASK))) { /* unlocked */
+		dev_err(&ksc->pdev->dev, "%s is unlock.\n", clk_hw_get_name(hw));
+		return 0;
+	}
+
+	reg = readl(pll->div);
+	r = ((reg >> K230_PLL_R_SHIFT) & K230_PLL_R_MASK) + 1;
+	f = ((reg >> K230_PLL_F_SHIFT) & K230_PLL_F_MASK) + 1;
+	od = ((reg >> K230_PLL_OD_SHIFT) & K230_PLL_OD_MASK) + 1;
+
+	return div_u64((u64)parent_rate * f, r * od);
+}
+
+static const struct clk_ops k230_pll_ops = {
+	.init		= k230_pll_init,
+	.prepare	= k230_pll_prepare,
+	.enable	        = k230_pll_enable,
+	.disable	= k230_pll_disable,
+	.is_enabled	= k230_pll_is_enabled,
+	.recalc_rate	= k230_pll_get_rate,
+};
+
+static int k230_register_pll(struct platform_device *pdev,
+			     struct k230_sysclk *ksc,
+			     enum k230_pll_id pll_id,
+			     const char *name,
+			     int num_parents,
+			     const struct clk_ops *ops)
+{
+	struct k230_pll *pll = &ksc->plls[pll_id];
+	struct clk_init_data init = {};
+	struct device *dev = &pdev->dev;
+	int ret;
+	const struct clk_parent_data parent_data[] = {
+		{ .index = 0, },
+	};
+
+	init.name = name;
+	init.parent_data = parent_data;
+	init.num_parents = num_parents;
+	init.ops = ops;
+
+	pll->hw.init = &init;
+	pll->ksc = ksc;
+
+	ret = devm_clk_hw_register(dev, &pll->hw);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register pll");
+
+	return 0;
+}
+
+static int k230_register_plls(struct platform_device *pdev, struct k230_sysclk *ksc)
+{
+	int i, ret;
+	const struct k230_pll_cfg *cfg;
+
+	for (i = 0; i < K230_PLL_NUM; i++) {
+		cfg = &k230_pll_cfgs[i];
+
+		k230_init_pll(ksc->pll_regs, i, &ksc->plls[i]);
+
+		ret = k230_register_pll(pdev, ksc, cfg->pll_id, cfg->name, 1,
+					&k230_pll_ops);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					     "register %s failed\n", cfg->name);
+	}
+
+	return 0;
+}
+
+static int k230_register_pll_divs(struct platform_device *pdev, struct k230_sysclk *ksc)
+{
+	struct device *dev = &pdev->dev;
+	struct clk_hw *hw;
+
+	for (int i = 0; i < K230_PLL_DIV_NUM; i++) {
+		hw = devm_clk_hw_register_fixed_factor(dev, k230_pll_div_cfgs[i].name,
+						       k230_pll_div_cfgs[i].parent_name,
+						       0, 1, k230_pll_div_cfgs[i].div);
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+		ksc->dclks[i].hw = hw;
+		ksc->dclks[i].ksc = ksc;
+	}
+
+	return 0;
+}
+
+static int k230_clk_enable(struct clk_hw *hw)
+{
+	struct k230_clk *clk = to_k230_clk(hw);
+	struct k230_sysclk *ksc = clk->ksc;
+	struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
+	u32 reg;
+
+	if (!cfg->have_gate)
+		return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have gate\n");
+
+	guard(spinlock)(&ksc->clk_lock);
+	reg = readl(cfg->gate_reg);
+	if (cfg->gate_bit_reverse)
+		reg &= ~BIT(cfg->gate_bit_enable);
+	else
+		reg |= BIT(cfg->gate_bit_enable);
+	writel(reg, cfg->gate_reg);
+
+	return 0;
+}
+
+static void k230_clk_disable(struct clk_hw *hw)
+{
+	struct k230_clk *clk = to_k230_clk(hw);
+	struct k230_sysclk *ksc = clk->ksc;
+	struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
+	u32 reg;
+
+	if (!cfg->have_gate) {
+		dev_err(&ksc->pdev->dev, "This clock doesn't have gate\n");
+		return;
+	}
+
+	guard(spinlock)(&ksc->clk_lock);
+	reg = readl(cfg->gate_reg);
+
+	if (cfg->gate_bit_reverse)
+		reg |= BIT(cfg->gate_bit_enable);
+	else
+		reg &= ~BIT(cfg->gate_bit_enable);
+
+	writel(reg, cfg->gate_reg);
+}
+
+static int k230_clk_is_enabled(struct clk_hw *hw)
+{
+	struct k230_clk *clk = to_k230_clk(hw);
+	struct k230_sysclk *ksc = clk->ksc;
+	struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
+	u32 reg;
+
+	if (!cfg->have_gate)
+		return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have gate\n");
+
+	guard(spinlock)(&ksc->clk_lock);
+	reg = readl(cfg->gate_reg);
+
+	/* Check gate bit condition based on configuration and then set ret */
+	if (cfg->gate_bit_reverse)
+		return (BIT(cfg->gate_bit_enable) & reg) ? 1 : 0;
+	else
+		return (BIT(cfg->gate_bit_enable) & ~reg) ? 1 : 0;
+}
+
+static int k230_clk_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct k230_clk *clk = to_k230_clk(hw);
+	struct k230_sysclk *ksc = clk->ksc;
+	struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
+	u8 reg;
+
+	if (!cfg->have_mux)
+		return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have mux\n");
+
+	guard(spinlock)(&ksc->clk_lock);
+	reg = (cfg->mux_reg_mask & index) << cfg->mux_reg_shift;
+	writeb(reg, cfg->mux_reg);
+
+	return 0;
+}
+
+static u8 k230_clk_get_parent(struct clk_hw *hw)
+{
+	struct k230_clk *clk = to_k230_clk(hw);
+	struct k230_sysclk *ksc = clk->ksc;
+	struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
+	u8 reg;
+
+	if (!cfg->have_mux)
+		return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have mux\n");
+
+	guard(spinlock)(&ksc->clk_lock);
+	reg = readb(cfg->mux_reg);
+
+	return reg;
+}
+
+static unsigned long k230_clk_get_rate(struct clk_hw *hw,
+				       unsigned long parent_rate)
+{
+	struct k230_clk *clk = to_k230_clk(hw);
+	struct k230_sysclk *ksc = clk->ksc;
+	struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
+	u32 mul, div;
+
+	if (!cfg->have_rate) /* no divider, return parents' clk */
+		return parent_rate;
+
+	guard(spinlock)(&ksc->clk_lock);
+	switch (cfg->method) {
+	/*
+	 * K230_MUL: div_mask+1/div_max...
+	 * K230_DIV: mul_max/div_mask+1
+	 * K230_MUL_DIV: mul_mask/div_mask...
+	 */
+	case K230_MUL:
+		div = cfg->rate_div_max;
+		mul = (readl(cfg->rate_reg) >> cfg->rate_div_shift)
+			& cfg->rate_div_mask;
+		mul++;
+		break;
+	case K230_DIV:
+		mul = cfg->rate_mul_max;
+		div = (readl(cfg->rate_reg) >> cfg->rate_div_shift)
+			& cfg->rate_div_mask;
+		div++;
+		break;
+	case K230_MUL_DIV:
+		if (!cfg->have_rate_c) {
+			mul = (readl(cfg->rate_reg) >> cfg->rate_mul_shift)
+				& cfg->rate_mul_mask;
+			div = (readl(cfg->rate_reg) >> cfg->rate_div_shift)
+				& cfg->rate_div_mask;
+		} else {
+			mul = (readl(cfg->rate_reg_c) >> cfg->rate_mul_shift_c)
+				& cfg->rate_mul_mask_c;
+			div = (readl(cfg->rate_reg_c) >> cfg->rate_div_shift)
+				& cfg->rate_div_mask;
+		}
+		break;
+	default:
+		return 0;
+	}
+
+	return div_u64((u64)parent_rate * mul, div);
+}
+
+static int k230_clk_find_approximate(struct k230_clk *clk,
+				     u32 mul_min,
+				     u32 mul_max,
+				     u32 div_min,
+				     u32 div_max,
+				     enum k230_clk_div_type method,
+				     unsigned long rate,
+				     unsigned long parent_rate,
+				     u32 *div,
+				     u32 *mul)
+{
+	long abs_min;
+	long abs_current;
+	long perfect_divide;
+	struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
+
+	const u32 codec_clk[9] = {
+		2048000,
+		3072000,
+		4096000,
+		6144000,
+		8192000,
+		11289600,
+		12288000,
+		24576000,
+		49152000
+	};
+
+	const u32 codec_div[9][2] = {
+		{3125, 16},
+		{3125, 24},
+		{3125, 32},
+		{3125, 48},
+		{3125, 64},
+		{15625, 441},
+		{3125, 96},
+		{3125, 192},
+		{3125, 384}
+	};
+
+	const u32 pdm_clk[20] = {
+		128000,
+		192000,
+		256000,
+		384000,
+		512000,
+		768000,
+		1024000,
+		1411200,
+		1536000,
+		2048000,
+		2822400,
+		3072000,
+		4096000,
+		5644800,
+		6144000,
+		8192000,
+		11289600,
+		12288000,
+		24576000,
+		49152000
+	};
+
+	const u32 pdm_div[20][2] = {
+		{3125, 1},
+		{6250, 3},
+		{3125, 2},
+		{3125, 3},
+		{3125, 4},
+		{3125, 6},
+		{3125, 8},
+		{125000, 441},
+		{3125, 12},
+		{3125, 16},
+		{62500, 441},
+		{3125, 24},
+		{3125, 32},
+		{31250, 441},
+		{3125, 48},
+		{3125, 64},
+		{15625, 441},
+		{3125, 96},
+		{3125, 192},
+		{3125, 384}
+	};
+
+	switch (method) {
+	/* only mul can be changeable 1/12,2/12,3/12...*/
+	case K230_MUL:
+		perfect_divide = (long)((parent_rate * 1000) / rate);
+		abs_min = abs(perfect_divide -
+			     (long)(((long)div_max * 1000) / (long)mul_min));
+		*mul = mul_min;
+
+		for (u32 i = mul_min + 1; i <= mul_max; i++) {
+			abs_current = abs(perfect_divide -
+					(long)((long)((long)div_max * 1000) / (long)i));
+			if (abs_min > abs_current) {
+				abs_min = abs_current;
+				*mul = i;
+			}
+		}
+
+		*div = div_max;
+		break;
+	/* only div can be changeable, 1/1,1/2,1/3...*/
+	case K230_DIV:
+		perfect_divide = (long)((parent_rate * 1000) / rate);
+		abs_min = abs(perfect_divide -
+			     (long)(((long)div_min * 1000) / (long)mul_max));
+		*div = div_min;
+
+		for (u32 i = div_min + 1; i <= div_max; i++) {
+			abs_current = abs(perfect_divide -
+					 (long)((long)((long)i * 1000) / (long)mul_max));
+			if (abs_min > abs_current) {
+				abs_min = abs_current;
+				*div = i;
+			}
+		}
+
+		*mul = mul_max;
+		break;
+	/* mul and div can be changeable. */
+	case K230_MUL_DIV:
+		if (cfg->rate_reg_off == K230_CLK_CODEC_ADC_MCLKDIV_OFFSET ||
+		    cfg->rate_reg_off == K230_CLK_CODEC_DAC_MCLKDIV_OFFSET) {
+			for (u32 j = 0; j < 9; j++) {
+				if (0 == (rate - codec_clk[j])) {
+					*div = codec_div[j][0];
+					*mul = codec_div[j][1];
+				}
+			}
+		} else if (cfg->rate_reg_off == K230_CLK_AUDIO_CLKDIV_OFFSET ||
+			   cfg->rate_reg_off == K230_CLK_PDM_CLKDIV_OFFSET) {
+			for (u32 j = 0; j < 20; j++) {
+				if (0 == (rate - pdm_clk[j])) {
+					*div = pdm_div[j][0];
+					*mul = pdm_div[j][1];
+				}
+			}
+		} else {
+			return -EINVAL;
+		}
+		break;
+	default:
+		WARN_ON_ONCE(true);
+		return -EPERM;
+	}
+	return 0;
+}
+
+static long k230_clk_round_rate(struct clk_hw *hw, unsigned long rate, unsigned long *parent_rate)
+{
+	struct k230_clk *clk = to_k230_clk(hw);
+	struct k230_sysclk *ksc = clk->ksc;
+	struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
+	u32 div = 0, mul = 0;
+
+	if (k230_clk_find_approximate(clk,
+				      cfg->rate_mul_min, cfg->rate_mul_max,
+				      cfg->rate_div_min, cfg->rate_div_max,
+				      cfg->method, rate, *parent_rate, &div, &mul)) {
+		dev_err(&ksc->pdev->dev,
+			"[%s]: clk %s round rate error!\n",
+			__func__,
+			clk_hw_get_name(hw));
+		return 0;
+	}
+
+	return div_u64((u64)(*parent_rate) * mul, div);
+}
+
+static int k230_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+			     unsigned long parent_rate)
+{
+	struct k230_clk *clk = to_k230_clk(hw);
+	struct k230_sysclk *ksc = clk->ksc;
+	struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
+	u32 div = 0, mul = 0, reg = 0, reg_c;
+
+	if (!cfg->have_rate || !cfg->rate_reg)
+		return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock may have no rate\n");
+
+	if (rate > parent_rate || rate == 0 || parent_rate == 0)
+		return dev_err_probe(&ksc->pdev->dev, -EINVAL, "rate or parent_rate error\n");
+
+	if (cfg->read_only)
+		return dev_err_probe(&ksc->pdev->dev, -EPERM, "This clk rate is read only\n");
+
+	if (k230_clk_find_approximate(clk,
+				      cfg->rate_mul_min, cfg->rate_mul_max,
+				      cfg->rate_div_min, cfg->rate_div_max,
+				      cfg->method, rate, parent_rate, &div, &mul))
+		return dev_err_probe(&ksc->pdev->dev, -EINVAL, "[%s]: clk %s set rate error!\n",
+				     __func__, clk_hw_get_name(hw));
+
+	guard(spinlock)(&ksc->clk_lock);
+	if (!cfg->have_rate_c) {
+		reg = readl(cfg->rate_reg);
+		reg &= ~((cfg->rate_div_mask) << (cfg->rate_div_shift));
+
+		if (cfg->method == K230_DIV) {
+			reg &= ~((cfg->rate_mul_mask) << (cfg->rate_mul_shift));
+			reg |= ((div - 1) & cfg->rate_div_mask) << (cfg->rate_div_shift);
+		} else if (cfg->method == K230_MUL) {
+			reg |= ((mul - 1) & cfg->rate_div_mask) << (cfg->rate_div_shift);
+		} else {
+			reg |= (mul & cfg->rate_mul_mask) << (cfg->rate_mul_shift);
+			reg |= (div & cfg->rate_div_mask) << (cfg->rate_div_shift);
+		}
+		reg |= BIT(cfg->rate_write_enable_bit);
+	} else {
+		reg = readl(cfg->rate_reg);
+		reg_c = readl(cfg->rate_reg_c);
+		reg &= ~((cfg->rate_div_mask) << (cfg->rate_div_shift));
+		reg_c &= ~((cfg->rate_mul_mask_c) << (cfg->rate_mul_shift_c));
+		reg_c |= BIT(cfg->rate_write_enable_bit_c);
+
+		reg_c |= (mul & cfg->rate_mul_mask_c) << (cfg->rate_mul_shift_c);
+		reg |= (div & cfg->rate_div_mask) << (cfg->rate_div_shift);
+
+		writel(reg_c, cfg->rate_reg_c);
+	}
+	writel(reg, cfg->rate_reg);
+
+	return 0;
+}
+
+static const struct clk_ops k230_clk_ops_arr[K230_CLK_OPS_ID_NUM] = {
+	[K230_CLK_OPS_ID_NONE] = {
+		/* Sentinel */
+	},
+	[K230_CLK_OPS_ID_GATE_ONLY] = {
+		K230_CLK_OPS_GATE,
+	},
+	[K230_CLK_OPS_ID_RATE_ONLY] = {
+		K230_CLK_OPS_RATE,
+	},
+	[K230_CLK_OPS_ID_RATE_GATE] = {
+		K230_CLK_OPS_RATE,
+		K230_CLK_OPS_GATE,
+	},
+	[K230_CLK_OPS_ID_MUX_ONLY] = {
+		K230_CLK_OPS_MUX,
+	},
+	[K230_CLK_OPS_ID_MUX_GATE] = {
+		K230_CLK_OPS_MUX,
+		K230_CLK_OPS_GATE,
+	},
+	[K230_CLK_OPS_ID_MUX_RATE] = {
+		K230_CLK_OPS_MUX,
+		K230_CLK_OPS_RATE,
+	},
+	[K230_CLK_OPS_ID_ALL] = {
+		K230_CLK_OPS_MUX,
+		K230_CLK_OPS_RATE,
+		K230_CLK_OPS_GATE,
+	},
+};
+
+static int k230_register_clk(struct platform_device *pdev,
+			     struct k230_sysclk *ksc,
+			     int id,
+			     const struct clk_parent_data *parent_data,
+			     u8 num_parents,
+			     unsigned long flags)
+{
+	struct k230_clk *clk = &ksc->clks[id];
+	struct k230_clk_cfg *cfg = &k230_clk_cfgs[id];
+	struct clk_init_data init = {};
+	int clk_id = 0;
+	int ret;
+
+	if (cfg->have_rate) {
+		cfg->rate_reg = (ksc->regs + cfg->rate_reg_off);
+		clk_id += K230_CLK_OPS_ID_RATE_ONLY;
+	}
+
+	if (cfg->have_mux) {
+		cfg->mux_reg = (ksc->regs + cfg->mux_reg_off);
+		clk_id += K230_CLK_OPS_ID_MUX_ONLY;
+
+		/* mux clock doesn't match the case that num_parents less than 2 */
+		if (num_parents < 2)
+			return -EINVAL;
+	}
+
+	if (cfg->have_gate) {
+		cfg->gate_reg = (ksc->regs + cfg->gate_reg_off);
+		clk_id += K230_CLK_OPS_ID_GATE_ONLY;
+	}
+
+	if (cfg->have_rate_c)
+		cfg->rate_reg_c = (ksc->regs + cfg->rate_reg_off_c);
+
+	init.name = k230_clk_cfgs[id].name;
+	init.flags = flags;
+	init.parent_data = parent_data;
+	init.num_parents = num_parents;
+	init.ops = &k230_clk_ops_arr[clk_id];
+
+	clk->id = id;
+	clk->ksc = ksc;
+	clk->hw.init = &init;
+
+	ret = devm_clk_hw_register(&pdev->dev, &clk->hw);
+	if (ret)
+		return dev_err_probe(&pdev->dev,
+				     ret,
+				     "register clock %s failed\n",
+				     k230_clk_cfgs[id].name);
+
+	return 0;
+}
+
+static int k230_register_mux_clk(struct platform_device *pdev,
+				 struct k230_sysclk *ksc,
+				 struct clk_parent_data *parent_data,
+				 int num_parent,
+				 int id)
+{
+	return k230_register_clk(pdev, ksc, id,
+				(const struct clk_parent_data *)parent_data,
+				num_parent, 0);
+}
+
+static int k230_register_osc24m_child(struct platform_device *pdev,
+				      struct k230_sysclk *ksc,
+				      int id)
+{
+	const struct clk_parent_data parent_data = {
+		.index = 0,
+	};
+	return k230_register_clk(pdev, ksc, id, &parent_data, 1, 0);
+}
+
+static int k230_register_pll_child(struct platform_device *pdev,
+				   struct k230_sysclk *ksc,
+				   int id,
+				   enum k230_pll_id pll_id,
+				   unsigned long flags)
+{
+	const struct clk_parent_data parent_data = {
+		.hw = &ksc->plls[pll_id].hw,
+	};
+	return k230_register_clk(pdev, ksc, id, &parent_data, 1, flags);
+}
+
+static int k230_register_pll_div_child(struct platform_device *pdev,
+				       struct k230_sysclk *ksc,
+				       int id,
+				       enum k230_pll_div_id pll_div_id,
+				       unsigned long flags)
+{
+	const struct clk_parent_data parent_data = {
+		.hw = ksc->dclks[pll_div_id].hw,
+	};
+	return k230_register_clk(pdev, ksc, id, &parent_data, 1, flags);
+}
+
+static int k230_register_clk_child(struct platform_device *pdev,
+				   struct k230_sysclk *ksc,
+				   int id,
+				   int parent_id)
+{
+	const struct clk_parent_data parent_data = {
+		.hw = &ksc->clks[parent_id].hw,
+	};
+	return k230_register_clk(pdev, ksc, id, &parent_data, 1, 0);
+}
+
+static int _k230_clk_mux_get_parent_data(struct k230_sysclk *ksc,
+					 struct k230_clk_parent *pclk,
+					 struct clk_parent_data *parent_data)
+{
+	switch (pclk->type) {
+	case K230_OSC24M:
+		parent_data->index = 0;
+		break;
+	case K230_PLL:
+		parent_data->hw = &ksc->plls[pclk->pll_id].hw;
+		break;
+	case K230_PLL_DIV:
+		parent_data->hw = ksc->dclks[pclk->pll_div_id].hw;
+		break;
+	case K230_CLK_COMPOSITE:
+		parent_data->hw = &ksc->clks[pclk->clk_id].hw;
+		break;
+	default:
+		WARN_ON_ONCE(true);
+		return -EPERM;
+	}
+
+	return 0;
+}
+
+static int k230_clk_mux_get_parent_data(struct k230_sysclk *ksc,
+					struct k230_clk_cfg *cfg,
+					struct clk_parent_data *parent_data,
+					int num_parent)
+{
+	int ret;
+	struct k230_clk_parent *pclk = cfg->parent;
+
+	for (int i = 0; i < num_parent; i++) {
+		ret = _k230_clk_mux_get_parent_data(ksc, &pclk[i], &parent_data[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int k230_register_clks(struct platform_device *pdev, struct k230_sysclk *ksc)
+{
+	struct k230_clk_cfg *cfg;
+	struct k230_clk_parent *pclk;
+	struct clk_parent_data parent_data[K230_CLK_MAX_PARENT_NUM];
+	int ret, i;
+
+	/*
+	 *  Single parent clock:
+	 *  pll0_div2 sons: cpu0_src
+	 *  pll0_div4 sons: cpu0_pclk
+	 *  cpu0_src sons: cpu0_aclk, cpu0_plic, cpu0_noc_ddrcp4, pmu_pclk
+	 *
+	 *  Mux clock:
+	 *  hs_ospi_src parents: pll0_div2, pll2_div4
+	 */
+	for (i = 0; i < K230_CLK_NUM; i++) {
+		cfg = &k230_clk_cfgs[i];
+		if (!cfg->status)
+			continue;
+
+		if (cfg->have_mux) {
+			ret = k230_clk_mux_get_parent_data(ksc, cfg, parent_data,
+							   cfg->num_parent);
+			if (ret)
+				return dev_err_probe(&pdev->dev, ret,
+						     "Failed to get parent data\n");
+
+			ret = k230_register_mux_clk(pdev, ksc, parent_data,
+						    cfg->num_parent, i);
+		} else {
+			pclk = cfg->parent;
+			switch (pclk->type) {
+			case K230_OSC24M:
+				ret = k230_register_osc24m_child(pdev, ksc, i);
+				break;
+			case K230_PLL:
+				ret = k230_register_pll_child(pdev, ksc, i,
+							      pclk->pll_id, cfg->flags);
+				break;
+			case K230_PLL_DIV:
+				ret = k230_register_pll_div_child(pdev, ksc, i,
+								  pclk->pll_div_id, cfg->flags);
+				break;
+			case K230_CLK_COMPOSITE:
+				ret = k230_register_clk_child(pdev, ksc, i,
+							      pclk->clk_id);
+				break;
+			default:
+				return dev_err_probe(&pdev->dev, -EINVAL, "Invalid type\n");
+			}
+		}
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret, "register child id %d failed\n", i);
+	}
+
+	return 0;
+}
+
+static struct clk_hw *k230_clk_hw_onecell_get(struct of_phandle_args *clkspec, void *data)
+{
+	struct k230_sysclk *ksc;
+	unsigned int idx;
+
+	if (clkspec->args_count != 1)
+		return ERR_PTR(-EINVAL);
+
+	idx = clkspec->args[0];
+	if (idx >= K230_CLK_NUM)
+		return ERR_PTR(-EINVAL);
+
+	if (!data)
+		return ERR_PTR(-EINVAL);
+
+	ksc = (struct k230_sysclk *)data;
+
+	return &ksc->clks[idx].hw;
+}
+
+static int k230_clk_init_plls(struct platform_device *pdev)
+{
+	int ret;
+	struct k230_sysclk *ksc = platform_get_drvdata(pdev);
+
+	spin_lock_init(&ksc->pll_lock);
+
+	ksc->pll_regs = devm_platform_ioremap_resource(pdev, 0);
+	if (!ksc->pll_regs)
+		return dev_err_probe(&pdev->dev, PTR_ERR(ksc->pll_regs), "map registers failed\n");
+
+	ret = k230_register_plls(pdev, ksc);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "register plls failed\n");
+
+	ret = k230_register_pll_divs(pdev, ksc);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "register pll_divs failed\n");
+
+	for (int i = 0; i < K230_PLL_DIV_NUM; i++) {
+		ret = devm_clk_hw_register_clkdev(&pdev->dev, ksc->dclks[i].hw,
+						  k230_pll_div_cfgs[i].name, NULL);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret, "clock_lookup create failed\n");
+	}
+
+	return 0;
+}
+
+static int k230_clk_init_clks(struct platform_device *pdev)
+{
+	int ret;
+	struct k230_sysclk *ksc = platform_get_drvdata(pdev);
+
+	spin_lock_init(&ksc->clk_lock);
+
+	ksc->regs = devm_platform_ioremap_resource(pdev, 1);
+	if (!ksc->regs)
+		return dev_err_probe(&pdev->dev, PTR_ERR(ksc->regs), "failed to map registers\n");
+
+	ret = k230_register_clks(pdev, ksc);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "register clock provider failed\n");
+
+	ret = devm_of_clk_add_hw_provider(&pdev->dev, k230_clk_hw_onecell_get, ksc);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "add clock provider failed\n");
+
+	return 0;
+}
+
+static int k230_clk_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct k230_sysclk *ksc;
+
+	ksc = devm_kzalloc(&pdev->dev, sizeof(struct k230_sysclk), GFP_KERNEL);
+	if (!ksc)
+		return -ENOMEM;
+
+	ksc->plls = devm_kmalloc_array(&pdev->dev, K230_PLL_NUM,
+				       sizeof(struct k230_pll), GFP_KERNEL);
+	if (!ksc->plls)
+		return -ENOMEM;
+
+	ksc->dclks = devm_kmalloc_array(&pdev->dev, K230_PLL_DIV_NUM,
+					sizeof(struct k230_pll_div), GFP_KERNEL);
+	if (!ksc->dclks)
+		return -ENOMEM;
+
+	ksc->clks = devm_kmalloc_array(&pdev->dev, K230_CLK_NUM,
+				       sizeof(struct k230_clk), GFP_KERNEL);
+	if (!ksc->clks)
+		return -ENOMEM;
+
+	ksc->pdev = pdev;
+	platform_set_drvdata(pdev, ksc);
+
+	ret = k230_clk_init_plls(pdev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "init plls failed\n");
+
+	ret = k230_clk_init_clks(pdev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "init clks failed\n");
+
+	return 0;
+}
+
+static const struct of_device_id k230_clk_ids[] = {
+	{ .compatible = "canaan,k230-clk" },
+	{ /* Sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, k230_clk_ids);
+
+static struct platform_driver k230_clk_driver = {
+	.driver = {
+		.name  = "k230_clock_controller",
+		.of_match_table = k230_clk_ids,
+	},
+	.probe = k230_clk_probe,
+};
+builtin_platform_driver(k230_clk_driver);

-- 
2.34.1

Re: [PATCH v4 2/3] clk: canaan: Add clock driver for Canaan K230

[Stephen Boyd]

Quoting Xukai Wang (2025-02-17 06:45:17)
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 299bc678ed1b9fcd9110bb8c5937a1bd1ea60e23..f63355ab8adeeee90d29d1a975607f5dc0a58bd6 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -464,6 +464,12 @@ config COMMON_CLK_K210
>         help
>           Support for the Canaan Kendryte K210 RISC-V SoC clocks.
>  
> +config COMMON_CLK_K230
> +       bool "Clock driver for the Canaan Kendryte K230 SoC"
> +       depends on OF && (ARCH_CANAAN || COMPILE_TEST)

It doesn't depend on OF to build from what I can tell. This should
simply be

	depends on ARCH_CANAAN || COMPILE_TEST

right?

> +        help
> +          Support for the Canaan Kendryte K230 RISC-V SoC clocks.
> +
>  config COMMON_CLK_SP7021
>         tristate "Clock driver for Sunplus SP7021 SoC"
>         depends on SOC_SP7021 || COMPILE_TEST
> diff --git a/drivers/clk/clk-k230.c b/drivers/clk/clk-k230.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..017ecad9ec47ed751f75fe5197aa119ae35e05f9
> --- /dev/null
> +++ b/drivers/clk/clk-k230.c
> @@ -0,0 +1,1347 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Kendryte Canaan K230 Clock Drivers
> + *
> + * Author: Xukai Wang <kingxukai@zohomail.com>
> + * Author: Troy Mitchell <troymitchell988@gmail.com>
> + */
> +
> +#include <dt-bindings/clock/canaan,k230-clk.h>

Please put dt-bindigns at the end of the include list.

> +#include <linux/clk.h>

Is this include used? Ideally a clk provider (clk-provider.h) isn't also
a clk consumer (clk.h).

> +#include <linux/clkdev.h>

Hopefully clkdev isn't used.

> +#include <linux/clk-provider.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>

Is this include used?

> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>

Need to include mod_devicetable.h

> +
> +/* PLL control register bits. */
> +#define K230_PLL_BYPASS_ENABLE                         BIT(19)
> +#define K230_PLL_GATE_ENABLE                           BIT(2)
> +#define K230_PLL_GATE_WRITE_ENABLE                     BIT(18)
> +#define K230_PLL_OD_SHIFT                              24
> +#define K230_PLL_OD_MASK                               0xF
> +#define K230_PLL_R_SHIFT                               16
> +#define K230_PLL_R_MASK                                        0x3F
> +#define K230_PLL_F_SHIFT                               0
> +#define K230_PLL_F_MASK                                        0x1FFFF
> +#define K230_PLL0_OFFSET_BASE                          0x00
> +#define K230_PLL1_OFFSET_BASE                          0x10
> +#define K230_PLL2_OFFSET_BASE                          0x20
> +#define K230_PLL3_OFFSET_BASE                          0x30
> +#define K230_PLL_DIV_REG_OFFSET                                0x00
> +#define K230_PLL_BYPASS_REG_OFFSET                     0x04
> +#define K230_PLL_GATE_REG_OFFSET                       0x08
> +#define K230_PLL_LOCK_REG_OFFSET                       0x0C
> +
> +/* PLL lock register bits.  */
> +#define K230_PLL_STATUS_MASK                           BIT(0)
> +
> +/* K230 CLK registers offset */
> +#define K230_CLK_AUDIO_CLKDIV_OFFSET                   0x34
> +#define K230_CLK_PDM_CLKDIV_OFFSET                     0x40
> +#define K230_CLK_CODEC_ADC_MCLKDIV_OFFSET              0x38
> +#define K230_CLK_CODEC_DAC_MCLKDIV_OFFSET              0x3c
> +
> +/* K230 CLK OPS. */
> +#define K230_CLK_OPS_GATE                              \
> +       .enable         = k230_clk_enable,              \
> +       .disable        = k230_clk_disable,             \
> +       .is_enabled     = k230_clk_is_enabled
> +
> +#define K230_CLK_OPS_RATE                              \
> +       .set_rate       = k230_clk_set_rate,            \
> +       .round_rate     = k230_clk_round_rate,          \
> +       .recalc_rate    = k230_clk_get_rate
> +
> +#define K230_CLK_OPS_MUX                               \
> +       .set_parent     = k230_clk_set_parent,          \
> +       .get_parent     = k230_clk_get_parent,          \
> +       .determine_rate = clk_hw_determine_rate_no_reparent
> +
> +#define K230_CLK_OPS_ID_NONE                           0
> +#define K230_CLK_OPS_ID_GATE_ONLY                      1
> +#define K230_CLK_OPS_ID_RATE_ONLY                      2
> +#define K230_CLK_OPS_ID_RATE_GATE                      3
> +#define K230_CLK_OPS_ID_MUX_ONLY                       4
> +#define K230_CLK_OPS_ID_MUX_GATE                       5
> +#define K230_CLK_OPS_ID_MUX_RATE                       6
> +#define K230_CLK_OPS_ID_ALL                            7
> +#define K230_CLK_OPS_ID_NUM                            8
> +
> +/* K230 CLK MACROS */
> +#define K230_CLK_MAX_PARENT_NUM                                6
> +
> +#define K230_GATE_FORMAT(_reg, _bit, _reverse, _have_gate)                     \
> +       .gate_reg_off = (_reg),                                                 \
> +       .gate_bit_enable = (_bit),                                              \
> +       .gate_bit_reverse = (_reverse),                                         \
> +       .have_gate = (_have_gate)
> +
> +#define K230_RATE_FORMAT(_mul_min, _mul_max, _mul_shift, _mul_mask,            \
> +                       _div_min, _div_max, _div_shift, _div_mask,              \
> +                       _reg, _bit, _method, _reg_c, _bit_c,                    \
> +                       _mul_min_c, _mul_max_c, _mul_shift_c, _mul_mask_c,      \
> +                       _have_rate, _have_rate_c)                               \
> +       .rate_mul_min = (_mul_min),                                             \
> +       .rate_mul_max = (_mul_max),                                             \
> +       .rate_mul_shift = (_mul_shift),                                         \
> +       .rate_mul_mask = (_mul_mask),                                           \
> +       .rate_mul_min_c = (_mul_min_c),                                         \
> +       .rate_mul_max_c = (_mul_max_c),                                         \
> +       .rate_mul_shift_c = (_mul_shift_c),                                     \
> +       .rate_mul_mask_c = (_mul_mask_c),                                       \
> +       .rate_div_min = (_div_min),                                             \
> +       .rate_div_max = (_div_max),                                             \
> +       .rate_div_shift = (_div_shift),                                         \
> +       .rate_div_mask = (_div_mask),                                           \
> +       .rate_reg_off = (_reg),                                                 \
> +       .rate_reg_off_c = (_reg_c),                                             \
> +       .rate_write_enable_bit = (_bit),                                        \
> +       .rate_write_enable_bit_c = (_bit_c),                                    \
> +       .method = (_method),                                                    \
> +       .have_rate = (_have_rate),                                              \
> +       .have_rate_c = (_have_rate_c)
> +
> +#define K230_MUX_FORMAT(_reg, _shift, _mask, _have_mux)                                \
> +       .mux_reg_off = (_reg),                                                  \
> +       .mux_reg_shift = (_shift),                                              \
> +       .mux_reg_mask = (_mask),                                                \
> +       .have_mux = (_have_mux)
> +
> +#define K230_GATE_FORMAT_ZERO  K230_GATE_FORMAT(0, 0, 0, false)
> +#define K230_RATE_FORMAT_ZERO  K230_RATE_FORMAT(0, 0, 0, 0, 0, 0,              \
> +                                               0, 0, 0, 0, 0, 0,               \
> +                                               0, 0, 0, 0, 0, false, false)
> +#define K230_MUX_FORMAT_ZERO   K230_MUX_FORMAT(0, 0, 0, false)
> +
> +struct k230_sysclk;
> +
> +/* K230 PLLs. */
> +enum k230_pll_id {
> +       K230_PLL0, K230_PLL1, K230_PLL2, K230_PLL3, K230_PLL_NUM
> +};
> +
> +struct k230_pll {
> +       enum k230_pll_id id;
> +       struct k230_sysclk *ksc;
> +       void __iomem *div, *bypass, *gate, *lock;
> +       struct clk_hw hw;
> +};
> +
> +#define to_k230_pll(_hw)       container_of(_hw, struct k230_pll, hw)
> +
> +struct k230_pll_cfg {
> +       u32 reg;
> +       enum k230_pll_id pll_id;
> +       const char *name;
> +};
> +
> +/* K230 PLL_DIVS. */
> +struct k230_pll_div {
> +       struct k230_sysclk *ksc;
> +       struct clk_hw *hw;
> +};
> +
> +struct k230_pll_div_cfg {
> +       const char *parent_name, *name;
> +       int div;
> +};
> +
> +enum k230_pll_div_id {
> +       K230_PLL0_DIV2,
> +       K230_PLL0_DIV3,
> +       K230_PLL0_DIV4,
> +       K230_PLL1_DIV2,
> +       K230_PLL1_DIV3,
> +       K230_PLL1_DIV4,
> +       K230_PLL2_DIV2,
> +       K230_PLL2_DIV3,
> +       K230_PLL2_DIV4,
> +       K230_PLL3_DIV2,
> +       K230_PLL3_DIV3,
> +       K230_PLL3_DIV4,
> +       K230_PLL_DIV_NUM
> +};
> +
> +enum k230_clk_div_type {
> +       K230_MUL,
> +       K230_DIV,
> +       K230_MUL_DIV,
> +};
> +
> +/* K230 CLKS. */
> +struct k230_clk {
> +       int id;
> +       struct k230_sysclk *ksc;
> +       struct clk_hw hw;
> +};
> +
> +#define to_k230_clk(_hw)       container_of(_hw, struct k230_clk, hw)
> +
> +enum k230_clk_parent_type {
> +       K230_OSC24M,
> +       K230_PLL,
> +       K230_PLL_DIV,
> +       K230_CLK_COMPOSITE,
> +};
> +
> +struct k230_clk_parent {
> +       enum k230_clk_parent_type type;
> +       union {
> +               enum k230_pll_div_id pll_div_id;
> +               enum k230_pll_id pll_id;
> +               int clk_id;
> +       };
> +};
> +
> +struct k230_clk_cfg {
> +       /* attr */
> +       const char *name;
> +       /* 0-read & write; 1-read only */
> +       bool read_only;
> +       int num_parent;
> +       struct k230_clk_parent parent[K230_CLK_MAX_PARENT_NUM];
> +       bool status;
> +       int flags;
> +
> +       /* rate reg */
> +       u32 rate_reg_off;
> +       u32 rate_reg_off_c;
> +       void __iomem *rate_reg;
> +       void __iomem *rate_reg_c;
> +       /* rate info*/
> +       u32 rate_write_enable_bit;
> +       u32 rate_write_enable_bit_c;
> +       enum k230_clk_div_type method;
> +       bool have_rate;
> +       bool have_rate_c;
> +       /* rate mul */
> +       u32 rate_mul_min;
> +       u32 rate_mul_max;
> +       u32 rate_mul_shift;
> +       u32 rate_mul_mask;
> +       /* rate mul-changable */
> +       u32 rate_mul_min_c;
> +       u32 rate_mul_max_c;
> +       u32 rate_mul_shift_c;
> +       u32 rate_mul_mask_c;
> +       /* rate div */
> +       u32 rate_div_min;
> +       u32 rate_div_max;
> +       u32 rate_div_shift;
> +       u32 rate_div_mask;
> +
> +       /* gate reg */
> +       u32 gate_reg_off;
> +       void __iomem *gate_reg;
> +       /* gate info*/
> +       bool have_gate;
> +       u32 gate_bit_enable;
> +       u32 gate_bit_reverse;
> +
> +       /* mux reg */
> +       u32 mux_reg_off;
> +       void __iomem *mux_reg;
> +       /* mux info */
> +       bool have_mux;
> +       u32 mux_reg_shift;
> +       u32 mux_reg_mask;
> +};
> +
> +/* K230 SYSCLK. */
> +struct k230_sysclk {
> +       struct platform_device *pdev;
> +       void __iomem           *pll_regs, *regs;
> +       spinlock_t             pll_lock, clk_lock;
> +       struct k230_pll        *plls;
> +       struct k230_clk        *clks;
> +       struct k230_pll_div    *dclks;
> +};
> +
> +static const struct k230_pll_cfg k230_pll_cfgs[] = {
> +       [K230_PLL0] = {
> +               .reg = K230_PLL0_OFFSET_BASE,
> +               .pll_id = K230_PLL0,
> +               .name = "pll0",
> +       },
> +       [K230_PLL1] = {
> +               .reg = K230_PLL1_OFFSET_BASE,
> +               .pll_id = K230_PLL1,
> +               .name = "pll1",
> +       },
> +       [K230_PLL2] = {
> +               .reg = K230_PLL2_OFFSET_BASE,
> +               .pll_id = K230_PLL2,
> +               .name = "pll2",
> +       },
> +       [K230_PLL3] = {
> +               .reg = K230_PLL3_OFFSET_BASE,
> +               .pll_id = K230_PLL3,
> +               .name = "pll3",
> +       },
> +};
> +
> +static const struct k230_pll_div_cfg k230_pll_div_cfgs[] = {
> +       [K230_PLL0_DIV2] = { "pll0", "pll0_div2", 2},
> +       [K230_PLL0_DIV3] = { "pll0", "pll0_div3", 3},
> +       [K230_PLL0_DIV4] = { "pll0", "pll0_div4", 4},
> +       [K230_PLL1_DIV2] = { "pll1", "pll1_div2", 2},
> +       [K230_PLL1_DIV3] = { "pll1", "pll1_div3", 3},
> +       [K230_PLL1_DIV4] = { "pll1", "pll1_div4", 4},
> +       [K230_PLL2_DIV2] = { "pll2", "pll2_div2", 2},
> +       [K230_PLL2_DIV3] = { "pll2", "pll2_div3", 3},
> +       [K230_PLL2_DIV4] = { "pll2", "pll2_div4", 4},
> +       [K230_PLL3_DIV2] = { "pll3", "pll3_div2", 2},
> +       [K230_PLL3_DIV3] = { "pll3", "pll3_div3", 3},
> +       [K230_PLL3_DIV4] = { "pll3", "pll3_div4", 4},
> +};
> +
> +static struct k230_clk_cfg k230_clk_cfgs[] = {
> +       [K230_CPU0_SRC] = {

Can these be named variables instead of array elements? Then we can map
quickly to parents instead of going through an intermediate enum.

> +               .name = "cpu0_src",
> +               .read_only = false,
> +               .flags = 0,
> +               .status = true,
> +               .num_parent = 1,
> +               .parent[0] = {
> +                       .type = K230_PLL_DIV,
> +                       .pll_div_id = K230_PLL0_DIV2,
> +               },
> +               K230_RATE_FORMAT(1, 16, 0, 0,
> +                                16, 16, 1, 0xF,
> +                                0x0, 31, K230_MUL, 0, 0,
> +                                0, 0, 0, 0,
> +                                true, false),
> +               K230_GATE_FORMAT(0, 0, 0, true),
> +               K230_MUX_FORMAT_ZERO,
> +       },
> +       [K230_CPU0_ACLK] = {
> +               .name = "cpu0_aclk",
> +               .read_only = false,
> +               .flags = 0,
> +               .status = true,
> +               .num_parent = 1,
> +               .parent[0] = {
> +                       .type = K230_CLK_COMPOSITE,
> +                       .clk_id = K230_CPU0_SRC,
> +               },
> +               K230_RATE_FORMAT(1, 1, 0, 0,
> +                                1, 8, 7, 0x7,
> +                                0x0, 31, K230_MUL, 0, 0,
> +                                0, 0, 0, 0,
> +                                true, false),
> +               K230_GATE_FORMAT_ZERO,
> +               K230_MUX_FORMAT_ZERO,
> +       },
> +       [K230_CPU0_PLIC] = {
> +               .name = "cpu0_plic",
> +               .read_only = false,
> +               .flags = 0,
> +               .status = true,
> +               .num_parent = 1,
> +               .parent[0] = {
> +                       .type = K230_CLK_COMPOSITE,
> +                       .clk_id = K230_CPU0_SRC,
> +               },
> +               K230_RATE_FORMAT(1, 1, 0, 0,
> +                                1, 8, 10, 0x7,
> +                                0x0, 31, K230_DIV, 0, 0,
> +                                0, 0, 0, 0,
> +                                true, false),
> +               K230_GATE_FORMAT(0, 9, 0, true),
> +               K230_MUX_FORMAT_ZERO,
> +       },
> +       [K230_CPU0_NOC_DDRCP4] = {
> +               .name = "cpu0_noc_ddrcp4",
> +               .read_only = false,
> +               .flags = 0,
> +               .status = true,
> +               .num_parent = 1,
> +               .parent[0] = {
> +                       .type = K230_CLK_COMPOSITE,
> +                       .clk_id = K230_CPU0_SRC,
> +               },
> +               K230_RATE_FORMAT_ZERO,
> +               K230_GATE_FORMAT(0x60, 7, 0, true),
> +               K230_MUX_FORMAT_ZERO,
> +       },
> +       [K230_CPU0_PCLK] = {
> +               .name = "cpu0_pclk",
> +               .read_only = false,
> +               .flags = 0,
> +               .status = true,
> +               .num_parent = 1,
> +               .parent[0] = {
> +                       .type = K230_PLL_DIV,
> +                       .pll_div_id = K230_PLL0_DIV4,
> +               },
> +               K230_RATE_FORMAT(1, 1, 0, 0,
> +                                1, 8, 15, 0x7,
> +                                0x0, 31, K230_DIV, 0, 0,
> +                                0, 0, 0, 0,
> +                                true, false),
> +               K230_GATE_FORMAT(0, 13, 0, true),
> +               K230_MUX_FORMAT_ZERO,
> +       },
> +       [K230_PMU_PCLK] = {
> +               .name = "pmu_pclk",
> +               .read_only = false,
> +               .flags = 0,
> +               .status = true,
> +               .num_parent = 1,
> +               .parent[0] = {
> +                       .type = K230_OSC24M,
> +               },
> +               K230_RATE_FORMAT_ZERO,
> +               K230_GATE_FORMAT(0x10, 0, 0, true),
> +               K230_MUX_FORMAT_ZERO,
> +       },
> +       [K230_HS_OSPI_SRC] = {
> +               .name = "hs_ospi_src",
> +               .read_only = false,
> +               .flags = 0,
> +               .status = true,
> +               .num_parent = 2,
> +               .parent[0] = {
> +                       .type = K230_PLL_DIV,
> +                       .pll_div_id = K230_PLL0_DIV2,
> +               },
> +               .parent[1] = {
> +                       .type = K230_PLL_DIV,
> +                       .pll_div_id = K230_PLL2_DIV4,
> +               },
> +               K230_RATE_FORMAT_ZERO,
> +               K230_GATE_FORMAT(0x18, 24, 0, true),
> +               K230_MUX_FORMAT(0x20, 18, 0x1, true),
> +       },
> +};
> +
> +#define K230_CLK_NUM   ARRAY_SIZE(k230_clk_cfgs)
> +
> +static void k230_init_pll(void __iomem *regs, enum k230_pll_id pll_id,
> +                         struct k230_pll *pll)
> +{
> +       void __iomem *base;
> +
> +       pll->id = pll_id;
> +       base = regs + k230_pll_cfgs[pll_id].reg;
> +       pll->div = base + K230_PLL_DIV_REG_OFFSET;
> +       pll->bypass = base + K230_PLL_BYPASS_REG_OFFSET;
> +       pll->gate = base + K230_PLL_GATE_REG_OFFSET;
> +       pll->lock = base + K230_PLL_LOCK_REG_OFFSET;
> +}
> +
> +static int k230_pll_prepare(struct clk_hw *hw)
> +{
> +       struct k230_pll *pll = to_k230_pll(hw);
> +       u32 reg;
> +
> +       /* wait for PLL lock until it reachs lock status */

s/reachs/reaches/

> +       return readl_poll_timeout(pll->lock, reg,
> +                                 (reg & K230_PLL_STATUS_MASK) == K230_PLL_STATUS_MASK,
> +                                 400, 0);
> +}
> +
> +static bool k230_pll_hw_is_enabled(struct k230_pll *pll)
> +{
> +       return (readl(pll->gate) & K230_PLL_GATE_ENABLE) == K230_PLL_GATE_ENABLE;
> +}
> +
> +static void k230_pll_enable_hw(void __iomem *regs, struct k230_pll *pll)
> +{
> +       u32 reg;
> +
> +       if (k230_pll_hw_is_enabled(pll))
> +               return;
> +
> +       /* Set PLL factors */
> +       reg = readl(pll->gate);
> +       reg |= (K230_PLL_GATE_ENABLE | K230_PLL_GATE_WRITE_ENABLE);
> +       writel(reg, pll->gate);
> +}
> +
> +static int k230_pll_enable(struct clk_hw *hw)
> +{
> +       struct k230_pll *pll = to_k230_pll(hw);
> +       struct k230_sysclk *ksc = pll->ksc;
> +
> +       guard(spinlock)(&ksc->pll_lock);
> +       k230_pll_enable_hw(ksc->regs, pll);
> +
> +       return 0;
> +}
> +
> +static void k230_pll_disable(struct clk_hw *hw)
> +{
> +       struct k230_pll *pll = to_k230_pll(hw);
> +       struct k230_sysclk *ksc = pll->ksc;
> +       u32 reg;
> +
> +       guard(spinlock)(&ksc->pll_lock);
> +       reg = readl(pll->gate);
> +
> +       reg &= ~(K230_PLL_GATE_ENABLE);
> +       reg |= (K230_PLL_GATE_WRITE_ENABLE);
> +
> +       writel(reg, pll->gate);
> +}
> +
> +static int k230_pll_is_enabled(struct clk_hw *hw)
> +{
> +       return k230_pll_hw_is_enabled(to_k230_pll(hw));
> +}
> +
> +static int k230_pll_init(struct clk_hw *hw)
> +{
> +       if (k230_pll_is_enabled(hw))
> +               return clk_prepare_enable(hw->clk);

Is this CLK_IS_CRITICAL?

> +
> +       return 0;
> +}
> +
> +static unsigned long k230_pll_get_rate(struct clk_hw *hw, unsigned long parent_rate)
> +{
> +       struct k230_pll *pll = to_k230_pll(hw);
> +       struct k230_sysclk *ksc = pll->ksc;
> +       u32 reg;
> +       u32 r, f, od;
> +
> +       reg = readl(pll->bypass);
> +       if (reg & K230_PLL_BYPASS_ENABLE)
> +               return parent_rate;
> +
> +       reg = readl(pll->lock);
> +       if (!(reg & (K230_PLL_STATUS_MASK))) { /* unlocked */
> +               dev_err(&ksc->pdev->dev, "%s is unlock.\n", clk_hw_get_name(hw));
> +               return 0;
> +       }
> +
> +       reg = readl(pll->div);
> +       r = ((reg >> K230_PLL_R_SHIFT) & K230_PLL_R_MASK) + 1;
> +       f = ((reg >> K230_PLL_F_SHIFT) & K230_PLL_F_MASK) + 1;
> +       od = ((reg >> K230_PLL_OD_SHIFT) & K230_PLL_OD_MASK) + 1;
> +
> +       return div_u64((u64)parent_rate * f, r * od);

Is this mul_u64_u32_div()?

> +}
> +
> +static const struct clk_ops k230_pll_ops = {
> +       .init           = k230_pll_init,
> +       .prepare        = k230_pll_prepare,
> +       .enable         = k230_pll_enable,
> +       .disable        = k230_pll_disable,
> +       .is_enabled     = k230_pll_is_enabled,
> +       .recalc_rate    = k230_pll_get_rate,
> +};
> +
> +static int k230_register_pll(struct platform_device *pdev,
> +                            struct k230_sysclk *ksc,
> +                            enum k230_pll_id pll_id,
> +                            const char *name,
> +                            int num_parents,
> +                            const struct clk_ops *ops)
> +{
> +       struct k230_pll *pll = &ksc->plls[pll_id];
> +       struct clk_init_data init = {};
> +       struct device *dev = &pdev->dev;
> +       int ret;
> +       const struct clk_parent_data parent_data[] = {
> +               { .index = 0, },
> +       };
> +
> +       init.name = name;
> +       init.parent_data = parent_data;
> +       init.num_parents = num_parents;
> +       init.ops = ops;
> +
> +       pll->hw.init = &init;
> +       pll->ksc = ksc;
> +
> +       ret = devm_clk_hw_register(dev, &pll->hw);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to register pll");

Missing newline on printk.

> +
> +       return 0;
> +}
> +
> +static int k230_register_plls(struct platform_device *pdev, struct k230_sysclk *ksc)
> +{
> +       int i, ret;
> +       const struct k230_pll_cfg *cfg;
> +
> +       for (i = 0; i < K230_PLL_NUM; i++) {
> +               cfg = &k230_pll_cfgs[i];
> +
> +               k230_init_pll(ksc->pll_regs, i, &ksc->plls[i]);
> +
> +               ret = k230_register_pll(pdev, ksc, cfg->pll_id, cfg->name, 1,
> +                                       &k230_pll_ops);
> +               if (ret)
> +                       return dev_err_probe(&pdev->dev, ret,
> +                                            "register %s failed\n", cfg->name);
> +       }
> +
> +       return 0;
> +}
> +
> +static int k230_register_pll_divs(struct platform_device *pdev, struct k230_sysclk *ksc)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct clk_hw *hw;
> +
> +       for (int i = 0; i < K230_PLL_DIV_NUM; i++) {
> +               hw = devm_clk_hw_register_fixed_factor(dev, k230_pll_div_cfgs[i].name,
> +                                                      k230_pll_div_cfgs[i].parent_name,
> +                                                      0, 1, k230_pll_div_cfgs[i].div);
> +               if (IS_ERR(hw))
> +                       return PTR_ERR(hw);
> +               ksc->dclks[i].hw = hw;
> +               ksc->dclks[i].ksc = ksc;
> +       }
> +
> +       return 0;
> +}
> +
> +static int k230_clk_enable(struct clk_hw *hw)
> +{
> +       struct k230_clk *clk = to_k230_clk(hw);
> +       struct k230_sysclk *ksc = clk->ksc;
> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
> +       u32 reg;
> +
> +       if (!cfg->have_gate)
> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have gate\n");
> +
> +       guard(spinlock)(&ksc->clk_lock);
> +       reg = readl(cfg->gate_reg);
> +       if (cfg->gate_bit_reverse)
> +               reg &= ~BIT(cfg->gate_bit_enable);
> +       else
> +               reg |= BIT(cfg->gate_bit_enable);
> +       writel(reg, cfg->gate_reg);
> +
> +       return 0;
> +}
> +
> +static void k230_clk_disable(struct clk_hw *hw)
> +{
> +       struct k230_clk *clk = to_k230_clk(hw);
> +       struct k230_sysclk *ksc = clk->ksc;
> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
> +       u32 reg;
> +
> +       if (!cfg->have_gate) {
> +               dev_err(&ksc->pdev->dev, "This clock doesn't have gate\n");

Why are the clk_ops assigned to this clk then?

> +               return;
> +       }
> +
> +       guard(spinlock)(&ksc->clk_lock);
> +       reg = readl(cfg->gate_reg);
> +
> +       if (cfg->gate_bit_reverse)
> +               reg |= BIT(cfg->gate_bit_enable);
> +       else
> +               reg &= ~BIT(cfg->gate_bit_enable);
> +
> +       writel(reg, cfg->gate_reg);
> +}
> +
> +static int k230_clk_is_enabled(struct clk_hw *hw)
> +{
> +       struct k230_clk *clk = to_k230_clk(hw);
> +       struct k230_sysclk *ksc = clk->ksc;
> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
> +       u32 reg;
> +
> +       if (!cfg->have_gate)
> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have gate\n");

Why are the clk_ops assigned to this clk then? Don't use
dev_err_probe() here.

> +
> +       guard(spinlock)(&ksc->clk_lock);
> +       reg = readl(cfg->gate_reg);
> +
> +       /* Check gate bit condition based on configuration and then set ret */
> +       if (cfg->gate_bit_reverse)
> +               return (BIT(cfg->gate_bit_enable) & reg) ? 1 : 0;
> +       else
> +               return (BIT(cfg->gate_bit_enable) & ~reg) ? 1 : 0;
> +}
> +
> +static int k230_clk_set_parent(struct clk_hw *hw, u8 index)
> +{
> +       struct k230_clk *clk = to_k230_clk(hw);
> +       struct k230_sysclk *ksc = clk->ksc;
> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
> +       u8 reg;
> +
> +       if (!cfg->have_mux)
> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have mux\n");

Why are the clk_ops assigned to this clk then? Don't use
dev_err_probe() here.

> +
> +       guard(spinlock)(&ksc->clk_lock);
> +       reg = (cfg->mux_reg_mask & index) << cfg->mux_reg_shift;
> +       writeb(reg, cfg->mux_reg);
> +
> +       return 0;
> +}
> +
> +static u8 k230_clk_get_parent(struct clk_hw *hw)
> +{
> +       struct k230_clk *clk = to_k230_clk(hw);
> +       struct k230_sysclk *ksc = clk->ksc;
> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
> +       u8 reg;
> +
> +       if (!cfg->have_mux)
> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have mux\n");

All these checks should go away and we should have different clk_ops for
different clk types.

> +
> +       guard(spinlock)(&ksc->clk_lock);
> +       reg = readb(cfg->mux_reg);
> +
> +       return reg;
> +}
> +
> +static unsigned long k230_clk_get_rate(struct clk_hw *hw,
> +                                      unsigned long parent_rate)
> +{
> +       struct k230_clk *clk = to_k230_clk(hw);
> +       struct k230_sysclk *ksc = clk->ksc;
> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
> +       u32 mul, div;
> +
> +       if (!cfg->have_rate) /* no divider, return parents' clk */
> +               return parent_rate;
> +
> +       guard(spinlock)(&ksc->clk_lock);
> +       switch (cfg->method) {
> +       /*
> +        * K230_MUL: div_mask+1/div_max...
> +        * K230_DIV: mul_max/div_mask+1
> +        * K230_MUL_DIV: mul_mask/div_mask...
> +        */
> +       case K230_MUL:
> +               div = cfg->rate_div_max;
> +               mul = (readl(cfg->rate_reg) >> cfg->rate_div_shift)
> +                       & cfg->rate_div_mask;
> +               mul++;
> +               break;
> +       case K230_DIV:
> +               mul = cfg->rate_mul_max;
> +               div = (readl(cfg->rate_reg) >> cfg->rate_div_shift)
> +                       & cfg->rate_div_mask;
> +               div++;
> +               break;
> +       case K230_MUL_DIV:
> +               if (!cfg->have_rate_c) {
> +                       mul = (readl(cfg->rate_reg) >> cfg->rate_mul_shift)
> +                               & cfg->rate_mul_mask;
> +                       div = (readl(cfg->rate_reg) >> cfg->rate_div_shift)
> +                               & cfg->rate_div_mask;
> +               } else {
> +                       mul = (readl(cfg->rate_reg_c) >> cfg->rate_mul_shift_c)
> +                               & cfg->rate_mul_mask_c;
> +                       div = (readl(cfg->rate_reg_c) >> cfg->rate_div_shift)
> +                               & cfg->rate_div_mask;
> +               }
> +               break;
> +       default:
> +               return 0;
> +       }
> +
> +       return div_u64((u64)parent_rate * mul, div);
> +}
> +
> +static int k230_clk_find_approximate(struct k230_clk *clk,
> +                                    u32 mul_min,
> +                                    u32 mul_max,
> +                                    u32 div_min,
> +                                    u32 div_max,
> +                                    enum k230_clk_div_type method,
> +                                    unsigned long rate,
> +                                    unsigned long parent_rate,
> +                                    u32 *div,
> +                                    u32 *mul)
> +{
> +       long abs_min;
> +       long abs_current;
> +       long perfect_divide;
> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
> +
> +       const u32 codec_clk[9] = {
> +               2048000,
> +               3072000,
> +               4096000,
> +               6144000,
> +               8192000,
> +               11289600,
> +               12288000,
> +               24576000,
> +               49152000
> +       };
> +
> +       const u32 codec_div[9][2] = {
> +               {3125, 16},
> +               {3125, 24},
> +               {3125, 32},
> +               {3125, 48},
> +               {3125, 64},
> +               {15625, 441},
> +               {3125, 96},
> +               {3125, 192},
> +               {3125, 384}
> +       };
> +
> +       const u32 pdm_clk[20] = {
> +               128000,
> +               192000,
> +               256000,
> +               384000,
> +               512000,
> +               768000,
> +               1024000,
> +               1411200,
> +               1536000,
> +               2048000,
> +               2822400,
> +               3072000,
> +               4096000,
> +               5644800,
> +               6144000,
> +               8192000,
> +               11289600,
> +               12288000,
> +               24576000,
> +               49152000
> +       };
> +
> +       const u32 pdm_div[20][2] = {
> +               {3125, 1},
> +               {6250, 3},
> +               {3125, 2},
> +               {3125, 3},
> +               {3125, 4},
> +               {3125, 6},
> +               {3125, 8},
> +               {125000, 441},
> +               {3125, 12},
> +               {3125, 16},
> +               {62500, 441},
> +               {3125, 24},
> +               {3125, 32},
> +               {31250, 441},
> +               {3125, 48},
> +               {3125, 64},
> +               {15625, 441},
> +               {3125, 96},
> +               {3125, 192},
> +               {3125, 384}
> +       };
> +
> +       switch (method) {
> +       /* only mul can be changeable 1/12,2/12,3/12...*/
> +       case K230_MUL:
> +               perfect_divide = (long)((parent_rate * 1000) / rate);
> +               abs_min = abs(perfect_divide -
> +                            (long)(((long)div_max * 1000) / (long)mul_min));
> +               *mul = mul_min;
> +
> +               for (u32 i = mul_min + 1; i <= mul_max; i++) {
> +                       abs_current = abs(perfect_divide -
> +                                       (long)((long)((long)div_max * 1000) / (long)i));
> +                       if (abs_min > abs_current) {
> +                               abs_min = abs_current;
> +                               *mul = i;
> +                       }
> +               }
> +
> +               *div = div_max;
> +               break;
> +       /* only div can be changeable, 1/1,1/2,1/3...*/
> +       case K230_DIV:
> +               perfect_divide = (long)((parent_rate * 1000) / rate);
> +               abs_min = abs(perfect_divide -
> +                            (long)(((long)div_min * 1000) / (long)mul_max));
> +               *div = div_min;
> +
> +               for (u32 i = div_min + 1; i <= div_max; i++) {
> +                       abs_current = abs(perfect_divide -
> +                                        (long)((long)((long)i * 1000) / (long)mul_max));
> +                       if (abs_min > abs_current) {
> +                               abs_min = abs_current;
> +                               *div = i;
> +                       }
> +               }
> +
> +               *mul = mul_max;
> +               break;
> +       /* mul and div can be changeable. */
> +       case K230_MUL_DIV:
> +               if (cfg->rate_reg_off == K230_CLK_CODEC_ADC_MCLKDIV_OFFSET ||
> +                   cfg->rate_reg_off == K230_CLK_CODEC_DAC_MCLKDIV_OFFSET) {
> +                       for (u32 j = 0; j < 9; j++) {
> +                               if (0 == (rate - codec_clk[j])) {
> +                                       *div = codec_div[j][0];
> +                                       *mul = codec_div[j][1];
> +                               }
> +                       }
> +               } else if (cfg->rate_reg_off == K230_CLK_AUDIO_CLKDIV_OFFSET ||
> +                          cfg->rate_reg_off == K230_CLK_PDM_CLKDIV_OFFSET) {
> +                       for (u32 j = 0; j < 20; j++) {
> +                               if (0 == (rate - pdm_clk[j])) {
> +                                       *div = pdm_div[j][0];
> +                                       *mul = pdm_div[j][1];
> +                               }
> +                       }
> +               } else {
> +                       return -EINVAL;
> +               }
> +               break;
> +       default:
> +               WARN_ON_ONCE(true);
> +               return -EPERM;

This is impossible? Remove the default case and the compiler will warn
if an enum case is missing.

> +       }
> +       return 0;
> +}
> +
> +static long k230_clk_round_rate(struct clk_hw *hw, unsigned long rate, unsigned long *parent_rate)
> +{
> +       struct k230_clk *clk = to_k230_clk(hw);
> +       struct k230_sysclk *ksc = clk->ksc;
> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
> +       u32 div = 0, mul = 0;
> +
> +       if (k230_clk_find_approximate(clk,
> +                                     cfg->rate_mul_min, cfg->rate_mul_max,
> +                                     cfg->rate_div_min, cfg->rate_div_max,
> +                                     cfg->method, rate, *parent_rate, &div, &mul)) {
> +               dev_err(&ksc->pdev->dev,
> +                       "[%s]: clk %s round rate error!\n",
> +                       __func__,
> +                       clk_hw_get_name(hw));
> +               return 0;

Why isn't an error returned?

> +       }
> +
> +       return div_u64((u64)(*parent_rate) * mul, div);
> +}
> +
> +static int k230_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> +                            unsigned long parent_rate)
> +{
> +       struct k230_clk *clk = to_k230_clk(hw);
> +       struct k230_sysclk *ksc = clk->ksc;
> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
> +       u32 div = 0, mul = 0, reg = 0, reg_c;
> +
> +       if (!cfg->have_rate || !cfg->rate_reg)
> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock may have no rate\n");
> +
> +       if (rate > parent_rate || rate == 0 || parent_rate == 0)
> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "rate or parent_rate error\n");
> +
> +       if (cfg->read_only)
> +               return dev_err_probe(&ksc->pdev->dev, -EPERM, "This clk rate is read only\n");
> +
> +       if (k230_clk_find_approximate(clk,
> +                                     cfg->rate_mul_min, cfg->rate_mul_max,
> +                                     cfg->rate_div_min, cfg->rate_div_max,
> +                                     cfg->method, rate, parent_rate, &div, &mul))
> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "[%s]: clk %s set rate error!\n",
> +                                    __func__, clk_hw_get_name(hw));
> +
> +       guard(spinlock)(&ksc->clk_lock);
> +       if (!cfg->have_rate_c) {
> +               reg = readl(cfg->rate_reg);
> +               reg &= ~((cfg->rate_div_mask) << (cfg->rate_div_shift));
> +
> +               if (cfg->method == K230_DIV) {
> +                       reg &= ~((cfg->rate_mul_mask) << (cfg->rate_mul_shift));
> +                       reg |= ((div - 1) & cfg->rate_div_mask) << (cfg->rate_div_shift);
> +               } else if (cfg->method == K230_MUL) {
> +                       reg |= ((mul - 1) & cfg->rate_div_mask) << (cfg->rate_div_shift);
> +               } else {
> +                       reg |= (mul & cfg->rate_mul_mask) << (cfg->rate_mul_shift);
> +                       reg |= (div & cfg->rate_div_mask) << (cfg->rate_div_shift);
> +               }
> +               reg |= BIT(cfg->rate_write_enable_bit);
> +       } else {
> +               reg = readl(cfg->rate_reg);
> +               reg_c = readl(cfg->rate_reg_c);
> +               reg &= ~((cfg->rate_div_mask) << (cfg->rate_div_shift));
> +               reg_c &= ~((cfg->rate_mul_mask_c) << (cfg->rate_mul_shift_c));
> +               reg_c |= BIT(cfg->rate_write_enable_bit_c);
> +
> +               reg_c |= (mul & cfg->rate_mul_mask_c) << (cfg->rate_mul_shift_c);
> +               reg |= (div & cfg->rate_div_mask) << (cfg->rate_div_shift);
> +
> +               writel(reg_c, cfg->rate_reg_c);
> +       }
> +       writel(reg, cfg->rate_reg);
> +
> +       return 0;
> +}
> +
> +static const struct clk_ops k230_clk_ops_arr[K230_CLK_OPS_ID_NUM] = {
> +       [K230_CLK_OPS_ID_NONE] = {
> +               /* Sentinel */
> +       },
> +       [K230_CLK_OPS_ID_GATE_ONLY] = {
> +               K230_CLK_OPS_GATE,
> +       },
> +       [K230_CLK_OPS_ID_RATE_ONLY] = {
> +               K230_CLK_OPS_RATE,
> +       },
> +       [K230_CLK_OPS_ID_RATE_GATE] = {
> +               K230_CLK_OPS_RATE,
> +               K230_CLK_OPS_GATE,
> +       },
> +       [K230_CLK_OPS_ID_MUX_ONLY] = {
> +               K230_CLK_OPS_MUX,
> +       },
> +       [K230_CLK_OPS_ID_MUX_GATE] = {
> +               K230_CLK_OPS_MUX,
> +               K230_CLK_OPS_GATE,
> +       },
> +       [K230_CLK_OPS_ID_MUX_RATE] = {
> +               K230_CLK_OPS_MUX,
> +               K230_CLK_OPS_RATE,
> +       },
> +       [K230_CLK_OPS_ID_ALL] = {
> +               K230_CLK_OPS_MUX,
> +               K230_CLK_OPS_RATE,
> +               K230_CLK_OPS_GATE,
> +       },
> +};
> +
> +static int k230_register_clk(struct platform_device *pdev,
> +                            struct k230_sysclk *ksc,
> +                            int id,
> +                            const struct clk_parent_data *parent_data,
> +                            u8 num_parents,
> +                            unsigned long flags)
> +{
> +       struct k230_clk *clk = &ksc->clks[id];
> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[id];
> +       struct clk_init_data init = {};
> +       int clk_id = 0;
> +       int ret;
> +
> +       if (cfg->have_rate) {
> +               cfg->rate_reg = (ksc->regs + cfg->rate_reg_off);

Drop useless parenthesis please.

> +               clk_id += K230_CLK_OPS_ID_RATE_ONLY;
> +       }
> +
> +       if (cfg->have_mux) {
> +               cfg->mux_reg = (ksc->regs + cfg->mux_reg_off);
> +               clk_id += K230_CLK_OPS_ID_MUX_ONLY;
> +
> +               /* mux clock doesn't match the case that num_parents less than 2 */
> +               if (num_parents < 2)
> +                       return -EINVAL;
> +       }
> +
> +       if (cfg->have_gate) {
> +               cfg->gate_reg = (ksc->regs + cfg->gate_reg_off);

Drop useless parenthesis please.

> +               clk_id += K230_CLK_OPS_ID_GATE_ONLY;
> +       }
> +
> +       if (cfg->have_rate_c)
> +               cfg->rate_reg_c = (ksc->regs + cfg->rate_reg_off_c);

Drop useless parenthesis please.

> +
> +       init.name = k230_clk_cfgs[id].name;
> +       init.flags = flags;
> +       init.parent_data = parent_data;
> +       init.num_parents = num_parents;
> +       init.ops = &k230_clk_ops_arr[clk_id];
> +
> +       clk->id = id;
> +       clk->ksc = ksc;
> +       clk->hw.init = &init;
> +
> +       ret = devm_clk_hw_register(&pdev->dev, &clk->hw);
> +       if (ret)
> +               return dev_err_probe(&pdev->dev,
> +                                    ret,
> +                                    "register clock %s failed\n",
> +                                    k230_clk_cfgs[id].name);
> +
> +       return 0;
> +}
> +
> +static int k230_register_mux_clk(struct platform_device *pdev,
> +                                struct k230_sysclk *ksc,
> +                                struct clk_parent_data *parent_data,
> +                                int num_parent,
> +                                int id)
> +{
> +       return k230_register_clk(pdev, ksc, id,
> +                               (const struct clk_parent_data *)parent_data,

Please remove useless cast.

> +                               num_parent, 0);
> +}
> +
> +static int k230_register_osc24m_child(struct platform_device *pdev,
> +                                     struct k230_sysclk *ksc,
> +                                     int id)
> +{
> +       const struct clk_parent_data parent_data = {
> +               .index = 0,
> +       };
> +       return k230_register_clk(pdev, ksc, id, &parent_data, 1, 0);
> +}
> +
> +static int k230_register_pll_child(struct platform_device *pdev,
> +                                  struct k230_sysclk *ksc,
> +                                  int id,
> +                                  enum k230_pll_id pll_id,
> +                                  unsigned long flags)
> +{
> +       const struct clk_parent_data parent_data = {
> +               .hw = &ksc->plls[pll_id].hw,
> +       };
> +       return k230_register_clk(pdev, ksc, id, &parent_data, 1, flags);
> +}
> +
> +static int k230_register_pll_div_child(struct platform_device *pdev,
> +                                      struct k230_sysclk *ksc,
> +                                      int id,
> +                                      enum k230_pll_div_id pll_div_id,
> +                                      unsigned long flags)
> +{
> +       const struct clk_parent_data parent_data = {
> +               .hw = ksc->dclks[pll_div_id].hw,
> +       };
> +       return k230_register_clk(pdev, ksc, id, &parent_data, 1, flags);
> +}
> +
> +static int k230_register_clk_child(struct platform_device *pdev,
> +                                  struct k230_sysclk *ksc,
> +                                  int id,
> +                                  int parent_id)
> +{
> +       const struct clk_parent_data parent_data = {
> +               .hw = &ksc->clks[parent_id].hw,
> +       };
> +       return k230_register_clk(pdev, ksc, id, &parent_data, 1, 0);
> +}
> +
> +static int _k230_clk_mux_get_parent_data(struct k230_sysclk *ksc,
> +                                        struct k230_clk_parent *pclk,
> +                                        struct clk_parent_data *parent_data)
> +{
> +       switch (pclk->type) {
> +       case K230_OSC24M:
> +               parent_data->index = 0;
> +               break;
> +       case K230_PLL:
> +               parent_data->hw = &ksc->plls[pclk->pll_id].hw;
> +               break;
> +       case K230_PLL_DIV:
> +               parent_data->hw = ksc->dclks[pclk->pll_div_id].hw;
> +               break;
> +       case K230_CLK_COMPOSITE:
> +               parent_data->hw = &ksc->clks[pclk->clk_id].hw;
> +               break;
> +       default:
> +               WARN_ON_ONCE(true);
> +               return -EPERM;
> +       }
> +
> +       return 0;
> +}
> +
> +static int k230_clk_mux_get_parent_data(struct k230_sysclk *ksc,
> +                                       struct k230_clk_cfg *cfg,
> +                                       struct clk_parent_data *parent_data,
> +                                       int num_parent)
> +{
> +       int ret;
> +       struct k230_clk_parent *pclk = cfg->parent;
> +
> +       for (int i = 0; i < num_parent; i++) {
> +               ret = _k230_clk_mux_get_parent_data(ksc, &pclk[i], &parent_data[i]);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static int k230_register_clks(struct platform_device *pdev, struct k230_sysclk *ksc)
> +{
> +       struct k230_clk_cfg *cfg;
> +       struct k230_clk_parent *pclk;
> +       struct clk_parent_data parent_data[K230_CLK_MAX_PARENT_NUM];
> +       int ret, i;
> +
> +       /*
> +        *  Single parent clock:
> +        *  pll0_div2 sons: cpu0_src
> +        *  pll0_div4 sons: cpu0_pclk
> +        *  cpu0_src sons: cpu0_aclk, cpu0_plic, cpu0_noc_ddrcp4, pmu_pclk
> +        *
> +        *  Mux clock:
> +        *  hs_ospi_src parents: pll0_div2, pll2_div4
> +        */
> +       for (i = 0; i < K230_CLK_NUM; i++) {
> +               cfg = &k230_clk_cfgs[i];
> +               if (!cfg->status)
> +                       continue;
> +
> +               if (cfg->have_mux) {
> +                       ret = k230_clk_mux_get_parent_data(ksc, cfg, parent_data,
> +                                                          cfg->num_parent);
> +                       if (ret)
> +                               return dev_err_probe(&pdev->dev, ret,
> +                                                    "Failed to get parent data\n");
> +
> +                       ret = k230_register_mux_clk(pdev, ksc, parent_data,
> +                                                   cfg->num_parent, i);
> +               } else {
> +                       pclk = cfg->parent;
> +                       switch (pclk->type) {
> +                       case K230_OSC24M:
> +                               ret = k230_register_osc24m_child(pdev, ksc, i);
> +                               break;
> +                       case K230_PLL:
> +                               ret = k230_register_pll_child(pdev, ksc, i,
> +                                                             pclk->pll_id, cfg->flags);
> +                               break;
> +                       case K230_PLL_DIV:
> +                               ret = k230_register_pll_div_child(pdev, ksc, i,
> +                                                                 pclk->pll_div_id, cfg->flags);
> +                               break;
> +                       case K230_CLK_COMPOSITE:
> +                               ret = k230_register_clk_child(pdev, ksc, i,
> +                                                             pclk->clk_id);
> +                               break;
> +                       default:
> +                               return dev_err_probe(&pdev->dev, -EINVAL, "Invalid type\n");
> +                       }
> +               }
> +               if (ret)
> +                       return dev_err_probe(&pdev->dev, ret, "register child id %d failed\n", i);
> +       }
> +
> +       return 0;
> +}
> +
> +static struct clk_hw *k230_clk_hw_onecell_get(struct of_phandle_args *clkspec, void *data)
> +{
> +       struct k230_sysclk *ksc;
> +       unsigned int idx;
> +
> +       if (clkspec->args_count != 1)
> +               return ERR_PTR(-EINVAL);
> +
> +       idx = clkspec->args[0];
> +       if (idx >= K230_CLK_NUM)
> +               return ERR_PTR(-EINVAL);
> +
> +       if (!data)
> +               return ERR_PTR(-EINVAL);
> +
> +       ksc = (struct k230_sysclk *)data;

Drop useless cast please.

> +
> +       return &ksc->clks[idx].hw;
> +}
> +
> +static int k230_clk_init_plls(struct platform_device *pdev)
> +{
> +       int ret;
> +       struct k230_sysclk *ksc = platform_get_drvdata(pdev);
> +
> +       spin_lock_init(&ksc->pll_lock);
> +
> +       ksc->pll_regs = devm_platform_ioremap_resource(pdev, 0);
> +       if (!ksc->pll_regs)

Does it return an error pointer or NULL on failure? The dev_err_probe()
below is incorrect or the if condition above is.

> +               return dev_err_probe(&pdev->dev, PTR_ERR(ksc->pll_regs), "map registers failed\n");

Doesn't this already print an error?

> +
> +       ret = k230_register_plls(pdev, ksc);
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret, "register plls failed\n");
> +
> +       ret = k230_register_pll_divs(pdev, ksc);
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret, "register pll_divs failed\n");
> +
> +       for (int i = 0; i < K230_PLL_DIV_NUM; i++) {
> +               ret = devm_clk_hw_register_clkdev(&pdev->dev, ksc->dclks[i].hw,
> +                                                 k230_pll_div_cfgs[i].name, NULL);

Why is clkdev being used?

> +               if (ret)
> +                       return dev_err_probe(&pdev->dev, ret, "clock_lookup create failed\n");
> +       }
> +
> +       return 0;
> +}
> +
> +static int k230_clk_init_clks(struct platform_device *pdev)
> +{
> +       int ret;
> +       struct k230_sysclk *ksc = platform_get_drvdata(pdev);
> +
> +       spin_lock_init(&ksc->clk_lock);
> +
> +       ksc->regs = devm_platform_ioremap_resource(pdev, 1);
> +       if (!ksc->regs)
> +               return dev_err_probe(&pdev->dev, PTR_ERR(ksc->regs), "failed to map registers\n");
> +
> +       ret = k230_register_clks(pdev, ksc);
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret, "register clock provider failed\n");
> +
> +       ret = devm_of_clk_add_hw_provider(&pdev->dev, k230_clk_hw_onecell_get, ksc);
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret, "add clock provider failed\n");
> +
> +       return 0;
> +}
> +
> +static int k230_clk_probe(struct platform_device *pdev)
> +{
> +       int ret;
> +       struct k230_sysclk *ksc;
> +
> +       ksc = devm_kzalloc(&pdev->dev, sizeof(struct k230_sysclk), GFP_KERNEL);
> +       if (!ksc)
> +               return -ENOMEM;
> +
> +       ksc->plls = devm_kmalloc_array(&pdev->dev, K230_PLL_NUM,

Any reason devm_kcalloc() isn't used?

> +                                      sizeof(struct k230_pll), GFP_KERNEL);
> +       if (!ksc->plls)
> +               return -ENOMEM;
> +
> +       ksc->dclks = devm_kmalloc_array(&pdev->dev, K230_PLL_DIV_NUM,
> +                                       sizeof(struct k230_pll_div), GFP_KERNEL);
> +       if (!ksc->dclks)
> +               return -ENOMEM;
> +
> +       ksc->clks = devm_kmalloc_array(&pdev->dev, K230_CLK_NUM,
> +                                      sizeof(struct k230_clk), GFP_KERNEL);
> +       if (!ksc->clks)
> +               return -ENOMEM;
> +
> +       ksc->pdev = pdev;
> +       platform_set_drvdata(pdev, ksc);
> +
> +       ret = k230_clk_init_plls(pdev);
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret, "init plls failed\n");
> +
> +       ret = k230_clk_init_clks(pdev);
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret, "init clks failed\n");
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id k230_clk_ids[] = {
> +       { .compatible = "canaan,k230-clk" },
> +       { /* Sentinel */ },

Nitpick: Please drop last comma so that nothing can come after without
causing a compilation error.

> +};
> +MODULE_DEVICE_TABLE(of, k230_clk_ids);

Re: [PATCH v4 2/3] clk: canaan: Add clock driver for Canaan K230

[Xukai Wang]

On 2025/2/19 05:48, Stephen Boyd wrote:
> Quoting Xukai Wang (2025-02-17 06:45:17)
>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
>> index 299bc678ed1b9fcd9110bb8c5937a1bd1ea60e23..f63355ab8adeeee90d29d1a975607f5dc0a58bd6 100644
>> --- a/drivers/clk/Kconfig
>> +++ b/drivers/clk/Kconfig
>> @@ -464,6 +464,12 @@ config COMMON_CLK_K210
>>         help
>>           Support for the Canaan Kendryte K210 RISC-V SoC clocks.
>>  
>> +config COMMON_CLK_K230
>> +       bool "Clock driver for the Canaan Kendryte K230 SoC"
>> +       depends on OF && (ARCH_CANAAN || COMPILE_TEST)
> It doesn't depend on OF to build from what I can tell. This should
> simply be
>
> 	depends on ARCH_CANAAN || COMPILE_TEST
>
> right?
right, it doesn't depend on OF.
>
>> +        help
>> +          Support for the Canaan Kendryte K230 RISC-V SoC clocks.
>> +
>>  config COMMON_CLK_SP7021
>>         tristate "Clock driver for Sunplus SP7021 SoC"
>>         depends on SOC_SP7021 || COMPILE_TEST
>> diff --git a/drivers/clk/clk-k230.c b/drivers/clk/clk-k230.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..017ecad9ec47ed751f75fe5197aa119ae35e05f9
>> --- /dev/null
>> +++ b/drivers/clk/clk-k230.c
>> @@ -0,0 +1,1347 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Kendryte Canaan K230 Clock Drivers
>> + *
>> + * Author: Xukai Wang <kingxukai@zohomail.com>
>> + * Author: Troy Mitchell <troymitchell988@gmail.com>
>> + */
>> +
>> +#include <dt-bindings/clock/canaan,k230-clk.h>
> Please put dt-bindigns at the end of the include list.
OK, thanks for reminder.
>> +#include <linux/clk.h>
> Is this include used? Ideally a clk provider (clk-provider.h) isn't also
> a clk consumer (clk.h).
Here I use `clk_prepare_enable()` in `k230_pll_init()` to enable the
software if the hardware is automatically enabled.
>> +#include <linux/clkdev.h>
> Hopefully clkdev isn't used.
I use devm_clk_hw_register_clkdev() below and am explained why it was
used. Please check.
>> +#include <linux/clk-provider.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/module.h>
> Is this include used?
Apologies, I'll drop it.
>> +#include <linux/platform_device.h>
>> +#include <linux/spinlock.h>
> Need to include mod_devicetable.h
Yes, it should be.
>> +
>> +/* PLL control register bits. */
>> +#define K230_PLL_BYPASS_ENABLE                         BIT(19)
>> +#define K230_PLL_GATE_ENABLE                           BIT(2)
>> +#define K230_PLL_GATE_WRITE_ENABLE                     BIT(18)
>> +#define K230_PLL_OD_SHIFT                              24
>> +#define K230_PLL_OD_MASK                               0xF
>> +#define K230_PLL_R_SHIFT                               16
>> +#define K230_PLL_R_MASK                                        0x3F
>> +#define K230_PLL_F_SHIFT                               0
>> +#define K230_PLL_F_MASK                                        0x1FFFF
>> +#define K230_PLL0_OFFSET_BASE                          0x00
>> +#define K230_PLL1_OFFSET_BASE                          0x10
>> +#define K230_PLL2_OFFSET_BASE                          0x20
>> +#define K230_PLL3_OFFSET_BASE                          0x30
>> +#define K230_PLL_DIV_REG_OFFSET                                0x00
>> +#define K230_PLL_BYPASS_REG_OFFSET                     0x04
>> +#define K230_PLL_GATE_REG_OFFSET                       0x08
>> +#define K230_PLL_LOCK_REG_OFFSET                       0x0C
>> +
>> +/* PLL lock register bits.  */
>> +#define K230_PLL_STATUS_MASK                           BIT(0)
>> +
>> +/* K230 CLK registers offset */
>> +#define K230_CLK_AUDIO_CLKDIV_OFFSET                   0x34
>> +#define K230_CLK_PDM_CLKDIV_OFFSET                     0x40
>> +#define K230_CLK_CODEC_ADC_MCLKDIV_OFFSET              0x38
>> +#define K230_CLK_CODEC_DAC_MCLKDIV_OFFSET              0x3c
>> +
>> +/* K230 CLK OPS. */
>> +#define K230_CLK_OPS_GATE                              \
>> +       .enable         = k230_clk_enable,              \
>> +       .disable        = k230_clk_disable,             \
>> +       .is_enabled     = k230_clk_is_enabled
>> +
>> +#define K230_CLK_OPS_RATE                              \
>> +       .set_rate       = k230_clk_set_rate,            \
>> +       .round_rate     = k230_clk_round_rate,          \
>> +       .recalc_rate    = k230_clk_get_rate
>> +
>> +#define K230_CLK_OPS_MUX                               \
>> +       .set_parent     = k230_clk_set_parent,          \
>> +       .get_parent     = k230_clk_get_parent,          \
>> +       .determine_rate = clk_hw_determine_rate_no_reparent
>> +
>> +#define K230_CLK_OPS_ID_NONE                           0
>> +#define K230_CLK_OPS_ID_GATE_ONLY                      1
>> +#define K230_CLK_OPS_ID_RATE_ONLY                      2
>> +#define K230_CLK_OPS_ID_RATE_GATE                      3
>> +#define K230_CLK_OPS_ID_MUX_ONLY                       4
>> +#define K230_CLK_OPS_ID_MUX_GATE                       5
>> +#define K230_CLK_OPS_ID_MUX_RATE                       6
>> +#define K230_CLK_OPS_ID_ALL                            7
>> +#define K230_CLK_OPS_ID_NUM                            8
>> +
>> +/* K230 CLK MACROS */
>> +#define K230_CLK_MAX_PARENT_NUM                                6
>> +
>> +#define K230_GATE_FORMAT(_reg, _bit, _reverse, _have_gate)                     \
>> +       .gate_reg_off = (_reg),                                                 \
>> +       .gate_bit_enable = (_bit),                                              \
>> +       .gate_bit_reverse = (_reverse),                                         \
>> +       .have_gate = (_have_gate)
>> +
>> +#define K230_RATE_FORMAT(_mul_min, _mul_max, _mul_shift, _mul_mask,            \
>> +                       _div_min, _div_max, _div_shift, _div_mask,              \
>> +                       _reg, _bit, _method, _reg_c, _bit_c,                    \
>> +                       _mul_min_c, _mul_max_c, _mul_shift_c, _mul_mask_c,      \
>> +                       _have_rate, _have_rate_c)                               \
>> +       .rate_mul_min = (_mul_min),                                             \
>> +       .rate_mul_max = (_mul_max),                                             \
>> +       .rate_mul_shift = (_mul_shift),                                         \
>> +       .rate_mul_mask = (_mul_mask),                                           \
>> +       .rate_mul_min_c = (_mul_min_c),                                         \
>> +       .rate_mul_max_c = (_mul_max_c),                                         \
>> +       .rate_mul_shift_c = (_mul_shift_c),                                     \
>> +       .rate_mul_mask_c = (_mul_mask_c),                                       \
>> +       .rate_div_min = (_div_min),                                             \
>> +       .rate_div_max = (_div_max),                                             \
>> +       .rate_div_shift = (_div_shift),                                         \
>> +       .rate_div_mask = (_div_mask),                                           \
>> +       .rate_reg_off = (_reg),                                                 \
>> +       .rate_reg_off_c = (_reg_c),                                             \
>> +       .rate_write_enable_bit = (_bit),                                        \
>> +       .rate_write_enable_bit_c = (_bit_c),                                    \
>> +       .method = (_method),                                                    \
>> +       .have_rate = (_have_rate),                                              \
>> +       .have_rate_c = (_have_rate_c)
>> +
>> +#define K230_MUX_FORMAT(_reg, _shift, _mask, _have_mux)                                \
>> +       .mux_reg_off = (_reg),                                                  \
>> +       .mux_reg_shift = (_shift),                                              \
>> +       .mux_reg_mask = (_mask),                                                \
>> +       .have_mux = (_have_mux)
>> +
>> +#define K230_GATE_FORMAT_ZERO  K230_GATE_FORMAT(0, 0, 0, false)
>> +#define K230_RATE_FORMAT_ZERO  K230_RATE_FORMAT(0, 0, 0, 0, 0, 0,              \
>> +                                               0, 0, 0, 0, 0, 0,               \
>> +                                               0, 0, 0, 0, 0, false, false)
>> +#define K230_MUX_FORMAT_ZERO   K230_MUX_FORMAT(0, 0, 0, false)
>> +
>> +struct k230_sysclk;
>> +
>> +/* K230 PLLs. */
>> +enum k230_pll_id {
>> +       K230_PLL0, K230_PLL1, K230_PLL2, K230_PLL3, K230_PLL_NUM
>> +};
>> +
>> +struct k230_pll {
>> +       enum k230_pll_id id;
>> +       struct k230_sysclk *ksc;
>> +       void __iomem *div, *bypass, *gate, *lock;
>> +       struct clk_hw hw;
>> +};
>> +
>> +#define to_k230_pll(_hw)       container_of(_hw, struct k230_pll, hw)
>> +
>> +struct k230_pll_cfg {
>> +       u32 reg;
>> +       enum k230_pll_id pll_id;
>> +       const char *name;
>> +};
>> +
>> +/* K230 PLL_DIVS. */
>> +struct k230_pll_div {
>> +       struct k230_sysclk *ksc;
>> +       struct clk_hw *hw;
>> +};
>> +
>> +struct k230_pll_div_cfg {
>> +       const char *parent_name, *name;
>> +       int div;
>> +};
>> +
>> +enum k230_pll_div_id {
>> +       K230_PLL0_DIV2,
>> +       K230_PLL0_DIV3,
>> +       K230_PLL0_DIV4,
>> +       K230_PLL1_DIV2,
>> +       K230_PLL1_DIV3,
>> +       K230_PLL1_DIV4,
>> +       K230_PLL2_DIV2,
>> +       K230_PLL2_DIV3,
>> +       K230_PLL2_DIV4,
>> +       K230_PLL3_DIV2,
>> +       K230_PLL3_DIV3,
>> +       K230_PLL3_DIV4,
>> +       K230_PLL_DIV_NUM
>> +};
>> +
>> +enum k230_clk_div_type {
>> +       K230_MUL,
>> +       K230_DIV,
>> +       K230_MUL_DIV,
>> +};
>> +
>> +/* K230 CLKS. */
>> +struct k230_clk {
>> +       int id;
>> +       struct k230_sysclk *ksc;
>> +       struct clk_hw hw;
>> +};
>> +
>> +#define to_k230_clk(_hw)       container_of(_hw, struct k230_clk, hw)
>> +
>> +enum k230_clk_parent_type {
>> +       K230_OSC24M,
>> +       K230_PLL,
>> +       K230_PLL_DIV,
>> +       K230_CLK_COMPOSITE,
>> +};
>> +
>> +struct k230_clk_parent {
>> +       enum k230_clk_parent_type type;
>> +       union {
>> +               enum k230_pll_div_id pll_div_id;
>> +               enum k230_pll_id pll_id;
>> +               int clk_id;
>> +       };
>> +};
>> +
>> +struct k230_clk_cfg {
>> +       /* attr */
>> +       const char *name;
>> +       /* 0-read & write; 1-read only */
>> +       bool read_only;
>> +       int num_parent;
>> +       struct k230_clk_parent parent[K230_CLK_MAX_PARENT_NUM];
>> +       bool status;
>> +       int flags;
>> +
>> +       /* rate reg */
>> +       u32 rate_reg_off;
>> +       u32 rate_reg_off_c;
>> +       void __iomem *rate_reg;
>> +       void __iomem *rate_reg_c;
>> +       /* rate info*/
>> +       u32 rate_write_enable_bit;
>> +       u32 rate_write_enable_bit_c;
>> +       enum k230_clk_div_type method;
>> +       bool have_rate;
>> +       bool have_rate_c;
>> +       /* rate mul */
>> +       u32 rate_mul_min;
>> +       u32 rate_mul_max;
>> +       u32 rate_mul_shift;
>> +       u32 rate_mul_mask;
>> +       /* rate mul-changable */
>> +       u32 rate_mul_min_c;
>> +       u32 rate_mul_max_c;
>> +       u32 rate_mul_shift_c;
>> +       u32 rate_mul_mask_c;
>> +       /* rate div */
>> +       u32 rate_div_min;
>> +       u32 rate_div_max;
>> +       u32 rate_div_shift;
>> +       u32 rate_div_mask;
>> +
>> +       /* gate reg */
>> +       u32 gate_reg_off;
>> +       void __iomem *gate_reg;
>> +       /* gate info*/
>> +       bool have_gate;
>> +       u32 gate_bit_enable;
>> +       u32 gate_bit_reverse;
>> +
>> +       /* mux reg */
>> +       u32 mux_reg_off;
>> +       void __iomem *mux_reg;
>> +       /* mux info */
>> +       bool have_mux;
>> +       u32 mux_reg_shift;
>> +       u32 mux_reg_mask;
>> +};
>> +
>> +/* K230 SYSCLK. */
>> +struct k230_sysclk {
>> +       struct platform_device *pdev;
>> +       void __iomem           *pll_regs, *regs;
>> +       spinlock_t             pll_lock, clk_lock;
>> +       struct k230_pll        *plls;
>> +       struct k230_clk        *clks;
>> +       struct k230_pll_div    *dclks;
>> +};
>> +
>> +static const struct k230_pll_cfg k230_pll_cfgs[] = {
>> +       [K230_PLL0] = {
>> +               .reg = K230_PLL0_OFFSET_BASE,
>> +               .pll_id = K230_PLL0,
>> +               .name = "pll0",
>> +       },
>> +       [K230_PLL1] = {
>> +               .reg = K230_PLL1_OFFSET_BASE,
>> +               .pll_id = K230_PLL1,
>> +               .name = "pll1",
>> +       },
>> +       [K230_PLL2] = {
>> +               .reg = K230_PLL2_OFFSET_BASE,
>> +               .pll_id = K230_PLL2,
>> +               .name = "pll2",
>> +       },
>> +       [K230_PLL3] = {
>> +               .reg = K230_PLL3_OFFSET_BASE,
>> +               .pll_id = K230_PLL3,
>> +               .name = "pll3",
>> +       },
>> +};
>> +
>> +static const struct k230_pll_div_cfg k230_pll_div_cfgs[] = {
>> +       [K230_PLL0_DIV2] = { "pll0", "pll0_div2", 2},
>> +       [K230_PLL0_DIV3] = { "pll0", "pll0_div3", 3},
>> +       [K230_PLL0_DIV4] = { "pll0", "pll0_div4", 4},
>> +       [K230_PLL1_DIV2] = { "pll1", "pll1_div2", 2},
>> +       [K230_PLL1_DIV3] = { "pll1", "pll1_div3", 3},
>> +       [K230_PLL1_DIV4] = { "pll1", "pll1_div4", 4},
>> +       [K230_PLL2_DIV2] = { "pll2", "pll2_div2", 2},
>> +       [K230_PLL2_DIV3] = { "pll2", "pll2_div3", 3},
>> +       [K230_PLL2_DIV4] = { "pll2", "pll2_div4", 4},
>> +       [K230_PLL3_DIV2] = { "pll3", "pll3_div2", 2},
>> +       [K230_PLL3_DIV3] = { "pll3", "pll3_div3", 3},
>> +       [K230_PLL3_DIV4] = { "pll3", "pll3_div4", 4},
>> +};
>> +
>> +static struct k230_clk_cfg k230_clk_cfgs[] = {
>> +       [K230_CPU0_SRC] = {
> Can these be named variables instead of array elements? Then we can map
> quickly to parents instead of going through an intermediate enum.

Yes, using named variables instead of array elements is a quick approach
to map parents.

However, I wonder how to access similar struct members without redundant
code. Would it be appropriate to use `struct list_head` to simplify
looping and access these members more efficiently?

>> +               .name = "cpu0_src",
>> +               .read_only = false,
>> +               .flags = 0,
>> +               .status = true,
>> +               .num_parent = 1,
>> +               .parent[0] = {
>> +                       .type = K230_PLL_DIV,
>> +                       .pll_div_id = K230_PLL0_DIV2,
>> +               },
>> +               K230_RATE_FORMAT(1, 16, 0, 0,
>> +                                16, 16, 1, 0xF,
>> +                                0x0, 31, K230_MUL, 0, 0,
>> +                                0, 0, 0, 0,
>> +                                true, false),
>> +               K230_GATE_FORMAT(0, 0, 0, true),
>> +               K230_MUX_FORMAT_ZERO,
>> +       },
>> +       [K230_CPU0_ACLK] = {
>> +               .name = "cpu0_aclk",
>> +               .read_only = false,
>> +               .flags = 0,
>> +               .status = true,
>> +               .num_parent = 1,
>> +               .parent[0] = {
>> +                       .type = K230_CLK_COMPOSITE,
>> +                       .clk_id = K230_CPU0_SRC,
>> +               },
>> +               K230_RATE_FORMAT(1, 1, 0, 0,
>> +                                1, 8, 7, 0x7,
>> +                                0x0, 31, K230_MUL, 0, 0,
>> +                                0, 0, 0, 0,
>> +                                true, false),
>> +               K230_GATE_FORMAT_ZERO,
>> +               K230_MUX_FORMAT_ZERO,
>> +       },
>> +       [K230_CPU0_PLIC] = {
>> +               .name = "cpu0_plic",
>> +               .read_only = false,
>> +               .flags = 0,
>> +               .status = true,
>> +               .num_parent = 1,
>> +               .parent[0] = {
>> +                       .type = K230_CLK_COMPOSITE,
>> +                       .clk_id = K230_CPU0_SRC,
>> +               },
>> +               K230_RATE_FORMAT(1, 1, 0, 0,
>> +                                1, 8, 10, 0x7,
>> +                                0x0, 31, K230_DIV, 0, 0,
>> +                                0, 0, 0, 0,
>> +                                true, false),
>> +               K230_GATE_FORMAT(0, 9, 0, true),
>> +               K230_MUX_FORMAT_ZERO,
>> +       },
>> +       [K230_CPU0_NOC_DDRCP4] = {
>> +               .name = "cpu0_noc_ddrcp4",
>> +               .read_only = false,
>> +               .flags = 0,
>> +               .status = true,
>> +               .num_parent = 1,
>> +               .parent[0] = {
>> +                       .type = K230_CLK_COMPOSITE,
>> +                       .clk_id = K230_CPU0_SRC,
>> +               },
>> +               K230_RATE_FORMAT_ZERO,
>> +               K230_GATE_FORMAT(0x60, 7, 0, true),
>> +               K230_MUX_FORMAT_ZERO,
>> +       },
>> +       [K230_CPU0_PCLK] = {
>> +               .name = "cpu0_pclk",
>> +               .read_only = false,
>> +               .flags = 0,
>> +               .status = true,
>> +               .num_parent = 1,
>> +               .parent[0] = {
>> +                       .type = K230_PLL_DIV,
>> +                       .pll_div_id = K230_PLL0_DIV4,
>> +               },
>> +               K230_RATE_FORMAT(1, 1, 0, 0,
>> +                                1, 8, 15, 0x7,
>> +                                0x0, 31, K230_DIV, 0, 0,
>> +                                0, 0, 0, 0,
>> +                                true, false),
>> +               K230_GATE_FORMAT(0, 13, 0, true),
>> +               K230_MUX_FORMAT_ZERO,
>> +       },
>> +       [K230_PMU_PCLK] = {
>> +               .name = "pmu_pclk",
>> +               .read_only = false,
>> +               .flags = 0,
>> +               .status = true,
>> +               .num_parent = 1,
>> +               .parent[0] = {
>> +                       .type = K230_OSC24M,
>> +               },
>> +               K230_RATE_FORMAT_ZERO,
>> +               K230_GATE_FORMAT(0x10, 0, 0, true),
>> +               K230_MUX_FORMAT_ZERO,
>> +       },
>> +       [K230_HS_OSPI_SRC] = {
>> +               .name = "hs_ospi_src",
>> +               .read_only = false,
>> +               .flags = 0,
>> +               .status = true,
>> +               .num_parent = 2,
>> +               .parent[0] = {
>> +                       .type = K230_PLL_DIV,
>> +                       .pll_div_id = K230_PLL0_DIV2,
>> +               },
>> +               .parent[1] = {
>> +                       .type = K230_PLL_DIV,
>> +                       .pll_div_id = K230_PLL2_DIV4,
>> +               },
>> +               K230_RATE_FORMAT_ZERO,
>> +               K230_GATE_FORMAT(0x18, 24, 0, true),
>> +               K230_MUX_FORMAT(0x20, 18, 0x1, true),
>> +       },
>> +};
>> +
>> +#define K230_CLK_NUM   ARRAY_SIZE(k230_clk_cfgs)
>> +
>> +static void k230_init_pll(void __iomem *regs, enum k230_pll_id pll_id,
>> +                         struct k230_pll *pll)
>> +{
>> +       void __iomem *base;
>> +
>> +       pll->id = pll_id;
>> +       base = regs + k230_pll_cfgs[pll_id].reg;
>> +       pll->div = base + K230_PLL_DIV_REG_OFFSET;
>> +       pll->bypass = base + K230_PLL_BYPASS_REG_OFFSET;
>> +       pll->gate = base + K230_PLL_GATE_REG_OFFSET;
>> +       pll->lock = base + K230_PLL_LOCK_REG_OFFSET;
>> +}
>> +
>> +static int k230_pll_prepare(struct clk_hw *hw)
>> +{
>> +       struct k230_pll *pll = to_k230_pll(hw);
>> +       u32 reg;
>> +
>> +       /* wait for PLL lock until it reachs lock status */
> s/reachs/reaches/
Apologies, I'll modify it.
>> +       return readl_poll_timeout(pll->lock, reg,
>> +                                 (reg & K230_PLL_STATUS_MASK) == K230_PLL_STATUS_MASK,
>> +                                 400, 0);
>> +}
>> +
>> +static bool k230_pll_hw_is_enabled(struct k230_pll *pll)
>> +{
>> +       return (readl(pll->gate) & K230_PLL_GATE_ENABLE) == K230_PLL_GATE_ENABLE;
>> +}
>> +
>> +static void k230_pll_enable_hw(void __iomem *regs, struct k230_pll *pll)
>> +{
>> +       u32 reg;
>> +
>> +       if (k230_pll_hw_is_enabled(pll))
>> +               return;
>> +
>> +       /* Set PLL factors */
>> +       reg = readl(pll->gate);
>> +       reg |= (K230_PLL_GATE_ENABLE | K230_PLL_GATE_WRITE_ENABLE);
>> +       writel(reg, pll->gate);
>> +}
>> +
>> +static int k230_pll_enable(struct clk_hw *hw)
>> +{
>> +       struct k230_pll *pll = to_k230_pll(hw);
>> +       struct k230_sysclk *ksc = pll->ksc;
>> +
>> +       guard(spinlock)(&ksc->pll_lock);
>> +       k230_pll_enable_hw(ksc->regs, pll);
>> +
>> +       return 0;
>> +}
>> +
>> +static void k230_pll_disable(struct clk_hw *hw)
>> +{
>> +       struct k230_pll *pll = to_k230_pll(hw);
>> +       struct k230_sysclk *ksc = pll->ksc;
>> +       u32 reg;
>> +
>> +       guard(spinlock)(&ksc->pll_lock);
>> +       reg = readl(pll->gate);
>> +
>> +       reg &= ~(K230_PLL_GATE_ENABLE);
>> +       reg |= (K230_PLL_GATE_WRITE_ENABLE);
>> +
>> +       writel(reg, pll->gate);
>> +}
>> +
>> +static int k230_pll_is_enabled(struct clk_hw *hw)
>> +{
>> +       return k230_pll_hw_is_enabled(to_k230_pll(hw));
>> +}
>> +
>> +static int k230_pll_init(struct clk_hw *hw)
>> +{
>> +       if (k230_pll_is_enabled(hw))
>> +               return clk_prepare_enable(hw->clk);
> Is this CLK_IS_CRITICAL?
I think it is not the case of CLK_IS_CRITICAL; it enables the software
if the hardware is automatically enabled.
>> +
>> +       return 0;
>> +}
>> +
>> +static unsigned long k230_pll_get_rate(struct clk_hw *hw, unsigned long parent_rate)
>> +{
>> +       struct k230_pll *pll = to_k230_pll(hw);
>> +       struct k230_sysclk *ksc = pll->ksc;
>> +       u32 reg;
>> +       u32 r, f, od;
>> +
>> +       reg = readl(pll->bypass);
>> +       if (reg & K230_PLL_BYPASS_ENABLE)
>> +               return parent_rate;
>> +
>> +       reg = readl(pll->lock);
>> +       if (!(reg & (K230_PLL_STATUS_MASK))) { /* unlocked */
>> +               dev_err(&ksc->pdev->dev, "%s is unlock.\n", clk_hw_get_name(hw));
>> +               return 0;
>> +       }
>> +
>> +       reg = readl(pll->div);
>> +       r = ((reg >> K230_PLL_R_SHIFT) & K230_PLL_R_MASK) + 1;
>> +       f = ((reg >> K230_PLL_F_SHIFT) & K230_PLL_F_MASK) + 1;
>> +       od = ((reg >> K230_PLL_OD_SHIFT) & K230_PLL_OD_MASK) + 1;
>> +
>> +       return div_u64((u64)parent_rate * f, r * od);
> Is this mul_u64_u32_div()?

Thanks for reminders.

It can be replaced with `mul_u64_u32_div(parent_rate, f, r * od)`

>> +}
>> +
>> +static const struct clk_ops k230_pll_ops = {
>> +       .init           = k230_pll_init,
>> +       .prepare        = k230_pll_prepare,
>> +       .enable         = k230_pll_enable,
>> +       .disable        = k230_pll_disable,
>> +       .is_enabled     = k230_pll_is_enabled,
>> +       .recalc_rate    = k230_pll_get_rate,
>> +};
>> +
>> +static int k230_register_pll(struct platform_device *pdev,
>> +                            struct k230_sysclk *ksc,
>> +                            enum k230_pll_id pll_id,
>> +                            const char *name,
>> +                            int num_parents,
>> +                            const struct clk_ops *ops)
>> +{
>> +       struct k230_pll *pll = &ksc->plls[pll_id];
>> +       struct clk_init_data init = {};
>> +       struct device *dev = &pdev->dev;
>> +       int ret;
>> +       const struct clk_parent_data parent_data[] = {
>> +               { .index = 0, },
>> +       };
>> +
>> +       init.name = name;
>> +       init.parent_data = parent_data;
>> +       init.num_parents = num_parents;
>> +       init.ops = ops;
>> +
>> +       pll->hw.init = &init;
>> +       pll->ksc = ksc;
>> +
>> +       ret = devm_clk_hw_register(dev, &pll->hw);
>> +       if (ret)
>> +               return dev_err_probe(dev, ret, "failed to register pll");
> Missing newline on printk.
OK, I get it.
>> +
>> +       return 0;
>> +}
>> +
>> +static int k230_register_plls(struct platform_device *pdev, struct k230_sysclk *ksc)
>> +{
>> +       int i, ret;
>> +       const struct k230_pll_cfg *cfg;
>> +
>> +       for (i = 0; i < K230_PLL_NUM; i++) {
>> +               cfg = &k230_pll_cfgs[i];
>> +
>> +               k230_init_pll(ksc->pll_regs, i, &ksc->plls[i]);
>> +
>> +               ret = k230_register_pll(pdev, ksc, cfg->pll_id, cfg->name, 1,
>> +                                       &k230_pll_ops);
>> +               if (ret)
>> +                       return dev_err_probe(&pdev->dev, ret,
>> +                                            "register %s failed\n", cfg->name);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int k230_register_pll_divs(struct platform_device *pdev, struct k230_sysclk *ksc)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct clk_hw *hw;
>> +
>> +       for (int i = 0; i < K230_PLL_DIV_NUM; i++) {
>> +               hw = devm_clk_hw_register_fixed_factor(dev, k230_pll_div_cfgs[i].name,
>> +                                                      k230_pll_div_cfgs[i].parent_name,
>> +                                                      0, 1, k230_pll_div_cfgs[i].div);
>> +               if (IS_ERR(hw))
>> +                       return PTR_ERR(hw);
>> +               ksc->dclks[i].hw = hw;
>> +               ksc->dclks[i].ksc = ksc;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int k230_clk_enable(struct clk_hw *hw)
>> +{
>> +       struct k230_clk *clk = to_k230_clk(hw);
>> +       struct k230_sysclk *ksc = clk->ksc;
>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
>> +       u32 reg;
>> +
>> +       if (!cfg->have_gate)
>> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have gate\n");
>> +
>> +       guard(spinlock)(&ksc->clk_lock);
>> +       reg = readl(cfg->gate_reg);
>> +       if (cfg->gate_bit_reverse)
>> +               reg &= ~BIT(cfg->gate_bit_enable);
>> +       else
>> +               reg |= BIT(cfg->gate_bit_enable);
>> +       writel(reg, cfg->gate_reg);
>> +
>> +       return 0;
>> +}
>> +
>> +static void k230_clk_disable(struct clk_hw *hw)
>> +{
>> +       struct k230_clk *clk = to_k230_clk(hw);
>> +       struct k230_sysclk *ksc = clk->ksc;
>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
>> +       u32 reg;
>> +
>> +       if (!cfg->have_gate) {
>> +               dev_err(&ksc->pdev->dev, "This clock doesn't have gate\n");
> Why are the clk_ops assigned to this clk then?

If this clock doesn't have a gate, then the clk_ops will not be assigned to it. In this case, I'm simply checking it again. You can verify this in the k230_register_clk function to ensure it's handled correctly.

If you think this check is redundant, I can drop it.

>> +               return;
>> +       }
>> +
>> +       guard(spinlock)(&ksc->clk_lock);
>> +       reg = readl(cfg->gate_reg);
>> +
>> +       if (cfg->gate_bit_reverse)
>> +               reg |= BIT(cfg->gate_bit_enable);
>> +       else
>> +               reg &= ~BIT(cfg->gate_bit_enable);
>> +
>> +       writel(reg, cfg->gate_reg);
>> +}
>> +
>> +static int k230_clk_is_enabled(struct clk_hw *hw)
>> +{
>> +       struct k230_clk *clk = to_k230_clk(hw);
>> +       struct k230_sysclk *ksc = clk->ksc;
>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
>> +       u32 reg;
>> +
>> +       if (!cfg->have_gate)
>> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have gate\n");
> Why are the clk_ops assigned to this clk then? 

ditto.

> Don't use
> dev_err_probe() here.
OK, I'll modify it.
>> +
>> +       guard(spinlock)(&ksc->clk_lock);
>> +       reg = readl(cfg->gate_reg);
>> +
>> +       /* Check gate bit condition based on configuration and then set ret */
>> +       if (cfg->gate_bit_reverse)
>> +               return (BIT(cfg->gate_bit_enable) & reg) ? 1 : 0;
>> +       else
>> +               return (BIT(cfg->gate_bit_enable) & ~reg) ? 1 : 0;
>> +}
>> +
>> +static int k230_clk_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> +       struct k230_clk *clk = to_k230_clk(hw);
>> +       struct k230_sysclk *ksc = clk->ksc;
>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
>> +       u8 reg;
>> +
>> +       if (!cfg->have_mux)
>> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have mux\n");
> Why are the clk_ops assigned to this clk then? 
ditto.
> Don't use
> dev_err_probe() here.
OK. I'll modify it.
>> +
>> +       guard(spinlock)(&ksc->clk_lock);
>> +       reg = (cfg->mux_reg_mask & index) << cfg->mux_reg_shift;
>> +       writeb(reg, cfg->mux_reg);
>> +
>> +       return 0;
>> +}
>> +
>> +static u8 k230_clk_get_parent(struct clk_hw *hw)
>> +{
>> +       struct k230_clk *clk = to_k230_clk(hw);
>> +       struct k230_sysclk *ksc = clk->ksc;
>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
>> +       u8 reg;
>> +
>> +       if (!cfg->have_mux)
>> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock doesn't have mux\n");
> All these checks should go away and we should have different clk_ops for
> different clk types.
OK, I'll remove these redundant check.
>> +
>> +       guard(spinlock)(&ksc->clk_lock);
>> +       reg = readb(cfg->mux_reg);
>> +
>> +       return reg;
>> +}
>> +
>> +static unsigned long k230_clk_get_rate(struct clk_hw *hw,
>> +                                      unsigned long parent_rate)
>> +{
>> +       struct k230_clk *clk = to_k230_clk(hw);
>> +       struct k230_sysclk *ksc = clk->ksc;
>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
>> +       u32 mul, div;
>> +
>> +       if (!cfg->have_rate) /* no divider, return parents' clk */
>> +               return parent_rate;
>> +
>> +       guard(spinlock)(&ksc->clk_lock);
>> +       switch (cfg->method) {
>> +       /*
>> +        * K230_MUL: div_mask+1/div_max...
>> +        * K230_DIV: mul_max/div_mask+1
>> +        * K230_MUL_DIV: mul_mask/div_mask...
>> +        */
>> +       case K230_MUL:
>> +               div = cfg->rate_div_max;
>> +               mul = (readl(cfg->rate_reg) >> cfg->rate_div_shift)
>> +                       & cfg->rate_div_mask;
>> +               mul++;
>> +               break;
>> +       case K230_DIV:
>> +               mul = cfg->rate_mul_max;
>> +               div = (readl(cfg->rate_reg) >> cfg->rate_div_shift)
>> +                       & cfg->rate_div_mask;
>> +               div++;
>> +               break;
>> +       case K230_MUL_DIV:
>> +               if (!cfg->have_rate_c) {
>> +                       mul = (readl(cfg->rate_reg) >> cfg->rate_mul_shift)
>> +                               & cfg->rate_mul_mask;
>> +                       div = (readl(cfg->rate_reg) >> cfg->rate_div_shift)
>> +                               & cfg->rate_div_mask;
>> +               } else {
>> +                       mul = (readl(cfg->rate_reg_c) >> cfg->rate_mul_shift_c)
>> +                               & cfg->rate_mul_mask_c;
>> +                       div = (readl(cfg->rate_reg_c) >> cfg->rate_div_shift)
>> +                               & cfg->rate_div_mask;
>> +               }
>> +               break;
>> +       default:
>> +               return 0;
>> +       }
>> +
>> +       return div_u64((u64)parent_rate * mul, div);
>> +}
>> +
>> +static int k230_clk_find_approximate(struct k230_clk *clk,
>> +                                    u32 mul_min,
>> +                                    u32 mul_max,
>> +                                    u32 div_min,
>> +                                    u32 div_max,
>> +                                    enum k230_clk_div_type method,
>> +                                    unsigned long rate,
>> +                                    unsigned long parent_rate,
>> +                                    u32 *div,
>> +                                    u32 *mul)
>> +{
>> +       long abs_min;
>> +       long abs_current;
>> +       long perfect_divide;
>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
>> +
>> +       const u32 codec_clk[9] = {
>> +               2048000,
>> +               3072000,
>> +               4096000,
>> +               6144000,
>> +               8192000,
>> +               11289600,
>> +               12288000,
>> +               24576000,
>> +               49152000
>> +       };
>> +
>> +       const u32 codec_div[9][2] = {
>> +               {3125, 16},
>> +               {3125, 24},
>> +               {3125, 32},
>> +               {3125, 48},
>> +               {3125, 64},
>> +               {15625, 441},
>> +               {3125, 96},
>> +               {3125, 192},
>> +               {3125, 384}
>> +       };
>> +
>> +       const u32 pdm_clk[20] = {
>> +               128000,
>> +               192000,
>> +               256000,
>> +               384000,
>> +               512000,
>> +               768000,
>> +               1024000,
>> +               1411200,
>> +               1536000,
>> +               2048000,
>> +               2822400,
>> +               3072000,
>> +               4096000,
>> +               5644800,
>> +               6144000,
>> +               8192000,
>> +               11289600,
>> +               12288000,
>> +               24576000,
>> +               49152000
>> +       };
>> +
>> +       const u32 pdm_div[20][2] = {
>> +               {3125, 1},
>> +               {6250, 3},
>> +               {3125, 2},
>> +               {3125, 3},
>> +               {3125, 4},
>> +               {3125, 6},
>> +               {3125, 8},
>> +               {125000, 441},
>> +               {3125, 12},
>> +               {3125, 16},
>> +               {62500, 441},
>> +               {3125, 24},
>> +               {3125, 32},
>> +               {31250, 441},
>> +               {3125, 48},
>> +               {3125, 64},
>> +               {15625, 441},
>> +               {3125, 96},
>> +               {3125, 192},
>> +               {3125, 384}
>> +       };
>> +
>> +       switch (method) {
>> +       /* only mul can be changeable 1/12,2/12,3/12...*/
>> +       case K230_MUL:
>> +               perfect_divide = (long)((parent_rate * 1000) / rate);
>> +               abs_min = abs(perfect_divide -
>> +                            (long)(((long)div_max * 1000) / (long)mul_min));
>> +               *mul = mul_min;
>> +
>> +               for (u32 i = mul_min + 1; i <= mul_max; i++) {
>> +                       abs_current = abs(perfect_divide -
>> +                                       (long)((long)((long)div_max * 1000) / (long)i));
>> +                       if (abs_min > abs_current) {
>> +                               abs_min = abs_current;
>> +                               *mul = i;
>> +                       }
>> +               }
>> +
>> +               *div = div_max;
>> +               break;
>> +       /* only div can be changeable, 1/1,1/2,1/3...*/
>> +       case K230_DIV:
>> +               perfect_divide = (long)((parent_rate * 1000) / rate);
>> +               abs_min = abs(perfect_divide -
>> +                            (long)(((long)div_min * 1000) / (long)mul_max));
>> +               *div = div_min;
>> +
>> +               for (u32 i = div_min + 1; i <= div_max; i++) {
>> +                       abs_current = abs(perfect_divide -
>> +                                        (long)((long)((long)i * 1000) / (long)mul_max));
>> +                       if (abs_min > abs_current) {
>> +                               abs_min = abs_current;
>> +                               *div = i;
>> +                       }
>> +               }
>> +
>> +               *mul = mul_max;
>> +               break;
>> +       /* mul and div can be changeable. */
>> +       case K230_MUL_DIV:
>> +               if (cfg->rate_reg_off == K230_CLK_CODEC_ADC_MCLKDIV_OFFSET ||
>> +                   cfg->rate_reg_off == K230_CLK_CODEC_DAC_MCLKDIV_OFFSET) {
>> +                       for (u32 j = 0; j < 9; j++) {
>> +                               if (0 == (rate - codec_clk[j])) {
>> +                                       *div = codec_div[j][0];
>> +                                       *mul = codec_div[j][1];
>> +                               }
>> +                       }
>> +               } else if (cfg->rate_reg_off == K230_CLK_AUDIO_CLKDIV_OFFSET ||
>> +                          cfg->rate_reg_off == K230_CLK_PDM_CLKDIV_OFFSET) {
>> +                       for (u32 j = 0; j < 20; j++) {
>> +                               if (0 == (rate - pdm_clk[j])) {
>> +                                       *div = pdm_div[j][0];
>> +                                       *mul = pdm_div[j][1];
>> +                               }
>> +                       }
>> +               } else {
>> +                       return -EINVAL;
>> +               }
>> +               break;
>> +       default:
>> +               WARN_ON_ONCE(true);
>> +               return -EPERM;
> This is impossible? Remove the default case and the compiler will warn
> if an enum case is missing.
Thanks for reminders. I get it.
>> +       }
>> +       return 0;
>> +}
>> +
>> +static long k230_clk_round_rate(struct clk_hw *hw, unsigned long rate, unsigned long *parent_rate)
>> +{
>> +       struct k230_clk *clk = to_k230_clk(hw);
>> +       struct k230_sysclk *ksc = clk->ksc;
>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
>> +       u32 div = 0, mul = 0;
>> +
>> +       if (k230_clk_find_approximate(clk,
>> +                                     cfg->rate_mul_min, cfg->rate_mul_max,
>> +                                     cfg->rate_div_min, cfg->rate_div_max,
>> +                                     cfg->method, rate, *parent_rate, &div, &mul)) {
>> +               dev_err(&ksc->pdev->dev,
>> +                       "[%s]: clk %s round rate error!\n",
>> +                       __func__,
>> +                       clk_hw_get_name(hw));
>> +               return 0;
> Why isn't an error returned?

k230_clk_round_rate is indeed a callback function in clk_ops.

According to the documentation, the return value of this function should
be the final clock rate, not an error code[1].

This means that even when an error occurs, the function needs to return
a valid rate, rather than directly returning an error code.

Link:
https://elixir.bootlin.com/linux/v6.12.15/source/include/linux/clk-provider.h#L139
[1]

>> +       }
>> +
>> +       return div_u64((u64)(*parent_rate) * mul, div);
>> +}
>> +
>> +static int k230_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>> +                            unsigned long parent_rate)
>> +{
>> +       struct k230_clk *clk = to_k230_clk(hw);
>> +       struct k230_sysclk *ksc = clk->ksc;
>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
>> +       u32 div = 0, mul = 0, reg = 0, reg_c;
>> +
>> +       if (!cfg->have_rate || !cfg->rate_reg)
>> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "This clock may have no rate\n");
>> +
>> +       if (rate > parent_rate || rate == 0 || parent_rate == 0)
>> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "rate or parent_rate error\n");
>> +
>> +       if (cfg->read_only)
>> +               return dev_err_probe(&ksc->pdev->dev, -EPERM, "This clk rate is read only\n");
>> +
>> +       if (k230_clk_find_approximate(clk,
>> +                                     cfg->rate_mul_min, cfg->rate_mul_max,
>> +                                     cfg->rate_div_min, cfg->rate_div_max,
>> +                                     cfg->method, rate, parent_rate, &div, &mul))
>> +               return dev_err_probe(&ksc->pdev->dev, -EINVAL, "[%s]: clk %s set rate error!\n",
>> +                                    __func__, clk_hw_get_name(hw));
>> +
>> +       guard(spinlock)(&ksc->clk_lock);
>> +       if (!cfg->have_rate_c) {
>> +               reg = readl(cfg->rate_reg);
>> +               reg &= ~((cfg->rate_div_mask) << (cfg->rate_div_shift));
>> +
>> +               if (cfg->method == K230_DIV) {
>> +                       reg &= ~((cfg->rate_mul_mask) << (cfg->rate_mul_shift));
>> +                       reg |= ((div - 1) & cfg->rate_div_mask) << (cfg->rate_div_shift);
>> +               } else if (cfg->method == K230_MUL) {
>> +                       reg |= ((mul - 1) & cfg->rate_div_mask) << (cfg->rate_div_shift);
>> +               } else {
>> +                       reg |= (mul & cfg->rate_mul_mask) << (cfg->rate_mul_shift);
>> +                       reg |= (div & cfg->rate_div_mask) << (cfg->rate_div_shift);
>> +               }
>> +               reg |= BIT(cfg->rate_write_enable_bit);
>> +       } else {
>> +               reg = readl(cfg->rate_reg);
>> +               reg_c = readl(cfg->rate_reg_c);
>> +               reg &= ~((cfg->rate_div_mask) << (cfg->rate_div_shift));
>> +               reg_c &= ~((cfg->rate_mul_mask_c) << (cfg->rate_mul_shift_c));
>> +               reg_c |= BIT(cfg->rate_write_enable_bit_c);
>> +
>> +               reg_c |= (mul & cfg->rate_mul_mask_c) << (cfg->rate_mul_shift_c);
>> +               reg |= (div & cfg->rate_div_mask) << (cfg->rate_div_shift);
>> +
>> +               writel(reg_c, cfg->rate_reg_c);
>> +       }
>> +       writel(reg, cfg->rate_reg);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct clk_ops k230_clk_ops_arr[K230_CLK_OPS_ID_NUM] = {
>> +       [K230_CLK_OPS_ID_NONE] = {
>> +               /* Sentinel */
>> +       },
>> +       [K230_CLK_OPS_ID_GATE_ONLY] = {
>> +               K230_CLK_OPS_GATE,
>> +       },
>> +       [K230_CLK_OPS_ID_RATE_ONLY] = {
>> +               K230_CLK_OPS_RATE,
>> +       },
>> +       [K230_CLK_OPS_ID_RATE_GATE] = {
>> +               K230_CLK_OPS_RATE,
>> +               K230_CLK_OPS_GATE,
>> +       },
>> +       [K230_CLK_OPS_ID_MUX_ONLY] = {
>> +               K230_CLK_OPS_MUX,
>> +       },
>> +       [K230_CLK_OPS_ID_MUX_GATE] = {
>> +               K230_CLK_OPS_MUX,
>> +               K230_CLK_OPS_GATE,
>> +       },
>> +       [K230_CLK_OPS_ID_MUX_RATE] = {
>> +               K230_CLK_OPS_MUX,
>> +               K230_CLK_OPS_RATE,
>> +       },
>> +       [K230_CLK_OPS_ID_ALL] = {
>> +               K230_CLK_OPS_MUX,
>> +               K230_CLK_OPS_RATE,
>> +               K230_CLK_OPS_GATE,
>> +       },
>> +};
>> +
>> +static int k230_register_clk(struct platform_device *pdev,
>> +                            struct k230_sysclk *ksc,
>> +                            int id,
>> +                            const struct clk_parent_data *parent_data,
>> +                            u8 num_parents,
>> +                            unsigned long flags)
>> +{
>> +       struct k230_clk *clk = &ksc->clks[id];
>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[id];
>> +       struct clk_init_data init = {};
>> +       int clk_id = 0;
>> +       int ret;
>> +
>> +       if (cfg->have_rate) {
>> +               cfg->rate_reg = (ksc->regs + cfg->rate_reg_off);
> Drop useless parenthesis please.
OK, I'll drop it.
>> +               clk_id += K230_CLK_OPS_ID_RATE_ONLY;
>> +       }
>> +
>> +       if (cfg->have_mux) {
>> +               cfg->mux_reg = (ksc->regs + cfg->mux_reg_off);
>> +               clk_id += K230_CLK_OPS_ID_MUX_ONLY;
>> +
>> +               /* mux clock doesn't match the case that num_parents less than 2 */
>> +               if (num_parents < 2)
>> +                       return -EINVAL;
>> +       }
>> +
>> +       if (cfg->have_gate) {
>> +               cfg->gate_reg = (ksc->regs + cfg->gate_reg_off);
> Drop useless parenthesis please.
OK, I'll drop it.
>> +               clk_id += K230_CLK_OPS_ID_GATE_ONLY;
>> +       }
>> +
>> +       if (cfg->have_rate_c)
>> +               cfg->rate_reg_c = (ksc->regs + cfg->rate_reg_off_c);
> Drop useless parenthesis please.
OK, I'll drop it.
>> +
>> +       init.name = k230_clk_cfgs[id].name;
>> +       init.flags = flags;
>> +       init.parent_data = parent_data;
>> +       init.num_parents = num_parents;
>> +       init.ops = &k230_clk_ops_arr[clk_id];
>> +
>> +       clk->id = id;
>> +       clk->ksc = ksc;
>> +       clk->hw.init = &init;
>> +
>> +       ret = devm_clk_hw_register(&pdev->dev, &clk->hw);
>> +       if (ret)
>> +               return dev_err_probe(&pdev->dev,
>> +                                    ret,
>> +                                    "register clock %s failed\n",
>> +                                    k230_clk_cfgs[id].name);
>> +
>> +       return 0;
>> +}
>> +
>> +static int k230_register_mux_clk(struct platform_device *pdev,
>> +                                struct k230_sysclk *ksc,
>> +                                struct clk_parent_data *parent_data,
>> +                                int num_parent,
>> +                                int id)
>> +{
>> +       return k230_register_clk(pdev, ksc, id,
>> +                               (const struct clk_parent_data *)parent_data,
> Please remove useless cast.
OK, I'll drop it.
>> +                               num_parent, 0);
>> +}
>> +
>> +static int k230_register_osc24m_child(struct platform_device *pdev,
>> +                                     struct k230_sysclk *ksc,
>> +                                     int id)
>> +{
>> +       const struct clk_parent_data parent_data = {
>> +               .index = 0,
>> +       };
>> +       return k230_register_clk(pdev, ksc, id, &parent_data, 1, 0);
>> +}
>> +
>> +static int k230_register_pll_child(struct platform_device *pdev,
>> +                                  struct k230_sysclk *ksc,
>> +                                  int id,
>> +                                  enum k230_pll_id pll_id,
>> +                                  unsigned long flags)
>> +{
>> +       const struct clk_parent_data parent_data = {
>> +               .hw = &ksc->plls[pll_id].hw,
>> +       };
>> +       return k230_register_clk(pdev, ksc, id, &parent_data, 1, flags);
>> +}
>> +
>> +static int k230_register_pll_div_child(struct platform_device *pdev,
>> +                                      struct k230_sysclk *ksc,
>> +                                      int id,
>> +                                      enum k230_pll_div_id pll_div_id,
>> +                                      unsigned long flags)
>> +{
>> +       const struct clk_parent_data parent_data = {
>> +               .hw = ksc->dclks[pll_div_id].hw,
>> +       };
>> +       return k230_register_clk(pdev, ksc, id, &parent_data, 1, flags);
>> +}
>> +
>> +static int k230_register_clk_child(struct platform_device *pdev,
>> +                                  struct k230_sysclk *ksc,
>> +                                  int id,
>> +                                  int parent_id)
>> +{
>> +       const struct clk_parent_data parent_data = {
>> +               .hw = &ksc->clks[parent_id].hw,
>> +       };
>> +       return k230_register_clk(pdev, ksc, id, &parent_data, 1, 0);
>> +}
>> +
>> +static int _k230_clk_mux_get_parent_data(struct k230_sysclk *ksc,
>> +                                        struct k230_clk_parent *pclk,
>> +                                        struct clk_parent_data *parent_data)
>> +{
>> +       switch (pclk->type) {
>> +       case K230_OSC24M:
>> +               parent_data->index = 0;
>> +               break;
>> +       case K230_PLL:
>> +               parent_data->hw = &ksc->plls[pclk->pll_id].hw;
>> +               break;
>> +       case K230_PLL_DIV:
>> +               parent_data->hw = ksc->dclks[pclk->pll_div_id].hw;
>> +               break;
>> +       case K230_CLK_COMPOSITE:
>> +               parent_data->hw = &ksc->clks[pclk->clk_id].hw;
>> +               break;
>> +       default:
>> +               WARN_ON_ONCE(true);
>> +               return -EPERM;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int k230_clk_mux_get_parent_data(struct k230_sysclk *ksc,
>> +                                       struct k230_clk_cfg *cfg,
>> +                                       struct clk_parent_data *parent_data,
>> +                                       int num_parent)
>> +{
>> +       int ret;
>> +       struct k230_clk_parent *pclk = cfg->parent;
>> +
>> +       for (int i = 0; i < num_parent; i++) {
>> +               ret = _k230_clk_mux_get_parent_data(ksc, &pclk[i], &parent_data[i]);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int k230_register_clks(struct platform_device *pdev, struct k230_sysclk *ksc)
>> +{
>> +       struct k230_clk_cfg *cfg;
>> +       struct k230_clk_parent *pclk;
>> +       struct clk_parent_data parent_data[K230_CLK_MAX_PARENT_NUM];
>> +       int ret, i;
>> +
>> +       /*
>> +        *  Single parent clock:
>> +        *  pll0_div2 sons: cpu0_src
>> +        *  pll0_div4 sons: cpu0_pclk
>> +        *  cpu0_src sons: cpu0_aclk, cpu0_plic, cpu0_noc_ddrcp4, pmu_pclk
>> +        *
>> +        *  Mux clock:
>> +        *  hs_ospi_src parents: pll0_div2, pll2_div4
>> +        */
>> +       for (i = 0; i < K230_CLK_NUM; i++) {
>> +               cfg = &k230_clk_cfgs[i];
>> +               if (!cfg->status)
>> +                       continue;
>> +
>> +               if (cfg->have_mux) {
>> +                       ret = k230_clk_mux_get_parent_data(ksc, cfg, parent_data,
>> +                                                          cfg->num_parent);
>> +                       if (ret)
>> +                               return dev_err_probe(&pdev->dev, ret,
>> +                                                    "Failed to get parent data\n");
>> +
>> +                       ret = k230_register_mux_clk(pdev, ksc, parent_data,
>> +                                                   cfg->num_parent, i);
>> +               } else {
>> +                       pclk = cfg->parent;
>> +                       switch (pclk->type) {
>> +                       case K230_OSC24M:
>> +                               ret = k230_register_osc24m_child(pdev, ksc, i);
>> +                               break;
>> +                       case K230_PLL:
>> +                               ret = k230_register_pll_child(pdev, ksc, i,
>> +                                                             pclk->pll_id, cfg->flags);
>> +                               break;
>> +                       case K230_PLL_DIV:
>> +                               ret = k230_register_pll_div_child(pdev, ksc, i,
>> +                                                                 pclk->pll_div_id, cfg->flags);
>> +                               break;
>> +                       case K230_CLK_COMPOSITE:
>> +                               ret = k230_register_clk_child(pdev, ksc, i,
>> +                                                             pclk->clk_id);
>> +                               break;
>> +                       default:
>> +                               return dev_err_probe(&pdev->dev, -EINVAL, "Invalid type\n");
>> +                       }
>> +               }
>> +               if (ret)
>> +                       return dev_err_probe(&pdev->dev, ret, "register child id %d failed\n", i);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static struct clk_hw *k230_clk_hw_onecell_get(struct of_phandle_args *clkspec, void *data)
>> +{
>> +       struct k230_sysclk *ksc;
>> +       unsigned int idx;
>> +
>> +       if (clkspec->args_count != 1)
>> +               return ERR_PTR(-EINVAL);
>> +
>> +       idx = clkspec->args[0];
>> +       if (idx >= K230_CLK_NUM)
>> +               return ERR_PTR(-EINVAL);
>> +
>> +       if (!data)
>> +               return ERR_PTR(-EINVAL);
>> +
>> +       ksc = (struct k230_sysclk *)data;
> Drop useless cast please.
OK, I'll drop it.
>> +
>> +       return &ksc->clks[idx].hw;
>> +}
>> +
>> +static int k230_clk_init_plls(struct platform_device *pdev)
>> +{
>> +       int ret;
>> +       struct k230_sysclk *ksc = platform_get_drvdata(pdev);
>> +
>> +       spin_lock_init(&ksc->pll_lock);
>> +
>> +       ksc->pll_regs = devm_platform_ioremap_resource(pdev, 0);
>> +       if (!ksc->pll_regs)
> Does it return an error pointer or NULL on failure?The dev_err_probe()
> below is incorrect or the if condition above is.

Yes，I've checked it and confirmed that the if condition is incorrect [2].

So it should be:

if (IS_ERR(ksc->pll_regs))

Link:
https://elixir.bootlin.com/linux/v6.12.15/source/drivers/base/platform.c#L111
[2]

>
>> +               return dev_err_probe(&pdev->dev, PTR_ERR(ksc->pll_regs), "map registers failed\n");
> Doesn't this already print an error?
>
>> +
>> +       ret = k230_register_plls(pdev, ksc);
>> +       if (ret)
>> +               return dev_err_probe(&pdev->dev, ret, "register plls failed\n");
>> +
>> +       ret = k230_register_pll_divs(pdev, ksc);
>> +       if (ret)
>> +               return dev_err_probe(&pdev->dev, ret, "register pll_divs failed\n");
>> +
>> +       for (int i = 0; i < K230_PLL_DIV_NUM; i++) {
>> +               ret = devm_clk_hw_register_clkdev(&pdev->dev, ksc->dclks[i].hw,
>> +                                                 k230_pll_div_cfgs[i].name, NULL);
> Why is clkdev being used?

Here is my thought:

clkdev is used to register clock devices internally and associate them
with a specific device identifier (con_id). This allows some certain
clock consumers, can access the clock of PLL_DIVs using `clk_get()` and
related APIs.

In this case, while the PLL_DIVs are internal clocks, which not exposed
in the DT since they are only used within the driver. So I use
`register_clkdev()` to make PLLs clock accessible.

If there is anything I ignored or misunderstand, please feel free to
correct me.

>> +               if (ret)
>> +                       return dev_err_probe(&pdev->dev, ret, "clock_lookup create failed\n");
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int k230_clk_init_clks(struct platform_device *pdev)
>> +{
>> +       int ret;
>> +       struct k230_sysclk *ksc = platform_get_drvdata(pdev);
>> +
>> +       spin_lock_init(&ksc->clk_lock);
>> +
>> +       ksc->regs = devm_platform_ioremap_resource(pdev, 1);
>> +       if (!ksc->regs)
>> +               return dev_err_probe(&pdev->dev, PTR_ERR(ksc->regs), "failed to map registers\n");
>> +
>> +       ret = k230_register_clks(pdev, ksc);
>> +       if (ret)
>> +               return dev_err_probe(&pdev->dev, ret, "register clock provider failed\n");
>> +
>> +       ret = devm_of_clk_add_hw_provider(&pdev->dev, k230_clk_hw_onecell_get, ksc);
>> +       if (ret)
>> +               return dev_err_probe(&pdev->dev, ret, "add clock provider failed\n");
>> +
>> +       return 0;
>> +}
>> +
>> +static int k230_clk_probe(struct platform_device *pdev)
>> +{
>> +       int ret;
>> +       struct k230_sysclk *ksc;
>> +
>> +       ksc = devm_kzalloc(&pdev->dev, sizeof(struct k230_sysclk), GFP_KERNEL);
>> +       if (!ksc)
>> +               return -ENOMEM;
>> +
>> +       ksc->plls = devm_kmalloc_array(&pdev->dev, K230_PLL_NUM,
> Any reason devm_kcalloc() isn't used?
Thanks for your reminder, I'll replace devm_kmalloc_array() with
devm_kcalloc() instead.
>> +                                      sizeof(struct k230_pll), GFP_KERNEL);
>> +       if (!ksc->plls)
>> +               return -ENOMEM;
>> +
>> +       ksc->dclks = devm_kmalloc_array(&pdev->dev, K230_PLL_DIV_NUM,
>> +                                       sizeof(struct k230_pll_div), GFP_KERNEL);
>> +       if (!ksc->dclks)
>> +               return -ENOMEM;
>> +
>> +       ksc->clks = devm_kmalloc_array(&pdev->dev, K230_CLK_NUM,
>> +                                      sizeof(struct k230_clk), GFP_KERNEL);
>> +       if (!ksc->clks)
>> +               return -ENOMEM;
>> +
>> +       ksc->pdev = pdev;
>> +       platform_set_drvdata(pdev, ksc);
>> +
>> +       ret = k230_clk_init_plls(pdev);
>> +       if (ret)
>> +               return dev_err_probe(&pdev->dev, ret, "init plls failed\n");
>> +
>> +       ret = k230_clk_init_clks(pdev);
>> +       if (ret)
>> +               return dev_err_probe(&pdev->dev, ret, "init clks failed\n");
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct of_device_id k230_clk_ids[] = {
>> +       { .compatible = "canaan,k230-clk" },
>> +       { /* Sentinel */ },
> Nitpick: Please drop last comma so that nothing can come after without
> causing a compilation error.
OK, I get it. Thanks for your reminder.
>> +};
>> +MODULE_DEVICE_TABLE(of, k230_clk_ids);
Thanks for your patient review!

Re: [PATCH v4 2/3] clk: canaan: Add clock driver for Canaan K230

[Xukai Wang]

On 2025/2/20 23:07, Xukai Wang wrote:
> On 2025/2/19 05:48, Stephen Boyd wrote:
>>> +static void k230_clk_disable(struct clk_hw *hw)
>>> +{
>>> +       struct k230_clk *clk = to_k230_clk(hw);
>>> +       struct k230_sysclk *ksc = clk->ksc;
>>> +       struct k230_clk_cfg *cfg = &k230_clk_cfgs[clk->id];
>>> +       u32 reg;
>>> +
>>> +       if (!cfg->have_gate) {
>>> +               dev_err(&ksc->pdev->dev, "This clock doesn't have gate\n");
>> Why are the clk_ops assigned to this clk then?
> If this clock doesn't have a gate, then the clk_ops will not be assigned to it. In this case, I'm simply checking it again. You can verify this in the k230_register_clk function to ensure it's handled correctly.
Apologies for my script doesn't wrap this paragraph.
> If you think this check is redundant, I can drop it.

[PATCH v4 3/3] riscv: dts: canaan: Add clock initial support for K230

[Xukai Wang]

This patch provides basic support for the K230 clock, which does not
cover all clocks.

The clock tree of the K230 SoC consists of OSC24M,
PLLs and sysclk.

Co-developed-by: Troy Mitchell <TroyMitchell988@gmail.com>
Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
Signed-off-by: Xukai Wang <kingxukai@zohomail.com>
---
 arch/riscv/boot/dts/canaan/k230.dtsi | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/riscv/boot/dts/canaan/k230.dtsi b/arch/riscv/boot/dts/canaan/k230.dtsi
index 95c1a3d8fb1192e30113d96d3e96329545bc6ae7..e50ba03c2c21597e5f7d04a652db08f84101cbfb 100644
--- a/arch/riscv/boot/dts/canaan/k230.dtsi
+++ b/arch/riscv/boot/dts/canaan/k230.dtsi
@@ -3,6 +3,7 @@
  * Copyright (C) 2024 Yangyu Chen <cyy@cyyself.name>
  */
 
+#include <dt-bindings/clock/canaan,k230-clk.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 
 /dts-v1/;
@@ -65,6 +66,13 @@ apb_clk: apb-clk-clock {
 		#clock-cells = <0>;
 	};
 
+	osc24m: clock-24m {
+		compatible = "fixed-clock";
+		clock-frequency = <24000000>;
+		clock-output-names = "osc24m";
+		#clock-cells = <0>;
+	};
+
 	soc {
 		compatible = "simple-bus";
 		interrupt-parent = <&plic>;
@@ -138,5 +146,29 @@ uart4: serial@91404000 {
 			reg-shift = <2>;
 			status = "disabled";
 		};
+
+		sysclk: clock-controller@91102000 {
+			compatible = "canaan,k230-clk";
+			reg = <0x0 0x91102000 0x0 0x1000>,
+			      <0x0 0x91100000 0x0 0x1000>;
+			clocks = <&osc24m>;
+			clock-output-names = "CPU0_ACLK", "CPU0_PLIC", "CPU0_NOC_DDRCP4",
+					     "CPU0_PCLK", "PMU_PCLK", "HS_HCLK_HIGH_SRC",
+					     "HS_HCLK_HIGH_GATE", "HS_HCLK_SRC",
+					     "HS_SD0_HS_AHB_GAT", "HS_SD1_HS_AHB_GAT",
+					     "HS_SSI1_HS_AHB_GA", "HS_SSI2_HS_AHB_GA",
+					     "HS_USB0_HS_AHB_GA", "HS_USB1_HS_AHB_GA",
+					     "HS_SSI0_AXI", "HS_SSI1", "HS_SSI2",
+					     "HS_QSPI_AXI_SRC", "HS_SSI1_ACLK_GATE",
+					     "HS_SSI2_ACLK_GATE", "HS_SD_CARD_SRC",
+					     "HS_SD0_CARD_TX", "HS_SD1_CARD_TX",
+					     "HS_SD_AXI_SRC", "HS_SD0_AXI_GATE",
+					     "HS_SD1_AXI_GATE", "HS_SD0_BASE_GATE",
+					     "HS_SD1_BASE_GATE", "HS_OSPI_SRC",
+					     "HS_USB_REF_51M", "HS_SD_TIMER_SRC",
+					     "HS_SD0_TIMER_GATE", "HS_SD1_TIMER_GATE",
+					     "HS_USB0_REFERENCE", "HS_USB1_REFERENCE";
+			#clock-cells = <1>;
+		};
 	};
 };

-- 
2.34.1

Re: [PATCH v4 3/3] riscv: dts: canaan: Add clock initial support for K230

[Conor Dooley]

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

On Mon, Feb 17, 2025 at 10:45:18PM +0800, Xukai Wang wrote:
> This patch provides basic support for the K230 clock, which does not
> cover all clocks.
> 
> The clock tree of the K230 SoC consists of OSC24M,
> PLLs and sysclk.
> 
> Co-developed-by: Troy Mitchell <TroyMitchell988@gmail.com>
> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
> Signed-off-by: Xukai Wang <kingxukai@zohomail.com>
> ---
>  arch/riscv/boot/dts/canaan/k230.dtsi | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/canaan/k230.dtsi b/arch/riscv/boot/dts/canaan/k230.dtsi
> index 95c1a3d8fb1192e30113d96d3e96329545bc6ae7..e50ba03c2c21597e5f7d04a652db08f84101cbfb 100644
> --- a/arch/riscv/boot/dts/canaan/k230.dtsi
> +++ b/arch/riscv/boot/dts/canaan/k230.dtsi
> @@ -3,6 +3,7 @@
>   * Copyright (C) 2024 Yangyu Chen <cyy@cyyself.name>
>   */
>  
> +#include <dt-bindings/clock/canaan,k230-clk.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
>  
>  /dts-v1/;
> @@ -65,6 +66,13 @@ apb_clk: apb-clk-clock {
>  		#clock-cells = <0>;
>  	};
>  
> +	osc24m: clock-24m {
> +		compatible = "fixed-clock";
> +		clock-frequency = <24000000>;
> +		clock-output-names = "osc24m";
> +		#clock-cells = <0>;
> +	};
> +
>  	soc {
>  		compatible = "simple-bus";
>  		interrupt-parent = <&plic>;
> @@ -138,5 +146,29 @@ uart4: serial@91404000 {
>  			reg-shift = <2>;
>  			status = "disabled";
>  		};
> +
> +		sysclk: clock-controller@91102000 {
> +			compatible = "canaan,k230-clk";
> +			reg = <0x0 0x91102000 0x0 0x1000>,
> +			      <0x0 0x91100000 0x0 0x1000>;
> +			clocks = <&osc24m>;
> +			clock-output-names = "CPU0_ACLK", "CPU0_PLIC", "CPU0_NOC_DDRCP4",
> +					     "CPU0_PCLK", "PMU_PCLK", "HS_HCLK_HIGH_SRC",
> +					     "HS_HCLK_HIGH_GATE", "HS_HCLK_SRC",
> +					     "HS_SD0_HS_AHB_GAT", "HS_SD1_HS_AHB_GAT",
> +					     "HS_SSI1_HS_AHB_GA", "HS_SSI2_HS_AHB_GA",
> +					     "HS_USB0_HS_AHB_GA", "HS_USB1_HS_AHB_GA",
> +					     "HS_SSI0_AXI", "HS_SSI1", "HS_SSI2",
> +					     "HS_QSPI_AXI_SRC", "HS_SSI1_ACLK_GATE",
> +					     "HS_SSI2_ACLK_GATE", "HS_SD_CARD_SRC",
> +					     "HS_SD0_CARD_TX", "HS_SD1_CARD_TX",
> +					     "HS_SD_AXI_SRC", "HS_SD0_AXI_GATE",
> +					     "HS_SD1_AXI_GATE", "HS_SD0_BASE_GATE",
> +					     "HS_SD1_BASE_GATE", "HS_OSPI_SRC",
> +					     "HS_USB_REF_51M", "HS_SD_TIMER_SRC",
> +					     "HS_SD0_TIMER_GATE", "HS_SD1_TIMER_GATE",
> +					     "HS_USB0_REFERENCE", "HS_USB1_REFERENCE";
> +			#clock-cells = <1>;
> +		};

Apologies for not commenting on it until now, but this patch seems like
there's some hunks missing from it. I'd expect you to remove the dummy
"apb-clk-clock" from the dts and replace it with a real one sourced from
the newly added clock controller.

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

Re: [PATCH v4 3/3] riscv: dts: canaan: Add clock initial support for K230

[Xukai Wang]

On 2025/2/19 01:00, Conor Dooley wrote:
> On Mon, Feb 17, 2025 at 10:45:18PM +0800, Xukai Wang wrote:
>> +		sysclk: clock-controller@91102000 {
>> +			compatible = "canaan,k230-clk";
>> +			reg = <0x0 0x91102000 0x0 0x1000>,
>> +			      <0x0 0x91100000 0x0 0x1000>;
>> +			clocks = <&osc24m>;
>> +			clock-output-names = "CPU0_ACLK", "CPU0_PLIC", "CPU0_NOC_DDRCP4",
>> +					     "CPU0_PCLK", "PMU_PCLK", "HS_HCLK_HIGH_SRC",
>> +					     "HS_HCLK_HIGH_GATE", "HS_HCLK_SRC",
>> +					     "HS_SD0_HS_AHB_GAT", "HS_SD1_HS_AHB_GAT",
>> +					     "HS_SSI1_HS_AHB_GA", "HS_SSI2_HS_AHB_GA",
>> +					     "HS_USB0_HS_AHB_GA", "HS_USB1_HS_AHB_GA",
>> +					     "HS_SSI0_AXI", "HS_SSI1", "HS_SSI2",
>> +					     "HS_QSPI_AXI_SRC", "HS_SSI1_ACLK_GATE",
>> +					     "HS_SSI2_ACLK_GATE", "HS_SD_CARD_SRC",
>> +					     "HS_SD0_CARD_TX", "HS_SD1_CARD_TX",
>> +					     "HS_SD_AXI_SRC", "HS_SD0_AXI_GATE",
>> +					     "HS_SD1_AXI_GATE", "HS_SD0_BASE_GATE",
>> +					     "HS_SD1_BASE_GATE", "HS_OSPI_SRC",
>> +					     "HS_USB_REF_51M", "HS_SD_TIMER_SRC",
>> +					     "HS_SD0_TIMER_GATE", "HS_SD1_TIMER_GATE",
>> +					     "HS_USB0_REFERENCE", "HS_USB1_REFERENCE";
>> +			#clock-cells = <1>;
>> +		};
> Apologies for not commenting on it until now, but this patch seems like
> there's some hunks missing from it. I'd expect you to remove the dummy
> "apb-clk-clock" from the dts and replace it with a real one sourced from
> the newly added clock controller.

Thanks for your time.

I've modified all nodes that previously depended on apb_clk， and they
are now referencing the new clock controller.

Re: [PATCH v4 0/3] riscv: canaan: Add support for K230-Canmv clock

[Conor Dooley]

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

Stephen,

Is the driver in this series satisfactory to you? If it is, can I send
you a PR containing it and the binding so that I can apply the final
patch in the series (and merge the basic support for the k230 soc)?

Cheers,
Conor.

On Mon, Feb 17, 2025 at 10:45:15PM +0800, Xukai Wang wrote:
> This patch series adds clock controller support for the Canaan Kendryte
> K230 SoC. The K230 SoC includes an external 24MHz OSC and 4 internal
> PLLs, with the controller managing these sources and their derived clocks.
> 
> The clock tree and hardware-specific definition can be found in the
> vendor's DTS [1],
> and this series is based on the K230 initial series [2].
> 
> Link: https://github.com/kendryte/k230_sdk/blob/main/src/little/linux/arch/riscv/boot/dts/kendryte/clock_provider.dtsi [1]
> Link: https://lore.kernel.org/linux-clk/tencent_F76EB8D731C521C18D5D7C4F8229DAA58E08@qq.com/ [2]
> 
> Co-developed-by: Troy Mitchell <TroyMitchell988@gmail.com>
> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
> Signed-off-by: Xukai Wang <kingxukai@zohomail.com>
> 
> ---
> Changes in v4:
> - Remove redundant onecell_get callback and add_provider function
> for pll_divs.
> - Modify the base-commit in cover letter.
> - Link to v3: https://lore.kernel.org/r/20250203-b4-k230-clk-v3-0-362c79124572@zohomail.com
> 
> Changes in v3:
> - Reorder the defination and declaration in drivers code.
> - Reorder the properties in dts node.
> - Replace global variable `k230_sysclk` with dynamic memory allocation.
> - Rename the macro K230_NUM_CLKS to K230_CLK_NUM.
> - Use dev_err_probe for error handling.
> - Remove unused includes.
> - Link to v2: https://lore.kernel.org/r/20250108-b4-k230-clk-v2-0-27b30a2ca52d@zohomail.com
> 
> Changes in v2:
> - Add items and description.
> - Rename k230-clk.h to canaan,k230-clk.h
> - Link to v1: https://lore.kernel.org/r/20241229-b4-k230-clk-v1-0-221a917e80ed@zohomail.com
> 
> ---
> Xukai Wang (3):
>       dt-bindings: clock: Add bindings for Canaan K230 clock controller
>       clk: canaan: Add clock driver for Canaan K230
>       riscv: dts: canaan: Add clock initial support for K230
> 
>  .../devicetree/bindings/clock/canaan,k230-clk.yaml |   43 +
>  arch/riscv/boot/dts/canaan/k230.dtsi               |   32 +
>  drivers/clk/Kconfig                                |    6 +
>  drivers/clk/Makefile                               |    1 +
>  drivers/clk/clk-k230.c                             | 1347 ++++++++++++++++++++
>  include/dt-bindings/clock/canaan,k230-clk.h        |   49 +
>  6 files changed, 1478 insertions(+)
> ---
> base-commit: 7fdb24bbac37ce692346d60b89f5aa29844b8b88
> change-id: 20241206-b4-k230-clk-925f33fed6c2
> 
> Best regards,
> -- 
> Xukai Wang <kingxukai@zohomail.com>
> 

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

Re: [PATCH v4 0/3] riscv: canaan: Add support for K230-Canmv clock

[Stephen Boyd]

Quoting Conor Dooley (2025-02-18 09:02:32)
> Stephen,
> 
> Is the driver in this series satisfactory to you? If it is, can I send
> you a PR containing it and the binding so that I can apply the final
> patch in the series (and merge the basic support for the k230 soc)?
> 

Sorry, the driver is not ready.

Re: [PATCH v4 0/3] riscv: canaan: Add support for K230-Canmv clock

[Conor Dooley]

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

On Tue, Feb 18, 2025 at 01:51:38PM -0800, Stephen Boyd wrote:
> Quoting Conor Dooley (2025-02-18 09:02:32)
> > Stephen,
> > 
> > Is the driver in this series satisfactory to you? If it is, can I send
> > you a PR containing it and the binding so that I can apply the final
> > patch in the series (and merge the basic support for the k230 soc)?
> > 
> 
> Sorry, the driver is not ready.

That's cool, was just basing off the v4 changelog being minimal, no
worries.

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