Re: Patch: Generic TODR for ARM ports

Peter Postma wrote:
> On Sat, Sep 16, 2006 at 10:09:17PM -0700, Garrett D'Amore wrote:
>   
>> I've created a patch which converts the remaining ARM ports over to
>> generic todr.  (evbarm and hpcarm). 
>>
>> As part of this code, I removed the conditional check in common ARM code
>> for this conversion -- since this converts the remaining ports, it is
>> reasonable to unidef the code.
>>
>> I do not have any ARM equipment.  However the change was largely
>> "mechanical", and it should be safe.  I've test compiled all the hpcarm
>> kernels, and I'm compiling most of the evbarm kernels as I write this. 
>> I don't expect any problems.
>>
>> I'd appreciate it if folks could review and test these changes, and if
>> the relevant port-masters or responsible parties can indicate to me when
>> it is safe to commit the changes.
>>
>> The patch itself is located at
>> ftp://ftp.NetBSD.org/pub/NetBSD/misc/gdamore/arm-todr.diff
>>
>> Enjoy!
>>
>>     
>
> Thanks for doing this. The hpcarm changes look and work fine, but the
> following 3 messages are shown when booting, and when coming out suspend
> mode:
>
> WARNING: no TOD clock present
> WARNING: using filesystem time
> WARNING: CHECK AND RESET THE DATE!
>
> I can add an RTC driver, but I'm not sure how useful that would be, the
> RTC value is invalid at boot-time and the jornada doesn't do real suspend
> so we won't need it for that.
>
> Anyway, I've converted hpcarm to use timecounters and they seem to work fine:
>
> timecounter: Timecounters tick every 10.000 msec
> timecounter: Timecounter "clockinterrupt" frequency 100 Hz quality 0
> timecounter: selected timecounter "clockinterrupt" frequency 100 Hz quality 0
> timecounter: Timecounter "saost_count" frequency 3686400 Hz quality 100
> timecounter: selected timecounter "saost_count" frequency 3686400 Hz quality 100
> timecounter: selected timecounter "saost_count" frequency 3686400 Hz quality 100
>   

The double selection messages suggest that you need to update the kernel
from CVS.  A change was made to todr to prevent tc_windup() from getting
called with interrupts enabled -- I think that is what has happened
here.  (You should not see double messages like this.)
> ./timetest -v -v -c -t 120 -A
> Will test active counter and counters with positive quality from saost_count(q=1
> 00, f=3686400 Hz) clockinterrupt(q=0, f=100 Hz) dummy(q=-1000000, f=1000000 Hz)
> Testing time for monotonicity of timecounter "saost_count" for 120 seconds...
> 000121
> claimed resolution 271 nsec (3690036.900369 Hz) or better, observed observed minimum non zero delta 9222 nsec
> switching to timecounter "clockinterrupt"...
> Testing time for monotonicity of timecounter "clockinterrupt" for 120 seconds...
> 000121
> claimed resolution 10000000 nsec (100.000000 Hz) or better, observed minimum non zero delta 9999999 nsec
> TEST SUCCESSFUL
>
> I can put the diffs online for review, but they are really trivial.
>   

That would be great.  If they are generic, it would be a good idea to
wrap them in #ifdef __HAVE_TIMECOUNTER so that evbarm can pick them up too.


-- 
Garrett D'Amore, Principal Software Engineer
Tadpole Computer / Computing Technologies Division,
General Dynamics C4 Systems
http://www.tadpolecomputer.com/
Phone: 951 325-2134  Fax: 951 325-2191