[PATCH] staging: rtl8723bs: replace msleep with usleep_range

Ofek Almog posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
drivers/staging/rtl8723bs/core/rtw_cmd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] staging: rtl8723bs: replace msleep with usleep_range
Posted by Ofek Almog 1 month, 2 weeks ago
This patch replaces msleep(10) with usleep_range(10000, 11000) to provide
more precise delay handling while allowing the timer subsystem to coalesce
timers efficiently.

Signed-off-by: Ofek Almog <ofekalm100@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index b2e7f479f72b..3f5b3ca2066d 100644
--- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
@@ -214,7 +214,7 @@ void _rtw_free_evt_priv(struct	evt_priv *pevtpriv)
 {
 	_cancel_workitem_sync(&pevtpriv->c2h_wk);
 	while (pevtpriv->c2h_wk_alive)
-		msleep(10);
+		usleep_range(10000, 11000);
 
 	while (!rtw_cbuf_empty(pevtpriv->c2h_queue)) {
 		void *c2h = rtw_cbuf_pop(pevtpriv->c2h_queue);
-- 
2.43.0
Re: [PATCH] staging: rtl8723bs: replace msleep with usleep_range
Posted by Ethan Tidmore 1 month, 2 weeks ago
On Sat Feb 14, 2026 at 11:31 AM CST, Ofek Almog wrote:
> This patch replaces msleep(10) with usleep_range(10000, 11000) to provide
> more precise delay handling while allowing the timer subsystem to coalesce
> timers efficiently.
>
> Signed-off-by: Ofek Almog <ofekalm100@gmail.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_cmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> index b2e7f479f72b..3f5b3ca2066d 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> @@ -214,7 +214,7 @@ void _rtw_free_evt_priv(struct	evt_priv *pevtpriv)
>  {
>  	_cancel_workitem_sync(&pevtpriv->c2h_wk);
>  	while (pevtpriv->c2h_wk_alive)
> -		msleep(10);
> +		usleep_range(10000, 11000);
>  
>  	while (!rtw_cbuf_empty(pevtpriv->c2h_queue)) {
>  		void *c2h = rtw_cbuf_pop(pevtpriv->c2h_queue);

Shouldn't we be using fsleep() now instead of replacing msleep() with
usleep_range() when the time is sub 20ms?

I feel like I read Andy Shevchenko mention this elsewhere.

Thanks,

ET
Re: [PATCH] staging: rtl8723bs: replace msleep with usleep_range
Posted by Andy Shevchenko 1 month, 2 weeks ago
On Sat, Feb 14, 2026 at 12:11:16PM -0600, Ethan Tidmore wrote:
> On Sat Feb 14, 2026 at 11:31 AM CST, Ofek Almog wrote:
> > This patch replaces msleep(10) with usleep_range(10000, 11000) to provide
> > more precise delay handling while allowing the timer subsystem to coalesce
> > timers efficiently.

...

> >  	while (pevtpriv->c2h_wk_alive)
> > -		msleep(10);
> > +		usleep_range(10000, 11000);

The whole approach here seems to be fragile. It should rather use completion
mechanism instead of infinite loop.

> >  	while (!rtw_cbuf_empty(pevtpriv->c2h_queue)) {
> >  		void *c2h = rtw_cbuf_pop(pevtpriv->c2h_queue);
> 
> Shouldn't we be using fsleep() now instead of replacing msleep() with
> usleep_range() when the time is sub 20ms?
> 
> I feel like I read Andy Shevchenko mention this elsewhere.

Yep.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] staging: rtl8723bs: replace msleep with usleep_range
Posted by Ofek Almog 1 month, 2 weeks ago
(Resending as plain text, apologies if you received a previous HTML version)

Hi Andy, Ethan,

Thank you both for the review.

Regarding the fsleep() suggestion - understood, that clearly seems
like the right helper to use here.

Regarding the completion mechanism logic - I completely agree that the
current infinite loop polling is fragile. However, as this is my very
first contribution and I do not have the hardware to verify logic
changes, I am hesitant to refactor the synchronization mechanism
blindly.

Would you prefer I send a v2 that strictly switches to fsleep() as an
incremental improvement over msleep(), or should I drop this patch
altogether until someone with the hardware can refactor the logic
properly?

Best regards,
Ofek

‫בתאריך שבת, 14 בפבר׳ 2026 ב-20:24 מאת ‪Andy Shevchenko‬‏
<‪andriy.shevchenko@intel.com‬‏>:‬
>
> On Sat, Feb 14, 2026 at 12:11:16PM -0600, Ethan Tidmore wrote:
> > On Sat Feb 14, 2026 at 11:31 AM CST, Ofek Almog wrote:
> > > This patch replaces msleep(10) with usleep_range(10000, 11000) to provide
> > > more precise delay handling while allowing the timer subsystem to coalesce
> > > timers efficiently.
>
> ...
>
> > >     while (pevtpriv->c2h_wk_alive)
> > > -           msleep(10);
> > > +           usleep_range(10000, 11000);
>
> The whole approach here seems to be fragile. It should rather use completion
> mechanism instead of infinite loop.
>
> > >     while (!rtw_cbuf_empty(pevtpriv->c2h_queue)) {
> > >             void *c2h = rtw_cbuf_pop(pevtpriv->c2h_queue);
> >
> > Shouldn't we be using fsleep() now instead of replacing msleep() with
> > usleep_range() when the time is sub 20ms?
> >
> > I feel like I read Andy Shevchenko mention this elsewhere.
>
> Yep.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Re: [PATCH] staging: rtl8723bs: replace msleep with usleep_range
Posted by Greg KH 1 month, 2 weeks ago
On Sat, Feb 14, 2026 at 09:23:45PM +0200, Ofek Almog wrote:
> (Resending as plain text, apologies if you received a previous HTML version)
> 
> Hi Andy, Ethan,
> 
> Thank you both for the review.
> 
> Regarding the fsleep() suggestion - understood, that clearly seems
> like the right helper to use here.
> 
> Regarding the completion mechanism logic - I completely agree that the
> current infinite loop polling is fragile. However, as this is my very
> first contribution and I do not have the hardware to verify logic
> changes, I am hesitant to refactor the synchronization mechanism
> blindly.
> 
> Would you prefer I send a v2 that strictly switches to fsleep() as an
> incremental improvement over msleep(), or should I drop this patch
> altogether until someone with the hardware can refactor the logic
> properly?

Please drop it, see the archives for why (you need someone with the
hardare and specs to figure out what needs to be done.)

thanks,

greg k-h