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

[PATCH v2 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 replace the hard coded 5ms delay with a
variable default to 5ms setting and allow the platform drivers
to override this setting as needed to improve the system resume latency.

v1 -> v2:
	- Added a check for vcc's always_on to skip the delay if the vcc
	  is an always-on regulator (Peter's comment)
	- Added a sleep_post_vcc_off variable to allow platform drivers to
	  override the default core driver's setting as needed (Bart and Peter's comments)

Bao D. Nguyen (2):
  scsi: ufs: core: Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk
  scsi: ufs: core: Replace hard coded vcc-off delay with a variable

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

-- 
2.7.4

[PATCH v2 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..e8842dc 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 && !hba->vreg_info.vcc->always_on)
 		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 v2 2/2] scsi: ufs: core: Replace hard coded vcc-off delay with a variable

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

Replace the hard coded 5ms delay with a variable using a default
setting of 5ms. This allows the platform drivers to override this
setting to improve the system resume latency by reducing the sleep
time as needed.

Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
---
 drivers/ufs/core/ufshcd.c | 10 +++++++++-
 include/ufs/ufshcd.h      |  2 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index e8842dc..593c9d0 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9741,7 +9741,8 @@ 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 && !hba->vreg_info.vcc->always_on)
-		usleep_range(5000, 5100);
+		usleep_range(hba->sleep_post_vcc_off,
+			     hba->sleep_post_vcc_off + 100);
 }
 
 #ifdef CONFIG_PM
@@ -10665,6 +10666,13 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 						UFS_SLEEP_PWR_MODE,
 						UIC_LINK_HIBERN8_STATE);
 
+	/*
+	 * Most ufs devices require 1ms delay after vcc is powered off before
+	 * it can be powered on again. Set the default to 5ms. The platform
+	 * drivers can override this setting as needed.
+	 */
+	hba->sleep_post_vcc_off = 5000;
+
 	init_completion(&hba->dev_cmd.complete);
 
 	err = ufshcd_hba_init(hba);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 1d39437..ad49979 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1140,6 +1140,8 @@ struct ufs_hba {
 	int critical_health_count;
 	atomic_t dev_lvl_exception_count;
 	u64 dev_lvl_exception_id;
+
+	u32 sleep_post_vcc_off;
 };
 
 /**
-- 
2.7.4

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

[Peter Wang (王信友)]

On Thu, 2025-10-09 at 13:10 -0700, Bao D. Nguyen wrote:
> 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>

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH v2 2/2] scsi: ufs: core: Replace hard coded vcc-off delay with a variable

[Peter Wang (王信友)]

On Thu, 2025-10-09 at 13:10 -0700, Bao D. Nguyen wrote:
> 
> +       /*
> +        * Most ufs devices require 1ms delay after vcc is powered
> off before
> +        * it can be powered on again. Set the default to 5ms. The
> platform
> +        * drivers can override this setting as needed.
> +        */
> +       hba->sleep_post_vcc_off = 5000;
> +
> 

Hi Bao,

Since 2ms is sufficient for most devices, wouldn't it make
sense to set the default value to 2ms?


>         init_completion(&hba->dev_cmd.complete);
> 
>         err = ufshcd_hba_init(hba);
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 1d39437..ad49979 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -1140,6 +1140,8 @@ struct ufs_hba {
>         int critical_health_count;
>         atomic_t dev_lvl_exception_count;
>         u64 dev_lvl_exception_id;
> +
> +       u32 sleep_post_vcc_off;
> 

The name sleep_post_vcc_off might be misunderstood as a 
status or a flag. I suggest changing it to a more explicit 
name, such as vcc_off_delay_ms.


Thanks
Peter

Re: [PATCH v2 0/2] *** Remove UFS_DEVICE_QUIRK_DELAY_AFTER_LPM quirk ***

[Bart Van Assche]

On 10/9/25 1:10 PM, Bao D. Nguyen wrote:
> 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 replace the hard coded 5ms delay with a
> variable default to 5ms setting and allow the platform drivers
> to override this setting as needed to improve the system resume latency.

For both patches:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH v2 2/2] scsi: ufs: core: Replace hard coded vcc-off delay with a variable

[Bao D. Nguyen]

On 10/12/2025 8:58 PM, Peter Wang (王信友) wrote:
> On Thu, 2025-10-09 at 13:10 -0700, Bao D. Nguyen wrote:
>>
>> +       /*
>> +        * Most ufs devices require 1ms delay after vcc is powered
>> off before
>> +        * it can be powered on again. Set the default to 5ms. The
>> platform
>> +        * drivers can override this setting as needed.
>> +        */
>> +       hba->sleep_post_vcc_off = 5000;
>> +
>>
> 
> Hi Bao,
> 
> Since 2ms is sufficient for most devices, wouldn't it make
> sense to set the default value to 2ms?

Agree Peter. This was my preference as well.

> 
> 
>>          init_completion(&hba->dev_cmd.complete);
>>
>>          err = ufshcd_hba_init(hba);
>> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
>> index 1d39437..ad49979 100644
>> --- a/include/ufs/ufshcd.h
>> +++ b/include/ufs/ufshcd.h
>> @@ -1140,6 +1140,8 @@ struct ufs_hba {
>>          int critical_health_count;
>>          atomic_t dev_lvl_exception_count;
>>          u64 dev_lvl_exception_id;
>> +
>> +       u32 sleep_post_vcc_off;
>>
> 
> The name sleep_post_vcc_off might be misunderstood as a
> status or a flag. I suggest changing it to a more explicit
> name, such as vcc_off_delay_ms.

Sure Peter. I will make the change.

Thanks, Bao

> 
> 
> Thanks
> Peter
> 
>