Hello Dmitry,
On Tue, Nov 12, 2019 at 12:31:30PM -0800, 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.
>
> Acked-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
Can you give an example where you save some casts? Given that i2c is a
byte oriented protocol (as opposed to for example spi) I think it's a
good idea to expose this in the API.
> 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
This change isn't motivated in the commit log. Is this here by mistake?
> @@ -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]);
Isn't this hunk an indicator that keeping char (or u8) as type of the
members of buf is a good idea?
> 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;
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
