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.
>
> > /* first nuke then test link, e.g. previous status has not sent */
> > if (!list_empty(&mReq->queue)) {
> > dev_err(mEp->ci->dev, "request already in queue\n");
> > @@ -1060,7 +1073,8 @@ static int ep_enable(struct usb_ep *ep,
> > mEp->num = usb_endpoint_num(desc);
> > mEp->type = usb_endpoint_type(desc);
> >
> > - mEp->ep.maxpacket = usb_endpoint_maxp(desc);
> > + mEp->ep.maxpacket = usb_endpoint_maxp(desc) & 0x07ff;
> > + mEp->ep.mult = QH_ISO_MULT(usb_endpoint_maxp(desc));
> >
>
> The above change doesn't need.
> maxpacket <= 1024 for high speed
> maxpacket <= 1023 for full speed
> For high speed high bandwidth, it just has two or three transactions per microframe
High bandwidth enpoints have multiplier encoded bits in the
wMaxPacketSize value. This has to be dynamically handled here.
>From dummy_hcd.c @434
------8<-----------8<------
/*
* For HS/FS devices only bits 0..10 of the wMaxPacketSize represent the
* maximum packet size.
* For SS devices the wMaxPacketSize is limited by 1024.
*/
max = usb_endpoint_maxp(desc) & 0x7ff;
------8<-----------8<------
Its probably worth a seperate patch.
>
> > if (mEp->type == USB_ENDPOINT_XFER_CONTROL)
> > cap |= QH_IOS;
> > @@ -1246,6 +1260,9 @@ static int ep_set_halt(struct usb_ep *ep, int value)
> > if (ep == NULL || m p->ep.desc == NULL)
> > return -EINVAL;
> >
> > + if (usb_endpoint_xfer_isoc(mEp->ep.desc))
> > + return -EOPNOTSUPP;
> > +
> > spin_lock_irqsave(mEp->lock, flags);
> >
> > #ifndef STALL_IN
> > diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
> > index d12e8b5..a75724a 100644
> > --- a/drivers/usb/chipidea/udc.h
> > +++ b/drivers/usb/chipidea/udc.h
> > @@ -50,6 +50,7 @@ struct ci13xxx_qh {
> > #define QH_MAX_PKT (0x07FFUL << 16)
> > #define QH_ZLT BIT(29)
> > #define QH_MULT (0x0003UL << 30)
> > +#define QH_ISO_MULT(x) ((x >> 11) & 0x03)
> > /* 1 */
> > u32 curr;
> > /* 2 - 8 */
> > --
> > 1.8.2.rc2
> >
Thanks,
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 |
--
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
