[U-Boot] mx6cuboxi fails to load u-boot.img
Adam Ford
aford173 at gmail.com
Sun Oct 6 12:30:08 UTC 2019
On Sun, Oct 6, 2019 at 7:22 AM Baruch Siach <baruch at tkos.co.il> wrote:
>
> Hi Adam,
>
> On Sun, Oct 06 2019, Adam Ford wrote:
> > On Sun, Oct 6, 2019 at 5:23 AM Baruch Siach <baruch at tkos.co.il> wrote:
> >> (Adding MMC and i.MX maintainers to Cc)
> >>
> >> On Fri, Sep 27 2019, Adam Ford wrote:
> >> > On Fri, Sep 27, 2019 at 4:38 AM Jonathan Gray <jsg at jsg.id.au> wrote:
> >> >>
> >> >> On Thu, Sep 26, 2019 at 05:07:21PM -0300, Fabio Estevam wrote:
> >> >> > Hi Vagrant,
> >> >> >
> >> >> > On Thu, Sep 26, 2019 at 4:16 PM Vagrant Cascadian <vagrant at debian.org> wrote:
> >> >> > >
> >> >> > > I just tested mx6cuboxi with 2019.10-rc4, and it fails to load
> >> >> > > u-boot.img from MMC:
> >> >> > >
> >> >> > > 1 2019-09-26_17:31:27.63089 U-Boot SPL 2019.10-rc4+dfsg-1 (Sep 24 2019 -
> >> >> > > 08:03:23 +0000)
> >> >> > > 2 2019-09-26_17:31:27.63092 Trying to boot from MMC2
> >> >> > > 3 2019-09-26_17:31:27.63095 MMC Device 1 not found
> >> >> > > 4 2019-09-26_17:31:27.63097 spl: could not find mmc device 1. error: -19
> >> >> > > 5 2019-09-26_17:31:27.63099 SPL: failed to boot from all boot devices
> >> >> > > 6 2019-09-26_17:31:27.63101 ### ERROR ### Please RESET the board ###
> >> >> >
> >> >> > Thanks for reporting this issue.
> >> >> >
> >> >> > Unfortunately, I don't have access to my Cuboxi, so I am adding Jon
> >> >> > and Baruch on Cc.
> >> >>
> >> >> Works after reverting the following commit.
> >> >>
> >> > I am going to argue that making the board comply with DM_MMC is why I
> >> > needed to make the patch, because when booting from MMC2, the function
> >> > was returning MMC1 which was clearly not the boot source.
> >> >
> >> > If the boards that fail accept MMC2 as a response when booting from
> >> > MMC2, that seems like a bug on the indvidual boards. Instead they
> >> > should setup their boot sequence to configure MMC2 when MMC2 is the
> >> > boot source. Instead, it seems like some boards are configuring MMC1
> >> > with MMC2 info which only prolongs the conversion to DM_MMC.
> >> >
> >> > If we revert the patch, then boards like imx6_logic who rely solely on
> >> > device tree and DM_MMC for booting will have to manually override the
> >> > MMC driver in order to boot from MMC2, and that seems like a step
> >> > backwards. I would argue that this board should migrate to DM_MMC and
> >> > use the device tree to boot, and the problem should go away.
> >>
> >> I started working on migration to DM_MMC as you suggested. Unfortunately
> >> I can't see how this solves the problem for Cubox-i/Hummingboard, nor in
> >> the general case.
> >>
> >> The imx6_logic board happens to use only usdhc1 and usdhc2 for boot, and
> >> both are always enabled. This matches perfectly to BOOT_DEVICE_MMC{1,2},
> >> and their corresponding DT representation.
> >>
> >> However, the 'index' parameter in uclass_get_device() that is set
> >> according to BOOT_DEVICE_MMC{1,2} selection has nothing to do with the
> >> usdhcX sequence number. It simply returns the Nth probed SD/eMMC device
> >> (see uclass_find_device()). In the case of Cubox-i/Hummingboard, usdhc1
> >> is never used for boot, usdhc2 is always an SD card, and usdhc3 is an
> >> optional eMMC. When booting from SD card, uclass_get_device(), returns
> >> -ENODEV when eMMC is not available, or the eMMC device when it is
> >> available. In both cases, boot fails.
>
> I think you missed this part. See more below.
>
> >> In addition, your patch returns BOOT_DEVICE_MMC2 only for usdhc2
> >> boot. All others return BOOT_DEVICE_MMC1. What about usdhc{3,4}?
> >>
> >
> > My patch only extended it to support MMC1 or MMC2. I don't have
> > hardware to test MMC3 or MMC4, nor where they defined in the boot
> > table.
> > The intention what to eliminate all functions from board files which
> > did a something like:
> >
> > static int mmc_init_spl(bd_t *bis)
> > {
> > struct src *psrc = (struct src *)SRC_BASE_ADDR;
> > unsigned reg = readl(&psrc->sbmr1) >> 11;
> >
> > /*
> > * Upon reading BOOT_CFG register the following map is done:
> > * Bit 11 and 12 of BOOT_CFG register can determine the current
> > * mmc port
> > * 0x1 SD2
> > * 0x2 SD3
> > */
> > switch (reg & 0x3) {
> > ...
> > }
> > }
> >
> >> How is all that intended to work?
> >
> > Basically the above function determines which BOOT_CFG regiser is used
> > and returns sets MMC1 values to the returned value. In my case MMC1
> > was going to be configured with the clock and pin mux of mmc1 or 2.
> > In your case, mmc1 gets configured with the information for mmc2 or 3.
>
> But there is another side effect to this change. The code in spl_mmc.c
> uses BOOT_DEVICE_MMC* macros to determine the boot device as I mentioned
> above. These macros have nothing to do with usdhcX sequence
> numbering. When usdhc1 is missing, BOOT_DEVICE_MMC1 refers to usdhc2
> which happens to be the first probed device, and BOOT_DEVICE_MMC2
> becomes usdhc3. This code is broken since commit 14d319b185.
>
> spl_boot_device() can not blindly return BOOT_DEVICE_MMC{1,2} without
> knowing which devices are actually available.
It returns MMC1 or MMC2 based on what loaded SPL. If SPL was loaded
from MMC1, it should return MMC1. If if SPL is loaded from MMC2, it
should return MMC2, not return MMC1. The previous code always assumed
that spl_boot_device() would always return MMC1. Each individual board
would then do a manual check to see what their boot source was, then
pin-mux and configure MMC1 with the clocking and pin mux for whatever
their MMC source is.
>
> There must be some other way to achieve what you want without breaking
> boot when usdhc1 is missing.
If your board is not using SPL_OF_CONTROL, maybe we can put an ifdef
before my patch so people who don't use device tree will default the
older style where all MMC/SDHC controllers return MMC1.
adam
>
> > Since it appears that arch/arm/mach-imx/spl.c is supposed to be able
> > to return the correct boot source, my goal was to make that function
> > actually return that which could eliminate the above function on all
> > boards. Unfortunately, I don't have hardware with MMC3 or MMC4, so I
> > couldn't test it and therefore didn't write it into the code. It was
> > my hope that someone with MMC3 or MMC4 would be able to easily expand
> > it in the hope to better facilitate support for DM_MMC and device tree
> > in SPL.
> >
> >> Aren't other i.MX boards impacted by this commit?
> >
> > Yes and no. If they only support MMC1 or MMC2 and have DM_MMC with
> > device tree support, the theory is that mmc_init_spl(bd_t *bis)
> > function can be completely eliminated. People with MMC3 and MMC4 as
> > boot sources are quite possibly impacted, but like I said before, I
> > was trying to lay the foundation for people to migrate into a
> > direction to eliminate individual functions and share common files
> > more easily.
> >
> > You can try this:
> >
> > diff --git a/arch/arm/include/asm/spl.h b/arch/arm/include/asm/spl.h
> > index e568af2561..e94a295eda 100644
> > --- a/arch/arm/include/asm/spl.h
> > +++ b/arch/arm/include/asm/spl.h
> > @@ -18,6 +18,8 @@ enum {
> > BOOT_DEVICE_MMC1,
> > BOOT_DEVICE_MMC2,
> > BOOT_DEVICE_MMC2_2,
> > + BOOT_DEVICE_MMC3,
> > + BOOT_DEVICE_MMC4,
> > BOOT_DEVICE_NAND,
> > BOOT_DEVICE_ONENAND,
> > BOOT_DEVICE_NOR,
> > diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> > index 1f230aca33..bf72d03eee 100644
> > --- a/arch/arm/mach-imx/spl.c
> > +++ b/arch/arm/mach-imx/spl.c
> > @@ -87,7 +87,11 @@ u32 spl_boot_device(void)
> > case IMX6_BMODE_ESD:
> > case IMX6_BMODE_MMC:
> > case IMX6_BMODE_EMMC:
> > - if (mmc_index == 1)
> > + if (mmc_index == 3)
> > + return BOOT_DEVICE_MMC4;
> > + else if (mmc_index == 2)
> > + return BOOT_DEVICE_MMC3;
> > + else if (mmc_index == 1)
> > return BOOT_DEVICE_MMC2;
> > else
> > return BOOT_DEVICE_MMC1;
> >
> > It's only compile-only tested.
>
> This patch deals with another issue that commit 14d319b185 causes. But
> I'm afraid this patch can not fix boot for me, as explained above.
>
> baruch
>
> > I am hoping someone from NXP or the MMC maintainer might having some
> > thoughts on what might be missing (if anything)
> >
> > adam
> >>
> >> Thanks,
> >> baruch
> >>
> >> >> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
> >> >> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> >> >> Author: Adam Ford <aford173 at gmail.com>
> >> >> Date: Thu May 23 14:11:30 2019 -0500
> >> >>
> >> >> spl: imx6: Let spl_boot_device return USDHC1 or USDHC2
> >> >>
> >> >> Currently, when the spl_boot_device checks the boot device, it
> >> >> will only return MMC1 when it's either sd or eMMC regardless
> >> >> of whether or not it's MMC1 or MMC2. This is a problem when
> >> >> booting from MMC2 if MMC isn't being manually configured like in
> >> >> the DM_SPL case with SPL_OF_CONTROL.
> >> >>
> >> >> This patch will check the register and return either MMC1 or MMC2.
> >> >>
> >> >> Signed-off-by: Adam Ford <aford173 at gmail.com>
> >> >>
> >> >> arch/arm/mach-imx/spl.c | 8 +++++---
> >> >> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> --
> http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
> - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
More information about the U-Boot
mailing list