| Message ID | 20221220-usb-dmadoc-v1-0-28386d2eb6cd@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | USB: Improve usb_fill_* documentation | expand |
On 12/20/22 14:52, Ricardo Ribalda wrote: > Make the developer aware of the requirements of transfer buffer. > > The buffer must be DMAble, if the developer uses an invalid buffer, data > corruption might happen. > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > USB: Improve usb_fill_* documentation > > After trying to "cleanup" the uvc code, I was patiently explained about > the requirements of the urb transfer buffers. > > Lets make this explicit, so other developers do not make the same mistake. > > To: Alan Stern <stern@rowland.harvard.edu> > To: Christoph Hellwig <hch@lst.de> > To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: linux-usb@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > include/linux/usb.h | 27 ++++++++++++++++++++++++--- > 1 file changed, 24 insertions(+), 3 deletions(-) > > diff --git a/include/linux/usb.h b/include/linux/usb.h > index 7d5325d47c45..033ca69b563d 100644 > --- a/include/linux/usb.h > +++ b/include/linux/usb.h > @@ -1627,13 +1627,20 @@ struct urb { > * @dev: pointer to the struct usb_device for this urb. > * @pipe: the endpoint pipe > * @setup_packet: pointer to the setup_packet buffer > - * @transfer_buffer: pointer to the transfer buffer > + * @transfer_buffer: pointer to the transfer buffer. Must be suitable for DMA. > * @buffer_length: length of the transfer buffer > * @complete_fn: pointer to the usb_complete_t function > * @context: what to set the urb context to. > * > * Initializes a control urb with the proper information needed to submit > * it to a device. > + * > + * The transfer buffer might be filled via DMA. The simplest way to get > + * a buffer that can be DMAed to, is allocatiing it via kmalloc() or allocating > + * equivalent, even for very small buffers. If transfer_buffer is embedded > + * in a bigger structure, there is a risk that the previous and following > + * fields are left in a corrupted state by the DMA engine, if the platform > + * is not cache coherent. > */ > static inline void usb_fill_control_urb(struct urb *urb, > struct usb_device *dev, > @@ -1658,13 +1665,20 @@ static inline void usb_fill_control_urb(struct urb *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 > + * @transfer_buffer: pointer to the transfer buffer. Must be suitable for DMA. > * @buffer_length: length of the transfer buffer > * @complete_fn: pointer to the usb_complete_t function > * @context: what to set the urb context to. > * > * Initializes a bulk urb with the proper information needed to submit it > * to a device. > + * > + * The transfer buffer might be filled via DMA. The simplest way to get > + * a buffer that can be DMAed to, is allocatiing it via kmalloc() or allocating > + * equivalent, even for very small buffers. If transfer_buffer is embedded > + * in a bigger structure, there is a risk that the previous and following > + * fields are left in a corrupted state by the DMA engine, if the platform > + * is not cache coherent. > */ > static inline void usb_fill_bulk_urb(struct urb *urb, > struct usb_device *dev, > @@ -1687,7 +1701,7 @@ static inline void usb_fill_bulk_urb(struct urb *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 > + * @transfer_buffer: pointer to the transfer buffer. Must be suitable for DMA. > * @buffer_length: length of the transfer buffer > * @complete_fn: pointer to the usb_complete_t function > * @context: what to set the urb context to. > @@ -1697,6 +1711,13 @@ static inline void usb_fill_bulk_urb(struct urb *urb, > * Initializes a interrupt urb with the proper information needed to submit > * it to a device. > * > + * The transfer buffer might be filled via DMA. The simplest way to get > + * a buffer that can be DMAed to, is allocatiing it via kmalloc() or allocating > + * equivalent, even for very small buffers. If transfer_buffer is embedded > + * in a bigger structure, there is a risk that the previous and following > + * fields are left in a corrupted state by the DMA engine, if the platform > + * is not cache coherent. > + * > * Note that High Speed and SuperSpeed(+) interrupt endpoints use a logarithmic > * encoding of the endpoint interval, and express polling intervals in > * microframes (eight per millisecond) rather than in frames (one per > > --- > base-commit: b6bb9676f2165d518b35ba3bea5f1fcfc0d969bf > change-id: 20221220-usb-dmadoc-29384acebd48 > > Best regards, Thanks.
diff --git a/include/linux/usb.h b/include/linux/usb.h index 7d5325d47c45..033ca69b563d 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1627,13 +1627,20 @@ struct urb { * @dev: pointer to the struct usb_device for this urb. * @pipe: the endpoint pipe * @setup_packet: pointer to the setup_packet buffer - * @transfer_buffer: pointer to the transfer buffer + * @transfer_buffer: pointer to the transfer buffer. Must be suitable for DMA. * @buffer_length: length of the transfer buffer * @complete_fn: pointer to the usb_complete_t function * @context: what to set the urb context to. * * Initializes a control urb with the proper information needed to submit * it to a device. + * + * The transfer buffer might be filled via DMA. The simplest way to get + * a buffer that can be DMAed to, is allocatiing it via kmalloc() or + * equivalent, even for very small buffers. If transfer_buffer is embedded + * in a bigger structure, there is a risk that the previous and following + * fields are left in a corrupted state by the DMA engine, if the platform + * is not cache coherent. */ static inline void usb_fill_control_urb(struct urb *urb, struct usb_device *dev, @@ -1658,13 +1665,20 @@ static inline void usb_fill_control_urb(struct urb *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 + * @transfer_buffer: pointer to the transfer buffer. Must be suitable for DMA. * @buffer_length: length of the transfer buffer * @complete_fn: pointer to the usb_complete_t function * @context: what to set the urb context to. * * Initializes a bulk urb with the proper information needed to submit it * to a device. + * + * The transfer buffer might be filled via DMA. The simplest way to get + * a buffer that can be DMAed to, is allocatiing it via kmalloc() or + * equivalent, even for very small buffers. If transfer_buffer is embedded + * in a bigger structure, there is a risk that the previous and following + * fields are left in a corrupted state by the DMA engine, if the platform + * is not cache coherent. */ static inline void usb_fill_bulk_urb(struct urb *urb, struct usb_device *dev, @@ -1687,7 +1701,7 @@ static inline void usb_fill_bulk_urb(struct urb *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 + * @transfer_buffer: pointer to the transfer buffer. Must be suitable for DMA. * @buffer_length: length of the transfer buffer * @complete_fn: pointer to the usb_complete_t function * @context: what to set the urb context to. @@ -1697,6 +1711,13 @@ static inline void usb_fill_bulk_urb(struct urb *urb, * Initializes a interrupt urb with the proper information needed to submit * it to a device. * + * The transfer buffer might be filled via DMA. The simplest way to get + * a buffer that can be DMAed to, is allocatiing it via kmalloc() or + * equivalent, even for very small buffers. If transfer_buffer is embedded + * in a bigger structure, there is a risk that the previous and following + * fields are left in a corrupted state by the DMA engine, if the platform + * is not cache coherent. + * * Note that High Speed and SuperSpeed(+) interrupt endpoints use a logarithmic * encoding of the endpoint interval, and express polling intervals in * microframes (eight per millisecond) rather than in frames (one per
Make the developer aware of the requirements of transfer buffer. The buffer must be DMAble, if the developer uses an invalid buffer, data corruption might happen. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- USB: Improve usb_fill_* documentation After trying to "cleanup" the uvc code, I was patiently explained about the requirements of the urb transfer buffers. Lets make this explicit, so other developers do not make the same mistake. To: Alan Stern <stern@rowland.harvard.edu> To: Christoph Hellwig <hch@lst.de> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: linux-usb@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- include/linux/usb.h | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) --- base-commit: b6bb9676f2165d518b35ba3bea5f1fcfc0d969bf change-id: 20221220-usb-dmadoc-29384acebd48 Best regards,