[PATCH] xhci: ring: use get_unaligned_le32() for setup packet

[PATCH] xhci: ring: use get_unaligned_le32() for setup packet

[Adriano Cordova]

The setup packet maps directly onto the two little-endian data words,
so read it with get_unaligned_le32() instead of manipulating
bRequestType/bRequest/wValue/wIndex/wLength by hand. No functional
change.

Signed-off-by: Adriano Cordova <adrianox@gmail.com>
---
 drivers/usb/host/xhci-ring.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ec278a9f9540..e428d3794717 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -58,6 +58,7 @@
 #include <linux/string_choices.h>
 #include <linux/dma-mapping.h>
 #include <linux/bitfield.h>
+#include <linux/unaligned.h>
 
 #include "xhci.h"
 #include "xhci-trace.h"
@@ -3837,7 +3838,6 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 	start_cycle = ep_ring->cycle_state;
 
 	/* Queue setup TRB - see section 6.4.1.2.1 */
-	/* FIXME better way to translate setup_packet into two u32 fields? */
 	setup = (struct usb_ctrlrequest *) urb->setup_packet;
 	field = 0;
 	field |= TRB_IDT | TRB_TYPE(TRB_SETUP);
@@ -3855,8 +3855,8 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 	}
 
 	queue_trb(xhci, ep_ring, true,
-		  setup->bRequestType | setup->bRequest << 8 | le16_to_cpu(setup->wValue) << 16,
-		  le16_to_cpu(setup->wIndex) | le16_to_cpu(setup->wLength) << 16,
+		  get_unaligned_le32(setup),
+		  get_unaligned_le32((u8 *)setup + sizeof(u32)),
 		  TRB_LEN(8) | TRB_INTR_TARGET(0),
 		  /* Immediate data in pointer */
 		  field);
-- 
2.51.0

Re: [PATCH] xhci: ring: use get_unaligned_le32() for setup packet

[Alan Stern]

On Thu, Sep 10, 2026 at 09:36:29AM -0300, Adriano Cordova wrote:
> The setup packet maps directly onto the two little-endian data words,
> so read it with get_unaligned_le32() instead of manipulating
> bRequestType/bRequest/wValue/wIndex/wLength by hand. No functional
> change.
> 
> Signed-off-by: Adriano Cordova <adrianox@gmail.com>
> ---

Nobody asked my opinion, but I'll give it anyway.

The original code was fine as it was, regardless of the FIXME comment.  
Using get_unaligned_32() relies on details of the layout of the setup 
packet and is harder for a reader to follow.  I would not do it unless 
it provided a measurable improvement in speed -- which it doesn't.

Alan Stern

>  drivers/usb/host/xhci-ring.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index ec278a9f9540..e428d3794717 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -58,6 +58,7 @@
>  #include <linux/string_choices.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/bitfield.h>
> +#include <linux/unaligned.h>
>  
>  #include "xhci.h"
>  #include "xhci-trace.h"
> @@ -3837,7 +3838,6 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
>  	start_cycle = ep_ring->cycle_state;
>  
>  	/* Queue setup TRB - see section 6.4.1.2.1 */
> -	/* FIXME better way to translate setup_packet into two u32 fields? */
>  	setup = (struct usb_ctrlrequest *) urb->setup_packet;
>  	field = 0;
>  	field |= TRB_IDT | TRB_TYPE(TRB_SETUP);
> @@ -3855,8 +3855,8 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
>  	}
>  
>  	queue_trb(xhci, ep_ring, true,
> -		  setup->bRequestType | setup->bRequest << 8 | le16_to_cpu(setup->wValue) << 16,
> -		  le16_to_cpu(setup->wIndex) | le16_to_cpu(setup->wLength) << 16,
> +		  get_unaligned_le32(setup),
> +		  get_unaligned_le32((u8 *)setup + sizeof(u32)),
>  		  TRB_LEN(8) | TRB_INTR_TARGET(0),
>  		  /* Immediate data in pointer */
>  		  field);
> -- 
> 2.51.0
> 
>