All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup
@ 2022-10-16 12:27 Bernhard Beschow
  2022-10-16 12:27 ` [PATCH v3 1/9] hw/block/pflash_cfi0{1, 2}: Error out if device length isn't a power of two Bernhard Beschow
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz, 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-6 perform some general cleanup which paves the way for the rest of
the series.

Patch 7 adds -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 8 and 9 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.

v3:
~~~
Phil:
- Also add power-of-2 fix to pflash_cfi02
- Resolve cfi01-specific assertion in e500 code
- Resolve unused define in eSDHC device model
- Resolve redundant alignment checks in eSDHC device model

Bin:
- Add dedicated flash chapter to documentation

Bernhard:
- Use is_power_of_2() instead of ctpop64() for better readability
- Only instantiate eSDHC device model in ppce500 (not used in MPC8544DS)
- Rebase onto gitlab.com/danielhb/qemu/tree/ppc-next

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 (9):
  hw/block/pflash_cfi0{1,2}: Error out if device length isn't a power of
    two
  hw/{arm,ppc}: Resolve unreachable code
  hw/block/pflash_cfi01: Attach memory region in boards
  hw/block/pflash_cfi02: Attach memory region in boards
  hw/sd/sdhci-internal: Unexport ESDHC defines
  hw/sd/sdhci: Rename ESDHC_* defines to USDHC_*
  hw/ppc/e500: Implement pflash handling
  hw/sd/sdhci: Implement Freescale eSDHC device model
  hw/ppc/e500: Add Freescale eSDHC to e500plat

 docs/system/ppc/ppce500.rst              |  28 ++++
 hw/arm/collie.c                          |  20 ++-
 hw/arm/digic_boards.c                    |  16 +-
 hw/arm/gumstix.c                         |  24 +--
 hw/arm/mainstone.c                       |  15 +-
 hw/arm/musicpal.c                        |  15 +-
 hw/arm/omap_sx1.c                        |  25 ++--
 hw/arm/versatilepb.c                     |  14 +-
 hw/arm/xilinx_zynq.c                     |  12 +-
 hw/arm/z2.c                              |  12 +-
 hw/block/pflash_cfi01.c                  |  12 +-
 hw/block/pflash_cfi02.c                  |  14 +-
 hw/microblaze/petalogix_ml605_mmu.c      |  16 +-
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  10 +-
 hw/mips/malta.c                          |   4 +-
 hw/ppc/Kconfig                           |   2 +
 hw/ppc/e500.c                            |  97 +++++++++++-
 hw/ppc/e500.h                            |   1 +
 hw/ppc/e500plat.c                        |   1 +
 hw/ppc/sam460ex.c                        |  19 ++-
 hw/ppc/virtex_ml507.c                    |   5 +-
 hw/sd/sdhci-internal.h                   |  20 ---
 hw/sd/sdhci.c                            | 183 ++++++++++++++++++++---
 hw/sh4/r2d.c                             |  11 +-
 include/hw/block/flash.h                 |   7 +-
 include/hw/sd/sdhci.h                    |   3 +
 26 files changed, 433 insertions(+), 153 deletions(-)

-- 
2.38.0



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 1/9] hw/block/pflash_cfi0{1, 2}: Error out if device length isn't a power of two
  2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
@ 2022-10-16 12:27 ` Bernhard Beschow
  2022-10-16 12:27 ` [PATCH v3 2/9] hw/{arm,ppc}: Resolve unreachable code Bernhard Beschow
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz, Bernhard Beschow, Bin Meng

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>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/block/pflash_cfi01.c | 8 ++++++--
 hw/block/pflash_cfi02.c | 5 +++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 0cbc2fb4cb..9c235bf66e 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 (!is_power_of_2(device_len)) {
+        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)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 2a99b286b0..ff2fe154c1 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -880,6 +880,11 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (!is_power_of_2(pfl->chip_len)) {
+        error_setg(errp, "Device size must be a power of two.");
+        return;
+    }
+
     memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
                                   &pflash_cfi02_ops, pfl, pfl->name,
                                   pfl->chip_len, errp);
-- 
2.38.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 2/9] hw/{arm,ppc}: Resolve unreachable code
  2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
  2022-10-16 12:27 ` [PATCH v3 1/9] hw/block/pflash_cfi0{1, 2}: Error out if device length isn't a power of two Bernhard Beschow
@ 2022-10-16 12:27 ` Bernhard Beschow
  2022-10-17 20:57   ` Philippe Mathieu-Daudé
  2022-10-16 12:27 ` [PATCH v3 3/9] hw/block/pflash_cfi01: Attach memory region in boards Bernhard Beschow
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz, Bernhard Beschow

pflash_cfi01_register() always returns with a non-NULL pointer (otherwise
it would crash internally). Therefore, the bodies of the if-statements
are unreachable.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/arm/gumstix.c     | 18 ++++++------------
 hw/arm/mainstone.c   | 13 +++++--------
 hw/arm/omap_sx1.c    | 22 ++++++++--------------
 hw/arm/versatilepb.c |  6 ++----
 hw/arm/z2.c          |  9 +++------
 hw/ppc/sam460ex.c    | 12 ++++--------
 6 files changed, 28 insertions(+), 52 deletions(-)

diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c
index 3a4bc332c4..1296628ed9 100644
--- a/hw/arm/gumstix.c
+++ b/hw/arm/gumstix.c
@@ -65,12 +65,9 @@ static void connex_init(MachineState *machine)
         exit(1);
     }
 
-    if (!pflash_cfi01_register(0x00000000, "connext.rom", connex_rom,
-                               dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                               sector_len, 2, 0, 0, 0, 0, 0)) {
-        error_report("Error registering flash memory");
-        exit(1);
-    }
+    pflash_cfi01_register(0x00000000, "connext.rom", connex_rom,
+                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                          sector_len, 2, 0, 0, 0, 0, 0);
 
     /* Interrupt line of NIC is connected to GPIO line 36 */
     smc91c111_init(&nd_table[0], 0x04000300,
@@ -95,12 +92,9 @@ static void verdex_init(MachineState *machine)
         exit(1);
     }
 
-    if (!pflash_cfi01_register(0x00000000, "verdex.rom", verdex_rom,
-                               dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                               sector_len, 2, 0, 0, 0, 0, 0)) {
-        error_report("Error registering flash memory");
-        exit(1);
-    }
+    pflash_cfi01_register(0x00000000, "verdex.rom", verdex_rom,
+                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                          sector_len, 2, 0, 0, 0, 0, 0);
 
     /* Interrupt line of NIC is connected to GPIO line 99 */
     smc91c111_init(&nd_table[0], 0x04000300,
diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
index 8454b65458..40f708f2d3 100644
--- a/hw/arm/mainstone.c
+++ b/hw/arm/mainstone.c
@@ -130,14 +130,11 @@ static void mainstone_common_init(MemoryRegion *address_space_mem,
     /* There are two 32MiB flash devices on the board */
     for (i = 0; i < 2; i ++) {
         dinfo = drive_get(IF_PFLASH, 0, i);
-        if (!pflash_cfi01_register(mainstone_flash_base[i],
-                                   i ? "mainstone.flash1" : "mainstone.flash0",
-                                   MAINSTONE_FLASH,
-                                   dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                                   sector_len, 4, 0, 0, 0, 0, 0)) {
-            error_report("Error registering flash memory");
-            exit(1);
-        }
+        pflash_cfi01_register(mainstone_flash_base[i],
+                              i ? "mainstone.flash1" : "mainstone.flash0",
+                              MAINSTONE_FLASH,
+                              dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                              sector_len, 4, 0, 0, 0, 0, 0);
     }
 
     mst_irq = sysbus_create_simple("mainstone-fpga", MST_FPGA_PHYS,
diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
index 57829b3744..820652265b 100644
--- a/hw/arm/omap_sx1.c
+++ b/hw/arm/omap_sx1.c
@@ -153,13 +153,10 @@ static void sx1_init(MachineState *machine, const int version)
 
     fl_idx = 0;
     if ((dinfo = drive_get(IF_PFLASH, 0, fl_idx)) != NULL) {
-        if (!pflash_cfi01_register(OMAP_CS0_BASE,
-                                   "omap_sx1.flash0-1", flash_size,
-                                   blk_by_legacy_dinfo(dinfo),
-                                   sector_size, 4, 0, 0, 0, 0, 0)) {
-            fprintf(stderr, "qemu: Error registering flash memory %d.\n",
-                           fl_idx);
-        }
+        pflash_cfi01_register(OMAP_CS0_BASE,
+                              "omap_sx1.flash0-1", flash_size,
+                              blk_by_legacy_dinfo(dinfo),
+                              sector_size, 4, 0, 0, 0, 0, 0);
         fl_idx++;
     }
 
@@ -175,13 +172,10 @@ static void sx1_init(MachineState *machine, const int version)
         memory_region_add_subregion(address_space,
                                 OMAP_CS1_BASE + flash1_size, &cs[1]);
 
-        if (!pflash_cfi01_register(OMAP_CS1_BASE,
-                                   "omap_sx1.flash1-1", flash1_size,
-                                   blk_by_legacy_dinfo(dinfo),
-                                   sector_size, 4, 0, 0, 0, 0, 0)) {
-            fprintf(stderr, "qemu: Error registering flash memory %d.\n",
-                           fl_idx);
-        }
+        pflash_cfi01_register(OMAP_CS1_BASE,
+                              "omap_sx1.flash1-1", flash1_size,
+                              blk_by_legacy_dinfo(dinfo),
+                              sector_size, 4, 0, 0, 0, 0, 0);
         fl_idx++;
     } else {
         memory_region_init_io(&cs[1], NULL, &static_ops, &cs1val,
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index ecc1f6cf74..43172d72ea 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -385,13 +385,11 @@ static void versatile_init(MachineState *machine, int board_id)
     /* 0x34000000 NOR Flash */
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    if (!pflash_cfi01_register(VERSATILE_FLASH_ADDR, "versatile.flash",
+    pflash_cfi01_register(VERSATILE_FLASH_ADDR, "versatile.flash",
                           VERSATILE_FLASH_SIZE,
                           dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
                           VERSATILE_FLASH_SECT_SIZE,
-                          4, 0x0089, 0x0018, 0x0000, 0x0, 0)) {
-        fprintf(stderr, "qemu: Error registering flash memory.\n");
-    }
+                          4, 0x0089, 0x0018, 0x0000, 0x0, 0);
 
     versatile_binfo.ram_size = machine->ram_size;
     versatile_binfo.board_id = board_id;
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index 9c1e876207..082ccc557e 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -311,12 +311,9 @@ static void z2_init(MachineState *machine)
     mpu = pxa270_init(address_space_mem, z2_binfo.ram_size, machine->cpu_type);
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    if (!pflash_cfi01_register(Z2_FLASH_BASE, "z2.flash0", Z2_FLASH_SIZE,
-                               dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                               sector_len, 4, 0, 0, 0, 0, 0)) {
-        error_report("Error registering flash memory");
-        exit(1);
-    }
+    pflash_cfi01_register(Z2_FLASH_BASE, "z2.flash0", Z2_FLASH_SIZE,
+                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                          sector_len, 4, 0, 0, 0, 0, 0);
 
     /* setup keypad */
     pxa27x_register_keypad(mpu->kp, map, 0x100);
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 850bb3b817..8089dd015b 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -111,14 +111,10 @@ static int sam460ex_load_uboot(void)
     DriveInfo *dinfo;
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    if (!pflash_cfi01_register(FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
-                               "sam460ex.flash", FLASH_SIZE,
-                               dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                               64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1)) {
-        error_report("Error registering flash memory");
-        /* XXX: return an error instead? */
-        exit(1);
-    }
+    pflash_cfi01_register(FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
+                          "sam460ex.flash", FLASH_SIZE,
+                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                          64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
 
     if (!dinfo) {
         /*error_report("No flash image given with the 'pflash' parameter,"
-- 
2.38.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 3/9] hw/block/pflash_cfi01: Attach memory region in boards
  2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
  2022-10-16 12:27 ` [PATCH v3 1/9] hw/block/pflash_cfi0{1, 2}: Error out if device length isn't a power of two Bernhard Beschow
  2022-10-16 12:27 ` [PATCH v3 2/9] hw/{arm,ppc}: Resolve unreachable code Bernhard Beschow
@ 2022-10-16 12:27 ` Bernhard Beschow
  2022-10-16 12:27 ` [PATCH v3 4/9] hw/block/pflash_cfi02: " Bernhard Beschow
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz, Bernhard Beschow

pflash_cfi01_register() had a parameter which was only passed to
sysbus_mmio_map() but not used otherwise. Pulling out sysbus_mmio_map()
resolves that parameter and concentrates the memory region setup in
board code. Furthermore, it allows attaching cfi01 devices relative to
some parent bus rather than to the global "sysbus".

While at it, replace sysbus_mmio_map() with non-sysbus equivalents.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/arm/collie.c                          | 20 +++++++++++++-------
 hw/arm/gumstix.c                         | 18 ++++++++++++------
 hw/arm/mainstone.c                       | 16 ++++++++++------
 hw/arm/omap_sx1.c                        | 19 +++++++++++--------
 hw/arm/versatilepb.c                     | 12 +++++++-----
 hw/arm/z2.c                              |  9 ++++++---
 hw/block/pflash_cfi01.c                  |  4 +---
 hw/microblaze/petalogix_ml605_mmu.c      | 16 ++++++++++------
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 10 ++++++----
 hw/mips/malta.c                          |  4 ++--
 hw/ppc/sam460ex.c                        | 15 +++++++++------
 hw/ppc/virtex_ml507.c                    |  5 ++++-
 include/hw/block/flash.h                 |  3 +--
 13 files changed, 92 insertions(+), 59 deletions(-)

diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index 8df31e2793..25fb5f657b 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -37,8 +37,10 @@ static struct arm_boot_info collie_binfo = {
 static void collie_init(MachineState *machine)
 {
     DriveInfo *dinfo;
+    PFlashCFI01 *pfl;
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     CollieMachineState *cms = COLLIE_MACHINE(machine);
+    MemoryRegion *system_memory = get_system_memory();
 
     if (machine->ram_size != mc->default_ram_size) {
         char *sz = size_to_str(mc->default_ram_size);
@@ -49,17 +51,21 @@ static void collie_init(MachineState *machine)
 
     cms->sa1110 = sa1110_init(machine->cpu_type);
 
-    memory_region_add_subregion(get_system_memory(), SA_SDCS0, machine->ram);
+    memory_region_add_subregion(system_memory, SA_SDCS0, machine->ram);
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    pflash_cfi01_register(SA_CS0, "collie.fl1", 0x02000000,
-                    dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                    64 * KiB, 4, 0x00, 0x00, 0x00, 0x00, 0);
+    pfl = pflash_cfi01_register("collie.fl1", 0x02000000,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                64 * KiB, 4, 0x00, 0x00, 0x00, 0x00, 0);
+    memory_region_add_subregion(system_memory, SA_CS0,
+                                pflash_cfi01_get_memory(pfl));
 
     dinfo = drive_get(IF_PFLASH, 0, 1);
-    pflash_cfi01_register(SA_CS1, "collie.fl2", 0x02000000,
-                    dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                    64 * KiB, 4, 0x00, 0x00, 0x00, 0x00, 0);
+    pfl = pflash_cfi01_register("collie.fl2", 0x02000000,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                64 * KiB, 4, 0x00, 0x00, 0x00, 0x00, 0);
+    memory_region_add_subregion(system_memory, SA_CS1,
+                                pflash_cfi01_get_memory(pfl));
 
     sysbus_create_simple("scoop", 0x40800000, NULL);
 
diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c
index 1296628ed9..d6c997ad8e 100644
--- a/hw/arm/gumstix.c
+++ b/hw/arm/gumstix.c
@@ -51,6 +51,7 @@ static void connex_init(MachineState *machine)
 {
     PXA2xxState *cpu;
     DriveInfo *dinfo;
+    PFlashCFI01 *pfl;
     MemoryRegion *address_space_mem = get_system_memory();
 
     uint32_t connex_rom = 0x01000000;
@@ -65,9 +66,11 @@ static void connex_init(MachineState *machine)
         exit(1);
     }
 
-    pflash_cfi01_register(0x00000000, "connext.rom", connex_rom,
-                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          sector_len, 2, 0, 0, 0, 0, 0);
+    pfl = pflash_cfi01_register("connext.rom", connex_rom,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                sector_len, 2, 0, 0, 0, 0, 0);
+    memory_region_add_subregion(address_space_mem, 0x00000000,
+                                pflash_cfi01_get_memory(pfl));
 
     /* Interrupt line of NIC is connected to GPIO line 36 */
     smc91c111_init(&nd_table[0], 0x04000300,
@@ -78,6 +81,7 @@ static void verdex_init(MachineState *machine)
 {
     PXA2xxState *cpu;
     DriveInfo *dinfo;
+    PFlashCFI01 *pfl;
     MemoryRegion *address_space_mem = get_system_memory();
 
     uint32_t verdex_rom = 0x02000000;
@@ -92,9 +96,11 @@ static void verdex_init(MachineState *machine)
         exit(1);
     }
 
-    pflash_cfi01_register(0x00000000, "verdex.rom", verdex_rom,
-                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          sector_len, 2, 0, 0, 0, 0, 0);
+    pfl = pflash_cfi01_register("verdex.rom", verdex_rom,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                sector_len, 2, 0, 0, 0, 0, 0);
+    memory_region_add_subregion(address_space_mem, 0x00000000,
+                                pflash_cfi01_get_memory(pfl));
 
     /* Interrupt line of NIC is connected to GPIO line 99 */
     smc91c111_init(&nd_table[0], 0x04000300,
diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
index 40f708f2d3..fbbaa4bf0c 100644
--- a/hw/arm/mainstone.c
+++ b/hw/arm/mainstone.c
@@ -116,7 +116,6 @@ static void mainstone_common_init(MemoryRegion *address_space_mem,
     hwaddr mainstone_flash_base[] = { MST_FLASH_0, MST_FLASH_1 };
     PXA2xxState *mpu;
     DeviceState *mst_irq;
-    DriveInfo *dinfo;
     int i;
     MemoryRegion *rom = g_new(MemoryRegion, 1);
 
@@ -129,12 +128,17 @@ static void mainstone_common_init(MemoryRegion *address_space_mem,
 
     /* There are two 32MiB flash devices on the board */
     for (i = 0; i < 2; i ++) {
+        DriveInfo *dinfo;
+        PFlashCFI01 *fl;
+
         dinfo = drive_get(IF_PFLASH, 0, i);
-        pflash_cfi01_register(mainstone_flash_base[i],
-                              i ? "mainstone.flash1" : "mainstone.flash0",
-                              MAINSTONE_FLASH,
-                              dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                              sector_len, 4, 0, 0, 0, 0, 0);
+        fl = pflash_cfi01_register(i ? "mainstone.flash1" : "mainstone.flash0",
+                                   MAINSTONE_FLASH,
+                                   dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                   sector_len, 4, 0, 0, 0, 0, 0);
+        memory_region_add_subregion(address_space_mem,
+                                    mainstone_flash_base[i],
+                                    pflash_cfi01_get_memory(fl));
     }
 
     mst_irq = sysbus_create_simple("mainstone-fpga", MST_FPGA_PHYS,
diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
index 820652265b..ce06455252 100644
--- a/hw/arm/omap_sx1.c
+++ b/hw/arm/omap_sx1.c
@@ -112,6 +112,7 @@ static void sx1_init(MachineState *machine, const int version)
     static uint32_t cs2val = 0x00001139;
     static uint32_t cs3val = 0x00001139;
     DriveInfo *dinfo;
+    PFlashCFI01 *pfl;
     int fl_idx;
     uint32_t flash_size = flash0_size;
 
@@ -153,10 +154,11 @@ static void sx1_init(MachineState *machine, const int version)
 
     fl_idx = 0;
     if ((dinfo = drive_get(IF_PFLASH, 0, fl_idx)) != NULL) {
-        pflash_cfi01_register(OMAP_CS0_BASE,
-                              "omap_sx1.flash0-1", flash_size,
-                              blk_by_legacy_dinfo(dinfo),
-                              sector_size, 4, 0, 0, 0, 0, 0);
+        pfl = pflash_cfi01_register("omap_sx1.flash0-1", flash_size,
+                                    blk_by_legacy_dinfo(dinfo),
+                                    sector_size, 4, 0, 0, 0, 0, 0);
+        memory_region_add_subregion(address_space, OMAP_CS0_BASE,
+                                    pflash_cfi01_get_memory(pfl));
         fl_idx++;
     }
 
@@ -172,10 +174,11 @@ static void sx1_init(MachineState *machine, const int version)
         memory_region_add_subregion(address_space,
                                 OMAP_CS1_BASE + flash1_size, &cs[1]);
 
-        pflash_cfi01_register(OMAP_CS1_BASE,
-                              "omap_sx1.flash1-1", flash1_size,
-                              blk_by_legacy_dinfo(dinfo),
-                              sector_size, 4, 0, 0, 0, 0, 0);
+        pfl = pflash_cfi01_register("omap_sx1.flash1-1", flash1_size,
+                                    blk_by_legacy_dinfo(dinfo),
+                                    sector_size, 4, 0, 0, 0, 0, 0);
+        memory_region_add_subregion(address_space, OMAP_CS1_BASE,
+                                    pflash_cfi01_get_memory(pfl));
         fl_idx++;
     } else {
         memory_region_init_io(&cs[1], NULL, &static_ops, &cs1val,
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 43172d72ea..6ab85e304a 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -196,6 +196,7 @@ static void versatile_init(MachineState *machine, int board_id)
     int n;
     int done_smc = 0;
     DriveInfo *dinfo;
+    PFlashCFI01 *pfl;
 
     if (machine->ram_size > 0x10000000) {
         /* Device starting at address 0x10000000,
@@ -385,11 +386,12 @@ static void versatile_init(MachineState *machine, int board_id)
     /* 0x34000000 NOR Flash */
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    pflash_cfi01_register(VERSATILE_FLASH_ADDR, "versatile.flash",
-                          VERSATILE_FLASH_SIZE,
-                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          VERSATILE_FLASH_SECT_SIZE,
-                          4, 0x0089, 0x0018, 0x0000, 0x0, 0);
+    pfl = pflash_cfi01_register("versatile.flash", VERSATILE_FLASH_SIZE,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                VERSATILE_FLASH_SECT_SIZE,
+                                4, 0x0089, 0x0018, 0x0000, 0x0, 0);
+    memory_region_add_subregion(sysmem, VERSATILE_FLASH_ADDR,
+                                pflash_cfi01_get_memory(pfl));
 
     versatile_binfo.ram_size = machine->ram_size;
     versatile_binfo.board_id = board_id;
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index 082ccc557e..79005cd171 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -303,6 +303,7 @@ static void z2_init(MachineState *machine)
     uint32_t sector_len = 0x10000;
     PXA2xxState *mpu;
     DriveInfo *dinfo;
+    PFlashCFI01 *pfl;
     void *z2_lcd;
     I2CBus *bus;
     DeviceState *wm;
@@ -311,9 +312,11 @@ static void z2_init(MachineState *machine)
     mpu = pxa270_init(address_space_mem, z2_binfo.ram_size, machine->cpu_type);
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    pflash_cfi01_register(Z2_FLASH_BASE, "z2.flash0", Z2_FLASH_SIZE,
-                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          sector_len, 4, 0, 0, 0, 0, 0);
+    pfl = pflash_cfi01_register("z2.flash0", Z2_FLASH_SIZE,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                sector_len, 4, 0, 0, 0, 0, 0);
+    memory_region_add_subregion(address_space_mem, Z2_FLASH_BASE,
+                                pflash_cfi01_get_memory(pfl));
 
     /* setup keypad */
     pxa27x_register_keypad(mpu->kp, map, 0x100);
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 9c235bf66e..25d70dc3c0 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -957,8 +957,7 @@ static void pflash_cfi01_register_types(void)
 
 type_init(pflash_cfi01_register_types)
 
-PFlashCFI01 *pflash_cfi01_register(hwaddr base,
-                                   const char *name,
+PFlashCFI01 *pflash_cfi01_register(const char *name,
                                    hwaddr size,
                                    BlockBackend *blk,
                                    uint32_t sector_len,
@@ -984,7 +983,6 @@ PFlashCFI01 *pflash_cfi01_register(hwaddr base,
     qdev_prop_set_string(dev, "name", name);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
     return PFLASH_CFI01(dev);
 }
 
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index a24fadddca..14450ad372 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -76,6 +76,7 @@ petalogix_ml605_init(MachineState *machine)
     MicroBlazeCPU *cpu;
     SysBusDevice *busdev;
     DriveInfo *dinfo;
+    PFlashCFI01 *pfl;
     int i;
     MemoryRegion *phys_lmb_bram = g_new(MemoryRegion, 1);
     MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
@@ -103,12 +104,15 @@ petalogix_ml605_init(MachineState *machine)
     memory_region_add_subregion(address_space_mem, MEMORY_BASEADDR, phys_ram);
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    /* 5th parameter 2 means bank-width
-     * 10th paremeter 0 means little-endian */
-    pflash_cfi01_register(FLASH_BASEADDR, "petalogix_ml605.flash", FLASH_SIZE,
-                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          64 * KiB, 2, 0x89, 0x18, 0x0000, 0x0, 0);
-
+    /*
+     * 4th parameter 2 means bank-width
+     * 9th paremeter 0 means little-endian
+     */
+    pfl = pflash_cfi01_register("petalogix_ml605.flash", FLASH_SIZE,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                64 * KiB, 2, 0x89, 0x18, 0x0000, 0x0, 0);
+    memory_region_add_subregion(address_space_mem, FLASH_BASEADDR,
+                                pflash_cfi01_get_memory(pfl));
 
     dev = qdev_new("xlnx.xps-intc");
     qdev_prop_set_uint32(dev, "kind-of-intr", 1 << TIMER_IRQ);
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 9d959d1ad8..a7eae72e02 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -62,6 +62,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
     DeviceState *dev;
     MicroBlazeCPU *cpu;
     DriveInfo *dinfo;
+    PFlashCFI01 *pfl;
     int i;
     hwaddr ddr_base = MEMORY_BASEADDR;
     MemoryRegion *phys_lmb_bram = g_new(MemoryRegion, 1);
@@ -84,10 +85,11 @@ petalogix_s3adsp1800_init(MachineState *machine)
     memory_region_add_subregion(sysmem, ddr_base, phys_ram);
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    pflash_cfi01_register(FLASH_BASEADDR,
-                          "petalogix_s3adsp1800.flash", FLASH_SIZE,
-                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
+    pfl = pflash_cfi01_register("petalogix_s3adsp1800.flash", FLASH_SIZE,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
+    memory_region_add_subregion(sysmem, FLASH_BASEADDR,
+                                pflash_cfi01_get_memory(pfl));
 
     dev = qdev_new("xlnx.xps-intc");
     qdev_prop_set_uint32(dev, "kind-of-intr",
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 0e932988e0..20407bd998 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1286,12 +1286,12 @@ void mips_malta_init(MachineState *machine)
 
     /* Load firmware in flash / BIOS. */
     dinfo = drive_get(IF_PFLASH, 0, fl_idx);
-    fl = pflash_cfi01_register(FLASH_ADDRESS, "mips_malta.bios",
-                               FLASH_SIZE,
+    fl = pflash_cfi01_register("mips_malta.bios", FLASH_SIZE,
                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
                                65536,
                                4, 0x0000, 0x0000, 0x0000, 0x0000, be);
     bios = pflash_cfi01_get_memory(fl);
+    memory_region_add_subregion(system_memory, FLASH_ADDRESS, bios);
     fl_idx++;
     if (kernel_filename) {
         ram_low_size = MIN(ram_size, 256 * MiB);
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 8089dd015b..6f4f9c7c4a 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -88,7 +88,7 @@ struct boot_info {
     uint32_t entry;
 };
 
-static int sam460ex_load_uboot(void)
+static int sam460ex_load_uboot(MemoryRegion *address_space_mem)
 {
     /*
      * This first creates 1MiB of flash memory mapped at the end of
@@ -109,12 +109,15 @@ static int sam460ex_load_uboot(void)
      */
 
     DriveInfo *dinfo;
+    PFlashCFI01 *pfl;
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    pflash_cfi01_register(FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
-                          "sam460ex.flash", FLASH_SIZE,
-                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
+    pfl = pflash_cfi01_register("sam460ex.flash", FLASH_SIZE,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
+    memory_region_add_subregion(address_space_mem,
+                                FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
+                                pflash_cfi01_get_memory(pfl));
 
     if (!dinfo) {
         /*error_report("No flash image given with the 'pflash' parameter,"
@@ -448,7 +451,7 @@ static void sam460ex_init(MachineState *machine)
 
     /* Load U-Boot image. */
     if (!machine->kernel_filename) {
-        success = sam460ex_load_uboot();
+        success = sam460ex_load_uboot(address_space_mem);
         if (success < 0) {
             error_report("could not load firmware");
             exit(1);
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 493ea0c19f..c98f1b2ab3 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -210,6 +210,7 @@ static void virtex_init(MachineState *machine)
     CPUPPCState *env;
     hwaddr ram_base = 0;
     DriveInfo *dinfo;
+    PFlashCFI01 *pfl;
     qemu_irq irq[32], cpu_irq;
     int kernel_size;
     int i;
@@ -229,9 +230,11 @@ static void virtex_init(MachineState *machine)
     memory_region_add_subregion(address_space_mem, ram_base, machine->ram);
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    pflash_cfi01_register(PFLASH_BASEADDR, "virtex.flash", FLASH_SIZE,
+    pfl = pflash_cfi01_register("virtex.flash", FLASH_SIZE,
                           dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
                           64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
+    memory_region_add_subregion(address_space_mem, PFLASH_BASEADDR,
+                                pflash_cfi01_get_memory(pfl));
 
     cpu_irq = qdev_get_gpio_in(DEVICE(cpu), PPC40x_INPUT_INT);
     dev = qdev_new("xlnx.xps-intc");
diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index 86d8363bb0..5f9ba18de1 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -12,8 +12,7 @@
 OBJECT_DECLARE_SIMPLE_TYPE(PFlashCFI01, PFLASH_CFI01)
 
 
-PFlashCFI01 *pflash_cfi01_register(hwaddr base,
-                                   const char *name,
+PFlashCFI01 *pflash_cfi01_register(const char *name,
                                    hwaddr size,
                                    BlockBackend *blk,
                                    uint32_t sector_len,
-- 
2.38.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 4/9] hw/block/pflash_cfi02: Attach memory region in boards
  2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
                   ` (2 preceding siblings ...)
  2022-10-16 12:27 ` [PATCH v3 3/9] hw/block/pflash_cfi01: Attach memory region in boards Bernhard Beschow
@ 2022-10-16 12:27 ` Bernhard Beschow
  2022-10-16 12:27 ` [PATCH v3 5/9] hw/sd/sdhci-internal: Unexport ESDHC defines Bernhard Beschow
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz, Bernhard Beschow

pflash_cfi02_register() had a parameter which was only passed to
sysbus_mmio_map() but not used otherwise. Pulling out sysbus_mmio_map()
resolves that parameter and concentrates the memory region setup in
board code. Furthermore, it allows attaching cfi02 devices relative to
some parent bus rather than to the global "sysbus".

While at it, replace sysbus_mmio_map() with non-sysbus equivalents.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/arm/digic_boards.c    | 16 ++++++++++------
 hw/arm/musicpal.c        | 15 +++++++++------
 hw/arm/xilinx_zynq.c     | 12 +++++++-----
 hw/block/pflash_cfi02.c  |  9 ++++++---
 hw/sh4/r2d.c             | 11 +++++++----
 include/hw/block/flash.h |  4 ++--
 6 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index 4093af09cb..d3c5426cf9 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -116,12 +116,16 @@ static void digic4_add_k8p3215uqb_rom(DigicState *s, hwaddr addr,
 #define FLASH_K8P3215UQB_SIZE (4 * 1024 * 1024)
 #define FLASH_K8P3215UQB_SECTOR_SIZE (64 * 1024)
 
-    pflash_cfi02_register(addr, "pflash", FLASH_K8P3215UQB_SIZE,
-                          NULL, FLASH_K8P3215UQB_SECTOR_SIZE,
-                          DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE,
-                          4,
-                          0x00EC, 0x007E, 0x0003, 0x0001,
-                          0x0555, 0x2aa, 0);
+    PFlashCFI02 *pfl;
+
+    pfl = pflash_cfi02_register("pflash", FLASH_K8P3215UQB_SIZE,
+                                NULL, FLASH_K8P3215UQB_SECTOR_SIZE,
+                                DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE,
+                                4,
+                                0x00EC, 0x007E, 0x0003, 0x0001,
+                                0x0555, 0x2aa, 0);
+    memory_region_add_subregion(get_system_memory(), addr,
+                                pflash_cfi02_get_memory(pfl));
 
     digic_load_rom(s, addr, FLASH_K8P3215UQB_SIZE, filename);
 }
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index b65c020115..efad741f6d 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1261,6 +1261,7 @@ static void musicpal_init(MachineState *machine)
     /* Register flash */
     dinfo = drive_get(IF_PFLASH, 0, 0);
     if (dinfo) {
+        PFlashCFI02 *pfl;
         BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
 
         flash_size = blk_getlength(blk);
@@ -1275,12 +1276,14 @@ static void musicpal_init(MachineState *machine)
          * 0xFF800000 (if there is 8 MB flash). So remap flash access if the
          * image is smaller than 32 MB.
          */
-        pflash_cfi02_register(0x100000000ULL - MP_FLASH_SIZE_MAX,
-                              "musicpal.flash", flash_size,
-                              blk, 0x10000,
-                              MP_FLASH_SIZE_MAX / flash_size,
-                              2, 0x00BF, 0x236D, 0x0000, 0x0000,
-                              0x5555, 0x2AAA, 0);
+        pfl = pflash_cfi02_register("musicpal.flash", flash_size,
+                                    blk, 0x10000,
+                                    MP_FLASH_SIZE_MAX / flash_size,
+                                    2, 0x00BF, 0x236D, 0x0000, 0x0000,
+                                    0x5555, 0x2AAA, 0);
+        memory_region_add_subregion(address_space_mem,
+                                    0x100000000ULL - MP_FLASH_SIZE_MAX,
+                                    pflash_cfi02_get_memory(pfl));
     }
     sysbus_create_simple(TYPE_MV88W8618_FLASHCFG, MP_FLASHCFG_BASE, NULL);
 
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 3190cc0b8d..a2abb1cf31 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -182,6 +182,7 @@ static void zynq_init(MachineState *machine)
     MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
     DeviceState *dev, *slcr;
     SysBusDevice *busdev;
+    PFlashCFI02 *pfl;
     qemu_irq pic[64];
     int n;
 
@@ -218,11 +219,12 @@ static void zynq_init(MachineState *machine)
     DriveInfo *dinfo = drive_get(IF_PFLASH, 0, 0);
 
     /* AMD */
-    pflash_cfi02_register(0xe2000000, "zynq.pflash", FLASH_SIZE,
-                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          FLASH_SECTOR_SIZE, 1,
-                          1, 0x0066, 0x0022, 0x0000, 0x0000, 0x0555, 0x2aa,
-                          0);
+    pfl = pflash_cfi02_register("zynq.pflash", FLASH_SIZE,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                FLASH_SECTOR_SIZE, 1, 1, 0x0066, 0x0022, 0x0000,
+                                0x0000, 0x0555, 0x2aa, 0);
+    memory_region_add_subregion(address_space_mem, 0xe2000000,
+                                pflash_cfi02_get_memory(pfl));
 
     /* Create the main clock source, and feed slcr with it */
     zynq_machine->ps_clk = CLOCK(object_new(TYPE_CLOCK));
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index ff2fe154c1..60039e0d52 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -999,8 +999,7 @@ static void pflash_cfi02_register_types(void)
 
 type_init(pflash_cfi02_register_types)
 
-PFlashCFI02 *pflash_cfi02_register(hwaddr base,
-                                   const char *name,
+PFlashCFI02 *pflash_cfi02_register(const char *name,
                                    hwaddr size,
                                    BlockBackend *blk,
                                    uint32_t sector_len,
@@ -1031,6 +1030,10 @@ PFlashCFI02 *pflash_cfi02_register(hwaddr base,
     qdev_prop_set_string(dev, "name", name);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
     return PFLASH_CFI02(dev);
 }
+
+MemoryRegion *pflash_cfi02_get_memory(PFlashCFI02 *fl)
+{
+    return sysbus_mmio_get_region(SYS_BUS_DEVICE(fl), 0);
+}
diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index 39fc4f19d9..0af8f0e137 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -239,6 +239,7 @@ static void r2d_init(MachineState *machine)
     MemoryRegion *sdram = g_new(MemoryRegion, 1);
     qemu_irq *irq;
     DriveInfo *dinfo;
+    PFlashCFI02 *pfl;
     int i;
     DeviceState *dev;
     SysBusDevice *busdev;
@@ -302,10 +303,12 @@ static void r2d_init(MachineState *machine)
      * addressable in words of 16bit.
      */
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    pflash_cfi02_register(0x0, "r2d.flash", FLASH_SIZE,
-                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          64 * KiB, 1, 2, 0x0001, 0x227e, 0x2220, 0x2200,
-                          0x555, 0x2aa, 0);
+    pfl = pflash_cfi02_register("r2d.flash", FLASH_SIZE,
+                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                                64 * KiB, 1, 2, 0x0001, 0x227e, 0x2220, 0x2200,
+                                0x555, 0x2aa, 0);
+    memory_region_add_subregion(get_system_memory(), 0x0,
+                                pflash_cfi02_get_memory(pfl));
 
     /* NIC: rtl8139 on-board, and 2 slots. */
     for (i = 0; i < nb_nics; i++)
diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index 5f9ba18de1..52d6bcd56a 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -30,8 +30,7 @@ void pflash_cfi01_legacy_drive(PFlashCFI01 *dev, DriveInfo *dinfo);
 OBJECT_DECLARE_SIMPLE_TYPE(PFlashCFI02, PFLASH_CFI02)
 
 
-PFlashCFI02 *pflash_cfi02_register(hwaddr base,
-                                   const char *name,
+PFlashCFI02 *pflash_cfi02_register(const char *name,
                                    hwaddr size,
                                    BlockBackend *blk,
                                    uint32_t sector_len,
@@ -42,6 +41,7 @@ PFlashCFI02 *pflash_cfi02_register(hwaddr base,
                                    uint16_t unlock_addr0,
                                    uint16_t unlock_addr1,
                                    int be);
+MemoryRegion *pflash_cfi02_get_memory(PFlashCFI02 *fl);
 
 /* nand.c */
 DeviceState *nand_init(BlockBackend *blk, int manf_id, int chip_id);
-- 
2.38.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 5/9] hw/sd/sdhci-internal: Unexport ESDHC defines
  2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
                   ` (3 preceding siblings ...)
  2022-10-16 12:27 ` [PATCH v3 4/9] hw/block/pflash_cfi02: " Bernhard Beschow
@ 2022-10-16 12:27 ` Bernhard Beschow
  2022-10-16 12:27 ` [PATCH v3 6/9] hw/sd/sdhci: Rename ESDHC_* defines to USDHC_* Bernhard Beschow
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz, Bernhard Beschow, Bin Meng

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.38.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 6/9] hw/sd/sdhci: Rename ESDHC_* defines to USDHC_*
  2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
                   ` (4 preceding siblings ...)
  2022-10-16 12:27 ` [PATCH v3 5/9] hw/sd/sdhci-internal: Unexport ESDHC defines Bernhard Beschow
@ 2022-10-16 12:27 ` Bernhard Beschow
  2022-10-16 12:27 ` [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling Bernhard Beschow
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz, Bernhard Beschow, Bin Meng

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.38.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling
  2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
                   ` (5 preceding siblings ...)
  2022-10-16 12:27 ` [PATCH v3 6/9] hw/sd/sdhci: Rename ESDHC_* defines to USDHC_* Bernhard Beschow
@ 2022-10-16 12:27 ` Bernhard Beschow
  2022-10-16 14:15   ` BALATON Zoltan
  2022-10-18  9:28   ` Peter Maydell
  2022-10-16 12:27 ` [PATCH v3 8/9] hw/sd/sdhci: Implement Freescale eSDHC device model Bernhard Beschow
                   ` (2 subsequent siblings)
  9 siblings, 2 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz, 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 | 16 ++++++++++
 hw/ppc/Kconfig              |  1 +
 hw/ppc/e500.c               | 62 +++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+)

diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
index ba6bcb7314..99d2c680d6 100644
--- a/docs/system/ppc/ppce500.rst
+++ b/docs/system/ppc/ppce500.rst
@@ -165,3 +165,19 @@ if “-device eTSEC” is given to QEMU:
 .. code-block:: bash
 
   -netdev tap,ifname=tap0,script=no,downscript=no,id=net0 -device eTSEC,netdev=net0
+
+Root file system on flash drive
+-------------------------------
+
+Rather than using a root file system on ram disk, it is possible to have it on
+CFI 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"
+
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..23d2c3451a 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,31 @@ 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;
+        PFlashCFI01 *pfl;
+
+        if (!is_power_of_2(size)) {
+            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);
+        }
+
+        pfl = pflash_cfi01_register("e500.flash", size, blk, 64 * KiB, 2,
+                                    0x89, 0x18, 0x0000, 0x0, 1);
+        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
+                                    pflash_cfi01_get_memory(pfl));
+    }
+
     /*
      * Smart firmware defaults ahead!
      *
-- 
2.38.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 8/9] hw/sd/sdhci: Implement Freescale eSDHC device model
  2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
                   ` (6 preceding siblings ...)
  2022-10-16 12:27 ` [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling Bernhard Beschow
@ 2022-10-16 12:27 ` Bernhard Beschow
  2022-10-16 12:27 ` [PATCH v3 9/9] hw/ppc/e500: Add Freescale eSDHC to e500plat Bernhard Beschow
  2022-10-16 12:41 ` [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
  9 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz, 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         | 120 +++++++++++++++++++++++++++++++++++++++++-
 include/hw/sd/sdhci.h |   3 ++
 2 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 306070c872..8d8ad9ff24 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,122 @@ static const TypeInfo sdhci_bus_info = {
     .class_init = sdhci_bus_class_init,
 };
 
+/* --- qdev Freescale eSDHC --- */
+
+/* Watermark Level Register */
+#define ESDHC_WML                    0x44
+
+/* 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;
+
+    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 @0x%02" HWADDR_PRIx
+                      " not implemented\n", offset);
+        break;
+
+    default:
+        ret = 0;
+        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd @0x%02" HWADDR_PRIx
+                      " unknown offset\n", offset);
+        break;
+    }
+
+    return ret;
+}
+
+static void esdhci_write(void *opaque, hwaddr offset, uint64_t val,
+                         unsigned size)
+{
+    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 @0x%02" HWADDR_PRIx " <- 0x%08lx "
+                      "not implemented\n", offset, val);
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr @0x%02" HWADDR_PRIx
+                      " <- 0x%08lx unknown offset\n", offset, val);
+        break;
+    }
+}
+
+static const MemoryRegionOps esdhc_mmio_ops = {
+    .read = esdhci_read,
+    .write = esdhci_write,
+    .valid = {
+        .min_access_size = 4,
+        .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 +2024,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.38.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 9/9] hw/ppc/e500: Add Freescale eSDHC to e500plat
  2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
                   ` (7 preceding siblings ...)
  2022-10-16 12:27 ` [PATCH v3 8/9] hw/sd/sdhci: Implement Freescale eSDHC device model Bernhard Beschow
@ 2022-10-16 12:27 ` Bernhard Beschow
  2022-10-16 12:41 ` [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
  9 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz, Bernhard Beschow

Adds missing functionality to e500plat machine 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 | 12 ++++++++++++
 hw/ppc/Kconfig              |  1 +
 hw/ppc/e500.c               | 35 ++++++++++++++++++++++++++++++++++-
 hw/ppc/e500.h               |  1 +
 hw/ppc/e500plat.c           |  1 +
 5 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
index 99d2c680d6..298ee9ee16 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
@@ -181,3 +182,14 @@ 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"
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 23d2c3451a..f43a21d8bb 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,10 @@ static int ppce500_load_device_tree(PPCE500MachineState *pms,
 
     dt_rtc_create(fdt, "i2c", "rtc");
 
+    /* sdhc */
+    if (pmc->has_esdhc) {
+        dt_sdhc_create(fdt, soc, mpic);
+    }
 
     gutil = g_strdup_printf("%s/global-utilities@%llx", soc,
                             MPC8544_UTIL_OFFSET);
@@ -982,7 +1006,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 +1017,14 @@ void ppce500_init(MachineState *machine)
     i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
     i2c_slave_create_simple(i2c, "ds1338", RTC_REGS_OFFSET);
 
+    /* eSDHC */
+    if (pmc->has_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");
diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
index 68f754ce50..8c09ef92e4 100644
--- a/hw/ppc/e500.h
+++ b/hw/ppc/e500.h
@@ -27,6 +27,7 @@ struct PPCE500MachineClass {
 
     int mpic_version;
     bool has_mpc8xxx_gpio;
+    bool has_esdhc;
     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 5bb1c603da..44bf874b0f 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -86,6 +86,7 @@ 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_esdhc = true;
     pmc->platform_bus_base = 0xf00000000ULL;
     pmc->platform_bus_size = 128 * MiB;
     pmc->platform_bus_first_irq = 5;
-- 
2.38.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup
  2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
                   ` (8 preceding siblings ...)
  2022-10-16 12:27 ` [PATCH v3 9/9] hw/ppc/e500: Add Freescale eSDHC to e500plat Bernhard Beschow
@ 2022-10-16 12:41 ` Bernhard Beschow
  9 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-16 12:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz

Am 16. Oktober 2022 12:27:28 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>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-6 perform some general cleanup which paves the way for the rest of
>
>the series.
>
>
>
>Patch 7 adds -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 8 and 9 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.
>
>
>
>v3:
>
>~~~
>
>Phil:
>
>- Also add power-of-2 fix to pflash_cfi02
>
>- Resolve cfi01-specific assertion in e500 code
>
>- Resolve unused define in eSDHC device model
>
>- Resolve redundant alignment checks in eSDHC device model
>
>
>
>Bin:
>
>- Add dedicated flash chapter to documentation
>
>
>
>Bernhard:
>
>- Use is_power_of_2() instead of ctpop64() for better readability
>
>- Only instantiate eSDHC device model in ppce500 (not used in MPC8544DS)
>
>- Rebase onto gitlab.com/danielhb/qemu/tree/ppc-next
- Move cfi0x memory region setup into board code to avoid cfi01-specific assertion there
- While at it, resolve unreachable code related to cfi01 device creation
- Reorder patches such that trivial patches come first

Best regards,
Bernhard

>
>
>
>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 (9):
>
>  hw/block/pflash_cfi0{1,2}: Error out if device length isn't a power of
>
>    two
>
>  hw/{arm,ppc}: Resolve unreachable code
>
>  hw/block/pflash_cfi01: Attach memory region in boards
>
>  hw/block/pflash_cfi02: Attach memory region in boards
>
>  hw/sd/sdhci-internal: Unexport ESDHC defines
>
>  hw/sd/sdhci: Rename ESDHC_* defines to USDHC_*
>
>  hw/ppc/e500: Implement pflash handling
>
>  hw/sd/sdhci: Implement Freescale eSDHC device model
>
>  hw/ppc/e500: Add Freescale eSDHC to e500plat
>
>
>
> docs/system/ppc/ppce500.rst              |  28 ++++
>
> hw/arm/collie.c                          |  20 ++-
>
> hw/arm/digic_boards.c                    |  16 +-
>
> hw/arm/gumstix.c                         |  24 +--
>
> hw/arm/mainstone.c                       |  15 +-
>
> hw/arm/musicpal.c                        |  15 +-
>
> hw/arm/omap_sx1.c                        |  25 ++--
>
> hw/arm/versatilepb.c                     |  14 +-
>
> hw/arm/xilinx_zynq.c                     |  12 +-
>
> hw/arm/z2.c                              |  12 +-
>
> hw/block/pflash_cfi01.c                  |  12 +-
>
> hw/block/pflash_cfi02.c                  |  14 +-
>
> hw/microblaze/petalogix_ml605_mmu.c      |  16 +-
>
> hw/microblaze/petalogix_s3adsp1800_mmu.c |  10 +-
>
> hw/mips/malta.c                          |   4 +-
>
> hw/ppc/Kconfig                           |   2 +
>
> hw/ppc/e500.c                            |  97 +++++++++++-
>
> hw/ppc/e500.h                            |   1 +
>
> hw/ppc/e500plat.c                        |   1 +
>
> hw/ppc/sam460ex.c                        |  19 ++-
>
> hw/ppc/virtex_ml507.c                    |   5 +-
>
> hw/sd/sdhci-internal.h                   |  20 ---
>
> hw/sd/sdhci.c                            | 183 ++++++++++++++++++++---
>
> hw/sh4/r2d.c                             |  11 +-
>
> include/hw/block/flash.h                 |   7 +-
>
> include/hw/sd/sdhci.h                    |   3 +
>
> 26 files changed, 433 insertions(+), 153 deletions(-)
>
>
>
>-- >
>2.38.0
>
>
>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling
  2022-10-16 12:27 ` [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling Bernhard Beschow
@ 2022-10-16 14:15   ` BALATON Zoltan
  2022-10-18  7:45     ` Bernhard Beschow
  2022-10-18  9:28   ` Peter Maydell
  1 sibling, 1 reply; 19+ messages in thread
From: BALATON Zoltan @ 2022-10-16 14:15 UTC (permalink / raw)
  To: Bernhard Beschow
  Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, Yoshinori Sato, Antony Pavlov,
	Hanna Reitz

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

On Sun, 16 Oct 2022, 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 | 16 ++++++++++
> hw/ppc/Kconfig              |  1 +
> hw/ppc/e500.c               | 62 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 79 insertions(+)
>
> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
> index ba6bcb7314..99d2c680d6 100644
> --- a/docs/system/ppc/ppce500.rst
> +++ b/docs/system/ppc/ppce500.rst
> @@ -165,3 +165,19 @@ if “-device eTSEC” is given to QEMU:
> .. code-block:: bash
>
>   -netdev tap,ifname=tap0,script=no,downscript=no,id=net0 -device eTSEC,netdev=net0
> +
> +Root file system on flash drive
> +-------------------------------
> +
> +Rather than using a root file system on ram disk, it is possible to have it on
> +CFI 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 \

We have qemu-system-ppc and qemu-system-ppc64 not qemu-system-ppc32 so 
maybe qemu-system-ppc[64] even though that looks odd so maybe just 
qemu-system-ppc and then people should know that ppc64 includes ppc config 
as well.

Regards,
BALATON Zoltan

> +      -display none -serial stdio \
> +      -kernel vmlinux \
> +      -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
> +      -append "rootwait root=/dev/mtdblock0"
> +
> 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..23d2c3451a 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,31 @@ 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;
> +        PFlashCFI01 *pfl;
> +
> +        if (!is_power_of_2(size)) {
> +            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);
> +        }
> +
> +        pfl = pflash_cfi01_register("e500.flash", size, blk, 64 * KiB, 2,
> +                                    0x89, 0x18, 0x0000, 0x0, 1);
> +        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
> +                                    pflash_cfi01_get_memory(pfl));
> +    }
> +
>     /*
>      * Smart firmware defaults ahead!
>      *
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/9] hw/{arm,ppc}: Resolve unreachable code
  2022-10-16 12:27 ` [PATCH v3 2/9] hw/{arm,ppc}: Resolve unreachable code Bernhard Beschow
@ 2022-10-17 20:57   ` Philippe Mathieu-Daudé
  2022-10-17 21:48     ` Bernhard Beschow
  2022-10-18  9:25     ` Peter Maydell
  0 siblings, 2 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-17 20:57 UTC (permalink / raw)
  To: Bernhard Beschow, qemu-devel
  Cc: Peter Maydell, qemu-arm, Alistair Francis, Jan Kiszka,
	Magnus Damm, Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz

On 16/10/22 14:27, Bernhard Beschow wrote:
> pflash_cfi01_register() always returns with a non-NULL pointer (otherwise
> it would crash internally). Therefore, the bodies of the if-statements
> are unreachable.

This is true, pflash_cfi0X_register() use an hardcoded &error_fatal.

Shouldn't it be better to pass an Error* argument?

 From the pflash API perspective I don't see much value in
returning a PFlashCFI0X type instead of a simple DeviceState
(but this is another story...).

> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/arm/gumstix.c     | 18 ++++++------------
>   hw/arm/mainstone.c   | 13 +++++--------
>   hw/arm/omap_sx1.c    | 22 ++++++++--------------
>   hw/arm/versatilepb.c |  6 ++----
>   hw/arm/z2.c          |  9 +++------
>   hw/ppc/sam460ex.c    | 12 ++++--------
>   6 files changed, 28 insertions(+), 52 deletions(-)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/9] hw/{arm,ppc}: Resolve unreachable code
  2022-10-17 20:57   ` Philippe Mathieu-Daudé
@ 2022-10-17 21:48     ` Bernhard Beschow
  2022-10-18  9:25     ` Peter Maydell
  1 sibling, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-17 21:48 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Peter Maydell, qemu-arm, Alistair Francis, Jan Kiszka,
	Magnus Damm, Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz

Am 17. Oktober 2022 20:57:06 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 16/10/22 14:27, Bernhard Beschow wrote:
>> pflash_cfi01_register() always returns with a non-NULL pointer (otherwise
>> it would crash internally). Therefore, the bodies of the if-statements
>> are unreachable.
>
>This is true, pflash_cfi0X_register() use an hardcoded &error_fatal.
>
>Shouldn't it be better to pass an Error* argument?

You mean that the callers would pass &error_fatal instead, including the ones in this patch?

>
>From the pflash API perspective I don't see much value in
>returning a PFlashCFI0X type instead of a simple DeviceState
>(but this is another story...).

It comes in handy in the following patches when retrieving the memory region for memory_region_add_subregion() using pflash_cfi0X_get_memory(). What do you think about these next patches?

Furthermore, returning PFlashCFI0X can be downcasted which seems safer than upcasting from DeviceState.

Best regards,
Bernhard

>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/arm/gumstix.c     | 18 ++++++------------
>>   hw/arm/mainstone.c   | 13 +++++--------
>>   hw/arm/omap_sx1.c    | 22 ++++++++--------------
>>   hw/arm/versatilepb.c |  6 ++----
>>   hw/arm/z2.c          |  9 +++------
>>   hw/ppc/sam460ex.c    | 12 ++++--------
>>   6 files changed, 28 insertions(+), 52 deletions(-)



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling
  2022-10-16 14:15   ` BALATON Zoltan
@ 2022-10-18  7:45     ` Bernhard Beschow
  2022-10-18  7:48       ` Bin Meng
  0 siblings, 1 reply; 19+ messages in thread
From: Bernhard Beschow @ 2022-10-18  7:45 UTC (permalink / raw)
  To: BALATON Zoltan, Bin Meng
  Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Aurelien Jarno,
	qemu-ppc, Yoshinori Sato, Antony Pavlov, Hanna Reitz

Am 16. Oktober 2022 14:15:09 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sun, 16 Oct 2022, 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 | 16 ++++++++++
>> hw/ppc/Kconfig              |  1 +
>> hw/ppc/e500.c               | 62 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 79 insertions(+)
>> 
>> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
>> index ba6bcb7314..99d2c680d6 100644
>> --- a/docs/system/ppc/ppce500.rst
>> +++ b/docs/system/ppc/ppce500.rst
>> @@ -165,3 +165,19 @@ if “-device eTSEC” is given to QEMU:
>> .. code-block:: bash
>> 
>>   -netdev tap,ifname=tap0,script=no,downscript=no,id=net0 -device eTSEC,netdev=net0
>> +
>> +Root file system on flash drive
>> +-------------------------------
>> +
>> +Rather than using a root file system on ram disk, it is possible to have it on
>> +CFI 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 \
>
>We have qemu-system-ppc and qemu-system-ppc64 not qemu-system-ppc32 so maybe qemu-system-ppc[64] even though that looks odd so maybe just qemu-system-ppc and then people should know that ppc64 includes ppc config as well.

True. This naming is used elsewhere in this document, so we kept it consistent. I think that users will get it either way.

@Bin: Any thoughts?

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
>
>> +      -display none -serial stdio \
>> +      -kernel vmlinux \
>> +      -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
>> +      -append "rootwait root=/dev/mtdblock0"
>> +
>> 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..23d2c3451a 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,31 @@ 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;
>> +        PFlashCFI01 *pfl;
>> +
>> +        if (!is_power_of_2(size)) {
>> +            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);
>> +        }
>> +
>> +        pfl = pflash_cfi01_register("e500.flash", size, blk, 64 * KiB, 2,
>> +                                    0x89, 0x18, 0x0000, 0x0, 1);
>> +        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
>> +                                    pflash_cfi01_get_memory(pfl));
>> +    }
>> +
>>     /*
>>      * Smart firmware defaults ahead!
>>      *
>> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling
  2022-10-18  7:45     ` Bernhard Beschow
@ 2022-10-18  7:48       ` Bin Meng
  2022-10-18 11:13         ` BALATON Zoltan
  0 siblings, 1 reply; 19+ messages in thread
From: Bin Meng @ 2022-10-18  7:48 UTC (permalink / raw)
  To: Bernhard Beschow
  Cc: BALATON Zoltan, Bin Meng, qemu-devel, Peter Maydell,
	Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Aurelien Jarno,
	qemu-ppc, Yoshinori Sato, Antony Pavlov, Hanna Reitz

On Tue, Oct 18, 2022 at 3:46 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Am 16. Oktober 2022 14:15:09 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
> >On Sun, 16 Oct 2022, 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 | 16 ++++++++++
> >> hw/ppc/Kconfig              |  1 +
> >> hw/ppc/e500.c               | 62 +++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 79 insertions(+)
> >>
> >> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
> >> index ba6bcb7314..99d2c680d6 100644
> >> --- a/docs/system/ppc/ppce500.rst
> >> +++ b/docs/system/ppc/ppce500.rst
> >> @@ -165,3 +165,19 @@ if “-device eTSEC” is given to QEMU:
> >> .. code-block:: bash
> >>
> >>   -netdev tap,ifname=tap0,script=no,downscript=no,id=net0 -device eTSEC,netdev=net0
> >> +
> >> +Root file system on flash drive
> >> +-------------------------------
> >> +
> >> +Rather than using a root file system on ram disk, it is possible to have it on
> >> +CFI 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 \
> >
> >We have qemu-system-ppc and qemu-system-ppc64 not qemu-system-ppc32 so maybe qemu-system-ppc[64] even though that looks odd so maybe just qemu-system-ppc and then people should know that ppc64 includes ppc config as well.
>
> True. This naming is used elsewhere in this document, so we kept it consistent. I think that users will get it either way.
>
> @Bin: Any thoughts?
>

How about a separate patch to remove the {64 | 32} suffix, and just
use qemu-system-ppc64 consistently since the *ppc64 executable can run
32-bit ppc codes too?

Regards,
Bin


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/9] hw/{arm,ppc}: Resolve unreachable code
  2022-10-17 20:57   ` Philippe Mathieu-Daudé
  2022-10-17 21:48     ` Bernhard Beschow
@ 2022-10-18  9:25     ` Peter Maydell
  1 sibling, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-10-18  9:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Bernhard Beschow, qemu-devel, qemu-arm, Alistair Francis,
	Jan Kiszka, Magnus Damm, Edgar E. Iglesias, Kevin Wolf,
	qemu-block, Bin Meng, Aurelien Jarno, qemu-ppc, BALATON Zoltan,
	Yoshinori Sato, Antony Pavlov, Hanna Reitz

On Mon, 17 Oct 2022 at 21:57, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 16/10/22 14:27, Bernhard Beschow wrote:
> > pflash_cfi01_register() always returns with a non-NULL pointer (otherwise
> > it would crash internally). Therefore, the bodies of the if-statements
> > are unreachable.
>
> This is true, pflash_cfi0X_register() use an hardcoded &error_fatal.
>
> Shouldn't it be better to pass an Error* argument?

The whole function is a legacy convenience-wrapper that's just
doing "create, configure, realize and wire up a device". New
code, and any callers that actually care about errors, should
just be directly creating and configuring the device anyway.
Almost all the callers are in old legacy board model code.

-- PMM


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling
  2022-10-16 12:27 ` [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling Bernhard Beschow
  2022-10-16 14:15   ` BALATON Zoltan
@ 2022-10-18  9:28   ` Peter Maydell
  1 sibling, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2022-10-18  9:28 UTC (permalink / raw)
  To: Bernhard Beschow
  Cc: qemu-devel, Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Bin Meng,
	Aurelien Jarno, qemu-ppc, BALATON Zoltan, Yoshinori Sato,
	Antony Pavlov, Hanna Reitz

On Sun, 16 Oct 2022 at 13:28, 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>

> +        pfl = pflash_cfi01_register("e500.flash", size, blk, 64 * KiB, 2,
> +                                    0x89, 0x18, 0x0000, 0x0, 1);
> +        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
> +                                    pflash_cfi01_get_memory(pfl));

pflash_cfi01_register() puts the memory region into the
system address space. It's just a legacy convenience wrapper
function, so if you need to put the resulting memory region somewhere
else, just directly create, configure and map the device in
this board code.


thanks
-- PMM


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling
  2022-10-18  7:48       ` Bin Meng
@ 2022-10-18 11:13         ` BALATON Zoltan
  0 siblings, 0 replies; 19+ messages in thread
From: BALATON Zoltan @ 2022-10-18 11:13 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bernhard Beschow, Bin Meng, qemu-devel, Peter Maydell,
	Philippe Mathieu-Daudé,
	qemu-arm, Alistair Francis, Jan Kiszka, Magnus Damm,
	Edgar E. Iglesias, Kevin Wolf, qemu-block, Aurelien Jarno,
	qemu-ppc, Yoshinori Sato, Antony Pavlov, Hanna Reitz

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

On Tue, 18 Oct 2022, Bin Meng wrote:
> On Tue, Oct 18, 2022 at 3:46 PM Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Am 16. Oktober 2022 14:15:09 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>>> On Sun, 16 Oct 2022, 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 | 16 ++++++++++
>>>> hw/ppc/Kconfig              |  1 +
>>>> hw/ppc/e500.c               | 62 +++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 79 insertions(+)
>>>>
>>>> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
>>>> index ba6bcb7314..99d2c680d6 100644
>>>> --- a/docs/system/ppc/ppce500.rst
>>>> +++ b/docs/system/ppc/ppce500.rst
>>>> @@ -165,3 +165,19 @@ if “-device eTSEC” is given to QEMU:
>>>> .. code-block:: bash
>>>>
>>>>   -netdev tap,ifname=tap0,script=no,downscript=no,id=net0 -device eTSEC,netdev=net0
>>>> +
>>>> +Root file system on flash drive
>>>> +-------------------------------
>>>> +
>>>> +Rather than using a root file system on ram disk, it is possible to have it on
>>>> +CFI 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 \
>>>
>>> We have qemu-system-ppc and qemu-system-ppc64 not qemu-system-ppc32 so maybe qemu-system-ppc[64] even though that looks odd so maybe just qemu-system-ppc and then people should know that ppc64 includes ppc config as well.
>>
>> True. This naming is used elsewhere in this document, so we kept it consistent. I think that users will get it either way.
>>
>> @Bin: Any thoughts?
>>
>
> How about a separate patch to remove the {64 | 32} suffix, and just
> use qemu-system-ppc64 consistently since the *ppc64 executable can run
> 32-bit ppc codes too?

In case it's already there then yes it's unrelated to this series so just 
disregard my comment or add a separate patch to fix it if you can. I did 
not check the rest of the doc just noticed this in this patch.

Regards,
BALATON Zoltan

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-10-18 11:20 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-16 12:27 [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow
2022-10-16 12:27 ` [PATCH v3 1/9] hw/block/pflash_cfi0{1, 2}: Error out if device length isn't a power of two Bernhard Beschow
2022-10-16 12:27 ` [PATCH v3 2/9] hw/{arm,ppc}: Resolve unreachable code Bernhard Beschow
2022-10-17 20:57   ` Philippe Mathieu-Daudé
2022-10-17 21:48     ` Bernhard Beschow
2022-10-18  9:25     ` Peter Maydell
2022-10-16 12:27 ` [PATCH v3 3/9] hw/block/pflash_cfi01: Attach memory region in boards Bernhard Beschow
2022-10-16 12:27 ` [PATCH v3 4/9] hw/block/pflash_cfi02: " Bernhard Beschow
2022-10-16 12:27 ` [PATCH v3 5/9] hw/sd/sdhci-internal: Unexport ESDHC defines Bernhard Beschow
2022-10-16 12:27 ` [PATCH v3 6/9] hw/sd/sdhci: Rename ESDHC_* defines to USDHC_* Bernhard Beschow
2022-10-16 12:27 ` [PATCH v3 7/9] hw/ppc/e500: Implement pflash handling Bernhard Beschow
2022-10-16 14:15   ` BALATON Zoltan
2022-10-18  7:45     ` Bernhard Beschow
2022-10-18  7:48       ` Bin Meng
2022-10-18 11:13         ` BALATON Zoltan
2022-10-18  9:28   ` Peter Maydell
2022-10-16 12:27 ` [PATCH v3 8/9] hw/sd/sdhci: Implement Freescale eSDHC device model Bernhard Beschow
2022-10-16 12:27 ` [PATCH v3 9/9] hw/ppc/e500: Add Freescale eSDHC to e500plat Bernhard Beschow
2022-10-16 12:41 ` [PATCH v3 0/9] ppc/e500: Add support for two types of flash, cleanup Bernhard Beschow

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.