[U-Boot] [PATCH v1 1/2] imx: fix USB boot mode detection for i.MX 6UL and 6ULL

[U-Boot] [PATCH v1 1/2] imx: fix USB boot mode detection for i.MX 6UL and 6ULL

[Stefan Agner]

From: Stefan Agner <stefan.agner@toradex.com>

Add the reserved boot mode used in the bmode command for i.MX 6UL
and 6ULL as introduced in commit 3fd9579085fa ("imx: mx6ull: fix USB
bmode for i.MX 6UL and 6ULL").

Also replace BMODE_UART with BMODE_RESERVED, which is more appropriate.
Commit 96aac843b68d ("imx: Use IMX6_BMODE_* macros instead of numericals")
added macros for boot modes, in the process the reserved boot mode got
named BMODE_UART. We use the reserved boot mode in the bmode command to
let the boot ROM enter serial downloader recovery mode. But this is only
a side effect, the actual boot mode is reserved...

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
---
Afaik there is no board yet using SPL on 6UL/ULL...

 arch/arm/include/asm/mach-imx/sys_proto.h | 7 ++++++-
 arch/arm/mach-imx/spl.c                   | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h
index 046df6291a..d94c095118 100644
--- a/arch/arm/include/asm/mach-imx/sys_proto.h
+++ b/arch/arm/include/asm/mach-imx/sys_proto.h
@@ -69,8 +69,13 @@ enum imx6_bmode_emi {
 
 enum imx6_bmode {
 	IMX6_BMODE_EMI,
-	IMX6_BMODE_UART,
+#if defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
+	IMX6_BMODE_QSPI,
+	IMX6_BMODE_RESERVED,
+#else
+	IMX6_BMODE_RESERVED,
 	IMX6_BMODE_SATA,
+#endif
 	IMX6_BMODE_SERIAL_ROM,
 	IMX6_BMODE_SD,
 	IMX6_BMODE_ESD,
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 75698c48ea..836b334fa9 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -42,7 +42,7 @@ u32 spl_boot_device(void)
 		break;
 		}
 	/* Reserved: Used to force Serial Downloader */
-	case IMX6_BMODE_UART:
+	case IMX6_BMODE_RESERVED:
 		return BOOT_DEVICE_UART;
 	/* SATA: See 8.5.4, Table 8-20 */
 	case IMX6_BMODE_SATA:
-- 
2.14.1

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

[Stefan Agner]

From: Stefan Agner <stefan.agner@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@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;
+		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;
-- 
2.14.1

[U-Boot] [PATCH v1 1/2] imx: fix USB boot mode detection for i.MX 6UL and 6ULL

[Stefano Babic]

On 16/08/2017 02:49, Stefan Agner wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
> 
> Add the reserved boot mode used in the bmode command for i.MX 6UL
> and 6ULL as introduced in commit 3fd9579085fa ("imx: mx6ull: fix USB
> bmode for i.MX 6UL and 6ULL").
> 
> Also replace BMODE_UART with BMODE_RESERVED, which is more appropriate.
> Commit 96aac843b68d ("imx: Use IMX6_BMODE_* macros instead of numericals")
> added macros for boot modes, in the process the reserved boot mode got
> named BMODE_UART. We use the reserved boot mode in the bmode command to
> let the boot ROM enter serial downloader recovery mode. But this is only
> a side effect, the actual boot mode is reserved...
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> ---
> Afaik there is no board yet using SPL on 6UL/ULL...

Not yet...

> 
>  arch/arm/include/asm/mach-imx/sys_proto.h | 7 ++++++-
>  arch/arm/mach-imx/spl.c                   | 2 +-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h
> index 046df6291a..d94c095118 100644
> --- a/arch/arm/include/asm/mach-imx/sys_proto.h
> +++ b/arch/arm/include/asm/mach-imx/sys_proto.h
> @@ -69,8 +69,13 @@ enum imx6_bmode_emi {
>  
>  enum imx6_bmode {
>  	IMX6_BMODE_EMI,
> -	IMX6_BMODE_UART,
> +#if defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
> +	IMX6_BMODE_QSPI,
> +	IMX6_BMODE_RESERVED,
> +#else
> +	IMX6_BMODE_RESERVED,
>  	IMX6_BMODE_SATA,
> +#endif
>  	IMX6_BMODE_SERIAL_ROM,
>  	IMX6_BMODE_SD,
>  	IMX6_BMODE_ESD,
> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> index 75698c48ea..836b334fa9 100644
> --- a/arch/arm/mach-imx/spl.c
> +++ b/arch/arm/mach-imx/spl.c
> @@ -42,7 +42,7 @@ u32 spl_boot_device(void)
>  		break;
>  		}
>  	/* Reserved: Used to force Serial Downloader */
> -	case IMX6_BMODE_UART:
> +	case IMX6_BMODE_RESERVED:
>  		return BOOT_DEVICE_UART;
>  	/* SATA: See 8.5.4, Table 8-20 */
>  	case IMX6_BMODE_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] [PATCH v1 2/2] imx: use BOOT_DEVICE_BOARD instead of UART

[Stefano Babic]

On 16/08/2017 02:49, Stefan Agner wrote:
> From: Stefan Agner <stefan.agner@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@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;
> +		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] [PATCH v1 1/2] imx: fix USB boot mode detection for i.MX 6UL and 6ULL

[Sébastien Szymanski]

Hello,

> 
> On 25 Aug 2017, at 13:32, Stefano Babic <sbabic@denx.de> wrote:
> 
> On 16/08/2017 02:49, Stefan Agner wrote:
>> From: Stefan Agner <stefan.agner@toradex.com>
>> 
>> Add the reserved boot mode used in the bmode command for i.MX 6UL
>> and 6ULL as introduced in commit 3fd9579085fa ("imx: mx6ull: fix USB
>> bmode for i.MX 6UL and 6ULL").
>> 
>> Also replace BMODE_UART with BMODE_RESERVED, which is more appropriate.
>> Commit 96aac843b68d ("imx: Use IMX6_BMODE_* macros instead of numericals")
>> added macros for boot modes, in the process the reserved boot mode got
>> named BMODE_UART. We use the reserved boot mode in the bmode command to
>> let the boot ROM enter serial downloader recovery mode. But this is only
>> a side effect, the actual boot mode is reserved...
>> 
>> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>> ---
>> Afaik there is no board yet using SPL on 6UL/ULL...
> 
> Not yet…

?

There are boards in the tree using SPL on i.MX6UL. OPOS6UL is one of them.

Regards,
Sébastien Szymanski

> 
>> 
>> arch/arm/include/asm/mach-imx/sys_proto.h | 7 ++++++-
>> arch/arm/mach-imx/spl.c                   | 2 +-
>> 2 files changed, 7 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h
>> index 046df6291a..d94c095118 100644
>> --- a/arch/arm/include/asm/mach-imx/sys_proto.h
>> +++ b/arch/arm/include/asm/mach-imx/sys_proto.h
>> @@ -69,8 +69,13 @@ enum imx6_bmode_emi {
>> 
>> enum imx6_bmode {
>> 	IMX6_BMODE_EMI,
>> -	IMX6_BMODE_UART,
>> +#if defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
>> +	IMX6_BMODE_QSPI,
>> +	IMX6_BMODE_RESERVED,
>> +#else
>> +	IMX6_BMODE_RESERVED,
>> 	IMX6_BMODE_SATA,
>> +#endif
>> 	IMX6_BMODE_SERIAL_ROM,
>> 	IMX6_BMODE_SD,
>> 	IMX6_BMODE_ESD,
>> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
>> index 75698c48ea..836b334fa9 100644
>> --- a/arch/arm/mach-imx/spl.c
>> +++ b/arch/arm/mach-imx/spl.c
>> @@ -42,7 +42,7 @@ u32 spl_boot_device(void)
>> 		break;
>> 		}
>> 	/* Reserved: Used to force Serial Downloader */
>> -	case IMX6_BMODE_UART:
>> +	case IMX6_BMODE_RESERVED:
>> 		return BOOT_DEVICE_UART;
>> 	/* SATA: See 8.5.4, Table 8-20 */
>> 	case IMX6_BMODE_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

[U-Boot] [PATCH v1 1/2] imx: fix USB boot mode detection for i.MX 6UL and 6ULL

[Jagan Teki]

On Sat, Aug 26, 2017 at 12:58 PM, Sébastien Szymanski
<sebastien.szymanski@armadeus.com> wrote:
> Hello,
>
>>
>> On 25 Aug 2017, at 13:32, Stefano Babic <sbabic@denx.de> wrote:
>>
>> On 16/08/2017 02:49, Stefan Agner wrote:
>>> From: Stefan Agner <stefan.agner@toradex.com>
>>>
>>> Add the reserved boot mode used in the bmode command for i.MX 6UL
>>> and 6ULL as introduced in commit 3fd9579085fa ("imx: mx6ull: fix USB
>>> bmode for i.MX 6UL and 6ULL").
>>>
>>> Also replace BMODE_UART with BMODE_RESERVED, which is more appropriate.
>>> Commit 96aac843b68d ("imx: Use IMX6_BMODE_* macros instead of numericals")
>>> added macros for boot modes, in the process the reserved boot mode got
>>> named BMODE_UART. We use the reserved boot mode in the bmode command to
>>> let the boot ROM enter serial downloader recovery mode. But this is only
>>> a side effect, the actual boot mode is reserved...
>>>
>>> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>>> ---
>>> Afaik there is no board yet using SPL on 6UL/ULL...
>>
>> Not yet…
>
> ?
>
> There are boards in the tree using SPL on i.MX6UL. OPOS6UL is one of them.

Engicam GEAM6 and IS.IoT uses SPL both are i.MX6UL

-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

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

[Sébastien Szymanski]

Hello,

> On 25 Aug 2017, at 13:33, Stefano Babic <sbabic@denx.de> wrote:
> 
> On 16/08/2017 02:49, Stefan Agner wrote:
>> From: Stefan Agner <stefan.agner@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@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.

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

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

[Stefan Agner]

On 2017-08-27 01:45, Sébastien Szymanski wrote:
> Hello,
> 
>> On 25 Aug 2017, at 13:33, Stefano Babic <sbabic@denx.de> wrote:
>>
>> On 16/08/2017 02:49, Stefan Agner wrote:
>>> From: Stefan Agner <stefan.agner@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@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 at 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.

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

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

[Sébastien Szymanski]

Hello,

> On 27 Aug 2017, at 21:17, Stefan Agner <stefan@agner.ch> wrote:
> 
> On 2017-08-27 01:45, Sébastien Szymanski wrote:
>> Hello,
>> 
>>> On 25 Aug 2017, at 13:33, Stefano Babic <sbabic@denx.de> wrote:
>>> 
>>> On 16/08/2017 02:49, Stefan Agner wrote:
>>>> From: Stefan Agner <stefan.agner@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@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 at 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. 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.

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.

?

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

[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@agner.ch> wrote:
>>
>> On 2017-08-27 01:45, Sébastien Szymanski wrote:
>>> Hello,
>>>
>>>> On 25 Aug 2017, at 13:33, Stefano Babic <sbabic@denx.de> wrote:
>>>>
>>>> On 16/08/2017 02:49, Stefan Agner wrote:
>>>>> From: Stefan Agner <stefan.agner@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@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 at 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

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

[Stefano Babic]

Hi Sébastien, Stefan,

On 28/08/2017 00:05, Stefan Agner wrote:
> On 2017-08-27 13:21, Sébastien Szymanski wrote:
>> Hello,
>>
>>> On 27 Aug 2017, at 21:17, Stefan Agner <stefan@agner.ch> wrote:
>>>
>>> On 2017-08-27 01:45, Sébastien Szymanski wrote:
>>>> Hello,
>>>>
>>>>> On 25 Aug 2017, at 13:33, Stefano Babic <sbabic@denx.de> wrote:
>>>>>
>>>>> On 16/08/2017 02:49, Stefan Agner wrote:
>>>>>> From: Stefan Agner <stefan.agner@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@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 at 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.

Right - even if I understand that this breaks the ymodem load, we have
to think about that ymodem was introduced as work-around because the
Serial Protocol was not supported. However, this was criticized because
a load takes significantly much time as with USB. With support for SDP
in U-Boot, it is logical that the load goes on with the same interface
SPL is loaded, that is USB.

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

Well, this is a trick. SRC / GPR should say us what's going on, rather
it is not.

Best regards,
Stefano

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


-- 
=====================================================================
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] [PATCH v1 1/2] imx: fix USB boot mode detection for i.MX 6UL and 6ULL

[Stefano Babic]

Hi Stefan,

On 16/08/2017 02:49, Stefan Agner wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
> 
> Add the reserved boot mode used in the bmode command for i.MX 6UL
> and 6ULL as introduced in commit 3fd9579085fa ("imx: mx6ull: fix USB
> bmode for i.MX 6UL and 6ULL").
> 
> Also replace BMODE_UART with BMODE_RESERVED, which is more appropriate.
> Commit 96aac843b68d ("imx: Use IMX6_BMODE_* macros instead of numericals")
> added macros for boot modes, in the process the reserved boot mode got
> named BMODE_UART. We use the reserved boot mode in the bmode command to
> let the boot ROM enter serial downloader recovery mode. But this is only
> a side effect, the actual boot mode is reserved...
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> ---

I applied it, but it breaks due the #ifdef in sys_proto.h that hides
some symbols as IMX6_BMODE_SATA. Could you take a look before I send my
PR to Tom ?

Thanks,
Stefano

> Afaik there is no board yet using SPL on 6UL/ULL...
> 
>  arch/arm/include/asm/mach-imx/sys_proto.h | 7 ++++++-
>  arch/arm/mach-imx/spl.c                   | 2 +-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h
> index 046df6291a..d94c095118 100644
> --- a/arch/arm/include/asm/mach-imx/sys_proto.h
> +++ b/arch/arm/include/asm/mach-imx/sys_proto.h
> @@ -69,8 +69,13 @@ enum imx6_bmode_emi {
>  
>  enum imx6_bmode {
>  	IMX6_BMODE_EMI,
> -	IMX6_BMODE_UART,
> +#if defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
> +	IMX6_BMODE_QSPI,
> +	IMX6_BMODE_RESERVED,
> +#else
> +	IMX6_BMODE_RESERVED,
>  	IMX6_BMODE_SATA,
> +#endif
>  	IMX6_BMODE_SERIAL_ROM,
>  	IMX6_BMODE_SD,
>  	IMX6_BMODE_ESD,
> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> index 75698c48ea..836b334fa9 100644
> --- a/arch/arm/mach-imx/spl.c
> +++ b/arch/arm/mach-imx/spl.c
> @@ -42,7 +42,7 @@ u32 spl_boot_device(void)
>  		break;
>  		}
>  	/* Reserved: Used to force Serial Downloader */
> -	case IMX6_BMODE_UART:
> +	case IMX6_BMODE_RESERVED:
>  		return BOOT_DEVICE_UART;
>  	/* SATA: See 8.5.4, Table 8-20 */
>  	case IMX6_BMODE_SATA:
> 


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