Am Mittwoch, 15. Juli 2009 15:59:01 schrieb Rémi Denis-Courmont:
Hello,
> + netif_stop_queue(dev);
> + if (atomic_inc_return(&pnd->tx_queue) < dev->tx_queue_len)
> + netif_wake_queue(dev);
This is unelegant.
> + err = usb_set_interface(pnd->usb, num, pnd->active_setting);
> + if (err)
> + return err;
> +
> + for (i = 0; i < rxq_size; i++) {
> + struct urb *req = usb_alloc_urb(0, GFP_KERNEL);
> +
> + if (req && rx_submit(pnd, req, GFP_KERNEL)) {
> + usb_free_urb(req);
> + req = NULL;
> + }
> + pnd->urbs[i] = req;
> + }
You should fail if you cannot get a minimum number of URBs running.
> +
> + netif_wake_queue(dev);
> + return 0;
> +}
[..]
> + /* Endpoints */
> + if ((data_desc->endpoint[0].desc.bEndpointAddress & USB_DIR_IN) ==
> + USB_DIR_IN) {
Please use the macro.
> + pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
> + data_desc->endpoint[0].desc.bEndpointAddress);
> + pnd->tx_pipe = usb_sndbulkpipe(usbdev,
> + data_desc->endpoint[1].desc.bEndpointAddress);
> + } else {
> + pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
> + data_desc->endpoint[1].desc.bEndpointAddress);
> + pnd->tx_pipe = usb_sndbulkpipe(usbdev,
> + data_desc->endpoint[0].desc.bEndpointAddress);
> + }
> + pnd->active_setting = data_desc - data_intf->altsetting;
> +
> + err = usb_driver_claim_interface(&usbpn_driver, data_intf, pnd);
> + if (err)
> + goto out;
> +
> + /* Force inactive mode until the network device is brought UP */
> + usb_set_interface(usbdev, union_header->bSlaveInterface0,
> + !pnd->active_setting);
> + usb_set_intfdata(intf, pnd);
> + BUG_ON(!usb_get_intfdata(intf));
What for?
> +
> + err = register_netdev(dev);
> + if (err) {
> + usb_driver_release_interface(&usbpn_driver, data_intf);
> + goto out;
> + }
> +
> + dev_dbg(&dev->dev, "USB CDC Phonet device found\n");
> + return 0;
> +
> +out:
> + usb_set_intfdata(intf, NULL);
> + free_netdev(dev);
> + return err;
> +}
> +
> +static void usbpn_disconnect(struct usb_interface *intf)
> +{
> + struct usbpn_dev *pnd = usb_get_intfdata(intf);
> +
What happens if this happens in the unexpected order?
> + if (intf != pnd->data_intf) {
> + usb_driver_release_interface(&usbpn_driver, pnd->data_intf);
> + return;
> + }
> +
> + unregister_netdev(pnd->dev);
> + usb_put_dev(pnd->usb);
> +}
Regards
Oliver
--
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
