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 <[email protected]>
---
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);
}
#else
static inline int board_staging_add_dev_domain(struct platform_device *pdev,
--
2.25.1
On Wed, 14 Jul 2021 at 12:13, Geert Uytterhoeven
<[email protected]> 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 <[email protected]>
> ---
> 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