On Tue, 10 Dec 2019, at 19:26, Cédric Le Goater wrote:
> On 10/12/2019 01:52, Andrew Jeffery wrote:
> > The AST2600 includes a second cut-down version of the SD/MMC controller
> > found in the AST2500, named the eMMC controller. It's cut down in the
> > sense that it only supports one slot rather than two, but it brings the
> > total number of slots supported by the AST2600 to three.
> >
> > The existing code assumed that the SD controller always provided two
> > slots. Rework the SDHCI object to expose the number of slots as a
> > property to be set by the SoC configuration.
> >
> > Signed-off-by: Andrew Jeffery <and...@aj.id.au>
>
> Reviewed-by: Cédric Le Goater <c...@kaod.org>
>
> One minor question below.
>
>
> > ---
> > hw/arm/aspeed.c | 2 +-
> > hw/arm/aspeed_ast2600.c | 2 ++
> > hw/arm/aspeed_soc.c | 3 +++
> > hw/sd/aspeed_sdhci.c | 11 +++++++++--
> > include/hw/sd/aspeed_sdhci.h | 1 +
> > 5 files changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > index 028191ff36fc..862549b1f3a9 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -259,7 +259,7 @@ static void aspeed_board_init(MachineState *machine,
> > cfg->i2c_init(bmc);
> > }
> >
> > - for (i = 0; i < ARRAY_SIZE(bmc->soc.sdhci.slots); i++) {
> > + 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;
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > index 931887ac681f..931ee5aae183 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -208,6 +208,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
> > sysbus_init_child_obj(obj, "sdc", OBJECT(&s->sdhci), sizeof(s->sdhci),
> > TYPE_ASPEED_SDHCI);
> >
> > + object_property_set_int(OBJECT(&s->sdhci), 2, "num-slots",
> > &error_abort);
>
> OK. This defines 2 SDHCI slots for the ast2600 SoC, but
>
> > +
> > /* Init sd card slot class here so that they're under the correct
> > parent */
> > for (i = 0; i < ASPEED_SDHCI_NUM_SLOTS; ++i) {
> > sysbus_init_child_obj(obj, "sdhci[*]", OBJECT(&s->sdhci.slots[i]),
> > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> > index f4fe243458fd..3498f55603f2 100644
> > --- a/hw/arm/aspeed_soc.c
> > +++ b/hw/arm/aspeed_soc.c
> > @@ -215,6 +215,9 @@ static void aspeed_soc_init(Object *obj)
> > sysbus_init_child_obj(obj, "sdc", OBJECT(&s->sdhci), sizeof(s->sdhci),
> > TYPE_ASPEED_SDHCI);
> >
> > + object_property_set_int(OBJECT(&s->sdhci), ASPEED_SDHCI_NUM_SLOTS,
> > + "num-slots", &error_abort);
>
>
> why use ASPEED_SDHCI_NUM_SLOTS here ?
No good reason. I'll just switch it to '2' like in the 2600.
Andrew