Convert the OMAP MMC controller to the new SDBus API:
* the controller creates an SDBus bus
* instead of sd_foo functions on the SDState object, call
sdbus_foo functions on the SDBus
* the board code creates a proper TYPE_SD_CARD object and attaches
it to the controller's SDBus, instead of the controller creating
a card directly via sd_init() that never gets attached to any bus
* because the SD card object is on a bus, it gets reset automatically
by the "traverse the qbus tree resetting things" code, and we don't
need to manually reset the card from the controller reset function
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/arm/omap.h | 1 -
hw/arm/omap1.c | 10 +++++++++-
hw/sd/omap_mmc.c | 30 ++++++++++--------------------
3 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/include/hw/arm/omap.h b/include/hw/arm/omap.h
index 7d1a1afc4f8..d20c55a8957 100644
--- a/include/hw/arm/omap.h
+++ b/include/hw/arm/omap.h
@@ -534,7 +534,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(OMAPMMCState, OMAP_MMC)
DeviceState *omap_mmc_init(hwaddr base,
MemoryRegion *sysmem,
- BlockBackend *blk,
qemu_irq irq, qemu_irq dma[], omap_clk clk);
/* omap_i2c.c */
diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index ea07b9aa31e..15ba0a0d0c4 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -29,6 +29,7 @@
#include "hw/qdev-properties.h"
#include "hw/arm/boot.h"
#include "hw/arm/omap.h"
+#include "hw/sd/sd.h"
#include "system/blockdev.h"
#include "system/system.h"
#include "hw/arm/soc_dma.h"
@@ -3981,11 +3982,18 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *dram,
warn_report("missing SecureDigital device");
}
s->mmc = omap_mmc_init(0xfffb7800, system_memory,
- dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
qdev_get_gpio_in(s->ih[1], OMAP_INT_OQN),
&s->drq[OMAP_DMA_MMC_TX],
omap_findclk(s, "mmc_ck"));
+ if (dinfo) {
+ DeviceState *card = qdev_new(TYPE_SD_CARD);
+ qdev_prop_set_drive_err(card, "drive", blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
+ qdev_realize_and_unref(card, qdev_get_child_bus(s->mmc, "sd-bus"),
+ &error_fatal);
+ }
+
s->mpuio = omap_mpuio_init(system_memory, 0xfffb5000,
qdev_get_gpio_in(s->ih[1], OMAP_INT_KEYBOARD),
qdev_get_gpio_in(s->ih[1], OMAP_INT_MPUIO),
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index d497e31a6c5..46fdeac1121 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -30,13 +30,14 @@
typedef struct OMAPMMCState {
SysBusDevice parent_obj;
+ SDBus sdbus;
+
qemu_irq irq;
qemu_irq dma_tx_gpio;
qemu_irq dma_rx_gpio;
qemu_irq coverswitch;
MemoryRegion iomem;
omap_clk clk;
- SDState *card;
uint16_t last_cmd;
uint16_t sdio;
uint16_t rsp[8];
@@ -158,7 +159,7 @@ static void omap_mmc_command(OMAPMMCState *host, int cmd, int dir,
request.arg = host->arg;
request.crc = 0; /* FIXME */
- rsplen = sd_do_command(host->card, &request, response);
+ rsplen = sdbus_do_command(&host->sdbus, &request, response);
/* TODO: validate CRCs */
switch (resptype) {
@@ -247,10 +248,10 @@ static void omap_mmc_transfer(OMAPMMCState *host)
if (host->fifo_len > host->af_level)
break;
- value = sd_read_byte(host->card);
+ value = sdbus_read_byte(&host->sdbus);
host->fifo[(host->fifo_start + host->fifo_len) & 31] = value;
if (-- host->blen_counter) {
- value = sd_read_byte(host->card);
+ value = sdbus_read_byte(&host->sdbus);
host->fifo[(host->fifo_start + host->fifo_len) & 31] |=
value << 8;
host->blen_counter --;
@@ -262,10 +263,10 @@ static void omap_mmc_transfer(OMAPMMCState *host)
break;
value = host->fifo[host->fifo_start] & 0xff;
- sd_write_byte(host->card, value);
+ sdbus_write_byte(&host->sdbus, value);
if (-- host->blen_counter) {
value = host->fifo[host->fifo_start] >> 8;
- sd_write_byte(host->card, value);
+ sdbus_write_byte(&host->sdbus, value);
host->blen_counter --;
}
@@ -328,14 +329,6 @@ static void omap_mmc_reset(OMAPMMCState *host)
host->clkdiv = 0;
omap_mmc_pseudo_reset(host);
-
- /* Since we're still using the legacy SD API the card is not plugged
- * into any bus, and we must reset it manually. When omap_mmc is
- * QOMified this must move into the QOM reset function.
- */
- if (host->card) {
- device_cold_reset(DEVICE(host->card));
- }
}
static uint64_t omap_mmc_read(void *opaque, hwaddr offset, unsigned size)
@@ -592,7 +585,6 @@ static const MemoryRegionOps omap_mmc_ops = {
DeviceState *omap_mmc_init(hwaddr base,
MemoryRegion *sysmem,
- BlockBackend *blk,
qemu_irq irq, qemu_irq dma[], omap_clk clk)
{
DeviceState *dev;
@@ -610,11 +602,6 @@ DeviceState *omap_mmc_init(hwaddr base,
qdev_connect_gpio_out_named(dev, "dma-rx", 0, dma[1]);
sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
- /* Instantiate the storage */
- s->card = sd_init(blk, false);
- if (s->card == NULL) {
- exit(1);
- }
return dev;
}
@@ -643,6 +630,9 @@ static void omap_mmc_initfn(Object *obj)
static void omap_mmc_realize(DeviceState *dev, Error **errp)
{
+ OMAPMMCState *s = OMAP_MMC(dev);
+
+ qbus_init(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS, dev, "sd-bus");
}
static void omap_mmc_class_init(ObjectClass *oc, void *data)
--
2.34.1On 28/1/25 11:45, Peter Maydell wrote:
> Convert the OMAP MMC controller to the new SDBus API:
> * the controller creates an SDBus bus
> * instead of sd_foo functions on the SDState object, call
> sdbus_foo functions on the SDBus
> * the board code creates a proper TYPE_SD_CARD object and attaches
> it to the controller's SDBus, instead of the controller creating
> a card directly via sd_init() that never gets attached to any bus
> * because the SD card object is on a bus, it gets reset automatically
> by the "traverse the qbus tree resetting things" code, and we don't
> need to manually reset the card from the controller reset function
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> include/hw/arm/omap.h | 1 -
> hw/arm/omap1.c | 10 +++++++++-
> hw/sd/omap_mmc.c | 30 ++++++++++--------------------
> 3 files changed, 19 insertions(+), 22 deletions(-)
> @@ -643,6 +630,9 @@ static void omap_mmc_initfn(Object *obj)
>
> static void omap_mmc_realize(DeviceState *dev, Error **errp)
> {
> + OMAPMMCState *s = OMAP_MMC(dev);
> +
> + qbus_init(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS, dev, "sd-bus");
Why you don't create the bus in omap_mmc_initfn()?
Anyhow,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> }
On Thu, 30 Jan 2025 at 22:26, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 28/1/25 11:45, Peter Maydell wrote:
> > Convert the OMAP MMC controller to the new SDBus API:
> > * the controller creates an SDBus bus
> > * instead of sd_foo functions on the SDState object, call
> > sdbus_foo functions on the SDBus
> > * the board code creates a proper TYPE_SD_CARD object and attaches
> > it to the controller's SDBus, instead of the controller creating
> > a card directly via sd_init() that never gets attached to any bus
> > * because the SD card object is on a bus, it gets reset automatically
> > by the "traverse the qbus tree resetting things" code, and we don't
> > need to manually reset the card from the controller reset function
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > include/hw/arm/omap.h | 1 -
> > hw/arm/omap1.c | 10 +++++++++-
> > hw/sd/omap_mmc.c | 30 ++++++++++--------------------
> > 3 files changed, 19 insertions(+), 22 deletions(-)
>
>
> > @@ -643,6 +630,9 @@ static void omap_mmc_initfn(Object *obj)
> >
> > static void omap_mmc_realize(DeviceState *dev, Error **errp)
> > {
> > + OMAPMMCState *s = OMAP_MMC(dev);
> > +
> > + qbus_init(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS, dev, "sd-bus");
>
> Why you don't create the bus in omap_mmc_initfn()?
Good question. I think I was using hw/sd/ssi-sd.c as a guide
(since it's the only other device that directly creates a
TYPE_SD_BUS rather than needing to subclass the bus), and
that put it in realize. But it's probably better in init.
> Anyhow,
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
thanks
-- PMM
© 2016 - 2025 Red Hat, Inc.