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

[Frank Li <Frank.li@oss.nxp.com>]

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