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@riscs... --- 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);
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@riscs...
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);elsemsleep(us_wait / 1000);}
if (us_wait)fsleep(us_wait);gb_pm_runtime_put_autosuspend(bundle);
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
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
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
-Alexthanks.
greg k-h
-
Alex Elder -
Greg KH -
pavankumaryalagadaï¼ gmail.com -
Yalagada Pavan Kumar