2020-07-29 05:19:20

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v1 0/2] scsi: ufs: Introduce and apply DELAY_AFTER_LPM device quirk

Hi,
This patchset introduces and applies DELAY_AFTER_LPM device quirk in MediaTek platforms.

Stanley Chu (2):
scsi: ufs: Introduce device quirk "DELAY_AFTER_LPM"
scsi: ufs-mediatek: Apply DELAY_AFTER_LPM quirk to Micron devices

drivers/scsi/ufs/ufs-mediatek.c | 2 ++
drivers/scsi/ufs/ufs_quirks.h | 7 +++++++
drivers/scsi/ufs/ufshcd.c | 11 +++++++++++
3 files changed, 20 insertions(+)

--
2.18.0


2020-07-29 05:19:53

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v1 2/2] scsi: ufs-mediatek: Apply DELAY_AFTER_LPM quirk to Micron devices

Micron UFS devices require DELAY_AFTER_LPM device quirk in
MediaTek platforms.

Signed-off-by: Andy Teng <[email protected]>
Signed-off-by: Peter Wang <[email protected]>
Signed-off-by: Stanley Chu <[email protected]>
---
drivers/scsi/ufs/ufs-mediatek.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 31af8b3d2b53..7ff2682f481c 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -36,6 +36,8 @@
ufs_mtk_smc(UFS_MTK_SIP_DEVICE_RESET, high, res)

static struct ufs_dev_fix ufs_mtk_dev_fixups[] = {
+ UFS_FIX(UFS_VENDOR_MICRON, UFS_ANY_MODEL,
+ UFS_DEVICE_QUIRK_DELAY_AFTER_LPM),
UFS_FIX(UFS_VENDOR_SKHYNIX, "H9HQ21AFAMZDAR",
UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES),
END_FIX
--
2.18.0

2020-07-29 05:20:04

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v1 1/2] scsi: ufs: Introduce device quirk "DELAY_AFTER_LPM"

Some UFS devices require delay after VCC power rail is turned-off.
Introduce a device quirk "DELAY_AFTER_LPM" to add 5ms delays after
VCC power-off during suspend flow.

Signed-off-by: Andy Teng <[email protected]>
Signed-off-by: Peter Wang <[email protected]>
Signed-off-by: Stanley Chu <[email protected]>
---
drivers/scsi/ufs/ufs_quirks.h | 7 +++++++
drivers/scsi/ufs/ufshcd.c | 11 +++++++++++
2 files changed, 18 insertions(+)

diff --git a/drivers/scsi/ufs/ufs_quirks.h b/drivers/scsi/ufs/ufs_quirks.h
index 2a0041493e30..07f559ac5883 100644
--- a/drivers/scsi/ufs/ufs_quirks.h
+++ b/drivers/scsi/ufs/ufs_quirks.h
@@ -109,4 +109,11 @@ struct ufs_dev_fix {
*/
#define UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES (1 << 10)

+/*
+ * Some UFS devices require delay after VCC power rail is turned-off.
+ * Enable this quirk to introduce 5ms delays after VCC power-off during
+ * suspend flow.
+ */
+#define UFS_DEVICE_QUIRK_DELAY_AFTER_LPM (1 << 11)
+
#endif /* UFS_QUIRKS_H_ */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index acba2271c5d3..63f4e2f75aa1 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8111,6 +8111,8 @@ static int ufshcd_link_state_transition(struct ufs_hba *hba,

static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
{
+ bool vcc_off = false;
+
/*
* It seems some UFS devices may keep drawing more than sleep current
* (atleast for 500us) from UFS rails (especially from VCCQ rail).
@@ -8139,13 +8141,22 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba) &&
!hba->dev_info.is_lu_power_on_wp) {
ufshcd_setup_vreg(hba, false);
+ vcc_off = true;
} else if (!ufshcd_is_ufs_dev_active(hba)) {
ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
+ vcc_off = true;
if (!ufshcd_is_link_active(hba)) {
ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq);
ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq2);
}
}
+
+ /*
+ * Some UFS devices require delay after VCC power rail is turned-off.
+ */
+ if (vcc_off && hba->vreg_info.vcc &&
+ hba->dev_quirks & UFS_DEVICE_QUIRK_DELAY_AFTER_LPM)
+ usleep_range(5000, 5100);
}

static int ufshcd_vreg_set_hpm(struct ufs_hba *hba)
--
2.18.0

2020-07-29 05:34:22

by Can Guo

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] scsi: ufs: Introduce device quirk "DELAY_AFTER_LPM"

Hi Stanley,

On 2020-07-29 13:18, Stanley Chu wrote:
> Some UFS devices require delay after VCC power rail is turned-off.
> Introduce a device quirk "DELAY_AFTER_LPM" to add 5ms delays after
> VCC power-off during suspend flow.
>

Just curious, I can understand if you want to add some delays before
turnning off VCC/VCCQ/VCCQ2, but what is the delay AFTER turnning
them off for? I mean the power has been cut by host from PMIC, how
can the delay benefit the UFS device?

Thanks,

Can Guo.

> Signed-off-by: Andy Teng <[email protected]>
> Signed-off-by: Peter Wang <[email protected]>
> Signed-off-by: Stanley Chu <[email protected]>
> ---
> drivers/scsi/ufs/ufs_quirks.h | 7 +++++++
> drivers/scsi/ufs/ufshcd.c | 11 +++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/drivers/scsi/ufs/ufs_quirks.h
> b/drivers/scsi/ufs/ufs_quirks.h
> index 2a0041493e30..07f559ac5883 100644
> --- a/drivers/scsi/ufs/ufs_quirks.h
> +++ b/drivers/scsi/ufs/ufs_quirks.h
> @@ -109,4 +109,11 @@ struct ufs_dev_fix {
> */
> #define UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES (1 << 10)
>
> +/*
> + * Some UFS devices require delay after VCC power rail is turned-off.
> + * Enable this quirk to introduce 5ms delays after VCC power-off
> during
> + * suspend flow.
> + */
> +#define UFS_DEVICE_QUIRK_DELAY_AFTER_LPM (1 << 11)
> +
> #endif /* UFS_QUIRKS_H_ */
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index acba2271c5d3..63f4e2f75aa1 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -8111,6 +8111,8 @@ static int ufshcd_link_state_transition(struct
> ufs_hba *hba,
>
> static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
> {
> + bool vcc_off = false;
> +
> /*
> * It seems some UFS devices may keep drawing more than sleep current
> * (atleast for 500us) from UFS rails (especially from VCCQ rail).
> @@ -8139,13 +8141,22 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba
> *hba)
> if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba) &&
> !hba->dev_info.is_lu_power_on_wp) {
> ufshcd_setup_vreg(hba, false);
> + vcc_off = true;
> } else if (!ufshcd_is_ufs_dev_active(hba)) {
> ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
> + vcc_off = true;
> if (!ufshcd_is_link_active(hba)) {
> ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq);
> ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq2);
> }
> }
> +
> + /*
> + * Some UFS devices require delay after VCC power rail is turned-off.
> + */
> + if (vcc_off && hba->vreg_info.vcc &&
> + hba->dev_quirks & UFS_DEVICE_QUIRK_DELAY_AFTER_LPM)
> + usleep_range(5000, 5100);
> }
>
> static int ufshcd_vreg_set_hpm(struct ufs_hba *hba)

2020-07-29 06:18:02

by Stanley Chu

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] scsi: ufs: Introduce device quirk "DELAY_AFTER_LPM"

Hi Can,

On Wed, 2020-07-29 at 13:31 +0800, Can Guo wrote:
> Hi Stanley,
>
> On 2020-07-29 13:18, Stanley Chu wrote:
> > Some UFS devices require delay after VCC power rail is turned-off.
> > Introduce a device quirk "DELAY_AFTER_LPM" to add 5ms delays after
> > VCC power-off during suspend flow.
> >
>
> Just curious, I can understand if you want to add some delays before
> turnning off VCC/VCCQ/VCCQ2, but what is the delay AFTER turnning
> them off for? I mean the power has been cut by host from PMIC, how
> can the delay benefit the UFS device?
>

The problem comes from both sides,
1. VCC power rail design by SoC vendors, and
2. Device strategy according to VCC changes

Take Micron UFS devices on MediaTek platform as example, our VCC drop
rate may be too slow and lead to incorrect resume strategy by device.
Without this delay, device may not resume successfully.

Thanks,
Stanley Chu


2020-07-29 07:34:31

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH v1 0/2] scsi: ufs: Introduce and apply DELAY_AFTER_LPM device quirk

Looks fine.
Thanks,
Avri


>
> Hi,
> This patchset introduces and applies DELAY_AFTER_LPM device quirk in
> MediaTek platforms.
>
> Stanley Chu (2):
> scsi: ufs: Introduce device quirk "DELAY_AFTER_LPM"
> scsi: ufs-mediatek: Apply DELAY_AFTER_LPM quirk to Micron devices
>
> drivers/scsi/ufs/ufs-mediatek.c | 2 ++
> drivers/scsi/ufs/ufs_quirks.h | 7 +++++++
> drivers/scsi/ufs/ufshcd.c | 11 +++++++++++
> 3 files changed, 20 insertions(+)
>
> --
> 2.18.0

2020-07-29 09:22:35

by Can Guo

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] scsi: ufs: Introduce device quirk "DELAY_AFTER_LPM"

On 2020-07-29 13:18, Stanley Chu wrote:
> Some UFS devices require delay after VCC power rail is turned-off.
> Introduce a device quirk "DELAY_AFTER_LPM" to add 5ms delays after
> VCC power-off during suspend flow.
>
> Signed-off-by: Andy Teng <[email protected]>
> Signed-off-by: Peter Wang <[email protected]>
> Signed-off-by: Stanley Chu <[email protected]>
> ---
> drivers/scsi/ufs/ufs_quirks.h | 7 +++++++
> drivers/scsi/ufs/ufshcd.c | 11 +++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/drivers/scsi/ufs/ufs_quirks.h
> b/drivers/scsi/ufs/ufs_quirks.h
> index 2a0041493e30..07f559ac5883 100644
> --- a/drivers/scsi/ufs/ufs_quirks.h
> +++ b/drivers/scsi/ufs/ufs_quirks.h
> @@ -109,4 +109,11 @@ struct ufs_dev_fix {
> */
> #define UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES (1 << 10)
>
> +/*
> + * Some UFS devices require delay after VCC power rail is turned-off.
> + * Enable this quirk to introduce 5ms delays after VCC power-off
> during
> + * suspend flow.
> + */
> +#define UFS_DEVICE_QUIRK_DELAY_AFTER_LPM (1 << 11)
> +
> #endif /* UFS_QUIRKS_H_ */
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index acba2271c5d3..63f4e2f75aa1 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -8111,6 +8111,8 @@ static int ufshcd_link_state_transition(struct
> ufs_hba *hba,
>
> static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
> {
> + bool vcc_off = false;
> +
> /*
> * It seems some UFS devices may keep drawing more than sleep current
> * (atleast for 500us) from UFS rails (especially from VCCQ rail).
> @@ -8139,13 +8141,22 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba
> *hba)
> if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba) &&
> !hba->dev_info.is_lu_power_on_wp) {
> ufshcd_setup_vreg(hba, false);
> + vcc_off = true;
> } else if (!ufshcd_is_ufs_dev_active(hba)) {
> ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
> + vcc_off = true;
> if (!ufshcd_is_link_active(hba)) {
> ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq);
> ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq2);
> }
> }
> +
> + /*
> + * Some UFS devices require delay after VCC power rail is turned-off.
> + */
> + if (vcc_off && hba->vreg_info.vcc &&
> + hba->dev_quirks & UFS_DEVICE_QUIRK_DELAY_AFTER_LPM)
> + usleep_range(5000, 5100);
> }
>
> static int ufshcd_vreg_set_hpm(struct ufs_hba *hba)

Reviewed-by: Can Guo <[email protected]>

2020-07-30 02:25:36

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] scsi: ufs: Introduce and apply DELAY_AFTER_LPM device quirk

On Wed, 29 Jul 2020 13:18:38 +0800, Stanley Chu wrote:

> This patchset introduces and applies DELAY_AFTER_LPM device quirk in MediaTek platforms.
>
> Stanley Chu (2):
> scsi: ufs: Introduce device quirk "DELAY_AFTER_LPM"
> scsi: ufs-mediatek: Apply DELAY_AFTER_LPM quirk to Micron devices
>
> drivers/scsi/ufs/ufs-mediatek.c | 2 ++
> drivers/scsi/ufs/ufs_quirks.h | 7 +++++++
> drivers/scsi/ufs/ufshcd.c | 11 +++++++++++
> 3 files changed, 20 insertions(+)

Applied to 5.9/scsi-queue, thanks!

[1/2] scsi: ufs: Introduce device quirk "DELAY_AFTER_LPM"
https://git.kernel.org/mkp/scsi/c/33166bebcd6d
[2/2] scsi: ufs-mediatek: Apply DELAY_AFTER_LPM quirk to Micron devices
https://git.kernel.org/mkp/scsi/c/4d4673745fe2

--
Martin K. Petersen Oracle Linux Engineering