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.grzesc...@pengutronix.de>
> > ---
> > 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
