* [PATCH v1] ufs: change the way to complete fDeviceInit
[not found] <CGME20200807070119epcas2p198639f0b065c97baf2b9bae231ec137b@epcas2p1.samsung.com>
@ 2020-08-07 6:52 ` Kiwoong Kim
2020-08-09 7:52 ` Avri Altman
0 siblings, 1 reply; 3+ messages in thread
From: Kiwoong Kim @ 2020-08-07 6:52 UTC (permalink / raw)
To: linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen,
beanhuo, asutoshd, cang, bvanassche, grant.jung, sc.suh,
hy50.seo, sh425.lee
Cc: Kiwoong Kim
Currently, UFS driver checks if fDeviceInit
is cleared at several times, not period. This patch
is to wait its completion with the period, not retrying.
Many device vendors usually provides the specification on
it with just period, not a combination of a number of retrying
and period. So it could be proper to regard to the information
coming from device vendors.
Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
drivers/scsi/ufs/ufshcd.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 092480a..c508931 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4148,7 +4148,8 @@ EXPORT_SYMBOL_GPL(ufshcd_config_pwr_mode);
*/
static int ufshcd_complete_dev_init(struct ufs_hba *hba)
{
- int i;
+ u32 dev_init_compl_in_ms = 1500;
+ unsigned long timeout;
int err;
bool flag_res = true;
@@ -4161,20 +4162,26 @@ static int ufshcd_complete_dev_init(struct ufs_hba *hba)
goto out;
}
- /* poll for max. 1000 iterations for fDeviceInit flag to clear */
- for (i = 0; i < 1000 && !err && flag_res; i++)
- err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG,
- QUERY_FLAG_IDN_FDEVICEINIT, 0, &flag_res);
+ /* Poll fDeviceInit flag to be cleared */
+ timeout = jiffies + msecs_to_jiffies(dev_init_compl_in_ms);
+ do {
+ err = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_READ_FLAG,
+ QUERY_FLAG_IDN_FDEVICEINIT, 0, &flag_res);
+ if (!flag_res)
+ break;
+ usleep_range(5000, 10000);
+ } while (time_before(jiffies, timeout));
- if (err)
+ if (err) {
dev_err(hba->dev,
- "%s reading fDeviceInit flag failed with error %d\n",
- __func__, err);
- else if (flag_res)
+ "%s reading fDeviceInit flag failed with error %d\n",
+ __func__, err);
+ } else if (flag_res) {
dev_err(hba->dev,
- "%s fDeviceInit was not cleared by the device\n",
- __func__);
-
+ "%s fDeviceInit was not cleared by the device\n",
+ __func__);
+ err = -EBUSY;
+ }
out:
return err;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: [PATCH v1] ufs: change the way to complete fDeviceInit
2020-08-07 6:52 ` [PATCH v1] ufs: change the way to complete fDeviceInit Kiwoong Kim
@ 2020-08-09 7:52 ` Avri Altman
2020-08-10 6:05 ` Kiwoong Kim
0 siblings, 1 reply; 3+ messages in thread
From: Avri Altman @ 2020-08-09 7:52 UTC (permalink / raw)
To: Kiwoong Kim, linux-scsi, alim.akhtar, jejb, martin.petersen,
beanhuo, asutoshd, cang, bvanassche, grant.jung, sc.suh,
hy50.seo, sh425.lee
>
> Currently, UFS driver checks if fDeviceInit
> is cleared at several times, not period. This patch
> is to wait its completion with the period, not retrying.
> Many device vendors usually provides the specification on
> it with just period, not a combination of a number of retrying
> and period. So it could be proper to regard to the information
> coming from device vendors.
>
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 31 +++++++++++++++++++------------
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 092480a..c508931 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -4148,7 +4148,8 @@ EXPORT_SYMBOL_GPL(ufshcd_config_pwr_mode);
> */
> static int ufshcd_complete_dev_init(struct ufs_hba *hba)
> {
> - int i;
> + u32 dev_init_compl_in_ms = 1500;
Please make the threshold a define. Otherwise, your code looks fine.
Recently we started to introduce the use of ktime convention and operators for such use cases.
Would you consider using it as well?
Thanks,
Avri
> + unsigned long timeout;
> int err;
> bool flag_res = true;
>
> @@ -4161,20 +4162,26 @@ static int ufshcd_complete_dev_init(struct ufs_hba
> *hba)
> goto out;
> }
>
> - /* poll for max. 1000 iterations for fDeviceInit flag to clear */
> - for (i = 0; i < 1000 && !err && flag_res; i++)
> - err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_READ_FLAG,
> - QUERY_FLAG_IDN_FDEVICEINIT, 0, &flag_res);
> + /* Poll fDeviceInit flag to be cleared */
> + timeout = jiffies + msecs_to_jiffies(dev_init_compl_in_ms);
> + do {
> + err = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_READ_FLAG,
> + QUERY_FLAG_IDN_FDEVICEINIT, 0, &flag_res);
> + if (!flag_res)
> + break;
> + usleep_range(5000, 10000);
> + } while (time_before(jiffies, timeout));
>
> - if (err)
> + if (err) {
> dev_err(hba->dev,
> - "%s reading fDeviceInit flag failed with error %d\n",
> - __func__, err);
> - else if (flag_res)
> + "%s reading fDeviceInit flag failed with error %d\n",
> + __func__, err);
> + } else if (flag_res) {
> dev_err(hba->dev,
> - "%s fDeviceInit was not cleared by the device\n",
> - __func__);
> -
> + "%s fDeviceInit was not cleared by the device\n",
> + __func__);
> + err = -EBUSY;
> + }
> out:
> return err;
> }
> --
> 2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH v1] ufs: change the way to complete fDeviceInit
2020-08-09 7:52 ` Avri Altman
@ 2020-08-10 6:05 ` Kiwoong Kim
0 siblings, 0 replies; 3+ messages in thread
From: Kiwoong Kim @ 2020-08-10 6:05 UTC (permalink / raw)
To: 'Avri Altman',
linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo,
asutoshd, cang, bvanassche, grant.jung, sc.suh, hy50.seo,
sh425.lee
> > Currently, UFS driver checks if fDeviceInit is cleared at several
> > times, not period. This patch is to wait its completion with the
> > period, not retrying.
> > Many device vendors usually provides the specification on it with just
> > period, not a combination of a number of retrying and period. So it
> > could be proper to regard to the information coming from device
> > vendors.
> >
> > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> > ---
> > drivers/scsi/ufs/ufshcd.c | 31 +++++++++++++++++++------------
> > 1 file changed, 19 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 092480a..c508931 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -4148,7 +4148,8 @@ EXPORT_SYMBOL_GPL(ufshcd_config_pwr_mode);
> > */
> > static int ufshcd_complete_dev_init(struct ufs_hba *hba) {
> > - int i;
> > + u32 dev_init_compl_in_ms = 1500;
> Please make the threshold a define. Otherwise, your code looks fine.
> Recently we started to introduce the use of ktime convention and operators
> for such use cases.
> Would you consider using it as well?
>
> Thanks,
> Avri
Got it
Thanks.
Kiwoong Kim
>
> > + unsigned long timeout;
> > int err;
> > bool flag_res = true;
> >
> > @@ -4161,20 +4162,26 @@ static int ufshcd_complete_dev_init(struct
> > ufs_hba
> > *hba)
> > goto out;
> > }
> >
> > - /* poll for max. 1000 iterations for fDeviceInit flag to clear */
> > - for (i = 0; i < 1000 && !err && flag_res; i++)
> > - err = ufshcd_query_flag_retry(hba,
> > UPIU_QUERY_OPCODE_READ_FLAG,
> > - QUERY_FLAG_IDN_FDEVICEINIT, 0, &flag_res);
> > + /* Poll fDeviceInit flag to be cleared */
> > + timeout = jiffies + msecs_to_jiffies(dev_init_compl_in_ms);
> > + do {
> > + err = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_READ_FLAG,
> > + QUERY_FLAG_IDN_FDEVICEINIT, 0, &flag_res);
> > + if (!flag_res)
> > + break;
> > + usleep_range(5000, 10000);
> > + } while (time_before(jiffies, timeout));
> >
> > - if (err)
> > + if (err) {
> > dev_err(hba->dev,
> > - "%s reading fDeviceInit flag failed with error %d\n",
> > - __func__, err);
> > - else if (flag_res)
> > + "%s reading fDeviceInit flag failed with
> error %d\n",
> > + __func__, err);
> > + } else if (flag_res) {
> > dev_err(hba->dev,
> > - "%s fDeviceInit was not cleared by the device\n",
> > - __func__);
> > -
> > + "%s fDeviceInit was not cleared by the
> device\n",
> > + __func__);
> > + err = -EBUSY;
> > + }
> > out:
> > return err;
> > }
> > --
> > 2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-10 6:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20200807070119epcas2p198639f0b065c97baf2b9bae231ec137b@epcas2p1.samsung.com>
2020-08-07 6:52 ` [PATCH v1] ufs: change the way to complete fDeviceInit Kiwoong Kim
2020-08-09 7:52 ` Avri Altman
2020-08-10 6:05 ` Kiwoong Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).