[PATCH 0/2] usb: gdaget: f_mass_storage: various fixes

[PATCH 0/2] usb: gdaget: f_mass_storage: various fixes

[Patrice Chotard]

- remove useless "rc" local variable
  - optimize schedule() call

 optimize schedule() call 

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
Patrice Chotard (2):
      usb: gadget: f_mass_storage: Remove useless rc variable
      usb: gadget: f_mass_storage: Optimize schedule() call

 drivers/usb/gadget/f_mass_storage.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
---
base-commit: 36c377b9859ffb53eb1e39ea31e8d96d1e0fe1e5
change-id: 20260817-move_schedule_inside_sleep_thread-134c7ed2f87a

Best regards,
--  
Patrice Chotard <patrice.chotard@foss.st.com>

[PATCH 1/2] usb: gadget: f_mass_storage: Remove useless rc variable

[Patrice Chotard]

rc variable is used as sleep_thread() return value.
It's initialized to 0 and never updated inside sleep_thread(),
remove it and return 0.

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
 drivers/usb/gadget/f_mass_storage.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index 7eb667c130d..621852080e2 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -650,7 +650,6 @@ static void busy_indicator(void)
 
 static int sleep_thread(struct fsg_common *common)
 {
-	int	rc = 0;
 	int i = 0, k = 0;
 
 	/* Wait until a signal arrives or we are woken up */
@@ -684,7 +683,8 @@ static int sleep_thread(struct fsg_common *common)
 		dm_usb_gadget_handle_interrupts(udcdev);
 	}
 	common->thread_wakeup_needed = 0;
-	return rc;
+
+	return 0;
 }
 
 /*-------------------------------------------------------------------------*/

-- 
2.43.0

Re: [PATCH 1/2] usb: gadget: f_mass_storage: Remove useless rc variable

[Heinrich Schuchardt]

On 8/17/26 17:41, Patrice Chotard wrote:
> rc variable is used as sleep_thread() return value.
> It's initialized to 0 and never updated inside sleep_thread(),
> remove it and return 0.
> 
> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> ---
>   drivers/usb/gadget/f_mass_storage.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
> index 7eb667c130d..621852080e2 100644
> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -650,7 +650,6 @@ static void busy_indicator(void)
>   
>   static int sleep_thread(struct fsg_common *common)
>   {
> -	int	rc = 0;
>   	int i = 0, k = 0;
>   
>   	/* Wait until a signal arrives or we are woken up */
> @@ -684,7 +683,8 @@ static int sleep_thread(struct fsg_common *common)
>   		dm_usb_gadget_handle_interrupts(udcdev);
>   	}
>   	common->thread_wakeup_needed = 0;
> -	return rc;
> +
> +	return 0;
>   }
>   
>   /*-------------------------------------------------------------------------*/
> 

Re: [PATCH 1/2] usb: gadget: f_mass_storage: Remove useless rc variable

[Marek Vasut]

On 8/17/26 5:41 PM, Patrice Chotard wrote:
> rc variable is used as sleep_thread() return value.
> It's initialized to 0 and never updated inside sleep_thread(),
> remove it and return 0.
> 
> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>

Reviewed-by: Marek Vasut <marek.vasut+usb@mailbox.org>

Re: [PATCH 1/2] usb: gadget: f_mass_storage: Remove useless rc variable

[Mattijs Korpershoek]

Hi Patrice,

Thank you for the patch.

On Mon, Aug 17, 2026 at 17:41, Patrice Chotard <patrice.chotard@foss.st.com> wrote:

> rc variable is used as sleep_thread() return value.
> It's initialized to 0 and never updated inside sleep_thread(),
> remove it and return 0.
>
> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

[PATCH 2/2] usb: gadget: f_mass_storage: Optimize schedule() call

[Patrice Chotard]

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 <patrice.chotard@foss.st.com>
---
 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();
 
 			k = 0;
 		}
 
-		schedule();
 		dm_usb_gadget_handle_interrupts(udcdev);
 	}
 	common->thread_wakeup_needed = 0;

-- 
2.43.0

Re: [PATCH 2/2] usb: gadget: f_mass_storage: Optimize schedule() call

[Mattijs Korpershoek]

Hi Patrice,

Thank you for the patch and sorry for the review delays.

On Mon, Aug 17, 2026 at 17:41, Patrice Chotard <patrice.chotard@foss.st.com> 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 <patrice.chotard@foss.st.com>
> ---
>  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?

>  
>  			k = 0;
>  		}
>  
> -		schedule();
>  		dm_usb_gadget_handle_interrupts(udcdev);
>  	}
>  	common->thread_wakeup_needed = 0;
>
> -- 
> 2.43.0

Re: [PATCH 2/2] usb: gadget: f_mass_storage: Optimize schedule() call

[Patrice CHOTARD]

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 <patrice.chotard@foss.st.com> 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 <patrice.chotard@foss.st.com>
>> ---
>>  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