Message ID | 200812071159.40197.david-b@pacbell.net |
---|---|
State | Accepted, archived |
Headers | show |
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.
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. -~----------~----~----~----~------~----~------~--~---
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.
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. -~----------~----~----~----~------~----~------~--~---
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.
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. -~----------~----~----~----~------~----~------~--~---
--- 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");