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@xxxxxxxxxxx>
> > > > ---
> ...
> > > > +     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