[PATCH 0/3] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers

[PATCH 0/3] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers

[Adrian Hunter]

Hi

Here are a couple of fixes for Intel MTL-like UFS host controllers,
related to link hibernate state, and a minor tidy-up.


Adrian Hunter (2):
      scsi: ufs: ufs-pci: Fix default runtime and system PM levels
      scsi: ufs: ufs-pci: Remove UFS PCI driver's ->late_init() call back

Archana Patni (1):
      scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers

 drivers/ufs/host/ufshcd-pci.c | 60 ++++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 24 deletions(-)


Regards
Adrian

[PATCH 1/3] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers

[Adrian Hunter]

From: Archana Patni <archana.patni@intel.com>

UFSHCD core disables the UIC completion interrupt when issuing UIC
hibernation commands, and re-enables it afterwards if it was enabled to
start with, refer ufshcd_uic_pwr_ctrl(). For Intel MTL-like host
controllers, accessing the register to re-enable the interrupt disrupts the
state transition.

Use hibern8_notify variant operation to disable the interrupt during the
entire hibernation, thereby preventing the disruption.

Fixes: 4049f7acef3eb ("scsi: ufs: ufs-pci: Add support for Intel MTL")
Cc: stable@vger.kernel.org
Signed-off-by: Archana Patni <archana.patni@intel.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/ufs/host/ufshcd-pci.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
index 996387906aa1..af1c272eef1c 100644
--- a/drivers/ufs/host/ufshcd-pci.c
+++ b/drivers/ufs/host/ufshcd-pci.c
@@ -216,6 +216,32 @@ static int ufs_intel_lkf_apply_dev_quirks(struct ufs_hba *hba)
 	return ret;
 }
 
+static void ufs_intel_ctrl_uic_compl(struct ufs_hba *hba, bool enable)
+{
+	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+
+	if (enable)
+		set |= UIC_COMMAND_COMPL;
+	else
+		set &= ~UIC_COMMAND_COMPL;
+	ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
+}
+
+static void ufs_intel_mtl_h8_notify(struct ufs_hba *hba,
+				    enum uic_cmd_dme cmd,
+				    enum ufs_notify_change_status status)
+{
+	/*
+	 * Disable UIC COMPL INTR to prevent access to UFSHCI after
+	 * checking HCS.UPMCRS
+	 */
+	if (status == PRE_CHANGE && cmd == UIC_CMD_DME_HIBER_ENTER)
+		ufs_intel_ctrl_uic_compl(hba, false);
+
+	if (status == POST_CHANGE && cmd == UIC_CMD_DME_HIBER_EXIT)
+		ufs_intel_ctrl_uic_compl(hba, true);
+}
+
 #define INTEL_ACTIVELTR		0x804
 #define INTEL_IDLELTR		0x808
 
@@ -533,6 +559,7 @@ static struct ufs_hba_variant_ops ufs_intel_mtl_hba_vops = {
 	.init			= ufs_intel_mtl_init,
 	.exit			= ufs_intel_common_exit,
 	.hce_enable_notify	= ufs_intel_hce_enable_notify,
+	.hibern8_notify		= ufs_intel_mtl_h8_notify,
 	.link_startup_notify	= ufs_intel_link_startup_notify,
 	.resume			= ufs_intel_resume,
 	.device_reset		= ufs_intel_device_reset,
-- 
2.48.1

[PATCH 2/3] scsi: ufs: ufs-pci: Fix default runtime and system PM levels

[Adrian Hunter]

Intel MTL-like host controllers support auto-hibernate.  Using
auto-hibernate with manual (driver initiated) hibernate produces more
complex operation.  For example, the host controller will have to exit
auto-hibernate simply to allow the driver to enter hibernate state
manually.  That is not recommended.

The default rpm_lvl and spm_lvl is 3, which includes manual hibernate.

Change the default values to 2, which does not.

Note, to be simpler to backport to stable kernels, utilize the UFS PCI
driver's ->late_init() call back.  Recent commits have made it possible
to set up a controller-specific default in the regular ->init() call back,
but not all stable kernels have those changes.

Fixes: 4049f7acef3eb ("scsi: ufs: ufs-pci: Add support for Intel MTL")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/ufs/host/ufshcd-pci.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
index af1c272eef1c..e42e5f69ff0c 100644
--- a/drivers/ufs/host/ufshcd-pci.c
+++ b/drivers/ufs/host/ufshcd-pci.c
@@ -468,10 +468,22 @@ static int ufs_intel_adl_init(struct ufs_hba *hba)
 	return ufs_intel_common_init(hba);
 }
 
+static void ufs_intel_mtl_late_init(struct ufs_hba *hba)
+{
+	hba->rpm_lvl = UFS_PM_LVL_2;
+	hba->spm_lvl = UFS_PM_LVL_2;
+}
+
 static int ufs_intel_mtl_init(struct ufs_hba *hba)
 {
+	struct ufs_host *ufs_host;
+	int err;
+
 	hba->caps |= UFSHCD_CAP_CRYPTO | UFSHCD_CAP_WB_EN;
-	return ufs_intel_common_init(hba);
+	err = ufs_intel_common_init(hba);
+	ufs_host = ufshcd_get_variant(hba);
+	ufs_host->late_init = ufs_intel_mtl_late_init;
+	return err;
 }
 
 static int ufs_qemu_get_hba_mac(struct ufs_hba *hba)
-- 
2.48.1

[PATCH 3/3] scsi: ufs: ufs-pci: Remove UFS PCI driver's ->late_init() call back

[Adrian Hunter]

 ->late_init() was introduced to allow the default values for rpm_lvl and
spm_lvl to be set.  Since commit bb9850704c04 ("scsi: ufs: core: Honor
runtime/system PM levels if set by host controller drivers") and
commit fe06b7c07f3f ("scsi: ufs: core: Set default runtime/system PM levels
before ufshcd_hba_init()"), those default values can be set in the ->init()
variant call back.

Move the setting of default values for rpm_lvl and spm_lvl to ->init() and
remove ->late_init().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/ufs/host/ufshcd-pci.c | 45 +++++++----------------------------
 1 file changed, 9 insertions(+), 36 deletions(-)

diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
index e42e5f69ff0c..b29ec1904482 100644
--- a/drivers/ufs/host/ufshcd-pci.c
+++ b/drivers/ufs/host/ufshcd-pci.c
@@ -22,17 +22,12 @@
 
 #define MAX_SUPP_MAC 64
 
-struct ufs_host {
-	void (*late_init)(struct ufs_hba *hba);
-};
-
 enum intel_ufs_dsm_func_id {
 	INTEL_DSM_FNS		=  0,
 	INTEL_DSM_RESET		=  1,
 };
 
 struct intel_host {
-	struct ufs_host ufs_host;
 	u32		dsm_fns;
 	u32		active_ltr;
 	u32		idle_ltr;
@@ -434,8 +429,14 @@ static int ufs_intel_ehl_init(struct ufs_hba *hba)
 	return ufs_intel_common_init(hba);
 }
 
-static void ufs_intel_lkf_late_init(struct ufs_hba *hba)
+static int ufs_intel_lkf_init(struct ufs_hba *hba)
 {
+	int err;
+
+	hba->nop_out_timeout = 200;
+	hba->quirks |= UFSHCD_QUIRK_BROKEN_AUTO_HIBERN8;
+	hba->caps |= UFSHCD_CAP_CRYPTO;
+	err = ufs_intel_common_init(hba);
 	/* LKF always needs a full reset, so set PM accordingly */
 	if (hba->caps & UFSHCD_CAP_DEEPSLEEP) {
 		hba->spm_lvl = UFS_PM_LVL_6;
@@ -444,19 +445,6 @@ static void ufs_intel_lkf_late_init(struct ufs_hba *hba)
 		hba->spm_lvl = UFS_PM_LVL_5;
 		hba->rpm_lvl = UFS_PM_LVL_5;
 	}
-}
-
-static int ufs_intel_lkf_init(struct ufs_hba *hba)
-{
-	struct ufs_host *ufs_host;
-	int err;
-
-	hba->nop_out_timeout = 200;
-	hba->quirks |= UFSHCD_QUIRK_BROKEN_AUTO_HIBERN8;
-	hba->caps |= UFSHCD_CAP_CRYPTO;
-	err = ufs_intel_common_init(hba);
-	ufs_host = ufshcd_get_variant(hba);
-	ufs_host->late_init = ufs_intel_lkf_late_init;
 	return err;
 }
 
@@ -468,22 +456,12 @@ static int ufs_intel_adl_init(struct ufs_hba *hba)
 	return ufs_intel_common_init(hba);
 }
 
-static void ufs_intel_mtl_late_init(struct ufs_hba *hba)
+static int ufs_intel_mtl_init(struct ufs_hba *hba)
 {
 	hba->rpm_lvl = UFS_PM_LVL_2;
 	hba->spm_lvl = UFS_PM_LVL_2;
-}
-
-static int ufs_intel_mtl_init(struct ufs_hba *hba)
-{
-	struct ufs_host *ufs_host;
-	int err;
-
 	hba->caps |= UFSHCD_CAP_CRYPTO | UFSHCD_CAP_WB_EN;
-	err = ufs_intel_common_init(hba);
-	ufs_host = ufshcd_get_variant(hba);
-	ufs_host->late_init = ufs_intel_mtl_late_init;
-	return err;
+	return ufs_intel_common_init(hba);
 }
 
 static int ufs_qemu_get_hba_mac(struct ufs_hba *hba)
@@ -613,7 +591,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
 static int
 ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
-	struct ufs_host *ufs_host;
 	struct ufs_hba *hba;
 	void __iomem *mmio_base;
 	int err;
@@ -646,10 +623,6 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return err;
 	}
 
-	ufs_host = ufshcd_get_variant(hba);
-	if (ufs_host && ufs_host->late_init)
-		ufs_host->late_init(hba);
-
 	pm_runtime_put_noidle(&pdev->dev);
 	pm_runtime_allow(&pdev->dev);
 
-- 
2.48.1

Re: [PATCH 1/3] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers

[Bart Van Assche]

On 7/2/25 11:43 PM, Adrian Hunter wrote:
> UFSHCD core disables the UIC completion interrupt when issuing UIC
> hibernation commands, and re-enables it afterwards if it was enabled to
> start with, refer ufshcd_uic_pwr_ctrl(). For Intel MTL-like host
> controllers, accessing the register to re-enable the interrupt disrupts the
> state transition.
> 
> Use hibern8_notify variant operation to disable the interrupt during the
> entire hibernation, thereby preventing the disruption.
> 
> Fixes: 4049f7acef3eb ("scsi: ufs: ufs-pci: Add support for Intel MTL")
> Cc: stable@vger.kernel.org
> Signed-off-by: Archana Patni <archana.patni@intel.com>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>   drivers/ufs/host/ufshcd-pci.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
> index 996387906aa1..af1c272eef1c 100644
> --- a/drivers/ufs/host/ufshcd-pci.c
> +++ b/drivers/ufs/host/ufshcd-pci.c
> @@ -216,6 +216,32 @@ static int ufs_intel_lkf_apply_dev_quirks(struct ufs_hba *hba)
>   	return ret;
>   }
>   
> +static void ufs_intel_ctrl_uic_compl(struct ufs_hba *hba, bool enable)
> +{
> +	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> +
> +	if (enable)
> +		set |= UIC_COMMAND_COMPL;
> +	else
> +		set &= ~UIC_COMMAND_COMPL;
> +	ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
> +}
> +
> +static void ufs_intel_mtl_h8_notify(struct ufs_hba *hba,
> +				    enum uic_cmd_dme cmd,
> +				    enum ufs_notify_change_status status)
> +{
> +	/*
> +	 * Disable UIC COMPL INTR to prevent access to UFSHCI after
> +	 * checking HCS.UPMCRS
> +	 */
> +	if (status == PRE_CHANGE && cmd == UIC_CMD_DME_HIBER_ENTER)
> +		ufs_intel_ctrl_uic_compl(hba, false);
> +
> +	if (status == POST_CHANGE && cmd == UIC_CMD_DME_HIBER_EXIT)
> +		ufs_intel_ctrl_uic_compl(hba, true);
> +}
> +
>   #define INTEL_ACTIVELTR		0x804
>   #define INTEL_IDLELTR		0x808
>   
> @@ -533,6 +559,7 @@ static struct ufs_hba_variant_ops ufs_intel_mtl_hba_vops = {
>   	.init			= ufs_intel_mtl_init,
>   	.exit			= ufs_intel_common_exit,
>   	.hce_enable_notify	= ufs_intel_hce_enable_notify,
> +	.hibern8_notify		= ufs_intel_mtl_h8_notify,
>   	.link_startup_notify	= ufs_intel_link_startup_notify,
>   	.resume			= ufs_intel_resume,
>   	.device_reset		= ufs_intel_device_reset,

Having both the UFS core driver and UFS host drivers modify
REG_INTERRUPT_ENABLE makes the UFS core driver much harder to maintain
than necessary. Instead of introducing a new .hibern8_notify callback,
please integrate the above logic directly in the UFS core driver. One
possible approach is to add an argument to ufshcd_uic_pwr_ctrl()
that indicates from which context ufshcd_uic_pwr_ctrl() is being called
(ufshcd_uic_hibern8_enter(), ufshcd_uic_hibern8_exit() or another 
function). I think it is safe to activate the behavior from this patch
for all UFS host drivers.

Thanks,

Bart.

Re: [PATCH 2/3] scsi: ufs: ufs-pci: Fix default runtime and system PM levels

[Bart Van Assche]

On 7/2/25 11:43 PM, Adrian Hunter wrote:
>   static int ufs_intel_mtl_init(struct ufs_hba *hba)
>   {
> +	struct ufs_host *ufs_host;
> +	int err;
> +
>   	hba->caps |= UFSHCD_CAP_CRYPTO | UFSHCD_CAP_WB_EN;
> -	return ufs_intel_common_init(hba);
> +	err = ufs_intel_common_init(hba);
> +	ufs_host = ufshcd_get_variant(hba);
> +	ufs_host->late_init = ufs_intel_mtl_late_init;
> +	return err;
>   }

Shouldn't the ufs_host declaration and assignment be combined as
follows?

struct ufs_host *ufs_host = ufshcd_get_variant(hba);

Otherwise this patch looks good to me.

Thanks,

Bart.

Re: [PATCH 3/3] scsi: ufs: ufs-pci: Remove UFS PCI driver's ->late_init() call back

[Bart Van Assche]

On 7/2/25 11:43 PM, Adrian Hunter wrote:
>   ->late_init() was introduced to allow the default values for rpm_lvl and
> spm_lvl to be set.  Since commit bb9850704c04 ("scsi: ufs: core: Honor
> runtime/system PM levels if set by host controller drivers") and
> commit fe06b7c07f3f ("scsi: ufs: core: Set default runtime/system PM levels
> before ufshcd_hba_init()"), those default values can be set in the ->init()
> variant call back.
> 
> Move the setting of default values for rpm_lvl and spm_lvl to ->init() and
> remove ->late_init().

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

Re: [PATCH 2/3] scsi: ufs: ufs-pci: Fix default runtime and system PM levels

[Adrian Hunter]

On 07/07/2025 22:57, Bart Van Assche wrote:
> On 7/2/25 11:43 PM, Adrian Hunter wrote:
>>   static int ufs_intel_mtl_init(struct ufs_hba *hba)
>>   {
>> +    struct ufs_host *ufs_host;
>> +    int err;
>> +
>>       hba->caps |= UFSHCD_CAP_CRYPTO | UFSHCD_CAP_WB_EN;
>> -    return ufs_intel_common_init(hba);
>> +    err = ufs_intel_common_init(hba);
>> +    ufs_host = ufshcd_get_variant(hba);
>> +    ufs_host->late_init = ufs_intel_mtl_late_init;
>> +    return err;
>>   }
> 
> Shouldn't the ufs_host declaration and assignment be combined as
> follows?
> 
> struct ufs_host *ufs_host = ufshcd_get_variant(hba);

The variant is not set until ufs_intel_common_init()

> 
> Otherwise this patch looks good to me.
> 
> Thanks,
> 
> Bart.

Re: [PATCH 1/3] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers

[Adrian Hunter]

On 07/07/2025 22:51, Bart Van Assche wrote:
> On 7/2/25 11:43 PM, Adrian Hunter wrote:
>> UFSHCD core disables the UIC completion interrupt when issuing UIC
>> hibernation commands, and re-enables it afterwards if it was enabled to
>> start with, refer ufshcd_uic_pwr_ctrl(). For Intel MTL-like host
>> controllers, accessing the register to re-enable the interrupt disrupts the
>> state transition.
>>
>> Use hibern8_notify variant operation to disable the interrupt during the
>> entire hibernation, thereby preventing the disruption.
>>
>> Fixes: 4049f7acef3eb ("scsi: ufs: ufs-pci: Add support for Intel MTL")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Archana Patni <archana.patni@intel.com>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>   drivers/ufs/host/ufshcd-pci.c | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
>> index 996387906aa1..af1c272eef1c 100644
>> --- a/drivers/ufs/host/ufshcd-pci.c
>> +++ b/drivers/ufs/host/ufshcd-pci.c
>> @@ -216,6 +216,32 @@ static int ufs_intel_lkf_apply_dev_quirks(struct ufs_hba *hba)
>>       return ret;
>>   }
>>   +static void ufs_intel_ctrl_uic_compl(struct ufs_hba *hba, bool enable)
>> +{
>> +    u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
>> +
>> +    if (enable)
>> +        set |= UIC_COMMAND_COMPL;
>> +    else
>> +        set &= ~UIC_COMMAND_COMPL;
>> +    ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
>> +}
>> +
>> +static void ufs_intel_mtl_h8_notify(struct ufs_hba *hba,
>> +                    enum uic_cmd_dme cmd,
>> +                    enum ufs_notify_change_status status)
>> +{
>> +    /*
>> +     * Disable UIC COMPL INTR to prevent access to UFSHCI after
>> +     * checking HCS.UPMCRS
>> +     */
>> +    if (status == PRE_CHANGE && cmd == UIC_CMD_DME_HIBER_ENTER)
>> +        ufs_intel_ctrl_uic_compl(hba, false);
>> +
>> +    if (status == POST_CHANGE && cmd == UIC_CMD_DME_HIBER_EXIT)
>> +        ufs_intel_ctrl_uic_compl(hba, true);
>> +}
>> +
>>   #define INTEL_ACTIVELTR        0x804
>>   #define INTEL_IDLELTR        0x808
>>   @@ -533,6 +559,7 @@ static struct ufs_hba_variant_ops ufs_intel_mtl_hba_vops = {
>>       .init            = ufs_intel_mtl_init,
>>       .exit            = ufs_intel_common_exit,
>>       .hce_enable_notify    = ufs_intel_hce_enable_notify,
>> +    .hibern8_notify        = ufs_intel_mtl_h8_notify,
>>       .link_startup_notify    = ufs_intel_link_startup_notify,
>>       .resume            = ufs_intel_resume,
>>       .device_reset        = ufs_intel_device_reset,
> 
> Having both the UFS core driver and UFS host drivers modify
> REG_INTERRUPT_ENABLE makes the UFS core driver much harder to maintain
> than necessary. Instead of introducing a new .hibern8_notify callback,
> please integrate the above logic directly in the UFS core driver. One
> possible approach is to add an argument to ufshcd_uic_pwr_ctrl()
> that indicates from which context ufshcd_uic_pwr_ctrl() is being called
> (ufshcd_uic_hibern8_enter(), ufshcd_uic_hibern8_exit() or another function). I think it is safe to activate the behavior from this patch
> for all UFS host drivers.

[ Sorry for slow reply - been away ]

It is still possible to do DME_GET / DME_SET while the link is in
Hibernation state.  Refer UniPro spec Table 105 "DME_SAP Restrictions"

The MediaTek driver does appear to do that:

	ufs_mtk_suspend()
		if (ufshcd_is_link_hibern8(hba))
			ufs_mtk_link_set_lpm()
				ufs_mtk_unipro_set_lpm()
					ufshcd_dme_set()
						ufshcd_dme_set_attr(DME_LOCAL)
							ufshcd_send_uic_cmd

In other words, disabling the UIC completion interrupt while the link
is in Hibernation state might break the MediaTek driver.

I would suggest sticking with the current patch for stable, and
considering improving UFS code going forward, for example setting
and clearing the interrupt as needed, instead of leaving it set
always:

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 50adfb8b335b..1401aa03f7e9 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -364,6 +364,32 @@ void ufshcd_disable_irq(struct ufs_hba *hba)
 }
 EXPORT_SYMBOL_GPL(ufshcd_disable_irq);
 
+/**
+ * ufshcd_enable_intr - enable interrupts
+ * @hba: per adapter instance
+ * @intrs: interrupt bits
+ */
+static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
+{
+	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+
+	set |= intrs;
+	ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
+}
+
+/**
+ * ufshcd_disable_intr - disable interrupts
+ * @hba: per adapter instance
+ * @intrs: interrupt bits
+ */
+static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
+{
+	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+
+	set &= ~intrs;
+	ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
+}
+
 static void ufshcd_configure_wb(struct ufs_hba *hba)
 {
 	if (!ufshcd_is_wb_allowed(hba))
@@ -2596,6 +2622,7 @@ __ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
  */
 int ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
 {
+	unsigned long flags;
 	int ret;
 
 	if (hba->quirks & UFSHCD_QUIRK_BROKEN_UIC_CMD)
@@ -2605,6 +2632,10 @@ int ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
 	mutex_lock(&hba->uic_cmd_mutex);
 	ufshcd_add_delay_before_dme_cmd(hba);
 
+	spin_lock_irqsave(hba->host->host_lock, flags);
+	ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
+	spin_unlock_irqrestore(hba->host->host_lock, flags);
+
 	ret = __ufshcd_send_uic_cmd(hba, uic_cmd);
 	if (!ret)
 		ret = ufshcd_wait_for_uic_cmd(hba, uic_cmd);
@@ -2681,32 +2712,6 @@ static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 	return ufshcd_crypto_fill_prdt(hba, lrbp);
 }
 
-/**
- * ufshcd_enable_intr - enable interrupts
- * @hba: per adapter instance
- * @intrs: interrupt bits
- */
-static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
-{
-	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
-
-	set |= intrs;
-	ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
-}
-
-/**
- * ufshcd_disable_intr - disable interrupts
- * @hba: per adapter instance
- * @intrs: interrupt bits
- */
-static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
-{
-	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
-
-	set &= ~intrs;
-	ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
-}
-
 /**
  * ufshcd_prepare_req_desc_hdr - Fill UTP Transfer request descriptor header according to request
  * descriptor according to request
@@ -4275,7 +4280,6 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
 	unsigned long flags;
 	u8 status;
 	int ret;
-	bool reenable_intr = false;
 
 	mutex_lock(&hba->uic_cmd_mutex);
 	ufshcd_add_delay_before_dme_cmd(hba);
@@ -4286,15 +4290,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
 		goto out_unlock;
 	}
 	hba->uic_async_done = &uic_async_done;
-	if (ufshcd_readl(hba, REG_INTERRUPT_ENABLE) & UIC_COMMAND_COMPL) {
-		ufshcd_disable_intr(hba, UIC_COMMAND_COMPL);
-		/*
-		 * Make sure UIC command completion interrupt is disabled before
-		 * issuing UIC command.
-		 */
-		ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
-		reenable_intr = true;
-	}
+	ufshcd_disable_intr(hba, UIC_COMMAND_COMPL);
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 	ret = __ufshcd_send_uic_cmd(hba, cmd);
 	if (ret) {
@@ -4338,8 +4334,6 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
 	spin_lock_irqsave(hba->host->host_lock, flags);
 	hba->active_uic_cmd = NULL;
 	hba->uic_async_done = NULL;
-	if (reenable_intr)
-		ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
 	if (ret) {
 		ufshcd_set_link_broken(hba);
 		ufshcd_schedule_eh_work(hba);
@@ -4360,6 +4354,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
  */
 int ufshcd_send_bsg_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
 {
+	unsigned long flags;
 	int ret;
 
 	if (hba->quirks & UFSHCD_QUIRK_BROKEN_UIC_CMD)
@@ -4376,6 +4371,10 @@ int ufshcd_send_bsg_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
 	mutex_lock(&hba->uic_cmd_mutex);
 	ufshcd_add_delay_before_dme_cmd(hba);
 
+	spin_lock_irqsave(hba->host->host_lock, flags);
+	ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
+	spin_unlock_irqrestore(hba->host->host_lock, flags);
+
 	ret = __ufshcd_send_uic_cmd(hba, uic_cmd);
 	if (!ret)
 		ret = ufshcd_wait_for_uic_cmd(hba, uic_cmd);

Re: [PATCH 1/3] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers

[Bart Van Assche]

On 7/22/25 2:27 AM, Adrian Hunter wrote:
> I would suggest sticking with the current patch for stable, and
> considering improving UFS code going forward, for example setting
> and clearing the interrupt as needed, instead of leaving it set
> always: [ ... ]

Although the patch below looks good to me, another possibility is to
add an argument to __ufshcd_send_uic_cmd() that indicates whether or
not the UIC command completion interrupt should be enabled.

Thanks,

Bart.