mirror of https://lore.kernel.org/linux-amlogic/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] clk: amlogic: Add A9 AO clock controller
@ 2026-06-03 12:17 Jian Hu via B4 Relay
  2026-06-03 12:17 ` [PATCH v2 1/2] dt-bindings: clock: Add Amlogic " Jian Hu via B4 Relay
  2026-06-03 12:17 ` [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver Jian Hu via B4 Relay
  0 siblings, 2 replies; 9+ messages in thread
From: Jian Hu via B4 Relay @ 2026-06-03 12:17 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao,
	Kevin Hilman, Martin Blumenstingl
  Cc: linux-amlogic, linux-clk, devicetree, linux-kernel,
	linux-arm-kernel, Jian Hu

This series adds Amlogic A9 AO clock support, including dt-binding and AO clock driver.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
Changes in v2:
- Split the A9 clock driver and send the AO clock separately.
- Rename aobus to soc
- Use CLK_HW_INIT_FW_NAME to describe clk_init_data
- Use CLK_HW_INIT_PARENTS_DATA to describe clk_init_data
- Use a9_ao prefix for MESON_COMP_SEL
- Correct duandiv name
- Fix pwm b reg
- Link to v1: https://lore.kernel.org/all/20260511-b4-a9_clk-v1-0-41cb4071b7c9@amlogic.com/

---
Jian Hu (2):
      dt-bindings: clock: Add Amlogic A9 AO clock controller
      clk: amlogic: Add A9 AO clock controller driver

 .../bindings/clock/amlogic,a9-aoclkc.yaml          |  76 ++++
 drivers/clk/meson/Kconfig                          |  13 +
 drivers/clk/meson/Makefile                         |   1 +
 drivers/clk/meson/a9-aoclk.c                       | 419 +++++++++++++++++++++
 include/dt-bindings/clock/amlogic,a9-aoclkc.h      |  76 ++++
 5 files changed, 585 insertions(+)
---
base-commit: ca89c88bcf69daca829044c638a8163d5ce47af0
change-id: 20260603-a9_aoclk-bbf531badc63

Best regards,
-- 
Jian Hu <jian.hu@amlogic.com>



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* [PATCH v2 1/2] dt-bindings: clock: Add Amlogic A9 AO clock controller
  2026-06-03 12:17 [PATCH v2 0/2] clk: amlogic: Add A9 AO clock controller Jian Hu via B4 Relay
@ 2026-06-03 12:17 ` Jian Hu via B4 Relay
  2026-06-03 15:51   ` Conor Dooley
  2026-06-03 12:17 ` [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver Jian Hu via B4 Relay
  1 sibling, 1 reply; 9+ messages in thread
From: Jian Hu via B4 Relay @ 2026-06-03 12:17 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao,
	Kevin Hilman, Martin Blumenstingl
  Cc: linux-amlogic, linux-clk, devicetree, linux-kernel,
	linux-arm-kernel, Jian Hu

From: Jian Hu <jian.hu@amlogic.com>

Add the Always-On clock controller dt-bindings for the Amlogic A9
SoC family.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 .../bindings/clock/amlogic,a9-aoclkc.yaml          | 76 ++++++++++++++++++++++
 include/dt-bindings/clock/amlogic,a9-aoclkc.h      | 76 ++++++++++++++++++++++
 2 files changed, 152 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/amlogic,a9-aoclkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a9-aoclkc.yaml
new file mode 100644
index 000000000000..1fa9b3a32fbb
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/amlogic,a9-aoclkc.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2026 Amlogic, Inc. All rights reserved
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/amlogic,a9-aoclkc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic A9 Series Always-On Clock Controller
+
+maintainers:
+  - Neil Armstrong <neil.armstrong@linaro.org>
+  - Jerome Brunet <jbrunet@baylibre.com>
+  - Jian Hu <jian.hu@amlogic.com>
+  - Xianwei Zhao <xianwei.zhao@amlogic.com>
+
+properties:
+  compatible:
+    const: amlogic,a9-aoclkc
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+  clocks:
+    minItems: 5
+    items:
+      - description: input oscillator
+      - description: input fclk div 3
+      - description: input fclk div 4
+      - description: input fclk div 5
+      - description: input sys clk
+      - description: external fixed 32k (optional)
+
+  clock-names:
+    minItems: 5
+    items:
+      - const: xtal
+      - const: fdiv3
+      - const: fdiv4
+      - const: fdiv5
+      - const: sys
+      - const: ext_32k
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        clock-controller@0 {
+            compatible = "amlogic,a9-aoclkc";
+            reg = <0x0 0x0 0x0 0x58>;
+            #clock-cells = <1>;
+            clocks = <&xtal>,
+                     <&scmi_clk 14>,
+                     <&scmi_clk 16>,
+                     <&scmi_clk 18>,
+                     <&scmi_clk 21>;
+            clock-names = "xtal",
+                          "fdiv3",
+                          "fdiv4",
+                          "fdiv5",
+                          "sys";
+        };
+    };
diff --git a/include/dt-bindings/clock/amlogic,a9-aoclkc.h b/include/dt-bindings/clock/amlogic,a9-aoclkc.h
new file mode 100644
index 000000000000..a7d704d4b58e
--- /dev/null
+++ b/include/dt-bindings/clock/amlogic,a9-aoclkc.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (C) 2026 Amlogic, Inc. All rights reserved.
+ */
+
+#ifndef __AMLOGIC_A9_AO_CLKC_H
+#define __AMLOGIC_A9_AO_CLKC_H
+
+#define CLKID_AO_XTAL_IN			0
+#define CLKID_AO_XTAL				1
+#define CLKID_AO_SYS				2
+#define CLKID_AO_SYS_I3C			3
+#define CLKID_AO_SYS_RTC_REG			4
+#define CLKID_AO_SYS_CLKTREE			5
+#define CLKID_AO_SYS_RST_CTRL			6
+#define CLKID_AO_SYS_PAD			7
+#define CLKID_AO_SYS_RTC_DIG			8
+#define CLKID_AO_SYS_IRQ			9
+#define CLKID_AO_SYS_PWRCTRL			10
+#define CLKID_AO_SYS_PWM_A			11
+#define CLKID_AO_SYS_PWM_B			12
+#define CLKID_AO_SYS_PWM_C			13
+#define CLKID_AO_SYS_PWM_D			14
+#define CLKID_AO_SYS_PWM_E			15
+#define CLKID_AO_SYS_PWM_F			16
+#define CLKID_AO_SYS_PWM_G			17
+#define CLKID_AO_SYS_I2C_A			18
+#define CLKID_AO_SYS_I2C_B			19
+#define CLKID_AO_SYS_I2C_C			20
+#define CLKID_AO_SYS_I2C_D			21
+#define CLKID_AO_SYS_SED			22
+#define CLKID_AO_SYS_IR_CTRL			23
+#define CLKID_AO_SYS_UART_B			24
+#define CLKID_AO_SYS_UART_C			25
+#define CLKID_AO_SYS_UART_D			26
+#define CLKID_AO_SYS_UART_E			27
+#define CLKID_AO_SYS_SPISG_0			28
+#define CLKID_AO_SYS_RTC_SECURE			29
+#define CLKID_AO_SYS_CEC			30
+#define CLKID_AO_SYS_AOCPU			31
+#define CLKID_AO_SYS_SRAM			32
+#define CLKID_AO_SYS_SPISG_1			33
+#define CLKID_AO_SYS_SPISG_2			34
+#define CLKID_AO_PWM_A_SEL			35
+#define CLKID_AO_PWM_A_DIV			36
+#define CLKID_AO_PWM_A				37
+#define CLKID_AO_PWM_B_SEL			38
+#define CLKID_AO_PWM_B_DIV			39
+#define CLKID_AO_PWM_B				40
+#define CLKID_AO_PWM_C_SEL			41
+#define CLKID_AO_PWM_C_DIV			42
+#define CLKID_AO_PWM_C				43
+#define CLKID_AO_PWM_D_SEL			44
+#define CLKID_AO_PWM_D_DIV			45
+#define CLKID_AO_PWM_D				46
+#define CLKID_AO_PWM_E_SEL			47
+#define CLKID_AO_PWM_E_DIV			48
+#define CLKID_AO_PWM_E				49
+#define CLKID_AO_PWM_F_SEL			50
+#define CLKID_AO_PWM_F_DIV			51
+#define CLKID_AO_PWM_F				52
+#define CLKID_AO_PWM_G_SEL			53
+#define CLKID_AO_PWM_G_DIV			54
+#define CLKID_AO_PWM_G				55
+#define CLKID_AO_RTC_DUALDIV_IN			56
+#define CLKID_AO_RTC_DUALDIV_DIV		57
+#define CLKID_AO_RTC_DUALDIV_SEL		58
+#define CLKID_AO_RTC_DUALDIV			59
+#define CLKID_AO_RTC				60
+#define CLKID_AO_CEC_DUALDIV_IN			61
+#define CLKID_AO_CEC_DUALDIV_DIV		62
+#define CLKID_AO_CEC_DUALDIV_SEL		63
+#define CLKID_AO_CEC_DUALDIV			64
+#define CLKID_AO_CEC				65
+
+#endif  /* __AMLOGIC_A9_AO_CLKC_H */

-- 
2.47.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver
  2026-06-03 12:17 [PATCH v2 0/2] clk: amlogic: Add A9 AO clock controller Jian Hu via B4 Relay
  2026-06-03 12:17 ` [PATCH v2 1/2] dt-bindings: clock: Add Amlogic " Jian Hu via B4 Relay
@ 2026-06-03 12:17 ` Jian Hu via B4 Relay
  2026-06-03 12:34   ` sashiko-bot
  2026-06-03 14:29   ` Jerome Brunet
  1 sibling, 2 replies; 9+ messages in thread
From: Jian Hu via B4 Relay @ 2026-06-03 12:17 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao,
	Kevin Hilman, Martin Blumenstingl
  Cc: linux-amlogic, linux-clk, devicetree, linux-kernel,
	linux-arm-kernel, Jian Hu

From: Jian Hu <jian.hu@amlogic.com>

Add the Always-on clock controller driver for the Amlogic A9 SoC family.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 drivers/clk/meson/Kconfig    |  13 ++
 drivers/clk/meson/Makefile   |   1 +
 drivers/clk/meson/a9-aoclk.c | 419 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 433 insertions(+)

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index cf8cf3f9e4ee..625e6788b940 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
 	  device, A1 SoC Family. Say Y if you want A1 Peripherals clock
 	  controller to work.
 
+config COMMON_CLK_A9_AO
+	tristate "Amlogic A9 SoC AO clock controller support"
+	depends on ARM64
+	default ARCH_MESON || COMPILE_TEST
+	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_MESON_CLKC_UTILS
+	select COMMON_CLK_MESON_DUALDIV
+	imply COMMON_CLK_SCMI
+	help
+	  Support for the AO clock controller on Amlogic A311Y3 based
+	  device, AKA A9.
+	  Say Y if you want A9 AO clock controller to work.
+
 config COMMON_CLK_C3_PLL
 	tristate "Amlogic C3 PLL clock controller"
 	depends on ARM64
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index c6719694a242..f89d027c282c 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
 obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
 obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
 obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
+obj-$(CONFIG_COMMON_CLK_A9_AO) += a9-aoclk.o
 obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o
 obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o
 obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
diff --git a/drivers/clk/meson/a9-aoclk.c b/drivers/clk/meson/a9-aoclk.c
new file mode 100644
index 000000000000..b7b3ca231a42
--- /dev/null
+++ b/drivers/clk/meson/a9-aoclk.c
@@ -0,0 +1,419 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
+/*
+ * Copyright (C) 2026 Amlogic, Inc. All rights reserved
+ */
+
+#include <dt-bindings/clock/amlogic,a9-aoclkc.h>
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+#include "clk-regmap.h"
+#include "clk-dualdiv.h"
+#include "meson-clkc-utils.h"
+
+#define AO_OSCIN_CTRL			0x00
+#define AO_SYS_CLK0			0x04
+#define AO_PWM_CLK_A_CTRL		0x1c
+#define AO_PWM_CLK_B_CTRL		0x20
+#define AO_PWM_CLK_C_CTRL		0x24
+#define AO_PWM_CLK_D_CTRL		0x28
+#define AO_PWM_CLK_E_CTRL		0x2c
+#define AO_PWM_CLK_F_CTRL		0x30
+#define AO_PWM_CLK_G_CTRL		0x34
+#define AO_CEC_CTRL0			0x38
+#define AO_CEC_CTRL1			0x3c
+#define AO_RTC_BY_OSCIN_CTRL0		0x50
+#define AO_RTC_BY_OSCIN_CTRL1		0x54
+
+#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
+	MESON_COMP_SEL(a9_ao_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
+
+#define A9_COMP_DIV(_name, _reg, _shift, _width) \
+	MESON_COMP_DIV(a9_ao_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
+
+#define A9_COMP_GATE(_name, _reg, _bit) \
+	MESON_COMP_GATE(a9_ao_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
+
+static struct clk_regmap a9_ao_xtal_in = {
+	.data = &(struct clk_regmap_gate_data){
+		.offset = AO_OSCIN_CTRL,
+		.bit_idx = 3,
+	},
+	/*
+	 * It may be ao_sys's parent clock, its child clocks mark
+	 * CLK_IS_CRITICAL, So mark CLK_IS_CRITICAL for it.
+	 */
+	.hw.init = CLK_HW_INIT_FW_NAME("ao_xtal_in", "xtal",
+				       &clk_regmap_gate_ops, CLK_IS_CRITICAL),
+};
+
+static struct clk_regmap a9_ao_xtal = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = AO_OSCIN_CTRL,
+		.mask = 0x1,
+		.shift = 0,
+	},
+	/* ext_32k is from external PAD, do not automatically reparent */
+	.hw.init = CLK_HW_INIT_PARENTS_DATA("ao_xtal",
+			((const struct clk_parent_data []) {
+				{ .hw = &a9_ao_xtal_in.hw },
+				{ .fw_name = "ext_32k" }
+			}), &clk_regmap_mux_ops, CLK_SET_RATE_NO_REPARENT),
+};
+
+static struct clk_regmap a9_ao_sys = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = AO_OSCIN_CTRL,
+		.mask = 0x1,
+		.shift = 1,
+	},
+	.hw.init = CLK_HW_INIT_PARENTS_DATA("ao_sys",
+			((const struct clk_parent_data []) {
+				{ .hw = &a9_ao_xtal.hw },
+				{ .fw_name = "sys" }
+			}), &clk_regmap_mux_ops, 0),
+};
+
+static const struct clk_parent_data a9_ao_pclk_parents = { .hw = &a9_ao_sys.hw };
+
+#define A9_AO_PCLK(_name, _bit, _flags)		       \
+	MESON_PCLK(a9_ao_sys_##_name, AO_SYS_CLK0, _bit, \
+		   &a9_ao_pclk_parents, _flags)
+
+/*
+ * A9 integrates a low-power microprocessor (Always-on CPU: AOCPU). Some AO sys
+ * clocks control the AOCPU modules. Mark the AOCPU-related clocks with
+ * CLK_IS_CRITICAL to avoid them being disabled and impacting AOCPU functionality.
+ * AOCPU-related clocks list:
+ * - clktree
+ * - rst_ctrl
+ * - pad
+ * - irq
+ * - pwrctrl
+ * - aocpu
+ * - sram
+ */
+static A9_AO_PCLK(i2c3,		0,	0);
+static A9_AO_PCLK(rtc_reg,	1,	0);
+static A9_AO_PCLK(clktree,	2,	CLK_IS_CRITICAL);
+static A9_AO_PCLK(rst_ctrl,	3,	CLK_IS_CRITICAL);
+static A9_AO_PCLK(pad,		4,	CLK_IS_CRITICAL);
+static A9_AO_PCLK(rtc_dig,	5,	0);
+static A9_AO_PCLK(irq,		6,	CLK_IS_CRITICAL);
+static A9_AO_PCLK(pwrctrl,	7,	CLK_IS_CRITICAL);
+static A9_AO_PCLK(pwm_a,	8,	0);
+static A9_AO_PCLK(pwm_b,	9,	0);
+static A9_AO_PCLK(pwm_c,	10,	0);
+static A9_AO_PCLK(pwm_d,	11,	0);
+static A9_AO_PCLK(pwm_e,	12,	0);
+static A9_AO_PCLK(pwm_f,	13,	0);
+static A9_AO_PCLK(pwm_g,	14,	0);
+static A9_AO_PCLK(i2c_a,	15,	0);
+static A9_AO_PCLK(i2c_b,	16,	0);
+static A9_AO_PCLK(i2c_c,	17,	0);
+static A9_AO_PCLK(i2c_d,	18,	0);
+static A9_AO_PCLK(sed,		19,	0);
+static A9_AO_PCLK(ir_ctrl,	20,	0);
+static A9_AO_PCLK(uart_b,	21,	0);
+static A9_AO_PCLK(uart_c,	22,	0);
+static A9_AO_PCLK(uart_d,	23,	0);
+static A9_AO_PCLK(uart_e,	24,	0);
+static A9_AO_PCLK(spisg_0,	25,	0);
+static A9_AO_PCLK(rtc_secure,	26,	0);
+static A9_AO_PCLK(cec,		27,	0);
+static A9_AO_PCLK(aocpu,	28,	CLK_IS_CRITICAL);
+static A9_AO_PCLK(sram,		29,	CLK_IS_CRITICAL);
+static A9_AO_PCLK(spisg_1,	30,	0);
+static A9_AO_PCLK(spisg_2,	31,	0);
+
+static const struct clk_parent_data a9_ao_pwm_parents[] = {
+	{ .hw = &a9_ao_xtal.hw },
+	{ .fw_name = "fdiv5", },
+	{ .fw_name = "fdiv4", },
+	{ .fw_name = "fdiv3", }
+};
+
+static A9_COMP_SEL(pwm_a, AO_PWM_CLK_A_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_a, AO_PWM_CLK_A_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_a, AO_PWM_CLK_A_CTRL, 8);
+
+static A9_COMP_SEL(pwm_b, AO_PWM_CLK_B_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_b, AO_PWM_CLK_B_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_b, AO_PWM_CLK_B_CTRL, 8);
+
+static A9_COMP_SEL(pwm_c, AO_PWM_CLK_C_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_c, AO_PWM_CLK_C_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_c, AO_PWM_CLK_C_CTRL, 8);
+
+static A9_COMP_SEL(pwm_d, AO_PWM_CLK_D_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_d, AO_PWM_CLK_D_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_d, AO_PWM_CLK_D_CTRL, 8);
+
+static A9_COMP_SEL(pwm_e, AO_PWM_CLK_E_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_e, AO_PWM_CLK_E_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_e, AO_PWM_CLK_E_CTRL, 8);
+
+static A9_COMP_SEL(pwm_f, AO_PWM_CLK_F_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_f, AO_PWM_CLK_F_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_f, AO_PWM_CLK_F_CTRL, 8);
+
+static A9_COMP_SEL(pwm_g, AO_PWM_CLK_G_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_g, AO_PWM_CLK_G_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_g, AO_PWM_CLK_G_CTRL, 8);
+
+static struct clk_regmap a9_ao_rtc_dualdiv_in = {
+	.data = &(struct clk_regmap_gate_data){
+		.offset = AO_RTC_BY_OSCIN_CTRL0,
+		.bit_idx = 31,
+	},
+	.hw.init = CLK_HW_INIT_HW("ao_rtc_dualdiv_in", &a9_ao_xtal.hw,
+				   &clk_regmap_gate_ops, 0),
+};
+
+static const struct meson_clk_dualdiv_param a9_ao_dualdiv_table[] = {
+	{ 733, 732, 8, 11, 1 },
+	{ /* sentinel */ }
+};
+
+static struct clk_regmap a9_ao_rtc_dualdiv_div = {
+	.data = &(struct meson_clk_dualdiv_data){
+		.n1 = {
+			.reg_off = AO_RTC_BY_OSCIN_CTRL0,
+			.shift   = 0,
+			.width   = 12,
+		},
+		.n2 = {
+			.reg_off = AO_RTC_BY_OSCIN_CTRL0,
+			.shift   = 12,
+			.width   = 12,
+		},
+		.m1 = {
+			.reg_off = AO_RTC_BY_OSCIN_CTRL1,
+			.shift   = 0,
+			.width   = 12,
+		},
+		.m2 = {
+			.reg_off = AO_RTC_BY_OSCIN_CTRL1,
+			.shift   = 12,
+			.width   = 12,
+		},
+		.dual = {
+			.reg_off = AO_RTC_BY_OSCIN_CTRL0,
+			.shift   = 28,
+			.width   = 1,
+		},
+		.table = a9_ao_dualdiv_table,
+	},
+	.hw.init = CLK_HW_INIT_HW("a9_ao_rtc_dualdiv_div", &a9_ao_rtc_dualdiv_in.hw,
+				   &meson_clk_dualdiv_ops, 0),
+};
+
+static struct clk_regmap a9_ao_rtc_dualdiv_sel = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = AO_RTC_BY_OSCIN_CTRL1,
+		.mask = 0x1,
+		.shift = 24,
+	},
+	.hw.init = CLK_HW_INIT_PARENTS_HW("ao_rtc_dualdiv_sel",
+			((const struct clk_hw *[]) {
+				&a9_ao_rtc_dualdiv_div.hw,
+				&a9_ao_rtc_dualdiv_in.hw,
+			}), &clk_regmap_mux_ops, CLK_SET_RATE_PARENT),
+};
+
+static struct clk_regmap a9_ao_rtc_dualdiv = {
+	.data = &(struct clk_regmap_gate_data){
+		.offset = AO_RTC_BY_OSCIN_CTRL0,
+		.bit_idx = 30,
+	},
+	.hw.init = CLK_HW_INIT_HW("ao_rtc_dualdiv", &a9_ao_rtc_dualdiv_sel.hw,
+				   &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
+};
+
+static struct clk_regmap a9_ao_rtc = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = AO_RTC_BY_OSCIN_CTRL1,
+		.mask = 0x1,
+		.shift = 30,
+	},
+	.hw.init = CLK_HW_INIT_PARENTS_HW("ao_rtc",
+			((const struct clk_hw *[]) {
+				&a9_ao_xtal.hw,
+				&a9_ao_rtc_dualdiv.hw,
+			}), &clk_regmap_mux_ops, CLK_SET_RATE_PARENT),
+};
+
+static struct clk_regmap a9_ao_cec_dualdiv_in = {
+	.data = &(struct clk_regmap_gate_data){
+		.offset = AO_CEC_CTRL0,
+		.bit_idx = 31,
+	},
+	.hw.init = CLK_HW_INIT_HW("ao_cec_dualdiv_in", &a9_ao_xtal.hw,
+				   &clk_regmap_gate_ops, 0),
+};
+
+static struct clk_regmap a9_ao_cec_dualdiv_div = {
+	.data = &(struct meson_clk_dualdiv_data){
+		.n1 = {
+			.reg_off = AO_CEC_CTRL0,
+			.shift   = 0,
+			.width   = 12,
+		},
+		.n2 = {
+			.reg_off = AO_CEC_CTRL0,
+			.shift   = 12,
+			.width   = 12,
+		},
+		.m1 = {
+			.reg_off = AO_CEC_CTRL1,
+			.shift   = 0,
+			.width   = 12,
+		},
+		.m2 = {
+			.reg_off = AO_CEC_CTRL1,
+			.shift   = 12,
+			.width   = 12,
+		},
+		.dual = {
+			.reg_off = AO_CEC_CTRL0,
+			.shift   = 28,
+			.width   = 1,
+		},
+		.table = a9_ao_dualdiv_table,
+	},
+	.hw.init = CLK_HW_INIT_HW("ao_cec_dualdiv_div", &a9_ao_cec_dualdiv_in.hw,
+				   &meson_clk_dualdiv_ops, 0),
+};
+
+static struct clk_regmap a9_ao_cec_dualdiv_sel = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = AO_CEC_CTRL1,
+		.mask = 0x1,
+		.shift = 24,
+	},
+	.hw.init = CLK_HW_INIT_PARENTS_HW("ao_cec_dualdiv_sel",
+			((const struct clk_hw *[]) {
+				&a9_ao_cec_dualdiv_div.hw,
+				&a9_ao_cec_dualdiv_in.hw,
+			}), &clk_regmap_mux_ops, CLK_SET_RATE_PARENT),
+};
+
+static struct clk_regmap a9_ao_cec_dualdiv = {
+	.data = &(struct clk_regmap_gate_data){
+		.offset = AO_CEC_CTRL0,
+		.bit_idx = 30,
+	},
+	.hw.init = CLK_HW_INIT_HW("ao_cec_dualdiv", &a9_ao_cec_dualdiv_sel.hw,
+				   &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
+};
+
+static struct clk_regmap a9_ao_cec = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = AO_CEC_CTRL1,
+		.mask = 0x1,
+		.shift = 30,
+	},
+	.hw.init = CLK_HW_INIT_PARENTS_HW("ao_cec",
+			((const struct clk_hw *[]) {
+				&a9_ao_cec_dualdiv.hw,
+				&a9_ao_rtc.hw,
+			}), &clk_regmap_mux_ops, CLK_SET_RATE_PARENT),
+};
+
+static struct clk_hw *a9_ao_hw_clks[] = {
+	[CLKID_AO_XTAL_IN]		= &a9_ao_xtal_in.hw,
+	[CLKID_AO_XTAL]			= &a9_ao_xtal.hw,
+	[CLKID_AO_SYS]			= &a9_ao_sys.hw,
+	[CLKID_AO_SYS_I3C]		= &a9_ao_sys_i2c3.hw,
+	[CLKID_AO_SYS_RTC_REG]		= &a9_ao_sys_rtc_reg.hw,
+	[CLKID_AO_SYS_CLKTREE]		= &a9_ao_sys_clktree.hw,
+	[CLKID_AO_SYS_RST_CTRL]		= &a9_ao_sys_rst_ctrl.hw,
+	[CLKID_AO_SYS_PAD]		= &a9_ao_sys_pad.hw,
+	[CLKID_AO_SYS_RTC_DIG]		= &a9_ao_sys_rtc_dig.hw,
+	[CLKID_AO_SYS_IRQ]		= &a9_ao_sys_irq.hw,
+	[CLKID_AO_SYS_PWRCTRL]		= &a9_ao_sys_pwrctrl.hw,
+	[CLKID_AO_SYS_PWM_A]		= &a9_ao_sys_pwm_a.hw,
+	[CLKID_AO_SYS_PWM_B]		= &a9_ao_sys_pwm_b.hw,
+	[CLKID_AO_SYS_PWM_C]		= &a9_ao_sys_pwm_c.hw,
+	[CLKID_AO_SYS_PWM_D]		= &a9_ao_sys_pwm_d.hw,
+	[CLKID_AO_SYS_PWM_E]		= &a9_ao_sys_pwm_e.hw,
+	[CLKID_AO_SYS_PWM_F]		= &a9_ao_sys_pwm_f.hw,
+	[CLKID_AO_SYS_PWM_G]		= &a9_ao_sys_pwm_g.hw,
+	[CLKID_AO_SYS_I2C_A]		= &a9_ao_sys_i2c_a.hw,
+	[CLKID_AO_SYS_I2C_B]		= &a9_ao_sys_i2c_b.hw,
+	[CLKID_AO_SYS_I2C_C]		= &a9_ao_sys_i2c_c.hw,
+	[CLKID_AO_SYS_I2C_D]		= &a9_ao_sys_i2c_d.hw,
+	[CLKID_AO_SYS_SED]		= &a9_ao_sys_sed.hw,
+	[CLKID_AO_SYS_IR_CTRL]		= &a9_ao_sys_ir_ctrl.hw,
+	[CLKID_AO_SYS_UART_B]		= &a9_ao_sys_uart_b.hw,
+	[CLKID_AO_SYS_UART_C]		= &a9_ao_sys_uart_c.hw,
+	[CLKID_AO_SYS_UART_D]		= &a9_ao_sys_uart_d.hw,
+	[CLKID_AO_SYS_UART_E]		= &a9_ao_sys_uart_e.hw,
+	[CLKID_AO_SYS_SPISG_0]		= &a9_ao_sys_spisg_0.hw,
+	[CLKID_AO_SYS_RTC_SECURE]	= &a9_ao_sys_rtc_secure.hw,
+	[CLKID_AO_SYS_CEC]		= &a9_ao_sys_cec.hw,
+	[CLKID_AO_SYS_AOCPU]		= &a9_ao_sys_aocpu.hw,
+	[CLKID_AO_SYS_SRAM]		= &a9_ao_sys_sram.hw,
+	[CLKID_AO_SYS_SPISG_1]		= &a9_ao_sys_spisg_1.hw,
+	[CLKID_AO_SYS_SPISG_2]		= &a9_ao_sys_spisg_2.hw,
+	[CLKID_AO_PWM_A_SEL]		= &a9_ao_pwm_a_sel.hw,
+	[CLKID_AO_PWM_A_DIV]		= &a9_ao_pwm_a_div.hw,
+	[CLKID_AO_PWM_A]		= &a9_ao_pwm_a.hw,
+	[CLKID_AO_PWM_B_SEL]		= &a9_ao_pwm_b_sel.hw,
+	[CLKID_AO_PWM_B_DIV]		= &a9_ao_pwm_b_div.hw,
+	[CLKID_AO_PWM_B]		= &a9_ao_pwm_b.hw,
+	[CLKID_AO_PWM_C_SEL]		= &a9_ao_pwm_c_sel.hw,
+	[CLKID_AO_PWM_C_DIV]		= &a9_ao_pwm_c_div.hw,
+	[CLKID_AO_PWM_C]		= &a9_ao_pwm_c.hw,
+	[CLKID_AO_PWM_D_SEL]		= &a9_ao_pwm_d_sel.hw,
+	[CLKID_AO_PWM_D_DIV]		= &a9_ao_pwm_d_div.hw,
+	[CLKID_AO_PWM_D]		= &a9_ao_pwm_d.hw,
+	[CLKID_AO_PWM_E_SEL]		= &a9_ao_pwm_e_sel.hw,
+	[CLKID_AO_PWM_E_DIV]		= &a9_ao_pwm_e_div.hw,
+	[CLKID_AO_PWM_E]		= &a9_ao_pwm_e.hw,
+	[CLKID_AO_PWM_F_SEL]		= &a9_ao_pwm_f_sel.hw,
+	[CLKID_AO_PWM_F_DIV]		= &a9_ao_pwm_f_div.hw,
+	[CLKID_AO_PWM_F]		= &a9_ao_pwm_f.hw,
+	[CLKID_AO_PWM_G_SEL]		= &a9_ao_pwm_g_sel.hw,
+	[CLKID_AO_PWM_G_DIV]		= &a9_ao_pwm_g_div.hw,
+	[CLKID_AO_PWM_G]		= &a9_ao_pwm_g.hw,
+	[CLKID_AO_RTC_DUALDIV_IN]	= &a9_ao_rtc_dualdiv_in.hw,
+	[CLKID_AO_RTC_DUALDIV_DIV]	= &a9_ao_rtc_dualdiv_div.hw,
+	[CLKID_AO_RTC_DUALDIV_SEL]	= &a9_ao_rtc_dualdiv_sel.hw,
+	[CLKID_AO_RTC_DUALDIV]		= &a9_ao_rtc_dualdiv.hw,
+	[CLKID_AO_RTC]			= &a9_ao_rtc.hw,
+	[CLKID_AO_CEC_DUALDIV_IN]	= &a9_ao_cec_dualdiv_in.hw,
+	[CLKID_AO_CEC_DUALDIV_DIV]	= &a9_ao_cec_dualdiv_div.hw,
+	[CLKID_AO_CEC_DUALDIV_SEL]	= &a9_ao_cec_dualdiv_sel.hw,
+	[CLKID_AO_CEC_DUALDIV]		= &a9_ao_cec_dualdiv.hw,
+	[CLKID_AO_CEC]			= &a9_ao_cec.hw,
+};
+
+static const struct meson_clkc_data a9_ao_clkc_data = {
+	.hw_clks = {
+		.hws = a9_ao_hw_clks,
+		.num = ARRAY_SIZE(a9_ao_hw_clks),
+	},
+};
+
+static const struct of_device_id a9_ao_clkc_match_table[] = {
+	{
+		.compatible	= "amlogic,a9-aoclkc",
+		.data		= &a9_ao_clkc_data,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, a9_ao_clkc_match_table);
+
+static struct platform_driver a9_ao_clkc_driver = {
+	.probe		= meson_clkc_mmio_probe,
+	.driver		= {
+		.name	= "a9-aoclkc",
+		.of_match_table = a9_ao_clkc_match_table,
+	},
+};
+module_platform_driver(a9_ao_clkc_driver);
+
+MODULE_DESCRIPTION("Amlogic A9 Always-ON Clock Controller driver");
+MODULE_AUTHOR("Jian Hu <jian.hu@amlogic.com>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("CLK_MESON");

-- 
2.47.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver
  2026-06-03 12:17 ` [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver Jian Hu via B4 Relay
@ 2026-06-03 12:34   ` sashiko-bot
  2026-06-03 14:29   ` Jerome Brunet
  1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-06-03 12:34 UTC (permalink / raw)
  To: Jian Hu via B4 Relay
  Cc: neil.armstrong, robh, conor+dt, devicetree, linux-amlogic

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The Kconfig symbol `COMMON_CLK_A9_AO` inappropriately uses `default ARCH_MESON || COMPILE_TEST`. This forces the driver to be enabled by default on all compile-test builds, polluting the default configuration and breaking cross-config consistency.
- [Low] The clock controller driver defines an internal clock as `i2c3` but maps it to `CLKID_AO_SYS_I3C`, indicating a typo where 'i3c' was mistakenly written as 'i2c3'.
--

commit a36917077cf54c71ca4b917e7b2122bd84e2b956
Author: Jian Hu <jian.hu@amlogic.com>

clk: amlogic: Add A9 AO clock controller driver

This commit adds the Always-on clock controller driver for the Amlogic A9
SoC family.

> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index cf8cf3f9e4ee8..625e6788b9406 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
>  	  device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>  	  controller to work.
>  
> +config COMMON_CLK_A9_AO
> +	tristate "Amlogic A9 SoC AO clock controller support"
> +	depends on ARM64
> +	default ARCH_MESON || COMPILE_TEST

[Severity: Low]
Should COMPILE_TEST be used in a default directive here? 

By including it in the default expression, the driver will automatically be
enabled on all compile-test builds, polluting the default configuration. 
Usually, COMPILE_TEST is placed in the 'depends on' clause instead to 
allow build coverage without forcing it on by default.

> +	select COMMON_CLK_MESON_REGMAP
> +	select COMMON_CLK_MESON_CLKC_UTILS

[ ... ]

> diff --git a/drivers/clk/meson/a9-aoclk.c b/drivers/clk/meson/a9-aoclk.c
> new file mode 100644
> index 0000000000000..b7b3ca231a42b
> --- /dev/null
> +++ b/drivers/clk/meson/a9-aoclk.c

[ ... ]

> +/*
> + * A9 integrates a low-power microprocessor (Always-on CPU: AOCPU). Some AO sys
> + * clocks control the AOCPU modules. Mark the AOCPU-related clocks with
> + * CLK_IS_CRITICAL to avoid them being disabled and impacting AOCPU functionality.
> + * AOCPU-related clocks list:
> + * - clktree
> + * - rst_ctrl
> + * - pad
> + * - irq
> + * - pwrctrl
> + * - aocpu
> + * - sram
> + */
> +static A9_AO_PCLK(i2c3,		0,	0);

[Severity: Low]
Is this intended to be an I3C clock rather than I2C? 

Since the driver defines the other I2C controllers alphabetically (i2c_a, 
i2c_b, etc.), and this gets mapped to CLKID_AO_SYS_I3C below, it looks like 
'i2c3' might be a typo for 'i3c'. 

Because of the macro stringification, this typo will cause the Common Clock 
Framework to register the clock with the wrong name.

> +static A9_AO_PCLK(rtc_reg,	1,	0);
> +static A9_AO_PCLK(clktree,	2,	CLK_IS_CRITICAL);

[ ... ]

> +static struct clk_hw *a9_ao_hw_clks[] = {
> +	[CLKID_AO_XTAL_IN]		= &a9_ao_xtal_in.hw,
> +	[CLKID_AO_XTAL]			= &a9_ao_xtal.hw,
> +	[CLKID_AO_SYS]			= &a9_ao_sys.hw,
> +	[CLKID_AO_SYS_I3C]		= &a9_ao_sys_i2c3.hw,

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-a9_aoclk-v2-0-f47ea616ee78@amlogic.com?part=2

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver
  2026-06-03 12:17 ` [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver Jian Hu via B4 Relay
  2026-06-03 12:34   ` sashiko-bot
@ 2026-06-03 14:29   ` Jerome Brunet
  2026-06-10  4:18     ` Jian Hu
  1 sibling, 1 reply; 9+ messages in thread
From: Jerome Brunet @ 2026-06-03 14:29 UTC (permalink / raw)
  To: Jian Hu via B4 Relay
  Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao, Kevin Hilman,
	Martin Blumenstingl, jian.hu, linux-amlogic, linux-clk,
	devicetree, linux-kernel, linux-arm-kernel

On Wed 03 Jun 2026 at 20:17, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@kernel.org> wrote:

> From: Jian Hu <jian.hu@amlogic.com>
>
> Add the Always-on clock controller driver for the Amlogic A9 SoC family.
>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> ---
>  drivers/clk/meson/Kconfig    |  13 ++
>  drivers/clk/meson/Makefile   |   1 +
>  drivers/clk/meson/a9-aoclk.c | 419 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 433 insertions(+)
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index cf8cf3f9e4ee..625e6788b940 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
>  	  device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>  	  controller to work.
>  
> +config COMMON_CLK_A9_AO
> +	tristate "Amlogic A9 SoC AO clock controller support"
> +	depends on ARM64
> +	default ARCH_MESON || COMPILE_TEST
> +	select COMMON_CLK_MESON_REGMAP
> +	select COMMON_CLK_MESON_CLKC_UTILS
> +	select COMMON_CLK_MESON_DUALDIV
> +	imply COMMON_CLK_SCMI
> +	help
> +	  Support for the AO clock controller on Amlogic A311Y3 based
> +	  device, AKA A9.
> +	  Say Y if you want A9 AO clock controller to work.
> +
>  config COMMON_CLK_C3_PLL
>  	tristate "Amlogic C3 PLL clock controller"
>  	depends on ARM64
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index c6719694a242..f89d027c282c 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>  obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>  obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
> +obj-$(CONFIG_COMMON_CLK_A9_AO) += a9-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o
>  obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o
>  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
> diff --git a/drivers/clk/meson/a9-aoclk.c b/drivers/clk/meson/a9-aoclk.c
> new file mode 100644
> index 000000000000..b7b3ca231a42
> --- /dev/null
> +++ b/drivers/clk/meson/a9-aoclk.c
> @@ -0,0 +1,419 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
> +/*
> + * Copyright (C) 2026 Amlogic, Inc. All rights reserved
> + */
> +
> +#include <dt-bindings/clock/amlogic,a9-aoclkc.h>
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +#include "clk-regmap.h"
> +#include "clk-dualdiv.h"
> +#include "meson-clkc-utils.h"
> +
> +#define AO_OSCIN_CTRL			0x00
> +#define AO_SYS_CLK0			0x04
> +#define AO_PWM_CLK_A_CTRL		0x1c
> +#define AO_PWM_CLK_B_CTRL		0x20
> +#define AO_PWM_CLK_C_CTRL		0x24
> +#define AO_PWM_CLK_D_CTRL		0x28
> +#define AO_PWM_CLK_E_CTRL		0x2c
> +#define AO_PWM_CLK_F_CTRL		0x30
> +#define AO_PWM_CLK_G_CTRL		0x34
> +#define AO_CEC_CTRL0			0x38
> +#define AO_CEC_CTRL1			0x3c
> +#define AO_RTC_BY_OSCIN_CTRL0		0x50
> +#define AO_RTC_BY_OSCIN_CTRL1		0x54
> +
> +#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
> +	MESON_COMP_SEL(a9_ao_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
> +
> +#define A9_COMP_DIV(_name, _reg, _shift, _width) \
> +	MESON_COMP_DIV(a9_ao_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
> +
> +#define A9_COMP_GATE(_name, _reg, _bit) \
> +	MESON_COMP_GATE(a9_ao_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
> +
> +static struct clk_regmap a9_ao_xtal_in = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = AO_OSCIN_CTRL,
> +		.bit_idx = 3,
> +	},
> +	/*
> +	 * It may be ao_sys's parent clock, its child clocks mark
> +	 * CLK_IS_CRITICAL, So mark CLK_IS_CRITICAL for it.
> +	 */

I don't really get what you mean ... Could you rephrase ?

> +	.hw.init = CLK_HW_INIT_FW_NAME("ao_xtal_in", "xtal",
> +				       &clk_regmap_gate_ops, CLK_IS_CRITICAL),

I'm honestly not sure about this. It is correct, sure and the macro exist to be
used but ... It does not really help readability here, does it ?

(I know that was a feedback you've got on v1)

Other than that, this looks good to me.

> +};
> +
> +static struct clk_regmap a9_ao_xtal = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = AO_OSCIN_CTRL,
> +		.mask = 0x1,
> +		.shift = 0,
> +	},
> +	/* ext_32k is from external PAD, do not automatically reparent */
> +	.hw.init = CLK_HW_INIT_PARENTS_DATA("ao_xtal",
> +			((const struct clk_parent_data []) {
> +				{ .hw = &a9_ao_xtal_in.hw },
> +				{ .fw_name = "ext_32k" }
> +			}), &clk_regmap_mux_ops, CLK_SET_RATE_NO_REPARENT),
> +};
> +
> +static struct clk_regmap a9_ao_sys = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = AO_OSCIN_CTRL,
> +		.mask = 0x1,
> +		.shift = 1,
> +	},
> +	.hw.init = CLK_HW_INIT_PARENTS_DATA("ao_sys",
> +			((const struct clk_parent_data []) {
> +				{ .hw = &a9_ao_xtal.hw },
> +				{ .fw_name = "sys" }
> +			}), &clk_regmap_mux_ops, 0),
> +};
> +
> +static const struct clk_parent_data a9_ao_pclk_parents = { .hw = &a9_ao_sys.hw };
> +
> +#define A9_AO_PCLK(_name, _bit, _flags)		       \
> +	MESON_PCLK(a9_ao_sys_##_name, AO_SYS_CLK0, _bit, \
> +		   &a9_ao_pclk_parents, _flags)
> +
> +/*
> + * A9 integrates a low-power microprocessor (Always-on CPU: AOCPU). Some AO sys
> + * clocks control the AOCPU modules. Mark the AOCPU-related clocks with
> + * CLK_IS_CRITICAL to avoid them being disabled and impacting AOCPU functionality.
> + * AOCPU-related clocks list:
> + * - clktree
> + * - rst_ctrl
> + * - pad
> + * - irq
> + * - pwrctrl
> + * - aocpu
> + * - sram
> + */
> +static A9_AO_PCLK(i2c3,		0,	0);
> +static A9_AO_PCLK(rtc_reg,	1,	0);
> +static A9_AO_PCLK(clktree,	2,	CLK_IS_CRITICAL);
> +static A9_AO_PCLK(rst_ctrl,	3,	CLK_IS_CRITICAL);
> +static A9_AO_PCLK(pad,		4,	CLK_IS_CRITICAL);
> +static A9_AO_PCLK(rtc_dig,	5,	0);
> +static A9_AO_PCLK(irq,		6,	CLK_IS_CRITICAL);
> +static A9_AO_PCLK(pwrctrl,	7,	CLK_IS_CRITICAL);
> +static A9_AO_PCLK(pwm_a,	8,	0);
> +static A9_AO_PCLK(pwm_b,	9,	0);
> +static A9_AO_PCLK(pwm_c,	10,	0);
> +static A9_AO_PCLK(pwm_d,	11,	0);
> +static A9_AO_PCLK(pwm_e,	12,	0);
> +static A9_AO_PCLK(pwm_f,	13,	0);
> +static A9_AO_PCLK(pwm_g,	14,	0);
> +static A9_AO_PCLK(i2c_a,	15,	0);
> +static A9_AO_PCLK(i2c_b,	16,	0);
> +static A9_AO_PCLK(i2c_c,	17,	0);
> +static A9_AO_PCLK(i2c_d,	18,	0);
> +static A9_AO_PCLK(sed,		19,	0);
> +static A9_AO_PCLK(ir_ctrl,	20,	0);
> +static A9_AO_PCLK(uart_b,	21,	0);
> +static A9_AO_PCLK(uart_c,	22,	0);
> +static A9_AO_PCLK(uart_d,	23,	0);
> +static A9_AO_PCLK(uart_e,	24,	0);
> +static A9_AO_PCLK(spisg_0,	25,	0);
> +static A9_AO_PCLK(rtc_secure,	26,	0);
> +static A9_AO_PCLK(cec,		27,	0);
> +static A9_AO_PCLK(aocpu,	28,	CLK_IS_CRITICAL);
> +static A9_AO_PCLK(sram,		29,	CLK_IS_CRITICAL);
> +static A9_AO_PCLK(spisg_1,	30,	0);
> +static A9_AO_PCLK(spisg_2,	31,	0);
> +
> +static const struct clk_parent_data a9_ao_pwm_parents[] = {
> +	{ .hw = &a9_ao_xtal.hw },
> +	{ .fw_name = "fdiv5", },
> +	{ .fw_name = "fdiv4", },
> +	{ .fw_name = "fdiv3", }
> +};
> +
> +static A9_COMP_SEL(pwm_a, AO_PWM_CLK_A_CTRL, 9, 0x7, a9_ao_pwm_parents);
> +static A9_COMP_DIV(pwm_a, AO_PWM_CLK_A_CTRL, 0, 8);
> +static A9_COMP_GATE(pwm_a, AO_PWM_CLK_A_CTRL, 8);
> +
> +static A9_COMP_SEL(pwm_b, AO_PWM_CLK_B_CTRL, 9, 0x7, a9_ao_pwm_parents);
> +static A9_COMP_DIV(pwm_b, AO_PWM_CLK_B_CTRL, 0, 8);
> +static A9_COMP_GATE(pwm_b, AO_PWM_CLK_B_CTRL, 8);
> +
> +static A9_COMP_SEL(pwm_c, AO_PWM_CLK_C_CTRL, 9, 0x7, a9_ao_pwm_parents);
> +static A9_COMP_DIV(pwm_c, AO_PWM_CLK_C_CTRL, 0, 8);
> +static A9_COMP_GATE(pwm_c, AO_PWM_CLK_C_CTRL, 8);
> +
> +static A9_COMP_SEL(pwm_d, AO_PWM_CLK_D_CTRL, 9, 0x7, a9_ao_pwm_parents);
> +static A9_COMP_DIV(pwm_d, AO_PWM_CLK_D_CTRL, 0, 8);
> +static A9_COMP_GATE(pwm_d, AO_PWM_CLK_D_CTRL, 8);
> +
> +static A9_COMP_SEL(pwm_e, AO_PWM_CLK_E_CTRL, 9, 0x7, a9_ao_pwm_parents);
> +static A9_COMP_DIV(pwm_e, AO_PWM_CLK_E_CTRL, 0, 8);
> +static A9_COMP_GATE(pwm_e, AO_PWM_CLK_E_CTRL, 8);
> +
> +static A9_COMP_SEL(pwm_f, AO_PWM_CLK_F_CTRL, 9, 0x7, a9_ao_pwm_parents);
> +static A9_COMP_DIV(pwm_f, AO_PWM_CLK_F_CTRL, 0, 8);
> +static A9_COMP_GATE(pwm_f, AO_PWM_CLK_F_CTRL, 8);
> +
> +static A9_COMP_SEL(pwm_g, AO_PWM_CLK_G_CTRL, 9, 0x7, a9_ao_pwm_parents);
> +static A9_COMP_DIV(pwm_g, AO_PWM_CLK_G_CTRL, 0, 8);
> +static A9_COMP_GATE(pwm_g, AO_PWM_CLK_G_CTRL, 8);
> +
> +static struct clk_regmap a9_ao_rtc_dualdiv_in = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = AO_RTC_BY_OSCIN_CTRL0,
> +		.bit_idx = 31,
> +	},
> +	.hw.init = CLK_HW_INIT_HW("ao_rtc_dualdiv_in", &a9_ao_xtal.hw,
> +				   &clk_regmap_gate_ops, 0),
> +};
> +
> +static const struct meson_clk_dualdiv_param a9_ao_dualdiv_table[] = {
> +	{ 733, 732, 8, 11, 1 },
> +	{ /* sentinel */ }
> +};
> +
> +static struct clk_regmap a9_ao_rtc_dualdiv_div = {
> +	.data = &(struct meson_clk_dualdiv_data){
> +		.n1 = {
> +			.reg_off = AO_RTC_BY_OSCIN_CTRL0,
> +			.shift   = 0,
> +			.width   = 12,
> +		},
> +		.n2 = {
> +			.reg_off = AO_RTC_BY_OSCIN_CTRL0,
> +			.shift   = 12,
> +			.width   = 12,
> +		},
> +		.m1 = {
> +			.reg_off = AO_RTC_BY_OSCIN_CTRL1,
> +			.shift   = 0,
> +			.width   = 12,
> +		},
> +		.m2 = {
> +			.reg_off = AO_RTC_BY_OSCIN_CTRL1,
> +			.shift   = 12,
> +			.width   = 12,
> +		},
> +		.dual = {
> +			.reg_off = AO_RTC_BY_OSCIN_CTRL0,
> +			.shift   = 28,
> +			.width   = 1,
> +		},
> +		.table = a9_ao_dualdiv_table,
> +	},
> +	.hw.init = CLK_HW_INIT_HW("a9_ao_rtc_dualdiv_div", &a9_ao_rtc_dualdiv_in.hw,
> +				   &meson_clk_dualdiv_ops, 0),
> +};
> +
> +static struct clk_regmap a9_ao_rtc_dualdiv_sel = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = AO_RTC_BY_OSCIN_CTRL1,
> +		.mask = 0x1,
> +		.shift = 24,
> +	},
> +	.hw.init = CLK_HW_INIT_PARENTS_HW("ao_rtc_dualdiv_sel",
> +			((const struct clk_hw *[]) {
> +				&a9_ao_rtc_dualdiv_div.hw,
> +				&a9_ao_rtc_dualdiv_in.hw,
> +			}), &clk_regmap_mux_ops, CLK_SET_RATE_PARENT),
> +};
> +
> +static struct clk_regmap a9_ao_rtc_dualdiv = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = AO_RTC_BY_OSCIN_CTRL0,
> +		.bit_idx = 30,
> +	},
> +	.hw.init = CLK_HW_INIT_HW("ao_rtc_dualdiv", &a9_ao_rtc_dualdiv_sel.hw,
> +				   &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
> +};
> +
> +static struct clk_regmap a9_ao_rtc = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = AO_RTC_BY_OSCIN_CTRL1,
> +		.mask = 0x1,
> +		.shift = 30,
> +	},
> +	.hw.init = CLK_HW_INIT_PARENTS_HW("ao_rtc",
> +			((const struct clk_hw *[]) {
> +				&a9_ao_xtal.hw,
> +				&a9_ao_rtc_dualdiv.hw,
> +			}), &clk_regmap_mux_ops, CLK_SET_RATE_PARENT),
> +};
> +
> +static struct clk_regmap a9_ao_cec_dualdiv_in = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = AO_CEC_CTRL0,
> +		.bit_idx = 31,
> +	},
> +	.hw.init = CLK_HW_INIT_HW("ao_cec_dualdiv_in", &a9_ao_xtal.hw,
> +				   &clk_regmap_gate_ops, 0),
> +};
> +
> +static struct clk_regmap a9_ao_cec_dualdiv_div = {
> +	.data = &(struct meson_clk_dualdiv_data){
> +		.n1 = {
> +			.reg_off = AO_CEC_CTRL0,
> +			.shift   = 0,
> +			.width   = 12,
> +		},
> +		.n2 = {
> +			.reg_off = AO_CEC_CTRL0,
> +			.shift   = 12,
> +			.width   = 12,
> +		},
> +		.m1 = {
> +			.reg_off = AO_CEC_CTRL1,
> +			.shift   = 0,
> +			.width   = 12,
> +		},
> +		.m2 = {
> +			.reg_off = AO_CEC_CTRL1,
> +			.shift   = 12,
> +			.width   = 12,
> +		},
> +		.dual = {
> +			.reg_off = AO_CEC_CTRL0,
> +			.shift   = 28,
> +			.width   = 1,
> +		},
> +		.table = a9_ao_dualdiv_table,
> +	},
> +	.hw.init = CLK_HW_INIT_HW("ao_cec_dualdiv_div", &a9_ao_cec_dualdiv_in.hw,
> +				   &meson_clk_dualdiv_ops, 0),
> +};
> +
> +static struct clk_regmap a9_ao_cec_dualdiv_sel = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = AO_CEC_CTRL1,
> +		.mask = 0x1,
> +		.shift = 24,
> +	},
> +	.hw.init = CLK_HW_INIT_PARENTS_HW("ao_cec_dualdiv_sel",
> +			((const struct clk_hw *[]) {
> +				&a9_ao_cec_dualdiv_div.hw,
> +				&a9_ao_cec_dualdiv_in.hw,
> +			}), &clk_regmap_mux_ops, CLK_SET_RATE_PARENT),
> +};
> +
> +static struct clk_regmap a9_ao_cec_dualdiv = {
> +	.data = &(struct clk_regmap_gate_data){
> +		.offset = AO_CEC_CTRL0,
> +		.bit_idx = 30,
> +	},
> +	.hw.init = CLK_HW_INIT_HW("ao_cec_dualdiv", &a9_ao_cec_dualdiv_sel.hw,
> +				   &clk_regmap_gate_ops, CLK_SET_RATE_PARENT),
> +};
> +
> +static struct clk_regmap a9_ao_cec = {
> +	.data = &(struct clk_regmap_mux_data) {
> +		.offset = AO_CEC_CTRL1,
> +		.mask = 0x1,
> +		.shift = 30,
> +	},
> +	.hw.init = CLK_HW_INIT_PARENTS_HW("ao_cec",
> +			((const struct clk_hw *[]) {
> +				&a9_ao_cec_dualdiv.hw,
> +				&a9_ao_rtc.hw,
> +			}), &clk_regmap_mux_ops, CLK_SET_RATE_PARENT),
> +};
> +
> +static struct clk_hw *a9_ao_hw_clks[] = {
> +	[CLKID_AO_XTAL_IN]		= &a9_ao_xtal_in.hw,
> +	[CLKID_AO_XTAL]			= &a9_ao_xtal.hw,
> +	[CLKID_AO_SYS]			= &a9_ao_sys.hw,
> +	[CLKID_AO_SYS_I3C]		= &a9_ao_sys_i2c3.hw,
> +	[CLKID_AO_SYS_RTC_REG]		= &a9_ao_sys_rtc_reg.hw,
> +	[CLKID_AO_SYS_CLKTREE]		= &a9_ao_sys_clktree.hw,
> +	[CLKID_AO_SYS_RST_CTRL]		= &a9_ao_sys_rst_ctrl.hw,
> +	[CLKID_AO_SYS_PAD]		= &a9_ao_sys_pad.hw,
> +	[CLKID_AO_SYS_RTC_DIG]		= &a9_ao_sys_rtc_dig.hw,
> +	[CLKID_AO_SYS_IRQ]		= &a9_ao_sys_irq.hw,
> +	[CLKID_AO_SYS_PWRCTRL]		= &a9_ao_sys_pwrctrl.hw,
> +	[CLKID_AO_SYS_PWM_A]		= &a9_ao_sys_pwm_a.hw,
> +	[CLKID_AO_SYS_PWM_B]		= &a9_ao_sys_pwm_b.hw,
> +	[CLKID_AO_SYS_PWM_C]		= &a9_ao_sys_pwm_c.hw,
> +	[CLKID_AO_SYS_PWM_D]		= &a9_ao_sys_pwm_d.hw,
> +	[CLKID_AO_SYS_PWM_E]		= &a9_ao_sys_pwm_e.hw,
> +	[CLKID_AO_SYS_PWM_F]		= &a9_ao_sys_pwm_f.hw,
> +	[CLKID_AO_SYS_PWM_G]		= &a9_ao_sys_pwm_g.hw,
> +	[CLKID_AO_SYS_I2C_A]		= &a9_ao_sys_i2c_a.hw,
> +	[CLKID_AO_SYS_I2C_B]		= &a9_ao_sys_i2c_b.hw,
> +	[CLKID_AO_SYS_I2C_C]		= &a9_ao_sys_i2c_c.hw,
> +	[CLKID_AO_SYS_I2C_D]		= &a9_ao_sys_i2c_d.hw,
> +	[CLKID_AO_SYS_SED]		= &a9_ao_sys_sed.hw,
> +	[CLKID_AO_SYS_IR_CTRL]		= &a9_ao_sys_ir_ctrl.hw,
> +	[CLKID_AO_SYS_UART_B]		= &a9_ao_sys_uart_b.hw,
> +	[CLKID_AO_SYS_UART_C]		= &a9_ao_sys_uart_c.hw,
> +	[CLKID_AO_SYS_UART_D]		= &a9_ao_sys_uart_d.hw,
> +	[CLKID_AO_SYS_UART_E]		= &a9_ao_sys_uart_e.hw,
> +	[CLKID_AO_SYS_SPISG_0]		= &a9_ao_sys_spisg_0.hw,
> +	[CLKID_AO_SYS_RTC_SECURE]	= &a9_ao_sys_rtc_secure.hw,
> +	[CLKID_AO_SYS_CEC]		= &a9_ao_sys_cec.hw,
> +	[CLKID_AO_SYS_AOCPU]		= &a9_ao_sys_aocpu.hw,
> +	[CLKID_AO_SYS_SRAM]		= &a9_ao_sys_sram.hw,
> +	[CLKID_AO_SYS_SPISG_1]		= &a9_ao_sys_spisg_1.hw,
> +	[CLKID_AO_SYS_SPISG_2]		= &a9_ao_sys_spisg_2.hw,
> +	[CLKID_AO_PWM_A_SEL]		= &a9_ao_pwm_a_sel.hw,
> +	[CLKID_AO_PWM_A_DIV]		= &a9_ao_pwm_a_div.hw,
> +	[CLKID_AO_PWM_A]		= &a9_ao_pwm_a.hw,
> +	[CLKID_AO_PWM_B_SEL]		= &a9_ao_pwm_b_sel.hw,
> +	[CLKID_AO_PWM_B_DIV]		= &a9_ao_pwm_b_div.hw,
> +	[CLKID_AO_PWM_B]		= &a9_ao_pwm_b.hw,
> +	[CLKID_AO_PWM_C_SEL]		= &a9_ao_pwm_c_sel.hw,
> +	[CLKID_AO_PWM_C_DIV]		= &a9_ao_pwm_c_div.hw,
> +	[CLKID_AO_PWM_C]		= &a9_ao_pwm_c.hw,
> +	[CLKID_AO_PWM_D_SEL]		= &a9_ao_pwm_d_sel.hw,
> +	[CLKID_AO_PWM_D_DIV]		= &a9_ao_pwm_d_div.hw,
> +	[CLKID_AO_PWM_D]		= &a9_ao_pwm_d.hw,
> +	[CLKID_AO_PWM_E_SEL]		= &a9_ao_pwm_e_sel.hw,
> +	[CLKID_AO_PWM_E_DIV]		= &a9_ao_pwm_e_div.hw,
> +	[CLKID_AO_PWM_E]		= &a9_ao_pwm_e.hw,
> +	[CLKID_AO_PWM_F_SEL]		= &a9_ao_pwm_f_sel.hw,
> +	[CLKID_AO_PWM_F_DIV]		= &a9_ao_pwm_f_div.hw,
> +	[CLKID_AO_PWM_F]		= &a9_ao_pwm_f.hw,
> +	[CLKID_AO_PWM_G_SEL]		= &a9_ao_pwm_g_sel.hw,
> +	[CLKID_AO_PWM_G_DIV]		= &a9_ao_pwm_g_div.hw,
> +	[CLKID_AO_PWM_G]		= &a9_ao_pwm_g.hw,
> +	[CLKID_AO_RTC_DUALDIV_IN]	= &a9_ao_rtc_dualdiv_in.hw,
> +	[CLKID_AO_RTC_DUALDIV_DIV]	= &a9_ao_rtc_dualdiv_div.hw,
> +	[CLKID_AO_RTC_DUALDIV_SEL]	= &a9_ao_rtc_dualdiv_sel.hw,
> +	[CLKID_AO_RTC_DUALDIV]		= &a9_ao_rtc_dualdiv.hw,
> +	[CLKID_AO_RTC]			= &a9_ao_rtc.hw,
> +	[CLKID_AO_CEC_DUALDIV_IN]	= &a9_ao_cec_dualdiv_in.hw,
> +	[CLKID_AO_CEC_DUALDIV_DIV]	= &a9_ao_cec_dualdiv_div.hw,
> +	[CLKID_AO_CEC_DUALDIV_SEL]	= &a9_ao_cec_dualdiv_sel.hw,
> +	[CLKID_AO_CEC_DUALDIV]		= &a9_ao_cec_dualdiv.hw,
> +	[CLKID_AO_CEC]			= &a9_ao_cec.hw,
> +};
> +
> +static const struct meson_clkc_data a9_ao_clkc_data = {
> +	.hw_clks = {
> +		.hws = a9_ao_hw_clks,
> +		.num = ARRAY_SIZE(a9_ao_hw_clks),
> +	},
> +};
> +
> +static const struct of_device_id a9_ao_clkc_match_table[] = {
> +	{
> +		.compatible	= "amlogic,a9-aoclkc",
> +		.data		= &a9_ao_clkc_data,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, a9_ao_clkc_match_table);
> +
> +static struct platform_driver a9_ao_clkc_driver = {
> +	.probe		= meson_clkc_mmio_probe,
> +	.driver		= {
> +		.name	= "a9-aoclkc",
> +		.of_match_table = a9_ao_clkc_match_table,
> +	},
> +};
> +module_platform_driver(a9_ao_clkc_driver);
> +
> +MODULE_DESCRIPTION("Amlogic A9 Always-ON Clock Controller driver");
> +MODULE_AUTHOR("Jian Hu <jian.hu@amlogic.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("CLK_MESON");

-- 
Jerome

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH v2 1/2] dt-bindings: clock: Add Amlogic A9 AO clock controller
  2026-06-03 12:17 ` [PATCH v2 1/2] dt-bindings: clock: Add Amlogic " Jian Hu via B4 Relay
@ 2026-06-03 15:51   ` Conor Dooley
  0 siblings, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2026-06-03 15:51 UTC (permalink / raw)
  To: jian.hu
  Cc: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao,
	Kevin Hilman, Martin Blumenstingl, linux-amlogic, linux-clk,
	devicetree, linux-kernel, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

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

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver
  2026-06-03 14:29   ` Jerome Brunet
@ 2026-06-10  4:18     ` Jian Hu
  2026-06-10 12:26       ` Jerome Brunet
  0 siblings, 1 reply; 9+ messages in thread
From: Jian Hu @ 2026-06-10  4:18 UTC (permalink / raw)
  To: Jerome Brunet, Jian Hu via B4 Relay
  Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao, Kevin Hilman,
	Martin Blumenstingl, linux-amlogic, linux-clk, devicetree,
	linux-kernel, linux-arm-kernel

Hi Jerome,

Thanks for your review

On 6/3/2026 10:29 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Wed 03 Jun 2026 at 20:17, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@kernel.org> wrote:
>
>> From: Jian Hu <jian.hu@amlogic.com>
>>
>> Add the Always-on clock controller driver for the Amlogic A9 SoC family.
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>>   drivers/clk/meson/Kconfig    |  13 ++
>>   drivers/clk/meson/Makefile   |   1 +
>>   drivers/clk/meson/a9-aoclk.c | 419 +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 433 insertions(+)
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index cf8cf3f9e4ee..625e6788b940 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
>>          device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>>          controller to work.
>>
>> +config COMMON_CLK_A9_AO
>> +     tristate "Amlogic A9 SoC AO clock controller support"
>> +     depends on ARM64
>> +     default ARCH_MESON || COMPILE_TEST
>> +     select COMMON_CLK_MESON_REGMAP
>> +     select COMMON_CLK_MESON_CLKC_UTILS
>> +     select COMMON_CLK_MESON_DUALDIV
>> +     imply COMMON_CLK_SCMI
>> +     help
>> +       Support for the AO clock controller on Amlogic A311Y3 based
>> +       device, AKA A9.
>> +       Say Y if you want A9 AO clock controller to work.
>> +
>>   config COMMON_CLK_C3_PLL
>>        tristate "Amlogic C3 PLL clock controller"
>>        depends on ARM64
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index c6719694a242..f89d027c282c 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>   obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>   obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>>   obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>> +obj-$(CONFIG_COMMON_CLK_A9_AO) += a9-aoclk.o
>>   obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o
>>   obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o
>>   obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>> diff --git a/drivers/clk/meson/a9-aoclk.c b/drivers/clk/meson/a9-aoclk.c
>> new file mode 100644
>> index 000000000000..b7b3ca231a42
>> --- /dev/null
>> +++ b/drivers/clk/meson/a9-aoclk.c
>> @@ -0,0 +1,419 @@
>> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
>> +/*
>> + * Copyright (C) 2026 Amlogic, Inc. All rights reserved
>> + */
>> +
>> +#include <dt-bindings/clock/amlogic,a9-aoclkc.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>> +#include "clk-regmap.h"
>> +#include "clk-dualdiv.h"
>> +#include "meson-clkc-utils.h"
>> +
>> +#define AO_OSCIN_CTRL                        0x00
>> +#define AO_SYS_CLK0                  0x04
>> +#define AO_PWM_CLK_A_CTRL            0x1c
>> +#define AO_PWM_CLK_B_CTRL            0x20
>> +#define AO_PWM_CLK_C_CTRL            0x24
>> +#define AO_PWM_CLK_D_CTRL            0x28
>> +#define AO_PWM_CLK_E_CTRL            0x2c
>> +#define AO_PWM_CLK_F_CTRL            0x30
>> +#define AO_PWM_CLK_G_CTRL            0x34
>> +#define AO_CEC_CTRL0                 0x38
>> +#define AO_CEC_CTRL1                 0x3c
>> +#define AO_RTC_BY_OSCIN_CTRL0                0x50
>> +#define AO_RTC_BY_OSCIN_CTRL1                0x54
>> +
>> +#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
>> +     MESON_COMP_SEL(a9_ao_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
>> +
>> +#define A9_COMP_DIV(_name, _reg, _shift, _width) \
>> +     MESON_COMP_DIV(a9_ao_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
>> +
>> +#define A9_COMP_GATE(_name, _reg, _bit) \
>> +     MESON_COMP_GATE(a9_ao_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
>> +
>> +static struct clk_regmap a9_ao_xtal_in = {
>> +     .data = &(struct clk_regmap_gate_data){
>> +             .offset = AO_OSCIN_CTRL,
>> +             .bit_idx = 3,
>> +     },
>> +     /*
>> +      * It may be ao_sys's parent clock, its child clocks mark
>> +      * CLK_IS_CRITICAL, So mark CLK_IS_CRITICAL for it.
>> +      */
> I don't really get what you mean ... Could you rephrase ?


The AO sys gate clock chain may be:

ao_xtal_in->ao_xtal->ao_sys-> AO sys gate clocks

"ao_xtal_in" is part of the parent chain of the AO sys gate clocks.

Some of its downstream clocks are marked with CLK_IS_CRITICAL. To ensure
those clocks remain functional, ao_xtal_in must not be disabled and is
therefore marked as CLK_IS_CRITICAL as well.


I will rephrase it like this in the next version:

         /*
          * ao_sys can select different clock sources. One possible 
clock path is:
          *      ao_xtal_in->ao_xtal->ao_sys-> ao sys gate clocks
          *
          * ao_xtal_in is in the parent chain of AO sys gate clocks.
          * Since some downstream clocks are marked CLK_IS_CRITICAL,
          * ao_xtal_in must remain enabled and is therefore marked
          * CLK_IS_CRITICAL as well.
          */

>> +     .hw.init = CLK_HW_INIT_FW_NAME("ao_xtal_in", "xtal",
>> +                                    &clk_regmap_gate_ops, CLK_IS_CRITICAL),
> I'm honestly not sure about this. It is correct, sure and the macro exist to be
> used but ... It does not really help readability here, does it ?
>
> (I know that was a feedback you've got on v1)
>
> Other than that, this looks good to me.
>
Ok, I will use the original clk_init_data for this one.


[ ... ]

> --
> Jerome

--

Jian



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver
  2026-06-10  4:18     ` Jian Hu
@ 2026-06-10 12:26       ` Jerome Brunet
  2026-06-11 12:24         ` Jian Hu
  0 siblings, 1 reply; 9+ messages in thread
From: Jerome Brunet @ 2026-06-10 12:26 UTC (permalink / raw)
  To: Jian Hu
  Cc: Jian Hu via B4 Relay, Neil Armstrong, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Xianwei Zhao, Kevin Hilman, Martin Blumenstingl, linux-amlogic,
	linux-clk, devicetree, linux-kernel, linux-arm-kernel

On mer. 10 juin 2026 at 12:18, Jian Hu <jian.hu@amlogic.com> wrote:

> Hi Jerome,
>
> Thanks for your review
>
> On 6/3/2026 10:29 PM, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On Wed 03 Jun 2026 at 20:17, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@kernel.org> wrote:
>>
>>> From: Jian Hu <jian.hu@amlogic.com>
>>>
>>> Add the Always-on clock controller driver for the Amlogic A9 SoC family.
>>>
>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>> ---
>>>   drivers/clk/meson/Kconfig    |  13 ++
>>>   drivers/clk/meson/Makefile   |   1 +
>>>   drivers/clk/meson/a9-aoclk.c | 419 +++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 433 insertions(+)
>>>
>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>> index cf8cf3f9e4ee..625e6788b940 100644
>>> --- a/drivers/clk/meson/Kconfig
>>> +++ b/drivers/clk/meson/Kconfig
>>> @@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
>>>          device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>>>          controller to work.
>>>
>>> +config COMMON_CLK_A9_AO
>>> +     tristate "Amlogic A9 SoC AO clock controller support"
>>> +     depends on ARM64
>>> +     default ARCH_MESON || COMPILE_TEST
>>> +     select COMMON_CLK_MESON_REGMAP
>>> +     select COMMON_CLK_MESON_CLKC_UTILS
>>> +     select COMMON_CLK_MESON_DUALDIV
>>> +     imply COMMON_CLK_SCMI
>>> +     help
>>> +       Support for the AO clock controller on Amlogic A311Y3 based
>>> +       device, AKA A9.
>>> +       Say Y if you want A9 AO clock controller to work.
>>> +
>>>   config COMMON_CLK_C3_PLL
>>>        tristate "Amlogic C3 PLL clock controller"
>>>        depends on ARM64
>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>> index c6719694a242..f89d027c282c 100644
>>> --- a/drivers/clk/meson/Makefile
>>> +++ b/drivers/clk/meson/Makefile
>>> @@ -19,6 +19,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>>   obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>>   obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>>>   obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>>> +obj-$(CONFIG_COMMON_CLK_A9_AO) += a9-aoclk.o
>>>   obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o
>>>   obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o
>>>   obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>> diff --git a/drivers/clk/meson/a9-aoclk.c b/drivers/clk/meson/a9-aoclk.c
>>> new file mode 100644
>>> index 000000000000..b7b3ca231a42
>>> --- /dev/null
>>> +++ b/drivers/clk/meson/a9-aoclk.c
>>> @@ -0,0 +1,419 @@
>>> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
>>> +/*
>>> + * Copyright (C) 2026 Amlogic, Inc. All rights reserved
>>> + */
>>> +
>>> +#include <dt-bindings/clock/amlogic,a9-aoclkc.h>
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/platform_device.h>
>>> +#include "clk-regmap.h"
>>> +#include "clk-dualdiv.h"
>>> +#include "meson-clkc-utils.h"
>>> +
>>> +#define AO_OSCIN_CTRL                        0x00
>>> +#define AO_SYS_CLK0                  0x04
>>> +#define AO_PWM_CLK_A_CTRL            0x1c
>>> +#define AO_PWM_CLK_B_CTRL            0x20
>>> +#define AO_PWM_CLK_C_CTRL            0x24
>>> +#define AO_PWM_CLK_D_CTRL            0x28
>>> +#define AO_PWM_CLK_E_CTRL            0x2c
>>> +#define AO_PWM_CLK_F_CTRL            0x30
>>> +#define AO_PWM_CLK_G_CTRL            0x34
>>> +#define AO_CEC_CTRL0                 0x38
>>> +#define AO_CEC_CTRL1                 0x3c
>>> +#define AO_RTC_BY_OSCIN_CTRL0                0x50
>>> +#define AO_RTC_BY_OSCIN_CTRL1                0x54
>>> +
>>> +#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
>>> +     MESON_COMP_SEL(a9_ao_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
>>> +
>>> +#define A9_COMP_DIV(_name, _reg, _shift, _width) \
>>> +     MESON_COMP_DIV(a9_ao_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
>>> +
>>> +#define A9_COMP_GATE(_name, _reg, _bit) \
>>> +     MESON_COMP_GATE(a9_ao_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
>>> +
>>> +static struct clk_regmap a9_ao_xtal_in = {
>>> +     .data = &(struct clk_regmap_gate_data){
>>> +             .offset = AO_OSCIN_CTRL,
>>> +             .bit_idx = 3,
>>> +     },
>>> +     /*
>>> +      * It may be ao_sys's parent clock, its child clocks mark
>>> +      * CLK_IS_CRITICAL, So mark CLK_IS_CRITICAL for it.
>>> +      */
>> I don't really get what you mean ... Could you rephrase ?
>
>
> The AO sys gate clock chain may be:
>
> ao_xtal_in->ao_xtal->ao_sys-> AO sys gate clocks
>
> "ao_xtal_in" is part of the parent chain of the AO sys gate clocks.
>
> Some of its downstream clocks are marked with CLK_IS_CRITICAL. To ensure
> those clocks remain functional, ao_xtal_in must not be disabled and is
> therefore marked as CLK_IS_CRITICAL as well.

If any of the downstream clocks are critical and marked as such, there is not
need to mark this one as well.

You should only mark the clocks that are actually critical with the flag
and let CCF figure out the dependencies.

>
>
> I will rephrase it like this in the next version:
>
>         /*
>          * ao_sys can select different clock sources. One possible clock
> path is:
>          *      ao_xtal_in->ao_xtal->ao_sys-> ao sys gate clocks
>          *
>          * ao_xtal_in is in the parent chain of AO sys gate clocks.
>          * Since some downstream clocks are marked CLK_IS_CRITICAL,
>          * ao_xtal_in must remain enabled and is therefore marked
>          * CLK_IS_CRITICAL as well.
>          */
>
>>> +     .hw.init = CLK_HW_INIT_FW_NAME("ao_xtal_in", "xtal",
>>> +                                    &clk_regmap_gate_ops, CLK_IS_CRITICAL),
>> I'm honestly not sure about this. It is correct, sure and the macro exist to be
>> used but ... It does not really help readability here, does it ?
>>
>> (I know that was a feedback you've got on v1)
>>
>> Other than that, this looks good to me.
>>
> Ok, I will use the original clk_init_data for this one.

Well my comment applies to whole thing really.

There are surely ways in which the macro but the way we statically
declare things, it adds a level of indirection that makes things harder
to review IMO.

>
>
> [ ... ]
>
>> --
>> Jerome

-- 
Jerome

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver
  2026-06-10 12:26       ` Jerome Brunet
@ 2026-06-11 12:24         ` Jian Hu
  0 siblings, 0 replies; 9+ messages in thread
From: Jian Hu @ 2026-06-11 12:24 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Jian Hu via B4 Relay, Neil Armstrong, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Xianwei Zhao, Kevin Hilman, Martin Blumenstingl, linux-amlogic,
	linux-clk, devicetree, linux-kernel, linux-arm-kernel

On 6/10/2026 8:26 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On mer. 10 juin 2026 at 12:18, Jian Hu <jian.hu@amlogic.com> wrote:
>
>> Hi Jerome,
>>
>> Thanks for your review
>>
>> On 6/3/2026 10:29 PM, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On Wed 03 Jun 2026 at 20:17, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@kernel.org> wrote:
>>>
>>>> From: Jian Hu <jian.hu@amlogic.com>
>>>>
>>>> Add the Always-on clock controller driver for the Amlogic A9 SoC family.
>>>>
>>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>>> ---
>>>>    drivers/clk/meson/Kconfig    |  13 ++
>>>>    drivers/clk/meson/Makefile   |   1 +
>>>>    drivers/clk/meson/a9-aoclk.c | 419 +++++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 433 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>>> index cf8cf3f9e4ee..625e6788b940 100644
>>>> --- a/drivers/clk/meson/Kconfig
>>>> +++ b/drivers/clk/meson/Kconfig
>>>> @@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
>>>>           device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>>>>           controller to work.
>>>>
>>>> +config COMMON_CLK_A9_AO
>>>> +     tristate "Amlogic A9 SoC AO clock controller support"
>>>> +     depends on ARM64
>>>> +     default ARCH_MESON || COMPILE_TEST
>>>> +     select COMMON_CLK_MESON_REGMAP
>>>> +     select COMMON_CLK_MESON_CLKC_UTILS
>>>> +     select COMMON_CLK_MESON_DUALDIV
>>>> +     imply COMMON_CLK_SCMI
>>>> +     help
>>>> +       Support for the AO clock controller on Amlogic A311Y3 based
>>>> +       device, AKA A9.
>>>> +       Say Y if you want A9 AO clock controller to work.
>>>> +
>>>>    config COMMON_CLK_C3_PLL
>>>>         tristate "Amlogic C3 PLL clock controller"
>>>>         depends on ARM64
>>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>>> index c6719694a242..f89d027c282c 100644
>>>> --- a/drivers/clk/meson/Makefile
>>>> +++ b/drivers/clk/meson/Makefile
>>>> @@ -19,6 +19,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>>>    obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>>>    obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>>>>    obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>>>> +obj-$(CONFIG_COMMON_CLK_A9_AO) += a9-aoclk.o
>>>>    obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o
>>>>    obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o
>>>>    obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>>> diff --git a/drivers/clk/meson/a9-aoclk.c b/drivers/clk/meson/a9-aoclk.c
>>>> new file mode 100644
>>>> index 000000000000..b7b3ca231a42
>>>> --- /dev/null
>>>> +++ b/drivers/clk/meson/a9-aoclk.c
>>>> @@ -0,0 +1,419 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
>>>> +/*
>>>> + * Copyright (C) 2026 Amlogic, Inc. All rights reserved
>>>> + */
>>>> +
>>>> +#include <dt-bindings/clock/amlogic,a9-aoclkc.h>
>>>> +#include <linux/clk-provider.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include "clk-regmap.h"
>>>> +#include "clk-dualdiv.h"
>>>> +#include "meson-clkc-utils.h"
>>>> +
>>>> +#define AO_OSCIN_CTRL                        0x00
>>>> +#define AO_SYS_CLK0                  0x04
>>>> +#define AO_PWM_CLK_A_CTRL            0x1c
>>>> +#define AO_PWM_CLK_B_CTRL            0x20
>>>> +#define AO_PWM_CLK_C_CTRL            0x24
>>>> +#define AO_PWM_CLK_D_CTRL            0x28
>>>> +#define AO_PWM_CLK_E_CTRL            0x2c
>>>> +#define AO_PWM_CLK_F_CTRL            0x30
>>>> +#define AO_PWM_CLK_G_CTRL            0x34
>>>> +#define AO_CEC_CTRL0                 0x38
>>>> +#define AO_CEC_CTRL1                 0x3c
>>>> +#define AO_RTC_BY_OSCIN_CTRL0                0x50
>>>> +#define AO_RTC_BY_OSCIN_CTRL1                0x54
>>>> +
>>>> +#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
>>>> +     MESON_COMP_SEL(a9_ao_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
>>>> +
>>>> +#define A9_COMP_DIV(_name, _reg, _shift, _width) \
>>>> +     MESON_COMP_DIV(a9_ao_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
>>>> +
>>>> +#define A9_COMP_GATE(_name, _reg, _bit) \
>>>> +     MESON_COMP_GATE(a9_ao_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
>>>> +
>>>> +static struct clk_regmap a9_ao_xtal_in = {
>>>> +     .data = &(struct clk_regmap_gate_data){
>>>> +             .offset = AO_OSCIN_CTRL,
>>>> +             .bit_idx = 3,
>>>> +     },
>>>> +     /*
>>>> +      * It may be ao_sys's parent clock, its child clocks mark
>>>> +      * CLK_IS_CRITICAL, So mark CLK_IS_CRITICAL for it.
>>>> +      */
>>> I don't really get what you mean ... Could you rephrase ?
>>
>> The AO sys gate clock chain may be:
>>
>> ao_xtal_in->ao_xtal->ao_sys-> AO sys gate clocks
>>
>> "ao_xtal_in" is part of the parent chain of the AO sys gate clocks.
>>
>> Some of its downstream clocks are marked with CLK_IS_CRITICAL. To ensure
>> those clocks remain functional, ao_xtal_in must not be disabled and is
>> therefore marked as CLK_IS_CRITICAL as well.
> If any of the downstream clocks are critical and marked as such, there is not
> need to mark this one as well.
>
> You should only mark the clocks that are actually critical with the flag
> and let CCF figure out the dependencies.


Thanks for the clarification.


Understood. CCF already keeps the parent clocks of critical clocks enabled
during __clk_core_init(), so the CLK_IS_CRITICAL flag is not needed here.
I'll drop it in the next revision.

>>
>> I will rephrase it like this in the next version:
>>
>>          /*
>>           * ao_sys can select different clock sources. One possible clock
>> path is:
>>           *      ao_xtal_in->ao_xtal->ao_sys-> ao sys gate clocks
>>           *
>>           * ao_xtal_in is in the parent chain of AO sys gate clocks.
>>           * Since some downstream clocks are marked CLK_IS_CRITICAL,
>>           * ao_xtal_in must remain enabled and is therefore marked
>>           * CLK_IS_CRITICAL as well.
>>           */
>>
>>>> +     .hw.init = CLK_HW_INIT_FW_NAME("ao_xtal_in", "xtal",
>>>> +                                    &clk_regmap_gate_ops, CLK_IS_CRITICAL),
>>> I'm honestly not sure about this. It is correct, sure and the macro exist to be
>>> used but ... It does not really help readability here, does it ?
>>>
>>> (I know that was a feedback you've got on v1)
>>>
>>> Other than that, this looks good to me.
>>>
>> Ok, I will use the original clk_init_data for this one.
> Well my comment applies to whole thing really.
>
> There are surely ways in which the macro but the way we statically
> declare things, it adds a level of indirection that makes things harder
> to review IMO.


Understood. The same reasoning applies to the PLL and peripheral clock 
controllers too.

I'll switch back to the explicit clk_init_data initialization and drop
CLK_HW_INIT_FW_NAME in the next revision.

>>
>> [ ... ]
>>
>>> --
>>> Jerome
> --
> Jerome

--

Jian



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

end of thread, other threads:[~2026-06-11 12:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-03 12:17 [PATCH v2 0/2] clk: amlogic: Add A9 AO clock controller Jian Hu via B4 Relay
2026-06-03 12:17 ` [PATCH v2 1/2] dt-bindings: clock: Add Amlogic " Jian Hu via B4 Relay
2026-06-03 15:51   ` Conor Dooley
2026-06-03 12:17 ` [PATCH v2 2/2] clk: amlogic: Add A9 AO clock controller driver Jian Hu via B4 Relay
2026-06-03 12:34   ` sashiko-bot
2026-06-03 14:29   ` Jerome Brunet
2026-06-10  4:18     ` Jian Hu
2026-06-10 12:26       ` Jerome Brunet
2026-06-11 12:24         ` Jian Hu

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

Powered by JetHome