On 1/9/26 10:59, Kane Chen wrote:
-----Original Message-----
From: Cédric Le Goater <[email protected]>
Sent: Thursday, January 8, 2026 6:24 PM
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 14/19] hw/arm/aspeed: attach I2C device to AST1700
model

Hi Kane,

Thanks for your suggestions. I have refactored the bus naming logic to
align with your comments. The decision making for the bus name has
been moved up to the SoC level, and the redundant "aspeed" prefix has
been removed.

Here is a summary of the changes:
1. Added a bus-label property to AspeedAST1700SoCState. This allows the
top-level
    SoC (e.g., AST2700) to define the label during its initialization or realize
phase.
2. The bus-label is passed from aspeed_ast1700_realize to the I2C controller
    (AspeedI2CState).
3. In aspeed_i2c_realize, the controller generates unique names using the
bus-label.
    These names are passed to the AspeedI2CBus through a new bus-name
property
    during the initialization of the buses.

With these changes, the new object hierarchies and bus names are as
follows:
BMC: /i2c/bus[0]/aspeed.i2c.bus.0
IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/ioexp0.0
IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/ioexp1.0

The names in the object hierarchy should not have changed, only the bus
names exposed to the user are impacted.

Sorry, I may not fully understand your point here, so I'd like to double-check.
 From my understanding, the object hierarchy itself has not been changed, and
only the user-visible bus names are affected. Below is the current object 
hierarchy
without my changes:

BMC: /i2c/bus[0]/aspeed.i2c.bus.0
IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/aspeed.i2c.bus.0
IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/aspeed.i2c.bus.0

ah yes. The "child<i2c-bus>" objects are really deep in the hierarchy.
I think it is fine.



I believe this matches your comment that the object hierarchy remains the same,
while the bus naming logic is adjusted. Please let me know if you were expecting
a different result here, or if there is still something I should change.


I have also verified that this naming convention does not require
changes to existing test scripts, and all functional tests passed successfully.

If you have no further concerns regarding this approach, I will submit
the updated patch series.

Please separate the bus-label change from the rest. I am expecting a
functional test case too, maybe we should update the sdk version to v10.00
first ?

Regarding the functional test case: currently, our BMC releases do not enable 
AST1700
by default. I tested the image on other platforms instead. I noticed that the 
AST2700
DCSCM image includes a DTB with AST1700 enabled, but I ran into an image size 
issue.

On AST2700 EVB, the BMC image size is 128 MB, while on AST2700 DCSCM it is 64 
MB.
This prevents directly loading the DCSCM image on the EVB. As a workaround, I 
concatenated
the DCSCM image twice to match the 128 MB size, which allowed the image to boot 
and
proceed with further testing. However, this feels like an unexpected and 
non-ideal
approach for testing.
Did you try using the "fmc-model" machine option ?

Do you have any suggestions on how you would prefer this situation to be 
handled?

We could add a new "ast2700-dc-scm" machine too, if it make sense.

Thanks,

C.


Reply via email to