On Mon, Dec 29, 2025 at 8:51 AM Cédric Le Goater <[email protected]> wrote:
>
> +phil
>
> On 12/29/25 12:41, Cédric Le Goater wrote:
> > On 12/29/25 11:00, Kane Chen wrote:
> >>> -----Original Message-----
> >>> From: Cédric Le Goater <[email protected]>
> >>> Sent: Sunday, December 28, 2025 1:51 AM
> >>> To: Kane Chen <[email protected]>; Peter Maydell
> >>> <[email protected]>; Steven Lee <[email protected]>; Troy
> >>> Lee <[email protected]>; Jamin Lin <[email protected]>; Andrew
> >>> Jeffery <[email protected]>; Joel Stanley <[email protected]>;
> >>> open list:ASPEED BMCs <[email protected]>; open list:All patches CC
> >>> here <[email protected]>
> >>> Cc: Troy Lee <[email protected]>; [email protected]
> >>> Subject: Re: [PATCH v4 06/19] hw/arm/aspeed: Integrate interrupt 
> >>> controller
> >>> for AST1700
> >>>
> >>> Hello Kane,
> >>>
> >>>> Thank you for the suggestion. Since I need to submit a v5 patch to
> >>>> split the I2C code changes anyway,
> >>>
> >>> Can you please introduce the bus label property at the end of the patch 
> >>> series ?
> >>> Please consider adding a functional test and updating the documentation 
> >>> too.
> >>>
> >>>> I will handle the naming adjustments and other minor fixes myself in
> >>>> that version.
> >>>
> >>> Thanks,
> >>>
> >>> C.
> >>
> >> Hi Cédric,
> >>
> >> If I move the bus label property to the end of this patch series, it will 
> >> trigger
> >> a test failure in the current patch series.
> >
> > Which test ?
> >
> >> To avoid this, I plan to move the bus
> >> label changes into a separate patch series and submit it before the AST1700
> >> series. I believe this approach ensures both series pass the tests 
> >> properly.
> >> What are your thoughts on this?
> >
> > I would like to understand the issue first.
> I see.
>
> The AST2700 functional tests fail :
>
>         self.vm.add_args('-device',
>                           
> 'tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test')
>
> The "bus label" proposal renames the IO expander I2C buses (32) to avoid
> the name conflicts :
>
>            /aspeed.ioexp0.i2c.bus.0 (i2c-bus)
>            ...
>            /aspeed.ioexp0.i2c.bus.15 (i2c-bus)
>
>            /aspeed.ioexp1.i2c.bus.0 (i2c-bus)
>            ...
>            /aspeed.ioexp1.i2c.bus.15 (i2c-bus)
>
> Since this will be exposed in the user API, it would be best to avoid
> introducing poorly chosen names. Having so many I2C buses (48) in a
> single machine is somewhat new in QEMU and I am not aware of any naming
> convention for this.
>
> May be others do ?
>
> Thanks,
>
> C.

I'm not aware of any convention, but I'd argue the current naming with
the bus label makes sense. A i2c bus on the main machine is
"aspeed.i2c.bus.%d" which clearly makes it easy to differenciate but
see that the two busses are somehow related. Maybe it'd be worth
changing the `aspeed_i2c_class_init` to make this relation more
obvious by not using TYPE_ASPEED_I2C_BUS but use the string
explicitly?

Thanks,
Nabih

Reply via email to