[U-Boot][bug report] clearfog: EMMC boot broken on clearfog pro

Petr Štetiar ynezz at true.cz
Mon Jan 9 22:34:02 CET 2023


Martin Rowe <martin.p.rowe at gmail.com> [2023-01-03 03:20:49]:

[Adding Florian, Tomasz and Oli to the Cc: loop]

Hi,

> I'm following up on this bug report from September (
> https://lists.denx.de/pipermail/u-boot/2022-September/494811.html )
> I also hit this issue on my Clearfog Pro when booting with eMMC.

FYI we've recently got similar bug report in OpenWrt as well
https://github.com/openwrt/openwrt/issues/11661

There Oli is having problem to boot v2022.07 from SD card on his Clearfog Pro,
version v2021.01 was working fine for him.

> Florian's bug report was incredibly helpful in isolating the issue.

Indeed, that eMMC handling looks quite different, as it seems that BootROM is
looking at offset 0x0:

 -> BootROM - 1.73
 -> Booting from MMC
 -> BootROM: Bad header at offset 00000000
 -> BootROM: Bad header at offset 00200000
 -> Switching BootPartitions.
 -> BootROM: Invalid HDR source addr

but in Oli's SD card boot log we can see, that it looks at 0x200 offset first:

 BootROM - 1.73
 Booting from MMC
 BootROM: Bad header at offset 00000200
 BootROM: Bad header at offset 00004400
 BootROM: Bad header at offset 00200000

So perhaps SD and eMMC booting needs to be treated differently?

> > After binary patching the the srcaddr (srcaddr = srcaddr * 512) in the
> > kwbimage v1 header, I got the BootROM to execute the SPL again.

So this means following commit 501a54a29cc2 ("tools: kwbimage: Fix
generation of SATA, SDIO and PCIe images") ? As reverting that commit yields
following kwb image diff related to srcaddr changes:

 --- v2021.10-rc1-108-g501a54a29cc2.xxd  2023-01-09 20:37:08.250562742 +0100
 +++ v2021.10-rc1-108-g501a54a29cc2-revert.xxd   2023-01-09 20:37:17.902718024 +0100
 @@ -1,5 +1,5 @@
 -00000000: ae00 0000 0098 0600 0102 0080 4001 0000  ............ at ...
 -00000010: c0ff 7f00 0000 8000 0000 0000 0000 01c2  ................
 +00000000: ae00 0000 4496 0600 0102 0080 0080 0200  ....D...........
 +00000010: c0ff 7f00 0000 8000 0000 0000 0000 0145  ...............E
  00000020: 0201 60df 0200 0000 5b00 0000 6800 0000  ..`.....[...h...
  00000030: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  00000040: 0f00 00ea 14f0 9fe5 14f0 9fe5 14f0 9fe5  ................

Which in human readable form is following:

 --- v2021.10-rc1-108-g501a54a29cc2.header	2023-01-09 20:51:56.116867268 +0100
 +++ v2021.10-rc1-108-g501a54a29cc2-revert.header	2023-01-09 20:52:55.987832692 +0100
 @@ -1,9 +1,9 @@
  blockid = 0xae,
  nandeccmode = 0x0,
  nandpagesize = 0x0,
 -blocksize = 0x69800,
 +blocksize = 0x69644,
  rsvd1 = 0x80000201,
 -srcaddr = 0x140,
 +srcaddr = 0x28000,
  destaddr = 0x7fffc0,
  execaddr = 0x800000,
  satapiomode = 0x0,
 @@ -11,4 +11,4 @@
  ddrinitdelay = 0x0,
  rsvd2 = 0x0,
  ext = 0x1,
 -checksum = 0xd8
 +checksum = 0x51

Does it mean, that going back to the commit before this change, thus commit
bd487ce08135 ("tools: kwbimage: Add constant for SDIO bootfrom") makes your
device bootable again from SD, SATA and eMMC?

In other words, it would help to know, which commit after v2021.01 breaks
booting of SD/SATA/eMMC for you. Could you help find it?

> I've gone a bit further and gotten eMMC boot working again, but I'm
> not a C programmer so this is just a proof of concept/hack, not a
> patch, and it would break SD card boot. Happy to test any proper
> patches, though!
>
> These steps require a working u-boot from another boot medium. I used
> SolidRun's u-boot-clearfog-pro-sata.kwb from
> https://images.solid-run.com/A38X/U-Boot

So am I parsing it properly, that you're not even able to boot v2022.10 from SATA?

> Steps to fix eMMC boot:
> # In u-boot source
> 1. Apply the patches at the bottom of the email to v2022.10 release
> 2. make clearfog_defconfig
> 3. make menuconfig
>     - Set CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1
>     - I also selected
>         CONFIG_CLEARFOG_CON3_SATA=y
>         CONFIG_CLEARFOG_CON2_SATA=y
>         CONFIG_CLEARFOG_SFP_25GB=y
>         CONFIG_DDR_RESET_ON_TRAINING_FAILURE=y
>      but they should not affect eMMC booting
> 4. make
> 5. Copy u-boot-spl.kwb to booted linux on Clearfog Pro
> # In linux on Clearfog Pro
> 6. echo 0 > /sys/block/mmcblk0boot0/force_ro
> 7. dd if=/dev/zero of=/dev/mmcblk0boot0 conv=sync [probably not required]
> 8. dd if=u-boot-spl.kwb of=/dev/mmcblk0boot0 conv=sync
> 9. dd if=u-boot-spl.kwb of=/dev/mmcblk0 bs=512 seek=1
> 10. reboot
> # In working u-boot on Clearfog Pro
> 11. mmc partconf 0 1 1 0 [doesn't seem to be needed, but recommended
> by board/solidrun/clearfog/README]
> # On board
> 12. Set boot select DIP to eMMC (00111)
> 13. Power on and boot succeeds
> 
> The instructions also work with
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x0 and step 9 changed to `dd
> if=u-boot-spl.kwb of=/dev/mmcblk0 conv=sync`, but this will clobber
> any partition table that exists on eMMC making it mostly useless other
> than for booting.
>
> Weirdly both the boot and the data parts of the eMMC need to have
> u-boot. It looks like SPL comes from a boot partition, but then tries
> to load u-boot proper from the data partition. This is also noted
> here: https://wiki.debian.org/InstallingDebianOn/ClearFog
> 
> Pali Rohar appears to have made some significant improvements to mvebu
> booting around July 2021, but these changes seem to only account for
> an SD card for MMC boot, not eMMC which doesn't appear to need the
> sector (512) multiplier. 

Well, Oli has reported broken SD card booting as well, so probably UART/SPI
boot source is the only tested/working combination?

> Most of the patches below remove special handling added for the
> IBR_HDR_SDIO_ID case which almost certainly breaks SD card booting.
> The removal of the error for CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR != 0
> appears to have no side effects for other boot modes.
>
> The dts change is a long known issue and
> functionally matches a patch carried by armbian and discussed here:
> https://forum.armbian.com/topic/3072-clearfog-pro-emmc-requires-sd-card-to-detect-device/
> 
> Let me know if I can help get a permanent fix merged.
>
> Martin
> 
> 
> diff --git a/arch/arm/dts/armada-388-clearfog.dts
> b/arch/arm/dts/armada-388-clearfog.dts
> index e4164f49b2..29a608abcf 100644
> --- a/arch/arm/dts/armada-388-clearfog.dts
> +++ b/arch/arm/dts/armada-388-clearfog.dts
> @@ -101,7 +101,7 @@
> 
>                         sdhci at d8000 {
>                                 bus-width = <4>;
> -                               cd-gpios = <&gpio0 20 GPIO_ACTIVE_LOW>;
> +                               non-removable;
>                                 no-1-8-v;
>                                 pinctrl-0 = <&microsom_sdhci_pins
>                                              &clearfog_sdhci_cd_pins>;
> diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> index ca2d5a59d7..00334562cf 100644
> --- a/arch/arm/mach-mvebu/spl.c
> +++ b/arch/arm/mach-mvebu/spl.c
> @@ -44,9 +44,6 @@
>  #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
>  #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION is unsupported
>  #endif
> -#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR) &&
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR != 0
> -#error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR must be set to 0
> -#endif
>  #if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) && \
>      CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0
>  #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET must be set to 0
> @@ -196,14 +193,6 @@ int spl_parse_board_header(struct spl_image_info
> *spl_image,
>                 spl_image->offset *= 512;
>         }
> 
> -       /*
> -        * For SDIO (eMMC) srcaddr is specified in number of sectors.
> -        * This expects that sector size is 512 bytes and recalculates
> -        * data offset to bytes.
> -        */
> -       if (IS_ENABLED(CONFIG_SPL_MMC) && mhdr->blockid == IBR_HDR_SDIO_ID)
> -               spl_image->offset *= 512;
> -
>         if (spl_image->offset % 4 != 0) {
>                 printf("ERROR: Wrong srcaddr (0x%08x) in kwbimage\n",
>                        spl_image->offset);
> diff --git a/tools/kwbimage.c b/tools/kwbimage.c
> index 94b7685392..43b51ffd1a 100644
> --- a/tools/kwbimage.c
> +++ b/tools/kwbimage.c
> @@ -1021,15 +1021,6 @@ static void *image_create_v0(size_t *imagesz,
> struct image_tool_params *params,
>         if (main_hdr->blockid == IBR_HDR_SATA_ID)
>                 main_hdr->srcaddr = cpu_to_le32(headersz / 512 + 1);
> 
> -       /*
> -        * For SDIO srcaddr is specified in number of sectors starting from
> -        * sector 0. The main header is stored at sector number 0.
> -        * This expects sector size to be 512 bytes.
> -        * Header size is already aligned.
> -        */
> -       if (main_hdr->blockid == IBR_HDR_SDIO_ID)
> -               main_hdr->srcaddr = cpu_to_le32(headersz / 512);
> -
>         /* For PCIe srcaddr is not used and must be set to 0xFFFFFFFF. */
>         if (main_hdr->blockid == IBR_HDR_PEX_ID)
>                 main_hdr->srcaddr = cpu_to_le32(0xFFFFFFFF);
> @@ -1478,15 +1469,6 @@ static void *image_create_v1(size_t *imagesz,
> struct image_tool_params *params,
>         if (main_hdr->blockid == IBR_HDR_SATA_ID)
>                 main_hdr->srcaddr = cpu_to_le32(headersz / 512 + 1);
> 
> -       /*
> -        * For SDIO srcaddr is specified in number of sectors starting from
> -        * sector 0. The main header is stored at sector number 0.
> -        * This expects sector size to be 512 bytes.
> -        * Header size is already aligned.
> -        */
> -       if (main_hdr->blockid == IBR_HDR_SDIO_ID)
> -               main_hdr->srcaddr = cpu_to_le32(headersz / 512);
> -
>         /* For PCIe srcaddr is not used and must be set to 0xFFFFFFFF. */
>         if (main_hdr->blockid == IBR_HDR_PEX_ID)
>                 main_hdr->srcaddr = cpu_to_le32(0xFFFFFFFF);
> @@ -2036,14 +2018,6 @@ static int kwbimage_verify_header(unsigned char
> *ptr, int image_size,
>                 offset *= 512;
>         }
> 
> -       /*
> -        * For SDIO srcaddr is specified in number of sectors.
> -        * This expects that sector size is 512 bytes and recalculates
> -        * data offset to bytes.
> -        */
> -       if (blockid == IBR_HDR_SDIO_ID)
> -               offset *= 512;
> -
>         /*
>          * For PCIe srcaddr is always set to 0xFFFFFFFF.
>          * This expects that data starts after all headers.
> @@ -2405,9 +2379,6 @@ static int kwbimage_extract_subimage(void *ptr,
> struct image_tool_params *params
>                         offset *= 512;
>                 }
> 
> -               if (mhdr->blockid == IBR_HDR_SDIO_ID)
> -                       offset *= 512;
> -
>                 if (mhdr->blockid == IBR_HDR_PEX_ID && offset == 0xFFFFFFFF)
>                         offset = header_size;
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index da4fe32da2..188f944263 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1894,10 +1894,6 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
>                 hdr->srcaddr = cpu_to_le32((srcaddr - 1) * 512);
>                 break;
> 
> -       case IBR_HDR_SDIO_ID:
> -               hdr->srcaddr = cpu_to_le32(srcaddr * 512);
> -               break;
> -
>         case IBR_HDR_PEX_ID:
>                 if (srcaddr == 0xFFFFFFFF)
>                         hdr->srcaddr = cpu_to_le32(hdrsz);
> 
> 
> Kernel fix for eMMC detect pin for completeness (without it kernel
> won't see eMMC, but u-boot will):
> diff --git a/arch/arm/boot/dts/armada-388-clearfog.dtsi
> b/arch/arm/boot/dts/armada-388-clearfog.dtsi
> index f8a06ae4a3c9..2276844d26ca 100644
> --- a/arch/arm/boot/dts/armada-388-clearfog.dtsi
> +++ b/arch/arm/boot/dts/armada-388-clearfog.dtsi
> @@ -42,7 +42,7 @@ sata at e0000 {
> 
>                         sdhci at d8000 {
>                                 bus-width = <4>;
> -                               cd-gpios = <&gpio0 20 GPIO_ACTIVE_LOW>;
> +                               non-removable;
>                                 no-1-8-v;
>                                 pinctrl-0 = <&microsom_sdhci_pins
>                                              &clearfog_sdhci_cd_pins>;


More information about the U-Boot mailing list