public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers
@ 2025-07-23 16:58 Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 1/8] " Adrian Hunter
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Adrian Hunter @ 2025-07-23 16:58 UTC (permalink / raw)
  To: Martin K Petersen
  Cc: James EJ Bottomley, Bart Van Assche, Avri Altman, Archana Patni,
	linux-scsi

Hi

Here is V2 of a couple of fixes for Intel MTL-like UFS host controllers,
related to link Hibernation state.

Following the fixes are some improvements for the enabling and disabling
of UIC Completion interrupts.


Changes in V2:
    scsi: ufs: ufs-pci: Fix default runtime and system PM levels
	Add comment about getting variant after it is set

    scsi: ufs: ufs-pci: Remove UFS PCI driver's ->late_init() call back
	Adjust for change in patch 2
	Add Bart's Rev'd-by

    scsi: ufs: core: Remove duplicated code in ufshcd_send_bsg_uic_cmd()
    scsi: ufs: core: Set and clear UIC Completion interrupt as needed
    scsi: ufs: core: Do not write interrupt enable register unnecessarily
    scsi: ufs: ufs-pci: Remove control of UIC Completion interrupt for Intel MTL
	New patches


Adrian Hunter (7):
      scsi: ufs: ufs-pci: Fix default runtime and system PM levels
      scsi: ufs: ufs-pci: Remove UFS PCI driver's ->late_init() call back
      scsi: ufs: core: Move ufshcd_enable_intr() and ufshcd_disable_intr()
      scsi: ufs: core: Remove duplicated code in ufshcd_send_bsg_uic_cmd()
      scsi: ufs: core: Set and clear UIC Completion interrupt as needed
      scsi: ufs: core: Do not write interrupt enable register unnecessarily
      scsi: ufs: ufs-pci: Remove control of UIC Completion interrupt for Intel MTL

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

 drivers/ufs/core/ufshcd.c     | 95 ++++++++++++++++++-------------------------
 drivers/ufs/host/ufshcd-pci.c | 33 ++++-----------
 2 files changed, 49 insertions(+), 79 deletions(-)


Regards
Adrian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 1/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
@ 2025-07-23 16:58 ` Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 2/8] scsi: ufs: ufs-pci: Fix default runtime and system PM levels Adrian Hunter
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Adrian Hunter @ 2025-07-23 16:58 UTC (permalink / raw)
  To: Martin K Petersen
  Cc: James EJ Bottomley, Bart Van Assche, Avri Altman, Archana Patni,
	linux-scsi

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 2/8] scsi: ufs: ufs-pci: Fix default runtime and system PM levels
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 1/8] " Adrian Hunter
@ 2025-07-23 16:58 ` Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 3/8] scsi: ufs: ufs-pci: Remove UFS PCI driver's ->late_init() call back Adrian Hunter
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Adrian Hunter @ 2025-07-23 16:58 UTC (permalink / raw)
  To: Martin K Petersen
  Cc: James EJ Bottomley, Bart Van Assche, Avri Altman, Archana Patni,
	linux-scsi

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


Changes in V2:
	Add comment about getting variant after it is set


 drivers/ufs/host/ufshcd-pci.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
index af1c272eef1c..8aff32d7057d 100644
--- a/drivers/ufs/host/ufshcd-pci.c
+++ b/drivers/ufs/host/ufshcd-pci.c
@@ -468,10 +468,23 @@ 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);
+	/* Get variant after it is set in ufs_intel_common_init() */
+	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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 3/8] scsi: ufs: ufs-pci: Remove UFS PCI driver's ->late_init() call back
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 1/8] " Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 2/8] scsi: ufs: ufs-pci: Fix default runtime and system PM levels Adrian Hunter
@ 2025-07-23 16:58 ` Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 4/8] scsi: ufs: core: Move ufshcd_enable_intr() and ufshcd_disable_intr() Adrian Hunter
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Adrian Hunter @ 2025-07-23 16:58 UTC (permalink / raw)
  To: Martin K Petersen
  Cc: James EJ Bottomley, Bart Van Assche, Avri Altman, Archana Patni,
	linux-scsi

 ->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>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:
	Adjust for change in patch 2
	Add Bart's Rev'd-by


 drivers/ufs/host/ufshcd-pci.c | 46 +++++++----------------------------
 1 file changed, 9 insertions(+), 37 deletions(-)

diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
index 8aff32d7057d..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,23 +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);
-	/* Get variant after it is set in ufs_intel_common_init() */
-	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)
@@ -614,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;
@@ -647,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 4/8] scsi: ufs: core: Move ufshcd_enable_intr() and ufshcd_disable_intr()
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
                   ` (2 preceding siblings ...)
  2025-07-23 16:58 ` [PATCH V2 3/8] scsi: ufs: ufs-pci: Remove UFS PCI driver's ->late_init() call back Adrian Hunter
@ 2025-07-23 16:58 ` Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 5/8] scsi: ufs: core: Remove duplicated code in ufshcd_send_bsg_uic_cmd() Adrian Hunter
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Adrian Hunter @ 2025-07-23 16:58 UTC (permalink / raw)
  To: Martin K Petersen
  Cc: James EJ Bottomley, Bart Van Assche, Avri Altman, Archana Patni,
	linux-scsi

Move ufshcd_enable_intr() and ufshcd_disable_intr() so they can be called
in subsequent patches without forward declarations.

No functional change.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/ufs/core/ufshcd.c | 52 +++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index acfc1b4691fa..2fbd44514308 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))
@@ -2681,32 +2707,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
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 5/8] scsi: ufs: core: Remove duplicated code in ufshcd_send_bsg_uic_cmd()
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
                   ` (3 preceding siblings ...)
  2025-07-23 16:58 ` [PATCH V2 4/8] scsi: ufs: core: Move ufshcd_enable_intr() and ufshcd_disable_intr() Adrian Hunter
@ 2025-07-23 16:58 ` Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 6/8] scsi: ufs: core: Set and clear UIC Completion interrupt as needed Adrian Hunter
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Adrian Hunter @ 2025-07-23 16:58 UTC (permalink / raw)
  To: Martin K Petersen
  Cc: James EJ Bottomley, Bart Van Assche, Avri Altman, Archana Patni,
	linux-scsi

Make ufshcd_send_bsg_uic_cmd() call ufshcd_send_uic_cmd() instead of
duplicating its code.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/ufs/core/ufshcd.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2fbd44514308..7fb0ca3576d4 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4405,28 +4405,17 @@ int ufshcd_send_bsg_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
 {
 	int ret;
 
+	if (uic_cmd->argument1 != UIC_ARG_MIB(PA_PWRMODE) ||
+	    uic_cmd->command != UIC_CMD_DME_SET)
+		return ufshcd_send_uic_cmd(hba, uic_cmd);
+
 	if (hba->quirks & UFSHCD_QUIRK_BROKEN_UIC_CMD)
 		return 0;
 
 	ufshcd_hold(hba);
-
-	if (uic_cmd->argument1 == UIC_ARG_MIB(PA_PWRMODE) &&
-	    uic_cmd->command == UIC_CMD_DME_SET) {
-		ret = ufshcd_uic_pwr_ctrl(hba, uic_cmd);
-		goto out;
-	}
-
-	mutex_lock(&hba->uic_cmd_mutex);
-	ufshcd_add_delay_before_dme_cmd(hba);
-
-	ret = __ufshcd_send_uic_cmd(hba, uic_cmd);
-	if (!ret)
-		ret = ufshcd_wait_for_uic_cmd(hba, uic_cmd);
-
-	mutex_unlock(&hba->uic_cmd_mutex);
-
-out:
+	ret = ufshcd_uic_pwr_ctrl(hba, uic_cmd);
 	ufshcd_release(hba);
+
 	return ret;
 }
 
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 6/8] scsi: ufs: core: Set and clear UIC Completion interrupt as needed
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
                   ` (4 preceding siblings ...)
  2025-07-23 16:58 ` [PATCH V2 5/8] scsi: ufs: core: Remove duplicated code in ufshcd_send_bsg_uic_cmd() Adrian Hunter
@ 2025-07-23 16:58 ` Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 7/8] scsi: ufs: core: Do not write interrupt enable register unnecessarily Adrian Hunter
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Adrian Hunter @ 2025-07-23 16:58 UTC (permalink / raw)
  To: Martin K Petersen
  Cc: James EJ Bottomley, Bart Van Assche, Avri Altman, Archana Patni,
	linux-scsi

Currently the UIC Completion interrupt is left enabled except for when
issuing link hibernate commands, in which case the interrupt is disabled
and then re-enabled.

Instead, set and clear the interrupt enable bit as needed.

That is slightly simpler and less error prone, but also avoids side effects
of accessing the interrupt enable register after entering link hibernation.
Specifically, for some host controllers like Intel MTL, doing so disrupts
the link state transition.

Note also, the interrupt register is not read back anymore after it is
updated.  No other code does that, so it is assumed to be no longer
necessary if it ever was.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/ufs/core/ufshcd.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 7fb0ca3576d4..1567aba866b5 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2622,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)
@@ -2631,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);
@@ -4318,7 +4323,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);
@@ -4329,15 +4333,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) {
@@ -4381,8 +4377,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);
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 7/8] scsi: ufs: core: Do not write interrupt enable register unnecessarily
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
                   ` (5 preceding siblings ...)
  2025-07-23 16:58 ` [PATCH V2 6/8] scsi: ufs: core: Set and clear UIC Completion interrupt as needed Adrian Hunter
@ 2025-07-23 16:58 ` Adrian Hunter
  2025-07-23 16:58 ` [PATCH V2 8/8] scsi: ufs: ufs-pci: Remove control of UIC Completion interrupt for Intel MTL Adrian Hunter
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Adrian Hunter @ 2025-07-23 16:58 UTC (permalink / raw)
  To: Martin K Petersen
  Cc: James EJ Bottomley, Bart Van Assche, Avri Altman, Archana Patni,
	linux-scsi

Write a new value to the interrupt enable register only if it is different
from the old value, thereby saving a register write operation.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/ufs/core/ufshcd.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 1567aba866b5..df475b3fb7c5 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -371,10 +371,11 @@ EXPORT_SYMBOL_GPL(ufshcd_disable_irq);
  */
 static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
 {
-	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+	u32 old_val = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+	u32 new_val = old_val | intrs;
 
-	set |= intrs;
-	ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
+	if (new_val != old_val)
+		ufshcd_writel(hba, new_val, REG_INTERRUPT_ENABLE);
 }
 
 /**
@@ -384,10 +385,11 @@ static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
  */
 static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
 {
-	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+	u32 old_val = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+	u32 new_val = old_val & ~intrs;
 
-	set &= ~intrs;
-	ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
+	if (new_val != old_val)
+		ufshcd_writel(hba, new_val, REG_INTERRUPT_ENABLE);
 }
 
 static void ufshcd_configure_wb(struct ufs_hba *hba)
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 8/8] scsi: ufs: ufs-pci: Remove control of UIC Completion interrupt for Intel MTL
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
                   ` (6 preceding siblings ...)
  2025-07-23 16:58 ` [PATCH V2 7/8] scsi: ufs: core: Do not write interrupt enable register unnecessarily Adrian Hunter
@ 2025-07-23 16:58 ` Adrian Hunter
  2025-07-23 17:14 ` [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Bart Van Assche
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Adrian Hunter @ 2025-07-23 16:58 UTC (permalink / raw)
  To: Martin K Petersen
  Cc: James EJ Bottomley, Bart Van Assche, Avri Altman, Archana Patni,
	linux-scsi

Now that UFS core enables the UIC Completion interrupt only when needed,
Intel MTL driver no longer needs to control the interrupt itself.  So
remove the associated code.

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

diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
index b29ec1904482..b39239f641f2 100644
--- a/drivers/ufs/host/ufshcd-pci.c
+++ b/drivers/ufs/host/ufshcd-pci.c
@@ -211,32 +211,6 @@ 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
 
@@ -549,7 +523,6 @@ 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
                   ` (7 preceding siblings ...)
  2025-07-23 16:58 ` [PATCH V2 8/8] scsi: ufs: ufs-pci: Remove control of UIC Completion interrupt for Intel MTL Adrian Hunter
@ 2025-07-23 17:14 ` Bart Van Assche
  2025-07-25  2:46 ` Martin K. Petersen
  2025-07-31  4:44 ` Martin K. Petersen
  10 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2025-07-23 17:14 UTC (permalink / raw)
  To: Adrian Hunter, Martin K Petersen
  Cc: James EJ Bottomley, Avri Altman, Archana Patni, linux-scsi

On 7/23/25 9:58 AM, Adrian Hunter wrote:
> Here is V2 of a couple of fixes for Intel MTL-like UFS host controllers,
> related to link Hibernation state.
> 
> Following the fixes are some improvements for the enabling and disabling
> of UIC Completion interrupts.
Nice work!

For the entire series:

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
                   ` (8 preceding siblings ...)
  2025-07-23 17:14 ` [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Bart Van Assche
@ 2025-07-25  2:46 ` Martin K. Petersen
  2025-07-31  4:44 ` Martin K. Petersen
  10 siblings, 0 replies; 12+ messages in thread
From: Martin K. Petersen @ 2025-07-25  2:46 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Martin K Petersen, James EJ Bottomley, Bart Van Assche,
	Avri Altman, Archana Patni, linux-scsi


Adrian,

> Here is V2 of a couple of fixes for Intel MTL-like UFS host
> controllers, related to link Hibernation state.
>
> Following the fixes are some improvements for the enabling and
> disabling of UIC Completion interrupts.

Applied to 6.17/scsi-staging, thanks!

-- 
Martin K. Petersen

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers
  2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
                   ` (9 preceding siblings ...)
  2025-07-25  2:46 ` Martin K. Petersen
@ 2025-07-31  4:44 ` Martin K. Petersen
  10 siblings, 0 replies; 12+ messages in thread
From: Martin K. Petersen @ 2025-07-31  4:44 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Martin K . Petersen, James EJ Bottomley, Bart Van Assche,
	Avri Altman, Archana Patni, linux-scsi

On Wed, 23 Jul 2025 19:58:48 +0300, Adrian Hunter wrote:

> Here is V2 of a couple of fixes for Intel MTL-like UFS host controllers,
> related to link Hibernation state.
> 
> Following the fixes are some improvements for the enabling and disabling
> of UIC Completion interrupts.
> 
> 
> [...]

Applied to 6.17/scsi-queue, thanks!

[1/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers
      https://git.kernel.org/mkp/scsi/c/4428ddea832c
[2/8] scsi: ufs: ufs-pci: Fix default runtime and system PM levels
      https://git.kernel.org/mkp/scsi/c/6de7435e6b81
[3/8] scsi: ufs: ufs-pci: Remove UFS PCI driver's ->late_init() call back
      https://git.kernel.org/mkp/scsi/c/28a60bbbe739
[4/8] scsi: ufs: core: Move ufshcd_enable_intr() and ufshcd_disable_intr()
      https://git.kernel.org/mkp/scsi/c/497027eade8c
[5/8] scsi: ufs: core: Remove duplicated code in ufshcd_send_bsg_uic_cmd()
      https://git.kernel.org/mkp/scsi/c/c5977c4c0731
[6/8] scsi: ufs: core: Set and clear UIC Completion interrupt as needed
      https://git.kernel.org/mkp/scsi/c/b4c0cab4eb8d
[7/8] scsi: ufs: core: Do not write interrupt enable register unnecessarily
      https://git.kernel.org/mkp/scsi/c/d402b20f9c31
[8/8] scsi: ufs: ufs-pci: Remove control of UIC Completion interrupt for Intel MTL
      https://git.kernel.org/mkp/scsi/c/22b246e3fc5e

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-07-31  4:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-23 16:58 [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Adrian Hunter
2025-07-23 16:58 ` [PATCH V2 1/8] " Adrian Hunter
2025-07-23 16:58 ` [PATCH V2 2/8] scsi: ufs: ufs-pci: Fix default runtime and system PM levels Adrian Hunter
2025-07-23 16:58 ` [PATCH V2 3/8] scsi: ufs: ufs-pci: Remove UFS PCI driver's ->late_init() call back Adrian Hunter
2025-07-23 16:58 ` [PATCH V2 4/8] scsi: ufs: core: Move ufshcd_enable_intr() and ufshcd_disable_intr() Adrian Hunter
2025-07-23 16:58 ` [PATCH V2 5/8] scsi: ufs: core: Remove duplicated code in ufshcd_send_bsg_uic_cmd() Adrian Hunter
2025-07-23 16:58 ` [PATCH V2 6/8] scsi: ufs: core: Set and clear UIC Completion interrupt as needed Adrian Hunter
2025-07-23 16:58 ` [PATCH V2 7/8] scsi: ufs: core: Do not write interrupt enable register unnecessarily Adrian Hunter
2025-07-23 16:58 ` [PATCH V2 8/8] scsi: ufs: ufs-pci: Remove control of UIC Completion interrupt for Intel MTL Adrian Hunter
2025-07-23 17:14 ` [PATCH V2 0/8] scsi: ufs: ufs-pci: Fix hibernate state transition for Intel MTL-like host controllers Bart Van Assche
2025-07-25  2:46 ` Martin K. Petersen
2025-07-31  4:44 ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox