Re: Linux USB file storage gadget with new UDC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

>> 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.
>
> This code is wrong.  See what happens when a request for ep1-out is
> submitted:
>
>>       /* 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;
>
> Why do you expect there to be any data in the hardware FIFO at this
> point?  You said that the hardware will not accept any data if a
> request is not queued.  Well, before this point the request wasn't
> queued, so there shouldn't be any data.

I am sorry for unclear writing. What i mean is: If a request is not
queued, the hardware will still accept data, but interrupt controller
will not read the data from the hardware FIFO.

>>
>>               // read from hardware fifo1 data
>>               for (i = 0; i < len/4; i++)
>>                   {
>>                   val_arr[i] = readl(dev->base_addr + 0x084);
>>                   }
>
> val_arr is declared as an array of 8 unsigned ints.  That means its
> total size is 32 bytes.  What happens if len > 32?  You will end up
> overwriting part of the stack.

Yes! This is a bug. I only thought about the 31 byte CBW for bulk out
endpoint. I forgot about the SCSI_WRITE_10 command. Thanks!!

>>                       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);
>
> You should not call req.complete until the request really is complete.
> For example, what happens here if the host hasn't sent any packets yet?
> Or what happens if req is waiting to receive 1024 bytes but the host
> has sent only 512 bytes so far?

How the UDC driver know when the request is really complete?

> Also, all the data gets copied _twice_: once from the hardware FIFO
> into val_arr, and then again from val_arr into req.buf.  That's a waste
> of time; the data should be copied directly from the FIFO into req.buf.

Agree.

>> 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.
>
> This is probably because you are copying the same data from the FIFO to
> req.buffer many times.

I am curious about it. After data is read from FIFO, the FIFO will
become empty. Still cannot figure out how the same data is read from
the FIFO many times.

Thanks,
victor
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux