[PATCH v2 0/2] Add Amlogic A9 I3C support

[PATCH v2 0/2] Add Amlogic A9 I3C support

[Junyi Zhao via B4 Relay]

Amlogic A9 SoCs integrate the Silvaco I3C IP, but their peripheral
clock setup differs from the original implementation: the EE-domain
controller (amlogic,a9-i3c) takes two clocks, "pclk" and "fast_clk",
while the AO-domain controller (amlogic,a9-ao-i3c) exposes only
"pclk" and derives its bus clock from an internal divider inside the
controller's own register region.

This series consists of:

  - Add a vendor_init callback for the clock setup and a
    vendor_quirks field for AO-specific behaviour to the Silvaco I3C
    master driver, and enable the existing Silvaco workaround quirks
    for both Amlogic variants.
  - Add the Amlogic compatibles and per-variant clock
    constraints to the silvaco,i3c-master binding schema.

Verified on Amlogic A9 (BY401) with both EE and AO controller
instances.  dt_binding_check and dtbs_check pass with this series.

Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
---
Changes in v2:

  - No need rework the Silvaco I3C driver. 
  - Based on vanilla driver, adapt A9 SoCS through callback 
    and quirks to handle the variant.

- Link to v1: https://patch.msgid.link/20261019-a9_i3c-v1-0-329d819d621f@amlogic.com

---
Junyi Zhao (2):
      i3c: master: svc: Add amlogic a9 support
      dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support

 .../bindings/i3c/silvaco,i3c-master.yaml           |  24 ++-
 drivers/i3c/master/svc-i3c-master.c                | 214 ++++++++++++++++++++-
 2 files changed, 231 insertions(+), 7 deletions(-)
---
base-commit: aac26bee2287c88af5be5a5ff96d783b19a28790
change-id: 20261010-a9_i3c-9d5245d094c8

Best regards,
--  
Junyi Zhao <junyi.zhao@amlogic.com>

[PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support

[Junyi Zhao via B4 Relay]

From: Junyi Zhao <junyi.zhao@amlogic.com>

Amlogic a9 SoCs integrate the Silvaco I3C IP, and their peripheral
clocks vary from the vanilla implementation:

  - The EE-domain controller (amlogic,a9-i3c) takes two clocks,
    "pclk" and "fast_clk", and the fast clock rate must be
    programmed before the controller can be used.

  - The AO-domain controller (amlogic,a9-ao-i3c) exposes only "pclk":
    its bus clock is generated by an internal divider located in the
    controller's own register space.

Add a vendor_init callback to handle this clock setup.
Add a vendor_quirks field to describe AO-specific behavior.

Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
---
 drivers/i3c/master/svc-i3c-master.c | 214 +++++++++++++++++++++++++++++++++++-
 1 file changed, 209 insertions(+), 5 deletions(-)

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index e700392f0..98aa6eb0f 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -9,6 +9,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/completion.h>
 #include <linux/errno.h>
 #include <linux/i3c/master.h>
@@ -166,6 +167,21 @@
  */
 #define SVC_I3C_QUIRK_DAA_CORRUPT	BIT(2)
 
+/* Amlogic specific registers */
+#define AML_AO_I3C_FCLK_DIV_CTRL		0x304
+#define AML_AO_I3C_FCLK_DIV_EN_SHIFT		8
+#define AML_AO_I3C_FCLK_DIV_SHIFT		0
+#define AML_AO_I3C_FCLK_DIV_WIDTH		8
+#define AML_AO_I3C_FCLK_EN_SHIFT		9
+
+/*
+ * AML_I3C_QUIRK_AO:
+ * AO is the Always-On domain on Amlogic SoCs (remains powered in suspend).
+ * Its clock is generated by an internal divider located in the I3C
+ * controller own register space.
+ */
+#define AML_I3C_QUIRK_AO	BIT(0)
+
 struct svc_i3c_cmd {
 	u8 addr;
 	union {
@@ -195,10 +211,6 @@ struct svc_i3c_regs_save {
 	u32 mdynaddr;
 };
 
-struct svc_i3c_drvdata {
-	u32 quirks;
-};
-
 /**
  * struct svc_i3c_master - Silvaco I3C Master structure
  * @base: I3C master controller
@@ -257,6 +269,12 @@ struct svc_i3c_master {
 	u32 mctrl_config;
 };
 
+struct svc_i3c_drvdata {
+	u32 quirks;
+	u32 vendor_quirks;
+	int (*vendor_init)(struct svc_i3c_master *master);
+};
+
 /**
  * struct svc_i3c_i2c_dev_data - Device specific data
  * @index: Index in the master tables corresponding to this device
@@ -274,6 +292,11 @@ static inline bool svc_has_quirk(struct svc_i3c_master *master, u32 quirk)
 	return (master->drvdata->quirks & quirk);
 }
 
+static inline bool vendor_has_quirk(struct svc_i3c_master *master, u32 quirk)
+{
+	return (master->drvdata->vendor_quirks & quirk);
+}
+
 static inline bool svc_has_daa_corrupt(struct svc_i3c_master *master)
 {
 	return ((master->drvdata->quirks & SVC_I3C_QUIRK_DAA_CORRUPT) &&
@@ -2011,6 +2034,7 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
 	if (!master->drvdata)
 		return -EINVAL;
 
+	master->dev = dev;
 	master->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(master->regs))
 		return PTR_ERR(master->regs);
@@ -2019,6 +2043,12 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
 	if (master->num_clks < 0)
 		return dev_err_probe(dev, -EINVAL, "can't get I3C clocks\n");
 
+	if (master->drvdata->vendor_init) {
+		ret = master->drvdata->vendor_init(master);
+		if (ret)
+			return ret;
+	}
+
 	for (i = 0; i < master->num_clks; i++) {
 		if (!strcmp(master->clks[i].id, "fast_clk"))
 			break;
@@ -2036,7 +2066,6 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
 	if (master->irq < 0)
 		return master->irq;
 
-	master->dev = dev;
 	ret = clk_bulk_prepare_enable(master->num_clks, master->clks);
 	if (ret)
 		return dev_err_probe(dev, ret, "can't enable I3C clocks\n");
@@ -2163,9 +2192,184 @@ static const struct svc_i3c_drvdata npcm845_drvdata = {
 
 static const struct svc_i3c_drvdata svc_default_drvdata = {};
 
+/*
+ * Container for the tail clocks of the AO domain variant. The clock HW
+ * structures are registered with the clock framework and must stay alive
+ * for the whole driver lifetime, so they are kept along with the shared
+ * lock used by their gate/divider operations.
+ */
+struct aml_i3c_ao_clk {
+	struct clk_gate div_en;
+	struct clk_divider div;
+	struct clk_gate gate;
+	spinlock_t lock;
+};
+
+/*
+ * aml_ao_i3c_tail_clocks - register the internal divider chain that
+ * produces the AO variant's fast_clk, and append it to the clk bulk
+ * array as "fast_clk".  @parent must be "pclk", the only clock the AO
+ * variant exposes (see silvaco,i3c-master.yaml).
+ */
+static int aml_ao_i3c_tail_clocks(struct svc_i3c_master *master, struct clk *parent)
+{
+	struct device *dev = master->dev;
+	struct aml_i3c_ao_clk *ao;
+	struct clk_init_data init = {};
+	char name[64];
+	int ret;
+	struct clk_bulk_data *new_clks;
+	struct clk *fclk;
+
+	ao = devm_kzalloc(dev, sizeof(*ao), GFP_KERNEL);
+	if (!ao)
+		return -ENOMEM;
+
+	spin_lock_init(&ao->lock);
+
+	snprintf(name, sizeof(name), "%s#div_en", dev_name(dev));
+	init.name = name;
+	init.ops = &clk_gate_ops;
+	init.flags = CLK_IGNORE_UNUSED;
+	init.parent_data = &(struct clk_parent_data) {
+		.index = -1,
+		.hw = __clk_get_hw(parent),
+	};
+	init.num_parents = 1;
+
+	ao->div_en.reg = master->regs + AML_AO_I3C_FCLK_DIV_CTRL;
+	ao->div_en.bit_idx = AML_AO_I3C_FCLK_DIV_EN_SHIFT;
+	ao->div_en.hw.init = &init;
+	ao->div_en.lock = &ao->lock;
+
+	ret = devm_clk_hw_register(dev, &ao->div_en.hw);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register %s\n", name);
+
+	snprintf(name, sizeof(name), "%s#div", dev_name(dev));
+	init.name = name;
+	init.ops = &clk_divider_ops;
+	init.flags = 0;
+	init.parent_data = &(struct clk_parent_data) {
+		.index = -1,
+		.hw = &ao->div_en.hw,
+	};
+	init.num_parents = 1;
+
+	ao->div.reg = master->regs + AML_AO_I3C_FCLK_DIV_CTRL;
+	ao->div.shift = AML_AO_I3C_FCLK_DIV_SHIFT;
+	ao->div.width = AML_AO_I3C_FCLK_DIV_WIDTH;
+	ao->div.hw.init = &init;
+	ao->div.lock = &ao->lock;
+
+	ret = devm_clk_hw_register(dev, &ao->div.hw);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register %s\n", name);
+
+	snprintf(name, sizeof(name), "%s#gate", dev_name(dev));
+	init.name = name;
+	init.ops = &clk_gate_ops;
+	init.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED;
+	init.parent_data = &(struct clk_parent_data) {
+		.index = -1,
+		.hw = &ao->div.hw,
+	};
+	init.num_parents = 1;
+
+	ao->gate.reg = master->regs + AML_AO_I3C_FCLK_DIV_CTRL;
+	ao->gate.bit_idx = AML_AO_I3C_FCLK_EN_SHIFT;
+	ao->gate.hw.init = &init;
+	ao->gate.lock = &ao->lock;
+
+	ret = devm_clk_hw_register(dev, &ao->gate.hw);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register %s\n", name);
+
+	fclk = devm_clk_hw_get_clk(dev, &ao->gate.hw, NULL);
+	if (IS_ERR(fclk))
+		return dev_err_probe(dev, PTR_ERR(fclk),
+				     "tail i3c fast clock failed\n");
+
+	new_clks = devm_kcalloc(dev,
+				master->num_clks + 1,
+				sizeof(*new_clks),
+				GFP_KERNEL);
+	memcpy(new_clks, master->clks,
+			master->num_clks * sizeof(*master->clks));
+	new_clks[master->num_clks].id = "fast_clk";
+	new_clks[master->num_clks].clk = fclk;
+	master->clks = new_clks;
+	master->num_clks++;
+
+	return 0;
+}
+
+static int aml_i3c_init(struct svc_i3c_master *master)
+{
+	struct device *dev = master->dev;
+	long fin_freq;
+	int ret, i;
+	struct clk *fclk;
+
+	if (vendor_has_quirk(master, AML_I3C_QUIRK_AO)) {
+		/*
+		 * clk_bulk_get_all() preserves the DT order, and the
+		 * binding restricts the AO variant to exactly one clock,
+		 * which is always "pclk".  So clks[0] is the input of the
+		 * internal divider chain.
+		 */
+		ret = aml_ao_i3c_tail_clocks(master, master->clks[0].clk);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < master->num_clks; i++) {
+		if (!strcmp(master->clks[i].id, "fast_clk"))
+			break;
+	}
+
+	if (i == master->num_clks)
+		return dev_err_probe(dev, -EINVAL,
+				     "can't get I3C fast clock\n");
+
+	fclk = master->clks[i].clk;
+	if (IS_ERR(fclk))
+		return PTR_ERR(fclk);
+
+	fin_freq = clk_round_rate(fclk, 25000000);
+	if (fin_freq <= 0) {
+		dev_err(dev, "invalid fast_clk\n");
+		return fin_freq ? fin_freq : -EINVAL;
+	}
+
+	ret = clk_set_rate(fclk, fin_freq);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "setting fast_clk rate failed\n");
+
+	return 0;
+}
+
+static const struct svc_i3c_drvdata aml_a9_drvdata = {
+	.quirks = SVC_I3C_QUIRK_FIFO_EMPTY |
+		  SVC_I3C_QUIRK_FALSE_SLVSTART |
+		  SVC_I3C_QUIRK_DAA_CORRUPT,
+	.vendor_init = aml_i3c_init,
+};
+
+static const struct svc_i3c_drvdata aml_a9_ao_drvdata = {
+	.quirks = SVC_I3C_QUIRK_FIFO_EMPTY |
+		  SVC_I3C_QUIRK_FALSE_SLVSTART |
+		  SVC_I3C_QUIRK_DAA_CORRUPT,
+	.vendor_quirks = AML_I3C_QUIRK_AO,
+	.vendor_init = aml_i3c_init,
+};
+
 static const struct of_device_id svc_i3c_master_of_match_tbl[] = {
 	{ .compatible = "nuvoton,npcm845-i3c", .data = &npcm845_drvdata },
 	{ .compatible = "silvaco,i3c-master-v1", .data = &svc_default_drvdata },
+	{ .compatible = "amlogic,a9-i3c", .data = &aml_a9_drvdata },
+	{ .compatible = "amlogic,a9-ao-i3c", .data = &aml_a9_ao_drvdata },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl);

-- 
2.42.0

[PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support

[Junyi Zhao via B4 Relay]

From: Junyi Zhao <junyi.zhao@amlogic.com>

Amlogic A9 SoCs integrate the Silvaco I3C IP with two controller
instances:

  - amlogic,a9-i3c for the EE domain, which takes two clocks,
    "pclk" and "fast_clk" like the NXP variants;

  - amlogic,a9-ao-i3c for the Always-On (AO) domain, which exposes
    only "pclk": its bus clock is generated by an internal divider
    located in the controller's own register space.

Allow both compatibles with the "silvaco,i3c-master-v1" fallback and
enforce the per-variant clock counts.

Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
---
 .../bindings/i3c/silvaco,i3c-master.yaml           | 24 ++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
index 853092f75..de1fdec65 100644
--- a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
+++ b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
@@ -19,6 +19,8 @@ properties:
           - enum:
               - nxp,imx94-i3c
               - nxp,imx95-i3c
+              - amlogic,a9-i3c
+              - amlogic,a9-ao-i3c
           - const: silvaco,i3c-master-v1
 
   reg:
@@ -28,14 +30,14 @@ properties:
     maxItems: 1
 
   clocks:
-    minItems: 2
+    minItems: 1
     items:
       - description: system clock
       - description: bus clock
       - description: other (slower) events clock
 
   clock-names:
-    minItems: 2
+    minItems: 1
     items:
       - const: pclk
       - const: fast_clk
@@ -72,12 +74,30 @@ allOf:
             enum:
               - nxp,imx94-i3c
               - nxp,imx95-i3c
+              - amlogic,a9-i3c
     then:
       properties:
         clocks:
+          minItems: 2
           maxItems: 2
         clock-names:
+          minItems: 2
           maxItems: 2
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: amlogic,a9-ao-i3c
+    then:
+      properties:
+        clocks:
+          maxItems: 1
+          description:
+            The "ao" variant is the controller instance in the Always-On (AO)
+            domain, which has no separate bus clock and derives the
+            bus clock from an internal divider.
+        clock-names:
+          maxItems: 1
 
 unevaluatedProperties: false
 

-- 
2.42.0

Re: [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support

[Frank Li]

On Mon, Nov 02, 2026 at 07:05:40PM +0800, Junyi Zhao wrote:
> Amlogic A9 SoCs integrate the Silvaco I3C IP with two controller
> instances:
>
>   - amlogic,a9-i3c for the EE domain, which takes two clocks,
>     "pclk" and "fast_clk" like the NXP variants;
>
>   - amlogic,a9-ao-i3c for the Always-On (AO) domain, which exposes
>     only "pclk": its bus clock is generated by an internal divider
>     located in the controller's own register space.
>
> Allow both compatibles with the "silvaco,i3c-master-v1" fallback and
> enforce the per-variant clock counts.
>
> Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
> ---
>  .../bindings/i3c/silvaco,i3c-master.yaml           | 24 ++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
> index 853092f75..de1fdec65 100644
> --- a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
> +++ b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
> @@ -19,6 +19,8 @@ properties:
>            - enum:
>                - nxp,imx94-i3c
>                - nxp,imx95-i3c
> +              - amlogic,a9-i3c
> +              - amlogic,a9-ao-i3c
>            - const: silvaco,i3c-master-v1
>
>    reg:
> @@ -28,14 +30,14 @@ properties:
>      maxItems: 1
>
>    clocks:
> -    minItems: 2
> +    minItems: 1
>      items:
>        - description: system clock
>        - description: bus clock
>        - description: other (slower) events clock
>
>    clock-names:
> -    minItems: 2
> +    minItems: 1
>      items:
>        - const: pclk
>        - const: fast_clk
> @@ -72,12 +74,30 @@ allOf:
>              enum:
>                - nxp,imx94-i3c
>                - nxp,imx95-i3c
> +              - amlogic,a9-i3c
>      then:
>        properties:
>          clocks:
> +          minItems: 2
>            maxItems: 2
>          clock-names:
> +          minItems: 2
>            maxItems: 2

Nit: if respin, add empty line here.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: amlogic,a9-ao-i3c
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 1
> +          description:
> +            The "ao" variant is the controller instance in the Always-On (AO)
> +            domain, which has no separate bus clock and derives the
> +            bus clock from an internal divider.
> +        clock-names:
> +          maxItems: 1
>
>  unevaluatedProperties: false
>
>
> --
> 2.42.0
>

Re: [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support

[Frank Li]

On Mon, Nov 02, 2026 at 07:05:40PM +0800, Junyi Zhao wrote:
> Amlogic A9 SoCs integrate the Silvaco I3C IP with two controller
> instances:
>
>   - amlogic,a9-i3c for the EE domain, which takes two clocks,
>     "pclk" and "fast_clk" like the NXP variants;
>
>   - amlogic,a9-ao-i3c for the Always-On (AO) domain, which exposes
>     only "pclk": its bus clock is generated by an internal divider
>     located in the controller's own register space.
>
> Allow both compatibles with the "silvaco,i3c-master-v1" fallback and
> enforce the per-variant clock counts.
>
> Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
> ---
>  .../bindings/i3c/silvaco,i3c-master.yaml           | 24 ++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
> index 853092f75..de1fdec65 100644
> --- a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
> +++ b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
> @@ -19,6 +19,8 @@ properties:
>            - enum:
>                - nxp,imx94-i3c
>                - nxp,imx95-i3c
> +              - amlogic,a9-i3c
> +              - amlogic,a9-ao-i3c
>            - const: silvaco,i3c-master-v1

You have drvdata for it. Needn't fallback to silvaco,i3c-master-v1

Frank
>
>    reg:
> @@ -28,14 +30,14 @@ properties:
>      maxItems: 1
>
>    clocks:
> -    minItems: 2
> +    minItems: 1
>      items:
>        - description: system clock
>        - description: bus clock
>        - description: other (slower) events clock
>
>    clock-names:
> -    minItems: 2
> +    minItems: 1
>      items:
>        - const: pclk
>        - const: fast_clk
> @@ -72,12 +74,30 @@ allOf:
>              enum:
>                - nxp,imx94-i3c
>                - nxp,imx95-i3c
> +              - amlogic,a9-i3c
>      then:
>        properties:
>          clocks:
> +          minItems: 2
>            maxItems: 2
>          clock-names:
> +          minItems: 2
>            maxItems: 2
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: amlogic,a9-ao-i3c
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 1
> +          description:
> +            The "ao" variant is the controller instance in the Always-On (AO)
> +            domain, which has no separate bus clock and derives the
> +            bus clock from an internal divider.
> +        clock-names:
> +          maxItems: 1
>
>  unevaluatedProperties: false
>
>
> --
> 2.42.0
>

Re: [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support

[Frank Li]

On Mon, Nov 02, 2026 at 07:05:39PM +0800, Junyi Zhao wrote:
> Amlogic a9 SoCs integrate the Silvaco I3C IP, and their peripheral
> clocks vary from the vanilla implementation:
>
>   - The EE-domain controller (amlogic,a9-i3c) takes two clocks,
>     "pclk" and "fast_clk", and the fast clock rate must be
>     programmed before the controller can be used.
>
>   - The AO-domain controller (amlogic,a9-ao-i3c) exposes only "pclk":
>     its bus clock is generated by an internal divider located in the
>     controller's own register space.
>
> Add a vendor_init callback to handle this clock setup.
> Add a vendor_quirks field to describe AO-specific behavior.
>
> Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
> ---
>  drivers/i3c/master/svc-i3c-master.c | 214 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 209 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index e700392f0..98aa6eb0f 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -9,6 +9,7 @@
>
>  #include <linux/bitfield.h>
>  #include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/completion.h>
>  #include <linux/errno.h>
>  #include <linux/i3c/master.h>
> @@ -166,6 +167,21 @@
>   */
>  #define SVC_I3C_QUIRK_DAA_CORRUPT	BIT(2)
>
> +/* Amlogic specific registers */
> +#define AML_AO_I3C_FCLK_DIV_CTRL		0x304
> +#define AML_AO_I3C_FCLK_DIV_EN_SHIFT		8
> +#define AML_AO_I3C_FCLK_DIV_SHIFT		0
> +#define AML_AO_I3C_FCLK_DIV_WIDTH		8
> +#define AML_AO_I3C_FCLK_EN_SHIFT		9
> +
> +/*
> + * AML_I3C_QUIRK_AO:
> + * AO is the Always-On domain on Amlogic SoCs (remains powered in suspend).
> + * Its clock is generated by an internal divider located in the I3C
> + * controller own register space.
> + */
> +#define AML_I3C_QUIRK_AO	BIT(0)
> +

There are not much quick now, just put after SVC_I3C_QUIRK_DAA_CORRUPT

#define AML_I3C_QUIRK_AO     BIT(3)

use a unified quirk define.

>  struct svc_i3c_cmd {
>  	u8 addr;
>  	union {
> @@ -195,10 +211,6 @@ struct svc_i3c_regs_save {
>  	u32 mdynaddr;
>  };
>
> -struct svc_i3c_drvdata {
> -	u32 quirks;
> -};
> -
>  /**
>   * struct svc_i3c_master - Silvaco I3C Master structure
>   * @base: I3C master controller
> @@ -257,6 +269,12 @@ struct svc_i3c_master {
>  	u32 mctrl_config;
>  };
>
> +struct svc_i3c_drvdata {
> +	u32 quirks;
> +	u32 vendor_quirks;

Needn't this, direct use quirks

> +	int (*vendor_init)(struct svc_i3c_master *master);
> +};
> +

why need move this struct

>  /**
>   * struct svc_i3c_i2c_dev_data - Device specific data
>   * @index: Index in the master tables corresponding to this device
> @@ -274,6 +292,11 @@ static inline bool svc_has_quirk(struct svc_i3c_master *master, u32 quirk)
>  	return (master->drvdata->quirks & quirk);
>  }
>
> +static inline bool vendor_has_quirk(struct svc_i3c_master *master, u32 quirk)
> +{
> +	return (master->drvdata->vendor_quirks & quirk);
> +}
> +

Needn't this one

>  static inline bool svc_has_daa_corrupt(struct svc_i3c_master *master)
>  {
>  	return ((master->drvdata->quirks & SVC_I3C_QUIRK_DAA_CORRUPT) &&
> @@ -2011,6 +2034,7 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>  	if (!master->drvdata)
>  		return -EINVAL;
>
> +	master->dev = dev;
>  	master->regs = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(master->regs))
>  		return PTR_ERR(master->regs);
> @@ -2019,6 +2043,12 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>  	if (master->num_clks < 0)
>  		return dev_err_probe(dev, -EINVAL, "can't get I3C clocks\n");
>
> +	if (master->drvdata->vendor_init) {
> +		ret = master->drvdata->vendor_init(master);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	for (i = 0; i < master->num_clks; i++) {
>  		if (!strcmp(master->clks[i].id, "fast_clk"))
>  			break;
> @@ -2036,7 +2066,6 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>  	if (master->irq < 0)
>  		return master->irq;
>
> -	master->dev = dev;
>  	ret = clk_bulk_prepare_enable(master->num_clks, master->clks);
>  	if (ret)
>  		return dev_err_probe(dev, ret, "can't enable I3C clocks\n");
> @@ -2163,9 +2192,184 @@ static const struct svc_i3c_drvdata npcm845_drvdata = {
>
>  static const struct svc_i3c_drvdata svc_default_drvdata = {};
>
> +/*
> + * Container for the tail clocks of the AO domain variant. The clock HW
> + * structures are registered with the clock framework and must stay alive
> + * for the whole driver lifetime, so they are kept along with the shared
> + * lock used by their gate/divider operations.
> + */
> +struct aml_i3c_ao_clk {
> +	struct clk_gate div_en;
> +	struct clk_divider div;
> +	struct clk_gate gate;
> +	spinlock_t lock;
> +};
> +
> +/*
> + * aml_ao_i3c_tail_clocks - register the internal divider chain that
> + * produces the AO variant's fast_clk, and append it to the clk bulk
> + * array as "fast_clk".  @parent must be "pclk", the only clock the AO
> + * variant exposes (see silvaco,i3c-master.yaml).
> + */

use kernel doc format.

> +static int aml_ao_i3c_tail_clocks(struct svc_i3c_master *master, struct clk *parent)
> +{
> +	struct device *dev = master->dev;
> +	struct aml_i3c_ao_clk *ao;
> +	struct clk_init_data init = {};
> +	char name[64];
> +	int ret;
> +	struct clk_bulk_data *new_clks;
> +	struct clk *fclk;

keep revise Chemistry order

Frank

> +
> +	ao = devm_kzalloc(dev, sizeof(*ao), GFP_KERNEL);
> +	if (!ao)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&ao->lock);
> +
> +	snprintf(name, sizeof(name), "%s#div_en", dev_name(dev));
> +	init.name = name;
> +	init.ops = &clk_gate_ops;
> +	init.flags = CLK_IGNORE_UNUSED;
> +	init.parent_data = &(struct clk_parent_data) {
> +		.index = -1,
> +		.hw = __clk_get_hw(parent),
> +	};
> +	init.num_parents = 1;
> +
> +	ao->div_en.reg = master->regs + AML_AO_I3C_FCLK_DIV_CTRL;
> +	ao->div_en.bit_idx = AML_AO_I3C_FCLK_DIV_EN_SHIFT;
> +	ao->div_en.hw.init = &init;
> +	ao->div_en.lock = &ao->lock;
> +
> +	ret = devm_clk_hw_register(dev, &ao->div_en.hw);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register %s\n", name);
> +
> +	snprintf(name, sizeof(name), "%s#div", dev_name(dev));
> +	init.name = name;
> +	init.ops = &clk_divider_ops;
> +	init.flags = 0;
> +	init.parent_data = &(struct clk_parent_data) {
> +		.index = -1,
> +		.hw = &ao->div_en.hw,
> +	};
> +	init.num_parents = 1;
> +
> +	ao->div.reg = master->regs + AML_AO_I3C_FCLK_DIV_CTRL;
> +	ao->div.shift = AML_AO_I3C_FCLK_DIV_SHIFT;
> +	ao->div.width = AML_AO_I3C_FCLK_DIV_WIDTH;
> +	ao->div.hw.init = &init;
> +	ao->div.lock = &ao->lock;
> +
> +	ret = devm_clk_hw_register(dev, &ao->div.hw);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register %s\n", name);
> +
> +	snprintf(name, sizeof(name), "%s#gate", dev_name(dev));
> +	init.name = name;
> +	init.ops = &clk_gate_ops;
> +	init.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED;
> +	init.parent_data = &(struct clk_parent_data) {
> +		.index = -1,
> +		.hw = &ao->div.hw,
> +	};
> +	init.num_parents = 1;
> +
> +	ao->gate.reg = master->regs + AML_AO_I3C_FCLK_DIV_CTRL;
> +	ao->gate.bit_idx = AML_AO_I3C_FCLK_EN_SHIFT;
> +	ao->gate.hw.init = &init;
> +	ao->gate.lock = &ao->lock;
> +
> +	ret = devm_clk_hw_register(dev, &ao->gate.hw);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register %s\n", name);
> +
> +	fclk = devm_clk_hw_get_clk(dev, &ao->gate.hw, NULL);
> +	if (IS_ERR(fclk))
> +		return dev_err_probe(dev, PTR_ERR(fclk),
> +				     "tail i3c fast clock failed\n");
> +
> +	new_clks = devm_kcalloc(dev,
> +				master->num_clks + 1,
> +				sizeof(*new_clks),
> +				GFP_KERNEL);
> +	memcpy(new_clks, master->clks,
> +			master->num_clks * sizeof(*master->clks));
> +	new_clks[master->num_clks].id = "fast_clk";
> +	new_clks[master->num_clks].clk = fclk;
> +	master->clks = new_clks;
> +	master->num_clks++;
> +
> +	return 0;
> +}
> +
> +static int aml_i3c_init(struct svc_i3c_master *master)
> +{
> +	struct device *dev = master->dev;
> +	long fin_freq;
> +	int ret, i;
> +	struct clk *fclk;
> +
> +	if (vendor_has_quirk(master, AML_I3C_QUIRK_AO)) {
> +		/*
> +		 * clk_bulk_get_all() preserves the DT order, and the
> +		 * binding restricts the AO variant to exactly one clock,
> +		 * which is always "pclk".  So clks[0] is the input of the
> +		 * internal divider chain.
> +		 */
> +		ret = aml_ao_i3c_tail_clocks(master, master->clks[0].clk);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (i = 0; i < master->num_clks; i++) {
> +		if (!strcmp(master->clks[i].id, "fast_clk"))
> +			break;
> +	}
> +
> +	if (i == master->num_clks)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "can't get I3C fast clock\n");
> +
> +	fclk = master->clks[i].clk;
> +	if (IS_ERR(fclk))
> +		return PTR_ERR(fclk);
> +
> +	fin_freq = clk_round_rate(fclk, 25000000);
> +	if (fin_freq <= 0) {
> +		dev_err(dev, "invalid fast_clk\n");
> +		return fin_freq ? fin_freq : -EINVAL;
> +	}
> +
> +	ret = clk_set_rate(fclk, fin_freq);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "setting fast_clk rate failed\n");
> +
> +	return 0;
> +}
> +
> +static const struct svc_i3c_drvdata aml_a9_drvdata = {
> +	.quirks = SVC_I3C_QUIRK_FIFO_EMPTY |
> +		  SVC_I3C_QUIRK_FALSE_SLVSTART |
> +		  SVC_I3C_QUIRK_DAA_CORRUPT,
> +	.vendor_init = aml_i3c_init,
> +};
> +
> +static const struct svc_i3c_drvdata aml_a9_ao_drvdata = {
> +	.quirks = SVC_I3C_QUIRK_FIFO_EMPTY |
> +		  SVC_I3C_QUIRK_FALSE_SLVSTART |
> +		  SVC_I3C_QUIRK_DAA_CORRUPT,
> +	.vendor_quirks = AML_I3C_QUIRK_AO,
> +	.vendor_init = aml_i3c_init,
> +};
> +
>  static const struct of_device_id svc_i3c_master_of_match_tbl[] = {
>  	{ .compatible = "nuvoton,npcm845-i3c", .data = &npcm845_drvdata },
>  	{ .compatible = "silvaco,i3c-master-v1", .data = &svc_default_drvdata },
> +	{ .compatible = "amlogic,a9-i3c", .data = &aml_a9_drvdata },
> +	{ .compatible = "amlogic,a9-ao-i3c", .data = &aml_a9_ao_drvdata },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl);
>
> --
> 2.42.0
>

Re: [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support

[Krzysztof Kozlowski]

On Mon, Nov 02, 2026 at 07:05:40PM +0800, Junyi Zhao wrote:
> Amlogic A9 SoCs integrate the Silvaco I3C IP with two controller
> instances:
> 
>   - amlogic,a9-i3c for the EE domain, which takes two clocks,
>     "pclk" and "fast_clk" like the NXP variants;
> 
>   - amlogic,a9-ao-i3c for the Always-On (AO) domain, which exposes
>     only "pclk": its bus clock is generated by an internal divider
>     located in the controller's own register space.
> 
> Allow both compatibles with the "silvaco,i3c-master-v1" fallback and
> enforce the per-variant clock counts.

This we see in the diff, but we do not see WHY.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support

[Junyi Zhao]

Hi Frank,

Thanks for the review.

On 03/11/2026 06:10, Frank Li wrote:
> [ EXTERNAL EMAIL ]
> 
> On Mon, Nov 02, 2026 at 07:05:39PM +0800, Junyi Zhao wrote:
>> Amlogic a9 SoCs integrate the Silvaco I3C IP, and their peripheral
>> clocks vary from the vanilla implementation:
>>
>>    - The EE-domain controller (amlogic,a9-i3c) takes two clocks,
>>      "pclk" and "fast_clk", and the fast clock rate must be
>>      programmed before the controller can be used.
>>
>>    - The AO-domain controller (amlogic,a9-ao-i3c) exposes only "pclk":
>>      its bus clock is generated by an internal divider located in the
>>      controller's own register space.
>>
>> Add a vendor_init callback to handle this clock setup.
>> Add a vendor_quirks field to describe AO-specific behavior.
>>
>> Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
>> ---
>>   drivers/i3c/master/svc-i3c-master.c | 214 +++++++++++++++++++++++++++++++++++-
>>   1 file changed, 209 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
>> index e700392f0..98aa6eb0f 100644
>> --- a/drivers/i3c/master/svc-i3c-master.c
>> +++ b/drivers/i3c/master/svc-i3c-master.c
>> @@ -9,6 +9,7 @@
>>
>>   #include <linux/bitfield.h>
>>   #include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>>   #include <linux/completion.h>
>>   #include <linux/errno.h>
>>   #include <linux/i3c/master.h>
>> @@ -166,6 +167,21 @@
>>    */
>>   #define SVC_I3C_QUIRK_DAA_CORRUPT    BIT(2)
>>
>> +/* Amlogic specific registers */
>> +#define AML_AO_I3C_FCLK_DIV_CTRL             0x304
>> +#define AML_AO_I3C_FCLK_DIV_EN_SHIFT         8
>> +#define AML_AO_I3C_FCLK_DIV_SHIFT            0
>> +#define AML_AO_I3C_FCLK_DIV_WIDTH            8
>> +#define AML_AO_I3C_FCLK_EN_SHIFT             9
>> +
>> +/*
>> + * AML_I3C_QUIRK_AO:
>> + * AO is the Always-On domain on Amlogic SoCs (remains powered in suspend).
>> + * Its clock is generated by an internal divider located in the I3C
>> + * controller own register space.
>> + */
>> +#define AML_I3C_QUIRK_AO     BIT(0)
>> +
> 
> There are not much quick now, just put after SVC_I3C_QUIRK_DAA_CORRUPT
> 
> #define AML_I3C_QUIRK_AO     BIT(3)
> 
> use a unified quirk define.

will do.

> 
>>   struct svc_i3c_cmd {
>>        u8 addr;
>>        union {
>> @@ -195,10 +211,6 @@ struct svc_i3c_regs_save {
>>        u32 mdynaddr;
>>   };
>>
>> -struct svc_i3c_drvdata {
>> -     u32 quirks;
>> -};
>> -
>>   /**
>>    * struct svc_i3c_master - Silvaco I3C Master structure
>>    * @base: I3C master controller
>> @@ -257,6 +269,12 @@ struct svc_i3c_master {
>>        u32 mctrl_config;
>>   };
>>
>> +struct svc_i3c_drvdata {
>> +     u32 quirks;
>> +     u32 vendor_quirks;
> 
> Needn't this, direct use quirks

will do.

>> +     int (*vendor_init)(struct svc_i3c_master *master);
>> +};
>> +
> 
> why need move this struct

because callback need param struct svc_i3c_master *master， and if not， 
compile error. undefined.

> 
>>   /**
>>    * struct svc_i3c_i2c_dev_data - Device specific data
>>    * @index: Index in the master tables corresponding to this device
>> @@ -274,6 +292,11 @@ static inline bool svc_has_quirk(struct svc_i3c_master *master, u32 quirk)
>>        return (master->drvdata->quirks & quirk);
>>   }
>>
>> +static inline bool vendor_has_quirk(struct svc_i3c_master *master, u32 quirk)
>> +{
>> +     return (master->drvdata->vendor_quirks & quirk);
>> +}
>> +
> 
> Needn't this one

will delete it.

>>   static inline bool svc_has_daa_corrupt(struct svc_i3c_master *master)
>>   {
>>        return ((master->drvdata->quirks & SVC_I3C_QUIRK_DAA_CORRUPT) &&
>> @@ -2011,6 +2034,7 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>>        if (!master->drvdata)
>>                return -EINVAL;
>>
>> +     master->dev = dev;
>>        master->regs = devm_platform_ioremap_resource(pdev, 0);
>>        if (IS_ERR(master->regs))
>>                return PTR_ERR(master->regs);
>> @@ -2019,6 +2043,12 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>>        if (master->num_clks < 0)
>>                return dev_err_probe(dev, -EINVAL, "can't get I3C clocks\n");
>>
>> +     if (master->drvdata->vendor_init) {
>> +             ret = master->drvdata->vendor_init(master);
>> +             if (ret)
>> +                     return ret;
>> +     }
>> +
>>        for (i = 0; i < master->num_clks; i++) {
>>                if (!strcmp(master->clks[i].id, "fast_clk"))
>>                        break;
>> @@ -2036,7 +2066,6 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>>        if (master->irq < 0)
>>                return master->irq;
>>
>> -     master->dev = dev;
>>        ret = clk_bulk_prepare_enable(master->num_clks, master->clks);
>>        if (ret)
>>                return dev_err_probe(dev, ret, "can't enable I3C clocks\n");
>> @@ -2163,9 +2192,184 @@ static const struct svc_i3c_drvdata npcm845_drvdata = {
>>
>>   static const struct svc_i3c_drvdata svc_default_drvdata = {};
>>
>> +/*
>> + * Container for the tail clocks of the AO domain variant. The clock HW
>> + * structures are registered with the clock framework and must stay alive
>> + * for the whole driver lifetime, so they are kept along with the shared
>> + * lock used by their gate/divider operations.
>> + */
>> +struct aml_i3c_ao_clk {
>> +     struct clk_gate div_en;
>> +     struct clk_divider div;
>> +     struct clk_gate gate;
>> +     spinlock_t lock;
>> +};
>> +
>> +/*
>> + * aml_ao_i3c_tail_clocks - register the internal divider chain that
>> + * produces the AO variant's fast_clk, and append it to the clk bulk
>> + * array as "fast_clk".  @parent must be "pclk", the only clock the AO
>> + * variant exposes (see silvaco,i3c-master.yaml).
>> + */
> 
> use kernel doc format.

  will change.

>> +static int aml_ao_i3c_tail_clocks(struct svc_i3c_master *master, struct clk *parent)
>> +{
>> +     struct device *dev = master->dev;
>> +     struct aml_i3c_ao_clk *ao;
>> +     struct clk_init_data init = {};
>> +     char name[64];
>> +     int ret;
>> +     struct clk_bulk_data *new_clks;
>> +     struct clk *fclk;
> 
> keep revise Chemistry order
> 
will change order：
	struct aml_i3c_ao_clk *ao;
	struct clk *fclk;
	struct clk_bulk_data *new_clks;
	struct device *dev = master->dev;
	struct clk_init_data init = {};
	char name[64];
	int ret;

> Frank
> 
>> +
>> +     ao = devm_kzalloc(dev, sizeof(*ao), GFP_KERNEL);
>> +     if (!ao)
>> +             return -ENOMEM;
>> +
>> +     spin_lock_init(&ao->lock);
>> +
>> +     snprintf(name, sizeof(name), "%s#div_en", dev_name(dev));
>> +     init.name = name;
>> +     init.ops = &clk_gate_ops;
>> +     init.flags = CLK_IGNORE_UNUSED;
>> +     init.parent_data = &(struct clk_parent_data) {
>> +             .index = -1,
>> +             .hw = __clk_get_hw(parent),
>> +     };
>> +     init.num_parents = 1;
>> +
>> +     ao->div_en.reg = master->regs + AML_AO_I3C_FCLK_DIV_CTRL;
>> +     ao->div_en.bit_idx = AML_AO_I3C_FCLK_DIV_EN_SHIFT;
>> +     ao->div_en.hw.init = &init;
>> +     ao->div_en.lock = &ao->lock;
>> +
>> +     ret = devm_clk_hw_register(dev, &ao->div_en.hw);
>> +     if (ret)
>> +             return dev_err_probe(dev, ret, "failed to register %s\n", name);
>> +
>> +     snprintf(name, sizeof(name), "%s#div", dev_name(dev));
>> +     init.name = name;
>> +     init.ops = &clk_divider_ops;
>> +     init.flags = 0;
>> +     init.parent_data = &(struct clk_parent_data) {
>> +             .index = -1,
>> +             .hw = &ao->div_en.hw,
>> +     };
>> +     init.num_parents = 1;
>> +
>> +     ao->div.reg = master->regs + AML_AO_I3C_FCLK_DIV_CTRL;
>> +     ao->div.shift = AML_AO_I3C_FCLK_DIV_SHIFT;
>> +     ao->div.width = AML_AO_I3C_FCLK_DIV_WIDTH;
>> +     ao->div.hw.init = &init;
>> +     ao->div.lock = &ao->lock;
>> +
>> +     ret = devm_clk_hw_register(dev, &ao->div.hw);
>> +     if (ret)
>> +             return dev_err_probe(dev, ret, "failed to register %s\n", name);
>> +
>> +     snprintf(name, sizeof(name), "%s#gate", dev_name(dev));
>> +     init.name = name;
>> +     init.ops = &clk_gate_ops;
>> +     init.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED;
>> +     init.parent_data = &(struct clk_parent_data) {
>> +             .index = -1,
>> +             .hw = &ao->div.hw,
>> +     };
>> +     init.num_parents = 1;
>> +
>> +     ao->gate.reg = master->regs + AML_AO_I3C_FCLK_DIV_CTRL;
>> +     ao->gate.bit_idx = AML_AO_I3C_FCLK_EN_SHIFT;
>> +     ao->gate.hw.init = &init;
>> +     ao->gate.lock = &ao->lock;
>> +
>> +     ret = devm_clk_hw_register(dev, &ao->gate.hw);
>> +     if (ret)
>> +             return dev_err_probe(dev, ret, "failed to register %s\n", name);
>> +
>> +     fclk = devm_clk_hw_get_clk(dev, &ao->gate.hw, NULL);
>> +     if (IS_ERR(fclk))
>> +             return dev_err_probe(dev, PTR_ERR(fclk),
>> +                                  "tail i3c fast clock failed\n");
>> +
>> +     new_clks = devm_kcalloc(dev,
>> +                             master->num_clks + 1,
>> +                             sizeof(*new_clks),
>> +                             GFP_KERNEL);
>> +     memcpy(new_clks, master->clks,
>> +                     master->num_clks * sizeof(*master->clks));
>> +     new_clks[master->num_clks].id = "fast_clk";
>> +     new_clks[master->num_clks].clk = fclk;
>> +     master->clks = new_clks;
>> +     master->num_clks++;
>> +
>> +     return 0;
>> +}
>> +
>> +static int aml_i3c_init(struct svc_i3c_master *master)
>> +{
>> +     struct device *dev = master->dev;
>> +     long fin_freq;
>> +     int ret, i;
>> +     struct clk *fclk;
>> +
>> +     if (vendor_has_quirk(master, AML_I3C_QUIRK_AO)) {
>> +             /*
>> +              * clk_bulk_get_all() preserves the DT order, and the
>> +              * binding restricts the AO variant to exactly one clock,
>> +              * which is always "pclk".  So clks[0] is the input of the
>> +              * internal divider chain.
>> +              */
>> +             ret = aml_ao_i3c_tail_clocks(master, master->clks[0].clk);
>> +             if (ret)
>> +                     return ret;
>> +     }
>> +
>> +     for (i = 0; i < master->num_clks; i++) {
>> +             if (!strcmp(master->clks[i].id, "fast_clk"))
>> +                     break;
>> +     }
>> +
>> +     if (i == master->num_clks)
>> +             return dev_err_probe(dev, -EINVAL,
>> +                                  "can't get I3C fast clock\n");
>> +
>> +     fclk = master->clks[i].clk;
>> +     if (IS_ERR(fclk))
>> +             return PTR_ERR(fclk);
>> +
>> +     fin_freq = clk_round_rate(fclk, 25000000);
>> +     if (fin_freq <= 0) {
>> +             dev_err(dev, "invalid fast_clk\n");
>> +             return fin_freq ? fin_freq : -EINVAL;
>> +     }
>> +
>> +     ret = clk_set_rate(fclk, fin_freq);
>> +     if (ret)
>> +             return dev_err_probe(dev, ret,
>> +                                  "setting fast_clk rate failed\n");
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct svc_i3c_drvdata aml_a9_drvdata = {
>> +     .quirks = SVC_I3C_QUIRK_FIFO_EMPTY |
>> +               SVC_I3C_QUIRK_FALSE_SLVSTART |
>> +               SVC_I3C_QUIRK_DAA_CORRUPT,
>> +     .vendor_init = aml_i3c_init,
>> +};
>> +
>> +static const struct svc_i3c_drvdata aml_a9_ao_drvdata = {
>> +     .quirks = SVC_I3C_QUIRK_FIFO_EMPTY |
>> +               SVC_I3C_QUIRK_FALSE_SLVSTART |
>> +               SVC_I3C_QUIRK_DAA_CORRUPT,
>> +     .vendor_quirks = AML_I3C_QUIRK_AO,
>> +     .vendor_init = aml_i3c_init,
>> +};
>> +
>>   static const struct of_device_id svc_i3c_master_of_match_tbl[] = {
>>        { .compatible = "nuvoton,npcm845-i3c", .data = &npcm845_drvdata },
>>        { .compatible = "silvaco,i3c-master-v1", .data = &svc_default_drvdata },
>> +     { .compatible = "amlogic,a9-i3c", .data = &aml_a9_drvdata },
>> +     { .compatible = "amlogic,a9-ao-i3c", .data = &aml_a9_ao_drvdata },
>>        { /* sentinel */ },
>>   };
>>   MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl);
>>
>> --
>> 2.42.0
>>

Re: [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support

[Junyi Zhao]

Hi Frank,

Thanks for the review.

On 03/11/2026 05:44, Frank Li wrote:
> [ EXTERNAL EMAIL ]
> 
> On Mon, Nov 02, 2026 at 07:05:40PM +0800, Junyi Zhao wrote:
>> Amlogic A9 SoCs integrate the Silvaco I3C IP with two controller
>> instances:
>>
>>    - amlogic,a9-i3c for the EE domain, which takes two clocks,
>>      "pclk" and "fast_clk" like the NXP variants;
>>
>>    - amlogic,a9-ao-i3c for the Always-On (AO) domain, which exposes
>>      only "pclk": its bus clock is generated by an internal divider
>>      located in the controller's own register space.
>>
>> Allow both compatibles with the "silvaco,i3c-master-v1" fallback and
>> enforce the per-variant clock counts.
>>
>> Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
>> ---
>>   .../bindings/i3c/silvaco,i3c-master.yaml           | 24 ++++++++++++++++++++--
>>   1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
>> index 853092f75..de1fdec65 100644
>> --- a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
>> +++ b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
>> @@ -19,6 +19,8 @@ properties:
>>             - enum:
>>                 - nxp,imx94-i3c
>>                 - nxp,imx95-i3c
>> +              - amlogic,a9-i3c
>> +              - amlogic,a9-ao-i3c
>>             - const: silvaco,i3c-master-v1
>>
>>     reg:
>> @@ -28,14 +30,14 @@ properties:
>>       maxItems: 1
>>
>>     clocks:
>> -    minItems: 2
>> +    minItems: 1
>>       items:
>>         - description: system clock
>>         - description: bus clock
>>         - description: other (slower) events clock
>>
>>     clock-names:
>> -    minItems: 2
>> +    minItems: 1
>>       items:
>>         - const: pclk
>>         - const: fast_clk
>> @@ -72,12 +74,30 @@ allOf:
>>               enum:
>>                 - nxp,imx94-i3c
>>                 - nxp,imx95-i3c
>> +              - amlogic,a9-i3c
>>       then:
>>         properties:
>>           clocks:
>> +          minItems: 2
>>             maxItems: 2
>>           clock-names:
>> +          minItems: 2
>>             maxItems: 2
> 
> Nit: if respin, add empty line here.

will add a blank line.

> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: amlogic,a9-ao-i3c
>> +    then:
>> +      properties:
>> +        clocks:
>> +          maxItems: 1
>> +          description:
>> +            The "ao" variant is the controller instance in the Always-On (AO)
>> +            domain, which has no separate bus clock and derives the
>> +            bus clock from an internal divider.
>> +        clock-names:
>> +          maxItems: 1
>>
>>   unevaluatedProperties: false
>>
>>
>> --
>> 2.42.0
>>

Re: [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support

[Junyi Zhao]

Hi frank,
     thanks for your review.

On 03/11/2026 05:48, Frank Li wrote:
> [ EXTERNAL EMAIL ]
> 
> On Mon, Nov 02, 2026 at 07:05:40PM +0800, Junyi Zhao wrote:
>> Amlogic A9 SoCs integrate the Silvaco I3C IP with two controller
>> instances:
>>
>>    - amlogic,a9-i3c for the EE domain, which takes two clocks,
>>      "pclk" and "fast_clk" like the NXP variants;
>>
>>    - amlogic,a9-ao-i3c for the Always-On (AO) domain, which exposes
>>      only "pclk": its bus clock is generated by an internal divider
>>      located in the controller's own register space.
>>
>> Allow both compatibles with the "silvaco,i3c-master-v1" fallback and
>> enforce the per-variant clock counts.
>>
>> Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
>> ---
>>   .../bindings/i3c/silvaco,i3c-master.yaml           | 24 ++++++++++++++++++++--
>>   1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
>> index 853092f75..de1fdec65 100644
>> --- a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
>> +++ b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
>> @@ -19,6 +19,8 @@ properties:
>>             - enum:
>>                 - nxp,imx94-i3c
>>                 - nxp,imx95-i3c
>> +              - amlogic,a9-i3c
>> +              - amlogic,a9-ao-i3c
>>             - const: silvaco,i3c-master-v1
> 
> You have drvdata for it. Needn't fallback to silvaco,i3c-master-v1

will move it.
follows nuvoton,npcm845-i3c.


> Frank
>>
>>     reg:
>> @@ -28,14 +30,14 @@ properties:
>>       maxItems: 1
>>
>>     clocks:
>> -    minItems: 2
>> +    minItems: 1
>>       items:
>>         - description: system clock
>>         - description: bus clock
>>         - description: other (slower) events clock
>>
>>     clock-names:
>> -    minItems: 2
>> +    minItems: 1
>>       items:
>>         - const: pclk
>>         - const: fast_clk
>> @@ -72,12 +74,30 @@ allOf:
>>               enum:
>>                 - nxp,imx94-i3c
>>                 - nxp,imx95-i3c
>> +              - amlogic,a9-i3c
>>       then:
>>         properties:
>>           clocks:
>> +          minItems: 2
>>             maxItems: 2
>>           clock-names:
>> +          minItems: 2
>>             maxItems: 2
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: amlogic,a9-ao-i3c
>> +    then:
>> +      properties:
>> +        clocks:
>> +          maxItems: 1
>> +          description:
>> +            The "ao" variant is the controller instance in the Always-On (AO)
>> +            domain, which has no separate bus clock and derives the
>> +            bus clock from an internal divider.
>> +        clock-names:
>> +          maxItems: 1
>>
>>   unevaluatedProperties: false
>>
>>
>> --
>> 2.42.0
>>

Re: [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support

[Junyi Zhao]

Hi Krzysztof,
     Thanks for your review.

On 03/11/2026 19:42, Krzysztof Kozlowski wrote:
> [ EXTERNAL EMAIL ]
> 
> On Mon, Nov 02, 2026 at 07:05:40PM +0800, Junyi Zhao wrote:
>> Amlogic A9 SoCs integrate the Silvaco I3C IP with two controller
>> instances:
>>
>>    - amlogic,a9-i3c for the EE domain, which takes two clocks,
>>      "pclk" and "fast_clk" like the NXP variants;
>>
>>    - amlogic,a9-ao-i3c for the Always-On (AO) domain, which exposes
>>      only "pclk": its bus clock is generated by an internal divider
>>      located in the controller's own register space.
>>
>> Allow both compatibles with the "silvaco,i3c-master-v1" fallback and
>> enforce the per-variant clock counts.
> 
> This we see in the diff, but we do not see WHY.

i will add a description of the I3C peripheral integratation detail at

hardware-level.

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support

[Frank Li]

On Wed, Nov 04, 2026 at 10:44:59AM +0800, Junyi Zhao wrote:
> Hi Frank,
>
> Thanks for the review.
>
> On 03/11/2026 06:10, Frank Li wrote:
> > [ EXTERNAL EMAIL ]
> >
> > On Mon, Nov 02, 2026 at 07:05:39PM +0800, Junyi Zhao wrote:
> > > Amlogic a9 SoCs integrate the Silvaco I3C IP, and their peripheral
> > > clocks vary from the vanilla implementation:
> > >
> > >    - The EE-domain controller (amlogic,a9-i3c) takes two clocks,
> > >      "pclk" and "fast_clk", and the fast clock rate must be
> > >      programmed before the controller can be used.
> > >
> > >    - The AO-domain controller (amlogic,a9-ao-i3c) exposes only "pclk":
> > >      its bus clock is generated by an internal divider located in the
> > >      controller's own register space.
> > >
> > > Add a vendor_init callback to handle this clock setup.
> > > Add a vendor_quirks field to describe AO-specific behavior.
> > >
> > > Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
> > > ---
...
> > > +     int (*vendor_init)(struct svc_i3c_master *master);
> > > +};
> > > +
> >
> > why need move this struct
>
> because callback need param struct svc_i3c_master *master， and if not，
> compile error. undefined.

Most likely svc_i3c_master need save a point for drvdata in future.

You add forward declear before drvdata

struct svc_i3c_drvdata;

>
...
> > > +static int aml_ao_i3c_tail_clocks(struct svc_i3c_master *master, struct clk *parent)
> > > +{
> > > +     struct device *dev = master->dev;
> > > +     struct aml_i3c_ao_clk *ao;
> > > +     struct clk_init_data init = {};
> > > +     char name[64];
> > > +     int ret;
> > > +     struct clk_bulk_data *new_clks;
> > > +     struct clk *fclk;
> >
> > keep revise Chemistry order
> >
> will change order：
> 	struct aml_i3c_ao_clk *ao;
> 	struct clk *fclk;
> 	struct clk_bulk_data *new_clks;
> 	struct device *dev = master->dev;
> 	struct clk_init_data init = {};
> 	char name[64];
> 	int ret;


longest is first

struct device *dev = master->dev;
struct clk_bulk_data *new_clks;
struct clk_init_data init = {};
struct aml_i3c_ao_clk *ao;
struct clk *fclk;
char name[64];
int ret;


Frank

Re: [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support

[Junyi Zhao]

Hi Frank,
	thanks for your detail.

On 04/11/2026 11:19, Frank Li wrote:
> [ EXTERNAL EMAIL ]
> 
> On Wed, Nov 04, 2026 at 10:44:59AM +0800, Junyi Zhao wrote:
>> Hi Frank,
>>
>> Thanks for the review.
>>
>> On 03/11/2026 06:10, Frank Li wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On Mon, Nov 02, 2026 at 07:05:39PM +0800, Junyi Zhao wrote:
>>>> Amlogic a9 SoCs integrate the Silvaco I3C IP, and their peripheral
>>>> clocks vary from the vanilla implementation:
>>>>
>>>>     - The EE-domain controller (amlogic,a9-i3c) takes two clocks,
>>>>       "pclk" and "fast_clk", and the fast clock rate must be
>>>>       programmed before the controller can be used.
>>>>
>>>>     - The AO-domain controller (amlogic,a9-ao-i3c) exposes only "pclk":
>>>>       its bus clock is generated by an internal divider located in the
>>>>       controller's own register space.
>>>>
>>>> Add a vendor_init callback to handle this clock setup.
>>>> Add a vendor_quirks field to describe AO-specific behavior.
>>>>
>>>> Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
>>>> ---
> ...
>>>> +     int (*vendor_init)(struct svc_i3c_master *master);
>>>> +};
>>>> +
>>>
>>> why need move this struct
>>
>> because callback need param struct svc_i3c_master *master， and if not，
>> compile error. undefined.
> 
> Most likely svc_i3c_master need save a point for drvdata in future.
> 
> You add forward declear before drvdata
> 
> struct svc_i3c_drvdata;
> 

will do.

>>
> ...
>>>> +static int aml_ao_i3c_tail_clocks(struct svc_i3c_master *master, struct clk *parent)
>>>> +{
>>>> +     struct device *dev = master->dev;
>>>> +     struct aml_i3c_ao_clk *ao;
>>>> +     struct clk_init_data init = {};
>>>> +     char name[64];
>>>> +     int ret;
>>>> +     struct clk_bulk_data *new_clks;
>>>> +     struct clk *fclk;
>>>
>>> keep revise Chemistry order
>>>
>> will change order：
>>        struct aml_i3c_ao_clk *ao;
>>        struct clk *fclk;
>>        struct clk_bulk_data *new_clks;
>>        struct device *dev = master->dev;
>>        struct clk_init_data init = {};
>>        char name[64];
>>        int ret;
> 
> 
> longest is first
> 
> struct device *dev = master->dev;
> struct clk_bulk_data *new_clks;
> struct clk_init_data init = {};
> struct aml_i3c_ao_clk *ao;
> struct clk *fclk;
> char name[64];
> int ret;
> 

will do. thanks the detail.

> 
> Frank