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

2014-11-05 Thread Johan Hovold
On Mon, Oct 27, 2014 at 06:31:09PM +0200, 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
> 
> Signed-off-by: Octavian Purdila 
> ---
>  drivers/mfd/Kconfig  |  11 +
>  drivers/mfd/Makefile |   1 +
>  drivers/mfd/dln2.c   | 761 
> +++
>  include/linux/mfd/dln2.h | 103 +++
>  4 files changed, 876 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 c9200d3..4538815a 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -189,6 +189,17 @@ 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 such as I2C_DLN2, GPIO_DLN2,
> +   etc. 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 61f8dbf..2cd7e74 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -175,6 +175,7 @@ obj-$(CONFIG_MFD_STW481X) += stw481x.o
>  obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
>  obj-$(CONFIG_MFD_MENF21BMC)  += menf21bmc.o
>  obj-$(CONFIG_MFD_HI6421_PMIC)+= hi6421-pmic-core.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 000..b3946ef
> --- /dev/null
> +++ b/drivers/mfd/dln2.c

[...]

> +struct dln2_header {
> + __le16 size;
> + __le16 id;
> + __le16 echo;
> + __le16 handle;
> +} __packed;
> +
> +struct dln2_response {
> + struct dln2_header hdr;
> + __le16 result;
> +} __packed;
> +

__packed not needed on either struct above.

[...]

> +/*
> + * Returns true if a valid transfer slot is found. In this case the URB must 
> not
> + * be resubmitted imediately in dln2_rx as we need the data when 
> dln2_transfer

s/imediately/immediately/

> + * is woke up. It will be resubmitted there.
> + */
> +static bool dln2_transfer_complete(struct dln2_dev *dln2, struct urb *urb,
> +u16 handle, u16 rx_slot)
> +{
> + struct device *dev = &dln2->interface->dev;
> + struct dln2_mod_rx_slots *rxs = &dln2->mod_rx_slots[handle];
> + struct dln2_rx_context *rxc;
> + bool valid_slot = false;
> +
> + rxc = &rxs->slots[rx_slot];
> +
> + /*
> +  * No need to disable interrupts as this lock is not taken in interrupt
> +  * context elsewhere in this driver. This function (or its callers) are
> +  * also not exported to other modules.
> +  */
> + spin_lock(&rxs->lock);
> + if (rxc->in_use && !rxc->urb) {
> + rxc->urb = urb;
> + complete(&rxc->done);
> + valid_slot = true;
> + }
> + spin_unlock(&rxs->lock);
> +
> + if (!valid_slot)
> + dev_warn(dev, "bad/late response %d/%d\n", handle, rx_slot);
> +
> + return valid_slot;
> +}
> +
> +static void dln2_run_event_callbacks(struct dln2_dev *dln2, u16 id, u16 echo,
> +  void *data, int len)
> +{
> + struct dln2_event_cb_entry *i;
> +
> + rcu_read_lock();
> +
> + list_for_each_entry_rcu(i, &dln2->event_cb_list, list)
> + if (i->id == id)
> + i->callback(i->pdev, echo, data, len);

No need to continue the search if id is found as it will be unique in
the list.

> +
> + rcu_read_unlock();
> +}
> +
> +static void dln2_rx(struct urb *urb)
>

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

2014-10-28 Thread Octavian Purdila
On Mon, Oct 27, 2014 at 6:57 PM, Joe Perches  wrote:
> On Mon, 2014-10-27 at 18:31 +0200, 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:
>
> trivia:
>

Thanks for the review Joe.

>> diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
> []
>> +static int _dln2_transfer(struct dln2_dev *dln2, u16 handle, u16 cmd,
>> +   const void *obuf, unsigned obuf_len,
>> +   void *ibuf, unsigned *ibuf_len)
>> +{
>> + int ret = 0;
>> + int rx_slot;
>> + struct dln2_response *rsp;
>> + struct dln2_rx_context *rxc;
>> + struct device *dev = &dln2->interface->dev;
>> + const int timeout = DLN2_USB_TIMEOUT * HZ / 1000;
>
> unsigned long timeout?
>

Ok.

> []
>
>> + ret = wait_for_completion_interruptible_timeout(&rxc->done, timeout);
>> + if (ret <= 0) {
>> + if (!ret)
>> + ret = -ETIMEDOUT;
>> + goto out_free_rx_slot;
>> + }
>
> []

I think you forgot to add your comments here?

>
>> +static int dln2_print_serialno(struct dln2_dev *dln2)
>> +{
>> + int ret;
>> + __le32 serial_no;
>> + int 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;
>> + if (len < sizeof(serial_no))
>> + return -EREMOTEIO;
>> +
>> + dev_info(dev, "Diolan DLN2 serial 0x%x\n", le32_to_cpu(serial_no));
>
> why a hex serial #?
>

Good point, I will switch it to decimal.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-10-27 Thread Joe Perches
On Mon, 2014-10-27 at 18:31 +0200, 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:

trivia:

> diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
[]
> +static int _dln2_transfer(struct dln2_dev *dln2, u16 handle, u16 cmd,
> +   const void *obuf, unsigned obuf_len,
> +   void *ibuf, unsigned *ibuf_len)
> +{
> + int ret = 0;
> + int rx_slot;
> + struct dln2_response *rsp;
> + struct dln2_rx_context *rxc;
> + struct device *dev = &dln2->interface->dev;
> + const int timeout = DLN2_USB_TIMEOUT * HZ / 1000;

unsigned long timeout?

[]

> + ret = wait_for_completion_interruptible_timeout(&rxc->done, timeout);
> + if (ret <= 0) {
> + if (!ret)
> + ret = -ETIMEDOUT;
> + goto out_free_rx_slot;
> + }

[]

> +static int dln2_print_serialno(struct dln2_dev *dln2)
> +{
> + int ret;
> + __le32 serial_no;
> + int 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;
> + if (len < sizeof(serial_no))
> + return -EREMOTEIO;
> +
> + dev_info(dev, "Diolan DLN2 serial 0x%x\n", le32_to_cpu(serial_no));

why a hex serial #?


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v9 1/4] mfd: add support for Diolan DLN-2 devices

2014-10-27 Thread 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 
---
 drivers/mfd/Kconfig  |  11 +
 drivers/mfd/Makefile |   1 +
 drivers/mfd/dln2.c   | 761 +++
 include/linux/mfd/dln2.h | 103 +++
 4 files changed, 876 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 c9200d3..4538815a 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -189,6 +189,17 @@ 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 such as I2C_DLN2, GPIO_DLN2,
+ etc. 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 61f8dbf..2cd7e74 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_MFD_STW481X)   += stw481x.o
 obj-$(CONFIG_MFD_IPAQ_MICRO)   += ipaq-micro.o
 obj-$(CONFIG_MFD_MENF21BMC)+= menf21bmc.o
 obj-$(CONFIG_MFD_HI6421_PMIC)  += hi6421-pmic-core.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 000..b3946ef
--- /dev/null
+++ b/drivers/mfd/dln2.c
@@ -0,0 +1,761 @@
+/*
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+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_URBS  16
+#define DLN2_RX_BUF_SIZE   512
+
+enum dln2_handle {
+   DLN2_HANDLE_EVENT = 0,  /* don't change, hardware defined */
+   DLN2_HANDLE_CTRL,
+   DLN2_HANDLE_GPIO,
+   DLN2_HANDLE_I2C,
+   DLN2_HANDLES
+};
+
+/*
+ * 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 {
+   /* completion used to wait for a response */
+   struct completion done;
+
+   /* if non-NULL the URB contains the response */
+   struct urb *urb;
+
+   /* if true then this context is used to wait for a response */
+   bool in_use;
+};
+
+/*
+ * Receive contexts for a particular DLN2 module (i2c, gpio, etc.). We use the
+ * handle header field to identify 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 parti