From d7ebca748e33176ede1a8675cea82271d2c0dac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 5 Jul 2020 23:22:10 +0200 Subject: [PATCH 01/23] hw/sd/pxa2xx_mmci: Do not create SD card within the SD host controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SD/MMC host controllers provide a SD Bus to plug SD cards, but don't come with SD card plugged in :) The machine/board object is where the SD cards are created. Since the PXA2xx is not qdevified, for now create the cards in pxa270_init() which is the SoC model. In the future we will move this to the board model. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Reviewed-by: Peter Maydell Acked-by: Peter Maydell Message-Id: <20200705213350.24725-2-f4bug@amsat.org> --- hw/arm/pxa2xx.c | 39 +++++++++++++++++++++++++++++---------- hw/sd/pxa2xx_mmci.c | 11 ++--------- include/hw/arm/pxa.h | 3 +-- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c index 6203c4cfe0..20fa201dd5 100644 --- a/hw/arm/pxa2xx.c +++ b/hw/arm/pxa2xx.c @@ -22,6 +22,7 @@ #include "hw/irq.h" #include "hw/qdev-properties.h" #include "hw/ssi/ssi.h" +#include "hw/sd/sd.h" #include "chardev/char-fe.h" #include "sysemu/blockdev.h" #include "sysemu/qtest.h" @@ -2136,15 +2137,24 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space, s->gpio = pxa2xx_gpio_init(0x40e00000, s->cpu, s->pic, 121); - dinfo = drive_get(IF_SD, 0, 0); - if (!dinfo && !qtest_enabled()) { - warn_report("missing SecureDigital device"); - } s->mmc = pxa2xx_mmci_init(address_space, 0x41100000, - dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC), qdev_get_gpio_in(s->dma, PXA2XX_RX_RQ_MMCI), qdev_get_gpio_in(s->dma, PXA2XX_TX_RQ_MMCI)); + dinfo = drive_get(IF_SD, 0, 0); + if (dinfo) { + DeviceState *carddev; + + /* Create and plug in the sd card */ + carddev = qdev_new(TYPE_SD_CARD); + qdev_prop_set_drive_err(carddev, "drive", + blk_by_legacy_dinfo(dinfo), &error_fatal); + qdev_realize_and_unref(carddev, qdev_get_child_bus(DEVICE(s->mmc), + "sd-bus"), + &error_fatal); + } else if (!qtest_enabled()) { + warn_report("missing SecureDigital device"); + } for (i = 0; pxa270_serial[i].io_base; i++) { if (serial_hd(i)) { @@ -2260,15 +2270,24 @@ PXA2xxState *pxa255_init(MemoryRegion *address_space, unsigned int sdram_size) s->gpio = pxa2xx_gpio_init(0x40e00000, s->cpu, s->pic, 85); - dinfo = drive_get(IF_SD, 0, 0); - if (!dinfo && !qtest_enabled()) { - warn_report("missing SecureDigital device"); - } s->mmc = pxa2xx_mmci_init(address_space, 0x41100000, - dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC), qdev_get_gpio_in(s->dma, PXA2XX_RX_RQ_MMCI), qdev_get_gpio_in(s->dma, PXA2XX_TX_RQ_MMCI)); + dinfo = drive_get(IF_SD, 0, 0); + if (dinfo) { + DeviceState *carddev; + + /* Create and plug in the sd card */ + carddev = qdev_new(TYPE_SD_CARD); + qdev_prop_set_drive_err(carddev, "drive", + blk_by_legacy_dinfo(dinfo), &error_fatal); + qdev_realize_and_unref(carddev, qdev_get_child_bus(DEVICE(s->mmc), + "sd-bus"), + &error_fatal); + } else if (!qtest_enabled()) { + warn_report("missing SecureDigital device"); + } for (i = 0; pxa255_serial[i].io_base; i++) { if (serial_hd(i)) { diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c index 68bed24480..9482b9212d 100644 --- a/hw/sd/pxa2xx_mmci.c +++ b/hw/sd/pxa2xx_mmci.c @@ -476,10 +476,9 @@ static const MemoryRegionOps pxa2xx_mmci_ops = { PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem, hwaddr base, - BlockBackend *blk, qemu_irq irq, - qemu_irq rx_dma, qemu_irq tx_dma) + qemu_irq irq, qemu_irq rx_dma, qemu_irq tx_dma) { - DeviceState *dev, *carddev; + DeviceState *dev; SysBusDevice *sbd; PXA2xxMMCIState *s; @@ -492,12 +491,6 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem, qdev_connect_gpio_out_named(dev, "tx-dma", 0, tx_dma); sysbus_realize_and_unref(sbd, &error_fatal); - /* Create and plug in the sd card */ - carddev = qdev_new(TYPE_SD_CARD); - qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal); - qdev_realize_and_unref(carddev, qdev_get_child_bus(dev, "sd-bus"), - &error_fatal); - return s; } diff --git a/include/hw/arm/pxa.h b/include/hw/arm/pxa.h index 8843e5f910..d99b6192da 100644 --- a/include/hw/arm/pxa.h +++ b/include/hw/arm/pxa.h @@ -89,8 +89,7 @@ void pxa2xx_lcd_vsync_notifier(PXA2xxLCDState *s, qemu_irq handler); typedef struct PXA2xxMMCIState PXA2xxMMCIState; PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem, hwaddr base, - BlockBackend *blk, qemu_irq irq, - qemu_irq rx_dma, qemu_irq tx_dma); + qemu_irq irq, qemu_irq rx_dma, qemu_irq tx_dma); void pxa2xx_mmci_handlers(PXA2xxMMCIState *s, qemu_irq readonly, qemu_irq coverswitch); From a0e63983a646338c06a146d0d9d52f9a06a88ec4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 5 Jul 2020 23:22:57 +0200 Subject: [PATCH 02/23] hw/sd/pxa2xx_mmci: Trivial simplification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid declaring PXA2xxMMCIState local variable, return it directly. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Reviewed-by: Laurent Vivier Acked-by: Peter Maydell Message-Id: <20200705213350.24725-3-f4bug@amsat.org> --- hw/sd/pxa2xx_mmci.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c index 9482b9212d..2996a2ef17 100644 --- a/hw/sd/pxa2xx_mmci.c +++ b/hw/sd/pxa2xx_mmci.c @@ -480,10 +480,8 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem, { DeviceState *dev; SysBusDevice *sbd; - PXA2xxMMCIState *s; dev = qdev_new(TYPE_PXA2XX_MMCI); - s = PXA2XX_MMCI(dev); sbd = SYS_BUS_DEVICE(dev); sysbus_mmio_map(sbd, 0, base); sysbus_connect_irq(sbd, 0, irq); @@ -491,7 +489,7 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem, qdev_connect_gpio_out_named(dev, "tx-dma", 0, tx_dma); sysbus_realize_and_unref(sbd, &error_fatal); - return s; + return PXA2XX_MMCI(dev); } static void pxa2xx_mmci_set_inserted(DeviceState *dev, bool inserted) From 3dce5842416bd0460d3e6e19f29fb8db44b7e668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 5 Jul 2020 22:50:52 +0200 Subject: [PATCH 03/23] hw/lm32/milkymist: Un-inline milkymist_memcard_create() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As we will modify milkymist_memcard_create(), move it first to the source file where it is used. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-Id: <20200705211016.15241-2-f4bug@amsat.org> --- hw/lm32/milkymist-hw.h | 11 ----------- hw/lm32/milkymist.c | 11 +++++++++++ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/hw/lm32/milkymist-hw.h b/hw/lm32/milkymist-hw.h index 05e2c2a5a7..5dca5d52f5 100644 --- a/hw/lm32/milkymist-hw.h +++ b/hw/lm32/milkymist-hw.h @@ -31,17 +31,6 @@ static inline DeviceState *milkymist_hpdmc_create(hwaddr base) return dev; } -static inline DeviceState *milkymist_memcard_create(hwaddr base) -{ - DeviceState *dev; - - dev = qdev_new("milkymist-memcard"); - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); - sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); - - return dev; -} - static inline DeviceState *milkymist_vgafb_create(hwaddr base, uint32_t fb_offset, uint32_t fb_mask) { diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c index 85913bb68b..469e3c4322 100644 --- a/hw/lm32/milkymist.c +++ b/hw/lm32/milkymist.c @@ -80,6 +80,17 @@ static void main_cpu_reset(void *opaque) env->deba = reset_info->flash_base; } +static DeviceState *milkymist_memcard_create(hwaddr base) +{ + DeviceState *dev; + + dev = qdev_new("milkymist-memcard"); + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); + + return dev; +} + static void milkymist_init(MachineState *machine) { From ae7ba8e04a4faccfb515c9e6a72892307c4bab1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 5 Jul 2020 22:56:07 +0200 Subject: [PATCH 04/23] hw/sd/milkymist: Create the SDBus at init() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't need to wait until realize() to create the SDBus, create it in init() directly. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-Id: <20200705211016.15241-4-f4bug@amsat.org> --- hw/sd/milkymist-memcard.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c index 11f61294fc..747c5c6136 100644 --- a/hw/sd/milkymist-memcard.c +++ b/hw/sd/milkymist-memcard.c @@ -261,6 +261,9 @@ static void milkymist_memcard_init(Object *obj) memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s, "milkymist-memcard", R_MAX * 4); sysbus_init_mmio(dev, &s->regs_region); + + qbus_create_inplace(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS, + DEVICE(obj), "sd-bus"); } static void milkymist_memcard_realize(DeviceState *dev, Error **errp) @@ -271,9 +274,6 @@ static void milkymist_memcard_realize(DeviceState *dev, Error **errp) DriveInfo *dinfo; Error *err = NULL; - qbus_create_inplace(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS, - dev, "sd-bus"); - /* Create and plug in the sd card */ /* FIXME use a qdev drive property instead of drive_get_next() */ dinfo = drive_get_next(IF_SD); From a8c73ca21a6f86daa53c2dbeeb8abf06dd1aeec9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 5 Jul 2020 23:03:34 +0200 Subject: [PATCH 05/23] hw/sd/milkymist: Do not create SD card within the SD host controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SD/MMC host controllers provide a SD Bus to plug SD cards, but don't come with SD card plugged in :) Let the machine/board model create and plug the SD cards when required. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-Id: <20200705211016.15241-5-f4bug@amsat.org> --- hw/lm32/milkymist.c | 13 +++++++++ hw/sd/milkymist-memcard.c | 55 +++++++++++++++++++++++---------------- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c index 469e3c4322..9f8fe9fef1 100644 --- a/hw/lm32/milkymist.c +++ b/hw/lm32/milkymist.c @@ -34,6 +34,7 @@ #include "elf.h" #include "milkymist-hw.h" #include "hw/display/milkymist_tmu2.h" +#include "hw/sd/sd.h" #include "lm32.h" #include "exec/address-spaces.h" #include "qemu/cutils.h" @@ -83,11 +84,23 @@ static void main_cpu_reset(void *opaque) static DeviceState *milkymist_memcard_create(hwaddr base) { DeviceState *dev; + DriveInfo *dinfo; dev = qdev_new("milkymist-memcard"); sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); + dinfo = drive_get_next(IF_SD); + if (dinfo) { + DeviceState *card; + + 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(dev, "sd-bus"), + &error_fatal); + } + return dev; } diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c index 747c5c6136..e9f5db5e22 100644 --- a/hw/sd/milkymist-memcard.c +++ b/hw/sd/milkymist-memcard.c @@ -66,6 +66,8 @@ enum { #define MILKYMIST_MEMCARD(obj) \ OBJECT_CHECK(MilkymistMemcardState, (obj), TYPE_MILKYMIST_MEMCARD) +#define TYPE_MILKYMIST_SDBUS "milkymist-sdbus" + struct MilkymistMemcardState { SysBusDevice parent_obj; @@ -253,6 +255,19 @@ static void milkymist_memcard_reset(DeviceState *d) } } +static void milkymist_memcard_set_readonly(DeviceState *dev, bool level) +{ + qemu_log_mask(LOG_UNIMP, + "milkymist_memcard: read-only mode not supported\n"); +} + +static void milkymist_memcard_set_inserted(DeviceState *dev, bool level) +{ + MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); + + s->enabled = !!level; +} + static void milkymist_memcard_init(Object *obj) { MilkymistMemcardState *s = MILKYMIST_MEMCARD(obj); @@ -266,27 +281,6 @@ static void milkymist_memcard_init(Object *obj) DEVICE(obj), "sd-bus"); } -static void milkymist_memcard_realize(DeviceState *dev, Error **errp) -{ - MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); - DeviceState *carddev; - BlockBackend *blk; - DriveInfo *dinfo; - Error *err = NULL; - - /* Create and plug in the sd card */ - /* FIXME use a qdev drive property instead of drive_get_next() */ - dinfo = drive_get_next(IF_SD); - blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; - carddev = qdev_new(TYPE_SD_CARD); - qdev_prop_set_drive(carddev, "drive", blk); - if (!qdev_realize_and_unref(carddev, BUS(&s->sdbus), &err)) { - error_propagate_prepend(errp, err, "failed to init SD card"); - return; - } - s->enabled = blk && blk_is_inserted(blk); -} - static const VMStateDescription vmstate_milkymist_memcard = { .name = "milkymist-memcard", .version_id = 1, @@ -308,10 +302,9 @@ static void milkymist_memcard_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - dc->realize = milkymist_memcard_realize; dc->reset = milkymist_memcard_reset; dc->vmsd = &vmstate_milkymist_memcard; - /* Reason: init() method uses drive_get_next() */ + /* Reason: output IRQs should be wired up */ dc->user_creatable = false; } @@ -323,9 +316,25 @@ static const TypeInfo milkymist_memcard_info = { .class_init = milkymist_memcard_class_init, }; +static void milkymist_sdbus_class_init(ObjectClass *klass, void *data) +{ + SDBusClass *sbc = SD_BUS_CLASS(klass); + + sbc->set_inserted = milkymist_memcard_set_inserted; + sbc->set_readonly = milkymist_memcard_set_readonly; +} + +static const TypeInfo milkymist_sdbus_info = { + .name = TYPE_MILKYMIST_SDBUS, + .parent = TYPE_SD_BUS, + .instance_size = sizeof(SDBus), + .class_init = milkymist_sdbus_class_init, +}; + static void milkymist_memcard_register_types(void) { type_register_static(&milkymist_memcard_info); + type_register_static(&milkymist_sdbus_info); } type_init(milkymist_memcard_register_types) From 4858e256bd475cdb68f61daa1083a562ecf9faec Mon Sep 17 00:00:00 2001 From: Alistair Francis Date: Wed, 20 Dec 2017 09:24:47 -0800 Subject: [PATCH 06/23] hw/sd/pl181: Replace fprintf(stderr, "*\n") with error_report() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace a large number of the fprintf(stderr, "*\n" calls with error_report(). The functions were renamed with these commands and then compiler issues where manually fixed. find ./* -type f -exec sed -i \ 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + find ./* -type f -exec sed -i \ 'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + find ./* -type f -exec sed -i \ 'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + find ./* -type f -exec sed -i \ 'N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + find ./* -type f -exec sed -i \ 'N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + find ./* -type f -exec sed -i \ 'N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + find ./* -type f -exec sed -i \ 'N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + find ./* -type f -exec sed -i \ 'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + find ./* -type f -exec sed -i \ 'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + find ./* -type f -exec sed -i \ 'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + find ./* -type f -exec sed -i \ 'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \ {} + Some lines where then manually tweaked to pass checkpatch. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Alistair Francis Message-Id: <488ba8d4c562ea44119de8ea0f385a898bd8fa1e.1513790495.git.alistair.francis@xilinx.com> Signed-off-by: Philippe Mathieu-Daudé Acked-by: Peter Maydell --- hw/sd/pl181.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index 2b3776a6a0..649386ec3d 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -15,6 +15,7 @@ #include "hw/sd/sd.h" #include "qemu/log.h" #include "qemu/module.h" +#include "qemu/error-report.h" #include "qapi/error.h" //#define DEBUG_PL181 1 @@ -148,7 +149,7 @@ static void pl181_fifo_push(PL181State *s, uint32_t value) int n; if (s->fifo_len == PL181_FIFO_LEN) { - fprintf(stderr, "pl181: FIFO overflow\n"); + error_report("%s: FIFO overflow", __func__); return; } n = (s->fifo_pos + s->fifo_len) & (PL181_FIFO_LEN - 1); @@ -162,7 +163,7 @@ static uint32_t pl181_fifo_pop(PL181State *s) uint32_t value; if (s->fifo_len == 0) { - fprintf(stderr, "pl181: FIFO underflow\n"); + error_report("%s: FIFO underflow", __func__); return 0; } value = s->fifo[s->fifo_pos]; From b67cd8f55bff74ed004b86072dc0a4dc2ac07fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 5 Jul 2020 19:42:05 +0200 Subject: [PATCH 07/23] hw/sd/pl181: Rename pl181_send_command() as pl181_do_command() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pl181_send_command() do a bus transaction (send or receive), rename it as pl181_do_command(). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Acked-by: Peter Maydell Message-Id: <20200705204630.4133-3-f4bug@amsat.org> --- hw/sd/pl181.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index 649386ec3d..3fc2cdd71a 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -173,7 +173,7 @@ static uint32_t pl181_fifo_pop(PL181State *s) return value; } -static void pl181_send_command(PL181State *s) +static void pl181_do_command(PL181State *s) { SDRequest request; uint8_t response[16]; @@ -402,7 +402,7 @@ static void pl181_write(void *opaque, hwaddr offset, qemu_log_mask(LOG_UNIMP, "pl181: Pending commands not implemented\n"); } else { - pl181_send_command(s); + pl181_do_command(s); pl181_fifo_run(s); } /* The command has completed one way or the other. */ From 0e33730c8924032eda53de6ebb7ecba232abb01d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 5 Jul 2020 19:54:31 +0200 Subject: [PATCH 08/23] hw/sd/pl181: Add TODO to use Fifo32 API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add TODO to use Fifo32 API from "qemu/fifo32.h". Signed-off-by: Philippe Mathieu-Daudé Acked-by: Peter Maydell Message-Id: <20200705204630.4133-4-f4bug@amsat.org> --- hw/sd/pl181.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index 3fc2cdd71a..86219c851d 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -57,7 +57,7 @@ typedef struct PL181State { http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4446/1 */ int32_t linux_hack; - uint32_t fifo[PL181_FIFO_LEN]; + uint32_t fifo[PL181_FIFO_LEN]; /* TODO use Fifo32 */ qemu_irq irq[2]; /* GPIO outputs for 'card is readonly' and 'card inserted' */ qemu_irq cardstatus[2]; From 26c5b0f4cbe19ad87d3aaba22cd1a8edc25f7ceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 5 Jul 2020 13:39:53 +0200 Subject: [PATCH 09/23] hw/sd/pl181: Use named GPIOs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To make the code easier to manage/review/use, rename the cardstatus[0] variable as 'card_readonly' and name the GPIO "card-read-only". Similarly with cardstatus[1], renamed as 'card_inserted' and name its GPIO "card-inserted". Adapt the users accordingly by using the qdev_init_gpio_out_named() function. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Acked-by: Peter Maydell Message-Id: <20200705204630.4133-6-f4bug@amsat.org> --- hw/arm/integratorcp.c | 4 ++-- hw/arm/realview.c | 4 ++-- hw/arm/vexpress.c | 4 ++-- hw/sd/pl181.c | 8 +++++--- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c index f304c2b4f0..16c4d750a4 100644 --- a/hw/arm/integratorcp.c +++ b/hw/arm/integratorcp.c @@ -645,9 +645,9 @@ static void integratorcp_init(MachineState *machine) sysbus_create_simple(TYPE_INTEGRATOR_DEBUG, 0x1a000000, 0); dev = sysbus_create_varargs("pl181", 0x1c000000, pic[23], pic[24], NULL); - qdev_connect_gpio_out(dev, 0, + qdev_connect_gpio_out_named(dev, "card-read-only", 0, qdev_get_gpio_in_named(icp, ICP_GPIO_MMC_WPROT, 0)); - qdev_connect_gpio_out(dev, 1, + qdev_connect_gpio_out_named(dev, "card-inserted", 0, qdev_get_gpio_in_named(icp, ICP_GPIO_MMC_CARDIN, 0)); sysbus_create_varargs("pl041", 0x1d000000, pic[25], NULL); diff --git a/hw/arm/realview.c b/hw/arm/realview.c index c1ff172b13..3e2360c261 100644 --- a/hw/arm/realview.c +++ b/hw/arm/realview.c @@ -234,8 +234,8 @@ static void realview_init(MachineState *machine, mmc_irq[1] = qemu_irq_split( qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_CARDIN), qemu_irq_invert(qdev_get_gpio_in(gpio2, 0))); - qdev_connect_gpio_out(dev, 0, mmc_irq[0]); - qdev_connect_gpio_out(dev, 1, mmc_irq[1]); + qdev_connect_gpio_out_named(dev, "card-read-only", 0, mmc_irq[0]); + qdev_connect_gpio_out_named(dev, "card-inserted", 0, mmc_irq[1]); sysbus_create_simple("pl031", 0x10017000, pic[10]); diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c index 1dc971c34f..049a0ec2c7 100644 --- a/hw/arm/vexpress.c +++ b/hw/arm/vexpress.c @@ -624,9 +624,9 @@ static void vexpress_common_init(MachineState *machine) dev = sysbus_create_varargs("pl181", map[VE_MMCI], pic[9], pic[10], NULL); /* Wire up MMC card detect and read-only signals */ - qdev_connect_gpio_out(dev, 0, + qdev_connect_gpio_out_named(dev, "card-read-only", 0, qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_WPROT)); - qdev_connect_gpio_out(dev, 1, + qdev_connect_gpio_out_named(dev, "card-inserted", 0, qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_CARDIN)); sysbus_create_simple("pl050_keyboard", map[VE_KMI0], pic[12]); diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index 86219c851d..ab4cd733a4 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -60,7 +60,8 @@ typedef struct PL181State { uint32_t fifo[PL181_FIFO_LEN]; /* TODO use Fifo32 */ qemu_irq irq[2]; /* GPIO outputs for 'card is readonly' and 'card inserted' */ - qemu_irq cardstatus[2]; + qemu_irq card_readonly; + qemu_irq card_inserted; } PL181State; static const VMStateDescription vmstate_pl181 = { @@ -479,7 +480,7 @@ static void pl181_reset(DeviceState *d) s->mask[1] = 0; /* We can assume our GPIO outputs have been wired up now */ - sd_set_cb(s->card, s->cardstatus[0], s->cardstatus[1]); + sd_set_cb(s->card, s->card_readonly, s->card_inserted); /* Since we're still using the legacy SD API the card is not plugged * into any bus, and we must reset it manually. */ @@ -496,7 +497,8 @@ static void pl181_init(Object *obj) sysbus_init_mmio(sbd, &s->iomem); sysbus_init_irq(sbd, &s->irq[0]); sysbus_init_irq(sbd, &s->irq[1]); - qdev_init_gpio_out(dev, s->cardstatus, 2); + qdev_init_gpio_out_named(dev, &s->card_readonly, "card-read-only", 1); + qdev_init_gpio_out_named(dev, &s->card_inserted, "card-inserted", 1); } static void pl181_realize(DeviceState *dev, Error **errp) From 2762eed1f5534074fcce703bdda8702905dc4c61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 23 Jan 2018 04:58:35 +0100 Subject: [PATCH 10/23] hw/sd/pl181: Expose a SDBus and connect the SDCard to it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the controller to the SDBus API: - add the a TYPE_PL181_BUS object of type TYPE_SD_BUS, - adapt the SDBusClass set_inserted/set_readonly handlers - create the bus in the PL181 controller - switch legacy sd_*() API to the sdbus_*() API. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Acked-by: Peter Maydell Message-Id: <20200705204630.4133-7-f4bug@amsat.org> --- hw/sd/pl181.c | 67 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index ab4cd733a4..f6de06ece8 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -17,6 +17,7 @@ #include "qemu/module.h" #include "qemu/error-report.h" #include "qapi/error.h" +#include "hw/qdev-properties.h" //#define DEBUG_PL181 1 @@ -32,11 +33,13 @@ do { printf("pl181: " fmt , ## __VA_ARGS__); } while (0) #define TYPE_PL181 "pl181" #define PL181(obj) OBJECT_CHECK(PL181State, (obj), TYPE_PL181) +#define TYPE_PL181_BUS "pl181-bus" + typedef struct PL181State { SysBusDevice parent_obj; MemoryRegion iomem; - SDState *card; + SDBus sdbus; uint32_t clock; uint32_t power; uint32_t cmdarg; @@ -183,7 +186,7 @@ static void pl181_do_command(PL181State *s) request.cmd = s->cmd & PL181_CMD_INDEX; request.arg = s->cmdarg; DPRINTF("Command %d %08x\n", request.cmd, request.arg); - rlen = sd_do_command(s->card, &request, response); + rlen = sdbus_do_command(&s->sdbus, &request, response); if (rlen < 0) goto error; if (s->cmd & PL181_CMD_RESPONSE) { @@ -224,12 +227,12 @@ static void pl181_fifo_run(PL181State *s) int is_read; is_read = (s->datactrl & PL181_DATA_DIRECTION) != 0; - if (s->datacnt != 0 && (!is_read || sd_data_ready(s->card)) + if (s->datacnt != 0 && (!is_read || sdbus_data_ready(&s->sdbus)) && !s->linux_hack) { if (is_read) { n = 0; while (s->datacnt && s->fifo_len < PL181_FIFO_LEN) { - value |= (uint32_t)sd_read_data(s->card) << (n * 8); + value |= (uint32_t)sdbus_read_data(&s->sdbus) << (n * 8); s->datacnt--; n++; if (n == 4) { @@ -250,7 +253,7 @@ static void pl181_fifo_run(PL181State *s) } n--; s->datacnt--; - sd_write_data(s->card, value & 0xff); + sdbus_write_data(&s->sdbus, value & 0xff); value >>= 8; } } @@ -456,6 +459,20 @@ static const MemoryRegionOps pl181_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; +static void pl181_set_readonly(DeviceState *dev, bool level) +{ + PL181State *s = (PL181State *)dev; + + qemu_set_irq(s->card_readonly, level); +} + +static void pl181_set_inserted(DeviceState *dev, bool level) +{ + PL181State *s = (PL181State *)dev; + + qemu_set_irq(s->card_inserted, level); +} + static void pl181_reset(DeviceState *d) { PL181State *s = PL181(d); @@ -479,12 +496,9 @@ static void pl181_reset(DeviceState *d) s->mask[0] = 0; s->mask[1] = 0; - /* We can assume our GPIO outputs have been wired up now */ - sd_set_cb(s->card, s->card_readonly, s->card_inserted); - /* Since we're still using the legacy SD API the card is not plugged - * into any bus, and we must reset it manually. - */ - device_legacy_reset(DEVICE(s->card)); + /* Reset other state based on current card insertion/readonly status */ + pl181_set_inserted(DEVICE(s), sdbus_get_inserted(&s->sdbus)); + pl181_set_readonly(DEVICE(s), sdbus_get_readonly(&s->sdbus)); } static void pl181_init(Object *obj) @@ -499,19 +513,24 @@ static void pl181_init(Object *obj) sysbus_init_irq(sbd, &s->irq[1]); qdev_init_gpio_out_named(dev, &s->card_readonly, "card-read-only", 1); qdev_init_gpio_out_named(dev, &s->card_inserted, "card-inserted", 1); + + qbus_create_inplace(&s->sdbus, sizeof(s->sdbus), + TYPE_PL181_BUS, dev, "sd-bus"); } static void pl181_realize(DeviceState *dev, Error **errp) { - PL181State *s = PL181(dev); + DeviceState *card; DriveInfo *dinfo; /* FIXME use a qdev drive property instead of drive_get_next() */ + card = qdev_new(TYPE_SD_CARD); dinfo = drive_get_next(IF_SD); - s->card = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, false); - if (s->card == NULL) { - error_setg(errp, "sd_init failed"); - } + qdev_prop_set_drive_err(card, "drive", blk_by_legacy_dinfo(dinfo), + &error_fatal); + qdev_realize_and_unref(card, + qdev_get_child_bus(dev, "sd-bus"), + &error_fatal); } static void pl181_class_init(ObjectClass *klass, void *data) @@ -533,9 +552,25 @@ static const TypeInfo pl181_info = { .class_init = pl181_class_init, }; +static void pl181_bus_class_init(ObjectClass *klass, void *data) +{ + SDBusClass *sbc = SD_BUS_CLASS(klass); + + sbc->set_inserted = pl181_set_inserted; + sbc->set_readonly = pl181_set_readonly; +} + +static const TypeInfo pl181_bus_info = { + .name = TYPE_PL181_BUS, + .parent = TYPE_SD_BUS, + .instance_size = sizeof(SDBus), + .class_init = pl181_bus_class_init, +}; + static void pl181_register_types(void) { type_register_static(&pl181_info); + type_register_static(&pl181_bus_info); } type_init(pl181_register_types) From 26c607b86b7bb90ad75a15bc6172c28aa48c768c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 5 Jul 2020 14:24:24 +0200 Subject: [PATCH 11/23] hw/sd/pl181: Do not create SD card within the SD host controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SD/MMC host controllers provide a SD Bus to plug SD cards, but don't come with SD card plugged in :) Let the machine/board model create and plug the SD cards when required. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Reviewed-by: Peter Maydell Acked-by: Peter Maydell Message-Id: <20200705204630.4133-8-f4bug@amsat.org> --- hw/arm/integratorcp.c | 13 +++++++++++++ hw/arm/realview.c | 12 ++++++++++++ hw/arm/versatilepb.c | 26 ++++++++++++++++++++++++-- hw/arm/vexpress.c | 11 +++++++++++ hw/sd/pl181.c | 19 +------------------ 5 files changed, 61 insertions(+), 20 deletions(-) diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c index 16c4d750a4..fe7c2b9d4b 100644 --- a/hw/arm/integratorcp.c +++ b/hw/arm/integratorcp.c @@ -25,6 +25,7 @@ #include "hw/char/pl011.h" #include "hw/hw.h" #include "hw/irq.h" +#include "hw/sd/sd.h" #define TYPE_INTEGRATOR_CM "integrator_core" #define INTEGRATOR_CM(obj) \ @@ -595,6 +596,7 @@ static void integratorcp_init(MachineState *machine) MemoryRegion *ram_alias = g_new(MemoryRegion, 1); qemu_irq pic[32]; DeviceState *dev, *sic, *icp; + DriveInfo *dinfo; int i; cpuobj = object_new(machine->cpu_type); @@ -649,6 +651,17 @@ static void integratorcp_init(MachineState *machine) qdev_get_gpio_in_named(icp, ICP_GPIO_MMC_WPROT, 0)); qdev_connect_gpio_out_named(dev, "card-inserted", 0, qdev_get_gpio_in_named(icp, ICP_GPIO_MMC_CARDIN, 0)); + dinfo = drive_get_next(IF_SD); + if (dinfo) { + DeviceState *card; + + 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(dev, "sd-bus"), + &error_fatal); + } + sysbus_create_varargs("pl041", 0x1d000000, pic[25], NULL); if (nd_table[0].used) diff --git a/hw/arm/realview.c b/hw/arm/realview.c index 3e2360c261..5f1f36b15c 100644 --- a/hw/arm/realview.c +++ b/hw/arm/realview.c @@ -27,6 +27,7 @@ #include "hw/intc/realview_gic.h" #include "hw/irq.h" #include "hw/i2c/arm_sbcon_i2c.h" +#include "hw/sd/sd.h" #define SMP_BOOT_ADDR 0xe0000000 #define SMP_BOOTREG_ADDR 0x10000030 @@ -69,6 +70,7 @@ static void realview_init(MachineState *machine, qemu_irq mmc_irq[2]; PCIBus *pci_bus = NULL; NICInfo *nd; + DriveInfo *dinfo; I2CBus *i2c; int n; unsigned int smp_cpus = machine->smp.cpus; @@ -236,6 +238,16 @@ static void realview_init(MachineState *machine, qemu_irq_invert(qdev_get_gpio_in(gpio2, 0))); qdev_connect_gpio_out_named(dev, "card-read-only", 0, mmc_irq[0]); qdev_connect_gpio_out_named(dev, "card-inserted", 0, mmc_irq[1]); + dinfo = drive_get_next(IF_SD); + if (dinfo) { + DeviceState *card; + + 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(dev, "sd-bus"), + &error_fatal); + } sysbus_create_simple("pl031", 0x10017000, pic[10]); diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c index 9dc93182b6..9127579984 100644 --- a/hw/arm/versatilepb.c +++ b/hw/arm/versatilepb.c @@ -25,6 +25,7 @@ #include "hw/block/flash.h" #include "qemu/error-report.h" #include "hw/char/pl011.h" +#include "hw/sd/sd.h" #define VERSATILE_FLASH_ADDR 0x34000000 #define VERSATILE_FLASH_SIZE (64 * 1024 * 1024) @@ -309,8 +310,29 @@ static void versatile_init(MachineState *machine, int board_id) /* Wire up the mux control signals from the SYS_CLCD register */ qdev_connect_gpio_out(sysctl, 0, qdev_get_gpio_in(dev, 0)); - sysbus_create_varargs("pl181", 0x10005000, sic[22], sic[1], NULL); - sysbus_create_varargs("pl181", 0x1000b000, sic[23], sic[2], NULL); + dev = sysbus_create_varargs("pl181", 0x10005000, sic[22], sic[1], NULL); + dinfo = drive_get_next(IF_SD); + if (dinfo) { + DeviceState *card; + + 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(dev, "sd-bus"), + &error_fatal); + } + + dev = sysbus_create_varargs("pl181", 0x1000b000, sic[23], sic[2], NULL); + dinfo = drive_get_next(IF_SD); + if (dinfo) { + DeviceState *card; + + 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(dev, "sd-bus"), + &error_fatal); + } /* Add PL031 Real Time Clock. */ sysbus_create_simple("pl031", 0x101e8000, pic[10]); diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c index 049a0ec2c7..95405f5940 100644 --- a/hw/arm/vexpress.c +++ b/hw/arm/vexpress.c @@ -43,6 +43,7 @@ #include "hw/cpu/a9mpcore.h" #include "hw/cpu/a15mpcore.h" #include "hw/i2c/arm_sbcon_i2c.h" +#include "hw/sd/sd.h" #define VEXPRESS_BOARD_ID 0x8e0 #define VEXPRESS_FLASH_SIZE (64 * 1024 * 1024) @@ -628,6 +629,16 @@ static void vexpress_common_init(MachineState *machine) qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_WPROT)); qdev_connect_gpio_out_named(dev, "card-inserted", 0, qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_CARDIN)); + dinfo = drive_get_next(IF_SD); + if (dinfo) { + DeviceState *card; + + 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(dev, "sd-bus"), + &error_fatal); + } sysbus_create_simple("pl050_keyboard", map[VE_KMI0], pic[12]); sysbus_create_simple("pl050_mouse", map[VE_KMI1], pic[13]); diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index f6de06ece8..f69488ebac 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -17,7 +17,6 @@ #include "qemu/module.h" #include "qemu/error-report.h" #include "qapi/error.h" -#include "hw/qdev-properties.h" //#define DEBUG_PL181 1 @@ -518,30 +517,14 @@ static void pl181_init(Object *obj) TYPE_PL181_BUS, dev, "sd-bus"); } -static void pl181_realize(DeviceState *dev, Error **errp) -{ - DeviceState *card; - DriveInfo *dinfo; - - /* FIXME use a qdev drive property instead of drive_get_next() */ - card = qdev_new(TYPE_SD_CARD); - dinfo = drive_get_next(IF_SD); - qdev_prop_set_drive_err(card, "drive", blk_by_legacy_dinfo(dinfo), - &error_fatal); - qdev_realize_and_unref(card, - qdev_get_child_bus(dev, "sd-bus"), - &error_fatal); -} - static void pl181_class_init(ObjectClass *klass, void *data) { DeviceClass *k = DEVICE_CLASS(klass); k->vmsd = &vmstate_pl181; k->reset = pl181_reset; - /* Reason: init() method uses drive_get_next() */ + /* Reason: output IRQs should be wired up */ k->user_creatable = false; - k->realize = pl181_realize; } static const TypeInfo pl181_info = { From 583d09f078c54862cd57f230a01e61b376bcde8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 5 Jul 2020 19:57:50 +0200 Subject: [PATCH 12/23] hw/sd/pl181: Replace disabled fprintf()s by trace events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert disabled DPRINTF() to trace events and remove ifdef'ry. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Acked-by: Peter Maydell Message-Id: <20200705204630.4133-9-f4bug@amsat.org> --- hw/sd/pl181.c | 26 +++++++++----------------- hw/sd/trace-events | 10 ++++++++++ 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index f69488ebac..574500ce60 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -17,15 +17,7 @@ #include "qemu/module.h" #include "qemu/error-report.h" #include "qapi/error.h" - -//#define DEBUG_PL181 1 - -#ifdef DEBUG_PL181 -#define DPRINTF(fmt, ...) \ -do { printf("pl181: " fmt , ## __VA_ARGS__); } while (0) -#else -#define DPRINTF(fmt, ...) do {} while(0) -#endif +#include "trace.h" #define PL181_FIFO_LEN 16 @@ -158,7 +150,7 @@ static void pl181_fifo_push(PL181State *s, uint32_t value) n = (s->fifo_pos + s->fifo_len) & (PL181_FIFO_LEN - 1); s->fifo_len++; s->fifo[n] = value; - DPRINTF("FIFO push %08x\n", (int)value); + trace_pl181_fifo_push(value); } static uint32_t pl181_fifo_pop(PL181State *s) @@ -172,7 +164,7 @@ static uint32_t pl181_fifo_pop(PL181State *s) value = s->fifo[s->fifo_pos]; s->fifo_len--; s->fifo_pos = (s->fifo_pos + 1) & (PL181_FIFO_LEN - 1); - DPRINTF("FIFO pop %08x\n", (int)value); + trace_pl181_fifo_pop(value); return value; } @@ -184,7 +176,7 @@ static void pl181_do_command(PL181State *s) request.cmd = s->cmd & PL181_CMD_INDEX; request.arg = s->cmdarg; - DPRINTF("Command %d %08x\n", request.cmd, request.arg); + trace_pl181_command_send(request.cmd, request.arg); rlen = sdbus_do_command(&s->sdbus, &request, response); if (rlen < 0) goto error; @@ -201,16 +193,16 @@ static void pl181_do_command(PL181State *s) s->response[2] = ldl_be_p(&response[8]); s->response[3] = ldl_be_p(&response[12]) & ~1; } - DPRINTF("Response received\n"); + trace_pl181_command_response_pending(); s->status |= PL181_STATUS_CMDRESPEND; } else { - DPRINTF("Command sent\n"); + trace_pl181_command_sent(); s->status |= PL181_STATUS_CMDSENT; } return; error: - DPRINTF("Timeout\n"); + trace_pl181_command_timeout(); s->status |= PL181_STATUS_CMDTIMEOUT; } @@ -262,11 +254,11 @@ static void pl181_fifo_run(PL181State *s) s->status |= PL181_STATUS_DATAEND; /* HACK: */ s->status |= PL181_STATUS_DATABLOCKEND; - DPRINTF("Transfer Complete\n"); + trace_pl181_fifo_transfer_complete(); } if (s->datacnt == 0 && s->fifo_len == 0) { s->datactrl &= ~PL181_DATA_ENABLE; - DPRINTF("Data engine idle\n"); + trace_pl181_data_engine_idle(); } else { /* Update FIFO bits. */ bits = PL181_STATUS_TXACTIVE | PL181_STATUS_RXACTIVE; diff --git a/hw/sd/trace-events b/hw/sd/trace-events index 5f09d32eb2..a87d7355fb 100644 --- a/hw/sd/trace-events +++ b/hw/sd/trace-events @@ -62,3 +62,13 @@ milkymist_memcard_memory_write(uint32_t addr, uint32_t value) "addr 0x%08x value # pxa2xx_mmci.c pxa2xx_mmci_read(uint8_t size, uint32_t addr, uint32_t value) "size %d addr 0x%02x value 0x%08x" pxa2xx_mmci_write(uint8_t size, uint32_t addr, uint32_t value) "size %d addr 0x%02x value 0x%08x" + +# pl181.c +pl181_command_send(uint8_t cmd, uint32_t arg) "sending CMD%02d arg 0x%08" PRIx32 +pl181_command_sent(void) "command sent" +pl181_command_response_pending(void) "response received" +pl181_command_timeout(void) "command timeouted" +pl181_fifo_push(uint32_t data) "FIFO push 0x%08" PRIx32 +pl181_fifo_pop(uint32_t data) "FIFO pop 0x%08" PRIx32 +pl181_fifo_transfer_complete(void) "FIFO transfer complete" +pl181_data_engine_idle(void) "data engine idle" From 38626a33145aa71b38e728ed0f178571fb49c1f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 15 Feb 2018 23:01:26 -0300 Subject: [PATCH 13/23] hw/sd/sdcard: Make sd_data_ready() static MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sd_data_ready() belongs to the legacy API. As its last user has been converted to the SDBus API, make it static. Reviewed-by: Alistair Francis Message-Id: <20180216022933.10945-7-f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé Acked-by: Peter Maydell --- hw/sd/sd.c | 2 +- include/hw/sd/sd.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index fad9cf1ee7..a5ae5dccbe 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -2082,7 +2082,7 @@ uint8_t sd_read_data(SDState *sd) return ret; } -bool sd_data_ready(SDState *sd) +static bool sd_data_ready(SDState *sd) { return sd->state == sd_sendingdata_state; } diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h index a84b8e274a..ace350e0e8 100644 --- a/include/hw/sd/sd.h +++ b/include/hw/sd/sd.h @@ -143,7 +143,6 @@ int sd_do_command(SDState *sd, SDRequest *req, void sd_write_data(SDState *sd, uint8_t value); uint8_t sd_read_data(SDState *sd); void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert); -bool sd_data_ready(SDState *sd); /* sd_enable should not be used -- it is only used on the nseries boards, * where it is part of a broken implementation of the MMC card slot switch * (there should be two card slots which are multiplexed to a single MMC From 9006f1e706e0f41efdb62556cefe88d746add88a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sat, 23 Dec 2017 17:29:33 -0300 Subject: [PATCH 14/23] hw/sd: Move sdcard legacy API to 'hw/sd/sdcard_legacy.h' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit omap_mmc.c is the last device left using the legacy sdcard API. Move the prototype declarations into a separate header, to make it clear this is a legacy API. Reviewed-by: Alistair Francis Message-Id: <20180216022933.10945-8-f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé Acked-by: Peter Maydell --- hw/sd/omap_mmc.c | 2 +- hw/sd/sd.c | 1 + include/hw/sd/sd.h | 16 ----------- include/hw/sd/sdcard_legacy.h | 50 +++++++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 17 deletions(-) create mode 100644 include/hw/sd/sdcard_legacy.h diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c index 4088a8a80b..7d33c59226 100644 --- a/hw/sd/omap_mmc.c +++ b/hw/sd/omap_mmc.c @@ -23,7 +23,7 @@ #include "qemu/log.h" #include "hw/irq.h" #include "hw/arm/omap.h" -#include "hw/sd/sd.h" +#include "hw/sd/sdcard_legacy.h" struct omap_mmc_s { qemu_irq irq; diff --git a/hw/sd/sd.c b/hw/sd/sd.c index a5ae5dccbe..5c6f5c94f3 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -37,6 +37,7 @@ #include "hw/registerfields.h" #include "sysemu/block-backend.h" #include "hw/sd/sd.h" +#include "hw/sd/sdcard_legacy.h" #include "migration/vmstate.h" #include "qapi/error.h" #include "qemu/bitmap.h" diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h index ace350e0e8..8767ab817c 100644 --- a/include/hw/sd/sd.h +++ b/include/hw/sd/sd.h @@ -136,22 +136,6 @@ typedef struct { void (*set_readonly)(DeviceState *dev, bool readonly); } SDBusClass; -/* Legacy functions to be used only by non-qdevified callers */ -SDState *sd_init(BlockBackend *bs, bool is_spi); -int sd_do_command(SDState *sd, SDRequest *req, - uint8_t *response); -void sd_write_data(SDState *sd, uint8_t value); -uint8_t sd_read_data(SDState *sd); -void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert); -/* sd_enable should not be used -- it is only used on the nseries boards, - * where it is part of a broken implementation of the MMC card slot switch - * (there should be two card slots which are multiplexed to a single MMC - * controller, but instead we model it with one card and controller and - * disable the card when the second slot is selected, so it looks like the - * second slot is always empty). - */ -void sd_enable(SDState *sd, bool enable); - /* Functions to be used by qdevified callers (working via * an SDBus rather than directly with SDState) */ diff --git a/include/hw/sd/sdcard_legacy.h b/include/hw/sd/sdcard_legacy.h new file mode 100644 index 0000000000..8681f8089b --- /dev/null +++ b/include/hw/sd/sdcard_legacy.h @@ -0,0 +1,50 @@ +/* + * SD Memory Card emulation (deprecated legacy API) + * + * Copyright (c) 2006 Andrzej Zaborowski + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY + * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +#ifndef HW_SDCARD_LEGACY_H +#define HW_SDCARD_LEGACY_H + +#include "hw/sd/sd.h" + +/* Legacy functions to be used only by non-qdevified callers */ +SDState *sd_init(BlockBackend *blk, bool is_spi); +int sd_do_command(SDState *card, SDRequest *request, uint8_t *response); +void sd_write_data(SDState *card, uint8_t value); +uint8_t sd_read_data(SDState *card); +void sd_set_cb(SDState *card, qemu_irq readonly, qemu_irq insert); + +/* sd_enable should not be used -- it is only used on the nseries boards, + * where it is part of a broken implementation of the MMC card slot switch + * (there should be two card slots which are multiplexed to a single MMC + * controller, but instead we model it with one card and controller and + * disable the card when the second slot is selected, so it looks like the + * second slot is always empty). + */ +void sd_enable(SDState *card, bool enable); + +#endif /* HW_SDCARD_LEGACY_H */ From c769a88d4475f648595c6a270d9d0b0f3f7f3740 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 14 Aug 2020 11:23:40 +0200 Subject: [PATCH 15/23] hw/sd: Rename read/write_data() as read/write_byte() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The read/write_data() methods write do a single byte access on the data line of a SD card. Rename them as read/write_byte(). Add some documentation (not in "hw/sd/sdcard_legacy.h" which we are going to remove soon). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200814092346.21825-2-f4bug@amsat.org> --- hw/sd/core.c | 4 ++-- hw/sd/omap_mmc.c | 8 ++++---- hw/sd/sd.c | 16 ++++++++-------- include/hw/sd/sd.h | 19 +++++++++++++++++-- include/hw/sd/sdcard_legacy.h | 4 ++-- 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/hw/sd/core.c b/hw/sd/core.c index abec48bccb..79d96576ea 100644 --- a/hw/sd/core.c +++ b/hw/sd/core.c @@ -110,7 +110,7 @@ void sdbus_write_data(SDBus *sdbus, uint8_t value) if (card) { SDCardClass *sc = SD_CARD_GET_CLASS(card); - sc->write_data(card, value); + sc->write_byte(card, value); } } @@ -122,7 +122,7 @@ uint8_t sdbus_read_data(SDBus *sdbus) if (card) { SDCardClass *sc = SD_CARD_GET_CLASS(card); - value = sc->read_data(card); + value = sc->read_byte(card); } trace_sdbus_read(sdbus_name(sdbus), value); diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c index 7d33c59226..1f946908fe 100644 --- a/hw/sd/omap_mmc.c +++ b/hw/sd/omap_mmc.c @@ -232,10 +232,10 @@ static void omap_mmc_transfer(struct omap_mmc_s *host) if (host->fifo_len > host->af_level) break; - value = sd_read_data(host->card); + value = sd_read_byte(host->card); host->fifo[(host->fifo_start + host->fifo_len) & 31] = value; if (-- host->blen_counter) { - value = sd_read_data(host->card); + value = sd_read_byte(host->card); host->fifo[(host->fifo_start + host->fifo_len) & 31] |= value << 8; host->blen_counter --; @@ -247,10 +247,10 @@ static void omap_mmc_transfer(struct omap_mmc_s *host) break; value = host->fifo[host->fifo_start] & 0xff; - sd_write_data(host->card, value); + sd_write_byte(host->card, value); if (-- host->blen_counter) { value = host->fifo[host->fifo_start] >> 8; - sd_write_data(host->card, value); + sd_write_byte(host->card, value); host->blen_counter --; } diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 5c6f5c94f3..7c9d956f11 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1809,7 +1809,7 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) #define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len) #define APP_WRITE_BLOCK(a, len) -void sd_write_data(SDState *sd, uint8_t value) +void sd_write_byte(SDState *sd, uint8_t value) { int i; @@ -1818,7 +1818,7 @@ void sd_write_data(SDState *sd, uint8_t value) if (sd->state != sd_receivingdata_state) { qemu_log_mask(LOG_GUEST_ERROR, - "sd_write_data: not in Receiving-Data state\n"); + "%s: not in Receiving-Data state\n", __func__); return; } @@ -1940,7 +1940,7 @@ void sd_write_data(SDState *sd, uint8_t value) break; default: - qemu_log_mask(LOG_GUEST_ERROR, "sd_write_data: unknown command\n"); + qemu_log_mask(LOG_GUEST_ERROR, "%s: unknown command\n", __func__); break; } } @@ -1959,7 +1959,7 @@ static const uint8_t sd_tuning_block_pattern[SD_TUNING_BLOCK_SIZE] = { 0xbb, 0xff, 0xf7, 0xff, 0xf7, 0x7f, 0x7b, 0xde, }; -uint8_t sd_read_data(SDState *sd) +uint8_t sd_read_byte(SDState *sd) { /* TODO: Append CRCs */ uint8_t ret; @@ -1970,7 +1970,7 @@ uint8_t sd_read_data(SDState *sd) if (sd->state != sd_sendingdata_state) { qemu_log_mask(LOG_GUEST_ERROR, - "sd_read_data: not in Sending-Data state\n"); + "%s: not in Sending-Data state\n", __func__); return 0x00; } @@ -2076,7 +2076,7 @@ uint8_t sd_read_data(SDState *sd) break; default: - qemu_log_mask(LOG_GUEST_ERROR, "sd_read_data: unknown command\n"); + qemu_log_mask(LOG_GUEST_ERROR, "%s: unknown command\n", __func__); return 0x00; } @@ -2192,8 +2192,8 @@ static void sd_class_init(ObjectClass *klass, void *data) sc->get_dat_lines = sd_get_dat_lines; sc->get_cmd_line = sd_get_cmd_line; sc->do_command = sd_do_command; - sc->write_data = sd_write_data; - sc->read_data = sd_read_data; + sc->write_byte = sd_write_byte; + sc->read_byte = sd_read_byte; sc->data_ready = sd_data_ready; sc->enable = sd_enable; sc->get_inserted = sd_get_inserted; diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h index 8767ab817c..b58b5a19af 100644 --- a/include/hw/sd/sd.h +++ b/include/hw/sd/sd.h @@ -104,8 +104,23 @@ typedef struct { /*< public >*/ int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response); - void (*write_data)(SDState *sd, uint8_t value); - uint8_t (*read_data)(SDState *sd); + /** + * Write a byte to a SD card. + * @sd: card + * @value: byte to write + * + * Write a byte on the data lines of a SD card. + */ + void (*write_byte)(SDState *sd, uint8_t value); + /** + * Read a byte from a SD card. + * @sd: card + * + * Read a byte from the data lines of a SD card. + * + * Return: byte value read + */ + uint8_t (*read_byte)(SDState *sd); bool (*data_ready)(SDState *sd); void (*set_voltage)(SDState *sd, uint16_t millivolts); uint8_t (*get_dat_lines)(SDState *sd); diff --git a/include/hw/sd/sdcard_legacy.h b/include/hw/sd/sdcard_legacy.h index 8681f8089b..0dc3889555 100644 --- a/include/hw/sd/sdcard_legacy.h +++ b/include/hw/sd/sdcard_legacy.h @@ -34,8 +34,8 @@ /* Legacy functions to be used only by non-qdevified callers */ SDState *sd_init(BlockBackend *blk, bool is_spi); int sd_do_command(SDState *card, SDRequest *request, uint8_t *response); -void sd_write_data(SDState *card, uint8_t value); -uint8_t sd_read_data(SDState *card); +void sd_write_byte(SDState *card, uint8_t value); +uint8_t sd_read_byte(SDState *card); void sd_set_cb(SDState *card, qemu_irq readonly, qemu_irq insert); /* sd_enable should not be used -- it is only used on the nseries boards, From 39017143d6d7a068573da272528d7df71d0d31b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 14 Aug 2020 11:23:41 +0200 Subject: [PATCH 16/23] hw/sd: Rename sdbus_write_data() as sdbus_write_byte() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sdbus_write_data() method do a single byte access on the data line of a SD bus. Rename it as sdbus_write_byte() and document it. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200814092346.21825-3-f4bug@amsat.org> --- hw/sd/allwinner-sdhost.c | 10 +++++----- hw/sd/bcm2835_sdhost.c | 2 +- hw/sd/core.c | 2 +- hw/sd/milkymist-memcard.c | 8 ++++---- hw/sd/pl181.c | 2 +- hw/sd/pxa2xx_mmci.c | 2 +- hw/sd/sdhci.c | 8 ++++---- include/hw/sd/sd.h | 9 ++++++++- 8 files changed, 25 insertions(+), 18 deletions(-) diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c index f404e1fdb4..e05e8a3864 100644 --- a/hw/sd/allwinner-sdhost.c +++ b/hw/sd/allwinner-sdhost.c @@ -335,7 +335,7 @@ static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, buf, buf_bytes); for (uint32_t i = 0; i < buf_bytes; i++) { - sdbus_write_data(&s->sdbus, buf[i]); + sdbus_write_byte(&s->sdbus, buf[i]); } /* Read from SD bus */ @@ -654,10 +654,10 @@ static void allwinner_sdhost_write(void *opaque, hwaddr offset, s->startbit_detect = value; break; case REG_SD_FIFO: /* Read/Write FIFO */ - sdbus_write_data(&s->sdbus, value & 0xff); - sdbus_write_data(&s->sdbus, (value >> 8) & 0xff); - sdbus_write_data(&s->sdbus, (value >> 16) & 0xff); - sdbus_write_data(&s->sdbus, (value >> 24) & 0xff); + sdbus_write_byte(&s->sdbus, value & 0xff); + sdbus_write_byte(&s->sdbus, (value >> 8) & 0xff); + sdbus_write_byte(&s->sdbus, (value >> 16) & 0xff); + sdbus_write_byte(&s->sdbus, (value >> 24) & 0xff); allwinner_sdhost_update_transfer_cnt(s, sizeof(uint32_t)); allwinner_sdhost_auto_stop(s); allwinner_sdhost_update_irq(s); diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c index 4a80fbcc86..16aba7cc92 100644 --- a/hw/sd/bcm2835_sdhost.c +++ b/hw/sd/bcm2835_sdhost.c @@ -223,7 +223,7 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s) } n--; s->datacnt--; - sdbus_write_data(&s->sdbus, value & 0xff); + sdbus_write_byte(&s->sdbus, value & 0xff); value >>= 8; } } diff --git a/hw/sd/core.c b/hw/sd/core.c index 79d96576ea..13b5ca0316 100644 --- a/hw/sd/core.c +++ b/hw/sd/core.c @@ -102,7 +102,7 @@ int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response) return 0; } -void sdbus_write_data(SDBus *sdbus, uint8_t value) +void sdbus_write_byte(SDBus *sdbus, uint8_t value) { SDState *card = get_card(sdbus); diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c index e9f5db5e22..4128109c04 100644 --- a/hw/sd/milkymist-memcard.c +++ b/hw/sd/milkymist-memcard.c @@ -209,10 +209,10 @@ static void memcard_write(void *opaque, hwaddr addr, uint64_t value, if (!s->enabled) { break; } - sdbus_write_data(&s->sdbus, (value >> 24) & 0xff); - sdbus_write_data(&s->sdbus, (value >> 16) & 0xff); - sdbus_write_data(&s->sdbus, (value >> 8) & 0xff); - sdbus_write_data(&s->sdbus, value & 0xff); + sdbus_write_byte(&s->sdbus, (value >> 24) & 0xff); + sdbus_write_byte(&s->sdbus, (value >> 16) & 0xff); + sdbus_write_byte(&s->sdbus, (value >> 8) & 0xff); + sdbus_write_byte(&s->sdbus, value & 0xff); break; case R_ENABLE: s->regs[addr] = value; diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index 574500ce60..771bae193f 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -244,7 +244,7 @@ static void pl181_fifo_run(PL181State *s) } n--; s->datacnt--; - sdbus_write_data(&s->sdbus, value & 0xff); + sdbus_write_byte(&s->sdbus, value & 0xff); value >>= 8; } } diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c index 2996a2ef17..07ddc2eba3 100644 --- a/hw/sd/pxa2xx_mmci.c +++ b/hw/sd/pxa2xx_mmci.c @@ -184,7 +184,7 @@ static void pxa2xx_mmci_fifo_update(PXA2xxMMCIState *s) if (s->cmdat & CMDAT_WR_RD) { while (s->bytesleft && s->tx_len) { - sdbus_write_data(&s->sdbus, s->tx_fifo[s->tx_start++]); + sdbus_write_byte(&s->sdbus, s->tx_fifo[s->tx_start++]); s->tx_start &= 0x1f; s->tx_len --; s->bytesleft --; diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index deac181865..4bf1ee88b2 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -515,7 +515,7 @@ static void sdhci_write_block_to_card(SDHCIState *s) } for (index = 0; index < (s->blksize & BLOCK_SIZE_MASK); index++) { - sdbus_write_data(&s->sdbus, s->fifo_buffer[index]); + sdbus_write_byte(&s->sdbus, s->fifo_buffer[index]); } /* Next data can be written through BUFFER DATORT register */ @@ -642,7 +642,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) s->sdmasysad += s->data_count - begin; if (s->data_count == block_size) { for (n = 0; n < block_size; n++) { - sdbus_write_data(&s->sdbus, s->fifo_buffer[n]); + sdbus_write_byte(&s->sdbus, s->fifo_buffer[n]); } s->data_count = 0; if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) { @@ -679,7 +679,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s) } else { dma_memory_read(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt); for (n = 0; n < datacnt; n++) { - sdbus_write_data(&s->sdbus, s->fifo_buffer[n]); + sdbus_write_byte(&s->sdbus, s->fifo_buffer[n]); } } s->blkcnt--; @@ -815,7 +815,7 @@ static void sdhci_do_adma(SDHCIState *s) dscr.addr += s->data_count - begin; if (s->data_count == block_size) { for (n = 0; n < block_size; n++) { - sdbus_write_data(&s->sdbus, s->fifo_buffer[n]); + sdbus_write_byte(&s->sdbus, s->fifo_buffer[n]); } s->data_count = 0; if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) { diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h index b58b5a19af..1e5ac955d0 100644 --- a/include/hw/sd/sd.h +++ b/include/hw/sd/sd.h @@ -158,7 +158,14 @@ void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts); uint8_t sdbus_get_dat_lines(SDBus *sdbus); bool sdbus_get_cmd_line(SDBus *sdbus); int sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *response); -void sdbus_write_data(SDBus *sd, uint8_t value); +/** + * Write a byte to a SD bus. + * @sd: bus + * @value: byte to write + * + * Write a byte on the data lines of a SD bus. + */ +void sdbus_write_byte(SDBus *sd, uint8_t value); uint8_t sdbus_read_data(SDBus *sd); bool sdbus_data_ready(SDBus *sd); bool sdbus_get_inserted(SDBus *sd); From 8467f62201fd2383d553702f9f9441e2493ece75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 14 Aug 2020 11:23:42 +0200 Subject: [PATCH 17/23] hw/sd: Rename sdbus_read_data() as sdbus_read_byte() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sdbus_read_data() method do a single byte access on the data line of a SD bus. Rename it as sdbus_read_byte() and document it. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200814092346.21825-4-f4bug@amsat.org> --- hw/sd/allwinner-sdhost.c | 10 +++++----- hw/sd/bcm2835_sdhost.c | 2 +- hw/sd/core.c | 2 +- hw/sd/milkymist-memcard.c | 8 ++++---- hw/sd/pl181.c | 2 +- hw/sd/pxa2xx_mmci.c | 2 +- hw/sd/sdhci.c | 8 ++++---- hw/sd/ssi-sd.c | 2 +- include/hw/sd/sd.h | 10 +++++++++- 9 files changed, 27 insertions(+), 19 deletions(-) diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c index e05e8a3864..c004aa39da 100644 --- a/hw/sd/allwinner-sdhost.c +++ b/hw/sd/allwinner-sdhost.c @@ -341,7 +341,7 @@ static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, /* Read from SD bus */ } else { for (uint32_t i = 0; i < buf_bytes; i++) { - buf[i] = sdbus_read_data(&s->sdbus); + buf[i] = sdbus_read_byte(&s->sdbus); } cpu_physical_memory_write((desc->addr & DESC_SIZE_MASK) + num_done, buf, buf_bytes); @@ -521,10 +521,10 @@ static uint64_t allwinner_sdhost_read(void *opaque, hwaddr offset, break; case REG_SD_FIFO: /* Read/Write FIFO */ if (sdbus_data_ready(&s->sdbus)) { - res = sdbus_read_data(&s->sdbus); - res |= sdbus_read_data(&s->sdbus) << 8; - res |= sdbus_read_data(&s->sdbus) << 16; - res |= sdbus_read_data(&s->sdbus) << 24; + res = sdbus_read_byte(&s->sdbus); + res |= sdbus_read_byte(&s->sdbus) << 8; + res |= sdbus_read_byte(&s->sdbus) << 16; + res |= sdbus_read_byte(&s->sdbus) << 24; allwinner_sdhost_update_transfer_cnt(s, sizeof(uint32_t)); allwinner_sdhost_auto_stop(s); allwinner_sdhost_update_irq(s); diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c index 16aba7cc92..2c7a675a2d 100644 --- a/hw/sd/bcm2835_sdhost.c +++ b/hw/sd/bcm2835_sdhost.c @@ -190,7 +190,7 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s) if (is_read) { n = 0; while (s->datacnt && s->fifo_len < BCM2835_SDHOST_FIFO_LEN) { - value |= (uint32_t)sdbus_read_data(&s->sdbus) << (n * 8); + value |= (uint32_t)sdbus_read_byte(&s->sdbus) << (n * 8); s->datacnt--; n++; if (n == 4) { diff --git a/hw/sd/core.c b/hw/sd/core.c index 13b5ca0316..a3b620b802 100644 --- a/hw/sd/core.c +++ b/hw/sd/core.c @@ -114,7 +114,7 @@ void sdbus_write_byte(SDBus *sdbus, uint8_t value) } } -uint8_t sdbus_read_data(SDBus *sdbus) +uint8_t sdbus_read_byte(SDBus *sdbus) { SDState *card = get_card(sdbus); uint8_t value = 0; diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c index 4128109c04..e8d055bb89 100644 --- a/hw/sd/milkymist-memcard.c +++ b/hw/sd/milkymist-memcard.c @@ -152,10 +152,10 @@ static uint64_t memcard_read(void *opaque, hwaddr addr, r = 0xffffffff; } else { r = 0; - r |= sdbus_read_data(&s->sdbus) << 24; - r |= sdbus_read_data(&s->sdbus) << 16; - r |= sdbus_read_data(&s->sdbus) << 8; - r |= sdbus_read_data(&s->sdbus); + r |= sdbus_read_byte(&s->sdbus) << 24; + r |= sdbus_read_byte(&s->sdbus) << 16; + r |= sdbus_read_byte(&s->sdbus) << 8; + r |= sdbus_read_byte(&s->sdbus); } break; case R_CLK2XDIV: diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index 771bae193f..579d68ad83 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -223,7 +223,7 @@ static void pl181_fifo_run(PL181State *s) if (is_read) { n = 0; while (s->datacnt && s->fifo_len < PL181_FIFO_LEN) { - value |= (uint32_t)sdbus_read_data(&s->sdbus) << (n * 8); + value |= (uint32_t)sdbus_read_byte(&s->sdbus) << (n * 8); s->datacnt--; n++; if (n == 4) { diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c index 07ddc2eba3..04f0a98f81 100644 --- a/hw/sd/pxa2xx_mmci.c +++ b/hw/sd/pxa2xx_mmci.c @@ -194,7 +194,7 @@ static void pxa2xx_mmci_fifo_update(PXA2xxMMCIState *s) } else while (s->bytesleft && s->rx_len < 32) { s->rx_fifo[(s->rx_start + (s->rx_len ++)) & 0x1f] = - sdbus_read_data(&s->sdbus); + sdbus_read_byte(&s->sdbus); s->bytesleft --; s->intreq |= INT_RXFIFO_REQ; } diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 4bf1ee88b2..b897b1121b 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -409,7 +409,7 @@ static void sdhci_read_block_from_card(SDHCIState *s) } for (index = 0; index < blk_size; index++) { - data = sdbus_read_data(&s->sdbus); + data = sdbus_read_byte(&s->sdbus); if (!FIELD_EX32(s->hostctl2, SDHC_HOSTCTL2, EXECUTE_TUNING)) { /* Device is not in tuning */ s->fifo_buffer[index] = data; @@ -601,7 +601,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) while (s->blkcnt) { if (s->data_count == 0) { for (n = 0; n < block_size; n++) { - s->fifo_buffer[n] = sdbus_read_data(&s->sdbus); + s->fifo_buffer[n] = sdbus_read_byte(&s->sdbus); } } begin = s->data_count; @@ -673,7 +673,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s) if (s->trnmod & SDHC_TRNS_READ) { for (n = 0; n < datacnt; n++) { - s->fifo_buffer[n] = sdbus_read_data(&s->sdbus); + s->fifo_buffer[n] = sdbus_read_byte(&s->sdbus); } dma_memory_write(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt); } else { @@ -774,7 +774,7 @@ static void sdhci_do_adma(SDHCIState *s) while (length) { if (s->data_count == 0) { for (n = 0; n < block_size; n++) { - s->fifo_buffer[n] = sdbus_read_data(&s->sdbus); + s->fifo_buffer[n] = sdbus_read_byte(&s->sdbus); } } begin = s->data_count; diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index 9210ef567f..a7ef9cb922 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -190,7 +190,7 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val) s->mode = SSI_SD_DATA_READ; return 0xfe; case SSI_SD_DATA_READ: - val = sdbus_read_data(&s->sdbus); + val = sdbus_read_byte(&s->sdbus); if (!sdbus_data_ready(&s->sdbus)) { DPRINTF("Data read end\n"); s->mode = SSI_SD_CMD; diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h index 1e5ac955d0..14ffc7f475 100644 --- a/include/hw/sd/sd.h +++ b/include/hw/sd/sd.h @@ -166,7 +166,15 @@ int sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *response); * Write a byte on the data lines of a SD bus. */ void sdbus_write_byte(SDBus *sd, uint8_t value); -uint8_t sdbus_read_data(SDBus *sd); +/** + * Read a byte from a SD bus. + * @sd: bus + * + * Read a byte from the data lines of a SD bus. + * + * Return: byte value read + */ +uint8_t sdbus_read_byte(SDBus *sd); bool sdbus_data_ready(SDBus *sd); bool sdbus_get_inserted(SDBus *sd); bool sdbus_get_readonly(SDBus *sd); From e35c343dd9c18d4d3e6424e2fab38f08085875d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 14 Aug 2020 11:23:43 +0200 Subject: [PATCH 18/23] hw/sd: Add sdbus_write_data() to write multiples bytes on the data line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a sdbus_write_data() method to write multiple bytes on the data line of a SD bus. We might improve the tracing later, for now keep logging each byte individually. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200814092346.21825-5-f4bug@amsat.org> --- hw/sd/core.c | 15 +++++++++++++++ include/hw/sd/sd.h | 9 +++++++++ 2 files changed, 24 insertions(+) diff --git a/hw/sd/core.c b/hw/sd/core.c index a3b620b802..9c2781ebf9 100644 --- a/hw/sd/core.c +++ b/hw/sd/core.c @@ -114,6 +114,21 @@ void sdbus_write_byte(SDBus *sdbus, uint8_t value) } } +void sdbus_write_data(SDBus *sdbus, const void *buf, size_t length) +{ + SDState *card = get_card(sdbus); + const uint8_t *data = buf; + + if (card) { + SDCardClass *sc = SD_CARD_GET_CLASS(card); + + for (size_t i = 0; i < length; i++) { + trace_sdbus_write(sdbus_name(sdbus), data[i]); + sc->write_byte(card, data[i]); + } + } +} + uint8_t sdbus_read_byte(SDBus *sdbus) { SDState *card = get_card(sdbus); diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h index 14ffc7f475..3ae3e8939b 100644 --- a/include/hw/sd/sd.h +++ b/include/hw/sd/sd.h @@ -175,6 +175,15 @@ void sdbus_write_byte(SDBus *sd, uint8_t value); * Return: byte value read */ uint8_t sdbus_read_byte(SDBus *sd); +/** + * Write data to a SD bus. + * @sdbus: bus + * @buf: data to write + * @length: number of bytes to write + * + * Write multiple bytes of data on the data lines of a SD bus. + */ +void sdbus_write_data(SDBus *sdbus, const void *buf, size_t length); bool sdbus_data_ready(SDBus *sd); bool sdbus_get_inserted(SDBus *sd); bool sdbus_get_readonly(SDBus *sd); From 62a21be60f8fdf0223fa099fb4e7a495eaf55cf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 14 Aug 2020 11:23:44 +0200 Subject: [PATCH 19/23] hw/sd: Use sdbus_write_data() instead of sdbus_write_byte when possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the recently added sdbus_write_data() to write multiple bytes at once, instead of looping calling sdbus_write_byte(). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200814092346.21825-6-f4bug@amsat.org> --- hw/sd/allwinner-sdhost.c | 14 +++++--------- hw/sd/milkymist-memcard.c | 7 +++---- hw/sd/sdhci.c | 18 ++++-------------- 3 files changed, 12 insertions(+), 27 deletions(-) diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c index c004aa39da..eea5659c5f 100644 --- a/hw/sd/allwinner-sdhost.c +++ b/hw/sd/allwinner-sdhost.c @@ -333,10 +333,7 @@ static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, if (is_write) { cpu_physical_memory_read((desc->addr & DESC_SIZE_MASK) + num_done, buf, buf_bytes); - - for (uint32_t i = 0; i < buf_bytes; i++) { - sdbus_write_byte(&s->sdbus, buf[i]); - } + sdbus_write_data(&s->sdbus, buf, buf_bytes); /* Read from SD bus */ } else { @@ -548,6 +545,7 @@ static void allwinner_sdhost_write(void *opaque, hwaddr offset, uint64_t value, unsigned size) { AwSdHostState *s = AW_SDHOST(opaque); + uint32_t u32; trace_allwinner_sdhost_write(offset, value, size); @@ -654,11 +652,9 @@ static void allwinner_sdhost_write(void *opaque, hwaddr offset, s->startbit_detect = value; break; case REG_SD_FIFO: /* Read/Write FIFO */ - sdbus_write_byte(&s->sdbus, value & 0xff); - sdbus_write_byte(&s->sdbus, (value >> 8) & 0xff); - sdbus_write_byte(&s->sdbus, (value >> 16) & 0xff); - sdbus_write_byte(&s->sdbus, (value >> 24) & 0xff); - allwinner_sdhost_update_transfer_cnt(s, sizeof(uint32_t)); + u32 = cpu_to_le32(value); + sdbus_write_data(&s->sdbus, &u32, sizeof(u32)); + allwinner_sdhost_update_transfer_cnt(s, sizeof(u32)); allwinner_sdhost_auto_stop(s); allwinner_sdhost_update_irq(s); break; diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c index e8d055bb89..12e091a46e 100644 --- a/hw/sd/milkymist-memcard.c +++ b/hw/sd/milkymist-memcard.c @@ -181,6 +181,7 @@ static void memcard_write(void *opaque, hwaddr addr, uint64_t value, unsigned size) { MilkymistMemcardState *s = opaque; + uint32_t val32; trace_milkymist_memcard_memory_write(addr, value); @@ -209,10 +210,8 @@ static void memcard_write(void *opaque, hwaddr addr, uint64_t value, if (!s->enabled) { break; } - sdbus_write_byte(&s->sdbus, (value >> 24) & 0xff); - sdbus_write_byte(&s->sdbus, (value >> 16) & 0xff); - sdbus_write_byte(&s->sdbus, (value >> 8) & 0xff); - sdbus_write_byte(&s->sdbus, value & 0xff); + val32 = cpu_to_be32(value); + sdbus_write_data(&s->sdbus, &val32, sizeof(val32)); break; case R_ENABLE: s->regs[addr] = value; diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index b897b1121b..ddf3691561 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -496,8 +496,6 @@ static uint32_t sdhci_read_dataport(SDHCIState *s, unsigned size) /* Write data from host controller FIFO to card */ static void sdhci_write_block_to_card(SDHCIState *s) { - int index = 0; - if (s->prnsts & SDHC_SPACE_AVAILABLE) { if (s->norintstsen & SDHC_NISEN_WBUFRDY) { s->norintsts |= SDHC_NIS_WBUFRDY; @@ -514,9 +512,7 @@ static void sdhci_write_block_to_card(SDHCIState *s) } } - for (index = 0; index < (s->blksize & BLOCK_SIZE_MASK); index++) { - sdbus_write_byte(&s->sdbus, s->fifo_buffer[index]); - } + sdbus_write_data(&s->sdbus, s->fifo_buffer, s->blksize & BLOCK_SIZE_MASK); /* Next data can be written through BUFFER DATORT register */ s->prnsts |= SDHC_SPACE_AVAILABLE; @@ -641,9 +637,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) &s->fifo_buffer[begin], s->data_count - begin); s->sdmasysad += s->data_count - begin; if (s->data_count == block_size) { - for (n = 0; n < block_size; n++) { - sdbus_write_byte(&s->sdbus, s->fifo_buffer[n]); - } + sdbus_write_data(&s->sdbus, s->fifo_buffer, block_size); s->data_count = 0; if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) { s->blkcnt--; @@ -678,9 +672,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s) dma_memory_write(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt); } else { dma_memory_read(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt); - for (n = 0; n < datacnt; n++) { - sdbus_write_byte(&s->sdbus, s->fifo_buffer[n]); - } + sdbus_write_data(&s->sdbus, s->fifo_buffer, datacnt); } s->blkcnt--; @@ -814,9 +806,7 @@ static void sdhci_do_adma(SDHCIState *s) s->data_count - begin); dscr.addr += s->data_count - begin; if (s->data_count == block_size) { - for (n = 0; n < block_size; n++) { - sdbus_write_byte(&s->sdbus, s->fifo_buffer[n]); - } + sdbus_write_data(&s->sdbus, s->fifo_buffer, block_size); s->data_count = 0; if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) { s->blkcnt--; From 6505a91a77d4efcc6d54b35851abcb137cdca30a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 14 Aug 2020 11:23:45 +0200 Subject: [PATCH 20/23] hw/sd: Add sdbus_read_data() to read multiples bytes on the data line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a sdbus_read_data() method to read multiple bytes on the data line of a SD bus. We might improve the tracing later, for now keep logging each byte individually. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200814092346.21825-7-f4bug@amsat.org> --- hw/sd/core.c | 15 +++++++++++++++ include/hw/sd/sd.h | 9 +++++++++ 2 files changed, 24 insertions(+) diff --git a/hw/sd/core.c b/hw/sd/core.c index 9c2781ebf9..957d116f1a 100644 --- a/hw/sd/core.c +++ b/hw/sd/core.c @@ -144,6 +144,21 @@ uint8_t sdbus_read_byte(SDBus *sdbus) return value; } +void sdbus_read_data(SDBus *sdbus, void *buf, size_t length) +{ + SDState *card = get_card(sdbus); + uint8_t *data = buf; + + if (card) { + SDCardClass *sc = SD_CARD_GET_CLASS(card); + + for (size_t i = 0; i < length; i++) { + data[i] = sc->read_byte(card); + trace_sdbus_read(sdbus_name(sdbus), data[i]); + } + } +} + bool sdbus_data_ready(SDBus *sdbus) { SDState *card = get_card(sdbus); diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h index 3ae3e8939b..ac02d61a7a 100644 --- a/include/hw/sd/sd.h +++ b/include/hw/sd/sd.h @@ -184,6 +184,15 @@ uint8_t sdbus_read_byte(SDBus *sd); * Write multiple bytes of data on the data lines of a SD bus. */ void sdbus_write_data(SDBus *sdbus, const void *buf, size_t length); +/** + * Read data from a SD bus. + * @sdbus: bus + * @buf: buffer to read data into + * @length: number of bytes to read + * + * Read multiple bytes of data on the data lines of a SD bus. + */ +void sdbus_read_data(SDBus *sdbus, void *buf, size_t length); bool sdbus_data_ready(SDBus *sd); bool sdbus_get_inserted(SDBus *sd); bool sdbus_get_readonly(SDBus *sd); From 618e0be1bac7ffe794242b422f7a9767f2a8be79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 14 Aug 2020 11:23:46 +0200 Subject: [PATCH 21/23] hw/sd: Use sdbus_read_data() instead of sdbus_read_byte() when possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the recently added sdbus_read_data() to read multiple bytes at once, instead of looping calling sdbus_read_byte(). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200814092346.21825-8-f4bug@amsat.org> --- hw/sd/allwinner-sdhost.c | 10 +++------- hw/sd/milkymist-memcard.c | 7 ++----- hw/sd/sdhci.c | 28 ++++++++-------------------- 3 files changed, 13 insertions(+), 32 deletions(-) diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c index eea5659c5f..f9eb92c09e 100644 --- a/hw/sd/allwinner-sdhost.c +++ b/hw/sd/allwinner-sdhost.c @@ -337,9 +337,7 @@ static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, /* Read from SD bus */ } else { - for (uint32_t i = 0; i < buf_bytes; i++) { - buf[i] = sdbus_read_byte(&s->sdbus); - } + sdbus_read_data(&s->sdbus, buf, buf_bytes); cpu_physical_memory_write((desc->addr & DESC_SIZE_MASK) + num_done, buf, buf_bytes); } @@ -518,10 +516,8 @@ static uint64_t allwinner_sdhost_read(void *opaque, hwaddr offset, break; case REG_SD_FIFO: /* Read/Write FIFO */ if (sdbus_data_ready(&s->sdbus)) { - res = sdbus_read_byte(&s->sdbus); - res |= sdbus_read_byte(&s->sdbus) << 8; - res |= sdbus_read_byte(&s->sdbus) << 16; - res |= sdbus_read_byte(&s->sdbus) << 24; + sdbus_read_data(&s->sdbus, &res, sizeof(uint32_t)); + le32_to_cpus(&res); allwinner_sdhost_update_transfer_cnt(s, sizeof(uint32_t)); allwinner_sdhost_auto_stop(s); allwinner_sdhost_update_irq(s); diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c index 12e091a46e..be89a93876 100644 --- a/hw/sd/milkymist-memcard.c +++ b/hw/sd/milkymist-memcard.c @@ -151,11 +151,8 @@ static uint64_t memcard_read(void *opaque, hwaddr addr, if (!s->enabled) { r = 0xffffffff; } else { - r = 0; - r |= sdbus_read_byte(&s->sdbus) << 24; - r |= sdbus_read_byte(&s->sdbus) << 16; - r |= sdbus_read_byte(&s->sdbus) << 8; - r |= sdbus_read_byte(&s->sdbus); + sdbus_read_data(&s->sdbus, &r, sizeof(r)); + be32_to_cpus(&r); } break; case R_CLK2XDIV: diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index ddf3691561..1785d7e1f7 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -399,8 +399,6 @@ static void sdhci_end_transfer(SDHCIState *s) /* Fill host controller's read buffer with BLKSIZE bytes of data from card */ static void sdhci_read_block_from_card(SDHCIState *s) { - int index = 0; - uint8_t data; const uint16_t blk_size = s->blksize & BLOCK_SIZE_MASK; if ((s->trnmod & SDHC_TRNS_MULTI) && @@ -408,12 +406,9 @@ static void sdhci_read_block_from_card(SDHCIState *s) return; } - for (index = 0; index < blk_size; index++) { - data = sdbus_read_byte(&s->sdbus); - if (!FIELD_EX32(s->hostctl2, SDHC_HOSTCTL2, EXECUTE_TUNING)) { - /* Device is not in tuning */ - s->fifo_buffer[index] = data; - } + if (!FIELD_EX32(s->hostctl2, SDHC_HOSTCTL2, EXECUTE_TUNING)) { + /* Device is not in tuning */ + sdbus_read_data(&s->sdbus, s->fifo_buffer, blk_size); } if (FIELD_EX32(s->hostctl2, SDHC_HOSTCTL2, EXECUTE_TUNING)) { @@ -574,7 +569,7 @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t value, unsigned size) static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) { bool page_aligned = false; - unsigned int n, begin; + unsigned int begin; const uint16_t block_size = s->blksize & BLOCK_SIZE_MASK; uint32_t boundary_chk = 1 << (((s->blksize & ~BLOCK_SIZE_MASK) >> 12) + 12); uint32_t boundary_count = boundary_chk - (s->sdmasysad % boundary_chk); @@ -596,9 +591,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) SDHC_DAT_LINE_ACTIVE; while (s->blkcnt) { if (s->data_count == 0) { - for (n = 0; n < block_size; n++) { - s->fifo_buffer[n] = sdbus_read_byte(&s->sdbus); - } + sdbus_read_data(&s->sdbus, s->fifo_buffer, block_size); } begin = s->data_count; if (((boundary_count + begin) < block_size) && page_aligned) { @@ -662,13 +655,10 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) /* single block SDMA transfer */ static void sdhci_sdma_transfer_single_block(SDHCIState *s) { - int n; uint32_t datacnt = s->blksize & BLOCK_SIZE_MASK; if (s->trnmod & SDHC_TRNS_READ) { - for (n = 0; n < datacnt; n++) { - s->fifo_buffer[n] = sdbus_read_byte(&s->sdbus); - } + sdbus_read_data(&s->sdbus, s->fifo_buffer, datacnt); dma_memory_write(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt); } else { dma_memory_read(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt); @@ -731,7 +721,7 @@ static void get_adma_description(SDHCIState *s, ADMADescr *dscr) static void sdhci_do_adma(SDHCIState *s) { - unsigned int n, begin, length; + unsigned int begin, length; const uint16_t block_size = s->blksize & BLOCK_SIZE_MASK; ADMADescr dscr = {}; int i; @@ -765,9 +755,7 @@ static void sdhci_do_adma(SDHCIState *s) if (s->trnmod & SDHC_TRNS_READ) { while (length) { if (s->data_count == 0) { - for (n = 0; n < block_size; n++) { - s->fifo_buffer[n] = sdbus_read_byte(&s->sdbus); - } + sdbus_read_data(&s->sdbus, s->fifo_buffer, block_size); } begin = s->data_count; if ((length + begin) < block_size) { From b638627c723a8d0d2bb73489bc6bf9ff09b8d53a Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Fri, 21 Aug 2020 22:45:35 +0800 Subject: [PATCH 22/23] hw/sd: Fix incorrect populated function switch status data structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At present the function switch status data structure bit [399:376] are wrongly pupulated. These 3 bytes encode function switch status for the 6 function groups, with 4 bits per group, starting from function group 6 at bit 399, then followed by function group 5 at bit 395, and so on. However the codes mistakenly fills in the function group 1 status at bit 399. This fixes the code logic. Fixes: a1bb27b1e9 ("SD card emulation (initial implementation)") Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Tested-by: Sai Pavan Boddu Message-Id: <1598021136-49525-1-git-send-email-bmeng.cn@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 7c9d956f11..805e21fc88 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -807,11 +807,12 @@ static void sd_function_switch(SDState *sd, uint32_t arg) sd->data[11] = 0x43; sd->data[12] = 0x80; /* Supported group 1 functions */ sd->data[13] = 0x03; + for (i = 0; i < 6; i ++) { new_func = (arg >> (i * 4)) & 0x0f; if (mode && new_func != 0x0f) sd->function_group[i] = new_func; - sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4); + sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4); } memset(&sd->data[17], 0, 47); stw_be_p(sd->data + 64, sd_crc16(sd->data, 64)); From 6d2d4069c47e23b9e3913f9c8204fd0edcb99fb3 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Fri, 21 Aug 2020 22:45:36 +0800 Subject: [PATCH 23/23] hw/sd: Correct the maximum size of a Standard Capacity SD Memory Card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the SD spec, Standard Capacity SD Memory Card (SDSC) supports capacity up to and including 2 GiB. Fixes: 2d7adea4fe ("hw/sd: Support SDHC size cards") Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Tested-by: Sai Pavan Boddu Message-Id: <1598021136-49525-2-git-send-email-bmeng.cn@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 805e21fc88..483c4f1720 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -51,6 +51,8 @@ //#define DEBUG_SD 1 +#define SDSC_MAX_CAPACITY (2 * GiB) + typedef enum { sd_r0 = 0, /* no response */ sd_r1, /* normal response command */ @@ -314,7 +316,7 @@ static void sd_ocr_powerup(void *opaque) /* card power-up OK */ sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1); - if (sd->size > 1 * GiB) { + if (sd->size > SDSC_MAX_CAPACITY) { sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1); } } @@ -386,7 +388,7 @@ static void sd_set_csd(SDState *sd, uint64_t size) uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1; uint32_t wpsize = (1 << (WPGROUP_SHIFT + 1)) - 1; - if (size <= 1 * GiB) { /* Standard Capacity SD */ + if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */ sd->csd[0] = 0x00; /* CSD structure */ sd->csd[1] = 0x26; /* Data read access-time-1 */ sd->csd[2] = 0x00; /* Data read access-time-2 */