| Message ID | 20240308010859.81987-1-18500469033@163.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] USB: Use EHCI control transfer pid macros instead of constant values. | expand |
On Fri, Mar 08, 2024 at 09:08: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> > --- You found some things that I missed! Good for you. When you resubmit with a list of changes from the original version, you can add: Reviewed-by: Alan Stern <stern@rowland.harvard.edu> > drivers/usb/host/ehci-dbg.c | 10 +++++----- > drivers/usb/host/ehci-q.c | 20 ++++++++------------ > drivers/usb/host/ehci.h | 8 +++++++- > 3 files changed, 20 insertions(+), 18 deletions(-) > > diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c > index c063fb042926..435001128221 100644 > --- a/drivers/usb/host/ehci-dbg.c > +++ b/drivers/usb/host/ehci-dbg.c > @@ -430,13 +430,13 @@ static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh, > mark = '/'; > } > switch ((scratch >> 8) & 0x03) { > - case 0: > + case PID_CODE_OUT: > type = "out"; > break; > - case 1: > + case PID_CODE_IN: > type = "in"; > break; > - case 2: > + case PID_CODE_SETUP: > type = "setup"; > break; > default: > @@ -602,10 +602,10 @@ static unsigned output_buf_tds_dir(char *buf, struct ehci_hcd *ehci, > list_for_each_entry(qtd, &qh->qtd_list, qtd_list) { > temp++; > switch ((hc32_to_cpu(ehci, qtd->hw_token) >> 8) & 0x03) { > - case 0: > + case PID_CODE_OUT: > type = "out"; > continue; > - case 1: > + case PID_CODE_IN: > type = "in"; > continue; > } > diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c > index 666f5c4db25a..ba37a9fcab92 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; > @@ -620,7 +616,7 @@ qh_urb_transaction ( > > /* for zero length DATA stages, STATUS is always IN */ > if (len == 0) > - token |= (1 /* "in" */ << 8); > + token |= (PID_CODE_IN << 8); > } > > /* > @@ -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); > @@ -709,7 +705,7 @@ qh_urb_transaction ( > > if (usb_pipecontrol (urb->pipe)) { > one_more = 1; > - token ^= 0x0100; /* "in" <--> "out" */ > + token ^= (PID_CODE_IN << 8); /* "in" <--> "out" */ > token |= QTD_TOGGLE; /* force DATA1 */ > } else if (usb_pipeout(urb->pipe) > && (urb->transfer_flags & URB_ZERO_PACKET) > @@ -1203,7 +1199,7 @@ static int ehci_submit_single_step_set_feature( > /* SETUP pid, and interrupt after SETUP completion */ > qtd_fill(ehci, qtd, urb->setup_dma, > sizeof(struct usb_ctrlrequest), > - QTD_IOC | token | (2 /* "setup" */ << 8), 8); > + QTD_IOC | token | (PID_CODE_SETUP << 8), 8); > > submit_async(ehci, urb, &qtd_list, GFP_ATOMIC); > return 0; /*Return now; we shall come back after 15 seconds*/ > @@ -1216,7 +1212,7 @@ static int ehci_submit_single_step_set_feature( > token ^= QTD_TOGGLE; /*We need to start IN with DATA-1 Pid-sequence*/ > buf = urb->transfer_dma; > > - token |= (1 /* "in" */ << 8); /*This is IN stage*/ > + token |= (PID_CODE_IN << 8); /*This is IN stage*/ > > maxpacket = usb_endpoint_maxp(&urb->ep->desc); > > @@ -1229,7 +1225,7 @@ static int ehci_submit_single_step_set_feature( > qtd->hw_alt_next = EHCI_LIST_END(ehci); > > /* STATUS stage for GetDesc control request */ > - token ^= 0x0100; /* "in" <--> "out" */ > + token ^= (PID_CODE_IN << 8); /* "in" <--> "out" */ > token |= QTD_TOGGLE; /* force DATA1 */ > > qtd_prev = qtd; > diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h > index 1441e3400796..d7a3c8d13f6b 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) > > /*-------------------------------------------------------------------------*/ > > -- > 2.25.1 >
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c index c063fb042926..435001128221 100644 --- a/drivers/usb/host/ehci-dbg.c +++ b/drivers/usb/host/ehci-dbg.c @@ -430,13 +430,13 @@ static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh, mark = '/'; } switch ((scratch >> 8) & 0x03) { - case 0: + case PID_CODE_OUT: type = "out"; break; - case 1: + case PID_CODE_IN: type = "in"; break; - case 2: + case PID_CODE_SETUP: type = "setup"; break; default: @@ -602,10 +602,10 @@ static unsigned output_buf_tds_dir(char *buf, struct ehci_hcd *ehci, list_for_each_entry(qtd, &qh->qtd_list, qtd_list) { temp++; switch ((hc32_to_cpu(ehci, qtd->hw_token) >> 8) & 0x03) { - case 0: + case PID_CODE_OUT: type = "out"; continue; - case 1: + case PID_CODE_IN: type = "in"; continue; } diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c index 666f5c4db25a..ba37a9fcab92 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; @@ -620,7 +616,7 @@ qh_urb_transaction ( /* for zero length DATA stages, STATUS is always IN */ if (len == 0) - token |= (1 /* "in" */ << 8); + token |= (PID_CODE_IN << 8); } /* @@ -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); @@ -709,7 +705,7 @@ qh_urb_transaction ( if (usb_pipecontrol (urb->pipe)) { one_more = 1; - token ^= 0x0100; /* "in" <--> "out" */ + token ^= (PID_CODE_IN << 8); /* "in" <--> "out" */ token |= QTD_TOGGLE; /* force DATA1 */ } else if (usb_pipeout(urb->pipe) && (urb->transfer_flags & URB_ZERO_PACKET) @@ -1203,7 +1199,7 @@ static int ehci_submit_single_step_set_feature( /* SETUP pid, and interrupt after SETUP completion */ qtd_fill(ehci, qtd, urb->setup_dma, sizeof(struct usb_ctrlrequest), - QTD_IOC | token | (2 /* "setup" */ << 8), 8); + QTD_IOC | token | (PID_CODE_SETUP << 8), 8); submit_async(ehci, urb, &qtd_list, GFP_ATOMIC); return 0; /*Return now; we shall come back after 15 seconds*/ @@ -1216,7 +1212,7 @@ static int ehci_submit_single_step_set_feature( token ^= QTD_TOGGLE; /*We need to start IN with DATA-1 Pid-sequence*/ buf = urb->transfer_dma; - token |= (1 /* "in" */ << 8); /*This is IN stage*/ + token |= (PID_CODE_IN << 8); /*This is IN stage*/ maxpacket = usb_endpoint_maxp(&urb->ep->desc); @@ -1229,7 +1225,7 @@ static int ehci_submit_single_step_set_feature( qtd->hw_alt_next = EHCI_LIST_END(ehci); /* STATUS stage for GetDesc control request */ - token ^= 0x0100; /* "in" <--> "out" */ + token ^= (PID_CODE_IN << 8); /* "in" <--> "out" */ token |= QTD_TOGGLE; /* force DATA1 */ qtd_prev = qtd; diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index 1441e3400796..d7a3c8d13f6b 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-dbg.c | 10 +++++----- drivers/usb/host/ehci-q.c | 20 ++++++++------------ drivers/usb/host/ehci.h | 8 +++++++- 3 files changed, 20 insertions(+), 18 deletions(-)