Hi,

On Mon, Oct 06, 2014 at 06:55:05PM +0300, Laurent Pinchart wrote:
> The ISP1761 is a dual-mode host and device controller backward
> compatible on the host side with the ISP1760. Add support for the device
> controller.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

finally my part :-)

Unfortunately I don't have HW to verify all this, so I'd really like to
see some test results. It would be nice if you had access to a windows
box to run USB20CV on this UDC too, but I guess that's too much to ask
:-p

Anyway, a few comments below. I might come up with more as I digest this
code.

> diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> index 34ebaa6..7bdfdf0 100644
> --- a/drivers/usb/gadget/udc/Kconfig
> +++ b/drivers/usb/gadget/udc/Kconfig
> @@ -109,6 +109,13 @@ config USB_GR_UDC
>            Select this to support Aeroflex Gaisler GRUSBDC cores from the 
> GRLIB
>         VHDL IP core library.
>  
> +config USB_ISP1761_UDC
> +     boolean "NXP ISP1761 USB Device Controller"

not tristate ?

> diff --git a/drivers/usb/host/isp1760-core.c b/drivers/usb/host/isp1760-core.c
> index 595f234..54aedc2 100644
> --- a/drivers/usb/host/isp1760-core.c
> +++ b/drivers/usb/host/isp1760-core.c
> @@ -24,9 +24,11 @@
>  #include "isp1760-core.h"
>  #include "isp1760-hcd.h"
>  #include "isp1760-regs.h"
> +#include "isp1760-udc.h"
>  
>  static void isp1760_init_core(struct isp1760_device *isp)
>  {
> +     u32 otgctrl;
>       u32 hwmode;
>  
>       /* Low-level chip reset */
> @@ -65,6 +67,17 @@ static void isp1760_init_core(struct isp1760_device *isp)
>               hwmode |= HW_INTR_EDGE_TRIG;
>  
>       /*
> +      * The ISP1761 has a dedicated DC IRQ line but supports sharing the HC
> +      * IRQ line for both the host and device controllers. Hardcode IRQ
> +      * sharing for now and disable the DC interrupts globally to avoid
> +      * spurious interrupts during HCD registration.
> +      */
> +     if (isp->devflags & ISP1760_FLAG_ISP1761) {
> +             isp1760_write32(isp->regs, DC_MODE, 0);
> +             hwmode |= HW_COMN_IRQ;
> +     }
> +
> +     /*
>        * We have to set this first in case we're in 16-bit mode.
>        * Write it twice to ensure correct upper bits if switching
>        * to 16-bit mode.
> @@ -72,11 +85,35 @@ static void isp1760_init_core(struct isp1760_device *isp)
>       isp1760_write32(isp->regs, HC_HW_MODE_CTRL, hwmode);
>       isp1760_write32(isp->regs, HC_HW_MODE_CTRL, hwmode);
>  
> +     /*
> +      * PORT 1 Control register of the ISP1760 is the OTG control register
> +      * on ISP1761.
> +      *
> +      * TODO: Really support OTG. For now we configure port 1 in device mode
> +      * when OTG is requested.
> +      */
> +     if ((isp->devflags & ISP1760_FLAG_ISP1761) &&
> +         (isp->devflags & ISP1760_FLAG_OTG_EN))
> +             otgctrl = ((HW_DM_PULLDOWN | HW_DP_PULLDOWN) << 16)
> +                     | HW_OTG_DISABLE;
> +     else
> +             otgctrl = (HW_SW_SEL_HC_DC << 16)
> +                     | (HW_VBUS_DRV | HW_SEL_CP_EXT);
> +
> +     isp1760_write32(isp->regs, HC_PORT1_CTRL, otgctrl);
> +     usleep_range(10000, 11000);

do you need a comment on this usleep_range() ? Perhaps pointing to a
section of documentation which state this is necessary ? 10ms is quite a
bit of wait.

> +
>       dev_info(isp->dev, "bus width: %u, oc: %s\n",
>                isp->devflags & ISP1760_FLAG_BUS_WIDTH_16 ? 16 : 32,
>                isp->devflags & ISP1760_FLAG_ANALOG_OC ? "analog" : "digital");
>  }
>  
> +void isp1760_set_pullup(struct isp1760_device *isp, bool enable)
> +{
> +     isp1760_write32(isp->regs, HW_OTG_CTRL_SET,
> +                     enable ? HW_DP_PULLUP : HW_DP_PULLUP << 16);
> +}
> +
>  int isp1760_register(struct resource *mem, int irq, unsigned long irqflags,
>                    int rst_gpio, struct device *dev, unsigned int devflags)
>  {
> @@ -126,6 +163,13 @@ int isp1760_register(struct resource *mem, int irq, 
> unsigned long irqflags,
>       if (ret < 0)
>               goto error;
>  
> +     if (devflags & ISP1760_FLAG_ISP1761) {
> +             ret = isp1760_udc_register(isp, irq, irqflags | IRQF_SHARED |
> +                                        IRQF_DISABLED);
> +             if (ret < 0)
> +                     goto error;
> +     }
> +
>       dev_set_drvdata(dev, isp);
>  
>       return 0;
> @@ -148,6 +192,9 @@ void isp1760_unregister(struct device *dev)
>  
>       release_mem_region(isp->mem_start, isp->mem_size);
>  
> +     if (isp->devflags & ISP1760_FLAG_ISP1761)
> +             isp1760_udc_unregister(isp);
> +
>       isp1760_hcd_unregister(&isp->hcd);
>  
>       iounmap(isp->regs);
> @@ -162,6 +209,11 @@ void isp1760_shutdown(struct device *dev)
>  {
>       struct isp1760_device *isp = dev_get_drvdata(dev);
>  
> +     if (isp->devflags & ISP1760_FLAG_ISP1761) {
> +             isp1760_write32(isp->regs, DC_MODE, DC_SFRESET);
> +             isp1760_write32(isp->regs, DC_MODE, 0);
> +     }

looks like you need to fix commit log of previous patch where you state
you'd maybe reset device side too, but that code only comes in here.

> +
>       isp1760_write32(isp->regs, HC_RESET_REG, SW_RESET_RESET_HC);
>  }
>  
> diff --git a/drivers/usb/host/isp1760-core.h b/drivers/usb/host/isp1760-core.h
> index fea1622..7d3aad4 100644
> --- a/drivers/usb/host/isp1760-core.h
> +++ b/drivers/usb/host/isp1760-core.h
> @@ -19,6 +19,7 @@
>  #include <linux/ioport.h>
>  
>  #include "isp1760-hcd.h"
> +#include "isp1760-udc.h"
>  
>  struct device;
>  
> @@ -48,6 +49,7 @@ struct isp1760_device {
>       int rst_gpio;
>  
>       struct isp1760_hcd hcd;
> +     struct isp1760_udc udc;
>  };
>  
>  int isp1760_register(struct resource *mem, int irq, unsigned long irqflags,
> @@ -55,6 +57,8 @@ int isp1760_register(struct resource *mem, int irq, 
> unsigned long irqflags,
>  void isp1760_unregister(struct device *dev);
>  void isp1760_shutdown(struct device *dev);
>  
> +void isp1760_set_pullup(struct isp1760_device *isp, bool enable);
> +
>  static inline u32 isp1760_read32(void __iomem *base, u32 reg)
>  {
>       return readl(base + reg);
> diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
> index a6081d5..1e3d1d9 100644
> --- a/drivers/usb/host/isp1760-hcd.c
> +++ b/drivers/usb/host/isp1760-hcd.c
> @@ -502,15 +502,6 @@ static int isp1760_hc_setup(struct usb_hcd *hcd)
>  
>       reg_write32(hcd->regs, HC_INTERRUPT_ENABLE, INTERRUPT_ENABLE_MASK);
>  
> -     /*
> -      * PORT 1 Control register of the ISP1760 is the OTG control
> -      * register on ISP1761. Since there is no OTG or device controller
> -      * support in this driver, we use port 1 as a "normal" USB host port on
> -      * both chips.
> -      */
> -     reg_write32(hcd->regs, HC_PORT1_CTRL, PORT1_POWER | PORT1_INIT2);
> -     mdelay(10);
> -

looks like moving this code should be part of its own patch.

>       priv->hcs_params = reg_read32(hcd->regs, HC_HCSPARAMS);
>  
>       return priv_init(hcd);
> diff --git a/drivers/usb/host/isp1760-regs.h b/drivers/usb/host/isp1760-regs.h
> index c2a3900..b67095c 100644
> --- a/drivers/usb/host/isp1760-regs.h
> +++ b/drivers/usb/host/isp1760-regs.h
> @@ -16,6 +16,10 @@
>  #ifndef _ISP1760_REGS_H_
>  #define _ISP1760_REGS_H_
>  
> +/* 
> -----------------------------------------------------------------------------
> + * Host Controller
> + */
> +
>  /* EHCI capability registers */
>  #define HC_CAPLENGTH         0x000
>  #define HC_LENGTH(p)         (((p) >> 00) & 0x00ff)  /* bits 7:0 */
> @@ -70,6 +74,9 @@
>  #define HC_HW_MODE_CTRL              0x300
>  #define ALL_ATX_RESET                (1 << 31)
>  #define HW_ANA_DIGI_OC               (1 << 15)
> +#define HW_DEV_DMA           (1 << 11)
> +#define HW_COMN_IRQ          (1 << 10)
> +#define HW_COMN_DMA          (1 << 9)
>  #define HW_DATA_BUS_32BIT    (1 << 8)
>  #define HW_DACK_POL_HIGH     (1 << 6)
>  #define HW_DREQ_POL_HIGH     (1 << 5)
> @@ -98,6 +105,17 @@
>  #define PORT1_INIT2          (1 << 23)
>  #define HW_OTG_CTRL_SET              0x374
>  #define HW_OTG_CTRL_CLR              0x376
> +#define HW_OTG_DISABLE               (1 << 10)
> +#define HW_OTG_SE0_EN                (1 << 9)
> +#define HW_BDIS_ACON_EN              (1 << 8)
> +#define HW_SW_SEL_HC_DC              (1 << 7)
> +#define HW_VBUS_CHRG         (1 << 6)
> +#define HW_VBUS_DISCHRG              (1 << 5)
> +#define HW_VBUS_DRV          (1 << 4)
> +#define HW_SEL_CP_EXT                (1 << 3)
> +#define HW_DM_PULLDOWN               (1 << 2)
> +#define HW_DP_PULLDOWN               (1 << 1)
> +#define HW_DP_PULLUP         (1 << 0)
>  
>  /* Interrupt Register */
>  #define HC_INTERRUPT_REG     0x310
> @@ -117,4 +135,96 @@
>  #define HC_INT_IRQ_MASK_AND_REG      0x328
>  #define HC_ATL_IRQ_MASK_AND_REG      0x32c
>  
> +/* 
> -----------------------------------------------------------------------------
> + * Peripheral Controller
> + */
> +
> +/* Initialization Registers */
> +#define DC_ADDRESS                   0x0200
> +#define DC_DEVEN                     (1 << 7)
> +
> +#define DC_MODE                              0x020c
> +#define DC_DMACLKON                  (1 << 9)
> +#define DC_VBUSSTAT                  (1 << 8)
> +#define DC_CLKAON                    (1 << 7)
> +#define DC_SNDRSU                    (1 << 6)
> +#define DC_GOSUSP                    (1 << 5)
> +#define DC_SFRESET                   (1 << 4)
> +#define DC_GLINTENA                  (1 << 3)
> +#define DC_WKUPCS                    (1 << 2)
> +
> +#define DC_INTCONF                   0x0210
> +#define DC_CDBGMOD_ACK_NAK           (0 << 6)
> +#define DC_CDBGMOD_ACK                       (1 << 6)
> +#define DC_CDBGMOD_ACK_1NAK          (2 << 6)
> +#define DC_DDBGMODIN_ACK_NAK         (0 << 4)
> +#define DC_DDBGMODIN_ACK             (1 << 4)
> +#define DC_DDBGMODIN_ACK_1NAK                (2 << 4)
> +#define DC_DDBGMODOUT_ACK_NYET_NAK   (0 << 2)
> +#define DC_DDBGMODOUT_ACK_NYET               (1 << 2)
> +#define DC_DDBGMODOUT_ACK_NYET_1NAK  (2 << 2)
> +#define DC_INTLVL                    (1 << 1)
> +#define DC_INTPOL                    (1 << 0)
> +
> +#define DC_DEBUG                     0x0212
> +#define DC_INTENABLE                 0x0214
> +#define DC_IEPTX(n)                  (1 << (11 + 2 * (n)))
> +#define DC_IEPRX(n)                  (1 << (10 + 2 * (n)))
> +#define DC_IEPRXTX(n)                        (3 << (10 + 2 * (n)))
> +#define DC_IEP0SETUP                 (1 << 8)
> +#define DC_IEVBUS                    (1 << 7)
> +#define DC_IEDMA                     (1 << 6)
> +#define DC_IEHS_STA                  (1 << 5)
> +#define DC_IERESM                    (1 << 4)
> +#define DC_IESUSP                    (1 << 3)
> +#define DC_IEPSOF                    (1 << 2)
> +#define DC_IESOF                     (1 << 1)
> +#define DC_IEBRST                    (1 << 0)
> +
> +/* Data Flow Registers */
> +#define DC_EPINDEX                   0x022c
> +#define DC_EP0SETUP                  (1 << 5)
> +#define DC_ENDPIDX(n)                        ((n) << 1)
> +#define DC_EPDIR                     (1 << 0)
> +
> +#define DC_CTRLFUNC                  0x0228
> +#define DC_CLBUF                     (1 << 4)
> +#define DC_VENDP                     (1 << 3)
> +#define DC_DSEN                              (1 << 2)
> +#define DC_STATUS                    (1 << 1)
> +#define DC_STALL                     (1 << 0)
> +
> +#define DC_DATAPORT                  0x0220
> +#define DC_BUFLEN                    0x021c
> +#define DC_DATACOUNT_MASK            0xffff
> +#define DC_BUFSTAT                   0x021e
> +#define DC_EPMAXPKTSZ                        0x0204
> +
> +#define DC_EPTYPE                    0x0208
> +#define DC_NOEMPKT                   (1 << 4)
> +#define DC_EPENABLE                  (1 << 3)
> +#define DC_DBLBUF                    (1 << 2)
> +#define DC_ENDPTYP_ISOC                      (1 << 0)
> +#define DC_ENDPTYP_BULK                      (2 << 0)
> +#define DC_ENDPTYP_INTERRUPT         (3 << 0)
> +
> +/* DMA Registers */
> +#define DC_DMACMD                    0x0230
> +#define DC_DMATXCOUNT                        0x0234
> +#define DC_DMACONF                   0x0238
> +#define DC_DMAHW                     0x023c
> +#define DC_DMAINTREASON                      0x0250
> +#define DC_DMAINTEN                  0x0254
> +#define DC_DMAEP                     0x0258
> +#define DC_DMABURSTCOUNT             0x0264
> +
> +/* General Registers */
> +#define DC_INTERRUPT                 0x0218
> +#define DC_CHIPID                    0x0270
> +#define DC_FRAMENUM                  0x0274
> +#define DC_SCRATCH                   0x0278
> +#define DC_UNLOCKDEV                 0x027c
> +#define DC_INTPULSEWIDTH             0x0280
> +#define DC_TESTMODE                  0x0284
> +
>  #endif
> diff --git a/drivers/usb/host/isp1760-udc.c b/drivers/usb/host/isp1760-udc.c
> new file mode 100644
> index 0000000..2f447ed
> --- /dev/null
> +++ b/drivers/usb/host/isp1760-udc.c
> @@ -0,0 +1,1418 @@
> +/*
> + * Driver for the NXP ISP1761 device controller
> + *
> + * Copyright 2014 Ideas on Board Oy
> + *
> + * Contacts:
> + *   Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/usb.h>
> +
> +#include "isp1760-core.h"
> +#include "isp1760-regs.h"
> +#include "isp1760-udc.h"
> +
> +struct isp1760_request {
> +     struct usb_request req;
> +     struct list_head queue;
> +     struct isp1760_ep *ep;
> +     unsigned int packet_size;
> +};
> +
> +static inline struct isp1760_udc *gadget_to_udc(struct usb_gadget *gadget)
> +{
> +     return container_of(gadget, struct isp1760_udc, gadget);
> +}
> +
> +static inline struct isp1760_ep *ep_to_udc_ep(struct usb_ep *ep)
> +{
> +     return container_of(ep, struct isp1760_ep, ep);
> +}
> +
> +static inline struct isp1760_request *req_to_udc_req(struct usb_request *req)
> +{
> +     return container_of(req, struct isp1760_request, req);
> +}
> +
> +static inline u32 isp1760_udc_read(struct isp1760_udc *udc, u16 reg)
> +{
> +     return isp1760_read32(udc->regs, reg);
> +}
> +
> +static inline void isp1760_udc_write(struct isp1760_udc *udc, u16 reg, u32 
> val)
> +{
> +     isp1760_write32(udc->regs, reg, val);
> +}
> +
> +/* 
> -----------------------------------------------------------------------------
> + * Endpoint Management
> + */
> +
> +static struct isp1760_ep *isp1760_udc_find_ep(struct isp1760_udc *udc,
> +                                           u16 index)
> +{
> +     unsigned int i;
> +
> +     if (index == 0)
> +             return &udc->ep[0];
> +
> +     for (i = 1; i < ARRAY_SIZE(udc->ep); ++i) {
> +             if (udc->ep[i].addr == index)
> +                     return udc->ep[i].desc ? &udc->ep[i] : NULL;
> +     }
> +
> +     return NULL;
> +}
> +
> +static void __isp1760_udc_select_ep(struct isp1760_ep *ep, int dir)
> +{
> +     isp1760_udc_write(ep->udc, DC_EPINDEX,
> +                       DC_ENDPIDX(ep->addr & USB_ENDPOINT_NUMBER_MASK) |
> +                       (dir == USB_DIR_IN ? DC_EPDIR : 0));
> +}
> +
> +/**
> + * isp1760_udc_select_ep - Select an endpoint for register access
> + * @ep: The endpoint
> + *
> + * The ISP1761 endpoint registers are banked. This function selects the 
> target
> + * endpoint for banked register access. The selection remains valid until the
> + * next call to this function, the next direct access to the EPINDEX register
> + * or the next reset, whichever comes first.
> + *
> + * Called with the UDC spinlock held.
> + */
> +static void isp1760_udc_select_ep(struct isp1760_ep *ep)
> +{
> +     __isp1760_udc_select_ep(ep, ep->addr & USB_ENDPOINT_DIR_MASK);
> +}
> +
> +/* Called with the UDC spinlock held. */
> +static void isp1760_udc_ctrl_send_status(struct isp1760_ep *ep, int dir)
> +{
> +     struct isp1760_udc *udc = ep->udc;
> +
> +     /*
> +      * Proceed to the status stage. The status stage data packet flows in
> +      * the direction opposite to the data stage data packets, we thus need
> +      * to select the OUT/IN endpoint for IN/OUT transfers.
> +      */
> +     isp1760_udc_write(udc, DC_EPINDEX, DC_ENDPIDX(0) |
> +                       (dir == USB_DIR_IN ? 0 : DC_EPDIR));
> +     isp1760_udc_write(udc, DC_CTRLFUNC, DC_STATUS);
> +
> +     udc->ep0_state = ISP1760_CTRL_SETUP;
> +}
> +
> +/* Called without the UDC spinlock held. */
> +static void isp1760_udc_request_complete(struct isp1760_ep *ep,
> +                                      struct isp1760_request *req,
> +                                      int status)
> +{
> +     struct isp1760_udc *udc = ep->udc;
> +     unsigned long flags;
> +
> +     dev_dbg(ep->udc->isp->dev, "completing request %p with status %d\n",
> +             req, status);
> +
> +     req->ep = NULL;
> +     req->req.status = status;
> +     req->req.complete(&ep->ep, &req->req);
> +
> +     spin_lock_irqsave(&udc->lock, flags);
> +
> +     /*
> +      * When completing control OUT requests, move to the status stage after
> +      * calling the request complete callback. This gives the gadget an
> +      * opportunity to stall the control transfer if needed.
> +      */
> +     if (status == 0 && ep->addr == 0 &&
> +         udc->ep0_state == ISP1760_CTRL_DATA_OUT)
> +             isp1760_udc_ctrl_send_status(ep, USB_DIR_OUT);
> +
> +     spin_unlock_irqrestore(&udc->lock, flags);
> +}
> +
> +static void isp1760_udc_ctrl_send_stall(struct isp1760_ep *ep)
> +{
> +     struct isp1760_udc *udc = ep->udc;
> +     unsigned long flags;
> +
> +     dev_dbg(ep->udc->isp->dev, "%s(ep%02x)\n", __func__, ep->addr);
> +
> +     spin_lock_irqsave(&udc->lock, flags);
> +
> +     /* Stall both the IN and OUT endpoints. */
> +     __isp1760_udc_select_ep(ep, USB_DIR_OUT);
> +     isp1760_udc_write(udc, DC_CTRLFUNC, DC_STALL);
> +     __isp1760_udc_select_ep(ep, USB_DIR_IN);
> +     isp1760_udc_write(udc, DC_CTRLFUNC, DC_STALL);
> +
> +     /* A protocol stall completes the control transaction. */
> +     udc->ep0_state = ISP1760_CTRL_SETUP;
> +
> +     spin_unlock_irqrestore(&udc->lock, flags);
> +}
> +
> +/* 
> -----------------------------------------------------------------------------
> + * Data Endpoints
> + */
> +
> +/* Called with the UDC spinlock held. */
> +static bool isp1760_udc_receive(struct isp1760_ep *ep,
> +                             struct isp1760_request *req)
> +{
> +     struct isp1760_udc *udc = ep->udc;
> +     unsigned int len;
> +     u32 *buf;
> +     int i;
> +
> +     isp1760_udc_select_ep(ep);
> +     len = isp1760_udc_read(udc, DC_BUFLEN) & DC_DATACOUNT_MASK;
> +
> +     dev_dbg(udc->isp->dev, "%s: received %u bytes (%u/%u done)\n",
> +             __func__, len, req->req.actual, req->req.length);
> +
> +     len = min(len, req->req.length - req->req.actual);
> +
> +     if (!len) {
> +             /*
> +              * There's no data to be read from the FIFO, acknowledge the RX
> +              * interrupt by clearing the buffer.
> +              *
> +              * FIXME: What if another packet arrives in the meantime ?
> +              */
> +             isp1760_udc_write(udc, DC_CTRLFUNC, DC_CLBUF);
> +             return false;
> +     }
> +
> +     buf = req->req.buf + req->req.actual;
> +
> +     /*
> +      * Make sure not to read more than one extra byte, otherwise data from
> +      * the next packet might be removed from the FIFO.
> +      */
> +     for (i = len; i > 2; i -= 4, ++buf)
> +             *buf = le32_to_cpu(isp1760_udc_read(udc, DC_DATAPORT));
> +     if (i > 0)
> +             *(u16 *)buf = le16_to_cpu(readw(udc->regs + DC_DATAPORT));
> +
> +     req->req.actual += len;
> +
> +     /* TODO Support short_not_ok */
> +
> +     dev_dbg(udc->isp->dev,
> +             "%s: req %p actual/length %u/%u maxpacket %u packet size %u\n",
> +             __func__, req, req->req.actual, req->req.length, ep->maxpacket,
> +             len);
> +
> +     ep->rx_pending = false;
> +
> +     /*
> +      * Complete the request if all data has been received or if a short
> +      * packet has been received.
> +      */
> +     if (req->req.actual == req->req.length || len < ep->maxpacket) {
> +             list_del(&req->queue);
> +             return true;
> +     }
> +
> +     return false;
> +}
> +
> +static void isp1760_udc_transmit(struct isp1760_ep *ep,
> +                              struct isp1760_request *req)
> +{
> +     struct isp1760_udc *udc = ep->udc;
> +     u32 *buf = req->req.buf + req->req.actual;
> +     int i;
> +
> +     req->packet_size = min(req->req.length - req->req.actual,
> +                            ep->maxpacket);
> +
> +     dev_dbg(udc->isp->dev, "%s: transferring %u bytes (%u/%u done)\n",
> +             __func__, req->packet_size, req->req.actual,
> +             req->req.length);
> +
> +     __isp1760_udc_select_ep(ep, USB_DIR_IN);
> +
> +     if (req->packet_size)
> +             isp1760_udc_write(udc, DC_BUFLEN, req->packet_size);
> +
> +     /*
> +      * Make sure not to write more than one extra byte, otherwise extra data
> +      * will stay in the FIFO and will be transmitted during the next control
> +      * request. The endpoint control CLBUF bit is supposed to allow flushing
> +      * the FIFO for this kind of conditions, but doesn't seem to work.
> +      */
> +     for (i = req->packet_size; i > 2; i -= 4, ++buf)
> +             isp1760_udc_write(udc, DC_DATAPORT, cpu_to_le32(*buf));
> +     if (i > 0)
> +             writew(cpu_to_le16(*(u16 *)buf), udc->regs + DC_DATAPORT);
> +
> +     if (ep->addr == 0)
> +             isp1760_udc_write(udc, DC_CTRLFUNC, DC_DSEN);
> +     if (!req->packet_size)
> +             isp1760_udc_write(udc, DC_CTRLFUNC, DC_VENDP);
> +}
> +
> +static void isp1760_ep_rx_ready(struct isp1760_ep *ep)
> +{
> +     struct isp1760_udc *udc = ep->udc;
> +     struct isp1760_request *req;
> +     bool complete;
> +
> +     spin_lock(&udc->lock);
> +
> +     if (ep->addr == 0 && udc->ep0_state != ISP1760_CTRL_DATA_OUT) {
> +             spin_unlock(&udc->lock);
> +             dev_dbg(udc->isp->dev, "%s: invalid ep0 state %u\n", __func__,
> +                     udc->ep0_state);
> +             return;
> +     }
> +
> +     if (ep->addr != 0 && !ep->desc) {
> +             spin_unlock(&udc->lock);
> +             dev_dbg(udc->isp->dev, "%s: ep%02x is disabled\n", __func__,
> +                     ep->addr);
> +             return;
> +     }
> +
> +     if (list_empty(&ep->queue)) {
> +             ep->rx_pending = true;
> +             spin_unlock(&udc->lock);
> +             dev_dbg(udc->isp->dev, "%s: ep%02x (%p) has no request 
> queued\n",
> +                     __func__, ep->addr, ep);
> +             return;
> +     }
> +
> +     req = list_first_entry(&ep->queue, struct isp1760_request,
> +                            queue);
> +     complete = isp1760_udc_receive(ep, req);
> +
> +     spin_unlock(&udc->lock);
> +
> +     if (complete)
> +             isp1760_udc_request_complete(ep, req, 0);
> +}
> +
> +static void isp1760_ep_tx_complete(struct isp1760_ep *ep)
> +{
> +     struct isp1760_udc *udc = ep->udc;
> +     struct isp1760_request *complete = NULL;
> +     struct isp1760_request *req;
> +     bool need_zlp;
> +
> +     spin_lock(&udc->lock);
> +
> +     if (ep->addr == 0 && udc->ep0_state != ISP1760_CTRL_DATA_IN) {
> +             spin_unlock(&udc->lock);
> +             dev_dbg(udc->isp->dev, "TX IRQ: invalid endpoint state %u\n",
> +                     udc->ep0_state);
> +             return;
> +     }
> +
> +     if (list_empty(&ep->queue)) {
> +             /*
> +              * This can happen for the control endpoint when the reply to
> +              * the GET_STATUS IN control request is sent directly by the
> +              * setup IRQ handler. Just proceed to the status stage.
> +              */
> +             if (ep->addr == 0) {
> +                     isp1760_udc_ctrl_send_status(ep, USB_DIR_IN);
> +                     spin_unlock(&udc->lock);
> +                     return;
> +             }
> +
> +             spin_unlock(&udc->lock);
> +             dev_dbg(udc->isp->dev, "%s: ep%02x has no request queued\n",
> +                     __func__, ep->addr);
> +             return;
> +     }
> +
> +     req = list_first_entry(&ep->queue, struct isp1760_request,
> +                            queue);
> +     req->req.actual += req->packet_size;
> +
> +     need_zlp = req->req.actual == req->req.length &&
> +                !(req->req.length % ep->maxpacket) &&
> +                req->packet_size && req->req.zero;
> +
> +     dev_dbg(udc->isp->dev,
> +             "TX IRQ: req %p actual/length %u/%u maxpacket %u packet size %u 
> zero %u need zlp %u\n",
> +              req, req->req.actual, req->req.length, ep->maxpacket,
> +              req->packet_size, req->req.zero, need_zlp);
> +
> +     /*
> +      * Complete the request if all data has been sent and we don't need to
> +      * transmit a zero length packet.
> +      */
> +     if (req->req.actual == req->req.length && !need_zlp) {
> +             complete = req;
> +             list_del(&req->queue);
> +
> +             if (ep->addr == 0)
> +                     isp1760_udc_ctrl_send_status(ep, USB_DIR_IN);
> +
> +             if (!list_empty(&ep->queue))
> +                     req = list_first_entry(&ep->queue,
> +                                            struct isp1760_request, queue);
> +             else
> +                     req = NULL;
> +     }
> +
> +     /* Transmit the next packet or start the next request, if any. */
> +     /*
> +      * TODO: If the endpoint is stalled the next request shouldn't be
> +      * started, but what about the next packet ?
> +      */
> +     if (req)
> +             isp1760_udc_transmit(ep, req);
> +
> +     spin_unlock(&udc->lock);
> +
> +     if (complete)
> +             isp1760_udc_request_complete(ep, complete, 0);
> +}
> +
> +static int __isp1760_udc_set_halt(struct isp1760_ep *ep, bool halt)
> +{
> +     struct isp1760_udc *udc = ep->udc;
> +
> +     dev_dbg(udc->isp->dev, "%s: %s halt on ep%02x\n", __func__,
> +             halt ? "set" : "clear", ep->addr);
> +
> +     if (ep->desc && usb_endpoint_xfer_isoc(ep->desc)) {
> +             dev_dbg(udc->isp->dev, "%s: ep%02x is isochronous\n", __func__,
> +                     ep->addr);
> +             return -EINVAL;
> +     }
> +
> +     isp1760_udc_select_ep(ep);
> +     isp1760_udc_write(udc, DC_CTRLFUNC, halt ? DC_STALL : 0);
> +
> +     if (ep->addr == 0) {
> +             /* When halting the control endpoint, stall both IN and OUT. */
> +             __isp1760_udc_select_ep(ep, USB_DIR_IN);
> +             isp1760_udc_write(udc, DC_CTRLFUNC, halt ? DC_STALL : 0);
> +     } else if (!halt) {
> +             /* Reset the data PID by cycling the endpoint enable bit. */
> +             u16 eptype = isp1760_udc_read(udc, DC_EPTYPE);
> +
> +             isp1760_udc_write(udc, DC_EPTYPE, eptype & ~DC_EPENABLE);
> +             isp1760_udc_write(udc, DC_EPTYPE, eptype);
> +
> +             /*
> +              * Disabling the endpoint emptied the transmit FIFO, fill it
> +              * again if a request is pending.
> +              *
> +              * TODO: Synchronize with the TX IRQ handler ?
> +              */
> +             if ((ep->addr & USB_DIR_IN) && !list_empty(&ep->queue)) {
> +                     struct isp1760_request *req;
> +
> +                     req = list_first_entry(&ep->queue,
> +                                            struct isp1760_request, queue);
> +                     isp1760_udc_transmit(ep, req);
> +             }
> +     }
> +
> +     ep->halted = halt;
> +
> +     return 0;
> +}
> +
> +/* 
> -----------------------------------------------------------------------------
> + * Control Endpoint
> + */
> +
> +static int isp1760_udc_get_status(struct isp1760_udc *udc,
> +                               const struct usb_ctrlrequest *req)
> +{
> +     struct isp1760_ep *ep;
> +     u16 status;
> +
> +     if (req->wLength != cpu_to_le16(2) || req->wValue != cpu_to_le16(0))
> +             return -EINVAL;
> +
> +     switch (req->bRequestType) {
> +     case USB_DIR_IN | USB_RECIP_DEVICE:
> +             status = udc->devstatus;
> +             break;
> +
> +     case USB_DIR_IN | USB_RECIP_INTERFACE:
> +             status = 0;
> +             break;
> +
> +     case USB_DIR_IN | USB_RECIP_ENDPOINT:
> +             ep = isp1760_udc_find_ep(udc, le16_to_cpu(req->wIndex));
> +             if (!ep)
> +                     return -EINVAL;
> +
> +             status = 0;
> +             if (ep->halted)
> +                     status |= 1 << USB_ENDPOINT_HALT;
> +             break;
> +
> +     default:
> +             return -EINVAL;
> +     }
> +
> +     isp1760_udc_write(udc, DC_EPINDEX, DC_ENDPIDX(0) | DC_EPDIR);
> +     isp1760_udc_write(udc, DC_BUFLEN, 2);
> +
> +     writew(cpu_to_le16(status), udc->regs + DC_DATAPORT);
> +
> +     isp1760_udc_write(udc, DC_CTRLFUNC, DC_DSEN);
> +
> +     dev_dbg(udc->isp->dev, "%s: status 0x%04x\n", __func__, status);
> +
> +     return 0;
> +}
> +
> +static int isp1760_udc_set_address(struct isp1760_udc *udc, u16 addr)
> +{
> +     if (addr > 127) {
> +             dev_dbg(udc->isp->dev, "invalid device address %u\n", addr);
> +             return -EINVAL;
> +     }

if you want to be pedantic, you could add a check here to stall
SetAdress when gadget is already in CONFIGURED state.

> +
> +     usb_gadget_set_state(&udc->gadget, addr ? USB_STATE_ADDRESS :
> +                          USB_STATE_DEFAULT);
> +
> +     isp1760_udc_write(udc, DC_ADDRESS, DC_DEVEN | addr);
> +
> +     spin_lock(&udc->lock);
> +     isp1760_udc_ctrl_send_status(&udc->ep[0], USB_DIR_OUT);
> +     spin_unlock(&udc->lock);
> +
> +     return 0;
> +}
> +
> +static bool isp1760_ep0_setup_standard(struct isp1760_udc *udc,
> +                                    struct usb_ctrlrequest *req)
> +{
> +     bool stall;
> +
> +     switch (req->bRequest) {
> +     case USB_REQ_GET_STATUS:
> +             return isp1760_udc_get_status(udc, req);
> +
> +     case USB_REQ_CLEAR_FEATURE:
> +             switch (req->bRequestType) {
> +             case USB_DIR_OUT | USB_RECIP_DEVICE: {
> +                     /* TODO: Handle remote wakeup feature */
> +                     return true;
> +             }
> +
> +             case USB_DIR_OUT | USB_RECIP_ENDPOINT: {
> +                     u16 index = le16_to_cpu(req->wIndex);
> +                     struct isp1760_ep *ep;
> +
> +                     if (req->wLength != cpu_to_le16(0) ||
> +                         req->wValue != cpu_to_le16(USB_ENDPOINT_HALT))
> +                             return true;
> +
> +                     ep = isp1760_udc_find_ep(udc, index);
> +                     if (!ep)
> +                             return true;
> +
> +                     spin_lock(&udc->lock);
> +
> +                     /*
> +                      * If the endpoint is wedged only the gadget can clear
> +                      * the halt feature. Pretend success in that case, but
> +                      * keep the endpoint halted.
> +                      */
> +                     if (!ep->wedged)
> +                             stall = __isp1760_udc_set_halt(ep, false);
> +                     else
> +                             stall = false;
> +
> +                     if (!stall)
> +                             isp1760_udc_ctrl_send_status(&udc->ep[0],
> +                                                          USB_DIR_OUT);
> +
> +                     spin_unlock(&udc->lock);
> +                     return stall;
> +             }
> +
> +             default:
> +                     return true;
> +             }
> +             break;
> +
> +     case USB_REQ_SET_FEATURE:
> +             switch (req->bRequestType) {
> +             case USB_DIR_OUT | USB_RECIP_DEVICE: {
> +                     /* TODO: Handle remote wakeup and test mode features */
> +                     return true;
> +             }
> +
> +             case USB_DIR_OUT | USB_RECIP_ENDPOINT: {
> +                     u16 index = le16_to_cpu(req->wIndex);
> +                     struct isp1760_ep *ep;
> +
> +                     if (req->wLength != cpu_to_le16(0) ||
> +                         req->wValue != cpu_to_le16(USB_ENDPOINT_HALT))
> +                             return true;
> +
> +                     ep = isp1760_udc_find_ep(udc, index);
> +                     if (!ep)
> +                             return true;
> +
> +                     /*
> +                      * TODO The stall feature must be reset to 0 by a
> +                      * SET_CONFIGURATION or SET_INTERFACE request.
> +                      */
> +                     spin_lock(&udc->lock);
> +
> +                     stall = __isp1760_udc_set_halt(ep, true);
> +                     if (!stall)
> +                             isp1760_udc_ctrl_send_status(&udc->ep[0],
> +                                                          USB_DIR_OUT);
> +
> +                     spin_unlock(&udc->lock);
> +                     return stall;
> +             }
> +
> +             default:
> +                     return true;
> +             }
> +             break;
> +
> +     case USB_REQ_SET_ADDRESS:
> +             if (req->bRequestType != (USB_DIR_OUT | USB_RECIP_DEVICE))
> +                     return true;
> +
> +             return isp1760_udc_set_address(udc, le16_to_cpu(req->wValue));
> +
> +     case USB_REQ_SET_CONFIGURATION:
> +             if (req->bRequestType != (USB_DIR_OUT | USB_RECIP_DEVICE))
> +                     return true;
> +
> +             if (udc->gadget.state != USB_STATE_ADDRESS &&
> +                 udc->gadget.state != USB_STATE_CONFIGURED)
> +                     return true;
> +
> +             stall = udc->driver->setup(&udc->gadget, req) < 0;
> +             if (stall)
> +                     return true;
> +
> +             usb_gadget_set_state(&udc->gadget, req->wValue ?
> +                                  USB_STATE_CONFIGURED : USB_STATE_ADDRESS);
> +             return false;
> +
> +     default:
> +             return udc->driver->setup(&udc->gadget, req) < 0;
> +     }
> +}
> +
> +static void isp1760_ep0_setup(struct isp1760_udc *udc)
> +{
> +     union {
> +             struct usb_ctrlrequest r;
> +             u32 data[2];
> +     } req;
> +     unsigned int count;
> +     bool stall = false;
> +
> +     spin_lock(&udc->lock);
> +
> +     isp1760_udc_write(udc, DC_EPINDEX, DC_EP0SETUP);
> +
> +     count = isp1760_udc_read(udc, DC_BUFLEN) & DC_DATACOUNT_MASK;
> +     if (count != sizeof(req)) {
> +             spin_unlock(&udc->lock);
> +
> +             dev_err(udc->isp->dev, "invalid length %u for setup packet\n",
> +                     count);
> +
> +             isp1760_udc_ctrl_send_stall(&udc->ep[0]);
> +             return;
> +     }
> +
> +     req.data[0] = isp1760_udc_read(udc, DC_DATAPORT);
> +     req.data[1] = isp1760_udc_read(udc, DC_DATAPORT);
> +
> +     if (udc->ep0_state != ISP1760_CTRL_SETUP) {
> +             spin_unlock(&udc->lock);
> +             dev_dbg(udc->isp->dev, "unexpected SETUP packet\n");
> +             return;
> +     }
> +
> +     /* Move to the data stage. */
> +     if (req.r.bRequestType & USB_DIR_IN)
> +             udc->ep0_state = ISP1760_CTRL_DATA_IN;
> +     else
> +             udc->ep0_state = ISP1760_CTRL_DATA_OUT;

data phase is optional. This should be something like:

        if (req.r.wLength > 0) {
                if (req.r.bRequestType & USB_DIR_IN)
                        udc->ep0_state = ISP1760_CTRL_DATA_IN;
                else
                        udc->ep0_state = ISP1760_CTRL_DATA_OUT;
        } else {
                udc->ep0_state = ISP1760_CTRL_STATUS;
        }

> +     spin_unlock(&udc->lock);
> +
> +     dev_dbg(udc->isp->dev,
> +             "%s: bRequestType 0x%02x bRequest 0x%02x wValue 0x%04x wIndex 
> 0x%04x wLength 0x%04x\n",
> +             __func__, req.r.bRequestType, req.r.bRequest,
> +             le16_to_cpu(req.r.wValue), le16_to_cpu(req.r.wIndex),
> +             le16_to_cpu(req.r.wLength));
> +
> +     if ((req.r.bRequestType & USB_TYPE_MASK) == USB_TYPE_STANDARD)
> +             stall = isp1760_ep0_setup_standard(udc, &req.r);
> +     else
> +             stall = udc->driver->setup(&udc->gadget, &req.r) < 0;
> +
> +     if (stall)
> +             isp1760_udc_ctrl_send_stall(&udc->ep[0]);
> +}
> +
> +/* 
> -----------------------------------------------------------------------------
> + * Gadget Endpoint Operations
> + */
> +
> +static int isp1760_ep_enable(struct usb_ep *ep,
> +                          const struct usb_endpoint_descriptor *desc)
> +{
> +     struct isp1760_ep *uep = ep_to_udc_ep(ep);
> +     struct isp1760_udc *udc = uep->udc;
> +     unsigned long flags;
> +     unsigned int type;
> +
> +     dev_dbg(uep->udc->isp->dev, "%s\n", __func__);
> +
> +     /*
> +      * Validate the descriptor. The control endpoint can't be enabled
> +      * manually.
> +      */
> +     if (desc->bDescriptorType != USB_DT_ENDPOINT ||
> +         desc->bEndpointAddress == 0 ||
> +         desc->bEndpointAddress != uep->addr ||
> +         le16_to_cpu(desc->wMaxPacketSize) > ep->maxpacket) {
> +             dev_dbg(udc->isp->dev,
> +                     "%s: invalid descriptor type %u addr %02x ep addr %02x 
> max packet size %u/%u\n",
> +                     __func__, desc->bDescriptorType,
> +                     desc->bEndpointAddress, uep->addr,
> +                     le16_to_cpu(desc->wMaxPacketSize), ep->maxpacket);
> +             return -EINVAL;
> +     }
> +
> +     switch (usb_endpoint_type(desc)) {
> +     case USB_ENDPOINT_XFER_ISOC:
> +             type = DC_ENDPTYP_ISOC;
> +             break;
> +     case USB_ENDPOINT_XFER_BULK:
> +             type = DC_ENDPTYP_BULK;
> +             break;
> +     case USB_ENDPOINT_XFER_INT:
> +             type = DC_ENDPTYP_INTERRUPT;
> +             break;
> +     case USB_ENDPOINT_XFER_CONTROL:
> +     default:
> +             dev_dbg(udc->isp->dev, "%s: control endpoints unsupported\n",
> +                     __func__);
> +             return -EINVAL;
> +     }
> +
> +     spin_lock_irqsave(&udc->lock, flags);
> +
> +     uep->desc = desc;
> +     uep->maxpacket = le16_to_cpu(desc->wMaxPacketSize);
> +     uep->rx_pending = false;
> +     uep->halted = false;
> +     uep->wedged = false;
> +
> +     isp1760_udc_select_ep(uep);
> +     isp1760_udc_write(udc, DC_EPMAXPKTSZ, uep->maxpacket);
> +     isp1760_udc_write(udc, DC_BUFLEN, uep->maxpacket);
> +     isp1760_udc_write(udc, DC_EPTYPE, DC_EPENABLE | type);
> +
> +     spin_unlock_irqrestore(&udc->lock, flags);
> +
> +     return 0;
> +}
> +
> +static int isp1760_ep_disable(struct usb_ep *ep)
> +{
> +     struct isp1760_ep *uep = ep_to_udc_ep(ep);
> +     struct isp1760_udc *udc = uep->udc;
> +     struct isp1760_request *req, *nreq;
> +     LIST_HEAD(req_list);
> +     unsigned long flags;
> +
> +     dev_dbg(udc->isp->dev, "%s\n", __func__);
> +
> +     spin_lock_irqsave(&udc->lock, flags);
> +
> +     if (!uep->desc) {
> +             dev_dbg(udc->isp->dev, "%s: endpoint not enabled\n", __func__);
> +             spin_unlock_irqrestore(&udc->lock, flags);
> +             return -EINVAL;
> +     }
> +
> +     uep->desc = NULL;
> +     uep->maxpacket = 0;
> +
> +     isp1760_udc_select_ep(uep);
> +     isp1760_udc_write(udc, DC_EPTYPE, 0);
> +
> +     /* TODO Synchronize with the IRQ handler */
> +
> +     list_splice_init(&uep->queue, &req_list);
> +
> +     spin_unlock_irqrestore(&udc->lock, flags);
> +
> +     list_for_each_entry_safe(req, nreq, &req_list, queue) {
> +             list_del(&req->queue);
> +             isp1760_udc_request_complete(uep, req, -ESHUTDOWN);
> +     }
> +
> +     return 0;
> +}
> +
> +static struct usb_request *isp1760_ep_alloc_request(struct usb_ep *ep,
> +                                                 gfp_t gfp_flags)
> +{
> +     struct isp1760_request *req;
> +
> +     req = kzalloc(sizeof(*req), gfp_flags);
> +
> +     return &req->req;
> +}
> +
> +static void isp1760_ep_free_request(struct usb_ep *ep, struct usb_request 
> *_req)
> +{
> +     struct isp1760_request *req = req_to_udc_req(_req);
> +
> +     kfree(req);
> +}
> +
> +static int isp1760_ep_queue(struct usb_ep *ep, struct usb_request *_req,
> +                         gfp_t gfp_flags)
> +{
> +     struct isp1760_request *req = req_to_udc_req(_req);
> +     struct isp1760_ep *uep = ep_to_udc_ep(ep);
> +     struct isp1760_udc *udc = uep->udc;
> +     bool complete = false;
> +     unsigned long flags;
> +     int ret = 0;
> +
> +     _req->status = -EINPROGRESS;
> +     _req->actual = 0;
> +
> +     spin_lock_irqsave(&udc->lock, flags);
> +
> +     dev_dbg(udc->isp->dev,
> +             "%s: req %p (%u bytes%s) ep %p(0x%02x)\n", __func__, _req,
> +             _req->length, _req->zero ? " (zlp)" : "", uep, uep->addr);
> +
> +     req->ep = uep;
> +
> +     if (uep->addr == 0) {
> +             switch (udc->ep0_state) {
> +             case ISP1760_CTRL_DATA_IN:
> +                     dev_dbg(udc->isp->dev, "%s: transmitting req %p\n",
> +                             __func__, req);
> +
> +                     list_add_tail(&req->queue, &uep->queue);
> +                     isp1760_udc_transmit(uep, req);
> +                     break;
> +
> +             case ISP1760_CTRL_DATA_OUT:
> +                     if (_req->length) {
> +                             list_add_tail(&req->queue, &uep->queue);
> +                             __isp1760_udc_select_ep(uep, USB_DIR_OUT);
> +                             isp1760_udc_write(udc, DC_CTRLFUNC, DC_DSEN);
> +                     } else {
> +                             complete = true;
> +                     }
> +                     break;
> +
> +             default:
> +                     dev_dbg(udc->isp->dev, "%s: invalid ep0 state\n",
> +                             __func__);
> +                     ret = -EINVAL;
> +                     break;
> +             }
> +     } else if (uep->desc) {
> +             bool empty = list_empty(&uep->queue);
> +
> +             list_add_tail(&req->queue, &uep->queue);
> +             if ((uep->addr & USB_DIR_IN) && !uep->halted && empty)
> +                     isp1760_udc_transmit(uep, req);
> +             else if (!(uep->addr & USB_DIR_IN) && uep->rx_pending)
> +                     complete = isp1760_udc_receive(uep, req);
> +     } else {
> +             dev_dbg(udc->isp->dev,
> +                     "%s: can't queue request to disabled ep%02x\n",
> +                     __func__, uep->addr);
> +             ret = -ESHUTDOWN;
> +     }
> +
> +     if (ret < 0)
> +             req->ep = NULL;
> +
> +     spin_unlock_irqrestore(&udc->lock, flags);
> +
> +     if (complete)
> +             isp1760_udc_request_complete(uep, req, 0);
> +
> +     return ret;
> +}
> +
> +static int isp1760_ep_dequeue(struct usb_ep *ep, struct usb_request *_req)
> +{
> +     struct isp1760_request *req = req_to_udc_req(_req);
> +     struct isp1760_ep *uep = ep_to_udc_ep(ep);
> +     struct isp1760_udc *udc = uep->udc;
> +     unsigned long flags;
> +
> +     dev_dbg(uep->udc->isp->dev, "%s(ep%02x)\n", __func__, uep->addr);
> +
> +     spin_lock_irqsave(&udc->lock, flags);
> +
> +     if (req->ep != uep)
> +             req = NULL;
> +     else
> +             list_del(&req->queue);
> +
> +     spin_unlock_irqrestore(&udc->lock, flags);
> +
> +     if (!req)
> +             return -EINVAL;
> +
> +     isp1760_udc_request_complete(uep, req, -ECONNRESET);
> +     return 0;
> +}
> +
> +static int __isp1760_ep_set_halt(struct isp1760_ep *uep, bool stall, bool 
> wedge)
> +{
> +     struct isp1760_udc *udc = uep->udc;
> +     int ret;
> +
> +     if (!uep->addr) {
> +             /*
> +              * Halting the control endpoint is only valid as a delayed error
> +              * response to a SETUP packet. Make sure EP0 is in the right
> +              * stage and that the gadget isn't trying to clear the halt
> +              * condition.
> +              */
> +             if (WARN_ON((udc->ep0_state != ISP1760_CTRL_DATA_IN &&
> +                          udc->ep0_state != ISP1760_CTRL_DATA_OUT) ||

no, this isn't really correct. You can very well stall a CTRL request.
Imagine you get a ctrl request which isn't valid at all, you can stall
right there. A data phase can also be stalled if gadget expected for
e.g. DATA IN but host tried to move DATA OUT.

> +                         !stall || wedge)) {
> +                     return -EINVAL;
> +             }
> +     }
> +
> +     if (uep->addr && !uep->desc) {
> +             dev_dbg(udc->isp->dev, "%s: ep%02x is disabled\n", __func__,
> +                     uep->addr);
> +             return -EINVAL;
> +     }
> +
> +     if (uep->addr & USB_DIR_IN) {
> +             /* Refuse to halt IN endpoints with active transfers. */
> +             if (!list_empty(&uep->queue)) {
> +                     dev_dbg(udc->isp->dev,
> +                             "%s: ep%02x has request pending\n", __func__,
> +                             uep->addr);
> +                     return -EAGAIN;
> +             }
> +     }
> +
> +     ret = __isp1760_udc_set_halt(uep, stall);
> +     if (ret < 0)
> +             return ret;
> +
> +     if (!uep->addr) {
> +             /*
> +              * Stalling EP0 completes the control transaction, move back to
> +              * the SETUP state.
> +              */
> +             udc->ep0_state = ISP1760_CTRL_SETUP;
> +             return 0;
> +     }
> +
> +     if (wedge)
> +             uep->wedged = true;
> +     else if (!stall)
> +             uep->wedged = false;
> +
> +     return 0;
> +}
> +
> +static int isp1760_ep_set_halt(struct usb_ep *ep, int value)
> +{
> +     struct isp1760_ep *uep = ep_to_udc_ep(ep);
> +     unsigned long flags;
> +     int ret;
> +
> +     dev_dbg(uep->udc->isp->dev, "%s: %s halt on ep%02x\n", __func__,
> +             value ? "set" : "clear", uep->addr);
> +
> +     spin_lock_irqsave(&uep->udc->lock, flags);
> +     ret = __isp1760_ep_set_halt(uep, value, false);
> +     spin_unlock_irqrestore(&uep->udc->lock, flags);
> +
> +     return ret;
> +}
> +
> +static int isp1760_ep_set_wedge(struct usb_ep *ep)
> +{
> +     struct isp1760_ep *uep = ep_to_udc_ep(ep);
> +     unsigned long flags;
> +     int ret;
> +
> +     dev_dbg(uep->udc->isp->dev, "%s: set wedge on ep%02x)\n", __func__,
> +             uep->addr);
> +
> +     spin_lock_irqsave(&uep->udc->lock, flags);
> +     ret = __isp1760_ep_set_halt(uep, true, true);
> +     spin_unlock_irqrestore(&uep->udc->lock, flags);
> +
> +     return ret;
> +}
> +
> +static void isp1760_ep_fifo_flush(struct usb_ep *ep)
> +{
> +     struct isp1760_ep *uep = ep_to_udc_ep(ep);
> +     struct isp1760_udc *udc = uep->udc;
> +     unsigned long flags;
> +
> +     spin_lock_irqsave(&udc->lock, flags);
> +
> +     isp1760_udc_select_ep(uep);
> +
> +     /*
> +      * Set the CLBUF bit twice to flush both buffers in case double
> +      * buffering is enabled.
> +      */
> +     isp1760_udc_write(udc, DC_CTRLFUNC, DC_CLBUF);
> +     isp1760_udc_write(udc, DC_CTRLFUNC, DC_CLBUF);
> +
> +     spin_unlock_irqrestore(&udc->lock, flags);
> +}
> +
> +static const struct usb_ep_ops isp1760_ep_ops = {
> +     .enable = isp1760_ep_enable,
> +     .disable = isp1760_ep_disable,
> +     .alloc_request = isp1760_ep_alloc_request,
> +     .free_request = isp1760_ep_free_request,
> +     .queue = isp1760_ep_queue,
> +     .dequeue = isp1760_ep_dequeue,
> +     .set_halt = isp1760_ep_set_halt,
> +     .set_wedge = isp1760_ep_set_wedge,
> +     .fifo_flush = isp1760_ep_fifo_flush,
> +};
> +
> +/* 
> -----------------------------------------------------------------------------
> + * Device States
> + */
> +
> +static void isp1760_udc_disconnect(struct isp1760_udc *udc)
> +{
> +     if (udc->gadget.state < USB_STATE_POWERED)
> +             return;
> +
> +     dev_dbg(udc->isp->dev, "Device disconnected in state %u\n",
> +              udc->gadget.state);
> +
> +     udc->gadget.speed = USB_SPEED_UNKNOWN;
> +     usb_gadget_set_state(&udc->gadget, USB_STATE_ATTACHED);
> +
> +     if (udc->driver->disconnect)
> +             udc->driver->disconnect(&udc->gadget);

alright, so this can be called from your reset IRQ handler. You need a
little more inteligence on this branch because if you're coming from
reset IRQ you only want to call gadget driver's disconnect in case speed
!= UNKNOWN.

That has become easier, however, because of a recent patch adding
->reset() method which should be called from reset unconditionally and
->disconnect() will only be called from disconnect IRQ.

> +
> +     /* TODO Reset all endpoints ? */
> +}
> +
> +static void isp1760_udc_reset(struct isp1760_udc *udc)
> +{
> +     unsigned long flags;
> +
> +     /*
> +      * The device controller currently shares its interrupt with the host
> +      * controller, the DC_IRQ polarity and signaling mode are ignored. Set
> +      * the to active-low level-triggered.
> +      *
> +      * Configure the control, in and out pipes to generate interrupts on
> +      * ACK tokens only (and NYET for the out pipe). The default
> +      * configuration also generates an interrupt on the first NACK token.
> +      */
> +     isp1760_udc_write(udc, DC_INTCONF, DC_CDBGMOD_ACK | DC_DDBGMODIN_ACK |
> +                       DC_DDBGMODOUT_ACK_NYET);
> +
> +     isp1760_udc_write(udc, DC_INTENABLE, DC_IEPRXTX(7) | DC_IEPRXTX(6) |
> +                       DC_IEPRXTX(5) | DC_IEPRXTX(4) | DC_IEPRXTX(3) |
> +                       DC_IEPRXTX(2) | DC_IEPRXTX(1) | DC_IEPRXTX(0) |
> +                       DC_IEP0SETUP | DC_IEVBUS | DC_IERESM | DC_IESUSP |
> +                       DC_IEHS_STA | DC_IEBRST);
> +
> +     isp1760_udc_disconnect(udc);
> +
> +     if (udc->connected)
> +             isp1760_set_pullup(udc->isp, true);
> +
> +     spin_lock_irqsave(&udc->lock, flags);
> +
> +     isp1760_udc_write(udc, DC_ADDRESS, DC_DEVEN);
> +
> +     usb_gadget_set_state(&udc->gadget, USB_STATE_DEFAULT);
> +
> +     udc->ep0_state = ISP1760_CTRL_SETUP;
> +     udc->gadget.speed = USB_SPEED_FULL;
> +
> +     spin_unlock_irqrestore(&udc->lock, flags);
> +}
> +
> +static void isp1760_udc_suspend(struct isp1760_udc *udc)
> +{
> +     if (udc->gadget.state < USB_STATE_DEFAULT)
> +             return;
> +
> +     if (udc->driver->suspend)
> +             udc->driver->suspend(&udc->gadget);
> +}
> +
> +static void isp1760_udc_resume(struct isp1760_udc *udc)
> +{
> +     if (udc->gadget.state < USB_STATE_DEFAULT)
> +             return;
> +
> +     if (udc->driver->resume)
> +             udc->driver->resume(&udc->gadget);
> +}
> +
> +/* 
> -----------------------------------------------------------------------------
> + * Gadget Operations
> + */
> +
> +static int isp1760_udc_get_frame(struct usb_gadget *gadget)
> +{
> +     struct isp1760_udc *udc = gadget_to_udc(gadget);
> +
> +     return isp1760_udc_read(udc, DC_FRAMENUM) & ((1 << 11) - 1);
> +}
> +
> +static int isp1760_udc_wakeup(struct usb_gadget *gadget)
> +{
> +     struct isp1760_udc *udc = gadget_to_udc(gadget);
> +
> +     dev_dbg(udc->isp->dev, "%s\n", __func__);
> +     return -ENOTSUPP;
> +}
> +
> +static int isp1760_udc_set_selfpowered(struct usb_gadget *gadget,
> +                                    int is_selfpowered)
> +{
> +     struct isp1760_udc *udc = gadget_to_udc(gadget);
> +
> +     if (is_selfpowered)
> +             udc->devstatus |= 1 << USB_DEVICE_SELF_POWERED;
> +     else
> +             udc->devstatus &= ~(1 << USB_DEVICE_SELF_POWERED);
> +
> +     return 0;
> +}
> +
> +static int isp1760_udc_pullup(struct usb_gadget *gadget, int is_on)
> +{
> +     struct isp1760_udc *udc = gadget_to_udc(gadget);
> +
> +     isp1760_set_pullup(udc->isp, is_on);
> +     udc->connected = is_on;
> +
> +     return 0;
> +}
> +
> +static int isp1760_udc_start(struct usb_gadget *gadget,
> +                          struct usb_gadget_driver *driver)
> +{
> +     struct isp1760_udc *udc = gadget_to_udc(gadget);
> +
> +     /* The hardware doesn't support low speed. */
> +     if (driver->max_speed < USB_SPEED_FULL) {
> +             dev_err(udc->isp->dev, "Invalid gadget driver\n");
> +             return -EINVAL;
> +     }
> +
> +     spin_lock(&udc->lock);
> +
> +     if (udc->driver) {
> +             dev_err(udc->isp->dev, "UDC already has a gadget driver\n");
> +             spin_unlock(&udc->lock);
> +             return -EBUSY;
> +     }
> +
> +     udc->driver = driver;
> +
> +     spin_unlock(&udc->lock);
> +
> +     dev_dbg(udc->isp->dev, "starting UDC with driver %s\n",
> +             driver->function);
> +
> +     udc->devstatus = 0;
> +     udc->connected = true;
> +
> +     usb_gadget_set_state(&udc->gadget, USB_STATE_ATTACHED);
> +
> +     /* DMA isn't supported yet, don't enable the DMA clock. */
> +     isp1760_udc_write(udc, DC_MODE, DC_GLINTENA);
> +
> +     isp1760_udc_reset(udc);
> +
> +     dev_dbg(udc->isp->dev, "UDC started with driver %s\n",
> +             driver->function);
> +
> +     return 0;
> +}
> +
> +static int isp1760_udc_stop(struct usb_gadget *gadget,
> +                         struct usb_gadget_driver *driver)
> +{
> +     struct isp1760_udc *udc = gadget_to_udc(gadget);
> +
> +     dev_dbg(udc->isp->dev, "%s\n", __func__);
> +
> +     isp1760_udc_write(udc, DC_MODE, 0);
> +
> +     spin_lock(&udc->lock);
> +     udc->driver = NULL;
> +     spin_unlock(&udc->lock);
> +
> +     return 0;
> +}
> +
> +static struct usb_gadget_ops isp1760_udc_ops = {
> +     .get_frame = isp1760_udc_get_frame,
> +     .wakeup = isp1760_udc_wakeup,
> +     .set_selfpowered = isp1760_udc_set_selfpowered,
> +     .pullup = isp1760_udc_pullup,
> +     .udc_start = isp1760_udc_start,
> +     .udc_stop = isp1760_udc_stop,
> +};
> +
> +/* 
> -----------------------------------------------------------------------------
> + * Interrupt Handling
> + */
> +
> +static irqreturn_t isp1760_udc_irq(int irq, void *dev)
> +{
> +     struct isp1760_udc *udc = dev;
> +     unsigned int i;
> +     u32 status;
> +
> +     status = isp1760_udc_read(udc, DC_INTERRUPT)
> +            & isp1760_udc_read(udc, DC_INTENABLE);
> +     isp1760_udc_write(udc, DC_INTERRUPT, status);
> +
> +     if (status & DC_IEBRST) {
> +             dev_dbg(udc->isp->dev, "%s(BRST)\n", __func__);
> +
> +             isp1760_udc_reset(udc);
> +     }
> +
> +     for (i = 0; i <= 7; ++i) {
> +             struct isp1760_ep *ep = &udc->ep[i*2];
> +
> +             if (status & DC_IEPTX(i)) {
> +                     dev_dbg(udc->isp->dev, "%s(EPTX%u)\n", __func__, i);
> +                     isp1760_ep_tx_complete(ep);
> +             }
> +
> +             if (status & DC_IEPRX(i)) {
> +                     dev_dbg(udc->isp->dev, "%s(EPRX%u)\n", __func__, i);
> +                     isp1760_ep_rx_ready(i ? ep - 1 : ep);
> +             }
> +     }
> +
> +     if (status & DC_IEP0SETUP) {
> +             dev_dbg(udc->isp->dev, "%s(EP0SETUP)\n", __func__);
> +
> +             isp1760_ep0_setup(udc);
> +     }
> +
> +     if (status & DC_IEVBUS) {
> +             dev_dbg(udc->isp->dev, "%s(VBUS)\n", __func__);
> +             /* The VBUS interrupt is only triggered when VBUS appearsr */
> +             usb_gadget_set_state(&udc->gadget, USB_STATE_POWERED);
> +     }
> +
> +     if (status & DC_IERESM) {
> +             dev_dbg(udc->isp->dev, "%s(RESM)\n", __func__);
> +             isp1760_udc_resume(udc);
> +     }
> +
> +     if (status & DC_IESUSP) {
> +             dev_dbg(udc->isp->dev, "%s(SUSP)\n", __func__);
> +
> +             if (!(isp1760_udc_read(udc, DC_MODE) & DC_VBUSSTAT))
> +                     isp1760_udc_disconnect(udc);
> +             else
> +                     isp1760_udc_suspend(udc);
> +     }
> +
> +     if (status & DC_IEHS_STA) {
> +             dev_dbg(udc->isp->dev, "%s(HS_STA)\n", __func__);
> +             udc->gadget.speed = USB_SPEED_HIGH;
> +     }
> +
> +     return status ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> +/* 
> -----------------------------------------------------------------------------
> + * Registration
> + */
> +
> +static void isp1760_udc_init_eps(struct isp1760_udc *udc)
> +{
> +     unsigned int i;
> +
> +     INIT_LIST_HEAD(&udc->gadget.ep_list);
> +
> +     for (i = 0; i < ARRAY_SIZE(udc->ep); ++i) {
> +             struct isp1760_ep *ep = &udc->ep[i];
> +             unsigned int ep_num = (i + 1) / 2;
> +             bool is_in = !(i & 1);
> +
> +             ep->udc = udc;
> +
> +             INIT_LIST_HEAD(&ep->queue);
> +
> +             ep->addr = (ep_num && is_in ? USB_DIR_IN : USB_DIR_OUT)
> +                      | ep_num;
> +             ep->desc = NULL;
> +
> +             sprintf(ep->name, "ep%u%s", ep_num,
> +                     ep_num ? (is_in ? "in" : "out") : "");
> +
> +             ep->ep.ops = &isp1760_ep_ops;
> +             ep->ep.name = ep->name;
> +
> +             /*
> +              * Hardcode the maximum packet sizes for now, to 64 bytes for
> +              * the control endpoint and 512 bytes for all other endpoints.
> +              * This fits in the 8kB FIFO without double-buffering.
> +              */
> +             if (ep_num == 0) {
> +                     ep->ep.maxpacket = 64;
> +                     ep->maxpacket = 64;
> +                     udc->gadget.ep0 = &ep->ep;
> +             } else {
> +                     ep->ep.maxpacket = 512;
> +                     ep->maxpacket = 0;
> +                     list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list);
> +             }
> +     }
> +}
> +
> +static int isp1760_udc_init(struct isp1760_udc *udc)
> +{
> +     u16 scratch;
> +     u32 chipid;
> +
> +     /*
> +      * Check that the controller is present by writing to the scratch
> +      * register, modifying the bus pattern by reading from the chip ID
> +      * register, and reading the scratch register value back. The chip ID
> +      * and scratch register contents must match the expected values.
> +      */
> +     isp1760_udc_write(udc, DC_SCRATCH, 0xbabe);

heh

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to