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

Reply via email to