Re: [Qemu-devel] [PATCH] hw/arm/raspi: avoid reparenting the sd card during qbus tree reset

[Damien Hedde]

On 9/6/19 12:36 PM, Peter Maydell wrote:
> On Wed, 4 Sep 2019 at 17:23, Damien Hedde <address@hidden> wrote:
>>
>> In the raspi machine, the sd card can be on several sd bus (in reality
>> there is one bus but several controllers). It is initially created in
>> the "sd-bus" child in the gpio peripheral. Then is moved (parent bus
>> changes) during machine reset in the "sdhci-bus". It can be moved again
>> by software between the "sdhci-bus" and another bus ("bcm2835-sdhost-bus").
>> Here's the corresponding qbus tree of the raspi machine:
>>  + sysbus
>>    * bcm2835_gpio
>>      + sd-bus
>>        * sd-card
>>    * bcm2835-sdhost
>>      + bcm2835-sdhost-bus
>>    * generic-sdhci
>>      + sdhci-bus
>>
>> During the initial machine reset, the sd card is moved. Since reset is
>> based on qbus tree, moving the card during the reset seems odd (it changes
>> the qbus tree). In this case, because of the order the qbus tree is
>> walked, the sd card ends up being reset twice; the effective reset order call
>> follows:
>>  1 sd-card
>>  2 sd-bus
>>  3 bcm2835_gpio        -> move the sd-card to sdhci_bus
>>  4 bcm2835-sdhost-bus
>>  5 bcm2835-sdhost
>>  6 sd-card             (again)
>>  7 sdhci-bus
>>  8 generic-sdhci
>>
>> This patch adds a raspi machine reset method which moves the sd card
>> to the sdhci-bus before doing the whole reset (which will try to do the
>> move too). By anticipating the move we avoid changing the qdev tree while
>> resetting it.
>>
>> In consequence the step 1 is skipped in the previous list: when reset starts
>> the sd-card is already not a child of bcm2835_gpio.
> 
> The solution proposed in this patch pushes something that should
> really be the business just of the SoC model out to the machine
> model level; it would be nice to be able to avoid that.

The problem is sysbus is the only common "reset" ancestor of all
sd-card-buses. So we don't really have a lot of places to do this.

I could move the proposed code to the reset method of
bcm2835_peripherals which creates the bcm3825_gpio, bcm2835-sdhost and
generic-sdhci so we don't have to seek these.
It will works because the reset of bcm2835_peripherals is called before
the others.
But in terms of reset/qdev hiearchy, the 'bcm2835_peripherals' it at the
same level of bcm2835_gpio.

The reset order would be:

1 bcm2835_peripherals -> move the sd-card to sdhci_bus.
2   sd-bus
3 bcm2835_gpio
4   bcm2835-sdhost-bus
5 bcm2835-sdhost
6     sd-card
7   sdhci-bus
8 generic-sdhci

Would that be an acceptable solution ?

--
Thanks
Damien