diff mbox series

[v2] i2c: use void pointers for supplying data for reads and writes

Message ID 20191112005826.GA96746@dtor-ws
State Superseded
Headers show
Series [v2] i2c: use void pointers for supplying data for reads and writes | expand

Commit Message

Dmitry Torokhov Nov. 12, 2019, 12:58 a.m. UTC
There is no need to force users of i2c_master_send()/i2c_master_recv()
and other i2c read/write bulk data API to cast everything into u8
pointers.  While everything can be considered byte stream, the drivers
are usually work with more structured data.

Let's switch the APIs to accept [const] void pointers to cut amount of
casting needed.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Changes in v2:

- adjusted max1363 to the new i2c_master_send/recv signatures

I sent the previous version of this patch 04/01/17 and kbuild promptly
noticed that it broke max1363. And it took me only 2.5 years to get back
to it ;)

Given that it is only one IIO driver that is affected I hope it can go
through Wolfram's tree and we do not need to go through staged API
rollout.

 drivers/i2c/i2c-core-base.c  |  2 +-
 drivers/i2c/i2c-core-smbus.c | 16 ++++++++--------
 drivers/iio/adc/max1363.c    | 14 ++++++++------
 include/linux/i2c.h          | 28 +++++++++++++++-------------
 4 files changed, 32 insertions(+), 28 deletions(-)

Comments

Luca Ceresoli Nov. 12, 2019, 8:45 a.m. UTC | #1
Hi Dmitry,

On 12/11/19 01:58, Dmitry Torokhov wrote:
> There is no need to force users of i2c_master_send()/i2c_master_recv()
> and other i2c read/write bulk data API to cast everything into u8
> pointers.  While everything can be considered byte stream, the drivers
> are usually work with more structured data.
> 
> Let's switch the APIs to accept [const] void pointers to cut amount of
> casting needed.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I agree on the principle, but I have a question, see below.

[...]
>  s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> -					      u8 command, u8 length, u8 *values)
> +					      u8 command, u8 length, void *values)
>  {
>  	u8 i = 0;
>  	int status;
> @@ -647,8 +648,7 @@ s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
>  			status = i2c_smbus_read_word_data(client, command + i);
>  			if (status < 0)
>  				return status;
> -			values[i] = status & 0xff;
> -			values[i + 1] = status >> 8;
> +			put_unaligned_le16(status, values + i);

The switch to put_unaligned_le16() looks unrelated, is it?

>  			i += 2;
>  		}
>  	}
> @@ -657,7 +657,7 @@ s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
>  		status = i2c_smbus_read_byte_data(client, command + i);
>  		if (status < 0)
>  			return status;
> -		values[i] = status;
> +		*(u8 *)(values + i) = status;

My preference is to use an u8* helper variable in these cases:

s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client
*client,
-			      u8 command, u8 length, u8 *values)
+			      u8 command, u8 length, void *buf)
 {
+	u8 *bytes = buf;
@@
-		values[i] = status;
+		bytes[i] = status;

This clarifies we are accessing the raw bytes, avoids typecasts in the
middle of code for readability and avoids void pointer math.

PS: look, it's exactly what you do in the max1363.c file below! :)

> diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> index 5c2cc61b666e7..48ed76a0e83d4 100644
> --- a/drivers/iio/adc/max1363.c
> +++ b/drivers/iio/adc/max1363.c
> @@ -182,9 +182,9 @@ struct max1363_state {
>  	struct regulator		*vref;
>  	u32				vref_uv;
>  	int				(*send)(const struct i2c_client *client,
> -						const char *buf, int count);
> +						const void *buf, int count);
>  	int				(*recv)(const struct i2c_client *client,
> -						char *buf, int count);
> +						void *buf, int count);
>  };
>  
>  #define MAX1363_MODE_SINGLE(_num, _mask) {				\
> @@ -310,27 +310,29 @@ static const struct max1363_mode
>  	return NULL;
>  }
>  
> -static int max1363_smbus_send(const struct i2c_client *client, const char *buf,
> +static int max1363_smbus_send(const struct i2c_client *client, const void *buf,
>  		int count)
>  {
> +	const u8 *data = buf;

Here it is! ^
Jonathan Cameron Nov. 12, 2019, 10:59 a.m. UTC | #2
On Mon, 11 Nov 2019 16:58:26 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> There is no need to force users of i2c_master_send()/i2c_master_recv()
> and other i2c read/write bulk data API to cast everything into u8
> pointers.  While everything can be considered byte stream, the drivers
> are usually work with more structured data.
> 
> Let's switch the APIs to accept [const] void pointers to cut amount of
> casting needed.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
> 
> Changes in v2:
> 
> - adjusted max1363 to the new i2c_master_send/recv signatures
> 
> I sent the previous version of this patch 04/01/17 and kbuild promptly
> noticed that it broke max1363. And it took me only 2.5 years to get back
> to it ;)
> 
> Given that it is only one IIO driver that is affected I hope it can go
> through Wolfram's tree and we do not need to go through staged API
> rollout.
> 
>  drivers/i2c/i2c-core-base.c  |  2 +-
>  drivers/i2c/i2c-core-smbus.c | 16 ++++++++--------
>  drivers/iio/adc/max1363.c    | 14 ++++++++------
>  include/linux/i2c.h          | 28 +++++++++++++++-------------
>  4 files changed, 32 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 6a5183cffdfc3..aeb4201ef55e4 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2065,7 +2065,7 @@ EXPORT_SYMBOL(i2c_transfer);
>   *
>   * Returns negative errno, or else the number of bytes transferred.
>   */
> -int i2c_transfer_buffer_flags(const struct i2c_client *client, char *buf,
> +int i2c_transfer_buffer_flags(const struct i2c_client *client, void *buf,
>  			      int count, u16 flags)
>  {
>  	int ret;
> diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
> index 3ac426a8ab5ab..4172f8e647f81 100644
> --- a/drivers/i2c/i2c-core-smbus.c
> +++ b/drivers/i2c/i2c-core-smbus.c
> @@ -15,6 +15,7 @@
>  #include <linux/i2c.h>
>  #include <linux/i2c-smbus.h>
>  #include <linux/slab.h>
> +#include <asm/unaligned.h>
>  
>  #include "i2c-core.h"
>  
> @@ -213,7 +214,7 @@ EXPORT_SYMBOL(i2c_smbus_write_word_data);
>   * mechanism (I2C_M_RECV_LEN) which may not be implemented.
>   */
>  s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command,
> -			      u8 *values)
> +			      void *values)
>  {
>  	union i2c_smbus_data data;
>  	int status;
> @@ -240,7 +241,7 @@ EXPORT_SYMBOL(i2c_smbus_read_block_data);
>   * else zero on success.
>   */
>  s32 i2c_smbus_write_block_data(const struct i2c_client *client, u8 command,
> -			       u8 length, const u8 *values)
> +			       u8 length, const void *values)
>  {
>  	union i2c_smbus_data data;
>  
> @@ -256,7 +257,7 @@ EXPORT_SYMBOL(i2c_smbus_write_block_data);
>  
>  /* Returns the number of read bytes */
>  s32 i2c_smbus_read_i2c_block_data(const struct i2c_client *client, u8 command,
> -				  u8 length, u8 *values)
> +				  u8 length, void *values)
>  {
>  	union i2c_smbus_data data;
>  	int status;
> @@ -276,7 +277,7 @@ s32 i2c_smbus_read_i2c_block_data(const struct i2c_client *client, u8 command,
>  EXPORT_SYMBOL(i2c_smbus_read_i2c_block_data);
>  
>  s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client, u8 command,
> -				   u8 length, const u8 *values)
> +				   u8 length, const void *values)
>  {
>  	union i2c_smbus_data data;
>  
> @@ -628,7 +629,7 @@ EXPORT_SYMBOL(__i2c_smbus_xfer);
>   * transfer.
>   */
>  s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> -					      u8 command, u8 length, u8 *values)
> +					      u8 command, u8 length, void *values)
>  {
>  	u8 i = 0;
>  	int status;
> @@ -647,8 +648,7 @@ s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
>  			status = i2c_smbus_read_word_data(client, command + i);
>  			if (status < 0)
>  				return status;
> -			values[i] = status & 0xff;
> -			values[i + 1] = status >> 8;
> +			put_unaligned_le16(status, values + i);
>  			i += 2;
>  		}
>  	}
> @@ -657,7 +657,7 @@ s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
>  		status = i2c_smbus_read_byte_data(client, command + i);
>  		if (status < 0)
>  			return status;
> -		values[i] = status;
> +		*(u8 *)(values + i) = status;
>  		i++;
>  	}
>  
> diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> index 5c2cc61b666e7..48ed76a0e83d4 100644
> --- a/drivers/iio/adc/max1363.c
> +++ b/drivers/iio/adc/max1363.c
> @@ -182,9 +182,9 @@ struct max1363_state {
>  	struct regulator		*vref;
>  	u32				vref_uv;
>  	int				(*send)(const struct i2c_client *client,
> -						const char *buf, int count);
> +						const void *buf, int count);
>  	int				(*recv)(const struct i2c_client *client,
> -						char *buf, int count);
> +						void *buf, int count);
>  };
>  
>  #define MAX1363_MODE_SINGLE(_num, _mask) {				\
> @@ -310,27 +310,29 @@ static const struct max1363_mode
>  	return NULL;
>  }
>  
> -static int max1363_smbus_send(const struct i2c_client *client, const char *buf,
> +static int max1363_smbus_send(const struct i2c_client *client, const void *buf,
>  		int count)
>  {
> +	const u8 *data = buf;
>  	int i, err;
>  
>  	for (i = err = 0; err == 0 && i < count; ++i)
> -		err = i2c_smbus_write_byte(client, buf[i]);
> +		err = i2c_smbus_write_byte(client, data[i]);
>  
>  	return err ? err : count;
>  }
>  
> -static int max1363_smbus_recv(const struct i2c_client *client, char *buf,
> +static int max1363_smbus_recv(const struct i2c_client *client, void *buf,
>  		int count)
>  {
> +	u8 *data = buf;
>  	int i, ret;
>  
>  	for (i = 0; i < count; ++i) {
>  		ret = i2c_smbus_read_byte(client);
>  		if (ret < 0)
>  			return ret;
> -		buf[i] = ret;
> +		data[i] = ret;
>  	}
>  
>  	return count;
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index aaf57d9b41dbb..64cf92e191aa8 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -51,7 +51,7 @@ struct property_entry;
>   * @count must be be less than 64k since msg.len is u16.
>   */
>  extern int i2c_transfer_buffer_flags(const struct i2c_client *client,
> -				     char *buf, int count, u16 flags);
> +				     void *buf, int count, u16 flags);
>  
>  /**
>   * i2c_master_recv - issue a single I2C message in master receive mode
> @@ -62,7 +62,7 @@ extern int i2c_transfer_buffer_flags(const struct i2c_client *client,
>   * Returns negative errno, or else the number of bytes read.
>   */
>  static inline int i2c_master_recv(const struct i2c_client *client,
> -				  char *buf, int count)
> +				  void *buf, int count)
>  {
>  	return i2c_transfer_buffer_flags(client, buf, count, I2C_M_RD);
>  };
> @@ -77,7 +77,7 @@ static inline int i2c_master_recv(const struct i2c_client *client,
>   * Returns negative errno, or else the number of bytes read.
>   */
>  static inline int i2c_master_recv_dmasafe(const struct i2c_client *client,
> -					  char *buf, int count)
> +					  void *buf, int count)
>  {
>  	return i2c_transfer_buffer_flags(client, buf, count,
>  					 I2C_M_RD | I2C_M_DMA_SAFE);
> @@ -92,9 +92,10 @@ static inline int i2c_master_recv_dmasafe(const struct i2c_client *client,
>   * Returns negative errno, or else the number of bytes written.
>   */
>  static inline int i2c_master_send(const struct i2c_client *client,
> -				  const char *buf, int count)
> +				  const void *buf, int count)
>  {
> -	return i2c_transfer_buffer_flags(client, (char *)buf, count, 0);
> +	return i2c_transfer_buffer_flags(client, (void *)buf /* const cast */,
> +					 count, 0);
>  };
>  
>  /**
> @@ -107,10 +108,10 @@ static inline int i2c_master_send(const struct i2c_client *client,
>   * Returns negative errno, or else the number of bytes written.
>   */
>  static inline int i2c_master_send_dmasafe(const struct i2c_client *client,
> -					  const char *buf, int count)
> +					  const void *buf, int count)
>  {
> -	return i2c_transfer_buffer_flags(client, (char *)buf, count,
> -					 I2C_M_DMA_SAFE);
> +	return i2c_transfer_buffer_flags(client, (void *)buf /* const cast */,
> +					 count, I2C_M_DMA_SAFE);
>  };
>  
>  /* Transfer num messages.
> @@ -166,18 +167,19 @@ i2c_smbus_write_word_swapped(const struct i2c_client *client,
>  
>  /* Returns the number of read bytes */
>  extern s32 i2c_smbus_read_block_data(const struct i2c_client *client,
> -				     u8 command, u8 *values);
> +				     u8 command, void *values);
>  extern s32 i2c_smbus_write_block_data(const struct i2c_client *client,
> -				      u8 command, u8 length, const u8 *values);
> +				      u8 command, u8 length,
> +				      const void *values);
>  /* Returns the number of read bytes */
>  extern s32 i2c_smbus_read_i2c_block_data(const struct i2c_client *client,
> -					 u8 command, u8 length, u8 *values);
> +					 u8 command, u8 length, void *values);
>  extern s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client,
>  					  u8 command, u8 length,
> -					  const u8 *values);
> +					  const void *values);
>  extern s32
>  i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> -					  u8 command, u8 length, u8 *values);
> +					  u8 command, u8 length, void *values);
>  int i2c_get_device_id(const struct i2c_client *client,
>  		      struct i2c_device_identity *id);
>  #endif /* I2C */
Dmitry Torokhov Nov. 12, 2019, 7:11 p.m. UTC | #3
On Tue, Nov 12, 2019 at 09:45:56AM +0100, Luca Ceresoli wrote:
> Hi Dmitry,
> 
> On 12/11/19 01:58, Dmitry Torokhov wrote:
> > There is no need to force users of i2c_master_send()/i2c_master_recv()
> > and other i2c read/write bulk data API to cast everything into u8
> > pointers.  While everything can be considered byte stream, the drivers
> > are usually work with more structured data.
> > 
> > Let's switch the APIs to accept [const] void pointers to cut amount of
> > casting needed.
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> I agree on the principle, but I have a question, see below.
> 
> [...]
> >  s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> > -					      u8 command, u8 length, u8 *values)
> > +					      u8 command, u8 length, void *values)
> >  {
> >  	u8 i = 0;
> >  	int status;
> > @@ -647,8 +648,7 @@ s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> >  			status = i2c_smbus_read_word_data(client, command + i);
> >  			if (status < 0)
> >  				return status;
> > -			values[i] = status & 0xff;
> > -			values[i + 1] = status >> 8;
> > +			put_unaligned_le16(status, values + i);
> 
> The switch to put_unaligned_le16() looks unrelated, is it?

Yeah, I'll split it out.

> 
> >  			i += 2;
> >  		}
> >  	}
> > @@ -657,7 +657,7 @@ s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> >  		status = i2c_smbus_read_byte_data(client, command + i);
> >  		if (status < 0)
> >  			return status;
> > -		values[i] = status;
> > +		*(u8 *)(values + i) = status;
> 
> My preference is to use an u8* helper variable in these cases:

Sure, I can do this.

> 
> s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client
> *client,
> -			      u8 command, u8 length, u8 *values)
> +			      u8 command, u8 length, void *buf)
>  {
> +	u8 *bytes = buf;
> @@
> -		values[i] = status;
> +		bytes[i] = status;
> 
> This clarifies we are accessing the raw bytes, avoids typecasts in the
> middle of code for readability and avoids void pointer math.
> 
> PS: look, it's exactly what you do in the max1363.c file below! :)
> 
> > diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> > index 5c2cc61b666e7..48ed76a0e83d4 100644
> > --- a/drivers/iio/adc/max1363.c
> > +++ b/drivers/iio/adc/max1363.c
> > @@ -182,9 +182,9 @@ struct max1363_state {
> >  	struct regulator		*vref;
> >  	u32				vref_uv;
> >  	int				(*send)(const struct i2c_client *client,
> > -						const char *buf, int count);
> > +						const void *buf, int count);
> >  	int				(*recv)(const struct i2c_client *client,
> > -						char *buf, int count);
> > +						void *buf, int count);
> >  };
> >  
> >  #define MAX1363_MODE_SINGLE(_num, _mask) {				\
> > @@ -310,27 +310,29 @@ static const struct max1363_mode
> >  	return NULL;
> >  }
> >  
> > -static int max1363_smbus_send(const struct i2c_client *client, const char *buf,
> > +static int max1363_smbus_send(const struct i2c_client *client, const void *buf,
> >  		int count)
> >  {
> > +	const u8 *data = buf;
> 
> Here it is! ^
>

Thanks for the review!
diff mbox series

Patch

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 6a5183cffdfc3..aeb4201ef55e4 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2065,7 +2065,7 @@  EXPORT_SYMBOL(i2c_transfer);
  *
  * Returns negative errno, or else the number of bytes transferred.
  */
-int i2c_transfer_buffer_flags(const struct i2c_client *client, char *buf,
+int i2c_transfer_buffer_flags(const struct i2c_client *client, void *buf,
 			      int count, u16 flags)
 {
 	int ret;
diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index 3ac426a8ab5ab..4172f8e647f81 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -15,6 +15,7 @@ 
 #include <linux/i2c.h>
 #include <linux/i2c-smbus.h>
 #include <linux/slab.h>
+#include <asm/unaligned.h>
 
 #include "i2c-core.h"
 
@@ -213,7 +214,7 @@  EXPORT_SYMBOL(i2c_smbus_write_word_data);
  * mechanism (I2C_M_RECV_LEN) which may not be implemented.
  */
 s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command,
-			      u8 *values)
+			      void *values)
 {
 	union i2c_smbus_data data;
 	int status;
@@ -240,7 +241,7 @@  EXPORT_SYMBOL(i2c_smbus_read_block_data);
  * else zero on success.
  */
 s32 i2c_smbus_write_block_data(const struct i2c_client *client, u8 command,
-			       u8 length, const u8 *values)
+			       u8 length, const void *values)
 {
 	union i2c_smbus_data data;
 
@@ -256,7 +257,7 @@  EXPORT_SYMBOL(i2c_smbus_write_block_data);
 
 /* Returns the number of read bytes */
 s32 i2c_smbus_read_i2c_block_data(const struct i2c_client *client, u8 command,
-				  u8 length, u8 *values)
+				  u8 length, void *values)
 {
 	union i2c_smbus_data data;
 	int status;
@@ -276,7 +277,7 @@  s32 i2c_smbus_read_i2c_block_data(const struct i2c_client *client, u8 command,
 EXPORT_SYMBOL(i2c_smbus_read_i2c_block_data);
 
 s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client, u8 command,
-				   u8 length, const u8 *values)
+				   u8 length, const void *values)
 {
 	union i2c_smbus_data data;
 
@@ -628,7 +629,7 @@  EXPORT_SYMBOL(__i2c_smbus_xfer);
  * transfer.
  */
 s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
-					      u8 command, u8 length, u8 *values)
+					      u8 command, u8 length, void *values)
 {
 	u8 i = 0;
 	int status;
@@ -647,8 +648,7 @@  s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
 			status = i2c_smbus_read_word_data(client, command + i);
 			if (status < 0)
 				return status;
-			values[i] = status & 0xff;
-			values[i + 1] = status >> 8;
+			put_unaligned_le16(status, values + i);
 			i += 2;
 		}
 	}
@@ -657,7 +657,7 @@  s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
 		status = i2c_smbus_read_byte_data(client, command + i);
 		if (status < 0)
 			return status;
-		values[i] = status;
+		*(u8 *)(values + i) = status;
 		i++;
 	}
 
diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
index 5c2cc61b666e7..48ed76a0e83d4 100644
--- a/drivers/iio/adc/max1363.c
+++ b/drivers/iio/adc/max1363.c
@@ -182,9 +182,9 @@  struct max1363_state {
 	struct regulator		*vref;
 	u32				vref_uv;
 	int				(*send)(const struct i2c_client *client,
-						const char *buf, int count);
+						const void *buf, int count);
 	int				(*recv)(const struct i2c_client *client,
-						char *buf, int count);
+						void *buf, int count);
 };
 
 #define MAX1363_MODE_SINGLE(_num, _mask) {				\
@@ -310,27 +310,29 @@  static const struct max1363_mode
 	return NULL;
 }
 
-static int max1363_smbus_send(const struct i2c_client *client, const char *buf,
+static int max1363_smbus_send(const struct i2c_client *client, const void *buf,
 		int count)
 {
+	const u8 *data = buf;
 	int i, err;
 
 	for (i = err = 0; err == 0 && i < count; ++i)
-		err = i2c_smbus_write_byte(client, buf[i]);
+		err = i2c_smbus_write_byte(client, data[i]);
 
 	return err ? err : count;
 }
 
-static int max1363_smbus_recv(const struct i2c_client *client, char *buf,
+static int max1363_smbus_recv(const struct i2c_client *client, void *buf,
 		int count)
 {
+	u8 *data = buf;
 	int i, ret;
 
 	for (i = 0; i < count; ++i) {
 		ret = i2c_smbus_read_byte(client);
 		if (ret < 0)
 			return ret;
-		buf[i] = ret;
+		data[i] = ret;
 	}
 
 	return count;
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index aaf57d9b41dbb..64cf92e191aa8 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -51,7 +51,7 @@  struct property_entry;
  * @count must be be less than 64k since msg.len is u16.
  */
 extern int i2c_transfer_buffer_flags(const struct i2c_client *client,
-				     char *buf, int count, u16 flags);
+				     void *buf, int count, u16 flags);
 
 /**
  * i2c_master_recv - issue a single I2C message in master receive mode
@@ -62,7 +62,7 @@  extern int i2c_transfer_buffer_flags(const struct i2c_client *client,
  * Returns negative errno, or else the number of bytes read.
  */
 static inline int i2c_master_recv(const struct i2c_client *client,
-				  char *buf, int count)
+				  void *buf, int count)
 {
 	return i2c_transfer_buffer_flags(client, buf, count, I2C_M_RD);
 };
@@ -77,7 +77,7 @@  static inline int i2c_master_recv(const struct i2c_client *client,
  * Returns negative errno, or else the number of bytes read.
  */
 static inline int i2c_master_recv_dmasafe(const struct i2c_client *client,
-					  char *buf, int count)
+					  void *buf, int count)
 {
 	return i2c_transfer_buffer_flags(client, buf, count,
 					 I2C_M_RD | I2C_M_DMA_SAFE);
@@ -92,9 +92,10 @@  static inline int i2c_master_recv_dmasafe(const struct i2c_client *client,
  * Returns negative errno, or else the number of bytes written.
  */
 static inline int i2c_master_send(const struct i2c_client *client,
-				  const char *buf, int count)
+				  const void *buf, int count)
 {
-	return i2c_transfer_buffer_flags(client, (char *)buf, count, 0);
+	return i2c_transfer_buffer_flags(client, (void *)buf /* const cast */,
+					 count, 0);
 };
 
 /**
@@ -107,10 +108,10 @@  static inline int i2c_master_send(const struct i2c_client *client,
  * Returns negative errno, or else the number of bytes written.
  */
 static inline int i2c_master_send_dmasafe(const struct i2c_client *client,
-					  const char *buf, int count)
+					  const void *buf, int count)
 {
-	return i2c_transfer_buffer_flags(client, (char *)buf, count,
-					 I2C_M_DMA_SAFE);
+	return i2c_transfer_buffer_flags(client, (void *)buf /* const cast */,
+					 count, I2C_M_DMA_SAFE);
 };
 
 /* Transfer num messages.
@@ -166,18 +167,19 @@  i2c_smbus_write_word_swapped(const struct i2c_client *client,
 
 /* Returns the number of read bytes */
 extern s32 i2c_smbus_read_block_data(const struct i2c_client *client,
-				     u8 command, u8 *values);
+				     u8 command, void *values);
 extern s32 i2c_smbus_write_block_data(const struct i2c_client *client,
-				      u8 command, u8 length, const u8 *values);
+				      u8 command, u8 length,
+				      const void *values);
 /* Returns the number of read bytes */
 extern s32 i2c_smbus_read_i2c_block_data(const struct i2c_client *client,
-					 u8 command, u8 length, u8 *values);
+					 u8 command, u8 length, void *values);
 extern s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client,
 					  u8 command, u8 length,
-					  const u8 *values);
+					  const void *values);
 extern s32
 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
-					  u8 command, u8 length, u8 *values);
+					  u8 command, u8 length, void *values);
 int i2c_get_device_id(const struct i2c_client *client,
 		      struct i2c_device_identity *id);
 #endif /* I2C */