[PATCH 0/8] Eight small UFS patches

[PATCH 0/8] Eight small UFS patches

[Bart Van Assche]

Hi Martin,

This patch series includes two bug fixes for this development cycle and six
small patches that are intended for the next merge window. If applying the
first two patches only during the current development cycle would be
inconvenient, postponing all patches until the next merge window is fine with
me.

Please consider including these patches in the upstream kernel. 

Thanks,

Bart.

Bart Van Assche (8):
  ufs: core: Fix a race condition related to the "hid" attribute group
  ufs: core: Reduce link startup failure logging
  ufs: core: Improve documentation in include/ufs/ufshci.h
  ufs: core: Change the type of uic_command::cmd_active
  ufs: core: Remove UFS_DEV_COMP
  ufs: core: Move the ufshcd_enable_intr() declaration
  ufs: core: Remove a goto label from ufshcd_uic_cmd_compl()
  ufs: core: Simplify ufshcd_mcq_sq_cleanup() using guard()

 drivers/ufs/core/ufs-mcq.c         |  6 ++----
 drivers/ufs/core/ufs_trace.h       |  1 -
 drivers/ufs/core/ufs_trace_types.h |  1 -
 drivers/ufs/core/ufshcd-priv.h     |  2 ++
 drivers/ufs/core/ufshcd.c          | 17 ++++++-----------
 include/ufs/ufshcd.h               |  3 +--
 include/ufs/ufshci.h               |  3 +++
 7 files changed, 14 insertions(+), 19 deletions(-)

[PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Bart Van Assche]

ufs_sysfs_add_nodes() is called concurrently with ufs_get_device_desc().
This may cause the following code to be called before
ufs_sysfs_add_nodes():

	sysfs_update_group(&hba->dev->kobj, &ufs_sysfs_hid_group);

If this happens, ufs_sysfs_add_nodes() triggers a kernel warning and
fails. Fix this by calling ufs_sysfs_add_nodes() before SCSI LUNs are
scanned since the sysfs_update_group() call happens from the context of
thread that executes ufshcd_async_scan(). This patch fixes the following
kernel warning:

sysfs: cannot create duplicate filename '/devices/platform/3c2d0000.ufs/hid'
Workqueue: async async_run_entry_fn
Call trace:
 dump_backtrace+0xfc/0x17c
 show_stack+0x18/0x28
 dump_stack_lvl+0x40/0x104
 dump_stack+0x18/0x3c
 sysfs_warn_dup+0x6c/0xc8
 internal_create_group+0x1c8/0x504
 sysfs_create_groups+0x38/0x9c
 ufs_sysfs_add_nodes+0x20/0x58
 ufshcd_init+0x1114/0x134c
 ufshcd_pltfrm_init+0x728/0x7d8
 ufs_google_probe+0x30/0x84
 platform_probe+0xa0/0xe0
 really_probe+0x114/0x454
 __driver_probe_device+0xa4/0x160
 driver_probe_device+0x44/0x23c
 __device_attach_driver+0x15c/0x1f4
 bus_for_each_drv+0x10c/0x168
 __device_attach_async_helper+0x80/0xf8
 async_run_entry_fn+0x4c/0x17c
 process_one_work+0x26c/0x65c
 worker_thread+0x33c/0x498
 kthread+0x110/0x134
 ret_from_fork+0x10/0x20
ufshcd 3c2d0000.ufs: ufs_sysfs_add_nodes: sysfs groups creation failed (err = -17)

Cc: Daniel Lee <chullee@google.com>
Fixes: bb7663dec67b ("scsi: ufs: sysfs: Make HID attributes visible")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 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 8339fec975b9..26c2aafd7338 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10885,8 +10885,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	if (err)
 		goto out_disable;
 
-	async_schedule(ufshcd_async_scan, hba);
 	ufs_sysfs_add_nodes(hba->dev);
+	async_schedule(ufshcd_async_scan, hba);
 
 	device_enable_async_suspend(dev);
 	ufshcd_pm_qos_init(hba);

[PATCH 2/8] ufs: core: Reduce link startup failure logging

[Bart Van Assche]

Some systems, e.g. Rock 4D, have a pluggable UFS module. Link startup
fails systematically on these systems. If no UFS module has been plugged
in, more than fourty lines are logged after the "link startup failed"
message. Avoid this by reducing link startup failure logging.

An intended side effect of this patch is that scsi_host_busy() is not
called before scsi_add_host() is called.

Commit 995412e23bb2 ("blk-mq: Replace tags->lock with SRCU for tag
iterators") introduced a regression - the warning shown below is
triggered during every boot. This patch fixes that regression.

Call trace:
 __srcu_read_lock+0x30/0x80 (P)
 blk_mq_tagset_busy_iter+0x44/0x300
 scsi_host_busy+0x38/0x70
 ufshcd_print_host_state+0x34/0x1bc
 ufshcd_link_startup.constprop.0+0xe4/0x2e0
 ufshcd_init+0x944/0xf80
 ufshcd_pltfrm_init+0x504/0x820
 ufs_rockchip_probe+0x2c/0x88
 platform_probe+0x5c/0xa4
 really_probe+0xc0/0x38c
 __driver_probe_device+0x7c/0x150
 driver_probe_device+0x40/0x120
 __driver_attach+0xc8/0x1e0
 bus_for_each_dev+0x7c/0xdc
 driver_attach+0x24/0x30
 bus_add_driver+0x110/0x230
 driver_register+0x68/0x130
 __platform_driver_register+0x20/0x2c
 ufs_rockchip_pltform_init+0x1c/0x28
 do_one_initcall+0x60/0x1e0
 kernel_init_freeable+0x248/0x2c4
 kernel_init+0x20/0x140
 ret_from_fork+0x10/0x20

Reported-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Closes: https://lore.kernel.org/linux-block/pnezafputodmqlpumwfbn644ohjybouveehcjhz2hmhtcf2rka@sdhoiivync4y/
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 26c2aafd7338..fa20082ac4d4 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5131,12 +5131,8 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
 	ufshcd_readl(hba, REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
 	ret = ufshcd_make_hba_operational(hba);
 out:
-	if (ret) {
+	if (ret)
 		dev_err(hba->dev, "link startup failed %d\n", ret);
-		ufshcd_print_host_state(hba);
-		ufshcd_print_pwr_info(hba);
-		ufshcd_print_evt_hist(hba);
-	}
 	return ret;
 }
 

[PATCH 3/8] ufs: core: Improve documentation in include/ufs/ufshci.h

[Bart Van Assche]

Make it easier to find the sections in the UFSHCI standard where these
constants come from.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/ufs/ufshci.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
index e64b70132101..ff96056b2ac3 100644
--- a/include/ufs/ufshci.h
+++ b/include/ufs/ufshci.h
@@ -83,12 +83,14 @@ enum {
 };
 
 enum {
+	/* Submission Queue (SQ) Configuration Registers */
 	REG_SQATTR		= 0x0,
 	REG_SQLBA		= 0x4,
 	REG_SQUBA		= 0x8,
 	REG_SQDAO		= 0xC,
 	REG_SQISAO		= 0x10,
 
+	/* Completion Queue (CQ) Configuration Registers */
 	REG_CQATTR		= 0x20,
 	REG_CQLBA		= 0x24,
 	REG_CQUBA		= 0x28,
@@ -96,6 +98,7 @@ enum {
 	REG_CQISAO		= 0x30,
 };
 
+/* Operation and Runtime Registers - Submission Queues and Completion Queues */
 enum {
 	REG_SQHP		= 0x0,
 	REG_SQTP		= 0x4,

[PATCH 4/8] ufs: core: Change the type of uic_command::cmd_active

[Bart Van Assche]

Since uic_command::cmd_active is used as a boolean variable, change its
type from 'int' into 'bool'. No functionality has been changed.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 6 +++---
 include/ufs/ufshcd.h      | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index fa20082ac4d4..43e527926bf3 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2618,7 +2618,7 @@ __ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
 
 	init_completion(&uic_cmd->done);
 
-	uic_cmd->cmd_active = 1;
+	uic_cmd->cmd_active = true;
 	ufshcd_dispatch_uic_cmd(hba, uic_cmd);
 
 	return 0;
@@ -5583,13 +5583,13 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
 		cmd->argument2 |= ufshcd_get_uic_cmd_result(hba);
 		cmd->argument3 = ufshcd_get_dme_attr_val(hba);
 		if (!hba->uic_async_done)
-			cmd->cmd_active = 0;
+			cmd->cmd_active = false;
 		complete(&cmd->done);
 		retval = IRQ_HANDLED;
 	}
 
 	if (intr_status & UFSHCD_UIC_PWR_MASK && hba->uic_async_done) {
-		cmd->cmd_active = 0;
+		cmd->cmd_active = false;
 		complete(hba->uic_async_done);
 		retval = IRQ_HANDLED;
 	}
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 9425cfd9d00e..4d215a18522c 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -78,7 +78,7 @@ struct uic_command {
 	const u32 argument1;
 	u32 argument2;
 	u32 argument3;
-	int cmd_active;
+	bool cmd_active;
 	struct completion done;
 };
 

[PATCH 5/8] ufs: core: Remove UFS_DEV_COMP

[Bart Van Assche]

Remove the UFS_DEV_COMP constant because it is not used.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufs_trace.h       | 1 -
 drivers/ufs/core/ufs_trace_types.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/ufs/core/ufs_trace.h b/drivers/ufs/core/ufs_trace.h
index 584c2b5c6ad9..309ae51b4906 100644
--- a/drivers/ufs/core/ufs_trace.h
+++ b/drivers/ufs/core/ufs_trace.h
@@ -42,7 +42,6 @@
 #define UFS_CMD_TRACE_STRINGS					\
 	EM(UFS_CMD_SEND,	"send_req")			\
 	EM(UFS_CMD_COMP,	"complete_rsp")			\
-	EM(UFS_DEV_COMP,	"dev_complete")			\
 	EM(UFS_QUERY_SEND,	"query_send")			\
 	EM(UFS_QUERY_COMP,	"query_complete")		\
 	EM(UFS_QUERY_ERR,	"query_complete_err")		\
diff --git a/drivers/ufs/core/ufs_trace_types.h b/drivers/ufs/core/ufs_trace_types.h
index f2d5ad1d92b9..bf821970f092 100644
--- a/drivers/ufs/core/ufs_trace_types.h
+++ b/drivers/ufs/core/ufs_trace_types.h
@@ -5,7 +5,6 @@
 enum ufs_trace_str_t {
 	UFS_CMD_SEND,
 	UFS_CMD_COMP,
-	UFS_DEV_COMP,
 	UFS_QUERY_SEND,
 	UFS_QUERY_COMP,
 	UFS_QUERY_ERR,

[PATCH 6/8] ufs: core: Move the ufshcd_enable_intr() declaration

[Bart Van Assche]

ufshcd_enable_intr() is not exported and hence should not be declared in
include/ufs/ufshcd.h.

Fixes: 253757797973 ("scsi: ufs: core: Change MCQ interrupt enable flow")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd-priv.h | 2 ++
 include/ufs/ufshcd.h           | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index d0a2c963a27d..1f0d38aa37f9 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -6,6 +6,8 @@
 #include <linux/pm_runtime.h>
 #include <ufs/ufshcd.h>
 
+void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs);
+
 static inline bool ufshcd_is_user_access_allowed(struct ufs_hba *hba)
 {
 	return !hba->shutting_down;
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 4d215a18522c..edfbc3a216be 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1295,7 +1295,6 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
 
 void ufshcd_enable_irq(struct ufs_hba *hba);
 void ufshcd_disable_irq(struct ufs_hba *hba);
-void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs);
 int ufshcd_alloc_host(struct device *, struct ufs_hba **);
 int ufshcd_hba_enable(struct ufs_hba *hba);
 int ufshcd_init(struct ufs_hba *, void __iomem *, unsigned int);

[PATCH 7/8] ufs: core: Remove a goto label from ufshcd_uic_cmd_compl()

[Bart Van Assche]

Return directly instead of jumping to a return statement. No
functionality has been changed.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 43e527926bf3..633fad65122e 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5574,7 +5574,7 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
 	guard(spinlock_irqsave)(hba->host->host_lock);
 	cmd = hba->active_uic_cmd;
 	if (!cmd)
-		goto unlock;
+		return retval;
 
 	if (ufshcd_is_auto_hibern8_error(hba, intr_status))
 		hba->errors |= (UFSHCD_UIC_HIBERN8_MASK & intr_status);
@@ -5597,7 +5597,6 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
 	if (retval == IRQ_HANDLED)
 		ufshcd_add_uic_command_trace(hba, cmd, UFS_CMD_COMP);
 
-unlock:
 	return retval;
 }
 

[PATCH 8/8] ufs: core: Simplify ufshcd_mcq_sq_cleanup() using guard()

[Bart Van Assche]

Simplify ufshcd_mcq_sq_cleanup() by using guard(mutex)() instead of
explicit mutex_lock() and mutex_unlock() calls. No functionality has
been changed.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufs-mcq.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index c9bdd4140fd0..d04662b57cd1 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -568,12 +568,12 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag)
 
 	id = hwq->id;
 
-	mutex_lock(&hwq->sq_mutex);
+	guard(mutex)(&hwq->sq_mutex);
 
 	/* stop the SQ fetching before working on it */
 	err = ufshcd_mcq_sq_stop(hba, hwq);
 	if (err)
-		goto unlock;
+		return err;
 
 	/* SQCTI = EXT_IID, IID, LUN, Task Tag */
 	nexus = lrbp->lun << 8 | task_tag;
@@ -600,8 +600,6 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag)
 	if (ufshcd_mcq_sq_start(hba, hwq))
 		err = -ETIMEDOUT;
 
-unlock:
-	mutex_unlock(&hwq->sq_mutex);
 	return err;
 }
 

Re: [PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Bart Van Assche]

On 10/14/25 1:00 PM, Bart Van Assche wrote:
> ufs_sysfs_add_nodes() is called concurrently with ufs_get_device_desc().
> This may cause the following code to be called before
> ufs_sysfs_add_nodes():
(replying to my own email)

I'm considering to replace this patch with the patch below:

diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 875aab4a9cce..05f700f9cfa7 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -2086,18 +2086,23 @@ const struct attribute_group 
ufs_sysfs_lun_attributes_group = {
  	.attrs = ufs_sysfs_lun_attributes,
  };

-void ufs_sysfs_add_nodes(struct device *dev)
+void ufs_sysfs_add_nodes(struct ufs_hba *hba)
  {
+	struct device *dev = hba->dev;
  	int ret;

  	ret = sysfs_create_groups(&dev->kobj, ufs_sysfs_groups);
-	if (ret)
+	if (ret) {
  		dev_err(dev,
  			"%s: sysfs groups creation failed (err = %d)\n",
  			__func__, ret);
+	} else {
+		WRITE_ONCE(hba->sysfs_attrs_added, true);
+	}
  }

-void ufs_sysfs_remove_nodes(struct device *dev)
+void ufs_sysfs_remove_nodes(struct ufs_hba *hba)
  {
-	sysfs_remove_groups(&dev->kobj, ufs_sysfs_groups);
+	WRITE_ONCE(hba->sysfs_attrs_added, false);
+	sysfs_remove_groups(&hba->dev->kobj, ufs_sysfs_groups);
  }
diff --git a/drivers/ufs/core/ufs-sysfs.h b/drivers/ufs/core/ufs-sysfs.h
index 6efb82a082fd..c9e3751c6793 100644
--- a/drivers/ufs/core/ufs-sysfs.h
+++ b/drivers/ufs/core/ufs-sysfs.h
@@ -8,9 +8,10 @@
  #include <linux/sysfs.h>

  struct device;
+struct ufs_hba;

-void ufs_sysfs_add_nodes(struct device *dev);
-void ufs_sysfs_remove_nodes(struct device *dev);
+void ufs_sysfs_add_nodes(struct ufs_hba *hba);
+void ufs_sysfs_remove_nodes(struct ufs_hba *hba);

  extern const struct attribute_group ufs_sysfs_unit_descriptor_group;
  extern const struct attribute_group ufs_sysfs_lun_attributes_group;
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 0013d21e2169..2887c5623992 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8604,7 +8604,8 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
  				DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP) &
  				UFS_DEV_HID_SUPPORT;

-	sysfs_update_group(&hba->dev->kobj, &ufs_sysfs_hid_group);
+	if (READ_ONCE(hba->sysfs_attrs_added))
+		sysfs_update_group(&hba->dev->kobj, &ufs_sysfs_hid_group);

  	model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME];

@@ -10539,7 +10540,7 @@ void ufshcd_remove(struct ufs_hba *hba)
  		ufshcd_rpm_get_sync(hba);
  	ufs_hwmon_remove(hba);
  	ufs_bsg_remove(hba);
-	ufs_sysfs_remove_nodes(hba->dev);
+	ufs_sysfs_remove_nodes(hba);
  	cancel_delayed_work_sync(&hba->ufs_rtc_update_work);
  	blk_mq_destroy_queue(hba->tmf_queue);
  	blk_put_queue(hba->tmf_queue);
@@ -11003,8 +11004,8 @@ int ufshcd_init(struct ufs_hba *hba, void 
__iomem *mmio_base, unsigned int irq)
  	if (err)
  		goto out_disable;

+	ufs_sysfs_add_nodes(hba);
  	async_schedule(ufshcd_async_scan, hba);
-	ufs_sysfs_add_nodes(dev);
  	trace_android_vh_ufs_update_sysfs(hba);

  	device_enable_async_suspend(dev);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index fba73d28ad87..9d7a88e73ba5 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -905,6 +905,8 @@ enum ufshcd_mcq_opr {
   * @ee_ctrl_mutex: Used to serialize exception event information.
   * @is_powered: flag to check if HBA is powered
   * @shutting_down: flag to check if shutdown has been invoked
+ * @sysfs_attrs_added: Whether or not the sysfs attributes have been 
added to
+ *	hba->dev.
   * @host_sem: semaphore used to serialize concurrent contexts
   * @eh_wq: Workqueue that eh_work works on
   * @eh_work: Worker to handle UFS errors that require s/w attention
@@ -1054,6 +1056,7 @@ struct ufs_hba {
  	struct mutex ee_ctrl_mutex;
  	bool is_powered;
  	bool shutting_down;
+	bool sysfs_attrs_added;
  	struct semaphore host_sem;

  	/* Work Queues */

Re: [PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Peter Wang (王信友)]

On Tue, 2025-10-14 at 13:00 -0700, Bart Van Assche wrote:
> ufs_sysfs_add_nodes() is called concurrently with
> ufs_get_device_desc().
> This may cause the following code to be called before
> ufs_sysfs_add_nodes():
> 
>         sysfs_update_group(&hba->dev->kobj, &ufs_sysfs_hid_group);
> 
> If this happens, ufs_sysfs_add_nodes() triggers a kernel warning and
> fails. Fix this by calling ufs_sysfs_add_nodes() before SCSI LUNs are
> scanned since the sysfs_update_group() call happens from the context
> of
> thread that executes ufshcd_async_scan(). This patch fixes the
> following

Hi Bart,

How does the sysfs_update_group() call happen from the context
of the thread that executes ufshcd_async_scan()? 
Do you have a backtrace?

Thanks
Peter

Re: [PATCH 5/8] ufs: core: Remove UFS_DEV_COMP

[Peter Wang (王信友)]

On Tue, 2025-10-14 at 13:00 -0700, Bart Van Assche wrote:
> Remove the UFS_DEV_COMP constant because it is not used.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

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

Re: [PATCH 6/8] ufs: core: Move the ufshcd_enable_intr() declaration

[Peter Wang (王信友)]

On Tue, 2025-10-14 at 13:00 -0700, Bart Van Assche wrote:
> ufshcd_enable_intr() is not exported and hence should not be declared
> in
> include/ufs/ufshcd.h.
> 
> Fixes: 253757797973 ("scsi: ufs: core: Change MCQ interrupt enable
> flow")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

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

Re: [PATCH 7/8] ufs: core: Remove a goto label from ufshcd_uic_cmd_compl()

[Peter Wang (王信友)]

On Tue, 2025-10-14 at 13:00 -0700, Bart Van Assche wrote:
> Return directly instead of jumping to a return statement. No
> functionality has been changed.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

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

Re: [PATCH 8/8] ufs: core: Simplify ufshcd_mcq_sq_cleanup() using guard()

[Peter Wang (王信友)]

On Tue, 2025-10-14 at 13:01 -0700, Bart Van Assche wrote:
> Simplify ufshcd_mcq_sq_cleanup() by using guard(mutex)() instead of
> explicit mutex_lock() and mutex_unlock() calls. No functionality has
> been changed.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> 

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

Re: [PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Bart Van Assche]

On 10/17/25 1:27 AM, Peter Wang (王信友) wrote:
> How does the sysfs_update_group() call happen from the context
> of the thread that executes ufshcd_async_scan()?
> Do you have a backtrace?

Sure. The call chain is as follows:

ufshcd_async_scan()
   ufshcd_probe_hba(hba, true)
     ufshcd_device_init()
       ufshcd_device_params_init()
         ufs_get_device_desc()
           sysfs_update_group()

Please note that this call chain is only taken if the following quirk
has been set: UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH.

Thanks,

Bart.

Re: [PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Peter Wang (王信友)]

On Fri, 2025-10-17 at 09:25 -0700, Bart Van Assche wrote:
> Sure. The call chain is as follows:
> 
> ufshcd_async_scan()
>    ufshcd_probe_hba(hba, true)
>      ufshcd_device_init()
>        ufshcd_device_params_init()
>          ufs_get_device_desc()
>            sysfs_update_group()
> 
> 

Hi Bart,

It seems that ufs_get_device_desc() doesn’t call sysfs_update_group().
Did I miss something?

Thanks
Peter

Re: [PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Bart Van Assche]

On 10/19/25 11:51 PM, Peter Wang (王信友) wrote:
> On Fri, 2025-10-17 at 09:25 -0700, Bart Van Assche wrote:
>> Sure. The call chain is as follows:
>>
>> ufshcd_async_scan()
>>     ufshcd_probe_hba(hba, true)
>>       ufshcd_device_init()
>>         ufshcd_device_params_init()
>>           ufs_get_device_desc()
>>             sysfs_update_group()
> 
> It seems that ufs_get_device_desc() doesn’t call sysfs_update_group().
> Did I miss something?

Hi Peter,

Please take a look at commit bb7663dec67b ("scsi: ufs: sysfs: Make HID
attributes visible"). That commit has been merged in the upstream Linux
kernel during the v6.18 merge window. See also this pull request from
October 11:

https://lore.kernel.org/linux-scsi/6e77e31acee95ebcba03c54dc1b34173cbaf831e.camel@HansenPartnership.com/

Thanks,

Bart.

Re: [PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Peter Wang (王信友)]

On Mon, 2025-10-20 at 09:13 -0700, Bart Van Assche wrote:
> 
> Hi Peter,
> 
> Please take a look at commit bb7663dec67b ("scsi: ufs: sysfs: Make
> HID
> attributes visible"). That commit has been merged in the upstream
> Linux
> kernel during the v6.18 merge window. See also this pull request from
> October 11:
> 
> https://lore.kernel.org/linux-scsi/6e77e31acee95ebcba03c54dc1b34173cbaf831e.camel@HansenPartnership.com/
> 
> Thanks,
> 
> Bart.
> 

Hi Bart,

Apologies, I missed the HID patch. However, this change will
not affect only HID; other sysfs groups, such as
ufs_sysfs_device_descriptor_group, should also be updated?
Would it be better to ensure that ufshcd_async_scan 
completes before invoking ufs_sysfs_add_nodes?

Thanks
Peter

Re: [PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Bart Van Assche]

On 10/20/25 8:04 PM, Peter Wang (王信友) wrote:
> On Mon, 2025-10-20 at 09:13 -0700, Bart Van Assche wrote:
> Apologies, I missed the HID patch. However, this change will
> not affect only HID; other sysfs groups, such as
> ufs_sysfs_device_descriptor_group, should also be updated?
Hi Peter,

Calling sysfs_update_group() is only necessary for sysfs groups that
define an .is_visible callback and only after the input parameters for
these callback functions have been modified. Here is an overview of the
groups in which an .is_visible callback has been defined:

$ git grep -nHw is_visible drivers/ufs/core/ufs-sysfs.c
drivers/ufs/core/ufs-sysfs.c:1955:	.is_visible = ufs_sysfs_hid_is_visible,
drivers/ufs/core/ufs-sysfs.c:2048:	.is_visible = 
ufs_unit_descriptor_is_visible,

The ufs_sysfs_hid_group is the only group for which the 
sysfs_update_group() function ever has to be called.

 > Would it be better to ensure that ufshcd_async_scan
 > completes before invoking ufs_sysfs_add_nodes?

That would make the sysfs attributes unavailable if the LUN scan hangs.

Thanks,

Bart.

Re: [PATCH 0/8] Eight small UFS patches

[Martin K. Petersen]

Bart,

> This patch series includes two bug fixes for this development cycle
> and six small patches that are intended for the next merge window. If
> applying the first two patches only during the current development
> cycle would be inconvenient, postponing all patches until the next
> merge window is fine with me.
>
> Please consider including these patches in the upstream kernel. 

Applied to 6.19/scsi-staging, thanks!

-- 
Martin K. Petersen

Re: [PATCH 0/8] Eight small UFS patches

[Manivannan Sadhasivam]

On Tue, Oct 21, 2025 at 10:13:39PM -0400, Martin K. Petersen wrote:
> 
> Bart,
> 
> > This patch series includes two bug fixes for this development cycle
> > and six small patches that are intended for the next merge window. If
> > applying the first two patches only during the current development
> > cycle would be inconvenient, postponing all patches until the next
> > merge window is fine with me.
> >
> > Please consider including these patches in the upstream kernel. 
> 
> Applied to 6.19/scsi-staging, thanks!
> 

Martin, could you please apply the first two patches to scsi-fixes? They are
fixing bugs introduced in v6.18-rc1.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Peter Wang (王信友)]

On Tue, 2025-10-21 at 08:01 -0700, Bart Van Assche wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 10/20/25 8:04 PM, Peter Wang (王信友) wrote:
> > On Mon, 2025-10-20 at 09:13 -0700, Bart Van Assche wrote:
> > Apologies, I missed the HID patch. However, this change will
> > not affect only HID; other sysfs groups, such as
> > ufs_sysfs_device_descriptor_group, should also be updated?
> Hi Peter,
> 
> Calling sysfs_update_group() is only necessary for sysfs groups that
> define an .is_visible callback and only after the input parameters
> for
> these callback functions have been modified. Here is an overview of
> the
> groups in which an .is_visible callback has been defined:

Hi Bart,

Thank you for your detailed explanation.

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

Re: [PATCH 0/8] Eight small UFS patches

[Bart Van Assche]

On 10/22/25 4:04 AM, Manivannan Sadhasivam wrote:
> On Tue, Oct 21, 2025 at 10:13:39PM -0400, Martin K. Petersen wrote:
>>> This patch series includes two bug fixes for this development cycle
>>> and six small patches that are intended for the next merge window. If
>>> applying the first two patches only during the current development
>>> cycle would be inconvenient, postponing all patches until the next
>>> merge window is fine with me.
>>>
>>> Please consider including these patches in the upstream kernel.
>>
>> Applied to 6.19/scsi-staging, thanks!
> 
> Martin, could you please apply the first two patches to scsi-fixes? They are
> fixing bugs introduced in v6.18-rc1.

I'm not sure that's the best approach. The more patches that are moved
from scsi-staging into scsi-fixes, the more likely it becomes that Linus
will have to resolve a merge conflict during the next merge window.

Thanks,

Bart.

Re: [PATCH 0/8] Eight small UFS patches

[Manivannan Sadhasivam]

On Wed, Oct 22, 2025 at 09:59:09AM -0700, Bart Van Assche wrote:
> On 10/22/25 4:04 AM, Manivannan Sadhasivam wrote:
> > On Tue, Oct 21, 2025 at 10:13:39PM -0400, Martin K. Petersen wrote:
> > > > This patch series includes two bug fixes for this development cycle
> > > > and six small patches that are intended for the next merge window. If
> > > > applying the first two patches only during the current development
> > > > cycle would be inconvenient, postponing all patches until the next
> > > > merge window is fine with me.
> > > > 
> > > > Please consider including these patches in the upstream kernel.
> > > 
> > > Applied to 6.19/scsi-staging, thanks!
> > 
> > Martin, could you please apply the first two patches to scsi-fixes? They are
> > fixing bugs introduced in v6.18-rc1.
> 
> I'm not sure that's the best approach. The more patches that are moved
> from scsi-staging into scsi-fixes, the more likely it becomes that Linus
> will have to resolve a merge conflict during the next merge window.
> 

I don't see how there will be merge conflict unless the remaining patches (3-8)
depend on fixes 1-2. In the cover letter, you also mentioned that the first two
patches are bug fixes targeted for v6.18-rc1.

I echoed the same thing since without these fixes, boards running v6.18-rc1 are
throwing a bunch of warnings making it inconvenient to use.

Ideally, we should try to fix the newly introduced warnings in the current
release itself without relying on stable maintainers to backport the fixes.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 0/8] Eight small UFS patches

[Bart Van Assche]

On 10/22/25 10:31 AM, Manivannan Sadhasivam wrote:
> I don't see how there will be merge conflict unless the remaining patches (3-8)
> depend on fixes 1-2. In the cover letter, you also mentioned that the first two
> patches are bug fixes targeted for v6.18-rc1.

With my comment about potential merge conflicts I was referring to this
patch series: "[PATCH v6 00/28] Optimize the hot path in the UFS driver"
(https://lore.kernel.org/linux-scsi/20251014201707.3396650-1-bvanassche@acm.org/).

> I echoed the same thing since without these fixes, boards running v6.18-rc1 are
> throwing a bunch of warnings making it inconvenient to use.
> 
> Ideally, we should try to fix the newly introduced warnings in the current
> release itself without relying on stable maintainers to backport the fixes.

Agreed that warnings/bugs introduced during the merge window should be
fixed during the rc phase. My understanding is that kernel patches
should be sent only once to Linus Torvalds. Hence, kernel patches should
exist either on the SCSI fixes branch or on the SCSI staging branch but
not on both branches at the same time.

Martin, is it still possible to migrate the first two patches from this 
series from the staging branch to the fixes branch?

Please note that patch 1/8 from this series is fine but incomplete and
that I plan to post a follow-up patch soon.

Thank you,

Bart.

Re: [PATCH 0/8] Eight small UFS patches

[Martin K. Petersen]

Bart,

> Martin, is it still possible to migrate the first two patches from
> this series from the staging branch to the fixes branch?
>
> Please note that patch 1/8 from this series is fine but incomplete and
> that I plan to post a follow-up patch soon.

I did consider taking the first two through fixes but was concerned
about conflicts given your large impending series.

But I'll shuffle them over. Will have to redo the merge but that's OK.

I can always pull fixes into staging once Linus has merged the two
commits in question. That way I am the one who gets to resolve the
conflict.

-- 
Martin K. Petersen

Re: [PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Neil Armstrong]

Hi Bart,

On 10/14/25 22:00, Bart Van Assche wrote:
> ufs_sysfs_add_nodes() is called concurrently with ufs_get_device_desc().
> This may cause the following code to be called before
> ufs_sysfs_add_nodes():
> 
> 	sysfs_update_group(&hba->dev->kobj, &ufs_sysfs_hid_group);
> 
> If this happens, ufs_sysfs_add_nodes() triggers a kernel warning and
> fails. Fix this by calling ufs_sysfs_add_nodes() before SCSI LUNs are
> scanned since the sysfs_update_group() call happens from the context of
> thread that executes ufshcd_async_scan(). This patch fixes the following
> kernel warning:
> 
> sysfs: cannot create duplicate filename '/devices/platform/3c2d0000.ufs/hid'
> Workqueue: async async_run_entry_fn
> Call trace:
>   dump_backtrace+0xfc/0x17c
>   show_stack+0x18/0x28
>   dump_stack_lvl+0x40/0x104
>   dump_stack+0x18/0x3c
>   sysfs_warn_dup+0x6c/0xc8
>   internal_create_group+0x1c8/0x504
>   sysfs_create_groups+0x38/0x9c
>   ufs_sysfs_add_nodes+0x20/0x58
>   ufshcd_init+0x1114/0x134c
>   ufshcd_pltfrm_init+0x728/0x7d8
>   ufs_google_probe+0x30/0x84
>   platform_probe+0xa0/0xe0
>   really_probe+0x114/0x454
>   __driver_probe_device+0xa4/0x160
>   driver_probe_device+0x44/0x23c
>   __device_attach_driver+0x15c/0x1f4
>   bus_for_each_drv+0x10c/0x168
>   __device_attach_async_helper+0x80/0xf8
>   async_run_entry_fn+0x4c/0x17c
>   process_one_work+0x26c/0x65c
>   worker_thread+0x33c/0x498
>   kthread+0x110/0x134
>   ret_from_fork+0x10/0x20
> ufshcd 3c2d0000.ufs: ufs_sysfs_add_nodes: sysfs groups creation failed (err = -17)

I've applied this change on top of v6.18-rc3, but I still get the same warning:

[    8.303145] sysfs: cannot create duplicate filename '/devices/platform/soc@0/1d84000.ufshc/hid'
[    8.320896] CPU: 2 UID: 0 PID: 72 Comm: kworker/u32:2 Tainted: G S                  6.18.0-rc3-00001-g7b760ac10637 #541 PREEMPT
[    8.320900] Tainted: [S]=CPU_OUT_OF_SPEC
[    8.320901] Hardware name: Qualcomm Technologies, Inc. SM8650 HDK (DT)
[    8.320902] Workqueue: events_unbound deferred_probe_work_func
[    8.320918] Call trace:
[    8.320919]  show_stack+0x18/0x24 (C)
[    8.320924]  dump_stack_lvl+0x74/0x8c
[    8.320928]  dump_stack+0x18/0x24
[    8.320930]  sysfs_warn_dup+0x64/0x80
[    8.320935]  internal_create_group+0x450/0x46c
[    8.320938]  internal_create_groups+0x50/0xd0
[    8.320939]  sysfs_create_groups+0x18/0x24
[    8.320941]  ufs_sysfs_add_nodes+0x24/0x68
[    8.320947]  ufshcd_init+0xb28/0xef0
[    8.320952]  ufshcd_pltfrm_init+0x514/0x7ec
[    8.320960]  ufs_qcom_probe+0x20/0x58 [ufs_qcom]
[    8.320963]  platform_probe+0x5c/0x98
[    8.320968]  really_probe+0xbc/0x29c
[    8.320969]  __driver_probe_device+0x78/0x12c
[    8.320970]  driver_probe_device+0x3c/0x15c
[    8.320971]  __device_attach_driver+0xb8/0x134
[    8.320972]  bus_for_each_drv+0x88/0xe8
[    8.320976]  __device_attach+0xa0/0x190
[    8.320977]  device_initial_probe+0x14/0x20
[    8.320978]  bus_probe_device+0xac/0xb0
[    8.320980]  deferred_probe_work_func+0x88/0xc0
[    8.320982]  process_one_work+0x148/0x28c
[    8.320993]  worker_thread+0x2d0/0x3d8
[    8.320995]  kthread+0x12c/0x204
[    8.321004]  ret_from_fork+0x10/0x20
[    8.321095] ufshcd-qcom 1d84000.ufshc: ufs_sysfs_add_nodes: sysfs groups creation failed (err = -17)

Commit log:

$ git log --oneline --no-merges v6.17..HEAD drivers/ufs/core/ufshcd.c
7b760ac10637 (HEAD) ufs: core: Fix a race condition related to the "hid" attribute group
bb7663dec67b scsi: ufs: sysfs: Make HID attributes visible
0ba7a254afd0 scsi: ufs: core: Fix PM QoS mutex initialization
f966e02ae521 scsi: ufs: core: Fix runtime suspend error deadlock
79dde5f7dc7c scsi: ufs: core: Fix data race in CPU latency PM QoS request handling
253757797973 scsi: ufs: core: Change MCQ interrupt enable flow
fb1f45683461 scsi: ufs: core: Disable timestamp functionality if not supported
faac32d4ece3 scsi: ufs: host: mediatek: Enhance recovery on hibernation exit failure
cb7cc0cfb38c scsi: ufs: core: Only collect timestamps if monitoring is enabled
dc60a408a1dc scsi: ufs: core: Improve IOPS

Reverting bb7663dec67b ("scsi: ufs: sysfs: Make HID attributes visible") removes the warning.

Neil

> 
> Cc: Daniel Lee <chullee@google.com>
> Fixes: bb7663dec67b ("scsi: ufs: sysfs: Make HID attributes visible")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   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 8339fec975b9..26c2aafd7338 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -10885,8 +10885,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>   	if (err)
>   		goto out_disable;
>   
> -	async_schedule(ufshcd_async_scan, hba);
>   	ufs_sysfs_add_nodes(hba->dev);
> +	async_schedule(ufshcd_async_scan, hba);
>   
>   	device_enable_async_suspend(dev);
>   	ufshcd_pm_qos_init(hba);
> 

Re: [PATCH 1/8] ufs: core: Fix a race condition related to the "hid" attribute group

[Bart Van Assche]

On 10/27/25 7:04 AM, Neil Armstrong wrote:
> I've applied this change on top of v6.18-rc3, but I still get the same 
> warning: [ ... ]

Hi Neil,

Please take a look at this patch series:
https://lore.kernel.org/linux-scsi/20251027170950.2919594-1-bvanassche@acm.org/

Thanks,

Bart.