On Sat, 2024-11-02 at 15:39 +0100, Cédric Le Goater wrote:
> Commit e554e45b4478 ("aspeed: Tune eMMC device properties to reflect
> HW strapping") added support to boot from an eMMC device by setting
> the boot properties of the eMMC device. This change made the
> assumption that the device always has boot areas.
>
> However, if the machine boots from the flash device (or -kernel) and
> uses an eMMC device without boot areas, support would be broken. This
> impacts the ast2600-evb machine which can choose to boot from flash or
> eMMC using the "boot-emmc" machine option.
>
> To provide some flexibility for Aspeed machine users to use different
> flavors of eMMC devices (with or without boot areas), do not set the
> eMMC device boot properties when the machine is not configured to boot
> from eMMC. However, this approach makes another assumption about eMMC
> devices, namely that eMMC devices from which the machine does not boot
> do not have boot areas.
>
> A preferable alternative would be to add support for user creatable
> eMMC devices and define the device boot properties on the QEMU command
> line :
>
> -blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb.raw \
> -device
> emmc,bus=sdhci-bus.2,drive=emmc0,boot-partition-size=1048576,boot-config=8
>
> This is a global change requiring more thinking. Nevertheless, in the
> case of the ast2600-evb machine booting from an eMMC device and when
> default devices are created, the proposed change still makes sense
> since the device is required to have boot areas.
>
> Cc: Jan Luebbe <[email protected]>
> Fixes: e554e45b4478 ("aspeed: Tune eMMC device properties to reflect
> HW strapping")
> Signed-off-by: Cédric Le Goater <[email protected]>
> Tested-by: Guenter Roeck <[email protected]>
> ---
> hw/arm/aspeed.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index e161e6b1c582..ac6d8dde71b3 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -338,7 +338,18 @@ static void sdhci_attach_drive(SDHCIState *sdhci,
> DriveInfo *dinfo, bool emmc,
> return;
> }
> card = qdev_new(emmc ? TYPE_EMMC : TYPE_SD_CARD);
> - if (emmc) {
> +
> + /*
> + * Force the boot properties of the eMMC device only when the
> + * machine is strapped to boot from eMMC. Without these
> + * settings, the machine would not boot.
> + *
> + * This also allows the machine to use an eMMC device without
> + * boot areas when booting from the flash device (or -kernel)
> + * Ideally, the device and its properties should be defined on
> + * the command line.
> + */
> + if (emmc && boot_emmc) {
> qdev_prop_set_uint64(card, "boot-partition-size", 1 * MiB);
> qdev_prop_set_uint8(card, "boot-config",
> boot_emmc ? 0x1 << 3 : 0x0);
With the change proposed by Guenter, this looks good to me.
Thanks,
Jan
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
