Document the transfer buffer requirement. That is, the buffer must be
DMAble - otherwise data corruption might occur.
Acked-by: Randy Dunlap <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Ricardo Ribalda <[email protected]>
---
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: Bagas Sanjaya <[email protected]>
To: Randy Dunlap <[email protected]>
To: Alan Stern <[email protected]>
To: Christoph Hellwig <[email protected]>
To: Laurent Pinchart <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
Changes in v4:
- Unpolite version :P. Thanks Laurent!
- Make explicit reference to DMA in/out. Thanks Alan!
- Reference struct urb. Thanks Alan!
- Link to v3: https://lore.kernel.org/r/[email protected]
Changes in v3:
- Improve commit message. Thanks Bagas!
- Improve field description. Thanks Alan!
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- s/allocatiing/allocating/ Thanks Randy
- Link to v1: https://lore.kernel.org/r/[email protected]
---
include/linux/usb.h | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 7d5325d47c45..4e98ebacec96 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1626,14 +1626,25 @@ struct urb {
* @urb: pointer to the urb to initialize.
* @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
+ * @setup_packet: pointer to the setup_packet buffer. The buffer must be
+ * suitable for DMA.
+ * @transfer_buffer: pointer to the transfer buffer. The 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 and the setup_packet buffer will most likely be filled
+ * or read via DMA. The simplest way to get a buffer that can be DMAed to is
+ * allocating it via kmalloc() or equivalent, even for very small buffers.
+ * If the buffers are embedded in a bigger structure, there is a risk that
+ * the buffer itself, the previous fields and/or the next fields are corrupted
+ * due to cache incoherencies; or slowed down if they are evicted from the
+ * cache. For more information, check &struct urb.
+ *
*/
static inline void usb_fill_control_urb(struct urb *urb,
struct usb_device *dev,
@@ -1658,13 +1669,17 @@ 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. The 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.
+ *
+ * Refer to usb_fill_control_urb() for a description of the requirements for
+ * transfer_buffer.
*/
static inline void usb_fill_bulk_urb(struct urb *urb,
struct usb_device *dev,
@@ -1687,7 +1702,8 @@ 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. The 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 +1713,9 @@ 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.
*
+ * Refer to usb_fill_control_urb() for a description of the requirements for
+ * transfer_buffer.
+ *
* 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,
--
Ricardo Ribalda <[email protected]>
On Wed, Dec 21, 2022 at 08:34:51PM +0100, Ricardo Ribalda wrote:
> Document the transfer buffer requirement. That is, the buffer must be
> DMAble - otherwise data corruption might occur.
>
> Acked-by: Randy Dunlap <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> 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: Bagas Sanjaya <[email protected]>
> To: Randy Dunlap <[email protected]>
> To: Alan Stern <[email protected]>
> To: Christoph Hellwig <[email protected]>
> To: Laurent Pinchart <[email protected]>
> To: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
Acked-by: Alan Stern <[email protected]>