Re: [PATCH] fastboot: release usb_gadget on reboot commands

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Marek,

On Tue, Jul 26, 2022 at 15:36, Marek Vasut <marex@denx.de> wrote:

> On 7/26/22 10:25, Mattijs Korpershoek wrote:
> [...]
>>>>> implementation, which would cover all such odd states for every other
>>>>> USB UDC mode of operation, not just fastboot ?
>>>>
>>>> Implementing a platform_reset to reset the usb controller also works.
>>>> I discussed this with Neil at [1] and he suggested to send the f_fastboot.c fix
>>>> (this one) because it's generic and might fix issues for other boards.
>>>>
>>>> So, should I abandon this version and go with the platform specific fix instead ?
>>>>
>>>> [1] https://gitlab.com/baylibre/amlogic/atv/u-boot/-/commit/94c79b8226875babc20311cac7dac30e79238c9d#note_1020214136
>>>
>>> If your controller is responsible for causing the platform to fail to
>>> reboot, then I think it makes sense to do a fix either in the reset
>>> implementation for that platform, or the controller driver.
>> 
>> To be clear, the usb controller is *not* causing the whole platform to
>> reboot.
>> On do_reset(), the platform resets fine. However, the usb controller is
>> *not* put in reset. Because of that, the host does not detect a USB
>> disconnection. Which is what I'm trying to solve here.
>
> Yes, this is how I understand the problem too.
>
> Assume someone would run e.g. ums code, which would bring up the 
> controller too, I suspect it would also cause the same reset issues. 
> Which is why I wonder whether we shouldn't add this kind of fix into the 
> reset handler for the platform itself.

Oh. Got it. I was solely focusing on my use case (fastboot) and not
considering ums or other things.
Thank you for the explanation.
It probably makes sense to add it in the platform reset in that case.

>
>> If we do the generic fix, there is not really a point into implementing
>> a platform specific reset.
>> Note that on linux, when we run "reboot" the driver's remove() function
>> takes care of usb reset.
>
> Does that also work with sysrq-B or reboot -nf (one of the fast reboots) ?

# echo b > /proc/sysrq-trigger 
[ 3713.306318] sysrq: Resetting
[ 3713.306462] SMP: stopping secondary CPUs

Also triggers the same bug as the one I'm attempting to solve here.
We observe a very long delay before the host reports:
[31124.104342] usb 1-1: USB disconnect, device number 20

Thank you for bringing that up!