Re: [PATCH] i2c: imx: choose the better clock divider
Sorry forgot to include Ben.
Ben, any ideas?
On Tue, Jun 14, 2011 at 3:17 PM, Eric Miao eric.m...@linaro.org wrote:
The original algorithm doesn't perform very well in some cases, e.g.
When the source clock of the I2C controller is 66MHz, and the
requested rate is 100KHz, it gives a divider of 768 instead of
the better 640.
Choose a better clock divider so the final clock rate is closer to
the requested one by comparing the rate distances calculated by
two adjacent dividers.
Cc: Richard Zhao richard.z...@linaro.org
Signed-off-by: Eric Miao eric.m...@linaro.org
---
drivers/i2c/busses/i2c-imx.c | 46 -
1 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 4c2a62b..cd640ff 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -112,6 +112,8 @@ static u16 __initdata i2c_clk_div[50][2] = {
{ 3072, 0x1E }, { 3840, 0x1F }
};
+#define I2C_CLK_DIV_NUM ARRAY_SIZE(i2c_clk_div)
+
struct imx_i2c_struct {
struct i2c_adapter adapter;
struct resource *res;
@@ -240,22 +242,37 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
clk_disable(i2c_imx-clk);
}
-static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
- unsigned int rate)
+/* find the index into i2c_clk_div[] array, compare with the two
+ * dividers found, return the one with smaller error
+ */
+static int find_div(unsigned long i2c_clk_rate, unsigned long rate)
{
- unsigned int i2c_clk_rate;
- unsigned int div;
+ unsigned long div, delta_l, delta_h;
int i;
- /* Divider value calculation */
- i2c_clk_rate = clk_get_rate(i2c_imx-clk);
div = (i2c_clk_rate + rate - 1) / rate;
+
if (div i2c_clk_div[0][0])
- i = 0;
- else if (div i2c_clk_div[ARRAY_SIZE(i2c_clk_div) - 1][0])
- i = ARRAY_SIZE(i2c_clk_div) - 1;
- else
- for (i = 0; i2c_clk_div[i][0] div; i++);
+ return 0;
+
+ if (div i2c_clk_div[I2C_CLK_DIV_NUM - 1][0])
+ return I2C_CLK_DIV_NUM;
+
+ for (i = 0; i I2C_CLK_DIV_NUM; i++)
+ if (i2c_clk_div[i][0] div)
+ break;
+
+ delta_h = (i2c_clk_rate / i2c_clk_div[i - 1][0]) - rate;
+ delta_l = rate - (i2c_clk_rate / i2c_clk_div[i][0]);
+
+ return (delta_l delta_h) ? i : i - 1;
+}
+
+static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
+ unsigned int rate)
+{
+ unsigned long i2c_clk_rate = clk_get_rate(i2c_imx-clk);
+ int i = find_div(i2c_clk_rate, rate);
/* Store divider value */
i2c_imx-ifdr = i2c_clk_div[i][1];
@@ -271,10 +288,9 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct
*i2c_imx,
/* dev_dbg() can't be used, because adapter is not yet registered */
#ifdef CONFIG_I2C_DEBUG_BUS
- printk(KERN_DEBUG I2C: %s I2C_CLK=%d, REQ DIV=%d\n,
- __func__, i2c_clk_rate, div);
- printk(KERN_DEBUG I2C: %s IFDR[IC]=0x%x, REAL DIV=%d\n,
- __func__, i2c_clk_div[i][1], i2c_clk_div[i][0]);
+ pr_debug(%s I2C_CLK=%ld, REQ RATE=%d, DIV=%d, IFDR[IC]=0x%x\n,
+ __func__, i2c_clk_rate, rate,
+ i2c_clk_div[i][0], i2c_clk_div[i][1]);
#endif
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] i2c: imx: choose the better clock divider
The original algorithm doesn't perform very well in some cases, e.g.
When the source clock of the I2C controller is 66MHz, and the
requested rate is 100KHz, it gives a divider of 768 instead of
the better 640.
Choose a better clock divider so the final clock rate is closer to
the requested one by comparing the rate distances calculated by
two adjacent dividers.
Cc: Richard Zhao richard.z...@linaro.org
Signed-off-by: Eric Miao eric.m...@linaro.org
---
drivers/i2c/busses/i2c-imx.c | 46 -
1 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 4c2a62b..cd640ff 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -112,6 +112,8 @@ static u16 __initdata i2c_clk_div[50][2] = {
{ 3072, 0x1E }, { 3840, 0x1F }
};
+#define I2C_CLK_DIV_NUMARRAY_SIZE(i2c_clk_div)
+
struct imx_i2c_struct {
struct i2c_adapter adapter;
struct resource *res;
@@ -240,22 +242,37 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
clk_disable(i2c_imx-clk);
}
-static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
- unsigned int rate)
+/* find the index into i2c_clk_div[] array, compare with the two
+ * dividers found, return the one with smaller error
+ */
+static int find_div(unsigned long i2c_clk_rate, unsigned long rate)
{
- unsigned int i2c_clk_rate;
- unsigned int div;
+ unsigned long div, delta_l, delta_h;
int i;
- /* Divider value calculation */
- i2c_clk_rate = clk_get_rate(i2c_imx-clk);
div = (i2c_clk_rate + rate - 1) / rate;
+
if (div i2c_clk_div[0][0])
- i = 0;
- else if (div i2c_clk_div[ARRAY_SIZE(i2c_clk_div) - 1][0])
- i = ARRAY_SIZE(i2c_clk_div) - 1;
- else
- for (i = 0; i2c_clk_div[i][0] div; i++);
+ return 0;
+
+ if (div i2c_clk_div[I2C_CLK_DIV_NUM - 1][0])
+ return I2C_CLK_DIV_NUM;
+
+ for (i = 0; i I2C_CLK_DIV_NUM; i++)
+ if (i2c_clk_div[i][0] div)
+ break;
+
+ delta_h = (i2c_clk_rate / i2c_clk_div[i - 1][0]) - rate;
+ delta_l = rate - (i2c_clk_rate / i2c_clk_div[i][0]);
+
+ return (delta_l delta_h) ? i : i - 1;
+}
+
+static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
+ unsigned int rate)
+{
+ unsigned long i2c_clk_rate = clk_get_rate(i2c_imx-clk);
+ int i = find_div(i2c_clk_rate, rate);
/* Store divider value */
i2c_imx-ifdr = i2c_clk_div[i][1];
@@ -271,10 +288,9 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct
*i2c_imx,
/* dev_dbg() can't be used, because adapter is not yet registered */
#ifdef CONFIG_I2C_DEBUG_BUS
- printk(KERN_DEBUG I2C: %s I2C_CLK=%d, REQ DIV=%d\n,
- __func__, i2c_clk_rate, div);
- printk(KERN_DEBUG I2C: %s IFDR[IC]=0x%x, REAL DIV=%d\n,
- __func__, i2c_clk_div[i][1], i2c_clk_div[i][0]);
+ pr_debug(%s I2C_CLK=%ld, REQ RATE=%d, DIV=%d, IFDR[IC]=0x%x\n,
+ __func__, i2c_clk_rate, rate,
+ i2c_clk_div[i][0], i2c_clk_div[i][1]);
#endif
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] i2c: append hardware lock with bus lock
On Thu, Apr 28, 2011 at 4:22 PM, Jean Delvare kh...@linux-fr.org wrote:
Hi Haojian,
On Thu, 28 Apr 2011 12:02:36 +0800, Haojian Zhuang wrote:
Both AP and CP are contained in Marvell PXA910 silicon. These two ARM
cores are sharing one pair of I2C pins.
In order to keep I2C transaction operated with atomic, hardware lock
(RIPC) is required. Because of this, bus lock in AP side can't afford
this requirement. Now hardware lock is appended.
I have no objection to the idea, but one question: when using the
hardware lock, isn't the software mutex redundant? I would expect that
you call the hardware_lock/unlock functions _instead_ of
rt_mutex_lock/unlock, rather than in addition to it. Or do you still
need the rt_mutex to prevent priority inversion?
Jean,
It's actually not redundant. The hardware lock is used to protect
access to the same register regions between two processors (AP
and CP so called), while the software lock is used to protect
access from within the AP side.
Signed-off-by: Haojian Zhuang haojian.zhu...@marvell.com
Cc: Ben Dooks ben-li...@fluff.org
---
drivers/i2c/i2c-core.c | 22 ++
include/linux/i2c.h | 3 +++
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 045ba6e..412c7a5 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -448,8 +448,11 @@ void i2c_lock_adapter(struct i2c_adapter *adapter)
if (parent)
i2c_lock_adapter(parent);
- else
+ else {
rt_mutex_lock(adapter-bus_lock);
+ if (adapter-hardware_lock)
+ adapter-hardware_lock();
+ }
}
EXPORT_SYMBOL_GPL(i2c_lock_adapter);
@@ -460,11 +463,19 @@ EXPORT_SYMBOL_GPL(i2c_lock_adapter);
static int i2c_trylock_adapter(struct i2c_adapter *adapter)
{
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
+ int ret = 0;
if (parent)
return i2c_trylock_adapter(parent);
- else
- return rt_mutex_trylock(adapter-bus_lock);
+ else {
+ ret = rt_mutex_trylock(adapter-bus_lock);
+ if (ret adapter-hardware_trylock) {
+ ret = adapter-hardware_trylock();
+ if (!ret)
+ i2c_unlock_adapter(adapter);
+ }
+ return ret;
+ }
}
/**
@@ -477,8 +488,11 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
if (parent)
i2c_unlock_adapter(parent);
- else
+ else {
+ if (adapter-hardware_unlock)
+ adapter-hardware_unlock();
rt_mutex_unlock(adapter-bus_lock);
+ }
}
EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 06a8d9c..b283b4e 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -361,6 +361,9 @@ struct i2c_adapter {
/* data fields that are valid for all devices */
struct rt_mutex bus_lock;
+ void (*hardware_lock)(void);
+ void (*hardware_unlock)(void);
+ int (*hardware_trylock)(void);
int timeout; /* in jiffies */
int retries;
--
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] ARM: pxa2xx: reorganize I2C files
On Wed, Mar 2, 2011 at 8:49 AM, Ben Dooks ben-li...@fluff.org wrote: On 23/02/11 11:38, Sebastian Andrzej Siewior wrote: This patch moves the platform data definition from arch/arm/plat-pxa/include/plat/i2c.h to include/linux/i2c/pxa-i2c.h so it can be accessed from x86 the same way as on ARM. This change should make no functional change to the PXA code. The move is verified by building the following defconfigs: cm_x2xx_defconfig corgi_defconfig em_x270_defconfig ezx_defconfig imote2_defconfig pxa3xx_defconfig spitz_defconfig zeus_defconfig raumfeld_defconfig magician_defconfig mmp2_defconfig pxa168_defconfig pxa910_defconfig Russell/others, you OK with this going through my tree? Ben, I'm fine that you take this change. Trivial changes to me, so I expect no significant merge conflicts. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-pxa: fix compiler warning, due to missing const
On Tue, Jun 15, 2010 at 4:56 PM, Marc Kleine-Budde m...@pengutronix.de wrote: This patch adds the missing const to struct platform_device_id to fix this warning: /home/frogger/pengutronix/linux/linux-2.6/drivers/i2c/busses/i2c-pxa.c: In function 'i2c_pxa_probe': /home/frogger/pengutronix/linux/linux-2.6/drivers/i2c/busses/i2c-pxa.c:1004: warning: initialization discards qualifiers from pointer target type Signed-off-by: Marc Kleine-Budde m...@pengutronix.de Cc: Eric Miao eric.y.m...@gmail.com Ack Cc: Roel Kluin roel.kl...@gmail.com Cc: Pavel Machek pa...@ucw.cz --- drivers/i2c/busses/i2c-pxa.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c index 020ff23..c94e51b 100644 --- a/drivers/i2c/busses/i2c-pxa.c +++ b/drivers/i2c/busses/i2c-pxa.c @@ -1001,7 +1001,7 @@ static int i2c_pxa_probe(struct platform_device *dev) struct pxa_i2c *i2c; struct resource *res; struct i2c_pxa_platform_data *plat = dev-dev.platform_data; - struct platform_device_id *id = platform_get_device_id(dev); + const struct platform_device_id *id = platform_get_device_id(dev); int ret; int irq; -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] mach-pxa/viper: Fix timeout usage for I2C
On Tue, Apr 20, 2010 at 4:40 PM, Marek Vasut marek.va...@gmail.com wrote: Dne Út 20. dubna 2010 10:27:00 Marc Zyngier napsal(a): On Sun, 18 Apr 2010 13:48:29 +0200, Wolfram Sang w.s...@pengutronix.de wrote: The timeout value is in jiffies, so it should be using HZ, not a plain number. Assume with HZ=100 '100' means 1s here and adapt accordingly. Signed-off-by: Wolfram Sang w.s...@pengutronix.de Cc: Eric Miao eric.y.m...@gmail.com Cc: Russell King li...@arm.linux.org.uk Cc: Marc Zyngier m...@misterjones.org Cc: Paul Shen paul.s...@marvell.com Cc: Mike Rapoport m...@compulab.co.il Cc: Jean Delvare kh...@linux-fr.org Acked-by: Marc Zyngier m...@misterjones.org Eric, can you merge this directly via your -devel branch? Thanks, M. And possibly updating your git tree? There are a few patches in -next that will need it's arm counterpart. Merged and updated. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] mach-pxa/viper: Fix timeout usage for I2C
On Sun, Apr 18, 2010 at 7:48 PM, Wolfram Sang w.s...@pengutronix.de wrote:
The timeout value is in jiffies, so it should be using HZ, not a plain
number. Assume with HZ=100 '100' means 1s here and adapt accordingly.
Signed-off-by: Wolfram Sang w.s...@pengutronix.de
Cc: Eric Miao eric.y.m...@gmail.com
Cc: Russell King li...@arm.linux.org.uk
Cc: Marc Zyngier m...@misterjones.org
Cc: Paul Shen paul.s...@marvell.com
Cc: Mike Rapoport m...@compulab.co.il
Cc: Jean Delvare kh...@linux-fr.org
---
Changes since V1:
* Don't assume 100 means 100ms
Thanks for the comments!
Admitted, the first try was a really bad guess. Maybe I got distracted from
the
previous patch for another arch where the value was 1.
While this may still not be the favoured solution for Eric, I think it is at
least better than before, so it might be worth applying after all?
Ack. The clearest fix would involve more code to be updated, which
can be postponed. This fix, at least, makes the time quantity clearer.
Janitorial fix, not tested due to no hardware.
arch/arm/mach-pxa/viper.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/viper.c b/arch/arm/mach-pxa/viper.c
index 1dd1334..12cf38c 100644
--- a/arch/arm/mach-pxa/viper.c
+++ b/arch/arm/mach-pxa/viper.c
@@ -33,6 +33,7 @@
#include linux/pm.h
#include linux/sched.h
#include linux/gpio.h
+#include linux/jiffies.h
#include linux/i2c-gpio.h
#include linux/serial_8250.h
#include linux/smc91x.h
@@ -453,7 +454,7 @@ static struct i2c_gpio_platform_data i2c_bus_data = {
.sda_pin = VIPER_RTC_I2C_SDA_GPIO,
.scl_pin = VIPER_RTC_I2C_SCL_GPIO,
.udelay = 10,
- .timeout = 100,
+ .timeout = HZ,
};
static struct platform_device i2c_bus_device = {
@@ -778,7 +779,7 @@ static void __init viper_tpm_init(void)
.sda_pin = VIPER_TPM_I2C_SDA_GPIO,
.scl_pin = VIPER_TPM_I2C_SCL_GPIO,
.udelay = 10,
- .timeout = 100,
+ .timeout = HZ,
};
char *errstr;
--
1.7.0
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mach-pxa/viper: Fix timeout usage for I2C
On Sun, Apr 4, 2010 at 10:08 PM, Wolfram Sang w.s...@pengutronix.de wrote:
The timeout value is in jiffies, so it should be using HZ, not a plain
number. Assume '100' means 100ms here and adapt accordingly.
Signed-off-by: Wolfram Sang w.s...@pengutronix.de
Cc: Eric Miao eric.y.m...@gmail.com
Cc: Russell King li...@arm.linux.org.uk
Cc: Marc Zyngier m...@misterjones.org
Cc: Paul Shen paul.s...@marvell.com
Cc: Mike Rapoport m...@compulab.co.il
---
Janitorial fix, not tested due to no hardware.
arch/arm/mach-pxa/viper.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/viper.c b/arch/arm/mach-pxa/viper.c
index 1dd1334..c25921f 100644
--- a/arch/arm/mach-pxa/viper.c
+++ b/arch/arm/mach-pxa/viper.c
@@ -33,6 +33,7 @@
#include linux/pm.h
#include linux/sched.h
#include linux/gpio.h
+#include linux/jiffies.h
#include linux/i2c-gpio.h
#include linux/serial_8250.h
#include linux/smc91x.h
@@ -453,7 +454,7 @@ static struct i2c_gpio_platform_data i2c_bus_data = {
.sda_pin = VIPER_RTC_I2C_SDA_GPIO,
.scl_pin = VIPER_RTC_I2C_SCL_GPIO,
.udelay = 10,
- .timeout = 100,
+ .timeout = HZ / 10,
};
static struct platform_device i2c_bus_device = {
@@ -778,7 +779,7 @@ static void __init viper_tpm_init(void)
.sda_pin = VIPER_TPM_I2C_SDA_GPIO,
.scl_pin = VIPER_TPM_I2C_SCL_GPIO,
.udelay = 10,
- .timeout = 100,
+ .timeout = HZ / 10,
};
char *errstr;
One other better and cleaner approach to such inconsistency issue is
to have a timeout_ms field, and having i2c-gpio.c driver to convert this
to jiffies using msec_to_jiffies() at run-time.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mach-pxa/viper: Fix timeout usage for I2C
On Tue, Apr 13, 2010 at 5:53 AM, Jamie Lokier ja...@shareable.org wrote: Russell King - ARM Linux wrote: On Mon, Apr 12, 2010 at 09:32:35PM +0200, Jean Delvare wrote: On Mon, 12 Apr 2010 20:20:10 +0100, Russell King - ARM Linux wrote: On Mon, Apr 12, 2010 at 09:13:19PM +0200, Jean Delvare wrote: On Tue, 13 Apr 2010 01:57:51 +0800, Eric Miao wrote: One other better and cleaner approach to such inconsistency issue is to have a timeout_ms field, and having i2c-gpio.c driver to convert this to jiffies using msec_to_jiffies() at run-time. With what benefit? Expressing time values in units of HZ is very frequent in the kernel code and shouldn't actually surprise anyone. Actually, this patch shows there is confusion. Assume '100' means 100ms here and adapt accordingly. Since this patch is for ARM, where HZ=100, the above patch is not a simple convert how we derive this constant patch - it's a functional change, reducing the timeouts by a factor of 10. Could that be because the patch author misinterpreted the HZ-based values? I admit I would have assumed 100 - HZ, as hard-coded HZ-dependent value typically assume HZ=100. I suspect I'm not the only one who thinks that the latter of HZ / 10 100ms is easier to read and comprehend without mistake. OTOH, converting from ms to jiffies each time you need the value has a cost. True; what I did for MMC stuff is converted it from ms to jiffies at initialization time when copying it in from platform data in the driver's probe function. I'm not saying that I care either way, I'm merely showing that dealing with HZ-based values can be (maybe unexpectedly) more error prone. HZ is used a lot in kernel timeouts, so even though it's confusing, it is something everyone ought to get used to. I don't understand why people has to live with confusions where there is apparently a cleaner way to go. HZ is everywhere, and as long as they live within their own driver code, that's OK. But I agree it is too confusing. An obvious remedy is: #define milliseconds(ms) (((ms) * HZ + 999) / 1000) #define seconds(s) ((s) * HZ) This is to reinvent the wheel as there are already msecs_to_jiffies() and usecs_to_jiffies(), and vice versa. The only benefit of using macros instead of a function is for constants. This, however, can be worked around as Russell suggested. The additional run-time cost by this function, compared with a unit of jiffies, is insignificant. -- Jamie -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] fix i2c_msg.len not aligning with i2c_master_send
Hi, Jean Thanks for your instruction. Here is patch to modify some comments of i2c_master_send i2c_master_recv, is this OK. Where's the screaming things, dude? Thanks Zhangfei From 30fbf1ebf1facba3d280c887e2ecfd0499e7b04b Mon Sep 17 00:00:00 2001 From: Zhangfei Gao zg...@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 zg...@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 -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] fix i2c_msg.len not aligning with i2c_master_send
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. 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. Thanks Zhangfei -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
