Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support
From: Felipe Balbi
Date: Thu Apr 03 2014 - 11:01:09 EST
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@xxxxxxxxxx>
> ---
> 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:
signature.asc
Description: Digital signature