Re: [PATCH 04/24] aspeed: Don't create unwanted "ftgmac100", "aspeed-mmi" devices

[Markus Armbruster]

"Andrew Jeffery" <address@hidden> writes:

> On Mon, 18 May 2020, at 21:49, Cédric Le Goater wrote:
>> On 5/18/20 7:03 AM, Markus Armbruster wrote:
>> > These devices are optional, and controlled by @nb_nics.
>> > aspeed_soc_ast2600_init() and aspeed_soc_init() create the maximum
>> > supported number.  aspeed_soc_ast2600_realize() and
>> > aspeed_soc_realize() realize only the wanted number.  Works, although
>> > it can leave unrealized devices hanging around in the QOM composition
>> > tree.  Affects machines ast2500-evb, ast2600-evb, palmetto-bmc,
>> > romulus-bmc, swift-bmc, tacoma-bmc, and witherspoon-bmc.
>> > 
>> > Make the init functions create only the wanted ones.  Visible in "info
>> > qom-tree"; here's the change for ast2600-evb:
>> > 
>> >      /machine (ast2600-evb-machine)
>> >        [...]
>> >        /soc (ast2600-a1)
>> >          [...]
>> >          /ftgmac100[0] (ftgmac100)
>> >            /ftgmac100[0] (qemu:memory-region)
>> >     -    /ftgmac100[1] (ftgmac100)
>> >     -    /ftgmac100[2] (ftgmac100)
>> >     -    /ftgmac100[3] (ftgmac100)
>> >          /gpio (aspeed.gpio-ast2600)
>> >          [...]
>> >          /mii[0] (aspeed-mmi)
>> >            /aspeed-mmi[0] (qemu:memory-region)
>> >     -    /mii[1] (aspeed-mmi)
>> >     -    /mii[2] (aspeed-mmi)
>> >     -    /mii[3] (aspeed-mmi)
>> >          /rtc (aspeed.rtc)
>> > 
>> > I'm not sure creating @nb_nics devices makes sense.  How many does the
>> > physical chip provide?
>> 
>> The AST2400, AST2500 SoC have 2 macs and the AST2600 has 4. Each machine
>> define the one it uses, generally MAC0 but the tacoma board uses MAC3.
>> 
>> Shouldn't the model reflect the real address space independently from
>> the NIC backends defined on the command line ?  
>
> That's my feeling too, though I'm not sure what to make of the unrealised 
> devices
> in the QOM tree. Does it matter? It hasn't bothered me.

Depending on what the initialization code does, unrealized devices can
be anything from a little wasted memory to open bear trap.  I don't
really expect the latter extreme in the code, as I expect bear traps to
quickly catch the developer that set them.

I guess the unrealized devices cleaned up in this patch did no actual
harm.

Still, it's an unhealthy state, and that's why I clean it up.  "[PATCH
24/24] qdev: Assert onboard devices all get realized properly" should
ensure we stay clean.