On Mon, Jul 25, 2022 at 17:30, Marek Vasut <ma...@denx.de> wrote:
> On 7/25/22 15:19, Mattijs Korpershoek wrote:
>> On ven., juil. 22, 2022 at 23:48, Marek Vasut <ma...@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.
ACK
>
>>> 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.
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.
