| Message ID | 20240307173159.318384-1-18500469033@163.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | USB: Use EHCI control transfer pid macros instead of constant values. | expand |
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@163.com> > --- 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
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; @@ -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); diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index 1441e3400796..dafa6628e134 100644 --- a/drivers/usb/host/ehci.h +++ b/drivers/usb/host/ehci.h @@ -321,10 +321,16 @@ struct ehci_qtd { size_t length; /* length of buffer */ } __aligned(32); +/* PID Codes that are used here, from EHCI specification, Table 3-16. */ +/* #define PID_CODE_OUT 0 */ +#define PID_CODE_IN 1 +#define PID_CODE_SETUP 2 + /* mask NakCnt+T in qh->hw_alt_next */ #define QTD_MASK(ehci) cpu_to_hc32(ehci, ~0x1f) -#define IS_SHORT_READ(token) (QTD_LENGTH(token) != 0 && QTD_PID(token) == 1) +#define IS_SHORT_READ(token) (QTD_LENGTH(token) != 0 && \ + QTD_PID(token) == PID_CODE_IN) /*-------------------------------------------------------------------------*/
Macros with good names offer better readability. Besides, also move the definition to ehci.h. Signed-off-by: Dingyan Li <18500469033@163.com> --- drivers/usb/host/ehci-q.c | 10 +++------- drivers/usb/host/ehci.h | 8 +++++++- 2 files changed, 10 insertions(+), 8 deletions(-)