[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220806154412.581a4f83@jic23-huawei>
Date: Sat, 6 Aug 2022 15:44:12 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Jason Gerecke <killertofu@...il.com>
Cc: linux-i2c@...r.kernel.org, linux-iio <linux-iio@...r.kernel.org>,
Lars-Peter Clausen <lars@...afoo.de>,
Wolfram Sang <wsa-dev@...g-engineering.com>,
Andy Shevchenko <andy.shevchenko@...il.com>,
LKML <linux-kernel@...r.kernel.org>,
Ping Cheng <pinglinux@...il.com>,
"Tobita, Tatsunosuke" <tatsunosuke.tobita@...om.com>,
Jason Gerecke <jason.gerecke@...om.com>,
Ping Cheng <ping.cheng@...om.com>
Subject: Re: [PATCH v2] i2c: Use u8 type in i2c transfer calls
On Wed, 3 Aug 2022 07:59:37 -0700
Jason Gerecke <killertofu@...il.com> wrote:
> The 'i2c_transfer_buffer_flags' function (and related inlines) defines its
> 'buf' argument to be of type 'char*'. This is a poor choice of type given
> that most callers actually pass a 'u8*' and that the function itself ends
> up just storing the variable to a 'u8*'-typed member of 'struct i2c_msg'
> anyway.
>
> Changing the type of the 'buf' argument to 'u8*' vastly reduces the number
> of (admittedly usually-silent) Wpointer-sign warnings that are generated
> as the types get needlessly juggled back and forth.
>
> At the same time, update the max1363 driver to match the new interface so
> we don't introduce a new Wincompatible-function-pointer-types warning.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@...om.com>
> Reviewed-by: Ping Cheng <ping.cheng@...om.com>
With the minor stuff Andy raised tidied up I'm fine with this change.
Acked-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
I'd forgotten all about the oddities of the max1363 :) That brings
back some memories!
Jonathan
> ---
> Changes in v2:
> - Added modifications to the max1363 driver required to avoid warnings
>
> drivers/i2c/i2c-core-base.c | 2 +-
> drivers/iio/adc/max1363.c | 8 ++++----
> include/linux/i2c.h | 14 +++++++-------
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 10f35f942066a..2925507e8626d 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2184,7 +2184,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, u8 *buf,
> int count, u16 flags)
> {
> int ret;
> diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> index eef55ed4814a6..ebe6eb99583da 100644
> --- a/drivers/iio/adc/max1363.c
> +++ b/drivers/iio/adc/max1363.c
> @@ -184,9 +184,9 @@ struct max1363_state {
> struct regulator *vref;
> u32 vref_uv;
> int (*send)(const struct i2c_client *client,
> - const char *buf, int count);
> + const u8 *buf, int count);
> int (*recv)(const struct i2c_client *client,
> - char *buf, int count);
> + u8 *buf, int count);
> };
>
> #define MAX1363_MODE_SINGLE(_num, _mask) { \
> @@ -312,7 +312,7 @@ 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 u8 *buf,
> int count)
> {
> int i, err;
> @@ -323,7 +323,7 @@ static int max1363_smbus_send(const struct i2c_client *client, const char *buf,
> 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, u8 *buf,
> int count)
> {
> int i, ret;
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 8eab5017bff30..3a94385f4642c 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -64,7 +64,7 @@ const char *i2c_freq_mode_string(u32 bus_freq_hz);
> * @count must be less than 64k since msg.len is u16.
> */
> int i2c_transfer_buffer_flags(const struct i2c_client *client,
> - char *buf, int count, u16 flags);
> + u8 *buf, int count, u16 flags);
>
> /**
> * i2c_master_recv - issue a single I2C message in master receive mode
> @@ -75,7 +75,7 @@ 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)
> + u8 *buf, int count)
> {
> return i2c_transfer_buffer_flags(client, buf, count, I2C_M_RD);
> };
> @@ -90,7 +90,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)
> + u8 *buf, int count)
> {
> return i2c_transfer_buffer_flags(client, buf, count,
> I2C_M_RD | I2C_M_DMA_SAFE);
> @@ -105,9 +105,9 @@ 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 u8 *buf, int count)
> {
> - return i2c_transfer_buffer_flags(client, (char *)buf, count, 0);
> + return i2c_transfer_buffer_flags(client, (u8 *)buf, count, 0);
> };
>
> /**
> @@ -120,9 +120,9 @@ 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 u8 *buf, int count)
> {
> - return i2c_transfer_buffer_flags(client, (char *)buf, count,
> + return i2c_transfer_buffer_flags(client, (u8 *)buf, count,
> I2C_M_DMA_SAFE);
> };
>
Powered by blists - more mailing lists