Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver

Sean O. Stalley
2014-11-12T22:58:22Z

Sorry, for got to respond to a couple comments. See responses below.

On Wed, Nov 12, 2014 at 01:40:21PM -0800, Sean O. Stalley wrote:
> Thanks for reviewing. My responses are inline.
>
> Greg has asked that we clean up this code internally before we
> send out another patchset to the mailing list. I will address
> the issues you pointed out, but it may be a while before you see
> another patchset.
>
> Thanks Again,
> Sean
>
> On Tue, Nov 11, 2014 at 10:54:30AM -0500, Alan Stern wrote:
> > On Mon, 10 Nov 2014, Stephanie Wallick wrote:
> > > + /* If the endpoint isn't activated, we can't enqueue anything. */
> > > + if (MAUSB_EP_HANDLE_UNASSIGNED == ep->ep_handle_state) {
> > > + mausb_err(&mhcd, "%s: endpoint handle unassigned\n", __func__);
> > > + return -EPIPE;
> > > + }
> > > +
> > > + if (USB_SPEED_FULL != urb->dev->speed) /* suppress checks */
> > > + ep->max_pkt = usb_endpoint_maxp(&urb->ep->desc);
> >
> > What happens to full-speed devices? Don't they have maxpacket values?
> >

This was part of a work-around.
Per the MA spec (section 7.3.2.2), wMaxPacketSize should be initially
set to 8 for EP0 of FS devices. The usbcore sets it to 64.

This makes sure the EPHandleReq Packet has the per-spec values.

> > > + if (ret < 0) {
> > > + mausb_err(&mhcd, "urb enqueue failed: error %d\n", ret);
> > > + usb_hcd_unlink_urb_from_ep(hcd, urb);
> > > + return ret;
> > > + }
> > > +
> > > + /* get usb device and increment reference counter */
> > > + if (!mhcd.udev) {
> > > + mhcd.udev = urb->dev;
> > > + usb_get_dev(mhcd.udev);
> > > + }
> >
> > What happens if more than one device is in use at a time?
> >

This is wrong. This call should be in mausb_internal_alloc_dev(). Will fix.