reset: sunxi: fix spinlock initialization

PATCH New Tyler Baker 4 participants 10 messages Jan 8, 2015 – Jan 13, 2015

Subsystems (2)

Arm/Allwinner sunXi SoC support Reset controller framework

lore

Share & tools

lore

Tyler Baker

author

2015-01-08 00:54 UTC #0 lore raw

PATCH 1/1 reset: sunxi: fix spinlock initialization

Call spin_lock_init() before the spinlocks are used, preventing a lockdep splat.

I have been observing lockdep complaining [1] during boot on my a80 optimus [2] when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat, and has been tested on a few other sunxi platforms without issue.

[1] http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html
[2] http://kernelci.org/boot/?a80-optimus

Signed-off-by Tyler Baker <[email protected]>

---
This patch was tested on 3.19.0-rc3

 drivers/reset/reset-sunxi.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/reset/reset-sunxi.c +2 -0

index eebc52c..e37250c 100644
@@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct platform_device *pdev)
 	if (IS_ERR(data->membase))
 		return PTR_ERR(data->membase);
 
+	spin_lock_init(&data->lock);
+
 	data->rcdev.owner = THIS_MODULE;
 	data->rcdev.nr_resets = resource_size(res) * 32;
 	data->rcdev.ops = &sunxi_reset_ops;

Signature · 1 line

2.1.3

Philipp Zabel

maintainer Applied

2015-01-08 09:09 UTC #1 lore raw

PATCH 1/1 reset: sunxi: fix spinlock initialization

Hi Tyler,

Am Mittwoch, den 07.01.2015, 16:54 -0800 schrieb Tyler Baker:

Quoted 7 lines

Call spin_lock_init() before the spinlocks are used, preventing a lockdep splat.
I have been observing lockdep complaining [1] during boot on my a80 optimus [2] when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat, and has been tested on a few other sunxi platforms without issue.

Quoted 6 lines

[1] http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html
[2] http://kernelci.org/boot/?a80-optimus
Signed-off-by Tyler Baker <[email protected]>
---
This patch was tested on 3.19.0-rc3

Applied, thanks!

regards
Philipp

Maxime Ripard

2015-01-08 09:30 UTC #2 lore raw

PATCH 1/1 reset: sunxi: fix spinlock initialization

In reply to Tyler Baker

Hi Tyler,

Quoted 11 lines · Tyler Baker

Call spin_lock_init() before the spinlocks are used, preventing a lockdep splat.
I have been observing lockdep complaining [1] during boot on my a80 optimus [2] when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat, and has been tested on a few other sunxi platforms without issue.
[1] http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html
[2] http://kernelci.org/boot/?a80-optimus
Signed-off-by Tyler Baker <[email protected]>

Thanks for this patch.

Can you send this to stable too please?

Maxime

Signature · 10 lines

Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150108/f5474105/attachment.sig>

Tyler Baker

author

2015-01-09 06:38 UTC #3 lore raw

PATCH 1/1 reset: sunxi: fix spinlock initialization

Hi Maxime,

Quoted 10 lines · Maxime Ripard

Hi Tyler,
Quoted 7 lines · Tyler Baker
Call spin_lock_init() before the spinlocks are used, preventing a lockdep splat.
I have been observing lockdep complaining [1] during boot on my a80 optimus [2] when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat, and has been tested on a few other sunxi platforms without issue.

Quoted 6 lines

Quoted 4 lines
[1] http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html
[2] http://kernelci.org/boot/?a80-optimus
Signed-off-by Tyler Baker <[email protected]>
Thanks for this patch.

You're welcome.

Quoted 2 lines

Can you send this to stable too please?

Sure. Once it has been merged in Linus' tree I'll send it for -stable review.

Quoted 7 lines

Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Cheers,

Signature · 5 lines

Tyler Baker
Tech Lead, LAVA
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

Maxime Coquelin

2015-01-09 11:59 UTC #4 lore raw

PATCH 1/1 reset: sunxi: fix spinlock initialization

In reply to Tyler Baker

Hi Tyler, Maxime,

Quoted 22 lines · Tyler Baker

Call spin_lock_init() before the spinlocks are used, preventing a lockdep splat.
I have been observing lockdep complaining [1] during boot on my a80 optimus [2] when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat, and has been tested on a few other sunxi platforms without issue.
[1] http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html
[2] http://kernelci.org/boot/?a80-optimus
Signed-off-by Tyler Baker <[email protected]>
---
This patch was tested on 3.19.0-rc3
  drivers/reset/reset-sunxi.c | 2 ++
  1 file changed, 2 insertions(+)
drivers/reset/reset-sunxi.c +0 -0
index eebc52c..e37250c 100644

drivers/reset/reset-sunxi.c static int sunxi_reset_probe(struct platform_device *pdev) · line 161

Quoted 5 lines

drivers/reset/reset-sunxi.c +2 -0

@@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct platform_device *pdev)
  	if (IS_ERR(data->membase))
  		return PTR_ERR(data->membase);
 
+	spin_lock_init(&data->lock);
+

Shouldn't the lock be initialized also in early init function (sunxi_reset_init) ?

Regards,
Maxime

Quoted 3 lines

drivers/reset/reset-sunxi.c +0 -0

  	data->rcdev.owner = THIS_MODULE;
  	data->rcdev.nr_resets = resource_size(res) * 32;
  	data->rcdev.ops = &sunxi_reset_ops;

Maxime Coquelin

2015-01-09 14:44 UTC #5 lore raw

PATCH 1/1 reset: sunxi: fix spinlock initialization

With Maxime in the To: list.

Quoted 43 lines · Maxime Coquelin

Hi Tyler, Maxime,
Quoted 26 lines · Tyler Baker
Call spin_lock_init() before the spinlocks are used, preventing a lockdep
splat.
I have been observing lockdep complaining [1] during boot on my a80 optimus [2]
when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat,
and has been tested on a few other sunxi platforms without issue.
[1]
http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html
[2] http://kernelci.org/boot/?a80-optimus
Signed-off-by Tyler Baker <[email protected]>
---
This patch was tested on 3.19.0-rc3
  drivers/reset/reset-sunxi.c | 2 ++
  1 file changed, 2 insertions(+)
drivers/reset/reset-sunxi.c +0 -0
index eebc52c..e37250c 100644
drivers/reset/reset-sunxi.c static int sunxi_reset_probe(struct · line 161
Quoted 5 lines
drivers/reset/reset-sunxi.c +1 -0
@@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct 
 platform_device *pdev)
      if (IS_ERR(data->membase))
          return PTR_ERR(data->membase);
  +    spin_lock_init(&data->lock);
+
Shouldn't the lock be initialized also in early init function (sunxi_reset_init) ?
Regards,
Maxime
Quoted 3 lines
drivers/reset/reset-sunxi.c +0 -0
      data->rcdev.owner = THIS_MODULE;
      data->rcdev.nr_resets = resource_size(res) * 32;
      data->rcdev.ops = &sunxi_reset_ops;

Tyler Baker

author

2015-01-09 20:44 UTC #6 lore raw

PATCH 1/1 reset: sunxi: fix spinlock initialization

Hi Maxime,

Quoted 4 lines · Maxime Coquelin

With Maxime in the To: list.

Quoted 39 lines

Quoted 38 lines · Maxime Coquelin
Hi Tyler, Maxime,
Quoted 26 lines · Tyler Baker
Call spin_lock_init() before the spinlocks are used, preventing a lockdep splat.
I have been observing lockdep complaining [1] during boot on my a80 optimus [2]
when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat,
and has been tested on a few other sunxi platforms without issue.
[1]
http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html
[2] http://kernelci.org/boot/?a80-optimus
Signed-off-by Tyler Baker <[email protected]>
---
This patch was tested on 3.19.0-rc3
  drivers/reset/reset-sunxi.c | 2 ++
  1 file changed, 2 insertions(+)
drivers/reset/reset-sunxi.c +0 -0
index eebc52c..e37250c 100644
drivers/reset/reset-sunxi.c static int sunxi_reset_probe(struct platform_device · line 161
Quoted 5 lines
drivers/reset/reset-sunxi.c +1 -0
@@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct platform_device
 *pdev)
      if (IS_ERR(data->membase))
          return PTR_ERR(data->membase);
  +    spin_lock_init(&data->lock);
+
Shouldn't the lock be initialized also in early init function (sunxi_reset_init) ?

Thanks for pointing this out. It seems reasonable to me to add this for the sun6i platforms that use the early init function. Currently trying to track down a sun6i platform so I can run a few locking validation tests.

Quoted 10 lines

Quoted 9 lines

Regards,
Maxime

Quoted 4 lines

drivers/reset/reset-sunxi.c +0 -0

      data->rcdev.owner = THIS_MODULE;
      data->rcdev.nr_resets = resource_size(res) * 32;
      data->rcdev.ops = &sunxi_reset_ops;

Signature · 5 lines

Tyler Baker
Tech Lead, LAVA
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

Tyler Baker

author

2015-01-10 00:22 UTC #7 lore raw

PATCH 1/1 reset: sunxi: fix spinlock initialization

Quoted 47 lines · Tyler Baker

Hi Maxime,
Quoted 4 lines · Maxime Coquelin
With Maxime in the To: list.
Quoted 39 lines
Quoted 38 lines · Maxime Coquelin
Hi Tyler, Maxime,
Quoted 26 lines · Tyler Baker
Call spin_lock_init() before the spinlocks are used, preventing a lockdep splat.
I have been observing lockdep complaining [1] during boot on my a80 optimus [2]
when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat,
and has been tested on a few other sunxi platforms without issue.
[1]
http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html
[2] http://kernelci.org/boot/?a80-optimus
Signed-off-by Tyler Baker <[email protected]>
---
This patch was tested on 3.19.0-rc3
  drivers/reset/reset-sunxi.c | 2 ++
  1 file changed, 2 insertions(+)
drivers/reset/reset-sunxi.c +0 -0
index eebc52c..e37250c 100644
drivers/reset/reset-sunxi.c static int sunxi_reset_probe(struct platform_device · line 161
Quoted 5 lines
drivers/reset/reset-sunxi.c +1 -0
@@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct platform_device
 *pdev)
      if (IS_ERR(data->membase))
          return PTR_ERR(data->membase);
  +    spin_lock_init(&data->lock);
+
Shouldn't the lock be initialized also in early init function (sunxi_reset_init) ?

Quoted 4 lines

Thanks for pointing this out. It seems reasonable to me to add this for the sun6i platforms that use the early init function. Currently trying to track down a sun6i platform so I can run a few locking validation tests.

Olof was kind enough to volunteer his sun6i platform (colombus) and provide the results. The results confirms there is a need to also initialize the lock in sunxi_reset_init as Maxime C. mentioned.

DEBUG: sunxi_reset_init()
DEBUG: sunxi_reset_deassert()
INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.

CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.19.0-rc3-00039-gccd0de2-dirty #5
Hardware name: Allwinner sun6i (A31) Family
[<c021679c>] (unwind_backtrace) from [<c0212140>] (show_stack+0x10/0x14)
[<c0212140>] (show_stack) from [<c09557c4>] (dump_stack+0x8c/0x9c)
[<c09557c4>] (dump_stack) from [<c028293c>] (__lock_acquire+0x1ef4/0x1f00)
[<c028293c>] (__lock_acquire) from [<c028311c>] (lock_acquire+0x6c/0x8c)
[<c028311c>] (lock_acquire) from [<c095b810>] (_raw_spin_lock_irqsave+0x3c/0x50)

[<c095b810>] (_raw_spin_lock_irqsave) from [<c0542d80>] (sunxi_reset_deassert+0x2c/0x88)
[<c0542d80>] (sunxi_reset_deassert) from [<c0d39a00>] (sun5i_timer_init+0xc0/0x1f0)
[<c0d39a00>] (sun5i_timer_init) from [<c0d3897c>] (clocksource_of_init+0x4c/0x8c)
[<c0d3897c>] (clocksource_of_init) from [<c0cf0b34>] (start_kernel+0x278/0x3c8) [<c0cf0b34>] (start_kernel) from [<40208074>] (0x40208074)

I'll go ahead and resend an updated patch and cc stable if Philipp and Maxime R. they agree.

Quoted 14 lines

Quoted 11 lines
Quoted 10 lines
Regards,
Maxime
Quoted 4 lines
drivers/reset/reset-sunxi.c +0 -0
      data->rcdev.owner = THIS_MODULE;
      data->rcdev.nr_resets = resource_size(res) * 32;
      data->rcdev.ops = &sunxi_reset_ops;

Quoted 6 lines

--
Tyler Baker
Tech Lead, LAVA
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

Cheers,

Signature · 5 lines

Tyler Baker
Tech Lead, LAVA
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

Philipp Zabel

maintainer

2015-01-12 08:37 UTC #8 lore raw

PATCH 1/1 reset: sunxi: fix spinlock initialization

Hi Tyler,

Am Freitag, den 09.01.2015, 16:22 -0800 schrieb Tyler Baker:

Quoted 2 lines

I'll go ahead and resend an updated patch and cc stable if Philipp and Maxime R. they agree.

Please do.

thanks
Philipp

Maxime Coquelin

2015-01-13 08:59 UTC #9 lore raw

PATCH 1/1 reset: sunxi: fix spinlock initialization

In reply to Tyler Baker

Hi Tyler,

Quoted 73 lines · Tyler Baker

Quoted 43 lines · Tyler Baker
Hi Maxime,
Quoted 4 lines · Maxime Coquelin
With Maxime in the To: list.
Quoted 36 lines
Quoted 35 lines · Maxime Coquelin
Hi Tyler, Maxime,
Quoted 25 lines · Tyler Baker
Call spin_lock_init() before the spinlocks are used, preventing a lockdep splat.
I have been observing lockdep complaining [1] during boot on my a80 optimus [2]
when CONFIG_PROVE_LOCKING has been enabled. This patch resolves the splat,
and has been tested on a few other sunxi platforms without issue.
[1]
http://storage.kernelci.org/next/next-20150107/arm-multi_v7_defconfig+CONFIG_PROVE_LOCKING=y/lab-tbaker/boot-sun9i-a80-optimus.html
[2] http://kernelci.org/boot/?a80-optimus
Signed-off-by Tyler Baker <[email protected]>
---
This patch was tested on 3.19.0-rc3
   drivers/reset/reset-sunxi.c | 2 ++
   1 file changed, 2 insertions(+)
drivers/reset/reset-sunxi.c +0 -0
index eebc52c..e37250c 100644
drivers/reset/reset-sunxi.c static int sunxi_reset_probe(struct platform_device · line 161
Quoted 5 lines
drivers/reset/reset-sunxi.c +1 -0
@@ -157,6 +157,8 @@ static int sunxi_reset_probe(struct platform_device
 *pdev)
       if (IS_ERR(data->membase))
           return PTR_ERR(data->membase);
   +    spin_lock_init(&data->lock);
+
Shouldn't the lock be initialized also in early init function (sunxi_reset_init) ?
Quoted 4 lines
Thanks for pointing this out. It seems reasonable to me to add this for the sun6i platforms that use the early init function. Currently trying to track down a sun6i platform so I can run a few locking validation tests.
Olof was kind enough to volunteer his sun6i platform (colombus) and provide the results. The results confirms there is a need to also initialize the lock in sunxi_reset_init as Maxime C. mentioned.
DEBUG: sunxi_reset_init()
DEBUG: sunxi_reset_deassert()
INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.19.0-rc3-00039-gccd0de2-dirty #5
Hardware name: Allwinner sun6i (A31) Family
[<c021679c>] (unwind_backtrace) from [<c0212140>] (show_stack+0x10/0x14)
[<c0212140>] (show_stack) from [<c09557c4>] (dump_stack+0x8c/0x9c)
[<c09557c4>] (dump_stack) from [<c028293c>] (__lock_acquire+0x1ef4/0x1f00)
[<c028293c>] (__lock_acquire) from [<c028311c>] (lock_acquire+0x6c/0x8c)
[<c028311c>] (lock_acquire) from [<c095b810>] (_raw_spin_lock_irqsave+0x3c/0x50)
[<c095b810>] (_raw_spin_lock_irqsave) from [<c0542d80>] (sunxi_reset_deassert+0x2c/0x88)
[<c0542d80>] (sunxi_reset_deassert) from [<c0d39a00>] (sun5i_timer_init+0xc0/0x1f0)
[<c0d39a00>] (sun5i_timer_init) from [<c0d3897c>] (clocksource_of_init+0x4c/0x8c)
[<c0d3897c>] (clocksource_of_init) from [<c0cf0b34>] (start_kernel+0x278/0x3c8) [<c0cf0b34>] (start_kernel) from [<40208074>] (0x40208074)

Quoted 2 lines

I'll go ahead and resend an updated patch and cc stable if Philipp and Maxime R. they agree.

Thanks for having done the test.
I have no Allwinner HW to try.

Best regards,
Maxime

Quoted 17 lines

Quoted 9 lines
Quoted 7 lines
Quoted 7 lines
Regards,
Maxime
Quoted 3 lines
drivers/reset/reset-sunxi.c +0 -0
       data->rcdev.owner = THIS_MODULE;
       data->rcdev.nr_resets = resource_size(res) * 32;
       data->rcdev.ops = &sunxi_reset_ops;
Quoted 6 lines
--
Tyler Baker
Tech Lead, LAVA
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog
Cheers,