Re: [Qemu-devel] [PATCH 1/3] hw/sd/pxa2xx_mmci: convert to SysBusDevice object

[Markus Armbruster]

"Kevin O'Connor" <address@hidden> writes:

> On Sun, Dec 06, 2015 at 04:02:14PM -0800, Peter Crosthwaite wrote:
>> On Fri, Dec 4, 2015 at 11:24 AM, Kevin O'Connor <address@hidden> wrote:
>> > On Fri, Dec 04, 2015 at 10:50:21AM -0800, Peter Crosthwaite wrote:
>> >> > FWIW, I don't think the SD card will be qdevified because it doesn't
>> >> > need a bus.  It's similar indeed to SerialState, which was supposed to
>> >> > be the poster child of QOM embedding and never got QOMified.
>> >>
>> >> SD is a bus in its own right and should be busified and QOMified IMO.
>> >> SDHCI can talk to non-sd cards (SDIO). There is also a range of
>> >> incompatible cards that you can talk to - MMC/eMMC/SD(H|S|X)C. I think
>> >> anything that couples the controller to an SD card is a bug, the card
>> >> and device should be arranged as separate devices.
>> >>
>> >> > A host controller controls exactly one SD card, the SSI bridge is also
>> >> > for exactly one SD card, etc.
>> >>
>> >> I think you can RYO chip selects with a GPIO and control multiple SD
>> >> cards with one SDHCI.
>> >
>> > In practice, the SDHCI controllers are one-to-one with cards.  This is
>> > codified in the sdhci spec as it has a "card present" bit and "port
>> > location" information that is per controller.
>> >
>> 
>> But SDHCI is only one SD controller implementation. Other SD
>> controllers may easily implement the SDIO bus as non 1:1. I also don't
>> see why a 1:1 bus should have a completely different implementation
>> that propagates through to UI level issues. SD is a bus like any other
>> IMO.
>> 
>> > I suppose in theory, one could put an SDHCI contoller into SPI
>> > compatibility mode and "hot wire" it into a bus, but qemu doesn't
>> > support that anyway, and it is a lot of complexity for something that
>> > is not done in practice.
>> >
>> 
>> Note that the chip selects for SD are on the copper interface as well,
>> so you can easily implement multi-device SD bus on the level above
>> SDHCI. SPI mode should not required.
>
> The "card select" line becomes a data line (DAT3) when not in SPI
> mode.  That said, the MMC specs do define a convoluted mechanism for
> assigning unique ids to each card so that a bus could be implemented.
> (Which doesn't apply to SD cards, by my read of the SD specs.)
>
>>Even if SDHCI does have
>> card-detection CS control logic this can be demultiplexed to multiple
>> cards by extra logic. We also have precedent of implementers of SDHCI
>> opting-out of features like power control, and assumptions about how
>> SDHCI is wired have already led to bugs like this:
>> 
>> https://www.mail-archive.com/address@hidden/msg338495.html
>> 
>> We don't need to be implementing the multi CS logic today, but making
>> UI level decisions based on this 1:1 assumption seems wrong.
>
> I have no objection to a different internal or external represention
> to the SD cards.  However, the immiediate choice seems to be whether
> to disable pci-sdhci or not.  It seems unfortunate to me that it would
> be disabled.  The QEMU SD code (beyond just the sdhci controller) is
> not organized as a bus, and it's not clear to me that there are real
> world gains to be had by modeling it as a bus.
>
> My interest in the sd cards is for testing purposes.  Several recent
> Intel chipsets include multiple sdhci controllers on them, and several
> currently shipping devices use them for both eMMC storage and for
> external SD card ports - in particular several of the Chromebooks have
> them.  Being able to perform simple sanity checks in QEMU is helpful.

The device is obviously useful.  However, there is dissent on how it
ought to be modelled.  The modelling is visible at the -device user
interface.  By making the device available there in 2.5, we commit to
the current modelling's user interface before we reach consensus on how
it ought to be modelled.  Options:

(1) Make device "sdhci-pci" unavailable with -device until we reach
    consensus.  This is what we normally do.  Trivial patch is on list.

(2) Mark the properties that belong to the card rather than the
    controller as experimental until we reach consensus, by prefixing
    their name with "x-".  Needs a patch.

(3) Keep it available, commit to the user interface, deal with the
    consequences if and when they arise.

I think (1) is the most prudent, but (2) should work, too.  Having dealt
with consequences of prior modelling mistakes, I dislike 3.

Opinions?