[i2c] [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
Hi,
This patch change all i2c access functions to SMBus access functions in order
to use the ds1307 on SMBus.
I expect that using SMBus access functions is correct for all boards with i2C
or SMBus adapter. Is it correct ?
I have tested it on my Geode LX board with a ds1307 device.
Please CC me your comments.
Thanks.
--- a/drivers/rtc/rtc-ds1307.c 2008-09-08 17:40:20.0 +
+++ b/drivers/rtc/rtc-ds1307.c 2008-10-07 13:21:57.0 +
@@ -92,7 +92,6 @@ struct ds1307 {
boolhas_nvram;
u8 regs[8];
enum ds_typetype;
- struct i2c_msg msg[2];
struct i2c_client *client;
struct i2c_client dev;
struct rtc_device *rtc;
@@ -138,12 +137,10 @@ static int ds1307_get_time(struct device
int tmp;
/* read the RTC date and time registers all at once */
- ds1307->msg[1].flags = I2C_M_RD;
- ds1307->msg[1].len = 7;
-
- tmp = i2c_transfer(to_i2c_adapter(ds1307->client->dev.parent),
- ds1307->msg, 2);
- if (tmp != 2) {
+ u8 *buf = ds1307->regs;
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ DS1307_REG_SECS, 7, buf);
+ if (tmp != 7) {
dev_err(dev, "%s error %d\n", "read", tmp);
return -EIO;
}
@@ -180,7 +177,6 @@ static int ds1307_get_time(struct device
static int ds1307_set_time(struct device *dev, struct rtc_time *t)
{
struct ds1307 *ds1307 = dev_get_drvdata(dev);
- int result;
int tmp;
u8 *buf = ds1307->regs;
@@ -190,7 +186,6 @@ static int ds1307_set_time(struct device
t->tm_hour, t->tm_mday,
t->tm_mon, t->tm_year, t->tm_wday);
- *buf++ = 0; /* first register addr */
buf[DS1307_REG_SECS] = BIN2BCD(t->tm_sec);
buf[DS1307_REG_MIN] = BIN2BCD(t->tm_min);
buf[DS1307_REG_HOUR] = BIN2BCD(t->tm_hour);
@@ -215,16 +210,12 @@ static int ds1307_set_time(struct device
break;
}
- ds1307->msg[1].flags = 0;
- ds1307->msg[1].len = 8;
-
dev_dbg(dev, "%s: %02x %02x %02x %02x %02x %02x %02x\n",
"write", buf[0], buf[1], buf[2], buf[3],
buf[4], buf[5], buf[6]);
- result = i2c_transfer(to_i2c_adapter(ds1307->client->dev.parent),
- &ds1307->msg[1], 1);
- if (result != 1) {
+ tmp = i2c_smbus_write_i2c_block_data(ds1307->client, 0, 7, buf);
+ if (tmp < 0) {
dev_err(dev, "%s error %d\n", "write", tmp);
return -EIO;
}
@@ -246,8 +237,7 @@ ds1307_nvram_read(struct kobject *kobj,
{
struct i2c_client *client;
struct ds1307 *ds1307;
- struct i2c_msg msg[2];
- int result;
+ int tmp;
client = kobj_to_i2c_client(kobj);
ds1307 = i2c_get_clientdata(client);
@@ -259,24 +249,13 @@ ds1307_nvram_read(struct kobject *kobj,
if (unlikely(!count))
return count;
- msg[0].addr = client->addr;
- msg[0].flags = 0;
- msg[0].len = 1;
- msg[0].buf = buf;
-
- buf[0] = 8 + off;
-
- msg[1].addr = client->addr;
- msg[1].flags = I2C_M_RD;
- msg[1].len = count;
- msg[1].buf = buf;
-
- result = i2c_transfer(to_i2c_adapter(client->dev.parent), msg, 2);
- if (result != 2) {
- dev_err(&client->dev, "%s error %d\n", "nvram read", result);
+ tmp = i2c_smbus_read_i2c_block_data(client, 8 + off, count, buf);
+ if (tmp < 0) {
+ dev_err(&client->dev, "%s error %d\n", "read", tmp);
return -EIO;
}
- return count;
+
+ return tmp;
}
static ssize_t
@@ -284,8 +263,8 @@ ds1307_nvram_write(struct kobject *kobj,
char *buf, loff_t off, size_t count)
{
struct i2c_client *client;
- u8 buffer[NVRAM_SIZE + 1];
- int ret;
+ u8 buffer[NVRAM_SIZE];
+ int tmp;
client = kobj_to_i2c_client(kobj);
@@ -296,11 +275,14 @@ ds1307_nvram_write(struct kobject *kobj,
if (unlikely(!count))
return count;
- buffer[0] = 8 + off;
- memcpy(buffer + 1, buf, count);
+ memcpy(buffer, buf, count);
- ret = i2c_master_send(client, buffer, count + 1);
- return (ret < 0) ? ret : (ret - 1);
+ tmp = i2c_smbus_write_i2c_block_data(client, 8 + off, count, buffer);
+ if (tmp < 0) {
+ dev_err(&client->dev, "%s error %d\n", "write", tmp);
+ return -EIO;
+ }
+ return count;
}
static struct bin_attribute nvram = {
@@ -325,11 +307,15 @@ static int __devinit ds1307_probe(struct
struct ds1307
Re: [i2c] [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
Thanks for your advices. Fixed patch is in attachement to avoid tabs replacement. Other comments are welcome. -- Sébastien Barré Bureau d'étude - Développement SDEL Contrôle Commande D2A - Rue Nungesser et Coli 44860 Saint Aignan de Grand Lieu FRANCE Tél : +33(0)2 40 84 50 88 Fax : +33(0)2 40 84 51 10 patch-rtc-ds1307 Description: patch-rtc-ds1307 ___ i2c mailing list i2c@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/i2c
Re: [i2c] [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
> -Original Message- > From: David Brownell [mailto:[EMAIL PROTECTED] ... > At a quick glance, this looks like a sane conversion ... unlike the > last > one proposed, which I had to NAK since it broke driver functionality by > trying to replace block transfers with non-equivalent byte-at-a-time > ones. > (Clock updates between bytes, boom!) Sorry, I don't understand what you talk about. The i2c_smbus_read_i2c_block_data function do block transfer not byte-at-a-time one. Doesn't it ? -- Sébastien Barré Bureau d'étude - Développement SDEL Contrôle Commande D2A - Rue Nungesser et Coli 44860 Saint Aignan de Grand Lieu FRANCE Tél : +33(0)2 40 84 50 88 Fax : +33(0)2 40 84 51 10 ___ i2c mailing list i2c@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/i2c
Re: [i2c] [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
Hi Jean,
> -Original Message-
> From: Jean Delvare [mailto:[EMAIL PROTECTED]
...
> Note: you need to include a comment describing what your patch does,
> as well as your Signed-off-by line. Here's a second review from me.
> After that it will be up to Alessandro and the RTC folks.
>
So I'm waiting for their comments.
> > --- a/drivers/rtc/rtc-ds1307.c2008-09-08 19:40:20.0
> +0200
> > +++ b/drivers/rtc/rtc-ds1307.c2008-10-10 16:30:59.0
> +0200
> > @@ -17,8 +17,6 @@
> > #include
> > #include
> >
> > -
> > -
>
> Unrelated white space change, please revert.
OK
> > /* We can't determine type by probing, but if we expect pre-Linux
> code
> > * to have set the chip up as a clock (turning on the oscillator and
> > * setting the date and time), Linux can ignore the non-clock
> features.
> > @@ -38,7 +36,6 @@ enum ds_type {
> > // rs5c372 too? different address...
> > };
> >
> > -
>
> Unrelated white space change, please revert.
OK
> > /* RTC registers don't differ much, except for the century flag */
> > #define DS1307_REG_SECS 0x00/* 00-59 */
> > #define DS1307_BIT_CH0x80
> > @@ -85,14 +82,11 @@ enum ds_type {
> > #define DS1337_BIT_A1I 0x01
> > #define DS1339_REG_TRICKLE 0x10
> >
> > -
> > -
>
> Unrelated white space change, please revert.
OK
> > struct ds1307 {
> > u8 reg_addr;
>
> reg_addr is unused after your changes, so you should remove it as well.
OK
> > boolhas_nvram;
> > u8 regs[8];
> > enum ds_typetype;
> > - struct i2c_msg msg[2];
> > struct i2c_client *client;
> > struct i2c_client dev;
> > struct rtc_device *rtc;
> > @@ -138,12 +132,9 @@ static int ds1307_get_time(struct device
> > int tmp;
> >
> > /* read the RTC date and time registers all at once */
> > - ds1307->msg[1].flags = I2C_M_RD;
> > - ds1307->msg[1].len = 7;
> > -
> > - tmp = i2c_transfer(to_i2c_adapter(ds1307->client->dev.parent),
> > - ds1307->msg, 2);
> > - if (tmp != 2) {
> > + tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
> > + DS1307_REG_SECS, 7, ds1307->regs);
> > + if (tmp != 7) {
> > dev_err(dev, "%s error %d\n", "read", tmp);
> > return -EIO;
> > }
> > @@ -181,8 +172,6 @@ static int ds1307_set_time(struct device {
> > struct ds1307 *ds1307 = dev_get_drvdata(dev);
> > int result;
> > - int tmp;
> > - u8 *buf = ds1307->regs;
> >
> > dev_dbg(dev, "%s secs=%d, mins=%d, "
> > "hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n", @@
> > -190,44 +179,41 @@ static int ds1307_set_time(struct device
> > t->tm_hour, t->tm_mday,
> > t->tm_mon, t->tm_year, t->tm_wday);
> >
> > - *buf++ = 0; /* first register addr */
> > - buf[DS1307_REG_SECS] = BIN2BCD(t->tm_sec);
> > - buf[DS1307_REG_MIN] = BIN2BCD(t->tm_min);
> > - buf[DS1307_REG_HOUR] = BIN2BCD(t->tm_hour);
> > - buf[DS1307_REG_WDAY] = BIN2BCD(t->tm_wday + 1);
> > - buf[DS1307_REG_MDAY] = BIN2BCD(t->tm_mday);
> > - buf[DS1307_REG_MONTH] = BIN2BCD(t->tm_mon + 1);
> > + ds1307->regs[DS1307_REG_SECS] = BIN2BCD(t->tm_sec);
> > + ds1307->regs[DS1307_REG_MIN] = BIN2BCD(t->tm_min);
> > + ds1307->regs[DS1307_REG_HOUR] = BIN2BCD(t->tm_hour);
> > + ds1307->regs[DS1307_REG_WDAY] = BIN2BCD(t->tm_wday + 1);
> > + ds1307->regs[DS1307_REG_MDAY] = BIN2BCD(t->tm_mday);
> > + ds1307->regs[DS1307_REG_MONTH] = BIN2BCD(t->tm_mon + 1);
>
> This change makes the patch larger (and thus harder to review) with
> almost no benefit. Same for many changes below... Why don't you keep
> the buf pointer? Please keep in mind that your patch should do just
> one thing and do it well.
It was to avoid the usage of buf, but I can reverse it if you think it's clearer
> > /* assume 20YY not 19YY */
> > - tmp = t->tm_year - 100;
> > - buf[DS1307_REG_YEAR] = BIN2BCD(tmp);
> > + ds1307->regs[DS1307_REG_YEAR] = BIN2BCD(t->tm_year - 100);
> >
> > switch (ds1307->type) {
> > case ds_1337:
> > case ds_1339:
> > - buf[DS1307_REG_MONTH] |= DS1337_BIT_CENTURY;
> > + ds1307->regs[DS1307_REG_MONTH] |= DS1337_BIT_CENTURY;
> > break;
> > case ds_1340:
> > - buf[DS1307_REG_HOUR] |= DS1340_BIT_CENTURY_EN
> > - | DS1340_BIT_CENTURY;
> > + ds1307->regs[DS1307_REG_HOUR] |= DS1340_BIT_CENTURY_EN
> > + | DS1340_BIT_CENTURY;
> > break;
> > default:
> > break;
> > }
> >
> > - ds1307->msg[1].flags = 0;
> > - ds1307->msg[1].len = 8;
> > -
> > dev_dbg(dev, "%s: %02x %02x
Re: [i2c] [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
> -Original Message- > From: Alessandro Zummo [mailto:[EMAIL PROTECTED] > Sent: Tuesday, October 14, 2008 3:22 PM > To: BARRE Sebastien > Cc: Jean Delvare; i2c@lm-sensors.org; frederic Rodo; David Brownell; > Rodolfo Giometti > Subject: Re: [i2c] [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility > > On Tue, 14 Oct 2008 15:15:10 +0200 > BARRE Sebastien <[EMAIL PROTECTED]> wrote: > > > Note: you need to include a comment describing what your patch does, > > > as well as your Signed-off-by line. Here's a second review from me. > > > After that it will be up to Alessandro and the RTC folks. > > > > > > > So I'm waiting for their comments. > > seems almost ok. please send your latest version to me and cc to > the rtc-list for the final review. if it passed Jean's tests > I'm confident it's quite ready for inclusion :) Patch is attached to preserve tabs. -- Sébastien Barré Bureau d'étude - Développement SDEL Contrôle Commande D2A - Rue Nungesser et Coli 44860 Saint Aignan de Grand Lieu FRANCE Tél : +33(0)2 40 84 50 88 Fax : +33(0)2 40 84 51 10 patch-rtc-ds1307 Description: patch-rtc-ds1307 ___ i2c mailing list i2c@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/i2c
Re: [i2c] [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
> -Original Message- > From: David Brownell [mailto:[EMAIL PROTECTED] > Sent: Tuesday, October 14, 2008 5:47 PM > To: Jean Delvare > Cc: BARRE Sebastien; i2c@lm-sensors.org; Alessandro Zummo; frederic Rodo; > Rodolfo Giometti > Subject: Re: [i2c] [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility > > On Tuesday 14 October 2008, Jean Delvare wrote: > > On Tue, 14 Oct 2008 15:15:10 +0200, BARRE Sebastien wrote: > > > New version is attached for tests and comments > > Could I see it too? Sorry David, you were in CC of this message but I received an error for your mail adress : >[<00>] XMail bounce: [EMAIL PROTECTED];Error=[553 5.3.0 flpi117 - >m9EFx6OS014455, DNSBL:521< 81.252.86.87 >>>_is_blocked.__For_information_see_http://att.net/blocks] > > >[<01>] Error sending message [122345380.3097504688.2a381e.smtp4] from >[vinci-energies.com]. > >ID: >Mail From: <[EMAIL PROTECTED]> >Rcpt To: <[EMAIL PROTECTED]> >Server: [207.115.21.23] > > >[<02>] The reason of the delivery failure was: > >553 5.3.0 flpi117 - m9EFx6OS014455, DNSBL:521< 81.252.86.87 >>_is_blocked.__For_information_see_http://att.net/blocks My mails are blocked by a server. Jean, Could you forward this message to David. Thanks. -- Sébastien Barré Bureau d'étude - Développement SDEL Contrôle Commande D2A - Rue Nungesser et Coli 44860 Saint Aignan de Grand Lieu FRANCE Tél : +33(0)2 40 84 50 88 Fax : +33(0)2 40 84 51 10 ___ i2c mailing list i2c@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/i2c
Re: [i2c] [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
This patch change i2c access functions to SMBus access functions
in order to use the ds1307 with SMBus adapter.
Signed-off-by: Sebastien Barre <[EMAIL PROTECTED]>
Acked-by: Jean Delvare <[EMAIL PROTECTED]>
--- a/drivers/rtc/rtc-ds1307.c 2008-09-08 19:40:20.0 +0200
+++ b/drivers/rtc/rtc-ds1307.c 2008-10-14 14:22:12.0 +0200
@@ -88,11 +88,9 @@ enum ds_type {
struct ds1307 {
- u8 reg_addr;
boolhas_nvram;
u8 regs[8];
enum ds_typetype;
- struct i2c_msg msg[2];
struct i2c_client *client;
struct i2c_client dev;
struct rtc_device *rtc;
@@ -138,12 +136,9 @@ static int ds1307_get_time(struct device
int tmp;
/* read the RTC date and time registers all at once */
- ds1307->msg[1].flags = I2C_M_RD;
- ds1307->msg[1].len = 7;
-
- tmp = i2c_transfer(to_i2c_adapter(ds1307->client->dev.parent),
- ds1307->msg, 2);
- if (tmp != 2) {
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ DS1307_REG_SECS, 7, ds1307->regs);
+ if (tmp != 7) {
dev_err(dev, "%s error %d\n", "read", tmp);
return -EIO;
}
@@ -190,7 +185,6 @@ static int ds1307_set_time(struct device
t->tm_hour, t->tm_mday,
t->tm_mon, t->tm_year, t->tm_wday);
- *buf++ = 0; /* first register addr */
buf[DS1307_REG_SECS] = BIN2BCD(t->tm_sec);
buf[DS1307_REG_MIN] = BIN2BCD(t->tm_min);
buf[DS1307_REG_HOUR] = BIN2BCD(t->tm_hour);
@@ -215,18 +209,14 @@ static int ds1307_set_time(struct device
break;
}
- ds1307->msg[1].flags = 0;
- ds1307->msg[1].len = 8;
-
dev_dbg(dev, "%s: %02x %02x %02x %02x %02x %02x %02x\n",
"write", buf[0], buf[1], buf[2], buf[3],
buf[4], buf[5], buf[6]);
- result = i2c_transfer(to_i2c_adapter(ds1307->client->dev.parent),
- &ds1307->msg[1], 1);
- if (result != 1) {
- dev_err(dev, "%s error %d\n", "write", tmp);
- return -EIO;
+ result = i2c_smbus_write_i2c_block_data(ds1307->client, 0, 7, buf);
+ if (result < 0) {
+ dev_err(dev, "%s error %d\n", "write", result);
+ return result;
}
return 0;
}
@@ -246,7 +236,6 @@ ds1307_nvram_read(struct kobject *kobj,
{
struct i2c_client *client;
struct ds1307 *ds1307;
- struct i2c_msg msg[2];
int result;
client = kobj_to_i2c_client(kobj);
@@ -259,24 +248,10 @@ ds1307_nvram_read(struct kobject *kobj,
if (unlikely(!count))
return count;
- msg[0].addr = client->addr;
- msg[0].flags = 0;
- msg[0].len = 1;
- msg[0].buf = buf;
-
- buf[0] = 8 + off;
-
- msg[1].addr = client->addr;
- msg[1].flags = I2C_M_RD;
- msg[1].len = count;
- msg[1].buf = buf;
-
- result = i2c_transfer(to_i2c_adapter(client->dev.parent), msg, 2);
- if (result != 2) {
+ result = i2c_smbus_read_i2c_block_data(client, 8 + off, count, buf);
+ if (result < 0)
dev_err(&client->dev, "%s error %d\n", "nvram read", result);
- return -EIO;
- }
- return count;
+ return result;
}
static ssize_t
@@ -284,8 +259,7 @@ ds1307_nvram_write(struct kobject *kobj,
char *buf, loff_t off, size_t count)
{
struct i2c_client *client;
- u8 buffer[NVRAM_SIZE + 1];
- int ret;
+ int result;
client = kobj_to_i2c_client(kobj);
@@ -296,11 +270,12 @@ ds1307_nvram_write(struct kobject *kobj,
if (unlikely(!count))
return count;
- buffer[0] = 8 + off;
- memcpy(buffer + 1, buf, count);
-
- ret = i2c_master_send(client, buffer, count + 1);
- return (ret < 0) ? ret : (ret - 1);
+ result = i2c_smbus_write_i2c_block_data(client, 8 + off, count, buf);
+ if (result < 0) {
+ dev_err(&client->dev, "%s error %d\n", "nvram write", result);
+ return result;
+ }
+ return count;
}
static struct bin_attribute nvram = {
@@ -325,11 +300,13 @@ static int __devinit ds1307_probe(struct
struct ds1307 *ds1307;
int err = -ENODEV;
int tmp;
+ u8 *buf;
const struct chip_desc *chip = &chips[id->driver_data];
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
if (!i2c_check_functionality(adapter,
- I2C_FUNC_I2C | I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+
[i2c] [PATCH] rtc-ds1307 : SMBus compatibility
This patch change i2c access functions to SMBus access functions in order to use the ds1307 with SMBus adapter. Signed-off-by: Sebastien Barre <[EMAIL PROTECTED]> -- The initial patch was based on 2.6.26.5, I've redone it against Linus's current git tree. Sorry, the patch is attached because my email client replace all the tabs with spaces. I've tested it with ds1307 chip. Is somebody can test it on ds1337/ds1338 ? -- Sébastien Barré Bureau d'étude - Développement SDEL Contrôle Commande D2A - Rue Nungesser et Coli 44860 Saint Aignan de Grand Lieu FRANCE Tél : +33(0)2 40 84 50 88 Fax : +33(0)2 40 84 51 10 patch-rtc-ds1307 Description: patch-rtc-ds1307 ___ i2c mailing list i2c@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/i2c
