On 9/1/20 12:27 PM, Bin Meng wrote: > Hi Philippe, > > On Tue, Sep 1, 2020 at 5:42 PM Philippe Mathieu-Daudé <f4...@amsat.org> wrote: >> >> On 9/1/20 3:39 AM, Bin Meng wrote: >>> From: Bin Meng <bin.m...@windriver.com> >>> >>> Microchip PolarFire SoC integrates one Cadence SDHCI controller. >>> On the Icicle Kit board, one eMMC chip and an external SD card >>> connect to this controller depending on different configuration. >>> >>> As QEMU does not support eMMC yet, we just emulate the SD card >>> configuration. To test this, the Hart Software Services (HSS) >>> should choose the SD card configuration: >>> >>> $ cp boards/icicle-kit-es/def_config.sdcard .config >>> $ make BOARD=icicle-kit-es >>> >>> The SD card image can be built from the Yocto BSP at: >>> https://github.com/polarfire-soc/meta-polarfire-soc-yocto-bsp >>> >>> Note the generated SD card image should be resized before use: >>> $ qemu-img resize /path/to/sdcard.img 4G >>> >>> Launch QEMU with the following command: >>> $ qemu-system-riscv64 -nographic -M microchip-icicle-kit -sd sdcard.img >>> >>> Signed-off-by: Bin Meng <bin.m...@windriver.com> >>> >>> --- >>> >>> (no changes since v2) >>> >>> Changes in v2: >>> - do not initialize TYPE_SYSBUS_SDHCI in the SoC instance_init(), >>> instead move that to the cadence_sdhci model >>> - do not access generic-sdhci's state directly, >>> instead move that to the cadence_sdhci model >>> >>> include/hw/riscv/microchip_pfsoc.h | 4 ++++ >>> hw/riscv/microchip_pfsoc.c | 23 +++++++++++++++++++++++ >>> hw/riscv/Kconfig | 1 + >>> 3 files changed, 28 insertions(+) >>> >>> diff --git a/include/hw/riscv/microchip_pfsoc.h >>> b/include/hw/riscv/microchip_pfsoc.h >>> index a5efa1d..d810ee8 100644 >>> --- a/include/hw/riscv/microchip_pfsoc.h >>> +++ b/include/hw/riscv/microchip_pfsoc.h >>> @@ -23,6 +23,7 @@ >>> #define HW_MICROCHIP_PFSOC_H >>> >>> #include "hw/char/mchp_pfsoc_mmuart.h" >>> +#include "hw/sd/cadence_sdhci.h" >>> >>> typedef struct MicrochipPFSoCState { >>> /*< private >*/ >>> @@ -39,6 +40,7 @@ typedef struct MicrochipPFSoCState { >>> MchpPfSoCMMUartState *serial2; >>> MchpPfSoCMMUartState *serial3; >>> MchpPfSoCMMUartState *serial4; >>> + CadenceSDHCIState sdhci; >>> } MicrochipPFSoCState; >>> >>> #define TYPE_MICROCHIP_PFSOC "microchip.pfsoc" >>> @@ -74,6 +76,7 @@ enum { >>> MICROCHIP_PFSOC_MMUART0, >>> MICROCHIP_PFSOC_SYSREG, >>> MICROCHIP_PFSOC_MPUCFG, >>> + MICROCHIP_PFSOC_EMMC_SD, >>> MICROCHIP_PFSOC_MMUART1, >>> MICROCHIP_PFSOC_MMUART2, >>> MICROCHIP_PFSOC_MMUART3, >>> @@ -85,6 +88,7 @@ enum { >>> }; >>> >>> enum { >>> + MICROCHIP_PFSOC_EMMC_SD_IRQ = 88, >>> MICROCHIP_PFSOC_MMUART0_IRQ = 90, >>> MICROCHIP_PFSOC_MMUART1_IRQ = 91, >>> MICROCHIP_PFSOC_MMUART2_IRQ = 92, >>> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c >>> index cee959a..0b2e9ca 100644 >>> --- a/hw/riscv/microchip_pfsoc.c >>> +++ b/hw/riscv/microchip_pfsoc.c >>> @@ -12,6 +12,7 @@ >>> * 1) PLIC (Platform Level Interrupt Controller) >>> * 2) eNVM (Embedded Non-Volatile Memory) >>> * 3) MMUARTs (Multi-Mode UART) >>> + * 4) Cadence eMMC/SDHC controller and an SD card connected to it >>> * >>> * This board currently generates devicetree dynamically that indicates at >>> least >>> * two harts and up to five harts. >>> @@ -75,6 +76,7 @@ static const struct MemmapEntry { >>> [MICROCHIP_PFSOC_MMUART0] = { 0x20000000, 0x1000 }, >>> [MICROCHIP_PFSOC_SYSREG] = { 0x20002000, 0x2000 }, >>> [MICROCHIP_PFSOC_MPUCFG] = { 0x20005000, 0x1000 }, >>> + [MICROCHIP_PFSOC_EMMC_SD] = { 0x20008000, 0x1000 }, >>> [MICROCHIP_PFSOC_MMUART1] = { 0x20100000, 0x1000 }, >>> [MICROCHIP_PFSOC_MMUART2] = { 0x20102000, 0x1000 }, >>> [MICROCHIP_PFSOC_MMUART3] = { 0x20104000, 0x1000 }, >>> @@ -111,6 +113,9 @@ static void microchip_pfsoc_soc_instance_init(Object >>> *obj) >>> qdev_prop_set_string(DEVICE(&s->u_cpus), "cpu-type", >>> TYPE_RISCV_CPU_SIFIVE_U54); >>> qdev_prop_set_uint64(DEVICE(&s->u_cpus), "resetvec", RESET_VECTOR); >>> + >>> + object_initialize_child(obj, "sd-controller", &s->sdhci, >>> + TYPE_CADENCE_SDHCI); >>> } >>> >>> static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp) >>> @@ -223,6 +228,13 @@ static void microchip_pfsoc_soc_realize(DeviceState >>> *dev, Error **errp) >>> memmap[MICROCHIP_PFSOC_MPUCFG].base, >>> memmap[MICROCHIP_PFSOC_MPUCFG].size); >>> >>> + /* SDHCI */ >>> + sysbus_realize(SYS_BUS_DEVICE(&s->sdhci), errp); >>> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdhci), 0, >>> + memmap[MICROCHIP_PFSOC_EMMC_SD].base); >>> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci), 0, >>> + qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_EMMC_SD_IRQ)); >>> + >>> /* MMUARTs */ >>> s->serial0 = mchp_pfsoc_mmuart_create(system_memory, >>> memmap[MICROCHIP_PFSOC_MMUART0].base, >>> @@ -290,6 +302,7 @@ static void >>> microchip_icicle_kit_machine_init(MachineState *machine) >>> MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(machine); >>> MemoryRegion *system_memory = get_system_memory(); >>> MemoryRegion *main_mem = g_new(MemoryRegion, 1); >>> + DriveInfo *dinfo = drive_get_next(IF_SD); >> >> Ideally this part should be in the machine, not the soc. >> >> Maybe you can alias the "drive" property as "sd-drive" in the soc, >> and move the card attach to the machine? > > Thanks for the review. > > I am not sure I understand your suggestion. Currently the SD card > attach codes are already in the machine codes > microchip_icicle_kit_machine_init()
Ah I have been confuse, since the file is named "microchip_pfsoc.c" I was expecting the machine code in another file, now I realize both machine + soc are altogether here. No problem then! > >> >> Can be done later if you prefer: >> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> > > Regards, > Bin >
