[RFC] mmc: Remove alignment hole for cmdidx in struct mmc_cmd

[Simon Glass]

On Fri, 29 Sept 2023 at 17:07, Jonas Karlman <jonas at kwiboo.se> wrote:
>
> The alignment hole caused by cmdidx in struct mmc_cmd cause strange
> issues together with the peephole2 optimization on Amlogic SoCs.
> Following was observed while working on SPL support for Amlogic SoCs.
>
> sd_get_capabilities() normally issue a CMD55 followed by a CMD51.
> However, on at least Amlogic S905 (Cortex-A53) and S905X3 (Cortex-A55),
> CMD55 was instead followed by CMD8 (and a few reties) in SPL.
>
> Code from the call site:
>
>   cmd.cmdidx = SD_CMD_APP_SEND_SCR; // 51
>   ...
>   data.blocksize = 8;
>   ...
>   err = mmc_send_cmd_retry(mmc, &cmd, &data, 3);
>
> Running the code with MMC_TRACE enabled shows:
>
> CMD_SEND:55
>                 ARG                      0x50480000
>                 MMC_RSP_R1,5,6,7         0x00000920
> CMD_SEND:8
>                 ARG                      0x00000000
>                 RET                      -110
>
> Removing the alignment hole by changing cmdidx from ushort to uint or
> building with -fno-peephole2 flag seem to resolve this issue.
>
> CMD_SEND:55
>                 ARG                      0x50480000
>                 MMC_RSP_R1,5,6,7         0x00000920
> CMD_SEND:51
>                 ARG                      0x00000000
>                 MMC_RSP_R1,5,6,7         0x00000920
>
> Same issue was observed building U-Boot with gcc 8-13. Please advise on
> how to best work around this possible gcc optimization bug.
>
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> ---
>  arch/arm/cpu/armv8/config.mk | 2 ++
>  include/mmc.h                | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/cpu/armv8/config.mk b/arch/arm/cpu/armv8/config.mk
> index 4d74b2a533e0..7177dcd7c73b 100644
> --- a/arch/arm/cpu/armv8/config.mk
> +++ b/arch/arm/cpu/armv8/config.mk
> @@ -7,6 +7,8 @@ PLATFORM_RELFLAGS += $(call cc-option,-mbranch-protection=none)
>  PF_NO_UNALIGNED := $(call cc-option, -mstrict-align)
>  PLATFORM_CPPFLAGS += $(PF_NO_UNALIGNED)
>
> +PLATFORM_CPPFLAGS += $(call cc-option,-fno-peephole2)

Eek really?

> +
>  EFI_LDS := elf_aarch64_efi.lds
>  EFI_CRT0 := crt0_aarch64_efi.o
>  EFI_RELOC := reloc_aarch64_efi.o
> diff --git a/include/mmc.h b/include/mmc.h
> index 9aef31ea5deb..7b3868bb5664 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -413,7 +413,7 @@ struct mmc_cid {
>  };
>
>  struct mmc_cmd {
> -       ushort cmdidx;
> +       uint cmdidx;

This bit looks OK.

>         uint resp_type;
>         uint cmdarg;
>         uint response[4];
> --
> 2.42.0
>

Regards,
Simon