mirror of https://lore.kernel.org/linux-amlogic/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add Amlogic A9 I3C support
@ 2026-11-02 11:05 Junyi Zhao via B4 Relay
  2026-11-02 11:05 ` [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support Junyi Zhao via B4 Relay
  2026-11-02 11:05 ` [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support Junyi Zhao via B4 Relay
  0 siblings, 2 replies; 8+ messages in thread
From: Junyi Zhao via B4 Relay @ 2026-11-02 11:05 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, Miquel Raynal, Kees Cook,
	Gustavo A. R. Silva
  Cc: linux-i3c, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic, imx, linux-hardening, xianwei.zhao, Kelvin.Zhang,
	Junyi Zhao

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>



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

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

* [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support
  2026-11-02 11:05 [PATCH v2 0/2] Add Amlogic A9 I3C support Junyi Zhao via B4 Relay
@ 2026-11-02 11:05 ` Junyi Zhao via B4 Relay
  2026-11-02 11:18   ` sashiko-bot
  2026-11-02 22:10   ` Frank Li
  2026-11-02 11:05 ` [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support Junyi Zhao via B4 Relay
  1 sibling, 2 replies; 8+ messages in thread
From: Junyi Zhao via B4 Relay @ 2026-11-02 11:05 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, Miquel Raynal, Kees Cook,
	Gustavo A. R. Silva
  Cc: linux-i3c, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic, imx, linux-hardening, xianwei.zhao, Kelvin.Zhang,
	Junyi Zhao

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



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

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

* [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support
  2026-11-02 11:05 [PATCH v2 0/2] Add Amlogic A9 I3C support Junyi Zhao via B4 Relay
  2026-11-02 11:05 ` [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support Junyi Zhao via B4 Relay
@ 2026-11-02 11:05 ` Junyi Zhao via B4 Relay
  2026-11-02 11:16   ` sashiko-bot
                     ` (2 more replies)
  1 sibling, 3 replies; 8+ messages in thread
From: Junyi Zhao via B4 Relay @ 2026-11-02 11:05 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, Miquel Raynal, Kees Cook,
	Gustavo A. R. Silva
  Cc: linux-i3c, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic, imx, linux-hardening, xianwei.zhao, Kelvin.Zhang,
	Junyi Zhao

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



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

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

* Re: [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support
  2026-11-02 11:05 ` [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support Junyi Zhao via B4 Relay
@ 2026-11-02 11:16   ` sashiko-bot
  2026-11-02 21:44   ` Frank Li
  2026-11-02 21:48   ` Frank Li
  2 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-11-02 11:16 UTC (permalink / raw)
  To: Junyi Zhao
  Cc: linux-i3c, devicetree, linux-amlogic, imx, robh, conor+dt,
	Frank.Li, neil.armstrong, Alexandre Belloni

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: i3c: silvaco: unwieldy conditionals and invalid fallback
--

commit 0af7894c213f5346efb5a1f1360dff5a7a1704bb
Author: Junyi Zhao <junyi.zhao@amlogic.com>

dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support

This commit adds Amlogic A9 I3C controller compatibles (amlogic,a9-i3c and
amlogic,a9-ao-i3c) with a silvaco,i3c-master-v1 fallback to the Silvaco I3C
master Device Tree binding schema and adjusts the clock count constraints for
each variant.

> diff --git a/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml b/Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
> index 853092f7522d3..de1fdec654a26 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

[Severity: Medium]
With the top-level clocks and clock-names minItems lowered to 1 and a third
if-then conditional block added under allOf to handle 1-clock, 2-clock, and
3-clock variants, the per-compatible conditionals in silvaco,i3c-master.yaml
are becoming unwieldy. Since the Amlogic variants (especially the AO domain
instance with its internal bus clock divider) differ significantly from the
base binding, would it be cleaner to split the Amlogic binding into a
separate schema document rather than piling more conditionals into this file?

Also, is silvaco,i3c-master-v1 an appropriate fallback compatible for
amlogic,a9-i3c and amlogic,a9-ao-i3c?

If a device tree node for amlogic,a9-ao-i3c binds via the generic
silvaco,i3c-master-v1 compatible (which uses svc_default_drvdata without a
vendor_init callback), svc_i3c_master_probe() looks up fast_clk in
master->clks and fails with -EINVAL because fast_clk is absent from the
Device Tree and the internal clock divider at offset 0x304 is never
registered. Similarly, amlogic,a9-i3c requires Amlogic-specific fast_clk rate
setup in aml_i3c_init() and hardware quirks (SVC_I3C_QUIRK_FIFO_EMPTY,
SVC_I3C_QUIRK_FALSE_SLVSTART, and SVC_I3C_QUIRK_DAA_CORRUPT) to operate,
much like nuvoton,npcm845-i3c which does not use silvaco,i3c-master-v1 as a
fallback.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-a9_i3c-v2-0-0c474bfccffa@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] 8+ messages in thread

* Re: [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support
  2026-11-02 11:05 ` [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support Junyi Zhao via B4 Relay
@ 2026-11-02 11:18   ` sashiko-bot
  2026-11-02 22:10   ` Frank Li
  1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-11-02 11:18 UTC (permalink / raw)
  To: Junyi Zhao
  Cc: linux-i3c, devicetree, linux-amlogic, imx, robh, conor+dt,
	Frank.Li, neil.armstrong, Alexandre Belloni

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] i3c: master: svc: NULL dereference on unchecked devm_kcalloc() in aml_ao_i3c_tail_clocks()
- [High] i3c: master: svc: unclocked MMIO access to AML_AO_I3C_FCLK_DIV_CTRL in aml_i3c_init()
- [Low] i3c: master: svc: missing COMMON_CLK dependency for aml_ao_i3c_tail_clocks()
--

commit 5d539aa0589681cba0b31f37c581a2a552c6d504
Author: Junyi Zhao <junyi.zhao@amlogic.com>

i3c: master: svc: Add amlogic a9 support

This commit adds support for the Silvaco I3C master controller on Amlogic
a9 SoCs (both EE-domain and AO-domain variants). It introduces a
vendor_init callback and vendor_quirks to register the AO-domain internal
clock divider chain and configure the fast_clk rate during probe.

> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index e700392f0801f..98aa6eb0f49ca 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>

[ ... ]

> @@ -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");

[Severity: Low]
Will this fail to link when the common clock framework is disabled?

aml_ao_i3c_tail_clocks() unconditionally references clk_gate_ops,
clk_divider_ops, devm_clk_hw_register(), and devm_clk_hw_get_clk(), which do
not have stub definitions in include/linux/clk-provider.h when common clock
support is not enabled.

In drivers/i3c/master/Kconfig, the driver currently has:

    config SVC_I3C_MASTER
        tristate "Silvaco I3C Dual-Role Master driver"
        depends on HAS_IOMEM
        depends on !(ALPHA || PARISC)

Should the Kconfig entry also depend on common clock support?

> +
> +	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;

[Severity: High]
What happens if devm_kcalloc() returns null in aml_ao_i3c_tail_clocks()?

new_clks is passed directly to memcpy() and dereferenced at
new_clks[master->num_clks] without checking whether the allocation succeeded.
Should this return -ENOMEM if !new_clks?

> +	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");

[Severity: High]
On the AO variant, is it safe to access master->regs + AML_AO_I3C_FCLK_DIV_CTRL
before pclk is prepared and enabled?

In svc_i3c_master_probe(), master->drvdata->vendor_init(master) is called right
after devm_clk_bulk_get_all(), before clk_bulk_prepare_enable() enables pclk
and before pm_runtime_enable() is called:

    svc_i3c_master_probe()
        devm_clk_bulk_get_all()
        aml_i3c_init()
            aml_ao_i3c_tail_clocks()
                devm_clk_hw_register(&ao->div.hw)
                    __clk_core_init()
                        clk_divider_recalc_rate()
            clk_set_rate(fclk, fin_freq)
                clk_divider_set_rate()
        ...
        clk_bulk_prepare_enable()
        ...
        pm_runtime_enable()

Both devm_clk_hw_register(&ao->div.hw) and clk_set_rate(fclk, fin_freq) read
and write master->regs + AML_AO_I3C_FCLK_DIV_CTRL while pclk is still
disabled, and none of the registered AO clocks set CLK_OPS_PARENT_ENABLE.

In addition, because devm_clk_hw_register() runs before pm_runtime_enable(),
runtime PM is not bound to the registered clocks, so reading debugfs
clk_summary while the controller is runtime-suspended (and pclk is gated by
svc_i3c_runtime_suspend()) will also read master->regs +
AML_AO_I3C_FCLK_DIV_CTRL with pclk disabled.

Should pclk be enabled before registering and programming the internal divider
clocks (or should the AO clocks use CLK_OPS_PARENT_ENABLE / runtime PM)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-a9_i3c-v2-0-0c474bfccffa@amlogic.com?part=1

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

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

* Re: [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support
  2026-11-02 11:05 ` [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support Junyi Zhao via B4 Relay
  2026-11-02 11:16   ` sashiko-bot
@ 2026-11-02 21:44   ` Frank Li
  2026-11-02 21:48   ` Frank Li
  2 siblings, 0 replies; 8+ messages in thread
From: Frank Li @ 2026-11-02 21:44 UTC (permalink / raw)
  To: Junyi Zhao
  Cc: Alexandre Belloni, Frank Li, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, Miquel Raynal, Kees Cook,
	Gustavo A. R. Silva, linux-i3c, devicetree, linux-kernel,
	linux-arm-kernel, linux-amlogic, imx, linux-hardening,
	xianwei.zhao, Kelvin.Zhang

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
>

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

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

* Re: [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support
  2026-11-02 11:05 ` [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support Junyi Zhao via B4 Relay
  2026-11-02 11:16   ` sashiko-bot
  2026-11-02 21:44   ` Frank Li
@ 2026-11-02 21:48   ` Frank Li
  2 siblings, 0 replies; 8+ messages in thread
From: Frank Li @ 2026-11-02 21:48 UTC (permalink / raw)
  To: Junyi Zhao
  Cc: Alexandre Belloni, Frank Li, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, Miquel Raynal, Kees Cook,
	Gustavo A. R. Silva, linux-i3c, devicetree, linux-kernel,
	linux-arm-kernel, linux-amlogic, imx, linux-hardening,
	xianwei.zhao, Kelvin.Zhang

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
>

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

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

* Re: [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support
  2026-11-02 11:05 ` [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support Junyi Zhao via B4 Relay
  2026-11-02 11:18   ` sashiko-bot
@ 2026-11-02 22:10   ` Frank Li
  1 sibling, 0 replies; 8+ messages in thread
From: Frank Li @ 2026-11-02 22:10 UTC (permalink / raw)
  To: Junyi Zhao
  Cc: Alexandre Belloni, Frank Li, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, Miquel Raynal, Kees Cook,
	Gustavo A. R. Silva, linux-i3c, devicetree, linux-kernel,
	linux-arm-kernel, linux-amlogic, imx, linux-hardening,
	xianwei.zhao, Kelvin.Zhang

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
>

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

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

end of thread, other threads:[~2026-11-02 22:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02 11:05 [PATCH v2 0/2] Add Amlogic A9 I3C support Junyi Zhao via B4 Relay
2026-11-02 11:05 ` [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support Junyi Zhao via B4 Relay
2026-11-02 11:18   ` sashiko-bot
2026-11-02 22:10   ` Frank Li
2026-11-02 11:05 ` [PATCH v2 2/2] dt-bindings: i3c: silvaco,i3c-master: Add Amlogic A9 support Junyi Zhao via B4 Relay
2026-11-02 11:16   ` sashiko-bot
2026-11-02 21:44   ` Frank Li
2026-11-02 21:48   ` Frank Li

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

all inboxes | Powered by JetHome®