[PATCH v2] ufs: change the way to complete fDeviceInit

[PATCH v2] ufs: change the way to complete fDeviceInit

[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.

v1 -> v2: switch the method to get time from jiffies to ktime

Tested-by: Kiwoong Kim <kwmad.kim@samsung.com>
Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 092480a..11d61e5 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -72,6 +72,9 @@
 /* Default value of wait time before gating device ref clock */
 #define UFSHCD_REF_CLK_GATING_WAIT_US 0xFF /* microsecs */
 
+/* Polling time to wait for fDeviceInit  */
+#define FDEVICEINIT_COMPL_TIMEOUT 1500 /* millisecs */
+
 #define ufshcd_toggle_vreg(_dev, _vreg, _on)				\
 	({                                                              \
 		int _ret;                                               \
@@ -4148,9 +4151,10 @@ EXPORT_SYMBOL_GPL(ufshcd_config_pwr_mode);
  */
 static int ufshcd_complete_dev_init(struct ufs_hba *hba)
 {
-	int i;
 	int err;
 	bool flag_res = true;
+	ktime_t start;
+	s64 time;
 
 	err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG,
 		QUERY_FLAG_IDN_FDEVICEINIT, 0, NULL);
@@ -4161,20 +4165,27 @@ 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 */
+	start = ktime_get();
+	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);
+		time = ktime_to_ms(ktime_sub(ktime_get(), start));
+	} while (time < FDEVICEINIT_COMPL_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

RE: [PATCH v2] ufs: change the way to complete fDeviceInit

[Avri Altman]

> +       ktime_t start;
> +       s64 time;
Can this be done with less variables?  e.g is this working?
ktime_t timeout;

> 
>         err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG,
>                 QUERY_FLAG_IDN_FDEVICEINIT, 0, NULL);
> @@ -4161,20 +4165,27 @@ 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 */
> +       start = ktime_get();
timeout = ktime_add_ms(ktime_get(), FDEVICEINIT_COMPL_TIMEOUT);

> +       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);
> +               time = ktime_to_ms(ktime_sub(ktime_get(), start));
> +       } while (time < FDEVICEINIT_COMPL_TIMEOUT);
while (ktime_before(ktime_get(), timeout));



Thanks,
Avri

RE: [PATCH v2] ufs: change the way to complete fDeviceInit

[Kiwoong Kim]

> 
> > +       ktime_t start;
> > +       s64 time;
> Can this be done with less variables?  e.g is this working?
> ktime_t timeout; 
I checked it worked. Nevertheless, I'll replace with your style.

Thanks.
Kiwoong Kim
> 
> >
> >         err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG,
> >                 QUERY_FLAG_IDN_FDEVICEINIT, 0, NULL); @@ -4161,20
> > +4165,27 @@ 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 */
> > +       start = ktime_get();
> timeout = ktime_add_ms(ktime_get(), FDEVICEINIT_COMPL_TIMEOUT);
> 
> > +       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);
> > +               time = ktime_to_ms(ktime_sub(ktime_get(), start));
> > +       } while (time < FDEVICEINIT_COMPL_TIMEOUT);
> while (ktime_before(ktime_get(), timeout));
> 
> 
> 
> Thanks,
> Avri