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

[Junyi Zhao <junyi.zhao@amlogic.com>]

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