mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Jules Maselbas <jmaselbas@kalray.eu>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH master] usb: dwc2: increase timeout for waiting on host mode
Date: Wed, 21 Apr 2021 10:50:52 +0200	[thread overview]
Message-ID: <27969059-7cfb-cb4a-cd49-52c03c156799@pengutronix.de> (raw)
In-Reply-To: <20210421084729.GD21066@tellis.lin.mbt.kalray.eu>

Hello Jules,

On 21.04.21 10:47, Jules Maselbas wrote:
> Hi Ahmad,
> 
> On Wed, Apr 21, 2021 at 09:27:04AM +0200, Ahmad Fatoum wrote:
>> Commit 26459ab7803a ("usb: dwc2: Rework wait for host mode during
>> core reset") effectively reduced the timeout on switch to host mode
>> from 200ms to 110 us, which is insufficient for the IP on the Raspberry
>> Pi 3b, leading to:
>>
>>   dwc2 3f980000.usb@7e980000.of: dwc2_wait_for_mode: Couldn't set host mode
>>
>> and an unusable USB (and Ethernet) after.
>>
>> Bump up the timeout to 200ms and help future debugging by logging how
>> much time it actually took. For the Raspberry 3b I got a value of 49ms.
> Indeed, I've changed the timeout in Commit 26459ab7803a, I've tried to
> follow what's done in Linux in drivers/usb/dwc2/core.c, see
> https://elixir.bootlin.com/linux/latest/source/drivers/usb/dwc2/core.c#L385
> 
> Turns out I've made a mistake, Linux is using 110 ms and not µs.
> So maybe we can use 110 * MSECOND ? it will still be greater than what's
> required for the Raspberry 3b.  In the other hand the wait will stop as
> soon as the mode is set. it will only make the worst case faster...
> which is not a big deal.

I am fine with aligning ourselves with what Linux is doing.
Can you post that as a separate patch? That way this here can go into
v2021.05.0 to fix the immediate breakage and yours can go into v2020.06.0,
which will hopefully afford users some more time to test.

Cheers,
Ahmad

>>
>> Note that this is also called from dwc2_force_mode, so worst case is
>> that a stuck IP delays barebox startup by 200ms.
>> An error message would alert to this fact, so it can be corrected.
> Ok, this can be useful.
> 
>> Fixes: 26459ab7803a ("usb: dwc2: Rework wait for host mode during core reset")
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/usb/dwc2/core.c | 20 ++++++++++++--------
>>  1 file changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
>> index 5d04a07b0393..7813344ffa65 100644
>> --- a/drivers/usb/dwc2/core.c
>> +++ b/drivers/usb/dwc2/core.c
>> @@ -688,19 +688,23 @@ int dwc2_get_dr_mode(struct dwc2 *dwc2)
>>   */
>>  void dwc2_wait_for_mode(struct dwc2 *dwc2, bool host_mode)
>>  {
>> -	unsigned int timeout = 110 * USECOND;
>> -	int ret;
>> +	unsigned int timeout = 200 * MSECOND;
>> +	uint64_t start;
>>  
>>  	dev_vdbg(dwc2->dev, "Waiting for %s mode\n",
>>  		 host_mode ? "host" : "device");
>>  
>> -	ret = wait_on_timeout(timeout, dwc2_is_host_mode(dwc2) == host_mode);
>> -	if (ret)
>> -		dev_err(dwc2->dev, "%s: Couldn't set %s mode\n",
>> -				 __func__, host_mode ? "host" : "device");
>> +	start = get_time_ns();
>> +	while (dwc2_is_host_mode(dwc2) != host_mode) {
>> +		if (is_timeout(start, timeout)) {
>> +			dev_err(dwc2->dev, "%s: Couldn't set %s mode\n",
>> +				__func__, host_mode ? "host" : "device");
>> +			return;
>> +		}
>> +	}
>>  
>> -	dev_vdbg(dwc2->dev, "%s mode set\n",
>> -		 host_mode ? "Host" : "Device");
>> +	dev_vdbg(dwc2->dev, "%s mode set after %lluns\n",
>> +		 host_mode ? "Host" : "Device", get_time_ns() - start);
>>  }
>>  
>>  /**
>> -- 
>> 2.29.2
>>
>>
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  reply	other threads:[~2021-04-21  8:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-21  7:27 Ahmad Fatoum
2021-04-21  8:47 ` Jules Maselbas
2021-04-21  8:50   ` Ahmad Fatoum [this message]
2021-04-21  9:11     ` Jules Maselbas
2021-05-07  8:07     ` Sascha Hauer
2021-06-02 11:21       ` Ahmad Fatoum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=27969059-7cfb-cb4a-cd49-52c03c156799@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=jmaselbas@kalray.eu \
    --subject='Re: [PATCH master] usb: dwc2: increase timeout for waiting on host mode' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox