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

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

[Bernhard Beschow]

Cover letter:
~~~~~~~~~~~~~

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-5 perform some general cleanup which paves the way for the rest of
the series.

Patches 6-9 add -pflash handling where memory-mapped flash can be added on
user's behalf. That is, the flash memory region in the eLBC 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 10-13 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.

v2:
~~~
Bin:
- Add source for MPC8544DS platform bus' memory map in commit message.
- Keep "ESDHC" in comment referring to Linux driver.
- Use "qemu-system-ppc{64|32} in documentation.
- Use g_autofree in device tree code.
- Remove unneeded device tree properties.
- Error out if pflash size doesn't fit into eLBC memory window.
- Remove unused ESDHC defines.
- Define macro ESDHC_WML for register offset with magic constant.
- Fix some whitespace issues when adding eSDHC device to e500.

Phil:
- Fix tense in commit message.

Bernhard Beschow (13):
  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/e500: Reduce usage of sysbus API
  hw/ppc/mpc8544ds: Rename wrongly named method
  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                           | 133 +++++++++++++--
 hw/ppc/e500.h                           |   1 -
 hw/ppc/e500plat.c                       |   1 -
 hw/ppc/meson.build                      |   6 +-
 hw/ppc/mpc8544ds.c                      |   9 +-
 hw/sd/sdhci-internal.h                  |  20 ---
 hw/sd/sdhci.c                           | 210 +++++++++++++++++++++---
 include/hw/sd/sdhci.h                   |   3 +
 14 files changed, 368 insertions(+), 70 deletions(-)

-- 
2.37.3

[PATCH v2 01/13] 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>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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 v2 02/13] 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>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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 v2 03/13] 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>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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 v2 04/13] hw/ppc/e500: Reduce usage of sysbus API

[Bernhard Beschow]

PlatformBusDevice has an mmio attribute which gets aliased to
SysBusDevice::mmio[0]. So PlatformbusDevice::mmio can be used directly,
avoiding the sysbus API.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/e500.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 32495d0123..496c61b612 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -1023,7 +1023,7 @@ void ppce500_init(MachineState *machine)
 
         memory_region_add_subregion(address_space_mem,
                                     pmc->platform_bus_base,
-                                    sysbus_mmio_get_region(s, 0));
+                                    &pms->pbus_dev->mmio);
     }
 
     /*
-- 
2.37.3

[PATCH v2 05/13] hw/ppc/mpc8544ds: Rename wrongly named method

[Bernhard Beschow]

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

diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 81177505f0..8e674ad195 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -36,7 +36,7 @@ static void mpc8544ds_init(MachineState *machine)
     ppce500_init(machine);
 }
 
-static void e500plat_machine_class_init(ObjectClass *oc, void *data)
+static void mpc8544ds_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
     PPCE500MachineClass *pmc = PPCE500_MACHINE_CLASS(oc);
@@ -63,7 +63,7 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data)
 static const TypeInfo mpc8544ds_info = {
     .name          = TYPE_MPC8544DS_MACHINE,
     .parent        = TYPE_PPCE500_MACHINE,
-    .class_init    = e500plat_machine_class_init,
+    .class_init    = mpc8544ds_machine_class_init,
 };
 
 static void mpc8544ds_register_types(void)
-- 
2.37.3

[PATCH v2 06/13] hw/ppc/mpc8544ds: Add platform bus

[Bernhard Beschow]

Models the real device more closely.

Address and size values are taken from mpc8544.dts from the linux-5.17.7
tree. The IRQ range is taken from e500plat.c.

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 8e674ad195..9c81477698 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 mpc8544ds_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 = 0xFF800000ULL;
+    pmc->platform_bus_size = 8 * 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 v2 07/13] 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
is always true.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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 496c61b612..3e950ea3ba 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,
-                                    &pms->pbus_dev->mmio);
+    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,
+                                &pms->pbus_dev->mmio);
+
     /*
      * 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 9c81477698..7dd5219736 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -46,7 +46,6 @@ static void mpc8544ds_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 = 0xFF800000ULL;
     pmc->platform_bus_size = 8 * MiB;
     pmc->platform_bus_first_irq = 5;
-- 
2.37.3

[PATCH v2 08/13] 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>
Reviewed-by: Bin Meng <bmeng.cn@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 v2 09/13] hw/ppc/e500: Implement pflash handling

[Bernhard Beschow]

Allows e500 boards to have their root file system reside on flash using
only builtin devices located in the eLBC memory region.

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               | 76 +++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+)

diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
index ba6bcb7314..1ed6c36599 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-ppc{64|32} -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 3e950ea3ba..2b1430fca4 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,31 @@ static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
     }
 }
 
+static void create_devtree_flash(SysBusDevice *sbdev,
+                                 PlatformDevtreeData *data)
+{
+    g_autofree char *name = NULL;
+    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_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);
+}
+
 static void platform_bus_create_devtree(PPCE500MachineState *pms,
                                         void *fdt, const char *mpic)
 {
@@ -276,6 +303,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 +331,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 +892,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 +1061,45 @@ void ppce500_init(MachineState *machine)
                                 pmc->platform_bus_base,
                                 &pms->pbus_dev->mmio);
 
+    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);
+        uint64_t mmio_size = pms->pbus_dev->mmio.size;
+        uint32_t sector_len = 64 * KiB;
+
+        if (ctpop64(size) != 1) {
+            error_report("Size of pflash file must be a power of two.");
+            exit(1);
+        }
+
+        if (size > mmio_size) {
+            error_report("Size of pflash file must not be bigger than %" PRIu64
+                         " bytes.", mmio_size);
+            exit(1);
+        }
+
+        assert(QEMU_IS_ALIGNED(size, sector_len));
+
+        dev = qdev_new(TYPE_PFLASH_CFI01);
+        qdev_prop_set_drive(dev, "drive", blk);
+        qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
+        qdev_prop_set_uint64(dev, "sector-length", sector_len);
+        qdev_prop_set_uint8(dev, "width", 2);
+        qdev_prop_set_bit(dev, "big-endian", true);
+        qdev_prop_set_uint16(dev, "id0", 0x89);
+        qdev_prop_set_uint16(dev, "id1", 0x18);
+        qdev_prop_set_uint16(dev, "id2", 0x0000);
+        qdev_prop_set_uint16(dev, "id3", 0x0);
+        qdev_prop_set_string(dev, "name", "e500.flash");
+        s = SYS_BUS_DEVICE(dev);
+        sysbus_realize_and_unref(s, &error_fatal);
+
+        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
+                                    sysbus_mmio_get_region(s, 0));
+    }
+
     /*
      * Smart firmware defaults ahead!
      *
-- 
2.37.3

[PATCH v2 10/13] 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>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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 v2 11/13] 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>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
 hw/sd/sdhci.c | 66 +++++++++++++++++++++++++--------------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 6da5e2c781..306070c872 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,7 +1768,7 @@ 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
-- 
2.37.3

[PATCH v2 12/13] 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 306070c872..ff92c6c4e5 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 --- */
+
+/* Watermark Level Register */
+#define ESDHC_WML                    0x44
+
+/* Host Controller Capabilities Register 2 */
+#define ESDHC_CAPABILITIES_1        0x114
+
+/* Control Register for DMA transfer */
+#define ESDHC_DMA_SYSCTL            0x40c
+
+#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_WML:
+    case ESDHC_DMA_SYSCTL:
+        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_WML:
+    case ESDHC_DMA_SYSCTL:
+        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 v2 13/13] 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               | 31 ++++++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
index 1ed6c36599..c758aa51eb 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-ppc{64|32} -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 2b1430fca4..379d89bf2d 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,22 @@ 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;
+    g_autofree char *name = NULL;
+
+    name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
+    qemu_fdt_add_subnode(fdt, name);
+    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");
+}
 
 typedef struct PlatformDevtreeData {
     void *fdt;
@@ -553,6 +573,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);
@@ -982,7 +1004,8 @@ void ppce500_init(MachineState *machine)
                        0, qdev_get_gpio_in(mpicdev, 42), 399193,
                        serial_hd(1), DEVICE_BIG_ENDIAN);
     }
-        /* I2C */
+
+    /* I2C */
     dev = qdev_new("mpc-i2c");
     s = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(s, &error_fatal);
@@ -992,6 +1015,12 @@ void ppce500_init(MachineState *machine)
     i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
     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");
-- 
2.37.3

Re: [PATCH v2 05/13] hw/ppc/mpc8544ds: Rename wrongly named method

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/ppc/mpc8544ds.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

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

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

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow 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>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>   hw/block/pflash_cfi01.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)

With CFI02 similarly fixed:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:
> 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               | 31 ++++++++++++++++++++++++++++++-
>   3 files changed, 44 insertions(+), 1 deletion(-)

> +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;

Why not pass these 3 variable as argument?

> +    g_autofree char *name = NULL;
> +
> +    name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
> +    qemu_fdt_add_subnode(fdt, name);
> +    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");
> +}
>   
>   typedef struct PlatformDevtreeData {
>       void *fdt;
> @@ -553,6 +573,8 @@ static int ppce500_load_device_tree(PPCE500MachineState *pms,
>   
>       dt_rtc_create(fdt, "i2c", "rtc");
>   
> +    /* sdhc */
> +    dt_sdhc_create(fdt, soc, mpic);
>   

Re: [PATCH v2 12/13] hw/sd/sdhci: Implement Freescale eSDHC device model

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow 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(-)

> +/* --- qdev Freescale eSDHC --- */
> +
> +/* Watermark Level Register */
> +#define ESDHC_WML                    0x44
> +
> +/* Host Controller Capabilities Register 2 */
> +#define ESDHC_CAPABILITIES_1        0x114

Not used?

> +
> +/* Control Register for DMA transfer */
> +#define ESDHC_DMA_SYSCTL            0x40c
> +
> +#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;

Isn't it already enforced by esdhc_mmio_ops.valid.unaligned = false?

> +    }
> +
> +    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_WML:
> +    case ESDHC_DMA_SYSCTL:
> +        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_WML:
> +    case ESDHC_DMA_SYSCTL:
> +        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,
> +};

Re: [PATCH v2 09/13] hw/ppc/e500: Implement pflash handling

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:
> Allows e500 boards to have their root file system reside on flash using
> only builtin devices located in the eLBC memory region.
> 
> 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               | 76 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 89 insertions(+)

> @@ -856,6 +892,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 +1061,45 @@ void ppce500_init(MachineState *machine)
>                                   pmc->platform_bus_base,
>                                   &pms->pbus_dev->mmio);
>   
> +    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);
> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;
> +        uint32_t sector_len = 64 * KiB;
> +
> +        if (ctpop64(size) != 1) {
> +            error_report("Size of pflash file must be a power of two.");

This is a PFLASH restriction (which you already fixed in the previous
patch), not a board one.

> +            exit(1);
> +        }
> +
> +        if (size > mmio_size) {
> +            error_report("Size of pflash file must not be bigger than %" PRIu64
> +                         " bytes.", mmio_size);

There is no hardware limitation here, you can wire flash bigger than the
memory aperture. What is above the aperture will simply be ignored.

Should we display a warning here instead of a fatal error?

> +            exit(1);
> +        }
> +
> +        assert(QEMU_IS_ALIGNED(size, sector_len));

Similarly, this doesn't seem a problem the board code should worry
about: better to defer it to PFLASH realize().

> +        dev = qdev_new(TYPE_PFLASH_CFI01);
> +        qdev_prop_set_drive(dev, "drive", blk);
> +        qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
> +        qdev_prop_set_uint64(dev, "sector-length", sector_len);
> +        qdev_prop_set_uint8(dev, "width", 2);
> +        qdev_prop_set_bit(dev, "big-endian", true);
> +        qdev_prop_set_uint16(dev, "id0", 0x89);
> +        qdev_prop_set_uint16(dev, "id1", 0x18);
> +        qdev_prop_set_uint16(dev, "id2", 0x0000);
> +        qdev_prop_set_uint16(dev, "id3", 0x0);
> +        qdev_prop_set_string(dev, "name", "e500.flash");
> +        s = SYS_BUS_DEVICE(dev);
> +        sysbus_realize_and_unref(s, &error_fatal);
> +
> +        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
> +                                    sysbus_mmio_get_region(s, 0));
> +    }
> +
>       /*
>        * Smart firmware defaults ahead!
>        *

Re: [PATCH v2 04/13] hw/ppc/e500: Reduce usage of sysbus API

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:
> PlatformBusDevice has an mmio attribute which gets aliased to
> SysBusDevice::mmio[0]. So PlatformbusDevice::mmio can be used directly,
> avoiding the sysbus API.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/ppc/e500.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

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

[Philippe Mathieu-Daudé via]

Hi Daniel,

On 3/10/22 22:31, Bernhard Beschow wrote:
> Cover letter:
> ~~~~~~~~~~~~~
> 
> 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.

> Bernhard Beschow (13):
>    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/e500: Reduce usage of sysbus API
>    hw/ppc/mpc8544ds: Rename wrongly named method
>    hw/ppc/mpc8544ds: Add platform bus
>    hw/ppc/e500: Remove if statement which is now always true

This first part is mostly reviewed and can already go via your
ppc-next queue.

>    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

This second part still need work. I can take it via the sdmmc-next
queue.

Regards,

Phil.

Re: [PATCH v2 07/13] hw/ppc/e500: Remove if statement which is now always true

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:
> Now that the MPC8544DS board also has a platform bus, the if statement
> is always true.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   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(-)

>       /* 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);

Should we abort(pmc->platform_bus_size > 0) now?

> +    pms->pbus_dev = PLATFORM_BUS_DEVICE(dev);
>   
> -        memory_region_add_subregion(address_space_mem,
> -                                    pmc->platform_bus_base,
> -                                    &pms->pbus_dev->mmio);
> +    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,
> +                                &pms->pbus_dev->mmio);

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

[Daniel Henrique Barboza]

Hey,

On 10/3/22 18:27, Philippe Mathieu-Daudé wrote:
> Hi Daniel,
> 
> On 3/10/22 22:31, Bernhard Beschow wrote:
>> Cover letter:
>> ~~~~~~~~~~~~~
>>
>> 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.
> 
>> Bernhard Beschow (13):
>>    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/e500: Reduce usage of sysbus API
>>    hw/ppc/mpc8544ds: Rename wrongly named method
>>    hw/ppc/mpc8544ds: Add platform bus
>>    hw/ppc/e500: Remove if statement which is now always true
> 
> This first part is mostly reviewed and can already go via your
> ppc-next queue.

We're missing an ACK in patch 6/13:

hw/ppc/mpc8544ds: Add platform bus

I'll need some time to understand what's been doing there to provide my own
R-b. Or you can toss a R-b there :D


Thanks,


Daniel



> 
>>    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
> 
> This second part still need work. I can take it via the sdmmc-next
> queue.
> 
> Regards,
> 
> Phil.

Re: [PATCH v2 09/13] hw/ppc/e500: Implement pflash handling

[Bernhard Beschow]

Am 3. Oktober 2022 21:21:15 UTC schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>:
>On 3/10/22 22:31, Bernhard Beschow wrote:
>> Allows e500 boards to have their root file system reside on flash using
>> only builtin devices located in the eLBC memory region.
>> 
>> 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               | 76 +++++++++++++++++++++++++++++++++++++
>>   3 files changed, 89 insertions(+)
>
>> @@ -856,6 +892,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 +1061,45 @@ void ppce500_init(MachineState *machine)
>>                                   pmc->platform_bus_base,
>>                                   &pms->pbus_dev->mmio);
>>   +    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);
>> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;
>> +        uint32_t sector_len = 64 * KiB;
>> +
>> +        if (ctpop64(size) != 1) {
>> +            error_report("Size of pflash file must be a power of two.");
>
>This is a PFLASH restriction (which you already fixed in the previous
>patch), not a board one.

I agree that this check seems redundant to the one in cfi01. I added this one for clearer error messages since cfi01 only complains about the "device size" not being a power of two while this message at least gives a hint towards the source of the problem (the file given in the pflash option).

Usually the size of the pflash area is hardcoded in the board while I choose to derive it from the size of the backing file in order to avoid hardcoding it. My idea is to put users into control by offering more flexibility.

>
>> +            exit(1);
>> +        }
>> +
>> +        if (size > mmio_size) {
>> +            error_report("Size of pflash file must not be bigger than %" PRIu64
>> +                         " bytes.", mmio_size);
>
>There is no hardware limitation here, you can wire flash bigger than the
>memory aperture. What is above the aperture will simply be ignored.
>
>Should we display a warning here instead of a fatal error?

While this is technically possible, is that what users would expect? Couldn't we just require users to truncate their files if they really want the "aperture" behavior?

>
>> +            exit(1);
>> +        }
>> +
>> +        assert(QEMU_IS_ALIGNED(size, sector_len));
>
>Similarly, this doesn't seem a problem the board code should worry
>about: better to defer it to PFLASH realize().

The reason for the assert() here is that size isn't stored directly in the cfi01 device. Instead, it must be calculated by the properties "num-blocks" times "sector-length". For this to work, size must be divisible by sector_len without remainder, which is checked by the assertion.

We could theoretically add a "size" property which would violate the single point of truth principle, though. Do you see a different solution?

Best regards,
Bernhard

>
>> +        dev = qdev_new(TYPE_PFLASH_CFI01);
>> +        qdev_prop_set_drive(dev, "drive", blk);
>> +        qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
>> +        qdev_prop_set_uint64(dev, "sector-length", sector_len);
>> +        qdev_prop_set_uint8(dev, "width", 2);
>> +        qdev_prop_set_bit(dev, "big-endian", true);
>> +        qdev_prop_set_uint16(dev, "id0", 0x89);
>> +        qdev_prop_set_uint16(dev, "id1", 0x18);
>> +        qdev_prop_set_uint16(dev, "id2", 0x0000);
>> +        qdev_prop_set_uint16(dev, "id3", 0x0);
>> +        qdev_prop_set_string(dev, "name", "e500.flash");
>> +        s = SYS_BUS_DEVICE(dev);
>> +        sysbus_realize_and_unref(s, &error_fatal);
>> +
>> +        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
>> +                                    sysbus_mmio_get_region(s, 0));
>> +    }
>> +
>>       /*
>>        * Smart firmware defaults ahead!
>>        *
>

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

[Bernhard Beschow]

Am 3. Oktober 2022 21:06:57 UTC schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>:
>On 3/10/22 22:31, Bernhard Beschow wrote:
>> 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               | 31 ++++++++++++++++++++++++++++++-
>>   3 files changed, 44 insertions(+), 1 deletion(-)
>
>> +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;
>
>Why not pass these 3 variable as argument?

In anticipation of data-driven board creation, I'd ideally infer those from the device's QOM properties. This seems similar to what Mark suggested in the BoF at KVM Forum [1], where -- IIUC -- he stated that QOM properties could be the foundation of all wiring representations. And device tree seems just like one specialized representation to me. (Note that I'm slightly hijacking the review here because I don't know where and how to express these thoughts elsewhere).

Does it make sense to add the missing properties here?

Best regards,
Bernhard

[1] https://etherpad.opendev.org/p/qemu-emulation-bof%40kvmforum2022

>
>> +    g_autofree char *name = NULL;
>> +
>> +    name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
>> +    qemu_fdt_add_subnode(fdt, name);
>> +    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");
>> +}
>>     typedef struct PlatformDevtreeData {
>>       void *fdt;
>> @@ -553,6 +573,8 @@ static int ppce500_load_device_tree(PPCE500MachineState *pms,
>>         dt_rtc_create(fdt, "i2c", "rtc");
>>   +    /* sdhc */
>> +    dt_sdhc_create(fdt, soc, mpic);
>>   

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

[Bernhard Beschow]

Am 4. Oktober 2022 12:43:35 UTC schrieb Daniel Henrique Barboza <danielhb413@gmail.com>:
>Hey,
>
>On 10/3/22 18:27, Philippe Mathieu-Daudé wrote:
>> Hi Daniel,
>> 
>> On 3/10/22 22:31, Bernhard Beschow wrote:
>>> Cover letter:
>>> ~~~~~~~~~~~~~
>>> 
>>> 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.
>> 
>>> Bernhard Beschow (13):
>>>    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/e500: Reduce usage of sysbus API
>>>    hw/ppc/mpc8544ds: Rename wrongly named method
>>>    hw/ppc/mpc8544ds: Add platform bus
>>>    hw/ppc/e500: Remove if statement which is now always true
>> 
>> This first part is mostly reviewed and can already go via your
>> ppc-next queue.
>
>We're missing an ACK in patch 6/13:
>
>hw/ppc/mpc8544ds: Add platform bus

Bin: Ping?

Best regards,
Bernhard
>
>I'll need some time to understand what's been doing there to provide my own
>R-b. Or you can toss a R-b there :D
>
>
>Thanks,
>
>
>Daniel
>
>
>
>> 
>>>    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
>> 
>> This second part still need work. I can take it via the sdmmc-next
>> queue.
>> 
>> Regards,
>> 
>> Phil.

Re: [PATCH v2 04/13] hw/ppc/e500: Reduce usage of sysbus API

[Bin Meng]

On Tue, Oct 4, 2022 at 5:24 AM Bernhard Beschow <shentey@gmail.com> wrote:
>
> PlatformBusDevice has an mmio attribute which gets aliased to
> SysBusDevice::mmio[0]. So PlatformbusDevice::mmio can be used directly,
> avoiding the sysbus API.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/ppc/e500.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

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

Re: [PATCH v2 05/13] hw/ppc/mpc8544ds: Rename wrongly named method

[Bin Meng]

On Tue, Oct 4, 2022 at 5:15 AM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/ppc/mpc8544ds.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

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

Re: [PATCH v2 06/13] hw/ppc/mpc8544ds: Add platform bus

[Bin Meng]

On Tue, Oct 4, 2022 at 5:22 AM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Models the real device more closely.
>
> Address and size values are taken from mpc8544.dts from the linux-5.17.7
> tree. The IRQ range is taken from e500plat.c.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/ppc/mpc8544ds.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>

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

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

[Bin Meng]

On Sun, Oct 9, 2022 at 12:11 AM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Am 4. Oktober 2022 12:43:35 UTC schrieb Daniel Henrique Barboza <danielhb413@gmail.com>:
> >Hey,
> >
> >On 10/3/22 18:27, Philippe Mathieu-Daudé wrote:
> >> Hi Daniel,
> >>
> >> On 3/10/22 22:31, Bernhard Beschow wrote:
> >>> Cover letter:
> >>> ~~~~~~~~~~~~~
> >>>
> >>> 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.
> >>
> >>> Bernhard Beschow (13):
> >>>    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/e500: Reduce usage of sysbus API
> >>>    hw/ppc/mpc8544ds: Rename wrongly named method
> >>>    hw/ppc/mpc8544ds: Add platform bus
> >>>    hw/ppc/e500: Remove if statement which is now always true
> >>
> >> This first part is mostly reviewed and can already go via your
> >> ppc-next queue.
> >
> >We're missing an ACK in patch 6/13:
> >
> >hw/ppc/mpc8544ds: Add platform bus
>
> Bin: Ping?
>

Sorry for the delay. I have provided the R-b to this patch.

Regards,
Bin

Re: [PATCH v2 09/13] hw/ppc/e500: Implement pflash handling

[Bin Meng]

On Tue, Oct 4, 2022 at 5:40 AM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Allows e500 boards to have their root file system reside on flash using
> only builtin devices located in the eLBC memory region.
>
> 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               | 76 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 89 insertions(+)
>
> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
> index ba6bcb7314..1ed6c36599 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-ppc{64|32} -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"

Could we add a separate sub-section "pflash" after the "networking"
part you did before?

> +
>  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 3e950ea3ba..2b1430fca4 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,31 @@ static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
>      }
>  }
>
> +static void create_devtree_flash(SysBusDevice *sbdev,
> +                                 PlatformDevtreeData *data)
> +{
> +    g_autofree char *name = NULL;
> +    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_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);
> +}
> +
>  static void platform_bus_create_devtree(PPCE500MachineState *pms,
>                                          void *fdt, const char *mpic)
>  {
> @@ -276,6 +303,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 +331,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 +892,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 +1061,45 @@ void ppce500_init(MachineState *machine)
>                                  pmc->platform_bus_base,
>                                  &pms->pbus_dev->mmio);
>
> +    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);
> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;
> +        uint32_t sector_len = 64 * KiB;
> +
> +        if (ctpop64(size) != 1) {
> +            error_report("Size of pflash file must be a power of two.");
> +            exit(1);
> +        }
> +
> +        if (size > mmio_size) {
> +            error_report("Size of pflash file must not be bigger than %" PRIu64
> +                         " bytes.", mmio_size);
> +            exit(1);
> +        }
> +
> +        assert(QEMU_IS_ALIGNED(size, sector_len));
> +
> +        dev = qdev_new(TYPE_PFLASH_CFI01);
> +        qdev_prop_set_drive(dev, "drive", blk);
> +        qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
> +        qdev_prop_set_uint64(dev, "sector-length", sector_len);
> +        qdev_prop_set_uint8(dev, "width", 2);
> +        qdev_prop_set_bit(dev, "big-endian", true);
> +        qdev_prop_set_uint16(dev, "id0", 0x89);
> +        qdev_prop_set_uint16(dev, "id1", 0x18);
> +        qdev_prop_set_uint16(dev, "id2", 0x0000);
> +        qdev_prop_set_uint16(dev, "id3", 0x0);
> +        qdev_prop_set_string(dev, "name", "e500.flash");
> +        s = SYS_BUS_DEVICE(dev);
> +        sysbus_realize_and_unref(s, &error_fatal);
> +
> +        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
> +                                    sysbus_mmio_get_region(s, 0));
> +    }
> +
>      /*
>       * Smart firmware defaults ahead!
>       *

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

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

[Daniel Henrique Barboza]

On 10/9/22 00:30, Bin Meng wrote:
> On Sun, Oct 9, 2022 at 12:11 AM Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Am 4. Oktober 2022 12:43:35 UTC schrieb Daniel Henrique Barboza <danielhb413@gmail.com>:
>>> Hey,
>>>
>>> On 10/3/22 18:27, Philippe Mathieu-Daudé wrote:
>>>> Hi Daniel,
>>>>
>>>> On 3/10/22 22:31, Bernhard Beschow wrote:
>>>>> Cover letter:
>>>>> ~~~~~~~~~~~~~
>>>>>
>>>>> 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.
>>>>
>>>>> Bernhard Beschow (13):
>>>>>     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/e500: Reduce usage of sysbus API
>>>>>     hw/ppc/mpc8544ds: Rename wrongly named method
>>>>>     hw/ppc/mpc8544ds: Add platform bus
>>>>>     hw/ppc/e500: Remove if statement which is now always true
>>>>
>>>> This first part is mostly reviewed and can already go via your
>>>> ppc-next queue.
>>>
>>> We're missing an ACK in patch 6/13:
>>>
>>> hw/ppc/mpc8544ds: Add platform bus
>>
>> Bin: Ping?
>>
> 
> Sorry for the delay. I have provided the R-b to this patch.

Thanks for the review.

Patches 1-7 queued in gitlab.com/danielhb/qemu/tree/ppc-next.


Daniel

> 
> Regards,
> Bin

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

[Bernhard Beschow]

Am 3. Oktober 2022 21:06:57 UTC schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>:
>On 3/10/22 22:31, Bernhard Beschow wrote:
>> 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               | 31 ++++++++++++++++++++++++++++++-
>>   3 files changed, 44 insertions(+), 1 deletion(-)
>
>> +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;
>
>Why not pass these 3 variable as argument?

Besides looking for a way to derive these parameters from QOM properties I wanted to keep the code consistent to existing one, e.g. dt_i2c_create().

Best regards,
Bernhard
>
>> +    g_autofree char *name = NULL;
>> +
>> +    name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
>> +    qemu_fdt_add_subnode(fdt, name);
>> +    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");
>> +}
>>     typedef struct PlatformDevtreeData {
>>       void *fdt;
>> @@ -553,6 +573,8 @@ static int ppce500_load_device_tree(PPCE500MachineState *pms,
>>         dt_rtc_create(fdt, "i2c", "rtc");
>>   +    /* sdhc */
>> +    dt_sdhc_create(fdt, soc, mpic);
>>