[PATCH v1] scsi: ufs: core: Hold a clock reference across the probe

[Naomi Chu]

Clock gating becomes possible as soon as ufshcd_init_clk_gating() has
run, and from that point on the probe keeps accessing host registers
without ever taking a clock reference. This has been safe only because
of the state check in __ufshcd_release(): gate_work is not queued
unless hba->ufshcd_state is UFSHCD_STATE_OPERATIONAL, and the promotion
to that state used to happen after the last register access of the
probe, at the end of ufshcd_probe_hba().

That is fragile: it only works while the promotion happens after the
register accesses. Commit a390e6677f41 ("scsi: ufs: core: Expand the
ufshcd_device_init(hba, true) call") changed that ordering by moving
the promotion into ufshcd_init(), which schedules ufshcd_async_scan()
afterwards. ufshcd_probe_hba() therefore now runs with the state
already promoted, and it accesses host registers without holding a
clock reference:

 - on hosts with UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH it calls
   ufshcd_hba_stop() and ufshcd_hba_enable() before
   ufshcd_device_init() sets the state back to UFSHCD_STATE_RESET, and
   both read REG_CONTROLLER_ENABLE, so gated clocks stall there instead
   of just losing a write.
 - it ends with an ufshcd_configure_auto_hibern8() write, which its
   other callers do take a clock reference for.

Take a clock reference as soon as clock gating has been initialised and
keep it until the probe is over. It then does not matter who drops a
clock reference while the probe is running, and the register accesses
of the probe no longer depend on hba->ufshcd_state. The reference is
dropped by ufshcd_async_scan() once the scan has finished, or by the
new out_release label if the probe fails after it was taken.

Fixes: a390e6677f41 ("scsi: ufs: core: Expand the ufshcd_device_init(hba, true) call")
Signed-off-by: Naomi Chu <naomi.chu at mediatek.com>
---
 drivers/ufs/core/ufshcd.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index ee21388e74f5..7d63287b5ca9 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9550,6 +9550,7 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
 	ret = ufshcd_add_lus(hba);
 
 out:
+	ufshcd_release(hba);
 	pm_runtime_put_sync(hba->dev);
 
 	if (ret)
@@ -11272,6 +11273,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	ufshcd_init_clk_gating(hba);
 
+	/* Released by ufshcd_async_scan(), or by out_release on failure. */
+	ufshcd_hold(hba);
+
 	ufshcd_init_clk_scaling(hba);
 
 	/*
@@ -11292,7 +11296,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	err = devm_request_irq(dev, irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
 	if (err) {
 		dev_err(hba->dev, "request irq failed\n");
-		goto out_disable;
+		goto out_release;
 	} else {
 		hba->is_irq_enabled = true;
 	}
@@ -11308,7 +11312,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 out_disable;
+		goto out_release;
 	}
 
 	INIT_DELAYED_WORK(&hba->rpm_dev_flush_recheck_work, ufshcd_rpm_dev_flush_recheck_work);
@@ -11322,7 +11326,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	err = ufshcd_add_scsi_host(hba);
 	if (err)
-		goto out_disable;
+		goto out_release;
 
 	/* Hold auto suspend until async scan completes */
 	pm_runtime_get_sync(dev);
@@ -11342,7 +11346,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	err = ufshcd_link_startup(hba);
 	if (err)
-		goto out_disable;
+		goto out_release;
 
 	if (hba->mcq_enabled)
 		ufshcd_config_mcq(hba);
@@ -11359,23 +11363,23 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	/* Verify device initialization by sending NOP OUT UPIU */
 	err = ufshcd_verify_dev_init(hba);
 	if (err)
-		goto out_disable;
+		goto out_release;
 
 	/* Initiate UFS initialization, and waiting until completion */
 	err = ufshcd_complete_dev_init(hba);
 	if (err)
-		goto out_disable;
+		goto out_release;
 
 	err = ufshcd_device_params_init(hba);
 	if (err)
-		goto out_disable;
+		goto out_release;
 
 	err = ufshcd_post_device_init(hba);
 
 initialized:
 	ufshcd_process_probe_result(hba, probe_start, err);
 	if (err)
-		goto out_disable;
+		goto out_release;
 
 	ufs_sysfs_add_nodes(hba->dev);
 	hba->dme_qos_sysfs_handle = sysfs_get_dirent(hba->dev->kobj.sd,
@@ -11386,6 +11390,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	ufshcd_pm_qos_init(hba);
 	return 0;
 
+out_release:
+	ufshcd_release(hba);
 out_disable:
 	hba->is_irq_enabled = false;
 	ufshcd_hba_exit(hba);
-- 
2.45.2