>>>>> On Thu, 18 Aug 2005 12:54:50 +0400, Sergey Podstavin <spodstavin@xxxxxxxxxxxxx> said:
spodstavin> genrtc doesn't work as a module because functions for
spodstavin> module defined in wrong place. Most architectures define
spodstavin> these functions in <asm/rtc.h>, so make MIPS follow their
spodstavin> example. It makes the generic MIPS RTC working as a
spodstavin> module for MIPS.
It seems this fix already checked in, but I have some comments.
1. There are unnecessary (and conflicting) prototype declarations.
2. Define static variable mips_rtc_lock in rtc.h is generally a bad
idea. There is already rtc_lock in arch/mips/kernel/time.h.
3. We should protect rtc_set_mmss() call during get_rtc_time or
set_rtc_time (this is not your patch's fault).
How about this patch? The rtc_lock could be manipulated in each
RTC-dependent routines, but I choose a simple way for now.
---
Atsushi Nemoto
diff -u linux-mips/include/asm-mips/rtc.h linux/include/asm-mips/rtc.h
--- linux-mips/include/asm-mips/rtc.h 2005-09-05 10:17:18.000000000 +0900
+++ linux/include/asm-mips/rtc.h 2005-09-05 13:32:51.000000000 +0900
@@ -29,23 +29,18 @@
#define RTC_24H 0x02 /* 24 hour mode - else hours bit 7 means pm */
#define RTC_DST_EN 0x01 /* auto switch DST - works f. USA only */
-unsigned int get_rtc_time(struct rtc_time *time);
-int set_rtc_time(struct rtc_time *time);
-unsigned int get_rtc_ss(void);
-int get_rtc_pll(struct rtc_pll_info *pll);
-int set_rtc_pll(struct rtc_pll_info *pll);
-
-static DEFINE_SPINLOCK(mips_rtc_lock);
+extern spinlock_t rtc_lock; /* in kernel/time.c */
static inline unsigned int get_rtc_time(struct rtc_time *time)
{
unsigned long nowtime;
+ unsigned long flags;
- spin_lock(&mips_rtc_lock);
+ spin_lock_irqsave(&rtc_lock, flags);
nowtime = rtc_get_time();
to_tm(nowtime, time);
time->tm_year -= 1900;
- spin_unlock(&mips_rtc_lock);
+ spin_unlock_irqrestore(&rtc_lock, flags);
return RTC_24H;
}
@@ -53,14 +48,15 @@
static inline int set_rtc_time(struct rtc_time *time)
{
unsigned long nowtime;
+ unsigned long flags;
int ret;
- spin_lock(&mips_rtc_lock);
+ spin_lock_irqsave(&rtc_lock, flags);
nowtime = mktime(time->tm_year+1900, time->tm_mon+1,
time->tm_mday, time->tm_hour, time->tm_min,
time->tm_sec);
ret = rtc_set_time(nowtime);
- spin_unlock(&mips_rtc_lock);
+ spin_unlock_irqrestore(&rtc_lock, flags);
return ret;
}
diff -u linux-mips/arch/mips/kernel/time.c linux/arch/mips/kernel/time.c
--- linux-mips/arch/mips/kernel/time.c 2005-08-30 11:02:01.000000000 +0900
+++ linux/arch/mips/kernel/time.c 2005-09-05 13:36:05.000000000 +0900
@@ -453,12 +453,14 @@
xtime.tv_sec > last_rtc_update + 660 &&
(xtime.tv_nsec / 1000) >= 500000 - ((unsigned) TICK_SIZE) / 2 &&
(xtime.tv_nsec / 1000) <= 500000 + ((unsigned) TICK_SIZE) / 2) {
+ spin_lock(&rtc_lock);
if (rtc_set_mmss(xtime.tv_sec) == 0) {
last_rtc_update = xtime.tv_sec;
} else {
/* do it again in 60 s */
last_rtc_update = xtime.tv_sec - 600;
}
+ spin_unlock(&rtc_lock);
}
write_sequnlock(&xtime_lock);
