On 9/17/26 10:43, Patrice CHOTARD wrote:
> 
> 
> 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]/

For information

I submitted Marek's proposal: 
https://patchwork.ozlabs.org/project/uboot/patch/20260917-ums_increase_ctrl-c_responsiveness-v1-1-95305f044...@foss.st.com/

Thanks
Patrice 

Reply via email to