* [patch 2.6.25-git] i2c-core: return -Errno, not -1
@ 2008-05-03 20:06 David Brownell
[not found] ` <200805031306.16698.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: David Brownell @ 2008-05-03 20:06 UTC (permalink / raw)
To: i2c-GZX6beZjE8VD60Wz+7aTrA
More updates to the I2C stack's fault reporting: make the core stop
returning "-1" (usually "-EPERM") for all faults. Instead, pass lower
level fault code up the stack, or return some appropriate errno.
This patch happens to touch almost exclusively SMBus calls.
Signed-off-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
---
Combine this and the previous i2c_adapter patch, and I suspect that
well over half the causes of bogus "-EPERM" reports are now gone ...
including in particular those reported by i2c-dev to userspace when
it issues SMBus calls "for portability".
drivers/i2c/i2c-core.c | 76 +++++++++++++++++++++++++++----------------------
1 file changed, 43 insertions(+), 33 deletions(-)
--- g26.orig/drivers/i2c/i2c-core.c 2008-05-03 12:57:16.000000000 -0700
+++ g26/drivers/i2c/i2c-core.c 2008-05-03 12:58:10.000000000 -0700
@@ -1113,7 +1113,7 @@ int i2c_probe(struct i2c_adapter *adapte
dev_warn(&adapter->dev, "SMBus Quick command not supported, "
"can't probe for chips\n");
- return -1;
+ return -EOPNOTSUPP;
}
/* Probe entries are done second, and are not affected by ignore
@@ -1305,7 +1305,7 @@ static int i2c_smbus_check_pec(u8 cpec,
if (rpec != cpec) {
pr_debug("i2c-core: Bad PEC 0x%02x vs. 0x%02x\n",
rpec, cpec);
- return -1;
+ return -EPROTO;
}
return 0;
}
@@ -1320,11 +1320,12 @@ EXPORT_SYMBOL(i2c_smbus_write_quick);
s32 i2c_smbus_read_byte(struct i2c_client *client)
{
union i2c_smbus_data data;
- if (i2c_smbus_xfer(client->adapter,client->addr,client->flags,
- I2C_SMBUS_READ,0,I2C_SMBUS_BYTE, &data))
- return -1;
- else
- return data.byte;
+ int status;
+
+ status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+ I2C_SMBUS_READ, 0,
+ I2C_SMBUS_BYTE, &data);
+ return (status < 0) ? status : data.byte;
}
EXPORT_SYMBOL(i2c_smbus_read_byte);
@@ -1338,11 +1339,12 @@ EXPORT_SYMBOL(i2c_smbus_write_byte);
s32 i2c_smbus_read_byte_data(struct i2c_client *client, u8 command)
{
union i2c_smbus_data data;
- if (i2c_smbus_xfer(client->adapter,client->addr,client->flags,
- I2C_SMBUS_READ,command, I2C_SMBUS_BYTE_DATA,&data))
- return -1;
- else
- return data.byte;
+ int status;
+
+ status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+ I2C_SMBUS_READ, command,
+ I2C_SMBUS_BYTE_DATA, &data);
+ return (status < 0) ? status : data.byte;
}
EXPORT_SYMBOL(i2c_smbus_read_byte_data);
@@ -1359,11 +1361,12 @@ EXPORT_SYMBOL(i2c_smbus_write_byte_data)
s32 i2c_smbus_read_word_data(struct i2c_client *client, u8 command)
{
union i2c_smbus_data data;
- if (i2c_smbus_xfer(client->adapter,client->addr,client->flags,
- I2C_SMBUS_READ,command, I2C_SMBUS_WORD_DATA, &data))
- return -1;
- else
- return data.word;
+ int status;
+
+ status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+ I2C_SMBUS_READ, command,
+ I2C_SMBUS_WORD_DATA, &data);
+ return (status < 0) ? status : data.word;
}
EXPORT_SYMBOL(i2c_smbus_read_word_data);
@@ -1397,11 +1400,13 @@ s32 i2c_smbus_read_block_data(struct i2c
u8 *values)
{
union i2c_smbus_data data;
+ int status;
- if (i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_READ, command,
- I2C_SMBUS_BLOCK_DATA, &data))
- return -1;
+ status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+ I2C_SMBUS_READ, command,
+ I2C_SMBUS_BLOCK_DATA, &data);
+ if (status)
+ return status;
memcpy(values, &data.block[1], data.block[0]);
return data.block[0];
@@ -1428,14 +1433,16 @@ s32 i2c_smbus_read_i2c_block_data(struct
u8 length, u8 *values)
{
union i2c_smbus_data data;
+ int status;
if (length > I2C_SMBUS_BLOCK_MAX)
length = I2C_SMBUS_BLOCK_MAX;
data.block[0] = length;
- if (i2c_smbus_xfer(client->adapter,client->addr,client->flags,
- I2C_SMBUS_READ,command,
- I2C_SMBUS_I2C_BLOCK_DATA,&data))
- return -1;
+ status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+ I2C_SMBUS_READ, command,
+ I2C_SMBUS_I2C_BLOCK_DATA, &data);
+ if (status < 0)
+ return status;
memcpy(values, &data.block[1], data.block[0]);
return data.block[0];
@@ -1476,6 +1483,7 @@ static s32 i2c_smbus_xfer_emulated(struc
};
int i;
u8 partial_pec = 0;
+ int status;
msgbuf0[0] = command;
switch(size) {
@@ -1528,7 +1536,7 @@ static s32 i2c_smbus_xfer_emulated(struc
dev_err(&adapter->dev, "smbus_access called with "
"invalid block write size (%d)\n",
data->block[0]);
- return -1;
+ return -EMSGSIZE;
}
for (i = 1; i < msg[0].len; i++)
msgbuf0[i] = data->block[i-1];
@@ -1541,7 +1549,7 @@ static s32 i2c_smbus_xfer_emulated(struc
dev_err(&adapter->dev, "%s called with invalid "
"block proc call size (%d)\n", __func__,
data->block[0]);
- return -1;
+ return -EMSGSIZE;
}
msg[0].len = data->block[0] + 2;
for (i = 1; i < msg[0].len; i++)
@@ -1559,7 +1567,7 @@ static s32 i2c_smbus_xfer_emulated(struc
dev_err(&adapter->dev, "i2c_smbus_xfer_emulated called with "
"invalid block write size (%d)\n",
data->block[0]);
- return -1;
+ return -EMSGSIZE;
}
for (i = 1; i <= data->block[0]; i++)
msgbuf0[i] = data->block[i];
@@ -1568,7 +1576,7 @@ static s32 i2c_smbus_xfer_emulated(struc
default:
dev_err(&adapter->dev, "smbus_access called with invalid size (%d)\n",
size);
- return -1;
+ return -EOPNOTSUPP;
}
i = ((flags & I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK
@@ -1586,13 +1594,15 @@ static s32 i2c_smbus_xfer_emulated(struc
msg[num-1].len++;
}
- if (i2c_transfer(adapter, msg, num) < 0)
- return -1;
+ status = i2c_transfer(adapter, msg, num);
+ if (status < 0)
+ return status;
/* Check PEC if last message is a read */
if (i && (msg[num-1].flags & I2C_M_RD)) {
- if (i2c_smbus_check_pec(partial_pec, &msg[num-1]) < 0)
- return -1;
+ status = i2c_smbus_check_pec(partial_pec, &msg[num-1]);
+ if (status < 0)
+ return status;
}
if (read_write == I2C_SMBUS_READ)
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch 2.6.26-rc1-git] i2c-core: return -Errno, not -1
[not found] ` <200805031306.16698.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
@ 2008-05-11 22:16 ` David Brownell
[not found] ` <200805111516.08453.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: David Brownell @ 2008-05-11 22:16 UTC (permalink / raw)
To: i2c-GZX6beZjE8VD60Wz+7aTrA
More updates to the I2C stack's fault reporting: make the core stop
returning "-1" (usually "-EPERM") for all faults. Instead, pass lower
level fault code up the stack, or return some appropriate errno.
This patch happens to touch almost exclusively SMBus calls.
Signed-off-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
---
This is an updated version of:
http://marc.info/?l=i2c&m=120984528415259&w=2
with some return codes updated to address feedback from the similar
patch for the x86 I2C/SMBus adapters.
drivers/i2c/i2c-core.c | 78 +++++++++++++++++++++++++++----------------------
1 file changed, 44 insertions(+), 34 deletions(-)
--- g26.orig/drivers/i2c/i2c-core.c 2008-05-11 13:07:56.000000000 -0700
+++ g26/drivers/i2c/i2c-core.c 2008-05-11 15:09:00.000000000 -0700
@@ -981,7 +981,7 @@ int i2c_transfer(struct i2c_adapter * ad
return ret;
} else {
dev_dbg(&adap->dev, "I2C level transfers not supported\n");
- return -ENOSYS;
+ return -EOPNOTSUPP;
}
}
EXPORT_SYMBOL(i2c_transfer);
@@ -1113,7 +1113,7 @@ int i2c_probe(struct i2c_adapter *adapte
dev_warn(&adapter->dev, "SMBus Quick command not supported, "
"can't probe for chips\n");
- return -1;
+ return -EOPNOTSUPP;
}
/* Probe entries are done second, and are not affected by ignore
@@ -1305,7 +1305,7 @@ static int i2c_smbus_check_pec(u8 cpec,
if (rpec != cpec) {
pr_debug("i2c-core: Bad PEC 0x%02x vs. 0x%02x\n",
rpec, cpec);
- return -1;
+ return -EBADMSG;
}
return 0;
}
@@ -1320,11 +1320,12 @@ EXPORT_SYMBOL(i2c_smbus_write_quick);
s32 i2c_smbus_read_byte(struct i2c_client *client)
{
union i2c_smbus_data data;
- if (i2c_smbus_xfer(client->adapter,client->addr,client->flags,
- I2C_SMBUS_READ,0,I2C_SMBUS_BYTE, &data))
- return -1;
- else
- return data.byte;
+ int status;
+
+ status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+ I2C_SMBUS_READ, 0,
+ I2C_SMBUS_BYTE, &data);
+ return (status < 0) ? status : data.byte;
}
EXPORT_SYMBOL(i2c_smbus_read_byte);
@@ -1338,11 +1339,12 @@ EXPORT_SYMBOL(i2c_smbus_write_byte);
s32 i2c_smbus_read_byte_data(struct i2c_client *client, u8 command)
{
union i2c_smbus_data data;
- if (i2c_smbus_xfer(client->adapter,client->addr,client->flags,
- I2C_SMBUS_READ,command, I2C_SMBUS_BYTE_DATA,&data))
- return -1;
- else
- return data.byte;
+ int status;
+
+ status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+ I2C_SMBUS_READ, command,
+ I2C_SMBUS_BYTE_DATA, &data);
+ return (status < 0) ? status : data.byte;
}
EXPORT_SYMBOL(i2c_smbus_read_byte_data);
@@ -1359,11 +1361,12 @@ EXPORT_SYMBOL(i2c_smbus_write_byte_data)
s32 i2c_smbus_read_word_data(struct i2c_client *client, u8 command)
{
union i2c_smbus_data data;
- if (i2c_smbus_xfer(client->adapter,client->addr,client->flags,
- I2C_SMBUS_READ,command, I2C_SMBUS_WORD_DATA, &data))
- return -1;
- else
- return data.word;
+ int status;
+
+ status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+ I2C_SMBUS_READ, command,
+ I2C_SMBUS_WORD_DATA, &data);
+ return (status < 0) ? status : data.word;
}
EXPORT_SYMBOL(i2c_smbus_read_word_data);
@@ -1397,11 +1400,13 @@ s32 i2c_smbus_read_block_data(struct i2c
u8 *values)
{
union i2c_smbus_data data;
+ int status;
- if (i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_READ, command,
- I2C_SMBUS_BLOCK_DATA, &data))
- return -1;
+ status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+ I2C_SMBUS_READ, command,
+ I2C_SMBUS_BLOCK_DATA, &data);
+ if (status)
+ return status;
memcpy(values, &data.block[1], data.block[0]);
return data.block[0];
@@ -1428,14 +1433,16 @@ s32 i2c_smbus_read_i2c_block_data(struct
u8 length, u8 *values)
{
union i2c_smbus_data data;
+ int status;
if (length > I2C_SMBUS_BLOCK_MAX)
length = I2C_SMBUS_BLOCK_MAX;
data.block[0] = length;
- if (i2c_smbus_xfer(client->adapter,client->addr,client->flags,
- I2C_SMBUS_READ,command,
- I2C_SMBUS_I2C_BLOCK_DATA,&data))
- return -1;
+ status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+ I2C_SMBUS_READ, command,
+ I2C_SMBUS_I2C_BLOCK_DATA, &data);
+ if (status < 0)
+ return status;
memcpy(values, &data.block[1], data.block[0]);
return data.block[0];
@@ -1476,6 +1483,7 @@ static s32 i2c_smbus_xfer_emulated(struc
};
int i;
u8 partial_pec = 0;
+ int status;
msgbuf0[0] = command;
switch(size) {
@@ -1528,7 +1536,7 @@ static s32 i2c_smbus_xfer_emulated(struc
dev_err(&adapter->dev, "smbus_access called with "
"invalid block write size (%d)\n",
data->block[0]);
- return -1;
+ return -EINVAL;
}
for (i = 1; i < msg[0].len; i++)
msgbuf0[i] = data->block[i-1];
@@ -1541,7 +1549,7 @@ static s32 i2c_smbus_xfer_emulated(struc
dev_err(&adapter->dev, "%s called with invalid "
"block proc call size (%d)\n", __func__,
data->block[0]);
- return -1;
+ return -EINVAL;
}
msg[0].len = data->block[0] + 2;
for (i = 1; i < msg[0].len; i++)
@@ -1559,7 +1567,7 @@ static s32 i2c_smbus_xfer_emulated(struc
dev_err(&adapter->dev, "i2c_smbus_xfer_emulated called with "
"invalid block write size (%d)\n",
data->block[0]);
- return -1;
+ return -EINVAL;
}
for (i = 1; i <= data->block[0]; i++)
msgbuf0[i] = data->block[i];
@@ -1568,7 +1576,7 @@ static s32 i2c_smbus_xfer_emulated(struc
default:
dev_err(&adapter->dev, "smbus_access called with invalid size (%d)\n",
size);
- return -1;
+ return -EOPNOTSUPP;
}
i = ((flags & I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK
@@ -1586,13 +1594,15 @@ static s32 i2c_smbus_xfer_emulated(struc
msg[num-1].len++;
}
- if (i2c_transfer(adapter, msg, num) < 0)
- return -1;
+ status = i2c_transfer(adapter, msg, num);
+ if (status < 0)
+ return status;
/* Check PEC if last message is a read */
if (i && (msg[num-1].flags & I2C_M_RD)) {
- if (i2c_smbus_check_pec(partial_pec, &msg[num-1]) < 0)
- return -1;
+ status = i2c_smbus_check_pec(partial_pec, &msg[num-1]);
+ if (status < 0)
+ return status;
}
if (read_write == I2C_SMBUS_READ)
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2.6.26-rc1-git] i2c-core: return -Errno, not -1
[not found] ` <200805111516.08453.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
@ 2008-05-16 7:14 ` Jean Delvare
[not found] ` <20080516091458.28f9f5ca-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Jean Delvare @ 2008-05-16 7:14 UTC (permalink / raw)
To: David Brownell; +Cc: i2c-GZX6beZjE8VD60Wz+7aTrA
Hi David,
On Sun, 11 May 2008 15:16:07 -0700, David Brownell wrote:
> More updates to the I2C stack's fault reporting: make the core stop
> returning "-1" (usually "-EPERM") for all faults. Instead, pass lower
> level fault code up the stack, or return some appropriate errno.
>
> This patch happens to touch almost exclusively SMBus calls.
>
> Signed-off-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> ---
> This is an updated version of:
> http://marc.info/?l=i2c&m=120984528415259&w=2
> with some return codes updated to address feedback from the similar
> patch for the x86 I2C/SMBus adapters.
>
> drivers/i2c/i2c-core.c | 78 +++++++++++++++++++++++++++----------------------
> 1 file changed, 44 insertions(+), 34 deletions(-)
>
Looks very good, with just one suggested change:
> --- g26.orig/drivers/i2c/i2c-core.c 2008-05-11 13:07:56.000000000 -0700
> +++ g26/drivers/i2c/i2c-core.c 2008-05-11 15:09:00.000000000 -0700
> (...)
> @@ -1568,7 +1576,7 @@ static s32 i2c_smbus_xfer_emulated(struc
> default:
> dev_err(&adapter->dev, "smbus_access called with invalid size (%d)\n",
> size);
> - return -1;
> + return -EOPNOTSUPP;
> }
I'd rather use -EINVAL here. This case is a bit different from what we
did in the bus drivers, because here the function handles all
transaction types that exist. So if we reach the default case, it means
that the caller passed an invalid parameter. -EOPNOTSUPP is not
fundamentally incorrect but it's a little less precise.
In theory, in bus drivers we should separate between invalid
transaction types and unsupported transaction types, returning -EINVAL
for the former and -EOPNOTSUPP only for the latter. We do not simply
because it makes little sense to make the drivers bigger just to report
finer-grained error codes, especially when these error paths are almost
never reached, and also because it's convenient to be able to grep for
I2C_SMBUS_* transaction type constants to find out which drivers
support a given transaction type.
BTW, feel free to adjust the debugging message above, as you did in
some bus drivers already, to clearly say that we're speaking about a
transaction type and not a "size".
All the rest looks OK to me from a functional point of view. As far as
style is concerned, please keep the alignment on opening parenthesis
when the original code did that. i2c-core uses this style consistently
and I like it.
Thanks,
--
Jean Delvare
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2.6.26-rc1-git] i2c-core: return -Errno, not -1
[not found] ` <20080516091458.28f9f5ca-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
@ 2008-05-17 6:15 ` Jean Delvare
[not found] ` <20080517081537.17220cd0-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Jean Delvare @ 2008-05-17 6:15 UTC (permalink / raw)
To: David Brownell; +Cc: i2c-GZX6beZjE8VD60Wz+7aTrA
On Fri, 16 May 2008 09:14:58 +0200, Jean Delvare wrote:
> Hi David,
>
> On Sun, 11 May 2008 15:16:07 -0700, David Brownell wrote:
> > More updates to the I2C stack's fault reporting: make the core stop
> > returning "-1" (usually "-EPERM") for all faults. Instead, pass lower
> > level fault code up the stack, or return some appropriate errno.
> >
> > This patch happens to touch almost exclusively SMBus calls.
> >
> > Signed-off-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> > ---
> > This is an updated version of:
> > http://marc.info/?l=i2c&m=120984528415259&w=2
> > with some return codes updated to address feedback from the similar
> > patch for the x86 I2C/SMBus adapters.
> >
> > drivers/i2c/i2c-core.c | 78 +++++++++++++++++++++++++++----------------------
> > 1 file changed, 44 insertions(+), 34 deletions(-)
> >
>
> Looks very good, with just one suggested change:
>
> > --- g26.orig/drivers/i2c/i2c-core.c 2008-05-11 13:07:56.000000000 -0700
> > +++ g26/drivers/i2c/i2c-core.c 2008-05-11 15:09:00.000000000 -0700
> > (...)
> > @@ -1568,7 +1576,7 @@ static s32 i2c_smbus_xfer_emulated(struc
> > default:
> > dev_err(&adapter->dev, "smbus_access called with invalid size (%d)\n",
> > size);
> > - return -1;
> > + return -EOPNOTSUPP;
> > }
>
> I'd rather use -EINVAL here. (...)
Hmh, scratch this. Thinking about it some more (night helps)
-EOPNOTSUPP is consistent with what we did for the bus drivers after
all, and it also anticipates addition of new transaction types to
<linux/i2c.h> or removal of support for some transaction types from
i2c_smbus_xfer_emulated() (which might as well happen, if you look
carefully you'll see that there are no in-kernel users of transaction
types I2C_SMBUS_PROC_CALL and I2C_SMBUS_BLOCK_PROC_CALL so we might
decide to remove thir support from i2c_smbus_xfer_emulated someday.)
> (..)
> BTW, feel free to adjust the debugging message above, as you did in
> some bus drivers already, to clearly say that we're speaking about a
> transaction type and not a "size".
>
> All the rest looks OK to me from a functional point of view. As far as
> style is concerned, please keep the alignment on opening parenthesis
> when the original code did that. i2c-core uses this style consistently
> and I like it.
I'll do all this myself now, no need to resend.
Thanks,
--
Jean Delvare
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2.6.26-rc1-git] i2c-core: return -Errno, not -1
[not found] ` <20080517081537.17220cd0-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
@ 2008-05-17 13:53 ` David Brownell
[not found] ` <200805170653.19424.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: David Brownell @ 2008-05-17 13:53 UTC (permalink / raw)
To: Jean Delvare; +Cc: i2c-GZX6beZjE8VD60Wz+7aTrA
On Friday 16 May 2008, Jean Delvare wrote:
> > > default:
> > > dev_err(&adapter->dev, "smbus_access called with invalid size (%d)\n",
> > > size);
> > > - return -1;
> > > + return -EOPNOTSUPP;
> > > }
> >
> > I'd rather use -EINVAL here. (...)
>
> Hmh, scratch this. Thinking about it some more (night helps)
> -EOPNOTSUPP is consistent with what we did for the bus drivers after
> all, and it also anticipates addition of new transaction types to
> <linux/i2c.h> or removal of support for some transaction types from
> i2c_smbus_xfer_emulated() ..
Right.
> > (..)
> > BTW, feel free to adjust the debugging message above, as you did in
> > some bus drivers already, to clearly say that we're speaking about a
> > transaction type and not a "size".
I must have missed a few. Those updates weren't initially on
a list of things-to-change, but I guess some of the issues were
egregious enough to make me add them -- after that particular
confusion percolated long enough. The third or fourth time
through a piece of code, it's a bit easier to notice that sort
of confusion. (Probably worth someone's time to switch *ALL*
drivers from calling that a "size". I think that bit of history
needs to be edited out of existence...)
> > All the rest looks OK to me from a functional point of view. As far as
> > style is concerned, please keep the alignment on opening parenthesis
> > when the original code did that. i2c-core uses this style consistently
> > and I like it.
>
> I'll do all this myself now, no need to resend.
Good. In general, when it's down to the last set of small
comments like this, it's probably easier all around if you
do stuff like that (rather than writing it up in email, having
someone like me both read it and make time to act on it, then
have you re-review). That's fairly common maintainer practice.
- Dave
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2.6.26-rc1-git] i2c-core: return -Errno, not -1
[not found] ` <200805170653.19424.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
@ 2008-05-17 14:43 ` Jean Delvare
0 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2008-05-17 14:43 UTC (permalink / raw)
To: David Brownell; +Cc: i2c-GZX6beZjE8VD60Wz+7aTrA
On Sat, 17 May 2008 06:53:18 -0700, David Brownell wrote:
> On Friday 16 May 2008, Jean Delvare wrote:
> > I'll do all this myself now, no need to resend.
>
> Good. In general, when it's down to the last set of small
> comments like this, it's probably easier all around if you
> do stuff like that (rather than writing it up in email, having
> someone like me both read it and make time to act on it, then
> have you re-review). That's fairly common maintainer practice.
Well, the patch has your name on it, so I think it's only fair to give
you a chance to nack if I am going to change it before I push it
upstream. But if you don't care, I can skip this step on future
patches, no problem.
--
Jean Delvare
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-05-17 14:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-03 20:06 [patch 2.6.25-git] i2c-core: return -Errno, not -1 David Brownell
[not found] ` <200805031306.16698.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-05-11 22:16 ` [patch 2.6.26-rc1-git] " David Brownell
[not found] ` <200805111516.08453.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-05-16 7:14 ` Jean Delvare
[not found] ` <20080516091458.28f9f5ca-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-05-17 6:15 ` Jean Delvare
[not found] ` <20080517081537.17220cd0-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-05-17 13:53 ` David Brownell
[not found] ` <200805170653.19424.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-05-17 14:43 ` Jean Delvare
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox