[PATCH 00/11] ppc/e500: Add support for two types of flash, cleanup

[PATCH 00/11] ppc/e500: Add support for two types of flash, cleanup

[Bernhard Beschow]

This series adds support for -pflash and direct SD card access to the
PPC e500 boards. The idea is to increase compatibility with "real" firmware
images where only the bare minimum of drivers is compiled in.

The series is structured as follows:

Patches 1-3 perform some general cleanup which paves the way for the rest of
the series.

Patches 4-7 add -pflash handling where memory-mapped flash can be added on
user's behalf. That is, the flash memory region is only added if the -pflash
argument is supplied. Note that the cfi01 device model becomes stricter in
checking the size of the emulated flash space.

Patches 8-11 add a new device model - the Freescale eSDHC - to the e500
boards which was missing so far.

User documentation is also added as the new features become available.

Tesing done:
* `qemu-system-ppc -M ppce500 -cpu e500mc -m 256 -kernel uImage -append
"console=ttyS0 rootwait root=/dev/mtdblock0 nokaslr" -drive
if=pflash,file=rootfs.ext2,format=raw`
* `qemu-system-ppc -M ppce500 -cpu e500mc -m 256 -kernel uImage -append
"console=ttyS0 rootwait root=/dev/mmcblk0" -device sd-card,drive=mydrive -drive
id=mydrive,if=none,file=rootfs.ext2,format=raw`

The load was created using latest Buildroot with `make
qemu_ppc_e500mc_defconfig` where the rootfs was configured to be of ext2 type.
In both cases it was possible to log in and explore the root file system.

Bernhard Beschow (11):
  hw/ppc/meson: Allow e500 boards to be enabled separately
  hw/gpio/meson: Introduce dedicated config switch for hw/gpio/mpc8xxx
  docs/system/ppc/ppce500: Add heading for networking chapter
  hw/ppc/mpc8544ds: Add platform bus
  hw/ppc/e500: Remove if statement which is now always true
  hw/block/pflash_cfi01: Error out if device length isn't a power of two
  hw/ppc/e500: Implement pflash handling
  hw/sd/sdhci-internal: Unexport ESDHC defines
  hw/sd/sdhci: Rename ESDHC_* defines to USDHC_*
  hw/sd/sdhci: Implement Freescale eSDHC device model
  hw/ppc/e500: Add Freescale eSDHC to e500 boards

 configs/devices/ppc-softmmu/default.mak |   3 +-
 docs/system/ppc/ppce500.rst             |  28 ++++
 hw/block/pflash_cfi01.c                 |   8 +-
 hw/gpio/Kconfig                         |   3 +
 hw/gpio/meson.build                     |   2 +-
 hw/ppc/Kconfig                          |  11 ++
 hw/ppc/e500.c                           | 116 +++++++++++--
 hw/ppc/e500.h                           |   1 -
 hw/ppc/e500plat.c                       |   1 -
 hw/ppc/meson.build                      |   6 +-
 hw/ppc/mpc8544ds.c                      |   5 +
 hw/sd/sdhci-internal.h                  |  20 ---
 hw/sd/sdhci.c                           | 212 +++++++++++++++++++++---
 include/hw/sd/sdhci.h                   |   3 +
 14 files changed, 349 insertions(+), 70 deletions(-)

-- 
2.37.3

[PATCH 01/11] hw/ppc/meson: Allow e500 boards to be enabled separately

[Bernhard Beschow]

Gives users more fine-grained control over what should be compiled into
QEMU.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 configs/devices/ppc-softmmu/default.mak | 3 ++-
 hw/ppc/Kconfig                          | 8 ++++++++
 hw/ppc/meson.build                      | 6 ++----
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/configs/devices/ppc-softmmu/default.mak b/configs/devices/ppc-softmmu/default.mak
index 658a454426..a887f5438b 100644
--- a/configs/devices/ppc-softmmu/default.mak
+++ b/configs/devices/ppc-softmmu/default.mak
@@ -1,7 +1,8 @@
 # Default configuration for ppc-softmmu
 
 # For embedded PPCs:
-CONFIG_E500=y
+CONFIG_E500PLAT=y
+CONFIG_MPC8544DS=y
 CONFIG_PPC405=y
 CONFIG_PPC440=y
 CONFIG_VIRTEX=y
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 3a4418a69e..22a64745d4 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -132,6 +132,14 @@ config E500
     select FDT_PPC
     select DS1338
 
+config E500PLAT
+    bool
+    select E500
+
+config MPC8544DS
+    bool
+    select E500
+
 config VIRTEX
     bool
     select PPC4XX
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index 62801923f3..32babc9b48 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -71,12 +71,10 @@ ppc_ss.add(when: 'CONFIG_MAC_OLDWORLD', if_true: files('mac_oldworld.c'))
 # NewWorld PowerMac
 ppc_ss.add(when: 'CONFIG_MAC_NEWWORLD', if_true: files('mac_newworld.c'))
 # e500
+ppc_ss.add(when: 'CONFIG_E500PLAT', if_true: files('e500plat.c'))
+ppc_ss.add(when: 'CONFIG_MPC8544DS', if_true: files('mpc8544ds.c'))
 ppc_ss.add(when: 'CONFIG_E500', if_true: files(
   'e500.c',
-  'mpc8544ds.c',
-  'e500plat.c'
-))
-ppc_ss.add(when: 'CONFIG_E500', if_true: files(
   'mpc8544_guts.c',
   'ppce500_spin.c'
 ))
-- 
2.37.3

[PATCH 02/11] hw/gpio/meson: Introduce dedicated config switch for hw/gpio/mpc8xxx

[Bernhard Beschow]

Having a dedicated config switch makes dependency handling cleaner.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/gpio/Kconfig     | 3 +++
 hw/gpio/meson.build | 2 +-
 hw/ppc/Kconfig      | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
index f0e7405f6e..d2cf3accc8 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -8,6 +8,9 @@ config PL061
 config GPIO_KEY
     bool
 
+config GPIO_MPC8XXX
+    bool
+
 config GPIO_PWR
     bool
 
diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
index 7bd6a57264..b726e6d27a 100644
--- a/hw/gpio/meson.build
+++ b/hw/gpio/meson.build
@@ -1,5 +1,5 @@
-softmmu_ss.add(when: 'CONFIG_E500', if_true: files('mpc8xxx.c'))
 softmmu_ss.add(when: 'CONFIG_GPIO_KEY', if_true: files('gpio_key.c'))
+softmmu_ss.add(when: 'CONFIG_GPIO_MPC8XXX', if_true: files('mpc8xxx.c'))
 softmmu_ss.add(when: 'CONFIG_GPIO_PWR', if_true: files('gpio_pwr.c'))
 softmmu_ss.add(when: 'CONFIG_MAX7310', if_true: files('max7310.c'))
 softmmu_ss.add(when: 'CONFIG_PL061', if_true: files('pl061.c'))
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 22a64745d4..791fe78a50 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -124,6 +124,7 @@ config E500
     imply AT24C
     imply VIRTIO_PCI
     select ETSEC
+    select GPIO_MPC8XXX
     select OPENPIC
     select PLATFORM_BUS
     select PPCE500_PCI
-- 
2.37.3

[PATCH 03/11] docs/system/ppc/ppce500: Add heading for networking chapter

[Bernhard Beschow]

The sudden change of topics is slightly confusing and makes the
networking information less visible. So separate the networking chapter
to improve comprehensibility.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 docs/system/ppc/ppce500.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
index 9beef39171..ba6bcb7314 100644
--- a/docs/system/ppc/ppce500.rst
+++ b/docs/system/ppc/ppce500.rst
@@ -146,6 +146,9 @@ You can specify a real world SoC device that QEMU has built-in support but all
 these SoCs are e500v2 based MPC85xx series, hence you cannot test anything
 built for P4080 (e500mc), P5020 (e5500) and T2080 (e6500).
 
+Networking
+----------
+
 By default a VirtIO standard PCI networking device is connected as an ethernet
 interface at PCI address 0.1.0, but we can switch that to an e1000 NIC by:
 
-- 
2.37.3

[PATCH 04/11] hw/ppc/mpc8544ds: Add platform bus

[Bernhard Beschow]

Models the real device more closely.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/mpc8544ds.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 81177505f0..cd6cd04bef 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -14,6 +14,7 @@
 #include "sysemu/device_tree.h"
 #include "hw/ppc/openpic.h"
 #include "qemu/error-report.h"
+#include "qemu/units.h"
 #include "cpu.h"
 
 static void mpc8544ds_fixup_devtree(void *fdt)
@@ -45,6 +46,11 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data)
     pmc->pci_nr_slots = 2;
     pmc->fixup_devtree = mpc8544ds_fixup_devtree;
     pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_20;
+    pmc->has_platform_bus = true;
+    pmc->platform_bus_base = 0xEC000000ULL;
+    pmc->platform_bus_size = 128 * MiB;
+    pmc->platform_bus_first_irq = 5;
+    pmc->platform_bus_num_irqs = 10;
     pmc->ccsrbar_base = 0xE0000000ULL;
     pmc->pci_mmio_base = 0xC0000000ULL;
     pmc->pci_mmio_bus_base = 0xC0000000ULL;
-- 
2.37.3

[PATCH 05/11] hw/ppc/e500: Remove if statement which is now always true

[Bernhard Beschow]

Now that the MPC8544DS board also has a platform bus, the if statement
was always true.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/e500.c      | 30 ++++++++++++++----------------
 hw/ppc/e500.h      |  1 -
 hw/ppc/e500plat.c  |  1 -
 hw/ppc/mpc8544ds.c |  1 -
 4 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 32495d0123..864b6f3d92 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -1007,25 +1007,23 @@ void ppce500_init(MachineState *machine)
     }
 
     /* Platform Bus Device */
-    if (pmc->has_platform_bus) {
-        dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE);
-        dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE);
-        qdev_prop_set_uint32(dev, "num_irqs", pmc->platform_bus_num_irqs);
-        qdev_prop_set_uint32(dev, "mmio_size", pmc->platform_bus_size);
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-        pms->pbus_dev = PLATFORM_BUS_DEVICE(dev);
-
-        s = SYS_BUS_DEVICE(pms->pbus_dev);
-        for (i = 0; i < pmc->platform_bus_num_irqs; i++) {
-            int irqn = pmc->platform_bus_first_irq + i;
-            sysbus_connect_irq(s, i, qdev_get_gpio_in(mpicdev, irqn));
-        }
+    dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE);
+    dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE);
+    qdev_prop_set_uint32(dev, "num_irqs", pmc->platform_bus_num_irqs);
+    qdev_prop_set_uint32(dev, "mmio_size", pmc->platform_bus_size);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    pms->pbus_dev = PLATFORM_BUS_DEVICE(dev);
 
-        memory_region_add_subregion(address_space_mem,
-                                    pmc->platform_bus_base,
-                                    sysbus_mmio_get_region(s, 0));
+    s = SYS_BUS_DEVICE(pms->pbus_dev);
+    for (i = 0; i < pmc->platform_bus_num_irqs; i++) {
+        int irqn = pmc->platform_bus_first_irq + i;
+        sysbus_connect_irq(s, i, qdev_get_gpio_in(mpicdev, irqn));
     }
 
+    memory_region_add_subregion(address_space_mem,
+                                pmc->platform_bus_base,
+                                sysbus_mmio_get_region(s, 0));
+
     /*
      * Smart firmware defaults ahead!
      *
diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
index 1e5853b032..68f754ce50 100644
--- a/hw/ppc/e500.h
+++ b/hw/ppc/e500.h
@@ -27,7 +27,6 @@ struct PPCE500MachineClass {
 
     int mpic_version;
     bool has_mpc8xxx_gpio;
-    bool has_platform_bus;
     hwaddr platform_bus_base;
     hwaddr platform_bus_size;
     int platform_bus_first_irq;
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index fc911bbb7b..5bb1c603da 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -86,7 +86,6 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data)
     pmc->fixup_devtree = e500plat_fixup_devtree;
     pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_42;
     pmc->has_mpc8xxx_gpio = true;
-    pmc->has_platform_bus = true;
     pmc->platform_bus_base = 0xf00000000ULL;
     pmc->platform_bus_size = 128 * MiB;
     pmc->platform_bus_first_irq = 5;
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index cd6cd04bef..4ca696b56a 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -46,7 +46,6 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data)
     pmc->pci_nr_slots = 2;
     pmc->fixup_devtree = mpc8544ds_fixup_devtree;
     pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_20;
-    pmc->has_platform_bus = true;
     pmc->platform_bus_base = 0xEC000000ULL;
     pmc->platform_bus_size = 128 * MiB;
     pmc->platform_bus_first_irq = 5;
-- 
2.37.3

[PATCH 06/11] hw/block/pflash_cfi01: Error out if device length isn't a power of two

[Bernhard Beschow]

According to the JEDEC standard the device length is communicated to an
OS as an exponent (power of two).

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/block/pflash_cfi01.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 0cbc2fb4cb..8c9b3f518a 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -690,7 +690,7 @@ static const MemoryRegionOps pflash_cfi01_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void pflash_cfi01_fill_cfi_table(PFlashCFI01 *pfl)
+static void pflash_cfi01_fill_cfi_table(PFlashCFI01 *pfl, Error **errp)
 {
     uint64_t blocks_per_device, sector_len_per_device, device_len;
     int num_devices;
@@ -708,6 +708,10 @@ static void pflash_cfi01_fill_cfi_table(PFlashCFI01 *pfl)
         sector_len_per_device = pfl->sector_len / num_devices;
     }
     device_len = sector_len_per_device * blocks_per_device;
+    if (ctpop64(device_len) != 1) {
+        error_setg(errp, "Device size must be a power of two.");
+        return;
+    }
 
     /* Hardcoded CFI table */
     /* Standard "QRY" string */
@@ -865,7 +869,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
      */
     pfl->cmd = 0x00;
     pfl->status = 0x80; /* WSM ready */
-    pflash_cfi01_fill_cfi_table(pfl);
+    pflash_cfi01_fill_cfi_table(pfl, errp);
 }
 
 static void pflash_cfi01_system_reset(DeviceState *dev)
-- 
2.37.3

[PATCH 07/11] hw/ppc/e500: Implement pflash handling

[Bernhard Beschow]

Allows e500 boards to have their root file system reside on flash using
only builtin devices.

Note that the flash memory area is only created when a -pflash argument is
given, and that the size is determined by the given file. The idea is to
put users into control.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 docs/system/ppc/ppce500.rst | 12 +++++++++
 hw/ppc/Kconfig              |  1 +
 hw/ppc/e500.c               | 54 +++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
index ba6bcb7314..c3f55c6f3d 100644
--- a/docs/system/ppc/ppce500.rst
+++ b/docs/system/ppc/ppce500.rst
@@ -119,6 +119,18 @@ To boot the 32-bit Linux kernel:
       -initrd /path/to/rootfs.cpio \
       -append "root=/dev/ram"
 
+Rather than using a root file system on ram disk, it is possible to have it on
+emulated flash. Given an ext2 image whose size must be a power of two, it can
+be used as follows:
+
+.. code-block:: bash
+
+  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \
+      -display none -serial stdio \
+      -kernel vmlinux \
+      -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
+      -append "rootwait root=/dev/mtdblock0"
+
 Running U-Boot
 --------------
 
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 791fe78a50..769a1ead1c 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -126,6 +126,7 @@ config E500
     select ETSEC
     select GPIO_MPC8XXX
     select OPENPIC
+    select PFLASH_CFI01
     select PLATFORM_BUS
     select PPCE500_PCI
     select SERIAL
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 864b6f3d92..7843a4e04b 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -23,8 +23,10 @@
 #include "e500-ccsr.h"
 #include "net/net.h"
 #include "qemu/config-file.h"
+#include "hw/block/flash.h"
 #include "hw/char/serial.h"
 #include "hw/pci/pci.h"
+#include "sysemu/block-backend-io.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
 #include "sysemu/reset.h"
@@ -267,6 +269,34 @@ static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
     }
 }
 
+static void create_devtree_flash(SysBusDevice *sbdev,
+                                 PlatformDevtreeData *data)
+{
+    char *name;
+    uint64_t num_blocks = object_property_get_uint(OBJECT(sbdev),
+                                                   "num-blocks",
+                                                   &error_fatal);
+    uint64_t sector_length = object_property_get_uint(OBJECT(sbdev),
+                                                      "sector-length",
+                                                      &error_fatal);
+    uint64_t bank_width = object_property_get_uint(OBJECT(sbdev),
+                                                   "width",
+                                                   &error_fatal);
+    hwaddr flashbase = 0;
+    hwaddr flashsize = num_blocks * sector_length;
+    void *fdt = data->fdt;
+
+    name = g_strdup_printf("%s/nor@%" PRIx64, data->node, flashbase);
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_cell(fdt, name, "#address-cells", 1);
+    qemu_fdt_setprop_cell(fdt, name, "#size-cells", 1);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "cfi-flash");
+    qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+                                 1, flashbase, 1, flashsize);
+    qemu_fdt_setprop_cell(fdt, name, "bank-width", bank_width);
+    g_free(name);
+}
+
 static void platform_bus_create_devtree(PPCE500MachineState *pms,
                                         void *fdt, const char *mpic)
 {
@@ -276,6 +306,8 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
     uint64_t addr = pmc->platform_bus_base;
     uint64_t size = pmc->platform_bus_size;
     int irq_start = pmc->platform_bus_first_irq;
+    SysBusDevice *sbdev;
+    bool ambiguous;
 
     /* Create a /platform node that we can put all devices into */
 
@@ -302,6 +334,13 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
     /* Loop through all dynamic sysbus devices and create nodes for them */
     foreach_dynamic_sysbus_device(sysbus_device_create_devtree, &data);
 
+    sbdev = SYS_BUS_DEVICE(object_resolve_path_type("", TYPE_PFLASH_CFI01,
+                                                    &ambiguous));
+    if (sbdev) {
+        assert(!ambiguous);
+        create_devtree_flash(sbdev, &data);
+    }
+
     g_free(node);
 }
 
@@ -856,6 +895,7 @@ void ppce500_init(MachineState *machine)
     unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4};
     IrqLines *irqs;
     DeviceState *dev, *mpicdev;
+    DriveInfo *dinfo;
     CPUPPCState *firstenv = NULL;
     MemoryRegion *ccsr_addr_space;
     SysBusDevice *s;
@@ -1024,6 +1064,20 @@ void ppce500_init(MachineState *machine)
                                 pmc->platform_bus_base,
                                 sysbus_mmio_get_region(s, 0));
 
+    dinfo = drive_get(IF_PFLASH, 0, 0);
+    if (dinfo) {
+        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
+        BlockDriverState *bs = blk_bs(blk);
+        uint64_t size = bdrv_getlength(bs);
+        if (ctpop64(size) != 1) {
+            error_report("Size of pflash file must be a power of two.");
+            exit(1);
+        }
+        pflash_cfi01_register(pmc->platform_bus_base, "e500.flash",
+                              size, blk,
+                              64 * KiB, 2, 0x89, 0x18, 0x0000, 0x0, 1);
+    }
+
     /*
      * Smart firmware defaults ahead!
      *
-- 
2.37.3

[PATCH 08/11] hw/sd/sdhci-internal: Unexport ESDHC defines

[Bernhard Beschow]

These defines aren't used outside of sdhci.c, so can be defined there.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/sd/sdhci-internal.h | 20 --------------------
 hw/sd/sdhci.c          | 19 +++++++++++++++++++
 2 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index e8c753d6d1..964570f8e8 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -288,26 +288,6 @@ enum {
 
 extern const VMStateDescription sdhci_vmstate;
 
-
-#define ESDHC_MIX_CTRL                  0x48
-
-#define ESDHC_VENDOR_SPEC               0xc0
-#define ESDHC_IMX_FRC_SDCLK_ON          (1 << 8)
-
-#define ESDHC_DLL_CTRL                  0x60
-
-#define ESDHC_TUNING_CTRL               0xcc
-#define ESDHC_TUNE_CTRL_STATUS          0x68
-#define ESDHC_WTMK_LVL                  0x44
-
-/* Undocumented register used by guests working around erratum ERR004536 */
-#define ESDHC_UNDOCUMENTED_REG27        0x6c
-
-#define ESDHC_CTRL_4BITBUS              (0x1 << 1)
-#define ESDHC_CTRL_8BITBUS              (0x2 << 1)
-
-#define ESDHC_PRNSTS_SDSTB              (1 << 3)
-
 /*
  * Default SD/MMC host controller features information, which will be
  * presented in CAPABILITIES register of generic SD host controller at reset.
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 0e5e988927..6da5e2c781 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1577,6 +1577,25 @@ static const TypeInfo sdhci_bus_info = {
 
 /* --- qdev i.MX eSDHC --- */
 
+#define ESDHC_MIX_CTRL                  0x48
+
+#define ESDHC_VENDOR_SPEC               0xc0
+#define ESDHC_IMX_FRC_SDCLK_ON          (1 << 8)
+
+#define ESDHC_DLL_CTRL                  0x60
+
+#define ESDHC_TUNING_CTRL               0xcc
+#define ESDHC_TUNE_CTRL_STATUS          0x68
+#define ESDHC_WTMK_LVL                  0x44
+
+/* Undocumented register used by guests working around erratum ERR004536 */
+#define ESDHC_UNDOCUMENTED_REG27        0x6c
+
+#define ESDHC_CTRL_4BITBUS              (0x1 << 1)
+#define ESDHC_CTRL_8BITBUS              (0x2 << 1)
+
+#define ESDHC_PRNSTS_SDSTB              (1 << 3)
+
 static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
 {
     SDHCIState *s = SYSBUS_SDHCI(opaque);
-- 
2.37.3

[PATCH 09/11] hw/sd/sdhci: Rename ESDHC_* defines to USDHC_*

[Bernhard Beschow]

The device model's functions start with "usdhc_", so rename the defines
accordingly for consistency.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/sd/sdhci.c | 68 +++++++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 6da5e2c781..7a5996caad 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1577,24 +1577,24 @@ static const TypeInfo sdhci_bus_info = {
 
 /* --- qdev i.MX eSDHC --- */
 
-#define ESDHC_MIX_CTRL                  0x48
+#define USDHC_MIX_CTRL                  0x48
 
-#define ESDHC_VENDOR_SPEC               0xc0
-#define ESDHC_IMX_FRC_SDCLK_ON          (1 << 8)
+#define USDHC_VENDOR_SPEC               0xc0
+#define USDHC_IMX_FRC_SDCLK_ON          (1 << 8)
 
-#define ESDHC_DLL_CTRL                  0x60
+#define USDHC_DLL_CTRL                  0x60
 
-#define ESDHC_TUNING_CTRL               0xcc
-#define ESDHC_TUNE_CTRL_STATUS          0x68
-#define ESDHC_WTMK_LVL                  0x44
+#define USDHC_TUNING_CTRL               0xcc
+#define USDHC_TUNE_CTRL_STATUS          0x68
+#define USDHC_WTMK_LVL                  0x44
 
 /* Undocumented register used by guests working around erratum ERR004536 */
-#define ESDHC_UNDOCUMENTED_REG27        0x6c
+#define USDHC_UNDOCUMENTED_REG27        0x6c
 
-#define ESDHC_CTRL_4BITBUS              (0x1 << 1)
-#define ESDHC_CTRL_8BITBUS              (0x2 << 1)
+#define USDHC_CTRL_4BITBUS              (0x1 << 1)
+#define USDHC_CTRL_8BITBUS              (0x2 << 1)
 
-#define ESDHC_PRNSTS_SDSTB              (1 << 3)
+#define USDHC_PRNSTS_SDSTB              (1 << 3)
 
 static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
 {
@@ -1615,11 +1615,11 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
         hostctl1 = SDHC_DMA_TYPE(s->hostctl1) << (8 - 3);
 
         if (s->hostctl1 & SDHC_CTRL_8BITBUS) {
-            hostctl1 |= ESDHC_CTRL_8BITBUS;
+            hostctl1 |= USDHC_CTRL_8BITBUS;
         }
 
         if (s->hostctl1 & SDHC_CTRL_4BITBUS) {
-            hostctl1 |= ESDHC_CTRL_4BITBUS;
+            hostctl1 |= USDHC_CTRL_4BITBUS;
         }
 
         ret  = hostctl1;
@@ -1630,21 +1630,21 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
 
     case SDHC_PRNSTS:
         /* Add SDSTB (SD Clock Stable) bit to PRNSTS */
-        ret = sdhci_read(opaque, offset, size) & ~ESDHC_PRNSTS_SDSTB;
+        ret = sdhci_read(opaque, offset, size) & ~USDHC_PRNSTS_SDSTB;
         if (s->clkcon & SDHC_CLOCK_INT_STABLE) {
-            ret |= ESDHC_PRNSTS_SDSTB;
+            ret |= USDHC_PRNSTS_SDSTB;
         }
         break;
 
-    case ESDHC_VENDOR_SPEC:
+    case USDHC_VENDOR_SPEC:
         ret = s->vendor_spec;
         break;
-    case ESDHC_DLL_CTRL:
-    case ESDHC_TUNE_CTRL_STATUS:
-    case ESDHC_UNDOCUMENTED_REG27:
-    case ESDHC_TUNING_CTRL:
-    case ESDHC_MIX_CTRL:
-    case ESDHC_WTMK_LVL:
+    case USDHC_DLL_CTRL:
+    case USDHC_TUNE_CTRL_STATUS:
+    case USDHC_UNDOCUMENTED_REG27:
+    case USDHC_TUNING_CTRL:
+    case USDHC_MIX_CTRL:
+    case USDHC_WTMK_LVL:
         ret = 0;
         break;
     }
@@ -1660,18 +1660,18 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
     uint32_t value = (uint32_t)val;
 
     switch (offset) {
-    case ESDHC_DLL_CTRL:
-    case ESDHC_TUNE_CTRL_STATUS:
-    case ESDHC_UNDOCUMENTED_REG27:
-    case ESDHC_TUNING_CTRL:
-    case ESDHC_WTMK_LVL:
+    case USDHC_DLL_CTRL:
+    case USDHC_TUNE_CTRL_STATUS:
+    case USDHC_UNDOCUMENTED_REG27:
+    case USDHC_TUNING_CTRL:
+    case USDHC_WTMK_LVL:
         break;
 
-    case ESDHC_VENDOR_SPEC:
+    case USDHC_VENDOR_SPEC:
         s->vendor_spec = value;
         switch (s->vendor) {
         case SDHCI_VENDOR_IMX:
-            if (value & ESDHC_IMX_FRC_SDCLK_ON) {
+            if (value & USDHC_IMX_FRC_SDCLK_ON) {
                 s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
             } else {
                 s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
@@ -1740,12 +1740,12 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
          * Second, split "Data Transfer Width" from bits 2 and 1 in to
          * bits 5 and 1
          */
-        if (value & ESDHC_CTRL_8BITBUS) {
+        if (value & USDHC_CTRL_8BITBUS) {
             hostctl1 |= SDHC_CTRL_8BITBUS;
         }
 
-        if (value & ESDHC_CTRL_4BITBUS) {
-            hostctl1 |= ESDHC_CTRL_4BITBUS;
+        if (value & USDHC_CTRL_4BITBUS) {
+            hostctl1 |= USDHC_CTRL_4BITBUS;
         }
 
         /*
@@ -1768,11 +1768,11 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
         sdhci_write(opaque, offset, value, size);
         break;
 
-    case ESDHC_MIX_CTRL:
+    case USDHC_MIX_CTRL:
         /*
          * So, when SD/MMC stack in Linux tries to write to "Transfer
          * Mode Register", ESDHC i.MX quirk code will translate it
-         * into a write to ESDHC_MIX_CTRL, so we do the opposite in
+         * into a write to USDHC_MIX_CTRL, so we do the opposite in
          * order to get where we started
          *
          * Note that Auto CMD23 Enable bit is located in a wrong place
-- 
2.37.3

[PATCH 10/11] hw/sd/sdhci: Implement Freescale eSDHC device model

[Bernhard Beschow]

Will allow e500 boards to access SD cards using just their own devices.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/sd/sdhci.c         | 147 +++++++++++++++++++++++++++++++++++++++++-
 include/hw/sd/sdhci.h |   3 +
 2 files changed, 149 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 7a5996caad..09285ccfa1 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1369,6 +1369,7 @@ void sdhci_initfn(SDHCIState *s)
     s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s);
 
     s->io_ops = &sdhci_mmio_ops;
+    s->io_registers_map_size = SDHC_REGISTERS_MAP_SIZE;
 }
 
 void sdhci_uninitfn(SDHCIState *s)
@@ -1392,7 +1393,7 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
     s->fifo_buffer = g_malloc0(s->buf_maxsz);
 
     memory_region_init_io(&s->iomem, OBJECT(s), s->io_ops, s, "sdhci",
-                          SDHC_REGISTERS_MAP_SIZE);
+                          s->io_registers_map_size);
 }
 
 void sdhci_common_unrealize(SDHCIState *s)
@@ -1575,6 +1576,149 @@ static const TypeInfo sdhci_bus_info = {
     .class_init = sdhci_bus_class_init,
 };
 
+/* --- qdev Freescale eSDHC --- */
+
+/* Host Controller Capabilities Register 2 */
+#define ESDHC_CAPABILITIES_1        0x114
+
+/* Control Register for DMA transfer */
+#define ESDHC_DMA_SYSCTL            0x40c
+#define ESDHC_PERIPHERAL_CLK_SEL    0x00080000
+#define ESDHC_FLUSH_ASYNC_FIFO      0x00040000
+#define ESDHC_DMA_SNOOP             0x00000040
+
+#define ESDHC_REGISTERS_MAP_SIZE    0x410
+
+static uint64_t esdhci_read(void *opaque, hwaddr offset, unsigned size)
+{
+    uint64_t ret;
+
+    if (size != 4) {
+        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
+                      " wrong size\n", size, offset);
+        return 0;
+    }
+
+    if (offset & 0x3) {
+        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
+                      " unaligned\n", size, offset);
+        return 0;
+    }
+
+    switch (offset) {
+    case SDHC_SYSAD:
+    case SDHC_BLKSIZE:
+    case SDHC_ARGUMENT:
+    case SDHC_TRNMOD:
+    case SDHC_RSPREG0:
+    case SDHC_RSPREG1:
+    case SDHC_RSPREG2:
+    case SDHC_RSPREG3:
+    case SDHC_BDATA:
+    case SDHC_PRNSTS:
+    case SDHC_HOSTCTL:
+    case SDHC_CLKCON:
+    case SDHC_NORINTSTS:
+    case SDHC_NORINTSTSEN:
+    case SDHC_NORINTSIGEN:
+    case SDHC_ACMD12ERRSTS:
+    case SDHC_CAPAB:
+    case SDHC_SLOT_INT_STATUS:
+        ret = sdhci_read(opaque, offset, size);
+        break;
+
+    case ESDHC_DMA_SYSCTL:
+    case 0x44:
+        ret = 0;
+        qemu_log_mask(LOG_UNIMP, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
+                      " not implemented\n", size, offset);
+        break;
+
+    default:
+        ret = 0;
+        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
+                      " unknown offset\n", size, offset);
+        break;
+    }
+
+    return ret;
+}
+
+static void esdhci_write(void *opaque, hwaddr offset, uint64_t val,
+                         unsigned size)
+{
+    if (size != 4) {
+        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
+                      " <- 0x%08lx wrong size\n", size, offset, val);
+        return;
+    }
+
+    if (offset & 0x3) {
+        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
+                      " <- 0x%08lx unaligned\n", size, offset, val);
+        return;
+    }
+
+    switch (offset) {
+    case SDHC_SYSAD:
+    case SDHC_BLKSIZE:
+    case SDHC_ARGUMENT:
+    case SDHC_TRNMOD:
+    case SDHC_BDATA:
+    case SDHC_HOSTCTL:
+    case SDHC_CLKCON:
+    case SDHC_NORINTSTS:
+    case SDHC_NORINTSTSEN:
+    case SDHC_NORINTSIGEN:
+    case SDHC_FEAER:
+        sdhci_write(opaque, offset, val, size);
+        break;
+
+    case ESDHC_DMA_SYSCTL:
+    case 0x44:
+        qemu_log_mask(LOG_UNIMP, "ESDHC wr_%ub @0x%02" HWADDR_PRIx " <- 0x%08lx "
+                      "not implemented\n", size, offset, val);
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
+                      " <- 0x%08lx unknown offset\n", size, offset, val);
+        break;
+    }
+}
+
+static const MemoryRegionOps esdhc_mmio_ops = {
+    .read = esdhci_read,
+    .write = esdhci_write,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+        .unaligned = false
+    },
+    .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void esdhci_init(Object *obj)
+{
+    DeviceState *dev = DEVICE(obj);
+    SDHCIState *s = SYSBUS_SDHCI(obj);
+
+    s->io_ops = &esdhc_mmio_ops;
+    s->io_registers_map_size = ESDHC_REGISTERS_MAP_SIZE;
+
+    /*
+     * Compatible with:
+     * - SD Host Controller Specification Version 2.0 Part A2
+     */
+    qdev_prop_set_uint8(dev, "sd-spec-version", 2);
+}
+
+static const TypeInfo esdhc_info = {
+    .name = TYPE_FSL_ESDHC,
+    .parent = TYPE_SYSBUS_SDHCI,
+    .instance_init = esdhci_init,
+};
+
 /* --- qdev i.MX eSDHC --- */
 
 #define USDHC_MIX_CTRL                  0x48
@@ -1907,6 +2051,7 @@ static void sdhci_register_types(void)
 {
     type_register_static(&sdhci_sysbus_info);
     type_register_static(&sdhci_bus_info);
+    type_register_static(&esdhc_info);
     type_register_static(&imx_usdhc_info);
     type_register_static(&sdhci_s3c_info);
 }
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index 01a64c5442..5b32e83eee 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -45,6 +45,7 @@ struct SDHCIState {
     AddressSpace *dma_as;
     MemoryRegion *dma_mr;
     const MemoryRegionOps *io_ops;
+    uint64_t io_registers_map_size;
 
     QEMUTimer *insert_timer;       /* timer for 'changing' sd card. */
     QEMUTimer *transfer_timer;
@@ -122,6 +123,8 @@ DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI,
 DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI,
                          TYPE_SYSBUS_SDHCI)
 
+#define TYPE_FSL_ESDHC "fsl-esdhc"
+
 #define TYPE_IMX_USDHC "imx-usdhc"
 
 #define TYPE_S3C_SDHCI "s3c-sdhci"
-- 
2.37.3

[PATCH 11/11] hw/ppc/e500: Add Freescale eSDHC to e500 boards

[Bernhard Beschow]

Adds missing functionality to emulated e500 SOCs which increases the
chance of given "real" firmware images to access SD cards.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 docs/system/ppc/ppce500.rst | 13 +++++++++++++
 hw/ppc/Kconfig              |  1 +
 hw/ppc/e500.c               | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
index c3f55c6f3d..50b199c8f3 100644
--- a/docs/system/ppc/ppce500.rst
+++ b/docs/system/ppc/ppce500.rst
@@ -19,6 +19,7 @@ The ``ppce500`` machine supports the following devices:
 * Power-off functionality via one GPIO pin
 * 1 Freescale MPC8xxx PCI host controller
 * VirtIO devices via PCI bus
+* 1 Freescale Enhanced Secure Digital Host controller (eSDHC)
 * 1 Freescale Enhanced Triple Speed Ethernet controller (eTSEC)
 
 Hardware configuration information
@@ -131,6 +132,18 @@ be used as follows:
       -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
       -append "rootwait root=/dev/mtdblock0"
 
+Alternatively, the root file system can also reside on an emulated SD card
+whose size must again be a power of two:
+
+.. code-block:: bash
+
+  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \
+      -display none -serial stdio \
+      -kernel vmlinux \
+      -device sd-card,drive=mydrive \
+      -drive id=mydrive,if=none,file=/path/to/rootfs.ext2,format=raw \
+      -append "rootwait root=/dev/mmcblk0"
+
 Running U-Boot
 --------------
 
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 769a1ead1c..6e31f568ba 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -129,6 +129,7 @@ config E500
     select PFLASH_CFI01
     select PLATFORM_BUS
     select PPCE500_PCI
+    select SDHCI
     select SERIAL
     select MPC_I2C
     select FDT_PPC
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 7843a4e04b..87a03fd4a9 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -48,6 +48,7 @@
 #include "hw/net/fsl_etsec/etsec.h"
 #include "hw/i2c/i2c.h"
 #include "hw/irq.h"
+#include "hw/sd/sdhci.h"
 
 #define EPAPR_MAGIC                (0x45504150)
 #define DTC_LOAD_PAD               0x1800000
@@ -66,11 +67,14 @@
 #define MPC8544_SERIAL1_REGS_OFFSET 0x4600ULL
 #define MPC8544_PCI_REGS_OFFSET    0x8000ULL
 #define MPC8544_PCI_REGS_SIZE      0x1000ULL
+#define MPC85XX_ESDHC_REGS_OFFSET  0x2e000ULL
+#define MPC85XX_ESDHC_REGS_SIZE    0x1000ULL
 #define MPC8544_UTIL_OFFSET        0xe0000ULL
 #define MPC8XXX_GPIO_OFFSET        0x000FF000ULL
 #define MPC8544_I2C_REGS_OFFSET    0x3000ULL
 #define MPC8XXX_GPIO_IRQ           47
 #define MPC8544_I2C_IRQ            43
+#define MPC85XX_ESDHC_IRQ          72
 #define RTC_REGS_OFFSET            0x68
 
 #define PLATFORM_CLK_FREQ_HZ       (400 * 1000 * 1000)
@@ -203,6 +207,25 @@ static void dt_i2c_create(void *fdt, const char *soc, const char *mpic,
     g_free(i2c);
 }
 
+static void dt_sdhc_create(void *fdt, const char *parent, const char *mpic)
+{
+    hwaddr mmio = MPC85XX_ESDHC_REGS_OFFSET;
+    hwaddr size = MPC85XX_ESDHC_REGS_SIZE;
+    int irq = MPC85XX_ESDHC_IRQ;
+    char *name;
+
+    name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
+    qemu_fdt_add_subnode(fdt, name);
+    /* qemu_fdt_setprop_cells(fdt, name, "voltage-ranges", 3300, 3300); */
+    qemu_fdt_setprop_cells(fdt, name, "clock-frequency", 167000000);
+    qemu_fdt_setprop(fdt, name, "sdhci,auto-cmd12", NULL, 0);
+    qemu_fdt_setprop_phandle(fdt, name, "interrupt-parent", mpic);
+    qemu_fdt_setprop_cells(fdt, name, "bus-width", 4);
+    qemu_fdt_setprop_cells(fdt, name, "interrupts", irq, 0x2);
+    qemu_fdt_setprop_cells(fdt, name, "reg", mmio, size);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "fsl,esdhc");
+    g_free(name);
+}
 
 typedef struct PlatformDevtreeData {
     void *fdt;
@@ -556,6 +579,8 @@ static int ppce500_load_device_tree(PPCE500MachineState *pms,
 
     dt_rtc_create(fdt, "i2c", "rtc");
 
+    /* sdhc */
+    dt_sdhc_create(fdt, soc, mpic);
 
     gutil = g_strdup_printf("%s/global-utilities@%llx", soc,
                             MPC8544_UTIL_OFFSET);
@@ -996,6 +1021,13 @@ void ppce500_init(MachineState *machine)
     i2c_slave_create_simple(i2c, "ds1338", RTC_REGS_OFFSET);
 
 
+    /* eSDHC */
+    dev = qdev_new(TYPE_FSL_ESDHC);
+    s = SYS_BUS_DEVICE(dev);
+    sysbus_realize_and_unref(s, &error_fatal);
+    sysbus_mmio_map(s, 0, pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET);
+    sysbus_connect_irq(s, 0, qdev_get_gpio_in(mpicdev, MPC85XX_ESDHC_IRQ));
+
     /* General Utility device */
     dev = qdev_new("mpc8544-guts");
     s = SYS_BUS_DEVICE(dev);
-- 
2.37.3

Re: [PATCH 01/11] hw/ppc/meson: Allow e500 boards to be enabled separately

[Bin Meng]

On Thu, Sep 15, 2022 at 11:25 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Gives users more fine-grained control over what should be compiled into
> QEMU.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  configs/devices/ppc-softmmu/default.mak | 3 ++-
>  hw/ppc/Kconfig                          | 8 ++++++++
>  hw/ppc/meson.build                      | 6 ++----
>  3 files changed, 12 insertions(+), 5 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH 02/11] hw/gpio/meson: Introduce dedicated config switch for hw/gpio/mpc8xxx

[Bin Meng]

On Thu, Sep 15, 2022 at 11:26 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Having a dedicated config switch makes dependency handling cleaner.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/gpio/Kconfig     | 3 +++
>  hw/gpio/meson.build | 2 +-
>  hw/ppc/Kconfig      | 1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH 03/11] docs/system/ppc/ppce500: Add heading for networking chapter

[Bin Meng]

On Thu, Sep 15, 2022 at 11:29 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> The sudden change of topics is slightly confusing and makes the
> networking information less visible. So separate the networking chapter
> to improve comprehensibility.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  docs/system/ppc/ppce500.rst | 3 +++
>  1 file changed, 3 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH 04/11] hw/ppc/mpc8544ds: Add platform bus

[Bin Meng]

On Thu, Sep 15, 2022 at 11:29 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Models the real device more closely.

Please describe the source (e.g.: I assume it's MPC8544DS board manual
or something like that?) that describe such memory map for the
platform bus.

Is this the eLBC bus range that includes the NOR flash device?

>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/ppc/mpc8544ds.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
> index 81177505f0..cd6cd04bef 100644
> --- a/hw/ppc/mpc8544ds.c
> +++ b/hw/ppc/mpc8544ds.c
> @@ -14,6 +14,7 @@
>  #include "sysemu/device_tree.h"
>  #include "hw/ppc/openpic.h"
>  #include "qemu/error-report.h"
> +#include "qemu/units.h"
>  #include "cpu.h"
>
>  static void mpc8544ds_fixup_devtree(void *fdt)
> @@ -45,6 +46,11 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data)
>      pmc->pci_nr_slots = 2;
>      pmc->fixup_devtree = mpc8544ds_fixup_devtree;
>      pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_20;
> +    pmc->has_platform_bus = true;
> +    pmc->platform_bus_base = 0xEC000000ULL;
> +    pmc->platform_bus_size = 128 * MiB;
> +    pmc->platform_bus_first_irq = 5;
> +    pmc->platform_bus_num_irqs = 10;
>      pmc->ccsrbar_base = 0xE0000000ULL;
>      pmc->pci_mmio_base = 0xC0000000ULL;
>      pmc->pci_mmio_bus_base = 0xC0000000ULL;
> --

Regards,
Bin

Re: [PATCH 05/11] hw/ppc/e500: Remove if statement which is now always true

[Bin Meng]

On Thu, Sep 15, 2022 at 11:34 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Now that the MPC8544DS board also has a platform bus, the if statement
> was always true.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/ppc/e500.c      | 30 ++++++++++++++----------------
>  hw/ppc/e500.h      |  1 -
>  hw/ppc/e500plat.c  |  1 -
>  hw/ppc/mpc8544ds.c |  1 -
>  4 files changed, 14 insertions(+), 19 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH 06/11] hw/block/pflash_cfi01: Error out if device length isn't a power of two

[Bin Meng]

On Thu, Sep 15, 2022 at 11:26 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> According to the JEDEC standard the device length is communicated to an
> OS as an exponent (power of two).
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/block/pflash_cfi01.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH 08/11] hw/sd/sdhci-internal: Unexport ESDHC defines

[Bin Meng]

On Thu, Sep 15, 2022 at 11:39 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> These defines aren't used outside of sdhci.c, so can be defined there.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/sd/sdhci-internal.h | 20 --------------------
>  hw/sd/sdhci.c          | 19 +++++++++++++++++++
>  2 files changed, 19 insertions(+), 20 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH 09/11] hw/sd/sdhci: Rename ESDHC_* defines to USDHC_*

[Bin Meng]

On Thu, Sep 15, 2022 at 11:42 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> The device model's functions start with "usdhc_", so rename the defines
> accordingly for consistency.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/sd/sdhci.c | 68 +++++++++++++++++++++++++--------------------------
>  1 file changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 6da5e2c781..7a5996caad 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1577,24 +1577,24 @@ static const TypeInfo sdhci_bus_info = {
>
>  /* --- qdev i.MX eSDHC --- */
>
> -#define ESDHC_MIX_CTRL                  0x48
> +#define USDHC_MIX_CTRL                  0x48
>
> -#define ESDHC_VENDOR_SPEC               0xc0
> -#define ESDHC_IMX_FRC_SDCLK_ON          (1 << 8)
> +#define USDHC_VENDOR_SPEC               0xc0
> +#define USDHC_IMX_FRC_SDCLK_ON          (1 << 8)
>
> -#define ESDHC_DLL_CTRL                  0x60
> +#define USDHC_DLL_CTRL                  0x60
>
> -#define ESDHC_TUNING_CTRL               0xcc
> -#define ESDHC_TUNE_CTRL_STATUS          0x68
> -#define ESDHC_WTMK_LVL                  0x44
> +#define USDHC_TUNING_CTRL               0xcc
> +#define USDHC_TUNE_CTRL_STATUS          0x68
> +#define USDHC_WTMK_LVL                  0x44
>
>  /* Undocumented register used by guests working around erratum ERR004536 */
> -#define ESDHC_UNDOCUMENTED_REG27        0x6c
> +#define USDHC_UNDOCUMENTED_REG27        0x6c
>
> -#define ESDHC_CTRL_4BITBUS              (0x1 << 1)
> -#define ESDHC_CTRL_8BITBUS              (0x2 << 1)
> +#define USDHC_CTRL_4BITBUS              (0x1 << 1)
> +#define USDHC_CTRL_8BITBUS              (0x2 << 1)
>
> -#define ESDHC_PRNSTS_SDSTB              (1 << 3)
> +#define USDHC_PRNSTS_SDSTB              (1 << 3)
>
>  static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
>  {
> @@ -1615,11 +1615,11 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
>          hostctl1 = SDHC_DMA_TYPE(s->hostctl1) << (8 - 3);
>
>          if (s->hostctl1 & SDHC_CTRL_8BITBUS) {
> -            hostctl1 |= ESDHC_CTRL_8BITBUS;
> +            hostctl1 |= USDHC_CTRL_8BITBUS;
>          }
>
>          if (s->hostctl1 & SDHC_CTRL_4BITBUS) {
> -            hostctl1 |= ESDHC_CTRL_4BITBUS;
> +            hostctl1 |= USDHC_CTRL_4BITBUS;
>          }
>
>          ret  = hostctl1;
> @@ -1630,21 +1630,21 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
>
>      case SDHC_PRNSTS:
>          /* Add SDSTB (SD Clock Stable) bit to PRNSTS */
> -        ret = sdhci_read(opaque, offset, size) & ~ESDHC_PRNSTS_SDSTB;
> +        ret = sdhci_read(opaque, offset, size) & ~USDHC_PRNSTS_SDSTB;
>          if (s->clkcon & SDHC_CLOCK_INT_STABLE) {
> -            ret |= ESDHC_PRNSTS_SDSTB;
> +            ret |= USDHC_PRNSTS_SDSTB;
>          }
>          break;
>
> -    case ESDHC_VENDOR_SPEC:
> +    case USDHC_VENDOR_SPEC:
>          ret = s->vendor_spec;
>          break;
> -    case ESDHC_DLL_CTRL:
> -    case ESDHC_TUNE_CTRL_STATUS:
> -    case ESDHC_UNDOCUMENTED_REG27:
> -    case ESDHC_TUNING_CTRL:
> -    case ESDHC_MIX_CTRL:
> -    case ESDHC_WTMK_LVL:
> +    case USDHC_DLL_CTRL:
> +    case USDHC_TUNE_CTRL_STATUS:
> +    case USDHC_UNDOCUMENTED_REG27:
> +    case USDHC_TUNING_CTRL:
> +    case USDHC_MIX_CTRL:
> +    case USDHC_WTMK_LVL:
>          ret = 0;
>          break;
>      }
> @@ -1660,18 +1660,18 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>      uint32_t value = (uint32_t)val;
>
>      switch (offset) {
> -    case ESDHC_DLL_CTRL:
> -    case ESDHC_TUNE_CTRL_STATUS:
> -    case ESDHC_UNDOCUMENTED_REG27:
> -    case ESDHC_TUNING_CTRL:
> -    case ESDHC_WTMK_LVL:
> +    case USDHC_DLL_CTRL:
> +    case USDHC_TUNE_CTRL_STATUS:
> +    case USDHC_UNDOCUMENTED_REG27:
> +    case USDHC_TUNING_CTRL:
> +    case USDHC_WTMK_LVL:
>          break;
>
> -    case ESDHC_VENDOR_SPEC:
> +    case USDHC_VENDOR_SPEC:
>          s->vendor_spec = value;
>          switch (s->vendor) {
>          case SDHCI_VENDOR_IMX:
> -            if (value & ESDHC_IMX_FRC_SDCLK_ON) {
> +            if (value & USDHC_IMX_FRC_SDCLK_ON) {
>                  s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
>              } else {
>                  s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
> @@ -1740,12 +1740,12 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>           * Second, split "Data Transfer Width" from bits 2 and 1 in to
>           * bits 5 and 1
>           */
> -        if (value & ESDHC_CTRL_8BITBUS) {
> +        if (value & USDHC_CTRL_8BITBUS) {
>              hostctl1 |= SDHC_CTRL_8BITBUS;
>          }
>
> -        if (value & ESDHC_CTRL_4BITBUS) {
> -            hostctl1 |= ESDHC_CTRL_4BITBUS;
> +        if (value & USDHC_CTRL_4BITBUS) {
> +            hostctl1 |= USDHC_CTRL_4BITBUS;
>          }
>
>          /*
> @@ -1768,11 +1768,11 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>          sdhci_write(opaque, offset, value, size);
>          break;
>
> -    case ESDHC_MIX_CTRL:
> +    case USDHC_MIX_CTRL:
>          /*
>           * So, when SD/MMC stack in Linux tries to write to "Transfer
>           * Mode Register", ESDHC i.MX quirk code will translate it

Here I assume ESDHC i.MX means the Linux eSDHC driver for i.MX, so no
need to replace ESDHC with USDHC?

> -         * into a write to ESDHC_MIX_CTRL, so we do the opposite in
> +         * into a write to USDHC_MIX_CTRL, so we do the opposite in
>           * order to get where we started
>           *
>           * Note that Auto CMD23 Enable bit is located in a wrong place
> --

Overall LGTM:
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH 07/11] hw/ppc/e500: Implement pflash handling

[Bin Meng]

On Thu, Sep 15, 2022 at 11:36 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Allows e500 boards to have their root file system reside on flash using
> only builtin devices.
>
> Note that the flash memory area is only created when a -pflash argument is
> given, and that the size is determined by the given file. The idea is to
> put users into control.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  docs/system/ppc/ppce500.rst | 12 +++++++++
>  hw/ppc/Kconfig              |  1 +
>  hw/ppc/e500.c               | 54 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+)
>
> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
> index ba6bcb7314..c3f55c6f3d 100644
> --- a/docs/system/ppc/ppce500.rst
> +++ b/docs/system/ppc/ppce500.rst
> @@ -119,6 +119,18 @@ To boot the 32-bit Linux kernel:
>        -initrd /path/to/rootfs.cpio \
>        -append "root=/dev/ram"
>
> +Rather than using a root file system on ram disk, it is possible to have it on
> +emulated flash. Given an ext2 image whose size must be a power of two, it can
> +be used as follows:
> +
> +.. code-block:: bash
> +
> +  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \

qemu-system-ppc{64|32}

> +      -display none -serial stdio \
> +      -kernel vmlinux \
> +      -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
> +      -append "rootwait root=/dev/mtdblock0"
> +
>  Running U-Boot
>  --------------
>
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 791fe78a50..769a1ead1c 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -126,6 +126,7 @@ config E500
>      select ETSEC
>      select GPIO_MPC8XXX
>      select OPENPIC
> +    select PFLASH_CFI01
>      select PLATFORM_BUS
>      select PPCE500_PCI
>      select SERIAL
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 864b6f3d92..7843a4e04b 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -23,8 +23,10 @@
>  #include "e500-ccsr.h"
>  #include "net/net.h"
>  #include "qemu/config-file.h"
> +#include "hw/block/flash.h"
>  #include "hw/char/serial.h"
>  #include "hw/pci/pci.h"
> +#include "sysemu/block-backend-io.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/kvm.h"
>  #include "sysemu/reset.h"
> @@ -267,6 +269,34 @@ static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
>      }
>  }
>
> +static void create_devtree_flash(SysBusDevice *sbdev,
> +                                 PlatformDevtreeData *data)
> +{
> +    char *name;

Use g_autofree

> +    uint64_t num_blocks = object_property_get_uint(OBJECT(sbdev),
> +                                                   "num-blocks",
> +                                                   &error_fatal);
> +    uint64_t sector_length = object_property_get_uint(OBJECT(sbdev),
> +                                                      "sector-length",
> +                                                      &error_fatal);
> +    uint64_t bank_width = object_property_get_uint(OBJECT(sbdev),
> +                                                   "width",
> +                                                   &error_fatal);
> +    hwaddr flashbase = 0;
> +    hwaddr flashsize = num_blocks * sector_length;
> +    void *fdt = data->fdt;
> +
> +    name = g_strdup_printf("%s/nor@%" PRIx64, data->node, flashbase);
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_cell(fdt, name, "#address-cells", 1);
> +    qemu_fdt_setprop_cell(fdt, name, "#size-cells", 1);

#address-cells and #size-cells are not needed.

> +    qemu_fdt_setprop_string(fdt, name, "compatible", "cfi-flash");
> +    qemu_fdt_setprop_sized_cells(fdt, name, "reg",
> +                                 1, flashbase, 1, flashsize);
> +    qemu_fdt_setprop_cell(fdt, name, "bank-width", bank_width);
> +    g_free(name);
> +}
> +
>  static void platform_bus_create_devtree(PPCE500MachineState *pms,
>                                          void *fdt, const char *mpic)
>  {
> @@ -276,6 +306,8 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>      uint64_t addr = pmc->platform_bus_base;
>      uint64_t size = pmc->platform_bus_size;
>      int irq_start = pmc->platform_bus_first_irq;
> +    SysBusDevice *sbdev;
> +    bool ambiguous;
>
>      /* Create a /platform node that we can put all devices into */
>
> @@ -302,6 +334,13 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>      /* Loop through all dynamic sysbus devices and create nodes for them */
>      foreach_dynamic_sysbus_device(sysbus_device_create_devtree, &data);
>
> +    sbdev = SYS_BUS_DEVICE(object_resolve_path_type("", TYPE_PFLASH_CFI01,
> +                                                    &ambiguous));

Can this be moved into sysbus_device_create_devtree(), and use the
same logic as the eTSEC device?

> +    if (sbdev) {
> +        assert(!ambiguous);
> +        create_devtree_flash(sbdev, &data);
> +    }
> +
>      g_free(node);
>  }
>
> @@ -856,6 +895,7 @@ void ppce500_init(MachineState *machine)
>      unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4};
>      IrqLines *irqs;
>      DeviceState *dev, *mpicdev;
> +    DriveInfo *dinfo;
>      CPUPPCState *firstenv = NULL;
>      MemoryRegion *ccsr_addr_space;
>      SysBusDevice *s;
> @@ -1024,6 +1064,20 @@ void ppce500_init(MachineState *machine)
>                                  pmc->platform_bus_base,
>                                  sysbus_mmio_get_region(s, 0));
>
> +    dinfo = drive_get(IF_PFLASH, 0, 0);
> +    if (dinfo) {
> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> +        BlockDriverState *bs = blk_bs(blk);
> +        uint64_t size = bdrv_getlength(bs);
> +        if (ctpop64(size) != 1) {
> +            error_report("Size of pflash file must be a power of two.");
> +            exit(1);
> +        }

I think we should also check whether the flash size plus the eTSEC
size exceeds the platform bus mmio window size otherwise it won't work
for both devices present, no?

> +        pflash_cfi01_register(pmc->platform_bus_base, "e500.flash",
> +                              size, blk,
> +                              64 * KiB, 2, 0x89, 0x18, 0x0000, 0x0, 1);
> +    }
> +
>      /*
>       * Smart firmware defaults ahead!
>       *

Regards,
Bin

Re: [PATCH 10/11] hw/sd/sdhci: Implement Freescale eSDHC device model

[Bin Meng]

On Thu, Sep 15, 2022 at 11:30 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Will allow e500 boards to access SD cards using just their own devices.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/sd/sdhci.c         | 147 +++++++++++++++++++++++++++++++++++++++++-
>  include/hw/sd/sdhci.h |   3 +
>  2 files changed, 149 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 7a5996caad..09285ccfa1 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1369,6 +1369,7 @@ void sdhci_initfn(SDHCIState *s)
>      s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s);
>
>      s->io_ops = &sdhci_mmio_ops;
> +    s->io_registers_map_size = SDHC_REGISTERS_MAP_SIZE;
>  }
>
>  void sdhci_uninitfn(SDHCIState *s)
> @@ -1392,7 +1393,7 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
>      s->fifo_buffer = g_malloc0(s->buf_maxsz);
>
>      memory_region_init_io(&s->iomem, OBJECT(s), s->io_ops, s, "sdhci",
> -                          SDHC_REGISTERS_MAP_SIZE);
> +                          s->io_registers_map_size);
>  }
>
>  void sdhci_common_unrealize(SDHCIState *s)
> @@ -1575,6 +1576,149 @@ static const TypeInfo sdhci_bus_info = {
>      .class_init = sdhci_bus_class_init,
>  };
>
> +/* --- qdev Freescale eSDHC --- */
> +
> +/* Host Controller Capabilities Register 2 */
> +#define ESDHC_CAPABILITIES_1        0x114
> +
> +/* Control Register for DMA transfer */
> +#define ESDHC_DMA_SYSCTL            0x40c
> +#define ESDHC_PERIPHERAL_CLK_SEL    0x00080000
> +#define ESDHC_FLUSH_ASYNC_FIFO      0x00040000
> +#define ESDHC_DMA_SNOOP             0x00000040

It looks the above 3 bit fields are not used?

> +
> +#define ESDHC_REGISTERS_MAP_SIZE    0x410
> +
> +static uint64_t esdhci_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    uint64_t ret;
> +
> +    if (size != 4) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
> +                      " wrong size\n", size, offset);
> +        return 0;
> +    }
> +
> +    if (offset & 0x3) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
> +                      " unaligned\n", size, offset);
> +        return 0;
> +    }
> +
> +    switch (offset) {
> +    case SDHC_SYSAD:
> +    case SDHC_BLKSIZE:
> +    case SDHC_ARGUMENT:
> +    case SDHC_TRNMOD:
> +    case SDHC_RSPREG0:
> +    case SDHC_RSPREG1:
> +    case SDHC_RSPREG2:
> +    case SDHC_RSPREG3:
> +    case SDHC_BDATA:
> +    case SDHC_PRNSTS:
> +    case SDHC_HOSTCTL:
> +    case SDHC_CLKCON:
> +    case SDHC_NORINTSTS:
> +    case SDHC_NORINTSTSEN:
> +    case SDHC_NORINTSIGEN:
> +    case SDHC_ACMD12ERRSTS:
> +    case SDHC_CAPAB:
> +    case SDHC_SLOT_INT_STATUS:
> +        ret = sdhci_read(opaque, offset, size);
> +        break;
> +
> +    case ESDHC_DMA_SYSCTL:
> +    case 0x44:

Can we define a macro for this offset?

> +        ret = 0;
> +        qemu_log_mask(LOG_UNIMP, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
> +                      " not implemented\n", size, offset);
> +        break;
> +
> +    default:
> +        ret = 0;
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
> +                      " unknown offset\n", size, offset);
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +static void esdhci_write(void *opaque, hwaddr offset, uint64_t val,
> +                         unsigned size)
> +{
> +    if (size != 4) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
> +                      " <- 0x%08lx wrong size\n", size, offset, val);
> +        return;
> +    }
> +
> +    if (offset & 0x3) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
> +                      " <- 0x%08lx unaligned\n", size, offset, val);
> +        return;
> +    }
> +
> +    switch (offset) {
> +    case SDHC_SYSAD:
> +    case SDHC_BLKSIZE:
> +    case SDHC_ARGUMENT:
> +    case SDHC_TRNMOD:
> +    case SDHC_BDATA:
> +    case SDHC_HOSTCTL:
> +    case SDHC_CLKCON:
> +    case SDHC_NORINTSTS:
> +    case SDHC_NORINTSTSEN:
> +    case SDHC_NORINTSIGEN:
> +    case SDHC_FEAER:
> +        sdhci_write(opaque, offset, val, size);
> +        break;
> +
> +    case ESDHC_DMA_SYSCTL:
> +    case 0x44:

ditto

> +        qemu_log_mask(LOG_UNIMP, "ESDHC wr_%ub @0x%02" HWADDR_PRIx " <- 0x%08lx "
> +                      "not implemented\n", size, offset, val);
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
> +                      " <- 0x%08lx unknown offset\n", size, offset, val);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps esdhc_mmio_ops = {
> +    .read = esdhci_read,
> +    .write = esdhci_write,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 4,
> +        .unaligned = false
> +    },
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static void esdhci_init(Object *obj)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    SDHCIState *s = SYSBUS_SDHCI(obj);
> +
> +    s->io_ops = &esdhc_mmio_ops;
> +    s->io_registers_map_size = ESDHC_REGISTERS_MAP_SIZE;
> +
> +    /*
> +     * Compatible with:
> +     * - SD Host Controller Specification Version 2.0 Part A2
> +     */
> +    qdev_prop_set_uint8(dev, "sd-spec-version", 2);
> +}
> +
> +static const TypeInfo esdhc_info = {
> +    .name = TYPE_FSL_ESDHC,
> +    .parent = TYPE_SYSBUS_SDHCI,
> +    .instance_init = esdhci_init,
> +};
> +
>  /* --- qdev i.MX eSDHC --- */
>
>  #define USDHC_MIX_CTRL                  0x48
> @@ -1907,6 +2051,7 @@ static void sdhci_register_types(void)
>  {
>      type_register_static(&sdhci_sysbus_info);
>      type_register_static(&sdhci_bus_info);
> +    type_register_static(&esdhc_info);
>      type_register_static(&imx_usdhc_info);
>      type_register_static(&sdhci_s3c_info);
>  }
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index 01a64c5442..5b32e83eee 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -45,6 +45,7 @@ struct SDHCIState {
>      AddressSpace *dma_as;
>      MemoryRegion *dma_mr;
>      const MemoryRegionOps *io_ops;
> +    uint64_t io_registers_map_size;
>
>      QEMUTimer *insert_timer;       /* timer for 'changing' sd card. */
>      QEMUTimer *transfer_timer;
> @@ -122,6 +123,8 @@ DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI,
>  DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI,
>                           TYPE_SYSBUS_SDHCI)
>
> +#define TYPE_FSL_ESDHC "fsl-esdhc"
> +
>  #define TYPE_IMX_USDHC "imx-usdhc"
>
>  #define TYPE_S3C_SDHCI "s3c-sdhci"
> --

Regards,
Bin

Re: [PATCH 11/11] hw/ppc/e500: Add Freescale eSDHC to e500 boards

[Bin Meng]

On Thu, Sep 15, 2022 at 11:30 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Adds missing functionality to emulated e500 SOCs which increases the
> chance of given "real" firmware images to access SD cards.

By "firmware" do you mean U-Boot?

>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  docs/system/ppc/ppce500.rst | 13 +++++++++++++
>  hw/ppc/Kconfig              |  1 +
>  hw/ppc/e500.c               | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 46 insertions(+)
>
> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
> index c3f55c6f3d..50b199c8f3 100644
> --- a/docs/system/ppc/ppce500.rst
> +++ b/docs/system/ppc/ppce500.rst
> @@ -19,6 +19,7 @@ The ``ppce500`` machine supports the following devices:
>  * Power-off functionality via one GPIO pin
>  * 1 Freescale MPC8xxx PCI host controller
>  * VirtIO devices via PCI bus
> +* 1 Freescale Enhanced Secure Digital Host controller (eSDHC)
>  * 1 Freescale Enhanced Triple Speed Ethernet controller (eTSEC)
>
>  Hardware configuration information
> @@ -131,6 +132,18 @@ be used as follows:
>        -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
>        -append "rootwait root=/dev/mtdblock0"
>
> +Alternatively, the root file system can also reside on an emulated SD card
> +whose size must again be a power of two:
> +
> +.. code-block:: bash
> +
> +  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \

qemu-system-ppc{64|32}

> +      -display none -serial stdio \
> +      -kernel vmlinux \
> +      -device sd-card,drive=mydrive \
> +      -drive id=mydrive,if=none,file=/path/to/rootfs.ext2,format=raw \
> +      -append "rootwait root=/dev/mmcblk0"
> +
>  Running U-Boot
>  --------------
>
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 769a1ead1c..6e31f568ba 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -129,6 +129,7 @@ config E500
>      select PFLASH_CFI01
>      select PLATFORM_BUS
>      select PPCE500_PCI
> +    select SDHCI
>      select SERIAL
>      select MPC_I2C
>      select FDT_PPC
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 7843a4e04b..87a03fd4a9 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -48,6 +48,7 @@
>  #include "hw/net/fsl_etsec/etsec.h"
>  #include "hw/i2c/i2c.h"
>  #include "hw/irq.h"
> +#include "hw/sd/sdhci.h"
>
>  #define EPAPR_MAGIC                (0x45504150)
>  #define DTC_LOAD_PAD               0x1800000
> @@ -66,11 +67,14 @@
>  #define MPC8544_SERIAL1_REGS_OFFSET 0x4600ULL
>  #define MPC8544_PCI_REGS_OFFSET    0x8000ULL
>  #define MPC8544_PCI_REGS_SIZE      0x1000ULL
> +#define MPC85XX_ESDHC_REGS_OFFSET  0x2e000ULL
> +#define MPC85XX_ESDHC_REGS_SIZE    0x1000ULL
>  #define MPC8544_UTIL_OFFSET        0xe0000ULL
>  #define MPC8XXX_GPIO_OFFSET        0x000FF000ULL
>  #define MPC8544_I2C_REGS_OFFSET    0x3000ULL
>  #define MPC8XXX_GPIO_IRQ           47
>  #define MPC8544_I2C_IRQ            43
> +#define MPC85XX_ESDHC_IRQ          72
>  #define RTC_REGS_OFFSET            0x68
>
>  #define PLATFORM_CLK_FREQ_HZ       (400 * 1000 * 1000)
> @@ -203,6 +207,25 @@ static void dt_i2c_create(void *fdt, const char *soc, const char *mpic,
>      g_free(i2c);
>  }
>
> +static void dt_sdhc_create(void *fdt, const char *parent, const char *mpic)
> +{
> +    hwaddr mmio = MPC85XX_ESDHC_REGS_OFFSET;
> +    hwaddr size = MPC85XX_ESDHC_REGS_SIZE;
> +    int irq = MPC85XX_ESDHC_IRQ;
> +    char *name;
> +
> +    name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
> +    qemu_fdt_add_subnode(fdt, name);
> +    /* qemu_fdt_setprop_cells(fdt, name, "voltage-ranges", 3300, 3300); */

Drop it if it is useless

> +    qemu_fdt_setprop_cells(fdt, name, "clock-frequency", 167000000);

Is this an arbitrary frequency?

> +    qemu_fdt_setprop(fdt, name, "sdhci,auto-cmd12", NULL, 0);
> +    qemu_fdt_setprop_phandle(fdt, name, "interrupt-parent", mpic);
> +    qemu_fdt_setprop_cells(fdt, name, "bus-width", 4);
> +    qemu_fdt_setprop_cells(fdt, name, "interrupts", irq, 0x2);
> +    qemu_fdt_setprop_cells(fdt, name, "reg", mmio, size);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "fsl,esdhc");
> +    g_free(name);
> +}
>
>  typedef struct PlatformDevtreeData {
>      void *fdt;
> @@ -556,6 +579,8 @@ static int ppce500_load_device_tree(PPCE500MachineState *pms,
>
>      dt_rtc_create(fdt, "i2c", "rtc");
>
> +    /* sdhc */
> +    dt_sdhc_create(fdt, soc, mpic);
>
>      gutil = g_strdup_printf("%s/global-utilities@%llx", soc,
>                              MPC8544_UTIL_OFFSET);
> @@ -996,6 +1021,13 @@ void ppce500_init(MachineState *machine)
>      i2c_slave_create_simple(i2c, "ds1338", RTC_REGS_OFFSET);
>
>

nits: use one line for the separation

> +    /* eSDHC */
> +    dev = qdev_new(TYPE_FSL_ESDHC);
> +    s = SYS_BUS_DEVICE(dev);
> +    sysbus_realize_and_unref(s, &error_fatal);
> +    sysbus_mmio_map(s, 0, pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET);
> +    sysbus_connect_irq(s, 0, qdev_get_gpio_in(mpicdev, MPC85XX_ESDHC_IRQ));
> +
>      /* General Utility device */
>      dev = qdev_new("mpc8544-guts");
>      s = SYS_BUS_DEVICE(dev);
> --

Regards,
Bin

Re: [PATCH 00/11] ppc/e500: Add support for two types of flash, cleanup

[Bin Meng]

Hi Bernhard,

On Thu, Sep 15, 2022 at 11:25 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> This series adds support for -pflash and direct SD card access to the
> PPC e500 boards. The idea is to increase compatibility with "real" firmware
> images where only the bare minimum of drivers is compiled in.
>
> The series is structured as follows:
>
> Patches 1-3 perform some general cleanup which paves the way for the rest of
> the series.
>
> Patches 4-7 add -pflash handling where memory-mapped flash can be added on
> user's behalf. That is, the flash memory region is only added if the -pflash
> argument is supplied. Note that the cfi01 device model becomes stricter in
> checking the size of the emulated flash space.
>
> Patches 8-11 add a new device model - the Freescale eSDHC - to the e500
> boards which was missing so far.
>
> User documentation is also added as the new features become available.
>
> Tesing done:
> * `qemu-system-ppc -M ppce500 -cpu e500mc -m 256 -kernel uImage -append
> "console=ttyS0 rootwait root=/dev/mtdblock0 nokaslr" -drive
> if=pflash,file=rootfs.ext2,format=raw`
> * `qemu-system-ppc -M ppce500 -cpu e500mc -m 256 -kernel uImage -append
> "console=ttyS0 rootwait root=/dev/mmcblk0" -device sd-card,drive=mydrive -drive
> id=mydrive,if=none,file=rootfs.ext2,format=raw`

Thanks for the patches!

Did you get a chance to test the U-Boot image to work with pflash and eSDHC?

>
> The load was created using latest Buildroot with `make
> qemu_ppc_e500mc_defconfig` where the rootfs was configured to be of ext2 type.
> In both cases it was possible to log in and explore the root file system.
>

Regards,
Bin

Re: [PATCH 00/11] ppc/e500: Add support for two types of flash, cleanup

[Bernhard Beschow]

[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]

On Fri, Sep 16, 2022 at 5:27 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> Hi Bernhard,
>

Hi Bin,


> On Thu, Sep 15, 2022 at 11:25 PM Bernhard Beschow <shentey@gmail.com>
> wrote:
> >
> > This series adds support for -pflash and direct SD card access to the
> > PPC e500 boards. The idea is to increase compatibility with "real"
> firmware
> > images where only the bare minimum of drivers is compiled in.
> >
> > The series is structured as follows:
> >
> > Patches 1-3 perform some general cleanup which paves the way for the
> rest of
> > the series.
> >
> > Patches 4-7 add -pflash handling where memory-mapped flash can be added
> on
> > user's behalf. That is, the flash memory region is only added if the
> -pflash
> > argument is supplied. Note that the cfi01 device model becomes stricter
> in
> > checking the size of the emulated flash space.
> >
> > Patches 8-11 add a new device model - the Freescale eSDHC - to the e500
> > boards which was missing so far.
> >
> > User documentation is also added as the new features become available.
> >
> > Tesing done:
> > * `qemu-system-ppc -M ppce500 -cpu e500mc -m 256 -kernel uImage -append
> > "console=ttyS0 rootwait root=/dev/mtdblock0 nokaslr" -drive
> > if=pflash,file=rootfs.ext2,format=raw`
> > * `qemu-system-ppc -M ppce500 -cpu e500mc -m 256 -kernel uImage -append
> > "console=ttyS0 rootwait root=/dev/mmcblk0" -device sd-card,drive=mydrive
> -drive
> > id=mydrive,if=none,file=rootfs.ext2,format=raw`
>
> Thanks for the patches!
>

My obligation!


> Did you get a chance to test the U-Boot image to work with pflash and
> eSDHC?
>

 No, unfortunately not. Testing U-Boot would involve familiarizing myself
with it first which will probably come at some point. Right now, however,
my focus is on running a proprietary firmware flash image.

Best regards,
Bernhard

> >
> > The load was created using latest Buildroot with `make
> > qemu_ppc_e500mc_defconfig` where the rootfs was configured to be of ext2
> type.
> > In both cases it was possible to log in and explore the root file system.
> >
>
> Regards,
> Bin
>

[-- Attachment #2: Type: text/html, Size: 3218 bytes --]

Re: [PATCH 11/11] hw/ppc/e500: Add Freescale eSDHC to e500 boards

[Bernhard Beschow]

[-- Attachment #1: Type: text/plain, Size: 5862 bytes --]

On Fri, Sep 16, 2022 at 5:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> On Thu, Sep 15, 2022 at 11:30 PM Bernhard Beschow <shentey@gmail.com>
> wrote:
> >
> > Adds missing functionality to emulated e500 SOCs which increases the
> > chance of given "real" firmware images to access SD cards.
>
> By "firmware" do you mean U-Boot?
>

No, I mean a proprietary flash blob including partitions for the kernel,
root fs, U-Boot, etc.

>
> > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> > ---
> >  docs/system/ppc/ppce500.rst | 13 +++++++++++++
> >  hw/ppc/Kconfig              |  1 +
> >  hw/ppc/e500.c               | 32 ++++++++++++++++++++++++++++++++
> >  3 files changed, 46 insertions(+)
> >
> > diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
> > index c3f55c6f3d..50b199c8f3 100644
> > --- a/docs/system/ppc/ppce500.rst
> > +++ b/docs/system/ppc/ppce500.rst
> > @@ -19,6 +19,7 @@ The ``ppce500`` machine supports the following devices:
> >  * Power-off functionality via one GPIO pin
> >  * 1 Freescale MPC8xxx PCI host controller
> >  * VirtIO devices via PCI bus
> > +* 1 Freescale Enhanced Secure Digital Host controller (eSDHC)
> >  * 1 Freescale Enhanced Triple Speed Ethernet controller (eTSEC)
> >
> >  Hardware configuration information
> > @@ -131,6 +132,18 @@ be used as follows:
> >        -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
> >        -append "rootwait root=/dev/mtdblock0"
> >
> > +Alternatively, the root file system can also reside on an emulated SD
> card
> > +whose size must again be a power of two:
> > +
> > +.. code-block:: bash
> > +
> > +  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \
>
> qemu-system-ppc{64|32}
>

Will fix.


> > +      -display none -serial stdio \
> > +      -kernel vmlinux \
> > +      -device sd-card,drive=mydrive \
> > +      -drive id=mydrive,if=none,file=/path/to/rootfs.ext2,format=raw \
> > +      -append "rootwait root=/dev/mmcblk0"
> > +
> >  Running U-Boot
> >  --------------
> >
> > diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> > index 769a1ead1c..6e31f568ba 100644
> > --- a/hw/ppc/Kconfig
> > +++ b/hw/ppc/Kconfig
> > @@ -129,6 +129,7 @@ config E500
> >      select PFLASH_CFI01
> >      select PLATFORM_BUS
> >      select PPCE500_PCI
> > +    select SDHCI
> >      select SERIAL
> >      select MPC_I2C
> >      select FDT_PPC
> > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> > index 7843a4e04b..87a03fd4a9 100644
> > --- a/hw/ppc/e500.c
> > +++ b/hw/ppc/e500.c
> > @@ -48,6 +48,7 @@
> >  #include "hw/net/fsl_etsec/etsec.h"
> >  #include "hw/i2c/i2c.h"
> >  #include "hw/irq.h"
> > +#include "hw/sd/sdhci.h"
> >
> >  #define EPAPR_MAGIC                (0x45504150)
> >  #define DTC_LOAD_PAD               0x1800000
> > @@ -66,11 +67,14 @@
> >  #define MPC8544_SERIAL1_REGS_OFFSET 0x4600ULL
> >  #define MPC8544_PCI_REGS_OFFSET    0x8000ULL
> >  #define MPC8544_PCI_REGS_SIZE      0x1000ULL
> > +#define MPC85XX_ESDHC_REGS_OFFSET  0x2e000ULL
> > +#define MPC85XX_ESDHC_REGS_SIZE    0x1000ULL
> >  #define MPC8544_UTIL_OFFSET        0xe0000ULL
> >  #define MPC8XXX_GPIO_OFFSET        0x000FF000ULL
> >  #define MPC8544_I2C_REGS_OFFSET    0x3000ULL
> >  #define MPC8XXX_GPIO_IRQ           47
> >  #define MPC8544_I2C_IRQ            43
> > +#define MPC85XX_ESDHC_IRQ          72
> >  #define RTC_REGS_OFFSET            0x68
> >
> >  #define PLATFORM_CLK_FREQ_HZ       (400 * 1000 * 1000)
> > @@ -203,6 +207,25 @@ static void dt_i2c_create(void *fdt, const char
> *soc, const char *mpic,
> >      g_free(i2c);
> >  }
> >
> > +static void dt_sdhc_create(void *fdt, const char *parent, const char
> *mpic)
> > +{
> > +    hwaddr mmio = MPC85XX_ESDHC_REGS_OFFSET;
> > +    hwaddr size = MPC85XX_ESDHC_REGS_SIZE;
> > +    int irq = MPC85XX_ESDHC_IRQ;
> > +    char *name;
> > +
> > +    name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
> > +    qemu_fdt_add_subnode(fdt, name);
> > +    /* qemu_fdt_setprop_cells(fdt, name, "voltage-ranges", 3300, 3300);
> */
>
> Drop it if it is useless
>
> > +    qemu_fdt_setprop_cells(fdt, name, "clock-frequency", 167000000);
>
> Is this an arbitrary frequency?
>

I'll drop both since the eSDHC works also without the frequency line.

> +    qemu_fdt_setprop(fdt, name, "sdhci,auto-cmd12", NULL, 0);
> > +    qemu_fdt_setprop_phandle(fdt, name, "interrupt-parent", mpic);
> > +    qemu_fdt_setprop_cells(fdt, name, "bus-width", 4);
> > +    qemu_fdt_setprop_cells(fdt, name, "interrupts", irq, 0x2);
> > +    qemu_fdt_setprop_cells(fdt, name, "reg", mmio, size);
> > +    qemu_fdt_setprop_string(fdt, name, "compatible", "fsl,esdhc");
> > +    g_free(name);
> > +}
> >
> >  typedef struct PlatformDevtreeData {
> >      void *fdt;
> > @@ -556,6 +579,8 @@ static int
> ppce500_load_device_tree(PPCE500MachineState *pms,
> >
> >      dt_rtc_create(fdt, "i2c", "rtc");
> >
> > +    /* sdhc */
> > +    dt_sdhc_create(fdt, soc, mpic);
> >
> >      gutil = g_strdup_printf("%s/global-utilities@%llx", soc,
> >                              MPC8544_UTIL_OFFSET);
> > @@ -996,6 +1021,13 @@ void ppce500_init(MachineState *machine)
> >      i2c_slave_create_simple(i2c, "ds1338", RTC_REGS_OFFSET);
> >
> >
>
> nits: use one line for the separation
>

The extra empty line was there before and it looks like it came from moving
I2C around - notice the broken indentation of it's comment.

I'll fix that in this patch.

Best regards,
Bernhard

> +    /* eSDHC */
> > +    dev = qdev_new(TYPE_FSL_ESDHC);
> > +    s = SYS_BUS_DEVICE(dev);
> > +    sysbus_realize_and_unref(s, &error_fatal);
> > +    sysbus_mmio_map(s, 0, pmc->ccsrbar_base +
> MPC85XX_ESDHC_REGS_OFFSET);
> > +    sysbus_connect_irq(s, 0, qdev_get_gpio_in(mpicdev,
> MPC85XX_ESDHC_IRQ));
> > +
> >      /* General Utility device */
> >      dev = qdev_new("mpc8544-guts");
> >      s = SYS_BUS_DEVICE(dev);
> > --
>
> Regards,
> Bin
>

[-- Attachment #2: Type: text/html, Size: 8298 bytes --]

Re: [PATCH 07/11] hw/ppc/e500: Implement pflash handling

[Bernhard Beschow]

Am 16. September 2022 15:00:06 UTC schrieb Bin Meng <bmeng.cn@gmail.com>:
>On Thu, Sep 15, 2022 at 11:36 PM Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Allows e500 boards to have their root file system reside on flash using
>> only builtin devices.
>>
>> Note that the flash memory area is only created when a -pflash argument is
>> given, and that the size is determined by the given file. The idea is to
>> put users into control.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>  docs/system/ppc/ppce500.rst | 12 +++++++++
>>  hw/ppc/Kconfig              |  1 +
>>  hw/ppc/e500.c               | 54 +++++++++++++++++++++++++++++++++++++
>>  3 files changed, 67 insertions(+)
>>
>> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
>> index ba6bcb7314..c3f55c6f3d 100644
>> --- a/docs/system/ppc/ppce500.rst
>> +++ b/docs/system/ppc/ppce500.rst
>> @@ -119,6 +119,18 @@ To boot the 32-bit Linux kernel:
>>        -initrd /path/to/rootfs.cpio \
>>        -append "root=/dev/ram"
>>
>> +Rather than using a root file system on ram disk, it is possible to have it on
>> +emulated flash. Given an ext2 image whose size must be a power of two, it can
>> +be used as follows:
>> +
>> +.. code-block:: bash
>> +
>> +  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \
>
>qemu-system-ppc{64|32}

Will fix.

>> +      -display none -serial stdio \
>> +      -kernel vmlinux \
>> +      -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
>> +      -append "rootwait root=/dev/mtdblock0"
>> +
>>  Running U-Boot
>>  --------------
>>
>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>> index 791fe78a50..769a1ead1c 100644
>> --- a/hw/ppc/Kconfig
>> +++ b/hw/ppc/Kconfig
>> @@ -126,6 +126,7 @@ config E500
>>      select ETSEC
>>      select GPIO_MPC8XXX
>>      select OPENPIC
>> +    select PFLASH_CFI01
>>      select PLATFORM_BUS
>>      select PPCE500_PCI
>>      select SERIAL
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 864b6f3d92..7843a4e04b 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -23,8 +23,10 @@
>>  #include "e500-ccsr.h"
>>  #include "net/net.h"
>>  #include "qemu/config-file.h"
>> +#include "hw/block/flash.h"
>>  #include "hw/char/serial.h"
>>  #include "hw/pci/pci.h"
>> +#include "sysemu/block-backend-io.h"
>>  #include "sysemu/sysemu.h"
>>  #include "sysemu/kvm.h"
>>  #include "sysemu/reset.h"
>> @@ -267,6 +269,34 @@ static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
>>      }
>>  }
>>
>> +static void create_devtree_flash(SysBusDevice *sbdev,
>> +                                 PlatformDevtreeData *data)
>> +{
>> +    char *name;
>
>Use g_autofree

Yes, good idea.

>> +    uint64_t num_blocks = object_property_get_uint(OBJECT(sbdev),
>> +                                                   "num-blocks",
>> +                                                   &error_fatal);
>> +    uint64_t sector_length = object_property_get_uint(OBJECT(sbdev),
>> +                                                      "sector-length",
>> +                                                      &error_fatal);
>> +    uint64_t bank_width = object_property_get_uint(OBJECT(sbdev),
>> +                                                   "width",
>> +                                                   &error_fatal);
>> +    hwaddr flashbase = 0;
>> +    hwaddr flashsize = num_blocks * sector_length;
>> +    void *fdt = data->fdt;
>> +
>> +    name = g_strdup_printf("%s/nor@%" PRIx64, data->node, flashbase);
>> +    qemu_fdt_add_subnode(fdt, name);
>> +    qemu_fdt_setprop_cell(fdt, name, "#address-cells", 1);
>> +    qemu_fdt_setprop_cell(fdt, name, "#size-cells", 1);
>
>#address-cells and #size-cells are not needed.

Will remove.

>> +    qemu_fdt_setprop_string(fdt, name, "compatible", "cfi-flash");
>> +    qemu_fdt_setprop_sized_cells(fdt, name, "reg",
>> +                                 1, flashbase, 1, flashsize);
>> +    qemu_fdt_setprop_cell(fdt, name, "bank-width", bank_width);
>> +    g_free(name);
>> +}
>> +
>>  static void platform_bus_create_devtree(PPCE500MachineState *pms,
>>                                          void *fdt, const char *mpic)
>>  {
>> @@ -276,6 +306,8 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>>      uint64_t addr = pmc->platform_bus_base;
>>      uint64_t size = pmc->platform_bus_size;
>>      int irq_start = pmc->platform_bus_first_irq;
>> +    SysBusDevice *sbdev;
>> +    bool ambiguous;
>>
>>      /* Create a /platform node that we can put all devices into */
>>
>> @@ -302,6 +334,13 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>>      /* Loop through all dynamic sysbus devices and create nodes for them */
>>      foreach_dynamic_sysbus_device(sysbus_device_create_devtree, &data);
>>
>> +    sbdev = SYS_BUS_DEVICE(object_resolve_path_type("", TYPE_PFLASH_CFI01,
>> +                                                    &ambiguous));
>
>Can this be moved into sysbus_device_create_devtree(), and use the
>same logic as the eTSEC device?

I've tried that, but the logic for eTSEC seems to get triggered for user-created devices only. Since TYPE_PFLASH_CFI01 isn't user-created we're not triggered I guess.

I think that the eTSEC handling could be moved into sysbus-fdt.c but I'm not sure whether this is allowed due to poisoning.

>> +    if (sbdev) {
>> +        assert(!ambiguous);
>> +        create_devtree_flash(sbdev, &data);
>> +    }
>> +
>>      g_free(node);
>>  }
>>
>> @@ -856,6 +895,7 @@ void ppce500_init(MachineState *machine)
>>      unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4};
>>      IrqLines *irqs;
>>      DeviceState *dev, *mpicdev;
>> +    DriveInfo *dinfo;
>>      CPUPPCState *firstenv = NULL;
>>      MemoryRegion *ccsr_addr_space;
>>      SysBusDevice *s;
>> @@ -1024,6 +1064,20 @@ void ppce500_init(MachineState *machine)
>>                                  pmc->platform_bus_base,
>>                                  sysbus_mmio_get_region(s, 0));
>>
>> +    dinfo = drive_get(IF_PFLASH, 0, 0);
>> +    if (dinfo) {
>> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>> +        BlockDriverState *bs = blk_bs(blk);
>> +        uint64_t size = bdrv_getlength(bs);
>> +        if (ctpop64(size) != 1) {
>> +            error_report("Size of pflash file must be a power of two.");
>> +            exit(1);
>> +        }
>
>I think we should also check whether the flash size plus the eTSEC
>size exceeds the platform bus mmio window size otherwise it won't work
>for both devices present, no?

I could check that the flash fits inside the eLBC memory window.

For user-created devices such as eTSEC, however, I'd like to rely on other parts of QEMU to check this. First, I don't know how to get access to all relevant devices and their memory windows, and second, catching all possible (future) cases here seems a bit ad-hoc and fragile to me.

Best regards,
Bernhard
>
>> +        pflash_cfi01_register(pmc->platform_bus_base, "e500.flash",
>> +                              size, blk,
>> +                              64 * KiB, 2, 0x89, 0x18, 0x0000, 0x0, 1);
>> +    }
>> +
>>      /*
>>       * Smart firmware defaults ahead!
>>       *
>
>Regards,
>Bin

Re: [PATCH 09/11] hw/sd/sdhci: Rename ESDHC_* defines to USDHC_*

[Bernhard Beschow]

Am 16. September 2022 10:07:28 UTC schrieb Bin Meng <bmeng.cn@gmail.com>:
>On Thu, Sep 15, 2022 at 11:42 PM Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> The device model's functions start with "usdhc_", so rename the defines
>> accordingly for consistency.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>  hw/sd/sdhci.c | 68 +++++++++++++++++++++++++--------------------------
>>  1 file changed, 34 insertions(+), 34 deletions(-)
>>
>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>> index 6da5e2c781..7a5996caad 100644
>> --- a/hw/sd/sdhci.c
>> +++ b/hw/sd/sdhci.c
>> @@ -1577,24 +1577,24 @@ static const TypeInfo sdhci_bus_info = {
>>
>>  /* --- qdev i.MX eSDHC --- */
>>
>> -#define ESDHC_MIX_CTRL                  0x48
>> +#define USDHC_MIX_CTRL                  0x48
>>
>> -#define ESDHC_VENDOR_SPEC               0xc0
>> -#define ESDHC_IMX_FRC_SDCLK_ON          (1 << 8)
>> +#define USDHC_VENDOR_SPEC               0xc0
>> +#define USDHC_IMX_FRC_SDCLK_ON          (1 << 8)
>>
>> -#define ESDHC_DLL_CTRL                  0x60
>> +#define USDHC_DLL_CTRL                  0x60
>>
>> -#define ESDHC_TUNING_CTRL               0xcc
>> -#define ESDHC_TUNE_CTRL_STATUS          0x68
>> -#define ESDHC_WTMK_LVL                  0x44
>> +#define USDHC_TUNING_CTRL               0xcc
>> +#define USDHC_TUNE_CTRL_STATUS          0x68
>> +#define USDHC_WTMK_LVL                  0x44
>>
>>  /* Undocumented register used by guests working around erratum ERR004536 */
>> -#define ESDHC_UNDOCUMENTED_REG27        0x6c
>> +#define USDHC_UNDOCUMENTED_REG27        0x6c
>>
>> -#define ESDHC_CTRL_4BITBUS              (0x1 << 1)
>> -#define ESDHC_CTRL_8BITBUS              (0x2 << 1)
>> +#define USDHC_CTRL_4BITBUS              (0x1 << 1)
>> +#define USDHC_CTRL_8BITBUS              (0x2 << 1)
>>
>> -#define ESDHC_PRNSTS_SDSTB              (1 << 3)
>> +#define USDHC_PRNSTS_SDSTB              (1 << 3)
>>
>>  static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
>>  {
>> @@ -1615,11 +1615,11 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
>>          hostctl1 = SDHC_DMA_TYPE(s->hostctl1) << (8 - 3);
>>
>>          if (s->hostctl1 & SDHC_CTRL_8BITBUS) {
>> -            hostctl1 |= ESDHC_CTRL_8BITBUS;
>> +            hostctl1 |= USDHC_CTRL_8BITBUS;
>>          }
>>
>>          if (s->hostctl1 & SDHC_CTRL_4BITBUS) {
>> -            hostctl1 |= ESDHC_CTRL_4BITBUS;
>> +            hostctl1 |= USDHC_CTRL_4BITBUS;
>>          }
>>
>>          ret  = hostctl1;
>> @@ -1630,21 +1630,21 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
>>
>>      case SDHC_PRNSTS:
>>          /* Add SDSTB (SD Clock Stable) bit to PRNSTS */
>> -        ret = sdhci_read(opaque, offset, size) & ~ESDHC_PRNSTS_SDSTB;
>> +        ret = sdhci_read(opaque, offset, size) & ~USDHC_PRNSTS_SDSTB;
>>          if (s->clkcon & SDHC_CLOCK_INT_STABLE) {
>> -            ret |= ESDHC_PRNSTS_SDSTB;
>> +            ret |= USDHC_PRNSTS_SDSTB;
>>          }
>>          break;
>>
>> -    case ESDHC_VENDOR_SPEC:
>> +    case USDHC_VENDOR_SPEC:
>>          ret = s->vendor_spec;
>>          break;
>> -    case ESDHC_DLL_CTRL:
>> -    case ESDHC_TUNE_CTRL_STATUS:
>> -    case ESDHC_UNDOCUMENTED_REG27:
>> -    case ESDHC_TUNING_CTRL:
>> -    case ESDHC_MIX_CTRL:
>> -    case ESDHC_WTMK_LVL:
>> +    case USDHC_DLL_CTRL:
>> +    case USDHC_TUNE_CTRL_STATUS:
>> +    case USDHC_UNDOCUMENTED_REG27:
>> +    case USDHC_TUNING_CTRL:
>> +    case USDHC_MIX_CTRL:
>> +    case USDHC_WTMK_LVL:
>>          ret = 0;
>>          break;
>>      }
>> @@ -1660,18 +1660,18 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>>      uint32_t value = (uint32_t)val;
>>
>>      switch (offset) {
>> -    case ESDHC_DLL_CTRL:
>> -    case ESDHC_TUNE_CTRL_STATUS:
>> -    case ESDHC_UNDOCUMENTED_REG27:
>> -    case ESDHC_TUNING_CTRL:
>> -    case ESDHC_WTMK_LVL:
>> +    case USDHC_DLL_CTRL:
>> +    case USDHC_TUNE_CTRL_STATUS:
>> +    case USDHC_UNDOCUMENTED_REG27:
>> +    case USDHC_TUNING_CTRL:
>> +    case USDHC_WTMK_LVL:
>>          break;
>>
>> -    case ESDHC_VENDOR_SPEC:
>> +    case USDHC_VENDOR_SPEC:
>>          s->vendor_spec = value;
>>          switch (s->vendor) {
>>          case SDHCI_VENDOR_IMX:
>> -            if (value & ESDHC_IMX_FRC_SDCLK_ON) {
>> +            if (value & USDHC_IMX_FRC_SDCLK_ON) {
>>                  s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
>>              } else {
>>                  s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
>> @@ -1740,12 +1740,12 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>>           * Second, split "Data Transfer Width" from bits 2 and 1 in to
>>           * bits 5 and 1
>>           */
>> -        if (value & ESDHC_CTRL_8BITBUS) {
>> +        if (value & USDHC_CTRL_8BITBUS) {
>>              hostctl1 |= SDHC_CTRL_8BITBUS;
>>          }
>>
>> -        if (value & ESDHC_CTRL_4BITBUS) {
>> -            hostctl1 |= ESDHC_CTRL_4BITBUS;
>> +        if (value & USDHC_CTRL_4BITBUS) {
>> +            hostctl1 |= USDHC_CTRL_4BITBUS;
>>          }
>>
>>          /*
>> @@ -1768,11 +1768,11 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>>          sdhci_write(opaque, offset, value, size);
>>          break;
>>
>> -    case ESDHC_MIX_CTRL:
>> +    case USDHC_MIX_CTRL:
>>          /*
>>           * So, when SD/MMC stack in Linux tries to write to "Transfer
>>           * Mode Register", ESDHC i.MX quirk code will translate it
>
>Here I assume ESDHC i.MX means the Linux eSDHC driver for i.MX, so no
>need to replace ESDHC with USDHC?

I see. Prefix with IMX_ then and rename the functions for consistency? Then prefix the MPC ones with MPC_?

>
>> -         * into a write to ESDHC_MIX_CTRL, so we do the opposite in
>> +         * into a write to USDHC_MIX_CTRL, so we do the opposite in
>>           * order to get where we started
>>           *
>>           * Note that Auto CMD23 Enable bit is located in a wrong place
>> --
>
>Overall LGTM:
>Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH 04/11] hw/ppc/mpc8544ds: Add platform bus

[Bernhard Beschow]

Am 16. September 2022 06:15:53 UTC schrieb Bin Meng <bmeng.cn@gmail.com>:
>On Thu, Sep 15, 2022 at 11:29 PM Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Models the real device more closely.
>
>Please describe the source (e.g.: I assume it's MPC8544DS board manual
>or something like that?) that describe such memory map for the
>platform bus.
>
>Is this the eLBC bus range that includes the NOR flash device?

Good point. My numbers come from a different board. I'll fix them according to the  mpc8544ds.dts in the Linux tree.

This will leave an eLBC memory window of just 8MB while my proprietary load needs 64MB. My proprietary load doesn't seem to have 64 bit physical memory support so I can't use e500plat either. Any suggestions?

Best regards,
Bernhard
>
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>  hw/ppc/mpc8544ds.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
>> index 81177505f0..cd6cd04bef 100644
>> --- a/hw/ppc/mpc8544ds.c
>> +++ b/hw/ppc/mpc8544ds.c
>> @@ -14,6 +14,7 @@
>>  #include "sysemu/device_tree.h"
>>  #include "hw/ppc/openpic.h"
>>  #include "qemu/error-report.h"
>> +#include "qemu/units.h"
>>  #include "cpu.h"
>>
>>  static void mpc8544ds_fixup_devtree(void *fdt)
>> @@ -45,6 +46,11 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data)
>>      pmc->pci_nr_slots = 2;
>>      pmc->fixup_devtree = mpc8544ds_fixup_devtree;
>>      pmc->mpic_version = OPENPIC_MODEL_FSL_MPIC_20;
>> +    pmc->has_platform_bus = true;
>> +    pmc->platform_bus_base = 0xEC000000ULL;
>> +    pmc->platform_bus_size = 128 * MiB;
>> +    pmc->platform_bus_first_irq = 5;
>> +    pmc->platform_bus_num_irqs = 10;
>>      pmc->ccsrbar_base = 0xE0000000ULL;
>>      pmc->pci_mmio_base = 0xC0000000ULL;
>>      pmc->pci_mmio_bus_base = 0xC0000000ULL;
>> --
>
>Regards,
>Bin

Re: [PATCH 10/11] hw/sd/sdhci: Implement Freescale eSDHC device model

[Bernhard Beschow]

Am 16. September 2022 15:15:03 UTC schrieb Bin Meng <bmeng.cn@gmail.com>:
>On Thu, Sep 15, 2022 at 11:30 PM Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Will allow e500 boards to access SD cards using just their own devices.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>  hw/sd/sdhci.c         | 147 +++++++++++++++++++++++++++++++++++++++++-
>>  include/hw/sd/sdhci.h |   3 +
>>  2 files changed, 149 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>> index 7a5996caad..09285ccfa1 100644
>> --- a/hw/sd/sdhci.c
>> +++ b/hw/sd/sdhci.c
>> @@ -1369,6 +1369,7 @@ void sdhci_initfn(SDHCIState *s)
>>      s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s);
>>
>>      s->io_ops = &sdhci_mmio_ops;
>> +    s->io_registers_map_size = SDHC_REGISTERS_MAP_SIZE;
>>  }
>>
>>  void sdhci_uninitfn(SDHCIState *s)
>> @@ -1392,7 +1393,7 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
>>      s->fifo_buffer = g_malloc0(s->buf_maxsz);
>>
>>      memory_region_init_io(&s->iomem, OBJECT(s), s->io_ops, s, "sdhci",
>> -                          SDHC_REGISTERS_MAP_SIZE);
>> +                          s->io_registers_map_size);
>>  }
>>
>>  void sdhci_common_unrealize(SDHCIState *s)
>> @@ -1575,6 +1576,149 @@ static const TypeInfo sdhci_bus_info = {
>>      .class_init = sdhci_bus_class_init,
>>  };
>>
>> +/* --- qdev Freescale eSDHC --- */
>> +
>> +/* Host Controller Capabilities Register 2 */
>> +#define ESDHC_CAPABILITIES_1        0x114
>> +
>> +/* Control Register for DMA transfer */
>> +#define ESDHC_DMA_SYSCTL            0x40c
>> +#define ESDHC_PERIPHERAL_CLK_SEL    0x00080000
>> +#define ESDHC_FLUSH_ASYNC_FIFO      0x00040000
>> +#define ESDHC_DMA_SNOOP             0x00000040
>
>It looks the above 3 bit fields are not used?

Yes, possibly. I'll check for more unused stuff.

>> +
>> +#define ESDHC_REGISTERS_MAP_SIZE    0x410
>> +
>> +static uint64_t esdhci_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> +    uint64_t ret;
>> +
>> +    if (size != 4) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
>> +                      " wrong size\n", size, offset);
>> +        return 0;
>> +    }
>> +
>> +    if (offset & 0x3) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
>> +                      " unaligned\n", size, offset);
>> +        return 0;
>> +    }
>> +
>> +    switch (offset) {
>> +    case SDHC_SYSAD:
>> +    case SDHC_BLKSIZE:
>> +    case SDHC_ARGUMENT:
>> +    case SDHC_TRNMOD:
>> +    case SDHC_RSPREG0:
>> +    case SDHC_RSPREG1:
>> +    case SDHC_RSPREG2:
>> +    case SDHC_RSPREG3:
>> +    case SDHC_BDATA:
>> +    case SDHC_PRNSTS:
>> +    case SDHC_HOSTCTL:
>> +    case SDHC_CLKCON:
>> +    case SDHC_NORINTSTS:
>> +    case SDHC_NORINTSTSEN:
>> +    case SDHC_NORINTSIGEN:
>> +    case SDHC_ACMD12ERRSTS:
>> +    case SDHC_CAPAB:
>> +    case SDHC_SLOT_INT_STATUS:
>> +        ret = sdhci_read(opaque, offset, size);
>> +        break;
>> +
>> +    case ESDHC_DMA_SYSCTL:
>> +    case 0x44:
>
>Can we define a macro for this offset?

Sure. Not sure why I didn't.

>> +        ret = 0;
>> +        qemu_log_mask(LOG_UNIMP, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
>> +                      " not implemented\n", size, offset);
>> +        break;
>> +
>> +    default:
>> +        ret = 0;
>> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
>> +                      " unknown offset\n", size, offset);
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static void esdhci_write(void *opaque, hwaddr offset, uint64_t val,
>> +                         unsigned size)
>> +{
>> +    if (size != 4) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
>> +                      " <- 0x%08lx wrong size\n", size, offset, val);
>> +        return;
>> +    }
>> +
>> +    if (offset & 0x3) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
>> +                      " <- 0x%08lx unaligned\n", size, offset, val);
>> +        return;
>> +    }
>> +
>> +    switch (offset) {
>> +    case SDHC_SYSAD:
>> +    case SDHC_BLKSIZE:
>> +    case SDHC_ARGUMENT:
>> +    case SDHC_TRNMOD:
>> +    case SDHC_BDATA:
>> +    case SDHC_HOSTCTL:
>> +    case SDHC_CLKCON:
>> +    case SDHC_NORINTSTS:
>> +    case SDHC_NORINTSTSEN:
>> +    case SDHC_NORINTSIGEN:
>> +    case SDHC_FEAER:
>> +        sdhci_write(opaque, offset, val, size);
>> +        break;
>> +
>> +    case ESDHC_DMA_SYSCTL:
>> +    case 0x44:
>
>ditto

Ack.

Best regards,
Bernhard
>
>> +        qemu_log_mask(LOG_UNIMP, "ESDHC wr_%ub @0x%02" HWADDR_PRIx " <- 0x%08lx "
>> +                      "not implemented\n", size, offset, val);
>> +        break;
>> +
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
>> +                      " <- 0x%08lx unknown offset\n", size, offset, val);
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps esdhc_mmio_ops = {
>> +    .read = esdhci_read,
>> +    .write = esdhci_write,
>> +    .valid = {
>> +        .min_access_size = 1,
>> +        .max_access_size = 4,
>> +        .unaligned = false
>> +    },
>> +    .endianness = DEVICE_BIG_ENDIAN,
>> +};
>> +
>> +static void esdhci_init(Object *obj)
>> +{
>> +    DeviceState *dev = DEVICE(obj);
>> +    SDHCIState *s = SYSBUS_SDHCI(obj);
>> +
>> +    s->io_ops = &esdhc_mmio_ops;
>> +    s->io_registers_map_size = ESDHC_REGISTERS_MAP_SIZE;
>> +
>> +    /*
>> +     * Compatible with:
>> +     * - SD Host Controller Specification Version 2.0 Part A2
>> +     */
>> +    qdev_prop_set_uint8(dev, "sd-spec-version", 2);
>> +}
>> +
>> +static const TypeInfo esdhc_info = {
>> +    .name = TYPE_FSL_ESDHC,
>> +    .parent = TYPE_SYSBUS_SDHCI,
>> +    .instance_init = esdhci_init,
>> +};
>> +
>>  /* --- qdev i.MX eSDHC --- */
>>
>>  #define USDHC_MIX_CTRL                  0x48
>> @@ -1907,6 +2051,7 @@ static void sdhci_register_types(void)
>>  {
>>      type_register_static(&sdhci_sysbus_info);
>>      type_register_static(&sdhci_bus_info);
>> +    type_register_static(&esdhc_info);
>>      type_register_static(&imx_usdhc_info);
>>      type_register_static(&sdhci_s3c_info);
>>  }
>> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
>> index 01a64c5442..5b32e83eee 100644
>> --- a/include/hw/sd/sdhci.h
>> +++ b/include/hw/sd/sdhci.h
>> @@ -45,6 +45,7 @@ struct SDHCIState {
>>      AddressSpace *dma_as;
>>      MemoryRegion *dma_mr;
>>      const MemoryRegionOps *io_ops;
>> +    uint64_t io_registers_map_size;
>>
>>      QEMUTimer *insert_timer;       /* timer for 'changing' sd card. */
>>      QEMUTimer *transfer_timer;
>> @@ -122,6 +123,8 @@ DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI,
>>  DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI,
>>                           TYPE_SYSBUS_SDHCI)
>>
>> +#define TYPE_FSL_ESDHC "fsl-esdhc"
>> +
>>  #define TYPE_IMX_USDHC "imx-usdhc"
>>
>>  #define TYPE_S3C_SDHCI "s3c-sdhci"
>> --
>
>Regards,
>Bin

Re: [PATCH 01/11] hw/ppc/meson: Allow e500 boards to be enabled separately

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:
> Gives users more fine-grained control over what should be compiled into
> QEMU.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   configs/devices/ppc-softmmu/default.mak | 3 ++-
>   hw/ppc/Kconfig                          | 8 ++++++++
>   hw/ppc/meson.build                      | 6 ++----
>   3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/configs/devices/ppc-softmmu/default.mak b/configs/devices/ppc-softmmu/default.mak
> index 658a454426..a887f5438b 100644
> --- a/configs/devices/ppc-softmmu/default.mak
> +++ b/configs/devices/ppc-softmmu/default.mak
> @@ -1,7 +1,8 @@
>   # Default configuration for ppc-softmmu
>   
>   # For embedded PPCs:
> -CONFIG_E500=y
> +CONFIG_E500PLAT=y
> +CONFIG_MPC8544DS=y
>   CONFIG_PPC405=y
>   CONFIG_PPC440=y
>   CONFIG_VIRTEX=y
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 3a4418a69e..22a64745d4 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -132,6 +132,14 @@ config E500
>       select FDT_PPC
>       select DS1338
>   
> +config E500PLAT
> +    bool
> +    select E500
> +
> +config MPC8544DS
> +    bool
> +    select E500
> +
>   config VIRTEX
>       bool
>       select PPC4XX
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index 62801923f3..32babc9b48 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -71,12 +71,10 @@ ppc_ss.add(when: 'CONFIG_MAC_OLDWORLD', if_true: files('mac_oldworld.c'))
>   # NewWorld PowerMac
>   ppc_ss.add(when: 'CONFIG_MAC_NEWWORLD', if_true: files('mac_newworld.c'))
>   # e500
> +ppc_ss.add(when: 'CONFIG_E500PLAT', if_true: files('e500plat.c'))
> +ppc_ss.add(when: 'CONFIG_MPC8544DS', if_true: files('mpc8544ds.c'))
>   ppc_ss.add(when: 'CONFIG_E500', if_true: files(
>     'e500.c',
> -  'mpc8544ds.c',
> -  'e500plat.c'
> -))
> -ppc_ss.add(when: 'CONFIG_E500', if_true: files(
>     'mpc8544_guts.c',

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>     'ppce500_spin.c'
>   ))

Re: [PATCH 02/11] hw/gpio/meson: Introduce dedicated config switch for hw/gpio/mpc8xxx

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:
> Having a dedicated config switch makes dependency handling cleaner.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/gpio/Kconfig     | 3 +++
>   hw/gpio/meson.build | 2 +-
>   hw/ppc/Kconfig      | 1 +
>   3 files changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 03/11] docs/system/ppc/ppce500: Add heading for networking chapter

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:
> The sudden change of topics is slightly confusing and makes the
> networking information less visible. So separate the networking chapter
> to improve comprehensibility.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   docs/system/ppc/ppce500.rst | 3 +++
>   1 file changed, 3 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 05/11] hw/ppc/e500: Remove if statement which is now always true

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:
> Now that the MPC8544DS board also has a platform bus, the if statement
> was always true.

s/was/is/.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/ppc/e500.c      | 30 ++++++++++++++----------------
>   hw/ppc/e500.h      |  1 -
>   hw/ppc/e500plat.c  |  1 -
>   hw/ppc/mpc8544ds.c |  1 -
>   4 files changed, 14 insertions(+), 19 deletions(-)

Re: [PATCH 08/11] hw/sd/sdhci-internal: Unexport ESDHC defines

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:
> These defines aren't used outside of sdhci.c, so can be defined there.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/sd/sdhci-internal.h | 20 --------------------
>   hw/sd/sdhci.c          | 19 +++++++++++++++++++
>   2 files changed, 19 insertions(+), 20 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 00/11] ppc/e500: Add support for two types of flash, cleanup

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:
> This series adds support for -pflash and direct SD card access to the
> PPC e500 boards. The idea is to increase compatibility with "real" firmware
> images where only the bare minimum of drivers is compiled in.
> 
> The series is structured as follows:
> 
> Patches 1-3 perform some general cleanup which paves the way for the rest of
> the series.
> 
> Patches 4-7 add -pflash handling where memory-mapped flash can be added on
> user's behalf. That is, the flash memory region is only added if the -pflash
> argument is supplied. Note that the cfi01 device model becomes stricter in
> checking the size of the emulated flash space.
> 
> Patches 8-11 add a new device model - the Freescale eSDHC - to the e500
> boards which was missing so far.
> 
> User documentation is also added as the new features become available.

Future possible cleanup: Looking at the e500 component, the MPC8544 GUTS
added in commit b0fb84236d ("PPC: E500: Implement reboot controller")
was used in hw/ppce500_mpc8544ds.c board, but in a later refactor
(commit 4a18e7c92a "PPC: e500: rename mpc8544ds into generic file")
this file got renamed as hw/ppc/e500.c; having a MPC8544 specific piece
in the common e500 seems odd.

Re: [PATCH 07/11] hw/ppc/e500: Implement pflash handling

[B]

Am 16. September 2022 17:05:13 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>Am 16. September 2022 15:00:06 UTC schrieb Bin Meng <bmeng.cn@gmail.com>:
>>On Thu, Sep 15, 2022 at 11:36 PM Bernhard Beschow <shentey@gmail.com> wrote:
>>>
>>> Allows e500 boards to have their root file system reside on flash using
>>> only builtin devices.
>>>
>>> Note that the flash memory area is only created when a -pflash argument is
>>> given, and that the size is determined by the given file. The idea is to
>>> put users into control.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>>  docs/system/ppc/ppce500.rst | 12 +++++++++
>>>  hw/ppc/Kconfig              |  1 +
>>>  hw/ppc/e500.c               | 54 +++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 67 insertions(+)
>>>
>>> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
>>> index ba6bcb7314..c3f55c6f3d 100644
>>> --- a/docs/system/ppc/ppce500.rst
>>> +++ b/docs/system/ppc/ppce500.rst
>>> @@ -119,6 +119,18 @@ To boot the 32-bit Linux kernel:
>>>        -initrd /path/to/rootfs.cpio \
>>>        -append "root=/dev/ram"
>>>
>>> +Rather than using a root file system on ram disk, it is possible to have it on
>>> +emulated flash. Given an ext2 image whose size must be a power of two, it can
>>> +be used as follows:
>>> +
>>> +.. code-block:: bash
>>> +
>>> +  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \
>>
>>qemu-system-ppc{64|32}
>
>Will fix.
>
>>> +      -display none -serial stdio \
>>> +      -kernel vmlinux \
>>> +      -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
>>> +      -append "rootwait root=/dev/mtdblock0"
>>> +
>>>  Running U-Boot
>>>  --------------
>>>
>>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>>> index 791fe78a50..769a1ead1c 100644
>>> --- a/hw/ppc/Kconfig
>>> +++ b/hw/ppc/Kconfig
>>> @@ -126,6 +126,7 @@ config E500
>>>      select ETSEC
>>>      select GPIO_MPC8XXX
>>>      select OPENPIC
>>> +    select PFLASH_CFI01
>>>      select PLATFORM_BUS
>>>      select PPCE500_PCI
>>>      select SERIAL
>>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>>> index 864b6f3d92..7843a4e04b 100644
>>> --- a/hw/ppc/e500.c
>>> +++ b/hw/ppc/e500.c
>>> @@ -23,8 +23,10 @@
>>>  #include "e500-ccsr.h"
>>>  #include "net/net.h"
>>>  #include "qemu/config-file.h"
>>> +#include "hw/block/flash.h"
>>>  #include "hw/char/serial.h"
>>>  #include "hw/pci/pci.h"
>>> +#include "sysemu/block-backend-io.h"
>>>  #include "sysemu/sysemu.h"
>>>  #include "sysemu/kvm.h"
>>>  #include "sysemu/reset.h"
>>> @@ -267,6 +269,34 @@ static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
>>>      }
>>>  }
>>>
>>> +static void create_devtree_flash(SysBusDevice *sbdev,
>>> +                                 PlatformDevtreeData *data)
>>> +{
>>> +    char *name;
>>
>>Use g_autofree
>
>Yes, good idea.
>
>>> +    uint64_t num_blocks = object_property_get_uint(OBJECT(sbdev),
>>> +                                                   "num-blocks",
>>> +                                                   &error_fatal);
>>> +    uint64_t sector_length = object_property_get_uint(OBJECT(sbdev),
>>> +                                                      "sector-length",
>>> +                                                      &error_fatal);
>>> +    uint64_t bank_width = object_property_get_uint(OBJECT(sbdev),
>>> +                                                   "width",
>>> +                                                   &error_fatal);
>>> +    hwaddr flashbase = 0;
>>> +    hwaddr flashsize = num_blocks * sector_length;
>>> +    void *fdt = data->fdt;
>>> +
>>> +    name = g_strdup_printf("%s/nor@%" PRIx64, data->node, flashbase);
>>> +    qemu_fdt_add_subnode(fdt, name);
>>> +    qemu_fdt_setprop_cell(fdt, name, "#address-cells", 1);
>>> +    qemu_fdt_setprop_cell(fdt, name, "#size-cells", 1);
>>
>>#address-cells and #size-cells are not needed.
>
>Will remove.
>
>>> +    qemu_fdt_setprop_string(fdt, name, "compatible", "cfi-flash");
>>> +    qemu_fdt_setprop_sized_cells(fdt, name, "reg",
>>> +                                 1, flashbase, 1, flashsize);
>>> +    qemu_fdt_setprop_cell(fdt, name, "bank-width", bank_width);
>>> +    g_free(name);
>>> +}
>>> +
>>>  static void platform_bus_create_devtree(PPCE500MachineState *pms,
>>>                                          void *fdt, const char *mpic)
>>>  {
>>> @@ -276,6 +306,8 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>>>      uint64_t addr = pmc->platform_bus_base;
>>>      uint64_t size = pmc->platform_bus_size;
>>>      int irq_start = pmc->platform_bus_first_irq;
>>> +    SysBusDevice *sbdev;
>>> +    bool ambiguous;
>>>
>>>      /* Create a /platform node that we can put all devices into */
>>>
>>> @@ -302,6 +334,13 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>>>      /* Loop through all dynamic sysbus devices and create nodes for them */
>>>      foreach_dynamic_sysbus_device(sysbus_device_create_devtree, &data);
>>>
>>> +    sbdev = SYS_BUS_DEVICE(object_resolve_path_type("", TYPE_PFLASH_CFI01,
>>> +                                                    &ambiguous));
>>
>>Can this be moved into sysbus_device_create_devtree(), and use the
>>same logic as the eTSEC device?
>
>I've tried that, but the logic for eTSEC seems to get triggered for user-created devices only. Since TYPE_PFLASH_CFI01 isn't user-created we're not triggered I guess.
>
>I think that the eTSEC handling could be moved into sysbus-fdt.c but I'm not sure whether this is allowed due to poisoning.
>
>>> +    if (sbdev) {
>>> +        assert(!ambiguous);
>>> +        create_devtree_flash(sbdev, &data);
>>> +    }
>>> +
>>>      g_free(node);
>>>  }
>>>
>>> @@ -856,6 +895,7 @@ void ppce500_init(MachineState *machine)
>>>      unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4};
>>>      IrqLines *irqs;
>>>      DeviceState *dev, *mpicdev;
>>> +    DriveInfo *dinfo;
>>>      CPUPPCState *firstenv = NULL;
>>>      MemoryRegion *ccsr_addr_space;
>>>      SysBusDevice *s;
>>> @@ -1024,6 +1064,20 @@ void ppce500_init(MachineState *machine)
>>>                                  pmc->platform_bus_base,
>>>                                  sysbus_mmio_get_region(s, 0));
>>>
>>> +    dinfo = drive_get(IF_PFLASH, 0, 0);
>>> +    if (dinfo) {
>>> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>>> +        BlockDriverState *bs = blk_bs(blk);
>>> +        uint64_t size = bdrv_getlength(bs);
>>> +        if (ctpop64(size) != 1) {
>>> +            error_report("Size of pflash file must be a power of two.");
>>> +            exit(1);
>>> +        }
>>
>>I think we should also check whether the flash size plus the eTSEC
>>size exceeds the platform bus mmio window size otherwise it won't work
>>for both devices present, no?
>
>I could check that the flash fits inside the eLBC memory window.
>
>For user-created devices such as eTSEC, however, I'd like to rely on other parts of QEMU to check this. First, I don't know how to get access to all relevant devices and their memory windows, and second, catching all possible (future) cases here seems a bit ad-hoc and fragile to me.

As it turns out, platform_bus_link_device() already aborts with an error if an eTSEC device doesn't fit in the remaining space. So only the pflash size remains to be checked.
>
>Best regards,
>Bernhard
>>
>>> +        pflash_cfi01_register(pmc->platform_bus_base, "e500.flash",
>>> +                              size, blk,
>>> +                              64 * KiB, 2, 0x89, 0x18, 0x0000, 0x0, 1);
>>> +    }
>>> +
>>>      /*
>>>       * Smart firmware defaults ahead!
>>>       *
>>
>>Regards,
>>Bin
>

Re: [PATCH 04/11] hw/ppc/mpc8544ds: Add platform bus

[Bin Meng]

Hi Bernhard,

On Sat, Sep 17, 2022 at 1:19 AM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Am 16. September 2022 06:15:53 UTC schrieb Bin Meng <bmeng.cn@gmail.com>:
> >On Thu, Sep 15, 2022 at 11:29 PM Bernhard Beschow <shentey@gmail.com> wrote:
> >>
> >> Models the real device more closely.
> >
> >Please describe the source (e.g.: I assume it's MPC8544DS board manual
> >or something like that?) that describe such memory map for the
> >platform bus.
> >
> >Is this the eLBC bus range that includes the NOR flash device?
>
> Good point. My numbers come from a different board. I'll fix them according to the  mpc8544ds.dts in the Linux tree.
>
> This will leave an eLBC memory window of just 8MB while my proprietary load needs 64MB. My proprietary load doesn't seem to have 64 bit physical memory support so I can't use e500plat either. Any suggestions?
>

Currently QEMU does not model the eLBC registers so these memory
regions have to be hardcoded, unfortunately. Once we support eLBC
memory map completely I think we can remove such limitations by having
QEMU dynamically create the memory map per programmed values.

I guess you have to create another machine for your board at this point.

Regards,
Bin