Re: [i2c] [PATCH 2.6.26.5] rtc-ds1307 : SMBus compatibility
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.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;
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
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/2] i2c: MPC8349E-mITX Power Management and GPIO expander driver
On Sep 23, 2008, at 9:13 AM, Anton Vorontsov wrote: On MPC8349E-mITX, MPC8315E-RDB and MPC837x-RDB boards there is a Freescale MC9S08QG8 (MCU) chip with the custom firmware pre-programmed. The chip is used to power-off the board by the software, and to control some GPIO pins. Signed-off-by: Anton Vorontsov [EMAIL PROTECTED] --- drivers/i2c/chips/Kconfig| 11 ++ drivers/i2c/chips/Makefile |1 + drivers/i2c/chips/mcu_mpc8349emitx.c | 209 + + 3 files changed, 221 insertions(+), 0 deletions(-) create mode 100644 drivers/i2c/chips/mcu_mpc8349emitx.c is the plan to connect ppc_md.machine_shutdown() with this? - k ___ i2c mailing list i2c@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/i2c
Re: [i2c] [PATCH 1/2] OF: add fsl, mcu-mpc8349emitx to the exception list
On Sep 23, 2008, at 9:12 AM, Anton Vorontsov wrote: of/base.c matches on the first (most specific) entries, which isn't quite practical but it was discussed[1] that this won't change. The bindings specifies verbose information for the devices, but it doesn't fit in the I2C ID's 20 characters limit. The limit won't change[2], and the bindings won't change either as they're correct. So we have to put an exception for the MPC8349E-mITX-compatible MCUs. [1] http://www.mail-archive.com/[EMAIL PROTECTED]/msg21196.html [2] http://www.nabble.com/-PATCH-1-2--i2c:-expand-I2C's-id.name-to-23-characters-td19577063.html Signed-off-by: Anton Vorontsov [EMAIL PROTECTED] --- drivers/of/base.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) applied. - k ___ i2c mailing list i2c@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/i2c
Re: [i2c] [PATCH 2/2] i2c: MPC8349E-mITX Power Management and GPIO expander driver
On Fri, Oct 10, 2008 at 09:22:02AM -0500, Kumar Gala wrote:
On Sep 23, 2008, at 9:13 AM, Anton Vorontsov wrote:
On MPC8349E-mITX, MPC8315E-RDB and MPC837x-RDB boards there is a
Freescale MC9S08QG8 (MCU) chip with the custom firmware
pre-programmed. The chip is used to power-off the board by the
software, and to control some GPIO pins.
Signed-off-by: Anton Vorontsov [EMAIL PROTECTED]
---
drivers/i2c/chips/Kconfig| 11 ++
drivers/i2c/chips/Makefile |1 +
drivers/i2c/chips/mcu_mpc8349emitx.c | 209 +
+
3 files changed, 221 insertions(+), 0 deletions(-)
create mode 100644 drivers/i2c/chips/mcu_mpc8349emitx.c
is the plan to connect ppc_md.machine_shutdown() with this?
You mean poweroff? It's already connected.
+ /* XXX: this is potentially racy, but there is no lock for ppc_md */
+ if (!ppc_md.power_off) {
+ glob_mcu = mcu;
+ ppc_md.power_off = mcu_power_off;
+ dev_info(client-dev, will provide power-off service\n);
+ }
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
Re: [i2c] [PATCH 2/2] i2c: MPC8349E-mITX Power Management and GPIO expander driver
On Oct 10, 2008, at 9:45 AM, Anton Vorontsov wrote:
On Fri, Oct 10, 2008 at 09:22:02AM -0500, Kumar Gala wrote:
On Sep 23, 2008, at 9:13 AM, Anton Vorontsov wrote:
On MPC8349E-mITX, MPC8315E-RDB and MPC837x-RDB boards there is a
Freescale MC9S08QG8 (MCU) chip with the custom firmware
pre-programmed. The chip is used to power-off the board by the
software, and to control some GPIO pins.
Signed-off-by: Anton Vorontsov [EMAIL PROTECTED]
---
drivers/i2c/chips/Kconfig| 11 ++
drivers/i2c/chips/Makefile |1 +
drivers/i2c/chips/mcu_mpc8349emitx.c | 209 +++
++
+
3 files changed, 221 insertions(+), 0 deletions(-)
create mode 100644 drivers/i2c/chips/mcu_mpc8349emitx.c
is the plan to connect ppc_md.machine_shutdown() with this?
You mean poweroff? It's already connected.
+ /* XXX: this is potentially racy, but there is no lock for
ppc_md */
+ if (!ppc_md.power_off) {
+ glob_mcu = mcu;
+ ppc_md.power_off = mcu_power_off;
+ dev_info(client-dev, will provide power-off
service\n);
+ }
uuh, yeah.. poweroff :)
and look at that. Is this in Jean's queue?
- k
___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
Re: [i2c] [PATCH 2/2] i2c: MPC8349E-mITX Power Management and GPIO expander driver
On Fri, Oct 10, 2008 at 09:58:44AM -0500, Kumar Gala wrote:
[...]
is the plan to connect ppc_md.machine_shutdown() with this?
You mean poweroff? It's already connected.
+ /* XXX: this is potentially racy, but there is no lock for
ppc_md */
+ if (!ppc_md.power_off) {
+ glob_mcu = mcu;
+ ppc_md.power_off = mcu_power_off;
+ dev_info(client-dev, will provide power-off
service\n);
+ }
uuh, yeah.. poweroff :)
and look at that. Is this in Jean's queue?
Nope. Here's what Jean wrote:
On Wed, Oct 01, 2008 at 01:22:48PM +0200, Jean Delvare wrote:
I do not have the time to review these patches (and, honestly, have no
interest in them.) So I will not merge them but I have no objection to
them being merged by somebody else.
So.. I think it could easily go through the powerpc tree.
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
