* [PATCH] staging: greybus: Use fsleep() for delay
@ 2026-10-22 11:02 pavankumaryalagada
2026-10-22 11:28 ` Johan Hovold
2026-10-22 13:28 ` Alex Elder
0 siblings, 2 replies; 3+ messages in thread
From: pavankumaryalagada @ 2026-10-22 11:02 UTC (permalink / raw)
To: greybus-dev, outreachy
Cc: pure.logic, johan, elder, gregkh, linux-staging, linux-kernel,
Yalagada Pavan Kumar, Alex Elder
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 selects a suitable
delay method based on the requested time. This avoids the need
to choose the delay function and convert the time manually.
Suggested-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
---
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: greybus: Use fsleep() for delay
2026-10-22 11:02 [PATCH] staging: greybus: Use fsleep() for delay pavankumaryalagada
@ 2026-10-22 11:28 ` Johan Hovold
2026-10-22 13:28 ` Alex Elder
1 sibling, 0 replies; 3+ messages in thread
From: Johan Hovold @ 2026-10-22 11:28 UTC (permalink / raw)
To: pavankumaryalagada
Cc: greybus-dev, outreachy, pure.logic, elder, gregkh, linux-staging,
linux-kernel, Alex Elder
On Thu, Oct 22, 2026 at 04:32:38PM +0530, 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().
Please include the module in Subject since this is not a subsystem wide
change:
staging: greybus: loopback: ...
> Replace both functions with fsleep(), which selects a suitable
> delay method based on the requested time. This avoids the need
> to choose the delay function and convert the time manually.
It probably doesn't matter here, but I think you should mentioned that
fsleep() will likely make short sleeps longer (25% instead of 100 us
slack).
Johan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: greybus: Use fsleep() for delay
2026-10-22 11:02 [PATCH] staging: greybus: Use fsleep() for delay pavankumaryalagada
2026-10-22 11:28 ` Johan Hovold
@ 2026-10-22 13:28 ` Alex Elder
1 sibling, 0 replies; 3+ messages in thread
From: Alex Elder @ 2026-10-22 13:28 UTC (permalink / raw)
To: pavankumaryalagada, greybus-dev, outreachy
Cc: pure.logic, johan, elder, gregkh, linux-staging, linux-kernel
On 10/22/26 6:02 AM, 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 selects a suitable
> delay method based on the requested time. This avoids the need
> to choose the delay function and convert the time manually.
>
> Suggested-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
Your description above is pretty good, but I agree with
Johan's comment. In fact, you should think about and
understand exactly how that behavior changes with the
switch to using fsleep().
- When would the delay be different from before?
- When it differs--by how much does it differ?
- Is this amount significant?
And the third question gets at exactly what Johan was
saying should be included in the patch description.
We want to know not just the mechanics of the change,
but at a higher level, what is its effect?
You don't need to provide a super high level of detail
in this particular change, but you should demonstrate
somehow that you've thought about these things.
Please submit a new version of the patch and reword
the description a bit to concisely capture why it
makes sense to make the change, and why you believe
that the change in behavior is likely harmless.
Thanks.
-Alex
> ---
> 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);
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-22 13:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-22 11:02 [PATCH] staging: greybus: Use fsleep() for delay pavankumaryalagada
2026-10-22 11:28 ` Johan Hovold
2026-10-22 13:28 ` Alex Elder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®