From: "Andrew Jeffery" <andrew@aj.id.au>
To: "Cédric Le Goater" <clg@kaod.org>, qemu-arm@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
Joel Stanley <joel@jms.id.au>,
qemu-devel@nongnu.org
Subject: Re: [PATCH 1/2] hw/sd: Configure number of slots exposed by the ASPEED SDHCI model
Date: Wed, 11 Dec 2019 08:47:50 +1030 [thread overview]
Message-ID: <27aa4292-cc3b-4a9b-a13d-ab1eced3f952@www.fastmail.com> (raw)
In-Reply-To: <c49584df-016d-9a84-b96e-6a3b2e98c6a1@kaod.org>
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 <andrew@aj.id.au>
>
> Reviewed-by: Cédric Le Goater <clg@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
next prev parent reply other threads:[~2019-12-10 22:17 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-10 0:52 [PATCH 0/2] hw/arm: ast2600: Wire up eMMC controller Andrew Jeffery
2019-12-10 0:52 ` [PATCH 1/2] hw/sd: Configure number of slots exposed by the ASPEED SDHCI model Andrew Jeffery
2019-12-10 7:49 ` Philippe Mathieu-Daudé
2019-12-10 8:56 ` Cédric Le Goater
2019-12-10 22:17 ` Andrew Jeffery [this message]
2019-12-10 0:52 ` [PATCH 2/2] hw/arm: ast2600: Wire up the eMMC controller Andrew Jeffery
2019-12-10 12:52 ` Cédric Le Goater
2019-12-10 22:15 ` Andrew Jeffery
2019-12-10 8:53 ` [PATCH 0/2] hw/arm: ast2600: Wire up " Cédric Le Goater
2019-12-10 22:19 ` Andrew Jeffery
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=27aa4292-cc3b-4a9b-a13d-ab1eced3f952@www.fastmail.com \
--to=andrew@aj.id.au \
--cc=clg@kaod.org \
--cc=joel@jms.id.au \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).