mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01
@ 2024-02-19 13:38 Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 01/15] usb: xhci: usb: xhci: avoid type conversion of void * Ahmad Fatoum
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox

The latest U-Boot release saw some fixes to the xHCI driver contributed
by the Asahi Linux project to improve robustness in case of errors.

Port these to barebox as well as a couple of cleanup commits that make
the code in barebox look more similar to changes that happened to U-Boot
in the meantime.

Ahmad Fatoum (15):
  usb: xhci: usb: xhci: avoid type conversion of void *
  usb: xhci: add various debugging prints
  usb: xhci: call xhci_flush_cache where appropriate
  usb: xhci: use macros for formatting values
  usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)
  usb: xhci: Add missing xhci_readl()
  usb: xhci: don't use xhci_writeq for normal SDRAM
  usb: xhci: support non-1:1 mapped xHCI
  usb: xhci: reset endpoint on USB stall
  usb: xhci: Fix root hub descriptor
  usb: xhci: Guard all calls to xhci_wait_for_event
  usb: xhci: Better error handling in abort_td()
  usb: xhci: Allow context state errors when halting an endpoint
  usb: xhci: Recover from halted bulk endpoints
  usb: xhci: Do not panic on event timeouts

 drivers/usb/host/xhci-mem.c  |  99 +++++++++---------
 drivers/usb/host/xhci-ring.c | 190 +++++++++++++++++++++++++++--------
 drivers/usb/host/xhci.c      |  62 ++++++++----
 drivers/usb/host/xhci.h      |  16 ++-
 4 files changed, 247 insertions(+), 120 deletions(-)

-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 01/15] usb: xhci: usb: xhci: avoid type conversion of void *
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 02/15] usb: xhci: add various debugging prints Ahmad Fatoum
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Original U-Boot commit 3fade88686e71c9acee4cbeb3ae9706bbc845608:

| Author:     Heinrich Schuchardt <xypron.glpk@gmx.de>
| AuthorDate: Tue Sep 29 22:03:01 2020 +0200
|
| usb: xhci: avoid type conversion of void *
|
| void * can be assigned to any pointer variable. Avoid
| unnecessary conversions.
|
| Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

removes 8 such instances, of which only one remains in barebox.
So remove that single remaining cast.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-mem.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index f975b1c0a931..5ae1512af53a 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -464,8 +464,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
 		return -EEXIST;
 	}
 
-	ctrl->devs[slot_id] = (struct xhci_virt_device *)
-					malloc(sizeof(struct xhci_virt_device));
+	ctrl->devs[slot_id] = malloc(sizeof(struct xhci_virt_device));
 
 	if (!ctrl->devs[slot_id]) {
 		dev_err(ctrl->dev, "Failed to allocate virtual device\n");
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 02/15] usb: xhci: add various debugging prints
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 01/15] usb: xhci: usb: xhci: avoid type conversion of void * Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 03/15] usb: xhci: call xhci_flush_cache where appropriate Ahmad Fatoum
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

For debugging potential issues around the driver, add some debug
prints.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-ring.c |  8 ++++++++
 drivers/usb/host/xhci.c      | 16 +++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 547047305dd0..804384e0307f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -461,6 +461,14 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
 			BUG_ON(GET_COMP_CODE(
 				le32_to_cpu(event->generic.field[2])) !=
 								COMP_SUCCESS);
+		else
+			dev_dbg(ctrl->dev, "Unexpected XHCI event TRB, skipping... "
+				"(%08x %08x %08x %08x)\n",
+				le32_to_cpu(event->generic.field[0]),
+				le32_to_cpu(event->generic.field[1]),
+				le32_to_cpu(event->generic.field[2]),
+				le32_to_cpu(event->generic.field[3]));
+
 		xhci_acknowledge_event(ctrl);
 	} while (!is_timeout_non_interruptible(start, timeout_ms * MSECOND));
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f149e784524d..01817d9c79cd 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -151,6 +151,8 @@ static int xhci_start(struct xhci_ctrl *ctrl)
 	u32 temp;
 	int ret;
 
+	dev_dbg(ctrl->dev, "Starting the controller\n");
+
 	temp = xhci_readl(&hcor->or_usbcmd);
 	temp |= (CMD_RUN);
 	xhci_writel(&hcor->or_usbcmd, temp);
@@ -1088,8 +1090,10 @@ static int xhci_submit_root(struct usb_device *udev, unsigned long pipe,
 static int _xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe,
 				void *buffer, int length, int interval)
 {
-	if (usb_pipetype(pipe) != PIPE_INTERRUPT)
+	if (usb_pipetype(pipe) != PIPE_INTERRUPT) {
+		dev_err(&udev->dev, "non-interrupt pipe (type=%lu)", usb_pipetype(pipe));
 		return -EINVAL;
+	}
 
 	/*
 	 * xHCI uses normal TRBs for both bulk and interrupt. When the
@@ -1112,8 +1116,10 @@ static int _xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe,
 static int _xhci_submit_bulk_msg(struct usb_device *udev, unsigned long pipe,
 				 void *buffer, int length, int timeout_ms)
 {
-	if (usb_pipetype(pipe) != PIPE_BULK)
+	if (usb_pipetype(pipe) != PIPE_BULK) {
+		dev_err(&udev->dev, "non-bulk pipe (type=%lu)", usb_pipetype(pipe));
 		return -EINVAL;
+	}
 
 	return xhci_bulk_tx(udev, pipe, length, buffer, timeout_ms);
 }
@@ -1137,8 +1143,10 @@ static int _xhci_submit_control_msg(struct usb_device *udev, unsigned long pipe,
 	struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
 	int ret = 0;
 
-	if (usb_pipetype(pipe) != PIPE_CONTROL)
+	if (usb_pipetype(pipe) != PIPE_CONTROL) {
+		dev_err(&udev->dev, "non-control pipe (type=%lu)", usb_pipetype(pipe));
 		return -EINVAL;
+	}
 
 	if (usb_pipedevice(pipe) == ctrl->rootdev)
 		return xhci_submit_root(udev, pipe, buffer, setup);
@@ -1218,6 +1226,8 @@ static int xhci_lowlevel_stop(struct xhci_ctrl *ctrl)
 
 	xhci_reset(ctrl);
 
+	dev_dbg(ctrl->dev, "Disabling event ring interrupts\n");
+
 	temp = xhci_readl(&ctrl->hcor->or_usbsts);
 	xhci_writel(&ctrl->hcor->or_usbsts, temp & ~STS_EINT);
 	temp = xhci_readl(&ctrl->ir_set->irq_pending);
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 03/15] usb: xhci: call xhci_flush_cache where appropriate
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 01/15] usb: xhci: usb: xhci: avoid type conversion of void * Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 02/15] usb: xhci: add various debugging prints Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 04/15] usb: xhci: use macros for formatting values Ahmad Fatoum
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

xhci_flush_cache() is a no-op in barebox, because we use dma-coherent
allocations. Nevertheless, we define a stub for xhci_flush_cache, which
is used should we ever want to change this behavior.

The original U-Boot code doesn't use dma-coherent allocations and has
run into a few cache invalidation issues. Import the xhci_flush_cache()
added in the meantime.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-mem.c  | 3 +++
 drivers/usb/host/xhci-ring.c | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 5ae1512af53a..db38fd43f65a 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -408,6 +408,9 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
 		scratchpad->sp_array[i] = cpu_to_le64(ptr);
 	}
 
+	xhci_flush_cache((uintptr_t)scratchpad->sp_array,
+			 sizeof(u64) * num_sp);
+
 	return 0;
 
 fail_sp3:
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 804384e0307f..d725b71983cd 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -670,6 +670,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 
 	first_trb = true;
 
+	/* flush the buffer before use */
+	xhci_flush_cache((uintptr_t)buffer, length);
+
 	/* Queue the first TRB, even if it's zero-length */
 	do {
 		u32 remainder = 0;
@@ -913,6 +916,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 		trb_fields[2] = length_field;
 		trb_fields[3] = field | ep_ring->cycle_state;
 
+		xhci_flush_cache((uintptr_t)buffer, length);
 		queue_trb(ctrl, ep_ring, true, trb_fields);
 	}
 
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 04/15] usb: xhci: use macros for formatting values
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 03/15] usb: xhci: call xhci_flush_cache where appropriate Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 05/15] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu) Ahmad Fatoum
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

We have function-like macros for formatting various values that we stuff
into the descriptors instead of manually having to mask and shift.

Make use of these to make the code more concise.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-mem.c  | 18 ++++++------------
 drivers/usb/host/xhci-ring.c | 29 +++++++++++++----------------
 drivers/usb/host/xhci.c      |  9 +++------
 3 files changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index db38fd43f65a..1a66bc6a61a4 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -249,8 +249,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
 		 */
 		val = le32_to_cpu(prev->trbs[TRBS_PER_SEGMENT-1].link.control);
 		val &= ~TRB_TYPE_BITMASK;
-		val |= (TRB_LINK << TRB_TYPE_SHIFT);
-
+		val |= TRB_TYPE(TRB_LINK);
 		prev->trbs[TRBS_PER_SEGMENT-1].link.control = cpu_to_le32(val);
 	}
 }
@@ -826,25 +825,22 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 
 	/* Step 4 - ring already allocated */
 	/* Step 5 */
-	ep0_ctx->ep_info2 = cpu_to_le32(CTRL_EP << EP_TYPE_SHIFT);
+	ep0_ctx->ep_info2 = cpu_to_le32(EP_TYPE(CTRL_EP));
 	dev_dbg(&udev->dev, "SPEED = %d\n", speed);
 
 	switch (speed) {
 	case USB_SPEED_SUPER:
-		ep0_ctx->ep_info2 |= cpu_to_le32(((512 & MAX_PACKET_MASK) <<
-					MAX_PACKET_SHIFT));
+		ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(512));
 		dev_dbg(&udev->dev, "Setting Packet size = 512bytes\n");
 		break;
 	case USB_SPEED_HIGH:
 	/* USB core guesses at a 64-byte max packet first for FS devices */
 	case USB_SPEED_FULL:
-		ep0_ctx->ep_info2 |= cpu_to_le32(((64 & MAX_PACKET_MASK) <<
-					MAX_PACKET_SHIFT));
+		ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(64));
 		dev_dbg(&udev->dev, "Setting Packet size = 64bytes\n");
 		break;
 	case USB_SPEED_LOW:
-		ep0_ctx->ep_info2 |= cpu_to_le32(((8 & MAX_PACKET_MASK) <<
-					MAX_PACKET_SHIFT));
+		ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(8));
 		dev_dbg(&udev->dev, "Setting Packet size = 8bytes\n");
 		break;
 	default:
@@ -853,9 +849,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 	}
 
 	/* EP 0 can handle "burst" sizes of 1, so Max Burst Size field is 0 */
-	ep0_ctx->ep_info2 |=
-			cpu_to_le32(((0 & MAX_BURST_MASK) << MAX_BURST_SHIFT) |
-			((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
+	ep0_ctx->ep_info2 |= cpu_to_le32(MAX_BURST(0) | ERROR_COUNT(3));
 
 	trb_64 = (uintptr_t)virt_dev->eps[0].ring->first_seg->trbs;
 	ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d725b71983cd..818b662e70d2 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -709,15 +709,14 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 							   maxpacketsize,
 							   num_trbs - 1);
 
-		length_field = ((trb_buff_len & TRB_LEN_MASK) |
+		length_field = (TRB_LEN(trb_buff_len) |
 				remainder |
-				((0 & TRB_INTR_TARGET_MASK) <<
-				TRB_INTR_TARGET_SHIFT));
+				TRB_INTR_TARGET(0));
 
 		trb_fields[0] = lower_32_bits(addr);
 		trb_fields[1] = upper_32_bits(addr);
 		trb_fields[2] = length_field;
-		trb_fields[3] = field | (TRB_NORMAL << TRB_TYPE_SHIFT);
+		trb_fields[3] = field | TRB_TYPE(TRB_NORMAL);
 
 		queue_trb(ctrl, ring, (num_trbs > 1), trb_fields);
 
@@ -853,7 +852,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	/* Queue setup TRB - see section 6.4.1.2.1 */
 	/* FIXME better way to translate setup_packet into two u32 fields? */
 	field = 0;
-	field |= TRB_IDT | (TRB_SETUP << TRB_TYPE_SHIFT);
+	field |= TRB_IDT | TRB_TYPE(TRB_SETUP);
 	if (start_cycle == 0)
 		field |= 0x1;
 
@@ -861,9 +860,9 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	if (HC_VERSION(xhci_readl(&ctrl->hccr->cr_capbase)) >= 0x100) {
 		if (length > 0) {
 			if (req->requesttype & USB_DIR_IN)
-				field |= (TRB_DATA_IN << TRB_TX_TYPE_SHIFT);
+				field |= TRB_TX_TYPE(TRB_DATA_IN);
 			else
-				field |= (TRB_DATA_OUT << TRB_TX_TYPE_SHIFT);
+				field |= TRB_TX_TYPE(TRB_DATA_OUT);
 		}
 	}
 
@@ -879,8 +878,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	trb_fields[1] = le16_to_cpu(req->index) |
 			le16_to_cpu(req->length) << 16;
 	/* TRB_LEN | (TRB_INTR_TARGET) */
-	trb_fields[2] = (8 | ((0 & TRB_INTR_TARGET_MASK) <<
-			TRB_INTR_TARGET_SHIFT));
+	trb_fields[2] = (TRB_LEN(8) | TRB_INTR_TARGET(0));
 	/* Immediate data in pointer */
 	trb_fields[3] = field;
 	queue_trb(ctrl, ep_ring, true, trb_fields);
@@ -890,15 +888,15 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	/* If there's data, queue data TRBs */
 	/* Only set interrupt on short packet for IN endpoints */
 	if (usb_pipein(pipe))
-		field = TRB_ISP | (TRB_DATA << TRB_TYPE_SHIFT);
+		field = TRB_ISP | TRB_TYPE(TRB_DATA);
 	else
-		field = (TRB_DATA << TRB_TYPE_SHIFT);
+		field = TRB_TYPE(TRB_DATA);
 
-	length_field = (length & TRB_LEN_MASK) | xhci_td_remainder(length) |
+	length_field = TRB_LEN(length) | xhci_td_remainder(length) |
 			((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
 	dev_dbg(&udev->dev, "length_field = %d, length = %d,"
 		"xhci_td_remainder(length) = %d , TRB_INTR_TARGET(0) = %d\n",
-		length_field, (length & TRB_LEN_MASK),
+		length_field, TRB_LEN(length),
 		xhci_td_remainder(length), 0);
 
 	if (req->requesttype & USB_DIR_IN)
@@ -934,11 +932,10 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 
 	trb_fields[0] = 0;
 	trb_fields[1] = 0;
-	trb_fields[2] = ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
+	trb_fields[2] = TRB_INTR_TARGET(0);
 		/* Event on completion */
 	trb_fields[3] = field | TRB_IOC |
-			(TRB_STATUS << TRB_TYPE_SHIFT) |
-			ep_ring->cycle_state;
+			TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state;
 
 	queue_trb(ctrl, ep_ring, false, trb_fields);
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 01817d9c79cd..92101f8f67d9 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -568,8 +568,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 			cpu_to_le32(EP_MAX_ESIT_PAYLOAD_HI(max_esit_payload) |
 			EP_INTERVAL(interval) | EP_MULT(mult));
 
-		ep_ctx[ep_index]->ep_info2 =
-			cpu_to_le32(ep_type << EP_TYPE_SHIFT);
+		ep_ctx[ep_index]->ep_info2 = cpu_to_le32(EP_TYPE(ep_type));
 		ep_ctx[ep_index]->ep_info2 |=
 			cpu_to_le32(MAX_PACKET
 			(get_unaligned(&endpt_desc->wMaxPacketSize)));
@@ -766,8 +765,7 @@ int xhci_check_maxpacket(struct usb_device *udev)
 				ctrl->devs[slot_id]->out_ctx, ep_index);
 		in_ctx = ctrl->devs[slot_id]->in_ctx;
 		ep_ctx = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
-		ep_ctx->ep_info2 &= cpu_to_le32(~((0xffff & MAX_PACKET_MASK)
-						<< MAX_PACKET_SHIFT));
+		ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET(MAX_PACKET_MASK));
 		ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size));
 
 		/*
@@ -1191,8 +1189,7 @@ static int xhci_lowlevel_init(struct xhci_ctrl *ctrl)
 		return -ENOMEM;
 
 	reg = xhci_readl(&hccr->cr_hcsparams1);
-	descriptor.hub.bNbrPorts = ((reg & HCS_MAX_PORTS_MASK) >>
-						HCS_MAX_PORTS_SHIFT);
+	descriptor.hub.bNbrPorts = HCS_MAX_PORTS(reg);
 
 	/* Port Indicators */
 	reg = xhci_readl(&hccr->cr_hccparams);
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 05/15] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 04/15] usb: xhci: use macros for formatting values Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 06/15] usb: xhci: Add missing xhci_readl() Ahmad Fatoum
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This imports U-Boot commit 543eb12ecd91df324554b8abc8d52e965bd4922b:

| Author:     Stefan Roese <sr@denx.de>
| AuthorDate: Tue Jul 21 10:46:02 2020 +0200
|
| usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)
|
| While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
| which is big endian, I noticed that the driver is missing a few endian
| conversion calls. This patch adds these missing endian conversion
| calls.
|
| Signed-off-by: Stefan Roese <sr@denx.de>

This introduces no functional change for existing platforms as the
barebox xHCI driver doesn't run on any big-endian CPUs.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-mem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 1a66bc6a61a4..c49693bd9f1b 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -498,7 +498,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
 	byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
 
 	/* Point to output device context in dcbaa. */
-	ctrl->dcbaa->dev_context_ptrs[slot_id] = byte_64;
+	ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
 
 	xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[slot_id],
 			 sizeof(__le64));
@@ -777,7 +777,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 
 	dev_dbg(&udev->dev, "route string 0x%x\n", route);
 
-	slot_ctx->dev_info |= route;
+	slot_ctx->dev_info |= cpu_to_le32(route);
 
 	switch (speed) {
 	case USB_SPEED_SUPER:
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 06/15] usb: xhci: Add missing xhci_readl()
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 05/15] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu) Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 07/15] usb: xhci: don't use xhci_writeq for normal SDRAM Ahmad Fatoum
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

xhci_readl() doesn't differ from readl(), but for uniformity, use
xhci_ prefixed accessors throughout.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-mem.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index c49693bd9f1b..915b083bfd1d 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -440,9 +440,9 @@ static struct xhci_container_ctx
 	BUG_ON((type != XHCI_CTX_TYPE_DEVICE) && (type != XHCI_CTX_TYPE_INPUT));
 	ctx->type = type;
 	ctx->size = (MAX_EP_CTX_NUM + 1) *
-			CTX_SIZE(readl(&ctrl->hccr->cr_hccparams));
+			CTX_SIZE(xhci_readl(&ctrl->hccr->cr_hccparams));
 	if (type == XHCI_CTX_TYPE_INPUT)
-		ctx->size += CTX_SIZE(readl(&ctrl->hccr->cr_hccparams));
+		ctx->size += CTX_SIZE(xhci_readl(&ctrl->hccr->cr_hccparams));
 
 	ctx->bytes = xhci_malloc(ctrl, ctx->size);
 
@@ -646,7 +646,7 @@ struct xhci_slot_ctx *xhci_get_slot_ctx(struct xhci_ctrl *ctrl,
 		return (struct xhci_slot_ctx *)ctx->bytes;
 
 	return (struct xhci_slot_ctx *)
-		(ctx->bytes + CTX_SIZE(readl(&ctrl->hccr->cr_hccparams)));
+		(ctx->bytes + CTX_SIZE(xhci_readl(&ctrl->hccr->cr_hccparams)));
 }
 
 /**
@@ -668,7 +668,7 @@ struct xhci_ep_ctx *xhci_get_ep_ctx(struct xhci_ctrl *ctrl,
 
 	return (struct xhci_ep_ctx *)
 		(ctx->bytes +
-		(ep_index * CTX_SIZE(readl(&ctrl->hccr->cr_hccparams))));
+		(ep_index * CTX_SIZE(xhci_readl(&ctrl->hccr->cr_hccparams))));
 }
 
 /**
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 07/15] usb: xhci: don't use xhci_writeq for normal SDRAM
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (5 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 06/15] usb: xhci: Add missing xhci_readl() Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 08/15] usb: xhci: support non-1:1 mapped xHCI Ahmad Fatoum
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

While this shouldn't matter, it's confusing to use cpu_to_le32
for one struct field and writeq for another, so use cpu_to_leXX
for both.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 915b083bfd1d..cfd52566d772 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -573,7 +573,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 
 		trb_64 = 0;
 		trb_64 = (uintptr_t)seg->trbs;
-		xhci_writeq(&entry->seg_addr, trb_64);
+		entry->seg_addr = cpu_to_le64(trb_64);
 		entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
 		entry->rsvd = 0;
 		seg = seg->next;
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 08/15] usb: xhci: support non-1:1 mapped xHCI
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (6 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 07/15] usb: xhci: don't use xhci_writeq for normal SDRAM Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 09/15] usb: xhci: reset endpoint on USB stall Ahmad Fatoum
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The code so far assumed 1:1 mapped xHCI, which would apparently fail for
the Raspberry Pi 4 (with its PCIe xHCI controller) and for Apple Silicon
(where xHCI sits behind an IOMMU).

The latter can't be fixed without having dma_alloc_coherent take a dev
pointer, but the former can be fixed with existing API if we would
just start using the DMA address return parameter instead of specifying
DMA_ADDRESS_BROKEN, so let's do just that.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-mem.c  | 61 ++++++++++++++++++------------------
 drivers/usb/host/xhci-ring.c | 51 +++++++++++++++++++++---------
 drivers/usb/host/xhci.c      | 11 ++++---
 drivers/usb/host/xhci.h      | 13 ++++++--
 4 files changed, 83 insertions(+), 53 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index cfd52566d772..e962bfde3f56 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -82,11 +82,11 @@ static void xhci_free(struct xhci_ctrl *ctrl, void *ptr)
  * @param size	size of memory to be allocated
  * @return allocates the memory and returns the aligned pointer
  */
-static void *xhci_malloc(struct xhci_ctrl *ctrl, unsigned int size)
+static void *xhci_malloc(struct xhci_ctrl *ctrl, unsigned int size, dma_addr_t *dma_addr)
 {
 	void *ptr;
 
-	ptr = dma_alloc_coherent(size, DMA_ADDRESS_BROKEN);
+	ptr = dma_alloc_coherent(size, dma_addr);
 	if (!ptr)
 		return NULL;
 
@@ -145,7 +145,7 @@ static void xhci_scratchpad_free(struct xhci_ctrl *ctrl)
 
 	ctrl->dcbaa->dev_context_ptrs[0] = 0;
 
-	xhci_free(ctrl, (void *)(uintptr_t)ctrl->scratchpad->sp_array[0]);
+	xhci_free(ctrl, ctrl->scratchpad->scratchpad);
 	xhci_free(ctrl, ctrl->scratchpad->sp_array);
 	free(ctrl->scratchpad);
 	ctrl->scratchpad = NULL;
@@ -234,14 +234,13 @@ static void xhci_link_segments(struct xhci_segment *prev,
 				struct xhci_segment *next, bool link_trbs)
 {
 	u32 val;
-	u64 val_64 = 0;
 
 	if (!prev || !next)
 		return;
 	prev->next = next;
 	if (link_trbs) {
-		val_64 = (uintptr_t)next->trbs;
-		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr = val_64;
+		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
+			cpu_to_le64(next->dma);
 
 		/*
 		 * Set the last TRB in the segment to
@@ -294,7 +293,7 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_ctrl *ctrl)
 
 	seg = xzalloc(sizeof(*seg));
 
-	seg->trbs = xhci_malloc(ctrl, SEGMENT_SIZE);
+	seg->trbs = xhci_malloc(ctrl, SEGMENT_SIZE, &seg->dma);
 
 	return seg;
 }
@@ -364,6 +363,7 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
 	struct xhci_hccr *hccr = ctrl->hccr;
 	struct xhci_hcor *hcor = ctrl->hcor;
 	struct xhci_scratchpad *scratchpad;
+	dma_addr_t val_64;
 	int num_sp;
 	uint32_t page_size;
 	void *buf;
@@ -378,11 +378,11 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
 		goto fail_sp;
 	ctrl->scratchpad = scratchpad;
 
-	scratchpad->sp_array = xhci_malloc(ctrl, num_sp * sizeof(u64));
+	scratchpad->sp_array = xhci_malloc(ctrl, num_sp * sizeof(u64), &val_64);
 	if (!scratchpad->sp_array)
 		goto fail_sp2;
-	ctrl->dcbaa->dev_context_ptrs[0] =
-		cpu_to_le64((uintptr_t)scratchpad->sp_array);
+
+	ctrl->dcbaa->dev_context_ptrs[0] = cpu_to_le64(val_64);
 
 	xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[0],
 		sizeof(ctrl->dcbaa->dev_context_ptrs[0]));
@@ -396,15 +396,16 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
 	BUG_ON(i == 16);
 
 	page_size = 1 << (i + 12);
-	buf = xhci_malloc(ctrl, num_sp * page_size);
+	buf = xhci_malloc(ctrl, num_sp * page_size, &val_64);
 	if (!buf)
 		goto fail_sp3;
 
 	xhci_flush_cache((uintptr_t)buf, num_sp * page_size);
 
+	scratchpad->scratchpad = buf;
 	for (i = 0; i < num_sp; i++) {
-		uintptr_t ptr = (uintptr_t)buf + i * page_size;
-		scratchpad->sp_array[i] = cpu_to_le64(ptr);
+		scratchpad->sp_array[i] = cpu_to_le64(val_64);
+		val_64 += page_size;
 	}
 
 	xhci_flush_cache((uintptr_t)scratchpad->sp_array,
@@ -444,7 +445,7 @@ static struct xhci_container_ctx
 	if (type == XHCI_CTX_TYPE_INPUT)
 		ctx->size += CTX_SIZE(xhci_readl(&ctrl->hccr->cr_hccparams));
 
-	ctx->bytes = xhci_malloc(ctrl, ctx->size);
+	ctx->bytes = xhci_malloc(ctrl, ctx->size, &ctx->dma);
 
 	return ctx;
 }
@@ -495,7 +496,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
 	/* Allocate endpoint 0 ring */
 	virt_dev->eps[0].ring = xhci_ring_alloc(ctrl, 1, true);
 
-	byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
+	byte_64 = virt_dev->out_ctx->dma;
 
 	/* Point to output device context in dcbaa. */
 	ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
@@ -517,29 +518,27 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
 int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 					struct xhci_hcor *hcor)
 {
+	dma_addr_t dma;
 	uint64_t val_64;
 	uint64_t trb_64;
 	uint32_t val;
-	unsigned long deq;
+	uint64_t deq;
 	int i;
 	struct xhci_segment *seg;
 
 	/* DCBAA initialization */
-	ctrl->dcbaa = xhci_malloc(ctrl, sizeof(*ctrl->dcbaa));
-	if (!ctrl->dcbaa) {
-		dev_err(ctrl->dev, "unable to allocate DCBA\n");
-		return -ENOMEM;
-	}
+	ctrl->dcbaa = xhci_malloc(ctrl, sizeof(struct xhci_device_context_array),
+				  &dma);
+	ctrl->dcbaa->dma = dma;
 
-	val_64 = (uintptr_t)ctrl->dcbaa;
 	/* Set the pointer in DCBAA register */
-	xhci_writeq(&hcor->or_dcbaap, val_64);
+	xhci_writeq(&hcor->or_dcbaap, dma);
 
 	/* Command ring control pointer register initialization */
 	ctrl->cmd_ring = xhci_ring_alloc(ctrl, 1, true);
 
 	/* Set the address in the Command Ring Control register */
-	trb_64 = (uintptr_t)ctrl->cmd_ring->first_seg->trbs;
+	trb_64 = ctrl->cmd_ring->first_seg->dma;
 	val_64 = xhci_readq(&hcor->or_crcr);
 	val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
 		(trb_64 & (u64) ~CMD_RING_RSVD_BITS) |
@@ -561,8 +560,8 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 
 	/* Event ring does not maintain link TRB */
 	ctrl->event_ring = xhci_ring_alloc(ctrl, ERST_NUM_SEGS, false);
-	ctrl->erst.entries =
-		xhci_malloc(ctrl, sizeof(struct xhci_erst_entry) * ERST_NUM_SEGS);
+	ctrl->erst.entries = xhci_malloc(ctrl, sizeof(struct xhci_erst_entry) *
+					 ERST_NUM_SEGS, &ctrl->erst.erst_dma_addr);
 
 	ctrl->erst.num_entries = ERST_NUM_SEGS;
 
@@ -571,8 +570,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 			val++) {
 		struct xhci_erst_entry *entry = &ctrl->erst.entries[val];
 
-		trb_64 = 0;
-		trb_64 = (uintptr_t)seg->trbs;
+		trb_64 = seg->dma;
 		entry->seg_addr = cpu_to_le64(trb_64);
 		entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
 		entry->rsvd = 0;
@@ -581,7 +579,8 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	xhci_flush_cache((uintptr_t)ctrl->erst.entries,
 			 ERST_NUM_SEGS * sizeof(struct xhci_erst_entry));
 
-	deq = (unsigned long)ctrl->event_ring->dequeue;
+	deq = xhci_trb_virt_to_dma(ctrl->event_ring->deq_seg,
+				   ctrl->event_ring->dequeue);
 
 	/* Update HC event ring dequeue pointer */
 	xhci_writeq(&ctrl->ir_set->erst_dequeue,
@@ -596,7 +595,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	/* this is the event ring segment table pointer */
 	val_64 = xhci_readq(&ctrl->ir_set->erst_base);
 	val_64 &= ERST_PTR_MASK;
-	val_64 |= ((uintptr_t)(ctrl->erst.entries) & ~ERST_PTR_MASK);
+	val_64 |= ctrl->erst.erst_dma_addr & ~ERST_PTR_MASK;
 
 	xhci_writeq(&ctrl->ir_set->erst_base, val_64);
 
@@ -851,7 +850,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 	/* EP 0 can handle "burst" sizes of 1, so Max Burst Size field is 0 */
 	ep0_ctx->ep_info2 |= cpu_to_le32(MAX_BURST(0) | ERROR_COUNT(3));
 
-	trb_64 = (uintptr_t)virt_dev->eps[0].ring->first_seg->trbs;
+	trb_64 = virt_dev->eps[0].ring->first_seg->dma;
 	ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
 
 	/*
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 818b662e70d2..c645285da7fc 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -25,6 +25,24 @@
 
 #include "xhci.h"
 
+/*
+ * Returns zero if the TRB isn't in this segment, otherwise it returns the DMA
+ * address of the TRB.
+ */
+dma_addr_t xhci_trb_virt_to_dma(struct xhci_segment *seg,
+				union xhci_trb *trb)
+{
+	unsigned long segment_offset;
+
+	BUG_ON(!seg || !trb || trb < seg->trbs);
+
+	/* offset in TRBs */
+	segment_offset = trb - seg->trbs;
+	BUG_ON(segment_offset >= TRBS_PER_SEGMENT);
+
+	return seg->dma + (segment_offset * sizeof(*trb));
+}
+
 /**
  * Is this TRB a link TRB or was the last TRB the last TRB in this event ring
  * segment?  I.e. would the updated event TRB pointer step off the end of the
@@ -181,12 +199,11 @@ static void inc_deq(struct xhci_ctrl *ctrl, struct xhci_ring *ring)
  * @param trb_fields	pointer to trb field array containing TRB contents
  * @return pointer to the enqueued trb
  */
-static struct xhci_generic_trb *queue_trb(struct xhci_ctrl *ctrl,
-					  struct xhci_ring *ring,
-					  bool more_trbs_coming,
-					  unsigned int *trb_fields)
+static dma_addr_t queue_trb(struct xhci_ctrl *ctrl, struct xhci_ring *ring,
+			    bool more_trbs_coming, unsigned int *trb_fields)
 {
 	struct xhci_generic_trb *trb;
+	dma_addr_t addr;
 	int i;
 
 	trb = &ring->enqueue->generic;
@@ -196,9 +213,11 @@ static struct xhci_generic_trb *queue_trb(struct xhci_ctrl *ctrl,
 
 	xhci_flush_cache((uintptr_t)trb, sizeof(struct xhci_generic_trb));
 
+	addr = xhci_trb_virt_to_dma(ring->enq_seg, (union xhci_trb *)trb);
+
 	inc_enq(ctrl, ring, more_trbs_coming);
 
-	return trb;
+	return addr;
 }
 
 /**
@@ -273,16 +292,15 @@ static int prepare_ring(struct xhci_ctrl *ctrl, struct xhci_ring *ep_ring,
  * @param cmd		Command type to enqueue
  * @return none
  */
-void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, u32 slot_id,
+void xhci_queue_command(struct xhci_ctrl *ctrl, dma_addr_t addr, u32 slot_id,
 			u32 ep_index, trb_type cmd)
 {
 	u32 fields[4];
-	u64 val_64 = (uintptr_t)ptr;
 
 	BUG_ON(prepare_ring(ctrl, ctrl->cmd_ring, EP_STATE_RUNNING));
 
-	fields[0] = lower_32_bits(val_64);
-	fields[1] = upper_32_bits(val_64);
+	fields[0] = lower_32_bits(addr);
+	fields[1] = upper_32_bits(addr);
 	fields[2] = 0;
 	fields[3] = TRB_TYPE(cmd) | SLOT_ID_FOR_TRB(slot_id) |
 		    ctrl->cmd_ring->cycle_state;
@@ -396,12 +414,15 @@ static void giveback_first_trb(struct usb_device *udev, int ep_index,
  */
 void xhci_acknowledge_event(struct xhci_ctrl *ctrl)
 {
+	dma_addr_t deq;
+
 	/* Advance our dequeue pointer to the next event */
 	inc_deq(ctrl, ctrl->event_ring);
 
 	/* Inform the hardware */
-	xhci_writeq(&ctrl->ir_set->erst_dequeue,
-		(uintptr_t)ctrl->event_ring->dequeue | ERST_EHB);
+	deq = xhci_trb_virt_to_dma(ctrl->event_ring->deq_seg,
+				   ctrl->event_ring->dequeue);
+	xhci_writeq(&ctrl->ir_set->erst_dequeue, deq | ERST_EHB);
 }
 
 /**
@@ -492,9 +513,10 @@ static void abort_td(struct usb_device *udev, int ep_index)
 	struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
 	struct xhci_ring *ring =  ctrl->devs[udev->slot_id]->eps[ep_index].ring;
 	union xhci_trb *event;
+	dma_addr_t addr;
 	u32 field;
 
-	xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
+	xhci_queue_command(ctrl, 0, udev->slot_id, ep_index, TRB_STOP_RING);
 
 	event = xhci_wait_for_event(ctrl, TRB_TRANSFER, XHCI_TIMEOUT_DEFAULT);
 	field = le32_to_cpu(event->trans_event.flags);
@@ -510,8 +532,9 @@ static void abort_td(struct usb_device *udev, int ep_index)
 		event->event_cmd.status)) != COMP_SUCCESS);
 	xhci_acknowledge_event(ctrl);
 
-	xhci_queue_command(ctrl, (void *)((uintptr_t)ring->enqueue |
-		ring->cycle_state), udev->slot_id, ep_index, TRB_SET_DEQ);
+	addr = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
+	addr |= ring->cycle_state;
+	xhci_queue_command(ctrl, addr, udev->slot_id, ep_index, TRB_SET_DEQ);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
 	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
 		!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 92101f8f67d9..2c923b9869ae 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -443,7 +443,7 @@ static int xhci_configure_endpoints(struct usb_device *udev, bool ctx_change)
 	in_ctx = virt_dev->in_ctx;
 
 	xhci_flush_cache((uintptr_t)in_ctx->bytes, in_ctx->size);
-	xhci_queue_command(ctrl, in_ctx->bytes, udev->slot_id, 0,
+	xhci_queue_command(ctrl, in_ctx->dma, udev->slot_id, 0,
 			   ctx_change ? TRB_EVAL_CONTEXT : TRB_CONFIG_EP);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
 	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
@@ -580,8 +580,8 @@ static int xhci_set_configuration(struct usb_device *udev)
 			cpu_to_le32(MAX_BURST(max_burst) |
 			ERROR_COUNT(err_count));
 
-		trb_64 = (uintptr_t)
-				virt_dev->eps[ep_index].ring->enqueue;
+		trb_64 = xhci_trb_virt_to_dma(virt_dev->eps[ep_index].ring->enq_seg,
+				virt_dev->eps[ep_index].ring->enqueue);
 		ep_ctx[ep_index]->deq = cpu_to_le64(trb_64 |
 				virt_dev->eps[ep_index].ring->cycle_state);
 
@@ -629,7 +629,8 @@ static int xhci_address_device(struct usb_device *udev, int root_portnr)
 	ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG | EP0_FLAG);
 	ctrl_ctx->drop_flags = 0;
 
-	xhci_queue_command(ctrl, (void *)ctrl_ctx, slot_id, 0, TRB_ADDR_DEV);
+	xhci_queue_command(ctrl, virt_dev->in_ctx->dma,
+			   slot_id, 0, TRB_ADDR_DEV);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
 	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags)) != slot_id);
 
@@ -704,7 +705,7 @@ static int _xhci_alloc_device(struct usb_device *udev)
 		return 0;
 	}
 
-	xhci_queue_command(ctrl, NULL, 0, 0, TRB_ENABLE_SLOT);
+	xhci_queue_command(ctrl, 0, 0, 0, TRB_ENABLE_SLOT);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
 	BUG_ON(GET_COMP_CODE(le32_to_cpu(event->event_cmd.status))
 		!= COMP_SUCCESS);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 5c72f62402c5..e676116f4266 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -16,7 +16,7 @@
 #ifndef HOST_XHCI_H_
 #define HOST_XHCI_H_
 
-#include <asm/types.h>
+#include <linux/types.h>
 #include <io.h>
 #include <io-64-nonatomic-lo-hi.h>
 #include <linux/list.h>
@@ -490,6 +490,7 @@ struct xhci_container_ctx {
 
 	int size;
 	u8 *bytes;
+	dma_addr_t dma;
 };
 
 /**
@@ -691,6 +692,8 @@ struct xhci_input_control_ctx {
 struct xhci_device_context_array {
 	/* 64-bit device addresses; we only write 32-bit addresses */
 	__le64			dev_context_ptrs[MAX_HC_SLOTS];
+	/* private xHCD pointers */
+	dma_addr_t	dma;
 };
 /* TODO: write function to set the 64-bit device DMA address */
 /*
@@ -1003,6 +1006,7 @@ struct xhci_segment {
 	union xhci_trb		*trbs;
 	/* private to HCD */
 	struct xhci_segment	*next;
+	dma_addr_t		dma;
 };
 
 struct xhci_ring {
@@ -1031,11 +1035,14 @@ struct xhci_erst_entry {
 struct xhci_erst {
 	struct xhci_erst_entry	*entries;
 	unsigned int		num_entries;
+	/* xhci->event_ring keeps track of segment dma addresses */
+	dma_addr_t		erst_dma_addr;
 	/* Num entries the ERST can contain */
 	unsigned int		erst_size;
 };
 
 struct xhci_scratchpad {
+	void *scratchpad;
 	u64 *sp_array;
 };
 
@@ -1225,7 +1232,7 @@ static inline struct xhci_ctrl *to_xhci(struct usb_host *host)
 	return container_of(host, struct xhci_ctrl, host);
 }
 
-unsigned long trb_addr(struct xhci_segment *seg, union xhci_trb *trb);
+dma_addr_t xhci_trb_virt_to_dma(struct xhci_segment *seg, union xhci_trb *trb);
 struct xhci_input_control_ctx
 		*xhci_get_input_control_ctx(struct xhci_container_ctx *ctx);
 struct xhci_slot_ctx *xhci_get_slot_ctx(struct xhci_ctrl *ctrl,
@@ -1242,7 +1249,7 @@ void xhci_slot_copy(struct xhci_ctrl *ctrl,
 		    struct xhci_container_ctx *out_ctx);
 void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 				     struct usb_device *udev, int hop_portnr);
-void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr,
+void xhci_queue_command(struct xhci_ctrl *ctrl, dma_addr_t addr,
 			u32 slot_id, u32 ep_index, trb_type cmd);
 void xhci_acknowledge_event(struct xhci_ctrl *ctrl);
 #define XHCI_TIMEOUT_DEFAULT 5000
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 09/15] usb: xhci: reset endpoint on USB stall
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (7 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 08/15] usb: xhci: support non-1:1 mapped xHCI Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 10/15] usb: xhci: Fix root hub descriptor Ahmad Fatoum
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This ports U-Boot commit d5daa02d8d9e7c403a3339db1966e8413e64e408:

| Author:     Stefan Agner <stefan@agner.ch>
| AuthorDate: Mon Sep 27 14:42:58 2021 +0200
|
| usb: xhci: reset endpoint on USB stall
|
| There are devices which cause a USB stall when trying to read strings.
| Specifically Arduino Mega R3 stalls when trying to read the product
| string.
|
| The stall currently remains unhandled, and subsequent retries submit new
| transfers on a stopped endpoint which ultimately cause a crash in
| abort_td():
| WARN halted endpoint, queueing URB anyway.
| XHCI control transfer timed out, aborting...
| Unexpected XHCI event TRB, skipping... (3affe040 00000000 13000000 02008401)
| BUG at drivers/usb/host/xhci-ring.c:505/abort_td()!
| BUG!
| resetting ...
|
| Linux seems to be able to recover from the stall by issuing a
| TRB_RESET_EP command.
|
| Introduce reset_ep() which issues a TRB_RESET_EP followed by setting the
| transfer ring dequeue pointer via TRB_SET_DEQ. This allows to properly
| recover from a USB stall error and continue communicating with the USB
| device.
|
| Signed-off-by: Stefan Agner <stefan@agner.ch>

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-ring.c | 40 ++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c645285da7fc..056613445330 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -500,6 +500,42 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
 	BUG();
 }
 
+/*
+ * Send reset endpoint command for given endpoint. This recovers from a
+ * halted endpoint (e.g. due to a stall error).
+ */
+static void reset_ep(struct usb_device *udev, int ep_index, unsigned int timeout_ms)
+{
+	struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
+	struct xhci_ring *ring =  ctrl->devs[udev->slot_id]->eps[ep_index].ring;
+	union xhci_trb *event;
+	u64 addr;
+	u32 field;
+
+	dev_info(&udev->dev, "Resetting EP %d...\n", ep_index);
+
+	xhci_queue_command(ctrl, 0, udev->slot_id, ep_index, TRB_RESET_EP);
+	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, timeout_ms);
+	if (!event)
+		return;
+
+	field = le32_to_cpu(event->trans_event.flags);
+	BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
+	xhci_acknowledge_event(ctrl);
+
+	addr = xhci_trb_virt_to_dma(ring->enq_seg,
+		(void *)((uintptr_t)ring->enqueue | ring->cycle_state));
+	xhci_queue_command(ctrl, addr, udev->slot_id, ep_index, TRB_SET_DEQ);
+	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, timeout_ms);
+	if (!event)
+		return;
+
+	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
+		!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
+		event->event_cmd.status)) != COMP_SUCCESS);
+	xhci_acknowledge_event(ctrl);
+}
+
 /*
  * Stops transfer processing for an endpoint and throws away all unprocessed
  * TRBs by setting the xHC's dequeue pointer to our enqueue pointer. The next
@@ -979,6 +1015,10 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 
 	record_transfer_result(udev, event, length);
 	xhci_acknowledge_event(ctrl);
+	if (udev->status == USB_ST_STALLED) {
+		reset_ep(udev, ep_index, timeout_ms);
+		return -EPIPE;
+	}
 
 	/* Invalidate buffer to make it available to usb-core */
 	if (length > 0)
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 10/15] usb: xhci: Fix root hub descriptor
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (8 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 09/15] usb: xhci: reset endpoint on USB stall Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 11/15] usb: xhci: Guard all calls to xhci_wait_for_event Ahmad Fatoum
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This ports U-Boot commit e330c8b83e8784d23614f80ca3f12b11ceb515d8:

| Author:     Mark Kettenis <kettenis@openbsd.org>
| AuthorDate: Sat Jan 21 20:28:00 2023 +0100
|
| usb: xhci: Fix root hub descriptor
|
| When a system has multiple XHCI controllers, some of the
| properties described in the descriptor of the root hub (such as
| the number of ports) might differ between controllers.  Fix this
| by switching from a single global hub descriptor to a hub
| descriptor per controller.
|
| Signed-off-by: Mark Kettenis <kettenis@openbsd.org>

and additionally marks the descriptor template const, so it's safe
against future inadvertent modification.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci.c | 19 +++++++++++--------
 drivers/usb/host/xhci.h |  1 +
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2c923b9869ae..c5c21a526a29 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -30,7 +30,7 @@
 
 #include "xhci.h"
 
-static struct descriptor {
+static const struct descriptor {
 	struct usb_hub_descriptor hub;
 	struct usb_device_descriptor device;
 	struct usb_config_descriptor config;
@@ -861,7 +861,7 @@ static int xhci_submit_root(struct usb_device *udev, unsigned long pipe,
 {
 	uint8_t tmpbuf[4];
 	u16 typeReq;
-	void *srcptr = NULL;
+	const void *srcptr = NULL;
 	int len, srclen;
 	uint32_t reg;
 	volatile uint32_t *status_reg;
@@ -929,7 +929,7 @@ static int xhci_submit_root(struct usb_device *udev, unsigned long pipe,
 		case USB_DT_HUB:
 		case USB_DT_SS_HUB:
 			dev_dbg(&udev->dev, "USB_DT_HUB config\n");
-			srcptr = &descriptor.hub;
+			srcptr = &ctrl->hub_desc;
 			srclen = 0x8;
 			break;
 		default:
@@ -1188,20 +1188,23 @@ static int xhci_lowlevel_init(struct xhci_ctrl *ctrl)
 	/* initializing xhci data structures */
 	if (xhci_mem_init(ctrl, hccr, hcor) < 0)
 		return -ENOMEM;
+	ctrl->hub_desc = descriptor.hub;
 
 	reg = xhci_readl(&hccr->cr_hcsparams1);
-	descriptor.hub.bNbrPorts = HCS_MAX_PORTS(reg);
+	ctrl->hub_desc.bNbrPorts = HCS_MAX_PORTS(reg);
+
+	dev_dbg(ctrl->dev, "Register 0x%x NbrPorts %d\n", reg, ctrl->hub_desc.bNbrPorts);
 
 	/* Port Indicators */
 	reg = xhci_readl(&hccr->cr_hccparams);
 	if (HCS_INDICATOR(reg))
-		put_unaligned(get_unaligned(&descriptor.hub.wHubCharacteristics)
-				| 0x80, &descriptor.hub.wHubCharacteristics);
+		put_unaligned(get_unaligned(&ctrl->hub_desc.wHubCharacteristics)
+				| 0x80, &ctrl->hub_desc.wHubCharacteristics);
 
 	/* Port Power Control */
 	if (HCC_PPC(reg))
-		put_unaligned(get_unaligned(&descriptor.hub.wHubCharacteristics)
-				| 0x01, &descriptor.hub.wHubCharacteristics);
+		put_unaligned(get_unaligned(&ctrl->hub_desc.wHubCharacteristics)
+				| 0x01, &ctrl->hub_desc.wHubCharacteristics);
 
 	if (xhci_start(ctrl)) {
 		xhci_reset(ctrl);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e676116f4266..4a08f3f9e0d2 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1223,6 +1223,7 @@ struct xhci_ctrl {
 	struct xhci_erst_entry entry[ERST_NUM_SEGS];
 	struct xhci_scratchpad *scratchpad;
 	struct xhci_virt_device *devs[MAX_HC_SLOTS];
+	struct usb_hub_descriptor hub_desc;
 	void *bounce_buffer;
 	int rootdev;
 };
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 11/15] usb: xhci: Guard all calls to xhci_wait_for_event
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (9 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 10/15] usb: xhci: Fix root hub descriptor Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 12/15] usb: xhci: Better error handling in abort_td() Ahmad Fatoum
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This ports U-Boot commit 8d1e03f984c7467d7c8883f15dea14b2f8b4c0e2:

| Author:     Hector Martin <marcan@marcan.st>
| AuthorDate: Sun Oct 29 15:37:38 2023 +0900
|
| usb: xhci: Guard all calls to xhci_wait_for_event
|
| xhci_wait_for_event returns NULL on timeout, so the caller always has to
| check for that. This addresses immediate explosions in this part
| of the code when timeouts happen, but not the root cause for the
| timeout.
|
| Signed-off-by: Hector Martin <marcan@marcan.st>

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-ring.c | 9 +++++++++
 drivers/usb/host/xhci.c      | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 056613445330..d3f2018422cc 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -555,6 +555,9 @@ static void abort_td(struct usb_device *udev, int ep_index)
 	xhci_queue_command(ctrl, 0, udev->slot_id, ep_index, TRB_STOP_RING);
 
 	event = xhci_wait_for_event(ctrl, TRB_TRANSFER, XHCI_TIMEOUT_DEFAULT);
+	if (!event)
+		return;
+
 	field = le32_to_cpu(event->trans_event.flags);
 	BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
 	BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
@@ -563,6 +566,9 @@ static void abort_td(struct usb_device *udev, int ep_index)
 	xhci_acknowledge_event(ctrl);
 
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
+	if (!event)
+		return;
+
 	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
 		!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
 		event->event_cmd.status)) != COMP_SUCCESS);
@@ -572,6 +578,9 @@ static void abort_td(struct usb_device *udev, int ep_index)
 	addr |= ring->cycle_state;
 	xhci_queue_command(ctrl, addr, udev->slot_id, ep_index, TRB_SET_DEQ);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
+	if (!event)
+		return;
+
 	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
 		!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
 		event->event_cmd.status)) != COMP_SUCCESS);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c5c21a526a29..e7b8344181ee 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -446,6 +446,9 @@ static int xhci_configure_endpoints(struct usb_device *udev, bool ctx_change)
 	xhci_queue_command(ctrl, in_ctx->dma, udev->slot_id, 0,
 			   ctx_change ? TRB_EVAL_CONTEXT : TRB_CONFIG_EP);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
+	if (!event)
+		return -ETIMEDOUT;
+
 	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
 		!= udev->slot_id);
 
@@ -632,6 +635,9 @@ static int xhci_address_device(struct usb_device *udev, int root_portnr)
 	xhci_queue_command(ctrl, virt_dev->in_ctx->dma,
 			   slot_id, 0, TRB_ADDR_DEV);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
+	if (!event)
+		return -ETIMEDOUT;
+
 	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags)) != slot_id);
 
 	switch (GET_COMP_CODE(le32_to_cpu(event->event_cmd.status))) {
@@ -707,6 +713,9 @@ static int _xhci_alloc_device(struct usb_device *udev)
 
 	xhci_queue_command(ctrl, 0, 0, 0, TRB_ENABLE_SLOT);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
+	if (!event)
+		return -ETIMEDOUT;
+
 	BUG_ON(GET_COMP_CODE(le32_to_cpu(event->event_cmd.status))
 		!= COMP_SUCCESS);
 
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 12/15] usb: xhci: Better error handling in abort_td()
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (10 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 11/15] usb: xhci: Guard all calls to xhci_wait_for_event Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 13/15] usb: xhci: Allow context state errors when halting an endpoint Ahmad Fatoum
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This ports U-Boot commit 2526cd993272966606cb64b1898343e6963fb1d9:

| Author:     Hector Martin <marcan@marcan.st>
| AuthorDate: Sun Oct 29 15:37:39 2023 +0900
|
| usb: xhci: Better error handling in abort_td()
|
| If the xHC has a problem with our STOP ENDPOINT command, it is likely to
| return a completion directly instead of first a transfer event for the
| in-progress transfer. Handle that more gracefully.
|
| We still BUG() on the error code, but at least we don't end up timing
| out on the event and ending up with unexpected event errors.
|
| Signed-off-by: Hector Martin <marcan@marcan.st>

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-ring.c | 36 +++++++++++++++++++++++-------------
 drivers/usb/host/xhci.h      |  2 ++
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d3f2018422cc..23357a882538 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -470,7 +470,8 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
 			continue;
 
 		type = TRB_FIELD_TO_TYPE(le32_to_cpu(event->event_cmd.flags));
-		if (type == expected)
+		if (type == expected ||
+		    (expected == TRB_NONE && type != TRB_PORT_STATUS))
 			return event;
 
 		if (type == TRB_PORT_STATUS)
@@ -549,29 +550,38 @@ static void abort_td(struct usb_device *udev, int ep_index)
 	struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
 	struct xhci_ring *ring =  ctrl->devs[udev->slot_id]->eps[ep_index].ring;
 	union xhci_trb *event;
+	trb_type type;
 	dma_addr_t addr;
 	u32 field;
 
 	xhci_queue_command(ctrl, 0, udev->slot_id, ep_index, TRB_STOP_RING);
 
-	event = xhci_wait_for_event(ctrl, TRB_TRANSFER, XHCI_TIMEOUT_DEFAULT);
+	event = xhci_wait_for_event(ctrl, TRB_NONE, XHCI_TIMEOUT_DEFAULT);
 	if (!event)
 		return;
 
-	field = le32_to_cpu(event->trans_event.flags);
-	BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
-	BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
-	BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
-		!= COMP_STOP)));
-	xhci_acknowledge_event(ctrl);
+	type = TRB_FIELD_TO_TYPE(le32_to_cpu(event->event_cmd.flags));
+	if (type == TRB_TRANSFER) {
+		field = le32_to_cpu(event->trans_event.flags);
+		BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
+		BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
+		BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
+			!= COMP_STOP)));
+		xhci_acknowledge_event(ctrl);
 
-	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
-	if (!event)
-		return;
+		event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
+		if (!event)
+			return;
+		type = TRB_FIELD_TO_TYPE(le32_to_cpu(event->event_cmd.flags));
 
-	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
+	} else {
+		dev_warn(ctrl->dev, "abort_td: Expected a TRB_TRANSFER TRB first\n");
+	}
+
+	BUG_ON(type != TRB_COMPLETION ||
+		TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
 		!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
-		event->event_cmd.status)) != COMP_SUCCESS);
+	        event->event_cmd.status)) != COMP_SUCCESS);
 	xhci_acknowledge_event(ctrl);
 
 	addr = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 4a08f3f9e0d2..37e8cee843cf 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -906,6 +906,8 @@ union xhci_trb {
 
 /* TRB type IDs */
 typedef enum {
+	/* reserved, used as a software sentinel */
+	TRB_NONE = 0,
 	/* bulk, interrupt, isoc scatter/gather, and control data stage */
 	TRB_NORMAL = 1,
 	/* setup stage for control transfers */
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 13/15] usb: xhci: Allow context state errors when halting an endpoint
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (11 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 12/15] usb: xhci: Better error handling in abort_td() Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 14/15] usb: xhci: Recover from halted bulk endpoints Ahmad Fatoum
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This ports U-Boot commit 6f64f0ae230f9e8f68c5d9bf56ffee438fa60a6a:

| Author:     Hector Martin <marcan@marcan.st>
| AuthorDate: Sun Oct 29 15:37:40 2023 +0900
|
| usb: xhci: Allow context state errors when halting an endpoint
|
| There is a race where an endpoint may halt by itself while we are trying
| to halt it, which results in a context state error. See xHCI 4.6.9 which
| mentions this case.
|
| This also avoids BUGging when we attempt to stop an endpoint which was
| already stopped to begin with, which is probably a bug elsewhere but
| not a good reason to crash.
|
| Signed-off-by: Hector Martin <marcan@marcan.st>
| Reviewed-by: Marek Vasut <marex@denx.de>

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-ring.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 23357a882538..1b9a5b7867e0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -550,6 +550,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 	struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
 	struct xhci_ring *ring =  ctrl->devs[udev->slot_id]->eps[ep_index].ring;
 	union xhci_trb *event;
+	xhci_comp_code comp;
 	trb_type type;
 	dma_addr_t addr;
 	u32 field;
@@ -578,10 +579,11 @@ static void abort_td(struct usb_device *udev, int ep_index)
 		dev_warn(ctrl->dev, "abort_td: Expected a TRB_TRANSFER TRB first\n");
 	}
 
+	comp = GET_COMP_CODE(le32_to_cpu(event->event_cmd.status));
 	BUG_ON(type != TRB_COMPLETION ||
 		TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
-		!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
-	        event->event_cmd.status)) != COMP_SUCCESS);
+		!= udev->slot_id || (comp != COMP_SUCCESS && comp
+		!= COMP_CTX_STATE));
 	xhci_acknowledge_event(ctrl);
 
 	addr = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 14/15] usb: xhci: Recover from halted bulk endpoints
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (12 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 13/15] usb: xhci: Allow context state errors when halting an endpoint Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-19 13:38 ` [PATCH 15/15] usb: xhci: Do not panic on event timeouts Ahmad Fatoum
  2024-02-20 11:07 ` [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Sascha Hauer
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This ports U-Boot commit 9d88bd4dcf1628bf129163eb5a25c48068423601:

| Author:     Hector Martin <marcan@marcan.st>
| AuthorDate: Sun Oct 29 15:37:41 2023 +0900
|
| usb: xhci: Recover from halted bulk endpoints
|
| There is currently no codepath to recover from this case. In principle
| we could require that the upper layer do this explicitly, but let's just
| do it in xHCI when the next bulk transfer is started, since that
| reasonably implies whatever caused the problem has been dealt with.
|
| Signed-off-by: Hector Martin <marcan@marcan.st>

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-ring.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1b9a5b7867e0..6a859dbbfac9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -695,6 +695,14 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 
 	ep_ctx = xhci_get_ep_ctx(ctrl, virt_dev->out_ctx, ep_index);
 
+	/*
+	 * If the endpoint was halted due to a prior error, resume it before
+	 * the next transfer. It is the responsibility of the upper layer to
+	 * have dealt with whatever caused the error.
+	 */
+	if ((le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK) == EP_STATE_HALTED)
+		reset_ep(udev, ep_index, timeout_ms);
+
 	ring = virt_dev->eps[ep_index].ring;
 	/*
 	 * How much data is (potentially) left before the 64KB boundary?
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 15/15] usb: xhci: Do not panic on event timeouts
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (13 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 14/15] usb: xhci: Recover from halted bulk endpoints Ahmad Fatoum
@ 2024-02-19 13:38 ` Ahmad Fatoum
  2024-02-20 11:07 ` [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Sascha Hauer
  15 siblings, 0 replies; 17+ messages in thread
From: Ahmad Fatoum @ 2024-02-19 13:38 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This ports U-Boot commit 2fd7037122a920ae22377b06aa5b32651cc71f13:

| Author:     Hector Martin <marcan@marcan.st>
| AuthorDate: Sun Oct 29 15:37:43 2023 +0900
|
| usb: xhci: Do not panic on event timeouts
|
| Now that we always check the return value, just return NULL on timeouts.
| We can still log the error since this is a problem, but it's not reason
| to panic.
|
| Signed-off-by: Hector Martin <marcan@marcan.st>

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/host/xhci-ring.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6a859dbbfac9..5f68bc7c2f35 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -497,8 +497,9 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
 	if (expected == TRB_TRANSFER)
 		return NULL;
 
-	dev_err(ctrl->dev, "XHCI timeout on event type %d... cannot recover.\n", expected);
-	BUG();
+	dev_warn(ctrl->dev, "XHCI timeout on event type %d...\n", expected);
+
+	return NULL;
 }
 
 /*
-- 
2.39.2




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01
  2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
                   ` (14 preceding siblings ...)
  2024-02-19 13:38 ` [PATCH 15/15] usb: xhci: Do not panic on event timeouts Ahmad Fatoum
@ 2024-02-20 11:07 ` Sascha Hauer
  15 siblings, 0 replies; 17+ messages in thread
From: Sascha Hauer @ 2024-02-20 11:07 UTC (permalink / raw)
  To: barebox, Ahmad Fatoum


On Mon, 19 Feb 2024 14:38:20 +0100, Ahmad Fatoum wrote:
> The latest U-Boot release saw some fixes to the xHCI driver contributed
> by the Asahi Linux project to improve robustness in case of errors.
> 
> Port these to barebox as well as a couple of cleanup commits that make
> the code in barebox look more similar to changes that happened to U-Boot
> in the meantime.
> 
> [...]

Applied, thanks!

[01/15] usb: xhci: usb: xhci: avoid type conversion of void *
        https://git.pengutronix.de/cgit/barebox/commit/?id=13509f24774c (link may not be stable)
[02/15] usb: xhci: add various debugging prints
        https://git.pengutronix.de/cgit/barebox/commit/?id=86da740d096c (link may not be stable)
[03/15] usb: xhci: call xhci_flush_cache where appropriate
        https://git.pengutronix.de/cgit/barebox/commit/?id=c4a99e4bfb48 (link may not be stable)
[04/15] usb: xhci: use macros for formatting values
        https://git.pengutronix.de/cgit/barebox/commit/?id=ad69d4395d97 (link may not be stable)
[05/15] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)
        https://git.pengutronix.de/cgit/barebox/commit/?id=668cefb5fa19 (link may not be stable)
[06/15] usb: xhci: Add missing xhci_readl()
        https://git.pengutronix.de/cgit/barebox/commit/?id=97d839040fcc (link may not be stable)
[07/15] usb: xhci: don't use xhci_writeq for normal SDRAM
        https://git.pengutronix.de/cgit/barebox/commit/?id=fb4e864c0726 (link may not be stable)
[08/15] usb: xhci: support non-1:1 mapped xHCI
        https://git.pengutronix.de/cgit/barebox/commit/?id=6fba5a3b0bd7 (link may not be stable)
[09/15] usb: xhci: reset endpoint on USB stall
        https://git.pengutronix.de/cgit/barebox/commit/?id=be01454be27a (link may not be stable)
[10/15] usb: xhci: Fix root hub descriptor
        https://git.pengutronix.de/cgit/barebox/commit/?id=cb1ead123c45 (link may not be stable)
[11/15] usb: xhci: Guard all calls to xhci_wait_for_event
        https://git.pengutronix.de/cgit/barebox/commit/?id=2708f615e75b (link may not be stable)
[12/15] usb: xhci: Better error handling in abort_td()
        https://git.pengutronix.de/cgit/barebox/commit/?id=0a2bf109353e (link may not be stable)
[13/15] usb: xhci: Allow context state errors when halting an endpoint
        https://git.pengutronix.de/cgit/barebox/commit/?id=3b3af837338f (link may not be stable)
[14/15] usb: xhci: Recover from halted bulk endpoints
        https://git.pengutronix.de/cgit/barebox/commit/?id=42db38526b2c (link may not be stable)
[15/15] usb: xhci: Do not panic on event timeouts
        https://git.pengutronix.de/cgit/barebox/commit/?id=43a3850ed840 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-02-20 11:07 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-19 13:38 [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 01/15] usb: xhci: usb: xhci: avoid type conversion of void * Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 02/15] usb: xhci: add various debugging prints Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 03/15] usb: xhci: call xhci_flush_cache where appropriate Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 04/15] usb: xhci: use macros for formatting values Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 05/15] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu) Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 06/15] usb: xhci: Add missing xhci_readl() Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 07/15] usb: xhci: don't use xhci_writeq for normal SDRAM Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 08/15] usb: xhci: support non-1:1 mapped xHCI Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 09/15] usb: xhci: reset endpoint on USB stall Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 10/15] usb: xhci: Fix root hub descriptor Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 11/15] usb: xhci: Guard all calls to xhci_wait_for_event Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 12/15] usb: xhci: Better error handling in abort_td() Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 13/15] usb: xhci: Allow context state errors when halting an endpoint Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 14/15] usb: xhci: Recover from halted bulk endpoints Ahmad Fatoum
2024-02-19 13:38 ` [PATCH 15/15] usb: xhci: Do not panic on event timeouts Ahmad Fatoum
2024-02-20 11:07 ` [PATCH 00/15] usb: xhci: pull in fixes from U-Boot v2024.01 Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox