Re: [PATCH v3 1/3] i2c: use void pointers for supplying data for reads and writes
From: Uwe Kleine-König <hidden>
Date: 2019-11-18 07:44:18
Also in:
linux-iio, lkml
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@huawei.com> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
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.
quoted hunk ↗ jump to hunk
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?
quoted hunk ↗ jump to hunk
@@ -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/ |