FW: RTC , ds1307 I2C driver and NTP does not work.

FW: RTC , ds1307 I2C driver and NTP does not work.

[Joakim Tjernlund]

switching to kernel/i2c list, see below for details.

 Jocke 

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org] 
Sent: 17 November 2006 17:57
To: Joakim Tjernlund
Cc: linuxppc-dev@ozlabs.org
Subject: Re: RTC , ds1307 I2C driver and NTP does not work.


On Nov 17, 2006, at 10:38 AM, Joakim Tjernlund wrote:

> I get this when I activathte NTP and ntp "sync" the time the I2C HW  
> clock.

You may be better off posting this to lkml and copy the i2c list (and  
rtc if one exists).  Since its more a driver issue than anything  
really ppc specific.  Clearly we are doing schedules() in mpc_xfer()  
and maybe we shouldn't be.

- kumar

> BUG: scheduling while atomic: swapper/0x00010000/0
> Call Trace:^M
> [C0245C80] [C000860C] show_stack+0x48/0x194 (unreliable)
> [C0245CB0] [C01BEFF4] schedule+0x5d4/0x618
> [C0245CF0] [C01BF9C8] schedule_timeout+0x70/0xd0
> [C0245D30] [C014416C] i2c_wait+0x164/0x1d8
> [C0245D80] [C0144490] mpc_xfer+0x2b0/0x3a8
> [C0245DD0] [C01423E8] i2c_transfer+0x58/0x7c
> [C0245DF0] [C0141124] ds1307_set_time+0x1bc/0x234
> [C0245E00] [C013F82C] rtc_set_time+0xb0/0xb8^M
> [C0245E20] [C000BFC4] set_rtc_class_time+0x34/0x58
> [C0245E40] [C000C8D0] timer_interrupt+0x5a0/0x5fc
> [C0245EE0] [C000F7B0] ret_from_except+0x0/0x14
> --- Exception: 901 at cpu_idle+0xc8/0xf0
>     LR = cpu_idle+0xec/0xf0
> [C0245FC0] [C000388C] rest_init+0x28/0x38
> [C0245FD0] [C01F36E0] start_kernel+0x1d8/0x228
> [C0245FF0] [00003438] 0x3438
>
> I have activated RTC CLASS and have this in my board file:
> #ifdef CONFIG_RTC_CLASS
> late_initcall(rtc_class_hookup);
> #endif
>
> kernel 2.6.19-rc5
>
>  Jocke
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: FW: RTC , ds1307 I2C driver and NTP does not work.

[Oleg Verych]

> -----Original Message-----
> From: 
> Sent: 17 November 2006 17:57
> To: Joakim Tjernlund
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: RTC , ds1307 I2C driver and NTP does not work.
>
>
> On Nov 17, 2006, at 10:38 AM, Joakim Tjernlund wrote:
>
>> I get this when I activathte NTP and ntp "sync" the time the I2C HW  
>> clock.
>
> You may be better off posting this to lkml and copy the i2c list (and  
> rtc if one exists).  Since its more a driver issue than anything  
> really ppc specific.  Clearly we are doing schedules() in mpc_xfer()  
> and maybe we shouldn't be.
>
> - kumar

Please try this patch:
<http://permalink.gmane.org/gmane.linux.kernel/467686>
Message-ID: <200611162327.37306.david-b@pacbell.net>
____

Re: RTC , ds1307 I2C driver and NTP does not work.

[Jean Delvare]

On Fri, 17 Nov 2006 18:38:10 +0100, Joakim Tjernlund wrote:
> On Nov 17, 2006, at 10:38 AM, Joakim Tjernlund wrote:
> 
> > I get this when I activathte NTP and ntp "sync" the time the I2C HW  
> > clock.
> 
> You may be better off posting this to lkml and copy the i2c list (and  
> rtc if one exists).  Since its more a driver issue than anything  
> really ppc specific.  Clearly we are doing schedules() in mpc_xfer()  
> and maybe we shouldn't be.

It's OK to schedule or sleep in mpc_xfer. It's not OK to call mpc_xfer
from an interrupt context, which is what appears to be happening here.
So the ds1307 driver would need to be changed not to directly call
i2c_transfer from the interrupt. Using a workqueue should work.

That being said, I wonder why one would want to set the time from an
interrupt context in the first place. Maybe that's what needs fixing.

> > BUG: scheduling while atomic: swapper/0x00010000/0
> > Call Trace:^M
> > [C0245C80] [C000860C] show_stack+0x48/0x194 (unreliable)
> > [C0245CB0] [C01BEFF4] schedule+0x5d4/0x618
> > [C0245CF0] [C01BF9C8] schedule_timeout+0x70/0xd0
> > [C0245D30] [C014416C] i2c_wait+0x164/0x1d8
> > [C0245D80] [C0144490] mpc_xfer+0x2b0/0x3a8
> > [C0245DD0] [C01423E8] i2c_transfer+0x58/0x7c
> > [C0245DF0] [C0141124] ds1307_set_time+0x1bc/0x234
> > [C0245E00] [C013F82C] rtc_set_time+0xb0/0xb8^M
> > [C0245E20] [C000BFC4] set_rtc_class_time+0x34/0x58
> > [C0245E40] [C000C8D0] timer_interrupt+0x5a0/0x5fc
> > [C0245EE0] [C000F7B0] ret_from_except+0x0/0x14
> > --- Exception: 901 at cpu_idle+0xc8/0xf0
> >     LR = cpu_idle+0xec/0xf0
> > [C0245FC0] [C000388C] rest_init+0x28/0x38
> > [C0245FD0] [C01F36E0] start_kernel+0x1d8/0x228
> > [C0245FF0] [00003438] 0x3438
> >
> > I have activated RTC CLASS and have this in my board file:
> > #ifdef CONFIG_RTC_CLASS
> > late_initcall(rtc_class_hookup);
> > #endif
> >
> > kernel 2.6.19-rc5
> >
> >  Jocke

-- 
Jean Delvare

RE: RTC , ds1307 I2C driver and NTP does not work.

[Joakim Tjernlund]

> 
> On Fri, 17 Nov 2006 18:38:10 +0100, Joakim Tjernlund wrote:
> > On Nov 17, 2006, at 10:38 AM, Joakim Tjernlund wrote:
> > 
> > > I get this when I activathte NTP and ntp "sync" the time 
> the I2C HW  
> > > clock.
> > 
> > You may be better off posting this to lkml and copy the i2c 
> list (and  
> > rtc if one exists).  Since its more a driver issue than anything  
> > really ppc specific.  Clearly we are doing schedules() in 
> mpc_xfer()  
> > and maybe we shouldn't be.
> 
> It's OK to schedule or sleep in mpc_xfer. It's not OK to call mpc_xfer
> from an interrupt context, which is what appears to be happening here.
> So the ds1307 driver would need to be changed not to directly call
> i2c_transfer from the interrupt. Using a workqueue should work.
> 
> That being said, I wonder why one would want to set the time from an
> interrupt context in the first place. Maybe that's what needs fixing.

That's the way kernel NTP code always has done it. Probably to minimize latency, ideally
you want to set the time when a new second occur since that's what most RTC HW expects.

Will a workqueue run directly after the one return from IRQ context?

> 
> > > BUG: scheduling while atomic: swapper/0x00010000/0
> > > Call Trace:^M
> > > [C0245C80] [C000860C] show_stack+0x48/0x194 (unreliable)
> > > [C0245CB0] [C01BEFF4] schedule+0x5d4/0x618
> > > [C0245CF0] [C01BF9C8] schedule_timeout+0x70/0xd0
> > > [C0245D30] [C014416C] i2c_wait+0x164/0x1d8
> > > [C0245D80] [C0144490] mpc_xfer+0x2b0/0x3a8
> > > [C0245DD0] [C01423E8] i2c_transfer+0x58/0x7c
> > > [C0245DF0] [C0141124] ds1307_set_time+0x1bc/0x234
> > > [C0245E00] [C013F82C] rtc_set_time+0xb0/0xb8^M
> > > [C0245E20] [C000BFC4] set_rtc_class_time+0x34/0x58
> > > [C0245E40] [C000C8D0] timer_interrupt+0x5a0/0x5fc
> > > [C0245EE0] [C000F7B0] ret_from_except+0x0/0x14
> > > --- Exception: 901 at cpu_idle+0xc8/0xf0
> > >     LR = cpu_idle+0xec/0xf0
> > > [C0245FC0] [C000388C] rest_init+0x28/0x38
> > > [C0245FD0] [C01F36E0] start_kernel+0x1d8/0x228
> > > [C0245FF0] [00003438] 0x3438
> > >
> > > I have activated RTC CLASS and have this in my board file:
> > > #ifdef CONFIG_RTC_CLASS
> > > late_initcall(rtc_class_hookup);
> > > #endif
> > >
> > > kernel 2.6.19-rc5
> > >
> > >  Jocke
> 
> -- 
> Jean Delvare
> 
> 

Re: FW: RTC, ds1307 I2C driver and NTP does not work.

[David Brownell]

> > Of course you can't do that.  You're calling rtc_set_time(), which
> > requires a task/sleeping context, from an atomic can't-sleep context
> > (timer irq handler in this case).
> > 
> > Whatever your rtc_class_hookup() is doing, it's clearly wrong.
>
> It isn't my rtc_class_hookup(), it is in arch/powerpc/kernel/time.c
> and I am only using it the only way I can:
>
>  #ifdef CONFIG_RTC_CLASS
>  late_initcall(rtc_class_hookup);
>  #endif

Yeah, and that routine is clearly a bug.  The mechanism has only been
there about six weeks; 7a69af63e788a324d162201a0b23df41bcf158dd should
be reverted ASAP, it's braindead by design:

  - the issue noted above, calling rtc_set_time() from irq context
    ... so it BUG()s with NTP clock synch

  - wrongly assumes CONFIG_RTC_CLASS implies CONFIG_RTC_HCTOSYS_DEVICE
    ... so it will cause build errors when the latter isn't present

  - makes system time be set TWICE from that RTC, given HCTOSYS_DEVICE
    ... once from the powerpc time_init(), once from rtc class setup

The second would be trivially fixed (it's just #ifdeffed wrong), but the
other points aren't.  It would however be good to see someone make sure
that the RTC class code works with NTP synch, and sort out some of those
arch specific clock setup issues.


> > You're on the right track that the RTC class code needs to hook up
> > to NTP, but you can't do it that way.
>
> Clearly, but where should it be fixed in RTC CLASS or in ds1307 driver?

Neither:  arch/powerpc/kernel/time.c should revert the patch referenced
above.  No need to ship 2.6.19 with such a regression, and 2.6.20 could
ship with working code.

- Dave


> > > BUG: scheduling while atomic: swapper/0x00010000/0
> > > Call Trace:
> > > [C0245C80] [C000860C] show_stack+0x48/0x194 (unreliable)
> > > [C0245CB0] [C01BEFF4] schedule+0x5d4/0x618
> > > [C0245CF0] [C01BF9C8] schedule_timeout+0x70/0xd0
> > > [C0245D30] [C014416C] i2c_wait+0x164/0x1d8
> > > [C0245D80] [C0144490] mpc_xfer+0x2b0/0x3a8
> > > [C0245DD0] [C01423E8] i2c_transfer+0x58/0x7c
> > > [C0245DF0] [C0141124] ds1307_set_time+0x1bc/0x234
> > > [C0245E00] [C013F82C] rtc_set_time+0xb0/0xb8^M
> > > [C0245E20] [C000BFC4] set_rtc_class_time+0x34/0x58
> > > [C0245E40] [C000C8D0] timer_interrupt+0x5a0/0x5fc
> > > [C0245EE0] [C000F7B0] ret_from_except+0x0/0x14
> > > --- Exception: 901 at cpu_idle+0xc8/0xf0
> > >     LR = cpu_idle+0xec/0xf0
> > > [C0245FC0] [C000388C] rest_init+0x28/0x38
> > > [C0245FD0] [C01F36E0] start_kernel+0x1d8/0x228
> > > [C0245FF0] [00003438] 0x3438

Re: RTC , ds1307 I2C driver and NTP does not work.

[Jean Delvare]

Hi Joakim,

On Sat, 18 Nov 2006 15:51:09 +0100, Joakim Tjernlund wrote:
> > It's OK to schedule or sleep in mpc_xfer. It's not OK to call mpc_xfer
> > from an interrupt context, which is what appears to be happening here.
> > So the ds1307 driver would need to be changed not to directly call
> > i2c_transfer from the interrupt. Using a workqueue should work.
> > 
> > That being said, I wonder why one would want to set the time from an
> > interrupt context in the first place. Maybe that's what needs fixing.
> 
> That's the way kernel NTP code always has done it. Probably to minimize
> latency, ideally you want to set the time when a new second occur since
> that's what most RTC HW expects.
> 
> Will a workqueue run directly after the one return from IRQ context?

There is no guarantee as to when the workqueue will process the request
as far as I know. The actual delay probably depends on the value of HZ.

There is no way around the delay in the case of I2C RTC chips anyway
(at least not with the current implementation), as the underlying I2C
bus driver may sleep as part of the bus transaction, and the RTC chip
driver can virtually be attached to every bus.

Note that the transaction itself will take some time anyway, I2C can be
quite slow.

-- 
Jean Delvare