Re: [PATCH 5/8] aspeed: Set eMMC 'boot-config' property to reflect HW strapping

[Cédric Le Goater]

On 7/5/24 5:41 AM, Andrew Jeffery wrote:
> On Thu, 2024-07-04 at 07:36 +0200, Cédric Le Goater wrote:
> > From: Cédric Le Goater <clg@kaod.org>
> >
> > When the boot-from-eMMC HW strapping bit is set, use the 'boot-config'
> > property to set the boot config register to boot from the first boot
> > area partition of the eMMC device.
> >
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> >   hw/arm/aspeed.c | 15 +++++++++++----
> >   1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > index 756deb91efd1..135f4eb72215 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -327,7 +327,8 @@ void aspeed_board_init_flashes(AspeedSMCState *s, const 
> > char *flashtype,
> >       }
> >   }
> >   
> > -static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc)
> > +static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
> > +                               bool boot_emmc)
> >   {
> >           DeviceState *card;
> >   
> > @@ -335,6 +336,9 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc)
> >               return;
> >           }
> >           card = qdev_new(emmc ? TYPE_EMMC : TYPE_SD_CARD);
> > +        if (emmc) {
> > +            qdev_prop_set_uint8(card, "boot-config", boot_emmc ? 0x48 : 0x0);
>
> 0x48 feels a little bit magic. I poked around a bit and there are some
> boot-config macros, but not the ones you need and they're all in an
> "internal" header anyway. I guess this is fine for now?

You are right and we should be using these :

hw/sd/sdmmc-internal.h:#define EXT_CSD_PART_CONFIG             179     /* R/W */

hw/sd/sdmmc-internal.h:#define EXT_CSD_PART_CONFIG_ACC_MASK            (0x7)
hw/sd/sdmmc-internal.h:#define EXT_CSD_PART_CONFIG_ACC_DEFAULT         (0x0)
hw/sd/sdmmc-internal.h:#define EXT_CSD_PART_CONFIG_ACC_BOOT0           (0x1)
hw/sd/sdmmc-internal.h:#define EXT_CSD_PART_CONFIG_EN_MASK             (0x7 << 
3)
hw/sd/sdmmc-internal.h:#define EXT_CSD_PART_CONFIG_EN_BOOT0            (0x1 << 
3)
hw/sd/sdmmc-internal.h:#define EXT_CSD_PART_CONFIG_EN_USER             (0x7 << 
3)

So I wonder where the 0x48 is coming from. Will change.

> Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>


Thanks,

C.