On Fri, 13 Jul 2018, Sebastian Andrzej Siewior wrote:
> Provide usb_fill_iso_urb() for the initialisation of isochronous URBs.
> We already have one of this helpers for control, bulk and interruptible
> URB types. This helps to keep the initialisation of the URB members in
> one place.
> Update the documentation by adding this to the available init functions
> and remove the suggestion to use the `_int_' helper which might provide
> wrong encoding for the `interval' member.
>
> This looks like it would cover most users nicely. The sound subsystem
> initialises the ->iso_frame_desc[].offset + length member (often) at a
> different location and I'm not sure ->interval will work always as
> expected. So we might need to overwrite those two in worst case.
>
> Some users also initialise ->iso_frame_desc[].actual_length but I don't
> this is required since it is the return value.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -1697,6 +1697,59 @@ static inline void usb_fill_int_urb(struct urb *urb,
> urb->start_frame = -1;
> }
>
> +/**
> + * usb_fill_iso_urb - macro to help initialize an isochronous urb
> + * @urb: pointer to the urb to initialize.
> + * @dev: pointer to the struct usb_device for this urb.
> + * @pipe: the endpoint pipe
> + * @transfer_buffer: pointer to the transfer buffer
> + * @buffer_length: length of the transfer buffer
> + * @complete_fn: pointer to the usb_complete_t function
> + * @context: what to set the urb context to.
> + * @interval: what to set the urb interval to, encoded like
> + * the endpoint descriptor's bInterval value.
> + * @packets: number of ISO packets.
> + * @packet_size: size of each ISO packet.
> + *
> + * Initializes an isochronous urb with the proper information needed to submit
> + * it to a device.
> + *
> + * Note that isochronous endpoints use a logarithmic encoding of the endpoint
> + * interval, and express polling intervals in microframes (eight per
> + * millisecond) rather than in frames (one per millisecond).
Full-speed devices express polling intervals in frames (1 per ms);
high-speed and SuperSpeed devices express polling intervals in
microframes (8 per ms).
> + */
> +static inline void usb_fill_iso_urb(struct urb *urb,
> + struct usb_device *dev,
> + unsigned int pipe,
> + void *transfer_buffer,
> + int buffer_length,
> + usb_complete_t complete_fn,
> + void *context,
> + int interval,
> + unsigned int packets,
> + unsigned int packet_size)
> +{
> + unsigned int i;
> +
> + urb->dev = dev;
> + urb->pipe = pipe;
> + urb->transfer_buffer = transfer_buffer;
> + urb->transfer_buffer_length = buffer_length;
> + urb->complete = complete_fn;
> + urb->context = context;
> +
> + interval = clamp(interval, 1, 16);
> + urb->interval = 1 << (interval - 1);
> + urb->start_frame = -1;
> +
> + urb->number_of_packets = packets;
> +
> + for (i = 0; i < packets; i++) {
> + urb->iso_frame_desc[i].offset = packet_size * i;
> + urb->iso_frame_desc[i].length = packet_size;
> + }
> +}
This initialization of the iso_frame_desc[] elements assumes that all
the packets are the same size. The kerneldoc should mention that this
is just an assumption; if it is wrong then the driver code will need to
rewrite the elements after calling this function.
One possibility is to allow the caller to set packets to 0; in that
case you could avoid setting either urb->number_of_packets or the
iso_frame_desc[] elements.
Alan Stern
