[U-Boot][bug report] clearfog: EMMC boot broken on clearfog pro
Martin Rowe
martin.p.rowe at gmail.com
Tue Jan 10 07:01:08 CET 2023
On Mon, 9 Jan 2023 at 21:34, Petr Štetiar <ynezz at true.cz> wrote:
>
> 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?
Much of the work that introduced these issues appears to have been
making a single kwboot version that works for all cases, rather than
having a board specific tool. When I tried creating a patch, that's
where I got stuck because there doesn't seem to be enough information
in the headers to decide between SD and eMMC. They both use the same
final header address, so maybe there's a possible solution, though it
still doesn't help with the sector size calculation.
> > > 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?
The commit from the git bisect is one of dozens that are associated
with a major refactoring of kwboot and kwbimage; you probably want to
go back to the previous merge commit to get a working build.
> > 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?
I didn't test SATA; I used a known working image to help isolate the
problem and have a stable base to test from.
> > 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 = <µsom_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 = <µsom_sdhci_pins
> > &clearfog_sdhci_cd_pins>;
More information about the U-Boot
mailing list