Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 6 Aug 2022 15:44:12 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     Jason Gerecke <killertofu@gmail.com>
Cc:     linux-i2c@vger.kernel.org, linux-iio <linux-iio@vger.kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Wolfram Sang <wsa-dev@sang-engineering.com>,
        Andy Shevchenko <andy.shevchenko@gmail.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Ping Cheng <pinglinux@gmail.com>,
        "Tobita, Tatsunosuke" <tatsunosuke.tobita@wacom.com>,
        Jason Gerecke <jason.gerecke@wacom.com>,
        Ping Cheng <ping.cheng@wacom.com>
Subject: Re: [PATCH v2] i2c: Use u8 type in i2c transfer calls
Message-ID: <20220806154412.581a4f83@jic23-huawei>
In-Reply-To: <20220803145937.698603-1-jason.gerecke@wacom.com>
References: <20220718153448.173652-1-jason.gerecke@wacom.com>
        <20220803145937.698603-1-jason.gerecke@wacom.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed,  3 Aug 2022 07:59:37 -0700
Jason Gerecke <killertofu@gmail.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@wacom.com>
> Reviewed-by: Ping Cheng <ping.cheng@wacom.com>

With the minor stuff Andy raised tidied up I'm fine with this change.

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.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);
>  };
>