[U-Boot] usb: musb-new: Add CONFIG_USB_MUSB_INIT_TIMEOUT

Message ID 20181207085651.12674-1-alex.kiernan@gmail.com
State New
Delegated to: Marek Vasut
Headers show
Series
  • [U-Boot] usb: musb-new: Add CONFIG_USB_MUSB_INIT_TIMEOUT
Related show

Commit Message

Alex Kiernan Dec. 7, 2018, 8:56 a.m.
Make the timeout in musb_lowlevel_init() configurable from the 1s default.
Some USB memory sticks take longer than 1s to connect and so whilst
usable in Linux aren't usable in U-Boot. The default in Kconfig is still
1000ms, so this should be a non-change for any existing users.

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

 drivers/usb/musb-new/Kconfig      | 8 ++++++++
 drivers/usb/musb-new/musb_uboot.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

Comments

Marek Vasut Dec. 7, 2018, 11:48 a.m. | #1
On 12/07/2018 09:56 AM, Alex Kiernan wrote:
[...]
> +++ b/drivers/usb/musb-new/musb_uboot.c
> @@ -214,7 +214,7 @@ int musb_lowlevel_init(struct musb_host_data *host)
>  {
>  	void *mbase;
>  	/* USB spec says it may take up to 1 second for a device to connect */
> -	unsigned long timeout = get_timer(0) + 1000;
> +	unsigned long timeout = get_timer(0) + CONFIG_USB_MUSB_INIT_TIMEOUT;

Isn't this the same as usb_pgood_delay ?

>  	int ret;
>  
>  	if (!host->host) {
>
Alex Kiernan Dec. 7, 2018, 12:29 p.m. | #2
On Fri, Dec 7, 2018 at 11:54 AM Marek Vasut <marex@denx.de> wrote:
>
> On 12/07/2018 09:56 AM, Alex Kiernan wrote:
> [...]
> > +++ b/drivers/usb/musb-new/musb_uboot.c
> > @@ -214,7 +214,7 @@ int musb_lowlevel_init(struct musb_host_data *host)
> >  {
> >       void *mbase;
> >       /* USB spec says it may take up to 1 second for a device to connect */
> > -     unsigned long timeout = get_timer(0) + 1000;
> > +     unsigned long timeout = get_timer(0) + CONFIG_USB_MUSB_INIT_TIMEOUT;
>
> Isn't this the same as usb_pgood_delay ?
>

Looks like it; I'll spin a v2 which uses that.
Marek Vasut Dec. 7, 2018, 12:30 p.m. | #3
On 12/07/2018 01:29 PM, Alex Kiernan wrote:
> On Fri, Dec 7, 2018 at 11:54 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 12/07/2018 09:56 AM, Alex Kiernan wrote:
>> [...]
>>> +++ b/drivers/usb/musb-new/musb_uboot.c
>>> @@ -214,7 +214,7 @@ int musb_lowlevel_init(struct musb_host_data *host)
>>>  {
>>>       void *mbase;
>>>       /* USB spec says it may take up to 1 second for a device to connect */
>>> -     unsigned long timeout = get_timer(0) + 1000;
>>> +     unsigned long timeout = get_timer(0) + CONFIG_USB_MUSB_INIT_TIMEOUT;
>>
>> Isn't this the same as usb_pgood_delay ?
>>
> 
> Looks like it; I'll spin a v2 which uses that.

I wonder if this code is needed at all, shouldn't the USB core deal with
this delay ?
Alex Kiernan Dec. 7, 2018, 12:59 p.m. | #4
On Fri, Dec 7, 2018 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>
> On 12/07/2018 01:29 PM, Alex Kiernan wrote:
> > On Fri, Dec 7, 2018 at 11:54 AM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 12/07/2018 09:56 AM, Alex Kiernan wrote:
> >> [...]
> >>> +++ b/drivers/usb/musb-new/musb_uboot.c
> >>> @@ -214,7 +214,7 @@ int musb_lowlevel_init(struct musb_host_data *host)
> >>>  {
> >>>       void *mbase;
> >>>       /* USB spec says it may take up to 1 second for a device to connect */
> >>> -     unsigned long timeout = get_timer(0) + 1000;
> >>> +     unsigned long timeout = get_timer(0) + CONFIG_USB_MUSB_INIT_TIMEOUT;
> >>
> >> Isn't this the same as usb_pgood_delay ?
> >>
> >
> > Looks like it; I'll spin a v2 which uses that.
>
> I wonder if this code is needed at all, shouldn't the USB core deal with
> this delay ?
>

Oh I see... certainly I can't just take that delay out, that just ends up at:

USB0:   scanning bus 0 for devices... USB device descriptor short read
(expected 8, got 0)
failed, error -5

Certainly without that delay, there's clearly delays in the rest of
the USB core, so I expect you're right, I just don't know where to put
the poll for MUSB_DEVCTL_HM.
Marek Vasut Dec. 7, 2018, 1:06 p.m. | #5
On 12/07/2018 01:59 PM, Alex Kiernan wrote:
> On Fri, Dec 7, 2018 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 12/07/2018 01:29 PM, Alex Kiernan wrote:
>>> On Fri, Dec 7, 2018 at 11:54 AM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 12/07/2018 09:56 AM, Alex Kiernan wrote:
>>>> [...]
>>>>> +++ b/drivers/usb/musb-new/musb_uboot.c
>>>>> @@ -214,7 +214,7 @@ int musb_lowlevel_init(struct musb_host_data *host)
>>>>>  {
>>>>>       void *mbase;
>>>>>       /* USB spec says it may take up to 1 second for a device to connect */
>>>>> -     unsigned long timeout = get_timer(0) + 1000;
>>>>> +     unsigned long timeout = get_timer(0) + CONFIG_USB_MUSB_INIT_TIMEOUT;
>>>>
>>>> Isn't this the same as usb_pgood_delay ?
>>>>
>>>
>>> Looks like it; I'll spin a v2 which uses that.
>>
>> I wonder if this code is needed at all, shouldn't the USB core deal with
>> this delay ?
>>
> 
> Oh I see... certainly I can't just take that delay out, that just ends up at:
> 
> USB0:   scanning bus 0 for devices... USB device descriptor short read
> (expected 8, got 0)
> failed, error -5
> 
> Certainly without that delay, there's clearly delays in the rest of
> the USB core, so I expect you're right, I just don't know where to put
> the poll for MUSB_DEVCTL_HM.

Can you research it a bit ? What does the _HM bit do ?

Patch

diff --git a/drivers/usb/musb-new/Kconfig b/drivers/usb/musb-new/Kconfig
index f8f2205a62d3..5b9df5227961 100644
--- a/drivers/usb/musb-new/Kconfig
+++ b/drivers/usb/musb-new/Kconfig
@@ -54,6 +54,14 @@  config USB_MUSB_SUNXI
 	Say y here to enable support for the sunxi OTG / DRC USB controller
 	used on almost all sunxi boards.
 
+config USB_MUSB_INIT_TIMEOUT
+	int "Low level initialization timeout"
+	default 1000
+	help
+	  The USB spec says it may take up to 1 second for a device to
+	  connect. Some devices take longer than this to connect, so
+	  you can increase the timeout here. The value here is in ms.
+
 endif
 
 config USB_MUSB_PIO_ONLY
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index d40772b1aa5c..82591ad8f6b5 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -214,7 +214,7 @@  int musb_lowlevel_init(struct musb_host_data *host)
 {
 	void *mbase;
 	/* USB spec says it may take up to 1 second for a device to connect */
-	unsigned long timeout = get_timer(0) + 1000;
+	unsigned long timeout = get_timer(0) + CONFIG_USB_MUSB_INIT_TIMEOUT;
 	int ret;
 
 	if (!host->host) {