[U-Boot] [PATCH v1 1/2] imx: add macro to detect whether USB has been initialized

[Stefano Babic]

Hi Stefan, Eric,

On 05/09/2017 06:50, Stefan Agner wrote:
> On 2017-09-04 19:57, Eric Nelson wrote:
>> Hi Stefan,
>>
>> On 09/04/2017 06:21 PM, Stefan Agner wrote:
>>> From: Stefan Agner <stefan.agner at toradex.com>
>>>
>>> This macro allows to detect whether the boot ROM initialized USB
>>> already (serial downloader). This is helpful to reliably detect
>>> if the system has been recovered via USB serial downloader.
>>>
>>> Signed-off-by: Stefan Agner <stefan.agner at toradex.com>
>>> Acked-by: Marcel Ziswiler <marcel.ziswiler at toradex.com>
>>> ---
>>> Hi Stefano,
>>>
>>> I noted already in my initial post that detection of serial
>>> downloader mode is somewhat brittle:
>>> https://lists.denx.de/pipermail/u-boot/2017-August/301952.html
>>>
>>> This came up quite fast now also for other boards:
>>> https://www.mail-archive.com/u-boot@lists.denx.de/msg262234.html
>>>
>>> We use this patches since quite some time. Also NXP uses this
>>> detection method to start their mfgr tools... Altough a hack,
>>> maybe we should still add it upstream?
>>>
>>> --
>>> Stefan
>>>
>>>
>>>   arch/arm/include/asm/arch-mx6/imx-regs.h | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h
>>> index 86e267087a..895ef4de83 100644
>>> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
>>> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
>>> @@ -985,5 +985,12 @@ struct pwm_regs {
>>>   	u32	pr;
>>>   	u32	cnr;
>>>   };
>>
>> It seems as if you've already named a constant, so you might as well
>> #define and use it (USBPH0_PWD or USB0_PWD).
> 
> Agreed.
> 
>>
>> The reference manual seems to call it RXPWDRX though.
> 
> I guess taking the latest naming from the manual make sense. Will update
> in v2.
> 
> Before I send out v2, I'd like to know from Stefano whether he agrees
> with the general direction of the patch.
> 

Green light from my side :-)

> 
> 
>>
>>> +
>>> +/*
>>> + * If ROM fell back to USB recover mode, USBPH0_PWD will be clear to use USB
>>> + * If boot from the other mode, USB0_PWD will keep reset value
>>> + */
>>> +#define	is_boot_from_usb(void) (!(readl(USB_PHY0_BASE_ADDR) & (1<<20)))
>>> +
>>>   #endif /* __ASSEMBLER__*/
>>>   #endif /* __ASM_ARCH_MX6_IMX_REGS_H__ */
>>>
>> If I'm reading your comment correctly, the RXPWDRX bit will be set (the
>> PHY will be powered down) unless it was enabled by the Boot ROM.
>>
>> Won't this also be clear if you've run 'usb start' under U-Boot?
> 
> Yes, this only works before a USB initialization...
> 
> This should be fine for the use case I have in mind (see patch 2).

Agree. We want to detect just in SPL which was the (last) bootdevice and
rather we cannot detect them from SCR/GPR that just reflect the
configured booting device and not a fallback.

> 
> Note this idea is borrowed from NXP downstream and seems to work here:
> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/arch/arm/include/asm/arch-mx7/imx-regs.h?h=imx_v2016.03_4.1.15_2.0.0_ga#n1204
> 

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