diff mbox

[2.6.28-rc6-davinci1,4/6] dm355evm rtc driver

Message ID 200812071159.40197.david-b@pacbell.net
State Accepted, archived
Headers show

Commit Message

David Brownell Dec. 7, 2008, 7:59 p.m. UTC
From: David Brownell <dbrownell@users.sourceforge.net>

Simple RTC driver for the MSP430 firmware on the DM355 EVM board.
Other than not supporting atomic reads/writes of all four bytes,
this is quite reasonable as a basic no-alarm RTC.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
Depends on the patch for the parent MFD driver.

NOTE:  not suitable for mainline until the dm355evm board support
(and parent MFD driver) is in the merge queue.

 drivers/rtc/Kconfig        |    6 +
 drivers/rtc/Makefile       |    1 
 drivers/rtc/rtc-dm355evm.c |  175 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 182 insertions(+)


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---

Comments

Alessandro Zummo Dec. 9, 2008, 9:39 a.m. UTC | #1
On Sun, 7 Dec 2008 11:59:39 -0800
David Brownell <david-b@pacbell.net> wrote:

> From: David Brownell <dbrownell@users.sourceforge.net>
> 
> Simple RTC driver for the MSP430 firmware on the DM355 EVM board.
> Other than not supporting atomic reads/writes of all four bytes,
> this is quite reasonable as a basic no-alarm RTC.

 Ok. This reminds me of that rtc-firmware thingy... I'll have to
 think about that.
David Brownell Dec. 9, 2008, 10:28 a.m. UTC | #2
On Tuesday 09 December 2008, Alessandro Zummo wrote:
> On Sun, 7 Dec 2008 11:59:39 -0800 David Brownell wrote:
> 
> > Simple RTC driver for the MSP430 firmware on the DM355 EVM board.
> > Other than not supporting atomic reads/writes of all four bytes,
> > this is quite reasonable as a basic no-alarm RTC.
> 
>  Ok. This reminds me of that rtc-firmware thingy... I'll have to
>  think about that.

Seems very different to me!  Board-specific.  The rtc-firmware
thingy evolved from rtc-ppc, which ISTR is a migration wrapper
for a powerpc-only RTC framework.  In this case there's no
such framework.  ARM's <asm/rtc.h> is gone, too, and that is
what enabled the PPC rtc-firmware driver ...

The MSP430 is accessed through I2C, and RTC functionality is
just one of several things it's been programmed to handle[1].
Think of it as just like any other RTC you talk to over RTC,
but it's implemented by a field-customizible ASIC.  ;)

- Dave

[1] http://c6000.spectrumdigital.com/evmdm355/revd/

    Almost at the bottom of that page, the "MSP430 Register
    definitions" pdf summarizes the I2C protocol requests
    implemented by that firmware.  Firmware source is
    downloadable from that page, so the board can be updated
    if you want the remote control to power the board up,
    or alarm, or whatever.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---
Alessandro Zummo Dec. 9, 2008, 10:42 a.m. UTC | #3
On Tue, 9 Dec 2008 02:28:05 -0800
David Brownell <david-b@pacbell.net> wrote:

> Seems very different to me!  Board-specific.  The rtc-firmware
> thingy evolved from rtc-ppc, which ISTR is a migration wrapper
> for a powerpc-only RTC framework.  In this case there's no
> such framework.  ARM's <asm/rtc.h> is gone, too, and that is
> what enabled the PPC rtc-firmware driver ...
> 
> The MSP430 is accessed through I2C, and RTC functionality is
> just one of several things it's been programmed to handle[1].
> Think of it as just like any other RTC you talk to over RTC,
> but it's implemented by a field-customizible ASIC.  ;)

 I was thinking that probably rtc-firmware could have handled
 the MSP430 too.
David Brownell Dec. 9, 2008, 11:12 a.m. UTC | #4
On Tuesday 09 December 2008, Alessandro Zummo wrote:
>  I was thinking that probably rtc-firmware could have handled
>  the MSP430 too.

I don't see how.  That would have required creating
a new arch/arm/include/asm/rtc.h with a framework for
other such stuff.  ARM finished getting rid of its
own RTC framework a few releases back; I certainly
wouldn't want to try creating a new one.

Plus, there's hardly any code gluing those two
methods into the RTC framework.  Even if this code
were wedged into some version of an rtc-firmware,
I'd expect the result would be bigger...

The reason to have wanted rtc-ppc (which would
morph into rtc-firmware) is that such an arch RTC
framework was already in place:  the "md" structs.
An rtc-ppc would help avoid "flag days" by helping
new and old frameworks coexist. Not an issue here.

- Dave





--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---
Alessandro Zummo Dec. 9, 2008, 11:21 a.m. UTC | #5
On Tue, 9 Dec 2008 03:12:11 -0800
David Brownell <david-b@pacbell.net> wrote:

> 
> On Tuesday 09 December 2008, Alessandro Zummo wrote:
> >  I was thinking that probably rtc-firmware could have handled
> >  the MSP430 too.
> 
> I don't see how.  That would have required creating
> a new arch/arm/include/asm/rtc.h with a framework for
> other such stuff.  ARM finished getting rid of its
> own RTC framework a few releases back; I certainly
> wouldn't want to try creating a new one.

 I'm just trying to understand if it does make sense to have
 one driver that can handle very simple platform devices
 that provide stateless get/set functions.

 It's not that I'm very favourable to this concept, just
 assessing if it can better serve the user's needs.
David Brownell Dec. 9, 2008, 6:07 p.m. UTC | #6
On Tuesday 09 December 2008, Alessandro Zummo wrote:
>  I'm just trying to understand if it does make sense to have
>  one driver that can handle very simple platform devices
>  that provide stateless get/set functions.

Other than the state held inside the RTC ... it's hard
to find any RTC driver that's not "stateless" like that!

That doesn't seem like a good motivation for adding one
more abstraction layer.  The use-cases I saw for the
rtc-ppc code were different:  reuse an existing layer,
to help phase it out gradually.


From what I've seen of the rtc-firmware thing, it would
not help in this case.  I think much the same accounting
would apply in other cases where there wasn't an existing
framework to wrap.

~/kernel/dm355/drivers/rtc$ nm --size-sort -S rtc-dm*o | egrep ' t '
00000000 00000004 t __initcall_dm355evm_rtc_init6
00000000 0000001c t dm355evm_rtc_exit
00000000 0000001c t dm355evm_rtc_init
	... that init/exit overhead is essentially fixed
00000000 00000024 t dm355evm_rtc_remove
00000000 00000068 t dm355evm_rtc_probe
	... while probe()/remove() would essentially be
	... what rtc-firmware provides, along with new
	... wrappers that call the set/read time methods
00000000 0000007c t dm355evm_rtc_set_time
0000007c 00000100 t dm355evm_rtc_read_time

~/kernel/dm355/drivers/rtc$ nm --size-sort -S rtc-dm*o | egrep ' d '
00000000 00000004 d __exitcall_dm355evm_rtc_exit
	... also fixed overhead
00000050 00000030 d dm355evm_rtc_ops
	... maybe rtc_ops could shrink
00000000 00000050 d rtc_dm355evm_driver
	... also fixed overhead, if there's a platform_device

So the *most* rtc-firmware could do here is morph the ops table
into 8 bytes, replacing the probe/remove logic with more complex
code that takes up more space (since it's got to do the same thing
PLUS provide wrappers) ... and is less obvious.

- Dave

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---
diff mbox

Patch

--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -246,6 +246,12 @@  config RTC_DRV_M41T80_WDT
 	  If you say Y here you will get support for the
 	  watchdog timer in the ST M41T60 and M41T80 RTC chips series.
 
+config RTC_DRV_DM355EVM
+	tristate "TI DaVinci DM355 EVM RTC"
+	depends on MFD_DM355EVM_MSP
+	help
+	  Supports the RTC firmware in the MSP430 on the DM355 EVM.
+
 config RTC_DRV_TWL92330
 	boolean "TI TWL92330/Menelaus"
 	depends on MENELAUS
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -23,6 +23,7 @@  obj-$(CONFIG_RTC_DRV_AT91SAM9)	+= rtc-at
 obj-$(CONFIG_RTC_DRV_BFIN)	+= rtc-bfin.o
 obj-$(CONFIG_RTC_DRV_CMOS)	+= rtc-cmos.o
 obj-$(CONFIG_RTC_DRV_DAVINCI_EVM) += rtc-davinci-evm.o
+obj-$(CONFIG_RTC_DRV_DM355EVM)	+= rtc-dm355evm.o
 obj-$(CONFIG_RTC_DRV_DS1216)	+= rtc-ds1216.o
 obj-$(CONFIG_RTC_DRV_DS1286)	+= rtc-ds1286.o
 obj-$(CONFIG_RTC_DRV_DS1302)	+= rtc-ds1302.o
--- /dev/null
+++ b/drivers/rtc/rtc-dm355evm.c
@@ -0,0 +1,175 @@ 
+/*
+ * rtc-dm355evm.c - access battery-backed counter in MSP430 firmware
+ *
+ * Copyright (c) 2008 by David Brownell
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/rtc.h>
+#include <linux/platform_device.h>
+
+#include <linux/i2c/dm355evm_msp.h>
+
+
+/*
+ * The MSP430 firmware on the DM355 EVM uses a watch crystal to feed
+ * a 1 Hz counter.  When a backup battery is supplied, that makes a
+ * reasonable RTC for applications where alarms and non-NTP drift
+ * compensation aren't important.
+ *
+ * The only real glitch is the inability to read or write all four
+ * counter bytes atomically:  the count may increment in the middle
+ * of an operation, causing trouble when the LSB rolls over.
+ *
+ * This driver was tested with firmware revision A4.
+ */
+union evm_time {
+	u8	bytes[4];
+	u32	value;
+};
+
+static int dm355evm_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	union evm_time	time;
+	int		status;
+	int		tries = 0;
+
+	do {
+		/*
+		 * Read LSB(0) to MSB(3) bytes.  Defend against the counter
+		 * rolling over by re-reading until the value is stable,
+		 * and assuming the four reads take at most a few seconds.
+		 */
+		status = dm355evm_msp_read(DM355EVM_MSP_RTC_0);
+		if (status < 0)
+			return status;
+		if (tries && time.bytes[0] == status)
+			break;
+		time.bytes[0] = status;
+
+		status = dm355evm_msp_read(DM355EVM_MSP_RTC_1);
+		if (status < 0)
+			return status;
+		if (tries && time.bytes[1] == status)
+			break;
+		time.bytes[1] = status;
+
+		status = dm355evm_msp_read(DM355EVM_MSP_RTC_2);
+		if (status < 0)
+			return status;
+		if (tries && time.bytes[2] == status)
+			break;
+		time.bytes[2] = status;
+
+		status = dm355evm_msp_read(DM355EVM_MSP_RTC_3);
+		if (status < 0)
+			return status;
+		if (tries && time.bytes[3] == status)
+			break;
+		time.bytes[3] = status;
+
+	} while (++tries < 5);
+
+	dev_dbg(dev, "read timestamp %08x\n", time.value);
+
+	rtc_time_to_tm(le32_to_cpu(time.value), tm);
+	return 0;
+}
+
+static int dm355evm_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	union evm_time	time;
+	unsigned long	value;
+	int		status;
+
+	rtc_tm_to_time(tm, &value);
+	time.value = cpu_to_le32(value);
+
+	dev_dbg(dev, "write timestamp %08x\n", time.value);
+
+	/*
+	 * REVISIT handle non-atomic writes ... maybe just retry until
+	 * byte[1] sticks (no rollover)?
+	 */
+	status = dm355evm_msp_write(time.bytes[0], DM355EVM_MSP_RTC_0);
+	if (status < 0)
+		return status;
+
+	status = dm355evm_msp_write(time.bytes[1], DM355EVM_MSP_RTC_1);
+	if (status < 0)
+		return status;
+
+	status = dm355evm_msp_write(time.bytes[2], DM355EVM_MSP_RTC_2);
+	if (status < 0)
+		return status;
+
+	status = dm355evm_msp_write(time.bytes[3], DM355EVM_MSP_RTC_3);
+	if (status < 0)
+		return status;
+
+	return 0;
+}
+
+static struct rtc_class_ops dm355evm_rtc_ops = {
+	.read_time	= dm355evm_rtc_read_time,
+	.set_time	= dm355evm_rtc_set_time,
+};
+
+/*----------------------------------------------------------------------*/
+
+static int __devinit dm355evm_rtc_probe(struct platform_device *pdev)
+{
+	struct rtc_device *rtc;
+
+	rtc = rtc_device_register(pdev->name,
+				  &pdev->dev, &dm355evm_rtc_ops, THIS_MODULE);
+	if (IS_ERR(rtc)) {
+		dev_err(&pdev->dev, "can't register RTC device, err %ld\n",
+			PTR_ERR(rtc));
+		return PTR_ERR(rtc);
+	}
+	platform_set_drvdata(pdev, rtc);
+
+	return 0;
+}
+
+static int __devexit dm355evm_rtc_remove(struct platform_device *pdev)
+{
+	struct rtc_device *rtc = platform_get_drvdata(pdev);
+
+	rtc_device_unregister(rtc);
+	platform_set_drvdata(pdev, NULL);
+	return 0;
+}
+
+/*
+ * I2C is used to talk to the MSP430, but this platform device is
+ * exposed by an MFD driver that manages I2C communications.
+ */
+static struct platform_driver rtc_dm355evm_driver = {
+	.probe		= dm355evm_rtc_probe,
+	.remove		= __devexit_p(dm355evm_rtc_remove),
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= "rtc-dm355evm",
+	},
+};
+
+static int __init dm355evm_rtc_init(void)
+{
+	return platform_driver_register(&rtc_dm355evm_driver);
+}
+module_init(dm355evm_rtc_init);
+
+static void __exit dm355evm_rtc_exit(void)
+{
+	platform_driver_unregister(&rtc_dm355evm_driver);
+}
+module_exit(dm355evm_rtc_exit);
+
+MODULE_LICENSE("GPL");