[PATCH] Add support for the S-35390A RTC chip.

[PATCH] Add support for the S-35390A RTC chip.

[Byron Bradley]

This adds basic get/set time support for the Seiko Instruments
S-35390A. This chip communicates using I2C and is used on the
QNAP TS-109/TS-209 NAS devices. It takes up eight addresses on
the I2C bus and depends on the following patches:

	i2c-remove-redundant-i2c_client-list.patch
	i2c-add-i2c_new_dummy-utility.patch

Signed-off-by: Byron Bradley <byron.bbradley-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Tested-by: Tim Ellis <tim-ReWupQYfQ/MAvxtiuMwx3w@public.gmane.org>
Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
---
 drivers/rtc/Kconfig       |    9 ++
 drivers/rtc/Makefile      |    1 +
 drivers/rtc/rtc-s35390a.c |  316 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 326 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc-s35390a.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 1e6715e..6c0fdf9 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -246,6 +246,15 @@ config RTC_DRV_TWL92330
 	  platforms.  The support is integrated with the rest of
 	  the Menelaus driver; it's not separate module.
 
+config RTC_DRV_S35390A
+	tristate "Seiko Instruments S-35390A"
+	help
+	  If you say yes here you will get support for the Seiko
+	  Instruments S-35390A.
+
+	  This driver can also be built as a module. If so the module
+	  will be called rtc-s35390a.
+
 endif # I2C
 
 comment "SPI RTC drivers"
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 465db4d..8d6218f 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_RTC_DRV_PL031)	+= rtc-pl031.o
 obj-$(CONFIG_RTC_DRV_RS5C313)	+= rtc-rs5c313.o
 obj-$(CONFIG_RTC_DRV_RS5C348)	+= rtc-rs5c348.o
 obj-$(CONFIG_RTC_DRV_RS5C372)	+= rtc-rs5c372.o
+obj-$(CONFIG_RTC_DRV_S35390A)	+= rtc-s35390a.o
 obj-$(CONFIG_RTC_DRV_S3C)	+= rtc-s3c.o
 obj-$(CONFIG_RTC_DRV_SA1100)	+= rtc-sa1100.o
 obj-$(CONFIG_RTC_DRV_SH)	+= rtc-sh.o
diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
new file mode 100644
index 0000000..6b2dc77
--- /dev/null
+++ b/drivers/rtc/rtc-s35390a.c
@@ -0,0 +1,316 @@
+/*
+ * Seiko Instruments S-35390A RTC Driver
+ *
+ * Copyright (c) 2007 Byron Bradley
+ *
+ * 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/module.h>
+#include <linux/rtc.h>
+#include <linux/i2c.h>
+#include <linux/bitrev.h>
+#include <linux/bcd.h>
+#include <linux/slab.h>
+
+#define S35390A_CMD_STATUS1	0
+#define S35390A_CMD_STATUS2	1
+#define S35390A_CMD_TIME1	2
+
+#define S35390A_BYTE_YEAR	0
+#define S35390A_BYTE_MONTH	1
+#define S35390A_BYTE_DAY	2
+#define S35390A_BYTE_WDAY	3
+#define S35390A_BYTE_HOURS	4
+#define S35390A_BYTE_MINS	5
+#define S35390A_BYTE_SECS	6
+
+#define S35390A_FLAG_POC	0x01
+#define S35390A_FLAG_BLD	0x02
+#define S35390A_FLAG_24H	0x40
+#define S35390A_FLAG_RESET	0x80
+#define S35390A_FLAG_TEST	0x01
+
+struct s35390a {
+	struct i2c_client *client[8];
+	struct rtc_device *rtc;
+	int twentyfourhour;
+};
+
+static int s35390a_set_reg(struct s35390a *s35390a, int reg, char *buf, int len)
+{
+	struct i2c_client *client = s35390a->client[reg];
+	struct i2c_msg msg[] = {
+		{ client->addr, 0, len, buf },
+	};
+
+	if ((i2c_transfer(client->adapter, msg, 1)) != 1)
+		return -EIO;
+
+	return 0;
+}
+
+static int s35390a_get_reg(struct s35390a *s35390a, int reg, char *buf, int len)
+{
+	struct i2c_client *client = s35390a->client[reg];
+	struct i2c_msg msg[] = {
+		{ client->addr, I2C_M_RD, len, buf },
+	};
+
+	if ((i2c_transfer(client->adapter, msg, 1)) != 1)
+		return -EIO;
+
+	return 0;
+}
+
+static int s35390a_reset(struct s35390a *s35390a)
+{
+	char buf[1];
+
+	if (s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, buf, sizeof(buf)) < 0)
+		return -EIO;
+
+	if (!(buf[0] & (S35390A_FLAG_POC | S35390A_FLAG_BLD)))
+		return 0;
+
+	buf[0] |= (S35390A_FLAG_RESET | S35390A_FLAG_24H);
+	buf[0] &= 0xf0;
+	return s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, buf, sizeof(buf));
+}
+
+static int s35390a_disable_test_mode(struct s35390a *s35390a)
+{
+	char buf[1];
+
+	if (s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, buf, sizeof(buf)) < 0)
+		return -EIO;
+
+	if (!(buf[0] & S35390A_FLAG_TEST))
+		return 0;
+
+	buf[0] &= ~S35390A_FLAG_TEST;
+	return s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, buf, sizeof(buf));
+}
+
+static char s35390a_hr2reg(struct s35390a *s35390a, int hour)
+{
+	if (s35390a->twentyfourhour)
+		return BIN2BCD(hour);
+
+	if (hour < 12)
+		return BIN2BCD(hour);
+
+	return 0x40 | BIN2BCD(hour - 12);
+}
+
+static int s35390a_reg2hr(struct s35390a *s35390a, char reg)
+{
+	unsigned hour;
+
+	if (s35390a->twentyfourhour)
+		return BCD2BIN(reg & 0x3f);
+
+	hour = BCD2BIN(reg & 0x3f);
+	if (reg & 0x40)
+		hour += 12;
+
+	return hour;
+}
+
+static int s35390a_set_datetime(struct i2c_client *client, struct rtc_time *tm)
+{
+	struct s35390a	*s35390a = i2c_get_clientdata(client);
+	int i, err;
+	char buf[7];
+
+	dev_dbg(&client->dev, "%s: tm is secs=%d, mins=%d, hours=%d mday=%d, "
+		"mon=%d, year=%d, wday=%d\n", __FUNCTION__, tm->tm_sec,
+		tm->tm_min, tm->tm_hour, tm->tm_mday, tm->tm_mon, tm->tm_year,
+		tm->tm_wday);
+
+	buf[S35390A_BYTE_YEAR] = BIN2BCD(tm->tm_year - 100);
+	buf[S35390A_BYTE_MONTH] = BIN2BCD(tm->tm_mon + 1);
+	buf[S35390A_BYTE_DAY] = BIN2BCD(tm->tm_mday);
+	buf[S35390A_BYTE_WDAY] = BIN2BCD(tm->tm_wday);
+	buf[S35390A_BYTE_HOURS] = s35390a_hr2reg(s35390a, tm->tm_hour);
+	buf[S35390A_BYTE_MINS] = BIN2BCD(tm->tm_min);
+	buf[S35390A_BYTE_SECS] = BIN2BCD(tm->tm_sec);
+
+	/* This chip expects the bits of each byte to be in reverse order */
+	for (i = 0; i < 7; ++i)
+		buf[i] = bitrev8(buf[i]);
+
+	err = s35390a_set_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
+
+	return err;
+}
+
+static int s35390a_get_datetime(struct i2c_client *client, struct rtc_time *tm)
+{
+	struct s35390a *s35390a = i2c_get_clientdata(client);
+	char buf[7];
+	int i, err;
+
+	err = s35390a_get_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
+	if (err < 0)
+		return err;
+
+	/* This chip returns the bits of each byte in reverse order */
+	for (i = 0; i < 7; ++i)
+		buf[i] = bitrev8(buf[i]);
+
+	tm->tm_sec = BCD2BIN(buf[S35390A_BYTE_SECS]);
+	tm->tm_min = BCD2BIN(buf[S35390A_BYTE_MINS]);
+	tm->tm_hour = s35390a_reg2hr(s35390a, buf[S35390A_BYTE_HOURS]);
+	tm->tm_wday = BCD2BIN(buf[S35390A_BYTE_WDAY]);
+	tm->tm_mday = BCD2BIN(buf[S35390A_BYTE_DAY]);
+	tm->tm_mon = BCD2BIN(buf[S35390A_BYTE_MONTH]) - 1;
+	tm->tm_year = BCD2BIN(buf[S35390A_BYTE_YEAR]) + 100;
+
+	dev_dbg(&client->dev, "%s: tm is secs=%d, mins=%d, hours=%d, mday=%d, "
+		"mon=%d, year=%d, wday=%d\n", __FUNCTION__, tm->tm_sec,
+		tm->tm_min, tm->tm_hour, tm->tm_mday, tm->tm_mon, tm->tm_year,
+		tm->tm_wday);
+
+	return rtc_valid_tm(tm);
+}
+
+static int s35390a_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	return s35390a_get_datetime(to_i2c_client(dev), tm);
+}
+
+static int s35390a_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	return s35390a_set_datetime(to_i2c_client(dev), tm);
+}
+
+static const struct rtc_class_ops s35390a_rtc_ops = {
+	.read_time	= s35390a_rtc_read_time,
+	.set_time	= s35390a_rtc_set_time,
+};
+
+static struct i2c_driver s35390a_driver;
+
+static int s35390a_probe(struct i2c_client *client)
+{
+	int err;
+	unsigned int i;
+	struct s35390a *s35390a;
+	struct rtc_time tm;
+	char buf[1];
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		err = -ENODEV;
+		goto exit;
+	}
+
+	s35390a = kzalloc(sizeof(struct s35390a), GFP_KERNEL);
+	if (!s35390a) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	s35390a->client[0] = client;
+	i2c_set_clientdata(client, s35390a);
+
+	/* This chip uses multiple addresses, use dummy devices for them */
+	for (i = 1; i < 8; ++i) {
+		s35390a->client[i] = i2c_new_dummy(client->adapter,
+					client->addr + i, "rtc-s35390a");
+		if (!s35390a->client[i]) {
+			dev_err(&client->dev, "Address %d unavailable\n",
+						client->addr + i);
+			err = -ENOCSI;
+			goto exit_dummy;
+		}
+	}
+
+	err = s35390a_reset(s35390a);
+	if (err < 0) {
+		dev_err(&client->dev, "error resetting chip\n");
+		goto exit_dummy;
+	}
+
+	err = s35390a_disable_test_mode(s35390a);
+	if (err < 0) {
+		dev_err(&client->dev, "error disabling test mode\n");
+		goto exit_dummy;
+	}
+
+	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, buf, sizeof(buf));
+	if (err < 0) {
+		dev_err(&client->dev, "error checking 12/24 hour mode\n");
+		goto exit_dummy;
+	}
+	if (buf[0] & S35390A_FLAG_24H)
+		s35390a->twentyfourhour = 1;
+	else
+		s35390a->twentyfourhour = 0;
+
+	if (s35390a_get_datetime(client, &tm) < 0)
+		dev_warn(&client->dev, "clock needs to be set\n");
+
+	s35390a->rtc = rtc_device_register(s35390a_driver.driver.name,
+				&client->dev, &s35390a_rtc_ops, THIS_MODULE);
+
+	if (IS_ERR(s35390a->rtc)) {
+		err = PTR_ERR(s35390a->rtc);
+		goto exit_dummy;
+	}
+	return 0;
+
+exit_dummy:
+	for (i = 1; i < 8; ++i)
+		if (s35390a->client[i])
+			i2c_unregister_device(s35390a->client[i]);
+	kfree(s35390a);
+	i2c_set_clientdata(client, NULL);
+
+exit:
+	return err;
+}
+
+static int s35390a_remove(struct i2c_client *client)
+{
+	unsigned int i;
+
+	struct s35390a *s35390a = i2c_get_clientdata(client);
+	for (i = 1; i < 8; ++i)
+		if (s35390a->client[i])
+			i2c_unregister_device(s35390a->client[i]);
+
+	rtc_device_unregister(s35390a->rtc);
+	kfree(s35390a);
+	i2c_set_clientdata(client, NULL);
+
+	return 0;
+}
+
+static struct i2c_driver s35390a_driver = {
+	.driver		= {
+		.name	= "rtc-s35390a",
+	},
+	.probe		= s35390a_probe,
+	.remove		= s35390a_remove,
+};
+
+static int __init s35390a_rtc_init(void)
+{
+	return i2c_add_driver(&s35390a_driver);
+}
+
+static void __exit s35390a_rtc_exit(void)
+{
+	i2c_del_driver(&s35390a_driver);
+}
+
+MODULE_AUTHOR("Byron Bradley <byron.bbradley-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
+MODULE_DESCRIPTION("S35390A RTC driver");
+MODULE_LICENSE("GPL");
+
+module_init(s35390a_rtc_init);
+module_exit(s35390a_rtc_exit);
-- 
1.5.3.5

Re: [PATCH] Add support for the S-35390A RTC chip.

[Jean Delvare]

Hi Byron,

On Sat,  5 Jan 2008 22:58:14 +0000, Byron Bradley wrote:
> This adds basic get/set time support for the Seiko Instruments
> S-35390A. This chip communicates using I2C and is used on the
> QNAP TS-109/TS-209 NAS devices. It takes up eight addresses on
> the I2C bus and depends on the following patches:
> 
> 	i2c-remove-redundant-i2c_client-list.patch
> 	i2c-add-i2c_new_dummy-utility.patch
> 
> Signed-off-by: Byron Bradley <byron.bbradley-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Tested-by: Tim Ellis <tim-ReWupQYfQ/MAvxtiuMwx3w@public.gmane.org>
> Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
> ---
>  drivers/rtc/Kconfig       |    9 ++
>  drivers/rtc/Makefile      |    1 +
>  drivers/rtc/rtc-s35390a.c |  316 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 326 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/rtc/rtc-s35390a.c

Acked-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>

With just one comment:

> +	/* This chip uses multiple addresses, use dummy devices for them */
> +	for (i = 1; i < 8; ++i) {
> +		s35390a->client[i] = i2c_new_dummy(client->adapter,
> +					client->addr + i, "rtc-s35390a");
> +		if (!s35390a->client[i]) {
> +			dev_err(&client->dev, "Address %d unavailable\n",
> +						client->addr + i);
> +			err = -ENOCSI;
> +			goto exit_dummy;
> +		}
> +	}

-ENOCSI? Don't you rather mean -EBUSY?

Also, the address would be better printed as an hexadecimal value -
decimal I2C addresses isn't something people are used to.

You should get your driver reviewed by Alessandro Zummo now, and then
it can be merged upstream... After the i2c cleanups it depends on are
upstream themselves, that is (working on it.)

-- 
Jean Delvare

Re: [PATCH] Add support for the S-35390A RTC chip.

[Byron Bradley]

On Jan 6, 2008 6:01 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> Hi Byron,
>
> On Sat,  5 Jan 2008 22:58:14 +0000, Byron Bradley wrote:
> > This adds basic get/set time support for the Seiko Instruments
> > S-35390A. This chip communicates using I2C and is used on the
> > QNAP TS-109/TS-209 NAS devices. It takes up eight addresses on
> > the I2C bus and depends on the following patches:
> >
> >       i2c-remove-redundant-i2c_client-list.patch
> >       i2c-add-i2c_new_dummy-utility.patch
> >
> > Signed-off-by: Byron Bradley <byron.bbradley-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Tested-by: Tim Ellis <tim-ReWupQYfQ/MAvxtiuMwx3w@public.gmane.org>
> > Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Cc: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
> > ---
> >  drivers/rtc/Kconfig       |    9 ++
> >  drivers/rtc/Makefile      |    1 +
> >  drivers/rtc/rtc-s35390a.c |  316 +++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 326 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/rtc/rtc-s35390a.c
>
> Acked-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
>
> With just one comment:
>
> > +     /* This chip uses multiple addresses, use dummy devices for them */
> > +     for (i = 1; i < 8; ++i) {
> > +             s35390a->client[i] = i2c_new_dummy(client->adapter,
> > +                                     client->addr + i, "rtc-s35390a");
> > +             if (!s35390a->client[i]) {
> > +                     dev_err(&client->dev, "Address %d unavailable\n",
> > +                                             client->addr + i);
> > +                     err = -ENOCSI;
> > +                     goto exit_dummy;
> > +             }
> > +     }
>
> -ENOCSI? Don't you rather mean -EBUSY?
>
> Also, the address would be better printed as an hexadecimal value -
> decimal I2C addresses isn't something people are used to.
>
> You should get your driver reviewed by Alessandro Zummo now, and then
> it can be merged upstream... After the i2c cleanups it depends on are
> upstream themselves, that is (working on it.)

Thanks Jean, I've sent it to Alessandro Zummo and the rtc-linux list.

David, the ENOCSI comment probably applies to your at24 eeprom driver
too. I was wondering why that error code was chosen since other than
"No CSI structure available" I couldn't find anything about it.

Cheers,

-- 
Byron Bradley

Re: [PATCH] Add support for the S-35390A RTC chip.

[David Brownell]

On Sunday 06 January 2008, Byron Bradley wrote:
> David, the ENOCSI comment probably applies to your at24 eeprom driver
> too. I was wondering why that error code was chosen since other than
> "No CSI structure available" I couldn't find anything about it.

It was chosen for its uniqueness ... ideally each fault path has its
own fault code and (possibly debug-only) diagnostic.  When you get
a fault code from a driver, and there's only one place that's used,
it's a lot easier to know what went wrong than if the fault code is
(over)used in many places.

In fact it's common practice to adopt subsystem-specific conventions
about what a given errno value indicates.  Otherwise, almost every
fault observed would map to a small handful ... making them useless
for fault recovery logic, and at best problematic in terms of any
diagnostic utility.

- Dave

Re: [PATCH] Add support for the S-35390A RTC chip.

[Jean Delvare]

Hi David,

On Sun, 6 Jan 2008 14:25:37 -0800, David Brownell wrote:
> On Sunday 06 January 2008, Byron Bradley wrote:
> > David, the ENOCSI comment probably applies to your at24 eeprom driver
> > too. I was wondering why that error code was chosen since other than
> > "No CSI structure available" I couldn't find anything about it.
> 
> It was chosen for its uniqueness ... ideally each fault path has its
> own fault code and (possibly debug-only) diagnostic.  When you get
> a fault code from a driver, and there's only one place that's used,
> it's a lot easier to know what went wrong than if the fault code is
> (over)used in many places.
> 
> In fact it's common practice to adopt subsystem-specific conventions
> about what a given errno value indicates.  Otherwise, almost every
> fault observed would map to a small handful ... making them useless
> for fault recovery logic, and at best problematic in terms of any
> diagnostic utility.

A common practice to use random error codes just to make sure they are
unique? I don't think so, no. Resource not available is EBUSY, not
ENOCSI, period. Please fix your driver.

-- 
Jean Delvare

Re: [PATCH] Add support for the S-35390A RTC chip.

[David Brownell]

On Tuesday 08 January 2008, Jean Delvare wrote:
> 
> > In fact it's common practice to adopt subsystem-specific conventions
> > about what a given errno value indicates.  Otherwise, almost every
> > fault observed would map to a small handful ... making them useless
> > for fault recovery logic, and at best problematic in terms of any
> > diagnostic utility.
> 
> A common practice to use random error codes

Hey, *I* didn't say "random error codes".


> just to make sure they are 
> unique? I don't think so, no. Resource not available is EBUSY, not
> ENOCSI, period. Please fix your driver.

Actually, "address in use" is EADDRINUSE.  I made that change.

(Another reason to use oddball errno values is as a placeholder
until a more appropriate one is found.)

- Dave

Re: [PATCH] Add support for the S-35390A RTC chip.

[Jean Delvare]

On Tue, 8 Jan 2008 04:39:27 -0800, David Brownell wrote:
> On Tuesday 08 January 2008, Jean Delvare wrote:
> > 
> > > In fact it's common practice to adopt subsystem-specific conventions
> > > about what a given errno value indicates.  Otherwise, almost every
> > > fault observed would map to a small handful ... making them useless
> > > for fault recovery logic, and at best problematic in terms of any
> > > diagnostic utility.
> > 
> > A common practice to use random error codes
> 
> Hey, *I* didn't say "random error codes".
> 
> 
> > just to make sure they are 
> > unique? I don't think so, no. Resource not available is EBUSY, not
> > ENOCSI, period. Please fix your driver.
> 
> Actually, "address in use" is EADDRINUSE.  I made that change.
> 
> (Another reason to use oddball errno values is as a placeholder
> until a more appropriate one is found.)

OK, EADDRINUSE is equally fine with me.

-- 
Jean Delvare