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

[Stefan Agner]

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.



> 
>> +
>> +/*
>> + * 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).

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

--
Stefan