[PATCH v2 0/3] mfd: add support for Diolan DLN-2

[PATCH v2 0/3] mfd: add support for Diolan DLN-2

[Octavian Purdila]

This patch series adds support for Diolan USB-I2C/GPIO Master Adapter DLN-2.
Details about device can be found here:

https://www.diolan.com/i2c/i2c_interface.html.

Changes since v1:

 * rewrite the drivers as an MFD

 * rewrite the irq part of the gpio driver to use GPIOLIB_IRQCHIP 

 * cleanup the I/O interface 

 * various fixes and cleanps: check received message sizes before
   parsing, error handling for usb_submit_urb, check URB status, use
   single bit manipulation functions instead of bitmap_*, move
   GFP_KERNEL URB submit away from under lock



Daniel Baluta (1):
  gpio: add support for the Diolan DLN-2 USB GPIO driver

Laurentiu Palcu (1):
  i2c: add support for Diolan DLN-2 USB-I2C adapter

Octavian Purdila (1):
  mfd: add support for Diolan DLN-2 devices

 drivers/gpio/Kconfig          |  12 +
 drivers/gpio/Makefile         |   1 +
 drivers/gpio/gpio-dln2.c      | 506 ++++++++++++++++++++++++++++++++
 drivers/i2c/busses/Kconfig    |  10 +
 drivers/i2c/busses/Makefile   |   1 +
 drivers/i2c/busses/i2c-dln2.c | 300 +++++++++++++++++++
 drivers/mfd/Kconfig           |   9 +
 drivers/mfd/Makefile          |   1 +
 drivers/mfd/dln2.c            | 652 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/dln2.h      |  64 +++++
 10 files changed, 1556 insertions(+)
 create mode 100644 drivers/gpio/gpio-dln2.c
 create mode 100644 drivers/i2c/busses/i2c-dln2.c
 create mode 100644 drivers/mfd/dln2.c
 create mode 100644 include/linux/mfd/dln2.h

-- 
1.9.1

[PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Octavian Purdila]

This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
Master Adapter DLN-2. Details about the device can be found here:

https://www.diolan.com/i2c/i2c_interface.html.

Information about the USB protocol can be found in the Programmer's
Reference Manual [1], see section 1.7.

Because the hardware has a single transmit endpoint and a single
receive endpoint the communication between the various DLN2 drivers
and the hardware will be muxed/demuxed by this driver.

Each DLN2 module will be identified by the handle field within the DLN2
message header. If a DLN2 module issues multiple commands in parallel
they will be identified by the echo counter field in the message header.

The DLN2 modules can use the dln2_transfer() function to issue a
command and wait for its response. They can also register a callback
that is going to be called when a specific event id is generated by
the device (e.g. GPIO interrupts). The device uses handle 0 for
sending events.

[1] https://www.diolan.com/downloads/dln-api-manual.pdf

Signed-off-by: Octavian Purdila <octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/mfd/Kconfig      |   9 +
 drivers/mfd/Makefile     |   1 +
 drivers/mfd/dln2.c       | 652 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/dln2.h |  64 +++++
 4 files changed, 726 insertions(+)
 create mode 100644 drivers/mfd/dln2.c
 create mode 100644 include/linux/mfd/dln2.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..7bcf895 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -183,6 +183,15 @@ config MFD_DA9063
 	  Additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_DLN2
+	tristate "Diolan DLN2 support"
+	select MFD_CORE
+	depends on USB
+	help
+	  This adds support for Diolan USB-I2C/SPI/GPIO Master Adapter DLN-2.
+	  Additional drivers must be enabled in order to use the functionality
+	  of the device.
+
 config MFD_MC13XXX
 	tristate
 	depends on (SPI_MASTER || I2C)
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..591988d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711)	+= as3711.o
 obj-$(CONFIG_MFD_AS3722)	+= as3722.o
 obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
 obj-$(CONFIG_MFD_IPAQ_MICRO)	+= ipaq-micro.o
+obj-$(CONFIG_MFD_DLN2)		+= dln2.o
 
 intel-soc-pmic-objs		:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
new file mode 100644
index 0000000..ccd44fe
--- /dev/null
+++ b/drivers/mfd/dln2.c
@@ -0,0 +1,652 @@
+/*
+ * Driver for the Diolan DLN-2 USB 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/usb.h>
+#include <linux/i2c.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/dln2.h>
+
+#define DRIVER_NAME			"usb-dln2"
+
+struct dln2_header {
+	__le16 size;
+	__le16 id;
+	__le16 echo;
+	__le16 handle;
+} __packed;
+
+struct dln2_response {
+	struct dln2_header hdr;
+	__le16 result;
+} __packed;
+
+#define DLN2_GENERIC_MODULE_ID		0x00
+#define DLN2_GENERIC_CMD(cmd)		DLN2_CMD(cmd, DLN2_GENERIC_MODULE_ID)
+#define CMD_GET_DEVICE_VER		DLN2_GENERIC_CMD(0x30)
+#define CMD_GET_DEVICE_SN		DLN2_GENERIC_CMD(0x31)
+
+#define DLN2_HW_ID			0x200
+#define DLN2_USB_TIMEOUT		200	/* in ms */
+#define DLN2_MAX_RX_SLOTS		16
+#define DLN2_MAX_MODULES		5
+#define DLN2_MAX_URBS			16
+
+#define DLN2_HANDLE_GPIO_EVENT		0
+#define DLN2_HANDLE_CTRL		1
+#define DLN2_HANDLE_GPIO		2
+#define DLN2_HANDLE_I2C			3
+
+/* Receive context used between the receive demultiplexer and the
+ * transfer routine. While sending a request the transfer routine
+ * will look for a free receive context and use it to wait for a
+ * response and to receive the URB and thus the response data. */
+struct dln2_rx_context {
+	struct completion done;
+	struct urb *urb;
+	bool connected;
+};
+
+/* Receive contexts for a particular DLN2 module (i2c, gpio, etc.). We
+ * use the handle header field to indentify the module in
+ * dln2_dev.mod_rx_slots and then the echo header field to index the
+ * slots field and find the receive context for a particular
+ * request. */
+struct dln2_mod_rx_slots {
+	/* RX slots bitmap */
+	unsigned long bmap;
+
+	/* used to wait for a free RX slot */
+	wait_queue_head_t wq;
+
+	/* used to wait for an RX operation to complete */
+	struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS];
+
+	/* avoid races between free_rx_slot and dln2_rx_transfer_cb */
+	spinlock_t lock;
+};
+
+struct dln2_dev {
+	struct usb_device *usb_dev;
+	struct usb_interface *interface;
+	u8 ep_in;
+	u8 ep_out;
+
+	struct urb *rx_urb[DLN2_MAX_URBS];
+	void *rx_buf[DLN2_MAX_URBS];
+
+	struct dln2_mod_rx_slots mod_rx_slots[DLN2_MAX_MODULES];
+
+	struct list_head rx_cb_list;
+	spinlock_t rx_cb_lock;
+};
+
+static bool find_free_slot(struct dln2_mod_rx_slots *rxs, int *slot)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&rxs->lock, flags);
+
+	*slot = find_first_zero_bit(&rxs->bmap, DLN2_MAX_RX_SLOTS);
+
+	if (*slot < DLN2_MAX_RX_SLOTS) {
+		struct dln2_rx_context *rxc = &rxs->slots[*slot];
+
+		init_completion(&rxc->done);
+		set_bit(*slot, &rxs->bmap);
+		rxc->connected = true;
+	}
+
+	spin_unlock_irqrestore(&rxs->lock, flags);
+
+	return *slot < DLN2_MAX_RX_SLOTS;
+}
+
+static int alloc_rx_slot(struct dln2_mod_rx_slots *rxs)
+{
+	int slot, ret;
+
+	/* No need to timeout here, the wait is bounded by the timeout
+	 * in _dln2_transfer */
+	ret = wait_event_interruptible(rxs->wq, find_free_slot(rxs, &slot));
+	if (ret < 0)
+		return ret;
+
+	return slot;
+}
+
+static void free_rx_slot(struct dln2_dev *dln2, struct dln2_mod_rx_slots *rxs,
+			 int slot)
+{
+	struct urb *urb = NULL;
+	unsigned long flags;
+	struct dln2_rx_context *rxc;
+	struct device *dev = &dln2->interface->dev;
+	int ret;
+
+	spin_lock_irqsave(&rxs->lock, flags);
+
+	clear_bit(slot, &rxs->bmap);
+
+	rxc = &rxs->slots[slot];
+	rxc->connected = false;
+	urb = rxc->urb;
+	rxc->urb = NULL;
+
+	spin_unlock_irqrestore(&rxs->lock, flags);
+
+	if (urb)  {
+		ret = usb_submit_urb(urb, GFP_KERNEL);
+		if (ret < 0)
+			dev_err(dev, "failed to re-submit RX URB: %d\n", ret);
+	}
+
+	wake_up_interruptible(&rxs->wq);
+}
+
+struct dln2_rx_cb_entry {
+	struct list_head list;
+	u16 id;
+	struct platform_device *pdev;
+	dln2_rx_cb_t callback;
+};
+
+int dln2_register_event_cb(struct platform_device *pdev, u16 id,
+			   dln2_rx_cb_t rx_cb)
+{
+	struct dln2_dev *dln2 = dev_get_drvdata(pdev->dev.parent);
+	struct dln2_rx_cb_entry *i, *new;
+	unsigned long flags;
+	int ret = 0;
+
+	new = kmalloc(sizeof(*new), GFP_KERNEL);
+	if (!new)
+		return -ENOMEM;
+
+	new->id = id;
+	new->callback = rx_cb;
+	new->pdev = pdev;
+
+	spin_lock_irqsave(&dln2->rx_cb_lock, flags);
+
+	list_for_each_entry(i, &dln2->rx_cb_list, list) {
+		if (i->id == id) {
+			ret = -EBUSY;
+			break;
+		}
+	}
+
+	if (!ret)
+		list_add(&new->list, &dln2->rx_cb_list);
+
+	spin_unlock_irqrestore(&dln2->rx_cb_lock, flags);
+
+	if (ret)
+		kfree(new);
+
+	return ret;
+}
+EXPORT_SYMBOL(dln2_register_event_cb);
+
+void dln2_unregister_event_cb(struct platform_device *pdev, u16 id)
+{
+	struct dln2_dev *dln2 = dev_get_drvdata(pdev->dev.parent);
+	struct dln2_rx_cb_entry *i, *tmp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dln2->rx_cb_lock, flags);
+
+	list_for_each_entry_safe(i, tmp, &dln2->rx_cb_list, list) {
+		if (i->id == id) {
+			list_del(&i->list);
+			kfree(i);
+		}
+	}
+
+	spin_unlock_irqrestore(&dln2->rx_cb_lock, flags);
+}
+EXPORT_SYMBOL(dln2_unregister_event_cb);
+
+static void dln2_rx_transfer(struct dln2_dev *dln2, struct urb *urb,
+			     u16 handle, u16 rx_slot)
+{
+	struct dln2_mod_rx_slots *rxs = &dln2->mod_rx_slots[handle];
+	struct dln2_rx_context *rxc;
+	struct device *dev = &dln2->interface->dev;
+	int err;
+
+	spin_lock(&rxs->lock);
+	rxc = &rxs->slots[rx_slot];
+	if (rxc->connected) {
+		rxc->urb = urb;
+		complete(&rxc->done);
+	} else {
+		dev_warn(dev, "droping response %d/%d", handle, rx_slot);
+		err = usb_submit_urb(urb, GFP_ATOMIC);
+		if (err < 0)
+			dev_err(dev, "failed to re-submit RX URB: %d\n", err);
+	}
+	spin_unlock(&rxs->lock);
+}
+
+static void dln2_run_rx_callbacks(struct dln2_dev *dln2, u16 id, u16 echo,
+				  void *data, int len)
+{
+	struct dln2_rx_cb_entry *i;
+
+	spin_lock(&dln2->rx_cb_lock);
+
+	list_for_each_entry(i, &dln2->rx_cb_list, list)
+		if (i->id == id)
+			i->callback(i->pdev, echo, data, len);
+
+	spin_unlock(&dln2->rx_cb_lock);
+}
+
+static void dln2_rx(struct urb *urb)
+{
+	struct dln2_dev *dln2 = urb->context;
+	struct dln2_header *hdr = urb->transfer_buffer;
+	struct device *dev = &dln2->interface->dev;
+	u16 id, echo, handle, size;
+	u8 *data;
+	int len, err;
+
+	switch (urb->status) {
+	case 0:
+		/* success */
+		break;
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+	case -EPIPE:
+		/* this urb is terminated, clean up */
+		dev_dbg(dev, "urb shutting down with status %d\n", urb->status);
+		return;
+	default:
+		dev_dbg(dev, "nonzero urb status received %d\n", urb->status);
+		goto out;
+	}
+
+	if (urb->actual_length < sizeof(struct dln2_header)) {
+		dev_err(dev, "short response: %d\n", urb->actual_length);
+		goto out;
+	}
+
+	handle = le16_to_cpu(hdr->handle);
+	id = le16_to_cpu(hdr->id);
+	echo = le16_to_cpu(hdr->echo);
+	size = le16_to_cpu(hdr->size);
+
+	if (size != urb->actual_length) {
+		dev_err(dev, "size mismatch: handle %x cmd %x echo %x size %d actual %d\n",
+			handle, id, echo, size, urb->actual_length);
+		goto out;
+	}
+
+	if (handle > DLN2_MAX_MODULES) {
+		dev_warn(dev, "RX: invalid handle %d\n", handle);
+		goto out;
+	}
+
+	data = urb->transfer_buffer + sizeof(struct dln2_header);
+	len = urb->actual_length - sizeof(struct dln2_header);
+
+	if (!handle) {
+		dln2_run_rx_callbacks(dln2, id, echo, data, len);
+		err = usb_submit_urb(urb, GFP_ATOMIC);
+		if (err < 0)
+			goto out_submit_failed;
+	} else {
+		dln2_rx_transfer(dln2, urb, handle, echo);
+	}
+
+	return;
+
+out:
+	err = usb_submit_urb(urb, GFP_ATOMIC);
+out_submit_failed:
+	if (err < 0)
+		dev_err(dev, "failed to re-submit RX URB: %d\n", err);
+}
+
+static void *dln2_prep_buf(u16 handle, u16 cmd, u16 echo, void *obuf,
+			   int *obuf_len, gfp_t gfp)
+{
+	void *buf;
+	int len;
+	struct dln2_header *hdr;
+
+	len = *obuf_len + sizeof(*hdr);
+	buf = kmalloc(len, gfp);
+	if (!buf)
+		return NULL;
+
+	hdr = (struct dln2_header *)buf;
+	hdr->id = cpu_to_le16(cmd);
+	hdr->size = cpu_to_le16(len);
+	hdr->echo = cpu_to_le16(echo);
+	hdr->handle = cpu_to_le16(handle);
+
+	memcpy(buf + sizeof(*hdr), obuf, *obuf_len);
+
+	*obuf_len = len;
+
+	return buf;
+}
+
+static int dln2_send_wait(struct dln2_dev *dln2, u16 handle, u16 cmd, u16 echo,
+			  void *obuf, int obuf_len)
+{
+	int len = obuf_len, ret = 0, actual;
+	void *buf;
+
+	buf = dln2_prep_buf(handle, cmd, echo, obuf, &len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = usb_bulk_msg(dln2->usb_dev,
+			   usb_sndbulkpipe(dln2->usb_dev, dln2->ep_out),
+			   buf, len, &actual, DLN2_USB_TIMEOUT);
+
+	kfree(buf);
+
+	return ret;
+}
+
+static int _dln2_transfer(struct dln2_dev *dln2, u16 handle, u16 cmd,
+			  void *obuf, int obuf_len, void *ibuf, int *ibuf_len)
+{
+	u16 result, rx_slot;
+	struct dln2_response *rsp;
+	int ret = 0;
+	const int timeout = DLN2_USB_TIMEOUT * HZ / 1000;
+	struct dln2_mod_rx_slots *rxs = &dln2->mod_rx_slots[handle];
+	struct dln2_rx_context *rxc;
+	struct device *dev = &dln2->interface->dev;
+
+	rx_slot = alloc_rx_slot(rxs);
+	if (rx_slot < 0)
+		return rx_slot;
+
+	ret = dln2_send_wait(dln2, handle, cmd, rx_slot, obuf, obuf_len);
+	if (ret < 0) {
+		free_rx_slot(dln2, rxs, rx_slot);
+		dev_err(dev, "USB write failed: %d", ret);
+		return ret;
+	}
+
+	rxc = &rxs->slots[rx_slot];
+
+	ret = wait_for_completion_interruptible_timeout(&rxc->done, timeout);
+	if (ret <= 0) {
+		ret = !ret ? -ETIMEDOUT : ret;
+		goto out_free_rx_slot;
+	}
+
+	/* if we got here we know that the response header has been checked */
+	rsp = rxc->urb->transfer_buffer;
+	result = le16_to_cpu(rsp->result);
+
+	if (result) {
+		dev_dbg(dev, "%d received response with error %d\n",
+			handle, result);
+		ret = -EREMOTEIO;
+		goto out_free_rx_slot;
+	}
+
+	if (!ibuf) {
+		ret = 0;
+		goto out_free_rx_slot;
+	}
+
+	if (*ibuf_len > rxc->urb->actual_length - sizeof(*rsp))
+		*ibuf_len = rxc->urb->actual_length - sizeof(*rsp);
+
+	memcpy(ibuf, rsp + 1, *ibuf_len);
+
+out_free_rx_slot:
+	free_rx_slot(dln2, rxs, rx_slot);
+
+	return ret;
+}
+
+struct dln2_platform_data {
+	u16 handle;
+};
+
+int dln2_transfer(struct platform_device *pdev, u16 cmd,
+		  void *obuf, int obuf_len, void *ibuf, int *ibuf_len)
+{
+	struct dln2_platform_data *dln2_pdata;
+	struct dln2_dev *dln2;
+	u16 h;
+
+	if (!pdev)
+		return -ENODEV;
+
+	dln2 = dev_get_drvdata(pdev->dev.parent);
+	dln2_pdata = dev_get_platdata(&pdev->dev);
+	h = dln2_pdata->handle;
+
+	return _dln2_transfer(dln2, h, cmd, obuf, obuf_len, ibuf, ibuf_len);
+}
+EXPORT_SYMBOL(dln2_transfer);
+
+static int dln2_check_hw(struct dln2_dev *dln2)
+{
+	__le32 hw_type;
+	int ret, len = sizeof(hw_type);
+
+	ret = _dln2_transfer(dln2, DLN2_HANDLE_CTRL, CMD_GET_DEVICE_VER,
+			     NULL, 0, &hw_type, &len);
+	if (ret < 0)
+		return ret;
+
+	if (le32_to_cpu(hw_type) != DLN2_HW_ID) {
+		dev_err(&dln2->interface->dev, "Device ID 0x%x not supported!",
+			le32_to_cpu(hw_type));
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int dln2_print_serialno(struct dln2_dev *dln2)
+{
+	__le32 serial_no;
+	int ret, len = sizeof(serial_no);
+	struct device *dev = &dln2->interface->dev;
+
+	ret = _dln2_transfer(dln2, DLN2_HANDLE_CTRL, CMD_GET_DEVICE_SN, NULL, 0,
+			     &serial_no, &len);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev, "Diolan DLN2 serial 0x%x\n", le32_to_cpu(serial_no));
+
+	return 0;
+}
+
+static int dln2_hw_init(struct dln2_dev *dln2)
+{
+	int ret = dln2_check_hw(dln2);
+
+	if (ret < 0)
+		return ret;
+
+	return dln2_print_serialno(dln2);
+}
+
+static void dln2_free_rx_urbs(struct dln2_dev *dln2)
+{
+	int i;
+
+	for (i = 0; i < DLN2_MAX_URBS; i++) {
+		usb_kill_urb(dln2->rx_urb[i]);
+		usb_free_urb(dln2->rx_urb[i]);
+		kfree(dln2->rx_buf[i]);
+	}
+}
+
+static void dln2_free(struct dln2_dev *dln2)
+{
+	dln2_free_rx_urbs(dln2);
+	usb_put_dev(dln2->usb_dev);
+	kfree(dln2);
+}
+
+static int dln2_setup_rx_urbs(struct dln2_dev *dln2,
+			      struct usb_host_interface *hostif)
+{
+	int i, ret;
+	int rx_max_size = le16_to_cpu(hostif->endpoint[1].desc.wMaxPacketSize);
+	struct device *dev = &dln2->interface->dev;
+
+	for (i = 0; i < DLN2_MAX_URBS; i++) {
+		dln2->rx_buf[i] = kmalloc(rx_max_size, GFP_KERNEL);
+		if (!dln2->rx_buf[i])
+			return -ENOMEM;
+
+		dln2->rx_urb[i] = usb_alloc_urb(0, GFP_KERNEL);
+		if (!dln2->rx_urb[i])
+			return -ENOMEM;
+
+		usb_fill_bulk_urb(dln2->rx_urb[i], dln2->usb_dev,
+				  usb_rcvbulkpipe(dln2->usb_dev, dln2->ep_in),
+				  dln2->rx_buf[i], rx_max_size, dln2_rx, dln2);
+
+		ret = usb_submit_urb(dln2->rx_urb[i], GFP_KERNEL);
+		if (ret < 0) {
+			dev_err(dev, "failed to submit RX URB: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static struct dln2_platform_data dln2_pdata_gpio = {
+	.handle = DLN2_HANDLE_GPIO,
+};
+
+static struct dln2_platform_data dln2_pdata_i2c = {
+	.handle = DLN2_HANDLE_I2C,
+};
+
+static const struct mfd_cell dln2_devs[] = {
+	{
+		.name	= "dln2-gpio",
+		.platform_data = &dln2_pdata_gpio,
+		.pdata_size = sizeof(struct dln2_platform_data),
+	},
+	{
+		.name	= "dln2-i2c",
+		.platform_data = &dln2_pdata_i2c,
+		.pdata_size = sizeof(struct dln2_platform_data),
+	},
+};
+
+static void dln2_disconnect(struct usb_interface *interface)
+{
+	struct dln2_dev *dln2 = usb_get_intfdata(interface);
+
+	mfd_remove_devices(&interface->dev);
+	dln2_free(dln2);
+}
+
+static int dln2_probe(struct usb_interface *interface,
+		      const struct usb_device_id *id)
+{
+	struct usb_host_interface *hostif = interface->cur_altsetting;
+	struct device *dev = &interface->dev;
+	struct dln2_dev *dln2;
+	int ret, i;
+
+	if (hostif->desc.bInterfaceNumber != 0 ||
+	    hostif->desc.bNumEndpoints < 2)
+		return -ENODEV;
+
+	dln2 = kzalloc(sizeof(*dln2), GFP_KERNEL);
+	if (!dln2) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	dln2->ep_out = hostif->endpoint[0].desc.bEndpointAddress;
+	dln2->ep_in = hostif->endpoint[1].desc.bEndpointAddress;
+	dln2->usb_dev = usb_get_dev(interface_to_usbdev(interface));
+	dln2->interface = interface;
+	usb_set_intfdata(interface, dln2);
+
+	for (i = 0; i < DLN2_MAX_MODULES; i++) {
+		init_waitqueue_head(&dln2->mod_rx_slots[i].wq);
+		spin_lock_init(&dln2->mod_rx_slots[i].lock);
+	}
+
+	spin_lock_init(&dln2->rx_cb_lock);
+	INIT_LIST_HEAD(&dln2->rx_cb_list);
+
+	ret = dln2_setup_rx_urbs(dln2, hostif);
+	if (ret) {
+		dln2_disconnect(interface);
+		return ret;
+	}
+
+	ret = dln2_hw_init(dln2);
+	if (ret < 0) {
+		dev_err(dev, "failed to initialize hardware\n");
+		goto out_cleanup;
+	}
+
+	ret = mfd_add_devices(dev, -1, dln2_devs, ARRAY_SIZE(dln2_devs),
+			      NULL, 0, NULL);
+	if (ret != 0) {
+		dev_err(dev, "Failed to add mfd devices to core.\n");
+		goto out_cleanup;
+	}
+
+	return 0;
+
+out_cleanup:
+	dln2_free(dln2);
+out:
+	return ret;
+}
+
+static const struct usb_device_id dln2_table[] = {
+	{ USB_DEVICE(0xa257, 0x2013) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(usb, dln2_table);
+
+static struct usb_driver dln2_driver = {
+	.name = DRIVER_NAME,
+	.probe = dln2_probe,
+	.disconnect = dln2_disconnect,
+	.id_table = dln2_table,
+};
+
+module_usb_driver(dln2_driver);
+
+MODULE_AUTHOR("Octavian Purdila <octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>");
+MODULE_DESCRIPTION(DRIVER_NAME " driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/dln2.h b/include/linux/mfd/dln2.h
new file mode 100644
index 0000000..7ecbbe7
--- /dev/null
+++ b/include/linux/mfd/dln2.h
@@ -0,0 +1,64 @@
+#ifndef __LINUX_USB_DLN2_H
+#define __LINUX_USB_DLN2_H
+
+#define DLN2_CMD(cmd, id)		((cmd) | ((id) << 8))
+
+/**
+ * dln2_rx_cb - event callback function signature
+ *
+ * @pdev - the sub-driver that registered this callback
+ * @echo - the echo header field received in the message
+ * @data - the data payload
+ * @len  - the data playload length
+ *
+ * The callback function is called in interrupt context and the data
+ * payload is only valid during the call. If the user needs later
+ * access of the data, it must copy it.
+ */
+
+typedef void (*dln2_rx_cb_t)(struct platform_device *pdev, u16 echo,
+			     const void *data, int len);
+
+/**
+ * dl2n_register_event_cb - register a callback function for an event
+ *
+ * @pdev - the sub-driver that registers the callback
+ * @event - the event for which to register a callback
+ * @rx_cb - the callback function
+ *
+ * @return 0 in case of success, negative value in case of error
+ */
+int dln2_register_event_cb(struct platform_device *pdev, u16 event,
+			   dln2_rx_cb_t rx_cb);
+
+/**
+ * dln2_unregister_event_cb - unregister the callback function for an event
+ *
+ * @pdev - the sub-driver that registered the callback
+ * @event - the event for which to register a callback
+ */
+void dln2_unregister_event_cb(struct platform_device *pdev, u16 event);
+
+/**
+ * dln2_transfer - issue a DLN2 command and wait for a response and
+ * the associated data
+ *
+ * Only modules that did *NOT* register a receive callback will be
+ * able to use this function.
+ *
+ * @pdev - the DLN2 sub-device which is issueing this transfer
+ * @cmd - the command to be sent to the device
+ * @obuf - the buffer to be sent to the device; can be NULL if the
+ * user doesn't need to transmit data with this command
+ * @obuf_len - the size of the buffer to be sent to the device
+ * @ibuf - any data associated with the response will be copied here;
+ * it can be NULL if the user doesn't need the response data
+ * @ibuf_len - must be initialized to the input buffer size; it will
+ * be modified to indicate the actual data transfered
+ *
+ * @returns 0 for success, negative value for errors
+ */
+int dln2_transfer(struct platform_device *pdev, u16 cmd,
+		  void *obuf, int obuf_len, void *ibuf, int *ibuf_len);
+
+#endif
-- 
1.9.1

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Lee Jones]

On Sat, 30 Aug 2014, Octavian Purdila wrote:

> This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
> Master Adapter DLN-2. Details about the device can be found here:
> 
> https://www.diolan.com/i2c/i2c_interface.html.
> 
> Information about the USB protocol can be found in the Programmer's
> Reference Manual [1], see section 1.7.
> 
> Because the hardware has a single transmit endpoint and a single
> receive endpoint the communication between the various DLN2 drivers
> and the hardware will be muxed/demuxed by this driver.
> 
> Each DLN2 module will be identified by the handle field within the DLN2
> message header. If a DLN2 module issues multiple commands in parallel
> they will be identified by the echo counter field in the message header.
> 
> The DLN2 modules can use the dln2_transfer() function to issue a
> command and wait for its response. They can also register a callback
> that is going to be called when a specific event id is generated by
> the device (e.g. GPIO interrupts). The device uses handle 0 for
> sending events.
> 
> [1] https://www.diolan.com/downloads/dln-api-manual.pdf

MFD is not a dumping ground for misfit h/w.  Almost all of this code
looks like it belongs in drivers/usb.  Please move it there.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Octavian Purdila]

On Mon, Sep 1, 2014 at 11:37 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Sat, 30 Aug 2014, Octavian Purdila wrote:
>
>> This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
>> Master Adapter DLN-2. Details about the device can be found here:
>>
>> https://www.diolan.com/i2c/i2c_interface.html.
>>
>> Information about the USB protocol can be found in the Programmer's
>> Reference Manual [1], see section 1.7.
>>
>> Because the hardware has a single transmit endpoint and a single
>> receive endpoint the communication between the various DLN2 drivers
>> and the hardware will be muxed/demuxed by this driver.
>>
>> Each DLN2 module will be identified by the handle field within the DLN2
>> message header. If a DLN2 module issues multiple commands in parallel
>> they will be identified by the echo counter field in the message header.
>>
>> The DLN2 modules can use the dln2_transfer() function to issue a
>> command and wait for its response. They can also register a callback
>> that is going to be called when a specific event id is generated by
>> the device (e.g. GPIO interrupts). The device uses handle 0 for
>> sending events.
>>
>> [1] https://www.diolan.com/downloads/dln-api-manual.pdf
>
> MFD is not a dumping ground for misfit h/w.  Almost all of this code
> looks like it belongs in drivers/usb.  Please move it there.
>

Hi Lee,

We initially submitted this driver as a pure USB driver, with our own
module registration mechanism, but during the first round of reviews
people pointed out that a MFD driver is the better approach, and I
agree. I also see that there are already a couple of USB drivers
implemented as MFD drivers.

Do you see a better approach?

Thanks,
Tavi

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Lee Jones]

On Mon, 01 Sep 2014, Octavian Purdila wrote:

> On Mon, Sep 1, 2014 at 11:37 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Sat, 30 Aug 2014, Octavian Purdila wrote:
> >
> >> This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
> >> Master Adapter DLN-2. Details about the device can be found here:
> >>
> >> https://www.diolan.com/i2c/i2c_interface.html.
> >>
> >> Information about the USB protocol can be found in the Programmer's
> >> Reference Manual [1], see section 1.7.
> >>
> >> Because the hardware has a single transmit endpoint and a single
> >> receive endpoint the communication between the various DLN2 drivers
> >> and the hardware will be muxed/demuxed by this driver.
> >>
> >> Each DLN2 module will be identified by the handle field within the DLN2
> >> message header. If a DLN2 module issues multiple commands in parallel
> >> they will be identified by the echo counter field in the message header.
> >>
> >> The DLN2 modules can use the dln2_transfer() function to issue a
> >> command and wait for its response. They can also register a callback
> >> that is going to be called when a specific event id is generated by
> >> the device (e.g. GPIO interrupts). The device uses handle 0 for
> >> sending events.
> >>
> >> [1] https://www.diolan.com/downloads/dln-api-manual.pdf
> >
> > MFD is not a dumping ground for misfit h/w.  Almost all of this code
> > looks like it belongs in drivers/usb.  Please move it there.
> >
> 
> We initially submitted this driver as a pure USB driver, with our own
> module registration mechanism, but during the first round of reviews
> people pointed out that a MFD driver is the better approach, and I
> agree. I also see that there are already a couple of USB drivers
> implemented as MFD drivers.

Can you link me to your previous submission please?

> Do you see a better approach?

You should have a small MFD driver which controls resources and
registers children.  All other functionality should live in their
respective drivers/X locations i.e. USB functionallity should normally
live in drivers/usb.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Octavian Purdila]

On Mon, Sep 1, 2014 at 12:51 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 01 Sep 2014, Octavian Purdila wrote:
>
>> On Mon, Sep 1, 2014 at 11:37 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Sat, 30 Aug 2014, Octavian Purdila wrote:
>> >
>> >> This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
>> >> Master Adapter DLN-2. Details about the device can be found here:
>> >>
>> >> https://www.diolan.com/i2c/i2c_interface.html.
>> >>
>> >> Information about the USB protocol can be found in the Programmer's
>> >> Reference Manual [1], see section 1.7.
>> >>
>> >> Because the hardware has a single transmit endpoint and a single
>> >> receive endpoint the communication between the various DLN2 drivers
>> >> and the hardware will be muxed/demuxed by this driver.
>> >>
>> >> Each DLN2 module will be identified by the handle field within the DLN2
>> >> message header. If a DLN2 module issues multiple commands in parallel
>> >> they will be identified by the echo counter field in the message header.
>> >>
>> >> The DLN2 modules can use the dln2_transfer() function to issue a
>> >> command and wait for its response. They can also register a callback
>> >> that is going to be called when a specific event id is generated by
>> >> the device (e.g. GPIO interrupts). The device uses handle 0 for
>> >> sending events.
>> >>
>> >> [1] https://www.diolan.com/downloads/dln-api-manual.pdf
>> >
>> > MFD is not a dumping ground for misfit h/w.  Almost all of this code
>> > looks like it belongs in drivers/usb.  Please move it there.
>> >
>>
>> We initially submitted this driver as a pure USB driver, with our own
>> module registration mechanism, but during the first round of reviews
>> people pointed out that a MFD driver is the better approach, and I
>> agree. I also see that there are already a couple of USB drivers
>> implemented as MFD drivers.
>
> Can you link me to your previous submission please?

Sure, here it is:

https://lkml.org/lkml/2014/8/20/228

>
>> Do you see a better approach?
>
> You should have a small MFD driver which controls resources and
> registers children.  All other functionality should live in their
> respective drivers/X locations i.e. USB functionallity should normally
> live in drivers/usb.
>

OK, that sounds better. I am not sure how to handle the registration
part though, since in this case we need to create the children at
runtime, from the usb probe routine.

The only solution I see is to move the driver completely to
usb/drivers and continue to use the MFD infrastructure. Does that
sound OK to you?

Thanks,
Tavi

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Lee Jones]

On Mon, 01 Sep 2014, Octavian Purdila wrote:

> On Mon, Sep 1, 2014 at 12:51 PM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Mon, 01 Sep 2014, Octavian Purdila wrote:
> >
> >> On Mon, Sep 1, 2014 at 11:37 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >> > On Sat, 30 Aug 2014, Octavian Purdila wrote:
> >> >
> >> >> This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
> >> >> Master Adapter DLN-2. Details about the device can be found here:
> >> >>
> >> >> https://www.diolan.com/i2c/i2c_interface.html.
> >> >>
> >> >> Information about the USB protocol can be found in the Programmer's
> >> >> Reference Manual [1], see section 1.7.
> >> >>
> >> >> Because the hardware has a single transmit endpoint and a single
> >> >> receive endpoint the communication between the various DLN2 drivers
> >> >> and the hardware will be muxed/demuxed by this driver.
> >> >>
> >> >> Each DLN2 module will be identified by the handle field within the DLN2
> >> >> message header. If a DLN2 module issues multiple commands in parallel
> >> >> they will be identified by the echo counter field in the message header.
> >> >>
> >> >> The DLN2 modules can use the dln2_transfer() function to issue a
> >> >> command and wait for its response. They can also register a callback
> >> >> that is going to be called when a specific event id is generated by
> >> >> the device (e.g. GPIO interrupts). The device uses handle 0 for
> >> >> sending events.
> >> >>
> >> >> [1] https://www.diolan.com/downloads/dln-api-manual.pdf
> >> >
> >> > MFD is not a dumping ground for misfit h/w.  Almost all of this code
> >> > looks like it belongs in drivers/usb.  Please move it there.
> >> >
> >>
> >> We initially submitted this driver as a pure USB driver, with our own
> >> module registration mechanism, but during the first round of reviews
> >> people pointed out that a MFD driver is the better approach, and I
> >> agree. I also see that there are already a couple of USB drivers
> >> implemented as MFD drivers.
> >
> > Can you link me to your previous submission please?
> 
> Sure, here it is:
> 
> https://lkml.org/lkml/2014/8/20/228
> 
> >
> >> Do you see a better approach?
> >
> > You should have a small MFD driver which controls resources and
> > registers children.  All other functionality should live in their
> > respective drivers/X locations i.e. USB functionallity should normally
> > live in drivers/usb.
> >
> 
> OK, that sounds better. I am not sure how to handle the registration
> part though, since in this case we need to create the children at
> runtime, from the usb probe routine.
> 
> The only solution I see is to move the driver completely to
> usb/drivers and continue to use the MFD infrastructure. Does that
> sound OK to you?

I have no problem with that.  If this is an MFD driver, it _should_
live in drivers/mfd.  However, all of that USB specific stuff
defiantly should not.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Octavian Purdila]

On Mon, Sep 1, 2014 at 2:39 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 01 Sep 2014, Octavian Purdila wrote:
>
>> On Mon, Sep 1, 2014 at 12:51 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Mon, 01 Sep 2014, Octavian Purdila wrote:
>> >
>> >> On Mon, Sep 1, 2014 at 11:37 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> > On Sat, 30 Aug 2014, Octavian Purdila wrote:
>> >> >
>> >> >> This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
>> >> >> Master Adapter DLN-2. Details about the device can be found here:
>> >> >>
>> >> >> https://www.diolan.com/i2c/i2c_interface.html.
>> >> >>
>> >> >> Information about the USB protocol can be found in the Programmer's
>> >> >> Reference Manual [1], see section 1.7.
>> >> >>
>> >> >> Because the hardware has a single transmit endpoint and a single
>> >> >> receive endpoint the communication between the various DLN2 drivers
>> >> >> and the hardware will be muxed/demuxed by this driver.
>> >> >>
>> >> >> Each DLN2 module will be identified by the handle field within the DLN2
>> >> >> message header. If a DLN2 module issues multiple commands in parallel
>> >> >> they will be identified by the echo counter field in the message header.
>> >> >>
>> >> >> The DLN2 modules can use the dln2_transfer() function to issue a
>> >> >> command and wait for its response. They can also register a callback
>> >> >> that is going to be called when a specific event id is generated by
>> >> >> the device (e.g. GPIO interrupts). The device uses handle 0 for
>> >> >> sending events.
>> >> >>
>> >> >> [1] https://www.diolan.com/downloads/dln-api-manual.pdf
>> >> >
>> >> > MFD is not a dumping ground for misfit h/w.  Almost all of this code
>> >> > looks like it belongs in drivers/usb.  Please move it there.
>> >> >
>> >>
>> >> We initially submitted this driver as a pure USB driver, with our own
>> >> module registration mechanism, but during the first round of reviews
>> >> people pointed out that a MFD driver is the better approach, and I
>> >> agree. I also see that there are already a couple of USB drivers
>> >> implemented as MFD drivers.
>> >
>> > Can you link me to your previous submission please?
>>
>> Sure, here it is:
>>
>> https://lkml.org/lkml/2014/8/20/228
>>
>> >
>> >> Do you see a better approach?
>> >
>> > You should have a small MFD driver which controls resources and
>> > registers children.  All other functionality should live in their
>> > respective drivers/X locations i.e. USB functionallity should normally
>> > live in drivers/usb.
>> >
>>
>> OK, that sounds better. I am not sure how to handle the registration
>> part though, since in this case we need to create the children at
>> runtime, from the usb probe routine.
>>
>> The only solution I see is to move the driver completely to
>> usb/drivers and continue to use the MFD infrastructure. Does that
>> sound OK to you?
>
> I have no problem with that.  If this is an MFD driver, it _should_
> live in drivers/mfd.  However, all of that USB specific stuff
> defiantly should not.
>

It is a multi-function driver which is using the USB interface, so I
am not sure where it belongs. The only driver that calls
mfd_add_devices and is not in drivers/mfd is the hid sensor hub
driver.

BTW, the mfd/viperboard.c driver is very similar with this driver. It
has less USB specific stuff because the protocol is simpler, but still
has some.

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Lee Jones]

On Mon, 01 Sep 2014, Octavian Purdila wrote:

> On Mon, Sep 1, 2014 at 2:39 PM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Mon, 01 Sep 2014, Octavian Purdila wrote:
> >
> >> On Mon, Sep 1, 2014 at 12:51 PM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >> > On Mon, 01 Sep 2014, Octavian Purdila wrote:
> >> >
> >> >> On Mon, Sep 1, 2014 at 11:37 AM, Lee Jones <lee.jones-QSEj5FYQhm5QFI55V6+gNQ@public.gmane.orgg> wrote:
> >> >> > On Sat, 30 Aug 2014, Octavian Purdila wrote:
> >> >> >
> >> >> >> This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
> >> >> >> Master Adapter DLN-2. Details about the device can be found here:
> >> >> >>
> >> >> >> https://www.diolan.com/i2c/i2c_interface.html.
> >> >> >>
> >> >> >> Information about the USB protocol can be found in the Programmer's
> >> >> >> Reference Manual [1], see section 1.7.
> >> >> >>
> >> >> >> Because the hardware has a single transmit endpoint and a single
> >> >> >> receive endpoint the communication between the various DLN2 drivers
> >> >> >> and the hardware will be muxed/demuxed by this driver.
> >> >> >>
> >> >> >> Each DLN2 module will be identified by the handle field within the DLN2
> >> >> >> message header. If a DLN2 module issues multiple commands in parallel
> >> >> >> they will be identified by the echo counter field in the message header.
> >> >> >>
> >> >> >> The DLN2 modules can use the dln2_transfer() function to issue a
> >> >> >> command and wait for its response. They can also register a callback
> >> >> >> that is going to be called when a specific event id is generated by
> >> >> >> the device (e.g. GPIO interrupts). The device uses handle 0 for
> >> >> >> sending events.
> >> >> >>
> >> >> >> [1] https://www.diolan.com/downloads/dln-api-manual.pdf
> >> >> >
> >> >> > MFD is not a dumping ground for misfit h/w.  Almost all of this code
> >> >> > looks like it belongs in drivers/usb.  Please move it there.
> >> >> >
> >> >>
> >> >> We initially submitted this driver as a pure USB driver, with our own
> >> >> module registration mechanism, but during the first round of reviews
> >> >> people pointed out that a MFD driver is the better approach, and I
> >> >> agree. I also see that there are already a couple of USB drivers
> >> >> implemented as MFD drivers.
> >> >
> >> > Can you link me to your previous submission please?
> >>
> >> Sure, here it is:
> >>
> >> https://lkml.org/lkml/2014/8/20/228
> >>
> >> >
> >> >> Do you see a better approach?
> >> >
> >> > You should have a small MFD driver which controls resources and
> >> > registers children.  All other functionality should live in their
> >> > respective drivers/X locations i.e. USB functionallity should normally
> >> > live in drivers/usb.
> >> >
> >>
> >> OK, that sounds better. I am not sure how to handle the registration
> >> part though, since in this case we need to create the children at
> >> runtime, from the usb probe routine.
> >>
> >> The only solution I see is to move the driver completely to
> >> usb/drivers and continue to use the MFD infrastructure. Does that
> >> sound OK to you?
> >
> > I have no problem with that.  If this is an MFD driver, it _should_
> > live in drivers/mfd.  However, all of that USB specific stuff
> > defiantly should not.
> >
> 
> It is a multi-function driver which is using the USB interface, so I
> am not sure where it belongs. The only driver that calls
> mfd_add_devices and is not in drivers/mfd is the hid sensor hub
> driver.
> 
> BTW, the mfd/viperboard.c driver is very similar with this driver. It
> has less USB specific stuff because the protocol is simpler, but still
> has some.

Looking at viperboard.c, it seems to use some basic generic framework
calls to obtain some information about the device information like
version numbers.  Your driver is leaps and bounds more USB centric.

Your MFD driver should know about things like; regmap, platform data,
memory allocation, same-chip devices (children), etc.  Your MFD driver
should not need to concern itself with; endpoints, slots, URBs, USB
device IDs and the like.  The later knowledge belongs in drivers/usb. 

You should be calling mfd_add_devices() from within the MFD driver.
At a guess, I would say that you need a new entry for the USB stuff in
your mfd_cells structure.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Octavian Purdila]

On Mon, Sep 1, 2014 at 6:46 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 01 Sep 2014, Octavian Purdila wrote:
>
>> On Mon, Sep 1, 2014 at 2:39 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Mon, 01 Sep 2014, Octavian Purdila wrote:
>> >
>> >> On Mon, Sep 1, 2014 at 12:51 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> > On Mon, 01 Sep 2014, Octavian Purdila wrote:
>> >> >
>> >> >> On Mon, Sep 1, 2014 at 11:37 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> >> > On Sat, 30 Aug 2014, Octavian Purdila wrote:
>> >> >> >
>> >> >> >> This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
>> >> >> >> Master Adapter DLN-2. Details about the device can be found here:
>> >> >> >>
>> >> >> >> https://www.diolan.com/i2c/i2c_interface.html.
>> >> >> >>
>> >> >> >> Information about the USB protocol can be found in the Programmer's
>> >> >> >> Reference Manual [1], see section 1.7.
>> >> >> >>
>> >> >> >> Because the hardware has a single transmit endpoint and a single
>> >> >> >> receive endpoint the communication between the various DLN2 drivers
>> >> >> >> and the hardware will be muxed/demuxed by this driver.
>> >> >> >>
>> >> >> >> Each DLN2 module will be identified by the handle field within the DLN2
>> >> >> >> message header. If a DLN2 module issues multiple commands in parallel
>> >> >> >> they will be identified by the echo counter field in the message header.
>> >> >> >>
>> >> >> >> The DLN2 modules can use the dln2_transfer() function to issue a
>> >> >> >> command and wait for its response. They can also register a callback
>> >> >> >> that is going to be called when a specific event id is generated by
>> >> >> >> the device (e.g. GPIO interrupts). The device uses handle 0 for
>> >> >> >> sending events.
>> >> >> >>
>> >> >> >> [1] https://www.diolan.com/downloads/dln-api-manual.pdf
>> >> >> >
>> >> >> > MFD is not a dumping ground for misfit h/w.  Almost all of this code
>> >> >> > looks like it belongs in drivers/usb.  Please move it there.
>> >> >> >
>> >> >>
>> >> >> We initially submitted this driver as a pure USB driver, with our own
>> >> >> module registration mechanism, but during the first round of reviews
>> >> >> people pointed out that a MFD driver is the better approach, and I
>> >> >> agree. I also see that there are already a couple of USB drivers
>> >> >> implemented as MFD drivers.
>> >> >
>> >> > Can you link me to your previous submission please?
>> >>
>> >> Sure, here it is:
>> >>
>> >> https://lkml.org/lkml/2014/8/20/228
>> >>
>> >> >
>> >> >> Do you see a better approach?
>> >> >
>> >> > You should have a small MFD driver which controls resources and
>> >> > registers children.  All other functionality should live in their
>> >> > respective drivers/X locations i.e. USB functionallity should normally
>> >> > live in drivers/usb.
>> >> >
>> >>
>> >> OK, that sounds better. I am not sure how to handle the registration
>> >> part though, since in this case we need to create the children at
>> >> runtime, from the usb probe routine.
>> >>
>> >> The only solution I see is to move the driver completely to
>> >> usb/drivers and continue to use the MFD infrastructure. Does that
>> >> sound OK to you?
>> >
>> > I have no problem with that.  If this is an MFD driver, it _should_
>> > live in drivers/mfd.  However, all of that USB specific stuff
>> > defiantly should not.
>> >
>>
>> It is a multi-function driver which is using the USB interface, so I
>> am not sure where it belongs. The only driver that calls
>> mfd_add_devices and is not in drivers/mfd is the hid sensor hub
>> driver.
>>
>> BTW, the mfd/viperboard.c driver is very similar with this driver. It
>> has less USB specific stuff because the protocol is simpler, but still
>> has some.
>
> Looking at viperboard.c, it seems to use some basic generic framework
> calls to obtain some information about the device information like
> version numbers.  Your driver is leaps and bounds more USB centric.
>
> Your MFD driver should know about things like; regmap, platform data,
> memory allocation, same-chip devices (children), etc.  Your MFD driver
> should not need to concern itself with; endpoints, slots, URBs, USB
> device IDs and the like.  The later knowledge belongs in drivers/usb.
>
> You should be calling mfd_add_devices() from within the MFD driver.
> At a guess, I would say that you need a new entry for the USB stuff in
> your mfd_cells structure.
>

Makes sense, thanks for making clearing up what the MFD part of the
driver should do.

Here is how I think it could work:

 * keep the usb probe routine in the MFD driver (and keep it a usb driver)

 * add a new cell for the usb part

 * pass the usb_interface via platform_data to the USB sub-driver's
platform_device probe routine and continue the USB setup there


Lee, USB folks, is this acceptable?

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Johan Hovold]

On Mon, Sep 01, 2014 at 07:22:39PM +0300, Octavian Purdila wrote:
> On Mon, Sep 1, 2014 at 6:46 PM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Mon, 01 Sep 2014, Octavian Purdila wrote:
> >
> >> On Mon, Sep 1, 2014 at 2:39 PM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >> > On Mon, 01 Sep 2014, Octavian Purdila wrote:
> >> >> On Mon, Sep 1, 2014 at 12:51 PM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> >> >> > You should have a small MFD driver which controls resources and
> >> >> > registers children.  All other functionality should live in their
> >> >> > respective drivers/X locations i.e. USB functionallity should normally
> >> >> > live in drivers/usb.
> >> >> >
> >> >>
> >> >> OK, that sounds better. I am not sure how to handle the registration
> >> >> part though, since in this case we need to create the children at
> >> >> runtime, from the usb probe routine.
> >> >>
> >> >> The only solution I see is to move the driver completely to
> >> >> usb/drivers and continue to use the MFD infrastructure. Does that
> >> >> sound OK to you?
> >> >
> >> > I have no problem with that.  If this is an MFD driver, it _should_
> >> > live in drivers/mfd.  However, all of that USB specific stuff
> >> > defiantly should not.
> >> >
> >> > >> It is a multi-function driver which is using the USB interface, so I
> >> am not sure where it belongs. The only driver that calls
> >> mfd_add_devices and is not in drivers/mfd is the hid sensor hub
> >> driver.
> >>
> >> BTW, the mfd/viperboard.c driver is very similar with this driver. It
> >> has less USB specific stuff because the protocol is simpler, but still
> >> has some.
> >
> > Looking at viperboard.c, it seems to use some basic generic framework
> > calls to obtain some information about the device information like
> > version numbers.  Your driver is leaps and bounds more USB centric.
> >
> > Your MFD driver should know about things like; regmap, platform data,
> > memory allocation, same-chip devices (children), etc.  Your MFD driver
> > should not need to concern itself with; endpoints, slots, URBs, USB
> > device IDs and the like.  The later knowledge belongs in drivers/usb.
> >
> > You should be calling mfd_add_devices() from within the MFD driver.
> > At a guess, I would say that you need a new entry for the USB stuff in
> > your mfd_cells structure.
> >
> 
> Makes sense, thanks for making clearing up what the MFD part of the
> driver should do.
> 
> Here is how I think it could work:
> 
>  * keep the usb probe routine in the MFD driver (and keep it a usb driver)
> 
>  * add a new cell for the usb part
> 
>  * pass the usb_interface via platform_data to the USB sub-driver's
> platform_device probe routine and continue the USB setup there
> 
> Lee, USB folks, is this acceptable?

No, no. USB is not a function of the MFD device, it's the transport.
Thus there should be no USB MFD-cell. No subdriver can work without it.

And the USB id belongs in the MFD-driver in the same way that an
i2c id (address) does.

Just like an MFD device with i2c as a transport, this driver would
function as an arbiter to a shared resource (i.e. the register space).
The reason it seems much more USB-centric than an i2c-mfd driver is that
that transport API is simpler and some code have also already been
generalised (e.g. regmap), whereas we appear to have only two USB
mfd-drivers thus far.

The viperboard is perhaps a bad example in so far that it has pushed the
transport details down into the subdrivers (and thus into gpio, i2c and
iio subsystems) instead of handling it one place.

I haven't looked at the details of the protocol for the device in
question, but it might even be possible to use regmap here (as I
mentioned in my comments on v1).

Johan

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Lee Jones]

On Mon, 01 Sep 2014, Johan Hovold wrote:
> On Mon, Sep 01, 2014 at 07:22:39PM +0300, Octavian Purdila wrote:
> > On Mon, Sep 1, 2014 at 6:46 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > > On Mon, 01 Sep 2014, Octavian Purdila wrote:
> > >
> > >> On Mon, Sep 1, 2014 at 2:39 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > >> > On Mon, 01 Sep 2014, Octavian Purdila wrote:
> > >> >> On Mon, Sep 1, 2014 at 12:51 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > >> >> > You should have a small MFD driver which controls resources and
> > >> >> > registers children.  All other functionality should live in their
> > >> >> > respective drivers/X locations i.e. USB functionallity should normally
> > >> >> > live in drivers/usb.
> > >> >> >
> > >> >>
> > >> >> OK, that sounds better. I am not sure how to handle the registration
> > >> >> part though, since in this case we need to create the children at
> > >> >> runtime, from the usb probe routine.
> > >> >>
> > >> >> The only solution I see is to move the driver completely to
> > >> >> usb/drivers and continue to use the MFD infrastructure. Does that
> > >> >> sound OK to you?
> > >> >
> > >> > I have no problem with that.  If this is an MFD driver, it _should_
> > >> > live in drivers/mfd.  However, all of that USB specific stuff
> > >> > defiantly should not.
> > >> >
> > >> > >> It is a multi-function driver which is using the USB interface, so I
> > >> am not sure where it belongs. The only driver that calls
> > >> mfd_add_devices and is not in drivers/mfd is the hid sensor hub
> > >> driver.
> > >>
> > >> BTW, the mfd/viperboard.c driver is very similar with this driver. It
> > >> has less USB specific stuff because the protocol is simpler, but still
> > >> has some.
> > >
> > > Looking at viperboard.c, it seems to use some basic generic framework
> > > calls to obtain some information about the device information like
> > > version numbers.  Your driver is leaps and bounds more USB centric.
> > >
> > > Your MFD driver should know about things like; regmap, platform data,
> > > memory allocation, same-chip devices (children), etc.  Your MFD driver
> > > should not need to concern itself with; endpoints, slots, URBs, USB
> > > device IDs and the like.  The later knowledge belongs in drivers/usb.
> > >
> > > You should be calling mfd_add_devices() from within the MFD driver.
> > > At a guess, I would say that you need a new entry for the USB stuff in
> > > your mfd_cells structure.
> > >
> > 
> > Makes sense, thanks for making clearing up what the MFD part of the
> > driver should do.
> > 
> > Here is how I think it could work:
> > 
> >  * keep the usb probe routine in the MFD driver (and keep it a usb driver)
> > 
> >  * add a new cell for the usb part
> > 
> >  * pass the usb_interface via platform_data to the USB sub-driver's
> > platform_device probe routine and continue the USB setup there
> > 
> > Lee, USB folks, is this acceptable?
> 
> No, no. USB is not a function of the MFD device, it's the transport.
> Thus there should be no USB MFD-cell. No subdriver can work without it.
> 
> And the USB id belongs in the MFD-driver in the same way that an
> i2c id (address) does.
> 
> Just like an MFD device with i2c as a transport, this driver would
> function as an arbiter to a shared resource (i.e. the register space).
> The reason it seems much more USB-centric than an i2c-mfd driver is that
> that transport API is simpler and some code have also already been
> generalised (e.g. regmap), whereas we appear to have only two USB
> mfd-drivers thus far.
> 
> The viperboard is perhaps a bad example in so far that it has pushed the
> transport details down into the subdrivers (and thus into gpio, i2c and
> iio subsystems) instead of handling it one place.

Thanks for your explanation.  I take your point about the USB ID and I
did say I was guessing that the USB part should exist as a child
device.

So after your comments I decided to do a little investigation.  It
appears that this MFD driver is _just_ using the common API which all
other devices utilising USB comms are forced to use.  Is that correct?

If so, I have a question.  Is there no way to hide more of the USB
specifics inside a better, simpler API?  It looks like the drivers
which use USB are subjected to a lot (too much) of what might be
considered internals.  Or is it just that the client has to tinker
with too many dials to get anything sensible out? *shudders*

> I haven't looked at the details of the protocol for the device in
> question, but it might even be possible to use regmap here (as I
> mentioned in my comments on v1).

Obviously that would be preferred.

Octavian, did you look into that?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Octavian Purdila]

On Tue, Sep 2, 2014 at 11:00 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Mon, 01 Sep 2014, Johan Hovold wrote:
>> On Mon, Sep 01, 2014 at 07:22:39PM +0300, Octavian Purdila wrote:
>> > On Mon, Sep 1, 2014 at 6:46 PM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > > On Mon, 01 Sep 2014, Octavian Purdila wrote:
>> > >
>> > >> On Mon, Sep 1, 2014 at 2:39 PM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > >> > On Mon, 01 Sep 2014, Octavian Purdila wrote:
>> > >> >> On Mon, Sep 1, 2014 at 12:51 PM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>
>> > >> >> > You should have a small MFD driver which controls resources and
>> > >> >> > registers children.  All other functionality should live in their
>> > >> >> > respective drivers/X locations i.e. USB functionallity should normally
>> > >> >> > live in drivers/usb.
>> > >> >> >
>> > >> >>
>> > >> >> OK, that sounds better. I am not sure how to handle the registration
>> > >> >> part though, since in this case we need to create the children at
>> > >> >> runtime, from the usb probe routine.
>> > >> >>
>> > >> >> The only solution I see is to move the driver completely to
>> > >> >> usb/drivers and continue to use the MFD infrastructure. Does that
>> > >> >> sound OK to you?
>> > >> >
>> > >> > I have no problem with that.  If this is an MFD driver, it _should_
>> > >> > live in drivers/mfd.  However, all of that USB specific stuff
>> > >> > defiantly should not.
>> > >> >
>> > >> > >> It is a multi-function driver which is using the USB interface, so I
>> > >> am not sure where it belongs. The only driver that calls
>> > >> mfd_add_devices and is not in drivers/mfd is the hid sensor hub
>> > >> driver.
>> > >>
>> > >> BTW, the mfd/viperboard.c driver is very similar with this driver. It
>> > >> has less USB specific stuff because the protocol is simpler, but still
>> > >> has some.
>> > >
>> > > Looking at viperboard.c, it seems to use some basic generic framework
>> > > calls to obtain some information about the device information like
>> > > version numbers.  Your driver is leaps and bounds more USB centric.
>> > >
>> > > Your MFD driver should know about things like; regmap, platform data,
>> > > memory allocation, same-chip devices (children), etc.  Your MFD driver
>> > > should not need to concern itself with; endpoints, slots, URBs, USB
>> > > device IDs and the like.  The later knowledge belongs in drivers/usb.
>> > >
>> > > You should be calling mfd_add_devices() from within the MFD driver.
>> > > At a guess, I would say that you need a new entry for the USB stuff in
>> > > your mfd_cells structure.
>> > >
>> >
>> > Makes sense, thanks for making clearing up what the MFD part of the
>> > driver should do.
>> >
>> > Here is how I think it could work:
>> >
>> >  * keep the usb probe routine in the MFD driver (and keep it a usb driver)
>> >
>> >  * add a new cell for the usb part
>> >
>> >  * pass the usb_interface via platform_data to the USB sub-driver's
>> > platform_device probe routine and continue the USB setup there
>> >
>> > Lee, USB folks, is this acceptable?
>>
>> No, no. USB is not a function of the MFD device, it's the transport.
>> Thus there should be no USB MFD-cell. No subdriver can work without it.
>>
>> And the USB id belongs in the MFD-driver in the same way that an
>> i2c id (address) does.
>>
>> Just like an MFD device with i2c as a transport, this driver would
>> function as an arbiter to a shared resource (i.e. the register space).
>> The reason it seems much more USB-centric than an i2c-mfd driver is that
>> that transport API is simpler and some code have also already been
>> generalised (e.g. regmap), whereas we appear to have only two USB
>> mfd-drivers thus far.
>>
>> The viperboard is perhaps a bad example in so far that it has pushed the
>> transport details down into the subdrivers (and thus into gpio, i2c and
>> iio subsystems) instead of handling it one place.
>
> Thanks for your explanation.  I take your point about the USB ID and I
> did say I was guessing that the USB part should exist as a child
> device.
>
> So after your comments I decided to do a little investigation.  It
> appears that this MFD driver is _just_ using the common API which all
> other devices utilising USB comms are forced to use.  Is that correct?
>
> If so, I have a question.  Is there no way to hide more of the USB
> specifics inside a better, simpler API?  It looks like the drivers
> which use USB are subjected to a lot (too much) of what might be
> considered internals.  Or is it just that the client has to tinker
> with too many dials to get anything sensible out? *shudders*
>

Yes, the driver is just using the common USB API to communicate with
the device.

It is more complicated then the average USB driver because there is a
single interface and a single receive endpoint which needs to serve
multiple drivers and requests, thus the need to multiplex the
communication.

>> I haven't looked at the details of the protocol for the device in
>> question, but it might even be possible to use regmap here (as I
>> mentioned in my comments on v1).
>
> Obviously that would be preferred.
>
> Octavian, did you look into that?
>

Yes, I did. Since this is the first time I am looking at regmap I may
be wrong but I don't see a way to use it. The dln2 i2c driver needs to
be able to send and receive arbitrary size buffers and this does not
seem possible to do with the regmap API.

(Also creating a regmap class for a particular device seems over
engineering since nobody else is going to use it)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Johan Hovold]

On Tue, Sep 02, 2014 at 11:45:55AM +0300, Octavian Purdila wrote:
> On Tue, Sep 2, 2014 at 11:00 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Mon, 01 Sep 2014, Johan Hovold wrote:

> >> I haven't looked at the details of the protocol for the device in
> >> question, but it might even be possible to use regmap here (as I
> >> mentioned in my comments on v1).
> >
> > Obviously that would be preferred.
> >
> > Octavian, did you look into that?
> >
> Yes, I did. Since this is the first time I am looking at regmap I may
> be wrong but I don't see a way to use it. The dln2 i2c driver needs to
> be able to send and receive arbitrary size buffers and this does not
> seem possible to do with the regmap API.

That should be possible using the regmap bus read and write operations.

> (Also creating a regmap class for a particular device seems over
> engineering since nobody else is going to use it)

Possibly, but it would allow subdrivers to be implemented using a
standard interface and also provide register caching for free.

The event callbacks of the device in questions would not fit this scheme
though, but perhaps only that part needs to be driver specific.

Johan

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Octavian Purdila]

On Tue, Sep 2, 2014 at 6:23 PM, Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Sep 02, 2014 at 11:45:55AM +0300, Octavian Purdila wrote:
>> On Tue, Sep 2, 2014 at 11:00 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > On Mon, 01 Sep 2014, Johan Hovold wrote:
>
>> >> I haven't looked at the details of the protocol for the device in
>> >> question, but it might even be possible to use regmap here (as I
>> >> mentioned in my comments on v1).
>> >
>> > Obviously that would be preferred.
>> >
>> > Octavian, did you look into that?
>> >
>> Yes, I did. Since this is the first time I am looking at regmap I may
>> be wrong but I don't see a way to use it. The dln2 i2c driver needs to
>> be able to send and receive arbitrary size buffers and this does not
>> seem possible to do with the regmap API.
>
> That should be possible using the regmap bus read and write operations.

I took a closer look on the regmap bus read/write operations and I
think they are not fit for what we need in the driver. The driver uses
a request/response model which, IMHO, does not fit well with a
register read/write API. Yes, maybe we can emulate it, but why do
that?

>> (Also creating a regmap class for a particular device seems over
>> engineering since nobody else is going to use it)
>
> Possibly, but it would allow subdrivers to be implemented using a
> standard interface and also provide register caching for free.
>

Using a standard interface is nice, but I think that using the right
interface type is more important. This hardware does not use registers
but a messages to communicate with the OS.

Also caching is not useful for i2c and only partially useful for GPIO
(to cache the pin direction, and pin status in output mode only) so
its not a big win.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Johan Hovold]

On Wed, Sep 03, 2014 at 04:39:48PM +0300, Octavian Purdila wrote:
> On Tue, Sep 2, 2014 at 6:23 PM, Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> > That should be possible using the regmap bus read and write operations.
> 
> I took a closer look on the regmap bus read/write operations and I
> think they are not fit for what we need in the driver. The driver uses
> a request/response model which, IMHO, does not fit well with a
> register read/write API. Yes, maybe we can emulate it, but why do
> that?
> 
> >> (Also creating a regmap class for a particular device seems over
> >> engineering since nobody else is going to use it)
> >
> > Possibly, but it would allow subdrivers to be implemented using a
> > standard interface and also provide register caching for free.
> 
> Using a standard interface is nice, but I think that using the right
> interface type is more important. This hardware does not use registers
> but a messages to communicate with the OS.

You might be right, and as I mentioned, I haven't looked that closely at
the protocol yet. I'll take a look at your updated I/O interface and how
you use it.

Johan

Re: [PATCH v2 1/3] mfd: add support for Diolan DLN-2 devices

[Johan Hovold]

On Tue, Sep 02, 2014 at 09:00:10AM +0100, Lee Jones wrote:
> On Mon, 01 Sep 2014, Johan Hovold wrote:

> > No, no. USB is not a function of the MFD device, it's the transport.
> > Thus there should be no USB MFD-cell. No subdriver can work without it.
> > 
> > And the USB id belongs in the MFD-driver in the same way that an
> > i2c id (address) does.
> > 
> > Just like an MFD device with i2c as a transport, this driver would
> > function as an arbiter to a shared resource (i.e. the register space).
> > The reason it seems much more USB-centric than an i2c-mfd driver is that
> > that transport API is simpler and some code have also already been
> > generalised (e.g. regmap), whereas we appear to have only two USB
> > mfd-drivers thus far.
> > 
> > The viperboard is perhaps a bad example in so far that it has pushed the
> > transport details down into the subdrivers (and thus into gpio, i2c and
> > iio subsystems) instead of handling it one place.
> 
> Thanks for your explanation.  I take your point about the USB ID and I
> did say I was guessing that the USB part should exist as a child
> device.
> 
> So after your comments I decided to do a little investigation.  It
> appears that this MFD driver is _just_ using the common API which all
> other devices utilising USB comms are forced to use.  Is that correct?

Yes, it's using the low-level USB API, but there's a lot of higher-level
interfaces in place for (fairly) standard things such as the USB class
drivers or the USB serial subsystem.

> If so, I have a question.  Is there no way to hide more of the USB
> specifics inside a better, simpler API?  It looks like the drivers
> which use USB are subjected to a lot (too much) of what might be
> considered internals.  Or is it just that the client has to tinker
> with too many dials to get anything sensible out? *shudders*

Unfortunately, anything that does not already have a driver is likely to
use some vendor-specific protocol and therefore must use the low-level
API.

> > I haven't looked at the details of the protocol for the device in
> > question, but it might even be possible to use regmap here (as I
> > mentioned in my comments on v1).
> 
> Obviously that would be preferred.

Simple register-based USB MFD devices (e.g. only using control
transfers) are conceivable though, and if we start seeing a lot of those
(which I doubt) perhaps that part could be refactored as a regmap bus.

Johan

[PATCH v2 2/3] i2c: add support for Diolan DLN-2 USB-I2C adapter

[Octavian Purdila]

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 | 300 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 311 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
+       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..cc1335a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-dln2.c
@@ -0,0 +1,300 @@
+/*
+ * 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;
+}
+
+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");
-- 
1.9.1

[PATCH v2 3/3] gpio: add support for the Diolan DLN-2 USB GPIO driver

[Octavian Purdila]

From: Daniel Baluta <daniel.baluta@intel.com>

This patch adds GPIO and IRQ support for the Diolan DLN-2 GPIO module.

Information about the USB protocol interface can be found in the
Programmer's Reference Manual [1], see section 2.9 for the GPIO
module commands and responses.

[1] https://www.diolan.com/downloads/dln-api-manual.pdf

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 drivers/gpio/Kconfig     |  12 ++
 drivers/gpio/Makefile    |   1 +
 drivers/gpio/gpio-dln2.c | 506 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 519 insertions(+)
 create mode 100644 drivers/gpio/gpio-dln2.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 9de1515..6a9e352 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -897,4 +897,16 @@ config GPIO_VIPERBOARD
           River Tech's viperboard.h for detailed meaning
           of the module parameters.
 
+config GPIO_DLN2
+	tristate "Diolan DLN2 GPIO support"
+	depends on USB && MFD_DLN2
+	select GPIOLIB_IRQCHIP
+
+	help
+	  Select this option to enable GPIO driver for the Diolan DLN2
+	  board.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called gpio-dln2.
+
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 5d024e3..eaa97a0 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_GPIO_CRYSTAL_COVE)	+= gpio-crystalcove.o
 obj-$(CONFIG_GPIO_DA9052)	+= gpio-da9052.o
 obj-$(CONFIG_GPIO_DA9055)	+= gpio-da9055.o
 obj-$(CONFIG_GPIO_DAVINCI)	+= gpio-davinci.o
+obj-$(CONFIG_GPIO_DLN2)		+= gpio-dln2.o
 obj-$(CONFIG_GPIO_DWAPB)	+= gpio-dwapb.o
 obj-$(CONFIG_GPIO_EM)		+= gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)	+= gpio-ep93xx.o
diff --git a/drivers/gpio/gpio-dln2.c b/drivers/gpio/gpio-dln2.c
new file mode 100644
index 0000000..bb6cb1e
--- /dev/null
+++ b/drivers/gpio/gpio-dln2.c
@@ -0,0 +1,506 @@
+/*
+ * Driver for the Diolan DLN-2 USB-GPIO adapter
+ *
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * 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/slab.h>
+#include <linux/types.h>
+#include <linux/mutex.h>
+#include <linux/irqdomain.h>
+#include <linux/irq.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/usb.h>
+#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
+#include <linux/ptrace.h>
+#include <linux/wait.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/dln2.h>
+
+#define DRIVER_NAME "dln2-gpio"
+
+#define DLN2_GPIO_ID			0x01
+
+#define DLN2_GPIO_GET_PORT_COUNT	DLN2_CMD(0x00, DLN2_GPIO_ID)
+#define DLN2_GPIO_GET_PIN_COUNT		DLN2_CMD(0x01, DLN2_GPIO_ID)
+#define DLN2_GPIO_SET_DEBOUNCE		DLN2_CMD(0x04, DLN2_GPIO_ID)
+#define DLN2_GPIO_GET_DEBOUNCE		DLN2_CMD(0x05, DLN2_GPIO_ID)
+#define DLN2_GPIO_PORT_GET_VAL		DLN2_CMD(0x06, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_GET_VAL		DLN2_CMD(0x0B, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_SET_OUT_VAL	DLN2_CMD(0x0C, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_GET_OUT_VAL	DLN2_CMD(0x0D, DLN2_GPIO_ID)
+#define DLN2_GPIO_CONDITION_MET_EV	DLN2_CMD(0x0F, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_ENABLE		DLN2_CMD(0x10, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_DISABLE		DLN2_CMD(0x11, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_SET_DIRECTION	DLN2_CMD(0x13, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_GET_DIRECTION	DLN2_CMD(0x14, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_SET_EVENT_CFG	DLN2_CMD(0x1E, DLN2_GPIO_ID)
+#define DLN2_GPIO_PIN_GET_EVENT_CFG	DLN2_CMD(0x1F, DLN2_GPIO_ID)
+
+#define DLN2_GPIO_EVENT_NONE		0
+#define DLN2_GPIO_EVENT_CHANGE		1
+#define DLN2_GPIO_EVENT_LVL_HIGH	2
+#define DLN2_GPIO_EVENT_LVL_LOW		3
+#define DLN2_GPIO_EVENT_CHANGE_RISING	0x11
+#define DLN2_GPIO_EVENT_CHANGE_FALLING  0x21
+#define DLN2_GPIO_EVENT_MASK		0x0F
+
+#define DLN2_GPIO_MAX_PINS 32
+
+struct dln2_irq_work {
+	struct work_struct work;
+	struct dln2_gpio *dln2;
+	int pin, type;
+};
+
+struct dln2_gpio {
+	struct platform_device *pdev;
+	struct gpio_chip gpio;
+
+	DECLARE_BITMAP(irqs_masked, DLN2_GPIO_MAX_PINS);
+	DECLARE_BITMAP(irqs_enabled, DLN2_GPIO_MAX_PINS);
+	DECLARE_BITMAP(irqs_pending, DLN2_GPIO_MAX_PINS);
+	struct dln2_irq_work irq_work[DLN2_GPIO_MAX_PINS];
+};
+
+struct dln2_gpio_pin {
+	__le16 pin;
+} __packed;
+
+struct dln2_gpio_pin_val {
+	__le16 pin;
+	u8 value;
+} __packed;
+
+static int dln2_gpio_get_pin_count(struct platform_device *pdev)
+{
+	__le16 count;
+	int ret, len = sizeof(count);
+
+	ret = dln2_transfer(pdev, DLN2_GPIO_GET_PIN_COUNT, NULL, 0, &count,
+			    &len);
+	if (ret < 0)
+		return ret;
+
+	if (len < sizeof(count))
+		return -EPROTO;
+
+	return le16_to_cpu(count);
+}
+
+static int dln2_gpio_pin_cmd(struct dln2_gpio *dln2, int cmd, unsigned pin)
+{
+	struct dln2_gpio_pin req = {
+		.pin = cpu_to_le16(pin),
+	};
+
+	return dln2_transfer(dln2->pdev, cmd, &req, sizeof(req), NULL, NULL);
+}
+
+static int dln2_gpio_pin_val(struct dln2_gpio *dln2, int cmd, unsigned int pin)
+{
+	struct dln2_gpio_pin req = {
+		.pin = cpu_to_le16(pin),
+	};
+	struct dln2_gpio_pin_val rsp;
+	int ret, len = sizeof(rsp);
+
+	ret = dln2_transfer(dln2->pdev, cmd, &req, sizeof(req), &rsp, &len);
+	if (ret < 0)
+		return ret;
+
+	if (len < sizeof(rsp) || req.pin != rsp.pin)
+		return -EPROTO;
+
+	return rsp.value;
+}
+
+static int dln2_gpio_pin_get_in_val(struct dln2_gpio *dln2, unsigned int pin)
+{
+	int ret = dln2_gpio_pin_val(dln2, DLN2_GPIO_PIN_GET_VAL, pin);
+
+	if (ret < 0)
+		return ret;
+	return !!ret;
+}
+
+static int dln2_gpio_pin_get_out_val(struct dln2_gpio *dln2, unsigned int pin)
+{
+	int ret = dln2_gpio_pin_val(dln2, DLN2_GPIO_PIN_GET_OUT_VAL, pin);
+
+	if (ret < 0)
+		return ret;
+	return !!ret;
+}
+
+static void dln2_gpio_pin_set_out_val(struct dln2_gpio *dln2,
+				      unsigned int pin, int value)
+{
+	struct dln2_gpio_pin_val req = {
+		.pin = cpu_to_le16(pin),
+		.value = cpu_to_le16(value),
+	};
+
+	dln2_transfer(dln2->pdev, DLN2_GPIO_PIN_SET_OUT_VAL, &req, sizeof(req),
+		      NULL, NULL);
+}
+
+static int dln2_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	struct dln2_gpio *dln2 = container_of(chip, struct dln2_gpio, gpio);
+
+	return dln2_gpio_pin_cmd(dln2, DLN2_GPIO_PIN_ENABLE, offset);
+}
+
+static void dln2_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+	struct dln2_gpio *dln2 = container_of(chip, struct dln2_gpio, gpio);
+
+	dln2_gpio_pin_cmd(dln2, DLN2_GPIO_PIN_DISABLE, offset);
+}
+
+#define DLN2_GPIO_DIRECTION_IN		0
+#define DLN2_GPIO_DIRECTION_OUT		1
+
+static int dln2_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
+{
+	struct dln2_gpio *dln2 = container_of(chip, struct dln2_gpio, gpio);
+	struct dln2_gpio_pin req = {
+		.pin = cpu_to_le16(offset),
+	};
+	struct dln2_gpio_pin_val rsp;
+	int ret, len = sizeof(rsp);
+
+	ret = dln2_transfer(dln2->pdev, DLN2_GPIO_PIN_GET_DIRECTION,
+			    &req, sizeof(req), &rsp, &len);
+	if (ret < 0)
+		return ret;
+
+	if (len < sizeof(rsp) || req.pin != rsp.pin)
+		return -EPROTO;
+
+	switch (rsp.value) {
+	case DLN2_GPIO_DIRECTION_IN:
+		return 1;
+	case DLN2_GPIO_DIRECTION_OUT:
+		return 0;
+	default:
+		return -EPROTO;
+	}
+}
+
+static int dln2_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct dln2_gpio *dln2 = container_of(chip, struct dln2_gpio, gpio);
+	int dir = dln2_gpio_get_direction(chip, offset);
+
+	if (dir < 0)
+		return dir;
+
+	if (dir)
+		return dln2_gpio_pin_get_in_val(dln2, offset);
+
+	return dln2_gpio_pin_get_out_val(dln2, offset);
+}
+
+static void dln2_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct dln2_gpio *dln2 = container_of(chip, struct dln2_gpio, gpio);
+
+	dln2_gpio_pin_set_out_val(dln2, offset, value);
+}
+
+static int dln2_gpio_set_direction(struct gpio_chip *chip, unsigned offset,
+				   unsigned dir)
+{
+	struct dln2_gpio *dln2 = container_of(chip, struct dln2_gpio, gpio);
+	struct dln2_gpio_pin_val req = {
+		.pin = cpu_to_le16(offset),
+		.value = cpu_to_le16(dir),
+	};
+
+	return dln2_transfer(dln2->pdev, DLN2_GPIO_PIN_SET_DIRECTION,
+			     &req, sizeof(req), NULL, NULL);
+}
+
+static int dln2_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+	return dln2_gpio_set_direction(chip, offset, DLN2_GPIO_DIRECTION_IN);
+}
+
+static int dln2_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
+				      int value)
+{
+	return dln2_gpio_set_direction(chip, offset, DLN2_GPIO_DIRECTION_OUT);
+}
+
+static int dln2_gpio_set_debounce(struct gpio_chip *chip, unsigned offset,
+				  unsigned debounce)
+{
+	struct dln2_gpio *dln2 = container_of(chip, struct dln2_gpio, gpio);
+	struct {
+		__le32 duration;
+	} __packed req = {
+		.duration = cpu_to_le32(debounce),
+	};
+
+	return dln2_transfer(dln2->pdev, DLN2_GPIO_SET_DEBOUNCE,
+			     &req, sizeof(req), NULL, NULL);
+}
+
+static int dln2_gpio_set_event_cfg(struct dln2_gpio *dln2, unsigned pin,
+				   unsigned type, unsigned period)
+{
+	struct {
+		__le16 pin;
+		u8 type;
+		__le16 period;
+	} __packed req = {
+		.pin = cpu_to_le16(pin),
+		.type = type,
+		.period = cpu_to_le16(period),
+	};
+
+	return dln2_transfer(dln2->pdev, DLN2_GPIO_PIN_SET_EVENT_CFG,
+			     &req, sizeof(req), NULL, NULL);
+}
+
+static void dln2_irq_work(struct work_struct *w)
+{
+	struct dln2_irq_work *iw = container_of(w, struct dln2_irq_work, work);
+	struct dln2_gpio *dln2 = iw->dln2;
+	u8 type = iw->type & DLN2_GPIO_EVENT_MASK;
+
+	/* USB device has been disconnected, bail out */
+	if (!dln2->pdev)
+		return;
+
+	if (test_bit(iw->pin, dln2->irqs_enabled))
+		dln2_gpio_set_event_cfg(dln2, iw->pin, type, 0);
+	else
+		dln2_gpio_set_event_cfg(dln2, iw->pin, DLN2_GPIO_EVENT_NONE, 0);
+}
+
+static void dln2_irq_enable(struct irq_data *irqd)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct dln2_gpio *dln2 = container_of(gc, struct dln2_gpio, gpio);
+	int pin = irqd_to_hwirq(irqd);
+
+	set_bit(pin, dln2->irqs_enabled);
+	schedule_work(&dln2->irq_work[pin].work);
+}
+
+static void dln2_irq_disable(struct irq_data *irqd)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct dln2_gpio *dln2 = container_of(gc, struct dln2_gpio, gpio);
+	int pin = irqd_to_hwirq(irqd);
+
+	clear_bit(pin, dln2->irqs_enabled);
+	schedule_work(&dln2->irq_work[pin].work);
+}
+
+static void dln2_irq_mask(struct irq_data *irqd)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct dln2_gpio *dln2 = container_of(gc, struct dln2_gpio, gpio);
+	int pin = irqd_to_hwirq(irqd);
+
+	set_bit(pin, dln2->irqs_masked);
+}
+
+static void dln2_irq_unmask(struct irq_data *irqd)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct dln2_gpio *dln2 = container_of(gc, struct dln2_gpio, gpio);
+	int pin = irqd_to_hwirq(irqd);
+
+	if (test_and_clear_bit(pin, dln2->irqs_masked))
+		generic_handle_irq(pin);
+}
+
+static int dln2_irq_set_type(struct irq_data *irqd, unsigned type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct dln2_gpio *dln2 = container_of(gc, struct dln2_gpio, gpio);
+	int pin = irqd_to_hwirq(irqd);
+
+	switch (type) {
+	case IRQ_TYPE_LEVEL_HIGH:
+		dln2->irq_work[pin].type = DLN2_GPIO_EVENT_LVL_HIGH;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		dln2->irq_work[pin].type = DLN2_GPIO_EVENT_LVL_LOW;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		dln2->irq_work[pin].type = DLN2_GPIO_EVENT_CHANGE;
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		dln2->irq_work[pin].type = DLN2_GPIO_EVENT_CHANGE_RISING;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		dln2->irq_work[pin].type = DLN2_GPIO_EVENT_CHANGE_FALLING;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static struct irq_chip dln2_gpio_irqchip = {
+	.name = "dln2-irq",
+	.irq_enable = dln2_irq_enable,
+	.irq_disable = dln2_irq_disable,
+	.irq_mask = dln2_irq_mask,
+	.irq_unmask = dln2_irq_unmask,
+	.irq_set_type = dln2_irq_set_type,
+};
+
+static void dln2_gpio_event(struct platform_device *pdev, u16 echo,
+			    const void *data, int len)
+{
+	int pin, irq;
+	const struct {
+		__le16 count;
+		__u8 type;
+		__le16 pin;
+		__u8 value;
+	} __packed *event = data;
+	struct dln2_gpio *dln2 = platform_get_drvdata(pdev);
+
+	pin = le16_to_cpu(event->pin);
+	irq = irq_find_mapping(dln2->gpio.irqdomain, pin);
+
+	if (!irq) {
+		dev_err(dln2->gpio.dev, "pin %d not mapped to IRQ\n", pin);
+		return;
+	}
+
+	if (!test_bit(pin, dln2->irqs_enabled) ||
+	    test_bit(pin, dln2->irqs_masked))
+		return;
+
+	switch (dln2->irq_work[pin].type) {
+	case DLN2_GPIO_EVENT_CHANGE_RISING:
+		if (event->value)
+			generic_handle_irq(irq);
+		break;
+	case DLN2_GPIO_EVENT_CHANGE_FALLING:
+		if (!event->value)
+			generic_handle_irq(irq);
+		break;
+	default:
+		generic_handle_irq(irq);
+	}
+}
+
+static int dln2_gpio_probe(struct platform_device *pdev)
+{
+	struct dln2_gpio *dln2;
+	struct device *dev = &pdev->dev;
+	int pins = dln2_gpio_get_pin_count(pdev), i, ret;
+
+	if (pins < 0) {
+		dev_err(dev, "failed to get pin count: %d\n", pins);
+		return pins;
+	}
+	if (pins > DLN2_GPIO_MAX_PINS)
+		dev_warn(dev, "clamping pins to %d\n", DLN2_GPIO_MAX_PINS);
+
+	dln2 = kzalloc(sizeof(*dln2), GFP_KERNEL);
+	if (!dln2)
+		return -ENOMEM;
+
+	for (i = 0; i < DLN2_GPIO_MAX_PINS; i++) {
+		INIT_WORK(&dln2->irq_work[i].work, dln2_irq_work);
+		dln2->irq_work[i].pin = i;
+		dln2->irq_work[i].dln2 = dln2;
+	}
+
+	ret = dln2_register_event_cb(pdev, DLN2_GPIO_CONDITION_MET_EV,
+				     dln2_gpio_event);
+	if (ret) {
+		kfree(dln2);
+		return ret;
+	}
+
+	dln2->pdev = pdev;
+
+	dln2->gpio.label = "dln2";
+	dln2->gpio.dev = dev;
+	dln2->gpio.owner = THIS_MODULE;
+	dln2->gpio.base = -1;
+	dln2->gpio.ngpio = pins;
+	dln2->gpio.exported = 1;
+	dln2->gpio.set = dln2_gpio_set;
+	dln2->gpio.get = dln2_gpio_get;
+	dln2->gpio.request = dln2_gpio_request;
+	dln2->gpio.free = dln2_gpio_free;
+	dln2->gpio.get_direction = dln2_gpio_get_direction;
+	dln2->gpio.direction_input = dln2_gpio_direction_input;
+	dln2->gpio.direction_output = dln2_gpio_direction_output;
+	dln2->gpio.set_debounce = dln2_gpio_set_debounce;
+
+	ret = gpiochip_add(&dln2->gpio);
+	if (ret < 0) {
+		dev_err(dev, "failed to add gpio chip: %d\n", ret);
+		dln2_unregister_event_cb(pdev, DLN2_GPIO_CONDITION_MET_EV);
+		kfree(dln2);
+		return ret;
+	}
+
+	ret = gpiochip_irqchip_add(&dln2->gpio, &dln2_gpio_irqchip, 0,
+				   handle_simple_irq, IRQ_TYPE_NONE);
+	if (ret < 0) {
+		dev_err(dev, "failed to add irq chip: %d\n", ret);
+		dln2_unregister_event_cb(pdev, DLN2_GPIO_CONDITION_MET_EV);
+		gpiochip_remove(&dln2->gpio);
+		kfree(dln2);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, dln2);
+
+	return 0;
+}
+
+static int dln2_gpio_remove(struct platform_device *pdev)
+{
+	struct dln2_gpio *dln2 = platform_get_drvdata(pdev);
+	int ret;
+
+	dln2_unregister_event_cb(pdev, DLN2_GPIO_CONDITION_MET_EV);
+	ret = gpiochip_remove(&dln2->gpio);
+	if (ret < 0) {
+		dev_warn(dln2->gpio.dev, "error removing gpio chip: %d\n", ret);
+		dln2->pdev = NULL;
+		return ret;
+	}
+
+	kfree(dln2);
+
+	return 0;
+}
+
+static struct platform_driver dln2_gpio_driver = {
+	.driver.name	= DRIVER_NAME,
+	.driver.owner	= THIS_MODULE,
+	.probe		= dln2_gpio_probe,
+	.remove		= dln2_gpio_remove,
+};
+
+module_platform_driver(dln2_gpio_driver);
+
+MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com");
+MODULE_DESCRIPTION(DRIVER_NAME "driver");
+MODULE_LICENSE("GPL");
-- 
1.9.1

Re: [PATCH v2 3/3] gpio: add support for the Diolan DLN-2 USB GPIO driver

[Linus Walleij]

On Sat, Aug 30, 2014 at 12:00 AM, Octavian Purdila
<octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> From: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> This patch adds GPIO and IRQ support for the Diolan DLN-2 GPIO module.
>
> Information about the USB protocol interface can be found in the
> Programmer's Reference Manual [1], see section 2.9 for the GPIO
> module commands and responses.
>
> [1] https://www.diolan.com/downloads/dln-api-manual.pdf
>
> Signed-off-by: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Octavian Purdila <octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

I like this version. I assume you need to funnel it with the
rest of the patches through the MFD tree so:

Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Yours,
Linus Walleij