[U-Boot] mx6cuboxi fails to load u-boot.img

[U-Boot] mx6cuboxi fails to load u-boot.img

[Vagrant Cascadian]

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 ###

Works fine with 2019.07.

< xypron> vagrantc: CONFIG_DM_MMC, CONFIG_DM_USB is not used. The board
still uses the pre-driver-model drivers. So a candidate to be kicked out
of U-Boot.

Happy to test patches; I have several mx6cuboxi supported models
available.


live well,
  vagrant
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190926/99bf1de4/attachment.sig>

[U-Boot] mx6cuboxi fails to load u-boot.img

[Fabio Estevam]

Hi Vagrant,

On Thu, Sep 26, 2019 at 4:16 PM Vagrant Cascadian <vagrant@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 fine with 2019.07.
>
> < xypron> vagrantc: CONFIG_DM_MMC, CONFIG_DM_USB is not used. The board
> still uses the pre-driver-model drivers. So a candidate to be kicked out
> of U-Boot.

Yes, this conversion need to be done.

Thanks

[U-Boot] mx6cuboxi fails to load u-boot.img

[Jonathan Gray]

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@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.

14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
Author: Adam Ford <aford173@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@gmail.com>

 arch/arm/mach-imx/spl.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Fri, Sep 27, 2019 at 4:38 AM Jonathan Gray <jsg@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@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.

adam

> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> Author: Adam Ford <aford173@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@gmail.com>
>
>  arch/arm/mach-imx/spl.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

[U-Boot] mx6cuboxi fails to load u-boot.img

[Baruch Siach]

Hi Adam,

Thanks Vagrant and Jonathan for the report and investigation.

On Fri, Sep 27 2019, Adam Ford wrote:
> On Fri, Sep 27, 2019 at 4:38 AM Jonathan Gray <jsg@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@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'll try to migrate Cubox-i/Hummingboard to DM and OF_CONTORL as you
suggest.

Is there a way to make a DT based U-Boot i.MX6 image work on both
Quad/Dual and DualLite/Solo?

The kernel provide separate DTs for Quad/Dual and DualLite/Solo. What is
the right course in U-Boot? Run time DT detection?

Thanks,
baruch

--
     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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Fabio Estevam]

Hi Baruch,

On Sun, Sep 29, 2019 at 2:58 AM Baruch Siach <baruch@tkos.co.il> wrote:

> I'll try to migrate Cubox-i/Hummingboard to DM and OF_CONTORL as you
> suggest.
>
> Is there a way to make a DT based U-Boot i.MX6 image work on both
> Quad/Dual and DualLite/Solo?
>
> The kernel provide separate DTs for Quad/Dual and DualLite/Solo. What is
> the right course in U-Boot? Run time DT detection?

Yes, please look at board/wandboard/spl.c in
board_fit_config_name_match() function for a reference.

[U-Boot] mx6cuboxi fails to load u-boot.img

[Baruch Siach]

Hi Adam,

(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@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@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.

In addition, your patch returns BOOT_DEVICE_MMC2 only for usdhc2
boot. All others return BOOT_DEVICE_MMC1. What about usdhc{3,4}?

How is all that intended to work?

Aren't other i.MX boards impacted by this commit?

Thanks,
baruch

>> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
>> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
>> Author: Adam Ford <aford173@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@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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Peter Robinson]

> > 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.

For reference I have also seen the same with the udoo-neo and I have
had reports from a Fedora user of a similar problem on the Wandboard.

Peter

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Sun, Oct 6, 2019 at 5:23 AM Baruch Siach <baruch@tkos.co.il> wrote:
>
> Hi Adam,
>
> (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@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@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.
>
> 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.
  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.

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@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@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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Sun, Oct 6, 2019 at 6:30 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Sun, Oct 6, 2019 at 5:23 AM Baruch Siach <baruch@tkos.co.il> wrote:
> >
> > Hi Adam,
> >
> > (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@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@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.
> >
> > 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.
>   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.
>
> I am hoping someone from NXP or the MMC maintainer might having some
> thoughts on what might be missing (if anything)
>
> adam
> >

I should also note that people who are only booting from MMC1 or MMC2
should be able to do something like was done here:

https://patchwork.ozlabs.org/patch/1104361/

Eventually, if boards can fit the pinmux  stuff into SPL, that
function modified in the above patch should be able to go away.

adam
> > Thanks,
> > baruch
> >
> > >> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
> > >> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> > >> Author: Adam Ford <aford173@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@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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Baruch Siach]

Hi Adam,

On Sun, Oct 06 2019, Adam Ford wrote:
> On Sun, Oct 6, 2019 at 5:23 AM Baruch Siach <baruch@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@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@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.

There must be some other way to achieve what you want without breaking
boot when usdhc1 is missing.

>   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@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@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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Sun, Oct 6, 2019 at 7:22 AM Baruch Siach <baruch@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@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@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@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@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@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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Sun, Oct 6, 2019 at 7:30 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Sun, Oct 6, 2019 at 7:22 AM Baruch Siach <baruch@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@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@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@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.
>

For boards without SPL_OF_CONTROL, this might help, I don't know if
you use OF_CONTROL or not.
Does this help:

diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 1f230aca33..27eb8cc0df 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -24,7 +24,9 @@ u32 spl_boot_device(void)
 {
        unsigned int bmode = readl(&src_base->sbmr2);
        u32 reg = imx6_src_get_boot_mode();
+#ifdef CONFIG_SPL_OF_CONTROL
        u32 mmc_index = ((reg >> 11) & 0x03);
+#endif

        /*
         * Check for BMODE if serial downloader is enabled
@@ -87,10 +89,13 @@ u32 spl_boot_device(void)
        case IMX6_BMODE_ESD:
        case IMX6_BMODE_MMC:
        case IMX6_BMODE_EMMC:
+#ifdef CONFIG_SPL_OF_CONTROL
                if (mmc_index == 1)
                        return BOOT_DEVICE_MMC2;
                else
+#else
                        return BOOT_DEVICE_MMC1;
+#endif
        /* NAND Flash: 8.5.2, Table 8-10 */
        case IMX6_BMODE_NAND_MIN ... IMX6_BMODE_NAND_MAX:
                return BOOT_DEVICE_NAND;


> 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@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@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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Baruch Siach]

Hi Adam,

On Sun, Oct 06 2019, Adam Ford wrote:
> On Sun, Oct 6, 2019 at 7:30 AM Adam Ford <aford173@gmail.com> wrote:
>>
>> On Sun, Oct 6, 2019 at 7:22 AM Baruch Siach <baruch@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@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@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@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.
>
> For boards without SPL_OF_CONTROL, this might help, I don't know if
> you use OF_CONTROL or not.

I do use SPL_OF_CONTROL. usdhc1 is not enabled in DT because there is no
SD/eMMC there.

> Does this help:
>
> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> index 1f230aca33..27eb8cc0df 100644
> --- a/arch/arm/mach-imx/spl.c
> +++ b/arch/arm/mach-imx/spl.c
> @@ -24,7 +24,9 @@ u32 spl_boot_device(void)
>  {
>         unsigned int bmode = readl(&src_base->sbmr2);
>         u32 reg = imx6_src_get_boot_mode();
> +#ifdef CONFIG_SPL_OF_CONTROL
>         u32 mmc_index = ((reg >> 11) & 0x03);
> +#endif
>
>         /*
>          * Check for BMODE if serial downloader is enabled
> @@ -87,10 +89,13 @@ u32 spl_boot_device(void)
>         case IMX6_BMODE_ESD:
>         case IMX6_BMODE_MMC:
>         case IMX6_BMODE_EMMC:
> +#ifdef CONFIG_SPL_OF_CONTROL
>                 if (mmc_index == 1)
>                         return BOOT_DEVICE_MMC2;
>                 else
> +#else
>                         return BOOT_DEVICE_MMC1;
> +#endif
>         /* NAND Flash: 8.5.2, Table 8-10 */
>         case IMX6_BMODE_NAND_MIN ... IMX6_BMODE_NAND_MAX:
>                 return BOOT_DEVICE_NAND;

So this patch can't help either.

baruch

>> > >   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@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@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 -


-- 
     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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Sun, Oct 6, 2019 at 7:48 AM Baruch Siach <baruch@tkos.co.il> wrote:
>
> Hi Adam,
>
> On Sun, Oct 06 2019, Adam Ford wrote:
> > On Sun, Oct 6, 2019 at 7:30 AM Adam Ford <aford173@gmail.com> wrote:
> >>
> >> On Sun, Oct 6, 2019 at 7:22 AM Baruch Siach <baruch@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@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@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@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.
> >
> > For boards without SPL_OF_CONTROL, this might help, I don't know if
> > you use OF_CONTROL or not.
>
> I do use SPL_OF_CONTROL. usdhc1 is not enabled in DT because there is no
> SD/eMMC there.

I guess i am confused as to how it works for you with the patch
reverted.  If this function returns MMC1 as the boot source with the
patch reverted, you and everyone else will need to check the register
to see what the boot source is.  This is the same register I am
reading in the patch.  From there, the MMC data gets loaded based on
the boot source.  This is the very thing my patch was trying to
address, but admittedly without automatic support for MMC3 or MMC4,
but my thought process was that those board would be no worse off than
they are now because mmc3 and mmc4 would still return mmc1 as it does
now.

I'd like to hear from either the NXP people or the MMC maintainer to
better determine how to utilize the return values of MMC1 or MMC2.

adam

>
> > Does this help:
> >
> > diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> > index 1f230aca33..27eb8cc0df 100644
> > --- a/arch/arm/mach-imx/spl.c
> > +++ b/arch/arm/mach-imx/spl.c
> > @@ -24,7 +24,9 @@ u32 spl_boot_device(void)
> >  {
> >         unsigned int bmode = readl(&src_base->sbmr2);
> >         u32 reg = imx6_src_get_boot_mode();
> > +#ifdef CONFIG_SPL_OF_CONTROL
> >         u32 mmc_index = ((reg >> 11) & 0x03);
> > +#endif
> >
> >         /*
> >          * Check for BMODE if serial downloader is enabled
> > @@ -87,10 +89,13 @@ u32 spl_boot_device(void)
> >         case IMX6_BMODE_ESD:
> >         case IMX6_BMODE_MMC:
> >         case IMX6_BMODE_EMMC:
> > +#ifdef CONFIG_SPL_OF_CONTROL
> >                 if (mmc_index == 1)
> >                         return BOOT_DEVICE_MMC2;
> >                 else
> > +#else
> >                         return BOOT_DEVICE_MMC1;
> > +#endif
> >         /* NAND Flash: 8.5.2, Table 8-10 */
> >         case IMX6_BMODE_NAND_MIN ... IMX6_BMODE_NAND_MAX:
> >                 return BOOT_DEVICE_NAND;
>
> So this patch can't help either.
>
> baruch
>
> >> > >   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@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@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 -
>
>
> --
>      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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Baruch Siach]

Hi Adam,

On Sun, Oct 06 2019, Adam Ford wrote:
> On Sun, Oct 6, 2019 at 7:48 AM Baruch Siach <baruch@tkos.co.il> wrote:
>> On Sun, Oct 06 2019, Adam Ford wrote:
>> > On Sun, Oct 6, 2019 at 7:30 AM Adam Ford <aford173@gmail.com> wrote:
>> >>
>> >> On Sun, Oct 6, 2019 at 7:22 AM Baruch Siach <baruch@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@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@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@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.
>> >
>> > For boards without SPL_OF_CONTROL, this might help, I don't know if
>> > you use OF_CONTROL or not.
>>
>> I do use SPL_OF_CONTROL. usdhc1 is not enabled in DT because there is no
>> SD/eMMC there.
>
> I guess i am confused as to how it works for you with the patch
> reverted.  If this function returns MMC1 as the boot source with the
> patch reverted, you and everyone else will need to check the register
> to see what the boot source is.  This is the same register I am
> reading in the patch.  From there, the MMC data gets loaded based on
> the boot source.  This is the very thing my patch was trying to
> address, but admittedly without automatic support for MMC3 or MMC4,
> but my thought process was that those board would be no worse off than
> they are now because mmc3 and mmc4 would still return mmc1 as it does
> now.

I think I explained the problem clearly enough already. I'll only add
this advice:

Carefully read spl_mmc_find_device() and spl_mmc_get_device_index() in
common/spl/spl_mmc.c. Keep in mind that when 'mmc_dev' is
BOOT_DEVICE_MMC1 it refers to the first _PROBED_ SD/eMMC device. That
is, the first device that is both enabled in DT and probed successfully,
regardless of the value of X in usdhcX.

For the rest, see above.

Thanks,
baruch

> I'd like to hear from either the NXP people or the MMC maintainer to
> better determine how to utilize the return values of MMC1 or MMC2.
>
> adam
>
>>
>> > Does this help:
>> >
>> > diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
>> > index 1f230aca33..27eb8cc0df 100644
>> > --- a/arch/arm/mach-imx/spl.c
>> > +++ b/arch/arm/mach-imx/spl.c
>> > @@ -24,7 +24,9 @@ u32 spl_boot_device(void)
>> >  {
>> >         unsigned int bmode = readl(&src_base->sbmr2);
>> >         u32 reg = imx6_src_get_boot_mode();
>> > +#ifdef CONFIG_SPL_OF_CONTROL
>> >         u32 mmc_index = ((reg >> 11) & 0x03);
>> > +#endif
>> >
>> >         /*
>> >          * Check for BMODE if serial downloader is enabled
>> > @@ -87,10 +89,13 @@ u32 spl_boot_device(void)
>> >         case IMX6_BMODE_ESD:
>> >         case IMX6_BMODE_MMC:
>> >         case IMX6_BMODE_EMMC:
>> > +#ifdef CONFIG_SPL_OF_CONTROL
>> >                 if (mmc_index == 1)
>> >                         return BOOT_DEVICE_MMC2;
>> >                 else
>> > +#else
>> >                         return BOOT_DEVICE_MMC1;
>> > +#endif
>> >         /* NAND Flash: 8.5.2, Table 8-10 */
>> >         case IMX6_BMODE_NAND_MIN ... IMX6_BMODE_NAND_MAX:
>> >                 return BOOT_DEVICE_NAND;
>>
>> So this patch can't help either.
>>
>> baruch
>>
>> >> > >   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@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@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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Sun, Oct 6, 2019 at 10:55 AM Baruch Siach <baruch@tkos.co.il> wrote:
>
> Hi Adam,
>
> On Sun, Oct 06 2019, Adam Ford wrote:
> > On Sun, Oct 6, 2019 at 7:48 AM Baruch Siach <baruch@tkos.co.il> wrote:
> >> On Sun, Oct 06 2019, Adam Ford wrote:
> >> > On Sun, Oct 6, 2019 at 7:30 AM Adam Ford <aford173@gmail.com> wrote:
> >> >>
> >> >> On Sun, Oct 6, 2019 at 7:22 AM Baruch Siach <baruch@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@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@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@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.
> >> >
> >> > For boards without SPL_OF_CONTROL, this might help, I don't know if
> >> > you use OF_CONTROL or not.
> >>
> >> I do use SPL_OF_CONTROL. usdhc1 is not enabled in DT because there is no
> >> SD/eMMC there.
> >
> > I guess i am confused as to how it works for you with the patch
> > reverted.  If this function returns MMC1 as the boot source with the
> > patch reverted, you and everyone else will need to check the register
> > to see what the boot source is.  This is the same register I am
> > reading in the patch.  From there, the MMC data gets loaded based on
> > the boot source.  This is the very thing my patch was trying to
> > address, but admittedly without automatic support for MMC3 or MMC4,
> > but my thought process was that those board would be no worse off than
> > they are now because mmc3 and mmc4 would still return mmc1 as it does
> > now.
>
> I think I explained the problem clearly enough already. I'll only add
> this advice:
>
> Carefully read spl_mmc_find_device() and spl_mmc_get_device_index() in
> common/spl/spl_mmc.c. Keep in mind that when 'mmc_dev' is
> BOOT_DEVICE_MMC1 it refers to the first _PROBED_ SD/eMMC device. That
> is, the first device that is both enabled in DT and probed successfully,
> regardless of the value of X in usdhcX.

That's where my assumption fell down.  I assumed that MMC1 was SDHC1
and not the first probed device, and MMC2 is SDHC2 and not the second
probed device.   I know that's what you were trying to explain, but it
took me a bit.

Could an alias somehow fix the problem? index0 /  MMC1 = SDHC2 when SDHC1.

What I was trying to do is eliminate the need for each board to have
an extra layer of code to override the default code because the
default code only returns MMC1 regardless of the actual MMC device
started.  I would think we should be able to have some i.mx6 family
common code that can return the address or index of the MMC device
used for all SDHCx devices.

adam

>
> For the rest, see above.
>
> Thanks,
> baruch
>
> > I'd like to hear from either the NXP people or the MMC maintainer to
> > better determine how to utilize the return values of MMC1 or MMC2.
> >
> > adam
> >
> >>
> >> > Does this help:
> >> >
> >> > diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> >> > index 1f230aca33..27eb8cc0df 100644
> >> > --- a/arch/arm/mach-imx/spl.c
> >> > +++ b/arch/arm/mach-imx/spl.c
> >> > @@ -24,7 +24,9 @@ u32 spl_boot_device(void)
> >> >  {
> >> >         unsigned int bmode = readl(&src_base->sbmr2);
> >> >         u32 reg = imx6_src_get_boot_mode();
> >> > +#ifdef CONFIG_SPL_OF_CONTROL
> >> >         u32 mmc_index = ((reg >> 11) & 0x03);
> >> > +#endif
> >> >
> >> >         /*
> >> >          * Check for BMODE if serial downloader is enabled
> >> > @@ -87,10 +89,13 @@ u32 spl_boot_device(void)
> >> >         case IMX6_BMODE_ESD:
> >> >         case IMX6_BMODE_MMC:
> >> >         case IMX6_BMODE_EMMC:
> >> > +#ifdef CONFIG_SPL_OF_CONTROL
> >> >                 if (mmc_index == 1)
> >> >                         return BOOT_DEVICE_MMC2;
> >> >                 else
> >> > +#else
> >> >                         return BOOT_DEVICE_MMC1;
> >> > +#endif
> >> >         /* NAND Flash: 8.5.2, Table 8-10 */
> >> >         case IMX6_BMODE_NAND_MIN ... IMX6_BMODE_NAND_MAX:
> >> >                 return BOOT_DEVICE_NAND;
> >>
> >> So this patch can't help either.
> >>
> >> baruch
> >>
> >> >> > >   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@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@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 -

[U-Boot] mx6cuboxi fails to load u-boot.img

[Peter Robinson]

> 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@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.

For reference reverting this on 2019.10 fixed my issues with the udoo_neo board.

> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> Author: Adam Ford <aford173@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@gmail.com>
>
>  arch/arm/mach-imx/spl.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] mx6cuboxi fails to load u-boot.img

[Stefano Babic]

Hi everybody,

thanks for deep analyses (I just jump at the end):

On 08/10/19 12:03, Peter Robinson 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@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.
> 
> For reference reverting this on 2019.10 fixed my issues with the udoo_neo board.
> 
>> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
>> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
>> Author: Adam Ford <aford173@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@gmail.com>
>>

I tend to revert the pathc and let the "standard" case working. A board
maintainer coould add a board_boot_order() function to still overwrite
the behavior of spl_boot_device().

Regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Tue, Oct 8, 2019 at 5:19 AM Stefano Babic <sbabic@denx.de> wrote:
>
> Hi everybody,
>
> thanks for deep analyses (I just jump at the end):
>
> On 08/10/19 12:03, Peter Robinson 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@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.
> >
> > For reference reverting this on 2019.10 fixed my issues with the udoo_neo board.
> >
> >> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
> >> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> >> Author: Adam Ford <aford173@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@gmail.com>
> >>
>
> I tend to revert the pathc and let the "standard" case working. A board
> maintainer coould add a board_boot_order() function to still overwrite
> the behavior of spl_boot_device().

I will revert this and the rest of the series that goes with it.  I
just need some time to wake up and test it.  I'll have something in a
couple hours.

adam
>
> Regards,
> Stefano
>
>
> --
> =====================================================================
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] mx6cuboxi fails to load u-boot.img

[Stefano Babic]

On 08/10/19 12:42, Adam Ford wrote:
> On Tue, Oct 8, 2019 at 5:19 AM Stefano Babic <sbabic@denx.de> wrote:
>>
>> Hi everybody,
>>
>> thanks for deep analyses (I just jump at the end):
>>
>> On 08/10/19 12:03, Peter Robinson 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@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.
>>>
>>> For reference reverting this on 2019.10 fixed my issues with the udoo_neo board.
>>>
>>>> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
>>>> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
>>>> Author: Adam Ford <aford173@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@gmail.com>
>>>>
>>
>> I tend to revert the pathc and let the "standard" case working. A board
>> maintainer coould add a board_boot_order() function to still overwrite
>> the behavior of spl_boot_device().
> 
> I will revert this and the rest of the series that goes with it. 

The series is merged since a very long time - do you propose to revert
all of them ?

commit 8f4691e31a18254d225524a4b018b8cbcddc70b1
Author: Adam Ford <aford173@gmail.com>
Date:   Thu May 23 14:11:32 2019 -0500

    ARM: imx6q_logic: With SPL_OF_CONTROL enabled, remove MMC init

    Since the board uses SPL_OF_CONTROL now, we don't need to
    explicitly initialize the MMC driver, but we still need to
    pinmux the corresponding pins.  This patch removes the
    initialization code and leave just the muxing behind.

    Signed-off-by: Adam Ford <aford173@gmail.com>

commit 0749bbb5c75d2b35eaf6acd438750cf08df314ef
Author: Adam Ford <aford173@gmail.com>
Date:   Thu May 23 14:11:31 2019 -0500

    ARM: imx6q_logic: Enable SPL_DM with SPL_OF_CONTROL

    With the spl code correctly returning either MMC1 or MMC2,
    this board can not boot either from internal eMMC (MMC1) or
    the uSD card on the baseboard (MMC2) using the device tree.

    Signed-off-by: Adam Ford <aford173@gmail.com>

commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
Author: Adam Ford <aford173@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@gmail.com>

Two of them affects just imx6q_logic, the only one with more
side-effects is the one we discuss here. Of course, I can revert all
three of them if you prefer to go on this way (I am happy with follow up
pathces,too, after reverting just  14d319b1856).

Regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Tue, Oct 8, 2019 at 6:04 AM Stefano Babic <sbabic@denx.de> wrote:
>
> On 08/10/19 12:42, Adam Ford wrote:
> > On Tue, Oct 8, 2019 at 5:19 AM Stefano Babic <sbabic@denx.de> wrote:
> >>
> >> Hi everybody,
> >>
> >> thanks for deep analyses (I just jump at the end):
> >>
> >> On 08/10/19 12:03, Peter Robinson 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@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.
> >>>
> >>> For reference reverting this on 2019.10 fixed my issues with the udoo_neo board.
> >>>
> >>>> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
> >>>> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> >>>> Author: Adam Ford <aford173@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@gmail.com>
> >>>>
> >>
> >> I tend to revert the pathc and let the "standard" case working. A board
> >> maintainer coould add a board_boot_order() function to still overwrite
> >> the behavior of spl_boot_device().
> >
> > I will revert this and the rest of the series that goes with it.
>
> The series is merged since a very long time - do you propose to revert
> all of them ?

I just got into my office. I'm looking into it now.  I should have
something shortly.  for sure, I'll revert the offending patch, but I
want to look into options on how to best approach keeping my board
booting without adding a bunch of extra layers.
I know time is of the essence if we want to get it into the final
release for 2019.10


adam
>
> commit 8f4691e31a18254d225524a4b018b8cbcddc70b1
> Author: Adam Ford <aford173@gmail.com>
> Date:   Thu May 23 14:11:32 2019 -0500
>
>     ARM: imx6q_logic: With SPL_OF_CONTROL enabled, remove MMC init
>
>     Since the board uses SPL_OF_CONTROL now, we don't need to
>     explicitly initialize the MMC driver, but we still need to
>     pinmux the corresponding pins.  This patch removes the
>     initialization code and leave just the muxing behind.
>
>     Signed-off-by: Adam Ford <aford173@gmail.com>
>
> commit 0749bbb5c75d2b35eaf6acd438750cf08df314ef
> Author: Adam Ford <aford173@gmail.com>
> Date:   Thu May 23 14:11:31 2019 -0500
>
>     ARM: imx6q_logic: Enable SPL_DM with SPL_OF_CONTROL
>
>     With the spl code correctly returning either MMC1 or MMC2,
>     this board can not boot either from internal eMMC (MMC1) or
>     the uSD card on the baseboard (MMC2) using the device tree.
>
>     Signed-off-by: Adam Ford <aford173@gmail.com>
>
> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> Author: Adam Ford <aford173@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@gmail.com>
>
> Two of them affects just imx6q_logic, the only one with more
> side-effects is the one we discuss here. Of course, I can revert all
> three of them if you prefer to go on this way (I am happy with follow up
> pathces,too, after reverting just  14d319b1856).
>
> Regards,
> Stefano
>
> --
> =====================================================================
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> =====================================================================

[U-Boot] mx6cuboxi fails to load u-boot.img

[Peter Robinson]

On Tue, Oct 8, 2019 at 12:43 PM Adam Ford <aford173@gmail.com> wrote:
>
> On Tue, Oct 8, 2019 at 6:04 AM Stefano Babic <sbabic@denx.de> wrote:
> >
> > On 08/10/19 12:42, Adam Ford wrote:
> > > On Tue, Oct 8, 2019 at 5:19 AM Stefano Babic <sbabic@denx.de> wrote:
> > >>
> > >> Hi everybody,
> > >>
> > >> thanks for deep analyses (I just jump at the end):
> > >>
> > >> On 08/10/19 12:03, Peter Robinson 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@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.
> > >>>
> > >>> For reference reverting this on 2019.10 fixed my issues with the udoo_neo board.
> > >>>
> > >>>> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
> > >>>> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> > >>>> Author: Adam Ford <aford173@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@gmail.com>
> > >>>>
> > >>
> > >> I tend to revert the pathc and let the "standard" case working. A board
> > >> maintainer coould add a board_boot_order() function to still overwrite
> > >> the behavior of spl_boot_device().
> > >
> > > I will revert this and the rest of the series that goes with it.
> >
> > The series is merged since a very long time - do you propose to revert
> > all of them ?
>
> I just got into my office. I'm looking into it now.  I should have
> something shortly.  for sure, I'll revert the offending patch, but I
> want to look into options on how to best approach keeping my board
> booting without adding a bunch of extra layers.
> I know time is of the essence if we want to get it into the final
> release for 2019.10

That ship sailed yesterday!

>
> adam
> >
> > commit 8f4691e31a18254d225524a4b018b8cbcddc70b1
> > Author: Adam Ford <aford173@gmail.com>
> > Date:   Thu May 23 14:11:32 2019 -0500
> >
> >     ARM: imx6q_logic: With SPL_OF_CONTROL enabled, remove MMC init
> >
> >     Since the board uses SPL_OF_CONTROL now, we don't need to
> >     explicitly initialize the MMC driver, but we still need to
> >     pinmux the corresponding pins.  This patch removes the
> >     initialization code and leave just the muxing behind.
> >
> >     Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > commit 0749bbb5c75d2b35eaf6acd438750cf08df314ef
> > Author: Adam Ford <aford173@gmail.com>
> > Date:   Thu May 23 14:11:31 2019 -0500
> >
> >     ARM: imx6q_logic: Enable SPL_DM with SPL_OF_CONTROL
> >
> >     With the spl code correctly returning either MMC1 or MMC2,
> >     this board can not boot either from internal eMMC (MMC1) or
> >     the uSD card on the baseboard (MMC2) using the device tree.
> >
> >     Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> > Author: Adam Ford <aford173@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@gmail.com>
> >
> > Two of them affects just imx6q_logic, the only one with more
> > side-effects is the one we discuss here. Of course, I can revert all
> > three of them if you prefer to go on this way (I am happy with follow up
> > pathces,too, after reverting just  14d319b1856).
> >
> > Regards,
> > Stefano
> >
> > --
> > =====================================================================
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> > =====================================================================

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Tue, Oct 8, 2019 at 6:45 AM Peter Robinson <pbrobinson@gmail.com> wrote:
>
> On Tue, Oct 8, 2019 at 12:43 PM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Tue, Oct 8, 2019 at 6:04 AM Stefano Babic <sbabic@denx.de> wrote:
> > >
> > > On 08/10/19 12:42, Adam Ford wrote:
> > > > On Tue, Oct 8, 2019 at 5:19 AM Stefano Babic <sbabic@denx.de> wrote:
> > > >>
> > > >> Hi everybody,
> > > >>
> > > >> thanks for deep analyses (I just jump at the end):
> > > >>
> > > >> On 08/10/19 12:03, Peter Robinson 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@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.
> > > >>>
> > > >>> For reference reverting this on 2019.10 fixed my issues with the udoo_neo board.
> > > >>>
> > > >>>> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
> > > >>>> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> > > >>>> Author: Adam Ford <aford173@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@gmail.com>
> > > >>>>
> > > >>
> > > >> I tend to revert the pathc and let the "standard" case working. A board
> > > >> maintainer coould add a board_boot_order() function to still overwrite
> > > >> the behavior of spl_boot_device().
> > > >
> > > > I will revert this and the rest of the series that goes with it.
> > >
> > > The series is merged since a very long time - do you propose to revert
> > > all of them ?
> >
> > I just got into my office. I'm looking into it now.  I should have
> > something shortly.  for sure, I'll revert the offending patch, but I
> > want to look into options on how to best approach keeping my board
> > booting without adding a bunch of extra layers.
> > I know time is of the essence if we want to get it into the final
> > release for 2019.10
>
> That ship sailed yesterday!

Sorry.
:-(

Either way, I'll have a revert patch series sent today.

adam
>
> >
> > adam
> > >
> > > commit 8f4691e31a18254d225524a4b018b8cbcddc70b1
> > > Author: Adam Ford <aford173@gmail.com>
> > > Date:   Thu May 23 14:11:32 2019 -0500
> > >
> > >     ARM: imx6q_logic: With SPL_OF_CONTROL enabled, remove MMC init
> > >
> > >     Since the board uses SPL_OF_CONTROL now, we don't need to
> > >     explicitly initialize the MMC driver, but we still need to
> > >     pinmux the corresponding pins.  This patch removes the
> > >     initialization code and leave just the muxing behind.
> > >
> > >     Signed-off-by: Adam Ford <aford173@gmail.com>
> > >
> > > commit 0749bbb5c75d2b35eaf6acd438750cf08df314ef
> > > Author: Adam Ford <aford173@gmail.com>
> > > Date:   Thu May 23 14:11:31 2019 -0500
> > >
> > >     ARM: imx6q_logic: Enable SPL_DM with SPL_OF_CONTROL
> > >
> > >     With the spl code correctly returning either MMC1 or MMC2,
> > >     this board can not boot either from internal eMMC (MMC1) or
> > >     the uSD card on the baseboard (MMC2) using the device tree.
> > >
> > >     Signed-off-by: Adam Ford <aford173@gmail.com>
> > >
> > > commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> > > Author: Adam Ford <aford173@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@gmail.com>
> > >
> > > Two of them affects just imx6q_logic, the only one with more
> > > side-effects is the one we discuss here. Of course, I can revert all
> > > three of them if you prefer to go on this way (I am happy with follow up
> > > pathces,too, after reverting just  14d319b1856).
> > >
> > > Regards,
> > > Stefano
> > >
> > > --
> > > =====================================================================
> > > DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> > > =====================================================================

[U-Boot] mx6cuboxi fails to load u-boot.img

[Peter Robinson]

> > > > >>>>>> 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.
> > > > >>>
> > > > >>> For reference reverting this on 2019.10 fixed my issues with the udoo_neo board.
> > > > >>>
> > > > >>>> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
> > > > >>>> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> > > > >>>> Author: Adam Ford <aford173@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@gmail.com>
> > > > >>>>
> > > > >>
> > > > >> I tend to revert the pathc and let the "standard" case working. A board
> > > > >> maintainer coould add a board_boot_order() function to still overwrite
> > > > >> the behavior of spl_boot_device().
> > > > >
> > > > > I will revert this and the rest of the series that goes with it.
> > > >
> > > > The series is merged since a very long time - do you propose to revert
> > > > all of them ?
> > >
> > > I just got into my office. I'm looking into it now.  I should have
> > > something shortly.  for sure, I'll revert the offending patch, but I
> > > want to look into options on how to best approach keeping my board
> > > booting without adding a bunch of extra layers.
> > > I know time is of the essence if we want to get it into the final
> > > release for 2019.10
> >
> > That ship sailed yesterday!
>
> Sorry.
> :-(

It happens

> Either way, I'll have a revert patch series sent today.

Probably better off getting it fixed properly now, if that involved
reverting it sure, if it involves patches on top fixing the situation
and moving things forward do that.

> adam
> >
> > >
> > > adam
> > > >
> > > > commit 8f4691e31a18254d225524a4b018b8cbcddc70b1
> > > > Author: Adam Ford <aford173@gmail.com>
> > > > Date:   Thu May 23 14:11:32 2019 -0500
> > > >
> > > >     ARM: imx6q_logic: With SPL_OF_CONTROL enabled, remove MMC init
> > > >
> > > >     Since the board uses SPL_OF_CONTROL now, we don't need to
> > > >     explicitly initialize the MMC driver, but we still need to
> > > >     pinmux the corresponding pins.  This patch removes the
> > > >     initialization code and leave just the muxing behind.
> > > >
> > > >     Signed-off-by: Adam Ford <aford173@gmail.com>
> > > >
> > > > commit 0749bbb5c75d2b35eaf6acd438750cf08df314ef
> > > > Author: Adam Ford <aford173@gmail.com>
> > > > Date:   Thu May 23 14:11:31 2019 -0500
> > > >
> > > >     ARM: imx6q_logic: Enable SPL_DM with SPL_OF_CONTROL
> > > >
> > > >     With the spl code correctly returning either MMC1 or MMC2,
> > > >     this board can not boot either from internal eMMC (MMC1) or
> > > >     the uSD card on the baseboard (MMC2) using the device tree.
> > > >
> > > >     Signed-off-by: Adam Ford <aford173@gmail.com>
> > > >
> > > > commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> > > > Author: Adam Ford <aford173@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@gmail.com>
> > > >
> > > > Two of them affects just imx6q_logic, the only one with more
> > > > side-effects is the one we discuss here. Of course, I can revert all
> > > > three of them if you prefer to go on this way (I am happy with follow up
> > > > pathces,too, after reverting just  14d319b1856).
> > > >
> > > > Regards,
> > > > Stefano
> > > >
> > > > --
> > > > =====================================================================
> > > > DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> > > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > > > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> > > > =====================================================================

[U-Boot] mx6cuboxi fails to load u-boot.img

[Adam Ford]

On Tue, Oct 8, 2019 at 6:51 AM Peter Robinson <pbrobinson@gmail.com> 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.
> > > > > >>>
> > > > > >>> For reference reverting this on 2019.10 fixed my issues with the udoo_neo board.
> > > > > >>>
> > > > > >>>> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
> > > > > >>>> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> > > > > >>>> Author: Adam Ford <aford173@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@gmail.com>
> > > > > >>>>
> > > > > >>
> > > > > >> I tend to revert the pathc and let the "standard" case working. A board
> > > > > >> maintainer coould add a board_boot_order() function to still overwrite
> > > > > >> the behavior of spl_boot_device().
> > > > > >
> > > > > > I will revert this and the rest of the series that goes with it.
> > > > >
> > > > > The series is merged since a very long time - do you propose to revert
> > > > > all of them ?
> > > >
> > > > I just got into my office. I'm looking into it now.  I should have
> > > > something shortly.  for sure, I'll revert the offending patch, but I
> > > > want to look into options on how to best approach keeping my board
> > > > booting without adding a bunch of extra layers.
> > > > I know time is of the essence if we want to get it into the final
> > > > release for 2019.10
> > >
> > > That ship sailed yesterday!
> >
> > Sorry.
> > :-(
>
> It happens
>
> > Either way, I'll have a revert patch series sent today.
>
> Probably better off getting it fixed properly now, if that involved
> reverting it sure, if it involves patches on top fixing the situation
> and moving things forward do that.

Apologizes to everyone.  I pushed the patch [1] to revert this change.
Thank you to everyone for allowing me the courtesy of undoing it.
I also want to thank Stefano for bringing to my attention that we can
override the standard settings by adding board_boot_order() [2]
which actually let me default back to the serial down-loader in the
event that something goes wrong.


adam

[1] - https://patchwork.ozlabs.org/patch/1173314/
[2] - https://patchwork.ozlabs.org/patch/1173316/

>
> > adam
> > >
> > > >
> > > > adam
> > > > >
> > > > > commit 8f4691e31a18254d225524a4b018b8cbcddc70b1
> > > > > Author: Adam Ford <aford173@gmail.com>
> > > > > Date:   Thu May 23 14:11:32 2019 -0500
> > > > >
> > > > >     ARM: imx6q_logic: With SPL_OF_CONTROL enabled, remove MMC init
> > > > >
> > > > >     Since the board uses SPL_OF_CONTROL now, we don't need to
> > > > >     explicitly initialize the MMC driver, but we still need to
> > > > >     pinmux the corresponding pins.  This patch removes the
> > > > >     initialization code and leave just the muxing behind.
> > > > >
> > > > >     Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > >
> > > > > commit 0749bbb5c75d2b35eaf6acd438750cf08df314ef
> > > > > Author: Adam Ford <aford173@gmail.com>
> > > > > Date:   Thu May 23 14:11:31 2019 -0500
> > > > >
> > > > >     ARM: imx6q_logic: Enable SPL_DM with SPL_OF_CONTROL
> > > > >
> > > > >     With the spl code correctly returning either MMC1 or MMC2,
> > > > >     this board can not boot either from internal eMMC (MMC1) or
> > > > >     the uSD card on the baseboard (MMC2) using the device tree.
> > > > >
> > > > >     Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > >
> > > > > commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
> > > > > Author: Adam Ford <aford173@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@gmail.com>
> > > > >
> > > > > Two of them affects just imx6q_logic, the only one with more
> > > > > side-effects is the one we discuss here. Of course, I can revert all
> > > > > three of them if you prefer to go on this way (I am happy with follow up
> > > > > pathces,too, after reverting just  14d319b1856).
> > > > >
> > > > > Regards,
> > > > > Stefano
> > > > >
> > > > > --
> > > > > =====================================================================
> > > > > DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> > > > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > > > > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> > > > > =====================================================================

[U-Boot] mx6cuboxi fails to load u-boot.img

[Stefano Babic]

On 08/10/19 15:05, Adam Ford wrote:
> On Tue, Oct 8, 2019 at 6:51 AM Peter Robinson <pbrobinson@gmail.com> 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.
>>>>>>>>>
>>>>>>>>> For reference reverting this on 2019.10 fixed my issues with the udoo_neo board.
>>>>>>>>>
>>>>>>>>>> 14d319b1856b86e593e01abd0a1e3c2d63b52a8a is the first bad commit
>>>>>>>>>> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
>>>>>>>>>> Author: Adam Ford <aford173@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@gmail.com>
>>>>>>>>>>
>>>>>>>>
>>>>>>>> I tend to revert the pathc and let the "standard" case working. A board
>>>>>>>> maintainer coould add a board_boot_order() function to still overwrite
>>>>>>>> the behavior of spl_boot_device().
>>>>>>>
>>>>>>> I will revert this and the rest of the series that goes with it.
>>>>>>
>>>>>> The series is merged since a very long time - do you propose to revert
>>>>>> all of them ?
>>>>>
>>>>> I just got into my office. I'm looking into it now.  I should have
>>>>> something shortly.  for sure, I'll revert the offending patch, but I
>>>>> want to look into options on how to best approach keeping my board
>>>>> booting without adding a bunch of extra layers.
>>>>> I know time is of the essence if we want to get it into the final
>>>>> release for 2019.10
>>>>
>>>> That ship sailed yesterday!
>>>
>>> Sorry.
>>> :-(
>>
>> It happens
>>
>>> Either way, I'll have a revert patch series sent today.
>>
>> Probably better off getting it fixed properly now, if that involved
>> reverting it sure, if it involves patches on top fixing the situation
>> and moving things forward do that.
> 
> Apologizes to everyone.  I pushed the patch [1] to revert this change.
> Thank you to everyone for allowing me the courtesy of undoing it.
> I also want to thank Stefano for bringing to my attention that we can
> override the standard settings by adding board_boot_order() [2]
> which actually let me default back to the serial down-loader in the
> event that something goes wrong.
> 
> 

Thanks Adam - I have already merged your two patches into my -next
branch (it will become u-boot-imx, -master after Travis will finish).

Best regards,
Stefano

> adam
> 
> [1] - https://patchwork.ozlabs.org/patch/1173314/
> [2] - https://patchwork.ozlabs.org/patch/1173316/
> 
>>
>>> adam
>>>>
>>>>>
>>>>> adam
>>>>>>
>>>>>> commit 8f4691e31a18254d225524a4b018b8cbcddc70b1
>>>>>> Author: Adam Ford <aford173@gmail.com>
>>>>>> Date:   Thu May 23 14:11:32 2019 -0500
>>>>>>
>>>>>>     ARM: imx6q_logic: With SPL_OF_CONTROL enabled, remove MMC init
>>>>>>
>>>>>>     Since the board uses SPL_OF_CONTROL now, we don't need to
>>>>>>     explicitly initialize the MMC driver, but we still need to
>>>>>>     pinmux the corresponding pins.  This patch removes the
>>>>>>     initialization code and leave just the muxing behind.
>>>>>>
>>>>>>     Signed-off-by: Adam Ford <aford173@gmail.com>
>>>>>>
>>>>>> commit 0749bbb5c75d2b35eaf6acd438750cf08df314ef
>>>>>> Author: Adam Ford <aford173@gmail.com>
>>>>>> Date:   Thu May 23 14:11:31 2019 -0500
>>>>>>
>>>>>>     ARM: imx6q_logic: Enable SPL_DM with SPL_OF_CONTROL
>>>>>>
>>>>>>     With the spl code correctly returning either MMC1 or MMC2,
>>>>>>     this board can not boot either from internal eMMC (MMC1) or
>>>>>>     the uSD card on the baseboard (MMC2) using the device tree.
>>>>>>
>>>>>>     Signed-off-by: Adam Ford <aford173@gmail.com>
>>>>>>
>>>>>> commit 14d319b1856b86e593e01abd0a1e3c2d63b52a8a
>>>>>> Author: Adam Ford <aford173@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@gmail.com>
>>>>>>
>>>>>> Two of them affects just imx6q_logic, the only one with more
>>>>>> side-effects is the one we discuss here. Of course, I can revert all
>>>>>> three of them if you prefer to go on this way (I am happy with follow up
>>>>>> pathces,too, after reverting just  14d319b1856).
>>>>>>
>>>>>> Regards,
>>>>>> Stefano
>>>>>>
>>>>>> --
>>>>>> =====================================================================
>>>>>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>>>>>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>>>>>> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
>>>>>> =====================================================================


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================