[U-Boot] [PATCH v1 2/2] imx: use BOOT_DEVICE_BOARD instead of UART

[Stefan Agner]

On 2017-08-27 13:21, Sébastien Szymanski wrote:
> Hello,
> 
>> On 27 Aug 2017, at 21:17, Stefan Agner <stefan at agner.ch> wrote:
>>
>> On 2017-08-27 01:45, Sébastien Szymanski wrote:
>>> Hello,
>>>
>>>> On 25 Aug 2017, at 13:33, Stefano Babic <sbabic at denx.de> wrote:
>>>>
>>>> On 16/08/2017 02:49, Stefan Agner wrote:
>>>>> From: Stefan Agner <stefan.agner at toradex.com>
>>>>>
>>>>> i.MX 6 serial downloader is not necessarily booting via UART but can
>>>>> also boot from USB. In fact only some i.MX chips have serial
>>>>> downloader support via UART (e.g. 6UL/ULL and Vybrid) but all of
>>>>> them have serial downloader support via USB. Use the more appropriate
>>>>> BOOT_DEVICE_BOARD define which is used for ROM provided recovery
>>>>> mechanisms in general.
>>>>>
>>>>> Signed-off-by: Stefan Agner <stefan.agner at toradex.com>
>>>>> ---
>>>>>
>>>>> arch/arm/mach-imx/spl.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
>>>>> index 836b334fa9..bcd1033fdb 100644
>>>>> --- a/arch/arm/mach-imx/spl.c
>>>>> +++ b/arch/arm/mach-imx/spl.c
>>>>> @@ -27,7 +27,7 @@ u32 spl_boot_device(void)
>>>>> 	 * BOOT_MODE - see IMX6DQRM Table 8-1
>>>>> 	 */
>>>>> 	if (((bmode >> 24) & 0x03) == 0x01) /* Serial Downloader */
>>>>> -		return BOOT_DEVICE_UART;
>>>
>>> Returning BOOT_DEVICE_UART here makes the SPL to load U-Boot from the
>>> debug UART using a ymodem transfer when enabled with
>>> CONFIG_SPL_YMODEM_SUPPORT like it is on the OPOS6ULDev board. This is
>>> now broken.
>>
>> For reference, the change has been discussed here:
>> https://www.mail-archive.com/u-boot@lists.denx.de/msg259729.html
>>
>> Returning UART is definitely wrong, because the boot ROM can also boot
>> from USB…
>> As I mentioned in the other thread, ideally we should be able to do a
>> runtime detection to discriminate between UART and USB loader. I think
>> Stefano did not like that since it relies on undocumented features.
> 
> I don’t understand.
> This has nothing to do with what the ROM can or can’t boot.

It should reflect what the boot ROM did... And the user presumably wants
SPL to continue booting from the same source.

One could imagine a board specific translation table, e.g. boot from ->
boot continue...

> spl_boot_device returns the device where the SPL will find the U-Boot
> image. Here, it makes the distinction between normal boots and Serial
> Downloader boots (BOOT_MODE[0:1] = 0b01) no matter how the SPL has
> been loaded. So, we actually could return whatever we want here, UART,
> USB, eMMC, etc…, as the function does for normal boots. Of course,
> UART or USB is the only two choices that make sense for Serial
> Downloader boots.

Sure every combination is possible, but most likely is to continue using
what the boot ROM used. So when serial downloader over USB has been
used, you probably want to continue booting through USB. Same for UART.

And that is the problem, from the SRC/GPR regiters we cannot detect
whether the boot ROM loaded SPL through UART or USB... There is a
method, by looking at the USB PHY power flag, which seems to work, but
it is not a really nice way.

I vote for using the USB PHY hack nonetheless....


> 
> Maybe we could do something like:
> 
> #if CONFIG_IS_ENABLED(SPL_YMODEM_SUPPORT)
> 	return BOOT_DEVICE_UART
> #else
> 	return BOOT_DEVICE_BOARD
> #endif
> 
> or let the boards return the device they want by overloading board_boot_order.
> 
> ?

All methods seem a bit hacky to me...

Stefano, any idea/preferences?

--
Stefan

> 
> Regards,
> 
>>
>> So BOOT_DEVICE is really ambiguous. Maybe BOOT_DEVICE_* should be a
>> bitfield? Then we could return BOOT_DEVICE_UART and BOOT_DEVICE_USB, and
>> boards can just compile in the support they need.
>>
>>
>> Changing the Y-Modem support to boot device BOOT_DEVICE_BOARD is not
>> possible since other SoCs use BOOT_DEVICE_UART.
>>
>> I guess we could just add a second SPL_LOAD_IMAGE_METHOD in
>> common/spl/spl_ymodem.c for BOOT_DEVICE_BOARD.
>>
>> --
>> Stefan
>>
>>
>>>
>>> Regards,
>>>
>>>>> +		return BOOT_DEVICE_BOARD;
>>>>>
>>>>> 	/* BOOT_CFG1[7:4] - see IMX6DQRM Table 8-8 */
>>>>> 	switch ((reg & IMX6_BMODE_MASK) >> IMX6_BMODE_SHIFT) {
>>>>> @@ -43,7 +43,7 @@ u32 spl_boot_device(void)
>>>>> 		}
>>>>> 	/* Reserved: Used to force Serial Downloader */
>>>>> 	case IMX6_BMODE_RESERVED:
>>>>> -		return BOOT_DEVICE_UART;
>>>>> +		return BOOT_DEVICE_BOARD;
>>>>> 	/* SATA: See 8.5.4, Table 8-20 */
>>>>> 	case IMX6_BMODE_SATA:
>>>>> 		return BOOT_DEVICE_SATA;
>>>>>
>>>>
>>>> Applied to u-boot-imx, -master, thanks !
>>>>
>>>> Best regards,
>>>> Stefano Babic
>>>>
>>>>
>>>> --
>>>> =====================================================================
>>>> 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