Re: Linux USB file storage gadget with new UDC

[victor yeo <victoryeo1974@xxxxxxxxx>]

Hi,

> While kagen2_ep_queue() is running, there shouldn't be any packets in
> the USB hardware.  The hardware should refuse to accept any packets,
> sending NAKs back to the host, until a request has been submitted and
> queued.
>
> When the request is queued, that's when you should tell the hardware to
> accept data from the host.  After that, each time a packet arrives from
> the host, either the hardware or the UDC driver should store the packet
> data in the request's buffer.  When the buffer is full or a short
> packet is received (the UDC driver's interrupt handler will know when
> this happens) then the UDC driver should call req.complete.

Please see the attached kagen2_ep_queue(). As long as it is called, it
will queue the request and read packets from hardware, in the same
if-else branch for bulk out endpoint. The interrupt handler will NOT
accept packet if request is NOT queued. If request is queued,
interrupt handler will accept the packet.

Somehow, there is still timing problem in UDC driver and it is hard to
pin down the root cause. It could be due to interaction of UDC driver
queue() and gadget driver fsg_main_thread() main loop.

1) When writing to gen2 gadget, SCSI_READ_10 or or SCSI_REQUEST_SENSE
commands are received by UDC driver, but gadget did not process the
commands. (cannot get past get_next_command() in fsg_main_thread)

2) Repeatedly (many many times), the same SCSI_READ_10 command is
received by UDC driver, processed by gadget driver, and UDC driver
sends out data and CSW to host. On usbmon trace, only one instance of
the SCSI_READ_10 is observed.

3) More severe case, if removing one printk statement in
bulk_in_complete(), USB gadget device cannot be recognised by host.

Thanks,
victor

static int kagen2_ep_queue(struct usb_ep *ep,
                struct usb_request *req, gfp_t gfp_flags)
{
	struct kagen2_ep *ka_ep;
	struct kagen2_request *ka_req;
	struct kagen2 * dev;
	unsigned phys;
	int num, len, in;

	ka_req = container_of(req, struct kagen2_request, req);
	
	if (!req || !req->complete || !req->buf
		|| !list_empty(&ka_req->queue))
	{
		printk("exit A\n");
		return -EINVAL;
	}
	ka_ep = container_of(ep, struct kagen2_ep, ep);
	
	if (!ep || (!ka_ep->desc && ka_ep->num != 0))
	{
		printk("exit B\n");
		return -EINVAL;
	}
	dev = ka_ep->dev;

	if (!dev || !dev->driver || dev->gadget.speed == USB_SPEED_UNKNOWN)
	{
		printk("exit C\n");
		return -ESHUTDOWN;
	}
	num = ka_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
	in = (ka_ep->desc->bEndpointAddress & USB_DIR_IN) != 0;
	phys = (unsigned)req->buf;
	len = req->length;

	printk(KERN_DEBUG "ept%d %s queue len 0x%x, buffer 0x%x\n",
		num, in ? "in" : "out", len, phys);

	/* ep0 IN endpoint */
	if ((len > 0) && (num == 0) && (in != 0))
	{
		req->actual = 0;
        ep0_in(phys, len, dev);
		req->actual += len;
		req->complete(ep, req);
		list_del_init(&ka_req->queue);
		return 0;	
	}
	/* ep1 IN endpoint */
	else if ((len >= 0) && (num == 1) && (in != 0))
	{
		req->actual = 0;
		ep1_in(phys, len, dev);
		req->actual += len;
		req->complete(ep, req);
		list_del_init(&ka_req->queue);
		return 0;	
	}
	/* ep1 OUT endpoint */
	else if (in == 0)
    {
		// read from EP1 OUT buffer
		if (num == 1)
        {
			unsigned int val;
			unsigned int val_arr[8];
			int i;

	        // get byte count from hardware
        	val = readl(dev->base_addr + 0x008);
		    len = val & 0xFF;

	        // read from hardware fifo1 data
        	for (i = 0; i < len/4; i++)
		    {
        	    val_arr[i] = readl(dev->base_addr + 0x084);
		    }

			list_add_tail(&ka_req->queue, &ka_ep->queue);

			ka_req->req.actual = len;
			memcpy(ka_req->req.buf, &val_arr[0], len);

			ka_req->req.complete(&ka_ep->ep, &ka_req->req);
			list_del_init(&ka_req->queue);
			 
            // clear hardware OUT1CS register
            val = readl(dev->base_addr + 0x008);
            val &= 0x00ffffff;
            writel(val, dev->base_addr + 0x008);
		}
    }
	return 0;
}