[PATCH 1/2] i2c: i2c-tiny-usb: Remove RobotFuzz USB vendor:product ID

[PATCH 1/2] i2c: i2c-tiny-usb: Remove RobotFuzz USB vendor:product ID

[Andrew Lunn]

The RobotFuzz device is not compatible with i2c-tiny-usb. Remove its
entry from the USB table so that the new correct driver can be used.

Signed-off-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
CC: Emmanuel Deloget <logout-GANU6spQydw@public.gmane.org>
CC: Barry Carter <barry.carter-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---

FYI: I'm away from the net for the next two weeks. So please take your time
with reviewing.

 drivers/i2c/busses/i2c-tiny-usb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-tiny-usb.c b/drivers/i2c/busses/i2c-tiny-usb.c
index e7d3b755af3b..0ed77eeff31e 100644
--- a/drivers/i2c/busses/i2c-tiny-usb.c
+++ b/drivers/i2c/busses/i2c-tiny-usb.c
@@ -162,7 +162,6 @@ static const struct i2c_algorithm usb_algorithm = {
 static const struct usb_device_id i2c_tiny_usb_table[] = {
 	{ USB_DEVICE(0x0403, 0xc631) },   /* FTDI */
 	{ USB_DEVICE(0x1c40, 0x0534) },   /* EZPrototypes */
-	{ USB_DEVICE(0x1964, 0x0001) },   /* Robofuzz OSIF */
 	{ }                               /* Terminating entry */
 };
 
-- 
1.8.4.rc3

[PATCH 2/2] i2c: Add bus driver for for OSIF USB i2c device.

[Andrew Lunn]

OSIF, Open Source InterFace, is a USB based i2c bus master.  The
origional design was based on i2c-tiny-usb, but more modern versions
of the firmware running on the MegaAVR microcontroller use a different
protocol over the USB. This code is based on Barry Carter
<barry.carter-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> driver.

Signed-off-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
CC: Emmanuel Deloget <logout-GANU6spQydw@public.gmane.org>
CC: Barry Carter <barry.carter-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/Kconfig    |  10 ++
 drivers/i2c/busses/Makefile   |   1 +
 drivers/i2c/busses/i2c-osif.c | 267 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 278 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-osif.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index cdcbd8368ed3..e7ed9edd79e2 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -788,6 +788,16 @@ config I2C_DIOLAN_U2C
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-diolan-u2c.
 
+config I2C_OSIF
+	tristate "RobotFuzz Open Source InterFace USB adapter"
+	depends on USB
+	help
+	  If you say yes to this option, support will be included for the
+	  RobotFuzz Open Source InterFace USB to I2C interface.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-osif.
+
 config I2C_PARPORT
 	tristate "Parallel port adapter"
 	depends on PARPORT
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index d00997f3eb3b..4c147862b2a7 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_I2C_RCAR)		+= i2c-rcar.o
 
 # External I2C/SMBus adapter drivers
 obj-$(CONFIG_I2C_DIOLAN_U2C)	+= i2c-diolan-u2c.o
+obj-$(CONFIG_I2C_OSIF)		+= i2c-osif.o
 obj-$(CONFIG_I2C_PARPORT)	+= i2c-parport.o
 obj-$(CONFIG_I2C_PARPORT_LIGHT)	+= i2c-parport-light.o
 obj-$(CONFIG_I2C_TAOS_EVM)	+= i2c-taos-evm.o
diff --git a/drivers/i2c/busses/i2c-osif.c b/drivers/i2c/busses/i2c-osif.c
new file mode 100644
index 000000000000..8c12407d88c2
--- /dev/null
+++ b/drivers/i2c/busses/i2c-osif.c
@@ -0,0 +1,267 @@
+/*
+ * Driver for RobotFuzz OSIF
+ *
+ * Copyright (c) 2007 Barry Carter <Barry.Carter-hIwygO75nSHCXR85Y8Hu3Q@public.gmane.org>
+ * Copyright (c) 2013 Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
+ *
+ * Based on the i2c-tiny-usb by
+ *
+ * Copyright (C) 2006 Til Harbaum (Till-zicpKgigMvpAfugRpC6u6w@public.gmane.org)
+ *
+ *	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/module.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+
+#define DRIVER_AUTHOR  "Barry Carter <barry.carter-hIwygO75nSHCXR85Y8Hu3Q@public.gmane.org>,"	\
+	"Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>"
+#define DRIVER_DESC    "OSIF driver"
+
+#define OSIFI2C_READ		20 /* Read from i2c bus */
+#define OSIFI2C_WRITE		21 /* Write to i2c bus */
+#define OSIFI2C_STOP		22 /* Send stop condition */
+#define OSIFI2C_STATUS		23 /* Get status from i2c action */
+#define OSIFI2C_SET_BIT_RATE	24 /* Set the bit rate & prescaler */
+
+#define STATUS_ADDRESS_ACK	0
+#define STATUS_ADDRESS_NAK	2
+
+static int usb_read(struct i2c_adapter *adapter, int cmd,
+		    int value, int index, void *data, int len);
+
+static int usb_write(struct i2c_adapter *adapter, int cmd,
+		     int value, int index, void *data, int len);
+
+/* ----- begin of i2c layer ---------------------------------------------- */
+
+static int usb_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
+{
+	unsigned char *pstatus;
+	struct i2c_msg *pmsg;
+	int ret = 0;
+	int i, cmd;
+
+	dev_dbg(&adapter->dev, "master xfer %d messages:\n", num);
+
+	pstatus = kmalloc(sizeof(*pstatus), GFP_KERNEL);
+	if (!pstatus)
+		return -ENOMEM;
+
+	for (i = 0; ret >= 0 && i < num; i++) {
+
+		pmsg = &msgs[i];
+
+		dev_dbg(&adapter->dev,
+			"  %d: %s (flags %d) %d bytes to 0x%02x\n",
+			i, pmsg->flags & I2C_M_RD ? "read" : "write",
+			pmsg->flags, pmsg->len, pmsg->addr);
+
+		if (pmsg->flags & I2C_M_RD) {
+			cmd = OSIFI2C_READ;
+
+			ret = usb_read(adapter, cmd, pmsg->flags, pmsg->addr,
+				       pmsg->buf, pmsg->len);
+			if (ret != pmsg->len) {
+				dev_err(&adapter->dev,
+					"failure reading data\n");
+				ret = -EREMOTEIO;
+				goto out;
+			}
+		} else {
+			cmd = OSIFI2C_WRITE;
+
+			ret = usb_write(adapter, cmd, pmsg->flags, pmsg->addr,
+					pmsg->buf, pmsg->len);
+			if (ret != pmsg->len) {
+				dev_err(&adapter->dev,
+					"failure writing data\n");
+				ret = -EREMOTEIO;
+				goto out;
+			}
+		}
+
+		ret = usb_read(adapter, OSIFI2C_STOP, 0, 0, NULL, 0);
+		if (ret != 0) {
+			dev_err(&adapter->dev, "failure sending STOP\n");
+			ret = -EREMOTEIO;
+			goto out;
+		}
+
+		/* read status */
+		ret = usb_read(adapter, OSIFI2C_STATUS, 0, 0, pstatus, 1);
+		if (ret != 1) {
+			dev_err(&adapter->dev, "failure reading status\n");
+			ret = -EREMOTEIO;
+			goto out;
+		}
+
+		dev_dbg(&adapter->dev, "status = %d\n", *pstatus);
+		if (*pstatus != STATUS_ADDRESS_ACK) {
+			ret = -EREMOTEIO;
+			goto out;
+		}
+	}
+
+	ret = i;
+out:
+	kfree(pstatus);
+	return ret;
+}
+
+static u32 usb_func(struct i2c_adapter *adapter)
+{
+	/* configure for I2c mode and SMBUS emulation */
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+/* This is the actual algorithm we define */
+static struct i2c_algorithm usb_algorithm = {
+	.master_xfer	= usb_xfer,
+	.functionality	= usb_func,
+};
+
+/* ----- end of i2c layer ---------------------------------------------- */
+
+/* ----- begin of usb layer ---------------------------------------------- */
+
+/* the usb i2c interface uses a vid/pid pair donated by ftdi */
+#define USB_OSIF_VENDOR_ID	0x1964
+#define USB_OSIF_PRODUCT_ID	0x0001
+
+/* table of devices that work with this driver */
+static struct usb_device_id osif_table[] = {
+	{ USB_DEVICE(USB_OSIF_VENDOR_ID, USB_OSIF_PRODUCT_ID) },
+	{ } /* Terminating entry */
+};
+
+MODULE_DEVICE_TABLE(usb, osif_table);
+
+/* Structure to hold all of our device specific stuff */
+struct priv {
+	struct usb_device *usb_dev; /* the usb device for this device */
+	struct usb_interface *interface; /* the interface for this device */
+	struct i2c_adapter adapter; /* i2c related things */
+};
+
+static int usb_read(struct i2c_adapter *adapter, int cmd,
+		    int value, int index, void *data, int len)
+{
+	struct priv *priv = (struct priv *)adapter->algo_data;
+
+	/* do control transfer */
+	return usb_control_msg(priv->usb_dev, usb_rcvctrlpipe(priv->usb_dev, 0),
+			       cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE |
+			       USB_DIR_IN, value, index, data, len, 2000);
+}
+
+static int usb_write(struct i2c_adapter *adapter, int cmd,
+		    int value, int index, void *data, int len)
+{
+
+	struct priv *priv = (struct priv *)adapter->algo_data;
+
+	/* do control transfer */
+	return usb_control_msg(priv->usb_dev, usb_sndctrlpipe(priv->usb_dev, 0),
+			       cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
+			       value, index, data, len, 2000);
+}
+
+static void osif_free(struct priv *priv)
+{
+	usb_put_dev(priv->usb_dev);
+}
+
+static int osif_probe(struct usb_interface *interface,
+			     const struct usb_device_id *id)
+{
+	struct priv *priv = NULL;
+	int retval = -ENOMEM;
+	u16 version;
+
+	dev_dbg(&interface->dev, "probing usb device");
+
+	/* allocate memory for our device state and initialize it */
+	priv = devm_kzalloc(&interface->dev, sizeof(*priv), GFP_KERNEL);
+	if (priv == NULL) {
+		dev_err(&interface->dev, "Out of memory\n");
+		goto error;
+	}
+
+	priv->usb_dev = usb_get_dev(interface_to_usbdev(interface));
+	priv->interface = interface;
+
+	/* save our data pointer in this interface device */
+	usb_set_intfdata(interface, priv);
+
+	version = le16_to_cpu(priv->usb_dev->descriptor.bcdDevice);
+	dev_info(&interface->dev,
+		 "version %x.%02x found at bus %03d address %03d",
+		 version >> 8, version & 0xff,
+		 priv->usb_dev->bus->busnum, priv->usb_dev->devnum);
+
+	/* setup i2c adapter description */
+	priv->adapter.owner = THIS_MODULE;
+	priv->adapter.class = I2C_CLASS_HWMON;
+	priv->adapter.algo = &usb_algorithm;
+	priv->adapter.algo_data = priv;
+	snprintf(priv->adapter.name, sizeof(priv->adapter.name),
+		 "OSIF at bus %03d device %03d",
+		 priv->usb_dev->bus->busnum, priv->usb_dev->devnum);
+
+	/*
+	 * Set bus frequency. The frequency is:
+	 * 120,000,000 / ( 16 + 2(div)*4^prescale).
+	 * Using dev=52, prescale =0 give 100KHz */
+	retval = usb_read(&priv->adapter, OSIFI2C_SET_BIT_RATE, 52, 0, NULL, 0);
+	if (retval != 0) {
+		dev_err(&interface->dev, "failure sending bit rate");
+		retval = -EREMOTEIO;
+		goto error;
+	}
+
+	/* and finally attach to i2c layer */
+	i2c_add_adapter(&(priv->adapter));
+
+	/* inform user about successful attachment to i2c layer */
+	dev_info(&priv->adapter.dev, "connected OSIF device\n");
+
+	return 0;
+
+error:
+
+	if (priv)
+		osif_free(priv);
+
+	return retval;
+}
+
+static void osif_disconnect(struct usb_interface *interface)
+{
+	struct priv *priv = usb_get_intfdata(interface);
+
+	i2c_del_adapter(&(priv->adapter));
+	usb_set_intfdata(interface, NULL);
+	osif_free(priv);
+}
+
+static struct usb_driver osif_driver = {
+	.name		= "Open Source InterFace OSIF",
+	.probe		= osif_probe,
+	.disconnect	= osif_disconnect,
+	.id_table	= osif_table,
+};
+
+module_usb_driver(osif_driver);
+
+/* ----- end of usb layer ---------------------------------------------- */
+
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
-- 
1.8.4.rc3

Re: [RESEND PATCH v1 2/2] i2c: Add bus driver for for OSIF USB i2c device.

[Wolfram Sang]

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

Hi, thanks for the submission...


> +#define DRIVER_AUTHOR  "Barry Carter <barry.carter-hIwygO75nSHCXR85Y8Hu3Q@public.gmane.org>,"	\
> +	"Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>"
> +#define DRIVER_DESC    "OSIF driver"

No defines, use directly.

> +static int usb_read(struct i2c_adapter *adapter, int cmd,
> +		    int value, int index, void *data, int len);
> +
> +static int usb_write(struct i2c_adapter *adapter, int cmd,
> +		     int value, int index, void *data, int len);

With a bit of reshuffling, these can be skipped.

> +	dev_dbg(&adapter->dev, "master xfer %d messages:\n", num);

Skip. This info is available with I2C core debug messages.

> +	pstatus = kmalloc(sizeof(*pstatus), GFP_KERNEL);
> +	if (!pstatus)
> +		return -ENOMEM;

Does it really make sense to allocate this every time?

> +		dev_dbg(&adapter->dev,
> +			"  %d: %s (flags %d) %d bytes to 0x%02x\n",
> +			i, pmsg->flags & I2C_M_RD ? "read" : "write",
> +			pmsg->flags, pmsg->len, pmsg->addr);

This is also available with i2c core debug messages.

> +/* Structure to hold all of our device specific stuff */
> +struct priv {
> +	struct usb_device *usb_dev; /* the usb device for this device */
> +	struct usb_interface *interface; /* the interface for this device */
> +	struct i2c_adapter adapter; /* i2c related things */
> +};

Remove comments, too obvious IMO.

> +static int usb_read(struct i2c_adapter *adapter, int cmd,
> +		    int value, int index, void *data, int len)
> +{
> +	struct priv *priv = (struct priv *)adapter->algo_data;
> +
> +	/* do control transfer */

ditto.

> +	return usb_control_msg(priv->usb_dev, usb_rcvctrlpipe(priv->usb_dev, 0),
> +			       cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE |
> +			       USB_DIR_IN, value, index, data, len, 2000);
> +}
> +
> +static int usb_write(struct i2c_adapter *adapter, int cmd,
> +		    int value, int index, void *data, int len)
> +{
> +
> +	struct priv *priv = (struct priv *)adapter->algo_data;
> +
> +	/* do control transfer */

ditto.

> +	return usb_control_msg(priv->usb_dev, usb_sndctrlpipe(priv->usb_dev, 0),
> +			       cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
> +			       value, index, data, len, 2000);
> +}
> +
> +static void osif_free(struct priv *priv)
> +{
> +	usb_put_dev(priv->usb_dev);
> +}

Not sure if this is worth a seperate function, but well...

> +static int osif_probe(struct usb_interface *interface,
> +			     const struct usb_device_id *id)
> +{
> +	struct priv *priv = NULL;

Unneeded assignment.

> +	int retval = -ENOMEM;
> +	u16 version;
> +
> +	dev_dbg(&interface->dev, "probing usb device");

This is available via driver core debug messages.

> +	/* inform user about successful attachment to i2c layer */
> +	dev_info(&priv->adapter.dev, "connected OSIF device\n");

I think this message should be merged with the "version xx found"
message above.

> +MODULE_LICENSE("GPL");

"GPL v2" according to header.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/2] i2c: i2c-tiny-usb: Remove RobotFuzz USB vendor:product ID

[Andrew Lunn]

The RobotFuzz device is not compatible with i2c-tiny-usb. Remove its
entry from the USB table so that the new correct driver can be used.

Signed-off-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
CC: Emmanuel Deloget <logout-GANU6spQydw@public.gmane.org>
CC: Barry Carter <barry.carter-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-tiny-usb.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-tiny-usb.c b/drivers/i2c/busses/i2c-tiny-usb.c
index e7d3b755af3b..0ed77eeff31e 100644
--- a/drivers/i2c/busses/i2c-tiny-usb.c
+++ b/drivers/i2c/busses/i2c-tiny-usb.c
@@ -162,7 +162,6 @@ static const struct i2c_algorithm usb_algorithm = {
 static const struct usb_device_id i2c_tiny_usb_table[] = {
 	{ USB_DEVICE(0x0403, 0xc631) },   /* FTDI */
 	{ USB_DEVICE(0x1c40, 0x0534) },   /* EZPrototypes */
-	{ USB_DEVICE(0x1964, 0x0001) },   /* Robofuzz OSIF */
 	{ }                               /* Terminating entry */
 };
 
-- 
1.7.10.4

[PATCH 2/2] i2c: Add bus driver for for OSIF USB i2c device.

[Andrew Lunn]

OSIF, Open Source InterFace, is a USB based i2c bus master.  The
origional design was based on i2c-tiny-usb, but more modern versions
of the firmware running on the MegaAVR microcontroller use a different
protocol over the USB. This code is based on Barry Carter
<barry.carter-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> driver.

Signed-off-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
CC: Emmanuel Deloget <logout-GANU6spQydw@public.gmane.org>
CC: Barry Carter <barry.carter-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
v2:

Rearranged order of functions to remove forward declarations
Remove some obvious comments
Remove debug prints which the core also provides
Remove defines for module authors.
Allocate status byte once in priv, rather than for each xfer.
Fix MODULE_LICENSE()
Rename file and Kconfig entry for consistency and ordering.
Inline osif_free()
---
 drivers/i2c/busses/Kconfig              |   10 ++
 drivers/i2c/busses/Makefile             |    1 +
 drivers/i2c/busses/i2c-robotfuzz-osif.c |  201 +++++++++++++++++++++++++++++++
 3 files changed, 212 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-robotfuzz-osif.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index cdcbd8368ed3..ed1bd7a46c20 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -837,6 +837,16 @@ config I2C_PARPORT_LIGHT
 	  This support is also available as a module.  If so, the module
 	  will be called i2c-parport-light.
 
+config I2C_ROBOTFUZZ_OSIF
+	tristate "RobotFuzz Open Source InterFace USB adapter"
+	depends on USB
+	help
+	  If you say yes to this option, support will be included for the
+	  RobotFuzz Open Source InterFace USB to I2C interface.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-osif.
+
 config I2C_TAOS_EVM
 	tristate "TAOS evaluation module"
 	depends on TTY
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index d00997f3eb3b..f171fa8b1a72 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_I2C_RCAR)		+= i2c-rcar.o
 obj-$(CONFIG_I2C_DIOLAN_U2C)	+= i2c-diolan-u2c.o
 obj-$(CONFIG_I2C_PARPORT)	+= i2c-parport.o
 obj-$(CONFIG_I2C_PARPORT_LIGHT)	+= i2c-parport-light.o
+obj-$(CONFIG_I2C_ROBOTFUZZ_OSIF)	+= i2c-robotfuzz-osif.o
 obj-$(CONFIG_I2C_TAOS_EVM)	+= i2c-taos-evm.o
 obj-$(CONFIG_I2C_TINY_USB)	+= i2c-tiny-usb.o
 obj-$(CONFIG_I2C_VIPERBOARD)	+= i2c-viperboard.o
diff --git a/drivers/i2c/busses/i2c-robotfuzz-osif.c b/drivers/i2c/busses/i2c-robotfuzz-osif.c
new file mode 100644
index 000000000000..3e03c008775a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-robotfuzz-osif.c
@@ -0,0 +1,201 @@
+/*
+ * Driver for RobotFuzz OSIF
+ *
+ * Copyright (c) 2013 Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
+ * Copyright (c) 2007 Barry Carter <Barry.Carter-hIwygO75nSHCXR85Y8Hu3Q@public.gmane.org>
+ *
+ * Based on the i2c-tiny-usb by
+ *
+ * Copyright (C) 2006 Til Harbaum (Till-zicpKgigMvpAfugRpC6u6w@public.gmane.org)
+ *
+ *	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/module.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+
+#define OSIFI2C_READ		20 /* Read from i2c bus */
+#define OSIFI2C_WRITE		21 /* Write to i2c bus */
+#define OSIFI2C_STOP		22 /* Send stop condition */
+#define OSIFI2C_STATUS		23 /* Get status from i2c action */
+#define OSIFI2C_SET_BIT_RATE	24 /* Set the bit rate & prescaler */
+
+#define STATUS_ADDRESS_ACK	0
+#define STATUS_ADDRESS_NAK	2
+
+struct priv {
+	struct usb_device *usb_dev;
+	struct usb_interface *interface;
+	struct i2c_adapter adapter;
+	unsigned char status;
+};
+
+static int usb_read(struct i2c_adapter *adapter, int cmd,
+		    int value, int index, void *data, int len)
+{
+	struct priv *priv = (struct priv *)adapter->algo_data;
+
+	return usb_control_msg(priv->usb_dev, usb_rcvctrlpipe(priv->usb_dev, 0),
+			       cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE |
+			       USB_DIR_IN, value, index, data, len, 2000);
+}
+
+static int usb_write(struct i2c_adapter *adapter, int cmd,
+		    int value, int index, void *data, int len)
+{
+
+	struct priv *priv = (struct priv *)adapter->algo_data;
+
+	return usb_control_msg(priv->usb_dev, usb_sndctrlpipe(priv->usb_dev, 0),
+			       cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
+			       value, index, data, len, 2000);
+}
+
+static int usb_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
+{
+	struct priv *priv = (struct priv *)adapter->algo_data;
+	struct i2c_msg *pmsg;
+	int ret = 0;
+	int i, cmd;
+
+	for (i = 0; ret >= 0 && i < num; i++) {
+		pmsg = &msgs[i];
+
+		if (pmsg->flags & I2C_M_RD) {
+			cmd = OSIFI2C_READ;
+
+			ret = usb_read(adapter, cmd, pmsg->flags, pmsg->addr,
+				       pmsg->buf, pmsg->len);
+			if (ret != pmsg->len) {
+				dev_err(&adapter->dev,
+					"failure reading data\n");
+				return -EREMOTEIO;
+			}
+		} else {
+			cmd = OSIFI2C_WRITE;
+
+			ret = usb_write(adapter, cmd, pmsg->flags, pmsg->addr,
+					pmsg->buf, pmsg->len);
+			if (ret != pmsg->len) {
+				dev_err(&adapter->dev,
+					"failure writing data\n");
+				return -EREMOTEIO;
+			}
+		}
+
+		ret = usb_read(adapter, OSIFI2C_STOP, 0, 0, NULL, 0);
+		if (ret != 0) {
+			dev_err(&adapter->dev, "failure sending STOP\n");
+			return -EREMOTEIO;
+		}
+
+		/* read status */
+		ret = usb_read(adapter, OSIFI2C_STATUS, 0, 0, &priv->status, 1);
+		if (ret != 1) {
+			dev_err(&adapter->dev, "failure reading status\n");
+			return -EREMOTEIO;
+		}
+
+		if (priv->status != STATUS_ADDRESS_ACK) {
+			dev_dbg(&adapter->dev, "status = %d\n", priv->status);
+			return -EREMOTEIO;
+		}
+	}
+
+	return i;
+}
+
+static u32 usb_func(struct i2c_adapter *adapter)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm usb_algorithm = {
+	.master_xfer	= usb_xfer,
+	.functionality	= usb_func,
+};
+
+#define USB_OSIF_VENDOR_ID	0x1964
+#define USB_OSIF_PRODUCT_ID	0x0001
+
+static struct usb_device_id osif_table[] = {
+	{ USB_DEVICE(USB_OSIF_VENDOR_ID, USB_OSIF_PRODUCT_ID) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(usb, osif_table);
+
+static int osif_probe(struct usb_interface *interface,
+			     const struct usb_device_id *id)
+{
+	int retval;
+	struct priv *priv;
+	u16 version;
+
+	priv = devm_kzalloc(&interface->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->usb_dev = usb_get_dev(interface_to_usbdev(interface));
+	priv->interface = interface;
+
+	usb_set_intfdata(interface, priv);
+
+	priv->adapter.owner = THIS_MODULE;
+	priv->adapter.class = I2C_CLASS_HWMON;
+	priv->adapter.algo = &usb_algorithm;
+	priv->adapter.algo_data = priv;
+	snprintf(priv->adapter.name, sizeof(priv->adapter.name),
+		 "OSIF at bus %03d device %03d",
+		 priv->usb_dev->bus->busnum, priv->usb_dev->devnum);
+
+	/*
+	 * Set bus frequency. The frequency is:
+	 * 120,000,000 / ( 16 + 2 * div * 4^prescale).
+	 * Using dev = 52, prescale = 0 give 100KHz */
+	retval = usb_read(&priv->adapter, OSIFI2C_SET_BIT_RATE, 52, 0, NULL, 0);
+	if (retval) {
+		dev_err(&interface->dev, "failure sending bit rate");
+		usb_put_dev(priv->usb_dev);
+		return retval;
+	}
+
+	i2c_add_adapter(&(priv->adapter));
+
+	version = le16_to_cpu(priv->usb_dev->descriptor.bcdDevice);
+	dev_info(&interface->dev,
+		 "version %x.%02x found at bus %03d address %03d",
+		 version >> 8, version & 0xff,
+		 priv->usb_dev->bus->busnum, priv->usb_dev->devnum);
+
+	return 0;
+}
+
+static void osif_disconnect(struct usb_interface *interface)
+{
+	struct priv *priv = usb_get_intfdata(interface);
+
+	i2c_del_adapter(&(priv->adapter));
+	usb_set_intfdata(interface, NULL);
+	usb_put_dev(priv->usb_dev);
+}
+
+static struct usb_driver osif_driver = {
+	.name		= "RobotFuzz Open Source InterFace, OSIF",
+	.probe		= osif_probe,
+	.disconnect	= osif_disconnect,
+	.id_table	= osif_table,
+};
+
+module_usb_driver(osif_driver);
+
+MODULE_AUTHOR("Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>");
+MODULE_AUTHOR("Barry Carter <barry.carter-hIwygO75nSHCXR85Y8Hu3Q@public.gmane.org>");
+MODULE_DESCRIPTION("RobotFuzz OSIF driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.10.4

Re: [PATCH 2/2] i2c: Add bus driver for for OSIF USB i2c device.

[Wolfram Sang]

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

On Mon, Jan 06, 2014 at 04:55:36AM +0100, Andrew Lunn wrote:
> OSIF, Open Source InterFace, is a USB based i2c bus master.  The
> origional design was based on i2c-tiny-usb, but more modern versions
> of the firmware running on the MegaAVR microcontroller use a different
> protocol over the USB. This code is based on Barry Carter
> <barry.carter-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> driver.
> 
> Signed-off-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>

We are getting close...


> +#define OSIFI2C_READ		20 /* Read from i2c bus */
> +#define OSIFI2C_WRITE		21 /* Write to i2c bus */
> +#define OSIFI2C_STOP		22 /* Send stop condition */
> +#define OSIFI2C_STATUS		23 /* Get status from i2c action */
> +#define OSIFI2C_SET_BIT_RATE	24 /* Set the bit rate & prescaler */

Comments could be seen as obvious, too. I'll leave the decision to you.

> +struct priv {

Namespace: 'osif_priv' please.

> +	struct usb_device *usb_dev;
> +	struct usb_interface *interface;
> +	struct i2c_adapter adapter;
> +	unsigned char status;
> +};
> +
> +static int usb_read(struct i2c_adapter *adapter, int cmd,

Namespace: 'osif_usb_read'.

> +		    int value, int index, void *data, int len)
> +{
> +	struct priv *priv = (struct priv *)adapter->algo_data;

No need to cast a void*.

> +
> +	return usb_control_msg(priv->usb_dev, usb_rcvctrlpipe(priv->usb_dev, 0),
> +			       cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE |
> +			       USB_DIR_IN, value, index, data, len, 2000);
> +}
> +
> +static int usb_write(struct i2c_adapter *adapter, int cmd,

Namespace

> +		    int value, int index, void *data, int len)
> +{
> +
> +	struct priv *priv = (struct priv *)adapter->algo_data;

no cast.

> +
> +	return usb_control_msg(priv->usb_dev, usb_sndctrlpipe(priv->usb_dev, 0),
> +			       cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
> +			       value, index, data, len, 2000);
> +}
> +
> +static int usb_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)

namespace

> +{
> +	struct priv *priv = (struct priv *)adapter->algo_data;

no cast

> +	struct i2c_msg *pmsg;
> +	int ret = 0;
> +	int i, cmd;
> +
> +	for (i = 0; ret >= 0 && i < num; i++) {
> +		pmsg = &msgs[i];
> +
> +		if (pmsg->flags & I2C_M_RD) {
> +			cmd = OSIFI2C_READ;
> +
> +			ret = usb_read(adapter, cmd, pmsg->flags, pmsg->addr,
> +				       pmsg->buf, pmsg->len);
> +			if (ret != pmsg->len) {
> +				dev_err(&adapter->dev,
> +					"failure reading data\n");

One line is more readable IMO. 80 chars is not a hard limit.

> +				return -EREMOTEIO;
> +			}
> +		} else {
> +			cmd = OSIFI2C_WRITE;
> +
> +			ret = usb_write(adapter, cmd, pmsg->flags, pmsg->addr,
> +					pmsg->buf, pmsg->len);
> +			if (ret != pmsg->len) {
> +				dev_err(&adapter->dev,
> +					"failure writing data\n");

ditto

> +				return -EREMOTEIO;
> +			}
> +		}
> +
> +		ret = usb_read(adapter, OSIFI2C_STOP, 0, 0, NULL, 0);
> +		if (ret != 0) {

'if (ret)' is enough

> +			dev_err(&adapter->dev, "failure sending STOP\n");
> +			return -EREMOTEIO;
> +		}
> +
> +		/* read status */
> +		ret = usb_read(adapter, OSIFI2C_STATUS, 0, 0, &priv->status, 1);
> +		if (ret != 1) {
> +			dev_err(&adapter->dev, "failure reading status\n");
> +			return -EREMOTEIO;
> +		}
> +
> +		if (priv->status != STATUS_ADDRESS_ACK) {
> +			dev_dbg(&adapter->dev, "status = %d\n", priv->status);
> +			return -EREMOTEIO;
> +		}
> +	}
> +
> +	return i;
> +}
> +
> +static u32 usb_func(struct i2c_adapter *adapter)
> +{
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;

Have you tried SMBUS_QUICK by the way?

> +}
> +
> +static struct i2c_algorithm usb_algorithm = {
> +	.master_xfer	= usb_xfer,
> +	.functionality	= usb_func,
> +};
> +
> +#define USB_OSIF_VENDOR_ID	0x1964
> +#define USB_OSIF_PRODUCT_ID	0x0001
> +
> +static struct usb_device_id osif_table[] = {
> +	{ USB_DEVICE(USB_OSIF_VENDOR_ID, USB_OSIF_PRODUCT_ID) },
> +	{ }
> +};
> +

Empty line can go.

> +MODULE_DEVICE_TABLE(usb, osif_table);
> +
> +static int osif_probe(struct usb_interface *interface,
> +			     const struct usb_device_id *id)
> +{
> +	int retval;

maybe just 'ret' for consistency? Very minor nit, though...

> +	struct priv *priv;
> +	u16 version;

Rest looks ready to go!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] i2c: Add bus driver for for OSIF USB i2c device.

[Andrew Lunn]

Hi Wolfgang

> > +static u32 usb_func(struct i2c_adapter *adapter)
> > +{
> > +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> 
> Have you tried SMBUS_QUICK by the way?

Nope, not tried it.

Documentation/i2c/smbus-protocol say:

SMBus Quick Command
===================

This sends a single bit to the device, at the place of the Rd/Wr bit.

A Addr Rd/Wr [A] P

Functionality flag: I2C_FUNC_SMBUS_QUICK

My first question is probably silly. Should the first A be an S?

So is this a zero byte transfer? Is that not what i2cdetect does?
I've used i2cdetect on this hardware to find my devices.

Thanks
	Andrew

Re: [PATCH 2/2] i2c: Add bus driver for for OSIF USB i2c device.

[Wolfram Sang]

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

Hi,

> Hi Wolfgang

Wolfram

> So is this a zero byte transfer? Is that not what i2cdetect does?

Yes, you can enforce it with -q. It will do it for some addresses
automatically and complain if it is not available. So, seems to work,
great!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] i2c: Add bus driver for for OSIF USB i2c device.

[Andrew Lunn]

Hi Wolfram

> > Hi Wolfgang
> 
> Wolfram

Upps, Sorry.

> > So is this a zero byte transfer? Is that not what i2cdetect does?
> 
> Yes, you can enforce it with -q. It will do it for some addresses
> automatically and complain if it is not available. So, seems to work,
> great!

O.K, i will double check and then add this to functions.

     Andrew 

Re: [PATCH 2/2] i2c: Add bus driver for for OSIF USB i2c device.

[Wolfram Sang]

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

> O.K, i will double check and then add this to functions.

It's default on :)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]