[U-Boot] [PATCH fix for v2014.10 0/5] USB bug-fixes for v2014.10

[U-Boot] [PATCH fix for v2014.10 0/5] USB bug-fixes for v2014.10

[Hans de Goede]

Hi Marek,

Here I thought I good get away from always spending my time working on USB
by hobbying a bit with ARM devices, only to end up doing USB work :)

I'm happy to report though that I've gotten USB keyboards to work reliable
with u-boot on Allwinner/sunxi devices. This is a first set of fixes stemming
from that work.

As the subject indicates I not only believe that these are suitable to go into
v2014.10, but I also believe that they actually should go into v2014.10 .

Regards,

Hans

[U-Boot] [PATCH fix for v2014.10 1/5] usb: ehci: Properly set hub devnum and portnr with usb-1 hubs in the chain

[Hans de Goede]

For full / low speed devices we need to get the devnum and portnr of the tt,
so of the first upstream usb-2 hub, not of the parent device (which may be a
usb-1 hub).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/host/ehci-hcd.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index eaf5913..41af302 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -273,6 +273,29 @@ static inline u8 ehci_encode_speed(enum usb_device_speed speed)
 	return QH_FULL_SPEED;
 }
 
+static void ehci_update_endpt2_dev_n_port(struct usb_device *dev,
+					  struct QH *qh)
+{
+	struct usb_device *ttdev;
+
+	if (dev->speed != USB_SPEED_LOW && dev->speed != USB_SPEED_FULL)
+		return;
+
+	/*
+	 * For full / low speed devices we need to get the devnum and portnr of
+	 * the tt, so of the first upstream usb-2 hub, there may be usb-1 hubs
+	 * in the tree before that one!
+	 */
+	ttdev = dev;
+	while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH)
+		ttdev = ttdev->parent;
+	if (!ttdev->parent)
+		return;
+
+	qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(ttdev->portnr) |
+				     QH_ENDPT2_HUBADDR(ttdev->parent->devnum));
+}
+
 static int
 ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		   int length, struct devrequest *req)
@@ -390,10 +413,9 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		QH_ENDPT1_ENDPT(usb_pipeendpoint(pipe)) | QH_ENDPT1_I(0) |
 		QH_ENDPT1_DEVADDR(usb_pipedevice(pipe));
 	qh->qh_endpt1 = cpu_to_hc32(endpt);
-	endpt = QH_ENDPT2_MULT(1) | QH_ENDPT2_PORTNUM(dev->portnr) |
-		QH_ENDPT2_HUBADDR(dev->parent->devnum) |
-		QH_ENDPT2_UFCMASK(0) | QH_ENDPT2_UFSMASK(0);
+	endpt = QH_ENDPT2_MULT(1) | QH_ENDPT2_UFCMASK(0) | QH_ENDPT2_UFSMASK(0);
 	qh->qh_endpt2 = cpu_to_hc32(endpt);
+	ehci_update_endpt2_dev_n_port(dev, qh);
 	qh->qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
 	qh->qh_overlay.qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
 
@@ -1201,12 +1223,10 @@ create_int_queue(struct usb_device *dev, unsigned long pipe, int queuesize,
 			(1 << 0)); /* S-mask: microframe 0 */
 		if (dev->speed == USB_SPEED_LOW ||
 				dev->speed == USB_SPEED_FULL) {
-			debug("TT: port: %d, hub address: %d\n",
-				dev->portnr, dev->parent->devnum);
-			qh->qh_endpt2 |= cpu_to_hc32((dev->portnr << 23) |
-				(dev->parent->devnum << 16) |
-				(0x1c << 8)); /* C-mask: microframes 2-4 */
+			/* C-mask: microframes 2-4 */
+			qh->qh_endpt2 |= cpu_to_hc32((0x1c << 8));
 		}
+		ehci_update_endpt2_dev_n_port(dev, qh);
 
 		td->qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
 		td->qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
-- 
2.1.0

[U-Boot] [PATCH fix for v2014.10 1/5] usb: ehci: Properly set hub devnum and portnr with usb-1 hubs in the chain

[Marek Vasut]

On Saturday, September 20, 2014 at 04:51:22 PM, Hans de Goede wrote:
> For full / low speed devices we need to get the devnum and portnr of the
> tt, so of the first upstream usb-2 hub, not of the parent device (which
> may be a usb-1 hub).
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied all, thanks a lot!

Best regards,
Marek Vasut

[U-Boot] [PATCH fix for v2014.10 2/5] usb: ehci: Add missing cache flush to destroy_int_queue

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/host/ehci-hcd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 41af302..1a0ddc3 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1342,6 +1342,8 @@ destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
 		if (NEXT_QH(cur) == queue->first) {
 			debug("found candidate. removing from chain\n");
 			cur->qh_link = queue->last->qh_link;
+			flush_dcache_range((uint32_t)cur,
+					   ALIGN_END_ADDR(struct QH, cur, 1));
 			result = 0;
 			break;
 		}
-- 
2.1.0

[U-Boot] [PATCH fix for v2014.10 2/5] usb: ehci: Add missing cache flush to destroy_int_queue

[Michael Trimarchi]

Hi

On Sat, Sep 20, 2014 at 4:51 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/host/ehci-hcd.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 41af302..1a0ddc3 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -1342,6 +1342,8 @@ destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
>                 if (NEXT_QH(cur) == queue->first) {
>                         debug("found candidate. removing from chain\n");
>                         cur->qh_link = queue->last->qh_link;
> +                       flush_dcache_range((uint32_t)cur,
> +                                          ALIGN_END_ADDR(struct QH, cur, 1));
>                         result = 0;
>                         break;
>                 }

Can you even check the return value to make consistent with create_queue?

Michael

> --
> 2.1.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH fix for v2014.10 3/5] usb: ehci: poll_int_queue check real qtd, not the overlay

[Hans de Goede]

When we first start an int queue, the qh's overlay area is all zeros. This
gets filled by the hc with the actual qtd values as soon as it advances
the queue, but we may call poll_int_queue before then, in which case we
would think the transfer has completed as the hc has not yet copied the
qt_token to the overlay, so the active flag is not set.

This fixes this by checking the actual qtd token, rather then the overlay.
This also fixes a (theoretical) race where we see the completion in the
overlay and free and re-use the qtd before the hc has completed writing back
the overlay to the actual qtd.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/host/ehci-hcd.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 1a0ddc3..635a3b4 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1297,6 +1297,7 @@ fail1:
 void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
 {
 	struct QH *cur = queue->current;
+	struct qTD *cur_td;
 
 	/* depleted queue */
 	if (cur == NULL) {
@@ -1304,20 +1305,21 @@ void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
 		return NULL;
 	}
 	/* still active */
-	invalidate_dcache_range((uint32_t)cur,
-				ALIGN_END_ADDR(struct QH, cur, 1));
-	if (cur->qh_overlay.qt_token & cpu_to_hc32(0x80)) {
-		debug("Exit poll_int_queue with no completed intr transfer. "
-		      "token is %x\n", cur->qh_overlay.qt_token);
+	cur_td = &queue->tds[queue->current - queue->first];
+	invalidate_dcache_range((uint32_t)cur_td,
+				ALIGN_END_ADDR(struct qTD, cur_td, 1));
+	if (QT_TOKEN_GET_STATUS(hc32_to_cpu(cur_td->qt_token)) &
+			QT_TOKEN_STATUS_ACTIVE) {
+		debug("Exit poll_int_queue with no completed intr transfer. token is %x\n",
+		      hc32_to_cpu(cur_td->qt_token));
 		return NULL;
 	}
 	if (!(cur->qh_link & QH_LINK_TERMINATE))
 		queue->current++;
 	else
 		queue->current = NULL;
-	debug("Exit poll_int_queue with completed intr transfer. "
-	      "token is %x at %p (first at %p)\n", cur->qh_overlay.qt_token,
-	      &cur->qh_overlay.qt_token, queue->first);
+	debug("Exit poll_int_queue with completed intr transfer. token is %x at %p (first at %p)\n",
+	      hc32_to_cpu(cur_td->qt_token), cur, queue->first);
 	return cur->buffer;
 }
 
-- 
2.1.0

[U-Boot] [PATCH fix for v2014.10 4/5] usb: ehci: Make periodic_schedules a per controller variable

[Hans de Goede]

Periodic schedules tracks how many int_queue-s are active, and decides whether
or not to en/disable the periodic schedule based on this. This is clearly
a per controller thing.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/host/ehci-hcd.c | 9 ++++-----
 drivers/usb/host/ehci.h     | 1 +
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 635a3b4..6323c50 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -996,6 +996,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
 	 * Set up periodic list
 	 * Step 1: Parent QH for all periodic transfers.
 	 */
+	ehcic[index].periodic_schedules = 0;
 	periodic = &ehcic[index].periodic_queue;
 	memset(periodic, 0, sizeof(*periodic));
 	periodic->qh_link = cpu_to_hc32(QH_LINK_TERMINATE);
@@ -1154,8 +1155,6 @@ disable_periodic(struct ehci_ctrl *ctrl)
 	return 0;
 }
 
-static int periodic_schedules;
-
 struct int_queue *
 create_int_queue(struct usb_device *dev, unsigned long pipe, int queuesize,
 		 int elementsize, void *buffer)
@@ -1278,7 +1277,7 @@ create_int_queue(struct usb_device *dev, unsigned long pipe, int queuesize,
 		debug("FATAL: periodic should never fail, but did");
 		goto fail3;
 	}
-	periodic_schedules++;
+	ctrl->periodic_schedules++;
 
 	debug("Exit create_int_queue\n");
 	return result;
@@ -1335,7 +1334,7 @@ destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
 		debug("FATAL: periodic should never fail, but did");
 		goto out;
 	}
-	periodic_schedules--;
+	ctrl->periodic_schedules--;
 
 	struct QH *cur = &ctrl->periodic_queue;
 	timeout = get_timer(0) + 500; /* abort after 500ms */
@@ -1357,7 +1356,7 @@ destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
 		}
 	}
 
-	if (periodic_schedules > 0) {
+	if (ctrl->periodic_schedules > 0) {
 		result = enable_periodic(ctrl);
 		if (result < 0)
 			debug("FATAL: periodic should never fail, but did");
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 093eb4b..433e703 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -246,6 +246,7 @@ struct ehci_ctrl {
 	struct QH qh_list __aligned(USB_DMA_MINALIGN);
 	struct QH periodic_queue __aligned(USB_DMA_MINALIGN);
 	uint32_t *periodic_list;
+	int periodic_schedules;
 	int ntds;
 };
 
-- 
2.1.0

[U-Boot] [PATCH fix for v2014.10 5/5] usb: kbd: Fix unaligned buffer usage in usb_kbd_setled()

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 common/usb_kbd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index c34fd5c..87f4125 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -170,11 +170,12 @@ static void usb_kbd_setled(struct usb_device *dev)
 {
 	struct usb_interface *iface = &dev->config.if_desc[0];
 	struct usb_kbd_pdata *data = dev->privptr;
-	uint32_t leds = data->flags & USB_KBD_LEDMASK;
+	ALLOC_ALIGN_BUFFER(uint32_t, leds, 1, USB_DMA_MINALIGN);
 
+	*leds = data->flags & USB_KBD_LEDMASK;
 	usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
 		USB_REQ_SET_REPORT, USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-		0x200, iface->desc.bInterfaceNumber, (void *)&leds, 1, 0);
+		0x200, iface->desc.bInterfaceNumber, leds, 1, 0);
 }
 
 #define CAPITAL_MASK	0x20
-- 
2.1.0