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@xxxxxxxxx wrote:
> > > > From: Yalagada Pavan Kumar <pavankumaryalagada@xxxxxxxxx>
> > > > 
> > > > 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
>