diff mbox series

[RESEND] scsi: ufs: Fix hynix ufs bug with quirk on hi36xx SoC

Message ID 1538434698-1042-1-git-send-email-john.stultz@linaro.org (mailing list archive)
State Changes Requested
Headers show
Series [RESEND] scsi: ufs: Fix hynix ufs bug with quirk on hi36xx SoC | expand

Commit Message

John Stultz Oct. 1, 2018, 10:58 p.m. UTC
From: Wei Li <liwei213@huawei.com>

Hynix ufs has deviations on hi36xx platform which will result in
ufs bursts transfer failures.

To fix the problem, the Hynix device must set the register
VS_DebugSaveConfigTime to 0x10, which will set time reference
for SaveConfigTime is 250 ns. The time reference for SaveConfigTime
is 40 ns by default.

This patch is necessary to boot on HiKey960 boards that use
Hynix UFS chips.

Cc: Vinayak Holikatti <vinholikatti@gmail.com>
Cc: "James E.J. Bottomley" <jejb@linux.vnet.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Wei Li <liwei213@huawei.com>
Signed-off-by: Dmitry Shmidt <dimitrysh@google.com>
[jstultz: Forward ported from older code, slight tweak to commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/scsi/ufs/ufs-hisi.c   | 9 +++++++++
 drivers/scsi/ufs/ufs_quirks.h | 6 ++++++
 drivers/scsi/ufs/ufshcd.c     | 2 ++
 3 files changed, 17 insertions(+)

Comments

Martin K. Petersen Oct. 16, 2018, 3:41 a.m. UTC | #1
John,

> Hynix ufs has deviations on hi36xx platform which will result in ufs
> bursts transfer failures.

Is this specific to the particular implementation on hi36xx or all SK
Hynix implementations?

> +	UFS_FIX(UFS_VENDOR_SKHYNIX, UFS_ANY_MODEL,
> +		UFS_DEVICE_QUIRK_HOST_VS_DEBUGSAVECONFIGTIME),

I am not so keen on matching on all current and future models.
John Stultz Oct. 16, 2018, 10:48 p.m. UTC | #2
On Mon, Oct 15, 2018 at 8:41 PM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
> John,
>
>> Hynix ufs has deviations on hi36xx platform which will result in ufs
>> bursts transfer failures.
>
> Is this specific to the particular implementation on hi36xx or all SK
> Hynix implementations?

I'd have to defer to Wei Li on that question.

Wei Li?


>> +     UFS_FIX(UFS_VENDOR_SKHYNIX, UFS_ANY_MODEL,
>> +             UFS_DEVICE_QUIRK_HOST_VS_DEBUGSAVECONFIGTIME),
>
> I am not so keen on matching on all current and future models.

I'll see if I can narrow down the affected model version on the board
I have. Hopefully it doesn't vary too much.

Really appreciate the review and the feedback!

thanks
-john
John Stultz Oct. 16, 2018, 11:50 p.m. UTC | #3
On Tue, Oct 16, 2018 at 3:48 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Mon, Oct 15, 2018 at 8:41 PM, Martin K. Petersen
> <martin.petersen@oracle.com> wrote:
>>
>> John,
>>
>>> Hynix ufs has deviations on hi36xx platform which will result in ufs
>>> bursts transfer failures.
>>
>> Is this specific to the particular implementation on hi36xx or all SK
>> Hynix implementations?
>
> I'd have to defer to Wei Li on that question.
>
> Wei Li?

Just to short-cut things a bit, as this is probably an obvious
followup: if it is hi36xx specific, how would you suggest a such a
targeted quirk be added?  I'm not sure I see how to get to the
ufs_dev_desc from the ufs-hisi.c code.

thanks
-john
John Stultz Oct. 22, 2018, 5:21 p.m. UTC | #4
On Tue, Oct 16, 2018 at 4:50 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Oct 16, 2018 at 3:48 PM, John Stultz <john.stultz@linaro.org> wrote:
>> On Mon, Oct 15, 2018 at 8:41 PM, Martin K. Petersen
>> <martin.petersen@oracle.com> wrote:
>>>
>>> John,
>>>
>>>> Hynix ufs has deviations on hi36xx platform which will result in ufs
>>>> bursts transfer failures.
>>>
>>> Is this specific to the particular implementation on hi36xx or all SK
>>> Hynix implementations?
>>
>> I'd have to defer to Wei Li on that question.

So I followed up with Wei Li and it seems its something specific to
hi36xx that the specific hynix chip brings out, which most other chips
don't seem to trigger.


> Just to short-cut things a bit, as this is probably an obvious
> followup: if it is hi36xx specific, how would you suggest a such a
> targeted quirk be added?  I'm not sure I see how to get to the
> ufs_dev_desc from the ufs-hisi.c code.

So yea, I've already reworked the quirk to be specific to the one
model of hynix chip, but I'm still a bit confused on best practices
for handling these quirks on the controller side. If there's a pointer
to similar controller side quirks that are device specific, I'd be
glad to rework the patch in that fashion. Otherwise I'll just try to
figure out how to get to the ufs_dev_desc from ufs-hisi.c

thanks
-john
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c
index 46df707..452e19f 100644
--- a/drivers/scsi/ufs/ufs-hisi.c
+++ b/drivers/scsi/ufs/ufs-hisi.c
@@ -20,6 +20,7 @@ 
 #include "unipro.h"
 #include "ufs-hisi.h"
 #include "ufshci.h"
+#include "ufs_quirks.h"
 
 static int ufs_hisi_check_hibern8(struct ufs_hba *hba)
 {
@@ -390,6 +391,14 @@  static void ufs_hisi_set_dev_cap(struct ufs_hisi_dev_params *hisi_param)
 
 static void ufs_hisi_pwr_change_pre_change(struct ufs_hba *hba)
 {
+	if (hba->dev_quirks & UFS_DEVICE_QUIRK_HOST_VS_DEBUGSAVECONFIGTIME) {
+		pr_info("ufs flash device must set VS_DebugSaveConfigTime 0x10\n");
+		/* VS_DebugSaveConfigTime */
+		ufshcd_dme_set(hba, UIC_ARG_MIB(0xD0A0), 0x10);
+		/* sync length */
+		ufshcd_dme_set(hba, UIC_ARG_MIB(0x1556), 0x48);
+	}
+
 	/* update */
 	ufshcd_dme_set(hba, UIC_ARG_MIB(0x15A8), 0x1);
 	/* PA_TxSkip */
diff --git a/drivers/scsi/ufs/ufs_quirks.h b/drivers/scsi/ufs/ufs_quirks.h
index 71f73d1..5d2dfdb 100644
--- a/drivers/scsi/ufs/ufs_quirks.h
+++ b/drivers/scsi/ufs/ufs_quirks.h
@@ -131,4 +131,10 @@  struct ufs_dev_fix {
  */
 #define UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME	(1 << 8)
 
+/*
+ * Some UFS devices require VS_DebugSaveConfigTime is 0x10,
+ * enabling this quirk ensure this.
+ */
+#define UFS_DEVICE_QUIRK_HOST_VS_DEBUGSAVECONFIGTIME	(1 << 9)
+
 #endif /* UFS_QUIRKS_H_ */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c55f38e..a67f298 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -230,6 +230,8 @@  static struct ufs_dev_fix ufs_fixups[] = {
 	UFS_FIX(UFS_VENDOR_SKHYNIX, UFS_ANY_MODEL, UFS_DEVICE_NO_VCCQ),
 	UFS_FIX(UFS_VENDOR_SKHYNIX, UFS_ANY_MODEL,
 		UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME),
+	UFS_FIX(UFS_VENDOR_SKHYNIX, UFS_ANY_MODEL,
+		UFS_DEVICE_QUIRK_HOST_VS_DEBUGSAVECONFIGTIME),
 
 	END_FIX
 };