[PATCH v1 1/2] scsi: ufs: core: Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk

[PATCH v1 1/2] scsi: ufs: core: Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk

[Bao D. Nguyen]

After the ufs device vcc is turned off, all the ufs device
manufacturers require a period of power-off time before the
vcc can be turned on again. This requirement has been confirmed
with all the ufs device manufacturer's datasheets.

Remove the UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk in the ufs
core driver and implement a universal delay that is required by
all the ufs device manufacturers. In addition, remove the
support for this quirk in the platform drivers.

Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
 drivers/ufs/core/ufshcd.c       |  5 ++---
 drivers/ufs/host/ufs-mediatek.c | 11 ++++-------
 drivers/ufs/host/ufs-qcom.c     |  3 ---
 include/ufs/ufs_quirks.h        |  7 -------
 4 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2e1fa8c..45e509b 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9738,10 +9738,9 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
 	}
 
 	/*
-	 * Some UFS devices require delay after VCC power rail is turned-off.
+	 * All 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)
+	if (vcc_off && hba->vreg_info.vcc)
 		usleep_range(5000, 5100);
 }
 
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index f902ce0..5c204d1 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -40,8 +40,7 @@ static int  ufs_mtk_config_mcq(struct ufs_hba *hba, bool irq);
 static const struct ufs_dev_quirk ufs_mtk_dev_fixups[] = {
 	{ .wmanufacturerid = UFS_ANY_VENDOR,
 	  .model = UFS_ANY_MODEL,
-	  .quirk = UFS_DEVICE_QUIRK_DELAY_AFTER_LPM |
-		UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM },
+	  .quirk = UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM },
 	{ .wmanufacturerid = UFS_VENDOR_SKHYNIX,
 	  .model = "H9HQ21AFAMZDAR",
 	  .quirk = UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES },
@@ -1713,15 +1712,13 @@ static void ufs_mtk_fixup_dev_quirks(struct ufs_hba *hba)
 {
 	ufshcd_fixup_dev_quirks(hba, ufs_mtk_dev_fixups);
 
-	if (ufs_mtk_is_broken_vcc(hba) && hba->vreg_info.vcc &&
-	    (hba->dev_quirks & UFS_DEVICE_QUIRK_DELAY_AFTER_LPM)) {
+	if (ufs_mtk_is_broken_vcc(hba) && hba->vreg_info.vcc) {
 		hba->vreg_info.vcc->always_on = true;
 		/*
 		 * VCC will be kept always-on thus we don't
-		 * need any delay during regulator operations
+		 * need any delay before putting device's VCC in LPM mode.
 		 */
-		hba->dev_quirks &= ~(UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM |
-			UFS_DEVICE_QUIRK_DELAY_AFTER_LPM);
+		hba->dev_quirks &= ~UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM;
 	}
 
 	ufs_mtk_vreg_fix_vcc(hba);
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index d15f1a1..15a9ffc8 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1025,9 +1025,6 @@ static struct ufs_dev_quirk ufs_qcom_dev_fixups[] = {
 	{ .wmanufacturerid = UFS_VENDOR_SKHYNIX,
 	  .model = UFS_ANY_MODEL,
 	  .quirk = UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM },
-	{ .wmanufacturerid = UFS_VENDOR_TOSHIBA,
-	  .model = UFS_ANY_MODEL,
-	  .quirk = UFS_DEVICE_QUIRK_DELAY_AFTER_LPM },
 	{ .wmanufacturerid = UFS_VENDOR_WDC,
 	  .model = UFS_ANY_MODEL,
 	  .quirk = UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE },
diff --git a/include/ufs/ufs_quirks.h b/include/ufs/ufs_quirks.h
index f52de5e..1b26932 100644
--- a/include/ufs/ufs_quirks.h
+++ b/include/ufs/ufs_quirks.h
@@ -101,13 +101,6 @@ struct ufs_dev_quirk {
 #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)
-
-/*
  * Some ufs devices may need more time to be in hibern8 before exiting.
  * Enable this quirk to give it an additional 100us.
  */
-- 
2.7.4

Re: [PATCH v1 1/2] scsi: ufs: core: Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk

[Peter Wang (王信友)]

On Wed, 2025-10-01 at 13:57 -0700, Bao D. Nguyen wrote:
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 2e1fa8c..45e509b 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -9738,10 +9738,9 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba
> *hba)
>         }
> 
>         /*
> -        * Some UFS devices require delay after VCC power rail is
> turned-off.
> +        * All 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)
> +       if (vcc_off && hba->vreg_info.vcc)
>                 usleep_range(5000, 5100);
>  }
> 
> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-
> mediatek.c
> index f902ce0..5c204d1 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -40,8 +40,7 @@ static int  ufs_mtk_config_mcq(struct ufs_hba *hba,
> bool irq);
>  static const struct ufs_dev_quirk ufs_mtk_dev_fixups[] = {
>         { .wmanufacturerid = UFS_ANY_VENDOR,
>           .model = UFS_ANY_MODEL,
> -         .quirk = UFS_DEVICE_QUIRK_DELAY_AFTER_LPM |
> -               UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM },
> +         .quirk = UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM },
>         { .wmanufacturerid = UFS_VENDOR_SKHYNIX,
>           .model = "H9HQ21AFAMZDAR",
>           .quirk = UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES },
> @@ -1713,15 +1712,13 @@ static void ufs_mtk_fixup_dev_quirks(struct
> ufs_hba *hba)
>  {
>         ufshcd_fixup_dev_quirks(hba, ufs_mtk_dev_fixups);
> 
> -       if (ufs_mtk_is_broken_vcc(hba) && hba->vreg_info.vcc &&
> -           (hba->dev_quirks & UFS_DEVICE_QUIRK_DELAY_AFTER_LPM)) {
> +       if (ufs_mtk_is_broken_vcc(hba) && hba->vreg_info.vcc) {
> 

Hi Bao,

Adding a delay is not reasonable if we have decided to
keep VCC always on.

Thanks
Peter


>                 hba->vreg_info.vcc->always_on = true;
>                 /*
>                  * VCC will be kept always-on thus we don't
> -                * need any delay during regulator operations
> +                * need any delay before putting device's VCC in LPM
> mode.
>                  */
> -               hba->dev_quirks &=
> ~(UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM |
> -                       UFS_DEVICE_QUIRK_DELAY_AFTER_LPM);
> +               hba->dev_quirks &=
> ~UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM;
> 

Re: [PATCH v1 1/2] scsi: ufs: core: Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk

[Bao D. Nguyen]

On 10/2/2025 12:57 AM, Peter Wang (王信友) wrote:
> On Wed, 2025-10-01 at 13:57 -0700, Bao D. Nguyen wrote:
>>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 2e1fa8c..45e509b 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -9738,10 +9738,9 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba
>> *hba)
>>          }
>>
>>          /*
>> -        * Some UFS devices require delay after VCC power rail is
>> turned-off.
>> +        * All 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)
>> +       if (vcc_off && hba->vreg_info.vcc)
>>                  usleep_range(5000, 5100);
>>   }
>>
>> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-
>> mediatek.c
>> index f902ce0..5c204d1 100644
>> --- a/drivers/ufs/host/ufs-mediatek.c
>> +++ b/drivers/ufs/host/ufs-mediatek.c
>> @@ -40,8 +40,7 @@ static int  ufs_mtk_config_mcq(struct ufs_hba *hba,
>> bool irq);
>>   static const struct ufs_dev_quirk ufs_mtk_dev_fixups[] = {
>>          { .wmanufacturerid = UFS_ANY_VENDOR,
>>            .model = UFS_ANY_MODEL,
>> -         .quirk = UFS_DEVICE_QUIRK_DELAY_AFTER_LPM |
>> -               UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM },
>> +         .quirk = UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM },
>>          { .wmanufacturerid = UFS_VENDOR_SKHYNIX,
>>            .model = "H9HQ21AFAMZDAR",
>>            .quirk = UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES },
>> @@ -1713,15 +1712,13 @@ static void ufs_mtk_fixup_dev_quirks(struct
>> ufs_hba *hba)
>>   {
>>          ufshcd_fixup_dev_quirks(hba, ufs_mtk_dev_fixups);
>>
>> -       if (ufs_mtk_is_broken_vcc(hba) && hba->vreg_info.vcc &&
>> -           (hba->dev_quirks & UFS_DEVICE_QUIRK_DELAY_AFTER_LPM)) {
>> +       if (ufs_mtk_is_broken_vcc(hba) && hba->vreg_info.vcc) {
>>
> 
> Hi Bao,
> 
> Adding a delay is not reasonable if we have decided to
> keep VCC always on.

Hi Peter,

The current Mediatek platform driver applies this quirk to all ufs 
vendors which is consistent with what we would like to do in the 
Qualcomm platform driver per the vendor's requests.

I do see that, about 5 years ago, Mediatek merged a patch to keep the 
device vcc always on, probably to workaround some HW issues. Since this 
is a very old patch and the impact of this change on a broken hardware 
is minimal, I would like weight the benefit of cleaning up the ufs core 
driver by removing the unnecessary quirk 
UFS_DEVICE_QUIRK_DELAY_AFTER_LPM vs the inconvenience of a 5ms 
(potentially reduce to 2ms) delay impact it may cause on an old broken 
HW in the suspend/shutdown path.

I believe removing the UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk in the ufs 
core driver as well as all the platform drivers yields positive net 
benefits in this case.

Thanks, Bao

> 
> Thanks
> Peter
> 
> 
>>                  hba->vreg_info.vcc->always_on = true;
>>                  /*
>>                   * VCC will be kept always-on thus we don't
>> -                * need any delay during regulator operations
>> +                * need any delay before putting device's VCC in LPM
>> mode.
>>                   */
>> -               hba->dev_quirks &=
>> ~(UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM |
>> -                       UFS_DEVICE_QUIRK_DELAY_AFTER_LPM);
>> +               hba->dev_quirks &=
>> ~UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM;
>>
> 
> 
> 
> 
> 

Re: [PATCH v1 1/2] scsi: ufs: core: Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk

[Peter Wang (王信友)]

On Thu, 2025-10-02 at 11:48 -0700, Bao D. Nguyen wrote:
> 
> 
> Hi Peter,
> 
> The current Mediatek platform driver applies this quirk to all ufs
> vendors which is consistent with what we would like to do in the
> Qualcomm platform driver per the vendor's requests.
> 
> I do see that, about 5 years ago, Mediatek merged a patch to keep the
> device vcc always on, probably to workaround some HW issues. Since 

Hi Bao,

Yes, some UFS devices may have issues when turning off VCC.

> this
> is a very old patch and the impact of this change on a broken
> hardware
> is minimal, I would like weight the benefit of cleaning up the ufs
> core
> driver by removing the unnecessary quirk
> UFS_DEVICE_QUIRK_DELAY_AFTER_LPM vs the inconvenience of a 5ms
> (potentially reduce to 2ms) delay impact it may cause on an old
> broken
> HW in the suspend/shutdown path.
> 
> I believe removing the UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk in the
> ufs
> core driver as well as all the platform drivers yields positive net
> benefits in this case.
> 
> Thanks, Bao
> 
> 

I think you misunderstood my point.
I am okay with removing this flag, but this patch will cause
devices with VCC always on to unnecessarily wait for the 
delay, resulting in wasted time.

Thanks
Peter

Re: [PATCH v1 1/2] scsi: ufs: core: Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk

[Bao D. Nguyen]

On 10/2/2025 8:10 PM, Peter Wang (王信友) wrote:
> On Thu, 2025-10-02 at 11:48 -0700, Bao D. Nguyen wrote:
>>
>>
>> Hi Peter,
>>
>> The current Mediatek platform driver applies this quirk to all ufs
>> vendors which is consistent with what we would like to do in the
>> Qualcomm platform driver per the vendor's requests.
>>
>> I do see that, about 5 years ago, Mediatek merged a patch to keep the
>> device vcc always on, probably to workaround some HW issues. Since
> 
> Hi Bao,
> 
> Yes, some UFS devices may have issues when turning off VCC.
> 
>> this
>> is a very old patch and the impact of this change on a broken
>> hardware
>> is minimal, I would like weight the benefit of cleaning up the ufs
>> core
>> driver by removing the unnecessary quirk
>> UFS_DEVICE_QUIRK_DELAY_AFTER_LPM vs the inconvenience of a 5ms
>> (potentially reduce to 2ms) delay impact it may cause on an old
>> broken
>> HW in the suspend/shutdown path.
>>
>> I believe removing the UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk in the
>> ufs
>> core driver as well as all the platform drivers yields positive net
>> benefits in this case.
>>
>> Thanks, Bao
>>
>>
> 
> I think you misunderstood my point.
> I am okay with removing this flag, but this patch will cause
> devices with VCC always on to unnecessarily wait for the
> delay, resulting in wasted time.

Are you referring to the always_on flag in the struct ufs_vreg? I 
believe currently the ufs_vreg's always_on flag isn't used in 
determining whether the delay is applied or not.

How about we add the check for the Vcc's always_on as shown below?
The Mediatek's workaround can avoid the extra delay by setting the 
always_on flag as it already did, without using the 
UFS_DEVICE_QUIRK_DELAY_AFTER_LPM.

if (vcc_off && hba->vreg_info.vcc && !hba->vreg_info.vcc->always_on)
	usleep_range(5000, 5100);

Thanks, Bao

> 
> Thanks
> Peter
> 
> 

Re: [PATCH v1 1/2] scsi: ufs: core: Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk

[Peter Wang (王信友)]

On Fri, 2025-10-03 at 14:11 -0700, Bao D. Nguyen wrote:
> Are you referring to the always_on flag in the struct ufs_vreg? I
> believe currently the ufs_vreg's always_on flag isn't used in
> determining whether the delay is applied or not.
> 

Hi Bao,

Yes, I mean vreg_info.vcc->always_on = true.
Before your change, Mediatek would set always_on = true 
and remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM to disable 
this delay.


> How about we add the check for the Vcc's always_on as shown below?
> The Mediatek's workaround can avoid the extra delay by setting the
> always_on flag as it already did, without using the
> UFS_DEVICE_QUIRK_DELAY_AFTER_LPM.
> 
> if (vcc_off && hba->vreg_info.vcc && !hba->vreg_info.vcc->always_on)
>         usleep_range(5000, 5100);
> 
> Thanks, Bao
> 
> 

Yes, I believe this is the correct way to do it.

Thanks
Peter