Re: [Qemu-devel] [PATCH v2 4/6] hw/sd/pl181: expose a SDBus and connect the SDCard to it

[Philippe Mathieu-Daudé]

Hi Peter,

On 02/06/2018 10:06 AM, Peter Maydell wrote:
> On 6 February 2018 at 12:43, Philippe Mathieu-Daudé <address@hidden> wrote:
>> Hi Alistair,
>>
>> On 01/31/2018 01:41 PM, Alistair Francis wrote:
>>> On Mon, Jan 22, 2018 at 7:58 PM, Philippe Mathieu-Daudé <address@hidden> 
>>> wrote:
>>>> using the sdbus_*() API.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>>>> ---
>>>>  hw/sd/pl181.c | 31 ++++++++++++++++++++-----------
>>>>  1 file changed, 20 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
>>>> index 3ba1f7dd23..ce696c5d7d 100644
>>>> --- a/hw/sd/pl181.c
>>>> +++ b/hw/sd/pl181.c
>>>> @@ -33,6 +33,7 @@ typedef struct PL181State {
>>>>      SysBusDevice parent_obj;
>>>>
>>>>      MemoryRegion iomem;
>>>> +    SDBus sdbus;
>>>>      SDState *card;
>>>
>>> Shouludn't card be removed?
>>
>> Not yet :( It is still used by sd_set_cb() in pl181_reset().
> 
> I think you have to change that sd_set_cb() code now.
> If you look at sd_cardchange() it uses "is this SD card
> object on an SDBus" to determine whether to notify the
> controller via the old-API IRQ lines, or using the
> set_inserted() and set_readonly() callbacks on the SDBusClass.

Oh, this can get simplified, cool!

>> In my first approach [1] I added the SDBus SLAVE/MASTER interfaces and
>> the cards inserted/readonly signals were only accessible by the bus, not
>> the HCI, leaving the SDCard objects only pluggable to SDBus (removing
>> the sdbus_reparent_card() need). But since it was out of scope for the
>> UHS cards goal, I kept it for later.
> 
> How do you manage to get rid of sdbus_reparent_card()? Raspi
> needs it for its weirdo multiplexed SD controller setup, and
> AFAIK we don't have a way to say "this thing is hotpluggable
> but not by the user" yet...

The card is hotpluggable, the bus isn't.

An unique SDBus is created in the bcm2835_peripherals object (model
closer than hardware), sdhci/sdhost/gpio controllers use it via property
links.
What I don't do is checking the gpio selector (sd_fsel), hoping the
guest isn't nasty enough to use both SD controllers at once...

> PS: have you checked that these sd card refactorings don't
> accidentally break the monitor "change" and "eject" commands
> operating on SD cards ? (They are a bit weird because they
> affect which backing file is attached to the SD card object,
> rather than actually deleting and recreating the SD card
> object.)

Nop, but I can now add a qtest for this :)

Regards,

Phil.