[Qemu-devel] [RFC][PATCH 0/4] Add support for SDHCI PCI devices

[Qemu-devel] [RFC][PATCH 0/4] Add support for SDHCI PCI devices

[Kevin O'Connor]

I'd like to be able to emulate SD cards on x86 machines for testing
purposes.  I put together a patch series to modify the existing SDHCI
code so that it can also be used as a PCI device, and can thus be used
on any PCI platform (such as x86).

Vincent Palatin had patches to do this a couple of years ago, but his
patches no longer apply to the current qemu tree.  See:
  https://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg02833.html
This series is not directly based on Vincent's patches, and is instead
mainly based on the sdhci code currently in the qemu tree.

I've lightly tested this series on x86 and confirmed that a standard
linux kernel can find and mount the given device.  To test, I add the
following to the command line: -device sdhci-pci -sd my-drive

Unfortunately, I have not been able to test the existing arm support
(which uses sysbus), but I will test if I can find an available qemu
arm image.

I'm not that familiar with QEMU internals.  Comments welcome.

-Kevin

Kevin O'Connor (4):
  sdhci: Remove class "virtual" methods
  sdhci: Add "sysbus" to sdhci QOM types and methods
  sdhci: Support SDHCI devices on PCI
  sdhci: Set a default frequency clock

 default-configs/pci.mak  |   2 +
 hw/sd/sdhci.c            | 199 +++++++++++++++++++++++++----------------------
 hw/sd/sdhci.h            |  39 +++-------
 include/hw/pci/pci.h     |   1 +
 include/hw/pci/pci_ids.h |   1 +
 5 files changed, 120 insertions(+), 122 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/4] sdhci: Remove class "virtual" methods

[Kevin O'Connor]

The SDHCIClass defines a series of class "methods".  However, no code
in the QEMU tree overrides these methods or even uses them outside of
sdhci.c.

Remove the virtual methods and replace them with direct calls to the
underlying functions.  This simplifies the process of extending the
sdhci code to support PCI devices (which have a different parent
class).

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
 hw/sd/sdhci.c | 112 ++++++++++++++++++----------------------------------------
 hw/sd/sdhci.h |  24 -------------
 2 files changed, 35 insertions(+), 101 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index b380050..4cbd354 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -198,12 +198,7 @@ static void sdhci_reset(SDHCIState *s)
     s->stopped_state = sdhc_not_stopped;
 }
 
-static void sdhci_do_data_transfer(void *opaque)
-{
-    SDHCIState *s = (SDHCIState *)opaque;
-
-    SDHCI_GET_CLASS(s)->data_transfer(s);
-}
+static void sdhci_data_transfer(void *opaque);
 
 static void sdhci_send_command(SDHCIState *s)
 {
@@ -261,7 +256,7 @@ static void sdhci_send_command(SDHCIState *s)
 
     if (s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
         s->data_count = 0;
-        sdhci_do_data_transfer(s);
+        sdhci_data_transfer(s);
     }
 }
 
@@ -367,9 +362,9 @@ static uint32_t sdhci_read_dataport(SDHCIState *s, unsigned size)
                  /* stop at gap request */
                 (s->stopped_state == sdhc_gap_read &&
                  !(s->prnsts & SDHC_DAT_LINE_ACTIVE))) {
-                SDHCI_GET_CLASS(s)->end_data_transfer(s);
+                sdhci_end_transfer(s);
             } else { /* if there are more data, read next block from card */
-                SDHCI_GET_CLASS(s)->read_block_from_card(s);
+                sdhci_read_block_from_card(s);
             }
             break;
         }
@@ -410,7 +405,7 @@ static void sdhci_write_block_to_card(SDHCIState *s)
     if ((s->trnmod & SDHC_TRNS_MULTI) == 0 ||
             ((s->trnmod & SDHC_TRNS_MULTI) &&
             (s->trnmod & SDHC_TRNS_BLK_CNT_EN) && (s->blkcnt == 0))) {
-        SDHCI_GET_CLASS(s)->end_data_transfer(s);
+        sdhci_end_transfer(s);
     } else if (s->norintstsen & SDHC_NISEN_WBUFRDY) {
         s->norintsts |= SDHC_NIS_WBUFRDY;
     }
@@ -422,7 +417,7 @@ static void sdhci_write_block_to_card(SDHCIState *s)
         if (s->norintstsen & SDHC_EISEN_BLKGAP) {
             s->norintsts |= SDHC_EIS_BLKGAP;
         }
-        SDHCI_GET_CLASS(s)->end_data_transfer(s);
+        sdhci_end_transfer(s);
     }
 
     sdhci_update_irq(s);
@@ -450,7 +445,7 @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t value, unsigned size)
             s->data_count = 0;
             s->prnsts &= ~SDHC_SPACE_AVAILABLE;
             if (s->prnsts & SDHC_DOING_WRITE) {
-                SDHCI_GET_CLASS(s)->write_block_to_card(s);
+                sdhci_write_block_to_card(s);
             }
         }
     }
@@ -537,7 +532,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
     }
 
     if (s->blkcnt == 0) {
-        SDHCI_GET_CLASS(s)->end_data_transfer(s);
+        sdhci_end_transfer(s);
     } else {
         if (s->norintstsen & SDHC_NISEN_DMA) {
             s->norintsts |= SDHC_NIS_DMA;
@@ -571,7 +566,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s)
         s->blkcnt--;
     }
 
-    SDHCI_GET_CLASS(s)->end_data_transfer(s);
+    sdhci_end_transfer(s);
 }
 
 typedef struct ADMADescr {
@@ -758,7 +753,7 @@ static void sdhci_do_adma(SDHCIState *s)
 
                 sdhci_update_irq(s);
             }
-            SDHCI_GET_CLASS(s)->end_data_transfer(s);
+            sdhci_end_transfer(s);
             return;
         }
 
@@ -771,9 +766,9 @@ static void sdhci_do_adma(SDHCIState *s)
 
 /* Perform data transfer according to controller configuration */
 
-static void sdhci_data_transfer(SDHCIState *s)
+static void sdhci_data_transfer(void *opaque)
 {
-    SDHCIClass *k = SDHCI_GET_CLASS(s);
+    SDHCIState *s = (SDHCIState *)opaque;
 
     if (s->trnmod & SDHC_TRNS_DMA) {
         switch (SDHC_DMA_TYPE(s->hostctl)) {
@@ -784,9 +779,9 @@ static void sdhci_data_transfer(SDHCIState *s)
             }
 
             if ((s->blkcnt == 1) || !(s->trnmod & SDHC_TRNS_MULTI)) {
-                k->do_sdma_single(s);
+                sdhci_sdma_transfer_single_block(s);
             } else {
-                k->do_sdma_multi(s);
+                sdhci_sdma_transfer_multi_blocks(s);
             }
 
             break;
@@ -796,7 +791,7 @@ static void sdhci_data_transfer(SDHCIState *s)
                 break;
             }
 
-            k->do_adma(s);
+            sdhci_do_adma(s);
             break;
         case SDHC_CTRL_ADMA2_32:
             if (!(s->capareg & SDHC_CAN_DO_ADMA2)) {
@@ -804,7 +799,7 @@ static void sdhci_data_transfer(SDHCIState *s)
                 break;
             }
 
-            k->do_adma(s);
+            sdhci_do_adma(s);
             break;
         case SDHC_CTRL_ADMA2_64:
             if (!(s->capareg & SDHC_CAN_DO_ADMA2) ||
@@ -813,7 +808,7 @@ static void sdhci_data_transfer(SDHCIState *s)
                 break;
             }
 
-            k->do_adma(s);
+            sdhci_do_adma(s);
             break;
         default:
             ERRPRINT("Unsupported DMA type\n");
@@ -823,11 +818,11 @@ static void sdhci_data_transfer(SDHCIState *s)
         if ((s->trnmod & SDHC_TRNS_READ) && sd_data_ready(s->card)) {
             s->prnsts |= SDHC_DOING_READ | SDHC_DATA_INHIBIT |
                     SDHC_DAT_LINE_ACTIVE;
-            SDHCI_GET_CLASS(s)->read_block_from_card(s);
+            sdhci_read_block_from_card(s);
         } else {
             s->prnsts |= SDHC_DOING_WRITE | SDHC_DAT_LINE_ACTIVE |
                     SDHC_SPACE_AVAILABLE | SDHC_DATA_INHIBIT;
-            SDHCI_GET_CLASS(s)->write_block_to_card(s);
+            sdhci_write_block_to_card(s);
         }
     }
 }
@@ -858,8 +853,9 @@ sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num)
     return true;
 }
 
-static uint32_t sdhci_read(SDHCIState *s, unsigned int offset, unsigned size)
+static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)
 {
+    SDHCIState *s = (SDHCIState *)opaque;
     uint32_t ret = 0;
 
     switch (offset & ~0x3) {
@@ -880,7 +876,7 @@ static uint32_t sdhci_read(SDHCIState *s, unsigned int offset, unsigned size)
         break;
     case  SDHC_BDATA:
         if (sdhci_buff_access_is_sequential(s, offset - SDHC_BDATA)) {
-            ret = SDHCI_GET_CLASS(s)->bdata_read(s, size);
+            ret = sdhci_read_dataport(s, size);
             DPRINT_L2("read %ub: addr[0x%04x] -> %u(0x%x)\n", size, offset,
                       ret, ret);
             return ret;
@@ -948,10 +944,10 @@ static inline void sdhci_blkgap_write(SDHCIState *s, uint8_t value)
             (s->blkgap & SDHC_STOP_AT_GAP_REQ) == 0) {
         if (s->stopped_state == sdhc_gap_read) {
             s->prnsts |= SDHC_DAT_LINE_ACTIVE | SDHC_DOING_READ;
-            SDHCI_GET_CLASS(s)->read_block_from_card(s);
+            sdhci_read_block_from_card(s);
         } else {
             s->prnsts |= SDHC_DAT_LINE_ACTIVE | SDHC_DOING_WRITE;
-            SDHCI_GET_CLASS(s)->write_block_to_card(s);
+            sdhci_write_block_to_card(s);
         }
         s->stopped_state = sdhc_not_stopped;
     } else if (!s->stopped_state && (value & SDHC_STOP_AT_GAP_REQ)) {
@@ -967,7 +963,7 @@ static inline void sdhci_reset_write(SDHCIState *s, uint8_t value)
 {
     switch (value) {
     case SDHC_RESET_ALL:
-        DEVICE_GET_CLASS(s)->reset(DEVICE(s));
+        sdhci_reset(s);
         break;
     case SDHC_RESET_CMD:
         s->prnsts &= ~SDHC_CMD_INHIBIT;
@@ -987,10 +983,12 @@ static inline void sdhci_reset_write(SDHCIState *s, uint8_t value)
 }
 
 static void
-sdhci_write(SDHCIState *s, unsigned int offset, uint32_t value, unsigned size)
+sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
 {
+    SDHCIState *s = (SDHCIState *)opaque;
     unsigned shift =  8 * (offset & 0x3);
     uint32_t mask = ~(((1ULL << (size * 8)) - 1) << shift);
+    uint32_t value = val;
     value <<= shift;
 
     switch (offset & ~0x3) {
@@ -1000,7 +998,7 @@ sdhci_write(SDHCIState *s, unsigned int offset, uint32_t value, unsigned size)
         /* Writing to last byte of sdmasysad might trigger transfer */
         if (!(mask & 0xFF000000) && TRANSFERRING_DATA(s->prnsts) && s->blkcnt &&
                 s->blksize && SDHC_DMA_TYPE(s->hostctl) == SDHC_CTRL_SDMA) {
-            SDHCI_GET_CLASS(s)->do_sdma_multi(s);
+            sdhci_sdma_transfer_multi_blocks(s);
         }
         break;
     case SDHC_BLKSIZE:
@@ -1022,15 +1020,15 @@ sdhci_write(SDHCIState *s, unsigned int offset, uint32_t value, unsigned size)
         MASKED_WRITE(s->cmdreg, mask >> 16, value >> 16);
 
         /* Writing to the upper byte of CMDREG triggers SD command generation */
-        if ((mask & 0xFF000000) || !SDHCI_GET_CLASS(s)->can_issue_command(s)) {
+        if ((mask & 0xFF000000) || !sdhci_can_issue_command(s)) {
             break;
         }
 
-        SDHCI_GET_CLASS(s)->send_command(s);
+        sdhci_send_command(s);
         break;
     case  SDHC_BDATA:
         if (sdhci_buff_access_is_sequential(s, offset - SDHC_BDATA)) {
-            SDHCI_GET_CLASS(s)->bdata_write(s, value >> shift, size);
+            sdhci_write_dataport(s, value >> shift, size);
         }
         break;
     case SDHC_HOSTCTL:
@@ -1118,25 +1116,9 @@ sdhci_write(SDHCIState *s, unsigned int offset, uint32_t value, unsigned size)
             size, offset, value >> shift, value >> shift);
 }
 
-static uint64_t
-sdhci_readfn(void *opaque, hwaddr offset, unsigned size)
-{
-    SDHCIState *s = (SDHCIState *)opaque;
-
-    return SDHCI_GET_CLASS(s)->mem_read(s, offset, size);
-}
-
-static void
-sdhci_writefn(void *opaque, hwaddr off, uint64_t val, unsigned sz)
-{
-    SDHCIState *s = (SDHCIState *)opaque;
-
-    SDHCI_GET_CLASS(s)->mem_write(s, off, val, sz);
-}
-
 static const MemoryRegionOps sdhci_mmio_ops = {
-    .read = sdhci_readfn,
-    .write = sdhci_writefn,
+    .read = sdhci_read,
+    .write = sdhci_write,
     .valid = {
         .min_access_size = 1,
         .max_access_size = 4,
@@ -1175,7 +1157,7 @@ static void sdhci_initfn(Object *obj)
     sd_set_cb(s->card, s->ro_cb, s->eject_cb);
 
     s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_raise_insertion_irq, s);
-    s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_do_data_transfer, s);
+    s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s);
 }
 
 static void sdhci_uninitfn(Object *obj)
@@ -1254,36 +1236,13 @@ static void sdhci_realize(DeviceState *dev, Error ** errp)
     sysbus_init_mmio(sbd, &s->iomem);
 }
 
-static void sdhci_generic_reset(DeviceState *ds)
-{
-    SDHCIState *s = SDHCI(ds);
-    SDHCI_GET_CLASS(s)->reset(s);
-}
-
 static void sdhci_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SDHCIClass *k = SDHCI_CLASS(klass);
 
     dc->vmsd = &sdhci_vmstate;
     dc->props = sdhci_properties;
-    dc->reset = sdhci_generic_reset;
     dc->realize = sdhci_realize;
-
-    k->reset = sdhci_reset;
-    k->mem_read = sdhci_read;
-    k->mem_write = sdhci_write;
-    k->send_command = sdhci_send_command;
-    k->can_issue_command = sdhci_can_issue_command;
-    k->data_transfer = sdhci_data_transfer;
-    k->end_data_transfer = sdhci_end_transfer;
-    k->do_sdma_single = sdhci_sdma_transfer_single_block;
-    k->do_sdma_multi = sdhci_sdma_transfer_multi_blocks;
-    k->do_adma = sdhci_do_adma;
-    k->read_block_from_card = sdhci_read_block_from_card;
-    k->write_block_to_card = sdhci_write_block_to_card;
-    k->bdata_read = sdhci_read_dataport;
-    k->bdata_write = sdhci_write_dataport;
 }
 
 static const TypeInfo sdhci_type_info = {
@@ -1293,7 +1252,6 @@ static const TypeInfo sdhci_type_info = {
     .instance_init = sdhci_initfn,
     .instance_finalize = sdhci_uninitfn,
     .class_init = sdhci_class_init,
-    .class_size = sizeof(SDHCIClass)
 };
 
 static void sdhci_register_types(void)
diff --git a/hw/sd/sdhci.h b/hw/sd/sdhci.h
index a560c3c..9a334ac 100644
--- a/hw/sd/sdhci.h
+++ b/hw/sd/sdhci.h
@@ -279,34 +279,10 @@ typedef struct SDHCIState {
     /* RO Host Controller Version Register always reads as 0x2401 */
 } SDHCIState;
 
-typedef struct SDHCIClass {
-    SysBusDeviceClass busdev_class;
-
-    void (*reset)(SDHCIState *s);
-    uint32_t (*mem_read)(SDHCIState *s, unsigned int offset, unsigned size);
-    void (*mem_write)(SDHCIState *s, unsigned int offset, uint32_t value,
-            unsigned size);
-    void (*send_command)(SDHCIState *s);
-    bool (*can_issue_command)(SDHCIState *s);
-    void (*data_transfer)(SDHCIState *s);
-    void (*end_data_transfer)(SDHCIState *s);
-    void (*do_sdma_single)(SDHCIState *s);
-    void (*do_sdma_multi)(SDHCIState *s);
-    void (*do_adma)(SDHCIState *s);
-    void (*read_block_from_card)(SDHCIState *s);
-    void (*write_block_to_card)(SDHCIState *s);
-    uint32_t (*bdata_read)(SDHCIState *s, unsigned size);
-    void (*bdata_write)(SDHCIState *s, uint32_t value, unsigned size);
-} SDHCIClass;
-
 extern const VMStateDescription sdhci_vmstate;
 
 #define TYPE_SDHCI            "generic-sdhci"
 #define SDHCI(obj)            \
      OBJECT_CHECK(SDHCIState, (obj), TYPE_SDHCI)
-#define SDHCI_CLASS(klass)    \
-     OBJECT_CLASS_CHECK(SDHCIClass, (klass), TYPE_SDHCI)
-#define SDHCI_GET_CLASS(obj)  \
-     OBJECT_GET_CLASS(SDHCIClass, (obj), TYPE_SDHCI)
 
 #endif /* SDHCI_H */
-- 
1.9.3

[Qemu-devel] [PATCH 2/4] sdhci: Add "sysbus" to sdhci QOM types and methods

[Kevin O'Connor]

Update the sdhci sysbus QOM types and methods so that sysbus is in
their name.  This is in preparation for adding PCI versions of these
types and methods.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
 hw/sd/sdhci.c | 39 ++++++++++++++++++++++++---------------
 hw/sd/sdhci.h |  6 +++---
 2 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 4cbd354..05b0c50 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1142,9 +1142,8 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
     }
 }
 
-static void sdhci_initfn(Object *obj)
+static void sdhci_initfn(SDHCIState *s)
 {
-    SDHCIState *s = SDHCI(obj);
     DriveInfo *di;
 
     di = drive_get_next(IF_SD);
@@ -1160,10 +1159,8 @@ static void sdhci_initfn(Object *obj)
     s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s);
 }
 
-static void sdhci_uninitfn(Object *obj)
+static void sdhci_uninitfn(SDHCIState *s)
 {
-    SDHCIState *s = SDHCI(obj);
-
     timer_del(s->insert_timer);
     timer_free(s->insert_timer);
     timer_del(s->transfer_timer);
@@ -1223,9 +1220,21 @@ static Property sdhci_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void sdhci_realize(DeviceState *dev, Error ** errp)
+static void sdhci_sysbus_init(Object *obj)
+{
+    SDHCIState *s = SYSBUS_SDHCI(obj);
+    sdhci_initfn(s);
+}
+
+static void sdhci_sysbus_finalize(Object *obj)
+{
+    SDHCIState *s = SYSBUS_SDHCI(obj);
+    sdhci_uninitfn(s);
+}
+
+static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp)
 {
-    SDHCIState *s = SDHCI(dev);
+    SDHCIState *s = SYSBUS_SDHCI(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
     s->buf_maxsz = sdhci_get_fifolen(s);
@@ -1236,27 +1245,27 @@ static void sdhci_realize(DeviceState *dev, Error ** errp)
     sysbus_init_mmio(sbd, &s->iomem);
 }
 
-static void sdhci_class_init(ObjectClass *klass, void *data)
+static void sdhci_sysbus_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &sdhci_vmstate;
     dc->props = sdhci_properties;
-    dc->realize = sdhci_realize;
+    dc->realize = sdhci_sysbus_realize;
 }
 
-static const TypeInfo sdhci_type_info = {
-    .name = TYPE_SDHCI,
+static const TypeInfo sdhci_sysbus_info = {
+    .name = TYPE_SYSBUS_SDHCI,
     .parent = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SDHCIState),
-    .instance_init = sdhci_initfn,
-    .instance_finalize = sdhci_uninitfn,
-    .class_init = sdhci_class_init,
+    .instance_init = sdhci_sysbus_init,
+    .instance_finalize = sdhci_sysbus_finalize,
+    .class_init = sdhci_sysbus_class_init,
 };
 
 static void sdhci_register_types(void)
 {
-    type_register_static(&sdhci_type_info);
+    type_register_static(&sdhci_sysbus_info);
 }
 
 type_init(sdhci_register_types)
diff --git a/hw/sd/sdhci.h b/hw/sd/sdhci.h
index 9a334ac..9fbf682 100644
--- a/hw/sd/sdhci.h
+++ b/hw/sd/sdhci.h
@@ -281,8 +281,8 @@ typedef struct SDHCIState {
 
 extern const VMStateDescription sdhci_vmstate;
 
-#define TYPE_SDHCI            "generic-sdhci"
-#define SDHCI(obj)            \
-     OBJECT_CHECK(SDHCIState, (obj), TYPE_SDHCI)
+#define TYPE_SYSBUS_SDHCI "generic-sdhci"
+#define SYSBUS_SDHCI(obj)                               \
+     OBJECT_CHECK(SDHCIState, (obj), TYPE_SYSBUS_SDHCI)
 
 #endif /* SDHCI_H */
-- 
1.9.3

[Qemu-devel] [PATCH 3/4] sdhci: Support SDHCI devices on PCI

[Kevin O'Connor]

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
 
 /* 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
-- 
1.9.3

[Qemu-devel] [PATCH 4/4] sdhci: Set a default frequency clock

[Kevin O'Connor]

The Linux SDHCI PCI driver will only register the device if there is a
clock frequency set.  So, set a default frequency of 52Mhz.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
 hw/sd/sdhci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 55709da..a16b734 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -74,10 +74,10 @@
 #define SDHC_CAPAB_MAXBLOCKLENGTH 512ul
 /* Maximum clock frequency for SDclock in MHz
  * value in range 10-63 MHz, 0 - not defined */
-#define SDHC_CAPAB_BASECLKFREQ    0ul
+#define SDHC_CAPAB_BASECLKFREQ    52ul
 #define SDHC_CAPAB_TOUNIT         1ul  /* Timeout clock unit 0 - kHz, 1 - MHz */
 /* Timeout clock frequency 1-63, 0 - not defined */
-#define SDHC_CAPAB_TOCLKFREQ      0ul
+#define SDHC_CAPAB_TOCLKFREQ      52ul
 
 /* Now check all parameters and calculate CAPABILITIES REGISTER value */
 #if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_18V > 1 || SDHC_CAPAB_30V > 1 ||     \
-- 
1.9.3

Re: [Qemu-devel] [PATCH 3/4] sdhci: Support SDHCI devices on PCI

[Paolo Bonzini]

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
> 

Re: [Qemu-devel] [PATCH 3/4] sdhci: Support SDHCI devices on PCI

[Kevin O'Connor]

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

Re: [Qemu-devel] [PATCH 3/4] sdhci: Support SDHCI devices on PCI

[Gerd Hoffmann]

  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

Re: [Qemu-devel] [PATCH 3/4] sdhci: Support SDHCI devices on PCI

[Paolo Bonzini]

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