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 <octavian.purdila@xxxxxxxxx> > --- > 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 0000000..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) > +{ > + 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; > + int 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_HANDLES) { > + dev_warn(dev, "RX: invalid handle %d\n", handle); Drop the RX: prefix for consistency. > + goto out; > + } > + > + data = urb->transfer_buffer + sizeof(struct dln2_header); > + len = urb->actual_length - sizeof(struct dln2_header); > + > + if (handle == DLN2_HANDLE_EVENT) { > + dln2_run_event_callbacks(dln2, id, echo, data, len); > + } else { > + /* URB will be re-submitted in _dln2_transfer (free_rx_slot) */ > + if (dln2_transfer_complete(dln2, urb, handle, echo)) > + return; > + } > + > +out: > + err = usb_submit_urb(urb, GFP_ATOMIC); > + if (err < 0) > + dev_err(dev, "failed to resubmit RX URB: %d\n", err); > +} [...] > +static int dln2_setup_rx_urbs(struct dln2_dev *dln2, > + struct usb_host_interface *hostif) > +{ > + int i; > + const int rx_max_size = DLN2_RX_BUF_SIZE; > + > + for (i = 0; i < DLN2_MAX_URBS; i++) { > + int ret; > + struct device *dev = &dln2->interface->dev; Again, no need to keep these inside the for-loop. > + > + 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; > +} Looks good otherwise. I'll respond with my reviewed by tag after you have addressed the above and Joe's comments. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
