public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration
  2026-03-10  4:09 [PATCH 0/7] slimbus: qcom-ngd-ctrl: Fix some race conditions and deadlocks Bjorn Andersson
@ 2026-03-10  4:09 ` Bjorn Andersson
  2026-03-10  7:33   ` Mukesh Ojha
  2026-03-11  1:30   ` Dmitry Baryshkov
  0 siblings, 2 replies; 6+ messages in thread
From: Bjorn Andersson @ 2026-03-10  4:09 UTC (permalink / raw)
  To: Srinivas Kandagatla, Greg Kroah-Hartman, Vinod Koul,
	Krzysztof Kozlowski
  Cc: linux-arm-msm, linux-sound, linux-kernel, Bjorn Andersson, stable

Device drivers should not invoke platform_driver_register()/unregister()
in their probe and remove paths. They should further not rely on
platform_driver_unregister() as their only means of "deleting" their
child devices.

Introduce a helper to unregister the child device and move the
platform_driver_register()/unregister() to module_init()/exit().

Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
Cc: stable@vger.kernel.org
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
---
 drivers/slimbus/qcom-ngd-ctrl.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index 9aa7218b4e8d2b350835626839371ed6e19860e2..c69656a0ef1766d5a9df40bdf37bae8f64789fab 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -1562,6 +1562,13 @@ static int of_qcom_slim_ngd_register(struct device *parent,
 	return -ENODEV;
 }
 
+static void qcom_slim_ngd_unregister(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	struct qcom_slim_ngd *ngd = ctrl->ngd;
+
+	platform_device_del(ngd->pdev);
+}
+
 static int qcom_slim_ngd_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1664,7 +1671,6 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
 		goto err_pdr_lookup;
 	}
 
-	platform_driver_register(&qcom_slim_ngd_driver);
 	return of_qcom_slim_ngd_register(dev, ctrl);
 
 err_pdr_alloc:
@@ -1678,7 +1684,9 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
 
 static void qcom_slim_ngd_ctrl_remove(struct platform_device *pdev)
 {
-	platform_driver_unregister(&qcom_slim_ngd_driver);
+	struct qcom_slim_ngd_ctrl *ctrl = platform_get_drvdata(pdev);
+
+	qcom_slim_ngd_unregister(ctrl);
 }
 
 static void qcom_slim_ngd_remove(struct platform_device *pdev)
@@ -1754,6 +1762,28 @@ static struct platform_driver qcom_slim_ngd_driver = {
 	},
 };
 
-module_platform_driver(qcom_slim_ngd_ctrl_driver);
+static int qcom_slim_ngd_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&qcom_slim_ngd_ctrl_driver);
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&qcom_slim_ngd_driver);
+	if (ret)
+		platform_driver_unregister(&qcom_slim_ngd_ctrl_driver);
+
+	return ret;
+}
+
+static void qcom_slim_ngd_exit(void)
+{
+	platform_driver_unregister(&qcom_slim_ngd_driver);
+	platform_driver_unregister(&qcom_slim_ngd_ctrl_driver);
+}
+
+module_init(qcom_slim_ngd_init);
+module_exit(qcom_slim_ngd_exit);
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Qualcomm SLIMBus NGD controller");

-- 
2.51.0


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

* Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration
  2026-03-10  4:09 ` [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration Bjorn Andersson
@ 2026-03-10  7:33   ` Mukesh Ojha
  2026-04-01  3:06     ` Bjorn Andersson
  2026-03-11  1:30   ` Dmitry Baryshkov
  1 sibling, 1 reply; 6+ messages in thread
From: Mukesh Ojha @ 2026-03-10  7:33 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Srinivas Kandagatla, Greg Kroah-Hartman, Vinod Koul,
	Krzysztof Kozlowski, linux-arm-msm, linux-sound, linux-kernel,
	stable

On Mon, Mar 09, 2026 at 11:09:02PM -0500, Bjorn Andersson wrote:
> Device drivers should not invoke platform_driver_register()/unregister()
> in their probe and remove paths. They should further not rely on
> platform_driver_unregister() as their only means of "deleting" their
> child devices.
> 
> Introduce a helper to unregister the child device and move the
> platform_driver_register()/unregister() to module_init()/exit().
> 
> Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>

Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

> ---
>  drivers/slimbus/qcom-ngd-ctrl.c | 36 +++++++++++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
> index 9aa7218b4e8d2b350835626839371ed6e19860e2..c69656a0ef1766d5a9df40bdf37bae8f64789fab 100644
> --- a/drivers/slimbus/qcom-ngd-ctrl.c
> +++ b/drivers/slimbus/qcom-ngd-ctrl.c
> @@ -1562,6 +1562,13 @@ static int of_qcom_slim_ngd_register(struct device *parent,
>  	return -ENODEV;
>  }
>  
> +static void qcom_slim_ngd_unregister(struct qcom_slim_ngd_ctrl *ctrl)
> +{
> +	struct qcom_slim_ngd *ngd = ctrl->ngd;
> +
> +	platform_device_del(ngd->pdev);

First, it surprised me why only once, then I saw there is return 0 in
for_each_available_child_of_node_scoped() loop..

> +}
> +
>  static int qcom_slim_ngd_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -1664,7 +1671,6 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
>  		goto err_pdr_lookup;
>  	}
>  
> -	platform_driver_register(&qcom_slim_ngd_driver);
>  	return of_qcom_slim_ngd_register(dev, ctrl);
>  
>  err_pdr_alloc:
> @@ -1678,7 +1684,9 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
>  
>  static void qcom_slim_ngd_ctrl_remove(struct platform_device *pdev)
>  {
> -	platform_driver_unregister(&qcom_slim_ngd_driver);
> +	struct qcom_slim_ngd_ctrl *ctrl = platform_get_drvdata(pdev);
> +
> +	qcom_slim_ngd_unregister(ctrl);
>  }
>  
>  static void qcom_slim_ngd_remove(struct platform_device *pdev)
> @@ -1754,6 +1762,28 @@ static struct platform_driver qcom_slim_ngd_driver = {
>  	},
>  };
>  
> -module_platform_driver(qcom_slim_ngd_ctrl_driver);
> +static int qcom_slim_ngd_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&qcom_slim_ngd_ctrl_driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = platform_driver_register(&qcom_slim_ngd_driver);
> +	if (ret)
> +		platform_driver_unregister(&qcom_slim_ngd_ctrl_driver);
> +
> +	return ret;
> +}
> +
> +static void qcom_slim_ngd_exit(void)
> +{
> +	platform_driver_unregister(&qcom_slim_ngd_driver);
> +	platform_driver_unregister(&qcom_slim_ngd_ctrl_driver);
> +}
> +
> +module_init(qcom_slim_ngd_init);
> +module_exit(qcom_slim_ngd_exit);
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Qualcomm SLIMBus NGD controller");
> 
> -- 
> 2.51.0
> 

-- 
-Mukesh Ojha

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

* Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration
  2026-03-10  4:09 ` [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration Bjorn Andersson
  2026-03-10  7:33   ` Mukesh Ojha
@ 2026-03-11  1:30   ` Dmitry Baryshkov
  1 sibling, 0 replies; 6+ messages in thread
From: Dmitry Baryshkov @ 2026-03-11  1:30 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Srinivas Kandagatla, Greg Kroah-Hartman, Vinod Koul,
	Krzysztof Kozlowski, linux-arm-msm, linux-sound, linux-kernel,
	stable

On Mon, Mar 09, 2026 at 11:09:02PM -0500, Bjorn Andersson wrote:
> Device drivers should not invoke platform_driver_register()/unregister()
> in their probe and remove paths. They should further not rely on
> platform_driver_unregister() as their only means of "deleting" their
> child devices.
> 
> Introduce a helper to unregister the child device and move the
> platform_driver_register()/unregister() to module_init()/exit().
> 
> Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> ---
>  drivers/slimbus/qcom-ngd-ctrl.c | 36 +++++++++++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

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

* Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration
@ 2026-03-13 12:36 Tj
  2026-03-31 22:32 ` Bjorn Andersson
  0 siblings, 1 reply; 6+ messages in thread
From: Tj @ 2026-03-13 12:36 UTC (permalink / raw)
  To: bjorn.andersson
  Cc: gregkh, krzk, linux-arm-msm, linux-kernel, linux-sound, srini,
	stable, vkoul, Konrad Dybcio

Following up on the registration problems on recommendation of Konrad 
Dybcio.

I previously reported a hang during driver registration due to lock 
contention. Konrad pointed me to this thread. Earlier, I had fixed the 
issue myself and whilst doing it saw that order of registration is 
important - ctrl must be last otherwise it causes:

qcom,slim-ngd-ctrl 171c0000.slim-ngd: Failed to create device link 
(0x180) with supplier 34000000.pinctrl for 
/soc@0/slim-ngd@171c0000/slim@1/codec@1,0

so I'd be surprised if Patch 1 works (not had chance to test the patch 
series as yet).

With my local patch the result is:

qcom,slim-ngd-ctrl 171c0000.slim-ngd: SLIM SAT: Rcvd master capability
qcom,slim-ngd-ctrl 171c0000.slim-ngd: SLIM controller Registered

---
     slimbus: ngd: fix lock hang on probe

     Module contains two platform_drivers. The initial probe calls
     platform_register_driver() with the second struct platform_driver.

     This caused a hung task due to mutex lock in __driver_attach():

     INFO: task swapper/0:1 blocked for more than 1232 seconds.
                Not tainted 7.0.0.-rc2-sdm845 #89
     task:swapper/0    state:D pid:1 tgid:1 ppid:0 task_flags:0x0140 
flags:0x00000010
     Call trace:
     __switch_to_0x104/0x1c8 (T)
     __schedule+0x438/0x1168
     schedule+0x3c/0x120
     schedule_preempt_disabled+0x2c/0x50
     __mutex_lock.constprop.0+0x3d0/0x938
     __mutex_lock_slowpath+0x1c/0x30
     __driver_attach+0x38/0x280
     bus_for_each_dev+0x80/0xc8
     driver_attach+0x2c/0x40
     bus_add_driver+0x128/0x258
     driver_register+0x68/0x138
     __platform_driver_register+0x2c/0x40
     qcom_slim_ngd_ctrl_probe+0x1f4/0x400
     platform_probe+0x64/0xa8
     really_probe+0xc8/0x3f0
     __driver_probe_device+0x88/0x190
     driver_probe_device+0x44/0x120
     __driver_attach+0x138/0x280
     bus_for_each_dev+0x80/0xc8
     driver_attach+0x2c/0x40
     bus_add_driver+0x128/0x258
     driver_register+0x68/0x138
     __platform_driver_register+0x2c/0x40
     qcom_slim_ngd_ctrl_driver_init+0x24/0x38
     do_one_initcall+0x60/0x450
     kernel_init_freeable+0x23c/0x630
     kernel_init+0x2c/0x1f8
     ret_from_fork+0x10/0x20
     INFO: task swapper/0:1 is blocked on a mutex likely owned by task
     swapper/0:1.

     Showing all locks held in the system:
     2 locks held by swapper/0/1:
      #0: ffff000080ff80f8 (&dev->mutex){....}-{4:4}, at:
          __driver_attach+0x19c/0x2c0
      #1: ffff000080ff80f8 (&dev->mutex){....}-{4:4}, at:
          __driver_attach+0x38/0x2c0
     1 lock held by khungtaskd/73:
      #0: ffffbc5dfc38f1d8 (rcu_read_lock){....}-{1:3}, at:
          debug_show_all_locks+0x18/0x1f0

     After this fix:

     qcom,slim-ngd-ctrl 171c0000.slim-ngd: SLIM SAT: Rcvd master capability
     qcom,slim-ngd-ctrl 171c0000.slim-ngd: SLIM controller Registered

diff --git a/drivers/slimbus/qcom-ngd-ctrl.c 
b/drivers/slimbus/qcom-ngd-ctrl.c
index 9aa7218b4e8d2..abdd4ad57f2d2 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -1664,7 +1664,6 @@ static int qcom_slim_ngd_ctrl_probe(struct 
platform_device *pdev)
                 goto err_pdr_lookup;
         }

-       platform_driver_register(&qcom_slim_ngd_driver);
         return of_qcom_slim_ngd_register(dev, ctrl);

  err_pdr_alloc:
@@ -1754,6 +1753,23 @@ static struct platform_driver 
qcom_slim_ngd_driver = {
         },
  };

-module_platform_driver(qcom_slim_ngd_ctrl_driver);
+static struct platform_driver * const qcom_slim_ngd_drivers[] = {
+       /* Order here is important; ctrl last */
+       &qcom_slim_ngd_driver,
+       &qcom_slim_ngd_ctrl_driver,
+};
+
+static int __init qcom_slim_ngd_init(void)
+{
+       return platform_register_drivers(qcom_slim_ngd_drivers, 
ARRAY_SIZE(qcom_slim_ngd_drivers));
+}
+module_init(qcom_slim_ngd_init);
+
+static void __exit qcom_slim_ngd_exit(void)
+{
+       return platform_unregister_drivers(qcom_slim_ngd_drivers, 
ARRAY_SIZE(qcom_slim_ngd_drivers));
+}
+module_exit(qcom_slim_ngd_exit);
+
  MODULE_LICENSE("GPL v2");
  MODULE_DESCRIPTION("Qualcomm SLIMBus NGD controller");



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

* Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration
  2026-03-13 12:36 [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration Tj
@ 2026-03-31 22:32 ` Bjorn Andersson
  0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2026-03-31 22:32 UTC (permalink / raw)
  To: Tj
  Cc: bjorn.andersson, gregkh, krzk, linux-arm-msm, linux-kernel,
	linux-sound, srini, stable, vkoul, Konrad Dybcio

On Fri, Mar 13, 2026 at 12:36:54PM +0000, Tj wrote:
> Following up on the registration problems on recommendation of Konrad 
> Dybcio.
> 
> I previously reported a hang during driver registration due to lock 
> contention. Konrad pointed me to this thread. Earlier, I had fixed the 
> issue myself and whilst doing it saw that order of registration is 
> important - ctrl must be last otherwise it causes:
> 

I've not seen that issue in my testing, but in the event that
qcom_slim_ngd_ctrl_probe() is being called before qcom_slim_ngd_driver
is registered we might complete the qcom_slim_ngd_ctrl_probe() with an
incomplete ngd instance - as qcom_slim_ngd_probe() has yet to execute.

I can flip the two platform_driver_register() calls around just to
remove/reduce the likelyhood for this to happen.


I think the correct solution here is to not have a separate
platform_driver for the inner device. I did some more work towards that,
but think it would be good to land this portion first.

> qcom,slim-ngd-ctrl 171c0000.slim-ngd: Failed to create device link 
> (0x180) with supplier 34000000.pinctrl for 
> /soc@0/slim-ngd@171c0000/slim@1/codec@1,0
> 

I don't think I've seen this in my testing.

> so I'd be surprised if Patch 1 works (not had chance to test the patch 
> series as yet).

It works on my machine<tm>, but the rest of the series is required as
well.

Regards,
Bjorn

> 
> With my local patch the result is:
> 
> qcom,slim-ngd-ctrl 171c0000.slim-ngd: SLIM SAT: Rcvd master capability
> qcom,slim-ngd-ctrl 171c0000.slim-ngd: SLIM controller Registered
> 
> ---
>      slimbus: ngd: fix lock hang on probe
> 
>      Module contains two platform_drivers. The initial probe calls
>      platform_register_driver() with the second struct platform_driver.
> 
>      This caused a hung task due to mutex lock in __driver_attach():
> 
>      INFO: task swapper/0:1 blocked for more than 1232 seconds.
>                 Not tainted 7.0.0.-rc2-sdm845 #89
>      task:swapper/0    state:D pid:1 tgid:1 ppid:0 task_flags:0x0140 
> flags:0x00000010
>      Call trace:
>      __switch_to_0x104/0x1c8 (T)
>      __schedule+0x438/0x1168
>      schedule+0x3c/0x120
>      schedule_preempt_disabled+0x2c/0x50
>      __mutex_lock.constprop.0+0x3d0/0x938
>      __mutex_lock_slowpath+0x1c/0x30
>      __driver_attach+0x38/0x280
>      bus_for_each_dev+0x80/0xc8
>      driver_attach+0x2c/0x40
>      bus_add_driver+0x128/0x258
>      driver_register+0x68/0x138
>      __platform_driver_register+0x2c/0x40
>      qcom_slim_ngd_ctrl_probe+0x1f4/0x400
>      platform_probe+0x64/0xa8
>      really_probe+0xc8/0x3f0
>      __driver_probe_device+0x88/0x190
>      driver_probe_device+0x44/0x120
>      __driver_attach+0x138/0x280
>      bus_for_each_dev+0x80/0xc8
>      driver_attach+0x2c/0x40
>      bus_add_driver+0x128/0x258
>      driver_register+0x68/0x138
>      __platform_driver_register+0x2c/0x40
>      qcom_slim_ngd_ctrl_driver_init+0x24/0x38
>      do_one_initcall+0x60/0x450
>      kernel_init_freeable+0x23c/0x630
>      kernel_init+0x2c/0x1f8
>      ret_from_fork+0x10/0x20
>      INFO: task swapper/0:1 is blocked on a mutex likely owned by task
>      swapper/0:1.
> 
>      Showing all locks held in the system:
>      2 locks held by swapper/0/1:
>       #0: ffff000080ff80f8 (&dev->mutex){....}-{4:4}, at:
>           __driver_attach+0x19c/0x2c0
>       #1: ffff000080ff80f8 (&dev->mutex){....}-{4:4}, at:
>           __driver_attach+0x38/0x2c0
>      1 lock held by khungtaskd/73:
>       #0: ffffbc5dfc38f1d8 (rcu_read_lock){....}-{1:3}, at:
>           debug_show_all_locks+0x18/0x1f0
> 
>      After this fix:
> 
>      qcom,slim-ngd-ctrl 171c0000.slim-ngd: SLIM SAT: Rcvd master capability
>      qcom,slim-ngd-ctrl 171c0000.slim-ngd: SLIM controller Registered
> 
> diff --git a/drivers/slimbus/qcom-ngd-ctrl.c 
> b/drivers/slimbus/qcom-ngd-ctrl.c
> index 9aa7218b4e8d2..abdd4ad57f2d2 100644
> --- a/drivers/slimbus/qcom-ngd-ctrl.c
> +++ b/drivers/slimbus/qcom-ngd-ctrl.c
> @@ -1664,7 +1664,6 @@ static int qcom_slim_ngd_ctrl_probe(struct 
> platform_device *pdev)
>                  goto err_pdr_lookup;
>          }
> 
> -       platform_driver_register(&qcom_slim_ngd_driver);
>          return of_qcom_slim_ngd_register(dev, ctrl);
> 
>   err_pdr_alloc:
> @@ -1754,6 +1753,23 @@ static struct platform_driver 
> qcom_slim_ngd_driver = {
>          },
>   };
> 
> -module_platform_driver(qcom_slim_ngd_ctrl_driver);
> +static struct platform_driver * const qcom_slim_ngd_drivers[] = {
> +       /* Order here is important; ctrl last */
> +       &qcom_slim_ngd_driver,
> +       &qcom_slim_ngd_ctrl_driver,
> +};
> +
> +static int __init qcom_slim_ngd_init(void)
> +{
> +       return platform_register_drivers(qcom_slim_ngd_drivers, 
> ARRAY_SIZE(qcom_slim_ngd_drivers));
> +}
> +module_init(qcom_slim_ngd_init);
> +
> +static void __exit qcom_slim_ngd_exit(void)
> +{
> +       return platform_unregister_drivers(qcom_slim_ngd_drivers, 
> ARRAY_SIZE(qcom_slim_ngd_drivers));
> +}
> +module_exit(qcom_slim_ngd_exit);
> +
>   MODULE_LICENSE("GPL v2");
>   MODULE_DESCRIPTION("Qualcomm SLIMBus NGD controller");
> 
> 

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

* Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration
  2026-03-10  7:33   ` Mukesh Ojha
@ 2026-04-01  3:06     ` Bjorn Andersson
  0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2026-04-01  3:06 UTC (permalink / raw)
  To: Mukesh Ojha
  Cc: Bjorn Andersson, Srinivas Kandagatla, Greg Kroah-Hartman,
	Vinod Koul, Krzysztof Kozlowski, linux-arm-msm, linux-sound,
	linux-kernel, stable

On Tue, Mar 10, 2026 at 01:03:09PM +0530, Mukesh Ojha wrote:
> On Mon, Mar 09, 2026 at 11:09:02PM -0500, Bjorn Andersson wrote:
> > Device drivers should not invoke platform_driver_register()/unregister()
> > in their probe and remove paths. They should further not rely on
> > platform_driver_unregister() as their only means of "deleting" their
> > child devices.
> > 
> > Introduce a helper to unregister the child device and move the
> > platform_driver_register()/unregister() to module_init()/exit().
> > 
> > Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> 
> Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> 
> > ---
> >  drivers/slimbus/qcom-ngd-ctrl.c | 36 +++++++++++++++++++++++++++++++++---
> >  1 file changed, 33 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
> > index 9aa7218b4e8d2b350835626839371ed6e19860e2..c69656a0ef1766d5a9df40bdf37bae8f64789fab 100644
> > --- a/drivers/slimbus/qcom-ngd-ctrl.c
> > +++ b/drivers/slimbus/qcom-ngd-ctrl.c
> > @@ -1562,6 +1562,13 @@ static int of_qcom_slim_ngd_register(struct device *parent,
> >  	return -ENODEV;
> >  }
> >  
> > +static void qcom_slim_ngd_unregister(struct qcom_slim_ngd_ctrl *ctrl)
> > +{
> > +	struct qcom_slim_ngd *ngd = ctrl->ngd;
> > +
> > +	platform_device_del(ngd->pdev);
> 
> First, it surprised me why only once, then I saw there is return 0 in
> for_each_available_child_of_node_scoped() loop..
> 

Yeah, I'm not sure about that one. If I read the binding correctly it
allows for an arbitrary number of child devices, but the implementation
will only consider the first active child, and silently ignore any
others.

I don't know if it's a valid usecase to have multiple NGDs on a
controller thought?

Regards,
Bjorn

> > +}
> > +
> >  static int qcom_slim_ngd_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> > @@ -1664,7 +1671,6 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
> >  		goto err_pdr_lookup;
> >  	}
> >  
> > -	platform_driver_register(&qcom_slim_ngd_driver);
> >  	return of_qcom_slim_ngd_register(dev, ctrl);
> >  
> >  err_pdr_alloc:
> > @@ -1678,7 +1684,9 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
> >  
> >  static void qcom_slim_ngd_ctrl_remove(struct platform_device *pdev)
> >  {
> > -	platform_driver_unregister(&qcom_slim_ngd_driver);
> > +	struct qcom_slim_ngd_ctrl *ctrl = platform_get_drvdata(pdev);
> > +
> > +	qcom_slim_ngd_unregister(ctrl);
> >  }
> >  
> >  static void qcom_slim_ngd_remove(struct platform_device *pdev)
> > @@ -1754,6 +1762,28 @@ static struct platform_driver qcom_slim_ngd_driver = {
> >  	},
> >  };
> >  
> > -module_platform_driver(qcom_slim_ngd_ctrl_driver);
> > +static int qcom_slim_ngd_init(void)
> > +{
> > +	int ret;
> > +
> > +	ret = platform_driver_register(&qcom_slim_ngd_ctrl_driver);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = platform_driver_register(&qcom_slim_ngd_driver);
> > +	if (ret)
> > +		platform_driver_unregister(&qcom_slim_ngd_ctrl_driver);
> > +
> > +	return ret;
> > +}
> > +
> > +static void qcom_slim_ngd_exit(void)
> > +{
> > +	platform_driver_unregister(&qcom_slim_ngd_driver);
> > +	platform_driver_unregister(&qcom_slim_ngd_ctrl_driver);
> > +}
> > +
> > +module_init(qcom_slim_ngd_init);
> > +module_exit(qcom_slim_ngd_exit);
> >  MODULE_LICENSE("GPL v2");
> >  MODULE_DESCRIPTION("Qualcomm SLIMBus NGD controller");
> > 
> > -- 
> > 2.51.0
> > 
> 
> -- 
> -Mukesh Ojha

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

end of thread, other threads:[~2026-04-01  3:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-13 12:36 [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration Tj
2026-03-31 22:32 ` Bjorn Andersson
  -- strict thread matches above, loose matches on Subject: below --
2026-03-10  4:09 [PATCH 0/7] slimbus: qcom-ngd-ctrl: Fix some race conditions and deadlocks Bjorn Andersson
2026-03-10  4:09 ` [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration Bjorn Andersson
2026-03-10  7:33   ` Mukesh Ojha
2026-04-01  3:06     ` Bjorn Andersson
2026-03-11  1:30   ` Dmitry Baryshkov

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