* [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
@ 2008-10-09 6:59 BARRE Sebastien
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE59E27-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: BARRE Sebastien @ 2008-10-09 6:59 UTC (permalink / raw)
To: i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
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.000000000 +0000
+++ b/drivers/rtc/rtc-ds1307.c 2008-10-07 13:21:57.000000000 +0000
@@ -92,7 +92,6 @@ struct ds1307 {
bool has_nvram;
u8 regs[8];
enum ds_type type;
- 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 *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_FUNC_SMBUS_WRITE_I2C_BLOCK
+ | I2C_FUNC_SMBUS_READ_I2C_BLOCK))
return -EIO;
if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL)))
@@ -338,35 +324,21 @@ static int __devinit ds1307_probe(struct
ds1307->client = client;
i2c_set_clientdata(client, ds1307);
- ds1307->msg[0].addr = client->addr;
- ds1307->msg[0].flags = 0;
- ds1307->msg[0].len = 1;
- ds1307->msg[0].buf = &ds1307->reg_addr;
-
- ds1307->msg[1].addr = client->addr;
- ds1307->msg[1].flags = I2C_M_RD;
- ds1307->msg[1].len = sizeof(ds1307->regs);
- ds1307->msg[1].buf = ds1307->regs;
-
ds1307->type = id->driver_data;
switch (ds1307->type) {
case ds_1337:
case ds_1339:
- ds1307->reg_addr = DS1337_REG_CONTROL;
- ds1307->msg[1].len = 2;
-
+ buf = &ds1307->regs[DS1337_REG_CONTROL];
/* get registers that the "rtc" read below won't read... */
- tmp = i2c_transfer(adapter, ds1307->msg, 2);
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ DS1337_REG_CONTROL, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
err = -EIO;
goto exit_free;
}
- ds1307->reg_addr = 0;
- ds1307->msg[1].len = sizeof(ds1307->regs);
-
/* oscillator off? turn it on, so clock can tick. */
if (ds1307->regs[0] & DS1337_BIT_nEOSC)
i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
@@ -385,9 +357,9 @@ static int __devinit ds1307_probe(struct
read_rtc:
/* read RTC registers */
-
- tmp = i2c_transfer(adapter, ds1307->msg, 2);
- if (tmp != 2) {
+ buf = ds1307->regs;
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client, 0, 8, buf);
+ if (tmp != 8) {
pr_debug("read error %d\n", tmp);
err = -EIO;
goto exit_free;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE59E27-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
@ 2008-10-10 11:33 ` Jean Delvare
[not found] ` <20081010133344.653431af-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Jean Delvare @ 2008-10-10 11:33 UTC (permalink / raw)
To: BARRE Sebastien
Cc: Rodolfo-cy1Wll9GaHOsTnJN9+BGXg, Giometti, Alessandro Zummo,
David Brownell, i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org,
frederic Rodo
Bonjour Sebastien,
On Thu, 9 Oct 2008 08:59:17 +0200, BARRE Sebastien wrote:
> 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 ?
Yes, this is correct, and I consider this kind of conversion a good
thing.
Unfortunately your e-mail client replaced all tabs with spaces in the
patch, so I can't apply it, which makes reviewing it much harder.
Here's a first pass anyway, but please fix that and resend the patch in
such a format that I (and other developers) can apply it.
Note that while this patch deals with I2C, it affects an RTC driver so
you should send it to the RTC subsystem maintainer (Alessandro, Cc'd)
and mailing list. You might also want to get the last developers who
touched the rtc-ds1307 driver to test your patch. I've Cc'd them as
well.
>
> 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.000000000 +0000
> +++ b/drivers/rtc/rtc-ds1307.c 2008-10-07 13:21:57.000000000 +0000
> @@ -92,7 +92,6 @@ struct ds1307 {
> bool has_nvram;
> u8 regs[8];
> enum ds_type type;
> - 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;
Please keep all variable declarations at the beginning of the function.
Not sure you really need a variable for that anyway, as you use it only
once.
> + 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;
tmp is the worst variable name you can think of. Why not just keep
result?
>
> 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;
Ditto.
>
> 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);
As far as I can see, you no longer need this buffer at all. You can
simply pass buf to i2c_smbus_write_i2c_block_data() below.
>
> - 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;
Please do not hard-code error error values, return the values you
received from i2c_smbus_* functions instead.
> + }
> + return count;
> }
>
> static struct bin_attribute nvram = {
> @@ -325,11 +307,15 @@ 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_FUNC_SMBUS_WRITE_I2C_BLOCK
> + | I2C_FUNC_SMBUS_READ_I2C_BLOCK))
(I2C_FUNC_SMBUS_WRITE_I2C_BLOCK | I2C_FUNC_SMBUS_READ_I2C_BLOCK) has a
shorter form: I2C_FUNC_SMBUS_I2C_BLOCK.
> return -EIO;
>
> if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL)))
> @@ -338,35 +324,21 @@ static int __devinit ds1307_probe(struct
> ds1307->client = client;
> i2c_set_clientdata(client, ds1307);
>
> - ds1307->msg[0].addr = client->addr;
> - ds1307->msg[0].flags = 0;
> - ds1307->msg[0].len = 1;
> - ds1307->msg[0].buf = &ds1307->reg_addr;
> -
> - ds1307->msg[1].addr = client->addr;
> - ds1307->msg[1].flags = I2C_M_RD;
> - ds1307->msg[1].len = sizeof(ds1307->regs);
> - ds1307->msg[1].buf = ds1307->regs;
> -
> ds1307->type = id->driver_data;
>
> switch (ds1307->type) {
> case ds_1337:
> case ds_1339:
> - ds1307->reg_addr = DS1337_REG_CONTROL;
> - ds1307->msg[1].len = 2;
> -
> + buf = &ds1307->regs[DS1337_REG_CONTROL];
> /* get registers that the "rtc" read below won't read... */
> - tmp = i2c_transfer(adapter, ds1307->msg, 2);
> + tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
> + DS1337_REG_CONTROL, 2, buf);
> if (tmp != 2) {
> pr_debug("read error %d\n", tmp);
> err = -EIO;
> goto exit_free;
> }
>
> - ds1307->reg_addr = 0;
> - ds1307->msg[1].len = sizeof(ds1307->regs);
> -
> /* oscillator off? turn it on, so clock can tick. */
> if (ds1307->regs[0] & DS1337_BIT_nEOSC)
> i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
> @@ -385,9 +357,9 @@ static int __devinit ds1307_probe(struct
>
> read_rtc:
> /* read RTC registers */
> -
> - tmp = i2c_transfer(adapter, ds1307->msg, 2);
> - if (tmp != 2) {
> + buf = ds1307->regs;
> + tmp = i2c_smbus_read_i2c_block_data(ds1307->client, 0, 8, buf);
> + if (tmp != 8) {
> pr_debug("read error %d\n", tmp);
> err = -EIO;
> goto exit_free;
>
--
Jean Delvare
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <20081010133344.653431af-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
@ 2008-10-10 13:46 ` BARRE Sebastien
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE5A032-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
2008-10-12 4:50 ` David Brownell
1 sibling, 1 reply; 15+ messages in thread
From: BARRE Sebastien @ 2008-10-10 13:46 UTC (permalink / raw)
To: Jean Delvare
Cc: frederic Rodo, David Brownell, Rodolfo Giometti, Alessandro Zummo,
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
[-- Attachment #1: Type: text/plain, Size: 319 bytes --]
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
[-- Attachment #2: patch-rtc-ds1307 --]
[-- Type: application/octet-stream, Size: 7582 bytes --]
--- a/drivers/rtc/rtc-ds1307.c 2008-09-08 19:40:20.000000000 +0200
+++ b/drivers/rtc/rtc-ds1307.c 2008-10-10 16:30:59.000000000 +0200
@@ -17,8 +17,6 @@
#include <linux/rtc.h>
#include <linux/bcd.h>
-
-
/* 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...
};
-
/* RTC registers don't differ much, except for the century flag */
#define DS1307_REG_SECS 0x00 /* 00-59 */
# define DS1307_BIT_CH 0x80
@@ -85,14 +82,11 @@ enum ds_type {
# define DS1337_BIT_A1I 0x01
#define DS1339_REG_TRICKLE 0x10
-
-
struct ds1307 {
u8 reg_addr;
bool has_nvram;
u8 regs[8];
enum ds_type type;
- 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);
/* 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 %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;
+ "write", ds1307->regs[0], ds1307->regs[1],
+ ds1307->regs[2], ds1307->regs[3],
+ ds1307->regs[4], ds1307->regs[5], ds1307->regs[6]);
+
+ result = i2c_smbus_write_i2c_block_data(ds1307->client,
+ 0, 7, ds1307->regs);
+ if (result < 0) {
+ dev_err(dev, "%s error %d\n", "write", result);
+ return result;
}
+
return 0;
}
@@ -246,7 +232,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 +244,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);
- return -EIO;
+ result = i2c_smbus_read_i2c_block_data(client, 8 + off, count, buf);
+ if (result < 0) {
+ dev_err(&client->dev, "%s error %d\n", "read", result);
+ return result;
}
- return count;
+
+ return result;
}
static ssize_t
@@ -284,8 +258,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 +269,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", "write", result);
+ return result;
+ }
+ return count;
}
static struct bin_attribute nvram = {
@@ -325,11 +299,14 @@ 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_FUNC_SMBUS_I2C_BLOCK))
return -EIO;
if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL)))
@@ -338,35 +315,21 @@ static int __devinit ds1307_probe(struct
ds1307->client = client;
i2c_set_clientdata(client, ds1307);
- ds1307->msg[0].addr = client->addr;
- ds1307->msg[0].flags = 0;
- ds1307->msg[0].len = 1;
- ds1307->msg[0].buf = &ds1307->reg_addr;
-
- ds1307->msg[1].addr = client->addr;
- ds1307->msg[1].flags = I2C_M_RD;
- ds1307->msg[1].len = sizeof(ds1307->regs);
- ds1307->msg[1].buf = ds1307->regs;
-
ds1307->type = id->driver_data;
switch (ds1307->type) {
case ds_1337:
case ds_1339:
- ds1307->reg_addr = DS1337_REG_CONTROL;
- ds1307->msg[1].len = 2;
-
+ buf = &ds1307->regs[DS1337_REG_CONTROL];
/* get registers that the "rtc" read below won't read... */
- tmp = i2c_transfer(adapter, ds1307->msg, 2);
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ DS1337_REG_CONTROL, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
err = -EIO;
goto exit_free;
}
- ds1307->reg_addr = 0;
- ds1307->msg[1].len = sizeof(ds1307->regs);
-
/* oscillator off? turn it on, so clock can tick. */
if (ds1307->regs[0] & DS1337_BIT_nEOSC)
i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
@@ -385,9 +348,9 @@ static int __devinit ds1307_probe(struct
read_rtc:
/* read RTC registers */
-
- tmp = i2c_transfer(adapter, ds1307->msg, 2);
- if (tmp != 2) {
+ buf = ds1307->regs;
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client, 0, 8, buf);
+ if (tmp != 8) {
pr_debug("read error %d\n", tmp);
err = -EIO;
goto exit_free;
@@ -430,7 +393,7 @@ read_rtc:
tmp = i2c_smbus_read_byte_data(client, DS1340_REG_FLAG);
if (tmp < 0) {
pr_debug("read error %d\n", tmp);
- err = -EIO;
+ err = tmp;
goto exit_free;
}
[-- Attachment #3: Type: text/plain, Size: 157 bytes --]
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <20081010133344.653431af-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-10-10 13:46 ` BARRE Sebastien
@ 2008-10-12 4:50 ` David Brownell
[not found] ` <200810112150.21370.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
1 sibling, 1 reply; 15+ messages in thread
From: David Brownell @ 2008-10-12 4:50 UTC (permalink / raw)
To: Jean Delvare, BARRE Sebastien
Cc: frederic Rodo, Rodolfo Giometti, Alessandro Zummo,
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
On Friday 10 October 2008, Jean Delvare wrote:
> Unfortunately your e-mail client replaced all tabs with spaces in the
> patch, so I can't apply it, which makes reviewing it much harder.
> Here's a first pass anyway, but please fix that and resend the patch in
> such a format that I (and other developers) can apply it.
... and test it.
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!)
> Note that while this patch deals with I2C, it affects an RTC driver so
> you should send it to the RTC subsystem maintainer (Alessandro, Cc'd)
> and mailing list. You might also want to get the last developers who
> touched the rtc-ds1307 driver to test your patch. I've Cc'd them as
> well.
Right. As a rule, use GIT history to see who's maintaining code
when there's no MAINTAINERS entry.
- Dave
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <200810112150.21370.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
@ 2008-10-13 14:55 ` BARRE Sebastien
0 siblings, 0 replies; 15+ messages in thread
From: BARRE Sebastien @ 2008-10-13 14:55 UTC (permalink / raw)
To: David Brownell
Cc: frederic Rodo, Rodolfo Giometti, Alessandro Zummo,
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
> -----Original Message-----
> From: David Brownell [mailto:david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org]
...
> 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 ?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE5A032-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
@ 2008-10-13 15:32 ` Jean Delvare
[not found] ` <20081013173206.6b7a5e1e-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Jean Delvare @ 2008-10-13 15:32 UTC (permalink / raw)
To: BARRE Sebastien
Cc: frederic Rodo, David Brownell, Rodolfo Giometti, Alessandro Zummo,
i2c-GZX6beZjE8VD60Wz+7aTrA
Hi Sebastien,
On Fri, 10 Oct 2008 15:46:11 +0200, BARRE Sebastien wrote:
> Thanks for your advices.
> Fixed patch is in attachement to avoid tabs replacement.
> Other comments are welcome.
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.
> --- a/drivers/rtc/rtc-ds1307.c 2008-09-08 19:40:20.000000000 +0200
> +++ b/drivers/rtc/rtc-ds1307.c 2008-10-10 16:30:59.000000000 +0200
> @@ -17,8 +17,6 @@
> #include <linux/rtc.h>
> #include <linux/bcd.h>
>
> -
> -
Unrelated white space change, please revert.
> /* 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.
> /* RTC registers don't differ much, except for the century flag */
> #define DS1307_REG_SECS 0x00 /* 00-59 */
> # define DS1307_BIT_CH 0x80
> @@ -85,14 +82,11 @@ enum ds_type {
> # define DS1337_BIT_A1I 0x01
> #define DS1339_REG_TRICKLE 0x10
>
> -
> -
Unrelated white space change, please revert.
> struct ds1307 {
> u8 reg_addr;
reg_addr is unused after your changes, so you should remove it as well.
> bool has_nvram;
> u8 regs[8];
> enum ds_type type;
> - 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.
>
> /* 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 %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;
> + "write", ds1307->regs[0], ds1307->regs[1],
> + ds1307->regs[2], ds1307->regs[3],
> + ds1307->regs[4], ds1307->regs[5], ds1307->regs[6]);
> +
> + result = i2c_smbus_write_i2c_block_data(ds1307->client,
> + 0, 7, ds1307->regs);
> + if (result < 0) {
> + dev_err(dev, "%s error %d\n", "write", result);
> + return result;
> }
> +
> return 0;
> }
>
> @@ -246,7 +232,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 +244,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);
> - return -EIO;
> + result = i2c_smbus_read_i2c_block_data(client, 8 + off, count, buf);
> + if (result < 0) {
> + dev_err(&client->dev, "%s error %d\n", "read", result);
Why did you change this error message for a less specific one?
> + return result;
> }
> - return count;
> +
> + return result;
> }
Note that "return result" can be factored out.
>
> static ssize_t
> @@ -284,8 +258,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 +269,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", "write", result);
I'd use "nvram write" in the error message.
> + return result;
> + }
> + return count;
> }
>
> static struct bin_attribute nvram = {
> @@ -325,11 +299,14 @@ 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_FUNC_SMBUS_I2C_BLOCK))
> return -EIO;
>
> if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL)))
> @@ -338,35 +315,21 @@ static int __devinit ds1307_probe(struct
> ds1307->client = client;
> i2c_set_clientdata(client, ds1307);
>
> - ds1307->msg[0].addr = client->addr;
> - ds1307->msg[0].flags = 0;
> - ds1307->msg[0].len = 1;
> - ds1307->msg[0].buf = &ds1307->reg_addr;
> -
> - ds1307->msg[1].addr = client->addr;
> - ds1307->msg[1].flags = I2C_M_RD;
> - ds1307->msg[1].len = sizeof(ds1307->regs);
> - ds1307->msg[1].buf = ds1307->regs;
> -
> ds1307->type = id->driver_data;
>
> switch (ds1307->type) {
> case ds_1337:
> case ds_1339:
> - ds1307->reg_addr = DS1337_REG_CONTROL;
> - ds1307->msg[1].len = 2;
> -
> + buf = &ds1307->regs[DS1337_REG_CONTROL];
> /* get registers that the "rtc" read below won't read... */
> - tmp = i2c_transfer(adapter, ds1307->msg, 2);
> + tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
> + DS1337_REG_CONTROL, 2, buf);
> if (tmp != 2) {
> pr_debug("read error %d\n", tmp);
> err = -EIO;
> goto exit_free;
> }
>
> - ds1307->reg_addr = 0;
> - ds1307->msg[1].len = sizeof(ds1307->regs);
> -
> /* oscillator off? turn it on, so clock can tick. */
> if (ds1307->regs[0] & DS1337_BIT_nEOSC)
> i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
> @@ -385,9 +348,9 @@ static int __devinit ds1307_probe(struct
>
> read_rtc:
> /* read RTC registers */
> -
> - tmp = i2c_transfer(adapter, ds1307->msg, 2);
> - if (tmp != 2) {
> + buf = ds1307->regs;
> + tmp = i2c_smbus_read_i2c_block_data(ds1307->client, 0, 8, buf);
> + if (tmp != 8) {
> pr_debug("read error %d\n", tmp);
> err = -EIO;
> goto exit_free;
> @@ -430,7 +393,7 @@ read_rtc:
> tmp = i2c_smbus_read_byte_data(client, DS1340_REG_FLAG);
> if (tmp < 0) {
> pr_debug("read error %d\n", tmp);
> - err = -EIO;
> + err = tmp;
> goto exit_free;
> }
>
>
--
Jean Delvare
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <20081013173206.6b7a5e1e-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
@ 2008-10-14 13:15 ` BARRE Sebastien
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE5A2DA-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: BARRE Sebastien @ 2008-10-14 13:15 UTC (permalink / raw)
To: Jean Delvare
Cc: frederic Rodo, David Brownell, Rodolfo Giometti, Alessandro Zummo,
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
[-- Attachment #1: Type: text/plain, Size: 8863 bytes --]
Hi Jean,
> -----Original Message-----
> From: Jean Delvare [mailto:khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org]
...
> 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.c 2008-09-08 19:40:20.000000000
> +0200
> > +++ b/drivers/rtc/rtc-ds1307.c 2008-10-10 16:30:59.000000000
> +0200
> > @@ -17,8 +17,6 @@
> > #include <linux/rtc.h>
> > #include <linux/bcd.h>
> >
> > -
> > -
>
> 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_CH 0x80
> > @@ -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
> > bool has_nvram;
> > u8 regs[8];
> > enum ds_type type;
> > - 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 %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;
> > + "write", ds1307->regs[0], ds1307->regs[1],
> > + ds1307->regs[2], ds1307->regs[3],
> > + ds1307->regs[4], ds1307->regs[5], ds1307->regs[6]);
> > +
> > + result = i2c_smbus_write_i2c_block_data(ds1307->client,
> > + 0, 7, ds1307->regs);
> > + if (result < 0) {
> > + dev_err(dev, "%s error %d\n", "write", result);
> > + return result;
> > }
> > +
> > return 0;
> > }
> >
> > @@ -246,7 +232,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 +244,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);
> > - return -EIO;
> > + result = i2c_smbus_read_i2c_block_data(client, 8 + off, count,
> buf);
> > + if (result < 0) {
> > + dev_err(&client->dev, "%s error %d\n", "read",
> > + result);
>
> Why did you change this error message for a less specific one?
I think it's better to use the error value returned from i2c_smbus_read_i2c_block_data than to hard-code an error value.
> > + return result;
> > }
> > - return count;
> > +
> > + return result;
> > }
>
> Note that "return result" can be factored out.
OK
> >
> > static ssize_t
> > @@ -284,8 +258,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 +269,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", "write",
> result);
>
> I'd use "nvram write" in the error message.
You're right, it's better.
New version is attached for tests and comments
--
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
[-- Attachment #2: patch-rtc-ds1307 --]
[-- Type: application/octet-stream, Size: 5522 bytes --]
This patch change i2c access functions to SMBus access functions
in order to use the ds1307 with SMBus adapter.
Signed-off-by: Sebastien Barre <sbarre@sdelcc.com>
--- a/drivers/rtc/rtc-ds1307.c 2008-09-08 19:40:20.000000000 +0200
+++ b/drivers/rtc/rtc-ds1307.c 2008-10-14 14:22:12.000000000 +0200
@@ -88,11 +88,9 @@ enum ds_type {
struct ds1307 {
- u8 reg_addr;
bool has_nvram;
u8 regs[8];
enum ds_type type;
- 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_FUNC_SMBUS_I2C_BLOCK))
return -EIO;
if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL)))
@@ -338,35 +315,21 @@ static int __devinit ds1307_probe(struct
ds1307->client = client;
i2c_set_clientdata(client, ds1307);
- ds1307->msg[0].addr = client->addr;
- ds1307->msg[0].flags = 0;
- ds1307->msg[0].len = 1;
- ds1307->msg[0].buf = &ds1307->reg_addr;
-
- ds1307->msg[1].addr = client->addr;
- ds1307->msg[1].flags = I2C_M_RD;
- ds1307->msg[1].len = sizeof(ds1307->regs);
- ds1307->msg[1].buf = ds1307->regs;
-
ds1307->type = id->driver_data;
switch (ds1307->type) {
case ds_1337:
case ds_1339:
- ds1307->reg_addr = DS1337_REG_CONTROL;
- ds1307->msg[1].len = 2;
-
+ buf = &ds1307->regs[DS1337_REG_CONTROL];
/* get registers that the "rtc" read below won't read... */
- tmp = i2c_transfer(adapter, ds1307->msg, 2);
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ DS1337_REG_CONTROL, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
err = -EIO;
goto exit_free;
}
- ds1307->reg_addr = 0;
- ds1307->msg[1].len = sizeof(ds1307->regs);
-
/* oscillator off? turn it on, so clock can tick. */
if (ds1307->regs[0] & DS1337_BIT_nEOSC)
i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
@@ -385,9 +348,9 @@ static int __devinit ds1307_probe(struct
read_rtc:
/* read RTC registers */
-
- tmp = i2c_transfer(adapter, ds1307->msg, 2);
- if (tmp != 2) {
+ buf = ds1307->regs;
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client, 0, 8, buf);
+ if (tmp != 8) {
pr_debug("read error %d\n", tmp);
err = -EIO;
goto exit_free;
[-- Attachment #3: Type: text/plain, Size: 157 bytes --]
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE5A2DA-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
@ 2008-10-14 13:22 ` Alessandro Zummo
[not found] ` <20081014152202.4a88920a-1Z+4F8PJzVxlecnr3/Sf3RLwuzhV/fVh@public.gmane.org>
2008-10-14 13:29 ` Jean Delvare
1 sibling, 1 reply; 15+ messages in thread
From: Alessandro Zummo @ 2008-10-14 13:22 UTC (permalink / raw)
To: BARRE Sebastien
Cc: Rodolfo Giometti, David-cy1Wll9GaHOsTnJN9+BGXg, Brownell,
frederic Rodo, i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
On Tue, 14 Oct 2008 15:15:10 +0200
BARRE Sebastien <sbarre-6lXSvc0s5hDQT0dZR+AlfA@public.gmane.org> 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 :)
--
Best regards,
Alessandro Zummo,
Tower Technologies - Torino, Italy
http://www.towertech.it
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE5A2DA-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
2008-10-14 13:22 ` Alessandro Zummo
@ 2008-10-14 13:29 ` Jean Delvare
[not found] ` <20081014152928.6d91eefd-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
1 sibling, 1 reply; 15+ messages in thread
From: Jean Delvare @ 2008-10-14 13:29 UTC (permalink / raw)
To: BARRE Sebastien
Cc: Rodolfo-cy1Wll9GaHOsTnJN9+BGXg, Giometti, Alessandro Zummo,
David Brownell, i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org,
frederic Rodo
On Tue, 14 Oct 2008 15:15:10 +0200, BARRE Sebastien wrote:
> New version is attached for tests and comments
Looks alright to me.
Acked-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
--
Jean Delvare
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <20081014152928.6d91eefd-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
@ 2008-10-14 15:47 ` David Brownell
[not found] ` <200810140847.18839.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: David Brownell @ 2008-10-14 15:47 UTC (permalink / raw)
To: Jean Delvare
Cc: frederic Rodo, Rodolfo Giometti, Alessandro Zummo,
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org, BARRE Sebastien
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?
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <20081014152202.4a88920a-1Z+4F8PJzVxlecnr3/Sf3RLwuzhV/fVh@public.gmane.org>
@ 2008-10-14 15:57 ` BARRE Sebastien
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE5A317-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: BARRE Sebastien @ 2008-10-14 15:57 UTC (permalink / raw)
To: Alessandro Zummo
Cc: Rodolfo Giometti,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
David-cy1Wll9GaHOsTnJN9+BGXg, Brownell, frederic Rodo,
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]
> -----Original Message-----
> From: Alessandro Zummo [mailto:alessandro.zummo@towertech.it]
> 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 <sbarre@sdelcc.com> 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
[-- Attachment #2: patch-rtc-ds1307 --]
[-- Type: application/octet-stream, Size: 5565 bytes --]
This patch change i2c access functions to SMBus access functions
in order to use the ds1307 with SMBus adapter.
Signed-off-by: Sebastien Barre <sbarre@sdelcc.com>
Acked-by: Jean Delvare <khali@linux-fr.org>
--- a/drivers/rtc/rtc-ds1307.c 2008-09-08 19:40:20.000000000 +0200
+++ b/drivers/rtc/rtc-ds1307.c 2008-10-14 14:22:12.000000000 +0200
@@ -88,11 +88,9 @@ enum ds_type {
struct ds1307 {
- u8 reg_addr;
bool has_nvram;
u8 regs[8];
enum ds_type type;
- 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_FUNC_SMBUS_I2C_BLOCK))
return -EIO;
if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL)))
@@ -338,35 +315,21 @@ static int __devinit ds1307_probe(struct
ds1307->client = client;
i2c_set_clientdata(client, ds1307);
- ds1307->msg[0].addr = client->addr;
- ds1307->msg[0].flags = 0;
- ds1307->msg[0].len = 1;
- ds1307->msg[0].buf = &ds1307->reg_addr;
-
- ds1307->msg[1].addr = client->addr;
- ds1307->msg[1].flags = I2C_M_RD;
- ds1307->msg[1].len = sizeof(ds1307->regs);
- ds1307->msg[1].buf = ds1307->regs;
-
ds1307->type = id->driver_data;
switch (ds1307->type) {
case ds_1337:
case ds_1339:
- ds1307->reg_addr = DS1337_REG_CONTROL;
- ds1307->msg[1].len = 2;
-
+ buf = &ds1307->regs[DS1337_REG_CONTROL];
/* get registers that the "rtc" read below won't read... */
- tmp = i2c_transfer(adapter, ds1307->msg, 2);
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ DS1337_REG_CONTROL, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
err = -EIO;
goto exit_free;
}
- ds1307->reg_addr = 0;
- ds1307->msg[1].len = sizeof(ds1307->regs);
-
/* oscillator off? turn it on, so clock can tick. */
if (ds1307->regs[0] & DS1337_BIT_nEOSC)
i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
@@ -385,9 +348,9 @@ static int __devinit ds1307_probe(struct
read_rtc:
/* read RTC registers */
-
- tmp = i2c_transfer(adapter, ds1307->msg, 2);
- if (tmp != 2) {
+ buf = ds1307->regs;
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client, 0, 8, buf);
+ if (tmp != 8) {
pr_debug("read error %d\n", tmp);
err = -EIO;
goto exit_free;
[-- Attachment #3: Type: text/plain, Size: 157 bytes --]
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <200810140847.18839.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
@ 2008-10-14 16:24 ` BARRE Sebastien
0 siblings, 0 replies; 15+ messages in thread
From: BARRE Sebastien @ 2008-10-14 16:24 UTC (permalink / raw)
To: David Brownell, Jean Delvare; +Cc: i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
> -----Original Message-----
> From: David Brownell [mailto:david-b@pacbell.net]
> 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: Rcpt=[david-b@pacbell.net];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 [1223999945380.3097504688.2a381e.smtp4] from [vinci-energies.com].
>
>ID: <SCDCBC5>
>Mail From: <sbarre@sdelcc.com>
>Rcpt To: <david-b@pacbell.net>
>Server: <ff-mx-vip4b.prodigy.net> [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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE5A317-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
@ 2008-10-14 16:40 ` Alessandro Zummo
[not found] ` <20081014184038.6f1b5c59-1Z+4F8PJzVxlecnr3/Sf3RLwuzhV/fVh@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Alessandro Zummo @ 2008-10-14 16:40 UTC (permalink / raw)
To: BARRE Sebastien
Cc: Rodolfo Giometti,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
David-cy1Wll9GaHOsTnJN9+BGXg, Brownell, frederic Rodo,
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
On Tue, 14 Oct 2008 17:57:29 +0200
BARRE Sebastien <sbarre-6lXSvc0s5hDQT0dZR+AlfA@public.gmane.org> wrote:
> > 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.
bzzzt. wrong! patches must be submitted inline
--
Best regards,
Alessandro Zummo,
Tower Technologies - Torino, Italy
http://www.towertech.it
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <20081014184038.6f1b5c59-1Z+4F8PJzVxlecnr3/Sf3RLwuzhV/fVh@public.gmane.org>
@ 2008-10-15 8:34 ` BARRE Sebastien
2008-10-16 18:06 ` David Brownell
1 sibling, 0 replies; 15+ messages in thread
From: BARRE Sebastien @ 2008-10-15 8:34 UTC (permalink / raw)
To: Alessandro Zummo
Cc: Rodolfo Giometti,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
David-cy1Wll9GaHOsTnJN9+BGXg, Brownell, frederic Rodo,
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
This patch change i2c access functions to SMBus access functions
in order to use the ds1307 with SMBus adapter.
Signed-off-by: Sebastien Barre <sbarre@sdelcc.com>
Acked-by: Jean Delvare <khali@linux-fr.org>
--- a/drivers/rtc/rtc-ds1307.c 2008-09-08 19:40:20.000000000 +0200
+++ b/drivers/rtc/rtc-ds1307.c 2008-10-14 14:22:12.000000000 +0200
@@ -88,11 +88,9 @@ enum ds_type {
struct ds1307 {
- u8 reg_addr;
bool has_nvram;
u8 regs[8];
enum ds_type type;
- 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_FUNC_SMBUS_I2C_BLOCK))
return -EIO;
if (!(ds1307 = kzalloc(sizeof(struct ds1307), GFP_KERNEL)))
@@ -338,35 +315,21 @@ static int __devinit ds1307_probe(struct
ds1307->client = client;
i2c_set_clientdata(client, ds1307);
- ds1307->msg[0].addr = client->addr;
- ds1307->msg[0].flags = 0;
- ds1307->msg[0].len = 1;
- ds1307->msg[0].buf = &ds1307->reg_addr;
-
- ds1307->msg[1].addr = client->addr;
- ds1307->msg[1].flags = I2C_M_RD;
- ds1307->msg[1].len = sizeof(ds1307->regs);
- ds1307->msg[1].buf = ds1307->regs;
-
ds1307->type = id->driver_data;
switch (ds1307->type) {
case ds_1337:
case ds_1339:
- ds1307->reg_addr = DS1337_REG_CONTROL;
- ds1307->msg[1].len = 2;
-
+ buf = &ds1307->regs[DS1337_REG_CONTROL];
/* get registers that the "rtc" read below won't read... */
- tmp = i2c_transfer(adapter, ds1307->msg, 2);
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
+ DS1337_REG_CONTROL, 2, buf);
if (tmp != 2) {
pr_debug("read error %d\n", tmp);
err = -EIO;
goto exit_free;
}
- ds1307->reg_addr = 0;
- ds1307->msg[1].len = sizeof(ds1307->regs);
-
/* oscillator off? turn it on, so clock can tick. */
if (ds1307->regs[0] & DS1337_BIT_nEOSC)
i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
@@ -385,9 +348,9 @@ static int __devinit ds1307_probe(struct
read_rtc:
/* read RTC registers */
-
- tmp = i2c_transfer(adapter, ds1307->msg, 2);
- if (tmp != 2) {
+ buf = ds1307->regs;
+ tmp = i2c_smbus_read_i2c_block_data(ds1307->client, 0, 8, buf);
+ if (tmp != 8) {
pr_debug("read error %d\n", tmp);
err = -EIO;
goto exit_free;
--
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
[not found] ` <20081014184038.6f1b5c59-1Z+4F8PJzVxlecnr3/Sf3RLwuzhV/fVh@public.gmane.org>
2008-10-15 8:34 ` BARRE Sebastien
@ 2008-10-16 18:06 ` David Brownell
1 sibling, 0 replies; 15+ messages in thread
From: David Brownell @ 2008-10-16 18:06 UTC (permalink / raw)
To: Alessandro Zummo, BARRE Sebastien
Cc: frederic Rodo, Rodolfo Giometti,
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
On Tuesday 14 October 2008, Alessandro Zummo wrote:
> On Tue, 14 Oct 2008 17:57:29 +0200
> BARRE Sebastien <sbarre-6lXSvc0s5hDQT0dZR+AlfA@public.gmane.org> wrote:
>
> > > 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.
>
> bzzzt. wrong! patches must be submitted inline
Though that's not a hard-and-fast policy; when
the submitter can't get relevant mailers to avoid
mangling the patches, it's better to attach them
than otherwise. However, see
Documentation/email-clients.txt
More generally, this patch isn't much good since
it doesn't even *apply* ... on top of a pending
patch to the same driver, which has been in the
MM tree since late July or so.
My suggestion: since that patch was recently
(last night) sent to Linus for merging, wait
until it's in mainline GIT. Then refresh and
re-send the patch.
- Dave
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2008-10-16 18:06 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-09 6:59 [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility BARRE Sebastien
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE59E27-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
2008-10-10 11:33 ` Jean Delvare
[not found] ` <20081010133344.653431af-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-10-10 13:46 ` BARRE Sebastien
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE5A032-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
2008-10-13 15:32 ` Jean Delvare
[not found] ` <20081013173206.6b7a5e1e-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-10-14 13:15 ` BARRE Sebastien
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE5A2DA-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
2008-10-14 13:22 ` Alessandro Zummo
[not found] ` <20081014152202.4a88920a-1Z+4F8PJzVxlecnr3/Sf3RLwuzhV/fVh@public.gmane.org>
2008-10-14 15:57 ` BARRE Sebastien
[not found] ` <C1C62166118DFA4A8BBA01A2411F0D170E9BE5A317-iGzfJtNnmHkUlixSki3fykIJAyBIbWHDnAJcWxkt5PDR7s880joybQ@public.gmane.org>
2008-10-14 16:40 ` Alessandro Zummo
[not found] ` <20081014184038.6f1b5c59-1Z+4F8PJzVxlecnr3/Sf3RLwuzhV/fVh@public.gmane.org>
2008-10-15 8:34 ` BARRE Sebastien
2008-10-16 18:06 ` David Brownell
2008-10-14 13:29 ` Jean Delvare
[not found] ` <20081014152928.6d91eefd-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-10-14 15:47 ` David Brownell
[not found] ` <200810140847.18839.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-10-14 16:24 ` BARRE Sebastien
2008-10-12 4:50 ` David Brownell
[not found] ` <200810112150.21370.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-10-13 14:55 ` BARRE Sebastien
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox