Patchwork rtc: rtc-imxdi: add missing spin lock initialization

login
register
mail settings
Submitter Jan Luebbe
Date Oct. 24, 2012, 10:20 a.m.
Message ID <1351074042-8814-1-git-send-email-jlu@pengutronix.de>
Download mbox | patch
Permalink /patch/193726/
State New
Headers show

Comments

Jan Luebbe - Oct. 24, 2012, 10:20 a.m.
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Roland Stigge <stigge@antcom.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---

I've seen that Andrew Morton has taken three patches for rtc-imxdi from
Roland Stigge into the -mm tree. So it might make sense for Andrew to
take this as well.

 drivers/rtc/rtc-imxdi.c |    2 ++
 1 file changed, 2 insertions(+)
Andrew Morton - Oct. 24, 2012, 8:20 p.m.
On Wed, 24 Oct 2012 12:20:42 +0200
Jan Luebbe <jlu@pengutronix.de> wrote:

> --- a/drivers/rtc/rtc-imxdi.c
> +++ b/drivers/rtc/rtc-imxdi.c
> @@ -393,6 +393,8 @@ static int dryice_rtc_probe(struct platform_device *pdev)
>  	if (imxdi->ioaddr == NULL)
>  		return -ENOMEM;
>  
> +	spin_lock_init(&imxdi->irq_lock);
> +
>  	imxdi->irq = platform_get_irq(pdev, 0);
>  	if (imxdi->irq < 0)
>  		return imxdi->irq;

hm, that bug's been there for a while, yes?

I wonder why.  I assume nobody has been running this driver on SMP kernels?
stigge@antcom.de - Oct. 25, 2012, 3:37 a.m.
On 10/24/2012 10:20 PM, Andrew Morton wrote:
> On Wed, 24 Oct 2012 12:20:42 +0200
> Jan Luebbe <jlu@pengutronix.de> wrote:
> 
>> --- a/drivers/rtc/rtc-imxdi.c
>> +++ b/drivers/rtc/rtc-imxdi.c
>> @@ -393,6 +393,8 @@ static int dryice_rtc_probe(struct platform_device *pdev)
>>  	if (imxdi->ioaddr == NULL)
>>  		return -ENOMEM;
>>  
>> +	spin_lock_init(&imxdi->irq_lock);
>> +
>>  	imxdi->irq = platform_get_irq(pdev, 0);
>>  	if (imxdi->irq < 0)
>>  		return imxdi->irq;
> 
> hm, that bug's been there for a while, yes?

Right.

> I wonder why.  I assume nobody has been running this driver on SMP kernels?

Yes: The platforms where this driver was enabled (at least in mainline)
have only 1 core. Further, they typically employ external RTCs. So I
guess the issue surfaced via code review rather than a crash or sth.? ;-)

Anyway:

Tested-by: Roland Stigge <stigge@antcom.de>

Thanks to Jan!
Robert Schwebel - Oct. 25, 2012, 6:39 a.m.
On Thu, Oct 25, 2012 at 05:37:09AM +0200, Roland Stigge wrote:
> > I wonder why.  I assume nobody has been running this driver on SMP kernels?
>
> Yes: The platforms where this driver was enabled (at least in mainline)
> have only 1 core. Further, they typically employ external RTCs. So I
> guess the issue surfaced via code review rather than a crash or sth.? ;-)

No, we tested on a hardware that didn't have an external RTC - for the
first time :-)

rsc
Jan Luebbe - Oct. 25, 2012, 8:12 a.m.
On Thu, 2012-10-25 at 05:37 +0200, Roland Stigge wrote:
> > I wonder why.  I assume nobody has been running this driver on SMP kernels?
> 
> Yes: The platforms where this driver was enabled (at least in mainline)
> have only 1 core. Further, they typically employ external RTCs. So I
> guess the issue surfaced via code review rather than a crash or sth.? ;-)

I found this problem by enabling the lock debugging options, but it's
not the bug I was looking for. ;)

Jan

Patch

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index 96fb8cf..18a4f0d 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -393,6 +393,8 @@  static int dryice_rtc_probe(struct platform_device *pdev)
 	if (imxdi->ioaddr == NULL)
 		return -ENOMEM;
 
+	spin_lock_init(&imxdi->irq_lock);
+
 	imxdi->irq = platform_get_irq(pdev, 0);
 	if (imxdi->irq < 0)
 		return imxdi->irq;