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(-)
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(-)
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.1On 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
>
Macros with good names offer better readability. Besides, also move
the definition to ehci.h.
Signed-off-by: Dingyan Li <18500469033@163.com>
Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
---
V1 -> V2: Replacement in more places where Alan pointed out.
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.1On Sat, Mar 09, 2024 at 11:37:09AM +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> > Reviewed-by: Alan Stern <stern@rowland.harvard.edu> > --- > V1 -> V2: Replacement in more places where Alan pointed out. > This should be v3, as you have added Alan's reviewed-by, and added the proper versioning information. Please resend it as such as our tools can't dig it out from the end of the thread like this :( thanks, greg k-h
At 2024-03-09 15:14:48, "Greg KH" <gregkh@linuxfoundation.org> wrote: >On Sat, Mar 09, 2024 at 11:37:09AM +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> >> Reviewed-by: Alan Stern <stern@rowland.harvard.edu> >> --- >> V1 -> V2: Replacement in more places where Alan pointed out. >> > >This should be v3, as you have added Alan's reviewed-by, and added the >proper versioning information. Please resend it as such as our tools >can't dig it out from the end of the thread like this :( > >thanks, > >greg k-h Hi Greg, Sorry that I'm not quite familiar with the process. Since I haven't got any feedback for a while after sending the v3 patch. Do that mean the patch is accepted? Or anything else I still need to improve? Regards, Dingyan
Macros with good names offer better readability. Besides, also move
the definition to ehci.h.
Signed-off-by: Dingyan Li <18500469033@163.com>
Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
---
V1 -> V2: Replacement in more places where Alan pointed out.
V2 -> V3: Add Reviewed-by for Alan
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.1On 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>
> ---
> 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
>
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/process/submitting-patches.rst for what
needs to be done here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
© 2016 - 2026 Red Hat, Inc.