2011-03-17 21:42:35

by Mark Brown

[permalink] [raw]
Subject: [PATCH 1/3] mfd: Avoid copying data in WM8994 I2C write

As well as providing a trivial performance optimisation this also avoids
allocating a copy of the message on the stack which is beneficial when
doing large transfers.

Signed-off-by: Mark Brown <[email protected]>
---
drivers/mfd/wm8994-core.c | 22 +++++++++++++---------
1 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
index 749c31d..ce93796 100644
--- a/drivers/mfd/wm8994-core.c
+++ b/drivers/mfd/wm8994-core.c
@@ -580,25 +580,29 @@ static int wm8994_i2c_read_device(struct wm8994 *wm8994, unsigned short reg,
return 0;
}

-/* Currently we allocate the write buffer on the stack; this is OK for
- * small writes - if we need to do large writes this will need to be
- * revised.
- */
static int wm8994_i2c_write_device(struct wm8994 *wm8994, unsigned short reg,
int bytes, void *src)
{
struct i2c_client *i2c = wm8994->control_data;
- unsigned char msg[bytes + 2];
+ struct i2c_msg xfer[2];
int ret;

reg = cpu_to_be16(reg);
- memcpy(&msg[0], &reg, 2);
- memcpy(&msg[2], src, bytes);

- ret = i2c_master_send(i2c, msg, bytes + 2);
+ xfer[0].addr = i2c->addr;
+ xfer[0].flags = 0;
+ xfer[0].len = 2;
+ xfer[0].buf = (char *)&reg;
+
+ xfer[1].addr = i2c->addr;
+ xfer[1].flags = I2C_M_NOSTART;
+ xfer[1].len = bytes;
+ xfer[1].buf = (char *)src;
+
+ ret = i2c_transfer(i2c->adapter, xfer, 2);
if (ret < 0)
return ret;
- if (ret < bytes + 2)
+ if (ret != 2)
return -EIO;

return 0;
--
1.7.4.1


2011-03-17 21:42:44

by Mark Brown

[permalink] [raw]
Subject: [PATCH 2/3] mfd: Push byte swap out of WM8994 bulk I/O

For bulk I/O it is both convenient and more sensible to pre-swap the data
rather than doing the swap as part of the I/O operation so move the byte
swaps we're currently doing into the core write function into the register
based functions, giving the bulk write function a straight pass through
to the chip.

This leaves reads inconsistent, this will be addressed as a followup patch.

Signed-off-by: Mark Brown <[email protected]>
---
drivers/mfd/wm8994-core.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
index ce93796..475bd50 100644
--- a/drivers/mfd/wm8994-core.c
+++ b/drivers/mfd/wm8994-core.c
@@ -107,9 +107,7 @@ static int wm8994_write(struct wm8994 *wm8994, unsigned short reg,

for (i = 0; i < bytes / 2; i++) {
dev_vdbg(wm8994->dev, "Write %04x to R%d(0x%x)\n",
- buf[i], reg + i, reg + i);
-
- buf[i] = cpu_to_be16(buf[i]);
+ be16_to_cpu(buf[i]), reg + i, reg + i);
}

return wm8994->write_dev(wm8994, reg, bytes, src);
@@ -127,6 +125,8 @@ int wm8994_reg_write(struct wm8994 *wm8994, unsigned short reg,
{
int ret;

+ val = cpu_to_be16(val);
+
mutex_lock(&wm8994->io_lock);

ret = wm8994_write(wm8994, reg, 2, &val);
@@ -143,7 +143,7 @@ EXPORT_SYMBOL_GPL(wm8994_reg_write);
* @wm8994: Device to write to
* @reg: First register
* @count: Number of registers
- * @buf: Buffer to write from.
+ * @buf: Buffer to write from. Data must be big-endian formatted.
*/
int wm8994_bulk_write(struct wm8994 *wm8994, unsigned short reg,
int count, u16 *buf)
@@ -183,6 +183,8 @@ int wm8994_set_bits(struct wm8994 *wm8994, unsigned short reg,
r &= ~mask;
r |= val;

+ r = cpu_to_be16(r);
+
ret = wm8994_write(wm8994, reg, 2, &r);

out:
--
1.7.4.1

2011-03-17 21:42:55

by Mark Brown

[permalink] [raw]
Subject: [PATCH 3/3] mfd: Constify WM8994 write path

Allow const buffers to be passed in without type safety issues.

Signed-off-by: Mark Brown <[email protected]>
---
drivers/mfd/wm8994-core.c | 8 ++++----
include/linux/mfd/wm8994/core.h | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
index 475bd50..3f5b7cc 100644
--- a/drivers/mfd/wm8994-core.c
+++ b/drivers/mfd/wm8994-core.c
@@ -97,9 +97,9 @@ int wm8994_bulk_read(struct wm8994 *wm8994, unsigned short reg,
EXPORT_SYMBOL_GPL(wm8994_bulk_read);

static int wm8994_write(struct wm8994 *wm8994, unsigned short reg,
- int bytes, void *src)
+ int bytes, const void *src)
{
- u16 *buf = src;
+ const u16 *buf = src;
int i;

BUG_ON(bytes % 2);
@@ -146,7 +146,7 @@ EXPORT_SYMBOL_GPL(wm8994_reg_write);
* @buf: Buffer to write from. Data must be big-endian formatted.
*/
int wm8994_bulk_write(struct wm8994 *wm8994, unsigned short reg,
- int count, u16 *buf)
+ int count, const u16 *buf)
{
int ret;

@@ -583,7 +583,7 @@ static int wm8994_i2c_read_device(struct wm8994 *wm8994, unsigned short reg,
}

static int wm8994_i2c_write_device(struct wm8994 *wm8994, unsigned short reg,
- int bytes, void *src)
+ int bytes, const void *src)
{
struct i2c_client *i2c = wm8994->control_data;
struct i2c_msg xfer[2];
diff --git a/include/linux/mfd/wm8994/core.h b/include/linux/mfd/wm8994/core.h
index cb7d3ae..f0b69cd 100644
--- a/include/linux/mfd/wm8994/core.h
+++ b/include/linux/mfd/wm8994/core.h
@@ -59,7 +59,7 @@ struct wm8994 {
int (*read_dev)(struct wm8994 *wm8994, unsigned short reg,
int bytes, void *dest);
int (*write_dev)(struct wm8994 *wm8994, unsigned short reg,
- int bytes, void *src);
+ int bytes, const void *src);

void *control_data;

@@ -89,7 +89,7 @@ int wm8994_set_bits(struct wm8994 *wm8994, unsigned short reg,
int wm8994_bulk_read(struct wm8994 *wm8994, unsigned short reg,
int count, u16 *buf);
int wm8994_bulk_write(struct wm8994 *wm8994, unsigned short reg,
- int count, u16 *buf);
+ int count, const u16 *buf);


/* Helper to save on boilerplate */
--
1.7.4.1

2011-03-17 23:49:37

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH 1/3] mfd: Avoid copying data in WM8994 I2C write

Hi Mark,

On Thu, Mar 17, 2011 at 09:42:28PM +0000, Mark Brown wrote:
> As well as providing a trivial performance optimisation this also avoids
> allocating a copy of the message on the stack which is beneficial when
> doing large transfers.
All 3 patches applied, thanks.

Cheers,
Samuel.

--
Intel Open Source Technology Centre
http://oss.intel.com/