Re: [PATCH v3] usb_8dev: Add support for USB2CAN interface from 8 devices

[Bernd Krumboeck <krumboeck@universalnet.at>]

Hi Marc!


Am 2012-12-04 22:45, schrieb Marc Kleine-Budde:
>> +
>> +        /* create a URB, and a buffer for it */
>> +        urb = usb_alloc_urb(0, GFP_KERNEL);
>> +        if (!urb) {
>> +            netdev_err(netdev, "No memory left for URBs\n");
>> +            return -ENOMEM;
>
> who will free the already allocated urbs if i != 0 ?

This function tries to submit 10 urbs (MAX_RX_URBS) for receiving. When we could not
submit all, we still proceed as long as we could submit at least 1.

We must not free, because we should break the loop not return from the function.
I'll correct.


>
>> +        }
>> +
>> +        buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE, GFP_KERNEL,
>> +                     &urb->transfer_dma);
>> +        if (!buf) {
>> +            netdev_err(netdev, "No memory left for USB buffer\n");
>> +            usb_free_urb(urb);
>
> same problem here for coherent

dito.

>
>> +            return -ENOMEM;
>> +        }
>> +
>> +        usb_fill_bulk_urb(urb, dev->udev,
>> +                  usb_rcvbulkpipe(dev->udev,
>> +                          USB_8DEV_ENDP_DATA_RX),
>> +                  buf, RX_BUFFER_SIZE,
>> +                  usb_8dev_read_bulk_callback, dev);
>> +        urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>> +        usb_anchor_urb(urb, &dev->rx_submitted);
>> +
>> +        err = usb_submit_urb(urb, GFP_KERNEL);
>> +        if (err) {
>> +            if (err == -ENODEV)
>> +                netif_device_detach(dev->netdev);
>> +
>> +            usb_unanchor_urb(urb);
>> +            usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
>> +                      urb->transfer_dma);
> same here
>
> add a loop that runs backwards at the end of the function

dito.

>
>> +            break;
>> +        }
>> +
>> +        /* Drop reference, USB core will take care of freeing it */
>> +        usb_free_urb(urb);
>> +    }
>> +
>> +    /* Did we submit any URBs */
>> +    if (i == 0) {
>> +        netdev_warn(netdev, "couldn't setup read URBs\n");
>> +        return err;
>> +    }
>> +
>
> Marc
>

regards,
Bernd