[3/4] sdhci: Support SDHCI devices on PCI

Support for PCI devices following the "SD Host Controller Simplified
Specification Version 2.00" spec.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
 default-configs/pci.mak  |  2 ++
 hw/sd/sdhci.c            | 44 ++++++++++++++++++++++++++++++++++++++++++++
 hw/sd/sdhci.h            |  9 ++++++++-
 include/hw/pci/pci.h     |  1 +
 include/hw/pci/pci_ids.h |  1 +
 5 files changed, 56 insertions(+), 1 deletion(-)

On 18/11/2014 05:26, Kevin O'Connor wrote:
> Support for PCI devices following the "SD Host Controller Simplified
> Specification Version 2.00" spec.
> 
> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
> ---
>  default-configs/pci.mak  |  2 ++
>  hw/sd/sdhci.c            | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/sd/sdhci.h            |  9 ++++++++-
>  include/hw/pci/pci.h     |  1 +
>  include/hw/pci/pci_ids.h |  1 +
>  5 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/default-configs/pci.mak b/default-configs/pci.mak
> index 91b1e92..a186c39 100644
> --- a/default-configs/pci.mak
> +++ b/default-configs/pci.mak
> @@ -30,3 +30,5 @@ CONFIG_IPACK=y
>  CONFIG_WDT_IB6300ESB=y
>  CONFIG_PCI_TESTDEV=y
>  CONFIG_NVME_PCI=y
> +CONFIG_SD=y
> +CONFIG_SDHCI=y
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 05b0c50..55709da 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1220,6 +1220,49 @@ static Property sdhci_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +static int sdhci_pci_init(PCIDevice *dev)
> +{
> +    SDHCIState *s = PCI_SDHCI(dev);
> +    dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */
> +    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
> +    sdhci_initfn(s);
> +    s->buf_maxsz = sdhci_get_fifolen(s);
> +    s->fifo_buffer = g_malloc0(s->buf_maxsz);
> +    s->irq = pci_allocate_irq(dev);
> +    memory_region_init_io(&s->iomem, OBJECT(s), &sdhci_mmio_ops, s, "sdhci",
> +            SDHC_REGISTERS_MAP_SIZE);
> +    pci_register_bar(dev, 0, 0, &s->iomem);
> +    return 0;
> +}
> +
> +static void sdhci_pci_exit(PCIDevice *dev)
> +{
> +    SDHCIState *s = PCI_SDHCI(dev);
> +    sdhci_uninitfn(s);
> +}
> +
> +static void sdhci_pci_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    k->init = sdhci_pci_init;
> +    k->exit = sdhci_pci_exit;
> +    k->vendor_id = PCI_VENDOR_ID_QEMU;
> +    k->device_id = PCI_DEVICE_ID_SDHCI;
> +    k->class_id = PCI_CLASS_SYSTEM_SDHCI;
> +    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> +    dc->vmsd = &sdhci_vmstate;
> +    dc->props = sdhci_properties;
> +}
> +
> +static const TypeInfo sdhci_pci_info = {
> +    .name = TYPE_PCI_SDHCI,
> +    .parent = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(SDHCIState),
> +    .class_init = sdhci_pci_class_init,
> +};
> +
>  static void sdhci_sysbus_init(Object *obj)
>  {
>      SDHCIState *s = SYSBUS_SDHCI(obj);
> @@ -1265,6 +1308,7 @@ static const TypeInfo sdhci_sysbus_info = {
>  
>  static void sdhci_register_types(void)
>  {
> +    type_register_static(&sdhci_pci_info);
>      type_register_static(&sdhci_sysbus_info);
>  }
>  
> diff --git a/hw/sd/sdhci.h b/hw/sd/sdhci.h
> index 9fbf682..3352d23 100644
> --- a/hw/sd/sdhci.h
> +++ b/hw/sd/sdhci.h
> @@ -26,6 +26,7 @@
>  #define SDHCI_H
>  
>  #include "qemu-common.h"
> +#include "hw/pci/pci.h"
>  #include "hw/sysbus.h"
>  #include "hw/sd.h"
>  
> @@ -232,7 +233,10 @@ enum {
>  
>  /* SD/MMC host controller state */
>  typedef struct SDHCIState {
> -    SysBusDevice busdev;
> +    union {
> +        PCIDevice pcidev;
> +        SysBusDevice busdev;
> +    };
>      SDState *card;
>      MemoryRegion iomem;
>  
> @@ -281,6 +285,9 @@ typedef struct SDHCIState {
>  
>  extern const VMStateDescription sdhci_vmstate;
>  
> +#define TYPE_PCI_SDHCI "sdhci-pci"
> +#define PCI_SDHCI(obj) OBJECT_CHECK(SDHCIState, (obj), TYPE_PCI_SDHCI)
> +
>  #define TYPE_SYSBUS_SDHCI "generic-sdhci"
>  #define SYSBUS_SDHCI(obj)                               \
>       OBJECT_CHECK(SDHCIState, (obj), TYPE_SYSBUS_SDHCI)
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index c352c7b..fae77cb 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -53,6 +53,7 @@
>  /* QEMU/Bochs VGA (0x1234) */
>  #define PCI_VENDOR_ID_QEMU               0x1234
>  #define PCI_DEVICE_ID_QEMU_VGA           0x1111
> +#define PCI_DEVICE_ID_SDHCI              0x2222

0x1234 is not a registered PCI id, and it's only used for VGA for
backwards-compatibility reasons.

Please use 1b36:0005 instead, and document it in docs/specs/pci-ids.txt,
or use a real-world PCI vendor/device pair (if you can find one that
Linux doesn't have quirks for; that could be hard).

Paolo

>  
>  /* VMWare (0x15ad) */
>  #define PCI_VENDOR_ID_VMWARE             0x15ad
> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
> index 321d622..d7be386 100644
> --- a/include/hw/pci/pci_ids.h
> +++ b/include/hw/pci/pci_ids.h
> @@ -31,6 +31,7 @@
>  
>  #define PCI_CLASS_MEMORY_RAM             0x0500
>  
> +#define PCI_CLASS_SYSTEM_SDHCI           0x0805
>  #define PCI_CLASS_SYSTEM_OTHER           0x0880
>  
>  #define PCI_CLASS_SERIAL_USB             0x0c03
>

On Tue, Nov 18, 2014 at 07:27:24AM +0100, Paolo Bonzini wrote:
> On 18/11/2014 05:26, Kevin O'Connor wrote:
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -53,6 +53,7 @@
> >  /* QEMU/Bochs VGA (0x1234) */
> >  #define PCI_VENDOR_ID_QEMU               0x1234
> >  #define PCI_DEVICE_ID_QEMU_VGA           0x1111
> > +#define PCI_DEVICE_ID_SDHCI              0x2222
> 
> 0x1234 is not a registered PCI id, and it's only used for VGA for
> backwards-compatibility reasons.
> 
> Please use 1b36:0005 instead, and document it in docs/specs/pci-ids.txt,
> or use a real-world PCI vendor/device pair (if you can find one that
> Linux doesn't have quirks for; that could be hard).

Hi Paolo.  Thanks for reviewing.

I know recent Intel chips (eg, baytrail) have a builtin sdhci
controller (eg, 8086:0f16).  However, that has quirks defined in the
Linux driver.  Basic functionality still does seem to work though when
I use those ids in qemu.  The same basic functionality also seems to
work when I use 1b36:0005 as well.

Is there a preference then to use the redhat ids?  Gerd, you seem to
be in charge of the redhat pci ids - are you okay if I us one (should
it be the existing 0005 or add 0006)?

Thanks,
-Kevin

Hi,

> I know recent Intel chips (eg, baytrail) have a builtin sdhci
> controller (eg, 8086:0f16).  However, that has quirks defined in the
> Linux driver.  Basic functionality still does seem to work though when
> I use those ids in qemu.  The same basic functionality also seems to
> work when I use 1b36:0005 as well.
> 
> Is there a preference then to use the redhat ids?

Yes, for pci class devices (so the guest driver do not match specific
pci vendor/device ids, except for quirks) we started using the redhat
ids.

> Gerd, you seem to
> be in charge of the redhat pci ids - are you okay if I us one (should
> it be the existing 0005 or add 0006)?

Just grab the first free (which seems to be 0005) and add a line to
docs/specs/pci-ids.txt with your patch.

cheers,
  Gerd

On 21/11/2014 08:20, Gerd Hoffmann wrote:
> Yes, for pci class devices (so the guest driver do not match specific
> pci vendor/device ids, except for quirks) we started using the redhat
> ids.
> 
> > Gerd, you seem to
> > be in charge of the redhat pci ids - are you okay if I us one (should
> > it be the existing 0005 or add 0006)?
> 
> Just grab the first free (which seems to be 0005) and add a line to
> docs/specs/pci-ids.txt with your patch

0005 is used by hw/misc/pci-testdev.c (see docs/specs/pci-testdev.txt,
but the device/vendor ID is undocumented there as well), so 0006.

Paolo