On Thu, Apr 22, 2010 at 02:15:06PM -0700, Dan Williams wrote:
> On Thu, 2010-04-22 at 12:44 -0700, Greg KH wrote:
> > On Thu, Apr 22, 2010 at 12:19:33PM -0700, Elina Pasheva wrote:
> > > +static void sierra_net_send_cmd(struct usbnet *dev,
> > > + u8 *cmd, int cmdlen, const char * cmd_name)
> > > +{
> > > + struct sierra_net_data *priv = sierra_net_get_private(dev);
> > > + int status;
> > > +
> > > + status = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
> > > + USB_CDC_SEND_ENCAPSULATED_COMMAND,
> > > + USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE, 0,
> > > + priv->ifnum, cmd, cmdlen, 0);
> >
> > No timeout?
> >
> > > + ifnum = priv->ifnum;
> > > + len = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > > + USB_CDC_GET_ENCAPSULATED_RESPONSE,
> > > + USB_DIR_IN|USB_TYPE_CLASS|USB_RECIP_INTERFACE,
> > > + 0, ifnum, buf, SIERRA_NET_USBCTL_BUF_LEN, 0);
> >
> > No timeout?
> >
> > > + if (unlikely(len < 0)) {
> > > + netdev_err(dev->net,
> > > + "usb_control_msg failed, status %d\n", len);
> >
> > You don't need "unlikely", this is an extreemly slow path here.
> > Also, what happens for a "short read"? You don't handle that properly.
>
> Is the code doc for usb_unlink_urb() in urb.c the best thing to look at
> for how to handle short reads? I assume that involves checking if the
> returned error is -EREMOTEIO and if so, ignoring it? I spent about an
> hour googling and poking around in the kernel sources and couldn't find
> much about how to handle short reads. The only non-host-side driver
> that cares about EREMOTEIO is misc/rio500.c, the rest is all under host/
> or gadget/.
Well, if the return value is less than what you expect it to be,
something went wrong and you should error out. Some of the calls handle
this properly in this driver, some do not. Consistency is key :)
thanks,
greg k-h
--
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
