[PATCH] fastboot: release usb_gadget on reboot commands

[Marek Vasut]

On 7/25/22 15:19, Mattijs Korpershoek wrote:
> On ven., juil. 22, 2022 at 23:48, Marek Vasut <marex at denx.de> wrote:
> 
>> On 7/21/22 15:59, Mattijs Korpershoek wrote:
>>
>> [...]
>>
>>> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
>>> index 8ba55aab9f8f..a00d1ca571d1 100644
>>> --- a/drivers/usb/gadget/f_fastboot.c
>>> +++ b/drivers/usb/gadget/f_fastboot.c
>>> @@ -420,7 +420,8 @@ static int fastboot_tx_write_str(const char *buffer)
>>>    
>>>    static void compl_do_reset(struct usb_ep *ep, struct usb_request *req)
>>>    {
>>> -	do_reset(NULL, 0, 0, NULL);
>>> +	g_dnl_trigger_detach();
>>> +	g_dnl_trigger_reset_request();
> 
> Hi Marek,
> 
> Thank you for your review and your suggestions.
> 
>>
>> Wouldn't it be enough to call usb_gadget_release() before do_reset()
>> here ? Or actually fix up the hardware state in your platform reset
> 
> Calling usb_gadget_release() just before do_reset() is sufficient as well.
> However usb_gadget_release(int index) takes a controller index as
> argument.
> This controller index is retrieved from the "=> fastboot" cmd and I
> could not come up with something elegant to retrieve it from
> f_fastboot.c.
> I can have another look if you think that's a better solution.

I think it would be nice to have this kind of generic fix in addition to 
one specific to your platform , as discussed below.

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