Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support

[Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>]

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

On Thu, Apr 03, 2014 at 01:05:19PM +0530, Subbaraya Sundeep Bhatta wrote:
> This patch adds xilinx axi usb2 device driver support
> 
> Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
> Changes for v2:
> 	- Added Resume
> 	- Added Remote wakeup
> 	- Fixed v1 comments
> 
>  drivers/usb/gadget/Kconfig      |   14 +
>  drivers/usb/gadget/Makefile     |    1 +
>  drivers/usb/gadget/udc-xilinx.c | 2057 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 2072 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/usb/gadget/udc-xilinx.c
> 
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index 8154165..db43a79 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -466,6 +466,20 @@ config USB_EG20T
>  	  ML7213/ML7831 is companion chip for Intel Atom E6xx series.
>  	  ML7213/ML7831 is completely compatible for Intel EG20T PCH.
>  
> +config USB_GADGET_XILINX
> +	tristate "Xilinx USB Driver"
> +	depends on COMPILE_TEST
> +	help
> +	  USB peripheral controller driver for Xilinx AXI USB2 device.
> +	  Xilinx AXI USB2 device is a soft IP which supports both full
> +	  and high speed USB 2.0 data transfers. It has seven configurable
> +	  endpoints(bulk or interrupt or isochronous), as well as
> +	  endpoint zero(for control transfers).
> +
> +	  Say "y" to link the driver statically, or "m" to build a
> +	  dynamically linked module called "xilinx_udc" and force all
> +	  gadget drivers to also be dynamically linked.
> +
>  #
>  # LAST -- dummy/emulated controller
>  #
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index 5f150bc..8a3fc0b 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_USB_FUSB300)	+= fusb300_udc.o
>  obj-$(CONFIG_USB_FOTG210_UDC)	+= fotg210-udc.o
>  obj-$(CONFIG_USB_MV_U3D)	+= mv_u3d_core.o
>  obj-$(CONFIG_USB_GR_UDC)	+= gr_udc.o
> +obj-$(CONFIG_USB_GADGET_XILINX)	+= udc-xilinx.o
>  
>  # USB Functions
>  usb_f_acm-y			:= f_acm.o
> diff --git a/drivers/usb/gadget/udc-xilinx.c b/drivers/usb/gadget/udc-xilinx.c
> new file mode 100644
> index 0000000..5709aeb
> --- /dev/null
> +++ b/drivers/usb/gadget/udc-xilinx.c
> @@ -0,0 +1,2057 @@
> +/*
> + * Xilinx USB peripheral controller driver
> + *
> + * Copyright (C) 2004 by Thomas Rathbone
> + * Copyright (C) 2005 by HP Labs
> + * Copyright (C) 2005 by David Brownell
> + * Copyright (C) 2010 - 2014 Xilinx, Inc.
> + *
> + * Some parts of this driver code is based on the driver for at91-series
> + * USB peripheral controller (at91_udc.c).
> + *
> + * 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;
> + * either version 2 of the License, or (at your option) any
> + * later version.
> + */
> +#include <linux/interrupt.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/prefetch.h>
> +#include <linux/usb/ch9.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/io.h>
> +#include <linux/seq_file.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_irq.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/delay.h>
> +#include "gadget_chips.h"
> +
> +/* Register offsets for the USB device.*/
> +#define XUSB_EP0_CONFIG_OFFSET		0x0000  /* EP0 Config Reg Offset */
> +#define XUSB_SETUP_PKT_ADDR_OFFSET	0x0080  /* Setup Packet Address */
> +#define XUSB_ADDRESS_OFFSET		0x0100  /* Address Register */
> +#define XUSB_CONTROL_OFFSET		0x0104  /* Control Register */
> +#define XUSB_STATUS_OFFSET		0x0108  /* Status Register */
> +#define XUSB_FRAMENUM_OFFSET		0x010C	/* Frame Number Register */
> +#define XUSB_IER_OFFSET			0x0110	/* Interrupt Enable Register */
> +#define XUSB_BUFFREADY_OFFSET		0x0114	/* Buffer Ready Register */
> +#define XUSB_TESTMODE_OFFSET		0x0118	/* Test Mode Register */
> +#define XUSB_DMA_RESET_OFFSET		0x0200  /* DMA Soft Reset Register */
> +#define XUSB_DMA_CONTROL_OFFSET		0x0204	/* DMA Control Register */
> +#define XUSB_DMA_DSAR_ADDR_OFFSET	0x0208	/* DMA source Address Reg */
> +#define XUSB_DMA_DDAR_ADDR_OFFSET	0x020C	/* DMA destination Addr Reg */
> +#define XUSB_DMA_LENGTH_OFFSET		0x0210	/* DMA Length Register */
> +#define XUSB_DMA_STATUS_OFFSET		0x0214	/* DMA Status Register */
> +
> +/* Endpoint Configuration Space offsets */
> +#define XUSB_EP_CFGSTATUS_OFFSET	0x00	/* Endpoint Config Status  */
> +#define XUSB_EP_BUF0COUNT_OFFSET	0x08	/* Buffer 0 Count */
> +#define XUSB_EP_BUF1COUNT_OFFSET	0x0C	/* Buffer 1 Count */
> +
> +#define XUSB_CONTROL_USB_READY_MASK	0x80000000 /* USB ready Mask */
> +#define XUSB_CONTROL_USB_RMTWAKE_MASK	0x40000000 /* Remote wake up mask */
> +
> +/* Interrupt register related masks.*/
> +#define XUSB_STATUS_GLOBAL_INTR_MASK	0x80000000 /* Global Intr Enable */
> +#define XUSB_STATUS_RESUME_MASK		0x01000000 /* USB Resume Mask */
> +#define XUSB_STATUS_RESET_MASK		0x00800000 /* USB Reset Mask */
> +#define XUSB_STATUS_SUSPEND_MASK	0x00400000 /* USB Suspend Mask */
> +#define XUSB_STATUS_FIFO_BUFF_RDY_MASK	0x00100000 /* FIFO Buff Ready Mask */
> +#define XUSB_STATUS_FIFO_BUFF_FREE_MASK	0x00080000 /* FIFO Buff Free Mask */
> +#define XUSB_STATUS_SETUP_PACKET_MASK	0x00040000 /* Setup packet received */
> +#define XUSB_STATUS_EP1_BUFF2_COMP_MASK	0x00000200 /* EP 1 Buff 2 Processed */
> +#define XUSB_STATUS_EP1_BUFF1_COMP_MASK	0x00000002 /* EP 1 Buff 1 Processed */
> +#define XUSB_STATUS_EP0_BUFF2_COMP_MASK	0x00000100 /* EP 0 Buff 2 Processed */
> +#define XUSB_STATUS_EP0_BUFF1_COMP_MASK	0x00000001 /* EP 0 Buff 1 Processed */
> +#define XUSB_STATUS_HIGH_SPEED_MASK	0x00010000 /* USB Speed Mask */
> +/* Suspend,Reset and Disconnect Mask */
> +#define XUSB_STATUS_INTR_EVENT_MASK	0x00C00000
> +/* Buffers  completion Mask */
> +#define XUSB_STATUS_INTR_BUFF_COMP_ALL_MASK	0x0000FEFF
> +/* Mask for buffer 0 and buffer 1 completion for all Endpoints */
> +#define XUSB_STATUS_INTR_BUFF_COMP_SHIFT_MASK	0x00000101
> +#define XUSB_STATUS_EP_BUFF2_SHIFT	8	   /* EP buffer offset */
> +
> +/* Endpoint Configuration Status Register */
> +#define XUSB_EP_CFG_VALID_MASK		0x80000000 /* Endpoint Valid bit */
> +#define XUSB_EP_CFG_STALL_MASK		0x40000000 /* Endpoint Stall bit */
> +#define XUSB_EP_CFG_DATA_TOGGLE_MASK	0x08000000 /* Endpoint Data toggle */
> +
> +/* USB device specific global configuration constants.*/
> +#define XUSB_MAX_ENDPOINTS		8	/* Maximum End Points */
> +#define XUSB_EP_NUMBER_ZERO		0	/* End point Zero */
> +/* DPRAM is the source address for DMA transfer */
> +#define XUSB_DMA_READ_FROM_DPRAM	0x80000000
> +#define XUSB_DMA_DMASR_BUSY		0x80000000 /* DMA busy */
> +#define XUSB_DMA_DMASR_ERROR		0x40000000 /* DMA Error */
> +/*
> + * When this bit is set, the DMA buffer ready bit is set by hardware upon
> + * DMA transfer completion.
> + */
> +#define XUSB_DMA_BRR_CTRL		0x40000000 /* DMA bufready ctrl bit */
> +/* Phase States */
> +#define SETUP_PHASE			0x0000	/* Setup Phase */
> +#define DATA_PHASE			0x0001  /* Data Phase */
> +#define STATUS_PHASE			0x0002  /* Status Phase */
> +
> +#define EP0_MAX_PACKET		64 /* Endpoint 0 maximum packet length */
> +
> +/* container_of helper macros */
> +#define to_udc(g)	 container_of((g), struct xusb_udc, gadget)
> +#define to_xusb_ep(ep)	 container_of((ep), struct xusb_ep, ep_usb)
> +#define to_xusb_req(req) container_of((req), struct xusb_req, usb_req)
> +
> +/*-------------------------------------------------------------------------*/
> +
> +#ifdef DEBUG
> +#define DBG(fmt, args...)	pr_debug("[%s]  " fmt "\n", \
> +				__func__, ## args)
> +#else
> +#define DBG(fmt, args...)	do {} while (0)
> +#endif
> +
> +#ifdef VERBOSE
> +#define VDBG		DBG
> +#else
> +#define VDBG(stuff...)	do {} while (0)
> +#endif
> +
> +#define ERR(stuff...)		pr_err("udc: " stuff)
> +#define WARNING(stuff...)		pr_warn("udc: " stuff)
> +#define INFO(stuff...)		pr_info("udc: " stuff)

NACK, this is always to cleanup later. Please stick to
dev_{info,err,warn,dbg,vdbg}()

> +struct xusb_udc {
> +	struct usb_gadget gadget;
> +	struct xusb_ep ep[8];
> +	struct usb_gadget_driver *driver;
> +	struct cmdbuf ch9cmd;
> +	u32 usb_state;
> +	u32 remote_wkp;
> +	unsigned int (*read_fn)(void __iomem *);
> +	void (*write_fn)(void __iomem *, u32, u32);

why do you need these to be function pointers ? Because of endianness ?
generic readl()/writel() already take care of that.

> +	void __iomem *base_address;
> +	spinlock_t lock;
> +	bool dma_enabled;
> +};
> +
> +/* Endpoint buffer start addresses in the core */
> +static u32 rambase[8] = { 0x22, 0x1000, 0x1100, 0x1200, 0x1300, 0x1400, 0x1500,
> +			0x1600 };
> +
> +static const char driver_name[] = "xilinx-udc";
> +static const char ep0name[] = "ep0";
> +
> +/* Control endpoint configuration.*/
> +static struct usb_endpoint_descriptor config_bulk_out_desc = {

should be const, you never modify this.

> +	.bLength = USB_DT_ENDPOINT_SIZE,
> +	.bDescriptorType = USB_DT_ENDPOINT,
> +	.bEndpointAddress = USB_DIR_OUT,
> +	.bmAttributes = USB_ENDPOINT_XFER_BULK,
> +	.wMaxPacketSize = __constant_cpu_to_le16(0x40),

let's use the decimal here just for clarity.

> +/**
> + * xudc_wrstatus - Sets up the usb device status stages.
> + * @udc: pointer to the usb device controller structure.
> + */
> +static void xudc_wrstatus(struct xusb_udc *udc)
> +{
> +	u32 epcfgreg;
> +
> +	epcfgreg = udc->read_fn(udc->base_address +
> +				udc->ep[XUSB_EP_NUMBER_ZERO].offset)|
> +				XUSB_EP_CFG_DATA_TOGGLE_MASK;

are you really trying to mask here ? If you're trying to mask you should
be using a bitwise and.

> +	udc->write_fn(udc->base_address, udc->ep[XUSB_EP_NUMBER_ZERO].offset,
> +			epcfgreg);
> +	udc->write_fn(udc->base_address, udc->ep[XUSB_EP_NUMBER_ZERO].offset +
> +			  XUSB_EP_BUF0COUNT_OFFSET, 0);
> +	udc->write_fn(udc->base_address, XUSB_BUFFREADY_OFFSET, 1);

you can improve redability on this by defining some local variables:

struct xusb_udc_ep *ep0 = &udc->ep[XUSB_EP_NUMBER_ZERO];
u32 reg;

reg = xudc_readl(udc->base_address, ep0->offset);
reg |= XUSB_EP_CFG_DATA_TOGGLE_MASK;
xudc_writel(udc->base_address, ep0->offset + XUSB_EP_BUF0COUNT_OFFSET, 0);
xudc_writel(udc->base_address, XUSB_BUFFREADY_OFFSET, 1);

and so on, likewise for all other functions.

> +static int start_dma(struct xusb_ep *ep, u32 src, u32 dst, u32 length)

please prepend this with xudc_, it makes tracing a lot easier.

> +{
> +	struct xusb_udc *udc;
> +	int rc = 0;
> +	unsigned long timeout;
> +
> +	udc = ep->udc;
> +	/*
> +	 * Set the addresses in the DMA source and
> +	 * destination registers and then set the length
> +	 * into the DMA length register.
> +	 */
> +	udc->write_fn(udc->base_address, XUSB_DMA_DSAR_ADDR_OFFSET, src);
> +	udc->write_fn(udc->base_address, XUSB_DMA_DDAR_ADDR_OFFSET, dst);
> +	udc->write_fn(udc->base_address, XUSB_DMA_LENGTH_OFFSET, length);
> +
> +	/*
> +	 * Wait till DMA transaction is complete and
> +	 * check whether the DMA transaction was
> +	 * successful.
> +	*/
> +	while ((udc->read_fn(ep->udc->base_address + XUSB_DMA_STATUS_OFFSET) &
> +			XUSB_DMA_DMASR_BUSY) == XUSB_DMA_DMASR_BUSY) {
> +		timeout = jiffies + 10000;
> +
> +		if (time_after(jiffies, timeout)) {
> +			rc = -ETIMEDOUT;
> +			goto clean;
> +		}
> +	}

don't you get an IRQ for DMA completion ? If you do, you could be using
wait_for_completion()

> +	if ((udc->read_fn(udc->base_address + XUSB_DMA_STATUS_OFFSET) &
> +				XUSB_DMA_DMASR_ERROR) == XUSB_DMA_DMASR_ERROR){
> +		DBG("DMA Error\n");
> +		rc = -EINVAL;
> +	}
> +clean:
> +	if (ep->is_in) {
> +		dma_unmap_single(udc->gadget.dev.parent, src,
> +					length, DMA_TO_DEVICE);
> +	} else {
> +		dma_unmap_single(udc->gadget.dev.parent, dst,
> +					length, DMA_FROM_DEVICE);

NACK, use generic usb_gadget_map_reqeust() and
usb_gadget_unmap_request().

> +static int dma_send(struct xusb_ep *ep, u8 *buffer, u32 length)

prepend with xudc_

> +{
> +	u32 *eprambase;
> +	dma_addr_t src;
> +	u32 dst;
> +	int ret;
> +	struct xusb_udc *udc;
> +
> +	udc = ep->udc;
> +	src = dma_map_single(udc->gadget.dev.parent, buffer, length,
> +				DMA_TO_DEVICE);
> +	if (dma_mapping_error(udc->gadget.dev.parent, src)) {
> +		DBG("failed to map DMA\n");
> +		return -EFAULT;
> +	}

use generic mapping functions.

> +static int dma_receive(struct xusb_ep *ep, u8 *buffer, u32 length)

prepend with xudc_

> +{
> +	u32 *eprambase;
> +	u32 src;
> +	dma_addr_t dst;
> +	int ret;
> +	struct xusb_udc *udc;
> +
> +	udc = ep->udc;
> +	dst = dma_map_single(udc->gadget.dev.parent, buffer, length,
> +				DMA_FROM_DEVICE);
> +	if (dma_mapping_error(udc->gadget.dev.parent, dst)) {
> +		DBG("failed to map DMA\n");
> +		return -EFAULT;
> +	}

use generic mapping functions

> +
> +	if (!ep->curbufnum && !ep->buffer0ready) {
> +		/* Get the Buffer address and copy the transmit data */
> +		eprambase = (u32 __force *)(ep->udc->base_address +
> +				ep->rambase);
> +		src = virt_to_phys(eprambase);
> +		udc->write_fn(udc->base_address, XUSB_DMA_CONTROL_OFFSET,
> +				XUSB_DMA_BRR_CTRL | XUSB_DMA_READ_FROM_DPRAM |
> +				(1 << ep->epnumber));
> +		ep->buffer0ready = 1;
> +		ep->curbufnum = 1;
> +	} else if (ep->curbufnum && !ep->buffer1ready) {
> +		/* Get the Buffer address and copy the transmit data */
> +		eprambase = (u32 __force *)(ep->udc->base_address +
> +				ep->rambase + ep->ep_usb.maxpacket);
> +		src = virt_to_phys(eprambase);
> +		udc->write_fn(udc->base_address, XUSB_DMA_CONTROL_OFFSET,
> +				XUSB_DMA_BRR_CTRL | XUSB_DMA_READ_FROM_DPRAM |
> +				(1 << (ep->epnumber +
> +				XUSB_STATUS_EP_BUFF2_SHIFT)));
> +		ep->buffer1ready = 1;
> +		ep->curbufnum = 0;
> +	} else {
> +		/* None of the ping-pong buffers are ready currently */
> +		return 1;

you *must* return a proper error code here. -EAGAIN sounds like the one
you want.

> +static int xudc_eptxrx(struct xusb_ep *ep, u8 *bufferptr, u32 bufferlen)
> +{
> +	u32 *eprambase;
> +	u32 bytestosend;
> +	u8 *temprambase;
> +	int rc = 0;
> +	struct xusb_udc *udc = ep->udc;
> +
> +	bytestosend = bufferlen;
> +	if (udc->dma_enabled) {
> +		if (ep->is_in)
> +			rc = dma_send(ep, bufferptr, bufferlen);
> +		else
> +			rc = dma_receive(ep, bufferptr, bufferlen);
> +		return rc;
> +	}
> +	/* Put the transmit buffer into the correct ping-pong buffer.*/
> +	if (!ep->curbufnum && !ep->buffer0ready) {
> +		/* Get the Buffer address and copy the transmit data.*/
> +		eprambase = (u32 __force *)(udc->base_address + ep->rambase);
> +		while (bytestosend > 3) {
> +			if (ep->is_in)
> +				*eprambase++ = *(u32 *)bufferptr;
> +			else
> +				*(u32 *)bufferptr = *eprambase++;
> +			bufferptr += 4;
> +			bytestosend -= 4;
> +		}
> +		temprambase = (u8 *)eprambase;
> +		while (bytestosend--) {
> +			if (ep->is_in)
> +				*temprambase++ = *bufferptr++;
> +			else
> +				*bufferptr++ = *temprambase++;
> +		}
> +		/*
> +		 * Set the Buffer count register with transmit length
> +		 * and enable the buffer for transmission.
> +		 */
> +		if (ep->is_in)
> +			udc->write_fn(udc->base_address, ep->offset +
> +					XUSB_EP_BUF0COUNT_OFFSET, bufferlen);
> +		udc->write_fn(udc->base_address, XUSB_BUFFREADY_OFFSET,
> +				1 << ep->epnumber);
> +		ep->buffer0ready = 1;
> +		ep->curbufnum = 1;
> +	} else if ((ep->curbufnum == 1) && (!ep->buffer1ready)) {
> +		/* Get the Buffer address and copy the transmit data.*/
> +		eprambase = (u32 __force *)(udc->base_address + ep->rambase +
> +				ep->ep_usb.maxpacket);
> +		while (bytestosend > 3) {
> +			if (ep->is_in)
> +				*eprambase++ = *(u32 *)bufferptr;
> +			else
> +				*(u32 *)bufferptr = *eprambase++;
> +			bufferptr += 4;
> +			bytestosend -= 4;
> +		}
> +		temprambase = (u8 *)eprambase;
> +		while (bytestosend--) {
> +			if (ep->is_in)
> +				*temprambase++ = *bufferptr++;
> +			else
> +				*bufferptr++ = *temprambase++;
> +		}
> +		/*
> +		 * Set the Buffer count register with transmit
> +		 * length and enable the buffer for
> +		 * transmission.
> +		 */
> +		if (ep->is_in)
> +			udc->write_fn(udc->base_address, ep->offset +
> +					XUSB_EP_BUF1COUNT_OFFSET, bufferlen);
> +		udc->write_fn(udc->base_address, XUSB_BUFFREADY_OFFSET,
> +				1 << (ep->epnumber +
> +				XUSB_STATUS_EP_BUFF2_SHIFT));
> +		ep->buffer1ready = 1;
> +		ep->curbufnum = 0;
> +	} else {
> +		/* None of the ping-pong buffers are ready currently */
> +		return 1;
> +	}
> +	return rc;
> +}
> +
> +/**
> + * xudc_done - Exeutes the endpoint data transfer completion tasks.
> + * @ep: pointer to the usb device endpoint structure.
> + * @req: pointer to the usb request structure.
> + * @status: Status of the data transfer.
> + *
> + * Deletes the message from the queue and updates data transfer completion
> + * status.
> + */
> +static void xudc_done(struct xusb_ep *ep, struct xusb_req *req, int status)
> +{
> +	u8 stopped = ep->stopped;
> +
> +	list_del_init(&req->queue);
> +
> +	if (req->usb_req.status == -EINPROGRESS)
> +		req->usb_req.status = status;
> +	else
> +		status = req->usb_req.status;
> +
> +	if (status && status != -ESHUTDOWN)
> +		DBG("%s done %p, status %d\n", ep->ep_usb.name, req, status);

dev_dbg()

> +	ep->stopped = 1;
> +
> +	spin_unlock(&ep->udc->lock);
> +	if (req->usb_req.complete)
> +		req->usb_req.complete(&ep->ep_usb, &req->usb_req);
> +	spin_lock(&ep->udc->lock);
> +
> +	ep->stopped = stopped;
> +}
> +
> +/**
> + * xudc_read_fifo - Reads the data from the given endpoint buffer.
> + * @ep: pointer to the usb device endpoint structure.
> + * @req: pointer to the usb request structure.
> + *
> + * Return: 1 if request completed/dequeued or 0 if req is not
> + *		completed

should return 0 on success and negative errno in case it doesn't
complete. You must review your error handling!!

> +static int xudc_read_fifo(struct xusb_ep *ep, struct xusb_req *req)
> +{
> +	u8 *buf;
> +	u32 is_short, count, bufferspace;

avoid multiple declarations in one line.

> +	u8 bufoffset;
> +	u8 two_pkts = 0;
> +	struct xusb_udc *udc = ep->udc;
> +
> +	if ((ep->buffer0ready == 1) && (ep->buffer1ready == 1)) {
> +		DBG("Packet NOT ready!\n");
> +		return 0;
> +	}
> +top:
> +	if (ep->curbufnum)
> +		bufoffset = XUSB_EP_BUF1COUNT_OFFSET;
> +	else
> +		bufoffset = XUSB_EP_BUF0COUNT_OFFSET;
> +	count = udc->read_fn(ep->udc->base_address + ep->offset + bufoffset);
> +	if (!ep->buffer0ready && !ep->buffer1ready)
> +		two_pkts = 1;
> +
> +	DBG("curbufnum is %d  and buf0rdy is %d, buf1rdy is %d\n",
> +		ep->curbufnum, ep->buffer0ready, ep->buffer1ready);
> +
> +	buf = req->usb_req.buf + req->usb_req.actual;
> +	prefetchw(buf);
> +	bufferspace = req->usb_req.length - req->usb_req.actual;
> +	req->usb_req.actual += min(count, bufferspace);
> +	is_short = count < ep->ep_usb.maxpacket;
> +
> +	if (unlikely(!bufferspace)) {
> +		/*
> +		 * This happens when the driver's buffer
> +		 * is smaller than what the host sent.
> +		 * discard the extra data.
> +		 */
> +		if (req->usb_req.status != -EOVERFLOW)
> +			DBG("%s overflow %d\n", ep->ep_usb.name, count);
> +			req->usb_req.status = -EOVERFLOW;
> +	} else {
> +		switch (xudc_eptxrx(ep, buf, count)) {
> +		case 0:
> +			VDBG("read %s, %d bytes%s req %p %d/%d\n",
> +				ep->ep_usb.name, count, is_short ? "/S" : "",
> +				req, req->usb_req.actual, req->usb_req.length);
> +			bufferspace -= count;
> +			/* Completion */
> +			if ((req->usb_req.actual ==
> +				req->usb_req.length) || is_short) {
> +				xudc_done(ep, req, 0);
> +				return 1;
> +			}
> +			if (two_pkts) {
> +				two_pkts = 0;
> +				goto top;
> +			}
> +			break;
> +		case 1:
> +			VDBG("Rx buffers busy\n");
> +			req->usb_req.actual -= min(count, bufferspace);
> +			break;
> +		case -EINVAL:
> +		case -EFAULT:
> +		case -ETIMEDOUT:
> +			/* DMA error, dequeue the request */
> +			xudc_done(ep, req, -ECONNRESET);
> +			return 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * xudc_write_fifo - Writes data into the given endpoint buffer.
> + * @ep: pointer to the usb device endpoint structure.
> + * @req: pointer to the usb request structure.
> + *
> + * Return: 1 if request completed/dequeued or 0 if req is not
> + *		completed
> + *
> + * Loads endpoint buffer for an IN packet.
> + */
> +static int xudc_write_fifo(struct xusb_ep *ep, struct xusb_req *req)
> +{
> +	u8 *buf;
> +	u32 max;
> +	u32 length;
> +	int is_last, is_short = 0;
> +
> +	max = le16_to_cpu(ep->desc->wMaxPacketSize);
> +	buf = req->usb_req.buf + req->usb_req.actual;
> +	prefetch(buf);
> +	length = req->usb_req.length - req->usb_req.actual;
> +	length = min(length, max);
> +
> +	switch (xudc_eptxrx(ep, buf, length)) {
> +	case 0:
> +		req->usb_req.actual += length;
> +		if (unlikely(length != max)) {
> +			is_last = is_short = 1;
> +		} else {
> +			if (likely(req->usb_req.length !=
> +				   req->usb_req.actual) || req->usb_req.zero)
> +				is_last = 0;
> +			else
> +				is_last = 1;
> +		}
> +		VDBG("wrote %s %d bytes%s%s %d left %p\n", ep->ep_usb.name,
> +			length, is_last ? "/L" : "", is_short ? "/S" : "",
> +			req->usb_req.length - req->usb_req.actual, req);
> +		if (is_last) {
> +			xudc_done(ep, req, 0);
> +			return 1;
> +		}
> +		break;
> +	case 1:
> +		VDBG("Tx buffers busy\n");
> +		break;
> +	case -EINVAL:
> +	case -EFAULT:
> +	case -ETIMEDOUT:
> +		/* DMA error, dequeue the request */
> +		xudc_done(ep, req, -ECONNRESET);
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * xudc_nuke - Cleans up the data transfer message list.
> + * @ep: pointer to the usb device endpoint structure.
> + * @status: Status of the data transfer.
> + */
> +static void xudc_nuke(struct xusb_ep *ep, int status)
> +{
> +	struct xusb_req *req;
> +
> +	while (!list_empty(&ep->queue)) {
> +		req = list_entry(ep->queue.next, struct xusb_req, queue);
> +		xudc_done(ep, req, status);
> +	}
> +}
> +
> +/***************************** Endpoint related functions*********************/
> +/**
> + * xudc_ep_set_halt - Stalls/unstalls the given endpoint.
> + * @_ep: pointer to the usb device endpoint structure.
> + * @value: value to indicate stall/unstall.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int xudc_ep_set_halt(struct usb_ep *_ep, int value)
> +{
> +	struct xusb_ep *ep = to_xusb_ep(_ep);
> +	unsigned long flags;
> +	u32 epcfgreg;
> +	struct xusb_udc *udc = ep->udc;
> +
> +	if (!_ep || (!ep->desc && ep->epnumber))
> +		return -EINVAL;
> +
> +	spin_lock_irqsave(&udc->lock, flags);
> +
> +	if (ep->is_in && (!list_empty(&ep->queue)) && value) {
> +		spin_unlock_irqrestore(&udc->lock, flags);
> +		return -EAGAIN;
> +	}
> +	if ((ep->buffer0ready == 1) || (ep->buffer1ready == 1)) {
> +		spin_unlock_irqrestore(&udc->lock, flags);
> +		return -EAGAIN;
> +	}
> +	if (value) {
> +		/* Stall the device.*/
> +		epcfgreg = udc->read_fn(udc->base_address +
> +				   ep->offset);
> +		epcfgreg |= XUSB_EP_CFG_STALL_MASK;
> +
> +		udc->write_fn(udc->base_address, ep->offset, epcfgreg);
> +		ep->stopped = 1;
> +	} else {
> +		ep->stopped = 0;
> +		/* Unstall the device.*/
> +		epcfgreg = udc->read_fn(udc->base_address +
> +					    ep->offset);
> +		epcfgreg &= ~XUSB_EP_CFG_STALL_MASK;
> +		udc->write_fn(udc->base_address, ep->offset, epcfgreg);
> +		if (ep->epnumber) {
> +			/* Reset the toggle bit.*/
> +			epcfgreg = udc->read_fn(ep->udc->base_address +
> +						    ep->offset);
> +			epcfgreg &= ~XUSB_EP_CFG_DATA_TOGGLE_MASK;
> +			udc->write_fn(udc->base_address, ep->offset, epcfgreg);
> +		}
> +	}
> +	spin_unlock_irqrestore(&udc->lock, flags);
> +	return 0;
> +}
> +
> +/**
> + * xudc_ep_enable - Enables the given endpoint.
> + * @_ep: pointer to the usb device endpoint structure.
> + * @desc: pointer to usb endpoint descriptor.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int xudc_ep_enable(struct usb_ep *_ep,
> +			  const struct usb_endpoint_descriptor *desc)
> +{
> +	struct xusb_ep *ep = to_xusb_ep(_ep);
> +	u32 tmp;
> +	u8 eptype = 0;
> +	unsigned long flags;
> +	u32 epcfg;
> +	struct xusb_udc *udc = ep->udc;
> +
> +	/*
> +	 * The check for _ep->name == ep0name is not done as this enable is used
> +	 * for enabling ep0 also. In other gadget drivers, this ep name is not
> +	 * used.
> +	 */
> +	if (!_ep || !desc || ep->desc ||
> +			desc->bDescriptorType != USB_DT_ENDPOINT) {
> +		DBG("bad ep or descriptor\n");
> +		return -EINVAL;
> +	}
> +	if (!udc->driver || udc->gadget.speed == USB_SPEED_UNKNOWN) {
> +		DBG("bogus device state\n");
> +		return -ESHUTDOWN;
> +	}
> +
> +	ep->is_in = ((desc->bEndpointAddress & USB_DIR_IN) != 0);
> +	/* Bit 3...0:endpoint number */
> +	ep->epnumber = (desc->bEndpointAddress & 0x0f);
> +	ep->stopped = 0;
> +	ep->desc = desc;
> +	ep->ep_usb.desc = desc;
> +	tmp = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
> +
> +	spin_lock_irqsave(&udc->lock, flags);
> +
> +	ep->ep_usb.maxpacket = le16_to_cpu(desc->wMaxPacketSize);
> +	switch (tmp) {
> +	case USB_ENDPOINT_XFER_CONTROL:
> +		DBG("only one control endpoint\n");
> +		/* NON- ISO */
> +		eptype = 0;
> +		spin_unlock_irqrestore(&ep->udc->lock, flags);
> +		return -EINVAL;
> +	case USB_ENDPOINT_XFER_INT:
> +		/* NON- ISO */
> +		eptype = 0;
> +		if (ep->ep_usb.maxpacket > 64)
> +			goto bogus_max;
> +		break;
> +	case USB_ENDPOINT_XFER_BULK:
> +		/* NON- ISO */
> +		eptype = 0;
> +		switch (ep->ep_usb.maxpacket) {
> +		case 8:
> +		case 16:
> +		case 32:
> +		case 64:
> +		case 512:
> +			goto ok;
> +		}
> +bogus_max:
> +		DBG("bogus maxpacket %d\n", ep->ep_usb.maxpacket);
> +		spin_unlock_irqrestore(&ep->udc->lock, flags);
> +		return -EINVAL;
> +	case USB_ENDPOINT_XFER_ISOC:
> +		/* ISO */
> +		eptype = 1;
> +		ep->is_iso = 1;
> +		break;
> +	}
> +ok:
> +	ep->eptype = eptype;
> +	ep->buffer0ready = 0;
> +	ep->buffer1ready = 0;
> +	ep->curbufnum = 0;
> +	ep->rambase = rambase[ep->epnumber];
> +	xudc_epconfig(ep, udc);
> +
> +	DBG("Enable Endpoint %d max pkt is %d\n",
> +			ep->epnumber, ep->ep_usb.maxpacket);
> +
> +	/* Enable the End point.*/
> +	epcfg = udc->read_fn(udc->base_address + ep->offset);
> +	epcfg |= XUSB_EP_CFG_VALID_MASK;
> +	udc->write_fn(udc->base_address, ep->offset, epcfg);
> +	if (ep->epnumber)
> +		ep->rambase <<= 2;
> +
> +	if (ep->epnumber)
> +		udc->write_fn(udc->base_address, XUSB_IER_OFFSET,
> +				(udc->read_fn(ep->udc->base_address +
> +				XUSB_IER_OFFSET) |
> +				(XUSB_STATUS_INTR_BUFF_COMP_SHIFT_MASK <<
> +				ep->epnumber)));
> +	if (ep->epnumber && !ep->is_in) {
> +		/* Set the buffer ready bits.*/
> +		udc->write_fn(udc->base_address, XUSB_BUFFREADY_OFFSET,
> +				1 << ep->epnumber);
> +		ep->buffer0ready = 1;
> +		udc->write_fn(udc->base_address, XUSB_BUFFREADY_OFFSET,
> +				(1 << (ep->epnumber +
> +				XUSB_STATUS_EP_BUFF2_SHIFT)));
> +		ep->buffer1ready = 1;
> +	}
> +
> +	spin_unlock_irqrestore(&udc->lock, flags);
> +
> +	return 0;
> +}
> +
> +/**
> + * xudc_ep_disable - Disables the given endpoint.
> + * @_ep: pointer to the usb device endpoint structure.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int xudc_ep_disable(struct usb_ep *_ep)
> +{
> +	struct xusb_ep *ep = to_xusb_ep(_ep);
> +	unsigned long flags;
> +	u32 epcfg;
> +	struct xusb_udc *udc = ep->udc;
> +
> +	udc = ep->udc;
> +	if (ep == &udc->ep[XUSB_EP_NUMBER_ZERO]) {
> +		DBG("Ep0 disable called\n");
> +		return -EINVAL;
> +	}
> +
> +	spin_lock_irqsave(&udc->lock, flags);
> +	xudc_nuke(ep, -ESHUTDOWN);
> +
> +	/* Restore the endpoint's pristine config */
> +	ep->desc = NULL;
> +	ep->ep_usb.desc = NULL;
> +	ep->stopped = 1;
> +
> +	DBG("USB Ep %d disable\n ", ep->epnumber);
> +	/* Disable the endpoint.*/
> +	epcfg = udc->read_fn(udc->base_address + ep->offset);
> +	epcfg &= ~XUSB_EP_CFG_VALID_MASK;
> +	udc->write_fn(udc->base_address, ep->offset, epcfg);
> +
> +	spin_unlock_irqrestore(&udc->lock, flags);
> +	return 0;
> +}
> +
> +/**
> + * xudc_ep_alloc_request - Initializes the request queue.
> + * @_ep: pointer to the usb device endpoint structure.
> + * @gfp_flags: Flags related to the request call.
> + *
> + * Return: pointer to request structure on success and a NULL on failure.
> + */
> +static struct usb_request *xudc_ep_alloc_request(struct usb_ep *_ep,
> +						 gfp_t gfp_flags)
> +{
> +	struct xusb_req *req;
> +
> +	req = kzalloc(sizeof(*req), gfp_flags);
> +	if (!req)
> +		return NULL;
> +	req->ep = to_xusb_ep(_ep);
> +	INIT_LIST_HEAD(&req->queue);
> +	return &req->usb_req;
> +}
> +
> +/**
> + * xudc_free_request - Releases the request from queue.
> + * @_ep: pointer to the usb device endpoint structure.
> + * @_req: pointer to the usb request structure.
> + */
> +static void xudc_free_request(struct usb_ep *_ep, struct usb_request *_req)
> +{
> +	struct xusb_req *req = to_xusb_req(_req);
> +
> +	kfree(req);
> +}
> +
> +/**
> + * xudc_ep_queue - Adds the request to the queue.
> + * @_ep: pointer to the usb device endpoint structure.
> + * @_req: pointer to the usb request structure.
> + * @gfp_flags: Flags related to the request call.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int xudc_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
> +			 gfp_t gfp_flags)
> +{
> +	struct xusb_req *req;
> +	struct xusb_ep *ep;
> +	unsigned long flags;
> +	u32 length, count;
> +	u8 *corebuf;
> +	struct xusb_udc *udc;
> +
> +	req = to_xusb_req(_req);
> +	ep = to_xusb_ep(_ep);
> +
> +	if (!_req || !_req->complete || !_req->buf
> +			|| !list_empty(&req->queue)) {
> +		DBG("invalid request\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!_ep || (!ep->desc && ep->ep_usb.name != ep0name)) {
> +		DBG("invalid ep\n");
> +		return -EINVAL;
> +	}
> +
> +	udc = ep->udc;
> +	if (!udc || !udc->driver || udc->gadget.speed == USB_SPEED_UNKNOWN) {
> +		DBG("bogus device state\n");
> +		return -EINVAL;
> +	}
> +	spin_lock_irqsave(&udc->lock, flags);
> +
> +	_req->status = -EINPROGRESS;
> +	_req->actual = 0;
> +	/* Try to kickstart any empty and idle queue */
> +	if (list_empty(&ep->queue)) {
> +		if (!ep->epnumber) {
> +			ep->data = req;
> +			if (udc->ch9cmd.setup.bRequestType & USB_DIR_IN) {
> +				udc->ch9cmd.wrtptr = req->usb_req.buf +
> +							req->usb_req.actual;
> +				prefetch(udc->ch9cmd.wrtptr);
> +				length = req->usb_req.length -
> +						req->usb_req.actual;
> +				corebuf = (void __force *) ((ep->rambase << 2) +
> +					    ep->udc->base_address);
> +				udc->ch9cmd.writecount = length;
> +				length = count = min_t(u32, length,
> +							EP0_MAX_PACKET);
> +				while (length--)
> +					*corebuf++ = *udc->ch9cmd.wrtptr++;
> +				udc->write_fn(udc->base_address,
> +						XUSB_EP_BUF0COUNT_OFFSET,
> +						count);
> +				udc->write_fn(udc->base_address,
> +						XUSB_BUFFREADY_OFFSET, 1);
> +				udc->ch9cmd.writecount -= count;
> +			} else {
> +				if (udc->ch9cmd.setup.wLength) {
> +					udc->ch9cmd.readptr =
> +						req->usb_req.buf +
> +							req->usb_req.actual;
> +					udc->write_fn(udc->base_address,
> +						  XUSB_EP_BUF0COUNT_OFFSET,
> +						  req->usb_req.length);
> +					udc->write_fn(udc->base_address,
> +						  XUSB_BUFFREADY_OFFSET, 1);
> +				} else {
> +					xudc_wrstatus(udc);
> +					req = NULL;
> +				}
> +			}
> +		} else {
> +			if (ep->is_in) {
> +				VDBG("xudc_write_fifo called from queue\n");
> +				if (xudc_write_fifo(ep, req) == 1)
> +					req = NULL;
> +			} else {
> +				VDBG("xudc_read_fifo called from queue\n");
> +				if (xudc_read_fifo(ep, req) == 1)
> +					req = NULL;
> +			}
> +		}
> +	}

looks like you need some refactoring here to avoid deep indentations.

> +	if (req != NULL)
> +		list_add_tail(&req->queue, &ep->queue);
> +
> +	spin_unlock_irqrestore(&udc->lock, flags);
> +	return 0;
> +}
> +
> +/**
> + * xudc_ep_dequeue - Removes the request from the queue.
> + * @_ep: pointer to the usb device endpoint structure.
> + * @_req: pointer to the usb request structure.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int xudc_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
> +{
> +	struct xusb_ep *ep;
> +	struct xusb_req *req;
> +	unsigned long flags;
> +
> +	ep = to_xusb_ep(_ep);
> +
> +	if (!_ep || ep->ep_usb.name == ep0name)
> +		return -EINVAL;
> +
> +	spin_lock_irqsave(&ep->udc->lock, flags);
> +	/* Make sure it's actually queued on this endpoint */
> +	list_for_each_entry(req, &ep->queue, queue) {
> +		if (&req->usb_req == _req)
> +			break;
> +	}
> +	if (&req->usb_req != _req) {
> +		spin_unlock_irqrestore(&ep->udc->lock, flags);
> +		return -EINVAL;
> +	}
> +
> +	xudc_done(ep, req, -ECONNRESET);
> +	spin_unlock_irqrestore(&ep->udc->lock, flags);
> +
> +	return 0;
> +}
> +
> +static struct usb_ep_ops xusb_ep_ops = {
> +	.enable = xudc_ep_enable,
> +	.disable = xudc_ep_disable,
> +
> +	.alloc_request = xudc_ep_alloc_request,
> +	.free_request = xudc_free_request,
> +
> +	.queue = xudc_ep_queue,
> +	.dequeue = xudc_ep_dequeue,
> +	.set_halt = xudc_ep_set_halt,
> +};
> +
> +/**
> + * xudc_get_frame - Reads the current usb frame number.
> + * @gadget: pointer to the usb gadget structure.
> + *
> + * Return: current frame number for success and error value on failure.
> + */
> +static int xudc_get_frame(struct usb_gadget *gadget)
> +{
> +
> +	struct xusb_udc *udc = to_udc(gadget);
> +	unsigned long flags;
> +	int retval;
> +
> +	if (!gadget)
> +		return -ENODEV;

oh boy... so you first deref gadget, then you check for it ?

> +	local_irq_save(flags);
> +	retval = udc->read_fn(udc->base_address + XUSB_FRAMENUM_OFFSET);
> +	local_irq_restore(flags);

yeah I'm not a big fan of anybody messing with local_irq_*.

> +	return retval;
> +}
> +
> +/**
> + * xudc_wakeup - Send remote wakeup signal to host
> + * @gadget: pointer to the usb gadget structure.
> + *
> + * Return: 0 on success and error on failure
> + */
> +
> +static int xudc_wakeup(struct usb_gadget *gadget)
> +{
> +	struct xusb_udc *udc = to_udc(gadget);
> +	u32             crtlreg;
> +	int             status = -EINVAL;
> +	unsigned long   flags;
> +
> +	spin_lock_irqsave(&udc->lock, flags);
> +
> +	/* Remote wake up not enabled by host */
> +	if (!udc->remote_wkp)
> +		goto done;
> +
> +	crtlreg = udc->read_fn(udc->base_address + XUSB_CONTROL_OFFSET);
> +	/* set remote wake up bit */
> +	udc->write_fn(udc->base_address, XUSB_CONTROL_OFFSET, crtlreg |
> +			XUSB_CONTROL_USB_RMTWAKE_MASK);
> +	/* wait for a while and reset remote wake up bit */
> +	mdelay(2);

why 2 ms ? why not 5 ? why not 1 ? shouldn't you be polling a bit in a
register or something ?

> +	udc->write_fn(udc->base_address, XUSB_CONTROL_OFFSET, crtlreg &
> +			~XUSB_CONTROL_USB_RMTWAKE_MASK);
> +	status = 0;
> +done:
> +	spin_unlock_irqrestore(&udc->lock, flags);
> +	return status;
> +}
> +
> +/**
> + * xudc_reinit - Restores inital software state.
> + * @udc: pointer to the usb device controller structure.
> + */
> +static void xudc_reinit(struct xusb_udc *udc)
> +{
> +	u32 ep_number;
> +	char name[4];
> +
> +	INIT_LIST_HEAD(&udc->gadget.ep_list);
> +
> +	for (ep_number = 0; ep_number < XUSB_MAX_ENDPOINTS; ep_number++) {
> +		struct xusb_ep *ep = &udc->ep[ep_number];
> +
> +		if (ep_number) {
> +			list_add_tail(&ep->ep_usb.ep_list,
> +					&udc->gadget.ep_list);
> +			ep->ep_usb.maxpacket = (unsigned short)~0;
> +			sprintf(name, "ep%d", ep_number);
> +			strcpy(ep->name, name);
> +			ep->ep_usb.name = ep->name;
> +		} else {
> +			ep->ep_usb.name = ep0name;
> +			ep->ep_usb.maxpacket = 0x40;
> +		}
> +
> +		ep->ep_usb.ops = &xusb_ep_ops;
> +		ep->udc = udc;
> +		ep->epnumber = ep_number;
> +		ep->desc = NULL;
> +		ep->stopped = 0;
> +		/*
> +		 * The configuration register address offset between
> +		 * each endpoint is 0x10.
> +		 */
> +		ep->offset = XUSB_EP0_CONFIG_OFFSET +
> +					(ep_number * 0x10);
> +		ep->is_in = 0;
> +		ep->is_iso = 0;
> +		ep->maxpacket = 0;
> +		xudc_epconfig(ep, udc);
> +
> +		/* Initialize one queue per endpoint */
> +		INIT_LIST_HEAD(&ep->queue);
> +	}
> +}
> +
> +/**
> + * xudc_stop_activity - Stops any further activity on the device.
> + * @udc: pointer to the usb device controller structure.
> + */
> +static void xudc_stop_activity(struct xusb_udc *udc)
> +{
> +	int i;
> +	struct xusb_ep *ep;
> +
> +	for (i = 0; i < XUSB_MAX_ENDPOINTS; i++) {
> +		ep = &udc->ep[i];
> +		xudc_nuke(ep, -ESHUTDOWN);
> +	}
> +}
> +
> +/**
> + * xudc_start - Starts the device.
> + * @gadget: pointer to the usb gadget structure
> + * @driver: pointer to gadget driver structure
> + *
> + * Return: zero always
> + */
> +static int xudc_start(struct usb_gadget *gadget,
> +			struct usb_gadget_driver *driver)
> +{
> +	struct xusb_udc *udc = to_udc(gadget);
> +	const struct usb_endpoint_descriptor *d = &config_bulk_out_desc;
> +	u32 crtlreg;
> +
> +	driver->driver.bus = NULL;
> +	/* hook up the driver */
> +	udc->driver = driver;
> +	udc->gadget.dev.driver = &driver->driver;
> +	udc->gadget.speed = driver->max_speed;
> +
> +	/* Enable the control endpoint. */
> +	xudc_ep_enable(&udc->ep[XUSB_EP_NUMBER_ZERO].ep_usb, d);
> +	/* Set Address to zero */
> +	udc->write_fn(udc->base_address, XUSB_ADDRESS_OFFSET, 0);
> +	/* Start device */
> +	crtlreg = udc->read_fn(udc->base_address + XUSB_CONTROL_OFFSET);
> +	crtlreg |= XUSB_CONTROL_USB_READY_MASK;
> +	udc->write_fn(udc->base_address, XUSB_CONTROL_OFFSET, crtlreg);
> +
> +	return 0;
> +}
> +
> +/**
> + * xudc_stop - stops the device.
> + * @gadget: pointer to the usb gadget structure
> + * @driver: pointer to usb gadget driver structure
> + *
> + * Return: zero always
> + */
> +static int xudc_stop(struct usb_gadget *gadget,
> +		struct usb_gadget_driver *driver)
> +{
> +	struct xusb_udc *udc = to_udc(gadget);
> +	unsigned long flags;
> +	u32 crtlreg;
> +
> +	/* Disable USB device.*/
> +	crtlreg = udc->read_fn(udc->base_address + XUSB_CONTROL_OFFSET);
> +	crtlreg &= ~XUSB_CONTROL_USB_READY_MASK;
> +	udc->write_fn(udc->base_address, XUSB_CONTROL_OFFSET, crtlreg);
> +	spin_lock_irqsave(&udc->lock, flags);
> +	udc->gadget.speed = USB_SPEED_UNKNOWN;
> +	xudc_stop_activity(udc);
> +	spin_unlock_irqrestore(&udc->lock, flags);
> +
> +	udc->gadget.dev.driver = NULL;
> +	udc->driver = NULL;
> +
> +	return 0;
> +}
> +
> +static const struct usb_gadget_ops xusb_udc_ops = {
> +	.get_frame = xudc_get_frame,
> +	.wakeup = xudc_wakeup,
> +	.udc_start = xudc_start,
> +	.udc_stop  = xudc_stop,

no pullup ??? What gives ? This HW doesn't support it ? really ?

> +static void xudc_startup_handler(struct xusb_udc *udc, u32 intrstatus)
> +{
> +	u32 intrreg;
> +
> +	if (intrstatus & XUSB_STATUS_RESET_MASK) {
> +
> +		DBG("Reset\n");
> +
> +		if (intrstatus & XUSB_STATUS_HIGH_SPEED_MASK)
> +			udc->gadget.speed = USB_SPEED_HIGH;
> +		else
> +			udc->gadget.speed = USB_SPEED_FULL;
> +
> +		/* Set device address and remote wakeup to 0 */
> +		udc->write_fn(udc->base_address, XUSB_ADDRESS_OFFSET, 0);
> +		udc->remote_wkp = 0;
> +
> +		/* Enable the suspend and resume */
> +		intrreg = udc->read_fn(udc->base_address + XUSB_IER_OFFSET);
> +		intrreg |= XUSB_STATUS_SUSPEND_MASK | XUSB_STATUS_RESUME_MASK;
> +		udc->write_fn(udc->base_address, XUSB_IER_OFFSET, intrreg);
> +
> +		xudc_stop_activity(udc);

you need to conditionally call gadget_driver->disconnect(), you also
need to make sure test modes are cleared, clear all stalls, etc.

> +	}
> +	if (intrstatus & XUSB_STATUS_SUSPEND_MASK) {
> +
> +		DBG("Suspend\n");
> +
> +		/* Enable the reset and resume */
> +		intrreg = udc->read_fn(udc->base_address + XUSB_IER_OFFSET);
> +		intrreg |= XUSB_STATUS_RESET_MASK | XUSB_STATUS_RESUME_MASK;
> +		udc->write_fn(udc->base_address, XUSB_IER_OFFSET, intrreg);
> +		udc->usb_state = USB_STATE_SUSPENDED;
> +
> +		if (udc->driver->suspend)
> +			udc->driver->suspend(&udc->gadget);
> +	}

when are you going to call driver->resume() ??

> +static void xudc_getstatus(struct xusb_udc *udc)
> +{
> +	u16 status = 0;
> +	u32 epcfgreg;
> +	struct xusb_ep *ep0;
> +	int epnum;
> +	struct xusb_ep *ep;
> +	u32 halt;
> +	u32 *corebuf;
> +
> +	ep0 = &udc->ep[XUSB_EP_NUMBER_ZERO];
> +	switch (udc->ch9cmd.setup.bRequestType & USB_RECIP_MASK) {
> +	case USB_RECIP_DEVICE:
> +		/* Get device status */
> +		status = 1 << USB_DEVICE_SELF_POWERED;
> +		if (udc->remote_wkp)
> +			status |= (1 << USB_DEVICE_REMOTE_WAKEUP);
> +		break;
> +	case USB_RECIP_INTERFACE:
> +		break;
> +	case USB_RECIP_ENDPOINT:
> +		epnum = udc->ch9cmd.setup.wIndex & USB_ENDPOINT_NUMBER_MASK;
> +		ep = &udc->ep[epnum];
> +		epcfgreg = udc->read_fn(udc->base_address +
> +				udc->ep[epnum].offset);
> +		halt = epcfgreg & XUSB_EP_CFG_STALL_MASK;
> +		if (udc->ch9cmd.setup.wIndex & USB_DIR_IN) {
> +			if (!ep->is_in)
> +				goto stall;
> +		} else {
> +			if (ep->is_in)
> +				goto stall;
> +		}
> +		if (halt)
> +			status = 1 << USB_ENDPOINT_HALT;
> +		break;
> +	default:
> +		goto stall;
> +	}
> +
> +	udc->ch9cmd.writecount = 0;
> +	corebuf = (void __force *) ((ep0->rambase << 2) + udc->base_address);
> +	status = cpu_to_le16(status);
> +	memcpy((void *)corebuf, (void *)&status, 2);
> +	udc->write_fn(udc->base_address, XUSB_EP_BUF0COUNT_OFFSET, 2);
> +	udc->write_fn(udc->base_address, XUSB_BUFFREADY_OFFSET, 1);
> +	return;
> +stall:
> +	dev_err(&udc->gadget.dev, "Can't respond to getstatus request\n");
> +	xudc_ep0_stall(udc);
> +	return;
> +}
> +
> +/**
> + * xudc_set_clear_feature - Executes the set feature and clear feature commands.
> + * @udc: pointer to the usb device controller structure.
> + *
> + * Processes the SET_FEATURE and CLEAR_FEATURE commands.
> + */
> +static void xudc_set_clear_feature(struct xusb_udc *udc)
> +{
> +	u8 endpoint;
> +	u8 outinbit;
> +	u32 epcfgreg;
> +	int flag = (udc->ch9cmd.setup.bRequest == USB_REQ_SET_FEATURE ? 1 : 0);
> +	switch (udc->ch9cmd.setup.bRequestType) {
> +	case USB_RECIP_DEVICE:
> +		switch (udc->ch9cmd.setup.wValue) {
> +		case USB_DEVICE_TEST_MODE:
> +			/*
> +			 * The Test Mode will be executed
> +			 * after the status phase.
> +			 */
> +			break;
> +		case USB_DEVICE_REMOTE_WAKEUP:
> +			if (flag)
> +				udc->remote_wkp = 1;
> +			else
> +				udc->remote_wkp = 0;
> +			break;
> +		default:
> +			xudc_ep0_stall(udc);
> +			break;
> +		}
> +		break;
> +	case USB_RECIP_ENDPOINT:
> +		if (!udc->ch9cmd.setup.wValue) {
> +			endpoint = udc->ch9cmd.setup.wIndex &
> +					USB_ENDPOINT_NUMBER_MASK;
> +			outinbit = udc->ch9cmd.setup.wIndex &
> +					USB_ENDPOINT_DIR_MASK;
> +			outinbit = outinbit >> 7;
> +
> +			/* Make sure direction matches.*/
> +			if (outinbit != udc->ep[endpoint].is_in) {
> +				xudc_ep0_stall(udc);
> +				return;
> +			}
> +			epcfgreg = udc->read_fn(udc->base_address +
> +						udc->ep[endpoint].
> +						offset);
> +			if (!endpoint) {
> +				/* Clear the stall.*/
> +				epcfgreg &= ~XUSB_EP_CFG_STALL_MASK;
> +				udc->write_fn(udc->base_address,
> +						udc->ep[endpoint].offset,
> +						epcfgreg);
> +			} else {
> +				if (flag) {
> +					epcfgreg |= XUSB_EP_CFG_STALL_MASK;
> +					udc->write_fn(udc->base_address,
> +							udc->ep[endpoint].
> +							offset, epcfgreg);
> +				} else {
> +					/* Unstall the endpoint.*/
> +					epcfgreg &= ~(XUSB_EP_CFG_STALL_MASK |
> +						  XUSB_EP_CFG_DATA_TOGGLE_MASK);
> +					udc->write_fn(udc->base_address,
> +							udc->ep[endpoint].
> +							offset, epcfgreg);
> +				}
> +			}
> +		}
> +		break;
> +	default:
> +		xudc_ep0_stall(udc);
> +		return;
> +	}
> +	/* Cause and valid status phase to be issued.*/
> +	xudc_wrstatus(udc);
> +	return;
> +}
> +
> +/**
> + * xudc_reset_ep0 - reset ep0 request
> + * @ep0: pointer to the endpoint structure.
> + *
> + * Resets endpoint 0 request.
> + */
> +static void xudc_reset_ep0(struct xusb_ep *ep0)
> +{
> +	ep0->no_queue = 0;
> +	xudc_nuke(ep0, -ECONNRESET);
> +}
> +
> +/**
> + * xudc_handle_setup - Processes the setup packet.
> + * @udc: pointer to the usb device controller structure.
> + * @setup: pointer to the usb control request structure.
> + *
> + * Process setup packet and delegate to gadget layer.
> + */
> +static void xudc_handle_setup(struct xusb_udc *udc,
> +				struct usb_ctrlrequest *setup)
> +{
> +	u32 *ep0rambase;
> +	struct xusb_ep *ep0 = &udc->ep[XUSB_EP_NUMBER_ZERO];
> +
> +	/* Load up the chapter 9 command buffer.*/
> +	ep0rambase = (u32 __force *) (udc->base_address +
> +				  XUSB_SETUP_PKT_ADDR_OFFSET);
> +	memcpy((void *)&udc->ch9cmd.setup, (void *)ep0rambase, 8);
> +
> +	setup->bRequestType = udc->ch9cmd.setup.bRequestType;
> +	setup->bRequest     = udc->ch9cmd.setup.bRequest;
> +	setup->wValue       = udc->ch9cmd.setup.wValue;
> +	setup->wIndex       = udc->ch9cmd.setup.wIndex;
> +	setup->wLength      = udc->ch9cmd.setup.wLength;
> +
> +	udc->ch9cmd.setup.wValue = cpu_to_le16(udc->ch9cmd.setup.wValue);
> +	udc->ch9cmd.setup.wIndex = cpu_to_le16(udc->ch9cmd.setup.wIndex);
> +	udc->ch9cmd.setup.wLength = cpu_to_le16(udc->ch9cmd.setup.wLength);
> +
> +	/* Restore ReadPtr to data buffer.*/
> +	udc->ch9cmd.readptr = &udc->ch9cmd.readbuff[0];
> +	udc->ch9cmd.readcount = 0;
> +
> +	if (udc->ch9cmd.setup.bRequestType & USB_DIR_IN) {
> +		/* Execute the get command.*/
> +		udc->ch9cmd.setupseqrx = STATUS_PHASE;
> +		udc->ch9cmd.setupseqtx = DATA_PHASE;
> +	} else {
> +		/* Execute the put command.*/
> +		udc->ch9cmd.setupseqrx = DATA_PHASE;
> +		udc->ch9cmd.setupseqtx = STATUS_PHASE;
> +	}
> +
> +	xudc_reset_ep0(ep0);
> +
> +	switch (udc->ch9cmd.setup.bRequest) {
> +	case USB_REQ_GET_STATUS:
> +		/* Data+Status phase form udc */
> +		if ((udc->ch9cmd.setup.bRequestType &
> +				(USB_DIR_IN | USB_TYPE_MASK)) !=
> +				(USB_DIR_IN | USB_TYPE_STANDARD))
> +			break;
> +		ep0->no_queue = 1;
> +		xudc_getstatus(udc);
> +		return;
> +	case USB_REQ_SET_ADDRESS:
> +		/* Status phase from udc */
> +		if (udc->ch9cmd.setup.bRequestType != (USB_DIR_OUT |
> +				USB_TYPE_STANDARD | USB_RECIP_DEVICE))
> +			break;
> +		ep0->no_queue = 1;
> +		xudc_wrstatus(udc);
> +		return;
> +	case USB_REQ_CLEAR_FEATURE:
> +	case USB_REQ_SET_FEATURE:
> +		/* Requests with no data phase, status phase from udc */
> +		if ((udc->ch9cmd.setup.bRequestType & USB_TYPE_MASK)
> +				!= USB_TYPE_STANDARD)
> +			break;
> +		ep0->no_queue = 1;
> +		xudc_set_clear_feature(udc);
> +		return;
> +	default:
> +		break;
> +	}
> +
> +	spin_unlock(&udc->lock);
> +	if (udc->driver->setup(&udc->gadget, setup) < 0)
> +		xudc_ep0_stall(udc);
> +	spin_lock(&udc->lock);
> +}
> +
> +/**
> + * xudc_ep0_out - Processes the endpoint 0 OUT token.
> + * @udc: pointer to the usb device controller structure.
> + */
> +static void xudc_ep0_out(struct xusb_udc *udc)
> +{
> +	struct xusb_ep *ep;
> +	u8 count;
> +	u8 *ep0rambase;
> +	u16 index;
> +
> +	ep = &udc->ep[0];
> +	switch (udc->ch9cmd.setupseqrx) {
> +	case STATUS_PHASE:
> +		/*
> +		 * This resets both state machines for the next
> +		 * Setup packet.
> +		 */
> +		udc->ch9cmd.setupseqrx = SETUP_PHASE;
> +		udc->ch9cmd.setupseqtx = SETUP_PHASE;
> +		if (!ep->no_queue) {
> +			ep->data->usb_req.actual = ep->data->usb_req.length;
> +			xudc_done(ep, ep->data, 0);
> +		}
> +		ep->no_queue = 0;
> +		break;
> +	case DATA_PHASE:
> +		count = udc->read_fn(udc->base_address +
> +				XUSB_EP_BUF0COUNT_OFFSET);
> +		/* Copy the data to be received from the DPRAM. */
> +		ep0rambase =
> +			(u8 __force *) (udc->base_address +
> +				(udc->ep[XUSB_EP_NUMBER_ZERO].rambase << 2));
> +
> +		for (index = 0; index < count; index++)
> +			*udc->ch9cmd.readptr++ = *ep0rambase++;
> +
> +		udc->ch9cmd.readcount += count;
> +		if (udc->ch9cmd.setup.wLength == udc->ch9cmd.readcount) {
> +			xudc_wrstatus(udc);
> +		} else {
> +			/* Set the Tx packet size and the Tx enable bit.*/
> +			udc->write_fn(udc->base_address,
> +					XUSB_EP_BUF0COUNT_OFFSET, 0);
> +			udc->write_fn(udc->base_address,
> +					XUSB_BUFFREADY_OFFSET, 1);
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +/**
> + * xudc_ep0_in - Processes the endpoint 0 IN token.
> + * @udc: pointer to the usb device controller structure.
> + */
> +static void xudc_ep0_in(struct xusb_udc *udc)
> +{
> +	struct xusb_ep *ep;
> +	u32 epcfgreg;
> +	u16 count;
> +	u16 length;
> +	u8 *ep0rambase;
> +
> +	ep = &udc->ep[0];
> +	switch (udc->ch9cmd.setupseqtx) {
> +	case STATUS_PHASE:
> +		switch (udc->ch9cmd.setup.bRequest) {
> +		case USB_REQ_SET_ADDRESS:
> +			/* Set the address of the device.*/
> +			udc->write_fn(udc->base_address,
> +					XUSB_ADDRESS_OFFSET,
> +					udc->ch9cmd.setup.wValue);
> +			break;
> +		case USB_REQ_SET_FEATURE:
> +			if (udc->ch9cmd.setup.bRequestType ==
> +					USB_RECIP_DEVICE) {
> +				if (udc->ch9cmd.setup.wValue ==
> +						USB_DEVICE_TEST_MODE)
> +					udc->write_fn(udc->base_address,
> +							XUSB_TESTMODE_OFFSET,
> +							TEST_J);
> +			}
> +			break;
> +		}
> +		if (!ep->no_queue) {
> +			ep->data->usb_req.actual = udc->ch9cmd.setup.wLength;
> +			xudc_done(ep, ep->data, 0);
> +		}
> +		ep->no_queue = 0;
> +		break;
> +	case DATA_PHASE:
> +		if (!udc->ch9cmd.writecount) {
> +			/*
> +			 * We're done with data transfer, next
> +			 * will be zero length OUT with data toggle of
> +			 * 1. Setup data_toggle.
> +			 */
> +			epcfgreg = udc->read_fn(udc->base_address +
> +				udc->ep[XUSB_EP_NUMBER_ZERO].offset);
> +			epcfgreg |= XUSB_EP_CFG_DATA_TOGGLE_MASK;
> +			udc->write_fn(udc->base_address,
> +				udc->ep[XUSB_EP_NUMBER_ZERO].offset, epcfgreg);
> +			count = 0;
> +			udc->ch9cmd.setupseqtx = STATUS_PHASE;
> +		} else {
> +			length = count = min_t(u32, udc->ch9cmd.writecount,
> +						EP0_MAX_PACKET);
> +			/* Copy the data to be transmitted into the DPRAM. */
> +			ep0rambase = (u8 __force *) (udc->base_address +
> +				(udc->ep[XUSB_EP_NUMBER_ZERO].rambase << 2));
> +			while (length--)
> +				*ep0rambase++ = *udc->ch9cmd.wrtptr++;
> +
> +			udc->ch9cmd.writecount -= count;
> +		}
> +		udc->write_fn(udc->base_address, XUSB_EP_BUF0COUNT_OFFSET,
> +				count);
> +		udc->write_fn(udc->base_address, XUSB_BUFFREADY_OFFSET, 1);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +/**
> + * xudc_ctrl_ep_handler - Endpoint 0 interrupt handler.
> + * @udc: pointer to the udc structure.
> + * @intrstatus:	It's the mask value for the interrupt sources on endpoint 0.
> + *
> + * Processes the commands received during enumeration phase.
> + */
> +static void xudc_ctrl_ep_handler(struct xusb_udc *udc, u32 intrstatus)
> +{
> +	struct usb_ctrlrequest ctrl;
> +
> +	if (intrstatus & XUSB_STATUS_SETUP_PACKET_MASK) {
> +		xudc_handle_setup(udc, &ctrl);
> +	} else {
> +		if (intrstatus & XUSB_STATUS_FIFO_BUFF_RDY_MASK)
> +			xudc_ep0_out(udc);
> +		else if (intrstatus & XUSB_STATUS_FIFO_BUFF_FREE_MASK)
> +			xudc_ep0_in(udc);
> +	}
> +}
> +
> +/**
> + * xudc_nonctrl_ep_handler - Non control endpoint interrupt handler.
> + * @udc: pointer to the udc structure.
> + * @epnum: End point number for which the interrupt is to be processed
> + * @intrstatus:	mask value for interrupt sources of endpoints other
> + *		than endpoint 0.
> + *
> + * Processes the buffer completion interrupts.
> + */
> +static void xudc_nonctrl_ep_handler(struct xusb_udc *udc, u8 epnum,
> +					u32 intrstatus)
> +{
> +
> +	struct xusb_req *req;
> +	struct xusb_ep *ep;
> +
> +	ep = &udc->ep[epnum];
> +	/* Process the End point interrupts.*/
> +	if (intrstatus & (XUSB_STATUS_EP0_BUFF1_COMP_MASK << epnum))
> +		ep->buffer0ready = 0;
> +	if (intrstatus & (XUSB_STATUS_EP0_BUFF2_COMP_MASK << epnum))
> +		ep->buffer1ready = 0;
> +
> +	if (list_empty(&ep->queue))
> +		req = NULL;
> +	else
> +		req = list_entry(ep->queue.next, struct xusb_req, queue);
> +	if (!req)
> +		return;
> +
> +	if (ep->is_in)
> +		xudc_write_fifo(ep, req);
> +	else
> +		xudc_read_fifo(ep, req);
> +}
> +
> +/**
> + * xudc_irq - The main interrupt handler.
> + * @irq: The interrupt number.
> + * @_udc: pointer to the usb device controller structure.
> + *
> + * Return: IRQ_HANDLED after the interrupt is handled.
> + */
> +static irqreturn_t xudc_irq(int irq, void *_udc)
> +{
> +	struct xusb_udc *udc = _udc;
> +	u32 intrstatus;
> +	u32 intrreg;
> +	u8 index;
> +	u32 bufintr;
> +
> +	spin_lock(&(udc->lock));
> +
> +	intrreg = udc->read_fn(udc->base_address + XUSB_IER_OFFSET);
> +	intrreg &= ~XUSB_STATUS_INTR_EVENT_MASK;
> +	udc->write_fn(udc->base_address, XUSB_IER_OFFSET, intrreg);
> +
> +	/* Read the Interrupt Status Register.*/
> +	intrstatus = udc->read_fn(udc->base_address + XUSB_STATUS_OFFSET);
> +
> +	if (udc->usb_state == USB_STATE_SUSPENDED) {
> +
> +		DBG("Resume\n");
> +
> +		if (intrstatus & XUSB_STATUS_RESUME_MASK) {
> +			/* Enable the reset and suspend */
> +			intrreg = udc->read_fn(udc->base_address +
> +						XUSB_IER_OFFSET);
> +			intrreg |= XUSB_STATUS_RESET_MASK |
> +					XUSB_STATUS_SUSPEND_MASK;
> +			udc->write_fn(udc->base_address, XUSB_IER_OFFSET,
> +					intrreg);
> +		}
> +		udc->usb_state = 0;
> +
> +		if (udc->driver->resume)
> +			udc->driver->resume(&udc->gadget);
> +	}
> +
> +	/* Call the handler for the event interrupt.*/
> +	if (intrstatus & XUSB_STATUS_INTR_EVENT_MASK) {
> +		/*
> +		 * Check if there is any action to be done for :
> +		 * - USB Reset received {XUSB_STATUS_RESET_MASK}
> +		 * - USB Suspend received {XUSB_STATUS_SUSPEND_MASK}
> +		 */
> +		xudc_startup_handler(udc, intrstatus);
> +	}
> +
> +	/* Check the buffer completion interrupts */
> +	if (intrstatus & XUSB_STATUS_INTR_BUFF_COMP_ALL_MASK) {
> +		/* Enable Reset and Suspend */
> +		intrreg = udc->read_fn(udc->base_address + XUSB_IER_OFFSET);
> +		intrreg |= XUSB_STATUS_SUSPEND_MASK | XUSB_STATUS_RESET_MASK;
> +		udc->write_fn(udc->base_address, XUSB_IER_OFFSET, intrreg);
> +
> +		if (intrstatus & XUSB_STATUS_EP0_BUFF1_COMP_MASK)
> +			xudc_ctrl_ep_handler(udc, intrstatus);
> +
> +		for (index = 1; index < 8; index++) {
> +			bufintr = ((intrstatus &
> +					(XUSB_STATUS_EP1_BUFF1_COMP_MASK <<
> +							(index - 1))) ||
> +				   (intrstatus &
> +					(XUSB_STATUS_EP1_BUFF2_COMP_MASK <<
> +							(index - 1))));
> +			if (bufintr)
> +				xudc_nonctrl_ep_handler(udc, index,
> +						intrstatus);
> +		}
> +	}
> +	spin_unlock(&(udc->lock));
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * xudc_probe - The device probe function for driver initialization.
> + * @pdev: pointer to the platform device structure.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int xudc_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct resource *res;
> +	struct xusb_udc *udc;
> +	int irq;
> +	int ret;
> +
> +	udc = devm_kzalloc(&pdev->dev, sizeof(*udc), GFP_KERNEL);
> +	if (!udc)
> +		return -ENOMEM;
> +
> +	/* Map the registers */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	udc->base_address = devm_ioremap_resource(&pdev->dev, res);
> +	if (!udc->base_address)
> +		return -ENOMEM;
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "unable to get irq\n");
> +		return irq;
> +	}
> +	ret = devm_request_irq(&pdev->dev, irq, xudc_irq, 0,
> +				dev_name(&pdev->dev), udc);
> +	if (ret < 0) {
> +		dev_dbg(&pdev->dev, "unable to request irq %d", irq);
> +		goto fail;
> +	}
> +
> +	udc->dma_enabled = of_property_read_bool(np, "xlnx,has-builtin-dma");
> +
> +	/* Setup gadget structure */
> +	udc->gadget.ops = &xusb_udc_ops;
> +	udc->gadget.max_speed = USB_SPEED_HIGH;
> +	udc->gadget.speed = USB_SPEED_UNKNOWN;
> +	udc->gadget.ep0 = &udc->ep[XUSB_EP_NUMBER_ZERO].ep_usb;
> +	udc->gadget.name = driver_name;
> +
> +	dev_set_name(&udc->gadget.dev, "xilinx_udc");

this line isn't necessary.

> +	spin_lock_init(&udc->lock);
> +
> +	/* Check for IP endianness */
> +	udc->write_fn = xudc_write32_be;
> +	udc->read_fn = xudc_read32_be;
> +	udc->write_fn(udc->base_address, XUSB_TESTMODE_OFFSET, TEST_J);
> +	if ((udc->read_fn(udc->base_address + XUSB_TESTMODE_OFFSET))
> +			!= TEST_J) {
> +		udc->write_fn = xudc_write32;
> +		udc->read_fn = xudc_read32;
> +	}

hmm... isn't there a configuration register to check this out ?

> +	udc->write_fn(udc->base_address, XUSB_TESTMODE_OFFSET, 0);
> +
> +	xudc_reinit(udc);
> +
> +	/* Set device address to 0.*/
> +	udc->write_fn(udc->base_address, XUSB_ADDRESS_OFFSET, 0);
> +
> +	ret = usb_add_gadget_udc(&pdev->dev, &udc->gadget);
> +	if (ret)
> +		goto fail;
> +
> +	/* Enable the interrupts.*/
> +	udc->write_fn(udc->base_address, XUSB_IER_OFFSET,
> +			XUSB_STATUS_GLOBAL_INTR_MASK | XUSB_STATUS_RESET_MASK |
> +			XUSB_STATUS_SUSPEND_MASK |
> +			XUSB_STATUS_RESUME_MASK |

you're enabling resume IRQ but never handling it.

> +			XUSB_STATUS_FIFO_BUFF_RDY_MASK |
> +			XUSB_STATUS_FIFO_BUFF_FREE_MASK |
> +			XUSB_STATUS_EP0_BUFF1_COMP_MASK);
> +
> +	platform_set_drvdata(pdev, udc);
> +
> +	VDBG("%s #%d at 0x%08X mapped to 0x%08X\n", driver_name, 0,
> +		(u32)res->start, (u32 __force)udc->base_address);
> +
> +	return 0;
> +
> +fail:
> +	dev_err(&pdev->dev, "probe failed, %d\n", ret);
> +	return ret;
> +}
> +
> +/**
> + * xudc_remove - Releases the resources allocated during the initialization.
> + * @pdev: pointer to the platform device structure.
> + *
> + * Return: 0 always
> + */
> +static int xudc_remove(struct platform_device *pdev)
> +{
> +	struct xusb_udc *udc = platform_get_drvdata(pdev);
> +
> +	dev_dbg(&pdev->dev, "remove\n");

this is *really* unnecessary.


-- 
balbi

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