linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge
@ 2021-03-17 10:30 Bence Csókás
  2021-03-17 12:24 ` kernel test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Bence Csókás @ 2021-03-17 10:30 UTC (permalink / raw)
  To: linux-i2c; +Cc: Bence Csókás, linux-kernel

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 | 282 ++++++++++++++++++++++++++++++++
 3 files changed, 293 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..75f76201fbb3
--- /dev/null
+++ b/drivers/i2c/busses/i2c-cp2615.c
@@ -0,0 +1,282 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * i2c support for Silicon Labs' CP2615 Digital Audio Bridge
+ *
+ * (c) 2021, Bence Csókás <bence98@sch.bme.hu>
+ */
+
+#include <linux/string.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/usb.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 {
+	uint16_t preamble, length, msg;
+	uint8_t data[MAX_IOP_PAYLOAD_SIZE];
+};
+
+#define PART_ID_A01 0x1400
+#define PART_ID_A02 0x1500
+
+struct cp2615_iop_accessory_info {
+	uint16_t part_id, option_id, proto_ver;
+};
+
+struct cp2615_i2c_transfer {
+	uint8_t tag, i2caddr, read_len, write_len;
+	uint8_t data[MAX_I2C_SIZE];
+};
+
+/** Possible values for struct cp2615_i2c_transfer_result.status
+ *
+ *  Values extracted from the USBXpress(r) SDK
+ */
+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 {
+	uint8_t tag, i2caddr;
+	int8_t status;
+	uint8_t read_len;
+	uint8_t 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) {
+		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;
+	} else {
+        return -EINVAL;
+	}
+}
+
+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 -ECOMM;
+	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(struct cp2615_iop_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(struct cp2615_iop_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.
+ *
+ * FIXME: There in no quirk flag for specifying that the adapter
+ * does not support empty transfers, or that it cannot emit a
+ * START condition between the combined 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,
+	.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(CP2615_VID, CP2615_PID) },
+	{ }
+};
+
+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.30.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge
  2021-03-17 10:30 [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge Bence Csókás
@ 2021-03-17 12:24 ` kernel test robot
  2021-03-17 12:33 ` Wolfram Sang
  2021-03-17 14:29 ` kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2021-03-17 12:24 UTC (permalink / raw)
  To: Bence Csókás, linux-i2c
  Cc: kbuild-all, Bence Csókás, linux-kernel

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

Hi "Bence,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v5.12-rc3 next-20210317]
[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/Adding-i2c-cp2615-i2c-support-for-Silicon-Labs-CP2615-Digital-Audio-Bridge/20210317-181539
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/c8c005a08175789b1874e69abf4c6da690d5b323
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bence-Cs-k-s/Adding-i2c-cp2615-i2c-support-for-Silicon-Labs-CP2615-Digital-Audio-Bridge/20210317-181539
        git checkout c8c005a08175789b1874e69abf4c6da690d5b323
        # 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 warnings (new ones prefixed by >>):

>> drivers/i2c/busses/i2c-cp2615.c:82:5: warning: no previous prototype for 'cp2615_init_iop_msg' [-Wmissing-prototypes]
      82 | int cp2615_init_iop_msg(struct cp2615_iop_msg *ret, enum cp2615_iop_msg_type msg, const void *data, size_t data_len)
         |     ^~~~~~~~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-cp2615.c:99:5: warning: no previous prototype for 'cp2615_init_i2c_msg' [-Wmissing-prototypes]
      99 | int cp2615_init_i2c_msg(struct cp2615_iop_msg *ret, const struct cp2615_i2c_transfer *data)
         |     ^~~~~~~~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-cp2615.c:105:5: warning: no previous prototype for 'cp2615_check_status' [-Wmissing-prototypes]
     105 | int cp2615_check_status(enum cp2615_i2c_status status)
         |     ^~~~~~~~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-cp2615.c:269:1: warning: data definition has no type or storage class
     269 | MODULE_DEVICE_TABLE(usb, id_table);
         | ^~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:269:1: error: type defaults to 'int' in declaration of 'MODULE_DEVICE_TABLE' [-Werror=implicit-int]
>> drivers/i2c/busses/i2c-cp2615.c:269: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:11:
>> 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:278:1: note: in expansion of macro 'module_usb_driver'
     278 | 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:278:1: note: in expansion of macro 'module_usb_driver'
     278 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
   In file included from include/linux/linkage.h:7,
                    from include/linux/kernel.h:7,
                    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:278:1: note: in expansion of macro 'module_usb_driver'
     278 | 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:11:
   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:278:1: note: in expansion of macro 'module_usb_driver'
     278 | 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:278:1: note: in expansion of macro 'module_usb_driver'
     278 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
   In file included from include/linux/linkage.h:7,
                    from include/linux/kernel.h:7,
                    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:278:1: note: in expansion of macro 'module_usb_driver'
     278 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:280:15: error: expected declaration specifiers or '...' before string constant
     280 | MODULE_AUTHOR("Bence Csókás <bence98@sch.bme.hu>");
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:281:20: error: expected declaration specifiers or '...' before string constant
     281 | MODULE_DESCRIPTION("CP2615 I2C bus driver");
         |                    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:282:16: error: expected declaration specifiers or '...' before string constant
     282 | 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:11:
   drivers/i2c/busses/i2c-cp2615.c:278:19: warning: 'cp2615_i2c_driver_init' defined but not used [-Wunused-function]
     278 | 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:278:1: note: in expansion of macro 'module_usb_driver'
     278 | module_usb_driver(cp2615_i2c_driver);
         | ^~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/cp2615_init_iop_msg +82 drivers/i2c/busses/i2c-cp2615.c

    81	
  > 82	int cp2615_init_iop_msg(struct cp2615_iop_msg *ret, enum cp2615_iop_msg_type msg, const void *data, size_t data_len)
    83	{
    84		if (data_len > MAX_IOP_PAYLOAD_SIZE)
    85			return -EFBIG;
    86	
    87		if (ret) {
    88			ret->preamble = 0x2A2A;
    89			ret->length = htons(data_len+6);
    90			ret->msg = htons(msg);
    91			if(data && data_len)
    92				memcpy(&ret->data, data, data_len);
    93	        return 0;
    94		} else {
    95	        return -EINVAL;
    96		}
    97	}
    98	
  > 99	int cp2615_init_i2c_msg(struct cp2615_iop_msg *ret, const struct cp2615_i2c_transfer *data)
   100	{
   101	    return cp2615_init_iop_msg(ret, iop_DoI2cTransfer, data, 4 + data->write_len);
   102	}
   103	
   104	/* Translates status codes to Linux errno's */
 > 105	int cp2615_check_status(enum cp2615_i2c_status status)
   106	{
   107		switch (status) {
   108		case CP2615_SUCCESS:
   109				return 0;
   110		case CP2615_BUS_ERROR:
   111			return -ECOMM;
   112		case CP2615_BUS_BUSY:
   113			return -EAGAIN;
   114		case CP2615_TIMEOUT:
   115			return -ETIMEDOUT;
   116		case CP2615_INVALID_PARAM:
   117			return -EINVAL;
   118		case CP2615_CFG_LOCKED:
   119			return -EPERM;
   120		}
   121		/* Unknown error code */
   122		return -EPROTO;
   123	}
   124	
   125	
   126	static int
   127	cp2615_i2c_send(struct usb_interface *usbif, struct cp2615_i2c_transfer *i2c_w)
   128	{
   129		struct cp2615_iop_msg *msg = kzalloc(sizeof(struct cp2615_iop_msg), GFP_KERNEL);
   130		struct usb_device *usbdev = interface_to_usbdev(usbif);
   131		int res = cp2615_init_i2c_msg(msg, i2c_w);
   132		if (!res)
   133			res = usb_bulk_msg(usbdev, usb_sndbulkpipe(usbdev, IOP_EP_OUT), msg, ntohs(msg->length), NULL, 0);
   134		kfree(msg);
   135		return res;
   136	}
   137	
   138	static int
   139	cp2615_i2c_recv(struct usb_interface *usbif, unsigned char tag, void *buf)
   140	{
   141		struct cp2615_iop_msg *msg = kzalloc(sizeof(struct cp2615_iop_msg), GFP_KERNEL);
   142		struct cp2615_i2c_transfer_result *i2c_r = (struct cp2615_i2c_transfer_result*) &msg->data;
   143		struct usb_device *usbdev = interface_to_usbdev(usbif);
   144		int res = usb_bulk_msg(usbdev, usb_rcvbulkpipe(usbdev, IOP_EP_IN), msg, sizeof(struct cp2615_iop_msg), NULL, 0);
   145		if (res < 0)
   146			return res;
   147	
   148		if (msg->msg != htons(iop_I2cTransferResult) || i2c_r->tag != tag)
   149			return -EIO;
   150	
   151		res = cp2615_check_status(i2c_r->status);
   152		if (res < 0)
   153			return res;
   154	
   155		memcpy(buf, &i2c_r->data, i2c_r->read_len);
   156		kfree(msg);
   157		return 0;
   158	}
   159	
   160	static int
   161	cp2615_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
   162	{
   163		struct usb_interface *usbif = adap->algo_data;
   164		int i = 0, ret = 0;
   165		struct i2c_msg *msg;
   166		struct cp2615_i2c_transfer i2c_w = {0};
   167		dev_dbg(&usbif->dev, "Doing %d I2C transactions\n", num);
   168	
   169		for(; !ret && i < num; i++) {
   170			msg = &msgs[i];
   171	
   172			i2c_w.tag = 0xdd;
   173			i2c_w.i2caddr = i2c_8bit_addr_from_msg(msg);
   174			if (msg->flags & I2C_M_RD) {
   175				i2c_w.read_len = msg->len;
   176				i2c_w.write_len = 0;
   177			} else {
   178				i2c_w.read_len = 0;
   179				i2c_w.write_len = msg->len;
   180				memcpy(&i2c_w.data, msg->buf, i2c_w.write_len);
   181			}
   182			ret = cp2615_i2c_send(usbif, &i2c_w);
   183			if (ret)
   184				break;
   185			ret = cp2615_i2c_recv(usbif, i2c_w.tag, msg->buf);
   186		}
   187		if (ret < 0)
   188			return ret;
   189		return i;
   190	}
   191	
   192	static u32
   193	cp2615_i2c_func(struct i2c_adapter *adap)
   194	{
   195		return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
   196	}
   197	
   198	static const struct i2c_algorithm cp2615_i2c_algo = {
   199		.master_xfer	= cp2615_i2c_master_xfer,
   200		.functionality	= cp2615_i2c_func,
   201	};
   202	
   203	/*
   204	 * This chip has some limitations: one is that the USB endpoint
   205	 * can only receive 64 bytes/transfer, that leaves 54 bytes for
   206	 * the I2C transfer. On top of that, EITHER read_len OR write_len
   207	 * may be zero, but not both. If both are non-zero, the adapter
   208	 * issues a write followed by a read. And the chip does not
   209	 * support repeated START between the write and read phases.
   210	 *
   211	 * FIXME: There in no quirk flag for specifying that the adapter
   212	 * does not support empty transfers, or that it cannot emit a
   213	 * START condition between the combined phases.
   214	 */
   215	struct i2c_adapter_quirks cp2615_i2c_quirks = {
   216		.max_write_len = MAX_I2C_SIZE,
   217		.max_read_len = MAX_I2C_SIZE,
   218		.flags = I2C_AQ_COMB_WRITE_THEN_READ,
   219		.max_comb_1st_msg_len = MAX_I2C_SIZE,
   220		.max_comb_2nd_msg_len = MAX_I2C_SIZE
   221	};
   222	
   223	static void
   224	cp2615_i2c_remove(struct usb_interface *usbif)
   225	{
   226		struct i2c_adapter *adap = usb_get_intfdata(usbif);
   227	
   228		usb_set_intfdata(usbif, NULL);
   229		i2c_del_adapter(adap);
   230	}
   231	
   232	static int
   233	cp2615_i2c_probe(struct usb_interface *usbif, const struct usb_device_id *id)
   234	{
   235		int ret = 0;
   236		struct i2c_adapter *adap;
   237		struct usb_device *usbdev = interface_to_usbdev(usbif);
   238	
   239		ret = usb_set_interface(usbdev, IOP_IFN, IOP_ALTSETTING);
   240		if (ret)
   241			return ret;
   242	
   243		adap = devm_kzalloc(&usbif->dev, sizeof(struct i2c_adapter), GFP_KERNEL);
   244		if (!adap)
   245			return -ENOMEM;
   246	
   247		strncpy(adap->name, usbdev->serial, sizeof(adap->name));
   248		adap->owner = THIS_MODULE;
   249		adap->dev.parent = &usbif->dev;
   250		adap->dev.of_node = usbif->dev.of_node;
   251		adap->timeout = HZ;
   252		adap->algo = &cp2615_i2c_algo;
   253		adap->quirks = &cp2615_i2c_quirks;
   254		adap->algo_data = usbif;
   255	
   256		ret = i2c_add_adapter(adap);
   257		if (ret)
   258			return ret;
   259	
   260		usb_set_intfdata(usbif, adap);
   261		return ret;
   262	}
   263	
   264	static const struct usb_device_id id_table[] = {
   265		{ USB_DEVICE(CP2615_VID, CP2615_PID) },
   266		{ }
   267	};
   268	
 > 269	MODULE_DEVICE_TABLE(usb, id_table);
   270	

---
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: 67374 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge
  2021-03-17 10:30 [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge Bence Csókás
  2021-03-17 12:24 ` kernel test robot
@ 2021-03-17 12:33 ` Wolfram Sang
  2021-03-18  1:55   ` Bence Csókás
  2021-03-17 14:29 ` kernel test robot
  2 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2021-03-17 12:33 UTC (permalink / raw)
  To: Bence Csókás; +Cc: linux-i2c, linux-kernel

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

On Wed, Mar 17, 2021 at 10:30:21AM +0000, Bence Csókás wrote:
> Signed-off-by: Bence Csókás <bence98@sch.bme.hu>

Thanks, this looks good now and I think we are very close.

> ---

Next, time please provide a small summary of changes since last version.
I get enough patches that it becomes confusing otherwise.

> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-cp2615.c
> @@ -0,0 +1,282 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

If you want GPL v2 only, then you need to say in MODULE_LICENSE also
"GPL v2".

> +#include <linux/string.h>
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/usb.h>

Please sort this to avoid double entries get added in the future.

> +enum cp2615_iop_msg_type {
> +	iop_GetAccessoryInfo = 0xD100,
> +	iop_AccessoryInfo = 0xA100,
> +	iop_GetPortConfiguration = 0xD203,
> +	iop_PortConfiguration = 0xA203,
> +	// ...

This comment can go?

> +/** Possible values for struct cp2615_i2c_transfer_result.status
> + *
> + *  Values extracted from the USBXpress(r) SDK
> + */

I'd think this can go, too.

> +int cp2615_init_iop_msg(struct cp2615_iop_msg *ret, enum cp2615_iop_msg_type msg, const void *data, size_t data_len)

Please break it into two lines. And please run 'checkpatch --strict' on
your patch to find things like too long lines and spaces around
operators etc... I will skip pointing them out here.

> +{
> +	if (data_len > MAX_IOP_PAYLOAD_SIZE)
> +		return -EFBIG;
> +
> +	if (ret) {

Minor nit:
	if (!ret)
		return -EINVAL;

and then save the indentation for the main code path.

> +		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;
> +	} else {
> +        return -EINVAL;
> +	}
> +}
> +
> +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);
> +}

I'll leave it to you but maybe this function can go and
cp2615_init_iop_msg can be used directly? Both are only called once.

> +
> +/* 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 -ECOMM;

-EIO probably.

We have 'Documentation/i2c/fault-codes.rst' as a guide.

> +	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;
> +}
> +

...

> +/*
> + * 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.

Good and useful paragraph!

> + * FIXME: There in no quirk flag for specifying that the adapter
> + * does not support empty transfers, or that it cannot emit a

Can't we use I2C_AQ_NO_ZERO_LEN here?

> + * START condition between the combined phases.

True! But it makes sense, so we can fix that. We just need to add
I2C_AQ_NO_REP_START and a short explanation to i2c.h. If you want, you
can do it in a seperate patch. I can do it, too, if you prefer.

> + */
> +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,
> +	.max_comb_1st_msg_len = MAX_I2C_SIZE,
> +	.max_comb_2nd_msg_len = MAX_I2C_SIZE
> +};

quirks struct looks good otherwise!

> +static const struct usb_device_id id_table[] = {
> +	{ USB_DEVICE(CP2615_VID, CP2615_PID) },

Maybe skip the defines for VID and PID and use the values directly?
I am not a USB expert, not really sure what the consistent way is.

So, this and the checkpatch issues and I think we are done.

Thanks,

   Wolfram


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge
  2021-03-17 10:30 [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge Bence Csókás
  2021-03-17 12:24 ` kernel test robot
  2021-03-17 12:33 ` Wolfram Sang
@ 2021-03-17 14:29 ` kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2021-03-17 14:29 UTC (permalink / raw)
  To: Bence Csókás, linux-i2c
  Cc: kbuild-all, Bence Csókás, linux-kernel

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

Hi "Bence,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v5.12-rc3 next-20210317]
[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/Adding-i2c-cp2615-i2c-support-for-Silicon-Labs-CP2615-Digital-Audio-Bridge/20210317-181539
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-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/c8c005a08175789b1874e69abf4c6da690d5b323
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bence-Cs-k-s/Adding-i2c-cp2615-i2c-support-for-Silicon-Labs-CP2615-Digital-Audio-Bridge/20210317-181539
        git checkout c8c005a08175789b1874e69abf4c6da690d5b323
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

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

All warnings (new ones prefixed by >>):

   drivers/i2c/busses/i2c-cp2615.c:82:5: warning: no previous prototype for 'cp2615_init_iop_msg' [-Wmissing-prototypes]
      82 | int cp2615_init_iop_msg(struct cp2615_iop_msg *ret, enum cp2615_iop_msg_type msg, const void *data, size_t data_len)
         |     ^~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:99:5: warning: no previous prototype for 'cp2615_init_i2c_msg' [-Wmissing-prototypes]
      99 | int cp2615_init_i2c_msg(struct cp2615_iop_msg *ret, const struct cp2615_i2c_transfer *data)
         |     ^~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-cp2615.c:105:5: warning: no previous prototype for 'cp2615_check_status' [-Wmissing-prototypes]
     105 | 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:247:2: warning: 'strncpy' specified bound 48 equals destination size [-Wstringop-truncation]
     247 |  strncpy(adap->name, usbdev->serial, sizeof(adap->name));
         |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
   Selected by
   - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86


vim +/strncpy +247 drivers/i2c/busses/i2c-cp2615.c

   231	
   232	static int
   233	cp2615_i2c_probe(struct usb_interface *usbif, const struct usb_device_id *id)
   234	{
   235		int ret = 0;
   236		struct i2c_adapter *adap;
   237		struct usb_device *usbdev = interface_to_usbdev(usbif);
   238	
   239		ret = usb_set_interface(usbdev, IOP_IFN, IOP_ALTSETTING);
   240		if (ret)
   241			return ret;
   242	
   243		adap = devm_kzalloc(&usbif->dev, sizeof(struct i2c_adapter), GFP_KERNEL);
   244		if (!adap)
   245			return -ENOMEM;
   246	
 > 247		strncpy(adap->name, usbdev->serial, sizeof(adap->name));
   248		adap->owner = THIS_MODULE;
   249		adap->dev.parent = &usbif->dev;
   250		adap->dev.of_node = usbif->dev.of_node;
   251		adap->timeout = HZ;
   252		adap->algo = &cp2615_i2c_algo;
   253		adap->quirks = &cp2615_i2c_quirks;
   254		adap->algo_data = usbif;
   255	
   256		ret = i2c_add_adapter(adap);
   257		if (ret)
   258			return ret;
   259	
   260		usb_set_intfdata(usbif, adap);
   261		return ret;
   262	}
   263	

---
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: 63503 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge
  2021-03-17 12:33 ` Wolfram Sang
@ 2021-03-18  1:55   ` Bence Csókás
  2021-03-18  5:59     ` Wolfram Sang
  0 siblings, 1 reply; 6+ messages in thread
From: Bence Csókás @ 2021-03-18  1:55 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-i2c, linux-kernel

Thanks for the lightning quick response!

Wolfram Sang <wsa@kernel.org> ezt írta (időpont: 2021. márc. 17., Sze, 13:34):
>
> On Wed, Mar 17, 2021 at 10:30:21AM +0000, Bence Csókás wrote:
> > Signed-off-by: Bence Csókás <bence98@sch.bme.hu>
>
> Thanks, this looks good now and I think we are very close.
>
> > ---
>
> Next, time please provide a small summary of changes since last version.
> I get enough patches that it becomes confusing otherwise.
>

You are right, sorry, I am still familiarizing myself with `git send-email`

> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-cp2615.c
> > @@ -0,0 +1,282 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
>
> If you want GPL v2 only, then you need to say in MODULE_LICENSE also
> "GPL v2".
>

GPLv2 or later is fine by me. If I change this to "//
SPDX-License-Identifier: GPL-2.0-or-later", is that OK?

> > +enum cp2615_iop_msg_type {
> > +     iop_GetAccessoryInfo = 0xD100,
> > +     iop_AccessoryInfo = 0xA100,
> > +     iop_GetPortConfiguration = 0xD203,
> > +     iop_PortConfiguration = 0xA203,
> > +     // ...
>
> This comment can go?

Sorry, this slipped in from before... I shall remove it.


> > +/*
> > + * 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.
>
> Good and useful paragraph!

Thank you!

>
> > + * FIXME: There in no quirk flag for specifying that the adapter
> > + * does not support empty transfers, or that it cannot emit a
>
> Can't we use I2C_AQ_NO_ZERO_LEN here?

I thought that meant the adapter cannot handle NEITHER zero-length
reads NOR writes, but the CP2615 can do a zero read combined with a
non-zero write or the other way around, just both cannot be zero. If
both are zero, the chip just ignores the request, as I've learned from
a very confusing situation with `i2cdetect`.

>
> > + * START condition between the combined phases.
>
> True! But it makes sense, so we can fix that. We just need to add
> I2C_AQ_NO_REP_START and a short explanation to i2c.h. If you want, you
> can do it in a seperate patch. I can do it, too, if you prefer.

Sure! I should just define it as BIT(7) or something, right? Should I
do it in a completely different patchset, or is it OK if I submit it
as the 2/2 of PATCH v3? Are there maybe other adapters that would be
affected?

> Maybe skip the defines for VID and PID and use the values directly?
> I am not a USB expert, not really sure what the consistent way is.

I think this is how they usually do it, or at least from what I've seen.

>
> So, this and the checkpatch issues and I think we are done.
>
> Thanks,
>
>    Wolfram
>

Thanks,
Bence

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge
  2021-03-18  1:55   ` Bence Csókás
@ 2021-03-18  5:59     ` Wolfram Sang
  0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2021-03-18  5:59 UTC (permalink / raw)
  To: Bence Csókás; +Cc: linux-i2c, linux-kernel

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

Hi Bence,

> You are right, sorry, I am still familiarizing myself with `git send-email`

No worries, the progress from your first to your second version was
really great! You can add such information e.g. using the "--annotate"
parameter og git-send-email.

> GPLv2 or later is fine by me. If I change this to "//
> SPDX-License-Identifier: GPL-2.0-or-later", is that OK?

Yes, also perfect.

> > > + * FIXME: There in no quirk flag for specifying that the adapter
> > > + * does not support empty transfers, or that it cannot emit a
> >
> > Can't we use I2C_AQ_NO_ZERO_LEN here?
> 
> I thought that meant the adapter cannot handle NEITHER zero-length
> reads NOR writes, but the CP2615 can do a zero read combined with a
> non-zero write or the other way around, just both cannot be zero. If
> both are zero, the chip just ignores the request, as I've learned from
> a very confusing situation with `i2cdetect`.

I think we still should use I2C_AQ_NO_ZERO_LEN. The almost only use case
is a standalone zero-len read or write. This is not supported by your
adapter as you found out.

> > True! But it makes sense, so we can fix that. We just need to add
> > I2C_AQ_NO_REP_START and a short explanation to i2c.h. If you want, you
> > can do it in a seperate patch. I can do it, too, if you prefer.
> 
> Sure! I should just define it as BIT(7) or something, right? Should I
> do it in a completely different patchset, or is it OK if I submit it
> as the 2/2 of PATCH v3? Are there maybe other adapters that would be

Yes, BIT(7) and same patchset please. But you need to make it 1/2
because you will use it in you driver which is then 2/2.

> affected?

Maybe, but we can fix them later incrementally. Currently, there is also
no client testing the flag, so we have a bit of time here.

> > Maybe skip the defines for VID and PID and use the values directly?
> > I am not a USB expert, not really sure what the consistent way is.
> 
> I think this is how they usually do it, or at least from what I've seen.

Then keep it.

All the best and happy hacking,

   Wolfram


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-18  6:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-17 10:30 [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge Bence Csókás
2021-03-17 12:24 ` kernel test robot
2021-03-17 12:33 ` Wolfram Sang
2021-03-18  1:55   ` Bence Csókás
2021-03-18  5:59     ` Wolfram Sang
2021-03-17 14:29 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).