[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Jean-Jacques Hiblot]

Hi Tom,

I'm using a TI DRA7 platform and the falcon boot from MMC is broken with 
v2017. The reason is that the standard "boot_os" is used to tell whether 
the falcon mode should be used or not, but we can't access it. The root 
cause is that the environment is stored in a eMMC which is dev 1 not dev 
0 on those platforms.

What is the purpose of commit b9c8ccab. Is it because we want to 
initialize only one MMC device in the SPL to reduce the boot time ?

Jean-Jacques

[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Tom Rini]

On Tue, Jan 24, 2017 at 10:26:38AM +0100, Jean-Jacques Hiblot wrote:

> Hi Tom,
> 
> I'm using a TI DRA7 platform and the falcon boot from MMC is broken
> with v2017. The reason is that the standard "boot_os" is used to
> tell whether the falcon mode should be used or not, but we can't
> access it. The root cause is that the environment is stored in a
> eMMC which is dev 1 not dev 0 on those platforms.
> 
> What is the purpose of commit b9c8ccab. Is it because we want to
> initialize only one MMC device in the SPL to reduce the boot time ?

Please note that b9c8ccaba77b has been in since April 2014, so this is
not some new behavior.  That said, we have CONFIG_SYS_MMC_ENV_DEV set
which is what the commit in question uses to know where the env is.  I
think you need to run a git bisect between v2016.11 (which I assume is
when you last tested this) and v2017.01 to see what commit changed
things.  It's possible something has changed and broken this case
requiring another tweak somewhere or another, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170124/fc751bc1/attachment.sig>

[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Jean-Jacques Hiblot]

On 24/01/2017 16:17, Tom Rini wrote:
> On Tue, Jan 24, 2017 at 10:26:38AM +0100, Jean-Jacques Hiblot wrote:
>
>> Hi Tom,
>>
>> I'm using a TI DRA7 platform and the falcon boot from MMC is broken
>> with v2017. The reason is that the standard "boot_os" is used to
>> tell whether the falcon mode should be used or not, but we can't
>> access it. The root cause is that the environment is stored in a
>> eMMC which is dev 1 not dev 0 on those platforms.
>>
>> What is the purpose of commit b9c8ccab. Is it because we want to
>> initialize only one MMC device in the SPL to reduce the boot time ?
> Please note that b9c8ccaba77b has been in since April 2014, so this is
> not some new behavior.
I had noticed that it's quite old indeed. I didn't mean that it's a 
regression. I'm just puzzled by the commit. what is its purpose ? why is 
SPL not using  CONFIG_SYS_MMC_ENV_DEV ?
I can trace this as the reason why I can't access the environment in 
eMMC (MMC2) when booting from SD (MMC1) and the fix is simply use dev = 
CONFIG_SYS_MMC_ENV_DEV instead of dev = 0.
But I guess that there is a reason for enforcing dev = 0. If the reason 
is still valid, I'll have to find another way of dealing with this. 
Otherwise we could revert to dev = CONFIG_SYS_MMC_ENV_DEV or
define a CONFIG_SYS_MMC_ENV_SPL_EV with a default value of 0.
> That said, we have CONFIG_SYS_MMC_ENV_DEV set
> which is what the commit in question uses to know where the env is.  I
> think you need to run a git bisect between v2016.11 (which I assume is
> when you last tested this)
Honestly I don't know when it broke. I still have to figure this. It may 
be that the test case I'm using hasn't work in a long time. From what I 
understand falcon boot on DRA7 is primarily used from QSPI or eMMC in 
raw mode. And those modes are not impacted by this problem with the 
environment.
>   and v2017.01 to see what commit changed
> things.  It's possible something has changed and broken this case
> requiring another tweak somewhere or another, thanks!
>

[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Tom Rini]

On Tue, Jan 24, 2017 at 04:35:58PM +0100, Jean-Jacques Hiblot wrote:
> 
> 
> On 24/01/2017 16:17, Tom Rini wrote:
> >On Tue, Jan 24, 2017 at 10:26:38AM +0100, Jean-Jacques Hiblot wrote:
> >
> >>Hi Tom,
> >>
> >>I'm using a TI DRA7 platform and the falcon boot from MMC is broken
> >>with v2017. The reason is that the standard "boot_os" is used to
> >>tell whether the falcon mode should be used or not, but we can't
> >>access it. The root cause is that the environment is stored in a
> >>eMMC which is dev 1 not dev 0 on those platforms.
> >>
> >>What is the purpose of commit b9c8ccab. Is it because we want to
> >>initialize only one MMC device in the SPL to reduce the boot time ?
> >Please note that b9c8ccaba77b has been in since April 2014, so this is
> >not some new behavior.
> I had noticed that it's quite old indeed. I didn't mean that it's a
> regression. I'm just puzzled by the commit. what is its purpose ?
> why is SPL not using  CONFIG_SYS_MMC_ENV_DEV ?

Because in SPL we do not have both MMC devices initialized.  We register
the one we booted from and thus it is device 0 to U-Boot in this case.
I suspect the rest of the issues stem from this quirk, or something
having broken around this quirk.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170124/229dd813/attachment.sig>

[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Jean-Jacques Hiblot]

On 24/01/2017 16:46, Tom Rini wrote:
>> I had noticed that it's quite old indeed. I didn't mean that it's a
>> regression. I'm just puzzled by the commit. what is its purpose ?
>> why is SPL not using  CONFIG_SYS_MMC_ENV_DEV ?
> Because in SPL we do not have both MMC devices initialized.
That is not always the case. Actually in spl_mmc.c the code requires us 
to register more than one MMC device to work properly when multiple MMC 
boot devices can be used (see spl_mmc_get_device_index())
I did the test of registering only MMC2 when booting from eMMC, the SPL 
fails because it can't find device 1:
Trying to boot from MMC2_2
MMC Device 1 not found
spl: could not find mmc device. error: -19

> We register
> the one we booted from and thus it is device 0 to U-Boot in this case.
> I suspect the rest of the issues stem from this quirk, or something
> having broken around this quirk.  Thanks!

[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Tom Rini]

On Tue, Jan 24, 2017 at 06:04:47PM +0100, Jean-Jacques Hiblot wrote:
> 
> 
> On 24/01/2017 16:46, Tom Rini wrote:
> >>I had noticed that it's quite old indeed. I didn't mean that it's a
> >>regression. I'm just puzzled by the commit. what is its purpose ?
> >>why is SPL not using  CONFIG_SYS_MMC_ENV_DEV ?
> >Because in SPL we do not have both MMC devices initialized.
> That is not always the case. Actually in spl_mmc.c the code requires
> us to register more than one MMC device to work properly when
> multiple MMC boot devices can be used (see
> spl_mmc_get_device_index())
> I did the test of registering only MMC2 when booting from eMMC, the
> SPL fails because it can't find device 1:
> Trying to boot from MMC2_2
> MMC Device 1 not found
> spl: could not find mmc device. error: -19
> 
> >We register
> >the one we booted from and thus it is device 0 to U-Boot in this case.
> >I suspect the rest of the issues stem from this quirk, or something
> >having broken around this quirk.  Thanks!

Right.  So I suspect the problem is that some level of the env_mmc.c
code needs to be adapted again for the change in how SPL now works with
the possibility of multiple devices.  It's possible that the change you
found is the right fix, please investigate a bit more and confirm
things before submitting a proper patch, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170124/86a87a85/attachment.sig>

[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Jean-Jacques Hiblot]

On 24/01/2017 20:11, Tom Rini wrote:
> On Tue, Jan 24, 2017 at 06:04:47PM +0100, Jean-Jacques Hiblot wrote:
>>
>> On 24/01/2017 16:46, Tom Rini wrote:
>>>> I had noticed that it's quite old indeed. I didn't mean that it's a
>>>> regression. I'm just puzzled by the commit. what is its purpose ?
>>>> why is SPL not using  CONFIG_SYS_MMC_ENV_DEV ?
>>> Because in SPL we do not have both MMC devices initialized.
>> That is not always the case. Actually in spl_mmc.c the code requires
>> us to register more than one MMC device to work properly when
>> multiple MMC boot devices can be used (see
>> spl_mmc_get_device_index())
>> I did the test of registering only MMC2 when booting from eMMC, the
>> SPL fails because it can't find device 1:
>> Trying to boot from MMC2_2
>> MMC Device 1 not found
>> spl: could not find mmc device. error: -19
>>
>>> We register
>>> the one we booted from and thus it is device 0 to U-Boot in this case.
>>> I suspect the rest of the issues stem from this quirk, or something
>>> having broken around this quirk.  Thanks!
> Right.  So I suspect the problem is that some level of the env_mmc.c
> code needs to be adapted again for the change in how SPL now works with
> the possibility of multiple devices.  It's possible that the change you
> found is the right fix, please investigate a bit more and confirm
> things before submitting a proper patch, thanks!
I did more tests and it turns out that there I find no real benefit of 
registering only the controller for the boot device.
The initialization of the MMC/SD/eMMC is done only prior accessing it, 
not when it's registered. So in terms of boot time the impact of 
registering many controllers is not significant.
By registering the same controllers in SPL and in u-boot, we would get 
the same mapping for the MMC devices in SPL and u-boot. It would remove 
a source of confusion and of #ifdef CONFIG_SPL_BUILD

The catch is that many boards register only one MMC controller in the 
SPL, depending on what the boot source is (ex: board_mmc_init() in 
board/freescale/mx6slevk/mx6slevk.c)
To reduce the risk of regression, we could deal with those boards in 2 
steps:
1) Don't change the code of the board except to override the weak 
function mmc_get_env_dev() and make it return 0. This is not likely to 
introduce a regression
2) One by one, change the code of the boards to register all the 
controllers in SPL as done in u-boot. Also we need to adapt 
spl_boot_device() to return the right boot device. There has been a 
partial attempt at this ""ARM: mx6: add MMC2 boot device detection 
support in SPL" but had to be reverted probably because it was not 
coherent with the registration of the controllers.

[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Tom Rini]

On Wed, Jan 25, 2017 at 11:21:32AM +0100, Jean-Jacques Hiblot wrote:
> 
> 
> On 24/01/2017 20:11, Tom Rini wrote:
> >On Tue, Jan 24, 2017 at 06:04:47PM +0100, Jean-Jacques Hiblot wrote:
> >>
> >>On 24/01/2017 16:46, Tom Rini wrote:
> >>>>I had noticed that it's quite old indeed. I didn't mean that it's a
> >>>>regression. I'm just puzzled by the commit. what is its purpose ?
> >>>>why is SPL not using  CONFIG_SYS_MMC_ENV_DEV ?
> >>>Because in SPL we do not have both MMC devices initialized.
> >>That is not always the case. Actually in spl_mmc.c the code requires
> >>us to register more than one MMC device to work properly when
> >>multiple MMC boot devices can be used (see
> >>spl_mmc_get_device_index())
> >>I did the test of registering only MMC2 when booting from eMMC, the
> >>SPL fails because it can't find device 1:
> >>Trying to boot from MMC2_2
> >>MMC Device 1 not found
> >>spl: could not find mmc device. error: -19
> >>
> >>>We register
> >>>the one we booted from and thus it is device 0 to U-Boot in this case.
> >>>I suspect the rest of the issues stem from this quirk, or something
> >>>having broken around this quirk.  Thanks!
> >Right.  So I suspect the problem is that some level of the env_mmc.c
> >code needs to be adapted again for the change in how SPL now works with
> >the possibility of multiple devices.  It's possible that the change you
> >found is the right fix, please investigate a bit more and confirm
> >things before submitting a proper patch, thanks!
> I did more tests and it turns out that there I find no real benefit
> of registering only the controller for the boot device.
> The initialization of the MMC/SD/eMMC is done only prior accessing
> it, not when it's registered. So in terms of boot time the impact of
> registering many controllers is not significant.
> By registering the same controllers in SPL and in u-boot, we would
> get the same mapping for the MMC devices in SPL and u-boot. It would
> remove a source of confusion and of #ifdef CONFIG_SPL_BUILD
> 
> The catch is that many boards register only one MMC controller in
> the SPL, depending on what the boot source is (ex: board_mmc_init()
> in board/freescale/mx6slevk/mx6slevk.c)
> To reduce the risk of regression, we could deal with those boards in
> 2 steps:
> 1) Don't change the code of the board except to override the weak
> function mmc_get_env_dev() and make it return 0. This is not likely
> to introduce a regression
> 2) One by one, change the code of the boards to register all the
> controllers in SPL as done in u-boot. Also we need to adapt
> spl_boot_device() to return the right boot device. There has been a
> partial attempt at this ""ARM: mx6: add MMC2 boot device detection
> support in SPL" but had to be reverted probably because it was not
> coherent with the registration of the controllers.

Due to the issue you mention in #2, we probably need to do #1 and with
care and testing, as there's enough places that assume SPL is a single
MMC device that it'll be problematic to do them one at a time.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170126/2f61773d/attachment.sig>

[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Jean-Jacques Hiblot]

On 27/01/2017 00:36, Tom Rini wrote:
> On Wed, Jan 25, 2017 at 11:21:32AM +0100, Jean-Jacques Hiblot wrote:
>>
>> On 24/01/2017 20:11, Tom Rini wrote:
>>> On Tue, Jan 24, 2017 at 06:04:47PM +0100, Jean-Jacques Hiblot wrote:
>>>> On 24/01/2017 16:46, Tom Rini wrote:
>>>>>> I had noticed that it's quite old indeed. I didn't mean that it's a
>>>>>> regression. I'm just puzzled by the commit. what is its purpose ?
>>>>>> why is SPL not using  CONFIG_SYS_MMC_ENV_DEV ?
>>>>> Because in SPL we do not have both MMC devices initialized.
>>>> That is not always the case. Actually in spl_mmc.c the code requires
>>>> us to register more than one MMC device to work properly when
>>>> multiple MMC boot devices can be used (see
>>>> spl_mmc_get_device_index())
>>>> I did the test of registering only MMC2 when booting from eMMC, the
>>>> SPL fails because it can't find device 1:
>>>> Trying to boot from MMC2_2
>>>> MMC Device 1 not found
>>>> spl: could not find mmc device. error: -19
>>>>
>>>>> We register
>>>>> the one we booted from and thus it is device 0 to U-Boot in this case.
>>>>> I suspect the rest of the issues stem from this quirk, or something
>>>>> having broken around this quirk.  Thanks!
>>> Right.  So I suspect the problem is that some level of the env_mmc.c
>>> code needs to be adapted again for the change in how SPL now works with
>>> the possibility of multiple devices.  It's possible that the change you
>>> found is the right fix, please investigate a bit more and confirm
>>> things before submitting a proper patch, thanks!
>> I did more tests and it turns out that there I find no real benefit
>> of registering only the controller for the boot device.
>> The initialization of the MMC/SD/eMMC is done only prior accessing
>> it, not when it's registered. So in terms of boot time the impact of
>> registering many controllers is not significant.
>> By registering the same controllers in SPL and in u-boot, we would
>> get the same mapping for the MMC devices in SPL and u-boot. It would
>> remove a source of confusion and of #ifdef CONFIG_SPL_BUILD
>>
>> The catch is that many boards register only one MMC controller in
>> the SPL, depending on what the boot source is (ex: board_mmc_init()
>> in board/freescale/mx6slevk/mx6slevk.c)
>> To reduce the risk of regression, we could deal with those boards in
>> 2 steps:
>> 1) Don't change the code of the board except to override the weak
>> function mmc_get_env_dev() and make it return 0. This is not likely
>> to introduce a regression
>> 2) One by one, change the code of the boards to register all the
>> controllers in SPL as done in u-boot. Also we need to adapt
>> spl_boot_device() to return the right boot device. There has been a
>> partial attempt at this ""ARM: mx6: add MMC2 boot device detection
>> support in SPL" but had to be reverted probably because it was not
>> coherent with the registration of the controllers.
> Due to the issue you mention in #2, we probably need to do #1 and with
> care and testing, as there's enough places that assume SPL is a single
> MMC device that it'll be problematic to do them one at a time.
I made a survey of the code in 2017.01 to identify the platforms that 
may be impacted by the change in env_mmc.c

Here is the method:
1)  *  find the platforms that override mmc_get_env_dev(). Those may not 
rely on CONFIG_SYS_MMC_ENV_DEV to get the MMC device where the env is 
stored.
     * find the platforms that define CONFIG_SYS_MMC_ENV_DEV as not 0
2) Find the paltforms that use CONFIG_SPL_ENV_SUPPORT
3) Cross the info from 1 and 2 to get the platforms that may be impacted 
by the change


1) Platforms that my use a device that is not dev 0 for en storage:

$ git grep -w mmc_get_env_dev
arch/arm/cpu/armv7/mx6/soc.c:int mmc_get_env_dev(void)
arch/arm/cpu/armv7/mx7/soc.c:int mmc_get_env_dev(void)
arch/arm/mach-uniphier/boot-mode/boot-mode.c:int mmc_get_env_dev(void)
board/freescale/mx7dsabresd/mx7dsabresd.c:      u32 dev_no = 
mmc_get_env_dev();
common/env_mmc.c:__weak int mmc_get_env_dev(void)
common/env_mmc.c:       int dev = mmc_get_env_dev();
common/env_mmc.c:       int dev = mmc_get_env_dev();
common/env_mmc.c:       int dev = mmc_get_env_dev();
common/env_mmc.c:       int dev = mmc_get_env_dev();
common/env_mmc.c:       int dev = mmc_get_env_dev();
include/mmc.h:int mmc_get_env_dev(void);


$ git grep -e 'ne\sCONFIG_SYS_MMC_ENV_DEV\s*[1-9a-zA-Z\(\)]' include/
include/configs/am335x_evm.h:#define CONFIG_SYS_MMC_ENV_DEV             1
include/configs/am335x_shc.h:#define CONFIG_SYS_MMC_ENV_DEV             1
include/configs/am335x_sl50.h:#define CONFIG_SYS_MMC_ENV_DEV            1
include/configs/bav335x.h:#define CONFIG_SYS_MMC_ENV_DEV                1
include/configs/cm_t54.h:#define CONFIG_SYS_MMC_ENV_DEV 1               
/* SLOT2: eMMC(1) */
include/configs/dra7xx_evm.h:#define CONFIG_SYS_MMC_ENV_DEV             
1       /* SLOT2: eMMC(1) */
include/configs/el6x_common.h:#define CONFIG_SYS_MMC_ENV_DEV            1
include/configs/embestmx6boards.h:#define 
CONFIG_SYS_MMC_ENV_DEV                2       /* SDHC4 */
include/configs/evb_rk3288.h:#define CONFIG_SYS_MMC_ENV_DEV 1
include/configs/evb_rk3399.h:#define CONFIG_SYS_MMC_ENV_DEV 1
include/configs/fennec_rk3288.h:#define CONFIG_SYS_MMC_ENV_DEV 1
include/configs/mx6qarm2.h:#define CONFIG_SYS_MMC_ENV_DEV               1
include/configs/mx6sabresd.h:#define CONFIG_SYS_MMC_ENV_DEV             
1       /* SDHC3 */
include/configs/mx6slevk.h:#define CONFIG_SYS_MMC_ENV_DEV               
1       /* SDHC2*/
include/configs/mx6sxsabresd.h:#define CONFIG_SYS_MMC_ENV_DEV           
2  /*USDHC4*/
include/configs/mx6ul_14x14_evk.h:#define 
CONFIG_SYS_MMC_ENV_DEV                1   /* USDHC2 */
include/configs/mx6ullevk.h:#define CONFIG_SYS_MMC_ENV_DEV              
1       /* USDHC2 */
include/configs/odroid.h:#define CONFIG_SYS_MMC_ENV_DEV 
CONFIG_MMC_DEFAULT_DEV
include/configs/omap4_sdp4430.h:#define CONFIG_SYS_MMC_ENV_DEV          
1       /* SLOT2: eMMC(1) */
include/configs/omap5_uevm.h:#define CONFIG_SYS_MMC_ENV_DEV             
1       /* SLOT2: eMMC(1) */
include/configs/popmetal_rk3288.h:#define CONFIG_SYS_MMC_ENV_DEV 1
include/configs/s5p_goni.h:#define CONFIG_SYS_MMC_ENV_DEV               
CONFIG_MMC_DEFAULT_DEV
include/configs/s5pc210_universal.h:#define 
CONFIG_SYS_MMC_ENV_DEV              CONFIG_MMC_DEFAULT_DEV
include/configs/tbs2910.h:#define CONFIG_SYS_MMC_ENV_DEV                
2 /* overwritten on SD boot */
include/configs/trats.h:#define CONFIG_SYS_MMC_ENV_DEV 
CONFIG_MMC_DEFAULT_DEV
include/configs/trats2.h:#define CONFIG_SYS_MMC_ENV_DEV 
CONFIG_MMC_DEFAULT_DEV


They can be sorted as such:
* exynos based: use CONFIG_MMC_DEFAULT_DEV wich is 0.
* uniphier : the code in u-boot locates the first MMC (not SD) and use 
it for the env. But this mechanism is not used in the SPL case => it 
probably doesn't work with SPL_ENV_SUPPORT.
* TI based: The code used to register the controllers is the same for 
all and all suffer from the problem I described. They will benefit from 
removing "dev = 0"
* Rockchip based : It looks like those platforms use DT to register the 
controllers. So I expect that they too would benefit from removing "dev = 0"
* iMX6/IMX7 based : This is a more complex situation:
  - imx6/imx7 common code provide relies on board_mmc_get_env_dev() to 
get the dev where the env is stored if booting from SD/MMC. When 
implemented this function is usually returns the boot device.The default 
(weak) implementation returns  CONFIG_SYS_MMC_ENV_DEV.
  - platforms register several MMC controllers in u-boot and usually 
only one in SPL though there are exceptions (embestmx6boards, mx6qarm2, 
tbs2910, imx7sabre, etc.).
  - Removing "dev = 0" in env_mmc.c, would break SPL_ENV_SUPPORT for 
most of the mx6 based boards


2) platform that use SPL env in their default config

$ git grep -e "CONFIG_SPL_ENV_SUPPORT=y"
configs/B4420QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/B4860QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1010RDB-PA_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1010RDB-PA_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1010RDB-PA_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1010RDB-PA_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1010RDB-PB_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1010RDB-PB_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1010RDB-PB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1010RDB-PB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1020MBG-PC_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1020MBG-PC_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1020RDB-PC_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1020RDB-PC_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1020RDB-PC_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1020RDB-PC_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1020RDB-PD_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1020RDB-PD_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1020UTM-PC_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1020UTM-PC_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1021RDB-PC_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1021RDB-PC_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1021RDB-PC_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1021RDB-PC_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1022DS_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1022DS_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1022DS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1022DS_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1024RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1024RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1025RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P1025RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P2020RDB-PC_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P2020RDB-PC_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P2020RDB-PC_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/P2020RDB-PC_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1023RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1023RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1023RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1024QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1024QDS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1024QDS_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1024RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1024RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1024RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1040D4RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1040D4RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1040D4RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1040RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1040RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1040RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1042D4RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1042D4RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1042D4RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1042RDB_PI_NAND_SECURE_BOOT_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1042RDB_PI_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1042RDB_PI_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T1042RDB_PI_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T2080QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T2080QDS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T2080QDS_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T2080RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T2080RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T2080RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T2081QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T2081QDS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T2081QDS_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T4160QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T4160QDS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T4240QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T4240QDS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/T4240RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/a3m071_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/a4m2k_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/am335x_shc_netboot_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/am335x_sl50_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1021aqds_nand_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1021aqds_sdcard_ifc_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1021aqds_sdcard_qspi_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1021atwr_sdcard_ifc_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1021atwr_sdcard_qspi_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1043aqds_nand_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1043aqds_sdcard_ifc_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1043aqds_sdcard_qspi_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1043ardb_nand_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls1043ardb_sdcard_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls2080aqds_nand_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/ls2080ardb_nand_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/pcm051_rev1_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/pcm051_rev3_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/pengwyn_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/sandbox_spl_defconfig:CONFIG_SPL_ENV_SUPPORT=y
configs/udoo_neo_defconfig:CONFIG_SPL_ENV_SUPPORT=y

$ git grep -e "define\sCONFIG_SPL_ENV_SUPPORT"
include/configs/ls1021aiot.h:#define CONFIG_SPL_ENV_SUPPORT
include/configs/ls1046a_common.h:#define CONFIG_SPL_ENV_SUPPORT
include/configs/ls1046a_common.h:#define CONFIG_SPL_ENV_SUPPORT
include/configs/xilinx_zynqmp.h:# define CONFIG_SPL_ENV_SUPPORT

$ git grep -A2 "SPL_ENV_SUPPORT" | grep -e 'default\sy'
board/ti/am335x/Kconfig-        default y
board/ti/common/Kconfig-        default y

3) Cross info.
- udoo_neo  is the only iMX6 based platform that uses SPL_ENV. But as it 
register only one SDHC port, there will be no problem. All other iMX6/7 
based platforms don't use SPL_ENV_SUPPORT in their default configuration.
- all TI platforms are impacted

=> IMO it's safe to make the remove all the 'dev = 0;' from env_mmc and 
rely on mmc_get_env_dev().


>

[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Tom Rini]

On Mon, Jan 30, 2017 at 01:44:04PM +0100, Jean-Jacques Hiblot wrote:
> 
> 
> On 27/01/2017 00:36, Tom Rini wrote:
> >On Wed, Jan 25, 2017 at 11:21:32AM +0100, Jean-Jacques Hiblot wrote:
> >>
> >>On 24/01/2017 20:11, Tom Rini wrote:
> >>>On Tue, Jan 24, 2017 at 06:04:47PM +0100, Jean-Jacques Hiblot wrote:
> >>>>On 24/01/2017 16:46, Tom Rini wrote:
> >>>>>>I had noticed that it's quite old indeed. I didn't mean that it's a
> >>>>>>regression. I'm just puzzled by the commit. what is its purpose ?
> >>>>>>why is SPL not using  CONFIG_SYS_MMC_ENV_DEV ?
> >>>>>Because in SPL we do not have both MMC devices initialized.
> >>>>That is not always the case. Actually in spl_mmc.c the code requires
> >>>>us to register more than one MMC device to work properly when
> >>>>multiple MMC boot devices can be used (see
> >>>>spl_mmc_get_device_index())
> >>>>I did the test of registering only MMC2 when booting from eMMC, the
> >>>>SPL fails because it can't find device 1:
> >>>>Trying to boot from MMC2_2
> >>>>MMC Device 1 not found
> >>>>spl: could not find mmc device. error: -19
> >>>>
> >>>>>We register
> >>>>>the one we booted from and thus it is device 0 to U-Boot in this case.
> >>>>>I suspect the rest of the issues stem from this quirk, or something
> >>>>>having broken around this quirk.  Thanks!
> >>>Right.  So I suspect the problem is that some level of the env_mmc.c
> >>>code needs to be adapted again for the change in how SPL now works with
> >>>the possibility of multiple devices.  It's possible that the change you
> >>>found is the right fix, please investigate a bit more and confirm
> >>>things before submitting a proper patch, thanks!
> >>I did more tests and it turns out that there I find no real benefit
> >>of registering only the controller for the boot device.
> >>The initialization of the MMC/SD/eMMC is done only prior accessing
> >>it, not when it's registered. So in terms of boot time the impact of
> >>registering many controllers is not significant.
> >>By registering the same controllers in SPL and in u-boot, we would
> >>get the same mapping for the MMC devices in SPL and u-boot. It would
> >>remove a source of confusion and of #ifdef CONFIG_SPL_BUILD
> >>
> >>The catch is that many boards register only one MMC controller in
> >>the SPL, depending on what the boot source is (ex: board_mmc_init()
> >>in board/freescale/mx6slevk/mx6slevk.c)
> >>To reduce the risk of regression, we could deal with those boards in
> >>2 steps:
> >>1) Don't change the code of the board except to override the weak
> >>function mmc_get_env_dev() and make it return 0. This is not likely
> >>to introduce a regression
> >>2) One by one, change the code of the boards to register all the
> >>controllers in SPL as done in u-boot. Also we need to adapt
> >>spl_boot_device() to return the right boot device. There has been a
> >>partial attempt at this ""ARM: mx6: add MMC2 boot device detection
> >>support in SPL" but had to be reverted probably because it was not
> >>coherent with the registration of the controllers.
> >Due to the issue you mention in #2, we probably need to do #1 and with
> >care and testing, as there's enough places that assume SPL is a single
> >MMC device that it'll be problematic to do them one at a time.
> I made a survey of the code in 2017.01 to identify the platforms
> that may be impacted by the change in env_mmc.c
> 
> Here is the method:
> 1)  *  find the platforms that override mmc_get_env_dev(). Those may
> not rely on CONFIG_SYS_MMC_ENV_DEV to get the MMC device where the
> env is stored.
>     * find the platforms that define CONFIG_SYS_MMC_ENV_DEV as not 0
> 2) Find the paltforms that use CONFIG_SPL_ENV_SUPPORT
> 3) Cross the info from 1 and 2 to get the platforms that may be
> impacted by the change
> 
> 
> 1) Platforms that my use a device that is not dev 0 for en storage:
> 
> $ git grep -w mmc_get_env_dev
> arch/arm/cpu/armv7/mx6/soc.c:int mmc_get_env_dev(void)
> arch/arm/cpu/armv7/mx7/soc.c:int mmc_get_env_dev(void)
> arch/arm/mach-uniphier/boot-mode/boot-mode.c:int mmc_get_env_dev(void)
> board/freescale/mx7dsabresd/mx7dsabresd.c:      u32 dev_no =
> mmc_get_env_dev();
> common/env_mmc.c:__weak int mmc_get_env_dev(void)
> common/env_mmc.c:       int dev = mmc_get_env_dev();
> common/env_mmc.c:       int dev = mmc_get_env_dev();
> common/env_mmc.c:       int dev = mmc_get_env_dev();
> common/env_mmc.c:       int dev = mmc_get_env_dev();
> common/env_mmc.c:       int dev = mmc_get_env_dev();
> include/mmc.h:int mmc_get_env_dev(void);
> 
> 
> $ git grep -e 'ne\sCONFIG_SYS_MMC_ENV_DEV\s*[1-9a-zA-Z\(\)]' include/
> include/configs/am335x_evm.h:#define CONFIG_SYS_MMC_ENV_DEV             1
> include/configs/am335x_shc.h:#define CONFIG_SYS_MMC_ENV_DEV             1
> include/configs/am335x_sl50.h:#define CONFIG_SYS_MMC_ENV_DEV            1
> include/configs/bav335x.h:#define CONFIG_SYS_MMC_ENV_DEV                1
> include/configs/cm_t54.h:#define CONFIG_SYS_MMC_ENV_DEV 1
> /* SLOT2: eMMC(1) */
> include/configs/dra7xx_evm.h:#define CONFIG_SYS_MMC_ENV_DEV
> 1       /* SLOT2: eMMC(1) */
> include/configs/el6x_common.h:#define CONFIG_SYS_MMC_ENV_DEV            1
> include/configs/embestmx6boards.h:#define CONFIG_SYS_MMC_ENV_DEV
> 2       /* SDHC4 */
> include/configs/evb_rk3288.h:#define CONFIG_SYS_MMC_ENV_DEV 1
> include/configs/evb_rk3399.h:#define CONFIG_SYS_MMC_ENV_DEV 1
> include/configs/fennec_rk3288.h:#define CONFIG_SYS_MMC_ENV_DEV 1
> include/configs/mx6qarm2.h:#define CONFIG_SYS_MMC_ENV_DEV               1
> include/configs/mx6sabresd.h:#define CONFIG_SYS_MMC_ENV_DEV
> 1       /* SDHC3 */
> include/configs/mx6slevk.h:#define CONFIG_SYS_MMC_ENV_DEV
> 1       /* SDHC2*/
> include/configs/mx6sxsabresd.h:#define CONFIG_SYS_MMC_ENV_DEV
> 2  /*USDHC4*/
> include/configs/mx6ul_14x14_evk.h:#define CONFIG_SYS_MMC_ENV_DEV
> 1   /* USDHC2 */
> include/configs/mx6ullevk.h:#define CONFIG_SYS_MMC_ENV_DEV
> 1       /* USDHC2 */
> include/configs/odroid.h:#define CONFIG_SYS_MMC_ENV_DEV
> CONFIG_MMC_DEFAULT_DEV
> include/configs/omap4_sdp4430.h:#define CONFIG_SYS_MMC_ENV_DEV
> 1       /* SLOT2: eMMC(1) */
> include/configs/omap5_uevm.h:#define CONFIG_SYS_MMC_ENV_DEV
> 1       /* SLOT2: eMMC(1) */
> include/configs/popmetal_rk3288.h:#define CONFIG_SYS_MMC_ENV_DEV 1
> include/configs/s5p_goni.h:#define CONFIG_SYS_MMC_ENV_DEV
> CONFIG_MMC_DEFAULT_DEV
> include/configs/s5pc210_universal.h:#define CONFIG_SYS_MMC_ENV_DEV
> CONFIG_MMC_DEFAULT_DEV
> include/configs/tbs2910.h:#define CONFIG_SYS_MMC_ENV_DEV
> 2 /* overwritten on SD boot */
> include/configs/trats.h:#define CONFIG_SYS_MMC_ENV_DEV
> CONFIG_MMC_DEFAULT_DEV
> include/configs/trats2.h:#define CONFIG_SYS_MMC_ENV_DEV
> CONFIG_MMC_DEFAULT_DEV
> 
> 
> They can be sorted as such:
> * exynos based: use CONFIG_MMC_DEFAULT_DEV wich is 0.
> * uniphier : the code in u-boot locates the first MMC (not SD) and
> use it for the env. But this mechanism is not used in the SPL case
> => it probably doesn't work with SPL_ENV_SUPPORT.
> * TI based: The code used to register the controllers is the same
> for all and all suffer from the problem I described. They will
> benefit from removing "dev = 0"
> * Rockchip based : It looks like those platforms use DT to register
> the controllers. So I expect that they too would benefit from
> removing "dev = 0"
> * iMX6/IMX7 based : This is a more complex situation:
>  - imx6/imx7 common code provide relies on board_mmc_get_env_dev()
> to get the dev where the env is stored if booting from SD/MMC. When
> implemented this function is usually returns the boot device.The
> default (weak) implementation returns  CONFIG_SYS_MMC_ENV_DEV.
>  - platforms register several MMC controllers in u-boot and usually
> only one in SPL though there are exceptions (embestmx6boards,
> mx6qarm2, tbs2910, imx7sabre, etc.).
>  - Removing "dev = 0" in env_mmc.c, would break SPL_ENV_SUPPORT for
> most of the mx6 based boards
> 
> 
> 2) platform that use SPL env in their default config
> 
> $ git grep -e "CONFIG_SPL_ENV_SUPPORT=y"
> configs/B4420QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/B4860QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1010RDB-PA_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1010RDB-PA_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1010RDB-PA_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1010RDB-PA_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1010RDB-PB_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1010RDB-PB_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1010RDB-PB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1010RDB-PB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1020MBG-PC_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1020MBG-PC_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1020RDB-PC_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1020RDB-PC_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1020RDB-PC_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1020RDB-PC_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1020RDB-PD_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1020RDB-PD_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1020UTM-PC_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1020UTM-PC_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1021RDB-PC_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1021RDB-PC_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1021RDB-PC_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1021RDB-PC_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1022DS_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1022DS_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1022DS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1022DS_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1024RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1024RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1025RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P1025RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P2020RDB-PC_36BIT_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P2020RDB-PC_36BIT_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P2020RDB-PC_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/P2020RDB-PC_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1023RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1023RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1023RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1024QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1024QDS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1024QDS_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1024RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1024RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1024RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1040D4RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1040D4RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1040D4RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1040RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1040RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1040RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1042D4RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1042D4RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1042D4RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1042RDB_PI_NAND_SECURE_BOOT_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1042RDB_PI_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1042RDB_PI_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T1042RDB_PI_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T2080QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T2080QDS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T2080QDS_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T2080RDB_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T2080RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T2080RDB_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T2081QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T2081QDS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T2081QDS_SPIFLASH_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T4160QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T4160QDS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T4240QDS_NAND_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T4240QDS_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/T4240RDB_SDCARD_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/a3m071_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/a4m2k_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/am335x_shc_netboot_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/am335x_sl50_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1021aqds_nand_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1021aqds_sdcard_ifc_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1021aqds_sdcard_qspi_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1021atwr_sdcard_ifc_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1021atwr_sdcard_qspi_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1043aqds_nand_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1043aqds_sdcard_ifc_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1043aqds_sdcard_qspi_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1043ardb_nand_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls1043ardb_sdcard_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls2080aqds_nand_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/ls2080ardb_nand_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/pcm051_rev1_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/pcm051_rev3_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/pengwyn_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/sandbox_spl_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> configs/udoo_neo_defconfig:CONFIG_SPL_ENV_SUPPORT=y
> 
> $ git grep -e "define\sCONFIG_SPL_ENV_SUPPORT"
> include/configs/ls1021aiot.h:#define CONFIG_SPL_ENV_SUPPORT
> include/configs/ls1046a_common.h:#define CONFIG_SPL_ENV_SUPPORT
> include/configs/ls1046a_common.h:#define CONFIG_SPL_ENV_SUPPORT
> include/configs/xilinx_zynqmp.h:# define CONFIG_SPL_ENV_SUPPORT
> 
> $ git grep -A2 "SPL_ENV_SUPPORT" | grep -e 'default\sy'
> board/ti/am335x/Kconfig-        default y
> board/ti/common/Kconfig-        default y
> 
> 3) Cross info.
> - udoo_neo  is the only iMX6 based platform that uses SPL_ENV. But
> as it register only one SDHC port, there will be no problem. All
> other iMX6/7 based platforms don't use SPL_ENV_SUPPORT in their
> default configuration.
> - all TI platforms are impacted
> 
> => IMO it's safe to make the remove all the 'dev = 0;' from env_mmc
> and rely on mmc_get_env_dev().
> 

OK, please make a patch, test it on as much of the TI hardware as you
can (including BBB), and lets move from there, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170130/8fb3d03d/attachment.sig>