[PATCH v3 0/2] Add i2c-cp2615

[PATCH v3 0/2] Add i2c-cp2615

[Bence Csókás]

For a hardware project, I need the I2C master of SiLabs' CP2615 chip
to be visible from under Linux. This patchset adds i2c-cp2615, a
driver which sets up an i2c_adapter for said chip.

Changes in v3:
* Fixed SPDX header
* Added I2C_AQ_NO_REP_START adapter quirk to i2c.h
* Added I2C_AQ_NO_ZERO_LEN | I2C_AQ_NO_REP_START to CP2615's quirk flags
* Made the driver only bind to the IOP interface
* Return -ENXIO instead of -ECOMM when the device does not ACK
* Formatting, fix almost all warnings and checks from `checkpatch --strict`
  * CamelCase checks remain. These identifiers were taken from SiLabs'
    Application Note, I thought it made sense to preserve these as-is.

Bence Csókás (2):
  i2c: Add I2C_AQ_NO_REP_START adapter quirk
  Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio
    Bridge

 drivers/i2c/busses/Kconfig      |  10 ++
 drivers/i2c/busses/Makefile     |   1 +
 drivers/i2c/busses/i2c-cp2615.c | 279 ++++++++++++++++++++++++++++++++
 include/linux/i2c.h             |   2 +
 4 files changed, 292 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-cp2615.c

-- 
2.31.0

[PATCH v3 1/2] i2c: Add I2C_AQ_NO_REP_START adapter quirk

[Bence Csókás]

This quirk signifies that the adapter cannot do a repeated
START, it always issues a STOP condition after transfers.

Signed-off-by: Bence Csókás <bence98@sch.bme.hu>
---
 include/linux/i2c.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 56622658b215..a670ae129f4b 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -687,6 +687,8 @@ struct i2c_adapter_quirks {
 #define I2C_AQ_NO_ZERO_LEN_READ		BIT(5)
 #define I2C_AQ_NO_ZERO_LEN_WRITE	BIT(6)
 #define I2C_AQ_NO_ZERO_LEN		(I2C_AQ_NO_ZERO_LEN_READ | I2C_AQ_NO_ZERO_LEN_WRITE)
+/* adapter cannot do repeated START */
+#define I2C_AQ_NO_REP_START		BIT(7)
 
 /*
  * i2c_adapter is the structure used to identify a physical i2c bus along
-- 
2.31.0

[PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge

[Bence Csókás]

Create an i2c_adapter for CP2615's I2C master interface by
implementing parts of the CP2615's I/O Protocol (IOP)

Signed-off-by: Bence Csókás <bence98@sch.bme.hu>
---
 drivers/i2c/busses/Kconfig      |  10 ++
 drivers/i2c/busses/Makefile     |   1 +
 drivers/i2c/busses/i2c-cp2615.c | 279 ++++++++++++++++++++++++++++++++
 3 files changed, 290 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-cp2615.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index a49e0ed4a599..7a0dd18140d3 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1227,6 +1227,16 @@ config I2C_DLN2
 	 This driver can also be built as a module.  If so, the module
 	 will be called i2c-dln2.
 
+config I2C_CP2615
+	tristate "Silicon Labs CP2615 USB sound card and I2C adapter"
+	depends on USB
+	help
+	  If you say yes to this option, support will be included for Silicon
+	  Labs CP2615's I2C interface.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-cp2615.
+
 config I2C_PARPORT
 	tristate "Parallel port adapter"
 	depends on PARPORT
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 683c49faca05..adb71d9c9d42 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -127,6 +127,7 @@ obj-$(CONFIG_I2C_ZX2967)	+= i2c-zx2967.o
 # External I2C/SMBus adapter drivers
 obj-$(CONFIG_I2C_DIOLAN_U2C)	+= i2c-diolan-u2c.o
 obj-$(CONFIG_I2C_DLN2)		+= i2c-dln2.o
+obj-$(CONFIG_I2C_CP2615) += i2c-cp2615.o
 obj-$(CONFIG_I2C_PARPORT)	+= i2c-parport.o
 obj-$(CONFIG_I2C_ROBOTFUZZ_OSIF)	+= i2c-robotfuzz-osif.o
 obj-$(CONFIG_I2C_TAOS_EVM)	+= i2c-taos-evm.o
diff --git a/drivers/i2c/busses/i2c-cp2615.c b/drivers/i2c/busses/i2c-cp2615.c
new file mode 100644
index 000000000000..ca6dba3fc7a3
--- /dev/null
+++ b/drivers/i2c/busses/i2c-cp2615.c
@@ -0,0 +1,279 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * i2c support for Silicon Labs' CP2615 Digital Audio Bridge
+ *
+ * (c) 2021, Bence Csókás <bence98@sch.bme.hu>
+ */
+
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/usb.h>
+#include <linux/string.h>
+
+/** CP2615 I/O Protocol implementation */
+
+#define CP2615_VID 0x10c4
+#define CP2615_PID 0xeac1
+
+#define IOP_EP_IN  0x82
+#define IOP_EP_OUT 0x02
+#define IOP_IFN 1
+#define IOP_ALTSETTING 2
+
+#define MAX_IOP_SIZE 64
+#define MAX_IOP_PAYLOAD_SIZE (MAX_IOP_SIZE - 6)
+#define MAX_I2C_SIZE (MAX_IOP_PAYLOAD_SIZE - 4)
+
+enum cp2615_iop_msg_type {
+	iop_GetAccessoryInfo = 0xD100,
+	iop_AccessoryInfo = 0xA100,
+	iop_GetPortConfiguration = 0xD203,
+	iop_PortConfiguration = 0xA203,
+	iop_DoI2cTransfer = 0xD400,
+	iop_I2cTransferResult = 0xA400,
+	iop_GetSerialState = 0xD501,
+	iop_SerialState = 0xA501
+};
+
+struct cp2615_iop_msg {
+	u16 preamble, length, msg;
+	u8 data[MAX_IOP_PAYLOAD_SIZE];
+};
+
+#define PART_ID_A01 0x1400
+#define PART_ID_A02 0x1500
+
+struct cp2615_iop_accessory_info {
+	u16 part_id, option_id, proto_ver;
+};
+
+struct cp2615_i2c_transfer {
+	u8 tag, i2caddr, read_len, write_len;
+	u8 data[MAX_I2C_SIZE];
+};
+
+/** Possible values for struct cp2615_i2c_transfer_result.status */
+enum cp2615_i2c_status {
+	/* Writing to the internal EEPROM failed, because it is locked */
+	CP2615_CFG_LOCKED = -6,
+	/* read_len or write_len out of range */
+	CP2615_INVALID_PARAM = -4,
+	/* I2C slave did not ACK in time */
+	CP2615_TIMEOUT,
+	/* I2C bus busy */
+	CP2615_BUS_BUSY,
+	/* I2C bus error (ie. device NAK'd the request) */
+	CP2615_BUS_ERROR,
+	CP2615_SUCCESS
+};
+
+struct cp2615_i2c_transfer_result {
+	u8 tag, i2caddr;
+	s8 status;
+	u8 read_len;
+	u8 data[MAX_I2C_SIZE];
+};
+
+int cp2615_init_iop_msg(struct cp2615_iop_msg *ret, enum cp2615_iop_msg_type msg,
+			const void *data, size_t data_len)
+{
+	if (data_len > MAX_IOP_PAYLOAD_SIZE)
+		return -EFBIG;
+
+	if (!ret)
+		return -EINVAL;
+
+	ret->preamble = 0x2A2A;
+	ret->length = htons(data_len + 6);
+	ret->msg = htons(msg);
+	if (data && data_len)
+		memcpy(&ret->data, data, data_len);
+
+	return 0;
+}
+
+int cp2615_init_i2c_msg(struct cp2615_iop_msg *ret, const struct cp2615_i2c_transfer *data)
+{
+	return cp2615_init_iop_msg(ret, iop_DoI2cTransfer, data, 4 + data->write_len);
+}
+
+/* Translates status codes to Linux errno's */
+int cp2615_check_status(enum cp2615_i2c_status status)
+{
+	switch (status) {
+	case CP2615_SUCCESS:
+			return 0;
+	case CP2615_BUS_ERROR:
+		return -ENXIO;
+	case CP2615_BUS_BUSY:
+		return -EAGAIN;
+	case CP2615_TIMEOUT:
+		return -ETIMEDOUT;
+	case CP2615_INVALID_PARAM:
+		return -EINVAL;
+	case CP2615_CFG_LOCKED:
+		return -EPERM;
+	}
+	/* Unknown error code */
+	return -EPROTO;
+}
+
+static int
+cp2615_i2c_send(struct usb_interface *usbif, struct cp2615_i2c_transfer *i2c_w)
+{
+	struct cp2615_iop_msg *msg = kzalloc(sizeof(*msg), GFP_KERNEL);
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+	int res = cp2615_init_i2c_msg(msg, i2c_w);
+
+	if (!res)
+		res = usb_bulk_msg(usbdev, usb_sndbulkpipe(usbdev, IOP_EP_OUT),
+				   msg, ntohs(msg->length), NULL, 0);
+	kfree(msg);
+	return res;
+}
+
+static int
+cp2615_i2c_recv(struct usb_interface *usbif, unsigned char tag, void *buf)
+{
+	struct cp2615_iop_msg *msg = kzalloc(sizeof(*msg), GFP_KERNEL);
+	struct cp2615_i2c_transfer_result *i2c_r = (struct cp2615_i2c_transfer_result *)&msg->data;
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+	int res = usb_bulk_msg(usbdev, usb_rcvbulkpipe(usbdev, IOP_EP_IN),
+			       msg, sizeof(struct cp2615_iop_msg), NULL, 0);
+
+	if (res < 0)
+		return res;
+
+	if (msg->msg != htons(iop_I2cTransferResult) || i2c_r->tag != tag)
+		return -EIO;
+
+	res = cp2615_check_status(i2c_r->status);
+	if (res < 0)
+		return res;
+
+	memcpy(buf, &i2c_r->data, i2c_r->read_len);
+	kfree(msg);
+	return 0;
+}
+
+static int
+cp2615_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	struct usb_interface *usbif = adap->algo_data;
+	int i = 0, ret = 0;
+	struct i2c_msg *msg;
+	struct cp2615_i2c_transfer i2c_w = {0};
+
+	dev_dbg(&usbif->dev, "Doing %d I2C transactions\n", num);
+
+	for (; !ret && i < num; i++) {
+		msg = &msgs[i];
+
+		i2c_w.tag = 0xdd;
+		i2c_w.i2caddr = i2c_8bit_addr_from_msg(msg);
+		if (msg->flags & I2C_M_RD) {
+			i2c_w.read_len = msg->len;
+			i2c_w.write_len = 0;
+		} else {
+			i2c_w.read_len = 0;
+			i2c_w.write_len = msg->len;
+			memcpy(&i2c_w.data, msg->buf, i2c_w.write_len);
+		}
+		ret = cp2615_i2c_send(usbif, &i2c_w);
+		if (ret)
+			break;
+		ret = cp2615_i2c_recv(usbif, i2c_w.tag, msg->buf);
+	}
+	if (ret < 0)
+		return ret;
+	return i;
+}
+
+static u32
+cp2615_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm cp2615_i2c_algo = {
+	.master_xfer	= cp2615_i2c_master_xfer,
+	.functionality	= cp2615_i2c_func,
+};
+
+/*
+ * This chip has some limitations: one is that the USB endpoint
+ * can only receive 64 bytes/transfer, that leaves 54 bytes for
+ * the I2C transfer. On top of that, EITHER read_len OR write_len
+ * may be zero, but not both. If both are non-zero, the adapter
+ * issues a write followed by a read. And the chip does not
+ * support repeated START between the write and read phases.
+ */
+struct i2c_adapter_quirks cp2615_i2c_quirks = {
+	.max_write_len = MAX_I2C_SIZE,
+	.max_read_len = MAX_I2C_SIZE,
+	.flags = I2C_AQ_COMB_WRITE_THEN_READ | I2C_AQ_NO_ZERO_LEN | I2C_AQ_NO_REP_START,
+	.max_comb_1st_msg_len = MAX_I2C_SIZE,
+	.max_comb_2nd_msg_len = MAX_I2C_SIZE
+};
+
+static void
+cp2615_i2c_remove(struct usb_interface *usbif)
+{
+	struct i2c_adapter *adap = usb_get_intfdata(usbif);
+
+	usb_set_intfdata(usbif, NULL);
+	i2c_del_adapter(adap);
+}
+
+static int
+cp2615_i2c_probe(struct usb_interface *usbif, const struct usb_device_id *id)
+{
+	int ret = 0;
+	struct i2c_adapter *adap;
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+
+	ret = usb_set_interface(usbdev, IOP_IFN, IOP_ALTSETTING);
+	if (ret)
+		return ret;
+
+	adap = devm_kzalloc(&usbif->dev, sizeof(struct i2c_adapter), GFP_KERNEL);
+	if (!adap)
+		return -ENOMEM;
+
+	strncpy(adap->name, usbdev->serial, sizeof(adap->name));
+	adap->owner = THIS_MODULE;
+	adap->dev.parent = &usbif->dev;
+	adap->dev.of_node = usbif->dev.of_node;
+	adap->timeout = HZ;
+	adap->algo = &cp2615_i2c_algo;
+	adap->quirks = &cp2615_i2c_quirks;
+	adap->algo_data = usbif;
+
+	ret = i2c_add_adapter(adap);
+	if (ret)
+		return ret;
+
+	usb_set_intfdata(usbif, adap);
+	return ret;
+}
+
+static const struct usb_device_id id_table[] = {
+	{ USB_DEVICE_INTERFACE_NUMBER(CP2615_VID, CP2615_PID, IOP_IFN) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(usb, id_table);
+
+static struct usb_driver cp2615_i2c_driver = {
+	.name = "i2c-cp2615",
+	.probe = cp2615_i2c_probe,
+	.disconnect = cp2615_i2c_remove,
+	.id_table = id_table,
+};
+
+module_usb_driver(cp2615_i2c_driver);
+
+MODULE_AUTHOR("Bence Csókás <bence98@sch.bme.hu>");
+MODULE_DESCRIPTION("CP2615 I2C bus driver");
+MODULE_LICENSE("GPL");
-- 
2.31.0

Re: [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 9654 bytes --]

Hi "Bence,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v5.12-rc3 next-20210319]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bence-Cs-k-s/Add-i2c-cp2615/20210318-193822
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7aa4ceb301ef5116752aef6e09f6ff845dedc106
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bence-Cs-k-s/Add-i2c-cp2615/20210318-193822
        git checkout 7aa4ceb301ef5116752aef6e09f6ff845dedc106
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-cp2615.c:78:5: warning: no previous prototype for 'cp2615_init_iop_msg' [-Wmissing-prototypes]
      78 | int cp2615_init_iop_msg(struct cp2615_iop_msg *ret, enum cp2615_iop_msg_type msg,
         |     ^~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:96:5: warning: no previous prototype for 'cp2615_init_i2c_msg' [-Wmissing-prototypes]
      96 | int cp2615_init_i2c_msg(struct cp2615_iop_msg *ret, const struct cp2615_i2c_transfer *data)
         |     ^~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:102:5: warning: no previous prototype for 'cp2615_check_status' [-Wmissing-prototypes]
     102 | int cp2615_check_status(enum cp2615_i2c_status status)
         |     ^~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:266:1: warning: data definition has no type or storage class
     266 | MODULE_DEVICE_TABLE(usb, id_table);
         | ^~~~~~~~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-cp2615.c:266:1: error: type defaults to 'int' in declaration of 'MODULE_DEVICE_TABLE' [-Werror=implicit-int]
   drivers/i2c/busses/i2c-cp2615.c:266:1: warning: parameter names (without types) in function declaration
   In file included from include/linux/device.h:32,
                    from include/linux/acpi.h:15,
                    from include/linux/i2c.h:13,
                    from drivers/i2c/busses/i2c-cp2615.c:9:
   include/linux/device/driver.h:263:1: warning: data definition has no type or storage class
     263 | module_init(__driver##_init); \
         | ^~~~~~~~~~~
   include/linux/usb.h:1303:2: note: in expansion of macro 'module_driver'
    1303 |  module_driver(__usb_driver, usb_register, \
         |  ^~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:275:1: note: in expansion of macro 'module_usb_driver'
     275 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
>> include/linux/device/driver.h:263:1: error: type defaults to 'int' in declaration of 'module_init' [-Werror=implicit-int]
     263 | module_init(__driver##_init); \
         | ^~~~~~~~~~~
   include/linux/usb.h:1303:2: note: in expansion of macro 'module_driver'
    1303 |  module_driver(__usb_driver, usb_register, \
         |  ^~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:275:1: note: in expansion of macro 'module_usb_driver'
     275 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
   In file included from include/linux/linkage.h:7,
                    from include/linux/kernel.h:7,
                    from include/linux/list.h:9,
                    from include/linux/kobject.h:19,
                    from include/linux/of.h:17,
                    from include/linux/irqdomain.h:35,
                    from include/linux/acpi.h:13,
                    from include/linux/i2c.h:13,
                    from drivers/i2c/busses/i2c-cp2615.c:9:
   include/linux/export.h:19:30: warning: parameter names (without types) in function declaration
      19 | #define THIS_MODULE ((struct module *)0)
         |                              ^~~~~~
   include/linux/usb.h:1290:30: note: in expansion of macro 'THIS_MODULE'
    1290 |  usb_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
         |                              ^~~~~~~~~~~
   include/linux/device/driver.h:261:9: note: in expansion of macro 'usb_register'
     261 |  return __register(&(__driver) , ##__VA_ARGS__); \
         |         ^~~~~~~~~~
   include/linux/usb.h:1303:2: note: in expansion of macro 'module_driver'
    1303 |  module_driver(__usb_driver, usb_register, \
         |  ^~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:275:1: note: in expansion of macro 'module_usb_driver'
     275 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
   In file included from include/linux/device.h:32,
                    from include/linux/acpi.h:15,
                    from include/linux/i2c.h:13,
                    from drivers/i2c/busses/i2c-cp2615.c:9:
   include/linux/device/driver.h:268:1: warning: data definition has no type or storage class
     268 | module_exit(__driver##_exit);
         | ^~~~~~~~~~~
   include/linux/usb.h:1303:2: note: in expansion of macro 'module_driver'
    1303 |  module_driver(__usb_driver, usb_register, \
         |  ^~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:275:1: note: in expansion of macro 'module_usb_driver'
     275 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
>> include/linux/device/driver.h:268:1: error: type defaults to 'int' in declaration of 'module_exit' [-Werror=implicit-int]
     268 | module_exit(__driver##_exit);
         | ^~~~~~~~~~~
   include/linux/usb.h:1303:2: note: in expansion of macro 'module_driver'
    1303 |  module_driver(__usb_driver, usb_register, \
         |  ^~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:275:1: note: in expansion of macro 'module_usb_driver'
     275 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
   In file included from include/linux/linkage.h:7,
                    from include/linux/kernel.h:7,
                    from include/linux/list.h:9,
                    from include/linux/kobject.h:19,
                    from include/linux/of.h:17,
                    from include/linux/irqdomain.h:35,
                    from include/linux/acpi.h:13,
                    from include/linux/i2c.h:13,
                    from drivers/i2c/busses/i2c-cp2615.c:9:
   include/linux/export.h:19:30: warning: parameter names (without types) in function declaration
      19 | #define THIS_MODULE ((struct module *)0)
         |                              ^~~~~~
   include/linux/usb.h:1290:30: note: in expansion of macro 'THIS_MODULE'
    1290 |  usb_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
         |                              ^~~~~~~~~~~
   include/linux/device/driver.h:261:9: note: in expansion of macro 'usb_register'
     261 |  return __register(&(__driver) , ##__VA_ARGS__); \
         |         ^~~~~~~~~~
   include/linux/usb.h:1303:2: note: in expansion of macro 'module_driver'
    1303 |  module_driver(__usb_driver, usb_register, \
         |  ^~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:275:1: note: in expansion of macro 'module_usb_driver'
     275 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-cp2615.c:277:15: error: expected declaration specifiers or '...' before string constant
     277 | MODULE_AUTHOR("Bence Csókás <bence98@sch.bme.hu>");
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:278:20: error: expected declaration specifiers or '...' before string constant
     278 | MODULE_DESCRIPTION("CP2615 I2C bus driver");
         |                    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:279:16: error: expected declaration specifiers or '...' before string constant
     279 | MODULE_LICENSE("GPL");
         |                ^~~~~
   In file included from include/linux/device.h:32,
                    from include/linux/acpi.h:15,
                    from include/linux/i2c.h:13,
                    from drivers/i2c/busses/i2c-cp2615.c:9:
   drivers/i2c/busses/i2c-cp2615.c:275:19: warning: 'cp2615_i2c_driver_init' defined but not used [-Wunused-function]
     275 | module_usb_driver(cp2615_i2c_driver);
         |                   ^~~~~~~~~~~~~~~~~
   include/linux/device/driver.h:259:19: note: in definition of macro 'module_driver'
     259 | static int __init __driver##_init(void) \
         |                   ^~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:275:1: note: in expansion of macro 'module_usb_driver'
     275 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +266 drivers/i2c/busses/i2c-cp2615.c

   265	
 > 266	MODULE_DEVICE_TABLE(usb, id_table);
   267	
   268	static struct usb_driver cp2615_i2c_driver = {
   269		.name = "i2c-cp2615",
   270		.probe = cp2615_i2c_probe,
   271		.disconnect = cp2615_i2c_remove,
   272		.id_table = id_table,
   273	};
   274	
   275	module_usb_driver(cp2615_i2c_driver);
   276	
 > 277	MODULE_AUTHOR("Bence Csókás <bence98@sch.bme.hu>");

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67378 bytes --]

Re: [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 3234 bytes --]

On Thu, Mar 18, 2021 at 11:52:10AM +0000, Bence Csókás wrote:
> Create an i2c_adapter for CP2615's I2C master interface by
> implementing parts of the CP2615's I/O Protocol (IOP)
> 
> Signed-off-by: Bence Csókás <bence98@sch.bme.hu>

So, my code checkers report:

    SPARSE
drivers/i2c/busses/i2c-cp2615.c:88:21: warning: incorrect type in assignment (different base types)
drivers/i2c/busses/i2c-cp2615.c:88:21:    expected unsigned short [usertype] length
drivers/i2c/busses/i2c-cp2615.c:88:21:    got restricted __be16 [usertype]
drivers/i2c/busses/i2c-cp2615.c:89:18: warning: incorrect type in assignment (different base types)
drivers/i2c/busses/i2c-cp2615.c:89:18:    expected unsigned short [usertype] msg
drivers/i2c/busses/i2c-cp2615.c:89:18:    got restricted __be16 [usertype]
drivers/i2c/busses/i2c-cp2615.c:78:5: warning: symbol 'cp2615_init_iop_msg' was not declared. Should it be static?
drivers/i2c/busses/i2c-cp2615.c:96:5: warning: symbol 'cp2615_init_i2c_msg' was not declared. Should it be static?
drivers/i2c/busses/i2c-cp2615.c:102:5: warning: symbol 'cp2615_check_status' was not declared. Should it be static?
drivers/i2c/busses/i2c-cp2615.c:131:41: warning: cast to restricted __be16
drivers/i2c/busses/i2c-cp2615.c:131:41: warning: cast to restricted __be16
drivers/i2c/busses/i2c-cp2615.c:131:41: warning: cast to restricted __be16
drivers/i2c/busses/i2c-cp2615.c:131:41: warning: cast to restricted __be16
drivers/i2c/busses/i2c-cp2615.c:148:25: warning: restricted __be16 degrades to integer
drivers/i2c/busses/i2c-cp2615.c:212:27: warning: symbol 'cp2615_i2c_quirks' was not declared. Should it be static?
    CPPCHECK
drivers/i2c/busses/i2c-cp2615.c:258:9: warning: Identical condition and return expression 'ret', return value is always 0 [identicalConditionAfterEarlyExit]
 return ret;
        ^
  CC      drivers/i2c/busses/i2c-cp2615.o
drivers/i2c/busses/i2c-cp2615.c:78:5: warning: no previous prototype for ‘cp2615_init_iop_msg’ [-Wmissing-prototypes]
   78 | int cp2615_init_iop_msg(struct cp2615_iop_msg *ret, enum cp2615_iop_msg_type msg,
      |     ^~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-cp2615.c:96:5: warning: no previous prototype for ‘cp2615_init_i2c_msg’ [-Wmissing-prototypes]
   96 | int cp2615_init_i2c_msg(struct cp2615_iop_msg *ret, const struct cp2615_i2c_transfer *data)
      |     ^~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-cp2615.c:102:5: warning: no previous prototype for ‘cp2615_check_status’ [-Wmissing-prototypes]
  102 | int cp2615_check_status(enum cp2615_i2c_status status)
      |     ^~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-cp2615.c: In function ‘cp2615_i2c_probe’:
drivers/i2c/busses/i2c-cp2615.c:244:2: warning: ‘strncpy’ specified bound 48 equals destination size [-Wstringop-truncation]
  244 |  strncpy(adap->name, usbdev->serial, sizeof(adap->name));
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The missing 'static' are what buildbot also reported and are correct.
Please check the others if they need action. Especially the strncpy
thing.

Other than that, it looks good. I like that you bind to the IOP
interface only. Keeping the CamelCase is also OK.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 110 bytes --]


Oh, and are you willing to maintain the driver? If so, please add an
entry to the MAINTAINERS file. Thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge

[Bence Csókás]

> drivers/i2c/busses/i2c-cp2615.c:88:21: warning: incorrect type in assignment (different base types)
> drivers/i2c/busses/i2c-cp2615.c:88:21:    expected unsigned short [usertype] length
> drivers/i2c/busses/i2c-cp2615.c:88:21:    got restricted __be16 [usertype]
> ...
Yes, I have already converted to using __be16 where needed, it will be
in the next version of the patch I send.

> drivers/i2c/busses/i2c-cp2615.c:78:5: warning: symbol 'cp2615_init_iop_msg' was not declared. Should it be static?
> drivers/i2c/busses/i2c-cp2615.c:96:5: warning: symbol 'cp2615_init_i2c_msg' was not declared. Should it be static?
> drivers/i2c/busses/i2c-cp2615.c:102:5: warning: symbol 'cp2615_check_status' was not declared. Should it be static?
I can forward declare these (copying from the header I used in v1 of
the patch), but I'm not sure I understand the rationale behind these
warnings...
> drivers/i2c/busses/i2c-cp2615.c:212:27: warning: symbol 'cp2615_i2c_quirks' was not declared. Should it be static?
Especially this. I think I will make this static instead, since it
won't ever be exported to any other module.

> The missing 'static' are what buildbot also reported and are correct.
The lkp bot complained about MODULE_DEVICE_TABLE and MODULE_AUTHOR,
which, again, I don't see what is wrong with it.

> drivers/i2c/busses/i2c-cp2615.c:244:2: warning: ‘strncpy’ specified bound 48 equals destination size [-Wstringop-truncation]
I thought this was the correct way of strncpy... Time to RTFM then, I guess :)

> Oh, and are you willing to maintain the driver? If so, please add an
> entry to the MAINTAINERS file. Thanks!
Sure!

I will now send an updated patch, with  few additions too.

Re: [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]


> > drivers/i2c/busses/i2c-cp2615.c:78:5: warning: symbol 'cp2615_init_iop_msg' was not declared. Should it be static?
> > drivers/i2c/busses/i2c-cp2615.c:96:5: warning: symbol 'cp2615_init_i2c_msg' was not declared. Should it be static?
> > drivers/i2c/busses/i2c-cp2615.c:102:5: warning: symbol 'cp2615_check_status' was not declared. Should it be static?
> I can forward declare these (copying from the header I used in v1 of
> the patch), but I'm not sure I understand the rationale behind these
> warnings...

Just make them static and all is good.

> > drivers/i2c/busses/i2c-cp2615.c:212:27: warning: symbol 'cp2615_i2c_quirks' was not declared. Should it be static?
> Especially this. I think I will make this static instead, since it
> won't ever be exported to any other module.

Exactly.

> > The missing 'static' are what buildbot also reported and are correct.
> The lkp bot complained about MODULE_DEVICE_TABLE and MODULE_AUTHOR,
> which, again, I don't see what is wrong with it.

Yeah, that may be a false positive.

> I will now send an updated patch, with  few additions too.

Cool, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]