Skip to contentSkip to conversation

RTC: add Xicor 1241 driver

PATCH In review Mark Zhan 2 participants 2 messages Sep 30, 2007 – Oct 1, 2007

Subsystems (2)
lore
Share & tools
lore

Mark Zhan

author
#0 lore raw

PATCH 3/4 RTC: add Xicor 1241 driver

This patch add the Xicor 1241 RTC driver which is used in MIPS Sibyte 1250/1480 boards.

---
 drivers/rtc/Kconfig         |   10 ++
 drivers/rtc/Makefile        |    2
 drivers/rtc/rtc-xicor1241.c |  184 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 196 insertions(+)
drivers/rtc/Kconfig +10 -0
@@ -184,6 +184,16 @@ config RTC_DRV_X1205
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-x1205.
 
+config RTC_DRV_XICOR1241
+	tristate "Xicor 1241"
+	depends on RTC_CLASS && I2C && MIPS
+	help
+	  If you say yes here you get support for the
+	  Xicor 1241 RTC chip.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-xicor1241.
+
 config RTC_DRV_PCF8563
 	tristate "Philips PCF8563/Epson RTC8564"
 	help
drivers/rtc/Makefile +2 -0
@@ -48,3 +48,5 @@ obj-$(CONFIG_RTC_DRV_TEST)	+= rtc-test.o
 obj-$(CONFIG_RTC_DRV_V3020)	+= rtc-v3020.o
 obj-$(CONFIG_RTC_DRV_VR41XX)	+= rtc-vr41xx.o
 obj-$(CONFIG_RTC_DRV_X1205)	+= rtc-x1205.o
+obj-$(CONFIG_RTC_DRV_XICOR1241)	+= rtc-xicor1241.o
+
drivers/rtc/rtc-xicor1241.c new file +184 -0

184 lines not on this page · raw message

#1 lore raw

Hi Mark,

Quoted 2 lines · Mark Zhan

This patch add the Xicor 1241 RTC driver which is used in MIPS Sibyte 1250/1480 boards.

So this chip is using two-byte register addressing, which isn't compatible with SMBus. Which explains why the register reads and writes in your driver look strange. I don't think it's quite correct.

Quoted 14 lines
drivers/rtc/rtc-xicor1241.c new file +14 -0
+/*
+ * Register Offset
+ */
+#define X1241_SEC	0x30		/* Seconds */
+#define X1241_MIN	0x31		/* Minutes */
+#define X1241_HOUR	0x32		/* Hours */
+#define X1241_MDAY	0x33		/* Day of month */
+#define X1241_MON	0x34		/* Month */
+#define X1241_YEAR	0x35		/* Year */
+#define X1241_WDAY	0x36		/* Day of Week */
+#define X1241_Y2K	0x37		/* Year 2K */
+#define X1241_SR	0x3F		/* Status register */
+
+DEFINE_SPINLOCK(xicor1241_lock);

drivers/rtc/rtc-xicor1241.c line 52

Quoted 10 lines
drivers/rtc/rtc-xicor1241.c new file +10 -0
+
+static int xicor1241_get_time(struct device *dev, struct rtc_time *tm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	unsigned int y2k, year, mon, mday, wday, hour, min, sec;
+	unsigned long flags;
+
+	spin_lock_irqsave(&xicor1241_lock, flags);
+
+	i2c_smbus_write_byte_data(client, X1241_SEC, X1241_SEC);

If I read the datasheet properly, this should be:

	i2c_smbus_write_byte_data(client, 0, X1241_SEC);

The SC register is at 0x0030, not 0x3030.

drivers/rtc/rtc-xicor1241.c line 60

Quoted 8 lines
drivers/rtc/rtc-xicor1241.c new file +8 -0
+	sec = i2c_smbus_read_byte_data(client, X1241_SEC);
+	min = i2c_smbus_read_byte_data(client, X1241_MIN);
+	hour = i2c_smbus_read_byte_data(client, X1241_HOUR);
+	mday = i2c_smbus_read_byte_data(client, X1241_MDAY);
+	mon = i2c_smbus_read_byte_data(client, X1241_MON);
+	year = i2c_smbus_read_byte_data(client, X1241_YEAR);
+	wday = i2c_smbus_read_byte_data(client, X1241_WDAY);
+	y2k = i2c_smbus_read_byte_data(client, X1241_Y2K);

You are using the "Current Address Read" mode here, right? If so, you aren't supposed to send any address information at all, i.e. you should use i2c_smbus_read_byte() instead of i2c_smbus_read_byte_data(). You are probably just lucky that the chip ignores the extra byte you're sending.

Quoted 10 lines

(...)

drivers/rtc/rtc-xicor1241.c new file +8 -0
+static int xicor1241_set_time(struct device *dev, struct rtc_time *tm)
+{
(...)
+	/* unlock writes to the CCR */
+	i2c_smbus_write_word_data(client, X1241_SR,
+			(X1241_SR_WEL << 8) | X1241_SR);
+	i2c_smbus_write_word_data(client, X1241_SR,
+			((X1241_SR_WEL | X1241_SR_RWEL) << 8) | X1241_SR);
+

drivers/rtc/rtc-xicor1241.c line 123

Quoted 10 lines
drivers/rtc/rtc-xicor1241.c new file +10 -0
+	i2c_smbus_write_word_data(client, X1241_SEC, (sec << 8) | X1241_SEC);
+	i2c_smbus_write_word_data(client, X1241_MIN, (min << 8) | X1241_MIN);
+	i2c_smbus_write_word_data(client, X1241_HOUR, (hour << 8) | X1241_HOUR);
+	i2c_smbus_write_word_data(client, X1241_MDAY, (mday << 8) | X1241_MDAY);
+	i2c_smbus_write_word_data(client, X1241_WDAY, (wday << 8) | X1241_WDAY);
+	i2c_smbus_write_word_data(client, X1241_MON, (mon << 8) | X1241_MON);
+	i2c_smbus_write_word_data(client, X1241_YEAR, (year << 8) | X1241_YEAR);
+	i2c_smbus_write_word_data(client, X1241_Y2K, (y2k << 8) | X1241_Y2K);
+
+	i2c_smbus_write_word_data(client, X1241_SR, (0 << 8) | X1241_SR);

Here again I am surprised. I expected:

	i2c_smbus_write_word_data(client, 0, (sec << 8) | X1241_SEC);

So that you write to register 0x0030, not 0x3030. Same for all the other register writes. Or am I misreading the datasheet?

Quoted 8 lines
drivers/rtc/rtc-xicor1241.c new file +8 -0
+
+	spin_unlock_irqrestore(&xicor1241_lock, flags);
+	return 0;
+}
+
+static const struct rtc_class_ops xicor1241_rtc_ops = {
+	.read_time = xicor1241_get_time,
+	.set_time  = xicor1241_set_time,

drivers/rtc/rtc-xicor1241.c line 141

Quoted 10 lines
drivers/rtc/rtc-xicor1241.c new file +10 -0
+};
+
+static int __devinit xicor1241_rtc_probe(struct i2c_client *client)
+{
+	struct rtc_device *rtc;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_dbg(&client->dev, "I2C adapter function check failure!\n");
+		return -ENODEV;
+	}

This check isn't sufficient, you must check for I2C_FUNC_SMBUS_WRITE_WORD_DATA as well, and possibly I2C_FUNC_SMBUS_READ_BYTE if my comment above is correct.

Signature · 1 line
Jean Delvare