On Fri, Mar 08, 2024 at 01:31:59AM +0800, Dingyan Li wrote:
> Macros with good names offer better readability. Besides, also move
> the definition to ehci.h.
>
> Signed-off-by: Dingyan Li <18500469033@xxxxxxx>
> ---
Good idea, but you missed a few spots.
> drivers/usb/host/ehci-q.c | 10 +++-------
> drivers/usb/host/ehci.h | 8 +++++++-
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
> index 666f5c4db25a..51b46001e344 100644
> --- a/drivers/usb/host/ehci-q.c
> +++ b/drivers/usb/host/ehci-q.c
> @@ -27,10 +27,6 @@
>
> /*-------------------------------------------------------------------------*/
>
> -/* PID Codes that are used here, from EHCI specification, Table 3-16. */
> -#define PID_CODE_IN 1
> -#define PID_CODE_SETUP 2
> -
> /* fill a qtd, returning how much of the buffer we were able to queue up */
>
> static unsigned int
> @@ -230,7 +226,7 @@ static int qtd_copy_status (
> /* fs/ls interrupt xfer missed the complete-split */
> status = -EPROTO;
> } else if (token & QTD_STS_DBE) {
> - status = (QTD_PID (token) == 1) /* IN ? */
> + status = (QTD_PID(token) == PID_CODE_IN) /* IN ? */
> ? -ENOSR /* hc couldn't read data */
> : -ECOMM; /* hc couldn't write data */
> } else if (token & QTD_STS_XACT) {
> @@ -606,7 +602,7 @@ qh_urb_transaction (
> /* SETUP pid */
> qtd_fill(ehci, qtd, urb->setup_dma,
> sizeof (struct usb_ctrlrequest),
> - token | (2 /* "setup" */ << 8), 8);
> + token | (PID_CODE_SETUP << 8), 8);
>
> /* ... and always at least one more pid */
> token ^= QTD_TOGGLE;
There is an occurrence on line 623.
> @@ -642,7 +638,7 @@ qh_urb_transaction (
> }
>
> if (is_input)
> - token |= (1 /* "in" */ << 8);
> + token |= (PID_CODE_IN << 8);
> /* else it's already initted to "out" pid (0 << 8) */
>
> maxpacket = usb_endpoint_maxp(&urb->ep->desc);
You could use PID_CODE_IN on lines 712 and 1232.
There are occurrences on lines 1206, 1219.
Also, there's a bunch of "switch" cases near line 433 in ehci-dbg.c.
Alan Stern
