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

[Bjorn Andersson <andersson@kernel.org>]

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