* [PATCH v3 0/2] Add i2c-cp2615
@ 2021-03-18 11:52 Bence Csókás
2021-03-18 11:52 ` [PATCH v3 1/2] i2c: Add I2C_AQ_NO_REP_START adapter quirk Bence Csókás
2021-03-18 11:52 ` [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge Bence Csókás
0 siblings, 2 replies; 8+ messages in thread
From: Bence Csókás @ 2021-03-18 11:52 UTC (permalink / raw)
To: linux-i2c; +Cc: Bence Csókás, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v3 1/2] i2c: Add I2C_AQ_NO_REP_START adapter quirk 2021-03-18 11:52 [PATCH v3 0/2] Add i2c-cp2615 Bence Csókás @ 2021-03-18 11:52 ` Bence Csókás 2021-03-18 11:52 ` [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge Bence Csókás 1 sibling, 0 replies; 8+ messages in thread From: Bence Csókás @ 2021-03-18 11:52 UTC (permalink / raw) To: linux-i2c; +Cc: Bence Csókás, linux-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge 2021-03-18 11:52 [PATCH v3 0/2] Add i2c-cp2615 Bence Csókás 2021-03-18 11:52 ` [PATCH v3 1/2] i2c: Add I2C_AQ_NO_REP_START adapter quirk Bence Csókás @ 2021-03-18 11:52 ` Bence Csókás 2021-03-20 2:30 ` kernel test robot ` (2 more replies) 1 sibling, 3 replies; 8+ messages in thread From: Bence Csókás @ 2021-03-18 11:52 UTC (permalink / raw) To: linux-i2c; +Cc: Bence Csókás, linux-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge 2021-03-18 11:52 ` [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge Bence Csókás @ 2021-03-20 2:30 ` kernel test robot 2021-03-31 7:09 ` Wolfram Sang 2021-03-31 9:58 ` Wolfram Sang 2 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2021-03-20 2:30 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: 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge 2021-03-18 11:52 ` [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge Bence Csókás 2021-03-20 2:30 ` kernel test robot @ 2021-03-31 7:09 ` Wolfram Sang 2021-03-31 9:58 ` Wolfram Sang 2 siblings, 0 replies; 8+ messages in thread From: Wolfram Sang @ 2021-03-31 7:09 UTC (permalink / raw) To: Bence Csókás; +Cc: linux-i2c, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge 2021-03-18 11:52 ` [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge Bence Csókás 2021-03-20 2:30 ` kernel test robot 2021-03-31 7:09 ` Wolfram Sang @ 2021-03-31 9:58 ` Wolfram Sang 2021-03-31 18:17 ` Bence Csókás 2 siblings, 1 reply; 8+ messages in thread From: Wolfram Sang @ 2021-03-31 9:58 UTC (permalink / raw) To: Bence Csókás; +Cc: linux-i2c, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge 2021-03-31 9:58 ` Wolfram Sang @ 2021-03-31 18:17 ` Bence Csókás 2021-03-31 20:55 ` Wolfram Sang 0 siblings, 1 reply; 8+ messages in thread From: Bence Csókás @ 2021-03-31 18:17 UTC (permalink / raw) To: Wolfram Sang; +Cc: linux-i2c, linux-kernel > 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge 2021-03-31 18:17 ` Bence Csókás @ 2021-03-31 20:55 ` Wolfram Sang 0 siblings, 0 replies; 8+ messages in thread From: Wolfram Sang @ 2021-03-31 20:55 UTC (permalink / raw) To: Bence Csókás; +Cc: linux-i2c, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-03-31 20:56 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-18 11:52 [PATCH v3 0/2] Add i2c-cp2615 Bence Csókás 2021-03-18 11:52 ` [PATCH v3 1/2] i2c: Add I2C_AQ_NO_REP_START adapter quirk Bence Csókás 2021-03-18 11:52 ` [PATCH v3 2/2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge Bence Csókás 2021-03-20 2:30 ` kernel test robot 2021-03-31 7:09 ` Wolfram Sang 2021-03-31 9:58 ` Wolfram Sang 2021-03-31 18:17 ` Bence Csókás 2021-03-31 20:55 ` Wolfram Sang
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).