On 9/11/26 14:38, Patrice CHOTARD wrote:
> 
> 
> On 9/7/26 11:07, Mattijs Korpershoek wrote:
>> Hi Patrice,
>>
>> Thank you for the patch and sorry for the review delays.
>>
>> On Mon, Aug 17, 2026 at 17:41, Patrice Chotard <[email protected]> 
>> wrote:
>>
>>> schedule was added in sleep_thread() by commit 4b6a3e860878
>>> ("usb: gadget: f_mass_storage: Add schedule() in sleep_thread()").
>>> to ensure that watchdog is still reset periodically even on platform
>>> that doesn't implement g_dnl_board_usb_cable_connected() and in case USB
>>> cable is not connected.
>>>
>>> Instead of calling schedule() for each for() loop iteration, call
>>> schedule() only in case g_dnl_board_usb_cable_connected() is not
>>> overloaded, in this particular case, g_dnl_board_usb_cable_connected()'s
>>> return value is -EOPNOTSUPP.
>>>
>>> Signed-off-by: Patrice Chotard <[email protected]>
>>> ---
>>>  drivers/usb/gadget/f_mass_storage.c | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/gadget/f_mass_storage.c 
>>> b/drivers/usb/gadget/f_mass_storage.c
>>> index 621852080e2..f467693d2cb 100644
>>> --- a/drivers/usb/gadget/f_mass_storage.c
>>> +++ b/drivers/usb/gadget/f_mass_storage.c
>>> @@ -651,6 +651,7 @@ static void busy_indicator(void)
>>>  static int sleep_thread(struct fsg_common *common)
>>>  {
>>>     int i = 0, k = 0;
>>> +   int ret;
>>>  
>>>     /* Wait until a signal arrives or we are woken up */
>>>     for (;;) {
>>> @@ -673,13 +674,15 @@ static int sleep_thread(struct fsg_common *common)
>>>                             return -EPIPE;
>>>  
>>>                     /* Check cable connection */
>>> -                   if (!g_dnl_board_usb_cable_connected())
>>> +                   ret = g_dnl_board_usb_cable_connected();
>>> +                   if (!ret)
>>>                             return -EIO;
>>> +                   if (ret == -EOPNOTSUPP)
>>> +                           schedule();
>>
>> What happens on boards that override g_dnl_board_usb_cable_connected()
>> that return a positive value?
>>
>> Per my understanding, schedule() will never be called, and the watchdog
>> will be triggered at some point, no?
> 
> Hi Mattijs,
> 
> In case g_dnl_board_usb_cable_connected() returns 1 (it's currently the case 
> on STM32MP157c-DK2 board), we stay inside sleep_thread() for(;;) loop until 
> common->thread_wakeup_needed is set to 1 by wakeup_thread(). 
> 
> wakeup_thread() is called periodically (around every second, it was what i 
> observed 
> during testing) by either bulk_out_complete() or bulk_in_complete().
> 
> Then we came back in fsg_main_thread() which was sleep_thread() caller, 
> and came back in while(1) loop of do_usb_mass_storage() where schedule() is 
> called.
> 
> So watchdog is reset periodically.
> 
> Patrice
>>
>>>  
>>>                     k = 0;
>>>             }
>>>  
>>> -           schedule();
>>>             dm_usb_gadget_handle_interrupts(udcdev);
>>>     }
>>>     common->thread_wakeup_needed = 0;
>>>
>>> -- 
>>> 2.43.0
> 
Hi Mattijs

We got a discussion with Marek on the previous version of this patch [1].
Marek's proposal is more simple/efficient than this current patch.

This patch will be abandoned.

Thanks 
Patrice

[1] 
https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/

Reply via email to