[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 08/16] hw/riscv: microchip_pfsoc: Connect a Cadence SDHCI
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v3 08/16] hw/riscv: microchip_pfsoc: Connect a Cadence SDHCI controller and an SD card |
Date: |
Tue, 1 Sep 2020 19:56:07 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 |
On 9/1/20 12:27 PM, Bin Meng wrote:
> Hi Philippe,
>
> On Tue, Sep 1, 2020 at 5:42 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 9/1/20 3:39 AM, Bin Meng wrote:
>>> From: Bin Meng <bin.meng@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.meng@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é <f4bug@amsat.org>
>
> Regards,
> Bin
>