Re: RTC: add RTC class interface to m41t00 driver

Re: RTC: add RTC class interface to m41t00 driver

[David Brownell]

Actually, it'd be worth trying drivers/rtc/rtc-ds1307.c ... the M41T00 is
one of a family of mostly-compatible RTC chips, and the ds1307 driver
should be pretty much the least-common-denominator there.  They all use
the same I2C address, and the same register layout for the calendar/time
function.

I'd expect rtc-ds1307 to handle the m41t00 already, or with at most minor
tweaks to recognize whatever's different.  It should already be ignoring
the bits in the clock/calendar registers that vary, as well as the SRAM
and (for some other chips) the alarm.  (Not that I2C has ways to tell us
what IRQ the alarm would use, but that's a different tale!)

- Dave

Re: RTC: add RTC class interface to m41t00 driver

[Atsushi Nemoto]

On Fri, 4 Aug 2006 19:33:39 -0700, David Brownell <david-b@pacbell.net> wrote:
> Actually, it'd be worth trying drivers/rtc/rtc-ds1307.c ... the M41T00 is
> one of a family of mostly-compatible RTC chips, and the ds1307 driver
> should be pretty much the least-common-denominator there.  They all use
> the same I2C address, and the same register layout for the calendar/time
> function.
> 
> I'd expect rtc-ds1307 to handle the m41t00 already, or with at most minor
> tweaks to recognize whatever's different.  It should already be ignoring
> the bits in the clock/calendar registers that vary, as well as the SRAM
> and (for some other chips) the alarm.  (Not that I2C has ways to tell us
> what IRQ the alarm would use, but that's a different tale!)

Thanks for your suggestion.  I have looked rtc-ds1307 too before I
tried to modify m41t00 driver.

It seems some works are still needed to support M41Txx chips by the
driver.

1. The driver contains ds_1340 (or st m41t00) definition, but it seems
   no way to select the ds_type.

2. As m41t00_chip_info_tbl[] in m41t00 driver shows, M41T81 and M41T85
   have different register layout.

3. It lacks some features (ST bit, HT bit, SQW freq.) in m41t00
   driver, though I personally does not need these features.

I choose changing m41t00 driver by (1) and (2).

If we really need a super generic driver, I suppose adding ds13xx
support to new m41txx driver is less hard.  I think having separate
drivers are good enough for now.

---
Atsushi Nemoto

Re: RTC: add RTC class interface to m41t00 driver

[Alexander Bigga]

Hi Atsushi,
Hi David,

I've seen very late that the rtc-ds1307.c driver supports the quite 
simple m41t00 as well. As Mark's m41t00.c claimed to support even the 
m41t81 and the m41st85, I startet at this point.

First, I sent my approach to Mark (m41t00.c), Alessandro (rtc-subsytem) 
and Jean (i2c-subsystem) to discuss the strategy. And if I understood 
them right, they found the idea good, to move the i2c/chips/m41t00.h to 
an rtc/rtc-m41txx.c driver, as this should be the general place for such 
rtc-drivers.

As Atsushi has done almost the same work, I postet my version on friday 
to pretend the next person to do this job and to start the discussion, 
how to get to a suitable version for all - including Mark with his 
arch/ppc/platforms/katana.c boards.

I confirm, that the rtc-ds1307.c driver works with m41t00. But the 
m41t8x or m41st8x differs a lot from the m41t00 (HT bit, ST bit, SQW 
freq - like Atsushi wrote it already).


Atsushi Nemoto wrote:
> 2. As m41t00_chip_info_tbl[] in m41t00 driver shows, M41T81 and M41T85
>    have different register layout.
>   

The register layout seems to depend on the watchdog and alarm 
functionality.
The features differs from chip to chip, that's why I intodruced a 
"features"-field in struct m41txx_chip_info.

> 3. It lacks some features (ST bit, HT bit, SQW freq.) in m41t00
>    driver, though I personally does not need these features.
>   

You need at least to clear the Stop Bit (ST) and the Halt Update Bit 
(HT) unless your m41t8x will always report the time of the last power 
fail and not the current time.

For me there is still the open question, if the workqueue-part and the 
exported symbols (m41t00_get_rtc_time, ) should stay or not. I don't 
need it and Atsushi seems to share my opinion. But...?

On monday, I can continue work on it.


Alexander

-- 
Alexander Bigga		Tel: +49 4873 90 10 866
mycable GmbH		Fax: +49 4873 90 19 76
Boeker Stieg 43
D-24613 Aukrug		eMail: ab@mycable.de

Re: RTC: add RTC class interface to m41t00 driver

[David Brownell]

On Saturday 05 August 2006 10:43 am, Alexander Bigga wrote:
> Hi Atsushi,
> Hi David,
> 
> I've seen very late that the rtc-ds1307.c driver supports the quite 
> simple m41t00 as well. As Mark's m41t00.c claimed to support even the 
> m41t81 and the m41st85, I startet at this point.

I touched on that in my reply to Atsushi ... basically, the approach
used in rtc-ds1307 is "least common denominator", and sticking to the
fully compatible register subset and generic I2C framework.

That helped avoid needing to work around the lack of driver model support
in the I2C stack, at the (minor to me!) price of not handling features
that would need platform_data hooks to support ... like <linux/clk.h>
support for SQW, and RTC alarm irqs.


> First, I sent my approach to Mark (m41t00.c), Alessandro (rtc-subsytem) 
> and Jean (i2c-subsystem) to discuss the strategy. And if I understood 
> them right, they found the idea good, to move the i2c/chips/m41t00.h to 
> an rtc/rtc-m41txx.c driver, as this should be the general place for such 
> rtc-drivers.

Agreed that RTC drivers should now use the (newish) RTC framework, and
these should move out of the "misc i2c chips" area.  Not all RTCs fit
in the drivers/rtc area though ... an RTC can be a secondary function
of a multipurpose chip.  (I suspect the Kconfig for their RTC interfaces
should probably live in drivers/rtc though...)


> As Atsushi has done almost the same work, I postet my version on friday 
> to pretend the next person to do this job and to start the discussion,

Discussion is now started.  :)


> how to get to a suitable version for all - including Mark with his 
> arch/ppc/platforms/katana.c boards.

I suspect not all that board support is upstream yet; I can't see
anything creating the m41t00 platform devices as required by the
current m41t00.c driver ... neither on katana, nor any other board.

So it looks to me like RTC class support is not the only issue!

Plus, Mr. Grep tells me there's a separate m41t81 driver in
mips/sibyte/swarm/rtc_m41t81.c ... 


> I confirm, that the rtc-ds1307.c driver works with m41t00.

Good to know that ... I tried to be careful, but I didn't have a
board with one of those to test with, just docs.  Thanks!  At
some point we may decide it's safe to remove i2c/chips/ds1337.c,
but I'd want similar confirmation for the ds1337 chip.


> Atsushi Nemoto wrote:
> > 2. As m41t00_chip_info_tbl[] in m41t00 driver shows, M41T81 and M41T85
> >    have different register layout.
> 
> The register layout seems to depend on the watchdog and alarm 
> functionality.
> The features differs from chip to chip, that's why I intodruced a 
> "features"-field in struct m41txx_chip_info.

I'm not sure that'd be sufficient; if you look at the various RTCs,
you'll notice that the control bits are in wildly different places.

You may end up doing more "switch (chip_type) {...}" than testing of
the feature bits, if you get beyond those three chips.


> > 3. It lacks some features (ST bit, HT bit, SQW freq.) in m41t00
> >    driver, though I personally does not need these features.
> >   
> 
> You need at least to clear the Stop Bit (ST) and the Halt Update Bit 
> (HT) unless your m41t8x will always report the time of the last power 
> fail and not the current time.

Yes, but the m41t8x chips aren't register-compatible with those other
RTCs, so I would not expect them to work the same.  :)


> For me there is still the open question, if the workqueue-part and the 
> exported symbols (m41t00_get_rtc_time, ) should stay or not. I don't 
> need it and Atsushi seems to share my opinion. But...?

None of the other RTC drivers export private APIs or require a workqueue,
so you won't need them on an m41t8x driver either.

I noticed that the katana board uses a different scheme for the "initialize
the system time/date" problem addressed by CONFIG_RTC_HCTOSYS, and that seems
to be the reason for the m41t00.c driver to export an API.  (Much the same way
that the PC-style "cmos clock" exports an API used early in x86 booting, which
likewise bypasses the RTC framework ...)

I suspect there are arch-specific issues to work through there, both for
initializing the clock at boot and for re-initializing it after resume.
(CONFIG_RTC_HCTOSYS doesn't currently address the latter...)

- Dave

Re: RTC: add RTC class interface to m41t00 driver

[Alexander Bigga]

On Saturday 05 August 2006 22:23, you wrote:
> Discussion is now started.  :)

Hope to hear more arguments ;-)

> I suspect not all that board support is upstream yet; I can't see
> anything creating the m41t00 platform devices as required by the
> current m41t00.c driver ... neither on katana, nor any other board.

Your're right. First, I was confused too, but then Mark pointed me to a patch, 
which is never gone into mainline:

http://lists.lm-sensors.org/pipermail/lm-sensors/2005-December/014727.html

Be aware of the different names: m41t00 and m41txx!

> Plus, Mr. Grep tells me there's a separate m41t81 driver in
> mips/sibyte/swarm/rtc_m41t81.c ...

Oh, yes. I found it too, but didn't checked it further.

> You may end up doing more "switch (chip_type) {...}" than testing of
> the feature bits, if you get beyond those three chips.

In deed. If I want to support all.

> I noticed that the katana board uses a different scheme for the "initialize
> the system time/date" problem addressed by CONFIG_RTC_HCTOSYS, and that
> seems to be the reason for the m41t00.c driver to export an API.  (Much the
> same way that the PC-style "cmos clock" exports an API used early in x86
> booting, which likewise bypasses the RTC framework ...)
>
> I suspect there are arch-specific issues to work through there, both for
> initializing the clock at boot and for re-initializing it after resume.
> (CONFIG_RTC_HCTOSYS doesn't currently address the latter...)

This question, only Mark can unswer, or?


Alexander
-- 
Alexander Bigga     Tel: +49 4873 90 10 866
mycable GmbH        Fax: +49 4873 901 976
Boeker Stieg 43
D-24613 Aukrug      eMail: ab@mycable.de

Re: RTC: add RTC class interface to m41t00 driver

[David Brownell]

On Saturday 05 August 2006 9:29 am, Atsushi Nemoto wrote:
> On Fri, 4 Aug 2006 19:33:39 -0700, David Brownell <david-b@pacbell.net> wrote:
> > Actually, it'd be worth trying drivers/rtc/rtc-ds1307.c ... the M41T00 is
> > one of a family of mostly-compatible RTC chips, and the ds1307 driver
> > should be pretty much the least-common-denominator there.  They all use
> > the same I2C address, and the same register layout for the calendar/time
> > function.
> > ...
> 
> Thanks for your suggestion.  I have looked rtc-ds1307 too before I
> tried to modify m41t00 driver.

I couldn't tell that from any email I've seen, but it's good to know.


> 1. The driver contains ds_1340 (or st m41t00) definition, but it seems
>    no way to select the ds_type.

Which is harmless, since the driver has no code that really needs to care.
As I said, it's a least-common-denominator driver ... used in my case
with a SOC that integrates a highly functional (primary) RTC, because
only the external RTC provides its own battery-backed power domain.


This is a limitation of the I2C stack ... one that I observe is being
worked around by m41t00.c platform_device tricks.  A side effect of those
tricks is to prevent hooking up more than one of this type I2C chip
to a system ... not pretty (especially given that it's not documented),
but often OK.


> 2. As m41t00_chip_info_tbl[] in m41t00 driver shows, M41T81 and M41T85
>    have different register layout.

Right, suggesting the m41t00.c driver is more misnamed than rtc-ds1307.c
because it doesn't even insist on software-compatible chips!  And I'll
note that its Kconfig doesn't mention that support either ...

It would have made more sense to me to have the M41T81 and M41T85 be
in a different driver, because of the incompatible register layout.
That's the more traditional approach.


> 3. It lacks some features (ST bit, HT bit, SQW freq.) in m41t00
>    driver, though I personally does not need these features.

Right:  "least-common-denominator".  Most of those chips can do square
wave output (SQW), which should arguably be exposed as a <linux/clk.h>
signal like other such signals.

Plus, handling square wave output would in general need board-specific
configuration (also not directly supported by I2C), since the SQW pin
often doubles as an alarm interrupt output ... I'd rather have an alarm!
The ds1337 has two alarm pins, ds1339 muxes two alarms to one pin.

The ds1307 chip has NVRAM too, also not exposed ... even though that's
the natural place to store manufacturing info like ethernet ids.

And there are many other chip differences that software might care about.

 
> I choose changing m41t00 driver by (1) and (2).
> 
> If we really need a super generic driver, I suppose adding ds13xx
> support to new m41txx driver is less hard.  I think having separate
> drivers are good enough for now.

My approach was to go for "least common denominator", not "super generic".
Also, I avoided trying to work around I2C stack issues like the omission
of per-board tables/customization.  When I2C eventually has a nice clean
solution for those issues, I expect that will mean adopting the better
solution involves fewer changes.

But I was mostly pointing out that if the goal was to support the m41t00
RTC (the normal reading of $SUBJECT), that work has been done for some
time and has already been pushed upstream.

- Dave

Re: RTC: add RTC class interface to m41t00 driver

[Atsushi Nemoto]

On Sat, 5 Aug 2006 12:13:49 -0700, David Brownell <david-b@pacbell.net> wrote:
> > 1. The driver contains ds_1340 (or st m41t00) definition, but it seems
> >    no way to select the ds_type.
> 
> Which is harmless, since the driver has no code that really needs to care.
> As I said, it's a least-common-denominator driver ... used in my case
> with a SOC that integrates a highly functional (primary) RTC, because
> only the external RTC provides its own battery-backed power domain.

Oh, I missed that point.  It should work for m41t00 indeed.

> This is a limitation of the I2C stack ... one that I observe is being
> worked around by m41t00.c platform_device tricks.  A side effect of those
> tricks is to prevent hooking up more than one of this type I2C chip
> to a system ... not pretty (especially given that it's not documented),
> but often OK.

Yes, some more works would be needed on m41t00 to support multiple
(different) M41Txx chips at same time.

> It would have made more sense to me to have the M41T81 and M41T85 be
> in a different driver, because of the incompatible register layout.
> That's the more traditional approach.

Now I'm thinking this way.  Still thinking...

> My approach was to go for "least common denominator", not "super generic".
> Also, I avoided trying to work around I2C stack issues like the omission
> of per-board tables/customization.  When I2C eventually has a nice clean
> solution for those issues, I expect that will mean adopting the better
> solution involves fewer changes.

OK, I see.  I think LCD approach is worth indeed.

> But I was mostly pointing out that if the goal was to support the m41t00
> RTC (the normal reading of $SUBJECT), that work has been done for some
> time and has already been pushed upstream.

My real target is M41T80, which seems a subset of M41T81.
(Sorry for not writing this before.)

I agree m41t00 can be supported by rtc-ds1307 driver already.

So the next theme would be how we support M41T8x chips.  I think
Alexander's rtc-m41txx is good candidate for them.

Then M41T00 users can choose rtc-ds1307 or rtc-m41t00.  Not so bad, I
think.

---
Atsushi Nemoto

Re: RTC: add RTC class interface to m41t00 driver

[Alexander Bigga]

On Sunday 06 August 2006 19:09, Atsushi Nemoto wrote:
> > It would have made more sense to me to have the M41T81 and M41T85 be
> > in a different driver, because of the incompatible register layout.
> > That's the more traditional approach.
>
> Now I'm thinking this way.  Still thinking...

Yes, but on the other side: except of this single shift in the registers, it 
works in the m41txx (or what the name will be) and won't do any extra work to 
the driver.
Even the m41t80 and m41t81 differs in their features. So you always have to 
distinguish inside the driver.

> My real target is M41T80, which seems a subset of M41T81.
> (Sorry for not writing this before.)

My target is a M41ST85.

> So the next theme would be how we support M41T8x chips.  I think
> Alexander's rtc-m41txx is good candidate for them.

There are also M41st9x chips and M41t6x chips and a M41t11... But ok, most 
difference is watchdog/alarm or not.

> Then M41T00 users can choose rtc-ds1307 or rtc-m41t00.  Not so bad, I
> think.

As long, as the user will be pointed out this possibilties in the Kconfig - I 
agree.


Alexander
-- 
Alexander Bigga     Tel: +49 4873 90 10 866
mycable GmbH        Fax: +49 4873 901 976
Boeker Stieg 43
D-24613 Aukrug      eMail: ab@mycable.de

RTC: add RTC class interface to m41t00 driver

[Atsushi Nemoto]

Hi.  I added new RTC class interface to existing m41t00 driver.  I
used CONFIG_GEN_RTC for traditional interface and CONFIG_RTC_CLASS for
new interface.  It should work with any platform not only for PPC32.

Is this a right direction?  Or should I create an another rtc-m41t00
driver in drivers/rtc/ directory?


Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
index 87ee3ce..2cc48d2 100644
--- a/drivers/i2c/chips/Kconfig
+++ b/drivers/i2c/chips/Kconfig
@@ -103,7 +103,7 @@ config TPS65010
 
 config SENSORS_M41T00
 	tristate "ST M41T00 RTC chip"
-	depends on I2C && PPC32
+	depends on I2C
 	help
 	  If you say yes here you get support for the ST M41T00 RTC chip.
 
diff --git a/drivers/i2c/chips/m41t00.c b/drivers/i2c/chips/m41t00.c
index 2dd0a34..b58f2f0 100644
--- a/drivers/i2c/chips/m41t00.c
+++ b/drivers/i2c/chips/m41t00.c
@@ -10,7 +10,7 @@
  */
 /*
  * This i2c client/driver wedges between the drivers/char/genrtc.c RTC
- * interface and the SMBus interface of the i2c subsystem.
+ * (and/or RTC class) interface and the SMBus interface of the i2c subsystem.
  */
 
 #include <linux/kernel.h>
@@ -26,7 +26,9 @@ #include <asm/time.h>
 #include <asm/rtc.h>
 
 static struct i2c_driver m41t00_driver;
+#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
 static struct i2c_client *save_client;
+#endif
 
 static unsigned short ignore[] = { I2C_CLIENT_END };
 static unsigned short normal_addr[] = { I2C_CLIENT_END, I2C_CLIENT_END };
@@ -96,8 +98,8 @@ static struct m41t00_chip_info m41t00_ch
 };
 static struct m41t00_chip_info *m41t00_chip;
 
-ulong
-m41t00_get_rtc_time(void)
+static int
+m41t00_get_time(struct i2c_client *client, struct rtc_time *tm)
 {
 	s32 sec, min, hour, day, mon, year;
 	s32 sec1, min1, hour1, day1, mon1, year1;
@@ -105,13 +107,13 @@ m41t00_get_rtc_time(void)
 	u8 buf[8], msgbuf[1] = { 0 }; /* offset into rtc's regs */
 	struct i2c_msg msgs[] = {
 		{
-			.addr	= save_client->addr,
+			.addr	= client->addr,
 			.flags	= 0,
 			.len	= 1,
 			.buf	= msgbuf,
 		},
 		{
-			.addr	= save_client->addr,
+			.addr	= client->addr,
 			.flags	= I2C_M_RD,
 			.len	= 8,
 			.buf	= buf,
@@ -121,7 +123,7 @@ m41t00_get_rtc_time(void)
 	sec = min = hour = day = mon = year = 0;
 
 	do {
-		if (i2c_transfer(save_client->adapter, msgs, 2) < 0)
+		if (i2c_transfer(client->adapter, msgs, 2) < 0)
 			goto read_err;
 
 		sec1 = sec;
@@ -146,61 +148,68 @@ m41t00_get_rtc_time(void)
 			|| (year != year1)))
 		goto read_err;
 
-	sec = BCD2BIN(sec);
-	min = BCD2BIN(min);
-	hour = BCD2BIN(hour);
-	day = BCD2BIN(day);
-	mon = BCD2BIN(mon);
-	year = BCD2BIN(year);
+	tm->tm_sec = BCD2BIN(sec);
+	tm->tm_min = BCD2BIN(min);
+	tm->tm_hour = BCD2BIN(hour);
+	tm->tm_mday = BCD2BIN(day);
+	tm->tm_mon = BCD2BIN(mon) - 1;
+	tm->tm_year = BCD2BIN(year);
 
-	year += 1900;
-	if (year < 1970)
-		year += 100;
+	if (tm->tm_year + 1900 < 1970)
+		tm->tm_year += 100;
 
-	return mktime(year, mon, day, hour, min, sec);
+	return 0;
 
 read_err:
-	dev_err(&save_client->dev, "m41t00_get_rtc_time: Read error\n");
-	return 0;
+	dev_err(&client->dev, "m41t00_get_rtc_time: Read error\n");
+	return -EIO;
 }
-EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);
 
-static void
-m41t00_set(void *arg)
+#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
+ulong
+m41t00_get_rtc_time(void)
 {
 	struct rtc_time	tm;
-	int nowtime = *(int *)arg;
+
+	if (m41t00_get_time(&save_client, &tm))
+		return 0;
+	return mktime(tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
+		      tm.tm_hour, tm.tm_min, tm.tm_sec);
+}
+EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);
+#endif
+
+static int
+m41t00_set_time(struct i2c_client *client, struct rtc_time *tm)
+{
 	s32 sec, min, hour, day, mon, year;
 	u8 wbuf[9], *buf = &wbuf[1], msgbuf[1] = { 0 };
 	struct i2c_msg msgs[] = {
 		{
-			.addr	= save_client->addr,
+			.addr	= client->addr,
 			.flags	= 0,
 			.len	= 1,
 			.buf	= msgbuf,
 		},
 		{
-			.addr	= save_client->addr,
+			.addr	= client->addr,
 			.flags	= I2C_M_RD,
 			.len	= 8,
 			.buf	= buf,
 		},
 	};
 
-	to_tm(nowtime, &tm);
-	tm.tm_year = (tm.tm_year - 1900) % 100;
-
-	sec = BIN2BCD(tm.tm_sec);
-	min = BIN2BCD(tm.tm_min);
-	hour = BIN2BCD(tm.tm_hour);
-	day = BIN2BCD(tm.tm_mday);
-	mon = BIN2BCD(tm.tm_mon);
-	year = BIN2BCD(tm.tm_year);
+	sec = BIN2BCD(tm->tm_sec);
+	min = BIN2BCD(tm->tm_min);
+	hour = BIN2BCD(tm->tm_hour);
+	day = BIN2BCD(tm->tm_mday);
+	mon = BIN2BCD(tm->tm_mon + 1);
+	year = BIN2BCD(tm->tm_year % 100);
 
 	/* Read reg values into buf[0..7]/wbuf[1..8] */
-	if (i2c_transfer(save_client->adapter, msgs, 2) < 0) {
-		dev_err(&save_client->dev, "m41t00_set: Read error\n");
-		return;
+	if (i2c_transfer(client->adapter, msgs, 2) < 0) {
+		dev_err(&client->dev, "m41t00_set: Read error\n");
+		return -EIO;
 	}
 
 	wbuf[0] = 0; /* offset into rtc's regs */
@@ -210,8 +219,24 @@ m41t00_set(void *arg)
 	buf[m41t00_chip->day] = (buf[m41t00_chip->day] & ~0x3f) | (day & 0x3f);
 	buf[m41t00_chip->mon] = (buf[m41t00_chip->mon] & ~0x1f) | (mon & 0x1f);
 
-	if (i2c_master_send(save_client, wbuf, 9) < 0)
-		dev_err(&save_client->dev, "m41t00_set: Write error\n");
+	if (i2c_master_send(client, wbuf, 9) < 0) {
+		dev_err(&client->dev, "m41t00_set: Write error\n");
+		return -EIO;
+	}
+	return 0;
+}
+
+#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
+static void
+m41t00_set(void *arg)
+{
+	struct rtc_time	tm;
+	int nowtime = *(int *)arg;
+
+	to_tm(nowtime, &tm);
+	tm.tm_year -= 1900;
+	tm.tm_mon -= 1;
+	m41t00_set_time(&save_client, &tm);
 }
 
 static ulong new_time;
@@ -231,6 +256,7 @@ m41t00_set_rtc_time(ulong nowtime)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(m41t00_set_rtc_time);
+#endif
 
 /*
  *****************************************************************************
@@ -273,6 +299,23 @@ static struct platform_driver m41t00_pla
 	},
 };
 
+#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
+static int m41t00_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	return m41t00_get_time(to_i2c_client(dev), tm);
+}
+
+static int m41t00_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	return m41t00_set_time(to_i2c_client(dev), tm);
+}
+
+static struct rtc_class_ops m41t00_rtc_ops = {
+	.read_time	= m41t00_rtc_read_time,
+	.set_time	= m41t00_rtc_set_time,
+};
+#endif
+
 /*
  *****************************************************************************
  *
@@ -284,6 +327,9 @@ static int
 m41t00_probe(struct i2c_adapter *adap, int addr, int kind)
 {
 	struct i2c_client *client;
+#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
+	struct rtc_device *rtc;
+#endif
 	int rc;
 
 	if (!i2c_check_functionality(adap, I2C_FUNC_I2C
@@ -336,8 +382,21 @@ m41t00_probe(struct i2c_adapter *adap, i
 				rc & ~0x80)) < 0)
 			goto st_err;
 
+#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
+	rtc = rtc_device_register(m41t00_driver.driver.name, &client->dev,
+				  &m41t00_rtc_ops, THIS_MODULE);
+
+	if (IS_ERR(rtc)) {
+		rc = PTR_ERR(rtc);
+		goto attach_err;
+	}
+	i2c_set_clientdata(client, rtc);
+#endif
+
+#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
 	m41t00_wq = create_singlethread_workqueue(m41t00_chip->name);
 	save_client = client;
+#endif
 	return 0;
 
 st_err:
@@ -363,10 +422,17 @@ static int
 m41t00_detach(struct i2c_client *client)
 {
 	int rc;
+#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
+	struct rtc_device *rtc = i2c_get_clientdata(client);
 
+	if (rtc)
+		rtc_device_unregister(rtc);
+#endif
 	if ((rc = i2c_detach_client(client)) == 0) {
 		kfree(client);
+#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
 		destroy_workqueue(m41t00_wq);
+#endif
 	}
 	return rc;
 }

Re: RTC: add RTC class interface to m41t00 driver

[Atsushi Nemoto]

On Fri, 04 Aug 2006 00:21:46 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> Hi.  I added new RTC class interface to existing m41t00 driver.  I
> used CONFIG_GEN_RTC for traditional interface and CONFIG_RTC_CLASS for
> new interface.  It should work with any platform not only for PPC32.

Oops, It can not compiled on archs which do not have asm/time.h.
Revised.


Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
index 87ee3ce..2cc48d2 100644
--- a/drivers/i2c/chips/Kconfig
+++ b/drivers/i2c/chips/Kconfig
@@ -103,7 +103,7 @@ config TPS65010
 
 config SENSORS_M41T00
 	tristate "ST M41T00 RTC chip"
-	depends on I2C && PPC32
+	depends on I2C
 	help
 	  If you say yes here you get support for the ST M41T00 RTC chip.
 
diff --git a/drivers/i2c/chips/m41t00.c b/drivers/i2c/chips/m41t00.c
index 2dd0a34..9c58f59 100644
--- a/drivers/i2c/chips/m41t00.c
+++ b/drivers/i2c/chips/m41t00.c
@@ -10,7 +10,7 @@
  */
 /*
  * This i2c client/driver wedges between the drivers/char/genrtc.c RTC
- * interface and the SMBus interface of the i2c subsystem.
+ * (and/or RTC class) interface and the SMBus interface of the i2c subsystem.
  */
 
 #include <linux/kernel.h>
@@ -22,11 +22,12 @@ #include <linux/bcd.h>
 #include <linux/workqueue.h>
 #include <linux/platform_device.h>
 #include <linux/m41t00.h>
-#include <asm/time.h>
 #include <asm/rtc.h>
 
 static struct i2c_driver m41t00_driver;
+#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
 static struct i2c_client *save_client;
+#endif
 
 static unsigned short ignore[] = { I2C_CLIENT_END };
 static unsigned short normal_addr[] = { I2C_CLIENT_END, I2C_CLIENT_END };
@@ -96,8 +97,8 @@ static struct m41t00_chip_info m41t00_ch
 };
 static struct m41t00_chip_info *m41t00_chip;
 
-ulong
-m41t00_get_rtc_time(void)
+static int
+m41t00_get_time(struct i2c_client *client, struct rtc_time *tm)
 {
 	s32 sec, min, hour, day, mon, year;
 	s32 sec1, min1, hour1, day1, mon1, year1;
@@ -105,13 +106,13 @@ m41t00_get_rtc_time(void)
 	u8 buf[8], msgbuf[1] = { 0 }; /* offset into rtc's regs */
 	struct i2c_msg msgs[] = {
 		{
-			.addr	= save_client->addr,
+			.addr	= client->addr,
 			.flags	= 0,
 			.len	= 1,
 			.buf	= msgbuf,
 		},
 		{
-			.addr	= save_client->addr,
+			.addr	= client->addr,
 			.flags	= I2C_M_RD,
 			.len	= 8,
 			.buf	= buf,
@@ -121,7 +122,7 @@ m41t00_get_rtc_time(void)
 	sec = min = hour = day = mon = year = 0;
 
 	do {
-		if (i2c_transfer(save_client->adapter, msgs, 2) < 0)
+		if (i2c_transfer(client->adapter, msgs, 2) < 0)
 			goto read_err;
 
 		sec1 = sec;
@@ -146,61 +147,68 @@ m41t00_get_rtc_time(void)
 			|| (year != year1)))
 		goto read_err;
 
-	sec = BCD2BIN(sec);
-	min = BCD2BIN(min);
-	hour = BCD2BIN(hour);
-	day = BCD2BIN(day);
-	mon = BCD2BIN(mon);
-	year = BCD2BIN(year);
+	tm->tm_sec = BCD2BIN(sec);
+	tm->tm_min = BCD2BIN(min);
+	tm->tm_hour = BCD2BIN(hour);
+	tm->tm_mday = BCD2BIN(day);
+	tm->tm_mon = BCD2BIN(mon) - 1;
+	tm->tm_year = BCD2BIN(year);
 
-	year += 1900;
-	if (year < 1970)
-		year += 100;
+	if (tm->tm_year + 1900 < 1970)
+		tm->tm_year += 100;
 
-	return mktime(year, mon, day, hour, min, sec);
+	return 0;
 
 read_err:
-	dev_err(&save_client->dev, "m41t00_get_rtc_time: Read error\n");
-	return 0;
+	dev_err(&client->dev, "m41t00_get_rtc_time: Read error\n");
+	return -EIO;
 }
-EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);
 
-static void
-m41t00_set(void *arg)
+#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
+ulong
+m41t00_get_rtc_time(void)
 {
 	struct rtc_time	tm;
-	int nowtime = *(int *)arg;
+
+	if (m41t00_get_time(&save_client, &tm))
+		return 0;
+	return mktime(tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
+		      tm.tm_hour, tm.tm_min, tm.tm_sec);
+}
+EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);
+#endif
+
+static int
+m41t00_set_time(struct i2c_client *client, struct rtc_time *tm)
+{
 	s32 sec, min, hour, day, mon, year;
 	u8 wbuf[9], *buf = &wbuf[1], msgbuf[1] = { 0 };
 	struct i2c_msg msgs[] = {
 		{
-			.addr	= save_client->addr,
+			.addr	= client->addr,
 			.flags	= 0,
 			.len	= 1,
 			.buf	= msgbuf,
 		},
 		{
-			.addr	= save_client->addr,
+			.addr	= client->addr,
 			.flags	= I2C_M_RD,
 			.len	= 8,
 			.buf	= buf,
 		},
 	};
 
-	to_tm(nowtime, &tm);
-	tm.tm_year = (tm.tm_year - 1900) % 100;
-
-	sec = BIN2BCD(tm.tm_sec);
-	min = BIN2BCD(tm.tm_min);
-	hour = BIN2BCD(tm.tm_hour);
-	day = BIN2BCD(tm.tm_mday);
-	mon = BIN2BCD(tm.tm_mon);
-	year = BIN2BCD(tm.tm_year);
+	sec = BIN2BCD(tm->tm_sec);
+	min = BIN2BCD(tm->tm_min);
+	hour = BIN2BCD(tm->tm_hour);
+	day = BIN2BCD(tm->tm_mday);
+	mon = BIN2BCD(tm->tm_mon + 1);
+	year = BIN2BCD(tm->tm_year % 100);
 
 	/* Read reg values into buf[0..7]/wbuf[1..8] */
-	if (i2c_transfer(save_client->adapter, msgs, 2) < 0) {
-		dev_err(&save_client->dev, "m41t00_set: Read error\n");
-		return;
+	if (i2c_transfer(client->adapter, msgs, 2) < 0) {
+		dev_err(&client->dev, "m41t00_set: Read error\n");
+		return -EIO;
 	}
 
 	wbuf[0] = 0; /* offset into rtc's regs */
@@ -210,8 +218,24 @@ m41t00_set(void *arg)
 	buf[m41t00_chip->day] = (buf[m41t00_chip->day] & ~0x3f) | (day & 0x3f);
 	buf[m41t00_chip->mon] = (buf[m41t00_chip->mon] & ~0x1f) | (mon & 0x1f);
 
-	if (i2c_master_send(save_client, wbuf, 9) < 0)
-		dev_err(&save_client->dev, "m41t00_set: Write error\n");
+	if (i2c_master_send(client, wbuf, 9) < 0) {
+		dev_err(&client->dev, "m41t00_set: Write error\n");
+		return -EIO;
+	}
+	return 0;
+}
+
+#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
+static void
+m41t00_set(void *arg)
+{
+	struct rtc_time	tm;
+	int nowtime = *(int *)arg;
+
+	to_tm(nowtime, &tm);
+	tm.tm_year -= 1900;
+	tm.tm_mon -= 1;
+	m41t00_set_time(&save_client, &tm);
 }
 
 static ulong new_time;
@@ -231,6 +255,7 @@ m41t00_set_rtc_time(ulong nowtime)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(m41t00_set_rtc_time);
+#endif
 
 /*
  *****************************************************************************
@@ -273,6 +298,23 @@ static struct platform_driver m41t00_pla
 	},
 };
 
+#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
+static int m41t00_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	return m41t00_get_time(to_i2c_client(dev), tm);
+}
+
+static int m41t00_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	return m41t00_set_time(to_i2c_client(dev), tm);
+}
+
+static struct rtc_class_ops m41t00_rtc_ops = {
+	.read_time	= m41t00_rtc_read_time,
+	.set_time	= m41t00_rtc_set_time,
+};
+#endif
+
 /*
  *****************************************************************************
  *
@@ -284,6 +326,9 @@ static int
 m41t00_probe(struct i2c_adapter *adap, int addr, int kind)
 {
 	struct i2c_client *client;
+#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
+	struct rtc_device *rtc;
+#endif
 	int rc;
 
 	if (!i2c_check_functionality(adap, I2C_FUNC_I2C
@@ -336,8 +381,21 @@ m41t00_probe(struct i2c_adapter *adap, i
 				rc & ~0x80)) < 0)
 			goto st_err;
 
+#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
+	rtc = rtc_device_register(m41t00_driver.driver.name, &client->dev,
+				  &m41t00_rtc_ops, THIS_MODULE);
+
+	if (IS_ERR(rtc)) {
+		rc = PTR_ERR(rtc);
+		goto attach_err;
+	}
+	i2c_set_clientdata(client, rtc);
+#endif
+
+#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
 	m41t00_wq = create_singlethread_workqueue(m41t00_chip->name);
 	save_client = client;
+#endif
 	return 0;
 
 st_err:
@@ -363,10 +421,17 @@ static int
 m41t00_detach(struct i2c_client *client)
 {
 	int rc;
+#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
+	struct rtc_device *rtc = i2c_get_clientdata(client);
 
+	if (rtc)
+		rtc_device_unregister(rtc);
+#endif
 	if ((rc = i2c_detach_client(client)) == 0) {
 		kfree(client);
+#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
 		destroy_workqueue(m41t00_wq);
+#endif
 	}
 	return rc;
 }

Re: RTC: add RTC class interface to m41t00 driver

[Mark A. Greer]

Atsushi,

Alexander Bigga <ab@mycable.de> has been working on moving this driver
over to [its proper place in] drivers/rtc.  Please sync up with him.

I've included him on the cc list so he can see your email.

Mark
--

On Fri, Aug 04, 2006 at 12:42:59AM +0900, Atsushi Nemoto wrote:
> On Fri, 04 Aug 2006 00:21:46 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> > Hi.  I added new RTC class interface to existing m41t00 driver.  I
> > used CONFIG_GEN_RTC for traditional interface and CONFIG_RTC_CLASS for
> > new interface.  It should work with any platform not only for PPC32.
> 
> Oops, It can not compiled on archs which do not have asm/time.h.
> Revised.
> 
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
> index 87ee3ce..2cc48d2 100644
> --- a/drivers/i2c/chips/Kconfig
> +++ b/drivers/i2c/chips/Kconfig
> @@ -103,7 +103,7 @@ config TPS65010
>  
>  config SENSORS_M41T00
>  	tristate "ST M41T00 RTC chip"
> -	depends on I2C && PPC32
> +	depends on I2C
>  	help
>  	  If you say yes here you get support for the ST M41T00 RTC chip.
>  
> diff --git a/drivers/i2c/chips/m41t00.c b/drivers/i2c/chips/m41t00.c
> index 2dd0a34..9c58f59 100644
> --- a/drivers/i2c/chips/m41t00.c
> +++ b/drivers/i2c/chips/m41t00.c
> @@ -10,7 +10,7 @@
>   */
>  /*
>   * This i2c client/driver wedges between the drivers/char/genrtc.c RTC
> - * interface and the SMBus interface of the i2c subsystem.
> + * (and/or RTC class) interface and the SMBus interface of the i2c subsystem.
>   */
>  
>  #include <linux/kernel.h>
> @@ -22,11 +22,12 @@ #include <linux/bcd.h>
>  #include <linux/workqueue.h>
>  #include <linux/platform_device.h>
>  #include <linux/m41t00.h>
> -#include <asm/time.h>
>  #include <asm/rtc.h>
>  
>  static struct i2c_driver m41t00_driver;
> +#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
>  static struct i2c_client *save_client;
> +#endif
>  
>  static unsigned short ignore[] = { I2C_CLIENT_END };
>  static unsigned short normal_addr[] = { I2C_CLIENT_END, I2C_CLIENT_END };
> @@ -96,8 +97,8 @@ static struct m41t00_chip_info m41t00_ch
>  };
>  static struct m41t00_chip_info *m41t00_chip;
>  
> -ulong
> -m41t00_get_rtc_time(void)
> +static int
> +m41t00_get_time(struct i2c_client *client, struct rtc_time *tm)
>  {
>  	s32 sec, min, hour, day, mon, year;
>  	s32 sec1, min1, hour1, day1, mon1, year1;
> @@ -105,13 +106,13 @@ m41t00_get_rtc_time(void)
>  	u8 buf[8], msgbuf[1] = { 0 }; /* offset into rtc's regs */
>  	struct i2c_msg msgs[] = {
>  		{
> -			.addr	= save_client->addr,
> +			.addr	= client->addr,
>  			.flags	= 0,
>  			.len	= 1,
>  			.buf	= msgbuf,
>  		},
>  		{
> -			.addr	= save_client->addr,
> +			.addr	= client->addr,
>  			.flags	= I2C_M_RD,
>  			.len	= 8,
>  			.buf	= buf,
> @@ -121,7 +122,7 @@ m41t00_get_rtc_time(void)
>  	sec = min = hour = day = mon = year = 0;
>  
>  	do {
> -		if (i2c_transfer(save_client->adapter, msgs, 2) < 0)
> +		if (i2c_transfer(client->adapter, msgs, 2) < 0)
>  			goto read_err;
>  
>  		sec1 = sec;
> @@ -146,61 +147,68 @@ m41t00_get_rtc_time(void)
>  			|| (year != year1)))
>  		goto read_err;
>  
> -	sec = BCD2BIN(sec);
> -	min = BCD2BIN(min);
> -	hour = BCD2BIN(hour);
> -	day = BCD2BIN(day);
> -	mon = BCD2BIN(mon);
> -	year = BCD2BIN(year);
> +	tm->tm_sec = BCD2BIN(sec);
> +	tm->tm_min = BCD2BIN(min);
> +	tm->tm_hour = BCD2BIN(hour);
> +	tm->tm_mday = BCD2BIN(day);
> +	tm->tm_mon = BCD2BIN(mon) - 1;
> +	tm->tm_year = BCD2BIN(year);
>  
> -	year += 1900;
> -	if (year < 1970)
> -		year += 100;
> +	if (tm->tm_year + 1900 < 1970)
> +		tm->tm_year += 100;
>  
> -	return mktime(year, mon, day, hour, min, sec);
> +	return 0;
>  
>  read_err:
> -	dev_err(&save_client->dev, "m41t00_get_rtc_time: Read error\n");
> -	return 0;
> +	dev_err(&client->dev, "m41t00_get_rtc_time: Read error\n");
> +	return -EIO;
>  }
> -EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);
>  
> -static void
> -m41t00_set(void *arg)
> +#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
> +ulong
> +m41t00_get_rtc_time(void)
>  {
>  	struct rtc_time	tm;
> -	int nowtime = *(int *)arg;
> +
> +	if (m41t00_get_time(&save_client, &tm))
> +		return 0;
> +	return mktime(tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
> +		      tm.tm_hour, tm.tm_min, tm.tm_sec);
> +}
> +EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);
> +#endif
> +
> +static int
> +m41t00_set_time(struct i2c_client *client, struct rtc_time *tm)
> +{
>  	s32 sec, min, hour, day, mon, year;
>  	u8 wbuf[9], *buf = &wbuf[1], msgbuf[1] = { 0 };
>  	struct i2c_msg msgs[] = {
>  		{
> -			.addr	= save_client->addr,
> +			.addr	= client->addr,
>  			.flags	= 0,
>  			.len	= 1,
>  			.buf	= msgbuf,
>  		},
>  		{
> -			.addr	= save_client->addr,
> +			.addr	= client->addr,
>  			.flags	= I2C_M_RD,
>  			.len	= 8,
>  			.buf	= buf,
>  		},
>  	};
>  
> -	to_tm(nowtime, &tm);
> -	tm.tm_year = (tm.tm_year - 1900) % 100;
> -
> -	sec = BIN2BCD(tm.tm_sec);
> -	min = BIN2BCD(tm.tm_min);
> -	hour = BIN2BCD(tm.tm_hour);
> -	day = BIN2BCD(tm.tm_mday);
> -	mon = BIN2BCD(tm.tm_mon);
> -	year = BIN2BCD(tm.tm_year);
> +	sec = BIN2BCD(tm->tm_sec);
> +	min = BIN2BCD(tm->tm_min);
> +	hour = BIN2BCD(tm->tm_hour);
> +	day = BIN2BCD(tm->tm_mday);
> +	mon = BIN2BCD(tm->tm_mon + 1);
> +	year = BIN2BCD(tm->tm_year % 100);
>  
>  	/* Read reg values into buf[0..7]/wbuf[1..8] */
> -	if (i2c_transfer(save_client->adapter, msgs, 2) < 0) {
> -		dev_err(&save_client->dev, "m41t00_set: Read error\n");
> -		return;
> +	if (i2c_transfer(client->adapter, msgs, 2) < 0) {
> +		dev_err(&client->dev, "m41t00_set: Read error\n");
> +		return -EIO;
>  	}
>  
>  	wbuf[0] = 0; /* offset into rtc's regs */
> @@ -210,8 +218,24 @@ m41t00_set(void *arg)
>  	buf[m41t00_chip->day] = (buf[m41t00_chip->day] & ~0x3f) | (day & 0x3f);
>  	buf[m41t00_chip->mon] = (buf[m41t00_chip->mon] & ~0x1f) | (mon & 0x1f);
>  
> -	if (i2c_master_send(save_client, wbuf, 9) < 0)
> -		dev_err(&save_client->dev, "m41t00_set: Write error\n");
> +	if (i2c_master_send(client, wbuf, 9) < 0) {
> +		dev_err(&client->dev, "m41t00_set: Write error\n");
> +		return -EIO;
> +	}
> +	return 0;
> +}
> +
> +#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
> +static void
> +m41t00_set(void *arg)
> +{
> +	struct rtc_time	tm;
> +	int nowtime = *(int *)arg;
> +
> +	to_tm(nowtime, &tm);
> +	tm.tm_year -= 1900;
> +	tm.tm_mon -= 1;
> +	m41t00_set_time(&save_client, &tm);
>  }
>  
>  static ulong new_time;
> @@ -231,6 +255,7 @@ m41t00_set_rtc_time(ulong nowtime)
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(m41t00_set_rtc_time);
> +#endif
>  
>  /*
>   *****************************************************************************
> @@ -273,6 +298,23 @@ static struct platform_driver m41t00_pla
>  	},
>  };
>  
> +#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
> +static int m41t00_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	return m41t00_get_time(to_i2c_client(dev), tm);
> +}
> +
> +static int m41t00_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> +	return m41t00_set_time(to_i2c_client(dev), tm);
> +}
> +
> +static struct rtc_class_ops m41t00_rtc_ops = {
> +	.read_time	= m41t00_rtc_read_time,
> +	.set_time	= m41t00_rtc_set_time,
> +};
> +#endif
> +
>  /*
>   *****************************************************************************
>   *
> @@ -284,6 +326,9 @@ static int
>  m41t00_probe(struct i2c_adapter *adap, int addr, int kind)
>  {
>  	struct i2c_client *client;
> +#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
> +	struct rtc_device *rtc;
> +#endif
>  	int rc;
>  
>  	if (!i2c_check_functionality(adap, I2C_FUNC_I2C
> @@ -336,8 +381,21 @@ m41t00_probe(struct i2c_adapter *adap, i
>  				rc & ~0x80)) < 0)
>  			goto st_err;
>  
> +#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
> +	rtc = rtc_device_register(m41t00_driver.driver.name, &client->dev,
> +				  &m41t00_rtc_ops, THIS_MODULE);
> +
> +	if (IS_ERR(rtc)) {
> +		rc = PTR_ERR(rtc);
> +		goto attach_err;
> +	}
> +	i2c_set_clientdata(client, rtc);
> +#endif
> +
> +#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
>  	m41t00_wq = create_singlethread_workqueue(m41t00_chip->name);
>  	save_client = client;
> +#endif
>  	return 0;
>  
>  st_err:
> @@ -363,10 +421,17 @@ static int
>  m41t00_detach(struct i2c_client *client)
>  {
>  	int rc;
> +#if defined(CONFIG_RTC_CLASS) || defined(CONFIG_RTC_CLASS_MODULE)
> +	struct rtc_device *rtc = i2c_get_clientdata(client);
>  
> +	if (rtc)
> +		rtc_device_unregister(rtc);
> +#endif
>  	if ((rc = i2c_detach_client(client)) == 0) {
>  		kfree(client);
> +#if defined(CONFIG_GEN_RTC) && defined(CONFIG_GEN_RTC_MODULE)
>  		destroy_workqueue(m41t00_wq);
> +#endif
>  	}
>  	return rc;
>  }

Re: RTC: add RTC class interface to m41t00 driver

[Alexander Bigga]

Hi Atsushi,


like you, I started recently with Mark's m41t00.c driver to add support for 
the new rtc-subsystem. Mark reviewed it and I added his changes.

There is still the question, if the code for the interrupt context 
(workqueues) should stay or not. You bracketed all this with CONFIG_GEN_RTC. 
I can't say, if this is a good idea. Maybe somebody else has some good 
comments.

The driver supports now four different STM rtc chips: M41T00, M41T80, M41T81 
and M41ST85. The chips M41ST84/85/87/94 should work similar as far the driver 
supports them. Maybe even more.

That's why I named the driver rtc-m41txx.c and not m41t00.c as the original 
one.

There is for sure still some work to do, so I appreciate any comments. 
The enclosed patch was tested with linux-2.6.18-rc3 on an embedded Alchemy 
MIPS-processor board (XXS1500).


Alexander


On Friday 04 August 2006 02:21, Mark A. Greer wrote:
> Atsushi,
>
> Alexander Bigga <ab@mycable.de> has been working on moving this driver
> over to [its proper place in] drivers/rtc.  Please sync up with him.
>
> I've included him on the cc list so he can see your email.
>
> Mark

--- linux-2.6.18-rc3/drivers/rtc/Makefile	2006-07-30 20:57:47.000000000 +0200
+++ linux-2.6.18-rc3-mips-dev/drivers/rtc/Makefile	2006-08-02 
11:16:44.000000000 +0200
@@ -23,6 +23,7 @@
 obj-$(CONFIG_RTC_DRV_S3C)	+= rtc-s3c.o
 obj-$(CONFIG_RTC_DRV_RS5C348)	+= rtc-rs5c348.o
 obj-$(CONFIG_RTC_DRV_M48T86)	+= rtc-m48t86.o
+obj-$(CONFIG_RTC_DRV_M41TXX)	+= rtc-m41txx.o
 obj-$(CONFIG_RTC_DRV_DS1553)	+= rtc-ds1553.o
 obj-$(CONFIG_RTC_DRV_EP93XX)	+= rtc-ep93xx.o
 obj-$(CONFIG_RTC_DRV_SA1100)	+= rtc-sa1100.o
--- linux-2.6.18-rc3/drivers/rtc/Kconfig	2006-07-30 20:57:47.000000000 +0200
+++ linux-2.6.18-rc3-mips-dev/drivers/rtc/Kconfig	2006-08-04 
14:26:12.000000000 +0200
@@ -218,6 +218,17 @@
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-m48t86.
 
+config RTC_DRV_M41TXX
+	tristate "ST M41Txx series RTC"
+	depends on RTC_CLASS
+	help
+	  If you say Y here you will get support for the
+	  ST M41T00 RTC chips series. Currently the following chips are
+	  supported: M41T00, M41T80, M41T81 and M41ST85
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-m41txx.
+
 config RTC_DRV_EP93XX
 	tristate "Cirrus Logic EP93XX"
 	depends on RTC_CLASS && ARCH_EP93XX
--- linux-2.6.18-rc3/drivers/rtc/rtc-m41txx.c	1970-01-01 01:00:00.000000000 
+0100
+++ linux-2.6.18-rc3-mips-dev/drivers/rtc/rtc-m41txx.c	2006-08-04 
14:23:08.000000000 +0200
@@ -0,0 +1,568 @@
+/*
+ * I2C client/driver for the ST M41T00 family of i2c rtc chips.
+ *
+ * Author: Mark A. Greer <mgreer@mvista.com>
+ *         Alexander Bigga <ab@mycable.de>
+ *
+ * Based on m41t00.c by Mark A. Greer
+ *
+ * 2005, 2006 (c) MontaVista Software, Inc.
+ * 2006 (c) mycable GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/rtc.h>
+#include <linux/bcd.h>
+#include <linux/workqueue.h>
+#include <linux/platform_device.h>
+#include <linux/m41t00.h>
+#include <asm/time.h>
+#include <asm/rtc.h>
+
+#define M41TXX_SEC_ST		(1 << 7)	/* ST: Stop Bit */
+#define M41TXX_HOUR_CB		(1 << 6)	/* CB: Century Bit */
+#define M41TXX_HOUR_CEB		(1 << 7)	/* CEB: Century Enable Bit */
+#define M41TXX_ALHOUR_HT	(1 << 6)	/* HT: Halt Update Bit */
+#define M41TXX_FLAGS_BATT_LOW	(1 << 4)	/* BL: Battery Low Bit */
+
+#define M41TXX_FEATURE_HT	(1 << 0)
+#define M41TXX_FEATURE_BL	(1 << 1)
+
+#define DRV_VERSION "0.03"
+
+
+static struct i2c_driver m41txx_driver;
+static struct i2c_client *save_client;
+
+static unsigned short normal_i2c[] = { M41T00_I2C_ADDR, I2C_CLIENT_END };
+
+/* Insmod parameters */
+I2C_CLIENT_INSMOD;
+
+/* Prototypes */
+static int m41txx_attach(struct i2c_adapter *adapter);
+static int m41txx_detach(struct i2c_client *client);
+static int m41txx_probe(struct i2c_adapter *adapter, int address,
+			int kind);
+
+/* exports the i2c_client structure; used by a m41t85 watchdog-driver. */
+struct i2c_client *m41txx_get_i2c_client(void)
+{
+	return save_client;
+}
+EXPORT_SYMBOL(m41txx_get_i2c_client);
+
+static struct i2c_driver m41txx_driver = {
+	.driver = {
+		   .name = M41T00_DRV_NAME,
+		   },
+	.id = I2C_DRIVERID_STM41T00,
+	.attach_adapter = &m41txx_attach,
+	.detach_client = &m41txx_detach,
+};
+
+struct m41txx_chip_info {
+	u8 type;
+	char *name;
+	u8 read_limit;
+	u8 sec;			/* Offsets for chip regs */
+	u8 min;
+	u8 hour;
+	u8 wday;
+	u8 day;
+	u8 mon;
+	u8 year;
+	u8 alarm_mon;
+	u8 alarm_hour;
+	u8 flags;
+	u8 sqw;
+	u8 sqw_freq;
+	u8 features;
+};
+
+static struct m41txx_chip_info m41txx_chip_info_tbl[] = {
+	{ /* M41T00/11 */
+	 .type		= M41T00_TYPE_M41T00,
+	 .name		= "m41t00",
+	 .read_limit	= 5,
+	 .sec		= 0,
+	 .min		= 1,
+	 .hour		= 2,
+	 .wday		= 3,
+	 .day		= 4,
+	 .mon		= 5,
+	 .year		= 6,
+	 .features	= 0,
+	 },
+	 { /* M41T80 */
+	 .type		= M41T00_TYPE_M41T80,
+	 .name		= "m41t80",
+	 .read_limit 	= 1,
+	 .sec		= 1,
+	 .min		= 2,
+	 .hour		= 3,
+	 .wday		= 4,
+	 .day		= 5,
+	 .mon		= 6,
+	 .year		= 7,
+	 .alarm_mon	= 0xa,
+	 .alarm_hour	= 0xc,
+	 .flags		= 0xf,
+	 .sqw		= 0x13,
+	 .features	= 0,
+	 },
+	 { /* M41T81 */
+	 .type		= M41T00_TYPE_M41T81,
+	 .name		= "m41t81",
+	 .read_limit 	= 1,
+	 .sec		= 1,
+	 .min		= 2,
+	 .hour		= 3,
+	 .wday		= 4,
+	 .day		= 5,
+	 .mon		= 6,
+	 .year		= 7,
+	 .alarm_mon	= 0xa,
+	 .alarm_hour	= 0xc,
+	 .flags		= 0xf,
+	 .sqw		= 0x13,
+	 .features	= M41TXX_FEATURE_HT,
+	 },
+	 { /* M41ST84/85/87/94 */
+	 .type		= M41T00_TYPE_M41T85,
+	 .name		= "m41t85",
+	 .read_limit	= 1,
+	 .sec		= 1,
+	 .min		= 2,
+	 .hour		= 3,
+	 .wday		= 4,
+	 .day		= 5,
+	 .mon		= 6,
+	 .year		= 7,
+	 .alarm_mon	= 0xa,
+	 .alarm_hour	= 0xc,
+	 .flags		= 0xf,
+	 .sqw		= 0x13,
+	 .features	= M41TXX_FEATURE_HT | M41TXX_FEATURE_BL,
+	 },
+};
+static struct m41txx_chip_info *m41txx_chip;
+
+static int m41txx_get_datetime(struct i2c_client *client,
+			       struct rtc_time *tm)
+{
+
+	s32 sec, min, hour, day, mon, year;
+	s32 sec1, min1, hour1, day1, mon1, year1;
+	u8 reads = 0;
+	u8 buf[8], dt_addr[1] = { 0 };	/* offset into rtc's regs */
+
+	struct i2c_msg msgs[] = {
+		{
+		 .addr 	= client->addr,
+		 .flags = 0,
+		 .len 	= 1,
+		 .buf 	= dt_addr,
+		 },
+		{
+		 .addr 	= client->addr,
+		 .flags = I2C_M_RD,
+		 .len 	= 8,
+		 .buf 	= buf,
+		 },
+	};
+
+	sec = min = hour = day = mon = year = 0;
+
+	do {
+		if (i2c_transfer(client->adapter, msgs, 2) < 0)
+			goto read_err;
+
+		sec1 = sec;
+		min1 = min;
+		hour1 = hour;
+		day1 = day;
+		mon1 = mon;
+		year1 = year;
+
+		sec = buf[m41txx_chip->sec] & 0x7f;
+		min = buf[m41txx_chip->min] & 0x7f;
+		hour = buf[m41txx_chip->hour] & 0x3f;
+		day = buf[m41txx_chip->day] & 0x3f;
+		mon = buf[m41txx_chip->mon] & 0x1f;
+		year = buf[m41txx_chip->year];
+
+	} while ((++reads < m41txx_chip->read_limit) && ((sec != sec1)
+			|| (min != min1) || (hour != hour1) || (day != day1)
+			|| (mon != mon1) || (year != year1)));
+
+	if ((m41txx_chip->read_limit > 1)
+	    && ((sec != sec1) || (min != min1)
+		|| (hour != hour1) || (day != day1) || (mon != mon1)
+		|| (year != year1)))
+		goto read_err;
+
+	tm->tm_sec = BCD2BIN(sec);
+	tm->tm_min = BCD2BIN(min);
+	tm->tm_hour = BCD2BIN(hour);
+	tm->tm_mday = BCD2BIN(day);
+	tm->tm_wday = buf[m41txx_chip->wday] & 0x07;
+	tm->tm_mon = BCD2BIN(mon) - 1;
+
+	if (buf[m41txx_chip->hour] & M41TXX_HOUR_CB)
+		tm->tm_year = BCD2BIN(year) + 100;
+	else
+		tm->tm_year = BCD2BIN(year);
+
+	return 0;
+
+read_err:
+	dev_err(&client->dev, "m41txx_get_datetime: Read error\n");
+	return 0;
+}
+
+
+/* Sets the given date and time to the real time clock. */
+int m41txx_set_datetime(struct i2c_client *client, struct rtc_time *tm)
+{
+	unsigned char buf[8];
+	unsigned char wbuf[8];
+	unsigned char dt_addr[1] = { 0 };	/* offset into rtc's regs */
+
+	struct i2c_msg msgs_in[] = {
+		{
+		.addr	= client->addr,
+		.flags	= 0,
+		.len	= 1,
+		.buf	= dt_addr,
+		},
+		{
+		.addr	= client->addr,
+		.flags	= I2C_M_RD,
+		.len	= 8,
+		.buf	= wbuf,
+		},
+	};
+	struct i2c_msg msgs[] = {
+		{
+		 .addr = client->addr,
+		 .flags = 0,
+		 .len = 8,
+		 .buf = buf,
+		 },
+	};
+
+	/* Read current reg values into wbuf[0..8] */
+	if (i2c_transfer(client->adapter, msgs_in, 2) < 0)
+		goto read_err;
+
+	/* Merge time-data and register flags into buf[0..8] */
+	buf[0] = 0x01;
+	buf[m41txx_chip->sec] = BIN2BCD(tm->tm_sec)
+					| (wbuf[m41txx_chip->sec] & ~0x7f);
+	buf[m41txx_chip->min] = BIN2BCD(tm->tm_min)
+					| (wbuf[m41txx_chip->min] & ~0x7f);
+	if (tm->tm_year > 100) {
+		/* M41T85_REG_HOUR_CB enables the CENTURY Bit (CB) */
+		buf[m41txx_chip->hour] = BIN2BCD(tm->tm_hour)
+			| M41TXX_HOUR_CB | M41TXX_HOUR_CEB ;
+		buf[m41txx_chip->year] = BIN2BCD(tm->tm_year - 100);
+	} else {
+		buf[m41txx_chip->hour] = BIN2BCD(tm->tm_hour);
+		buf[m41txx_chip->year] = BIN2BCD(tm->tm_year);
+	}
+	buf[m41txx_chip->wday] = (tm->tm_wday & 0x07)
+					| (wbuf[m41txx_chip->wday] & ~0x07);
+	buf[m41txx_chip->day] = BIN2BCD(tm->tm_mday)
+					| (wbuf[m41txx_chip->day] & ~0x3f);
+	buf[m41txx_chip->mon] = BIN2BCD(tm->tm_mon + 1)
+					| (wbuf[m41txx_chip->mon] & ~0x1f);
+
+	if (i2c_transfer(client->adapter, msgs, 1) != 1) {
+		dev_err(&client->dev, "%s: write error\n", __FUNCTION__);
+		return -EIO;
+	}
+	return 0;
+read_err:
+	dev_err(&client->dev, "m41txx_set_datetime: Read error\n");
+	return 0;
+}
+
+ulong m41t00_get_rtc_time(void)
+{
+	struct rtc_time tm;
+
+	m41txx_get_datetime(save_client, &tm);
+
+	return mktime(tm.tm_year, tm.tm_mon,
+		      tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec);
+}
+EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);
+
+static void m41txx_set(void *arg)
+{
+	struct rtc_time tm;
+	int nowtime = *(int *) arg;
+
+	to_tm(nowtime, &tm);
+	m41txx_set_datetime(save_client, &tm);
+
+}
+
+static ulong new_time;
+static struct workqueue_struct *m41txx_wq;
+static DECLARE_WORK(m41txx_work, m41txx_set, &new_time);
+
+int m41t00_set_rtc_time(ulong nowtime)
+{
+	new_time = nowtime;
+
+	if (in_interrupt())
+		queue_work(m41txx_wq, &m41txx_work);
+	else
+		m41txx_set(&new_time);
+
+	return 0;
+}
+
+EXPORT_SYMBOL_GPL(m41t00_set_rtc_time);
+
+/*
+ 
*****************************************************************************
+ *
+ *	platform_data Driver Interface
+ *
+ 
*****************************************************************************
+ */
+static int __init m41txx_platform_probe(struct platform_device *pdev)
+{
+	struct m41t00_platform_data *pdata;
+	int i;
+
+	if (pdev && (pdata = pdev->dev.platform_data)) {
+		normal_i2c[0] = pdata->i2c_addr;
+
+		for (i = 0; i < ARRAY_SIZE(m41txx_chip_info_tbl); i++)
+			if (m41txx_chip_info_tbl[i].type == pdata->type) {
+				m41txx_chip = &m41txx_chip_info_tbl[i];
+				m41txx_chip->sqw_freq = pdata->sqw_freq;
+				return i2c_add_driver(&m41txx_driver);
+			}
+	}
+	return -ENODEV;
+}
+
+static int __exit m41txx_platform_remove(struct platform_device *pdev)
+{
+	return i2c_del_driver(&m41txx_driver);
+}
+
+static struct platform_driver m41txx_platform_driver = {
+	.probe = m41txx_platform_probe,
+	.remove = m41txx_platform_remove,
+	.driver = {
+		   .owner = THIS_MODULE,
+		   .name = M41T00_DRV_NAME,
+		   },
+};
+
+static int m41txx_rtc_proc(struct device *dev, struct seq_file *seq)
+{
+	unsigned char reg;
+
+	if (m41txx_chip->flags & M41TXX_FEATURE_BL ) {
+		reg = i2c_smbus_read_byte_data(to_i2c_client(dev),
+							m41txx_chip->flags);
+		seq_printf(seq, "battery\t\t: %s\n",
+			(reg & M41TXX_FLAGS_BATT_LOW) ? "exhausted" : "ok");
+	}
+	return 0;
+}
+
+static int m41txx_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	return m41txx_get_datetime(to_i2c_client(dev), tm);
+}
+
+static int m41txx_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	return m41txx_set_datetime(to_i2c_client(dev), tm);
+}
+
+
+static struct rtc_class_ops m41txx_rtc_ops = {
+	.read_time = m41txx_rtc_read_time,
+	.set_time = m41txx_rtc_set_time,
+	.proc = m41txx_rtc_proc,
+};
+
+/*
+ 
*****************************************************************************
+ *
+ *	Driver Interface
+ *
+ 
*****************************************************************************
+ */
+static int m41txx_probe(struct i2c_adapter *adapter, int address, int kind)
+{
+	int rc = 0;
+
+	struct i2c_client *client;
+	struct rtc_device *rtc;
+	struct rtc_time tm;
+
+	dev_dbg(&adapter->dev, "%s\n", __FUNCTION__);
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_I2C
+				     | I2C_FUNC_SMBUS_BYTE_DATA)) {
+		rc = -ENODEV;
+		goto exit;
+	}
+
+	if (!(client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL))) {
+		rc = -ENOMEM;
+		goto exit;
+	}
+
+	/* I2C client */
+	strlcpy(client->name, m41txx_chip->name, I2C_NAME_SIZE);
+	client->addr = address;
+	client->adapter = adapter;
+	client->driver = &m41txx_driver;
+
+	/* Inform the i2c layer */
+	if ((rc = i2c_attach_client(client)))
+		goto exit_kfree;
+
+	dev_info(&client->dev,
+		 "chip found, driver version " DRV_VERSION "\n");
+
+	rtc = rtc_device_register(m41txx_chip->name, &client->dev,
+				  &m41txx_rtc_ops, THIS_MODULE);
+
+	if (IS_ERR(rtc)) {
+		rc = PTR_ERR(rtc);
+		goto exit_detach;
+	}
+
+	i2c_set_clientdata(client, rtc);
+
+	if (m41txx_chip->type != M41T00_TYPE_M41T00) {
+		/* If asked, disable SQW, set SQW frequency & re-enable */
+		if (m41txx_chip->sqw_freq)
+			if (((rc = i2c_smbus_read_byte_data(client,
+					m41txx_chip->alarm_mon)) < 0)
+			 || ((rc = i2c_smbus_write_byte_data(client,
+					m41txx_chip->alarm_mon,
+					rc & ~0x40)) < 0)
+			 || ((rc = i2c_smbus_write_byte_data(client,
+					m41txx_chip->sqw,
+					m41txx_chip->sqw_freq)) < 0)
+			 || ((rc = i2c_smbus_write_byte_data(client,
+					m41txx_chip->alarm_mon,
+					rc | 0x40)) < 0))
+				goto sqw_err;
+
+		/* Make sure HT (Halt Update) bit is cleared */
+		if ((rc = i2c_smbus_read_byte_data(client,
+				m41txx_chip->alarm_hour)) < 0)
+			goto ht_err;
+
+		if (rc & M41TXX_ALHOUR_HT) {
+			if (m41txx_chip->features & M41TXX_FEATURE_HT) {
+				m41txx_get_datetime(client, &tm);
+				dev_info(&client->dev,
+					  "%s HT bit was set!\n", __FUNCTION__);
+				dev_info(&client->dev,
+					"%s Power Down at %04i-%02i-%02i %02i:%02i:%02i\n",
+					__FUNCTION__, tm.tm_year + 1900,
+					tm.tm_mon + 1, tm.tm_mday, tm.tm_hour,
+					tm.tm_min, tm.tm_sec);
+			}
+			if ((rc = i2c_smbus_write_byte_data(client,
+					m41txx_chip->alarm_hour,
+					rc & ~M41TXX_ALHOUR_HT)) < 0)
+				goto ht_err;
+		}
+	}
+
+	/* Make sure ST (stop) bit is cleared */
+	if ((rc = i2c_smbus_read_byte_data(client, m41txx_chip->sec)) < 0)
+		goto st_err;
+
+	if (rc & M41TXX_SEC_ST)
+		if ((rc = i2c_smbus_write_byte_data(client, m41txx_chip->sec,
+		     rc & ~M41TXX_SEC_ST)) < 0)
+			goto st_err;
+
+	m41txx_wq = create_singlethread_workqueue(m41txx_chip->name);
+	save_client = client;
+	return 0;
+
+st_err:
+	dev_err(&client->dev, "%s: Can't clear ST bit\n", __FUNCTION__);
+	goto exit_detach;
+ht_err:
+	dev_err(&client->dev, "%s: Can't clear HT bit\n", __FUNCTION__);
+	goto exit_detach;
+sqw_err:
+	dev_err(&client->dev, "%s: Can't set SQW Frequency\n",
+		__FUNCTION__);
+
+exit_detach:
+	i2c_detach_client(client);
+exit_kfree:
+	kfree(client);
+exit:
+	return rc;
+}
+
+static int m41txx_attach(struct i2c_adapter *adap)
+{
+	return i2c_probe(adap, &addr_data, m41txx_probe);
+}
+
+static int m41txx_detach(struct i2c_client *client)
+{
+	int rc;
+
+	struct rtc_device *rtc = i2c_get_clientdata(client);
+
+	if (rtc)
+		rtc_device_unregister(rtc);
+
+	if ((rc = i2c_detach_client(client)) == 0) {
+		if (m41txx_wq)
+			destroy_workqueue(m41txx_wq);
+		kfree(client);
+	}
+
+	return rc;
+}
+
+static int __init m41txx_rtc_init(void)
+{
+	return platform_driver_register(&m41txx_platform_driver);
+}
+
+
+static void __exit m41txx_rtc_exit(void)
+{
+	platform_driver_unregister(&m41txx_platform_driver);
+}
+
+MODULE_AUTHOR
+    ("Mark A. Greer <mgreer@mvista.com>, Alexander Bigga <ab@mycable.de>");
+MODULE_DESCRIPTION("ST Microelectronics M41T00 RTC I2C Client Driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
+
+module_init(m41txx_rtc_init);
+module_exit(m41txx_rtc_exit);
--- linux-2.6.18-rc3/include/linux/m41t00.h	2006-07-30 20:57:47.000000000 
+0200
+++ linux-2.6.18-rc3-mips-dev/include/linux/m41t00.h	2006-08-04 
11:50:19.000000000 +0200
@@ -16,6 +16,7 @@
 #define	M41T00_I2C_ADDR		0x68
 
 #define	M41T00_TYPE_M41T00	0
+#define	M41T00_TYPE_M41T80	80
 #define	M41T00_TYPE_M41T81	81
 #define	M41T00_TYPE_M41T85	85
 
-- 
Alexander Bigga     Tel: +49 4873 90 10 866
mycable GmbH        Fax: +49 4873 901 976
Boeker Stieg 43
D-24613 Aukrug      eMail: ab@mycable.de

Re: RTC: add RTC class interface to m41t00 driver

[Atsushi Nemoto]

On Fri, 4 Aug 2006 16:01:02 +0200, Alexander Bigga <ab@mycable.de> wrote:
> like you, I started recently with Mark's m41t00.c driver to add support for 
> the new rtc-subsystem. Mark reviewed it and I added his changes.

Thank you.  Though my patch for m41t00.c intended to keep original
code as is as possible, I like your approach.  I'll work with your new
driver.

> There is still the question, if the code for the interrupt context 
> (workqueues) should stay or not. You bracketed all this with CONFIG_GEN_RTC. 
> I can't say, if this is a good idea. Maybe somebody else has some good 
> comments.

I think read_time and set_time routine of rtc_class never called from
the interrupt context.  It looks true on current RTC class framework
and some RTC class drivers depend on it already.

> +#include <asm/time.h>
> +#include <asm/rtc.h>

The asm/time.h is not exist on some archs.  And while all asm/time.h
are included by asm/rtc.h, this can be removed safely.

> +int m41txx_set_datetime(struct i2c_client *client, struct rtc_time *tm)

static.

> +ulong m41t00_get_rtc_time(void)
> +{
> +	struct rtc_time tm;
> +
> +	m41txx_get_datetime(save_client, &tm);
> +
> +	return mktime(tm.tm_year, tm.tm_mon,
> +		      tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec);
> +}
> +EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);

Please drop this old interface from new driver.  There are other way
to glue new driver to existing platform, as hctosys.c does.

Then we can remove save_client too.

> +static struct workqueue_struct *m41txx_wq;

As I wrote above, I think this is not needed.  If this is really
needed, it should be done in RTC framework instead of lowlevel driver.

> +st_err:
> +	dev_err(&client->dev, "%s: Can't clear ST bit\n", __FUNCTION__);
> +	goto exit_detach;
> +ht_err:
> +	dev_err(&client->dev, "%s: Can't clear HT bit\n", __FUNCTION__);
> +	goto exit_detach;
> +sqw_err:
> +	dev_err(&client->dev, "%s: Can't set SQW Frequency\n",
> +		__FUNCTION__);
> +
> +exit_detach:
> +	i2c_detach_client(client);

rtc_device_unregister() must be called somewhere in error path.

---
Atsushi Nemoto

Re: RTC: add RTC class interface to m41t00 driver

[Andrew Morton]

On Fri, 4 Aug 2006 16:01:02 +0200
Alexander Bigga <ab@mycable.de> wrote:

> Mark's m41t00.c driver

I've lost track of what is happening with this patch, so I'll drop it. 
Please send an updated patch when it's all worked out, thanks.

Re: RTC: add RTC class interface to m41t00 driver

[Atsushi Nemoto]

On Fri, 4 Aug 2006 15:57:49 -0700, Andrew Morton <akpm@osdl.org> wrote:
> I've lost track of what is happening with this patch, so I'll drop it. 
> Please send an updated patch when it's all worked out, thanks.

Yes, we should talk about which direction to go.  Please drop it.  Thanks.