[PATCH v1 0/2] *** Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk ***

[PATCH v1 0/2] *** Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk ***

[Bao D. Nguyen]

Multiple ufs device manufacturers request support for the
UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk in the Qualcomm's platform driver.
After checking further with the major UFS manufacturers engineering teams
such as Samsung, Kioxia, SK Hynix and Micron, all the manufacturers require
this quirk. Since the quirk is needed by all the ufs device manufacturers,
remove the quirk in the ufs core driver and implement a universal delay
for all the ufs devices.

In addition to verifying with the public device's datasheets, the ufs
device manufacturer's engineering teams confirmed the required vcc
power-off time for the devices is a minimum of 1ms before vcc can be
powered on again. The existing 5ms delay implemented in the ufs core
driver seems too conservative, so reduce this time to 2ms to improve
the system resume latency.

Bao D. Nguyen (2):
  scsi: ufs: core: Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk
  scsi: ufs: core: Reduce the sleep before vcc can be powered on

 drivers/ufs/core/ufshcd.c       |  7 +++----
 drivers/ufs/host/ufs-mediatek.c | 11 ++++-------
 drivers/ufs/host/ufs-qcom.c     |  3 ---
 include/ufs/ufs_quirks.h        |  7 -------
 4 files changed, 7 insertions(+), 21 deletions(-)

-- 
2.7.4

[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

[PATCH v1 2/2] scsi: ufs: core: Reduce the sleep before vcc can be powered on

[Bao D. Nguyen]

After the ufs device vcc is powered off, all the ufs device
manufacturers require a minimum of 1ms of power-off time before
vcc can be powered on again. This requirement has been verified
with all the ufs device manufacturer's datasheets.
Improve the system resume latency by reducing the required power-off
time from 5ms to 2ms. The chosen 2ms should include enough
additional buffer time without being wasteful.

Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
 drivers/ufs/core/ufshcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 45e509b..83bd731 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9741,7 +9741,7 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
 	 * All UFS devices require delay after VCC power rail is turned-off.
 	 */
 	if (vcc_off && hba->vreg_info.vcc)
-		usleep_range(5000, 5100);
+		usleep_range(2000, 2100);
 }
 
 #ifdef CONFIG_PM
-- 
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 2/2] scsi: ufs: core: Reduce the sleep before vcc can be powered on

[Peter Wang (王信友)]

On Wed, 2025-10-01 at 13:57 -0700, Bao D. Nguyen wrote:
> 
> After the ufs device vcc is powered off, all the ufs device
> manufacturers require a minimum of 1ms of power-off time before
> vcc can be powered on again. This requirement has been verified
> with all the ufs device manufacturer's datasheets.
> Improve the system resume latency by reducing the required power-off
> time from 5ms to 2ms. The chosen 2ms should include enough
> additional buffer time without being wasteful.
> 
> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> ---
>  drivers/ufs/core/ufshcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 45e509b..83bd731 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -9741,7 +9741,7 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba
> *hba)
>          * All UFS devices require delay after VCC power rail is
> turned-off.
>          */
>         if (vcc_off && hba->vreg_info.vcc)
> -               usleep_range(5000, 5100);
> +               usleep_range(2000, 2100);

Hi Bao,

This delay should be compatible with legacy devices.
The initial value was set to 5ms, does that mean there 
is a device that actually needs 5ms?

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 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 2/2] scsi: ufs: core: Reduce the sleep before vcc can be powered on

[Bao D. Nguyen]

On 10/2/2025 12:59 AM, Peter Wang (王信友) wrote:
> On Wed, 2025-10-01 at 13:57 -0700, Bao D. Nguyen wrote:
>>
>> After the ufs device vcc is powered off, all the ufs device
>> manufacturers require a minimum of 1ms of power-off time before
>> vcc can be powered on again. This requirement has been verified
>> with all the ufs device manufacturer's datasheets.
>> Improve the system resume latency by reducing the required power-off
>> time from 5ms to 2ms. The chosen 2ms should include enough
>> additional buffer time without being wasteful.
>>
>> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
>> ---
>>   drivers/ufs/core/ufshcd.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 45e509b..83bd731 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -9741,7 +9741,7 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba
>> *hba)
>>           * All UFS devices require delay after VCC power rail is
>> turned-off.
>>           */
>>          if (vcc_off && hba->vreg_info.vcc)
>> -               usleep_range(5000, 5100);
>> +               usleep_range(2000, 2100);
> 
> Hi Bao,
> 
> This delay should be compatible with legacy devices.
> The initial value was set to 5ms, does that mean there
> is a device that actually needs 5ms?

Hi Peter,
I have discussed with the major ufs vendors (Samsung, Kioxia, Micron, 
and SK Hynix) via emails. They are all in agreement that 2ms is good. I 
did check the current device's datasheets and 1ms is what their 
specifications require. I admit that I may have missed some very old ufs 
device's datasheets. However, I take the words of the ufs vendor's 
engineering teams and the current device's datasheets that the 2ms is 
good for their devices and try to improve the potentially conservative 
5ms delay parameter.

Thanks, Bao

> 
> Thanks
> Peter
> 

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 2/2] scsi: ufs: core: Reduce the sleep before vcc can be powered on

[Peter Wang (王信友)]

On Thu, 2025-10-02 at 12:00 -0700, Bao D. Nguyen wrote:
> 
> Hi Peter,
> I have discussed with the major ufs vendors (Samsung, Kioxia, Micron,
> and SK Hynix) via emails. They are all in agreement that 2ms is good.
> I
> did check the current device's datasheets and 1ms is what their
> specifications require. I admit that I may have missed some very old
> ufs
> device's datasheets. However, I take the words of the ufs vendor's
> engineering teams and the current device's datasheets that the 2ms is
> good for their devices and try to improve the potentially
> conservative
> 5ms delay parameter.
> 
> Thanks, Bao
> 
> 
> 

Hi Bao,

Yes, I am concerned that legacy UFS devices may encounter errors
when upgrading the kernel if the delay is not sufficient.

Furthermore, the vendor claims that 2ms is sufficient. Is this
based on a typical scenario? or should we be concerned about 
the worst-case scenario? I am also worried that the worst-case
delay may not be enough.

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 2/2] scsi: ufs: core: Reduce the sleep before vcc can be powered on

[Bao D. Nguyen]

On 10/2/2025 8:11 PM, Peter Wang (王信友) wrote:
> On Thu, 2025-10-02 at 12:00 -0700, Bao D. Nguyen wrote:
>>
>> Hi Peter,
>> I have discussed with the major ufs vendors (Samsung, Kioxia, Micron,
>> and SK Hynix) via emails. They are all in agreement that 2ms is good.
>> I
>> did check the current device's datasheets and 1ms is what their
>> specifications require. I admit that I may have missed some very old
>> ufs
>> device's datasheets. However, I take the words of the ufs vendor's
>> engineering teams and the current device's datasheets that the 2ms is
>> good for their devices and try to improve the potentially
>> conservative
>> 5ms delay parameter.
>>
>> Thanks, Bao
>>
>>
>>
> 
> Hi Bao,
> 
> Yes, I am concerned that legacy UFS devices may encounter errors
> when upgrading the kernel if the delay is not sufficient.
> 
> Furthermore, the vendor claims that 2ms is sufficient. Is this
> based on a typical scenario? or should we be concerned about
> the worst-case scenario? I am also worried that the worst-case
> delay may not be enough.

With the current or recent offerings of ufs devices in the market, the 
requirement is 1ms. For example, the Kioxia datasheet says "Vcc shall be 
kept less than 0.3V for at least 1ms before it goes beyond 0.3V again". 
Similarly other vendors have this 1ms requirement. So I believe this 
indicates the worst case scenario.
I understand there may be very old devices that are upgrading the kernel 
only. In that case I don't know the specifics for these old ufs parts as 
mentioned.

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

Re: [PATCH v1 2/2] scsi: ufs: core: Reduce the sleep before vcc can be powered on

[Peter Wang (王信友)]

On Fri, 2025-10-03 at 14:27 -0700, Bao D. Nguyen wrote:
> With the current or recent offerings of ufs devices in the market,
> the
> requirement is 1ms. For example, the Kioxia datasheet says "Vcc shall
> be
> kept less than 0.3V for at least 1ms before it goes beyond 0.3V
> again".
> Similarly other vendors have this 1ms requirement. So I believe this
> indicates the worst case scenario.
> I understand there may be very old devices that are upgrading the
> kernel
> only. In that case I don't know the specifics for these old ufs parts
> as
> mentioned.
> 
> Thanks, Bao
> 

Hi Bao,

Please consider using module_param_cb to set the default 
delay to 2ms(or 1ms). At the same time, we should keep the
flexibility for devices that may require a longer delay by
allowing them to extend the delay through a module parameter.

Thanks
Peter
> 

Re: [PATCH v1 2/2] scsi: ufs: core: Reduce the sleep before vcc can be powered on

[Bart Van Assche]

On 10/7/25 12:04 AM, Peter Wang (王信友) wrote:
> On Fri, 2025-10-03 at 14:27 -0700, Bao D. Nguyen wrote:
>> With the current or recent offerings of ufs devices in the market, 
>> the requirement is 1ms. For example, the Kioxia datasheet says
>> "Vcc shall be kept less than 0.3V for at least 1ms before it goes
>> beyond 0.3V again". Similarly other vendors have this 1ms
>> requirement. So I believe this indicates the worst case scenario. 
>> I understand there may be very old devices that are upgrading the 
>> kernel only. In that case I don't know the specifics for these old
>> ufs parts as mentioned.
> 
> Hi Bao,
> 
> Please consider using module_param_cb to set the default
> delay to 2ms(or 1ms). At the same time, we should keep the
> flexibility for devices that may require a longer delay by
> allowing them to extend the delay through a module parameter.

Why a kernel module parameter? Why can't the default delay be set by
ufshcd_variant_ops.init()?

Thanks,

Bart.

Re: [PATCH v1 2/2] scsi: ufs: core: Reduce the sleep before vcc can be powered on

[Bao D. Nguyen]

On 10/7/2025 9:19 AM, Bart Van Assche wrote:
> 
> On 10/7/25 12:04 AM, Peter Wang (王信友) wrote:
>> On Fri, 2025-10-03 at 14:27 -0700, Bao D. Nguyen wrote:
>>> With the current or recent offerings of ufs devices in the market, 
>>> the requirement is 1ms. For example, the Kioxia datasheet says
>>> "Vcc shall be kept less than 0.3V for at least 1ms before it goes
>>> beyond 0.3V again". Similarly other vendors have this 1ms
>>> requirement. So I believe this indicates the worst case scenario. I 
>>> understand there may be very old devices that are upgrading the 
>>> kernel only. In that case I don't know the specifics for these old
>>> ufs parts as mentioned.
>>
>> Hi Bao,
>>
>> Please consider using module_param_cb to set the default
>> delay to 2ms(or 1ms). At the same time, we should keep the
>> flexibility for devices that may require a longer delay by
>> allowing them to extend the delay through a module parameter.
> 
> Why a kernel module parameter? Why can't the default delay be set by
> ufshcd_variant_ops.init()?
> 

I am also not sure if it is worth adding complexity to the driver using 
a kernel parameter for this particular case.
I can try Bart's suggestion to override the default value with the 
platform driver init, or drop the change trying to reduce the sleep time 
from 5ms to 2ms.

Thanks, Bao

> Thanks,
> 
> Bart.

Re: [PATCH v1 2/2] scsi: ufs: core: Reduce the sleep before vcc can be powered on

[Peter Wang (王信友)]

On Tue, 2025-10-07 at 09:19 -0700, Bart Van Assche wrote:
> Why a kernel module parameter? Why can't the default delay be set by
> ufshcd_variant_ops.init()?
> 
> Thanks,
> 
> Bart.

Hi Bart,

Yes, that is another method. The main point is that we should
have the flexibility to extend the delay time if we find that
2ms is not sufficient for some devices.

Thanks
Peter

Re: [PATCH v1 2/2] scsi: ufs: core: Reduce the sleep before vcc can be powered on

[Peter Wang (王信友)]

On Tue, 2025-10-07 at 17:10 -0700, Bao D. Nguyen wrote:
> I am also not sure if it is worth adding complexity to the driver
> using
> a kernel parameter for this particular case.
> I can try Bart's suggestion to override the default value with the
> platform driver init, or drop the change trying to reduce the sleep
> time
> from 5ms to 2ms.
> 
> Thanks, Bao

Bart’s suggestion is preferable from my perspective.

Thanks
Peter