Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration

Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration

[Tj]

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");

Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration

[Bjorn Andersson]

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");
> 
> 

[PATCH 0/7] slimbus: qcom-ngd-ctrl: Fix some race conditions and deadlocks

[Bjorn Andersson]

When the qcom-ngd-ctrl driver is probed after the ADSP remoteproc, the
SSR notifier will fire immediately, which results in
qcom_slim_ngd_ssr_pdr_notify() attempting to schedule_work() on an
unitialized work_struct.

The concrete result of this is that my db845c/RB3 now fails to boot 100%
of the time.

In reviewing the problematic code, a few other problems where
discovered, such that platform_driver_unregister() is used to unregister
the child device.

Lastly, with the db845c booting, it was determined that attempting to
stop the ADSP remoteproc causes the slimbus driver to deadlock.

Note that while this solves the problems described above, and unblock
boot as well as restart of the remoteproc, this stack needs more love.

Upon tearing down the slimbus controller (when the ADSP goes down), the
slimbus devices attempts to access their slimbus devices - which is
prevented by the controller being runtime suspended. This results in a
wall of errors in the log, about failing transactions.

Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
---
Bjorn Andersson (7):
      slimbus: qcom-ngd-ctrl: Fix up platform_driver registration
      slimbus: qcom-ngd-ctrl: Fix probe error path ordering
      slimbus: qcom-ngd-ctrl: Correct PDR and SSR cleanup ownership
      slimbus: qcom-ngd-ctrl: Register callbacks after creating the ngd
      slimbus: qcom-ngd-ctrl: Initialize controller resources in controller
      slimbus: qcom-ngd-ctrl: Balance pm_runtime enablement for NGD
      slimbus: qcom-ngd-ctrl: Avoid ABBA on tx_lock/ctrl->lock

 drivers/slimbus/qcom-ngd-ctrl.c | 127 +++++++++++++++++++++++++---------------
 1 file changed, 80 insertions(+), 47 deletions(-)
---
base-commit: a0ae2a256046c0c5d3778d1a194ff2e171f16e5f
change-id: 20260211-slim-ngd-dev-74166f29f035

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>

[PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration

[Bjorn Andersson]

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

Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration

[Mukesh Ojha]

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

Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration

[Bjorn Andersson]

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

Re: [PATCH 1/7] slimbus: qcom-ngd-ctrl: Fix up platform_driver registration

[Dmitry Baryshkov]

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