All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ed Swierk <eswierk-BGArkANP9klv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>,
	Alessandro Zummo
	<a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	BAR
Subject: [PATCH] rtc-ds1307: True SMBus compatibility
Date: Mon, 05 Jan 2009 09:41:01 -0800	[thread overview]
Message-ID: <1231177261.13443.20.camel@localhost.localdomain> (raw)

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

[Resent to correct linux-i2c address]

Following up to Sebastien Barre's recent patch, here I go one step
further, replacing i2c block transfers with SMBus byte transfers.
Sticking to pure SMBus makes the driver work on our board, which has an
nVidia SMBus controller driving a DS1339; nforce2 controllers
unfortunately don't support any flavor of i2c block transfer.

Reading or writing registers repeatedly until they stick is rather
nauseating but I'm aiming for correctness if not elegance.  I would
appreciate any suggestions for improvement.

Signed-off-by: Ed Swierk <eswierk-BGArkANP9klv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>


[-- Attachment #2: rtc-ds1307-pure-smbus-compatibility.patch --]
[-- Type: text/x-patch, Size: 5414 bytes --]

Index: linux-2.6.27.4/drivers/rtc/rtc-ds1307.c
===================================================================
--- linux-2.6.27.4.orig/drivers/rtc/rtc-ds1307.c
+++ linux-2.6.27.4/drivers/rtc/rtc-ds1307.c
@@ -132,6 +132,64 @@ MODULE_DEVICE_TABLE(i2c, ds1307_id);
 
 /*----------------------------------------------------------------------*/
 
+static s32 ds1307_read_block_data_once(struct i2c_client *client, u8 command,
+				  u8 length, u8 *values)
+{
+	s32 i, data;
+	for (i = 0; i < length; i++) {
+		data = i2c_smbus_read_byte_data(client, command + i);
+		if (data < 0)
+			return data;
+		*(values + i) = data;
+	}
+	return i;
+}
+
+static s32 ds1307_read_block_data(struct i2c_client *client, u8 command,
+				  u8 length, u8 *values)
+{
+	u8 oldvalues[I2C_SMBUS_BLOCK_MAX];
+	s32 ret;
+	pr_debug("ds1307_read_block_data (length=%d)\n", length);
+	ret = ds1307_read_block_data_once(client, command, length, values);
+	if (ret < 0)
+		return ret;
+	do {
+		s32 ret;
+		memcpy(oldvalues, values, length);
+		ret = ds1307_read_block_data_once(client, command, length, values);
+		if (ret < 0)
+			return ret;
+	} while (memcmp(oldvalues, values, length));
+	return length;
+}
+
+static s32 ds1307_write_block_data(struct i2c_client *client, u8 command,
+				   u8 length, const u8 *values)
+{
+	u8 currvalues[I2C_SMBUS_BLOCK_MAX];
+	s32 ret;
+	pr_debug("ds1307_write_block_data (length=%d)\n", length);
+	ret = ds1307_read_block_data_once(client, command, length, currvalues);
+	if (ret < 0)
+		return ret;
+	do {
+		s32 i, ret;
+		for (i = 0; i < length; i++) {
+			ret = i2c_smbus_write_byte_data(client, command + i,
+							*(values + i));
+			if (ret < 0)
+				return ret;
+		}
+		ret = ds1307_read_block_data_once(client, command, length, currvalues);
+		if (ret < 0)
+			return ret;
+	} while (memcmp(currvalues, values, length));
+	return length;
+}
+
+/*----------------------------------------------------------------------*/
+
 /*
  * The IRQ logic includes a "real" handler running in IRQ context just
  * long enough to schedule this workqueue entry.   We need a task context
@@ -202,7 +266,7 @@ static int ds1307_get_time(struct device
 	int		tmp;
 
 	/* read the RTC date and time registers all at once */
-	tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+	tmp = ds1307_read_block_data(ds1307->client,
 		DS1307_REG_SECS, 7, ds1307->regs);
 	if (tmp != 7) {
 		dev_err(dev, "%s error %d\n", "read", tmp);
@@ -279,7 +343,7 @@ static int ds1307_set_time(struct device
 		"write", buf[0], buf[1], buf[2], buf[3],
 		buf[4], buf[5], buf[6]);
 
-	result = i2c_smbus_write_i2c_block_data(ds1307->client, 0, 7, buf);
+	result = ds1307_write_block_data(ds1307->client, 0, 7, buf);
 	if (result < 0) {
 		dev_err(dev, "%s error %d\n", "write", result);
 		return result;
@@ -297,7 +361,7 @@ static int ds1307_read_alarm(struct devi
 		return -EINVAL;
 
 	/* read all ALARM1, ALARM2, and status registers at once */
-	ret = i2c_smbus_read_i2c_block_data(client,
+	ret = ds1307_read_block_data(client,
 			DS1339_REG_ALARM1_SECS, 9, ds1307->regs);
 	if (ret != 9) {
 		dev_err(dev, "%s error %d\n", "alarm read", ret);
@@ -356,7 +420,7 @@ static int ds1307_set_alarm(struct devic
 		t->enabled, t->pending);
 
 	/* read current status of both alarms and the chip */
-	ret = i2c_smbus_read_i2c_block_data(client,
+	ret = ds1307_read_block_data(client,
 			DS1339_REG_ALARM1_SECS, 9, buf);
 	if (ret != 9) {
 		dev_err(dev, "%s error %d\n", "alarm write", ret);
@@ -391,7 +455,7 @@ static int ds1307_set_alarm(struct devic
 	}
 	buf[8] = status & ~(DS1337_BIT_A1I | DS1337_BIT_A2I);
 
-	ret = i2c_smbus_write_i2c_block_data(client,
+	ret = ds1307_write_block_data(client,
 			DS1339_REG_ALARM1_SECS, 9, buf);
 	if (ret < 0) {
 		dev_err(dev, "can't set alarm time\n");
@@ -479,7 +543,7 @@ ds1307_nvram_read(struct kobject *kobj, 
 	if (unlikely(!count))
 		return count;
 
-	result = i2c_smbus_read_i2c_block_data(client, 8 + off, count, buf);
+	result = ds1307_read_block_data(client, 8 + off, count, buf);
 	if (result < 0)
 		dev_err(&client->dev, "%s error %d\n", "nvram read", result);
 	return result;
@@ -501,7 +565,7 @@ ds1307_nvram_write(struct kobject *kobj,
 	if (unlikely(!count))
 		return count;
 
-	result = i2c_smbus_write_i2c_block_data(client, 8 + off, count, buf);
+	result = ds1307_write_block_data(client, 8 + off, count, buf);
 	if (result < 0) {
 		dev_err(&client->dev, "%s error %d\n", "nvram write", result);
 		return result;
@@ -537,8 +601,7 @@ static int __devinit ds1307_probe(struct
 	unsigned char		*buf;
 
 	if (!i2c_check_functionality(adapter,
-			I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
-			I2C_FUNC_SMBUS_I2C_BLOCK))
+			I2C_FUNC_SMBUS_BYTE_DATA))
 		return -EIO;
 
 	if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL)))
@@ -558,7 +621,7 @@ static int __devinit ds1307_probe(struct
 			want_irq = true;
 		}
 		/* get registers that the "rtc" read below won't read... */
-		tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+		tmp = ds1307_read_block_data(ds1307->client,
 				DS1337_REG_CONTROL, 2, buf);
 		if (tmp != 2) {
 			pr_debug("read error %d\n", tmp);
@@ -596,7 +659,7 @@ static int __devinit ds1307_probe(struct
 
 read_rtc:
 	/* read RTC registers */
-	tmp = i2c_smbus_read_i2c_block_data(ds1307->client, 0, 8, buf);
+	tmp = ds1307_read_block_data(ds1307->client, 0, 8, buf);
 	if (tmp != 8) {
 		pr_debug("read error %d\n", tmp);
 		err = -EIO;

WARNING: multiple messages have this Message-ID (diff)
From: Ed Swierk <eswierk@aristanetworks.com>
To: linux-i2c@vger.kernel.org, David Brownell <david-b@pacbell.net>,
	Alessandro Zummo <a.zummo@towertech.it>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	BARRE Sebastien <sbarre@sdelcc.com>
Subject: [PATCH] rtc-ds1307: True SMBus compatibility
Date: Mon, 05 Jan 2009 09:41:01 -0800	[thread overview]
Message-ID: <1231177261.13443.20.camel@localhost.localdomain> (raw)

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

[Resent to correct linux-i2c address]

Following up to Sebastien Barre's recent patch, here I go one step
further, replacing i2c block transfers with SMBus byte transfers.
Sticking to pure SMBus makes the driver work on our board, which has an
nVidia SMBus controller driving a DS1339; nforce2 controllers
unfortunately don't support any flavor of i2c block transfer.

Reading or writing registers repeatedly until they stick is rather
nauseating but I'm aiming for correctness if not elegance.  I would
appreciate any suggestions for improvement.

Signed-off-by: Ed Swierk <eswierk@aristanetworks.com>


[-- Attachment #2: rtc-ds1307-pure-smbus-compatibility.patch --]
[-- Type: text/x-patch, Size: 5414 bytes --]

Index: linux-2.6.27.4/drivers/rtc/rtc-ds1307.c
===================================================================
--- linux-2.6.27.4.orig/drivers/rtc/rtc-ds1307.c
+++ linux-2.6.27.4/drivers/rtc/rtc-ds1307.c
@@ -132,6 +132,64 @@ MODULE_DEVICE_TABLE(i2c, ds1307_id);
 
 /*----------------------------------------------------------------------*/
 
+static s32 ds1307_read_block_data_once(struct i2c_client *client, u8 command,
+				  u8 length, u8 *values)
+{
+	s32 i, data;
+	for (i = 0; i < length; i++) {
+		data = i2c_smbus_read_byte_data(client, command + i);
+		if (data < 0)
+			return data;
+		*(values + i) = data;
+	}
+	return i;
+}
+
+static s32 ds1307_read_block_data(struct i2c_client *client, u8 command,
+				  u8 length, u8 *values)
+{
+	u8 oldvalues[I2C_SMBUS_BLOCK_MAX];
+	s32 ret;
+	pr_debug("ds1307_read_block_data (length=%d)\n", length);
+	ret = ds1307_read_block_data_once(client, command, length, values);
+	if (ret < 0)
+		return ret;
+	do {
+		s32 ret;
+		memcpy(oldvalues, values, length);
+		ret = ds1307_read_block_data_once(client, command, length, values);
+		if (ret < 0)
+			return ret;
+	} while (memcmp(oldvalues, values, length));
+	return length;
+}
+
+static s32 ds1307_write_block_data(struct i2c_client *client, u8 command,
+				   u8 length, const u8 *values)
+{
+	u8 currvalues[I2C_SMBUS_BLOCK_MAX];
+	s32 ret;
+	pr_debug("ds1307_write_block_data (length=%d)\n", length);
+	ret = ds1307_read_block_data_once(client, command, length, currvalues);
+	if (ret < 0)
+		return ret;
+	do {
+		s32 i, ret;
+		for (i = 0; i < length; i++) {
+			ret = i2c_smbus_write_byte_data(client, command + i,
+							*(values + i));
+			if (ret < 0)
+				return ret;
+		}
+		ret = ds1307_read_block_data_once(client, command, length, currvalues);
+		if (ret < 0)
+			return ret;
+	} while (memcmp(currvalues, values, length));
+	return length;
+}
+
+/*----------------------------------------------------------------------*/
+
 /*
  * The IRQ logic includes a "real" handler running in IRQ context just
  * long enough to schedule this workqueue entry.   We need a task context
@@ -202,7 +266,7 @@ static int ds1307_get_time(struct device
 	int		tmp;
 
 	/* read the RTC date and time registers all at once */
-	tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+	tmp = ds1307_read_block_data(ds1307->client,
 		DS1307_REG_SECS, 7, ds1307->regs);
 	if (tmp != 7) {
 		dev_err(dev, "%s error %d\n", "read", tmp);
@@ -279,7 +343,7 @@ static int ds1307_set_time(struct device
 		"write", buf[0], buf[1], buf[2], buf[3],
 		buf[4], buf[5], buf[6]);
 
-	result = i2c_smbus_write_i2c_block_data(ds1307->client, 0, 7, buf);
+	result = ds1307_write_block_data(ds1307->client, 0, 7, buf);
 	if (result < 0) {
 		dev_err(dev, "%s error %d\n", "write", result);
 		return result;
@@ -297,7 +361,7 @@ static int ds1307_read_alarm(struct devi
 		return -EINVAL;
 
 	/* read all ALARM1, ALARM2, and status registers at once */
-	ret = i2c_smbus_read_i2c_block_data(client,
+	ret = ds1307_read_block_data(client,
 			DS1339_REG_ALARM1_SECS, 9, ds1307->regs);
 	if (ret != 9) {
 		dev_err(dev, "%s error %d\n", "alarm read", ret);
@@ -356,7 +420,7 @@ static int ds1307_set_alarm(struct devic
 		t->enabled, t->pending);
 
 	/* read current status of both alarms and the chip */
-	ret = i2c_smbus_read_i2c_block_data(client,
+	ret = ds1307_read_block_data(client,
 			DS1339_REG_ALARM1_SECS, 9, buf);
 	if (ret != 9) {
 		dev_err(dev, "%s error %d\n", "alarm write", ret);
@@ -391,7 +455,7 @@ static int ds1307_set_alarm(struct devic
 	}
 	buf[8] = status & ~(DS1337_BIT_A1I | DS1337_BIT_A2I);
 
-	ret = i2c_smbus_write_i2c_block_data(client,
+	ret = ds1307_write_block_data(client,
 			DS1339_REG_ALARM1_SECS, 9, buf);
 	if (ret < 0) {
 		dev_err(dev, "can't set alarm time\n");
@@ -479,7 +543,7 @@ ds1307_nvram_read(struct kobject *kobj, 
 	if (unlikely(!count))
 		return count;
 
-	result = i2c_smbus_read_i2c_block_data(client, 8 + off, count, buf);
+	result = ds1307_read_block_data(client, 8 + off, count, buf);
 	if (result < 0)
 		dev_err(&client->dev, "%s error %d\n", "nvram read", result);
 	return result;
@@ -501,7 +565,7 @@ ds1307_nvram_write(struct kobject *kobj,
 	if (unlikely(!count))
 		return count;
 
-	result = i2c_smbus_write_i2c_block_data(client, 8 + off, count, buf);
+	result = ds1307_write_block_data(client, 8 + off, count, buf);
 	if (result < 0) {
 		dev_err(&client->dev, "%s error %d\n", "nvram write", result);
 		return result;
@@ -537,8 +601,7 @@ static int __devinit ds1307_probe(struct
 	unsigned char		*buf;
 
 	if (!i2c_check_functionality(adapter,
-			I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
-			I2C_FUNC_SMBUS_I2C_BLOCK))
+			I2C_FUNC_SMBUS_BYTE_DATA))
 		return -EIO;
 
 	if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL)))
@@ -558,7 +621,7 @@ static int __devinit ds1307_probe(struct
 			want_irq = true;
 		}
 		/* get registers that the "rtc" read below won't read... */
-		tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+		tmp = ds1307_read_block_data(ds1307->client,
 				DS1337_REG_CONTROL, 2, buf);
 		if (tmp != 2) {
 			pr_debug("read error %d\n", tmp);
@@ -596,7 +659,7 @@ static int __devinit ds1307_probe(struct
 
 read_rtc:
 	/* read RTC registers */
-	tmp = i2c_smbus_read_i2c_block_data(ds1307->client, 0, 8, buf);
+	tmp = ds1307_read_block_data(ds1307->client, 0, 8, buf);
 	if (tmp != 8) {
 		pr_debug("read error %d\n", tmp);
 		err = -EIO;

             reply	other threads:[~2009-01-05 17:41 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-05 17:41 Ed Swierk [this message]
2009-01-05 17:41 ` [PATCH] rtc-ds1307: True SMBus compatibility Ed Swierk
2009-01-06 22:48 ` David Brownell
     [not found] ` <1231177261.13443.20.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2009-01-07 13:24   ` Jean Delvare
2009-01-07 13:24     ` Jean Delvare
     [not found]     ` <20090107142426.4be04d4d-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2009-01-07 15:22       ` Ed Swierk
2009-01-07 15:22         ` Ed Swierk
     [not found]         ` <9ae48b020901070722l77bebc6boc8fa2fd0bcc8da28-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-01-07 15:27           ` Jean Delvare
2009-01-07 15:27             ` Jean Delvare
     [not found]             ` <20090107162709.755982c0-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2009-01-07 15:43               ` Ed Swierk
2009-01-07 15:43                 ` Ed Swierk
     [not found]                 ` <9ae48b020901070743x1a5eaf72k5314cd969dc580ef-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-01-07 15:49                   ` Jean Delvare
2009-01-07 15:49                     ` Jean Delvare
2009-01-19 23:59               ` Ed Swierk
2009-01-19 23:59                 ` Ed Swierk
     [not found]                 ` <1232409577.25123.11.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2009-01-20 10:43                   ` Jean Delvare
2009-01-20 10:43                     ` Jean Delvare
     [not found]                     ` <20090120114304.3b956189-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2009-01-26  6:33                       ` Ed Swierk
2009-01-26  6:33                         ` Ed Swierk
     [not found]                         ` <1232951630.1329.4.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2009-01-26  9:37                           ` Jean Delvare
2009-01-26  9:37                             ` Jean Delvare
2009-01-26 21:06                           ` David Brownell
2009-01-26 21:06                             ` David Brownell
     [not found]                             ` <200901261306.21059.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2009-01-26 21:54                               ` Ed Swierk
2009-01-26 21:54                                 ` Ed Swierk
  -- strict thread matches above, loose matches on Subject: below --
2009-01-05 17:36 Ed Swierk
2009-01-06 13:35 ` BARRE Sebastien
2009-01-06 22:06   ` Alessandro Zummo
     [not found]   ` <C1C62166118DFA4A8BBA01A2411F0D171224AECDC1-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
2009-01-06 22:13     ` Ed Swierk
2009-01-06 22:13       ` Ed Swierk
2009-01-07 14:22       ` BARRE Sebastien
2009-01-07 14:22         ` BARRE Sebastien

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1231177261.13443.20.camel@localhost.localdomain \
    --to=eswierk-bgarkanp9klv6pq1l3v1odbpr1lh4cv8@public.gmane.org \
    --cc=a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.