[PATCH 0/5] scsi: ufs: Bug fixes for ufs core and platform drivers

[PATCH 0/5] scsi: ufs: Bug fixes for ufs core and platform drivers

[Manivannan Sadhasivam via B4 Relay]

Hi,

This series has several bug fixes that I encountered when the ufs-qcom driver
was removed and inserted back. But the fixes are applicable to other platform
glue drivers as well.

This series is tested on Qcom RB5 development board based on SM8250 SoC.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
Manivannan Sadhasivam (5):
      scsi: ufs: core: Cancel RTC work during ufshcd_remove()
      scsi: ufs: qcom: Only free platform MSIs when ESI is enabled
      scsi: ufs: pltfrm: Disable runtime PM during removal of glue drivers
      scsi: ufs: pltfrm: Drop PM runtime reference count after ufshcd_remove()
      scsi: ufs: pltfrm: Dellocate HBA during ufshcd_pltfrm_remove()

 drivers/ufs/core/ufshcd.c             |  1 +
 drivers/ufs/host/cdns-pltfrm.c        |  4 +---
 drivers/ufs/host/tc-dwc-g210-pltfrm.c |  5 +----
 drivers/ufs/host/ufs-exynos.c         |  3 +--
 drivers/ufs/host/ufs-hisi.c           |  4 +---
 drivers/ufs/host/ufs-mediatek.c       |  5 +----
 drivers/ufs/host/ufs-qcom.c           |  7 ++++---
 drivers/ufs/host/ufs-renesas.c        |  4 +---
 drivers/ufs/host/ufs-sprd.c           |  5 +----
 drivers/ufs/host/ufshcd-pltfrm.c      | 16 ++++++++++++++++
 drivers/ufs/host/ufshcd-pltfrm.h      |  1 +
 11 files changed, 29 insertions(+), 26 deletions(-)
---
base-commit: 59b723cd2adbac2a34fc8e12c74ae26ae45bf230
change-id: 20241111-ufs_bug_fix-6d17f39afaa4

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

[PATCH 1/5] scsi: ufs: core: Cancel RTC work during ufshcd_remove()

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Currently, RTC work is only cancelled during __ufshcd_wl_suspend(). When
ufshcd is removed in ufshcd_remove(), RTC work is not cancelled. Due to
this, any further trigger of the RTC work after ufshcd_remove() would
result in a NULL pointer dereference as below:

Unable to handle kernel NULL pointer dereference at virtual address 00000000000002a4
Workqueue: events ufshcd_rtc_work
Call trace:
 _raw_spin_lock_irqsave+0x34/0x8c
 pm_runtime_get_if_active+0x24/0xb4
 ufshcd_rtc_work+0x124/0x19c
 process_scheduled_works+0x18c/0x2d8
 worker_thread+0x144/0x280
 kthread+0x11c/0x128
 ret_from_fork+0x10/0x20

Since RTC work accesses the ufshcd internal structures, it should be cancelled
when ufshcd is removed. So do that in ufshcd_remove(), as per the order in
ufshcd_init().

Cc: stable@vger.kernel.org # 6.8
Fixes: 6bf999e0eb41 ("scsi: ufs: core: Add UFS RTC support")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/ufs/core/ufshcd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index f5846598d80e..cc2555333512 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10225,6 +10225,7 @@ void ufshcd_remove(struct ufs_hba *hba)
 	ufs_hwmon_remove(hba);
 	ufs_bsg_remove(hba);
 	ufs_sysfs_remove_nodes(hba->dev);
+	cancel_delayed_work_sync(&hba->ufs_rtc_update_work);
 	blk_mq_destroy_queue(hba->tmf_queue);
 	blk_put_queue(hba->tmf_queue);
 	blk_mq_free_tag_set(&hba->tmf_tag_set);

-- 
2.25.1

Re: [PATCH 1/5] scsi: ufs: core: Cancel RTC work during ufshcd_remove()

[Peter Wang (王信友)]

On Mon, 2024-11-11 at 23:18 +0530, Manivannan Sadhasivam via B4 Relay
wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Currently, RTC work is only cancelled during __ufshcd_wl_suspend().
> When
> ufshcd is removed in ufshcd_remove(), RTC work is not cancelled. Due
> to
> this, any further trigger of the RTC work after ufshcd_remove() would
> result in a NULL pointer dereference as below:
> 
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000000002a4
> Workqueue: events ufshcd_rtc_work
> Call trace:
>  _raw_spin_lock_irqsave+0x34/0x8c
>  pm_runtime_get_if_active+0x24/0xb4
>  ufshcd_rtc_work+0x124/0x19c
>  process_scheduled_works+0x18c/0x2d8
>  worker_thread+0x144/0x280
>  kthread+0x11c/0x128
>  ret_from_fork+0x10/0x20
> 
> Since RTC work accesses the ufshcd internal structures, it should be
> cancelled
> when ufshcd is removed. So do that in ufshcd_remove(), as per the
> order in
> ufshcd_init().
> 
> Cc: stable@vger.kernel.org # 6.8
> Fixes: 6bf999e0eb41 ("scsi: ufs: core: Add UFS RTC support")
> Signed-off-by: Manivannan Sadhasivam <
> manivannan.sadhasivam@linaro.org>
> ---
>  drivers/ufs/core/ufshcd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> 

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

Re: [PATCH 1/5] scsi: ufs: core: Cancel RTC work during ufshcd_remove()

[Bean Huo]

Reviewed-by: Bean Huo <beanhuo@micron.com>

[PATCH 2/5] scsi: ufs: qcom: Only free platform MSIs when ESI is enabled

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Otherwise, it will result in a NULL pointer dereference as below:

Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
Call trace:
 mutex_lock+0xc/0x54
 platform_device_msi_free_irqs_all+0x14/0x20
 ufs_qcom_remove+0x34/0x48 [ufs_qcom]
 platform_remove+0x28/0x44
 device_remove+0x4c/0x80
 device_release_driver_internal+0xd8/0x178
 driver_detach+0x50/0x9c
 bus_remove_driver+0x6c/0xbc
 driver_unregister+0x30/0x60
 platform_driver_unregister+0x14/0x20
 ufs_qcom_pltform_exit+0x18/0xb94 [ufs_qcom]
 __arm64_sys_delete_module+0x180/0x260
 invoke_syscall+0x44/0x100
 el0_svc_common.constprop.0+0xc0/0xe0
 do_el0_svc+0x1c/0x28
 el0_svc+0x34/0xdc
 el0t_64_sync_handler+0xc0/0xc4
 el0t_64_sync+0x190/0x194

Cc: stable@vger.kernel.org # 6.3
Fixes: 519b6274a777 ("scsi: ufs: qcom: Add MCQ ESI config vendor specific ops")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/ufs/host/ufs-qcom.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index ecdfff2456e3..1bcc538ef8af 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1843,10 +1843,12 @@ static int ufs_qcom_probe(struct platform_device *pdev)
 static void ufs_qcom_remove(struct platform_device *pdev)
 {
 	struct ufs_hba *hba =  platform_get_drvdata(pdev);
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
 
 	pm_runtime_get_sync(&(pdev)->dev);
 	ufshcd_remove(hba);
-	platform_device_msi_free_irqs_all(hba->dev);
+	if (host->esi_enabled)
+		platform_device_msi_free_irqs_all(hba->dev);
 }
 
 static const struct of_device_id ufs_qcom_of_match[] __maybe_unused = {

-- 
2.25.1

Re: [PATCH 2/5] scsi: ufs: qcom: Only free platform MSIs when ESI is enabled

[Bean Huo]

On Mon, 2024-11-11 at 23:18 +0530, Manivannan Sadhasivam via B4 Relay
wrote:
> Otherwise, it will result in a NULL pointer dereference as below:
> 
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000008
> Call trace:
>  mutex_lock+0xc/0x54
>  platform_device_msi_free_irqs_all+0x14/0x20
>  ufs_qcom_remove+0x34/0x48 [ufs_qcom]
>  platform_remove+0x28/0x44
>  device_remove+0x4c/0x80
>  device_release_driver_internal+0xd8/0x178
>  driver_detach+0x50/0x9c
>  bus_remove_driver+0x6c/0xbc
>  driver_unregister+0x30/0x60
>  platform_driver_unregister+0x14/0x20
>  ufs_qcom_pltform_exit+0x18/0xb94 [ufs_qcom]
>  __arm64_sys_delete_module+0x180/0x260
>  invoke_syscall+0x44/0x100
>  el0_svc_common.constprop.0+0xc0/0xe0
>  do_el0_svc+0x1c/0x28
>  el0_svc+0x34/0xdc
>  el0t_64_sync_handler+0xc0/0xc4
>  el0t_64_sync+0x190/0x194
> 
> Cc: stable@vger.kernel.org # 6.3
> Fixes: 519b6274a777 ("scsi: ufs: qcom: Add MCQ ESI config vendor
> specific ops")
> Signed-off-by: Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org>

Reviewed-by: Bean Huo <beanhuo@micron.com>

[PATCH 3/5] scsi: ufs: pltfrm: Disable runtime PM during removal of glue drivers

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

When the UFSHCD platform glue drivers are removed, runtime PM should be
disabled using pm_runtime_disable() to balance the enablement done in
ufshcd_pltfrm_init(). This is also reported by PM core when the glue driver
is removed and inserted again:

ufshcd-qcom 1d84000.ufshc: Unbalanced pm_runtime_enable!

So disable runtime PM using a new helper API ufshcd_pltfrm_remove(), that
also takes care of removing ufshcd. This helper should be called during the
remove() stage of glue drivers.

Cc: stable@vger.kernel.org # 3.12
Fixes: 62694735ca95 ("[SCSI] ufs: Add runtime PM support for UFS host controller driver")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/ufs/host/cdns-pltfrm.c        |  4 +---
 drivers/ufs/host/tc-dwc-g210-pltfrm.c |  4 +---
 drivers/ufs/host/ufs-exynos.c         |  2 +-
 drivers/ufs/host/ufs-hisi.c           |  4 +---
 drivers/ufs/host/ufs-mediatek.c       |  4 +---
 drivers/ufs/host/ufs-qcom.c           |  2 +-
 drivers/ufs/host/ufs-renesas.c        |  4 +---
 drivers/ufs/host/ufs-sprd.c           |  4 +---
 drivers/ufs/host/ufshcd-pltfrm.c      | 13 +++++++++++++
 drivers/ufs/host/ufshcd-pltfrm.h      |  1 +
 10 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/ufs/host/cdns-pltfrm.c b/drivers/ufs/host/cdns-pltfrm.c
index 66811d8d1929..b31aa8411151 100644
--- a/drivers/ufs/host/cdns-pltfrm.c
+++ b/drivers/ufs/host/cdns-pltfrm.c
@@ -307,9 +307,7 @@ static int cdns_ufs_pltfrm_probe(struct platform_device *pdev)
  */
 static void cdns_ufs_pltfrm_remove(struct platform_device *pdev)
 {
-	struct ufs_hba *hba =  platform_get_drvdata(pdev);
-
-	ufshcd_remove(hba);
+	ufshcd_pltfrm_remove(pdev);
 }
 
 static const struct dev_pm_ops cdns_ufs_dev_pm_ops = {
diff --git a/drivers/ufs/host/tc-dwc-g210-pltfrm.c b/drivers/ufs/host/tc-dwc-g210-pltfrm.c
index a3877592604d..113e0ef7b2cf 100644
--- a/drivers/ufs/host/tc-dwc-g210-pltfrm.c
+++ b/drivers/ufs/host/tc-dwc-g210-pltfrm.c
@@ -76,10 +76,8 @@ static int tc_dwc_g210_pltfm_probe(struct platform_device *pdev)
  */
 static void tc_dwc_g210_pltfm_remove(struct platform_device *pdev)
 {
-	struct ufs_hba *hba =  platform_get_drvdata(pdev);
-
 	pm_runtime_get_sync(&(pdev)->dev);
-	ufshcd_remove(hba);
+	ufshcd_pltfrm_remove(pdev);
 }
 
 static const struct dev_pm_ops tc_dwc_g210_pltfm_pm_ops = {
diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
index 5867e6338562..953103b69e2c 100644
--- a/drivers/ufs/host/ufs-exynos.c
+++ b/drivers/ufs/host/ufs-exynos.c
@@ -1961,7 +1961,7 @@ static void exynos_ufs_remove(struct platform_device *pdev)
 	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
 
 	pm_runtime_get_sync(&(pdev)->dev);
-	ufshcd_remove(hba);
+	ufshcd_pltfrm_remove(pdev);
 
 	phy_power_off(ufs->phy);
 	phy_exit(ufs->phy);
diff --git a/drivers/ufs/host/ufs-hisi.c b/drivers/ufs/host/ufs-hisi.c
index 5ee73ff05251..501609521b26 100644
--- a/drivers/ufs/host/ufs-hisi.c
+++ b/drivers/ufs/host/ufs-hisi.c
@@ -576,9 +576,7 @@ static int ufs_hisi_probe(struct platform_device *pdev)
 
 static void ufs_hisi_remove(struct platform_device *pdev)
 {
-	struct ufs_hba *hba =  platform_get_drvdata(pdev);
-
-	ufshcd_remove(hba);
+	ufshcd_pltfrm_remove(pdev);
 }
 
 static const struct dev_pm_ops ufs_hisi_pm_ops = {
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 9a5919434c4e..512b4e43787c 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -1869,10 +1869,8 @@ static int ufs_mtk_probe(struct platform_device *pdev)
  */
 static void ufs_mtk_remove(struct platform_device *pdev)
 {
-	struct ufs_hba *hba =  platform_get_drvdata(pdev);
-
 	pm_runtime_get_sync(&(pdev)->dev);
-	ufshcd_remove(hba);
+	ufshcd_pltfrm_remove(pdev);
 }
 
 #ifdef CONFIG_PM_SLEEP
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 1bcc538ef8af..38732dd48331 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1846,7 +1846,7 @@ static void ufs_qcom_remove(struct platform_device *pdev)
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
 
 	pm_runtime_get_sync(&(pdev)->dev);
-	ufshcd_remove(hba);
+	ufshcd_pltfrm_remove(pdev);
 	if (host->esi_enabled)
 		platform_device_msi_free_irqs_all(hba->dev);
 }
diff --git a/drivers/ufs/host/ufs-renesas.c b/drivers/ufs/host/ufs-renesas.c
index 8711e5cbc968..b3a5fc2d44e7 100644
--- a/drivers/ufs/host/ufs-renesas.c
+++ b/drivers/ufs/host/ufs-renesas.c
@@ -390,9 +390,7 @@ static int ufs_renesas_probe(struct platform_device *pdev)
 
 static void ufs_renesas_remove(struct platform_device *pdev)
 {
-	struct ufs_hba *hba = platform_get_drvdata(pdev);
-
-	ufshcd_remove(hba);
+	ufshcd_pltfrm_remove(pdev);
 }
 
 static struct platform_driver ufs_renesas_platform = {
diff --git a/drivers/ufs/host/ufs-sprd.c b/drivers/ufs/host/ufs-sprd.c
index d8b165908809..e455890cf7d4 100644
--- a/drivers/ufs/host/ufs-sprd.c
+++ b/drivers/ufs/host/ufs-sprd.c
@@ -427,10 +427,8 @@ static int ufs_sprd_probe(struct platform_device *pdev)
 
 static void ufs_sprd_remove(struct platform_device *pdev)
 {
-	struct ufs_hba *hba =  platform_get_drvdata(pdev);
-
 	pm_runtime_get_sync(&(pdev)->dev);
-	ufshcd_remove(hba);
+	ufshcd_pltfrm_remove(pdev);
 }
 
 static const struct dev_pm_ops ufs_sprd_pm_ops = {
diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
index 1f4f30d6cb42..bad5b1303eb6 100644
--- a/drivers/ufs/host/ufshcd-pltfrm.c
+++ b/drivers/ufs/host/ufshcd-pltfrm.c
@@ -524,6 +524,19 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 }
 EXPORT_SYMBOL_GPL(ufshcd_pltfrm_init);
 
+/**
+ * ufshcd_pltfrm_remove - Remove ufshcd platform
+ * @pdev: pointer to Platform device handle
+ */
+void ufshcd_pltfrm_remove(struct platform_device *pdev)
+{
+	struct ufs_hba *hba =  platform_get_drvdata(pdev);
+
+	ufshcd_remove(hba);
+	pm_runtime_disable(&pdev->dev);
+}
+EXPORT_SYMBOL_GPL(ufshcd_pltfrm_remove);
+
 MODULE_AUTHOR("Santosh Yaragnavi <santosh.sy@samsung.com>");
 MODULE_AUTHOR("Vinayak Holikatti <h.vinayak@samsung.com>");
 MODULE_DESCRIPTION("UFS host controller Platform bus based glue driver");
diff --git a/drivers/ufs/host/ufshcd-pltfrm.h b/drivers/ufs/host/ufshcd-pltfrm.h
index df387be5216b..3017f8e8f93c 100644
--- a/drivers/ufs/host/ufshcd-pltfrm.h
+++ b/drivers/ufs/host/ufshcd-pltfrm.h
@@ -31,6 +31,7 @@ int ufshcd_negotiate_pwr_params(const struct ufs_host_params *host_params,
 void ufshcd_init_host_params(struct ufs_host_params *host_params);
 int ufshcd_pltfrm_init(struct platform_device *pdev,
 		       const struct ufs_hba_variant_ops *vops);
+void ufshcd_pltfrm_remove(struct platform_device *pdev);
 int ufshcd_populate_vreg(struct device *dev, const char *name,
 			 struct ufs_vreg **out_vreg, bool skip_current);
 

-- 
2.25.1

Re: [PATCH 3/5] scsi: ufs: pltfrm: Disable runtime PM during removal of glue drivers

[Peter Wang (王信友)]

On Mon, 2024-11-11 at 23:18 +0530, Manivannan Sadhasivam via B4 Relay
wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> When the UFSHCD platform glue drivers are removed, runtime PM should
> be
> disabled using pm_runtime_disable() to balance the enablement done in
> ufshcd_pltfrm_init(). This is also reported by PM core when the glue
> driver
> is removed and inserted again:
> 
> ufshcd-qcom 1d84000.ufshc: Unbalanced pm_runtime_enable!
> 
> So disable runtime PM using a new helper API ufshcd_pltfrm_remove(),
> that
> also takes care of removing ufshcd. This helper should be called
> during the
> remove() stage of glue drivers.
> 
> Cc: stable@vger.kernel.org # 3.12
> Fixes: 62694735ca95 ("[SCSI] ufs: Add runtime PM support for UFS host
> controller driver")
> Signed-off-by: Manivannan Sadhasivam <
> manivannan.sadhasivam@linaro.org>
> ---
>  drivers/ufs/host/cdns-pltfrm.c        |  4 +---
>  drivers/ufs/host/tc-dwc-g210-pltfrm.c |  4 +---
>  drivers/ufs/host/ufs-exynos.c         |  2 +-
>  drivers/ufs/host/ufs-hisi.c           |  4 +---
>  drivers/ufs/host/ufs-mediatek.c       |  4 +---
>  drivers/ufs/host/ufs-qcom.c           |  2 +-
>  drivers/ufs/host/ufs-renesas.c        |  4 +---
>  drivers/ufs/host/ufs-sprd.c           |  4 +---
>  drivers/ufs/host/ufshcd-pltfrm.c      | 13 +++++++++++++
>  drivers/ufs/host/ufshcd-pltfrm.h      |  1 +
>  10 files changed, 22 insertions(+), 20 deletions(-)
> 
> 

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

Re: [PATCH 3/5] scsi: ufs: pltfrm: Disable runtime PM during removal of glue drivers

[Bean Huo]

Reviewed-by: Bean Huo <beanhuo@micron.com>

[PATCH 4/5] scsi: ufs: pltfrm: Drop PM runtime reference count after ufshcd_remove()

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

During the remove stage of glue drivers, some of them are incrementing the
reference count using pm_runtime_get_sync(), before removing the ufshcd
using ufshcd_remove(). But they are not dropping that reference count after
ufshcd_remove() to balance the refcount.

So drop the reference count by calling pm_runtime_put_noidle() after
ufshcd_remove(). Since the behavior is applicable to all glue drivers, move
the PM handling to ufshcd_pltfrm_remove().

Cc: stable@vger.kernel.org # 3.12
Fixes: 62694735ca95 ("[SCSI] ufs: Add runtime PM support for UFS host controller driver")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/ufs/host/tc-dwc-g210-pltfrm.c | 1 -
 drivers/ufs/host/ufs-exynos.c         | 1 -
 drivers/ufs/host/ufs-mediatek.c       | 1 -
 drivers/ufs/host/ufs-qcom.c           | 1 -
 drivers/ufs/host/ufs-sprd.c           | 1 -
 drivers/ufs/host/ufshcd-pltfrm.c      | 2 ++
 6 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/host/tc-dwc-g210-pltfrm.c b/drivers/ufs/host/tc-dwc-g210-pltfrm.c
index 113e0ef7b2cf..c6f8565ede21 100644
--- a/drivers/ufs/host/tc-dwc-g210-pltfrm.c
+++ b/drivers/ufs/host/tc-dwc-g210-pltfrm.c
@@ -76,7 +76,6 @@ static int tc_dwc_g210_pltfm_probe(struct platform_device *pdev)
  */
 static void tc_dwc_g210_pltfm_remove(struct platform_device *pdev)
 {
-	pm_runtime_get_sync(&(pdev)->dev);
 	ufshcd_pltfrm_remove(pdev);
 }
 
diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
index 953103b69e2c..6dedcc8b22b4 100644
--- a/drivers/ufs/host/ufs-exynos.c
+++ b/drivers/ufs/host/ufs-exynos.c
@@ -1960,7 +1960,6 @@ static void exynos_ufs_remove(struct platform_device *pdev)
 	struct ufs_hba *hba =  platform_get_drvdata(pdev);
 	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
 
-	pm_runtime_get_sync(&(pdev)->dev);
 	ufshcd_pltfrm_remove(pdev);
 
 	phy_power_off(ufs->phy);
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 512b4e43787c..c834d38921b6 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -1869,7 +1869,6 @@ static int ufs_mtk_probe(struct platform_device *pdev)
  */
 static void ufs_mtk_remove(struct platform_device *pdev)
 {
-	pm_runtime_get_sync(&(pdev)->dev);
 	ufshcd_pltfrm_remove(pdev);
 }
 
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 38732dd48331..91127fb17186 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1845,7 +1845,6 @@ static void ufs_qcom_remove(struct platform_device *pdev)
 	struct ufs_hba *hba =  platform_get_drvdata(pdev);
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
 
-	pm_runtime_get_sync(&(pdev)->dev);
 	ufshcd_pltfrm_remove(pdev);
 	if (host->esi_enabled)
 		platform_device_msi_free_irqs_all(hba->dev);
diff --git a/drivers/ufs/host/ufs-sprd.c b/drivers/ufs/host/ufs-sprd.c
index e455890cf7d4..d220978c2d8c 100644
--- a/drivers/ufs/host/ufs-sprd.c
+++ b/drivers/ufs/host/ufs-sprd.c
@@ -427,7 +427,6 @@ static int ufs_sprd_probe(struct platform_device *pdev)
 
 static void ufs_sprd_remove(struct platform_device *pdev)
 {
-	pm_runtime_get_sync(&(pdev)->dev);
 	ufshcd_pltfrm_remove(pdev);
 }
 
diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
index bad5b1303eb6..b8dadd0a2f4c 100644
--- a/drivers/ufs/host/ufshcd-pltfrm.c
+++ b/drivers/ufs/host/ufshcd-pltfrm.c
@@ -532,8 +532,10 @@ void ufshcd_pltfrm_remove(struct platform_device *pdev)
 {
 	struct ufs_hba *hba =  platform_get_drvdata(pdev);
 
+	pm_runtime_get_sync(&pdev->dev);
 	ufshcd_remove(hba);
 	pm_runtime_disable(&pdev->dev);
+	pm_runtime_put_noidle(&pdev->dev);
 }
 EXPORT_SYMBOL_GPL(ufshcd_pltfrm_remove);
 

-- 
2.25.1

Re: [PATCH 4/5] scsi: ufs: pltfrm: Drop PM runtime reference count after ufshcd_remove()

[Peter Wang (王信友)]

On Mon, 2024-11-11 at 23:18 +0530, Manivannan Sadhasivam via B4 Relay
wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> During the remove stage of glue drivers, some of them are
> incrementing the
> reference count using pm_runtime_get_sync(), before removing the
> ufshcd
> using ufshcd_remove(). But they are not dropping that reference count
> after
> ufshcd_remove() to balance the refcount.
> 
> So drop the reference count by calling pm_runtime_put_noidle() after
> ufshcd_remove(). Since the behavior is applicable to all glue
> drivers, move
> the PM handling to ufshcd_pltfrm_remove().
> 
> Cc: stable@vger.kernel.org # 3.12
> Fixes: 62694735ca95 ("[SCSI] ufs: Add runtime PM support for UFS host
> controller driver")
> Signed-off-by: Manivannan Sadhasivam <
> manivannan.sadhasivam@linaro.org>
> ---
>  drivers/ufs/host/tc-dwc-g210-pltfrm.c | 1 -
>  drivers/ufs/host/ufs-exynos.c         | 1 -
>  drivers/ufs/host/ufs-mediatek.c       | 1 -
>  drivers/ufs/host/ufs-qcom.c           | 1 -
>  drivers/ufs/host/ufs-sprd.c           | 1 -
>  drivers/ufs/host/ufshcd-pltfrm.c      | 2 ++
>  6 files changed, 2 insertions(+), 5 deletions(-)
> 
> 

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

Re: [PATCH 4/5] scsi: ufs: pltfrm: Drop PM runtime reference count after ufshcd_remove()

[Bean Huo]

Reviewed-by: Bean Huo <beanhuo@micron.com>

[PATCH 5/5] scsi: ufs: pltfrm: Dellocate HBA during ufshcd_pltfrm_remove()

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

This will ensure that the scsi host is cleaned up properly using
scsi_host_dev_release(). Otherwise, it may lead to memory leaks.

Cc: stable@vger.kernel.org # 4.4
Fixes: 03b1781aa978 ("[SCSI] ufs: Add Platform glue driver for ufshcd")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/ufs/host/ufshcd-pltfrm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
index b8dadd0a2f4c..505572d4fa87 100644
--- a/drivers/ufs/host/ufshcd-pltfrm.c
+++ b/drivers/ufs/host/ufshcd-pltfrm.c
@@ -534,6 +534,7 @@ void ufshcd_pltfrm_remove(struct platform_device *pdev)
 
 	pm_runtime_get_sync(&pdev->dev);
 	ufshcd_remove(hba);
+	ufshcd_dealloc_host(hba);
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
 }

-- 
2.25.1

Re: [PATCH 5/5] scsi: ufs: pltfrm: Dellocate HBA during ufshcd_pltfrm_remove()

[Peter Wang (王信友)]

On Mon, 2024-11-11 at 23:18 +0530, Manivannan Sadhasivam via B4 Relay
wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> This will ensure that the scsi host is cleaned up properly using
> scsi_host_dev_release(). Otherwise, it may lead to memory leaks.
> 
> Cc: stable@vger.kernel.org # 4.4
> Fixes: 03b1781aa978 ("[SCSI] ufs: Add Platform glue driver for
> ufshcd")
> Signed-off-by: Manivannan Sadhasivam <
> manivannan.sadhasivam@linaro.org>
> ---
>  drivers/ufs/host/ufshcd-pltfrm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> 

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

Re: [PATCH 5/5] scsi: ufs: pltfrm: Dellocate HBA during ufshcd_pltfrm_remove()

[Bean Huo]

Reviewed-by: Bean Huo <beanhuo@micron.com>

Re: [PATCH 0/5] scsi: ufs: Bug fixes for ufs core and platform drivers

[Martin K. Petersen]

Manivannan,

> This series has several bug fixes that I encountered when the ufs-qcom
> driver was removed and inserted back. But the fixes are applicable to
> other platform glue drivers as well.
>
> This series is tested on Qcom RB5 development board based on SM8250
> SoC.

Applied to 6.13/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/5] scsi: ufs: Bug fixes for ufs core and platform drivers

[Shawn Lin]

 > Hi,
 >
 > This series has several bug fixes that I encountered when the 
ufs-qcom > driver was removed and inserted back. But the fixes are 
applicable to
 > other platform glue drivers as well.
 >
 > This series is tested on Qcom RB5 development board based on SM8250
 > SoC.
 >

Test this series against the under-reviewed ufshc
for Rockchip, it works:

Tested-by: Shawn Lin <shawn.lin@rock-chips.com>