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

[Philippe Mathieu-Daudé]

On 5/7/24 07:38, Cédric Le Goater wrote:
> 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 */

This field is R/W and expected to be configured by the guest.

Why the guest (u-boot) doesn't detect partitioning support?

See eMMC v4.5 section 7.4.60 PARTITIONING_SUPPORT [160] and earlier

  For e•MMC 4.5 Devices, Bit 2-0 in PARTITIONING_SUPPORT
  shall be set to 1.

We don't set it so far.

I see in u-boot mmc_startup_v4():

    /* store the partition info of emmc */
    mmc->part_support = ext_csd[EXT_CSD_PARTITIONING_SUPPORT];
    if ((ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & PART_SUPPORT) ||
        ext_csd[EXT_CSD_BOOT_MULT])
            mmc->part_config = ext_csd[EXT_CSD_PART_CONF];
    if (part_completed &&
        (ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & ENHNCD_SUPPORT))
            mmc->part_attr = ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE];

I'm still waiting for the eMMC prerequisite to be reviewed
before looking at the eMMC patches in detail :/

Regards,

Phil.