On Mon, 06 Oct 2014, Muthu Mani wrote: > Adds support for USB-I2C/GPIO interfaces of Cypress Semiconductor > CYUSBS234 USB-Serial Bridge controller. > > Details about the device can be found at: > http://www.cypress.com/?rID=84126 > > Signed-off-by: Muthu Mani <muth@xxxxxxxxxxx> > Signed-off-by: Rajaram Regupathy <rera@xxxxxxxxxxx> > --- > Changes since v2: > * Used auto mfd id to support multiple devices > * Cleaned up the code > > Changes since v1: > * Identified different serial interface and loaded correct cell driver > * Formed a mfd id to support multiple devices > * Removed unused platform device > > drivers/mfd/Kconfig | 12 +++ > drivers/mfd/Makefile | 1 + > drivers/mfd/cyusbs23x.c | 167 ++++++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/cyusbs23x.h | 62 ++++++++++++++++ > 4 files changed, 242 insertions(+) > create mode 100644 drivers/mfd/cyusbs23x.c > create mode 100644 include/linux/mfd/cyusbs23x.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index de5abf2..a31e9e3 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -116,6 +116,18 @@ config MFD_ASIC3 > This driver supports the ASIC3 multifunction chip found on many > PDAs (mainly iPAQ and HTC based ones) > > +config MFD_CYUSBS23X > + tristate "Cypress CYUSBS23x USB Serial Bridge controller" White space issue here. > + select MFD_CORE > + depends on USB > + default n > + help > + Say yes here if you want support for Cypress Semiconductor > + CYUSBS23x USB-Serial Bridge controller. > + > + This driver can also be built as a module. If so, the module will be > + called cyusbs23x. > + > config PMIC_DA903X > bool "Dialog Semiconductor DA9030/DA9034 PMIC Support" > depends on I2C=y > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index f001487..fc5bcd1 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -151,6 +151,7 @@ si476x-core-y := si476x-cmd.o si476x-prop.o si476x-i2c.o > obj-$(CONFIG_MFD_SI476X_CORE) += si476x-core.o > > obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o > +obj-$(CONFIG_MFD_CYUSBS23X) += cyusbs23x.o > obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o omap-usb-tll.o > obj-$(CONFIG_MFD_PM8921_CORE) += pm8921-core.o ssbi.o > obj-$(CONFIG_TPS65911_COMPARATOR) += tps65911-comparator.o > diff --git a/drivers/mfd/cyusbs23x.c b/drivers/mfd/cyusbs23x.c > new file mode 100644 > index 0000000..c70ea40 > --- /dev/null > +++ b/drivers/mfd/cyusbs23x.c > @@ -0,0 +1,167 @@ > +/* > + * Cypress USB-Serial Bridge Controller USB adapter driver > + * > + * Copyright (c) 2014 Cypress Semiconductor Corporation. > + * > + * Author: > + * Muthu Mani <muth@xxxxxxxxxxx> > + * > + * Additional contributors include: > + * Rajaram Regupathy <rera@xxxxxxxxxxx> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published by > + * the Free Software Foundation. > + */ > + > +/* > + * This is core MFD driver for Cypress Semiconductor CYUSBS234 USB-Serial > + * Bridge controller. CYUSBS234 offers a single channel serial interface > + * (I2C/SPI/UART). It can be configured to enable either of I2C, SPI, UART > + * interfaces. The GPIOs are also available to access. > + * Details about the device can be found at: > + * http://www.cypress.com/?rID=84126 > + * > + * Separate cell drivers are available for I2C and GPIO. SPI and UART are not > + * supported yet. All GPIOs are exposed for get operation. However, only > + * unused GPIOs can be set. > + */ > + > +#include <linux/kernel.h> > +#include <linux/errno.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/types.h> > +#include <linux/mutex.h> > + This '\n' is superfluous, please remove it. > +#include <linux/mfd/core.h> > +#include <linux/mfd/cyusbs23x.h> > + > +#include <linux/usb.h> > + > +static const struct usb_device_id cyusbs23x_usb_table[] = { > + { USB_DEVICE(0x04b4, 0x0004) }, /* Cypress Semiconductor */ > + { } /* Terminating entry */ This comment is not required, please remove it. > +}; > + > +MODULE_DEVICE_TABLE(usb, cyusbs23x_usb_table); > + > +static const struct mfd_cell cyusbs23x_i2c_devs[] = { > + { > + .name = "cyusbs23x-i2c", > + }, > + { > + .name = "cyusbs23x-gpio", > + }, > +}; > + > +static int update_ep_details(struct usb_interface *interface, > + struct cyusbs23x *cyusbs) > +{ > + struct usb_host_interface *iface_desc; > + struct usb_endpoint_descriptor *ep; > + int i; > + > + iface_desc = interface->cur_altsetting; > + > + for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { > + > + ep = &iface_desc->endpoint[i].desc; > + > + if (!cyusbs->bulk_in_ep_num && usb_endpoint_is_bulk_in(ep)) > + cyusbs->bulk_in_ep_num = ep->bEndpointAddress; > + if (!cyusbs->bulk_out_ep_num && usb_endpoint_is_bulk_out(ep)) > + cyusbs->bulk_out_ep_num = ep->bEndpointAddress; > + if (!cyusbs->intr_in_ep_num && usb_endpoint_is_int_in(ep)) > + cyusbs->intr_in_ep_num = ep->bEndpointAddress; > + } All of the USB specific code in this driver will require a USB Ack. > + dev_dbg(&interface->dev, "%s intr_in=%d, bulk_in=%d, bulk_out=%d\n", > + __func__, cyusbs->intr_in_ep_num , > + cyusbs->bulk_in_ep_num, cyusbs->bulk_out_ep_num); > + > + if (!cyusbs->bulk_in_ep_num || !cyusbs->bulk_out_ep_num || > + !cyusbs->intr_in_ep_num) > + return -ENODEV; > + > + return 0; > +} > + > +static int cyusbs23x_probe(struct usb_interface *interface, > + const struct usb_device_id *id) > +{ > + struct cyusbs23x *cyusbs; > + const struct mfd_cell *cyusbs23x_devs; > + int ret, ndevs = 0; > + u8 sub_class; > + > + cyusbs = kzalloc(sizeof(*cyusbs), GFP_KERNEL); Any reason for not using managed resources (devm_*)? > + if (cyusbs == NULL) if (!cyusbs) > + return -ENOMEM; > + > + cyusbs->usb_dev = usb_get_dev(interface_to_usbdev(interface)); Can you do this last? Then you can remove the 'error' error path. > + cyusbs->usb_intf = interface; > + cyusbs->intf_num = interface->cur_altsetting->desc.bInterfaceNumber; If you're already saving 'interface' there is no need to save this also, just extract it from 'cyusbs->usb_intf' when you need it. > + ret = update_ep_details(interface, cyusbs); > + if (ret < 0) { Can ret be > 0? If not, just do 'if (ret)' > + dev_err(&interface->dev, "invalid interface\n"); If you put the error message in update_ep_details() can you print out a more specific error message and remove this line. > + goto error; > + } > + > + usb_set_intfdata(interface, cyusbs); > + > + /* Serial interfaces (I2C, SPI, UART) differ in interface subclass */ > + sub_class = interface->cur_altsetting->desc.bInterfaceSubClass; > + switch (sub_class) { This is a waste of a variable and adds nothing to the code. Just switch() for 'interface->cur_altsetting->desc.bInterfaceSubClass'. > + case CY_USBS_SCB_I2C: > + dev_info(&interface->dev, "I2C interface found\n"); Was it even lost? Would "using I2C interface" be better? > + cyusbs23x_devs = cyusbs23x_i2c_devs; > + ndevs = ARRAY_SIZE(cyusbs23x_i2c_devs); > + break; I assume there will be other interfaces supported at a later date? If not, this switch() is pretty over-the-top. > + default: > + dev_err(&interface->dev, "unsupported subclass\n"); > + ret = -ENODEV; > + goto error; > + } > + > + ret = mfd_add_devices(&interface->dev, PLATFORM_DEVID_AUTO, > + cyusbs23x_devs, ndevs, NULL, 0, NULL); > + if (ret != 0) { if (ret) > + dev_err(&interface->dev, "Failed to add mfd devices to core\n"); "Failed to register devices" > + goto error; > + } > + > + return 0; > + > +error: > + usb_put_dev(cyusbs->usb_dev); > + kfree(cyusbs); > + > + return ret; > +} > + > +static void cyusbs23x_disconnect(struct usb_interface *interface) > +{ > + struct cyusbs23x *cyusbs = usb_get_intfdata(interface); > + > + mfd_remove_devices(&interface->dev); > + usb_put_dev(cyusbs->usb_dev); > + kfree(cyusbs); If you use managed resources, you can remove this line. > + dev_dbg(&interface->dev, "disconnected\n"); Please remove this line. > +} > + > +static struct usb_driver cyusbs23x_usb_driver = { > + .name = "cyusbs23x", > + .probe = cyusbs23x_probe, > + .disconnect = cyusbs23x_disconnect, > + .id_table = cyusbs23x_usb_table, > +}; > + > +module_usb_driver(cyusbs23x_usb_driver); > + > +MODULE_AUTHOR("Rajaram Regupathy <rera@xxxxxxxxxxx>"); > +MODULE_AUTHOR("Muthu Mani <muth@xxxxxxxxxxx>"); > +MODULE_DESCRIPTION("Cypress CYUSBS23x mfd core driver"); s/mfd/MFD/ > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/mfd/cyusbs23x.h b/include/linux/mfd/cyusbs23x.h > new file mode 100644 > index 0000000..1580d98 > --- /dev/null > +++ b/include/linux/mfd/cyusbs23x.h > @@ -0,0 +1,62 @@ > +/* > + * Cypress USB-Serial Bridge Controller definitions > + * > + * Copyright (c) 2014 Cypress Semiconductor Corporation. > + * > + * Author: > + * Muthu Mani <muth@xxxxxxxxxxx> > + * > + * Additional contributors include: > + * Rajaram Regupathy <rera@xxxxxxxxxxx> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published by > + * the Free Software Foundation. > + */ > + > +#ifndef __MFD_CYUSBS23X_H__ > +#define __MFD_CYUSBS23X_H__ > + > +#include <linux/types.h> > +#include <linux/usb.h> > + > +/* Structure to hold all device specific stuff */ > +struct cyusbs23x { > + struct usb_device *usb_dev; > + struct usb_interface *usb_intf; > + > + u8 intf_num; > + u8 bulk_in_ep_num; > + u8 bulk_out_ep_num; > + u8 intr_in_ep_num; > +}; > + > +enum cy_usbs_vendor_cmds { > + CY_I2C_GET_CONFIG_CMD = 0xC4, > + CY_I2C_SET_CONFIG_CMD = 0xC5, > + CY_I2C_WRITE_CMD = 0xC6, > + CY_I2C_READ_CMD = 0xC7, > + CY_I2C_GET_STATUS_CMD = 0xC8, > + CY_I2C_RESET_CMD = 0xC9, > + CY_GPIO_GET_CONFIG_CMD = 0xD8, > + CY_GPIO_SET_CONFIG_CMD = 0xD9, > + CY_GPIO_GET_VALUE_CMD = 0xDA, > + CY_GPIO_SET_VALUE_CMD = 0xDB, > +}; > + > +/* Serial interfaces (I2C, SPI, UART) differ in interface subclass */ > +enum cy_scb_modes { > + CY_USBS_SCB_DISABLED = 0, > + CY_USBS_SCB_UART = 1, > + CY_USBS_SCB_SPI = 2, > + CY_USBS_SCB_I2C = 3 No need to number these. > +}; > + > +/* SCB index shift */ > +#define CY_SCB_INDEX_SHIFT 15 > + > +#define CY_USBS_CTRL_XFER_TIMEOUT 2000 > +#define CY_USBS_BULK_XFER_TIMEOUT 5000 > +#define CY_USBS_INTR_XFER_TIMEOUT 5000 > + > +#endif /* __MFD_CYUSBS23X_H__ */ -- 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-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html