* [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support [not found] <1396510519-8555-1-git-send-email-sbhatta@xilinx.com> @ 2014-04-03 7:35 ` Subbaraya Sundeep Bhatta 2014-04-03 14:58 ` Felipe Balbi 0 siblings, 1 reply; 25+ messages in thread From: Subbaraya Sundeep Bhatta @ 2014-04-03 7:35 UTC (permalink / raw) To: Felipe Balbi Cc: Greg Kroah-Hartman, michals, linux-usb, linux-kernel, devicetree, Subbaraya Sundeep Bhatta This patch adds xilinx axi usb2 device driver support Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com> --- 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) + +/*-------------------------------------------------------------------------*/ + +/** + * struct xusb_req - Xilinx USB device request structure + * @usb_req: Linux usb request structure + * @queue: usb device request queue + * @ep: pointer to xusb_endpoint structure + */ +struct xusb_req { + struct usb_request usb_req; + struct list_head queue; + struct xusb_ep *ep; +}; + +/** + * struct xusb_ep - USB end point structure. + * @ep_usb: usb endpoint instance + * @queue: endpoint message queue + * @udc: xilinx usb peripheral driver instance pointer + * @desc: pointer to the usb endpoint descriptor + * @data: pointer to the xusb_req structure + * @rambase: the endpoint buffer address + * @offset: the endpoint register offset value + * @name: name of the endpoint + * @epnumber: endpoint number + * @maxpacket: maximum packet size the endpoint can store + * @buffer0count: the size of the packet recieved in the first buffer + * @buffer0ready: the busy state of first buffer + * @buffer1count: the size of the packet received in the second buffer + * @buffer1ready: the busy state of second buffer + * @eptype: endpoint transfer type (BULK, INTERRUPT) + * @curbufnum: current buffer of endpoint that will be processed next + * @is_in: endpoint direction (IN or OUT) + * @stopped: endpoint active status + * @is_iso: endpoint type(isochronous or non isochronous) + * @no_queue: EP0 flag to call completion or not + */ +struct xusb_ep { + struct usb_ep ep_usb; + struct list_head queue; + struct xusb_udc *udc; + const struct usb_endpoint_descriptor *desc; + struct xusb_req *data; + u32 rambase; + u32 offset; + char name[4]; + u16 epnumber; + u16 maxpacket; + u16 buffer0count; + u16 buffer1count; + u8 buffer0ready; + u8 buffer1ready; + u8 eptype; + u8 curbufnum; + u8 is_in; + u8 stopped; + u8 is_iso; + u8 no_queue; +}; + +/** + * struct cmdbuf - Standard USB Command Buffer Structure defined + * @setup: usb_ctrlrequest structure for control requests + * @readcount: read data bytes count + * @writecount: write data bytes count + * @setupseqtx: tx status + * @setupseqrx: rx status + * @readptr: pointer to endpoint0 read data + * @wrtptr: pointer to endpoint0 write data + * @readbuff: read data buffer for endpoint0 + */ +struct cmdbuf { + struct usb_ctrlrequest setup; + u32 readcount; + u32 writecount; + u32 setupseqtx; + u32 setupseqrx; + u8 *readptr; + u8 *wrtptr; + u8 readbuff[64]; +}; + +/** + * struct xusb_udc - USB peripheral driver structure + * @gadget: USB gadget driver instance + * @ep: an array of endpoint structures + * @driver: pointer to the usb gadget driver instance + * @ch9cmd: command buffer to process chapter9 commands + * @usb_state: device in suspended state or not + * @remote_wkp: remote wakeup enabled by host + * @read_fn: function pointer to read device registers + * @write_fn: function pointer to write to device registers + * @base_address: the usb device base address + * @lock: instance of spinlock + * @dma_enabled: flag indicating whether the dma is included in the system + */ +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); + 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 = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + .bEndpointAddress = USB_DIR_OUT, + .bmAttributes = USB_ENDPOINT_XFER_BULK, + .wMaxPacketSize = __constant_cpu_to_le16(0x40), +}; + +/** + * xudc_write32 - little endian write to device registers + * @addr: base addr of device registers + * @offset: register offset + * @val: data to be written + */ +static void xudc_write32(void __iomem *addr, u32 offset, u32 val) +{ + iowrite32(val, addr + offset); +} + +/** + * xudc_read32 - little endian read from device registers + * @addr: addr of device register + * Return: value at addr + */ +static unsigned int xudc_read32(void __iomem *addr) +{ + return ioread32(addr); +} + +/** + * xudc_write32_be - big endian write to device registers + * @addr: base addr of device registers + * @offset: register offset + * @val: data to be written + */ +static void xudc_write32_be(void __iomem *addr, u32 offset, u32 val) +{ + iowrite32be(val, addr + offset); +} + +/** + * xudc_read32_be - big endian read from device registers + * @addr: addr of device register + * Return: value at addr + */ +static unsigned int xudc_read32_be(void __iomem *addr) +{ + return ioread32be(addr); +} + +/** + * 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; + 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); +} + +/** + * xudc_epconfig - Configures the given endpoint. + * @ep: pointer to the usb device endpoint structure. + * @udc: pointer to the usb peripheral controller structure. + * + * This function configures a specific endpoint with the given configuration + * data. + */ +static void xudc_epconfig(struct xusb_ep *ep, struct xusb_udc *udc) +{ + u32 epcfgreg; + + /* + * Configure the end point direction, type, Max Packet Size and the + * EP buffer location. + */ + epcfgreg = ((ep->is_in << 29) | (ep->eptype << 28) | + (ep->ep_usb.maxpacket << 15) | (ep->rambase)); + udc->write_fn(udc->base_address, ep->offset, epcfgreg); + + /* Set the Buffer count and the Buffer ready bits.*/ + udc->write_fn(udc->base_address, ep->offset + XUSB_EP_BUF0COUNT_OFFSET, + ep->buffer0count); + udc->write_fn(udc->base_address, ep->offset + XUSB_EP_BUF1COUNT_OFFSET, + ep->buffer1count); + if (ep->buffer0ready) + udc->write_fn(udc->base_address, XUSB_BUFFREADY_OFFSET, + 1 << ep->epnumber); + if (ep->buffer1ready) + udc->write_fn(udc->base_address, XUSB_BUFFREADY_OFFSET, 1 << + (ep->epnumber + XUSB_STATUS_EP_BUFF2_SHIFT)); +} + +/** + * start_dma - Starts DMA transfer. + * @ep: pointer to the usb device endpoint structure. + * @src: DMA source address. + * @dst: DMA destination address. + * @length: number of bytes to transfer. + * + * Return: 0 on success and error on failure + * + * This function starts DMA transfer by writing to DMA source, + * destination and lenth registers. + */ +static int start_dma(struct xusb_ep *ep, u32 src, u32 dst, u32 length) +{ + 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; + } + } + 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); + } + return rc; +} + +/** + * dma_send - Sends IN data using DMA. + * @ep: pointer to the usb device endpoint structure. + * @buffer: pointer to data to be sent. + * @length: number of bytes to send. + * + * Return: 0 on success, 1 if no buffer is free and error on failure. + * + * This function sends data using DMA. + */ +static int dma_send(struct xusb_ep *ep, u8 *buffer, u32 length) +{ + 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; + } + + if (!ep->curbufnum && !ep->buffer0ready) { + /* Get the Buffer address and copy the transmit data.*/ + eprambase = (u32 __force *)(ep->udc->base_address + + ep->rambase); + dst = virt_to_phys(eprambase); + udc->write_fn(udc->base_address, ep->offset + + XUSB_EP_BUF0COUNT_OFFSET, length); + udc->write_fn(udc->base_address, XUSB_DMA_CONTROL_OFFSET, + XUSB_DMA_BRR_CTRL | (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); + dst = virt_to_phys(eprambase); + udc->write_fn(ep->udc->base_address, ep->offset + + XUSB_EP_BUF1COUNT_OFFSET, length); + udc->write_fn(udc->base_address, XUSB_DMA_CONTROL_OFFSET, + XUSB_DMA_BRR_CTRL | (1 << (ep->epnumber + + XUSB_STATUS_EP_BUFF2_SHIFT))); + ep->buffer1ready = 1; + ep->curbufnum = 0; + } else { + /* None of ping pong buffers are ready currently .*/ + return 1; + } + ret = start_dma(ep, src, dst, length); + return ret; +} + +/** + * dma_receive - Receives OUT data using DMA. + * @ep: pointer to the usb device endpoint structure. + * @buffer: pointer to storage buffer of received data. + * @length: number of bytes to receive. + * + * Return: 0 on success, 1 if no buffer is free and error on failure. + * + * This function receives data using DMA. + */ +static int dma_receive(struct xusb_ep *ep, u8 *buffer, u32 length) +{ + 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; + } + + 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; + } + ret = start_dma(ep, src, dst, length); + return ret; +} + +/** + * xudc_eptxrx - Transmits or receives data to or from an endpoint. + * @ep: pointer to the usb endpoint configuration structure. + * @bufferptr: pointer to buffer containing the data to be sent. + * @bufferlen: The number of data bytes to be sent. + * + * Return: 0 on success and 1 on failure + * + * This function copies the transmit/receive data to/from the end point buffer + * and enables the buffer for transmission/reception. + */ +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); + 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 + * + * Pulls OUT packet data from the endpoint buffer. + */ +static int xudc_read_fifo(struct xusb_ep *ep, struct xusb_req *req) +{ + u8 *buf; + u32 is_short, count, bufferspace; + 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; + } + } + } + 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; + local_irq_save(flags); + retval = udc->read_fn(udc->base_address + XUSB_FRAMENUM_OFFSET); + local_irq_restore(flags); + 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); + + 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, +}; + +/** + * xudc_startup_handler - The usb device controller interrupt handler. + * @udc: pointer to the udc structure. + * @intrstatus: The mask value containing the interrupt sources. + * + * This handler handles the RESET and SUSPEND interrupts. + */ +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); + } + 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); + } +} + +/** + * xudc_ep0_stall - Stall endpoint zero. + * @udc: pointer to the udc structure. + * + * This function stalls endpoint zero. + */ +static void xudc_ep0_stall(struct xusb_udc *udc) +{ + u32 epcfgreg; + + epcfgreg = udc->read_fn(udc->base_address + + udc->ep[XUSB_EP_NUMBER_ZERO].offset); + epcfgreg |= XUSB_EP_CFG_STALL_MASK; + udc->write_fn(udc->base_address, udc->ep[XUSB_EP_NUMBER_ZERO].offset, + epcfgreg); +} + +/** + * xudc_getstatus - executes GET_STATUS command + * @udc: pointer to the udc structure. + * + * This function executes USB GET_STATUS command + */ +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"); + + 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; + } + 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 | + 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"); + usb_del_gadget_udc(&udc->gadget); + + return 0; +} + +/* Match table for of_platform binding */ +static const struct of_device_id usb_of_match[] = { + { .compatible = "xlnx,xps-usb2-device-4.00.a", }, + { /* end of list */ }, +}; +MODULE_DEVICE_TABLE(of, usb_of_match); + +static struct platform_driver xudc_driver = { + .driver = { + .name = driver_name, + .owner = THIS_MODULE, + .of_match_table = usb_of_match, + }, + .probe = xudc_probe, + .remove = xudc_remove, +}; + +module_platform_driver(xudc_driver); + +MODULE_DESCRIPTION("Xilinx udc driver"); +MODULE_AUTHOR("Xilinx, Inc"); +MODULE_LICENSE("GPL"); -- 1.7.4 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-03 7:35 ` [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support Subbaraya Sundeep Bhatta @ 2014-04-03 14:58 ` Felipe Balbi 2014-04-03 15:23 ` Michal Simek ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Felipe Balbi @ 2014-04-03 14:58 UTC (permalink / raw) To: Subbaraya Sundeep Bhatta Cc: Felipe Balbi, Greg Kroah-Hartman, michals, linux-usb, linux-kernel, devicetree, Subbaraya Sundeep Bhatta [-- Attachment #1: Type: text/plain, Size: 59048 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@xilinx.com> > --- > 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 --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-03 14:58 ` Felipe Balbi @ 2014-04-03 15:23 ` Michal Simek 2014-04-07 9:01 ` sundeep subbaraya [not found] ` <CALHRZupk2ywMNKjzAtCPgg=9-KsDqrVBAKFNoHOoSJOQ4FDJkA@mail.gmail.com> 2014-04-16 10:39 ` sundeep subbaraya 2 siblings, 1 reply; 25+ messages in thread From: Michal Simek @ 2014-04-03 15:23 UTC (permalink / raw) To: balbi Cc: Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, linux-usb, linux-kernel, devicetree, Subbaraya Sundeep Bhatta [-- Attachment #1: Type: text/plain, Size: 1803 bytes --] >> +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. readl from asm-generic/io.h is converting value from little endian IO to cpu endianness. This IP exists also in big endian version. It means we have to support all possible combinations. IP little and reading it on little or big endian CPU IP big and reading it on little or big endian CPU. Look below. >> + 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 ? Sundeep can tell us if there is any configuration register but I don't think so in connection to my experience with Xilinx soft IPs. Endian detection directly on IP itself came from my discussion on drivers/spi/spi-xilinx.c that this is only one way how to do it. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 263 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-03 15:23 ` Michal Simek @ 2014-04-07 9:01 ` sundeep subbaraya 0 siblings, 0 replies; 25+ messages in thread From: sundeep subbaraya @ 2014-04-07 9:01 UTC (permalink / raw) To: monstr Cc: balbi, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta Hi Michal, On Thu, Apr 3, 2014 at 8:53 PM, Michal Simek <monstr@monstr.eu> wrote: >>> +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. > > readl from asm-generic/io.h is converting value from little endian IO > to cpu endianness. > This IP exists also in big endian version. > It means we have to support all possible combinations. > IP little and reading it on little or big endian CPU > IP big and reading it on little or big endian CPU. > > Look below. > >>> + 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 ? > > Sundeep can tell us if there is any configuration register but > I don't think so in connection to my experience with Xilinx soft IPs. > Yes, there is no configuration register for endianess. Thanks, Sundeep. > Endian detection directly on IP itself came from my discussion > on drivers/spi/spi-xilinx.c that this is only one way how to do it. > > Thanks, > Michal > > -- > Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 > w: www.monstr.eu p: +42-0-721842854 > Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ > Maintainer of Linux kernel - Xilinx Zynq ARM architecture > Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform > > ^ permalink raw reply [flat|nested] 25+ messages in thread
[parent not found: <CALHRZupk2ywMNKjzAtCPgg=9-KsDqrVBAKFNoHOoSJOQ4FDJkA@mail.gmail.com>]
* Fwd: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support [not found] ` <CALHRZupk2ywMNKjzAtCPgg=9-KsDqrVBAKFNoHOoSJOQ4FDJkA@mail.gmail.com> @ 2014-04-07 9:06 ` sundeep subbaraya 2014-04-07 16:35 ` Felipe Balbi 0 siblings, 1 reply; 25+ messages in thread From: sundeep subbaraya @ 2014-04-07 9:06 UTC (permalink / raw) To: balbi Cc: Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta Hi Felipe, On Thu, Apr 3, 2014 at 8:28 PM, Felipe Balbi <balbi@ti.com> wrote: > 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@xilinx.com> >> --- >> 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}() Sure. i will revert earlier code for debug messages using dev_ >> +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. Ok. > >> + .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. Ok > >> +/** >> + * 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. This is used for making DATA1 packet for status stage during control transfers. IP has internally a weak check for alternating between DATA0 and DATA1 packets using this bit. Firmware can set this bit to send a DATA1 packet. As we always need to send DATA1 for status stage, we force IP to send DATA1 packet whatever maybe in this bit because of alternating behavior. Is this is confusing for the name *_TOGGLE_MASK ? > >> + 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: Ok . > > 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. Ok > >> +{ >> + 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() ok will use DMA irqs. > >> + 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(). Ok > >> +static int dma_send(struct xusb_ep *ep, u8 *buffer, u32 length) > > prepend with xudc_ Ok. > >> +{ >> + 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. Ok. > >> +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 Ok > >> + >> + 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. Ok. > >> +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!! Ok > >> +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. Ok. > >> + 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. Ok > >> + 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 ? Yes i too had this doubt after looking at some of the functions (like ep_queue) of other controller drivers. I tested sending a NULL to container_of macro I see no NULL exception. Hence using container_of macro first and then checking for NULL input is fine. If you insist this i need to change code at other places too. > >> + 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_*. Ok > >> + 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 ? IP datasheet says writing Remote wake bit to this register will send remote wake up signalling to host immediately and this bit will not be cleared by hardware, firmware has to clear it. There is no bit for polling. > >> + 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 ? Yes, there is no pull up. writing to control register to start udc is sufficient for this IP. > >> +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. Ok . before udc_stop is called gadget layer calls disconnect. I will add driver->disconnect when device is disconnected from host. > >> + } >> + 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() ?? When an interrupt occurs we first check if udc->usb_state = USB_STATE_SUSPENDED, if yes driver->resume is called. Also if Resume bit is set then it is cleared too. Resume status bit is set when device is resumed by host after device sends Remote wakeup signalling to host. > >> +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); Here. calling driver->resume. >> + } >> + >> + /* 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. Ok > >> + 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 ? No. There is no config register for checking endainess. > >> + 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. it is handled in interrupt handler. Please take a look at xudc_irq. > >> + 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. Ok. will remove. Thanks, Sundeep.B.S. > > > -- > balbi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Fwd: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-07 9:06 ` Fwd: " sundeep subbaraya @ 2014-04-07 16:35 ` Felipe Balbi 2014-04-08 16:01 ` sundeep subbaraya 0 siblings, 1 reply; 25+ messages in thread From: Felipe Balbi @ 2014-04-07 16:35 UTC (permalink / raw) To: sundeep subbaraya Cc: balbi, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta [-- Attachment #1: Type: text/plain, Size: 8293 bytes --] Hi, On Mon, Apr 07, 2014 at 02:36:13PM +0530, sundeep subbaraya wrote: > >> +/** > >> + * 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. > > This is used for making DATA1 packet for status stage during control transfers. > IP has internally a weak check for alternating between DATA0 and DATA1 > packets using > this bit. Firmware can set this bit to send a DATA1 packet. As we > always need to send DATA1 > for status stage, we force IP to send DATA1 packet whatever maybe in this > bit because of alternating behavior. Is this is confusing for the name > *_TOGGLE_MASK ? yeah, I guess it was the suffix _MASK, nevermind then ;-) > >> +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 ? > > Yes i too had this doubt after looking at some of the functions (like > ep_queue) of other controller drivers. if there are other gadget drivers doing this, they're wrong and need to be fixed. > I tested sending a NULL to container_of macro I see no NULL exception. yeah, container_of() will *always* return a valid pointer, even if it's argument is NULL. > Hence using container_of > macro first and then checking for NULL input is fine. If you insist no, it's not. You'd waste cpu cycles with pointer arithmetic for no reason. > this i need to change code at other > places too. yes. > >> +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 ? > > IP datasheet says writing Remote wake bit to this register will send > remote wake up > signalling to host immediately and this bit will not be cleared by > hardware, firmware has > to clear it. There is no bit for polling. then you need a better comment stating this detail. > >> +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 ? > > Yes, there is no pull up. writing to control register to start udc is > sufficient for this IP. right, but is there a bit inside control register which actually starts the IP ? You might want to move that to ->pullup(), see how we did on drivers/usb/dwc3/gadget.c::dwc3_gadget_pullup(); we're basically using that to control RUN/STOP bit in DCTL register. That bit has two functions: a) actually enable the ip; b) connect data pullups. You can actually see with a scope that the line goes high and low when you mess with that bit. The reason I suggest this is because you don't want to let your host see a connection until your gadget driver is registered and ready to go and that's what the pullup method would guarantee. > >> + } > >> + 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() ?? > > When an interrupt occurs we first check if udc->usb_state = > USB_STATE_SUSPENDED, > if yes driver->resume is called. Also if Resume bit is set then it is > cleared too. Resume status bit is set > when device is resumed by host after device sends Remote wakeup > signalling to host. <snip> > >> +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); this is wrong, note that you would call ->resume() *every time* usb_state == SUSPENDED and there's an interrupt. This means that if gadget is suspended and you remove the cable, then you first call ->resume() and then ->disconnect(). > Here. calling driver->resume. Here's what I would do: if (intrstatus & XUSB_STATUS_RESUME_MASK) { bool condition = udc->usb_state != USB_STATE_SUSPENDED; dev_WARN_ONCE(dev, condition, "Resume IRQ while not suspended\n"); /* 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); if (udc->driver->resume) udc->driver_resume(&udc->gadget); } > >> + 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. > > it is handled in interrupt handler. Please take a look at xudc_irq. it's mishandled. -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Fwd: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-07 16:35 ` Felipe Balbi @ 2014-04-08 16:01 ` sundeep subbaraya 2014-04-08 21:24 ` Felipe Balbi 0 siblings, 1 reply; 25+ messages in thread From: sundeep subbaraya @ 2014-04-08 16:01 UTC (permalink / raw) To: balbi Cc: Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta On Mon, Apr 7, 2014 at 10:05 PM, Felipe Balbi <balbi@ti.com> wrote: > Hi, > > On Mon, Apr 07, 2014 at 02:36:13PM +0530, sundeep subbaraya wrote: >> >> +/** >> >> + * 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. >> >> This is used for making DATA1 packet for status stage during control transfers. >> IP has internally a weak check for alternating between DATA0 and DATA1 >> packets using >> this bit. Firmware can set this bit to send a DATA1 packet. As we >> always need to send DATA1 >> for status stage, we force IP to send DATA1 packet whatever maybe in this >> bit because of alternating behavior. Is this is confusing for the name >> *_TOGGLE_MASK ? > > yeah, I guess it was the suffix _MASK, nevermind then ;-) OK :) > >> >> +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 ? >> >> Yes i too had this doubt after looking at some of the functions (like >> ep_queue) of other controller drivers. > > if there are other gadget drivers doing this, they're wrong and need to > be fixed. > >> I tested sending a NULL to container_of macro I see no NULL exception. > > yeah, container_of() will *always* return a valid pointer, even if it's > argument is NULL. > >> Hence using container_of >> macro first and then checking for NULL input is fine. If you insist > > no, it's not. You'd waste cpu cycles with pointer arithmetic for no > reason. > >> this i need to change code at other >> places too. > > yes. ok will modify > >> >> +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 ? >> >> IP datasheet says writing Remote wake bit to this register will send >> remote wake up >> signalling to host immediately and this bit will not be cleared by >> hardware, firmware has >> to clear it. There is no bit for polling. > > then you need a better comment stating this detail. sure > >> >> +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 ? >> >> Yes, there is no pull up. writing to control register to start udc is >> sufficient for this IP. > > right, but is there a bit inside control register which actually starts > the IP ? You might want to move that to ->pullup(), see how we did on > drivers/usb/dwc3/gadget.c::dwc3_gadget_pullup(); we're basically using > that to control RUN/STOP bit in DCTL register. That bit has two > functions: a) actually enable the ip; b) connect data pullups. > > You can actually see with a scope that the line goes high and low when > you mess with that bit. > > The reason I suggest this is because you don't want to let your host see > a connection until your gadget driver is registered and ready to go and > that's what the pullup method would guarantee. No.There are only two bits in Control register, one for Ready bit, another for sending remote wakeup and remaining are reserved as zeroes. Until ready is set host do not see the gadget. > >> >> + } >> >> + 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() ?? >> >> When an interrupt occurs we first check if udc->usb_state = >> USB_STATE_SUSPENDED, >> if yes driver->resume is called. Also if Resume bit is set then it is >> cleared too. Resume status bit is set >> when device is resumed by host after device sends Remote wakeup >> signalling to host. > > <snip> > >> >> +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); > > this is wrong, note that you would call ->resume() *every time* > usb_state == SUSPENDED and there's an interrupt. This means that if > gadget is suspended and you remove the cable, then you first call > ->resume() and then ->disconnect(). > >> Here. calling driver->resume. > > Here's what I would do: > > if (intrstatus & XUSB_STATUS_RESUME_MASK) { > bool condition = udc->usb_state != USB_STATE_SUSPENDED; > dev_WARN_ONCE(dev, condition, "Resume IRQ while not suspended\n"); > > /* 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); > > if (udc->driver->resume) > udc->driver_resume(&udc->gadget); > } Resume Interrupt bit is set only when Resume happens by device sending Remote wakeup. I am assuming we need to call driver->resume for every driver->suspend. Hence I implemented like this. Please suggest me what needs to be done for this case. > > >> >> + 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. >> >> it is handled in interrupt handler. Please take a look at xudc_irq. > > it's mishandled. > > -- > balbi Thanks for the comments and I will work on next iteration. Sundeep.B.S. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Fwd: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-08 16:01 ` sundeep subbaraya @ 2014-04-08 21:24 ` Felipe Balbi 0 siblings, 0 replies; 25+ messages in thread From: Felipe Balbi @ 2014-04-08 21:24 UTC (permalink / raw) To: sundeep subbaraya Cc: balbi, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta [-- Attachment #1: Type: text/plain, Size: 5679 bytes --] Hi, On Tue, Apr 08, 2014 at 09:31:29PM +0530, sundeep subbaraya wrote: > >> >> +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 ? > >> > >> Yes, there is no pull up. writing to control register to start udc is > >> sufficient for this IP. > > > > right, but is there a bit inside control register which actually starts > > the IP ? You might want to move that to ->pullup(), see how we did on > > drivers/usb/dwc3/gadget.c::dwc3_gadget_pullup(); we're basically using > > that to control RUN/STOP bit in DCTL register. That bit has two > > functions: a) actually enable the ip; b) connect data pullups. > > > > You can actually see with a scope that the line goes high and low when > > you mess with that bit. > > > > The reason I suggest this is because you don't want to let your host see > > a connection until your gadget driver is registered and ready to go and > > that's what the pullup method would guarantee. > > No.There are only two bits in Control register, one for Ready bit, another for > sending remote wakeup and remaining are reserved as zeroes. Until ready is > set host do not see the gadget. so this READY bit is what you want to toggle on pullup(). > >> >> + } > >> >> + 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() ?? > >> > >> When an interrupt occurs we first check if udc->usb_state = > >> USB_STATE_SUSPENDED, > >> if yes driver->resume is called. Also if Resume bit is set then it is > >> cleared too. Resume status bit is set > >> when device is resumed by host after device sends Remote wakeup > >> signalling to host. > > > > <snip> > > > >> >> +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); > > > > this is wrong, note that you would call ->resume() *every time* > > usb_state == SUSPENDED and there's an interrupt. This means that if > > gadget is suspended and you remove the cable, then you first call > > ->resume() and then ->disconnect(). > > > >> Here. calling driver->resume. > > > > Here's what I would do: > > > > if (intrstatus & XUSB_STATUS_RESUME_MASK) { > > bool condition = udc->usb_state != USB_STATE_SUSPENDED; > > dev_WARN_ONCE(dev, condition, "Resume IRQ while not suspended\n"); > > > > /* 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); > > > > if (udc->driver->resume) > > udc->driver_resume(&udc->gadget); > > } > > Resume Interrupt bit is set only when Resume happens by device sending > Remote wakeup. so if the host drives resume signalling there will be no interrupt ? Well, this is wrong :-) The gadget driver needs to know about that too. The host side can decide to wake you up. > I am assuming we need to call driver->resume for every > driver->suspend. Hence I implemented no you don't, you need to let host wake you up or you drive remote wakeup on the bus. The thing is: if the bus is suspended, there will be *no* activity on the bus prior to host driving Resume signal or device driving Remote Wakeup signal. Enable RESUME IRQ on probe and never disable it, it'll be a lot easier to implement it. -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-03 14:58 ` Felipe Balbi 2014-04-03 15:23 ` Michal Simek [not found] ` <CALHRZupk2ywMNKjzAtCPgg=9-KsDqrVBAKFNoHOoSJOQ4FDJkA@mail.gmail.com> @ 2014-04-16 10:39 ` sundeep subbaraya 2014-04-16 18:02 ` Felipe Balbi 2014-04-16 20:08 ` Paul Zimmerman 2 siblings, 2 replies; 25+ messages in thread From: sundeep subbaraya @ 2014-04-16 10:39 UTC (permalink / raw) To: balbi Cc: Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta Hi Felipe, On Thu, Apr 3, 2014 at 8:28 PM, Felipe Balbi <balbi@ti.com> wrote: >> +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() This function is called in interrupt context when buffer is ready or free. It initiates DMA to transfer data from IP buffer to memory. Hence it waits in busy loop till DMA completes Thanks, Sundeep.B.S. > -- > balbi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-16 10:39 ` sundeep subbaraya @ 2014-04-16 18:02 ` Felipe Balbi 2014-04-17 9:31 ` sundeep subbaraya 2014-04-16 20:08 ` Paul Zimmerman 1 sibling, 1 reply; 25+ messages in thread From: Felipe Balbi @ 2014-04-16 18:02 UTC (permalink / raw) To: sundeep subbaraya Cc: balbi, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta [-- Attachment #1: Type: text/plain, Size: 1837 bytes --] Hi, On Wed, Apr 16, 2014 at 04:09:28PM +0530, sundeep subbaraya wrote: > Hi Felipe, > > On Thu, Apr 3, 2014 at 8:28 PM, Felipe Balbi <balbi@ti.com> wrote: > > >> +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() > > This function is called in interrupt context when buffer is ready or > free. It initiates DMA to transfer data from IP buffer to memory. > Hence it waits in busy loop till DMA completes wait, so you start_dma() before your gadget driver asks you to ? -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-16 18:02 ` Felipe Balbi @ 2014-04-17 9:31 ` sundeep subbaraya 2014-04-17 15:01 ` Felipe Balbi 0 siblings, 1 reply; 25+ messages in thread From: sundeep subbaraya @ 2014-04-17 9:31 UTC (permalink / raw) To: balbi Cc: Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta Hi Felipe, On Wed, Apr 16, 2014 at 11:32 PM, Felipe Balbi <balbi@ti.com> wrote: > Hi, > > On Wed, Apr 16, 2014 at 04:09:28PM +0530, sundeep subbaraya wrote: >> Hi Felipe, >> >> On Thu, Apr 3, 2014 at 8:28 PM, Felipe Balbi <balbi@ti.com> wrote: >> >> >> +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() >> >> This function is called in interrupt context when buffer is ready or >> free. It initiates DMA to transfer data from IP buffer to memory. >> Hence it waits in busy loop till DMA completes > > wait, so you start_dma() before your gadget driver asks you to ? in ep_queue driver starts dma transfer from/to IP buffer to/from req->buf. If transfer is completed then request is not added to ep request queue and returns from ep_queue. If transfer is not completed (actual < length) then request is added to queue and returns from ep_queue. when buffer processed interrupt occurs, handler starts dma if there is request in queue and calls complete call back (when actual == length) Hence answer is Yes for some transfers (start dma called from interrupt handler not from ep_queue). Thanks, Sundeep.B.S. > > -- > balbi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-17 9:31 ` sundeep subbaraya @ 2014-04-17 15:01 ` Felipe Balbi 2014-04-18 14:04 ` sundeep subbaraya 0 siblings, 1 reply; 25+ messages in thread From: Felipe Balbi @ 2014-04-17 15:01 UTC (permalink / raw) To: sundeep subbaraya Cc: balbi, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta [-- Attachment #1: Type: text/plain, Size: 3312 bytes --] On Thu, Apr 17, 2014 at 03:01:37PM +0530, sundeep subbaraya wrote: > Hi Felipe, > > On Wed, Apr 16, 2014 at 11:32 PM, Felipe Balbi <balbi@ti.com> wrote: > > Hi, > > > > On Wed, Apr 16, 2014 at 04:09:28PM +0530, sundeep subbaraya wrote: > >> Hi Felipe, > >> > >> On Thu, Apr 3, 2014 at 8:28 PM, Felipe Balbi <balbi@ti.com> wrote: > >> > >> >> +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() > >> > >> This function is called in interrupt context when buffer is ready or > >> free. It initiates DMA to transfer data from IP buffer to memory. > >> Hence it waits in busy loop till DMA completes > > > > wait, so you start_dma() before your gadget driver asks you to ? > > in ep_queue driver starts dma transfer from/to IP buffer to/from req->buf. > If transfer is completed then request is not added to ep request queue > and returns from ep_queue. > If transfer is not completed (actual < length) then request is added > to queue and returns from ep_queue. This is wrong. Why wouldn't you give gadget driver the chance to decide if it needs to queue the request again or not ? > when buffer processed interrupt occurs, handler starts dma if there is > request in queue and calls complete call back (when actual == length) > Hence answer is Yes for some transfers (start dma called from > interrupt handler not from ep_queue). this also seems wrong(-ish). 1) as Paul pointed out, you can't rely on jiffies if you're calling this with IRQs disabled. 2) you don't really need to wait until DMA finishes its transaction before returning from start_dma(), just use the DMA completion IRQ, 3) I don't see anywhere any sanitizing of arguments, can your DMA really handle any alignment/unaligned addresses or should you make sure you're getting good arguments? You need to work on this a little bit, I guess. -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-17 15:01 ` Felipe Balbi @ 2014-04-18 14:04 ` sundeep subbaraya 2014-04-21 16:08 ` Felipe Balbi 0 siblings, 1 reply; 25+ messages in thread From: sundeep subbaraya @ 2014-04-18 14:04 UTC (permalink / raw) To: balbi@ti.com Cc: Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta Hi Felipe, On Thu, Apr 17, 2014 at 8:31 PM, Felipe Balbi <balbi@ti.com> wrote: > On Thu, Apr 17, 2014 at 03:01:37PM +0530, sundeep subbaraya wrote: >> Hi Felipe, >> >> On Wed, Apr 16, 2014 at 11:32 PM, Felipe Balbi <balbi@ti.com> wrote: >> > Hi, >> > >> > On Wed, Apr 16, 2014 at 04:09:28PM +0530, sundeep subbaraya wrote: >> >> Hi Felipe, >> >> >> >> On Thu, Apr 3, 2014 at 8:28 PM, Felipe Balbi <balbi@ti.com> wrote: >> >> >> >> >> +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() >> >> >> >> This function is called in interrupt context when buffer is ready or >> >> free. It initiates DMA to transfer data from IP buffer to memory. >> >> Hence it waits in busy loop till DMA completes >> > >> > wait, so you start_dma() before your gadget driver asks you to ? >> >> in ep_queue driver starts dma transfer from/to IP buffer to/from req->buf. >> If transfer is completed then request is not added to ep request queue >> and returns from ep_queue. >> If transfer is not completed (actual < length) then request is added >> to queue and returns from ep_queue. > > This is wrong. Why wouldn't you give gadget driver the chance to decide > if it needs to queue the request again or not ? When does gadget driver decides to queue the same request again? if -EBUSY is returned from ep_queue or req.status != 0 in completion routine? there are cases where ping-pong buffers both are busy. currently this driver adds request to queue only when buffers are busy. In normal cases request is processed without adding to queue. do i need to have another local queue for requests waiting since buffers are busy? Or dequeue the request ? > >> when buffer processed interrupt occurs, handler starts dma if there is >> request in queue and calls complete call back (when actual == length) >> Hence answer is Yes for some transfers (start dma called from >> interrupt handler not from ep_queue). > > this also seems wrong(-ish). > > 1) as Paul pointed out, you can't rely on jiffies if you're calling this > with IRQs disabled. > > 2) you don't really need to wait until DMA finishes its transaction > before returning from start_dma(), just use the DMA completion IRQ, > > 3) I don't see anywhere any sanitizing of arguments, can your DMA really > handle any alignment/unaligned addresses or should you make sure you're > getting good arguments? > > You need to work on this a little bit, I guess. Yes am working on this and will be using seperate ep_ops for ep0 like drivers/usb/dwc3/gadget.c since ep0 has no dma and ping-pong buffers Thanks, Sundeep.B.S. > > -- > balbi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-18 14:04 ` sundeep subbaraya @ 2014-04-21 16:08 ` Felipe Balbi 2014-04-21 16:39 ` Alan Stern 0 siblings, 1 reply; 25+ messages in thread From: Felipe Balbi @ 2014-04-21 16:08 UTC (permalink / raw) To: sundeep subbaraya Cc: balbi@ti.com, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta [-- Attachment #1: Type: text/plain, Size: 3103 bytes --] Hi, On Fri, Apr 18, 2014 at 07:34:08PM +0530, sundeep subbaraya wrote: <snip> > >> in ep_queue driver starts dma transfer from/to IP buffer to/from req->buf. > >> If transfer is completed then request is not added to ep request queue > >> and returns from ep_queue. > >> If transfer is not completed (actual < length) then request is added > >> to queue and returns from ep_queue. > > > > This is wrong. Why wouldn't you give gadget driver the chance to decide > > if it needs to queue the request again or not ? > > When does gadget driver decides to queue the same request again? > if -EBUSY is returned from ep_queue or req.status != 0 in completion > routine? whenever it so decides. Different gadget drivers might have different requirements. The code is open and sits under drivers/usb/gadget/ why don't you have a read ? > there are cases where ping-pong buffers both are busy. currently this driver > adds request to queue only when buffers are busy. In normal cases request is > processed without adding to queue. it shouldn't do that. You can't add requests by yourself. If you can't initiate transfers right now, let the HW NAK or something. > do i need to have another local queue for requests waiting since > buffers are busy? you should probably move the request to another queue, yes. Here's what dwc3 does: . gadget queues request . dwc3 puts request in a 'queued' list . HW sends XferNotReady event to notify it needs transfer requests . dwc3 moves requests to 'pending' list . dwc3 tells hw to consume 'pending' list if gadget driver queues another request, we accept it into 'queued' list and wait for another XferNotReady before moving it to 'pending' and consuming it. > Or dequeue the request ? you could return -EAGAIN, if you wish. But don't add requests by yourself, this could end up in hard-to-debug scenarios. Remember you don't *own* the reqeusts, only gadget drivers do. You can keep the request around until you have space in your FIFOs to start it, but then, in that case, don't try to list_del() -> start() -> list_add(). > >> when buffer processed interrupt occurs, handler starts dma if there is > >> request in queue and calls complete call back (when actual == length) > >> Hence answer is Yes for some transfers (start dma called from > >> interrupt handler not from ep_queue). > > > > this also seems wrong(-ish). > > > > 1) as Paul pointed out, you can't rely on jiffies if you're calling this > > with IRQs disabled. > > > > 2) you don't really need to wait until DMA finishes its transaction > > before returning from start_dma(), just use the DMA completion IRQ, > > > > 3) I don't see anywhere any sanitizing of arguments, can your DMA really > > handle any alignment/unaligned addresses or should you make sure you're > > getting good arguments? > > > > You need to work on this a little bit, I guess. > > Yes am working on this and will be using seperate ep_ops for ep0 like > drivers/usb/dwc3/gadget.c since ep0 has no dma and ping-pong buffers ok -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-21 16:08 ` Felipe Balbi @ 2014-04-21 16:39 ` Alan Stern 2014-04-22 7:28 ` sundeep subbaraya 0 siblings, 1 reply; 25+ messages in thread From: Alan Stern @ 2014-04-21 16:39 UTC (permalink / raw) To: Felipe Balbi Cc: sundeep subbaraya, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta On Mon, 21 Apr 2014, Felipe Balbi wrote: > Hi, > > On Fri, Apr 18, 2014 at 07:34:08PM +0530, sundeep subbaraya wrote: > > <snip> > > > >> in ep_queue driver starts dma transfer from/to IP buffer to/from req->buf. > > >> If transfer is completed then request is not added to ep request queue > > >> and returns from ep_queue. > > >> If transfer is not completed (actual < length) then request is added > > >> to queue and returns from ep_queue. > > > > > > This is wrong. Why wouldn't you give gadget driver the chance to decide > > > if it needs to queue the request again or not ? > > > > When does gadget driver decides to queue the same request again? > > if -EBUSY is returned from ep_queue or req.status != 0 in completion > > routine? > > whenever it so decides. Different gadget drivers might have different > requirements. The code is open and sits under drivers/usb/gadget/ why > don't you have a read ? I get the impression that the two of you are arguing past each other. It appears that Sundeep is talking about transferring data from the gadget driver's buffer to an internal buffer in the UDC hardware, but Felipe is talking about transferring data from the UDC to the host. As I understand it, Sundeep said that when the gadget driver queues a data-IN request, the UDC driver copies as much of the data buffer as possible into a hardware FIFO. If it succeeds in copying all the data into the FIFO then the request's completion routine gets called immediately, even though the data doesn't get sent from the FIFO to the host until the host asks for it. If only part of the data can be copied into the FIFO then the request is added to the ep's request queue before the usb_ep_queue() call returns. When space becomes available in the FIFO, the data will be copied and eventually sent to the host. When all the data has been copied to the FIFO, the request's completion routine will be called. Thus there never is any need for the gadget driver to queue the request again. An incomplete transfer means the FIFO didn't have enough room when the request was submitted; it doesn't mean that the data didn't eventually get sent to the host. HTH, Alan Stern ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-21 16:39 ` Alan Stern @ 2014-04-22 7:28 ` sundeep subbaraya 2014-04-22 13:57 ` Alan Stern 2014-04-22 14:49 ` Felipe Balbi 0 siblings, 2 replies; 25+ messages in thread From: sundeep subbaraya @ 2014-04-22 7:28 UTC (permalink / raw) To: Alan Stern Cc: Felipe Balbi, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta Hi, On Mon, Apr 21, 2014 at 10:09 PM, Alan Stern <stern@rowland.harvard.edu> wrote: > On Mon, 21 Apr 2014, Felipe Balbi wrote: > >> Hi, >> >> On Fri, Apr 18, 2014 at 07:34:08PM +0530, sundeep subbaraya wrote: >> >> <snip> >> >> > >> in ep_queue driver starts dma transfer from/to IP buffer to/from req->buf. >> > >> If transfer is completed then request is not added to ep request queue >> > >> and returns from ep_queue. >> > >> If transfer is not completed (actual < length) then request is added >> > >> to queue and returns from ep_queue. >> > > >> > > This is wrong. Why wouldn't you give gadget driver the chance to decide >> > > if it needs to queue the request again or not ? >> > >> > When does gadget driver decides to queue the same request again? >> > if -EBUSY is returned from ep_queue or req.status != 0 in completion >> > routine? >> >> whenever it so decides. Different gadget drivers might have different >> requirements. The code is open and sits under drivers/usb/gadget/ why >> don't you have a read ? > > I get the impression that the two of you are arguing past each other. > It appears that Sundeep is talking about transferring data from the > gadget driver's buffer to an internal buffer in the UDC hardware, but > Felipe is talking about transferring data from the UDC to the host. > > As I understand it, Sundeep said that when the gadget driver queues a > data-IN request, the UDC driver copies as much of the data buffer as > possible into a hardware FIFO. If it succeeds in copying all the data > into the FIFO then the request's completion routine gets called > immediately, even though the data doesn't get sent from the FIFO to the > host until the host asks for it. > > If only part of the data can be copied into the FIFO then the request > is added to the ep's request queue before the usb_ep_queue() call > returns. When space becomes available in the FIFO, the data will be > copied and eventually sent to the host. When all the data has been > copied to the FIFO, the request's completion routine will be called. > > Thus there never is any need for the gadget driver to queue the request > again. An incomplete transfer means the FIFO didn't have enough room > when the request was submitted; it doesn't mean that the data didn't > eventually get sent to the host. Exactly Alan,this is what I was trying to say. Probably I was not clear in explaining. I didnt see any harm this way and even this implementation is same like at91_udc.c. I have been reading mas_storage to understand when does gadget driver tries to enqueue a request again. Since different gadget drivers might have different requirements (agree with Felipe), wanted to know criteria for queuing a same request again. I will change this implementation as per Felipe comments and test with some of the gadgets. Thanks Alan and Felipe, Sundeep > > HTH, > > Alan Stern > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-22 7:28 ` sundeep subbaraya @ 2014-04-22 13:57 ` Alan Stern 2014-04-22 14:49 ` Felipe Balbi 1 sibling, 0 replies; 25+ messages in thread From: Alan Stern @ 2014-04-22 13:57 UTC (permalink / raw) To: sundeep subbaraya Cc: Felipe Balbi, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta On Tue, 22 Apr 2014, sundeep subbaraya wrote: > Hi, > > On Mon, Apr 21, 2014 at 10:09 PM, Alan Stern <stern@rowland.harvard.edu> wrote: > > On Mon, 21 Apr 2014, Felipe Balbi wrote: > > > >> Hi, > >> > >> On Fri, Apr 18, 2014 at 07:34:08PM +0530, sundeep subbaraya wrote: > >> > >> <snip> > >> > >> > >> in ep_queue driver starts dma transfer from/to IP buffer to/from req->buf. > >> > >> If transfer is completed then request is not added to ep request queue > >> > >> and returns from ep_queue. > >> > >> If transfer is not completed (actual < length) then request is added > >> > >> to queue and returns from ep_queue. > >> > > > >> > > This is wrong. Why wouldn't you give gadget driver the chance to decide > >> > > if it needs to queue the request again or not ? > >> > > >> > When does gadget driver decides to queue the same request again? > >> > if -EBUSY is returned from ep_queue or req.status != 0 in completion > >> > routine? > >> > >> whenever it so decides. Different gadget drivers might have different > >> requirements. The code is open and sits under drivers/usb/gadget/ why > >> don't you have a read ? > > > > I get the impression that the two of you are arguing past each other. > > It appears that Sundeep is talking about transferring data from the > > gadget driver's buffer to an internal buffer in the UDC hardware, but > > Felipe is talking about transferring data from the UDC to the host. > > > > As I understand it, Sundeep said that when the gadget driver queues a > > data-IN request, the UDC driver copies as much of the data buffer as > > possible into a hardware FIFO. If it succeeds in copying all the data > > into the FIFO then the request's completion routine gets called > > immediately, even though the data doesn't get sent from the FIFO to the > > host until the host asks for it. > > > > If only part of the data can be copied into the FIFO then the request > > is added to the ep's request queue before the usb_ep_queue() call > > returns. When space becomes available in the FIFO, the data will be > > copied and eventually sent to the host. When all the data has been > > copied to the FIFO, the request's completion routine will be called. > > > > Thus there never is any need for the gadget driver to queue the request > > again. An incomplete transfer means the FIFO didn't have enough room > > when the request was submitted; it doesn't mean that the data didn't > > eventually get sent to the host. > > Exactly Alan,this is what I was trying to say. Probably I was not > clear in explaining. I didnt see > any harm this way and even this implementation is same like > at91_udc.c. I have been reading > mas_storage to understand when does gadget driver tries to enqueue a > request again. Since > different gadget drivers might have different requirements (agree with > Felipe), wanted to > know criteria for queuing a same request again. > > I will change this implementation as per Felipe comments and test with > some of the gadgets. Wait until you hear what Felipe has to say about this discussion. Maybe he will decide that the original implementation was correct. Alan Stern ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-22 7:28 ` sundeep subbaraya 2014-04-22 13:57 ` Alan Stern @ 2014-04-22 14:49 ` Felipe Balbi 2014-04-23 4:27 ` sundeep subbaraya 1 sibling, 1 reply; 25+ messages in thread From: Felipe Balbi @ 2014-04-22 14:49 UTC (permalink / raw) To: sundeep subbaraya Cc: Alan Stern, Felipe Balbi, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta [-- Attachment #1: Type: text/plain, Size: 3514 bytes --] Hi, On Tue, Apr 22, 2014 at 12:58:41PM +0530, sundeep subbaraya wrote: > Hi, > > On Mon, Apr 21, 2014 at 10:09 PM, Alan Stern <stern@rowland.harvard.edu> wrote: > > On Mon, 21 Apr 2014, Felipe Balbi wrote: > > > >> Hi, > >> > >> On Fri, Apr 18, 2014 at 07:34:08PM +0530, sundeep subbaraya wrote: > >> > >> <snip> > >> > >> > >> in ep_queue driver starts dma transfer from/to IP buffer to/from req->buf. > >> > >> If transfer is completed then request is not added to ep request queue > >> > >> and returns from ep_queue. > >> > >> If transfer is not completed (actual < length) then request is added > >> > >> to queue and returns from ep_queue. > >> > > > >> > > This is wrong. Why wouldn't you give gadget driver the chance to decide > >> > > if it needs to queue the request again or not ? > >> > > >> > When does gadget driver decides to queue the same request again? > >> > if -EBUSY is returned from ep_queue or req.status != 0 in completion > >> > routine? > >> > >> whenever it so decides. Different gadget drivers might have different > >> requirements. The code is open and sits under drivers/usb/gadget/ why > >> don't you have a read ? > > > > I get the impression that the two of you are arguing past each other. > > It appears that Sundeep is talking about transferring data from the > > gadget driver's buffer to an internal buffer in the UDC hardware, but > > Felipe is talking about transferring data from the UDC to the host. > > > > As I understand it, Sundeep said that when the gadget driver queues a > > data-IN request, the UDC driver copies as much of the data buffer as > > possible into a hardware FIFO. If it succeeds in copying all the data > > into the FIFO then the request's completion routine gets called > > immediately, even though the data doesn't get sent from the FIFO to the > > host until the host asks for it. > > > > If only part of the data can be copied into the FIFO then the request > > is added to the ep's request queue before the usb_ep_queue() call > > returns. When space becomes available in the FIFO, the data will be > > copied and eventually sent to the host. When all the data has been > > copied to the FIFO, the request's completion routine will be called. there seems to be a slight problem with this approach: how will the IP know that even though you copied X bytes into the FIFO, it should wait for another Y bytes before shifting data to the wire ? How will it know that it shouldn't generate CRC yet because there's still data to be added ? If there's no space in the FIFO yet, why copy data at all ? > > Thus there never is any need for the gadget driver to queue the request > > again. An incomplete transfer means the FIFO didn't have enough room > > when the request was submitted; it doesn't mean that the data didn't > > eventually get sent to the host. > > Exactly Alan,this is what I was trying to say. Probably I was not > clear in explaining. I didnt see any harm this way and even this > implementation is same like at91_udc.c. I have been reading > mas_storage to understand when does gadget driver tries to enqueue a > request again. Since different gadget drivers might have different > requirements (agree with Felipe), wanted to know criteria for queuing > a same request again. > > I will change this implementation as per Felipe comments and test with > some of the gadgets. Let's see, please help me understand the questions above. -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-22 14:49 ` Felipe Balbi @ 2014-04-23 4:27 ` sundeep subbaraya 2014-04-30 16:54 ` Felipe Balbi 0 siblings, 1 reply; 25+ messages in thread From: sundeep subbaraya @ 2014-04-23 4:27 UTC (permalink / raw) To: balbi@ti.com Cc: Alan Stern, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta Hi, On Tue, Apr 22, 2014 at 8:19 PM, Felipe Balbi <balbi@ti.com> wrote: > Hi, > > On Tue, Apr 22, 2014 at 12:58:41PM +0530, sundeep subbaraya wrote: >> Hi, >> >> On Mon, Apr 21, 2014 at 10:09 PM, Alan Stern <stern@rowland.harvard.edu> wrote: >> > On Mon, 21 Apr 2014, Felipe Balbi wrote: >> > >> >> Hi, >> >> >> >> On Fri, Apr 18, 2014 at 07:34:08PM +0530, sundeep subbaraya wrote: >> >> >> >> <snip> >> >> >> >> > >> in ep_queue driver starts dma transfer from/to IP buffer to/from req->buf. >> >> > >> If transfer is completed then request is not added to ep request queue >> >> > >> and returns from ep_queue. >> >> > >> If transfer is not completed (actual < length) then request is added >> >> > >> to queue and returns from ep_queue. >> >> > > >> >> > > This is wrong. Why wouldn't you give gadget driver the chance to decide >> >> > > if it needs to queue the request again or not ? >> >> > >> >> > When does gadget driver decides to queue the same request again? >> >> > if -EBUSY is returned from ep_queue or req.status != 0 in completion >> >> > routine? >> >> >> >> whenever it so decides. Different gadget drivers might have different >> >> requirements. The code is open and sits under drivers/usb/gadget/ why >> >> don't you have a read ? >> > >> > I get the impression that the two of you are arguing past each other. >> > It appears that Sundeep is talking about transferring data from the >> > gadget driver's buffer to an internal buffer in the UDC hardware, but >> > Felipe is talking about transferring data from the UDC to the host. >> > >> > As I understand it, Sundeep said that when the gadget driver queues a >> > data-IN request, the UDC driver copies as much of the data buffer as >> > possible into a hardware FIFO. If it succeeds in copying all the data >> > into the FIFO then the request's completion routine gets called >> > immediately, even though the data doesn't get sent from the FIFO to the >> > host until the host asks for it. >> > >> > If only part of the data can be copied into the FIFO then the request >> > is added to the ep's request queue before the usb_ep_queue() call >> > returns. When space becomes available in the FIFO, the data will be >> > copied and eventually sent to the host. When all the data has been >> > copied to the FIFO, the request's completion routine will be called. > > there seems to be a slight problem with this approach: how will the IP > know that even though you copied X bytes into the FIFO, it should wait > for another Y bytes before shifting data to the wire ? How will it know > that it shouldn't generate CRC yet because there's still data to be > added ? No. IP does/need not know that it has to wait for Y bytes.We just write X bytes into HW buffer and count as X in buffer count register. IP generates CRC for bytes based on Count register and sends data to Host. Let us consider this scenario of bulk IN transfer: req.length = 5120 and wMaxPacketSize = 512, ep_queue is called once and is returned with status 0. In ep_queue this code snippet, if (xudc_write_fifo(ep, req) == 1) req = NULL; if(req != NULL) list_add_tail(&req->queue, &ep->queue); xudc_write_fifo does the following if HW buffers not busy: copies 512 bytes to HW buffer set count and ready registers so that IP can start data transfer to host changes req.actual to 512 and returns 0(if req.length > wMaxPacketSize) and 1(if req.length < wMaxPacketSize). Since return is zero this request is added to queue. When data transfer to host is completed IP generates an interrupt. In the interrupt handler we again call write_fifo if request list is not empty. 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); This happens 10 times(since length 5120) and completion is called. > If there's no space in the FIFO yet, why copy data at all ? If HW buffers are busy(IP is still transferring previous data to Host from buffer) then xudc_write_fifo returns 0 without changing req.actual. When previous data transfer completes then Interrupt then again write_fifo from handler. >> > Thus there never is any need for the gadget driver to queue the request >> > again. Yes >> >An incomplete transfer means the FIFO didn't have enough room >> > when the request was submitted; it doesn't mean that the data didn't >> > eventually get sent to the host. >> >> Exactly Alan,this is what I was trying to say. Probably I was not >> clear in explaining. I didnt see any harm this way and even this >> implementation is same like at91_udc.c. I have been reading >> mas_storage to understand when does gadget driver tries to enqueue a >> request again. Since different gadget drivers might have different >> requirements (agree with Felipe), wanted to know criteria for queuing >> a same request again. >> >> I will change this implementation as per Felipe comments and test with >> some of the gadgets. > > Let's see, please help me understand the questions above. Hope this helps. Thanks, Sundeep.B.S. > > -- > balbi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-23 4:27 ` sundeep subbaraya @ 2014-04-30 16:54 ` Felipe Balbi 2014-05-25 17:40 ` sundeep subbaraya 0 siblings, 1 reply; 25+ messages in thread From: Felipe Balbi @ 2014-04-30 16:54 UTC (permalink / raw) To: sundeep subbaraya Cc: balbi@ti.com, Alan Stern, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta [-- Attachment #1: Type: text/plain, Size: 4689 bytes --] Hi, On Wed, Apr 23, 2014 at 09:57:52AM +0530, sundeep subbaraya wrote: > >> > I get the impression that the two of you are arguing past each other. > >> > It appears that Sundeep is talking about transferring data from the > >> > gadget driver's buffer to an internal buffer in the UDC hardware, but > >> > Felipe is talking about transferring data from the UDC to the host. > >> > > >> > As I understand it, Sundeep said that when the gadget driver queues a > >> > data-IN request, the UDC driver copies as much of the data buffer as > >> > possible into a hardware FIFO. If it succeeds in copying all the data > >> > into the FIFO then the request's completion routine gets called > >> > immediately, even though the data doesn't get sent from the FIFO to the > >> > host until the host asks for it. > >> > > >> > If only part of the data can be copied into the FIFO then the request > >> > is added to the ep's request queue before the usb_ep_queue() call > >> > returns. When space becomes available in the FIFO, the data will be > >> > copied and eventually sent to the host. When all the data has been > >> > copied to the FIFO, the request's completion routine will be called. > > > > there seems to be a slight problem with this approach: how will the IP > > know that even though you copied X bytes into the FIFO, it should wait > > for another Y bytes before shifting data to the wire ? How will it know > > that it shouldn't generate CRC yet because there's still data to be > > added ? > > No. IP does/need not know that it has to wait for Y bytes.We just > write X bytes into > HW buffer and count as X in buffer count register. IP generates CRC > for bytes based > on Count register and sends data to Host. Let us consider this > scenario of bulk IN transfer: > req.length = 5120 and wMaxPacketSize = 512, ep_queue is called once > and is returned with > status 0. In ep_queue this code snippet, > if (xudc_write_fifo(ep, req) == 1) > req = NULL; > if(req != NULL) > list_add_tail(&req->queue, &ep->queue); > > xudc_write_fifo does the following if HW buffers not busy: > copies 512 bytes to HW buffer > set count and ready registers so that IP can start data transfer to host > changes req.actual to 512 and returns 0(if req.length > > wMaxPacketSize) and 1(if req.length < wMaxPacketSize). you should return a proper error code, not 1. > Since return is zero this request is added to queue. When data > transfer to host is completed IP generates > an interrupt. In the interrupt handler we again call write_fifo if > request list is not empty. > if (list_empty(&ep->queue)) > req = NULL; > else > req = list_entry(ep->queue.next, struct xusb_req, queue); > if (!req) > return; okay, this can be improved a bit though: if (list_empty(&ep->queue)) return; req = list_first_entry(&ep->queue, struct xusb_req, queue); > if (ep->is_in) > xudc_write_fifo(ep, req); > else > xudc_read_fifo(ep, req); > > This happens 10 times(since length 5120) and completion is called. ok. > > If there's no space in the FIFO yet, why copy data at all ? > > If HW buffers are busy(IP is still transferring previous data to Host > from buffer) then xudc_write_fifo returns 0 without changing > req.actual. When previous data transfer completes then Interrupt then > again write_fifo from handler. ok, sounds good to me. > >> > Thus there never is any need for the gadget driver to queue the request > >> > again. > > Yes > > >> >An incomplete transfer means the FIFO didn't have enough room > >> > when the request was submitted; it doesn't mean that the data didn't > >> > eventually get sent to the host. > >> > >> Exactly Alan,this is what I was trying to say. Probably I was not > >> clear in explaining. I didnt see any harm this way and even this > >> implementation is same like at91_udc.c. I have been reading > >> mas_storage to understand when does gadget driver tries to enqueue a > >> request again. Since different gadget drivers might have different > >> requirements (agree with Felipe), wanted to know criteria for queuing > >> a same request again. > >> > >> I will change this implementation as per Felipe comments and test with > >> some of the gadgets. > > > > Let's see, please help me understand the questions above. > > Hope this helps. it does, but please sort out the small comments I had above. cheers -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-30 16:54 ` Felipe Balbi @ 2014-05-25 17:40 ` sundeep subbaraya 2014-07-02 16:46 ` Felipe Balbi 0 siblings, 1 reply; 25+ messages in thread From: sundeep subbaraya @ 2014-05-25 17:40 UTC (permalink / raw) To: balbi@ti.com Cc: Alan Stern, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta Hi Felipe, Please take a look at below about how this IP works: IN: req.buf ---> DMA (transfers from ddr to IP buffer, raise DMA done interrupt and set Buffer ready to transfer data to Host)---->Host PC buffer sent interrupt OUT: Host PC--->buffer ready interrupt--->DMA (transfer from IP buffer to DDR,DMA done interrupt, set Buffer ready to receive next data from Host)-->req.buf I written logic to call completion in DMA done handler because it works for both IN and OUT eps. But I see significant performance degradation (by copying a file to mass storage gadget). DMA can handle unaligned address too but it has ep max limit (say 512 for bulk). Hence it is SW job to split packets and to drive DMA till req.length completes. I feel polling for a while is faster than switching between interrupt handler back and forth rapidly. Moreover, DMA may or may not be present in IP based on user configuration at design time. I fixed all other comments expect this one if you do not agree for polling then I may have to create threads and do this stuff. What do you suggest? Thanks, Sundeep On Wed, Apr 30, 2014 at 10:24 PM, Felipe Balbi <balbi@ti.com> wrote: > Hi, > > On Wed, Apr 23, 2014 at 09:57:52AM +0530, sundeep subbaraya wrote: >> >> > I get the impression that the two of you are arguing past each other. >> >> > It appears that Sundeep is talking about transferring data from the >> >> > gadget driver's buffer to an internal buffer in the UDC hardware, but >> >> > Felipe is talking about transferring data from the UDC to the host. >> >> > >> >> > As I understand it, Sundeep said that when the gadget driver queues a >> >> > data-IN request, the UDC driver copies as much of the data buffer as >> >> > possible into a hardware FIFO. If it succeeds in copying all the data >> >> > into the FIFO then the request's completion routine gets called >> >> > immediately, even though the data doesn't get sent from the FIFO to the >> >> > host until the host asks for it. >> >> > >> >> > If only part of the data can be copied into the FIFO then the request >> >> > is added to the ep's request queue before the usb_ep_queue() call >> >> > returns. When space becomes available in the FIFO, the data will be >> >> > copied and eventually sent to the host. When all the data has been >> >> > copied to the FIFO, the request's completion routine will be called. >> > >> > there seems to be a slight problem with this approach: how will the IP >> > know that even though you copied X bytes into the FIFO, it should wait >> > for another Y bytes before shifting data to the wire ? How will it know >> > that it shouldn't generate CRC yet because there's still data to be >> > added ? >> >> No. IP does/need not know that it has to wait for Y bytes.We just >> write X bytes into >> HW buffer and count as X in buffer count register. IP generates CRC >> for bytes based >> on Count register and sends data to Host. Let us consider this >> scenario of bulk IN transfer: >> req.length = 5120 and wMaxPacketSize = 512, ep_queue is called once >> and is returned with >> status 0. In ep_queue this code snippet, >> if (xudc_write_fifo(ep, req) == 1) >> req = NULL; >> if(req != NULL) >> list_add_tail(&req->queue, &ep->queue); >> >> xudc_write_fifo does the following if HW buffers not busy: >> copies 512 bytes to HW buffer >> set count and ready registers so that IP can start data transfer to host >> changes req.actual to 512 and returns 0(if req.length > >> wMaxPacketSize) and 1(if req.length < wMaxPacketSize). > > you should return a proper error code, not 1. > >> Since return is zero this request is added to queue. When data >> transfer to host is completed IP generates >> an interrupt. In the interrupt handler we again call write_fifo if >> request list is not empty. >> if (list_empty(&ep->queue)) >> req = NULL; >> else >> req = list_entry(ep->queue.next, struct xusb_req, queue); >> if (!req) >> return; > > okay, this can be improved a bit though: > > if (list_empty(&ep->queue)) > return; > > req = list_first_entry(&ep->queue, struct xusb_req, queue); > >> if (ep->is_in) >> xudc_write_fifo(ep, req); >> else >> xudc_read_fifo(ep, req); >> >> This happens 10 times(since length 5120) and completion is called. > > ok. > >> > If there's no space in the FIFO yet, why copy data at all ? >> >> If HW buffers are busy(IP is still transferring previous data to Host >> from buffer) then xudc_write_fifo returns 0 without changing >> req.actual. When previous data transfer completes then Interrupt then >> again write_fifo from handler. > > ok, sounds good to me. > >> >> > Thus there never is any need for the gadget driver to queue the request >> >> > again. >> >> Yes >> >> >> >An incomplete transfer means the FIFO didn't have enough room >> >> > when the request was submitted; it doesn't mean that the data didn't >> >> > eventually get sent to the host. >> >> >> >> Exactly Alan,this is what I was trying to say. Probably I was not >> >> clear in explaining. I didnt see any harm this way and even this >> >> implementation is same like at91_udc.c. I have been reading >> >> mas_storage to understand when does gadget driver tries to enqueue a >> >> request again. Since different gadget drivers might have different >> >> requirements (agree with Felipe), wanted to know criteria for queuing >> >> a same request again. >> >> >> >> I will change this implementation as per Felipe comments and test with >> >> some of the gadgets. >> > >> > Let's see, please help me understand the questions above. >> >> Hope this helps. > > it does, but please sort out the small comments I had above. > > cheers > > -- > balbi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-05-25 17:40 ` sundeep subbaraya @ 2014-07-02 16:46 ` Felipe Balbi 2014-07-07 6:28 ` sundeep subbaraya 0 siblings, 1 reply; 25+ messages in thread From: Felipe Balbi @ 2014-07-02 16:46 UTC (permalink / raw) To: sundeep subbaraya Cc: balbi@ti.com, Alan Stern, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta [-- Attachment #1: Type: text/plain, Size: 1399 bytes --] Hi, On Sun, May 25, 2014 at 11:10:30PM +0530, sundeep subbaraya wrote: > Hi Felipe, > > Please take a look at below about how this IP works: > > IN: > req.buf ---> DMA (transfers from ddr to IP buffer, raise DMA > done interrupt and set Buffer ready to transfer data to Host)---->Host > PC > buffer sent interrupt > > OUT: > Host PC--->buffer ready interrupt--->DMA (transfer from IP buffer > to DDR,DMA done interrupt, set Buffer ready to receive next data from > Host)-->req.buf > > I written logic to call completion in DMA done handler because it > works for both IN and OUT eps. But I see significant performance > degradation (by copying a file to mass storage gadget). DMA can handle > unaligned address too but it has ep max limit (say 512 for bulk). > Hence it is SW job to split packets and to drive DMA till req.length > completes. I feel polling for a while is faster than switching between > interrupt handler back and forth rapidly. Moreover, DMA may or may not > be present in IP based on user configuration at design time. > > I fixed all other comments expect this one if you do not agree for > polling then I may have to create threads and do this stuff. What do > you suggest? Can you resend your driver so we can review again everything you have fixed ? I guess you have no other way... polling it is. cheers -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-07-02 16:46 ` Felipe Balbi @ 2014-07-07 6:28 ` sundeep subbaraya 0 siblings, 0 replies; 25+ messages in thread From: sundeep subbaraya @ 2014-07-07 6:28 UTC (permalink / raw) To: balbi@ti.com Cc: Alan Stern, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta Hi Felipe, On Wed, Jul 2, 2014 at 10:16 PM, Felipe Balbi <balbi@ti.com> wrote: > Hi, > > On Sun, May 25, 2014 at 11:10:30PM +0530, sundeep subbaraya wrote: >> Hi Felipe, >> >> Please take a look at below about how this IP works: >> >> IN: >> req.buf ---> DMA (transfers from ddr to IP buffer, raise DMA >> done interrupt and set Buffer ready to transfer data to Host)---->Host >> PC >> buffer sent interrupt >> >> OUT: >> Host PC--->buffer ready interrupt--->DMA (transfer from IP buffer >> to DDR,DMA done interrupt, set Buffer ready to receive next data from >> Host)-->req.buf >> >> I written logic to call completion in DMA done handler because it >> works for both IN and OUT eps. But I see significant performance >> degradation (by copying a file to mass storage gadget). DMA can handle >> unaligned address too but it has ep max limit (say 512 for bulk). >> Hence it is SW job to split packets and to drive DMA till req.length >> completes. I feel polling for a while is faster than switching between >> interrupt handler back and forth rapidly. Moreover, DMA may or may not >> be present in IP based on user configuration at design time. >> >> I fixed all other comments expect this one if you do not agree for >> polling then I may have to create threads and do this stuff. What do >> you suggest? > > Can you resend your driver so we can review again everything you have > fixed ? I guess you have no other way... polling it is. Sure. I will resend the driver. Thanks, Sundeep.B.S. > > cheers > > -- > balbi ^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-16 10:39 ` sundeep subbaraya 2014-04-16 18:02 ` Felipe Balbi @ 2014-04-16 20:08 ` Paul Zimmerman 2014-04-17 14:27 ` sundeep subbaraya 1 sibling, 1 reply; 25+ messages in thread From: Paul Zimmerman @ 2014-04-16 20:08 UTC (permalink / raw) To: sundeep subbaraya, balbi@ti.com Cc: Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2125 bytes --] > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-owner@vger.kernel.org] On Behalf Of sundeep subbaraya > Sent: Wednesday, April 16, 2014 3:39 AM > > Hi Felipe, > > On Thu, Apr 3, 2014 at 8:28 PM, Felipe Balbi <balbi@ti.com> wrote: > > >> +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() > > This function is called in interrupt context when buffer is ready or > free. It initiates > DMA to transfer data from IP buffer to memory. Hence it waits in busy > loop till DMA > completes If this function is called in interrupt context, then you can't use jiffies for your timeout, since jiffies may not get updated while in interrupt context. -- Paul ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support 2014-04-16 20:08 ` Paul Zimmerman @ 2014-04-17 14:27 ` sundeep subbaraya 0 siblings, 0 replies; 25+ messages in thread From: sundeep subbaraya @ 2014-04-17 14:27 UTC (permalink / raw) To: Paul Zimmerman Cc: balbi@ti.com, Subbaraya Sundeep Bhatta, Greg Kroah-Hartman, Michal Simek, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Subbaraya Sundeep Bhatta Hi, On Thu, Apr 17, 2014 at 1:38 AM, Paul Zimmerman <Paul.Zimmerman@synopsys.com> wrote: >> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-owner@vger.kernel.org] On Behalf Of sundeep subbaraya >> Sent: Wednesday, April 16, 2014 3:39 AM >> >> Hi Felipe, >> >> On Thu, Apr 3, 2014 at 8:28 PM, Felipe Balbi <balbi@ti.com> wrote: >> >> >> +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() >> >> This function is called in interrupt context when buffer is ready or >> free. It initiates >> DMA to transfer data from IP buffer to memory. Hence it waits in busy >> loop till DMA >> completes > > If this function is called in interrupt context, then you can't use > jiffies for your timeout, since jiffies may not get updated while in > interrupt context. Yes. It is obvious that this logic is buggy, will change this. Thanks, Sundeep.B.S. > > -- > Paul > ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2014-07-07 6:28 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1396510519-8555-1-git-send-email-sbhatta@xilinx.com>
2014-04-03 7:35 ` [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support Subbaraya Sundeep Bhatta
2014-04-03 14:58 ` Felipe Balbi
2014-04-03 15:23 ` Michal Simek
2014-04-07 9:01 ` sundeep subbaraya
[not found] ` <CALHRZupk2ywMNKjzAtCPgg=9-KsDqrVBAKFNoHOoSJOQ4FDJkA@mail.gmail.com>
2014-04-07 9:06 ` Fwd: " sundeep subbaraya
2014-04-07 16:35 ` Felipe Balbi
2014-04-08 16:01 ` sundeep subbaraya
2014-04-08 21:24 ` Felipe Balbi
2014-04-16 10:39 ` sundeep subbaraya
2014-04-16 18:02 ` Felipe Balbi
2014-04-17 9:31 ` sundeep subbaraya
2014-04-17 15:01 ` Felipe Balbi
2014-04-18 14:04 ` sundeep subbaraya
2014-04-21 16:08 ` Felipe Balbi
2014-04-21 16:39 ` Alan Stern
2014-04-22 7:28 ` sundeep subbaraya
2014-04-22 13:57 ` Alan Stern
2014-04-22 14:49 ` Felipe Balbi
2014-04-23 4:27 ` sundeep subbaraya
2014-04-30 16:54 ` Felipe Balbi
2014-05-25 17:40 ` sundeep subbaraya
2014-07-02 16:46 ` Felipe Balbi
2014-07-07 6:28 ` sundeep subbaraya
2014-04-16 20:08 ` Paul Zimmerman
2014-04-17 14:27 ` sundeep subbaraya
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).