Skip to contentSkip to conversation

i2c: use void pointers for supplying data for reads and writes

PATCH New Dmitry Torokhov 2 participants 3 messages Apr 1, 2017 – Apr 3, 2017

Subsystems (2)
I2C subsystem host drivers I2C
lore
Share & tools
lore

Dmitry Torokhov

author
#0 lore raw

PATCH 1/1 i2c: use void pointers for supplying data for reads and writes

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 u* 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 <[email protected]>
---
 drivers/i2c/i2c-core.c | 23 ++++++++++++-----------
 include/linux/i2c.h    | 15 ++++++++-------
 2 files changed, 20 insertions(+), 18 deletions(-)
drivers/i2c/i2c-core.c +12 -11
index 9ff5ce3d5fba..6efeba42d10b 100644
@@ -56,6 +56,7 @@
 #include <linux/property.h>
 #include <linux/rwsem.h>
 #include <linux/slab.h>
+#include <asm/unaligned.h>
 
 #include "i2c-core.h"
 

@@ -2817,7 +2818,7 @@ EXPORT_SYMBOL(i2c_transfer);
  *
  * Returns negative errno, or else the number of bytes written.
  */
-int i2c_master_send(const struct i2c_client *client, const char *buf, int count)
+int i2c_master_send(const struct i2c_client *client, const void *buf, int count)
 {
 	int ret;
 	struct i2c_adapter *adap = client->adapter;

@@ -2826,7 +2827,7 @@ int i2c_master_send(const struct i2c_client *client, const char *buf, int count)
 	msg.addr = client->addr;
 	msg.flags = client->flags & I2C_M_TEN;
 	msg.len = count;
-	msg.buf = (char *)buf;
+	msg.buf = (u8 *)buf; /* cast away const */
 
 	ret = i2c_transfer(adap, &msg, 1);
 

@@ -2846,7 +2847,7 @@ EXPORT_SYMBOL(i2c_master_send);
  *
  * Returns negative errno, or else the number of bytes read.
  */
-int i2c_master_recv(const struct i2c_client *client, char *buf, int count)
+int i2c_master_recv(const struct i2c_client *client, void *buf, int count)
 {
 	struct i2c_adapter *adap = client->adapter;
 	struct i2c_msg msg;

@@ -3290,7 +3291,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;

@@ -3317,7 +3318,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;
 

@@ -3333,7 +3334,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;

@@ -3353,7 +3354,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;
 

@@ -3633,7 +3634,8 @@ 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;

@@ -3652,8 +3654,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;
 		}
 	}

@@ -3662,7 +3663,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++;
 	}
include/linux/i2c.h +8 -7
index b4b6583d0e07..d9f9601fce4c 100644
@@ -62,9 +62,9 @@ struct property_entry;
  * transmit an arbitrary number of messages without interruption.
  * @count must be be less than 64k since msg.len is u16.
  */
-extern int i2c_master_send(const struct i2c_client *client, const char *buf,
+extern int i2c_master_send(const struct i2c_client *client, const void *buf,
 			   int count);
-extern int i2c_master_recv(const struct i2c_client *client, char *buf,
+extern int i2c_master_recv(const struct i2c_client *client, void *buf,
 			   int count);
 
 /* Transfer num messages.

@@ -115,18 +115,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);
 #endif /* I2C */
 
 enum i2c_alert_protocol {
Signature · 5 lines
2.12.2.564.g063fe858b8-goog


-- 
Dmitry
build bot
#1 lore raw
reproduce:

Hi Dmitry,

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.11-rc4 next-20170331]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Torokhov/i2c-use-void-pointers-for-supplying-data-for-reads-and-writes/20170403-074306
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: i386-randconfig-i0-201714 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386

All warnings (new ones prefixed by >>):

drivers/iio/adc/max1363.c: In function 'max1363_probe':

>> drivers/iio/adc/max1363.c:1633:12: warning: assignment from incompatible pointer type [enabled by default]
      st->send = i2c_master_send;
               ^
   drivers/iio/adc/max1363.c:1634:12: warning: assignment from incompatible pointer type [enabled by default]
      st->recv = i2c_master_recv;
               ^

vim +1633 drivers/iio/adc/max1363.c

a405b00e Guenter Roeck  2013-02-03  1617  	if (!IS_ERR(vref)) {
a405b00e Guenter Roeck  2013-02-03  1618  		int vref_uv;
a405b00e Guenter Roeck  2013-02-03  1619  
a405b00e Guenter Roeck  2013-02-03  1620  		ret = regulator_enable(vref);
a405b00e Guenter Roeck  2013-02-03  1621  		if (ret)
a405b00e Guenter Roeck  2013-02-03  1622  			goto error_disable_reg;
a405b00e Guenter Roeck  2013-02-03  1623  		st->vref = vref;
a405b00e Guenter Roeck  2013-02-03  1624  		vref_uv = regulator_get_voltage(vref);
a405b00e Guenter Roeck  2013-02-03  1625  		if (vref_uv <= 0) {
a405b00e Guenter Roeck  2013-02-03  1626  			ret = -EINVAL;
a405b00e Guenter Roeck  2013-02-03  1627  			goto error_disable_reg;
a405b00e Guenter Roeck  2013-02-03  1628  		}
a405b00e Guenter Roeck  2013-02-03  1629  		st->vref_uv = vref_uv;
a405b00e Guenter Roeck  2013-02-03  1630  	}
a405b00e Guenter Roeck  2013-02-03  1631  
61bdda69 Vivien Didelot 2013-10-20  1632  	if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
61bdda69 Vivien Didelot 2013-10-20 @1633  		st->send = i2c_master_send;
61bdda69 Vivien Didelot 2013-10-20  1634  		st->recv = i2c_master_recv;
61bdda69 Vivien Didelot 2013-10-20  1635  	} else if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE)
61bdda69 Vivien Didelot 2013-10-20  1636  			&& st->chip_info->bits == 8) {
61bdda69 Vivien Didelot 2013-10-20  1637  		st->send = max1363_smbus_send;
61bdda69 Vivien Didelot 2013-10-20  1638  		st->recv = max1363_smbus_recv;
61bdda69 Vivien Didelot 2013-10-20  1639  	} else {
61bdda69 Vivien Didelot 2013-10-20  1640  		ret = -EOPNOTSUPP;
61bdda69 Vivien Didelot 2013-10-20  1641  		goto error_disable_reg;

:::::: The code at line 1633 was first introduced by commit
:::::: 61bdda69222c09efd8943d240cd2a8e0bb659d82 iio:adc:max1363 support SMBus for 8-bit devices

:::::: TO: Vivien Didelot <[email protected]> :::::: CC: Jonathan Cameron <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

build bot
#2 lore raw
In reply to Dmitry Torokhov
reproduce:

Hi Dmitry,

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.11-rc4 next-20170331]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Torokhov/i2c-use-void-pointers-for-supplying-data-for-reads-and-writes/20170403-074306
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: i386-randconfig-a0-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386

All errors (new ones prefixed by >>):

drivers/iio/adc/max1363.c: In function 'max1363_probe':

>> drivers/iio/adc/max1363.c:1633:12: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
      st->send = i2c_master_send;
               ^
   drivers/iio/adc/max1363.c:1634:12: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
      st->recv = i2c_master_recv;
               ^

Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:constant_test_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:variable_test_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls
Cyclomatic Complexity 1 arch/x86/include/asm/arch_hweight.h:__arch_hweight32 Cyclomatic Complexity 2 include/linux/bitops.h:hweight_long
Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u32
Cyclomatic Complexity 1 include/linux/kernel.h:kstrtoul
Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order Cyclomatic Complexity 3 include/linux/bitmap.h:bitmap_copy
Cyclomatic Complexity 3 include/linux/bitmap.h:bitmap_subset
Cyclomatic Complexity 3 include/linux/bitmap.h:bitmap_weight
Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
Cyclomatic Complexity 1 include/linux/err.h:IS_ERR
Cyclomatic Complexity 1 include/linux/device.h:devm_kzalloc
Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata Cyclomatic Complexity 1 include/linux/i2c.h:i2c_get_clientdata Cyclomatic Complexity 1 include/linux/i2c.h:i2c_set_clientdata Cyclomatic Complexity 1 include/linux/i2c.h:i2c_get_functionality Cyclomatic Complexity 1 include/linux/i2c.h:i2c_check_functionality Cyclomatic Complexity 28 include/linux/slab.h:kmalloc_index
Cyclomatic Complexity 67 include/linux/slab.h:kmalloc_large
Cyclomatic Complexity 5 include/linux/slab.h:kmalloc
Cyclomatic Complexity 1 include/linux/iio/iio.h:dev_to_iio_dev Cyclomatic Complexity 1 include/linux/iio/iio.h:iio_priv
Cyclomatic Complexity 1 include/linux/iio/iio.h:iio_buffer_enabled Cyclomatic Complexity 2 include/linux/iio/buffer.h:iio_push_to_buffers_with_timestamp Cyclomatic Complexity 4 drivers/iio/adc/max1363.c:max1363_match_mode Cyclomatic Complexity 1 drivers/iio/adc/max1363.c:max1363_write_basic_config Cyclomatic Complexity 1 drivers/iio/adc/max1363.c:max1363_set_scan_mode Cyclomatic Complexity 2 drivers/iio/adc/max1363.c:max1363_read_thresh Cyclomatic Complexity 7 drivers/iio/adc/max1363.c:max1363_write_thresh Cyclomatic Complexity 6 drivers/iio/adc/max1363.c:__max1363_check_event_mask Cyclomatic Complexity 2 drivers/iio/adc/max1363.c:max1363_update_scan_mode Cyclomatic Complexity 2 drivers/iio/adc/max1363.c:max1363_initial_setup Cyclomatic Complexity 3 drivers/iio/adc/max1363.c:max1363_alloc_scan_masks Cyclomatic Complexity 8 drivers/iio/adc/max1363.c:max1363_read_single_chan Cyclomatic Complexity 4 drivers/iio/adc/max1363.c:max1363_read_raw Cyclomatic Complexity 2 drivers/iio/adc/max1363.c:max1363_read_event_config Cyclomatic Complexity 5 drivers/iio/adc/max1363.c:max1363_monitor_store_freq Cyclomatic Complexity 16 drivers/iio/adc/max1363.c:max1363_monitor_mode_update Cyclomatic Complexity 6 drivers/iio/adc/max1363.c:max1363_write_event_config Cyclomatic Complexity 1 drivers/iio/adc/max1363.c:max1363_monitor_show_freq Cyclomatic Complexity 2 drivers/iio/adc/max1363.c:max1363_remove Cyclomatic Complexity 19 drivers/iio/adc/max1363.c:max1363_probe Cyclomatic Complexity 2 drivers/iio/adc/max1363.c:max1363_event_handler Cyclomatic Complexity 8 drivers/iio/adc/max1363.c:max1363_trigger_handler Cyclomatic Complexity 3 drivers/iio/adc/max1363.c:max1363_smbus_recv Cyclomatic Complexity 3 drivers/iio/adc/max1363.c:max1363_smbus_send Cyclomatic Complexity 1 drivers/iio/adc/max1363.c:max1363_driver_init Cyclomatic Complexity 1 drivers/iio/adc/max1363.c:max1363_driver_exit
cc1: some warnings being treated as errors

vim +1633 drivers/iio/adc/max1363.c

a405b00e Guenter Roeck  2013-02-03  1617  	if (!IS_ERR(vref)) {
a405b00e Guenter Roeck  2013-02-03  1618  		int vref_uv;
a405b00e Guenter Roeck  2013-02-03  1619  
a405b00e Guenter Roeck  2013-02-03  1620  		ret = regulator_enable(vref);
a405b00e Guenter Roeck  2013-02-03  1621  		if (ret)
a405b00e Guenter Roeck  2013-02-03  1622  			goto error_disable_reg;
a405b00e Guenter Roeck  2013-02-03  1623  		st->vref = vref;
a405b00e Guenter Roeck  2013-02-03  1624  		vref_uv = regulator_get_voltage(vref);
a405b00e Guenter Roeck  2013-02-03  1625  		if (vref_uv <= 0) {
a405b00e Guenter Roeck  2013-02-03  1626  			ret = -EINVAL;
a405b00e Guenter Roeck  2013-02-03  1627  			goto error_disable_reg;
a405b00e Guenter Roeck  2013-02-03  1628  		}
a405b00e Guenter Roeck  2013-02-03  1629  		st->vref_uv = vref_uv;
a405b00e Guenter Roeck  2013-02-03  1630  	}
a405b00e Guenter Roeck  2013-02-03  1631  
61bdda69 Vivien Didelot 2013-10-20  1632  	if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
61bdda69 Vivien Didelot 2013-10-20 @1633  		st->send = i2c_master_send;
61bdda69 Vivien Didelot 2013-10-20  1634  		st->recv = i2c_master_recv;
61bdda69 Vivien Didelot 2013-10-20  1635  	} else if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE)
61bdda69 Vivien Didelot 2013-10-20  1636  			&& st->chip_info->bits == 8) {
61bdda69 Vivien Didelot 2013-10-20  1637  		st->send = max1363_smbus_send;
61bdda69 Vivien Didelot 2013-10-20  1638  		st->recv = max1363_smbus_recv;
61bdda69 Vivien Didelot 2013-10-20  1639  	} else {
61bdda69 Vivien Didelot 2013-10-20  1640  		ret = -EOPNOTSUPP;
61bdda69 Vivien Didelot 2013-10-20  1641  		goto error_disable_reg;

:::::: The code at line 1633 was first introduced by commit
:::::: 61bdda69222c09efd8943d240cd2a8e0bb659d82 iio:adc:max1363 support SMBus for 8-bit devices

:::::: TO: Vivien Didelot <[email protected]> :::::: CC: Jonathan Cameron <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation