Re: [PATCH 1/1] fix i2c_msg.len not aligning with i2c_master_send

[zhangfei gao <zhangfei.gao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

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

On Thu, Feb 4, 2010 at 9:14 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> On Thu, 4 Feb 2010 04:47:41 -0800, Eric Miao wrote:
>> > How about return error in i2c_master_send & i2c_master_recv when count
>> > is bigger than 64K, as suggested by Ben.
>>
>> I think that's more preferable. Making the count parameter as u16,
>> though is going to generate a warning, yet that's usually ignored
>> by careless programmer, screaming out when this happens might
>> be more useful sometimes.
>
> Developers ignoring warnings get the pain the deserve.
>
> A check on "count" would come at the price of a small performance hit
> for every caller, even though in 99% of the cases the check isn't
> needed.
>
> That being said I don't care too much and will take whatever patch is
> sent to me.
>
> It would be a good idea to add a note about this limit in
> Documentation/i2c/writing-clients and/or include/linux/i2c.h.
>
>> > The device I used could receive 32K one time instead, the firmware
>> > download only takes place on-demand in fact.
>> > However, it took some time to debug, since no error info come out.
>> > Add error msg may notify users, though transfering more than 64K data
>> > one time is rarely happen.
>
> --
> Jean Delvare
>

Hi, Jean

Thanks for your instruction.
Here is patch to modify some comments of i2c_master_send &
i2c_master_recv, is this OK.

Thanks
Zhangfei

>From 30fbf1ebf1facba3d280c887e2ecfd0499e7b04b Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Date: Sat, 6 Feb 2010 05:38:59 +0800
Subject: [PATCH] i2c: notes of i2c_master_send & i2c_master_recv

	i2c_master_send & i2c_master_recv not support more than 64bytes
transfer, since msg.len is __u16 type

Signed-off-by: Zhangfei Gao <zgao6-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/i2c/writing-clients |    3 ++-
 drivers/i2c/i2c-core.c            |    4 ++--
 include/linux/i2c.h               |    1 +
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/i2c/writing-clients
b/Documentation/i2c/writing-clients
index 7860aaf..929a3c3 100644
--- a/Documentation/i2c/writing-clients
+++ b/Documentation/i2c/writing-clients
@@ -318,7 +318,8 @@ Plain I2C communication
 These routines read and write some bytes from/to a client. The client
 contains the i2c address, so you do not have to include it. The second
 parameter contains the bytes to read/write, the third the number of bytes
-to read/write (must be less than the length of the buffer.) Returned is
+to read/write (must be less than the length of the buffer, also should be
+less than 64K since msg.len is __u16 type.) Returned is
 the actual number of bytes read/written.

 	int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msg,
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 8d80fce..9607dcc 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1112,7 +1112,7 @@ EXPORT_SYMBOL(i2c_transfer);
  * i2c_master_send - issue a single I2C message in master transmit mode
  * @client: Handle to slave device
  * @buf: Data that will be written to the slave
- * @count: How many bytes to write
+ * @count: How many bytes to write, should be less than 64K since
msg.len is u16
  *
  * Returns negative errno, or else the number of bytes written.
  */
@@ -1139,7 +1139,7 @@ EXPORT_SYMBOL(i2c_master_send);
  * i2c_master_recv - issue a single I2C message in master receive mode
  * @client: Handle to slave device
  * @buf: Where to store data read from slave
- * @count: How many bytes to read
+ * @count: How many bytes to read, should be less than 64K since msg.len is u16
  *
  * Returns negative errno, or else the number of bytes read.
  */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 57d41b0..b2dea18 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -53,6 +53,7 @@ struct i2c_board_info;
  * on a bus (or read from them). Apart from two basic transfer functions to
  * transmit one message at a time, a more complex version can be used to
  * transmit an arbitrary number of messages without interruption.
+ * Parameter count should be less than 64K since msg.len is __u16
  */
 extern int i2c_master_send(struct i2c_client *client, const char *buf,
 			   int count);
-- 
1.6.0.4

[-- Attachment #2: 0001-i2c-notes-of-i2c_master_send-i2c_master_recv.patch --]
[-- Type: application/octet-stream, Size: 2844 bytes --]

From 30fbf1ebf1facba3d280c887e2ecfd0499e7b04b Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Sat, 6 Feb 2010 05:38:59 +0800
Subject: [PATCH] i2c: notes of i2c_master_send & i2c_master_recv

	i2c_master_send & i2c_master_recv not support more than 64bytes transfer, since msg.len is __u16 type

Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
---
 Documentation/i2c/writing-clients |    3 ++-
 drivers/i2c/i2c-core.c            |    4 ++--
 include/linux/i2c.h               |    1 +
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/i2c/writing-clients b/Documentation/i2c/writing-clients
index 7860aaf..929a3c3 100644
--- a/Documentation/i2c/writing-clients
+++ b/Documentation/i2c/writing-clients
@@ -318,7 +318,8 @@ Plain I2C communication
 These routines read and write some bytes from/to a client. The client
 contains the i2c address, so you do not have to include it. The second
 parameter contains the bytes to read/write, the third the number of bytes
-to read/write (must be less than the length of the buffer.) Returned is
+to read/write (must be less than the length of the buffer, also should be
+less than 64K since msg.len is __u16 type.) Returned is
 the actual number of bytes read/written.
 
 	int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msg,
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 8d80fce..9607dcc 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1112,7 +1112,7 @@ EXPORT_SYMBOL(i2c_transfer);
  * i2c_master_send - issue a single I2C message in master transmit mode
  * @client: Handle to slave device
  * @buf: Data that will be written to the slave
- * @count: How many bytes to write
+ * @count: How many bytes to write, should be less than 64K since msg.len is u16
  *
  * Returns negative errno, or else the number of bytes written.
  */
@@ -1139,7 +1139,7 @@ EXPORT_SYMBOL(i2c_master_send);
  * i2c_master_recv - issue a single I2C message in master receive mode
  * @client: Handle to slave device
  * @buf: Where to store data read from slave
- * @count: How many bytes to read
+ * @count: How many bytes to read, should be less than 64K since msg.len is u16
  *
  * Returns negative errno, or else the number of bytes read.
  */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 57d41b0..b2dea18 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -53,6 +53,7 @@ struct i2c_board_info;
  * on a bus (or read from them). Apart from two basic transfer functions to
  * transmit one message at a time, a more complex version can be used to
  * transmit an arbitrary number of messages without interruption.
+ * Parameter count should be less than 64K since msg.len is __u16
  */
 extern int i2c_master_send(struct i2c_client *client, const char *buf,
 			   int count);
-- 
1.6.0.4