| Message ID | 20240919112442.48491-1-avri.altman@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: ufs: Do not open code read_poll_timeout | expand |
On 9/19/24 4:24 AM, Avri Altman wrote: > ufshcd_wait_for_register practically does just that - replace with > read_poll_timeout. > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > --- > drivers/ufs/core/ufshcd.c | 22 ++++++---------------- > 1 file changed, 6 insertions(+), 16 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 8ea5a82503a9..e9d06fab5f45 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -739,25 +739,15 @@ EXPORT_SYMBOL_GPL(ufshcd_delay_us); > * Return: -ETIMEDOUT on error, zero on success. > */ > static int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask, > - u32 val, unsigned long interval_us, > - unsigned long timeout_ms) > + u32 val, unsigned long interval_us, > + unsigned long timeout_ms) > { > - int err = 0; > - unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms); > - > - /* ignore bits that we don't intend to wait on */ > - val = val & mask; > + u32 v; > > - while ((ufshcd_readl(hba, reg) & mask) != val) { > - usleep_range(interval_us, interval_us + 50); > - if (time_after(jiffies, timeout)) { > - if ((ufshcd_readl(hba, reg) & mask) != val) > - err = -ETIMEDOUT; > - break; > - } > - } > + val &= mask; /* ignore bits that we don't intend to wait on */ > > - return err; > + return read_poll_timeout(ufshcd_readl, v, (v & mask) == val, > + interval_us, timeout_ms * 1000, false, hba, reg); > } > > /** Has it been considered to remove the ufshcd_wait_for_register() function and to use read_poll_timeout() directly in the ufshcd_wait_for_register() callers? The above patch makes ufshcd_wait_for_register() so short that it's probably better to remove this function entirely. Thanks, Bart.
> Has it been considered to remove the ufshcd_wait_for_register() function and > to use read_poll_timeout() directly in the > ufshcd_wait_for_register() callers? The above patch makes > ufshcd_wait_for_register() so short that it's probably better to remove this > function entirely. Yes - I thought about it. I think that the wait_for_register makes it much clearer what the function actually does, And for that reason, only, it worth keeping the function name. However, if it’s a must I can remove it as you suggested. Thanks, Avri > > Thanks, > > Bart.
On 9/19/24 4:24 AM, Avri Altman wrote: > ufshcd_wait_for_register practically does just that - replace with > read_poll_timeout. Reviewed-by: Bart Van Assche <bvanassche@acm.org>
On 9/20/24 12:00 PM, Avri Altman wrote: > I think that the wait_for_register makes it much clearer what the function actually does, > And for that reason, only, it worth keeping the function name. That makes sense to me. Thanks, Bart.
Avri, > ufshcd_wait_for_register practically does just that - replace with > read_poll_timeout. Applied to 6.13/scsi-staging, thanks!
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 8ea5a82503a9..e9d06fab5f45 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -739,25 +739,15 @@ EXPORT_SYMBOL_GPL(ufshcd_delay_us); * Return: -ETIMEDOUT on error, zero on success. */ static int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask, - u32 val, unsigned long interval_us, - unsigned long timeout_ms) + u32 val, unsigned long interval_us, + unsigned long timeout_ms) { - int err = 0; - unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms); - - /* ignore bits that we don't intend to wait on */ - val = val & mask; + u32 v; - while ((ufshcd_readl(hba, reg) & mask) != val) { - usleep_range(interval_us, interval_us + 50); - if (time_after(jiffies, timeout)) { - if ((ufshcd_readl(hba, reg) & mask) != val) - err = -ETIMEDOUT; - break; - } - } + val &= mask; /* ignore bits that we don't intend to wait on */ - return err; + return read_poll_timeout(ufshcd_readl, v, (v & mask) == val, + interval_us, timeout_ms * 1000, false, hba, reg); } /**
ufshcd_wait_for_register practically does just that - replace with read_poll_timeout. Signed-off-by: Avri Altman <avri.altman@wdc.com> --- drivers/ufs/core/ufshcd.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-)