On Fri, 6 Sept 2024 at 17:51, Jan Luebbe <j...@pengutronix.de> wrote:
>
> The enable bits in the EXT_CSD_PART_CONFIG ext_csd register do *not*
> specify whether the boot partitions exist, but whether they are enabled
> for booting. Existence of the boot partitions is specified by a
> EXT_CSD_BOOT_MULT != 0.
>
> Currently, in the case of boot-partition-size=1M and boot-config=0,
> Linux detects boot partitions of 1M. But as sd_bootpart_offset always
> returns 0, all reads/writes are mapped to the same offset in the backing
> file.
>
> Fix this bug by calculating the offset independent of which partition is
> enabled for booting.

Looking at the spec this change seems correct to me.

Can you elaborate on when users might run into this bug?
As far as I can see only aspeed.c sets boot-partition-size,
and when it does so it also sets boot-config to 8. Or are
we fixing this for the benefit of future board types?

> Signed-off-by: Jan Luebbe <j...@pengutronix.de>
> ---
>  hw/sd/sd.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index a140a32ccd46..26d6eebe898d 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -774,19 +774,12 @@ static uint32_t sd_blk_len(SDState *sd)
>   */
>  static uint32_t sd_bootpart_offset(SDState *sd)
>  {
> -    bool partitions_enabled;
>      unsigned partition_access;
>
>      if (!sd->boot_part_size || !sd_is_emmc(sd)) {
>          return 0;
>      }
>
> -    partitions_enabled = sd->ext_csd[EXT_CSD_PART_CONFIG]
> -                                   & EXT_CSD_PART_CONFIG_EN_MASK;
> -    if (!partitions_enabled) {
> -        return 0;
> -    }
> -
>      partition_access = sd->ext_csd[EXT_CSD_PART_CONFIG]
>                                   & EXT_CSD_PART_CONFIG_ACC_MASK;
>      switch (partition_access) {
> --

thanks
-- PMM

Reply via email to