[PATCH 0/9] Simplify the UFS driver initialization code

[PATCH 0/9] Simplify the UFS driver initialization code

[Bart Van Assche]

Hi Martin,

This patch series addresses the following issues in the UFS driver
initialization code:
* The legacy and MCQ scsi_add_host() calls occur in different functions. This
  patch series moves both calls into the same function.
* Two functions have a boolean 'init_dev_params' argument. This patch series
  removes that argument from both functions by splitting functions and by
  pushing some function calls from caller into callee.

Please consider this patch series for the next merge window.

Thanks,

Bart.

Bart Van Assche (9):
  ufs: core: Introduce ufshcd_add_scsi_host()
  ufs: core: Introduce ufshcd_activate_link()
  ufs: core: Introduce ufshcd_post_device_init()
  ufs: core: Call ufshcd_add_scsi_host() later
  ufs: core: Move the ufshcd_device_init() call
  ufs: core: Move the ufshcd_device_init(hba, true) call
  ufs: core: Expand the ufshcd_device_init(hba, true) call
  ufs: core: Move the MCQ scsi_add_host() call
  ufs: core: Remove the second argument of ufshcd_device_init()

 drivers/ufs/core/ufshcd.c | 241 ++++++++++++++++++++++----------------
 1 file changed, 139 insertions(+), 102 deletions(-)

[PATCH 1/9] ufs: core: Introduce ufshcd_add_scsi_host()

[Bart Van Assche]

Move the code for adding a SCSI host and also the code for managing
TMF tags from ufshcd_init() into a new function called
ufshcd_add_scsi_host(). No functionality has been changed.

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

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 0dd26059f5d7..d29e469c3873 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10381,6 +10381,56 @@ static const struct blk_mq_ops ufshcd_tmf_ops = {
 	.queue_rq = ufshcd_queue_tmf,
 };
 
+static int ufshcd_add_scsi_host(struct ufs_hba *hba)
+{
+	int err;
+
+	if (!is_mcq_supported(hba)) {
+		err = scsi_add_host(hba->host, hba->dev);
+		if (err) {
+			dev_err(hba->dev, "scsi_add_host failed\n");
+			return err;
+		}
+		hba->scsi_host_added = true;
+	}
+
+	hba->tmf_tag_set = (struct blk_mq_tag_set) {
+		.nr_hw_queues	= 1,
+		.queue_depth	= hba->nutmrs,
+		.ops		= &ufshcd_tmf_ops,
+		.flags		= BLK_MQ_F_NO_SCHED,
+	};
+	err = blk_mq_alloc_tag_set(&hba->tmf_tag_set);
+	if (err < 0)
+		goto remove_scsi_host;
+	hba->tmf_queue = blk_mq_alloc_queue(&hba->tmf_tag_set, NULL, NULL);
+	if (IS_ERR(hba->tmf_queue)) {
+		err = PTR_ERR(hba->tmf_queue);
+		goto free_tmf_tag_set;
+	}
+	hba->tmf_rqs = devm_kcalloc(hba->dev, hba->nutmrs,
+				    sizeof(*hba->tmf_rqs), GFP_KERNEL);
+	if (!hba->tmf_rqs) {
+		err = -ENOMEM;
+		goto free_tmf_queue;
+	}
+
+	return 0;
+
+free_tmf_queue:
+	blk_mq_destroy_queue(hba->tmf_queue);
+	blk_put_queue(hba->tmf_queue);
+
+free_tmf_tag_set:
+	blk_mq_free_tag_set(&hba->tmf_tag_set);
+
+remove_scsi_host:
+	if (hba->scsi_host_added)
+		scsi_remove_host(hba->host);
+
+	return err;
+}
+
 /**
  * ufshcd_init - Driver initialization routine
  * @hba: per-adapter instance
@@ -10514,35 +10564,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 		hba->is_irq_enabled = true;
 	}
 
-	if (!is_mcq_supported(hba)) {
-		err = scsi_add_host(host, hba->dev);
-		if (err) {
-			dev_err(hba->dev, "scsi_add_host failed\n");
-			goto out_disable;
-		}
-		hba->scsi_host_added = true;
-	}
-
-	hba->tmf_tag_set = (struct blk_mq_tag_set) {
-		.nr_hw_queues	= 1,
-		.queue_depth	= hba->nutmrs,
-		.ops		= &ufshcd_tmf_ops,
-		.flags		= BLK_MQ_F_NO_SCHED,
-	};
-	err = blk_mq_alloc_tag_set(&hba->tmf_tag_set);
-	if (err < 0)
-		goto out_remove_scsi_host;
-	hba->tmf_queue = blk_mq_alloc_queue(&hba->tmf_tag_set, NULL, NULL);
-	if (IS_ERR(hba->tmf_queue)) {
-		err = PTR_ERR(hba->tmf_queue);
-		goto free_tmf_tag_set;
-	}
-	hba->tmf_rqs = devm_kcalloc(hba->dev, hba->nutmrs,
-				    sizeof(*hba->tmf_rqs), GFP_KERNEL);
-	if (!hba->tmf_rqs) {
-		err = -ENOMEM;
-		goto free_tmf_queue;
-	}
+	err = ufshcd_add_scsi_host(hba);
+	if (err)
+		goto out_disable;
 
 	/* Reset the attached device */
 	ufshcd_device_reset(hba);
@@ -10600,9 +10624,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 free_tmf_queue:
 	blk_mq_destroy_queue(hba->tmf_queue);
 	blk_put_queue(hba->tmf_queue);
-free_tmf_tag_set:
 	blk_mq_free_tag_set(&hba->tmf_tag_set);
-out_remove_scsi_host:
 	if (hba->scsi_host_added)
 		scsi_remove_host(hba->host);
 out_disable:

[PATCH 2/9] ufs: core: Introduce ufshcd_activate_link()

[Bart Van Assche]

Prepare for introducing a second caller by moving the code for
activating the link between UFS controller and device into a new
function. No functionality has been changed.

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

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index d29e469c3873..04d94bf5cc2d 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8733,10 +8733,9 @@ static void ufshcd_config_mcq(struct ufs_hba *hba)
 		 hba->nutrs);
 }
 
-static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
+static int ufshcd_activate_link(struct ufs_hba *hba)
 {
 	int ret;
-	struct Scsi_Host *host = hba->host;
 
 	hba->ufshcd_state = UFSHCD_STATE_RESET;
 
@@ -8753,6 +8752,18 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
 	/* UniPro link is active now */
 	ufshcd_set_link_active(hba);
 
+	return 0;
+}
+
+static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
+{
+	struct Scsi_Host *host = hba->host;
+	int ret;
+
+	ret = ufshcd_activate_link(hba);
+	if (ret)
+		return ret;
+
 	/* Reconfigure MCQ upon reset */
 	if (hba->mcq_enabled && !init_dev_params) {
 		ufshcd_config_mcq(hba);

[PATCH 3/9] ufs: core: Introduce ufshcd_post_device_init()

[Bart Van Assche]

Prepare for introducing a second caller by moving more code from
ufshcd_device_init() into a new function.

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

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 04d94bf5cc2d..9e64b08eaa5e 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8755,6 +8755,40 @@ static int ufshcd_activate_link(struct ufs_hba *hba)
 	return 0;
 }
 
+static int ufshcd_post_device_init(struct ufs_hba *hba)
+{
+	int ret;
+
+	ufshcd_tune_unipro_params(hba);
+
+	/* UFS device is also active now */
+	ufshcd_set_ufs_dev_active(hba);
+	ufshcd_force_reset_auto_bkops(hba);
+
+	ufshcd_set_timestamp_attr(hba);
+	schedule_delayed_work(&hba->ufs_rtc_update_work,
+			      msecs_to_jiffies(UFS_RTC_UPDATE_INTERVAL_MS));
+
+	/* Gear up to HS gear if supported */
+	if (hba->max_pwr_info.is_valid) {
+		/*
+		 * Set the right value to bRefClkFreq before attempting to
+		 * switch to HS gears.
+		 */
+		if (hba->dev_ref_clk_freq != REF_CLK_FREQ_INVAL)
+			ufshcd_set_dev_ref_clk(hba);
+		ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
+		if (ret) {
+			dev_err(hba->dev,
+				"%s: Failed setting power mode, err = %d\n",
+				__func__, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
 {
 	struct Scsi_Host *host = hba->host;
@@ -8813,33 +8847,7 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
 		}
 	}
 
-	ufshcd_tune_unipro_params(hba);
-
-	/* UFS device is also active now */
-	ufshcd_set_ufs_dev_active(hba);
-	ufshcd_force_reset_auto_bkops(hba);
-
-	ufshcd_set_timestamp_attr(hba);
-	schedule_delayed_work(&hba->ufs_rtc_update_work,
-			      msecs_to_jiffies(UFS_RTC_UPDATE_INTERVAL_MS));
-
-	/* Gear up to HS gear if supported */
-	if (hba->max_pwr_info.is_valid) {
-		/*
-		 * Set the right value to bRefClkFreq before attempting to
-		 * switch to HS gears.
-		 */
-		if (hba->dev_ref_clk_freq != REF_CLK_FREQ_INVAL)
-			ufshcd_set_dev_ref_clk(hba);
-		ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
-		if (ret) {
-			dev_err(hba->dev, "%s: Failed setting power mode, err = %d\n",
-					__func__, ret);
-			return ret;
-		}
-	}
-
-	return 0;
+	return ufshcd_post_device_init(hba);
 }
 
 /**

[PATCH 4/9] ufs: core: Call ufshcd_add_scsi_host() later

[Bart Van Assche]

Call ufshcd_add_scsi_host() after host controller initialization has
completed. This is possible because no code between the old and new
ufshcd_add_scsi_host() call site depends on the scsi_add_host() call.

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

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 9e64b08eaa5e..502d0f5a95c0 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10583,10 +10583,6 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 		hba->is_irq_enabled = true;
 	}
 
-	err = ufshcd_add_scsi_host(hba);
-	if (err)
-		goto out_disable;
-
 	/* Reset the attached device */
 	ufshcd_device_reset(hba);
 
@@ -10598,7 +10594,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 		dev_err(hba->dev, "Host controller enable failed\n");
 		ufshcd_print_evt_hist(hba);
 		ufshcd_print_host_state(hba);
-		goto free_tmf_queue;
+		goto out_disable;
 	}
 
 	/*
@@ -10633,6 +10629,10 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	 */
 	ufshcd_set_ufs_dev_active(hba);
 
+	err = ufshcd_add_scsi_host(hba);
+	if (err)
+		goto out_disable;
+
 	async_schedule(ufshcd_async_scan, hba);
 	ufs_sysfs_add_nodes(hba->dev);
 
@@ -10640,12 +10640,6 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	ufshcd_pm_qos_init(hba);
 	return 0;
 
-free_tmf_queue:
-	blk_mq_destroy_queue(hba->tmf_queue);
-	blk_put_queue(hba->tmf_queue);
-	blk_mq_free_tag_set(&hba->tmf_tag_set);
-	if (hba->scsi_host_added)
-		scsi_remove_host(hba->host);
 out_disable:
 	hba->is_irq_enabled = false;
 	ufshcd_hba_exit(hba);

[PATCH 5/9] ufs: core: Move the ufshcd_device_init() call

[Bart Van Assche]

Move the ufshcd_device_init() call to the ufshcd_probe_hba() callers and
remove the 'init_dev_params' argument of ufshcd_probe_hba().

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

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 502d0f5a95c0..86deea546b44 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -298,7 +298,8 @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba);
 static int ufshcd_eh_host_reset_handler(struct scsi_cmnd *cmd);
 static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag);
 static void ufshcd_hba_exit(struct ufs_hba *hba);
-static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params);
+static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params);
+static int ufshcd_probe_hba(struct ufs_hba *hba);
 static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
 static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
 static int ufshcd_host_reset_and_restore(struct ufs_hba *hba);
@@ -7717,8 +7718,11 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba)
 	err = ufshcd_hba_enable(hba);
 
 	/* Establish the link again and restore the device */
-	if (!err)
-		err = ufshcd_probe_hba(hba, false);
+	if (!err) {
+		err = ufshcd_device_init(hba, /*init_dev_params=*/false);
+		if (!err)
+			err = ufshcd_probe_hba(hba);
+	}
 
 	if (err)
 		dev_err(hba->dev, "%s: Host init failed %d\n", __func__, err);
@@ -8853,22 +8857,17 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
 /**
  * ufshcd_probe_hba - probe hba to detect device and initialize it
  * @hba: per-adapter instance
- * @init_dev_params: whether or not to call ufshcd_device_params_init().
  *
  * Execute link-startup and verify device initialization
  *
  * Return: 0 upon success; < 0 upon failure.
  */
-static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params)
+static int ufshcd_probe_hba(struct ufs_hba *hba)
 {
 	ktime_t start = ktime_get();
 	unsigned long flags;
 	int ret;
 
-	ret = ufshcd_device_init(hba, init_dev_params);
-	if (ret)
-		goto out;
-
 	if (!hba->pm_op_in_progress &&
 	    (hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {
 		/* Reset the device and controller before doing reinit */
@@ -8885,7 +8884,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params)
 		}
 
 		/* Reinit the device */
-		ret = ufshcd_device_init(hba, init_dev_params);
+		ret = ufshcd_device_init(hba, /*init_dev_params=*/false);
 		if (ret)
 			goto out;
 	}
@@ -8933,7 +8932,9 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
 
 	down(&hba->host_sem);
 	/* Initialize hba, detect and initialize UFS device */
-	ret = ufshcd_probe_hba(hba, true);
+	ret = ufshcd_device_init(hba, /*init_dev_params=*/true);
+	if (ret == 0)
+		ret = ufshcd_probe_hba(hba);
 	up(&hba->host_sem);
 	if (ret)
 		goto out;

Re: [PATCH 5/9] ufs: core: Move the ufshcd_device_init() call

[kernel test robot]

Hi Bart,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mkp-scsi/for-next]
[cannot apply to jejb-scsi/for-next linus/master v6.11-rc4 next-20240822]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bart-Van-Assche/ufs-core-Introduce-ufshcd_add_scsi_host/20240820-065414
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/20240819225102.2437307-6-bvanassche%40acm.org
patch subject: [PATCH 5/9] ufs: core: Move the ufshcd_device_init() call
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240822/202408222305.wOhpxPXn-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240822/202408222305.wOhpxPXn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408222305.wOhpxPXn-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/ufs/core/ufshcd.c:8871:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
    8871 |         if (!hba->pm_op_in_progress &&
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
    8872 |             (hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {
         |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/ufs/core/ufshcd.c:8911:6: note: uninitialized use occurs here
    8911 |         if (ret)
         |             ^~~
   drivers/ufs/core/ufshcd.c:8871:2: note: remove the 'if' if its condition is always true
    8871 |         if (!hba->pm_op_in_progress &&
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    8872 |             (hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {
         |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/ufs/core/ufshcd.c:8871:6: warning: variable 'ret' is used uninitialized whenever '&&' condition is false [-Wsometimes-uninitialized]
    8871 |         if (!hba->pm_op_in_progress &&
         |             ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/ufs/core/ufshcd.c:8911:6: note: uninitialized use occurs here
    8911 |         if (ret)
         |             ^~~
   drivers/ufs/core/ufshcd.c:8871:6: note: remove the '&&' if its condition is always true
    8871 |         if (!hba->pm_op_in_progress &&
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/ufs/core/ufshcd.c:8869:9: note: initialize the variable 'ret' to silence this warning
    8869 |         int ret;
         |                ^
         |                 = 0
   2 warnings generated.


vim +8871 drivers/ufs/core/ufshcd.c

96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8856  
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8857  /**
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8858   * ufshcd_probe_hba - probe hba to detect device and initialize it
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8859   * @hba: per-adapter instance
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8860   *
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8861   * Execute link-startup and verify device initialization
fd4bffb54dc0f6 drivers/ufs/core/ufshcd.c Bart Van Assche       2023-07-27  8862   *
fd4bffb54dc0f6 drivers/ufs/core/ufshcd.c Bart Van Assche       2023-07-27  8863   * Return: 0 upon success; < 0 upon failure.
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8864   */
56161bc6c24d06 drivers/ufs/core/ufshcd.c Bart Van Assche       2024-08-19  8865  static int ufshcd_probe_hba(struct ufs_hba *hba)
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8866  {
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8867  	ktime_t start = ktime_get();
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8868  	unsigned long flags;
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8869  	int ret;
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8870  
fc88ca19ad0989 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2023-09-08 @8871  	if (!hba->pm_op_in_progress &&
fc88ca19ad0989 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2023-09-08  8872  	    (hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8873  		/* Reset the device and controller before doing reinit */
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8874  		ufshcd_device_reset(hba);
135c6eb27a85c8 drivers/ufs/core/ufshcd.c Joel Slebodnick       2024-06-13  8875  		ufs_put_device_desc(hba);
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8876  		ufshcd_hba_stop(hba);
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8877  		ufshcd_vops_reinit_notify(hba);
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8878  		ret = ufshcd_hba_enable(hba);
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8879  		if (ret) {
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8880  			dev_err(hba->dev, "Host controller enable failed\n");
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8881  			ufshcd_print_evt_hist(hba);
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8882  			ufshcd_print_host_state(hba);
8643ae66ce749f drivers/scsi/ufs/ufshcd.c Dov Levenglick        2016-10-17  8883  			goto out;
8643ae66ce749f drivers/scsi/ufs/ufshcd.c Dov Levenglick        2016-10-17  8884  		}
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8885  
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8886  		/* Reinit the device */
56161bc6c24d06 drivers/ufs/core/ufshcd.c Bart Van Assche       2024-08-19  8887  		ret = ufshcd_device_init(hba, /*init_dev_params=*/false);
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8888  		if (ret)
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8889  			goto out;
7eb584db73bebb drivers/scsi/ufs/ufshcd.c Dolev Raviv           2014-09-25  8890  	}
57d104c153d3d6 drivers/scsi/ufs/ufshcd.c Subhash Jadavani      2014-09-25  8891  
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8892  	ufshcd_print_pwr_info(hba);
96a7141da33207 drivers/ufs/core/ufshcd.c Manivannan Sadhasivam 2022-12-22  8893  
e89860f196fca3 drivers/scsi/ufs/ufshcd.c Can Guo               2020-03-26  8894  	/*
e89860f196fca3 drivers/scsi/ufs/ufshcd.c Can Guo               2020-03-26  8895  	 * bActiveICCLevel is volatile for UFS device (as per latest v2.1 spec)
e89860f196fca3 drivers/scsi/ufs/ufshcd.c Can Guo               2020-03-26  8896  	 * and for removable UFS card as well, hence always set the parameter.
e89860f196fca3 drivers/scsi/ufs/ufshcd.c Can Guo               2020-03-26  8897  	 * Note: Error handler may issue the device reset hence resetting
e89860f196fca3 drivers/scsi/ufs/ufshcd.c Can Guo               2020-03-26  8898  	 * bActiveICCLevel as well so it is always safe to set this here.
e89860f196fca3 drivers/scsi/ufs/ufshcd.c Can Guo               2020-03-26  8899  	 */
e89860f196fca3 drivers/scsi/ufs/ufshcd.c Can Guo               2020-03-26  8900  	ufshcd_set_active_icc_lvl(hba);
e89860f196fca3 drivers/scsi/ufs/ufshcd.c Can Guo               2020-03-26  8901  
4450a1653a935d drivers/ufs/core/ufshcd.c Jinyoung Choi         2022-08-04  8902  	/* Enable UFS Write Booster if supported */
4450a1653a935d drivers/ufs/core/ufshcd.c Jinyoung Choi         2022-08-04  8903  	ufshcd_configure_wb(hba);
4450a1653a935d drivers/ufs/core/ufshcd.c Jinyoung Choi         2022-08-04  8904  
cd4694756188dc drivers/scsi/ufs/ufshcd.c Adrian Hunter         2021-02-09  8905  	if (hba->ee_usr_mask)
cd4694756188dc drivers/scsi/ufs/ufshcd.c Adrian Hunter         2021-02-09  8906  		ufshcd_write_ee_control(hba);
bdf5c0bb4dd9e7 drivers/ufs/core/ufshcd.c Bart Van Assche       2023-12-14  8907  	ufshcd_configure_auto_hibern8(hba);
71d848b8d97ec0 drivers/scsi/ufs/ufshcd.c Can Guo               2019-11-14  8908  
5a0b0cb9bee767 drivers/scsi/ufs/ufshcd.c Sujit Reddy Thumma    2013-07-30  8909  out:
4db7a23605973a drivers/scsi/ufs/ufshcd.c Can Guo               2020-08-09  8910  	spin_lock_irqsave(hba->host->host_lock, flags);
4db7a23605973a drivers/scsi/ufs/ufshcd.c Can Guo               2020-08-09  8911  	if (ret)
4db7a23605973a drivers/scsi/ufs/ufshcd.c Can Guo               2020-08-09  8912  		hba->ufshcd_state = UFSHCD_STATE_ERROR;
4db7a23605973a drivers/scsi/ufs/ufshcd.c Can Guo               2020-08-09  8913  	else if (hba->ufshcd_state == UFSHCD_STATE_RESET)
4db7a23605973a drivers/scsi/ufs/ufshcd.c Can Guo               2020-08-09  8914  		hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
4db7a23605973a drivers/scsi/ufs/ufshcd.c Can Guo               2020-08-09  8915  	spin_unlock_irqrestore(hba->host->host_lock, flags);
1d337ec2f35e69 drivers/scsi/ufs/ufshcd.c Sujit Reddy Thumma    2014-09-25  8916  
7ff5ab47363334 drivers/scsi/ufs/ufshcd.c Subhash Jadavani      2016-12-22  8917  	trace_ufshcd_init(dev_name(hba->dev), ret,
7ff5ab47363334 drivers/scsi/ufs/ufshcd.c Subhash Jadavani      2016-12-22  8918  		ktime_to_us(ktime_sub(ktime_get(), start)),
73eba2be9203c0 drivers/scsi/ufs/ufshcd.c Subhash Jadavani      2017-01-10  8919  		hba->curr_dev_pwr_mode, hba->uic_link_state);
1d337ec2f35e69 drivers/scsi/ufs/ufshcd.c Sujit Reddy Thumma    2014-09-25  8920  	return ret;
1d337ec2f35e69 drivers/scsi/ufs/ufshcd.c Sujit Reddy Thumma    2014-09-25  8921  }
1d337ec2f35e69 drivers/scsi/ufs/ufshcd.c Sujit Reddy Thumma    2014-09-25  8922  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 5/9] ufs: core: Move the ufshcd_device_init() call

[Bart Van Assche]

On 8/22/24 9:07 AM, kernel test robot wrote:
>>> drivers/ufs/core/ufshcd.c:8871:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]

I plan to fix this compiler warning by combining the following change
into patch 5/9:

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 9af293b39672..313b62509162 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8832,7 +8832,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
  {
         ktime_t start = ktime_get();
         unsigned long flags;
-       int ret;
+       int ret = 0;

         if (!hba->pm_op_in_progress &&
             (hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {

Bart.

[PATCH 6/9] ufs: core: Move the ufshcd_device_init(hba, true) call

[Bart Van Assche]

Move the ufshcd_device_init(hba, true) call from ufshcd_async_scan()
into ufshcd_init(). This patch prepares for moving both scsi_add_host()
calls into ufshcd_add_scsi_host().

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

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 86deea546b44..2754f496d10e 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8931,10 +8931,7 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
 	int ret;
 
 	down(&hba->host_sem);
-	/* Initialize hba, detect and initialize UFS device */
-	ret = ufshcd_device_init(hba, /*init_dev_params=*/true);
-	if (ret == 0)
-		ret = ufshcd_probe_hba(hba);
+	ret = ufshcd_probe_hba(hba);
 	up(&hba->host_sem);
 	if (ret)
 		goto out;
@@ -10630,6 +10627,11 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	 */
 	ufshcd_set_ufs_dev_active(hba);
 
+	/* Initialize hba, detect and initialize UFS device */
+	err = ufshcd_device_init(hba, /*init_dev_params=*/true);
+	if (err)
+		goto out_disable;
+
 	err = ufshcd_add_scsi_host(hba);
 	if (err)
 		goto out_disable;

[PATCH 7/9] ufs: core: Expand the ufshcd_device_init(hba, true) call

[Bart Van Assche]

Expand the ufshcd_device_init(hba, true) call and remove all code that
depends on init_dev_params == false.

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

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2754f496d10e..dedbef27d5c5 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10627,8 +10627,48 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	 */
 	ufshcd_set_ufs_dev_active(hba);
 
-	/* Initialize hba, detect and initialize UFS device */
-	err = ufshcd_device_init(hba, /*init_dev_params=*/true);
+	err = ufshcd_activate_link(hba);
+	if (err)
+		goto out_disable;
+
+	/* Verify device initialization by sending NOP OUT UPIU. */
+	err = ufshcd_verify_dev_init(hba);
+	if (err)
+		goto out_disable;
+
+	/* Initiate UFS initialization and waiting for completion. */
+	err = ufshcd_complete_dev_init(hba);
+	if (err)
+		goto out_disable;
+
+	/*
+	 * Initialize UFS device parameters used by driver, these
+	 * parameters are associated with UFS descriptors.
+	 */
+	err = ufshcd_device_params_init(hba);
+	if (err)
+		goto out_disable;
+	if (is_mcq_supported(hba)) {
+		ufshcd_mcq_enable(hba);
+		err = ufshcd_alloc_mcq(hba);
+		if (!err) {
+			ufshcd_config_mcq(hba);
+		} else {
+			/* Continue with SDB mode */
+			ufshcd_mcq_disable(hba);
+			use_mcq_mode = false;
+			dev_err(hba->dev, "MCQ mode is disabled, err=%d\n",
+				err);
+		}
+		err = scsi_add_host(host, hba->dev);
+		if (err) {
+			dev_err(hba->dev, "scsi_add_host failed\n");
+			goto out_disable;
+		}
+		hba->scsi_host_added = true;
+	}
+
+	err = ufshcd_post_device_init(hba);
 	if (err)
 		goto out_disable;
 

[PATCH 8/9] ufs: core: Move the MCQ scsi_add_host() call

[Bart Van Assche]

Whether or not MCQ is used, call scsi_add_host from ufshcd_add_scsi_host().
For MCQ this patch swaps the order of the scsi_add_host() and
ufshcd_post_device_init() calls. This patch also prepares for moving
both scsi_add_host() calls into ufshcd_add_scsi_host().

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

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index dedbef27d5c5..b3fae37e4f6a 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10402,15 +10402,27 @@ static int ufshcd_add_scsi_host(struct ufs_hba *hba)
 {
 	int err;
 
-	if (!is_mcq_supported(hba)) {
-		err = scsi_add_host(hba->host, hba->dev);
-		if (err) {
-			dev_err(hba->dev, "scsi_add_host failed\n");
-			return err;
+	if (is_mcq_supported(hba)) {
+		ufshcd_mcq_enable(hba);
+		err = ufshcd_alloc_mcq(hba);
+		if (!err) {
+			ufshcd_config_mcq(hba);
+		} else {
+			/* Continue with SDB mode */
+			ufshcd_mcq_disable(hba);
+			use_mcq_mode = false;
+			dev_err(hba->dev, "MCQ mode is disabled, err=%d\n",
+				err);
 		}
-		hba->scsi_host_added = true;
 	}
 
+	err = scsi_add_host(hba->host, hba->dev);
+	if (err) {
+		dev_err(hba->dev, "scsi_add_host failed\n");
+		return err;
+	}
+	hba->scsi_host_added = true;
+
 	hba->tmf_tag_set = (struct blk_mq_tag_set) {
 		.nr_hw_queues	= 1,
 		.queue_depth	= hba->nutmrs,
@@ -10648,25 +10660,6 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	err = ufshcd_device_params_init(hba);
 	if (err)
 		goto out_disable;
-	if (is_mcq_supported(hba)) {
-		ufshcd_mcq_enable(hba);
-		err = ufshcd_alloc_mcq(hba);
-		if (!err) {
-			ufshcd_config_mcq(hba);
-		} else {
-			/* Continue with SDB mode */
-			ufshcd_mcq_disable(hba);
-			use_mcq_mode = false;
-			dev_err(hba->dev, "MCQ mode is disabled, err=%d\n",
-				err);
-		}
-		err = scsi_add_host(host, hba->dev);
-		if (err) {
-			dev_err(hba->dev, "scsi_add_host failed\n");
-			goto out_disable;
-		}
-		hba->scsi_host_added = true;
-	}
 
 	err = ufshcd_post_device_init(hba);
 	if (err)

[PATCH 9/9] ufs: core: Remove the second argument of ufshcd_device_init()

[Bart Van Assche]

Both ufshcd_device_init() callers pass 'false' as second argument. Hence,
remove that second argument.

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

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index b3fae37e4f6a..9af293b39672 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -298,7 +298,7 @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba);
 static int ufshcd_eh_host_reset_handler(struct scsi_cmnd *cmd);
 static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag);
 static void ufshcd_hba_exit(struct ufs_hba *hba);
-static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params);
+static int ufshcd_device_init(struct ufs_hba *hba);
 static int ufshcd_probe_hba(struct ufs_hba *hba);
 static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
 static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
@@ -7719,7 +7719,7 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba)
 
 	/* Establish the link again and restore the device */
 	if (!err) {
-		err = ufshcd_device_init(hba, /*init_dev_params=*/false);
+		err = ufshcd_device_init(hba);
 		if (!err)
 			err = ufshcd_probe_hba(hba);
 	}
@@ -8793,9 +8793,8 @@ static int ufshcd_post_device_init(struct ufs_hba *hba)
 	return 0;
 }
 
-static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
+static int ufshcd_device_init(struct ufs_hba *hba)
 {
-	struct Scsi_Host *host = hba->host;
 	int ret;
 
 	ret = ufshcd_activate_link(hba);
@@ -8803,7 +8802,7 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
 		return ret;
 
 	/* Reconfigure MCQ upon reset */
-	if (hba->mcq_enabled && !init_dev_params) {
+	if (hba->mcq_enabled) {
 		ufshcd_config_mcq(hba);
 		ufshcd_mcq_enable(hba);
 	}
@@ -8818,39 +8817,6 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
 	if (ret)
 		return ret;
 
-	/*
-	 * Initialize UFS device parameters used by driver, these
-	 * parameters are associated with UFS descriptors.
-	 */
-	if (init_dev_params) {
-		ret = ufshcd_device_params_init(hba);
-		if (ret)
-			return ret;
-		if (is_mcq_supported(hba) && !hba->scsi_host_added) {
-			ufshcd_mcq_enable(hba);
-			ret = ufshcd_alloc_mcq(hba);
-			if (!ret) {
-				ufshcd_config_mcq(hba);
-			} else {
-				/* Continue with SDB mode */
-				ufshcd_mcq_disable(hba);
-				use_mcq_mode = false;
-				dev_err(hba->dev, "MCQ mode is disabled, err=%d\n",
-					 ret);
-			}
-			ret = scsi_add_host(host, hba->dev);
-			if (ret) {
-				dev_err(hba->dev, "scsi_add_host failed\n");
-				return ret;
-			}
-			hba->scsi_host_added = true;
-		} else if (is_mcq_supported(hba)) {
-			/* UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH is set */
-			ufshcd_config_mcq(hba);
-			ufshcd_mcq_enable(hba);
-		}
-	}
-
 	return ufshcd_post_device_init(hba);
 }
 
@@ -8884,7 +8850,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
 		}
 
 		/* Reinit the device */
-		ret = ufshcd_device_init(hba, /*init_dev_params=*/false);
+		ret = ufshcd_device_init(hba);
 		if (ret)
 			goto out;
 	}