Dear Łukasz Dałek,
[...]
The following comment still doesn't seem right.
> +/*
> --------------------------------------------------------------------------
> - + * endpoint related parts of the api to the usb controller hardware, +
> * used by gadget driver; and the inner talker-to-hardware core.
> + *
> --------------------------------------------------------------------------
> - + */
[...]
> +
> +/*
> + * when a driver is successfully registered, it will receive
> + * control requests including set_configuration(), which enables
> + * non-control requests. then usb traffic follows until a
> + * disconnect is reported. then a host may connect again, or
> + * the driver might get unbound.
> + */
> +int usb_gadget_register_driver(struct usb_gadget_driver *driver)
> +{
All this should certainly go into arch/arm/cpu/pxa/ ... as something like
"get_cpu_revision" or such ... don't you think? Then it can be used elsewhere
too if needed, give it some thought ;-)
> + struct pxa25x_udc *dev = &memory;
> + int retval;
> + uint32_t chiprev;
> +
> + if (!driver
> + || driver->speed < USB_SPEED_FULL
> + || !driver->disconnect
> + || !driver->setup)
> + return -EINVAL;
> + if (!dev)
> + return -ENODEV;
> + if (dev->driver)
> + return -EBUSY;
> +
> + /* Enable clock for usb controller */
> + setbits_le32(CKEN, CKEN11_USB);
> +
> + /* first hook up the driver ... */
> + dev->driver = driver;
> + dev->pullup = 1;
> +
> + /* insist on Intel/ARM/XScale */
> + asm("mrc%? p15, 0, %0, c0, c0" : "=r" (chiprev));
> + if ((chiprev & CP15R0_VENDOR_MASK) != CP15R0_XSCALE_VALUE) {
> + printf("%s: not XScale!\n", DRIVER_NAME);
> + return -ENODEV;
> + }
> +
> + /* trigger chiprev-specific logic */
> + switch (chiprev & CP15R0_PRODREV_MASK) {
> + case PXA255_A0:
> + dev->has_cfr = 1;
> + break;
> + case PXA250_A0:
> + case PXA250_A1:
> + /* A0/A1 "not released"; ep 13, 15 unusable */
> + /* fall through */
> + case PXA250_B2: case PXA210_B2:
> + case PXA250_B1: case PXA210_B1:
> + case PXA250_B0: case PXA210_B0:
> + /* OUT-DMA is broken ... */
> + /* fall through */
> + case PXA250_C0: case PXA210_C0:
> + break;
> + default:
> + printf("%s: unrecognized processor: %08x\n",
> + DRIVER_NAME, chiprev);
> + return -ENODEV;
> + }
> +
> + the_controller = dev;
> +
> + /* prepare watchdog timer */
> + dev->watchdog.running = 0;
> + dev->watchdog.period = 5000 * CONFIG_SYS_HZ / 1000000; /* 5 ms */
> + dev->watchdog.function = udc_watchdog;
> +
> + udc_disable(dev);
> + udc_reinit(dev);
> +
> + dev->mach = &mach_info;
> +
> + dev->gadget.name = "pxa2xx_udc";
> + retval = driver->bind(&dev->gadget);
> + if (retval) {
> + printf("bind to driver %s --> error %d\n",
> + DRIVER_NAME, retval);
> + dev->driver = NULL;
> + return retval;
> + }
> +
> + /*
> + * ... then enable host detection and ep0; and we're ready
> + * for set_configuration as well as eventual disconnect.
> + */
> + printf("registered gadget driver '%s'\n", DRIVER_NAME);
> +
> + pullup(dev);
> + dump_state(dev);
> + return 0;
> +}
[...]
> +struct pxa25x_request {
> + struct usb_request req;
> + struct list_head queue;
> +};
> +
> +enum ep0_state {
> + EP0_IDLE,
> + EP0_IN_DATA_PHASE,
> + EP0_OUT_DATA_PHASE,
> + EP0_END_XFER,
> + EP0_STALL,
> +};
> +
> +#define EP0_FIFO_SIZE ((unsigned)16)
Wasn't there something like 16U or something instead of the typecast?
> +#define BULK_FIFO_SIZE ((unsigned)64)
> +#define ISO_FIFO_SIZE ((unsigned)256)
> +#define INT_FIFO_SIZE ((unsigned)8)
> +
> +struct udc_stats {
> + struct ep0stats {
> + unsigned long ops;
> + unsigned long bytes;
> + } read, write;
> + unsigned long irqs;
> +};
> +
> +#ifdef CONFIG_USB_PXA25X_SMALL
> +/* when memory's tight, SMALL config saves code+data. */
> +#define PXA_UDC_NUM_ENDPOINTS 3
> +#endif
> +
> +#ifndef PXA_UDC_NUM_ENDPOINTS
> +#define PXA_UDC_NUM_ENDPOINTS 16
> +#endif
> +
> +struct pxa25x_watchdog {
> + unsigned running:1;
> + ulong period;
> + ulong base;
> + struct pxa25x_udc *udc;
> +
> + void (*function)(struct pxa25x_udc *udc);
> +};
> +
> +struct pxa25x_udc {
> + struct usb_gadget gadget;
> + struct usb_gadget_driver *driver;
> + struct pxa25x_udc_regs *regs;
> +
> + enum ep0_state ep0state;
> + struct udc_stats stats;
> + unsigned got_irq:1,
> + pullup:1,
> + has_cfr:1,
> + req_pending:1,
> + req_std:1,
> + req_config:1,
> + active:1;
> +
> + struct clk *clk;
> + struct pxa2xx_udc_mach_info *mach;
> + u64 dma_mask;
> + struct pxa25x_ep ep[PXA_UDC_NUM_ENDPOINTS];
> +
> + struct pxa25x_watchdog watchdog;
> +};
> +
> +/*------------------------------------------------------------------------
> -*/ +
> +static struct pxa25x_udc *the_controller;
> +
> +/*------------------------------------------------------------------------
> -*/ +
> +#ifdef DEBUG
Functions should certainly not be in a header file.
> +static const char * const state_name[] = {
> + "EP0_IDLE",
> + "EP0_IN_DATA_PHASE", "EP0_OUT_DATA_PHASE",
> + "EP0_END_XFER", "EP0_STALL"
> +};
> +
> +static void
> +dump_udccr(const char *label)
> +{
> + u32 udccr = readl(&UDC_REGS->udccr);
> + debug("%s %02X =%s%s%s%s%s%s%s%s\n",
> + label, udccr,
> + (udccr & UDCCR_REM) ? " rem" : "",
> + (udccr & UDCCR_RSTIR) ? " rstir" : "",
> + (udccr & UDCCR_SRM) ? " srm" : "",
> + (udccr & UDCCR_SUSIR) ? " susir" : "",
> + (udccr & UDCCR_RESIR) ? " resir" : "",
> + (udccr & UDCCR_RSM) ? " rsm" : "",
> + (udccr & UDCCR_UDA) ? " uda" : "",
> + (udccr & UDCCR_UDE) ? " ude" : "");
> +}
> +
> +static void
> +dump_udccs0(const char *label)
> +{
> + u32 udccs0 = readl(&UDC_REGS->udccs[0]);
> +
> + debug("%s %s %02X =%s%s%s%s%s%s%s%s\n",
> + label, state_name[the_controller->ep0state], udccs0,
> + (udccs0 & UDCCS0_SA) ? " sa" : "",
> + (udccs0 & UDCCS0_RNE) ? " rne" : "",
> + (udccs0 & UDCCS0_FST) ? " fst" : "",
> + (udccs0 & UDCCS0_SST) ? " sst" : "",
> + (udccs0 & UDCCS0_DRWF) ? " dwrf" : "",
> + (udccs0 & UDCCS0_FTF) ? " ftf" : "",
> + (udccs0 & UDCCS0_IPR) ? " ipr" : "",
> + (udccs0 & UDCCS0_OPR) ? " opr" : "");
> +}
> +
> +static void
> +dump_state(struct pxa25x_udc *dev)
> +{
> + u32 tmp;
> + unsigned i;
> +
> + debug("%s, uicr %02X.%02X, usir %02X.%02x, ufnr %02X.%02X\n",
> + state_name[dev->ep0state],
> + readl(&UDC_REGS->uicr1), readl(&UDC_REGS->uicr0),
> + readl(&UDC_REGS->usir1), readl(&UDC_REGS->usir0),
> + readl(&UDC_REGS->ufnrh), readl(&UDC_REGS->ufnrl));
> + dump_udccr("udccr");
> + if (dev->has_cfr) {
> + tmp = readl(&UDC_REGS->udccfr);
> + debug("udccfr %02X =%s%s\n", tmp,
> + (tmp & UDCCFR_AREN) ? " aren" : "",
> + (tmp & UDCCFR_ACM) ? " acm" : "");
> + }
> +
> + if (!dev->driver) {
> + debug("no gadget driver bound\n");
> + return;
> + } else
> + debug("ep0 driver '%s'\n", "ether");
> +
> + dump_udccs0("udccs0");
> + debug("ep0 IN %lu/%lu, OUT %lu/%lu\n",
> + dev->stats.write.bytes, dev->stats.write.ops,
> + dev->stats.read.bytes, dev->stats.read.ops);
> +
> + for (i = 1; i < PXA_UDC_NUM_ENDPOINTS; i++) {
> + if (dev->ep[i].desc == NULL)
> + continue;
> + debug("udccs%d = %02x\n", i, *dev->ep->reg_udccs);
> + }
> +}
> +
> +#ifdef DEBUG_NOISY
> +# define NOISY 1
> +#else
> +# define NOISY 0
> +#endif
> +
> +#else
> +
> +#define NOISY 0
> +#define dump_udccr(x) do {} while (0)
> +#define dump_udccs0(x) do {} while (0)
> +#define dump_state(x) do {} while (0)
Please make these empty inline functions (for the sake of typechecking, compiler
will optimize them out well).
> +#endif
> +
> +#endif /* __LINUX_USB_GADGET_PXA25X_H */