Hi Bart,
On Mon, 2020-03-16 at 09:23 -0700, Bart Van Assche wrote:
> On 3/16/20 1:52 AM, Stanley Chu wrote:
> > +void ufshcd_wait_us(unsigned long us, unsigned long tolerance, bool can_sleep)
> > +{
> > + if (!us)
> > + return;
> > +
> > + if (us < 10 || !can_sleep)
> > + udelay(us);
> > + else
> > + usleep_range(us, us + tolerance);
> > +}
> > +EXPORT_SYMBOL_GPL(ufshcd_wait_us);
>
> I don't like this function because I think it makes the UFS code harder
> to read instead of easier. The 'can_sleep' argument is only set by one
> caller which I think is a strong argument to remove that argument again
> and to move the code that depends on that argument from the above
> function into the caller. Additionally, it is not possible to comprehend
> what a ufshcd_wait_us() call does without looking up the function
> definition to see what the meaning of the third argument is.
>
> Please drop this patch.
Thanks for your review and comments.
If the problem is the third argument 'can_sleep' which makes the code
not be easily comprehensible, how about just removing 'can_sleep' from
this function and keeping left parts because this function provides good
flexibility to users to choose udelay or usleep_range according to the
'us' argument?
Thanks,
Stanley Chu
>
> Thanks,
>
> Bart.
>
