| Message ID | 20200107073423.30043-3-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | aspeed: extensions and fixes | expand |
On 1/7/20 8:34 AM, Cédric Le Goater wrote: > From: Andrew Jeffery <andrew@aj.id.au> > > Initialise another SDHCI model instance for the AST2600's eMMC > controller and use the SDHCI's num_slots value introduced previously to > determine whether we should create an SD card instance for the new slot. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > Reviewed-by: Cédric Le Goater <clg@kaod.org> > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > include/hw/arm/aspeed_soc.h | 2 ++ > hw/arm/aspeed.c | 25 ++++++++++++++++--------- > hw/arm/aspeed_ast2600.c | 21 +++++++++++++++++++++ > 3 files changed, 39 insertions(+), 9 deletions(-) > > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h > index e84380984f7b..90ac7f7ffa3b 100644 > --- a/include/hw/arm/aspeed_soc.h > +++ b/include/hw/arm/aspeed_soc.h > @@ -57,6 +57,7 @@ typedef struct AspeedSoCState { > AspeedGPIOState gpio; > AspeedGPIOState gpio_1_8v; > AspeedSDHCIState sdhci; > + AspeedSDHCIState emmc; > } AspeedSoCState; > > #define TYPE_ASPEED_SOC "aspeed-soc" > @@ -126,6 +127,7 @@ enum { > ASPEED_MII4, > ASPEED_SDRAM, > ASPEED_XDMA, > + ASPEED_EMMC, > }; > > #endif /* ASPEED_SOC_H */ > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 4174e313cae5..0a7dfd29868b 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -171,6 +171,18 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, > } > } > > +static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo) > +{ > + BlockBackend *blk; > + DeviceState *card; > + > + blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; > + card = qdev_create(qdev_get_child_bus(DEVICE(sdhci), "sd-bus"), > + TYPE_SD_CARD); > + qdev_prop_set_drive(card, "drive", blk, &error_fatal); I find this form easier to review: if (dinfo) { qdev_prop_set_drive(card, "drive", blk_by_legacy_dinfo(dinfo), &error_fatal); } > + object_property_set_bool(OBJECT(card), true, "realized", &error_fatal); > +} > + > static void aspeed_machine_init(MachineState *machine) > { > AspeedBoardState *bmc; > @@ -264,16 +276,11 @@ static void aspeed_machine_init(MachineState *machine) > } > > for (i = 0; i < bmc->soc.sdhci.num_slots; i++) { > - SDHCIState *sdhci = &bmc->soc.sdhci.slots[i]; > - DriveInfo *dinfo = drive_get_next(IF_SD); > - BlockBackend *blk; > - DeviceState *card; > + sdhci_attach_drive(&bmc->soc.sdhci.slots[i], drive_get_next(IF_SD)); > + } > > - blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; > - card = qdev_create(qdev_get_child_bus(DEVICE(sdhci), "sd-bus"), > - TYPE_SD_CARD); > - qdev_prop_set_drive(card, "drive", blk, &error_fatal); > - object_property_set_bool(OBJECT(card), true, "realized", &error_fatal); > + if (bmc->soc.emmc.num_slots) { > + sdhci_attach_drive(&bmc->soc.emmc.slots[0], drive_get_next(IF_SD)); > } > > arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo); > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c > index fb73c4043ea3..e59cdd291925 100644 > --- a/hw/arm/aspeed_ast2600.c > +++ b/hw/arm/aspeed_ast2600.c > @@ -46,6 +46,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = { > [ASPEED_ADC] = 0x1E6E9000, > [ASPEED_VIDEO] = 0x1E700000, > [ASPEED_SDHCI] = 0x1E740000, > + [ASPEED_EMMC] = 0x1E750000, > [ASPEED_GPIO] = 0x1E780000, > [ASPEED_GPIO_1_8V] = 0x1E780800, > [ASPEED_RTC] = 0x1E781000, > @@ -64,6 +65,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = { > > #define ASPEED_SOC_AST2600_MAX_IRQ 128 > > +/* Shared Peripheral Interrupt values below are offset by -32 from datasheet */ > static const int aspeed_soc_ast2600_irqmap[] = { > [ASPEED_UART1] = 47, > [ASPEED_UART2] = 48, > @@ -77,6 +79,7 @@ static const int aspeed_soc_ast2600_irqmap[] = { > [ASPEED_ADC] = 78, > [ASPEED_XDMA] = 6, > [ASPEED_SDHCI] = 43, > + [ASPEED_EMMC] = 15, > [ASPEED_GPIO] = 40, > [ASPEED_GPIO_1_8V] = 11, > [ASPEED_RTC] = 13, > @@ -206,6 +209,14 @@ static void aspeed_soc_ast2600_init(Object *obj) > sysbus_init_child_obj(obj, "sdhci[*]", OBJECT(&s->sdhci.slots[i]), > sizeof(s->sdhci.slots[i]), TYPE_SYSBUS_SDHCI); > } > + > + sysbus_init_child_obj(obj, "emmc", OBJECT(&s->emmc), sizeof(s->emmc), > + TYPE_ASPEED_SDHCI); > + > + object_property_set_int(OBJECT(&s->emmc), 1, "num-slots", &error_abort); > + > + sysbus_init_child_obj(obj, "emmc[*]", OBJECT(&s->emmc.slots[0]), Single block, so use "emmc" instead. Using "emmc": Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + sizeof(s->emmc.slots[0]), TYPE_SYSBUS_SDHCI); > } > > /* > @@ -497,6 +508,16 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) > sc->memmap[ASPEED_SDHCI]); > sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci), 0, > aspeed_soc_get_irq(s, ASPEED_SDHCI)); > + > + /* eMMC */ > + object_property_set_bool(OBJECT(&s->emmc), true, "realized", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + sysbus_mmio_map(SYS_BUS_DEVICE(&s->emmc), 0, sc->memmap[ASPEED_EMMC]); > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0, > + aspeed_soc_get_irq(s, ASPEED_EMMC)); > } > > static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data) >
On 1/7/20 9:25 AM, Philippe Mathieu-Daudé wrote: > On 1/7/20 8:34 AM, Cédric Le Goater wrote: >> From: Andrew Jeffery <andrew@aj.id.au> >> >> Initialise another SDHCI model instance for the AST2600's eMMC >> controller and use the SDHCI's num_slots value introduced previously to >> determine whether we should create an SD card instance for the new slot. >> >> Signed-off-by: Andrew Jeffery <andrew@aj.id.au> >> Reviewed-by: Cédric Le Goater <clg@kaod.org> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> include/hw/arm/aspeed_soc.h | 2 ++ >> hw/arm/aspeed.c | 25 ++++++++++++++++--------- >> hw/arm/aspeed_ast2600.c | 21 +++++++++++++++++++++ >> 3 files changed, 39 insertions(+), 9 deletions(-) >> >> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h >> index e84380984f7b..90ac7f7ffa3b 100644 >> --- a/include/hw/arm/aspeed_soc.h >> +++ b/include/hw/arm/aspeed_soc.h >> @@ -57,6 +57,7 @@ typedef struct AspeedSoCState { >> AspeedGPIOState gpio; >> AspeedGPIOState gpio_1_8v; >> AspeedSDHCIState sdhci; >> + AspeedSDHCIState emmc; >> } AspeedSoCState; >> #define TYPE_ASPEED_SOC "aspeed-soc" >> @@ -126,6 +127,7 @@ enum { >> ASPEED_MII4, >> ASPEED_SDRAM, >> ASPEED_XDMA, >> + ASPEED_EMMC, >> }; >> #endif /* ASPEED_SOC_H */ >> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c >> index 4174e313cae5..0a7dfd29868b 100644 >> --- a/hw/arm/aspeed.c >> +++ b/hw/arm/aspeed.c >> @@ -171,6 +171,18 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, >> } >> } >> +static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo) >> +{ >> + BlockBackend *blk; >> + DeviceState *card; >> + >> + blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; >> + card = qdev_create(qdev_get_child_bus(DEVICE(sdhci), "sd-bus"), >> + TYPE_SD_CARD); >> + qdev_prop_set_drive(card, "drive", blk, &error_fatal); > > I find this form easier to review: > > if (dinfo) { > qdev_prop_set_drive(card, "drive", > blk_by_legacy_dinfo(dinfo), > &error_fatal); > } yes. >> + object_property_set_bool(OBJECT(card), true, "realized", &error_fatal); >> +} >> + >> static void aspeed_machine_init(MachineState *machine) >> { >> AspeedBoardState *bmc; >> @@ -264,16 +276,11 @@ static void aspeed_machine_init(MachineState *machine) >> } >> for (i = 0; i < bmc->soc.sdhci.num_slots; i++) { >> - SDHCIState *sdhci = &bmc->soc.sdhci.slots[i]; >> - DriveInfo *dinfo = drive_get_next(IF_SD); >> - BlockBackend *blk; >> - DeviceState *card; >> + sdhci_attach_drive(&bmc->soc.sdhci.slots[i], drive_get_next(IF_SD)); >> + } >> - blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; >> - card = qdev_create(qdev_get_child_bus(DEVICE(sdhci), "sd-bus"), >> - TYPE_SD_CARD); >> - qdev_prop_set_drive(card, "drive", blk, &error_fatal); >> - object_property_set_bool(OBJECT(card), true, "realized", &error_fatal); >> + if (bmc->soc.emmc.num_slots) { >> + sdhci_attach_drive(&bmc->soc.emmc.slots[0], drive_get_next(IF_SD)); >> } >> arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo); >> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c >> index fb73c4043ea3..e59cdd291925 100644 >> --- a/hw/arm/aspeed_ast2600.c >> +++ b/hw/arm/aspeed_ast2600.c >> @@ -46,6 +46,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = { >> [ASPEED_ADC] = 0x1E6E9000, >> [ASPEED_VIDEO] = 0x1E700000, >> [ASPEED_SDHCI] = 0x1E740000, >> + [ASPEED_EMMC] = 0x1E750000, >> [ASPEED_GPIO] = 0x1E780000, >> [ASPEED_GPIO_1_8V] = 0x1E780800, >> [ASPEED_RTC] = 0x1E781000, >> @@ -64,6 +65,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = { >> #define ASPEED_SOC_AST2600_MAX_IRQ 128 >> +/* Shared Peripheral Interrupt values below are offset by -32 from datasheet */ >> static const int aspeed_soc_ast2600_irqmap[] = { >> [ASPEED_UART1] = 47, >> [ASPEED_UART2] = 48, >> @@ -77,6 +79,7 @@ static const int aspeed_soc_ast2600_irqmap[] = { >> [ASPEED_ADC] = 78, >> [ASPEED_XDMA] = 6, >> [ASPEED_SDHCI] = 43, >> + [ASPEED_EMMC] = 15, >> [ASPEED_GPIO] = 40, >> [ASPEED_GPIO_1_8V] = 11, >> [ASPEED_RTC] = 13, >> @@ -206,6 +209,14 @@ static void aspeed_soc_ast2600_init(Object *obj) >> sysbus_init_child_obj(obj, "sdhci[*]", OBJECT(&s->sdhci.slots[i]), >> sizeof(s->sdhci.slots[i]), TYPE_SYSBUS_SDHCI); >> } >> + >> + sysbus_init_child_obj(obj, "emmc", OBJECT(&s->emmc), sizeof(s->emmc), >> + TYPE_ASPEED_SDHCI); >> + >> + object_property_set_int(OBJECT(&s->emmc), 1, "num-slots", &error_abort); >> + >> + sysbus_init_child_obj(obj, "emmc[*]", OBJECT(&s->emmc.slots[0]), > > Single block, so use "emmc" instead. > > Using "emmc": > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> I will do the fixes and resend. Thanks, C. > >> + sizeof(s->emmc.slots[0]), TYPE_SYSBUS_SDHCI); >> } >> /* >> @@ -497,6 +508,16 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) >> sc->memmap[ASPEED_SDHCI]); >> sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci), 0, >> aspeed_soc_get_irq(s, ASPEED_SDHCI)); >> + >> + /* eMMC */ >> + object_property_set_bool(OBJECT(&s->emmc), true, "realized", &err); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> + } >> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->emmc), 0, sc->memmap[ASPEED_EMMC]); >> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0, >> + aspeed_soc_get_irq(s, ASPEED_EMMC)); >> } >> static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data) >> >
>> + >> + sysbus_init_child_obj(obj, "emmc", OBJECT(&s->emmc), sizeof(s->emmc), >> + TYPE_ASPEED_SDHCI); >> + >> + object_property_set_int(OBJECT(&s->emmc), 1, "num-slots", &error_abort); >> + >> + sysbus_init_child_obj(obj, "emmc[*]", OBJECT(&s->emmc.slots[0]), > > Single block, so use "emmc" instead. Andrew, how should we call the objects in the slots ? "sdhci" ? Thanks, C.
On Fri, 10 Jan 2020, at 22:26, Cédric Le Goater wrote: > >> + > >> + sysbus_init_child_obj(obj, "emmc", OBJECT(&s->emmc), sizeof(s->emmc), > >> + TYPE_ASPEED_SDHCI); > >> + > >> + object_property_set_int(OBJECT(&s->emmc), 1, "num-slots", &error_abort); > >> + > >> + sysbus_init_child_obj(obj, "emmc[*]", OBJECT(&s->emmc.slots[0]), > > > > Single block, so use "emmc" instead. > > Andrew, how should we call the objects in the slots ? "sdhci" ? I think that's the right way to go, but maybe we need to rethink the naming at the controller level. Andrew
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h index e84380984f7b..90ac7f7ffa3b 100644 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -57,6 +57,7 @@ typedef struct AspeedSoCState { AspeedGPIOState gpio; AspeedGPIOState gpio_1_8v; AspeedSDHCIState sdhci; + AspeedSDHCIState emmc; } AspeedSoCState; #define TYPE_ASPEED_SOC "aspeed-soc" @@ -126,6 +127,7 @@ enum { ASPEED_MII4, ASPEED_SDRAM, ASPEED_XDMA, + ASPEED_EMMC, }; #endif /* ASPEED_SOC_H */ diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 4174e313cae5..0a7dfd29868b 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -171,6 +171,18 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, } } +static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo) +{ + BlockBackend *blk; + DeviceState *card; + + blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; + card = qdev_create(qdev_get_child_bus(DEVICE(sdhci), "sd-bus"), + TYPE_SD_CARD); + qdev_prop_set_drive(card, "drive", blk, &error_fatal); + object_property_set_bool(OBJECT(card), true, "realized", &error_fatal); +} + static void aspeed_machine_init(MachineState *machine) { AspeedBoardState *bmc; @@ -264,16 +276,11 @@ static void aspeed_machine_init(MachineState *machine) } for (i = 0; i < bmc->soc.sdhci.num_slots; i++) { - SDHCIState *sdhci = &bmc->soc.sdhci.slots[i]; - DriveInfo *dinfo = drive_get_next(IF_SD); - BlockBackend *blk; - DeviceState *card; + sdhci_attach_drive(&bmc->soc.sdhci.slots[i], drive_get_next(IF_SD)); + } - blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; - card = qdev_create(qdev_get_child_bus(DEVICE(sdhci), "sd-bus"), - TYPE_SD_CARD); - qdev_prop_set_drive(card, "drive", blk, &error_fatal); - object_property_set_bool(OBJECT(card), true, "realized", &error_fatal); + if (bmc->soc.emmc.num_slots) { + sdhci_attach_drive(&bmc->soc.emmc.slots[0], drive_get_next(IF_SD)); } arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo); diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index fb73c4043ea3..e59cdd291925 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -46,6 +46,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = { [ASPEED_ADC] = 0x1E6E9000, [ASPEED_VIDEO] = 0x1E700000, [ASPEED_SDHCI] = 0x1E740000, + [ASPEED_EMMC] = 0x1E750000, [ASPEED_GPIO] = 0x1E780000, [ASPEED_GPIO_1_8V] = 0x1E780800, [ASPEED_RTC] = 0x1E781000, @@ -64,6 +65,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = { #define ASPEED_SOC_AST2600_MAX_IRQ 128 +/* Shared Peripheral Interrupt values below are offset by -32 from datasheet */ static const int aspeed_soc_ast2600_irqmap[] = { [ASPEED_UART1] = 47, [ASPEED_UART2] = 48, @@ -77,6 +79,7 @@ static const int aspeed_soc_ast2600_irqmap[] = { [ASPEED_ADC] = 78, [ASPEED_XDMA] = 6, [ASPEED_SDHCI] = 43, + [ASPEED_EMMC] = 15, [ASPEED_GPIO] = 40, [ASPEED_GPIO_1_8V] = 11, [ASPEED_RTC] = 13, @@ -206,6 +209,14 @@ static void aspeed_soc_ast2600_init(Object *obj) sysbus_init_child_obj(obj, "sdhci[*]", OBJECT(&s->sdhci.slots[i]), sizeof(s->sdhci.slots[i]), TYPE_SYSBUS_SDHCI); } + + sysbus_init_child_obj(obj, "emmc", OBJECT(&s->emmc), sizeof(s->emmc), + TYPE_ASPEED_SDHCI); + + object_property_set_int(OBJECT(&s->emmc), 1, "num-slots", &error_abort); + + sysbus_init_child_obj(obj, "emmc[*]", OBJECT(&s->emmc.slots[0]), + sizeof(s->emmc.slots[0]), TYPE_SYSBUS_SDHCI); } /* @@ -497,6 +508,16 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) sc->memmap[ASPEED_SDHCI]); sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci), 0, aspeed_soc_get_irq(s, ASPEED_SDHCI)); + + /* eMMC */ + object_property_set_bool(OBJECT(&s->emmc), true, "realized", &err); + if (err) { + error_propagate(errp, err); + return; + } + sysbus_mmio_map(SYS_BUS_DEVICE(&s->emmc), 0, sc->memmap[ASPEED_EMMC]); + sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0, + aspeed_soc_get_irq(s, ASPEED_EMMC)); } static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)