2016-04-18 16:21:55

by Alexandre Belloni

[permalink] [raw]
Subject: Re: rtc ds3232 call trace in kernel

On 18/04/2016 at 06:15:40 +0000, Qianyu Gong wrote :
> Hi Akinobu,
>
> I got an rtc call trace when booting 4.6 kernel on our board and I found it
> was caused by this patch:
>
> commit fc1dcb0b39dbb10d3290f2fcd6e154670f699166
> Author: Akinobu Mita <[email protected]>
> Date: Mon Mar 7 00:27:53 2016 +0900
>
> rtc: ds3232: use rtc->ops_lock to protect alarm operations
>
> ds3232->mutex is used to protect for alarm operations which
> need to access status and control registers.
>
> But we can use rtc->ops_lock instead. rtc->ops_lock is held when most
> of rtc_class_ops methods are called, so we only need to explicitly
> acquire it from irq handler in order to protect form concurrent
> accesses.
>
> Signed-off-by: Akinobu Mita <[email protected]>
> Signed-off-by: Alexandre Belloni <[email protected]
>
> The problem is that rtc->ops_lock would be accessed in ds3232_irq()
> without being initialized as rtc_device_register() is called too late.
>
> As I'm not familiar with rtc things, could I just revert the patch or you already
> have a solution to this problem? Thanks in advance.
>
>

Well, many RTC drivers actually suffer from that. I'm planning to change
the infrastructure to handle this case better.
Meanwhile, this is worth fixing.


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


2016-04-19 02:54:08

by Qianyu Gong

[permalink] [raw]
Subject: RE: rtc ds3232 call trace in kernel


> -----Original Message-----
> From: Alexandre Belloni [mailto:[email protected]]
> Sent: Tuesday, April 19, 2016 12:22 AM
> To: Qianyu Gong <[email protected]>
> Cc: [email protected]; Mingkai Hu <[email protected]>; rtc-
> [email protected]; [email protected]
> Subject: Re: rtc ds3232 call trace in kernel
>
> On 18/04/2016 at 06:15:40 +0000, Qianyu Gong wrote :
> > Hi Akinobu,
> >
> > I got an rtc call trace when booting 4.6 kernel on our board and I
> > found it was caused by this patch:
> >
> > commit fc1dcb0b39dbb10d3290f2fcd6e154670f699166
> > Author: Akinobu Mita <[email protected]>
> > Date: Mon Mar 7 00:27:53 2016 +0900
> >
> > rtc: ds3232: use rtc->ops_lock to protect alarm operations
> >
> > ds3232->mutex is used to protect for alarm operations which
> > need to access status and control registers.
> >
> > But we can use rtc->ops_lock instead. rtc->ops_lock is held when most
> > of rtc_class_ops methods are called, so we only need to explicitly
> > acquire it from irq handler in order to protect form concurrent
> > accesses.
> >
> > Signed-off-by: Akinobu Mita <[email protected]>
> > Signed-off-by: Alexandre Belloni <[email protected]
> >
> > The problem is that rtc->ops_lock would be accessed in ds3232_irq()
> > without being initialized as rtc_device_register() is called too late.
> >
> > As I'm not familiar with rtc things, could I just revert the patch or
> > you already have a solution to this problem? Thanks in advance.
> >
> >
>
> Well, many RTC drivers actually suffer from that. I'm planning to change the
> infrastructure to handle this case better.
> Meanwhile, this is worth fixing.
>
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering http://free-electrons.com

Thanks. I followed Akinobu's suggest and put devm_rtc_device_register() just
before registering the irq handler. The call trace disappeared. Looks like it's
working well.


Regards,
Qianyu