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

From: Frank Li <Frank.li@oss.nxp.com>
To: Junyi Zhao <junyi.zhao@amlogic.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Frank Li <Frank.Li@nxp.com>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Kees Cook <kees@kernel.org>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	linux-i3c@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-amlogic@lists.infradead.org, imx@lists.linux.dev,
	linux-hardening@vger.kernel.org, xianwei.zhao@amlogic.com,
	Kelvin.Zhang@amlogic.com
Subject: Re: [PATCH v2 1/2] i3c: master: svc: Add amlogic a9 support
Date: Tue, 3 Nov 2026 21:19:04 -0600	[thread overview]
Message-ID: <auqkKAKRZZ0pA-R3@SMW015318> (raw)
In-Reply-To: <2210c679-0577-49b9-b8c9-fffe1d88baed@amlogic.com>

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

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