On Wed, May 15, 2013 at 11:44:39AM +0200, Michael Grzeschik wrote:
> Hi Peter,
>
> On Wed, May 15, 2013 at 02:07:44AM +0000, Chen Peter-B29397 wrote:
> >
> > >
> > > This patch adds iso endpoint support to the device controller.
> > > It makes use of the multiplication bits in the maxpacket field
> > > of the endpoint and calculates the multiplier bits for each
> > > transfer description on every request.
> > >
> > > Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> > > ---
> > > drivers/usb/chipidea/core.c | 2 +-
> > > drivers/usb/chipidea/udc.c | 19 ++++++++++++++++++-
> > > drivers/usb/chipidea/udc.h | 1 +
> > > 3 files changed, 20 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > index 450107e..3cdb889 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -43,7 +43,7 @@
> > > *
> > > * TODO List
> > > * - OTG
> > > - * - Isochronous & Interrupt Traffic
> > > + * - Interrupt Traffic
> > > * - Handle requests which spawns into several TDs
> > > * - GET_STATUS(device) - always reports 0
> > > * - Gadget API (majority of optional features)
> > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > > index 3d90e61..84f5ec0 100644
> > > --- a/drivers/usb/chipidea/udc.c
> > > +++ b/drivers/usb/chipidea/udc.c
> > > @@ -466,6 +466,13 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp,
> > > struct ci13xxx_req *mReq)
> > > mEp->qh.ptr->td.token &=
> > > cpu_to_le32(~(TD_STATUS_HALTED|TD_STATUS_ACTIVE));
> > >
> > > + if (mEp->type == USB_ENDPOINT_XFER_ISOC) {
> > > + u32 mul = mReq->req.length >> __ffs(mEp->ep.maxpacket);
> > > + if (mReq->req.length & ~(mul << __ffs(mEp->ep.maxpacket)))
> > > + mul++;
> > > + mEp->qh.ptr->cap |= mul << __ffs(QH_MULT);
> > > + }
> > > +
> >
> > I think it is a little hard to read, why not
> > if (mEp->type == USB_ENDPOINT_XFER_ISOC) {
> > u32 mul = mReq->req.length / mEp->ep.maxpacket;
> > if (mReq->req.length % mEp->ep.maxpacket)
> > mul++;
> > mEp->qh.ptr->cap |= mul << __ffs(QH_MULT);
>
> Right, will change.
>
> > }
> >
> > > wmb(); /* synchronize before ep prime */
> > >
> > > ret = hw_ep_prime(ci, mEp->num, mEp->dir,
> > > @@ -678,6 +685,12 @@ static int _ep_queue(struct usb_ep *ep, struct
> > > usb_request *req,
> > > }
> > > }
> > >
> > > + if (usb_endpoint_xfer_isoc(mEp->ep.desc)
> > > + && mReq->req.length > (1 + mEp->ep.mult) * mEp->ep.maxpacket) {
> > > + dev_err(mEp->ci->dev, "request length to big for
> > > isochronous\n");
> > > + return -EMSGSIZE;
> > > + }
> > > +
> >
> > typo, "too big"
> > the request length should be less than 3*1024.
>
> Why not additionally depend on the currently configured
> multiplier for the endpoint.
Right, I did not know wMaxPacketSize includes mult info.
>
> High bandwidth enpoints have multiplier encoded bits in the
> wMaxPacketSize value. This has to be dynamically handled here.
>
Yes.
--
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
