Re: [RFC/PATCH 24/45] usb: host: ehci: make use of new usb_endpoint_maxp_mult()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes:
> On Wed, 28 Sep 2016, Felipe Balbi wrote:
>
>> We have introduced a helper to calculate multiplier
>> value from wMaxPacketSize. Start using it.
>> 
>> Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>> Cc: <linux-usb@xxxxxxxxxxxxxxx>
>> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
>> ---
>>  drivers/usb/host/ehci-sched.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
>> index 1dfe54f14737..6a9fa2c3a24e 100644
>> --- a/drivers/usb/host/ehci-sched.c
>> +++ b/drivers/usb/host/ehci-sched.c
>> @@ -1064,7 +1064,7 @@ iso_stream_init(
>>  
>>  	/* knows about ITD vs SITD */
>>  	if (dev->speed == USB_SPEED_HIGH) {
>> -		unsigned multi = hb_mult(maxp);
>> +		unsigned multi = usb_endpoint_maxp_mult(&urb->ep->desc);
>>  
>>  		stream->highspeed = 1;
>
> There are lots of other places where the new helpers might be used.  
> Search for "max_packet" and "hb_mult" in ehci-q.c and ehci-sched.c.

Here's a new version:

8<------------------------------------------------------------------------------
From f4daa62bbbd1c03e349e576aef8f2a5f039afb1a Mon Sep 17 00:00:00 2001
From: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
Date: Wed, 28 Sep 2016 13:38:18 +0300
Subject: [PATCH] usb: host: ehci: make use of new usb_endpoint_maxp_mult()

We have introduced a helper to calculate multiplier
value from wMaxPacketSize. Start using it.

Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: <linux-usb@xxxxxxxxxxxxxxx>
Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
---
 drivers/usb/host/ehci-q.c     | 10 ++++++----
 drivers/usb/host/ehci-sched.c |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index eca3710d8fc4..a45a5dc7ed9f 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -550,8 +550,6 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
 
 /*-------------------------------------------------------------------------*/
 
-// high bandwidth multiplier, as encoded in highspeed endpoint descriptors
-#define hb_mult(wMaxPacketSize) (1 + (((wMaxPacketSize) >> 11) & 0x03))
 // ... and packet size, for any kind of endpoint descriptor
 #define max_packet(wMaxPacketSize) ((wMaxPacketSize) & 0x07ff)
 
@@ -770,9 +768,11 @@ qh_make (
 	gfp_t			flags
 ) {
 	struct ehci_qh		*qh = ehci_qh_alloc (ehci, flags);
+	struct usb_host_endpoint *ep;
 	u32			info1 = 0, info2 = 0;
 	int			is_input, type;
 	int			maxp = 0;
+	int			mult;
 	struct usb_tt		*tt = urb->dev->tt;
 	struct ehci_qh_hw	*hw;
 
@@ -787,7 +787,9 @@ qh_make (
 
 	is_input = usb_pipein (urb->pipe);
 	type = usb_pipetype (urb->pipe);
+	ep = usb_pipe_endpoint (urb->dev, urb->pipe);
 	maxp = usb_maxpacket (urb->dev, urb->pipe, !is_input);
+	mult = usb_endpoint_maxp_mult (&ep->desc);
 
 	/* 1024 byte maxpacket is a hardware ceiling.  High bandwidth
 	 * acts like up to 3KB, but is built from smaller packets.
@@ -810,7 +812,7 @@ qh_make (
 
 		qh->ps.usecs = NS_TO_US(usb_calc_bus_time(USB_SPEED_HIGH,
 				is_input, 0,
-				hb_mult(maxp) * max_packet(maxp)));
+				mult * max_packet(maxp)));
 		qh->ps.phase = NO_FRAME;
 
 		if (urb->dev->speed == USB_SPEED_HIGH) {
@@ -929,7 +931,7 @@ qh_make (
 			info2 |= (EHCI_TUNE_MULT_HS << 30);
 		} else {		/* PIPE_INTERRUPT */
 			info1 |= max_packet (maxp) << 16;
-			info2 |= hb_mult (maxp) << 30;
+			info2 |= mult << 30;
 		}
 		break;
 	default:
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 1dfe54f14737..6a9fa2c3a24e 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1064,7 +1064,7 @@ iso_stream_init(
 
 	/* knows about ITD vs SITD */
 	if (dev->speed == USB_SPEED_HIGH) {
-		unsigned multi = hb_mult(maxp);
+		unsigned multi = usb_endpoint_maxp_mult(&urb->ep->desc);
 
 		stream->highspeed = 1;
 
-- 
2.10.0.440.g21f862b


And I have also removed max_packet() macro as a separate patch. I'll
resend this series later with these patches included, but I wanted to
check if you're okay with the changes.

8<------------------------------------------------------------------------------
From 79361c0d6cc8c9815c7741b0801629460d0906fe Mon Sep 17 00:00:00 2001
From: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
Date: Fri, 30 Sep 2016 11:24:59 +0300
Subject: [PATCH] usb: host: ehci: remove unnecessary max_packet() macro

Now that usb_endpoint_maxp() only returns the lowest
11 bits from wMaxPacketSize, we can remove this macro
from the driver.

Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: <linux-usb@xxxxxxxxxxxxxxx>
Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
---
 drivers/usb/host/ehci-q.c     | 22 +++++++++-------------
 drivers/usb/host/ehci-sched.c |  1 -
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index a45a5dc7ed9f..8f3f055c05fa 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -550,9 +550,6 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
 
 /*-------------------------------------------------------------------------*/
 
-// ... and packet size, for any kind of endpoint descriptor
-#define max_packet(wMaxPacketSize) ((wMaxPacketSize) & 0x07ff)
-
 /*
  * reverse of qh_urb_transaction:  free a list of TDs.
  * used for cleanup after errors, before HC sees an URB's TDs.
@@ -649,7 +646,7 @@ qh_urb_transaction (
 		token |= (1 /* "in" */ << 8);
 	/* else it's already initted to "out" pid (0 << 8) */
 
-	maxpacket = max_packet(usb_maxpacket(urb->dev, urb->pipe, !is_input));
+	maxpacket = usb_maxpacket(urb->dev, urb->pipe, !is_input);
 
 	/*
 	 * buffer gets wrapped in one or more qtds;
@@ -788,14 +785,14 @@ qh_make (
 	is_input = usb_pipein (urb->pipe);
 	type = usb_pipetype (urb->pipe);
 	ep = usb_pipe_endpoint (urb->dev, urb->pipe);
-	maxp = usb_maxpacket (urb->dev, urb->pipe, !is_input);
+	maxp = usb_endpoint_maxp (&ep->desc);
 	mult = usb_endpoint_maxp_mult (&ep->desc);
 
 	/* 1024 byte maxpacket is a hardware ceiling.  High bandwidth
 	 * acts like up to 3KB, but is built from smaller packets.
 	 */
-	if (max_packet(maxp) > 1024) {
-		ehci_dbg(ehci, "bogus qh maxpacket %d\n", max_packet(maxp));
+	if (maxp > 1024) {
+		ehci_dbg(ehci, "bogus qh maxpacket %d\n", maxp);
 		goto done;
 	}
 
@@ -811,8 +808,7 @@ qh_make (
 		unsigned	tmp;
 
 		qh->ps.usecs = NS_TO_US(usb_calc_bus_time(USB_SPEED_HIGH,
-				is_input, 0,
-				mult * max_packet(maxp)));
+				is_input, 0, mult * maxp));
 		qh->ps.phase = NO_FRAME;
 
 		if (urb->dev->speed == USB_SPEED_HIGH) {
@@ -856,7 +852,7 @@ qh_make (
 			think_time = tt ? tt->think_time : 0;
 			qh->ps.tt_usecs = NS_TO_US(think_time +
 					usb_calc_bus_time (urb->dev->speed,
-					is_input, 0, max_packet (maxp)));
+					is_input, 0, maxp));
 			if (urb->interval > ehci->periodic_size)
 				urb->interval = ehci->periodic_size;
 			qh->ps.period = urb->interval;
@@ -927,10 +923,10 @@ qh_make (
 			 * to help them do so.  So now people expect to use
 			 * such nonconformant devices with Linux too; sigh.
 			 */
-			info1 |= max_packet(maxp) << 16;
+			info1 |= maxp << 16;
 			info2 |= (EHCI_TUNE_MULT_HS << 30);
 		} else {		/* PIPE_INTERRUPT */
-			info1 |= max_packet (maxp) << 16;
+			info1 |= maxp << 16;
 			info2 |= mult << 30;
 		}
 		break;
@@ -1223,7 +1219,7 @@ static int submit_single_step_set_feature(
 
 	token |= (1 /* "in" */ << 8);  /*This is IN stage*/
 
-	maxpacket = max_packet(usb_maxpacket(urb->dev, urb->pipe, 0));
+	maxpacket = usb_maxpacket(urb->dev, urb->pipe, 0);
 
 	qtd_fill(ehci, qtd, buf, len, token, maxpacket);
 
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 6a9fa2c3a24e..980a6b3b2da2 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1068,7 +1068,6 @@ iso_stream_init(
 
 		stream->highspeed = 1;
 
-		maxp = max_packet(maxp);
 		buf1 |= maxp;
 		maxp *= multi;
 
-- 
2.10.0.440.g21f862b

-- 
balbi

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux