Re: [PATCH 3/5] usb: chipidea: udc: add the define TD_COUNT and fix all users

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

 



On Tue, Mar 19, 2013 at 01:02:59PM +0100, Michael Grzeschik wrote:
> On Tue, Mar 19, 2013 at 09:34:54AM +0800, Peter Chen wrote:
> > On Mon, Mar 18, 2013 at 03:28:26PM +0100, Michael Grzeschik wrote:
> > > On Fri, Mar 08, 2013 at 05:54:37PM +0200, Alexander Shishkin wrote:
> > > > Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx> writes:
> > > > 
> > > > > A static count of transfer descriptors was used everywhere in the driver
> > > > > with the fixed number 4. This patch adds a define, named TD_COUNT, and
> > > > > replaces all users of this value. This way its possible to have only one
> > > > > parameter to change and limit the amount of tds per transfer.
> > > > 
> > > > I think Svetoslav made exactly the same patch in his patchset, but I
> > > > think this patchset will go first.
> > > 
> > > I did not find any patch comparable by Svetoslav. But, that patch
> > > is superseeded by that hunk in my current branch anyway, as every TD can
> > > maintain five DMA buffers:
> > > 
> > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > > index 09bc6ea..c961e3b 100644
> > > --- a/drivers/usb/chipidea/udc.c
> > > +++ b/drivers/usb/chipidea/udc.c
> > > @@ -688,8 +688,8 @@ static int _ep_queue(struct usb_ep *ep, struct usb_request *req)
> > >                 goto done;
> > >                         }
> > > 
> > >                         -       if (req->length > 4 * CI13XXX_PAGE_SIZE) {
> > >                         -               req->length = 4 * CI13XXX_PAGE_SIZE;
> > >                         +       if (req->length > 5 * CI13XXX_PAGE_SIZE) {
> > >                         +               req->length = 5 * CI13XXX_PAGE_SIZE;
> > >                                         retval = -EMSGSIZE;
> > >                                                         dev_warn(mEp->ci->dev, "request length truncated\n");
> > >                                                                 }
> > 
> > No, 4 is ok. There are 5 buffer Pointers are dTD, but the Buffer Point 0
> > may not 4K aligned. Eg, if the reg->length is 18KB, and the buf DMA address
> > is 1K aligned, it needs two dTDs.
> > The first dTD only uses: 1KB, 4KB, 4KB, 4KB, 4KB as 5 buffers space.
> 
> When the first pointer is not aligned, then all other buffer pointers
> will also not be. 

Please have a look of dTD descriptor at spec, the pointer 0 can be 
not 4k-aligned (the size is address - address & 4KB), but other
buffer pointers are 4k-aligned required.

> Because the payload will very likeley contiguous.
> 
> If the hardware limitation is to have 4K aligned buffer pointers in the
> dma addresses, than this fix should be handled different than that.

hardware no 4K aligned buffer limitation.

> 
> So this limitation of 4 is a bogus workaround, and my attached hunk is
> still valid.
> 
> Regards,
> Michael
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 

Best Regards,
Peter Chen

--
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