| Message ID | 57783ece7ddae55f2bda2f59f452180bff744ea0.1626257398.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v2] staging: board: Fix uninitialized spinlock when attaching genpd | expand |
On Wed, 14 Jul 2021 at 12:13, Geert Uytterhoeven <geert+renesas@glider.be> wrote: > > On Armadillo-800-EVA with CONFIG_DEBUG_SPINLOCK=y: > > BUG: spinlock bad magic on CPU#0, swapper/1 > lock: lcdc0_device+0x10c/0x308, .magic: 00000000, .owner: <none>/-1, .owner_cpu: 0 > CPU: 0 PID: 1 Comm: swapper Not tainted 5.11.0-rc5-armadillo-00036-gbbca04be7a80-dirty #287 > Hardware name: Generic R8A7740 (Flattened Device Tree) > [<c010c3c8>] (unwind_backtrace) from [<c010a49c>] (show_stack+0x10/0x14) > [<c010a49c>] (show_stack) from [<c0159534>] (do_raw_spin_lock+0x20/0x94) > [<c0159534>] (do_raw_spin_lock) from [<c040858c>] (dev_pm_get_subsys_data+0x8c/0x11c) > [<c040858c>] (dev_pm_get_subsys_data) from [<c05fbcac>] (genpd_add_device+0x78/0x2b8) > [<c05fbcac>] (genpd_add_device) from [<c0412db4>] (of_genpd_add_device+0x34/0x4c) > [<c0412db4>] (of_genpd_add_device) from [<c0a1ea74>] (board_staging_register_device+0x11c/0x148) > [<c0a1ea74>] (board_staging_register_device) from [<c0a1eac4>] (board_staging_register_devices+0x24/0x28) > > of_genpd_add_device() is called before platform_device_register(), as it > needs to attach the genpd before the device is probed. But the spinlock > is only initialized when the device is registered. > > Fix this by open-coding the spinlock initialization, cfr. > device_pm_init_common() in the internal drivers/base code, and in the > SuperH early platform code. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Exposed by fw_devlinks changing probe order. > Masked before due to an unrelated wait context check failure, which > disabled any further spinlock checks. > https://lore.kernel.org/linux-acpi/CAMuHMdVL-1RKJ5u-HDVA4F4w_+8yGvQQuJQBcZMsdV4yXzzfcw@mail.gmail.com > > v2: > - Improve code comment. > --- > drivers/staging/board/board.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c > index cb6feb34dd401ae3..f980af0373452cab 100644 > --- a/drivers/staging/board/board.c > +++ b/drivers/staging/board/board.c > @@ -136,6 +136,7 @@ int __init board_staging_register_clock(const struct board_staging_clk *bsc) > static int board_staging_add_dev_domain(struct platform_device *pdev, > const char *domain) > { > + struct device *dev = &pdev->dev; > struct of_phandle_args pd_args; > struct device_node *np; > > @@ -148,7 +149,11 @@ static int board_staging_add_dev_domain(struct platform_device *pdev, > pd_args.np = np; > pd_args.args_count = 0; > > - return of_genpd_add_device(&pd_args, &pdev->dev); > + /* Initialization similar to device_pm_init_common() */ > + spin_lock_init(&dev->power.lock); > + dev->power.early_init = true; > + > + return of_genpd_add_device(&pd_args, dev); It looks like the only device that is being managed here, is a platform device that corresponds to the sh_mobile_lcdc_driver (drivers/video/fbdev/sh_mobile_lcdcfb.c). Wouldn't it be better to move the domain information into the platform data of the above platform device and then deal with the attach to the genpd from the sh_mobile_lcdc_driver? In this way, the behaviour would be more consistent with others probe/attach to genpd - and $subject patch would not be needed. > } > #else > static inline int board_staging_add_dev_domain(struct platform_device *pdev, > -- > 2.25.1 > Kind regards Uffe
diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c index cb6feb34dd401ae3..f980af0373452cab 100644 --- a/drivers/staging/board/board.c +++ b/drivers/staging/board/board.c @@ -136,6 +136,7 @@ int __init board_staging_register_clock(const struct board_staging_clk *bsc) static int board_staging_add_dev_domain(struct platform_device *pdev, const char *domain) { + struct device *dev = &pdev->dev; struct of_phandle_args pd_args; struct device_node *np; @@ -148,7 +149,11 @@ static int board_staging_add_dev_domain(struct platform_device *pdev, pd_args.np = np; pd_args.args_count = 0; - return of_genpd_add_device(&pd_args, &pdev->dev); + /* Initialization similar to device_pm_init_common() */ + spin_lock_init(&dev->power.lock); + dev->power.early_init = true; + + return of_genpd_add_device(&pd_args, dev); } #else static inline int board_staging_add_dev_domain(struct platform_device *pdev,
On Armadillo-800-EVA with CONFIG_DEBUG_SPINLOCK=y: BUG: spinlock bad magic on CPU#0, swapper/1 lock: lcdc0_device+0x10c/0x308, .magic: 00000000, .owner: <none>/-1, .owner_cpu: 0 CPU: 0 PID: 1 Comm: swapper Not tainted 5.11.0-rc5-armadillo-00036-gbbca04be7a80-dirty #287 Hardware name: Generic R8A7740 (Flattened Device Tree) [<c010c3c8>] (unwind_backtrace) from [<c010a49c>] (show_stack+0x10/0x14) [<c010a49c>] (show_stack) from [<c0159534>] (do_raw_spin_lock+0x20/0x94) [<c0159534>] (do_raw_spin_lock) from [<c040858c>] (dev_pm_get_subsys_data+0x8c/0x11c) [<c040858c>] (dev_pm_get_subsys_data) from [<c05fbcac>] (genpd_add_device+0x78/0x2b8) [<c05fbcac>] (genpd_add_device) from [<c0412db4>] (of_genpd_add_device+0x34/0x4c) [<c0412db4>] (of_genpd_add_device) from [<c0a1ea74>] (board_staging_register_device+0x11c/0x148) [<c0a1ea74>] (board_staging_register_device) from [<c0a1eac4>] (board_staging_register_devices+0x24/0x28) of_genpd_add_device() is called before platform_device_register(), as it needs to attach the genpd before the device is probed. But the spinlock is only initialized when the device is registered. Fix this by open-coding the spinlock initialization, cfr. device_pm_init_common() in the internal drivers/base code, and in the SuperH early platform code. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Exposed by fw_devlinks changing probe order. Masked before due to an unrelated wait context check failure, which disabled any further spinlock checks. https://lore.kernel.org/linux-acpi/CAMuHMdVL-1RKJ5u-HDVA4F4w_+8yGvQQuJQBcZMsdV4yXzzfcw@mail.gmail.com v2: - Improve code comment. --- drivers/staging/board/board.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)