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

2022-10-16 Thread Bernhard Beschow 
Am 3. Oktober 2022 21:06:57 UTC schrieb "Philippe Mathieu-Daudé" 
:
>On 3/10/22 22:31, Bernhard Beschow wrote:
>> Adds missing functionality to emulated e500 SOCs which increases the
>> chance of given "real" firmware images to access SD cards.
>> 
>> Signed-off-by: Bernhard Beschow 
>> ---
>>   docs/system/ppc/ppce500.rst | 13 +
>>   hw/ppc/Kconfig  |  1 +
>>   hw/ppc/e500.c   | 31 ++-
>>   3 files changed, 44 insertions(+), 1 deletion(-)
>
>> +static void dt_sdhc_create(void *fdt, const char *parent, const char *mpic)
>> +{
>> +hwaddr mmio = MPC85XX_ESDHC_REGS_OFFSET;
>> +hwaddr size = MPC85XX_ESDHC_REGS_SIZE;
>> +int irq = MPC85XX_ESDHC_IRQ;
>
>Why not pass these 3 variable as argument?

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

Best regards,
Bernhard
>
>> +g_autofree char *name = NULL;
>> +
>> +name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
>> +qemu_fdt_add_subnode(fdt, name);
>> +qemu_fdt_setprop(fdt, name, "sdhci,auto-cmd12", NULL, 0);
>> +qemu_fdt_setprop_phandle(fdt, name, "interrupt-parent", mpic);
>> +qemu_fdt_setprop_cells(fdt, name, "bus-width", 4);
>> +qemu_fdt_setprop_cells(fdt, name, "interrupts", irq, 0x2);
>> +qemu_fdt_setprop_cells(fdt, name, "reg", mmio, size);
>> +qemu_fdt_setprop_string(fdt, name, "compatible", "fsl,esdhc");
>> +}
>> typedef struct PlatformDevtreeData {
>>   void *fdt;
>> @@ -553,6 +573,8 @@ static int ppce500_load_device_tree(PPCE500MachineState 
>> *pms,
>> dt_rtc_create(fdt, "i2c", "rtc");
>>   +/* sdhc */
>> +dt_sdhc_create(fdt, soc, mpic);
>>

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

2022-10-04 Thread Bernhard Beschow 
Am 3. Oktober 2022 21:06:57 UTC schrieb "Philippe Mathieu-Daudé" 
:
>On 3/10/22 22:31, Bernhard Beschow wrote:
>> Adds missing functionality to emulated e500 SOCs which increases the
>> chance of given "real" firmware images to access SD cards.
>> 
>> Signed-off-by: Bernhard Beschow 
>> ---
>>   docs/system/ppc/ppce500.rst | 13 +
>>   hw/ppc/Kconfig  |  1 +
>>   hw/ppc/e500.c   | 31 ++-
>>   3 files changed, 44 insertions(+), 1 deletion(-)
>
>> +static void dt_sdhc_create(void *fdt, const char *parent, const char *mpic)
>> +{
>> +hwaddr mmio = MPC85XX_ESDHC_REGS_OFFSET;
>> +hwaddr size = MPC85XX_ESDHC_REGS_SIZE;
>> +int irq = MPC85XX_ESDHC_IRQ;
>
>Why not pass these 3 variable as argument?

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

Does it make sense to add the missing properties here?

Best regards,
Bernhard

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

>
>> +g_autofree char *name = NULL;
>> +
>> +name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
>> +qemu_fdt_add_subnode(fdt, name);
>> +qemu_fdt_setprop(fdt, name, "sdhci,auto-cmd12", NULL, 0);
>> +qemu_fdt_setprop_phandle(fdt, name, "interrupt-parent", mpic);
>> +qemu_fdt_setprop_cells(fdt, name, "bus-width", 4);
>> +qemu_fdt_setprop_cells(fdt, name, "interrupts", irq, 0x2);
>> +qemu_fdt_setprop_cells(fdt, name, "reg", mmio, size);
>> +qemu_fdt_setprop_string(fdt, name, "compatible", "fsl,esdhc");
>> +}
>> typedef struct PlatformDevtreeData {
>>   void *fdt;
>> @@ -553,6 +573,8 @@ static int ppce500_load_device_tree(PPCE500MachineState 
>> *pms,
>> dt_rtc_create(fdt, "i2c", "rtc");
>>   +/* sdhc */
>> +dt_sdhc_create(fdt, soc, mpic);
>>

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

2022-10-03 Thread Philippe Mathieu-Daudé via 

On 3/10/22 22:31, Bernhard Beschow wrote:

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

Signed-off-by: Bernhard Beschow 
---
  docs/system/ppc/ppce500.rst | 13 +
  hw/ppc/Kconfig  |  1 +
  hw/ppc/e500.c   | 31 ++-
  3 files changed, 44 insertions(+), 1 deletion(-)



+static void dt_sdhc_create(void *fdt, const char *parent, const char *mpic)
+{
+hwaddr mmio = MPC85XX_ESDHC_REGS_OFFSET;
+hwaddr size = MPC85XX_ESDHC_REGS_SIZE;
+int irq = MPC85XX_ESDHC_IRQ;


Why not pass these 3 variable as argument?


+g_autofree char *name = NULL;
+
+name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
+qemu_fdt_add_subnode(fdt, name);
+qemu_fdt_setprop(fdt, name, "sdhci,auto-cmd12", NULL, 0);
+qemu_fdt_setprop_phandle(fdt, name, "interrupt-parent", mpic);
+qemu_fdt_setprop_cells(fdt, name, "bus-width", 4);
+qemu_fdt_setprop_cells(fdt, name, "interrupts", irq, 0x2);
+qemu_fdt_setprop_cells(fdt, name, "reg", mmio, size);
+qemu_fdt_setprop_string(fdt, name, "compatible", "fsl,esdhc");
+}
  
  typedef struct PlatformDevtreeData {

  void *fdt;
@@ -553,6 +573,8 @@ static int ppce500_load_device_tree(PPCE500MachineState 
*pms,
  
  dt_rtc_create(fdt, "i2c", "rtc");
  
+/* sdhc */

+dt_sdhc_create(fdt, soc, mpic);

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

2022-10-03 Thread Bernhard Beschow 
Adds missing functionality to emulated e500 SOCs which increases the
chance of given "real" firmware images to access SD cards.

Signed-off-by: Bernhard Beschow 
---
 docs/system/ppc/ppce500.rst | 13 +
 hw/ppc/Kconfig  |  1 +
 hw/ppc/e500.c   | 31 ++-
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
index 1ed6c36599..c758aa51eb 100644
--- a/docs/system/ppc/ppce500.rst
+++ b/docs/system/ppc/ppce500.rst
@@ -19,6 +19,7 @@ The ``ppce500`` machine supports the following devices:
 * Power-off functionality via one GPIO pin
 * 1 Freescale MPC8xxx PCI host controller
 * VirtIO devices via PCI bus
+* 1 Freescale Enhanced Secure Digital Host controller (eSDHC)
 * 1 Freescale Enhanced Triple Speed Ethernet controller (eTSEC)
 
 Hardware configuration information
@@ -131,6 +132,18 @@ be used as follows:
   -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
   -append "rootwait root=/dev/mtdblock0"
 
+Alternatively, the root file system can also reside on an emulated SD card
+whose size must again be a power of two:
+
+.. code-block:: bash
+
+  $ qemu-system-ppc{64|32} -M ppce500 -cpu e500mc -smp 4 -m 2G \
+  -display none -serial stdio \
+  -kernel vmlinux \
+  -device sd-card,drive=mydrive \
+  -drive id=mydrive,if=none,file=/path/to/rootfs.ext2,format=raw \
+  -append "rootwait root=/dev/mmcblk0"
+
 Running U-Boot
 --
 
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 769a1ead1c..6e31f568ba 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -129,6 +129,7 @@ config E500
 select PFLASH_CFI01
 select PLATFORM_BUS
 select PPCE500_PCI
+select SDHCI
 select SERIAL
 select MPC_I2C
 select FDT_PPC
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 2b1430fca4..379d89bf2d 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -48,6 +48,7 @@
 #include "hw/net/fsl_etsec/etsec.h"
 #include "hw/i2c/i2c.h"
 #include "hw/irq.h"
+#include "hw/sd/sdhci.h"
 
 #define EPAPR_MAGIC(0x45504150)
 #define DTC_LOAD_PAD   0x180
@@ -66,11 +67,14 @@
 #define MPC8544_SERIAL1_REGS_OFFSET 0x4600ULL
 #define MPC8544_PCI_REGS_OFFSET0x8000ULL
 #define MPC8544_PCI_REGS_SIZE  0x1000ULL
+#define MPC85XX_ESDHC_REGS_OFFSET  0x2e000ULL
+#define MPC85XX_ESDHC_REGS_SIZE0x1000ULL
 #define MPC8544_UTIL_OFFSET0xeULL
 #define MPC8XXX_GPIO_OFFSET0x000FF000ULL
 #define MPC8544_I2C_REGS_OFFSET0x3000ULL
 #define MPC8XXX_GPIO_IRQ   47
 #define MPC8544_I2C_IRQ43
+#define MPC85XX_ESDHC_IRQ  72
 #define RTC_REGS_OFFSET0x68
 
 #define PLATFORM_CLK_FREQ_HZ   (400 * 1000 * 1000)
@@ -203,6 +207,22 @@ static void dt_i2c_create(void *fdt, const char *soc, 
const char *mpic,
 g_free(i2c);
 }
 
+static void dt_sdhc_create(void *fdt, const char *parent, const char *mpic)
+{
+hwaddr mmio = MPC85XX_ESDHC_REGS_OFFSET;
+hwaddr size = MPC85XX_ESDHC_REGS_SIZE;
+int irq = MPC85XX_ESDHC_IRQ;
+g_autofree char *name = NULL;
+
+name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
+qemu_fdt_add_subnode(fdt, name);
+qemu_fdt_setprop(fdt, name, "sdhci,auto-cmd12", NULL, 0);
+qemu_fdt_setprop_phandle(fdt, name, "interrupt-parent", mpic);
+qemu_fdt_setprop_cells(fdt, name, "bus-width", 4);
+qemu_fdt_setprop_cells(fdt, name, "interrupts", irq, 0x2);
+qemu_fdt_setprop_cells(fdt, name, "reg", mmio, size);
+qemu_fdt_setprop_string(fdt, name, "compatible", "fsl,esdhc");
+}
 
 typedef struct PlatformDevtreeData {
 void *fdt;
@@ -553,6 +573,8 @@ static int ppce500_load_device_tree(PPCE500MachineState 
*pms,
 
 dt_rtc_create(fdt, "i2c", "rtc");
 
+/* sdhc */
+dt_sdhc_create(fdt, soc, mpic);
 
 gutil = g_strdup_printf("%s/global-utilities@%llx", soc,
 MPC8544_UTIL_OFFSET);
@@ -982,7 +1004,8 @@ void ppce500_init(MachineState *machine)
0, qdev_get_gpio_in(mpicdev, 42), 399193,
serial_hd(1), DEVICE_BIG_ENDIAN);
 }
-/* I2C */
+
+/* I2C */
 dev = qdev_new("mpc-i2c");
 s = SYS_BUS_DEVICE(dev);
 sysbus_realize_and_unref(s, _fatal);
@@ -992,6 +1015,12 @@ void ppce500_init(MachineState *machine)
 i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
 i2c_slave_create_simple(i2c, "ds1338", RTC_REGS_OFFSET);
 
+/* eSDHC */
+dev = qdev_new(TYPE_FSL_ESDHC);
+s = SYS_BUS_DEVICE(dev);
+sysbus_realize_and_unref(s, _fatal);
+sysbus_mmio_map(s, 0, pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET);
+sysbus_connect_irq(s, 0, qdev_get_gpio_in(mpicdev, MPC85XX_ESDHC_IRQ));
 
 /* General Utility device */
 dev = qdev_new("mpc8544-guts");
-- 
2.37.3