| Message ID | 1409930279-1382-3-git-send-email-octavian.purdila@intel.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Fri, Sep 05, 2014 at 06:17:58PM +0300, Octavian Purdila wrote: > From: Laurentiu Palcu <laurentiu.palcu@intel.com> > > This patch adds support for the Diolan DLN-2 I2C master module. Due > to hardware limitations it does not support SMBUS quick commands. > > Information about the USB protocol interface can be found in the > Programmer's Reference Manual [1], see section 6.2.2 for the I2C > master module commands and responses. > > [1] https://www.diolan.com/downloads/dln-api-manual.pdf > > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com> > Signed-off-by: Octavian Purdila <octavian.purdila@intel.com> > --- > drivers/i2c/busses/Kconfig | 10 ++ > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-dln2.c | 301 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 312 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-dln2.c > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 2ac87fa..4873161 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -1021,4 +1021,14 @@ config SCx200_ACB > This support is also available as a module. If so, the module > will be called scx200_acb. > > +config I2C_DLN2 > + tristate "Diolan DLN-2 USB I2C adapter" > + depends on USB && MFD_DLN2 MFD_DLN2 should be sufficient. > + help > + If you say yes to this option, support will be included for Diolan > + DLN2, a USB to I2C interface. > + > + This driver can also be built as a module. If so, the module > + will be called i2c-dln2. > + > endmenu > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 49bf07e..3118fea 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -100,5 +100,6 @@ obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o > obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o > obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o > obj-$(CONFIG_SCx200_ACB) += scx200_acb.o > +obj-$(CONFIG_I2C_DLN2) += i2c-dln2.o > > ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG > diff --git a/drivers/i2c/busses/i2c-dln2.c b/drivers/i2c/busses/i2c-dln2.c > new file mode 100644 > index 0000000..93e85ff > --- /dev/null > +++ b/drivers/i2c/busses/i2c-dln2.c > @@ -0,0 +1,301 @@ > +/* > + * Driver for the Diolan DLN-2 USB-I2C adapter > + * > + * Copyright (c) 2014 Intel Corporation > + * > + * Derived from: > + * i2c-diolan-u2c.c > + * Copyright (c) 2010-2011 Ericsson AB > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation, version 2. > + */ > + > +#include <linux/kernel.h> > +#include <linux/errno.h> > +#include <linux/module.h> > +#include <linux/types.h> > +#include <linux/slab.h> > +#include <linux/i2c.h> > + No newline. > +#include <linux/platform_device.h> > +#include <linux/mfd/dln2.h> > + > +#define DRIVER_NAME "dln2-i2c" > + > +#define DLN2_I2C_MODULE_ID 0x03 > +#define DLN2_I2C_CMD(cmd) DLN2_CMD(cmd, DLN2_I2C_MODULE_ID) > + > +/* I2C commands */ > +#define DLN2_I2C_GET_PORT_COUNT DLN2_I2C_CMD(0x00) > +#define DLN2_I2C_ENABLE DLN2_I2C_CMD(0x01) > +#define DLN2_I2C_DISABLE DLN2_I2C_CMD(0x02) > +#define DLN2_I2C_IS_ENABLED DLN2_I2C_CMD(0x03) > +#define DLN2_I2C_SET_FREQUENCY DLN2_I2C_CMD(0x04) > +#define DLN2_I2C_GET_FREQUENCY DLN2_I2C_CMD(0x05) > +#define DLN2_I2C_WRITE DLN2_I2C_CMD(0x06) > +#define DLN2_I2C_READ DLN2_I2C_CMD(0x07) > +#define DLN2_I2C_SCAN_DEVICES DLN2_I2C_CMD(0x08) > +#define DLN2_I2C_PULLUP_ENABLE DLN2_I2C_CMD(0x09) > +#define DLN2_I2C_PULLUP_DISABLE DLN2_I2C_CMD(0x0A) > +#define DLN2_I2C_PULLUP_IS_ENABLED DLN2_I2C_CMD(0x0B) > +#define DLN2_I2C_TRANSFER DLN2_I2C_CMD(0x0C) > +#define DLN2_I2C_SET_MAX_REPLY_COUNT DLN2_I2C_CMD(0x0D) > +#define DLN2_I2C_GET_MAX_REPLY_COUNT DLN2_I2C_CMD(0x0E) > +#define DLN2_I2C_GET_MIN_FREQUENCY DLN2_I2C_CMD(0x40) > +#define DLN2_I2C_GET_MAX_FREQUENCY DLN2_I2C_CMD(0x41) > + > +#define DLN2_I2C_FREQ_FAST 400000 > +#define DLN2_I2C_FREQ_STD 100000 > + > +#define DLN2_I2C_MAX_XFER_SIZE 256 > + > +struct dln2_i2c { > + struct platform_device *pdev; > + struct i2c_adapter adapter; > +}; > + > +static uint frequency = DLN2_I2C_FREQ_STD; /* I2C clock frequency in Hz */ > + > +module_param(frequency, uint, S_IRUGO | S_IWUSR); > +MODULE_PARM_DESC(frequency, "I2C clock frequency in hertz"); These seems like a very bad idea. Why set one common frequency for all connected USB-I2C devices using a module parameter? That might have made sense a long time ago with embedded i2c-controller, but certainly does not with usb-i2c controllers. This should probably be set through sysfs on a per-device basis. > + > +static int dln2_i2c_set_state(struct dln2_i2c *dln2, u8 state) > +{ > + int ret; > + u8 port = 0; So these devices can apparently have more than one i2c "master port". You could query the device from the core MFD driver and add one i2c-dln2 platform device per master. Either way, you shouldn't hard-code the port number throughout the driver. > + > + ret = dln2_transfer(dln2->pdev, > + state ? DLN2_I2C_ENABLE : DLN2_I2C_DISABLE, Please try to avoid using ?: constructs. > + &port, sizeof(port), NULL, NULL); > + > + if (ret < 0) > + return ret; > + > + return 0; > +} > + > +#define dln2_i2c_enable(dln2) dln2_i2c_set_state(dln2, 1) > +#define dln2_i2c_disable(dln2) dln2_i2c_set_state(dln2, 0) Skip the macros and simply call one appropriately renamed function with a boolean argument. > + > +static int dln2_i2c_set_frequency(struct dln2_i2c *dln2, u32 freq) > +{ > + int ret; > + struct tx_data { > + u8 port; > + __le32 freq; > + } __packed tx; > + > + tx.port = 0; > + tx.freq = cpu_to_le32(freq); > + > + ret = dln2_transfer(dln2->pdev, DLN2_I2C_SET_FREQUENCY, &tx, sizeof(tx), > + NULL, NULL); > + if (ret < 0) > + return ret; > + > + return 0; > +} > + > +static int dln2_i2c_write(struct dln2_i2c *dln2, u8 addr, > + u8 *data, u16 data_len) > +{ > + int ret, len; > + struct tx_data { > + u8 port; > + u8 addr; > + u8 mem_addr_len; > + __le32 mem_addr; > + __le16 buf_len; > + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; > + } __packed tx; Allocate these buffers dynamically (possibly at probe). > + > + if (data_len > DLN2_I2C_MAX_XFER_SIZE) > + return -ENOSPC; -EINVAL > + > + tx.port = 0; > + tx.addr = addr; > + tx.mem_addr_len = 0; > + tx.mem_addr = 0; > + tx.buf_len = cpu_to_le16(data_len); > + memcpy(tx.buf, data, data_len); > + > + len = sizeof(tx) + data_len - DLN2_I2C_MAX_XFER_SIZE; > + ret = dln2_transfer(dln2->pdev, DLN2_I2C_WRITE, &tx, len, > + NULL, NULL); > + if (ret < 0) > + return ret; > + > + return data_len; > +} > + > +static int dln2_i2c_read(struct dln2_i2c *dln2, u16 addr, u8 *data, > + u16 data_len) > +{ > + struct tx_data { > + u8 port; > + u8 addr; > + u8 mem_addr_len; > + __le32 mem_addr; > + __le16 buf_len; > + } __packed tx; > + struct rx_data { > + __le16 buf_len; > + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; > + } __packed rx; > + int ret, buf_len, rx_len = sizeof(rx); Again, one declaration per line. > + > + tx.port = 0; > + tx.addr = addr; > + tx.mem_addr_len = 0; > + tx.mem_addr = 0; > + tx.buf_len = cpu_to_le16(data_len); > + > + ret = dln2_transfer(dln2->pdev, DLN2_I2C_READ, &tx, sizeof(tx), > + &rx, &rx_len); > + if (ret < 0) > + return ret; > + if (rx_len < 2) > + return -EPROTO; > + > + buf_len = le16_to_cpu(rx.buf_len); > + if (rx_len + sizeof(__le16) < buf_len) Aren't you counting sizeof(rx.buf_len) twice here? > + return -EPROTO; > + Either way, you are not verifying that the returned data does not overflow the supplied buffer (if you received more data than you asked for). > + memcpy(data, rx.buf, buf_len); > + > + return buf_len; > +} > + > +static int dln2_i2c_setup(struct dln2_i2c *dln2) > +{ > + int ret; > + > + ret = dln2_i2c_disable(dln2); > + if (ret < 0) > + return ret; > + > + /* Set I2C frequency */ > + ret = dln2_i2c_set_frequency(dln2, frequency); > + if (ret < 0) > + return ret; > + > + ret = dln2_i2c_enable(dln2); > + > + return ret; > +} > + > +static int dln2_i2c_xfer(struct i2c_adapter *adapter, > + struct i2c_msg *msgs, int num) > +{ > + struct dln2_i2c *dln2 = i2c_get_adapdata(adapter); > + struct i2c_msg *pmsg; > + int i; > + int ret = 0; No need to initialise. > + > + for (i = 0; i < num; i++) { > + pmsg = &msgs[i]; > + > + if (pmsg->len > DLN2_I2C_MAX_XFER_SIZE) > + return -EINVAL; > + > + if (pmsg->flags & I2C_M_RD) { > + ret = dln2_i2c_read(dln2, pmsg->addr, pmsg->buf, > + pmsg->len); > + if (ret < 0) > + return ret; > + > + pmsg->len = ret; > + } else { > + ret = dln2_i2c_write(dln2, pmsg->addr, pmsg->buf, > + pmsg->len); > + if (ret != pmsg->len) > + return -EPROTO; > + } > + } > + > + return num; > +} > + > +/* > + * Return list of supported functionality. > + */ > +static u32 dln2_i2c_func(struct i2c_adapter *a) > +{ > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA | > + I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BLOCK_PROC_CALL | > + I2C_FUNC_SMBUS_I2C_BLOCK; > +} > + > +static const struct i2c_algorithm dln2_i2c_usb_algorithm = { > + .master_xfer = dln2_i2c_xfer, > + .functionality = dln2_i2c_func, > +}; > + > +/* device layer */ > + > +static int dln2_i2c_probe(struct platform_device *pdev) > +{ > + int ret; > + struct device *dev = &pdev->dev; > + struct dln2_i2c *dln2 = kzalloc(sizeof(*dln2), GFP_KERNEL); Split declaration and non-trivial initialisation as I already mentioned in my comments to patch 1/3. Generally, any review-comment regarding style applies to the whole series. > + > + if (!dln2) > + return -ENOMEM; > + > + dln2->pdev = pdev; > + > + /* setup i2c adapter description */ > + dln2->adapter.owner = THIS_MODULE; > + dln2->adapter.class = I2C_CLASS_HWMON; > + dln2->adapter.algo = &dln2_i2c_usb_algorithm; > + dln2->adapter.dev.parent = dev; > + i2c_set_adapdata(&dln2->adapter, dln2); > + snprintf(dln2->adapter.name, sizeof(dln2->adapter.name), DRIVER_NAME); This probably needs to include the USB bus and device number since you can have more than of these devices connected. > + > + /* initialize the i2c interface */ > + ret = dln2_i2c_setup(dln2); > + if (ret < 0) { > + dev_err(dev, "failed to initialize adapter\n"); > + goto error_free; > + } > + > + /* and finally attach to i2c layer */ > + ret = i2c_add_adapter(&dln2->adapter); > + if (ret < 0) { > + dev_err(dev, "failed to add I2C adapter\n"); Shouldn't you disable the i2c master in the device? > + goto error_free; > + } > + > + platform_set_drvdata(pdev, dln2); > + > + return 0; > + > +error_free: > + kfree(dln2); > + > + return ret; > +} > + > +static int dln2_i2c_remove(struct platform_device *pdev) > +{ > + struct dln2_i2c *dln2 = platform_get_drvdata(pdev); > + > + i2c_del_adapter(&dln2->adapter); Same here. > + > + return 0; > +} > + > +static struct platform_driver dln2_i2c_driver = { > + .driver.name = DRIVER_NAME, > + .driver.owner = THIS_MODULE, > + .probe = dln2_i2c_probe, > + .remove = dln2_i2c_remove, > +}; > + > +module_platform_driver(dln2_i2c_driver); > + > +MODULE_AUTHOR("Laurentiu Palcu <laurentiu.palcu@intel.com>"); > +MODULE_DESCRIPTION(DRIVER_NAME " driver"); > +MODULE_LICENSE("GPL"); Johan -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 8, 2014 at 5:44 PM, Johan Hovold <johan@kernel.org> wrote: <snip> Hi Johan, Again, thanks for the detailed review, I am addressing your review comments as we speak. Some questions below. <snip> > > + int ret, len; > > + struct tx_data { > > + u8 port; > > + u8 addr; > > + u8 mem_addr_len; > > + __le32 mem_addr; > > + __le16 buf_len; > > + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; > > + } __packed tx; > > Allocate these buffers dynamically (possibly at probe). > I double checked this, and DLN2_I2C_MAX_XFER_SIZE should actually be < 64 as the USB endpoint configuration max packet size is 64. In this case, can we keep it on the stack? <snip> > > + int ret, buf_len, rx_len = sizeof(rx); > > Again, one declaration per line. > AFAICS there are many places where declaration on the same line (initialization included) are used. When did this became a coding style issue? Thanks, Tavi -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 08, 2014 at 06:57:29PM +0300, Octavian Purdila wrote: > On Mon, Sep 8, 2014 at 5:44 PM, Johan Hovold <johan@kernel.org> wrote: > > <snip> > > Hi Johan, > > Again, thanks for the detailed review, I am addressing your review > comments as we speak. Some questions below. > > <snip> > > > > + int ret, len; > > > + struct tx_data { > > > + u8 port; > > > + u8 addr; > > > + u8 mem_addr_len; > > > + __le32 mem_addr; > > > + __le16 buf_len; > > > + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; > > > + } __packed tx; > > > > Allocate these buffers dynamically (possibly at probe). > > > > I double checked this, and DLN2_I2C_MAX_XFER_SIZE should actually be < > 64 as the USB endpoint configuration max packet size is 64. In this > case, can we keep it on the stack? It's better to lift that restriction and allocate it dynamically. Using larger buffers (> EP size) is also more efficient. > <snip> > > > > + int ret, buf_len, rx_len = sizeof(rx); > > > > Again, one declaration per line. > > AFAICS there are many places where declaration on the same line > (initialization included) are used. When did this became a coding > style issue? It's ugly, hurts readability, and can also obfuscate the fact that your function really needs to be refactored. And it's in the CodingStyle: "To this end, use just one data declaration per line (no commas for multiple data declarations)." Johan -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 8, 2014 at 7:30 PM, Johan Hovold <johan@kernel.org> wrote: > On Mon, Sep 08, 2014 at 06:57:29PM +0300, Octavian Purdila wrote: >> On Mon, Sep 8, 2014 at 5:44 PM, Johan Hovold <johan@kernel.org> wrote: >> >> <snip> >> >> Hi Johan, >> >> Again, thanks for the detailed review, I am addressing your review >> comments as we speak. Some questions below. >> >> <snip> >> >> > > + int ret, len; >> > > + struct tx_data { >> > > + u8 port; >> > > + u8 addr; >> > > + u8 mem_addr_len; >> > > + __le32 mem_addr; >> > > + __le16 buf_len; >> > > + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; >> > > + } __packed tx; >> > >> > Allocate these buffers dynamically (possibly at probe). >> > >> >> I double checked this, and DLN2_I2C_MAX_XFER_SIZE should actually be < >> 64 as the USB endpoint configuration max packet size is 64. In this >> case, can we keep it on the stack? > > It's better to lift that restriction and allocate it dynamically. Using > larger buffers (> EP size) is also more efficient. > >> <snip> >> >> > > + int ret, buf_len, rx_len = sizeof(rx); >> > >> > Again, one declaration per line. >> >> AFAICS there are many places where declaration on the same line >> (initialization included) are used. When did this became a coding >> style issue? > > It's ugly, hurts readability, and can also obfuscate the fact that your > function really needs to be refactored. > > And it's in the CodingStyle: > > "To this end, use just one data declaration per line (no commas > for multiple data declarations)." > OK, I always thought that was for when declaring structures/unions. Just one more question on this subject: is the following allowed: int ret, len; or should it be: int ret; int len; -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 08, 2014 at 08:15:07PM +0300, Octavian Purdila wrote: > Just one more question on this subject: is the following allowed: > > int ret, len; > > or should it be: > > int ret; > int len; I try to avoid it, at least unless obviously related such as min/max or x/y. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 08 Sep 2014, Octavian Purdila wrote: > On Mon, Sep 8, 2014 at 7:30 PM, Johan Hovold <johan@kernel.org> wrote: > > On Mon, Sep 08, 2014 at 06:57:29PM +0300, Octavian Purdila wrote: > >> On Mon, Sep 8, 2014 at 5:44 PM, Johan Hovold <johan@kernel.org> wrote: > >> > >> <snip> > >> > >> Hi Johan, > >> > >> Again, thanks for the detailed review, I am addressing your review > >> comments as we speak. Some questions below. > >> > >> <snip> > >> > >> > > + int ret, len; > >> > > + struct tx_data { > >> > > + u8 port; > >> > > + u8 addr; > >> > > + u8 mem_addr_len; > >> > > + __le32 mem_addr; > >> > > + __le16 buf_len; > >> > > + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; > >> > > + } __packed tx; > >> > > >> > Allocate these buffers dynamically (possibly at probe). > >> > > >> > >> I double checked this, and DLN2_I2C_MAX_XFER_SIZE should actually be < > >> 64 as the USB endpoint configuration max packet size is 64. In this > >> case, can we keep it on the stack? > > > > It's better to lift that restriction and allocate it dynamically. Using > > larger buffers (> EP size) is also more efficient. > > > >> <snip> > >> > >> > > + int ret, buf_len, rx_len = sizeof(rx); > >> > > >> > Again, one declaration per line. > >> > >> AFAICS there are many places where declaration on the same line > >> (initialization included) are used. When did this became a coding > >> style issue? > > > > It's ugly, hurts readability, and can also obfuscate the fact that your > > function really needs to be refactored. > > > > And it's in the CodingStyle: > > > > "To this end, use just one data declaration per line (no commas > > for multiple data declarations)." > > > > OK, I always thought that was for when declaring structures/unions. > Just one more question on this subject: is the following allowed: > > int ret, len; > > or should it be: > > int ret; > int len; Common sense usually prevails here. I sometimes bunch up the really simple/obvious/throw-away variables like; i, j, k, ret, val etc, especially when there are a lot of variables in use, but it's nicer to see the less used, more complex ones on separate lines.
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 2ac87fa..4873161 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -1021,4 +1021,14 @@ config SCx200_ACB This support is also available as a module. If so, the module will be called scx200_acb. +config I2C_DLN2 + tristate "Diolan DLN-2 USB I2C adapter" + depends on USB && MFD_DLN2 + help + If you say yes to this option, support will be included for Diolan + DLN2, a USB to I2C interface. + + This driver can also be built as a module. If so, the module + will be called i2c-dln2. + endmenu diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index 49bf07e..3118fea 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -100,5 +100,6 @@ obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o obj-$(CONFIG_SCx200_ACB) += scx200_acb.o +obj-$(CONFIG_I2C_DLN2) += i2c-dln2.o ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG diff --git a/drivers/i2c/busses/i2c-dln2.c b/drivers/i2c/busses/i2c-dln2.c new file mode 100644 index 0000000..93e85ff --- /dev/null +++ b/drivers/i2c/busses/i2c-dln2.c @@ -0,0 +1,301 @@ +/* + * Driver for the Diolan DLN-2 USB-I2C adapter + * + * Copyright (c) 2014 Intel Corporation + * + * Derived from: + * i2c-diolan-u2c.c + * Copyright (c) 2010-2011 Ericsson AB + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, version 2. + */ + +#include <linux/kernel.h> +#include <linux/errno.h> +#include <linux/module.h> +#include <linux/types.h> +#include <linux/slab.h> +#include <linux/i2c.h> + +#include <linux/platform_device.h> +#include <linux/mfd/dln2.h> + +#define DRIVER_NAME "dln2-i2c" + +#define DLN2_I2C_MODULE_ID 0x03 +#define DLN2_I2C_CMD(cmd) DLN2_CMD(cmd, DLN2_I2C_MODULE_ID) + +/* I2C commands */ +#define DLN2_I2C_GET_PORT_COUNT DLN2_I2C_CMD(0x00) +#define DLN2_I2C_ENABLE DLN2_I2C_CMD(0x01) +#define DLN2_I2C_DISABLE DLN2_I2C_CMD(0x02) +#define DLN2_I2C_IS_ENABLED DLN2_I2C_CMD(0x03) +#define DLN2_I2C_SET_FREQUENCY DLN2_I2C_CMD(0x04) +#define DLN2_I2C_GET_FREQUENCY DLN2_I2C_CMD(0x05) +#define DLN2_I2C_WRITE DLN2_I2C_CMD(0x06) +#define DLN2_I2C_READ DLN2_I2C_CMD(0x07) +#define DLN2_I2C_SCAN_DEVICES DLN2_I2C_CMD(0x08) +#define DLN2_I2C_PULLUP_ENABLE DLN2_I2C_CMD(0x09) +#define DLN2_I2C_PULLUP_DISABLE DLN2_I2C_CMD(0x0A) +#define DLN2_I2C_PULLUP_IS_ENABLED DLN2_I2C_CMD(0x0B) +#define DLN2_I2C_TRANSFER DLN2_I2C_CMD(0x0C) +#define DLN2_I2C_SET_MAX_REPLY_COUNT DLN2_I2C_CMD(0x0D) +#define DLN2_I2C_GET_MAX_REPLY_COUNT DLN2_I2C_CMD(0x0E) +#define DLN2_I2C_GET_MIN_FREQUENCY DLN2_I2C_CMD(0x40) +#define DLN2_I2C_GET_MAX_FREQUENCY DLN2_I2C_CMD(0x41) + +#define DLN2_I2C_FREQ_FAST 400000 +#define DLN2_I2C_FREQ_STD 100000 + +#define DLN2_I2C_MAX_XFER_SIZE 256 + +struct dln2_i2c { + struct platform_device *pdev; + struct i2c_adapter adapter; +}; + +static uint frequency = DLN2_I2C_FREQ_STD; /* I2C clock frequency in Hz */ + +module_param(frequency, uint, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(frequency, "I2C clock frequency in hertz"); + +static int dln2_i2c_set_state(struct dln2_i2c *dln2, u8 state) +{ + int ret; + u8 port = 0; + + ret = dln2_transfer(dln2->pdev, + state ? DLN2_I2C_ENABLE : DLN2_I2C_DISABLE, + &port, sizeof(port), NULL, NULL); + + if (ret < 0) + return ret; + + return 0; +} + +#define dln2_i2c_enable(dln2) dln2_i2c_set_state(dln2, 1) +#define dln2_i2c_disable(dln2) dln2_i2c_set_state(dln2, 0) + +static int dln2_i2c_set_frequency(struct dln2_i2c *dln2, u32 freq) +{ + int ret; + struct tx_data { + u8 port; + __le32 freq; + } __packed tx; + + tx.port = 0; + tx.freq = cpu_to_le32(freq); + + ret = dln2_transfer(dln2->pdev, DLN2_I2C_SET_FREQUENCY, &tx, sizeof(tx), + NULL, NULL); + if (ret < 0) + return ret; + + return 0; +} + +static int dln2_i2c_write(struct dln2_i2c *dln2, u8 addr, + u8 *data, u16 data_len) +{ + int ret, len; + struct tx_data { + u8 port; + u8 addr; + u8 mem_addr_len; + __le32 mem_addr; + __le16 buf_len; + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; + } __packed tx; + + if (data_len > DLN2_I2C_MAX_XFER_SIZE) + return -ENOSPC; + + tx.port = 0; + tx.addr = addr; + tx.mem_addr_len = 0; + tx.mem_addr = 0; + tx.buf_len = cpu_to_le16(data_len); + memcpy(tx.buf, data, data_len); + + len = sizeof(tx) + data_len - DLN2_I2C_MAX_XFER_SIZE; + ret = dln2_transfer(dln2->pdev, DLN2_I2C_WRITE, &tx, len, + NULL, NULL); + if (ret < 0) + return ret; + + return data_len; +} + +static int dln2_i2c_read(struct dln2_i2c *dln2, u16 addr, u8 *data, + u16 data_len) +{ + struct tx_data { + u8 port; + u8 addr; + u8 mem_addr_len; + __le32 mem_addr; + __le16 buf_len; + } __packed tx; + struct rx_data { + __le16 buf_len; + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; + } __packed rx; + int ret, buf_len, rx_len = sizeof(rx); + + tx.port = 0; + tx.addr = addr; + tx.mem_addr_len = 0; + tx.mem_addr = 0; + tx.buf_len = cpu_to_le16(data_len); + + ret = dln2_transfer(dln2->pdev, DLN2_I2C_READ, &tx, sizeof(tx), + &rx, &rx_len); + if (ret < 0) + return ret; + if (rx_len < 2) + return -EPROTO; + + buf_len = le16_to_cpu(rx.buf_len); + if (rx_len + sizeof(__le16) < buf_len) + return -EPROTO; + + memcpy(data, rx.buf, buf_len); + + return buf_len; +} + +static int dln2_i2c_setup(struct dln2_i2c *dln2) +{ + int ret; + + ret = dln2_i2c_disable(dln2); + if (ret < 0) + return ret; + + /* Set I2C frequency */ + ret = dln2_i2c_set_frequency(dln2, frequency); + if (ret < 0) + return ret; + + ret = dln2_i2c_enable(dln2); + + return ret; +} + +static int dln2_i2c_xfer(struct i2c_adapter *adapter, + struct i2c_msg *msgs, int num) +{ + struct dln2_i2c *dln2 = i2c_get_adapdata(adapter); + struct i2c_msg *pmsg; + int i; + int ret = 0; + + for (i = 0; i < num; i++) { + pmsg = &msgs[i]; + + if (pmsg->len > DLN2_I2C_MAX_XFER_SIZE) + return -EINVAL; + + if (pmsg->flags & I2C_M_RD) { + ret = dln2_i2c_read(dln2, pmsg->addr, pmsg->buf, + pmsg->len); + if (ret < 0) + return ret; + + pmsg->len = ret; + } else { + ret = dln2_i2c_write(dln2, pmsg->addr, pmsg->buf, + pmsg->len); + if (ret != pmsg->len) + return -EPROTO; + } + } + + return num; +} + +/* + * Return list of supported functionality. + */ +static u32 dln2_i2c_func(struct i2c_adapter *a) +{ + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA | + I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BLOCK_PROC_CALL | + I2C_FUNC_SMBUS_I2C_BLOCK; +} + +static const struct i2c_algorithm dln2_i2c_usb_algorithm = { + .master_xfer = dln2_i2c_xfer, + .functionality = dln2_i2c_func, +}; + +/* device layer */ + +static int dln2_i2c_probe(struct platform_device *pdev) +{ + int ret; + struct device *dev = &pdev->dev; + struct dln2_i2c *dln2 = kzalloc(sizeof(*dln2), GFP_KERNEL); + + if (!dln2) + return -ENOMEM; + + dln2->pdev = pdev; + + /* setup i2c adapter description */ + dln2->adapter.owner = THIS_MODULE; + dln2->adapter.class = I2C_CLASS_HWMON; + dln2->adapter.algo = &dln2_i2c_usb_algorithm; + dln2->adapter.dev.parent = dev; + i2c_set_adapdata(&dln2->adapter, dln2); + snprintf(dln2->adapter.name, sizeof(dln2->adapter.name), DRIVER_NAME); + + /* initialize the i2c interface */ + ret = dln2_i2c_setup(dln2); + if (ret < 0) { + dev_err(dev, "failed to initialize adapter\n"); + goto error_free; + } + + /* and finally attach to i2c layer */ + ret = i2c_add_adapter(&dln2->adapter); + if (ret < 0) { + dev_err(dev, "failed to add I2C adapter\n"); + goto error_free; + } + + platform_set_drvdata(pdev, dln2); + + return 0; + +error_free: + kfree(dln2); + + return ret; +} + +static int dln2_i2c_remove(struct platform_device *pdev) +{ + struct dln2_i2c *dln2 = platform_get_drvdata(pdev); + + i2c_del_adapter(&dln2->adapter); + + return 0; +} + +static struct platform_driver dln2_i2c_driver = { + .driver.name = DRIVER_NAME, + .driver.owner = THIS_MODULE, + .probe = dln2_i2c_probe, + .remove = dln2_i2c_remove, +}; + +module_platform_driver(dln2_i2c_driver); + +MODULE_AUTHOR("Laurentiu Palcu <laurentiu.palcu@intel.com>"); +MODULE_DESCRIPTION(DRIVER_NAME " driver"); +MODULE_LICENSE("GPL");