[PATCH v2] staging: greybus: loopback: Use fsleep() for delay

[PATCH v2] staging: greybus: loopback: Use fsleep() for delay

[pavankumaryalagada]

From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>

In the loopback function, usleep_range() is used for short delays
and msleep() for longer delays. This requires choosing the delay
function based on the requested time and converting microseconds
to milliseconds when using msleep().

Replace both functions with fsleep(), which chooses a delay
delay method based on the requested time. For short delays,
fsleep() allows up to 25% extra time instead of the fixed 100
microseconds. For longer delays, the old code rounds the
requested time down to whole milliseconds, while fsleep() rounds
it up when using msleep().

These changes may affect time between loopback operations and
the reported performance.

Suggested-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
---
v2:
- Add more details about how fsleep() affects delays.
- Update the subject to include the loopback module.

v1: https://lore.kernel.org/outreachy/c482704b-903e-413d-ba7b-5bf31c0a6104@riscstar.com/T/#t
---
 drivers/staging/greybus/loopback.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index efc69c10cffe..2fe845eaef29 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -898,12 +898,8 @@ static int gb_loopback_fn(void *data)
 		gb->send_count++;
 		mutex_unlock(&gb->mutex);
 
-		if (us_wait) {
-			if (us_wait < 20000)
-				usleep_range(us_wait, us_wait + 100);
-			else
-				msleep(us_wait / 1000);
-		}
+		if (us_wait)
+			fsleep(us_wait);
 	}
 
 	gb_pm_runtime_put_autosuspend(bundle);
-- 
2.43.0

Re: [PATCH v2] staging: greybus: loopback: Use fsleep() for delay

[Alex Elder]

On 10/22/26 3:24 PM, pavankumaryalagada@gmail.com wrote:
> From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> 
> In the loopback function, usleep_range() is used for short delays
> and msleep() for longer delays. This requires choosing the delay
> function based on the requested time and converting microseconds
> to milliseconds when using msleep().
> 
> Replace both functions with fsleep(), which chooses a delay
> delay method based on the requested time. For short delays,

s/delay delay/delay/

I think "checkpatch.pl" would have caught this.

Maybe Greg can fix this up.


> fsleep() allows up to 25% extra time instead of the fixed 100
> microseconds. For longer delays, the old code rounds the
> requested time down to whole milliseconds, while fsleep() rounds
> it up when using msleep().
> 
> These changes may affect time between loopback operations and
> the reported performance.
> 
> Suggested-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>

Looks good to me.  Nice job.

Reviewed-by: Alex Elder <elder@riscstar.com>

> ---
> v2:
> - Add more details about how fsleep() affects delays.
> - Update the subject to include the loopback module.
> 
> v1: https://lore.kernel.org/outreachy/c482704b-903e-413d-ba7b-5bf31c0a6104@riscstar.com/T/#t
> ---
>   drivers/staging/greybus/loopback.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index efc69c10cffe..2fe845eaef29 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -898,12 +898,8 @@ static int gb_loopback_fn(void *data)
>   		gb->send_count++;
>   		mutex_unlock(&gb->mutex);
>   
> -		if (us_wait) {
> -			if (us_wait < 20000)
> -				usleep_range(us_wait, us_wait + 100);
> -			else
> -				msleep(us_wait / 1000);
> -		}
> +		if (us_wait)
> +			fsleep(us_wait);
>   	}
>   
>   	gb_pm_runtime_put_autosuspend(bundle);

Re: [PATCH v2] staging: greybus: loopback: Use fsleep() for delay

[Greg KH]

On Fri, Oct 23, 2026 at 07:48:54AM -0500, Alex Elder wrote:
> On 10/22/26 3:24 PM, pavankumaryalagada@gmail.com wrote:
> > From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > 
> > In the loopback function, usleep_range() is used for short delays
> > and msleep() for longer delays. This requires choosing the delay
> > function based on the requested time and converting microseconds
> > to milliseconds when using msleep().
> > 
> > Replace both functions with fsleep(), which chooses a delay
> > delay method based on the requested time. For short delays,
> 
> s/delay delay/delay/
> 
> I think "checkpatch.pl" would have caught this.
> 
> Maybe Greg can fix this up.

Nope, not for staging patches, this is for "learning" :)

thanks.

greg k-h

Re: [PATCH v2] staging: greybus: loopback: Use fsleep() for delay

[Alex Elder]

On 10/23/26 8:04 AM, Greg KH wrote:
> On Fri, Oct 23, 2026 at 07:48:54AM -0500, Alex Elder wrote:
>> On 10/22/26 3:24 PM, pavankumaryalagada@gmail.com wrote:
>>> From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
>>>
>>> In the loopback function, usleep_range() is used for short delays
>>> and msleep() for longer delays. This requires choosing the delay
>>> function based on the requested time and converting microseconds
>>> to milliseconds when using msleep().
>>>
>>> Replace both functions with fsleep(), which chooses a delay
>>> delay method based on the requested time. For short delays,
>>
>> s/delay delay/delay/
>>
>> I think "checkpatch.pl" would have caught this.
>>
>> Maybe Greg can fix this up.
> 
> Nope, not for staging patches, this is for "learning" :)

Sounds good.

Please provide version 3.  And given that, I'll add one more
suggestion.

You don't provide the *reason* for making this change in your
description.  You say that fsleep() does roughly the same thing
as the existing code.  But... why make the change?

The answer is roughly that fsleep() is a common or "standard"
function, and it's better to use those where available.  I can
think of at least three reasons:
- It's better to use a well-known common function where available,
   because reviewers will already be familiar with what it does
- A common function might get optimizations and improvements that
   your code would otherwise not benefit from
- It might (slightly) reduce code size

I am not suggesting you include all that, but you should
understand it.  My point is that you should be clear on
the reason for the suggested change in your patch header.

Even something like:
   Replace this block of code with a simple call to fsleep(),
   which does the same thing.
is enough to explain *why*.

Also, before sending *any* patch for review, please *always*
re-run checkpatch.pl so it can find trivial problems for you.

					-Alex

> 
> thanks.
> 
> greg k-h

Re: [PATCH v2] staging: greybus: loopback: Use fsleep() for delay

[Yalagada Pavan Kumar]

On Fri, Oct 23, 2026 at 08:18:04AM -0500, Alex Elder wrote:
> On 10/23/26 8:04 AM, Greg KH wrote:
> > On Fri, Oct 23, 2026 at 07:48:54AM -0500, Alex Elder wrote:
> > > On 10/22/26 3:24 PM, pavankumaryalagada@gmail.com wrote:
> > > > From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > > > 
> > > > In the loopback function, usleep_range() is used for short delays
> > > > and msleep() for longer delays. This requires choosing the delay
> > > > function based on the requested time and converting microseconds
> > > > to milliseconds when using msleep().
> > > > 
> > > > Replace both functions with fsleep(), which chooses a delay
> > > > delay method based on the requested time. For short delays,
> > > 
> > > s/delay delay/delay/
> > > 
> > > I think "checkpatch.pl" would have caught this.
> > > 
> > > Maybe Greg can fix this up.
> > 
> > Nope, not for staging patches, this is for "learning" :)
> 
> Sounds good.
> 
> Please provide version 3.  And given that, I'll add one more
> suggestion.

Yes, sure. I will send version 3.

> 
> You don't provide the *reason* for making this change in your
> description.  You say that fsleep() does roughly the same thing
> as the existing code.  But... why make the change?

Understood, I will make the reason for this change clearer in the
commit message.

> 
> The answer is roughly that fsleep() is a common or "standard"
> function, and it's better to use those where available.  I can
> think of at least three reasons:
> - It's better to use a well-known common function where available,
>   because reviewers will already be familiar with what it does
> - A common function might get optimizations and improvements that
>   your code would otherwise not benefit from
> - It might (slightly) reduce code size
> 
> I am not suggesting you include all that, but you should
> understand it.  My point is that you should be clear on
> the reason for the suggested change in your patch header.
> 
> Even something like:
>   Replace this block of code with a simple call to fsleep(),
>   which does the same thing.
> is enough to explain *why*.

Yes, I understand.

Thank you for explaining the reasons behind this change.
I will update the commit message accordingly.

> 
> Also, before sending *any* patch for review, please *always*
> re-run checkpatch.pl so it can find trivial problems for you.

I did run checkpatch.pl before sending v2, but it didn't catch
the duplicate word. I should have checked the commit message
more carefully myself. I will be more careful with future
patches.

Thanks for your guidance.

Pavan

> 
> 					-Alex
> 
> > 
> > thanks.
> > 
> > greg k-h
>