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@xxxxxxx>
> ---
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@xxxxxxxxxxxxxxxxxxx>
> 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
>
