Am 3. Oktober 2022 21:06:57 UTC schrieb "Philippe Mathieu-Daudé"
<f4...@amsat.org>:
>On 3/10/22 22:31, Bernhard Beschow wrote:
>> Adds missing functionality to emulated e500 SOCs which increases the
>> chance of given "real" firmware images to access SD cards.
>>
>> Signed-off-by: Bernhard Beschow <shen...@gmail.com>
>> ---
>> docs/system/ppc/ppce500.rst | 13 +++++++++++++
>> hw/ppc/Kconfig | 1 +
>> hw/ppc/e500.c | 31 ++++++++++++++++++++++++++++++-
>> 3 files changed, 44 insertions(+), 1 deletion(-)
>
>> +static void dt_sdhc_create(void *fdt, const char *parent, const char *mpic)
>> +{
>> + hwaddr mmio = MPC85XX_ESDHC_REGS_OFFSET;
>> + hwaddr size = MPC85XX_ESDHC_REGS_SIZE;
>> + int irq = MPC85XX_ESDHC_IRQ;
>
>Why not pass these 3 variable as argument?
In anticipation of data-driven board creation, I'd ideally infer those from the
device's QOM properties. This seems similar to what Mark suggested in the BoF
at KVM Forum [1], where -- IIUC -- he stated that QOM properties could be the
foundation of all wiring representations. And device tree seems just like one
specialized representation to me. (Note that I'm slightly hijacking the review
here because I don't know where and how to express these thoughts elsewhere).
Does it make sense to add the missing properties here?
Best regards,
Bernhard
[1] https://etherpad.opendev.org/p/qemu-emulation-bof%40kvmforum2022
>
>> + g_autofree char *name = NULL;
>> +
>> + name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
>> + qemu_fdt_add_subnode(fdt, name);
>> + qemu_fdt_setprop(fdt, name, "sdhci,auto-cmd12", NULL, 0);
>> + qemu_fdt_setprop_phandle(fdt, name, "interrupt-parent", mpic);
>> + qemu_fdt_setprop_cells(fdt, name, "bus-width", 4);
>> + qemu_fdt_setprop_cells(fdt, name, "interrupts", irq, 0x2);
>> + qemu_fdt_setprop_cells(fdt, name, "reg", mmio, size);
>> + qemu_fdt_setprop_string(fdt, name, "compatible", "fsl,esdhc");
>> +}
>> typedef struct PlatformDevtreeData {
>> void *fdt;
>> @@ -553,6 +573,8 @@ static int ppce500_load_device_tree(PPCE500MachineState
>> *pms,
>> dt_rtc_create(fdt, "i2c", "rtc");
>> + /* sdhc */
>> + dt_sdhc_create(fdt, soc, mpic);
>>
