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
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 <[email protected]>
---
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
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 <[email protected]>
---
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 <[email protected]>
+ */
+
+#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 <[email protected]>");
+MODULE_DESCRIPTION("CP2615 I2C bus driver");
+MODULE_LICENSE("GPL");
--
2.31.0
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 <[email protected]>
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 <[email protected]>");
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
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 <[email protected]>");
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
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 <[email protected]>
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.
Oh, and are you willing to maintain the driver? If so, please add an
entry to the MAINTAINERS file. Thanks!
> 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.
> > 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!