[PATCH v2] Add Dallas DS1390 RTC chip

[PATCH v2] Add Dallas DS1390 RTC chip

[Mark Jackson]

v2 of this patch with some code tidying as per previous comments.

This patch adds support for the Dallas DS1390 SPI RTC chip.

Signed-off-by: M.Jackson <mpfj@mimc.co.uk>
---
 drivers/rtc/Kconfig      |   14 +++
 drivers/rtc/Makefile     |    1 +
 drivers/rtc/rtc-ds1390.c |  258 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 273 insertions(+), 0 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 9a9755c..7ec6abb 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -292,6 +292,20 @@ config RTC_DRV_DS1305
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-ds1305.
 
+config RTC_DRV_DS1390
+	tristate "Dallas/Maxim DS1390"
+	help
+	  If you say yes here you get support for the DS1390 and probably
+	  other chips connected with SPI.
+
+	  The first seven registers on these chips hold an RTC, and other
+	  registers may add features such as NVRAM, a trickle charger for
+	  the RTC/NVRAM backup power, and alarms.  This driver may not
+	  expose all those available chip features.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-ds1390.
+
 config RTC_DRV_MAX6902
 	tristate "Maxim MAX6902"
 	help
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 18622ef..6306900 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_RTC_DRV_DS1302)	+= rtc-ds1302.o
 obj-$(CONFIG_RTC_DRV_DS1305)	+= rtc-ds1305.o
 obj-$(CONFIG_RTC_DRV_DS1307)	+= rtc-ds1307.o
 obj-$(CONFIG_RTC_DRV_DS1374)	+= rtc-ds1374.o
+obj-$(CONFIG_RTC_DRV_DS1390)	+= rtc-ds1390.o
 obj-$(CONFIG_RTC_DRV_DS1511)	+= rtc-ds1511.o
 obj-$(CONFIG_RTC_DRV_DS1553)	+= rtc-ds1553.o
 obj-$(CONFIG_RTC_DRV_DS1672)	+= rtc-ds1672.o
diff --git a/drivers/rtc/rtc-ds1390.c b/drivers/rtc/rtc-ds1390.c
new file mode 100644
index 0000000..2513d60
--- /dev/null
+++ b/drivers/rtc/rtc-ds1390.c
@@ -0,0 +1,258 @@
+/* drivers/rtc/rtc-ds1390.c
+ *
+ * Copyright (C) 2008 Mercury IMC Ltd
+ *
+ * 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.
+ *
+ * Driver for DS1390 spi RTC
+ *
+ * Changelog:
+ *
+ * 04-Apr-2008: Mark Jackson <mpfj@mimc.co.uk>
+ *		- Initial version based on rtc-max6902
+ *		- No alarm support though !!
+ */
+
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/spi/spi.h>
+#include <linux/bcd.h>
+
+#define DS1390_REG_100THS		0x00
+#define DS1390_REG_SECONDS		0x01
+#define DS1390_REG_MINUTES		0x02
+#define DS1390_REG_HOURS		0x03
+#define DS1390_REG_DAY			0x04
+#define DS1390_REG_DATE			0x05
+#define DS1390_REG_MONTH_CENT		0x06
+#define DS1390_REG_YEAR			0x07
+
+#define DS1390_REG_ALARM_100THS		0x08
+#define DS1390_REG_ALARM_SECONDS	0x09
+#define DS1390_REG_ALARM_MINUTES	0x0A
+#define DS1390_REG_ALARM_HOURS		0x0B
+#define DS1390_REG_ALARM_DAY_DATE	0x0C
+
+#define DS1390_REG_CONTROL		0x0D
+#define DS1390_REG_STATUS		0x0E
+#define DS1390_REG_TRICKLE		0x0F
+
+struct ds1390 {
+	struct rtc_device *rtc;
+	u8 buf[9];	/* cmd + 8 registers */
+	u8 tx_buf[2];
+	u8 rx_buf[2];
+};
+
+static void ds1390_set_reg(struct device *dev, unsigned char address,
+				unsigned char data)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	unsigned char buf[2];
+
+	/* MSB must be '1' to write */
+	buf[0] = address | 0x80;
+	buf[1] = data;
+
+	spi_write(spi, buf, 2);
+}
+
+static int ds1390_get_reg(struct device *dev, unsigned char address,
+				unsigned char *data)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct ds1390 *chip = dev_get_drvdata(dev);
+	struct spi_message message;
+	struct spi_transfer xfer;
+	int status;
+
+	if (!data)
+		return -EINVAL;
+
+	/* Build our spi message */
+	spi_message_init(&message);
+	memset(&xfer, 0, sizeof(xfer));
+	xfer.len = 2;
+	/* Can tx_buf and rx_buf be equal? The doc in spi.h is not sure... */
+	xfer.tx_buf = chip->tx_buf;
+	xfer.rx_buf = chip->rx_buf;
+
+	/* Clear MSB to indicate read */
+	chip->tx_buf[0] = address & 0x7f;
+
+	spi_message_add_tail(&xfer, &message);
+
+	/* do the i/o */
+	status = spi_sync(spi, &message);
+	if (status != 0)
+		return status;
+
+	*data = chip->rx_buf[1];
+
+	return message.status;
+}
+
+static int ds1390_get_datetime(struct device *dev, struct rtc_time *dt)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct ds1390 *chip = dev_get_drvdata(dev);
+	struct spi_message message;
+	struct spi_transfer xfer;
+	int status;
+
+	/* build the message */
+	spi_message_init(&message);
+	memset(&xfer, 0, sizeof(xfer));
+	xfer.len = 1 + 7;	/* read command + 7 registers */
+	xfer.tx_buf = chip->buf;
+	xfer.rx_buf = chip->buf;
+	chip->buf[0] = DS1390_REG_SECONDS;
+	spi_message_add_tail(&xfer, &message);
+
+	/* do the i/o */
+	status = spi_sync(spi, &message);
+	if (status != 0)
+		return status;
+
+	/* The chip sends data in this order:
+	 * Seconds, Minutes, Hours, Day, Date, Month / Century, Year */
+	dt->tm_sec	= BCD2BIN(chip->buf[1]);
+	dt->tm_min	= BCD2BIN(chip->buf[2]);
+	dt->tm_hour	= BCD2BIN(chip->buf[3]);
+	dt->tm_wday	= BCD2BIN(chip->buf[4]);
+	dt->tm_mday	= BCD2BIN(chip->buf[5]);
+	/* mask off century bit */
+	dt->tm_mon	= BCD2BIN(chip->buf[6] & 0x7f) - 1;
+	/* adjust for century bit */
+	dt->tm_year = BCD2BIN(chip->buf[7]) + ((chip->buf[6] & 0x80) ? 100 : 0);
+
+	return 0;
+}
+
+static int ds1390_set_datetime(struct device *dev, struct rtc_time *dt)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct ds1390 *chip = dev_get_drvdata(dev);
+	struct spi_message message;
+	struct spi_transfer xfer;
+	int status;
+
+	/* build the message */
+	spi_message_init(&message);
+	memset(&xfer, 0, sizeof(xfer));
+	xfer.len = 1 + 8;	/* write command + 8 registers */
+	xfer.tx_buf = chip->buf;
+	xfer.rx_buf = chip->buf;
+	chip->buf[0] = DS1390_REG_SECONDS | 0x80;
+	chip->buf[1] = BIN2BCD(dt->tm_sec);
+	chip->buf[2] = BIN2BCD(dt->tm_min);
+	chip->buf[3] = BIN2BCD(dt->tm_hour);
+	chip->buf[4] = BIN2BCD(dt->tm_wday);
+	chip->buf[5] = BIN2BCD(dt->tm_mday);
+	chip->buf[6] = BIN2BCD(dt->tm_mon + 1) |
+			((dt->tm_year > 99) ? 0x80 : 0x00);
+	chip->buf[7] = BIN2BCD(dt->tm_year % 100);
+	spi_message_add_tail(&xfer, &message);
+
+	/* do the i/o */
+	status = spi_sync(spi, &message);
+	if (status == 0)
+		return message.status;
+
+	return 0;
+}
+
+static int ds1390_read_time(struct device *dev, struct rtc_time *tm)
+{
+	return ds1390_get_datetime(dev, tm);
+}
+
+static int ds1390_set_time(struct device *dev, struct rtc_time *tm)
+{
+	return ds1390_set_datetime(dev, tm);
+}
+
+static const struct rtc_class_ops ds1390_rtc_ops = {
+	.read_time	= ds1390_read_time,
+	.set_time	= ds1390_set_time,
+};
+
+static int __devinit ds1390_probe(struct spi_device *spi)
+{
+	struct rtc_device *rtc;
+	unsigned char tmp;
+	struct ds1390 *chip;
+	int res;
+
+	printk(KERN_DEBUG "DS1390 SPI RTC driver\n");
+
+	rtc = rtc_device_register("ds1390",
+				&spi->dev, &ds1390_rtc_ops, THIS_MODULE);
+	if (IS_ERR(rtc)) {
+		printk(KERN_ALERT "RTC : unable to register device\n");
+		return PTR_ERR(rtc);
+	}
+
+	spi->mode = SPI_MODE_3;
+	spi->bits_per_word = 8;
+	spi_setup(spi);
+
+	chip = kzalloc(sizeof *chip, GFP_KERNEL);
+	if (!chip) {
+		printk(KERN_ALERT "RTC : unable to allocate device memory\n");
+		rtc_device_unregister(rtc);
+		return -ENOMEM;
+	}
+	chip->rtc = rtc;
+	dev_set_drvdata(&spi->dev, chip);
+
+	res = ds1390_get_reg(&spi->dev, DS1390_REG_SECONDS, &tmp);
+	if (res) {
+		printk(KERN_ALERT "RTC : unable to read device\n");
+		rtc_device_unregister(rtc);
+		return res;
+	}
+
+	return 0;
+}
+
+static int __devexit ds1390_remove(struct spi_device *spi)
+{
+	struct ds1390 *chip = platform_get_drvdata(spi);
+	struct rtc_device *rtc = chip->rtc;
+
+	if (rtc)
+		rtc_device_unregister(rtc);
+
+	kfree(chip);
+
+	return 0;
+}
+
+static struct spi_driver ds1390_driver = {
+	.driver = {
+		.name	= "rtc-ds1390",
+		.bus	= &spi_bus_type,
+		.owner	= THIS_MODULE,
+	},
+	.probe	= ds1390_probe,
+	.remove = __devexit_p(ds1390_remove),
+};
+
+static __init int ds1390_init(void)
+{
+	return spi_register_driver(&ds1390_driver);
+}
+module_init(ds1390_init);
+
+static __exit void ds1390_exit(void)
+{
+	spi_unregister_driver(&ds1390_driver);
+}
+module_exit(ds1390_exit);
+
+MODULE_DESCRIPTION("DS1390 SPI RTC driver");
+MODULE_AUTHOR("Mark Jackson <mpfj@mimc.co.uk>");
+MODULE_LICENSE("GPL");

Re: [PATCH v2] Add Dallas DS1390 RTC chip

[Alessandro Zummo]

On Mon, 20 Oct 2008 09:41:18 +0100
Mark Jackson <mpfj@mimc.co.uk> wrote:

> +struct ds1390 {
> +	struct rtc_device *rtc;
> +	u8 buf[9];	/* cmd + 8 registers */
> +	u8 tx_buf[2];
> +	u8 rx_buf[2];
> +};
> +

 don't we still have too many buffers?

 the use of bin2bcd an bcd2bin (lowercase) would also be appreciated.

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

Re: [PATCH v2] Add Dallas DS1390 RTC chip

[Mark Jackson]

Alessandro Zummo wrote:
> On Mon, 20 Oct 2008 09:41:18 +0100
> Mark Jackson <mpfj@mimc.co.uk> wrote:
> 
>> +struct ds1390 {
>> +	struct rtc_device *rtc;
>> +	u8 buf[9];	/* cmd + 8 registers */
>> +	u8 tx_buf[2];
>> +	u8 rx_buf[2];
>> +};
>> +
> 
>  don't we still have too many buffers?

Ah yes ... ooops !!

> 
>  the use of bin2bcd an bcd2bin (lowercase) would also be appreciated.
> 

Will do.

Mark

Re: [PATCH v2] Add Dallas DS1390 RTC chip

[David Brownell]

On Monday 20 October 2008, Mark Jackson wrote:
> +config RTC_DRV_DS1390
> +       tristate "Dallas/Maxim DS1390"
> +       help
> +         If you say yes here you get support for the DS1390 and probably
> +         other chips connected with SPI.

According to the datasheet at

  http://www.maxim-ic.com/quick_view2.cfm/qv_pk/4359

the DS1390, DS1391, DS1392, DS1393, and DS1394 chips are essentially
all the same except that the control register for the '91 and '92
has different bits.

So it looks like this driver should already support three chips, if
the spi_board_info has the right spi->mode bits set:  SPI_MODE_* and
SPI_3WIRE.  And if there were platform_data defined to hold the
initial control register setting, two more ... configuring that
trickle charger would already justify having platform_data too.

Please strike the "probably" and list at least those three chips.


> +         The first seven registers on these chips hold an RTC, and other
> +         registers may add features such as NVRAM, a trickle charger for
> +         the RTC/NVRAM backup power, and alarms.  This driver may not
> +         expose all those available chip features.

The "these chips" language bothers me.  This looks like cut'n'paste
from the rtc-ds1307 language, but you don't actually describe what
chips the driver handles.

I'd rather see the description say which other chips it expects to
handle, and have the description match.  All of these have alarms
and a trickle charger for backup power, but none have NVRAM.


> +/* drivers/rtc/rtc-ds1390.c

It's a bit of a nit, but the convention is 

	/*
	 * rtc-ds1390.c -- driver for DS1390 and related SPI RTCs
	 *
	 * Copyright (C) ... etc

Maybe with a separate "written by Mark Jackson" next to the copyright.

> + *
> + * Copyright (C) 2008 Mercury IMC Ltd
> + *
> + * 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.
> + *
> + * Driver for DS1390 spi RTC
> + *

And to keep changelogs out of source code; that's what GIT is for!


> + * Changelog:
> + *
> + * 04-Apr-2008: Mark Jackson <mpfj@mimc.co.uk>
> + *             - Initial version based on rtc-max6902
> + *             - No alarm support though !!
> + */

My personal taste is to have the copyright comment cover ONLY the
copyright (and licensing) issues, and have a separate comment
describing open issues with the code.  In this case, that would
be no support for the alarm, any other aspect of the control
register, or (unless it was set up by pre-OS code) the trickle
charger ... 


> +static int ds1390_get_datetime(struct device *dev, struct rtc_time *dt)
> +{
> +       ...
> +       return 0;

Especially since you don't currently have logic to handle the
initialization from first power up (clearing OSF etc), it'd be
good to "return rtc_valid_tm(dt);".


> +       status = spi_sync(spi, &message);
> +       if (status == 0)
> +               return message.status;
> +
> +       return 0;

Nowadays that can just be "return spi_sync(...)".

Although ... you can make this code be a bunch simpler
by using spi_write_then_read().


> +static struct spi_driver ds1390_driver = {
> +       .driver = {
> +               .name   = "rtc-ds1390",
> +               .bus    = &spi_bus_type,

Rule of thumb:  always let the bus code set driver->bus,
never set it yourself.

> +               .owner  = THIS_MODULE,
> +       },
> +       .probe  = ds1390_probe,
> +       .remove = __devexit_p(ds1390_remove),
> +};