[PATCH 0/9] xhci features for usb-next

[PATCH 0/9] xhci features for usb-next

[Mathias Nyman]

Hi Greg

Some xhci features for usb-next.

Includes minor xhci port refactoring, improvements to dbc event
polling, and a couple transfer event handling changes that are
almost fixes.
I'd still only add those to usb-next as they are improvements
discovered while fixing other issues. No real world bugs.

Also includes a small kzalloc failure check tagged for stable.

Thanks
Mathias

Mathias Nyman (1):
  xhci: dbc: poll at different rate depending on data transfer activity

Michal Pecio (2):
  xhci: fix matching completion events with TDs
  xhci: retry Stop Endpoint on buggy NEC controllers

Niklas Neronin (5):
  xhci: rework how real & fake ports are found
  xhci: replace real & fake port with pointer to root hub port
  xhci: save slot ID in struct 'xhci_port'
  usb: xhci: remove duplicate code from 'xhci_clear_command_ring()'
  usb: xhci: utilize 'xhci_free_segments_for_ring()' for freeing
    segments

Prashanth K (1):
  usb: xhci: Add error handling in xhci_map_urb_for_dma

 drivers/usb/host/xhci-dbgcap.c  | 13 ++++-
 drivers/usb/host/xhci-dbgcap.h  |  2 +
 drivers/usb/host/xhci-hub.c     | 69 ++++--------------------
 drivers/usb/host/xhci-mem.c     | 93 +++++++++++++++------------------
 drivers/usb/host/xhci-mtk-sch.c | 14 ++---
 drivers/usb/host/xhci-pci.c     | 10 ++--
 drivers/usb/host/xhci-ring.c    | 27 ++++++----
 drivers/usb/host/xhci-trace.h   | 12 ++---
 drivers/usb/host/xhci.c         | 28 +++-------
 drivers/usb/host/xhci.h         |  7 ++-
 10 files changed, 107 insertions(+), 168 deletions(-)

-- 
2.25.1

[PATCH 1/9] xhci: rework how real & fake ports are found

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@intel.com>

xHC hardware needs to know which roothub port a USB device is attached to
when controlling the device, so the xHCI driver stores in each device the
roothub port which it's connected behind. This is done with two different
port index values, the 'real_port' which is an index to the xHC hardware
port register array, and the 'fake_port' which is the per hub port index
used by the hub driver.

Instead of finding real & fake port separately, find the root hub port
'xhci_port' structure which contains both real & fake port values:
    - 'real_port' is ('hw_portnum' + 1)
    - 'fake_port' is ('hcd_portnum' + 1)
i.e. real & fake port are 'hw_portnum' & 'hcd_portnum' in one-based
format.

The 'xhci_port' structure is a better way to refer to roothub ports than
the 'real_port' & 'fake_port'. As a result, these port indexes are slated
to be replaced with a direct pointer to the root hub port. This patch
setups the ground work for the future changes.

Signed-off-by: Niklas Neronin <niklas.neronin@intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 450adaca68ee..92160b96d4c0 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1051,16 +1051,16 @@ void xhci_copy_ep0_dequeue_into_input_ctx(struct xhci_hcd *xhci,
  * The xHCI roothub may have ports of differing speeds in any order in the port
  * status registers.
  *
- * The xHCI hardware wants to know the roothub port number that the USB device
+ * The xHCI hardware wants to know the roothub port that the USB device
  * is attached to (or the roothub port its ancestor hub is attached to).  All we
  * know is the index of that port under either the USB 2.0 or the USB 3.0
  * roothub, but that doesn't give us the real index into the HW port status
- * registers. Call xhci_find_raw_port_number() to get real index.
+ * registers.
  */
-static u32 xhci_find_real_port_number(struct xhci_hcd *xhci,
-		struct usb_device *udev)
+static struct xhci_port *xhci_find_rhub_port(struct xhci_hcd *xhci, struct usb_device *udev)
 {
 	struct usb_device *top_dev;
+	struct xhci_hub *rhub;
 	struct usb_hcd *hcd;
 
 	if (udev->speed >= USB_SPEED_SUPER)
@@ -1072,7 +1072,8 @@ static u32 xhci_find_real_port_number(struct xhci_hcd *xhci,
 			top_dev = top_dev->parent)
 		/* Found device below root hub */;
 
-	return	xhci_find_raw_port_number(hcd, top_dev->portnum);
+	rhub = xhci_get_rhub(hcd);
+	return rhub->ports[top_dev->portnum - 1];
 }
 
 /* Setup an xHCI virtual device for a Set Address command */
@@ -1081,9 +1082,8 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud
 	struct xhci_virt_device *dev;
 	struct xhci_ep_ctx	*ep0_ctx;
 	struct xhci_slot_ctx    *slot_ctx;
-	u32			port_num;
+	struct xhci_port	*rhub_port;
 	u32			max_packets;
-	struct usb_device *top_dev;
 
 	dev = xhci->devs[udev->slot_id];
 	/* Slot ID 0 is reserved */
@@ -1124,17 +1124,13 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud
 		return -EINVAL;
 	}
 	/* Find the root hub port this device is under */
-	port_num = xhci_find_real_port_number(xhci, udev);
-	if (!port_num)
+	rhub_port = xhci_find_rhub_port(xhci, udev);
+	if (!rhub_port)
 		return -EINVAL;
-	slot_ctx->dev_info2 |= cpu_to_le32(ROOT_HUB_PORT(port_num));
-	/* Set the port number in the virtual_device to the faked port number */
-	for (top_dev = udev; top_dev->parent && top_dev->parent->parent;
-			top_dev = top_dev->parent)
-		/* Found device below root hub */;
-	dev->fake_port = top_dev->portnum;
-	dev->real_port = port_num;
-	xhci_dbg(xhci, "Set root hub portnum to %d\n", port_num);
+	dev->real_port = rhub_port->hw_portnum + 1;
+	dev->fake_port = rhub_port->hcd_portnum + 1;
+	slot_ctx->dev_info2 |= cpu_to_le32(ROOT_HUB_PORT(dev->real_port));
+	xhci_dbg(xhci, "Set root hub portnum to %d\n", dev->real_port);
 	xhci_dbg(xhci, "Set fake root hub portnum to %d\n", dev->fake_port);
 
 	/* Find the right bandwidth table that this device will be a part of.
@@ -1144,12 +1140,12 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud
 	 * will never be created for the HS root hub.
 	 */
 	if (!udev->tt || !udev->tt->hub->parent) {
-		dev->bw_table = &xhci->rh_bw[port_num - 1].bw_table;
+		dev->bw_table = &xhci->rh_bw[dev->real_port - 1].bw_table;
 	} else {
 		struct xhci_root_port_bw_info *rh_bw;
 		struct xhci_tt_bw_info *tt_bw;
 
-		rh_bw = &xhci->rh_bw[port_num - 1];
+		rh_bw = &xhci->rh_bw[dev->real_port - 1];
 		/* Find the right TT. */
 		list_for_each_entry(tt_bw, &rh_bw->tts, tt_list) {
 			if (tt_bw->slot_id != udev->tt->hub->slot_id)
-- 
2.25.1

[PATCH 2/9] xhci: replace real & fake port with pointer to root hub port

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@intel.com>

Variables real & fake port do not convey their purpose, thus they are
replaced with a pointer to the root hub port 'struct xhci_port *rhub_port'.
'rhub_port' contains real & fake ports in zero-based format, which happens
to be more widely used inside the xHCI driver:
 - 'real_port' is ('rhub_port->hw_portnum' + 1)
 - 'fake_port' is ('rhub_port->hcd_portnum' + 1)

One reason for real port being one-based, is to signal other functions in
case struct 'xhci_virt_device' initialization failed, in this case the
value will remain 0. This is no longer needed, instead we check whether
or not 'rhub_port' is 'NULL'.

Signed-off-by: Niklas Neronin <niklas.neronin@intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c     |  4 +++-
 drivers/usb/host/xhci-mem.c     | 35 ++++++++++++++-------------------
 drivers/usb/host/xhci-mtk-sch.c | 14 +++++--------
 drivers/usb/host/xhci-trace.h   | 12 +++++------
 drivers/usb/host/xhci.c         | 12 +++++------
 drivers/usb/host/xhci.h         |  3 +--
 6 files changed, 35 insertions(+), 45 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0980ade2a234..37128a777a58 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -464,13 +464,15 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
 	int i;
 	enum usb_device_speed speed;
 
+	/* 'hcd_portnum' is zero-based, thus convert one-based 'port' to zero-based */
+	port -= 1;
 	slot_id = 0;
 	for (i = 0; i < MAX_HC_SLOTS; i++) {
 		if (!xhci->devs[i] || !xhci->devs[i]->udev)
 			continue;
 		speed = xhci->devs[i]->udev->speed;
 		if (((speed >= USB_SPEED_SUPER) == (hcd->speed >= HCD_USB3))
-				&& xhci->devs[i]->fake_port == port) {
+				&& xhci->devs[i]->rhub_port->hcd_portnum == port) {
 			slot_id = i;
 			break;
 		}
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 92160b96d4c0..9fa68fa17ac7 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -789,15 +789,14 @@ static void xhci_free_tt_info(struct xhci_hcd *xhci,
 	bool slot_found = false;
 
 	/* If the device never made it past the Set Address stage,
-	 * it may not have the real_port set correctly.
+	 * it may not have the root hub port pointer set correctly.
 	 */
-	if (virt_dev->real_port == 0 ||
-			virt_dev->real_port > HCS_MAX_PORTS(xhci->hcs_params1)) {
-		xhci_dbg(xhci, "Bad real port.\n");
+	if (!virt_dev->rhub_port) {
+		xhci_dbg(xhci, "Bad rhub port.\n");
 		return;
 	}
 
-	tt_list_head = &(xhci->rh_bw[virt_dev->real_port - 1].tts);
+	tt_list_head = &(xhci->rh_bw[virt_dev->rhub_port->hw_portnum].tts);
 	list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) {
 		/* Multi-TT hubs will have more than one entry */
 		if (tt_info->slot_id == slot_id) {
@@ -834,7 +833,7 @@ int xhci_alloc_tt_info(struct xhci_hcd *xhci,
 			goto free_tts;
 		INIT_LIST_HEAD(&tt_info->tt_list);
 		list_add(&tt_info->tt_list,
-				&xhci->rh_bw[virt_dev->real_port - 1].tts);
+				&xhci->rh_bw[virt_dev->rhub_port->hw_portnum].tts);
 		tt_info->slot_id = virt_dev->udev->slot_id;
 		if (tt->multi)
 			tt_info->ttport = i+1;
@@ -929,13 +928,12 @@ static void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_i
 	if (!vdev)
 		return;
 
-	if (vdev->real_port == 0 ||
-			vdev->real_port > HCS_MAX_PORTS(xhci->hcs_params1)) {
-		xhci_dbg(xhci, "Bad vdev->real_port.\n");
+	if (!vdev->rhub_port) {
+		xhci_dbg(xhci, "Bad rhub port.\n");
 		goto out;
 	}
 
-	tt_list_head = &(xhci->rh_bw[vdev->real_port - 1].tts);
+	tt_list_head = &(xhci->rh_bw[vdev->rhub_port->hw_portnum].tts);
 	list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) {
 		/* is this a hub device that added a tt_info to the tts list */
 		if (tt_info->slot_id == slot_id) {
@@ -1082,7 +1080,6 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud
 	struct xhci_virt_device *dev;
 	struct xhci_ep_ctx	*ep0_ctx;
 	struct xhci_slot_ctx    *slot_ctx;
-	struct xhci_port	*rhub_port;
 	u32			max_packets;
 
 	dev = xhci->devs[udev->slot_id];
@@ -1124,14 +1121,12 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud
 		return -EINVAL;
 	}
 	/* Find the root hub port this device is under */
-	rhub_port = xhci_find_rhub_port(xhci, udev);
-	if (!rhub_port)
+	dev->rhub_port = xhci_find_rhub_port(xhci, udev);
+	if (!dev->rhub_port)
 		return -EINVAL;
-	dev->real_port = rhub_port->hw_portnum + 1;
-	dev->fake_port = rhub_port->hcd_portnum + 1;
-	slot_ctx->dev_info2 |= cpu_to_le32(ROOT_HUB_PORT(dev->real_port));
-	xhci_dbg(xhci, "Set root hub portnum to %d\n", dev->real_port);
-	xhci_dbg(xhci, "Set fake root hub portnum to %d\n", dev->fake_port);
+	slot_ctx->dev_info2 |= cpu_to_le32(ROOT_HUB_PORT(dev->rhub_port->hw_portnum + 1));
+	xhci_dbg(xhci, "Slot ID %d: HW portnum %d, hcd portnum %d\n",
+		 udev->slot_id, dev->rhub_port->hw_portnum, dev->rhub_port->hcd_portnum);
 
 	/* Find the right bandwidth table that this device will be a part of.
 	 * If this is a full speed device attached directly to a root port (or a
@@ -1140,12 +1135,12 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud
 	 * will never be created for the HS root hub.
 	 */
 	if (!udev->tt || !udev->tt->hub->parent) {
-		dev->bw_table = &xhci->rh_bw[dev->real_port - 1].bw_table;
+		dev->bw_table = &xhci->rh_bw[dev->rhub_port->hw_portnum].bw_table;
 	} else {
 		struct xhci_root_port_bw_info *rh_bw;
 		struct xhci_tt_bw_info *tt_bw;
 
-		rh_bw = &xhci->rh_bw[dev->real_port - 1];
+		rh_bw = &xhci->rh_bw[dev->rhub_port->hw_portnum];
 		/* Find the right TT. */
 		list_for_each_entry(tt_bw, &rh_bw->tts, tt_list) {
 			if (tt_bw->slot_id != udev->tt->hub->slot_id)
diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index 61f3f8bbdcea..27eb384a3963 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -122,10 +122,6 @@ static u32 get_bw_boundary(enum usb_device_speed speed)
 * each HS root port is treated as a single bandwidth domain,
 * but each SS root port is treated as two bandwidth domains, one for IN eps,
 * one for OUT eps.
-* @real_port value is defined as follow according to xHCI spec:
-* 1 for SSport0, ..., N+1 for SSportN, N+2 for HSport0, N+3 for HSport1, etc
-* so the bandwidth domain array is organized as follow for simplification:
-* SSport0-OUT, SSport0-IN, ..., SSportX-OUT, SSportX-IN, HSport0, ..., HSportY
 */
 static struct mu3h_sch_bw_info *
 get_bw_info(struct xhci_hcd_mtk *mtk, struct usb_device *udev,
@@ -136,19 +132,19 @@ get_bw_info(struct xhci_hcd_mtk *mtk, struct usb_device *udev,
 	int bw_index;
 
 	virt_dev = xhci->devs[udev->slot_id];
-	if (!virt_dev->real_port) {
-		WARN_ONCE(1, "%s invalid real_port\n", dev_name(&udev->dev));
+	if (!virt_dev->rhub_port) {
+		WARN_ONCE(1, "%s invalid rhub port\n", dev_name(&udev->dev));
 		return NULL;
 	}
 
 	if (udev->speed >= USB_SPEED_SUPER) {
 		if (usb_endpoint_dir_out(&ep->desc))
-			bw_index = (virt_dev->real_port - 1) * 2;
+			bw_index = (virt_dev->rhub_port->hw_portnum) * 2;
 		else
-			bw_index = (virt_dev->real_port - 1) * 2 + 1;
+			bw_index = (virt_dev->rhub_port->hw_portnum) * 2 + 1;
 	} else {
 		/* add one more for each SS port */
-		bw_index = virt_dev->real_port + xhci->usb3_rhub.num_ports - 1;
+		bw_index = virt_dev->rhub_port->hw_portnum + xhci->usb3_rhub.num_ports;
 	}
 
 	return &mtk->sch_array[bw_index];
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index ac47b1c0544a..1740000d54c2 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -172,8 +172,8 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
 		__field(void *, vdev)
 		__field(unsigned long long, out_ctx)
 		__field(unsigned long long, in_ctx)
-		__field(u8, fake_port)
-		__field(u8, real_port)
+		__field(int, hcd_portnum)
+		__field(int, hw_portnum)
 		__field(u16, current_mel)
 
 	),
@@ -181,13 +181,13 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
 		__entry->vdev = vdev;
 		__entry->in_ctx = (unsigned long long) vdev->in_ctx->dma;
 		__entry->out_ctx = (unsigned long long) vdev->out_ctx->dma;
-		__entry->fake_port = (u8) vdev->fake_port;
-		__entry->real_port = (u8) vdev->real_port;
+		__entry->hcd_portnum = (int) vdev->rhub_port->hcd_portnum;
+		__entry->hw_portnum = (int) vdev->rhub_port->hw_portnum;
 		__entry->current_mel = (u16) vdev->current_mel;
 		),
-	TP_printk("vdev %p ctx %llx | %llx fake_port %d real_port %d current_mel %d",
+	TP_printk("vdev %p ctx %llx | %llx hcd_portnum %d hw_portnum %d current_mel %d",
 		__entry->vdev, __entry->in_ctx, __entry->out_ctx,
-		__entry->fake_port, __entry->real_port, __entry->current_mel
+		__entry->hcd_portnum, __entry->hw_portnum, __entry->current_mel
 	)
 );
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index b405b8236134..c50d5881e214 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2273,7 +2273,7 @@ static int xhci_check_tt_bw_table(struct xhci_hcd *xhci,
 	struct xhci_tt_bw_info *tt_info;
 
 	/* Find the bandwidth table for the root port this TT is attached to. */
-	bw_table = &xhci->rh_bw[virt_dev->real_port - 1].bw_table;
+	bw_table = &xhci->rh_bw[virt_dev->rhub_port->hw_portnum].bw_table;
 	tt_info = virt_dev->tt_info;
 	/* If this TT already had active endpoints, the bandwidth for this TT
 	 * has already been added.  Removing all periodic endpoints (and thus
@@ -2391,7 +2391,7 @@ static int xhci_check_bw_table(struct xhci_hcd *xhci,
 	if (virt_dev->tt_info) {
 		xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
 				"Recalculating BW for rootport %u",
-				virt_dev->real_port);
+				virt_dev->rhub_port->hw_portnum + 1);
 		if (xhci_check_tt_bw_table(xhci, virt_dev, old_active_eps)) {
 			xhci_warn(xhci, "Not enough bandwidth on HS bus for "
 					"newly activated TT.\n");
@@ -2404,7 +2404,7 @@ static int xhci_check_bw_table(struct xhci_hcd *xhci,
 	} else {
 		xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
 				"Recalculating BW for rootport %u",
-				virt_dev->real_port);
+				virt_dev->rhub_port->hw_portnum + 1);
 	}
 
 	/* Add in how much bandwidth will be used for interval zero, or the
@@ -2501,14 +2501,12 @@ static int xhci_check_bw_table(struct xhci_hcd *xhci,
 		bw_used += overhead + packet_size;
 
 	if (!virt_dev->tt_info && virt_dev->udev->speed == USB_SPEED_HIGH) {
-		unsigned int port_index = virt_dev->real_port - 1;
-
 		/* OK, we're manipulating a HS device attached to a
 		 * root port bandwidth domain.  Include the number of active TTs
 		 * in the bandwidth used.
 		 */
 		bw_used += TT_HS_OVERHEAD *
-			xhci->rh_bw[port_index].num_active_tts;
+			xhci->rh_bw[virt_dev->rhub_port->hw_portnum].num_active_tts;
 	}
 
 	xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
@@ -2695,7 +2693,7 @@ void xhci_update_tt_active_eps(struct xhci_hcd *xhci,
 	if (!virt_dev->tt_info)
 		return;
 
-	rh_bw_info = &xhci->rh_bw[virt_dev->real_port - 1];
+	rh_bw_info = &xhci->rh_bw[virt_dev->rhub_port->hw_portnum];
 	if (old_active_eps == 0 &&
 				virt_dev->tt_info->active_eps != 0) {
 		rh_bw_info->num_active_tts += 1;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 6e09b9130fae..634ab517a776 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -739,8 +739,7 @@ struct xhci_virt_device {
 	/* Used for addressing devices and configuration changes */
 	struct xhci_container_ctx       *in_ctx;
 	struct xhci_virt_ep		eps[EP_CTX_PER_DEV];
-	u8				fake_port;
-	u8				real_port;
+	struct xhci_port		*rhub_port;
 	struct xhci_interval_bw_table	*bw_table;
 	struct xhci_tt_bw_info		*tt_info;
 	/*
-- 
2.25.1

[PATCH 3/9] xhci: save slot ID in struct 'xhci_port'

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@intel.com>

Slot ID is a index of a virtual device in struct 'xhci_hcd->devs[]'.
Previously, to get the slot ID associated with a port, we had to loop
through all devices and compare ports, which is very inefficient.
Instead, the slot ID (of the device which is directly connected to the
port), is added to the its corresponding 'xhci_port' struct. As a result,
finding the port's device is quick and easy.

Function 'xhci_find_slot_id_by_port()' is removed, as it is no longer
needed.

Signed-off-by: Niklas Neronin <niklas.neronin@intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c  | 71 ++++++------------------------------
 drivers/usb/host/xhci-mem.c  |  5 +++
 drivers/usb/host/xhci-pci.c  | 10 ++---
 drivers/usb/host/xhci-ring.c | 11 ++----
 drivers/usb/host/xhci.h      |  4 +-
 5 files changed, 26 insertions(+), 75 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 37128a777a58..61f083de6e19 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -448,40 +448,6 @@ u32 xhci_port_state_to_neutral(u32 state)
 }
 EXPORT_SYMBOL_GPL(xhci_port_state_to_neutral);
 
-/**
- * xhci_find_slot_id_by_port() - Find slot id of a usb device on a roothub port
- * @hcd: pointer to hcd of the roothub
- * @xhci: pointer to xhci structure
- * @port: one-based port number of the port in this roothub.
- *
- * Return: Slot id of the usb device connected to the root port, 0 if not found
- */
-
-int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
-		u16 port)
-{
-	int slot_id;
-	int i;
-	enum usb_device_speed speed;
-
-	/* 'hcd_portnum' is zero-based, thus convert one-based 'port' to zero-based */
-	port -= 1;
-	slot_id = 0;
-	for (i = 0; i < MAX_HC_SLOTS; i++) {
-		if (!xhci->devs[i] || !xhci->devs[i]->udev)
-			continue;
-		speed = xhci->devs[i]->udev->speed;
-		if (((speed >= USB_SPEED_SUPER) == (hcd->speed >= HCD_USB3))
-				&& xhci->devs[i]->rhub_port->hcd_portnum == port) {
-			slot_id = i;
-			break;
-		}
-	}
-
-	return slot_id;
-}
-EXPORT_SYMBOL_GPL(xhci_find_slot_id_by_port);
-
 /*
  * Stop device
  * It issues stop endpoint command for EP 0 to 30. And wait the last command
@@ -932,7 +898,6 @@ static int xhci_handle_usb2_port_link_resume(struct xhci_port *port,
 	struct xhci_bus_state *bus_state;
 	struct xhci_hcd	*xhci;
 	struct usb_hcd *hcd;
-	int slot_id;
 	u32 wIndex;
 
 	hcd = port->rhub->hcd;
@@ -988,13 +953,11 @@ static int xhci_handle_usb2_port_link_resume(struct xhci_port *port,
 		spin_lock_irqsave(&xhci->lock, *flags);
 
 		if (time_left) {
-			slot_id = xhci_find_slot_id_by_port(hcd, xhci,
-							    wIndex + 1);
-			if (!slot_id) {
+			if (!port->slot_id) {
 				xhci_dbg(xhci, "slot_id is zero\n");
 				return -ENODEV;
 			}
-			xhci_ring_device(xhci, slot_id);
+			xhci_ring_device(xhci, port->slot_id);
 		} else {
 			int port_status = readl(port->addr);
 
@@ -1204,7 +1167,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 	unsigned long flags;
 	u32 temp, status;
 	int retval = 0;
-	int slot_id;
 	struct xhci_bus_state *bus_state;
 	u16 link_state = 0;
 	u16 wake_mask = 0;
@@ -1334,15 +1296,13 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 				goto error;
 			}
 
-			slot_id = xhci_find_slot_id_by_port(hcd, xhci,
-							    portnum1);
-			if (!slot_id) {
+			if (!port->slot_id) {
 				xhci_warn(xhci, "slot_id is zero\n");
 				goto error;
 			}
 			/* unlock to execute stop endpoint commands */
 			spin_unlock_irqrestore(&xhci->lock, flags);
-			xhci_stop_device(xhci, slot_id, 1);
+			xhci_stop_device(xhci, port->slot_id, 1);
 			spin_lock_irqsave(&xhci->lock, flags);
 
 			xhci_set_link_state(xhci, port, XDEV_U3);
@@ -1465,14 +1425,12 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 
 			if (link_state == USB_SS_PORT_LS_U3) {
 				int retries = 16;
-				slot_id = xhci_find_slot_id_by_port(hcd, xhci,
-								    portnum1);
-				if (slot_id) {
+				if (port->slot_id) {
 					/* unlock to execute stop endpoint
 					 * commands */
 					spin_unlock_irqrestore(&xhci->lock,
 								flags);
-					xhci_stop_device(xhci, slot_id, 1);
+					xhci_stop_device(xhci, port->slot_id, 1);
 					spin_lock_irqsave(&xhci->lock, flags);
 				}
 				xhci_set_link_state(xhci, port, USB_SS_PORT_LS_U3);
@@ -1586,13 +1544,11 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			}
 			bus_state->port_c_suspend |= 1 << wIndex;
 
-			slot_id = xhci_find_slot_id_by_port(hcd, xhci,
-					portnum1);
-			if (!slot_id) {
+			if (!port->slot_id) {
 				xhci_dbg(xhci, "slot_id is zero\n");
 				goto error;
 			}
-			xhci_ring_device(xhci, slot_id);
+			xhci_ring_device(xhci, port->slot_id);
 			break;
 		case USB_PORT_FEAT_C_SUSPEND:
 			bus_state->port_c_suspend &= ~(1 << wIndex);
@@ -1823,10 +1779,7 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
 		if (!portsc_buf[port_index])
 			continue;
 		if (test_bit(port_index, &bus_state->bus_suspended)) {
-			int slot_id;
-
-			slot_id = xhci_find_slot_id_by_port(hcd, xhci,
-							    port_index + 1);
+			int slot_id = ports[port_index]->slot_id;
 			if (slot_id) {
 				spin_unlock_irqrestore(&xhci->lock, flags);
 				xhci_stop_device(xhci, slot_id, 1);
@@ -1879,7 +1832,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
 	struct xhci_bus_state *bus_state;
 	unsigned long flags;
 	int max_ports, port_index;
-	int slot_id;
 	int sret;
 	u32 next_state;
 	u32 temp, portsc;
@@ -1972,9 +1924,8 @@ int xhci_bus_resume(struct usb_hcd *hcd)
 			continue;
 		}
 		xhci_test_and_clear_bit(xhci, ports[port_index], PORT_PLC);
-		slot_id = xhci_find_slot_id_by_port(hcd, xhci, port_index + 1);
-		if (slot_id)
-			xhci_ring_device(xhci, slot_id);
+		if (ports[port_index]->slot_id)
+			xhci_ring_device(xhci, ports[port_index]->slot_id);
 	}
 	(void) readl(&xhci->op_regs->command);
 
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 9fa68fa17ac7..c4b3e425bd19 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -907,6 +907,8 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id)
 
 	if (dev->udev && dev->udev->slot_id)
 		dev->udev->slot_id = 0;
+	if (dev->rhub_port && dev->rhub_port->slot_id == slot_id)
+		dev->rhub_port->slot_id = 0;
 	kfree(xhci->devs[slot_id]);
 	xhci->devs[slot_id] = NULL;
 }
@@ -1124,6 +1126,9 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud
 	dev->rhub_port = xhci_find_rhub_port(xhci, udev);
 	if (!dev->rhub_port)
 		return -EINVAL;
+	/* Slot ID is set to the device directly below the root hub */
+	if (!udev->parent->parent)
+		dev->rhub_port->slot_id = udev->slot_id;
 	slot_ctx->dev_info2 |= cpu_to_le32(ROOT_HUB_PORT(dev->rhub_port->hw_portnum + 1));
 	xhci_dbg(xhci, "Slot ID %d: HW portnum %d, hcd portnum %d\n",
 		 udev->slot_id, dev->rhub_port->hw_portnum, dev->rhub_port->hcd_portnum);
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index b534ca9752be..1843f3d5b1e6 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -820,7 +820,6 @@ static int xhci_pci_poweroff_late(struct usb_hcd *hcd, bool do_wakeup)
 	struct xhci_hcd		*xhci = hcd_to_xhci(hcd);
 	struct xhci_port	*port;
 	struct usb_device	*udev;
-	unsigned int		slot_id;
 	u32			portsc;
 	int			i;
 
@@ -843,15 +842,14 @@ static int xhci_pci_poweroff_late(struct usb_hcd *hcd, bool do_wakeup)
 		if ((portsc & PORT_PLS_MASK) != XDEV_U3)
 			continue;
 
-		slot_id = xhci_find_slot_id_by_port(port->rhub->hcd, xhci,
-						    port->hcd_portnum + 1);
-		if (!slot_id || !xhci->devs[slot_id]) {
+		if (!port->slot_id || !xhci->devs[port->slot_id]) {
 			xhci_err(xhci, "No dev for slot_id %d for port %d-%d in U3\n",
-				 slot_id, port->rhub->hcd->self.busnum, port->hcd_portnum + 1);
+				 port->slot_id, port->rhub->hcd->self.busnum,
+				 port->hcd_portnum + 1);
 			continue;
 		}
 
-		udev = xhci->devs[slot_id]->udev;
+		udev = xhci->devs[port->slot_id]->udev;
 
 		/* if wakeup is enabled then don't disable the port */
 		if (udev->do_remote_wakeup && do_wakeup)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d4a28ac4967f..95ed26114ee8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1870,7 +1870,6 @@ static void handle_port_status(struct xhci_hcd *xhci,
 	u32 port_id;
 	u32 portsc, cmd_reg;
 	int max_ports;
-	int slot_id;
 	unsigned int hcd_portnum;
 	struct xhci_bus_state *bus_state;
 	bool bogus_port_status = false;
@@ -1922,9 +1921,8 @@ static void handle_port_status(struct xhci_hcd *xhci,
 
 	if (hcd->speed >= HCD_USB3 &&
 	    (portsc & PORT_PLS_MASK) == XDEV_INACTIVE) {
-		slot_id = xhci_find_slot_id_by_port(hcd, xhci, hcd_portnum + 1);
-		if (slot_id && xhci->devs[slot_id])
-			xhci->devs[slot_id]->flags |= VDEV_PORT_ERROR;
+		if (port->slot_id && xhci->devs[port->slot_id])
+			xhci->devs[port->slot_id]->flags |= VDEV_PORT_ERROR;
 	}
 
 	if ((portsc & PORT_PLC) && (portsc & PORT_PLS_MASK) == XDEV_RESUME) {
@@ -1982,9 +1980,8 @@ static void handle_port_status(struct xhci_hcd *xhci,
 		 * so the roothub behavior is consistent with external
 		 * USB 3.0 hub behavior.
 		 */
-		slot_id = xhci_find_slot_id_by_port(hcd, xhci, hcd_portnum + 1);
-		if (slot_id && xhci->devs[slot_id])
-			xhci_ring_device(xhci, slot_id);
+		if (port->slot_id && xhci->devs[port->slot_id])
+			xhci_ring_device(xhci, port->slot_id);
 		if (bus_state->port_remote_wakeup & (1 << hcd_portnum)) {
 			xhci_test_and_clear_bit(xhci, port, PORT_PLC);
 			usb_wakeup_notification(hcd->self.root_hub,
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 634ab517a776..6f4bf98a6282 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1462,6 +1462,8 @@ struct xhci_port {
 	unsigned int		lpm_incapable:1;
 	unsigned long		resume_timestamp;
 	bool			rexit_active;
+	/* Slot ID is the index of the device directly connected to the port */
+	int			slot_id;
 	struct completion	rexit_done;
 	struct completion	u3exit_done;
 };
@@ -1944,8 +1946,6 @@ unsigned long xhci_get_resuming_ports(struct usb_hcd *hcd);
 #endif	/* CONFIG_PM */
 
 u32 xhci_port_state_to_neutral(u32 state);
-int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
-		u16 port);
 void xhci_ring_device(struct xhci_hcd *xhci, int slot_id);
 
 /* xHCI contexts */
-- 
2.25.1

[PATCH 4/9] usb: xhci: remove duplicate code from 'xhci_clear_command_ring()'

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

Replace a segment of code within 'xhci_clear_command_ring()' with a
function call to 'xhci_initialize_ring_info()'. This change eliminates
code duplication, as 'xhci_initialize_ring_info()' performs the same
operations as the replaced code.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c50d5881e214..5d70e0176527 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -794,19 +794,7 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)
 		seg = seg->next;
 	} while (seg != ring->deq_seg);
 
-	/* Reset the software enqueue and dequeue pointers */
-	ring->deq_seg = ring->first_seg;
-	ring->dequeue = ring->first_seg->trbs;
-	ring->enq_seg = ring->deq_seg;
-	ring->enqueue = ring->dequeue;
-
-	ring->num_trbs_free = ring->num_segs * (TRBS_PER_SEGMENT - 1) - 1;
-	/*
-	 * Ring is now zeroed, so the HW should look for change of ownership
-	 * when the cycle bit is set to 1.
-	 */
-	ring->cycle_state = 1;
-
+	xhci_initialize_ring_info(ring, 1);
 	/*
 	 * Reset the hardware dequeue pointer.
 	 * Yes, this will need to be re-written after resume, but we're paranoid
-- 
2.25.1

[PATCH 5/9] usb: xhci: utilize 'xhci_free_segments_for_ring()' for freeing segments

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

Refactor the code to improve readability by using
'xhci_free_segments_for_ring()' function for freeing ring segments.
This replaces the custom while loop previously used within
'xhci_ring_expansion()' and 'xhci_alloc_segments_for_ring()'.

Slightly modify 'xhci_free_segments_for_ring()' to handle lists
which do not loop. This makes it possible to use it in error
paths of 'xhci_alloc_segments_for_ring()'.

This change also prepares for switching the custom xhci linked segment
list into to more standard list.h lists.

[minor commit message rewording -Mathias]
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index c4b3e425bd19..69dd86669883 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -84,7 +84,7 @@ static void xhci_free_segments_for_ring(struct xhci_hcd *xhci,
 	struct xhci_segment *seg;
 
 	seg = first->next;
-	while (seg != first) {
+	while (seg && seg != first) {
 		struct xhci_segment *next = seg->next;
 		xhci_segment_free(xhci, seg);
 		seg = next;
@@ -351,17 +351,10 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
 
 		next = xhci_segment_alloc(xhci, cycle_state, max_packet, num,
 					  flags);
-		if (!next) {
-			prev = *first;
-			while (prev) {
-				next = prev->next;
-				xhci_segment_free(xhci, prev);
-				prev = next;
-			}
-			return -ENOMEM;
-		}
-		xhci_link_segments(prev, next, type, chain_links);
+		if (!next)
+			goto free_segments;
 
+		xhci_link_segments(prev, next, type, chain_links);
 		prev = next;
 		num++;
 	}
@@ -369,6 +362,10 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
 	*last = prev;
 
 	return 0;
+
+free_segments:
+	xhci_free_segments_for_ring(xhci, *first);
+	return -ENOMEM;
 }
 
 /*
@@ -444,19 +441,11 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
 	if (ret)
 		return -ENOMEM;
 
-	if (ring->type == TYPE_STREAM)
+	if (ring->type == TYPE_STREAM) {
 		ret = xhci_update_stream_segment_mapping(ring->trb_address_map,
 						ring, first, last, flags);
-	if (ret) {
-		struct xhci_segment *next;
-		do {
-			next = first->next;
-			xhci_segment_free(xhci, first);
-			if (first == last)
-				break;
-			first = next;
-		} while (true);
-		return ret;
+		if (ret)
+			goto free_segments;
 	}
 
 	xhci_link_rings(xhci, ring, first, last, num_new_segs);
@@ -466,6 +455,10 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
 			ring->num_segs);
 
 	return 0;
+
+free_segments:
+	xhci_free_segments_for_ring(xhci, first);
+	return ret;
 }
 
 struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
-- 
2.25.1

[PATCH 6/9] xhci: fix matching completion events with TDs

[Mathias Nyman]

From: Michal Pecio <michal.pecio@gmail.com>

A trb_in_td() call is used to determine if a completion event matches
any TRB of the currently executing TD. This function is told to start
searching right after the last finished TD, which is not at all where
the currently expected TD is guaranteed to begin, because some TDs in
between may have been cancelled.

Not only is a pointless work performed, but a bug resulting in the HC
executing cancelled TDs was seen to trick the driver into associating
events from a TD just cancelled with an unrelated future TD.

Since the ring is being traversed for the specific purpose of finding
a match with the current TD, always start from its first TRB. This is
the most reliable bit of information that we posses.

Tracking of HC's work progress is not affected, except for cases when
a misattributed event would have moved dequeue past a pending TD.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 95ed26114ee8..b2116501048c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2813,7 +2813,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 			td_num--;
 
 		/* Is this a TRB in the currently executing TD? */
-		ep_seg = trb_in_td(xhci, ep_ring->deq_seg, ep_ring->dequeue,
+		ep_seg = trb_in_td(xhci, td->start_seg, td->first_trb,
 				td->last_trb, ep_trb_dma, false);
 
 		/*
@@ -2881,9 +2881,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 					"part of current TD ep_index %d "
 					"comp_code %u\n", ep_index,
 					trb_comp_code);
-				trb_in_td(xhci, ep_ring->deq_seg,
-					  ep_ring->dequeue, td->last_trb,
-					  ep_trb_dma, true);
+				trb_in_td(xhci, td->start_seg, td->first_trb,
+					  td->last_trb, ep_trb_dma, true);
 				return -ESHUTDOWN;
 			}
 		}
-- 
2.25.1

[PATCH 7/9] xhci: retry Stop Endpoint on buggy NEC controllers

[Mathias Nyman]

From: Michal Pecio <michal.pecio@gmail.com>

Two NEC uPD720200 adapters have been observed to randomly misbehave:
a Stop Endpoint command fails with Context Error, the Output Context
indicates Stopped state, and the endpoint keeps running. Very often,
Set TR Dequeue Pointer is seen to fail next with Context Error too,
in addition to problems from unexpectedly completed cancelled work.

The pathology is common on fast running isoc endpoints like uvcvideo,
but has also been reproduced on a full-speed bulk endpoint of pl2303.
It seems all EPs are affected, with risk proportional to their load.

Reproduction involves receiving any kind of stream and closing it to
make the device driver cancel URBs already queued in advance.

Deal with it by retrying the command like in the Running state.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b2116501048c..6a29ebd6682d 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1154,6 +1154,15 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
 				break;
 			ep->ep_state &= ~EP_STOP_CMD_PENDING;
 			return;
+		case EP_STATE_STOPPED:
+			/*
+			 * NEC uPD720200 sometimes sets this state and fails with
+			 * Context Error while continuing to process TRBs.
+			 * Be conservative and trust EP_CTX_STATE on other chips.
+			 */
+			if (!(xhci->quirks & XHCI_NEC_HOST))
+				break;
+			fallthrough;
 		case EP_STATE_RUNNING:
 			/* Race, HW handled stop ep cmd before ep was running */
 			xhci_dbg(xhci, "Stop ep completion ctx error, ep is running\n");
-- 
2.25.1

[PATCH 8/9] xhci: dbc: poll at different rate depending on data transfer activity

[Mathias Nyman]

DbC driver starts polling for events immediately when DbC is enabled.
The current polling interval is 1ms, which keeps the CPU busy, impacting
power management even when there are no active data transfers.

Solve this by polling at a slower rate, with a 64ms interval as default
until a transfer request is queued, or if there are still are pending
unhandled transfers at event completion.

Tested-by: Uday M Bhat <uday.m.bhat@intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-dbgcap.c | 13 +++++++++++--
 drivers/usb/host/xhci-dbgcap.h |  2 ++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index d82935d31126..8a9869ef0db6 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -634,7 +634,8 @@ static int xhci_dbc_start(struct xhci_dbc *dbc)
 		return ret;
 	}
 
-	return mod_delayed_work(system_wq, &dbc->event_work, 1);
+	return mod_delayed_work(system_wq, &dbc->event_work,
+				msecs_to_jiffies(dbc->poll_interval));
 }
 
 static void xhci_dbc_stop(struct xhci_dbc *dbc)
@@ -899,8 +900,10 @@ static void xhci_dbc_handle_events(struct work_struct *work)
 	enum evtreturn		evtr;
 	struct xhci_dbc		*dbc;
 	unsigned long		flags;
+	unsigned int		poll_interval;
 
 	dbc = container_of(to_delayed_work(work), struct xhci_dbc, event_work);
+	poll_interval = dbc->poll_interval;
 
 	spin_lock_irqsave(&dbc->lock, flags);
 	evtr = xhci_dbc_do_handle_events(dbc);
@@ -916,13 +919,18 @@ static void xhci_dbc_handle_events(struct work_struct *work)
 			dbc->driver->disconnect(dbc);
 		break;
 	case EVT_DONE:
+		/* set fast poll rate if there are pending data transfers */
+		if (!list_empty(&dbc->eps[BULK_OUT].list_pending) ||
+		    !list_empty(&dbc->eps[BULK_IN].list_pending))
+			poll_interval = 1;
 		break;
 	default:
 		dev_info(dbc->dev, "stop handling dbc events\n");
 		return;
 	}
 
-	mod_delayed_work(system_wq, &dbc->event_work, 1);
+	mod_delayed_work(system_wq, &dbc->event_work,
+			 msecs_to_jiffies(poll_interval));
 }
 
 static const char * const dbc_state_strings[DS_MAX] = {
@@ -1175,6 +1183,7 @@ xhci_alloc_dbc(struct device *dev, void __iomem *base, const struct dbc_driver *
 	dbc->idVendor = DBC_VENDOR_ID;
 	dbc->bcdDevice = DBC_DEVICE_REV;
 	dbc->bInterfaceProtocol = DBC_PROTOCOL;
+	dbc->poll_interval = DBC_POLL_INTERVAL_DEFAULT;
 
 	if (readl(&dbc->regs->control) & DBC_CTRL_DBC_ENABLE)
 		goto err;
diff --git a/drivers/usb/host/xhci-dbgcap.h b/drivers/usb/host/xhci-dbgcap.h
index e39e3ae1677a..92661b555c2a 100644
--- a/drivers/usb/host/xhci-dbgcap.h
+++ b/drivers/usb/host/xhci-dbgcap.h
@@ -94,6 +94,7 @@ struct dbc_ep {
 
 #define DBC_QUEUE_SIZE			16
 #define DBC_WRITE_BUF_SIZE		8192
+#define DBC_POLL_INTERVAL_DEFAULT	64	/* milliseconds */
 
 /*
  * Private structure for DbC hardware state:
@@ -140,6 +141,7 @@ struct xhci_dbc {
 
 	enum dbc_state			state;
 	struct delayed_work		event_work;
+	unsigned int			poll_interval;	/* ms */
 	unsigned			resume_required:1;
 	struct dbc_ep			eps[2];
 
-- 
2.25.1

[PATCH 9/9] usb: xhci: Add error handling in xhci_map_urb_for_dma

[Mathias Nyman]

From: Prashanth K <quic_prashk@quicinc.com>

Currently xhci_map_urb_for_dma() creates a temporary buffer and copies
the SG list to the new linear buffer. But if the kzalloc_node() fails,
then the following sg_pcopy_to_buffer() can lead to crash since it
tries to memcpy to NULL pointer.

So return -ENOMEM if kzalloc returns null pointer.

Cc: <stable@vger.kernel.org> # 5.11
Fixes: 2017a1e58472 ("usb: xhci: Use temporary buffer to consolidate SG")
Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5d70e0176527..8579603edaff 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1219,6 +1219,8 @@ static int xhci_map_temp_buffer(struct usb_hcd *hcd, struct urb *urb)
 
 	temp = kzalloc_node(buf_len, GFP_ATOMIC,
 			    dev_to_node(hcd->self.sysdev));
+	if (!temp)
+		return -ENOMEM;
 
 	if (usb_urb_dir_out(urb))
 		sg_pcopy_to_buffer(urb->sg, urb->num_sgs,
-- 
2.25.1

Re: [PATCH 2/9] xhci: replace real & fake port with pointer to root hub port

[Thinh Nguyen]

[-- Attachment #1: Type: text/plain, Size: 6109 bytes --]

Hi Mathias,

On Thu, Feb 29, 2024, Mathias Nyman wrote:
> From: Niklas Neronin <niklas.neronin@intel.com>
> 
> Variables real & fake port do not convey their purpose, thus they are
> replaced with a pointer to the root hub port 'struct xhci_port *rhub_port'.
> 'rhub_port' contains real & fake ports in zero-based format, which happens
> to be more widely used inside the xHCI driver:
>  - 'real_port' is ('rhub_port->hw_portnum' + 1)
>  - 'fake_port' is ('rhub_port->hcd_portnum' + 1)
> 
> One reason for real port being one-based, is to signal other functions in
> case struct 'xhci_virt_device' initialization failed, in this case the
> value will remain 0. This is no longer needed, instead we check whether
> or not 'rhub_port' is 'NULL'.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@intel.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/usb/host/xhci-hub.c     |  4 +++-
>  drivers/usb/host/xhci-mem.c     | 35 ++++++++++++++-------------------
>  drivers/usb/host/xhci-mtk-sch.c | 14 +++++--------
>  drivers/usb/host/xhci-trace.h   | 12 +++++------
>  drivers/usb/host/xhci.c         | 12 +++++------
>  drivers/usb/host/xhci.h         |  3 +--
>  6 files changed, 35 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 0980ade2a234..37128a777a58 100644

We're getting a NULL pointer dereference bug for this patch. To
reproduce this, just unload and reload the xhci driver while a device is
connected. It may take a few times to hit the issue.

[ 1515.737623] BUG: kernel NULL pointer dereference, address: 000000000000000c
[ 1515.737629] #PF: supervisor read access in kernel mode
[ 1515.737631] #PF: error_code(0x0000) - not-present page
[ 1515.737633] PGD 0 P4D 0 
[ 1515.737636] Oops: 0000 [#1] PREEMPT SMP NOPTI
[ 1515.737639] CPU: 1 PID: 2905 Comm: kworker/1:0 Not tainted 6.9.0-rc2-snps-detached-HEAD-g5bab5dc780c9 #8
[ 1515.737642] Hardware name: System manufacturer System Product Name/PRIME Q370M-C, BIOS 2605 10/16/2019
[ 1515.737643] Workqueue: usb_hub_wq hub_event
[ 1515.737648] RIP: 0010:trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd]
[ 1515.737667] Code: 85 c0 74 59 48 89 58 08 48 8b 53 18 48 89 e7 48 8b 52 10 48 89 50 18 48 8b 53 10 48 8b 52 10 48 89 50 10 48 8b 93 90 11 00 00 <8b> 52 0c 89 50 20 48 8b 93 90 11 00 00 8b 52 08 89 50 24 8b 93 b0
[ 1515.737669] RSP: 0018:ffffac30c0bdbce8 EFLAGS: 00010086
[ 1515.737671] RAX: ffff99e0003309b8 RBX: ffff99e040fb4000 RCX: ffff99e0003309b4
[ 1515.737673] RDX: 0000000000000000 RSI: 0000000000000616 RDI: ffffac30c0bdbce8
[ 1515.737729] RBP: ffff99e007b58a20 R08: 0000000000000002 R09: 0000000000000010
[ 1515.737731] R10: 000000014d33cf30 R11: 0000000000000020 R12: 0000000000000001
[ 1515.737733] R13: ffff99e008c9a294 R14: ffff99e008c9a258 R15: ffff99e004ca5000
[ 1515.737734] FS:  0000000000000000(0000) GS:ffff99e35bc40000(0000) knlGS:0000000000000000
[ 1515.737736] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1515.737738] CR2: 000000000000000c CR3: 00000001024d6004 CR4: 00000000003706f0
[ 1515.737740] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1515.737741] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1515.737743] Call Trace:
[ 1515.737746]  <TASK>
[ 1515.737748]  ? __die_body+0x1a/0x5c
[ 1515.737751]  ? page_fault_oops+0x2ea/0x380
[ 1515.737806]  ? sched_clock+0x10/0x23
[ 1515.737808]  ? trace_clock_local+0x10/0x23
[ 1515.737812]  ? exc_page_fault+0xfe/0x11e
[ 1515.737815]  ? asm_exc_page_fault+0x26/0x30
[ 1515.737818]  ? trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd]
[ 1515.737832]  ? trace_event_raw_event_xhci_log_free_virt_dev+0x39/0xb7 [xhci_hcd]
[ 1515.737844]  xhci_free_virt_device+0x76/0x1d8 [xhci_hcd]
[ 1515.737856]  xhci_free_dev+0x111/0x12f [xhci_hcd]
[ 1515.737867]  hub_event+0xca6/0x137e
[ 1515.737870]  ? __schedule+0x656/0x69f
[ 1515.737873]  process_scheduled_works+0x1a4/0x2e3
[ 1515.737876]  worker_thread+0x191/0x1e9
[ 1515.737878]  ? __pfx_worker_thread+0x10/0x10
[ 1515.737881]  kthread+0xe6/0xf1
[ 1515.737883]  ? __pfx_kthread+0x10/0x10
[ 1515.737885]  ret_from_fork+0x20/0x37
[ 1515.737887]  ? __pfx_kthread+0x10/0x10
[ 1515.737889]  ret_from_fork_asm+0x1a/0x30
[ 1515.737892]  </TASK>



To capture the tracepoints and avoid the NULL pointer, I made this
change:

diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 1740000d54c2..b4b20e3f7539 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -172,8 +172,7 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
 		__field(void *, vdev)
 		__field(unsigned long long, out_ctx)
 		__field(unsigned long long, in_ctx)
-		__field(int, hcd_portnum)
-		__field(int, hw_portnum)
+		__field(struct xhci_port *, rhub_port)
 		__field(u16, current_mel)
 
 	),
@@ -181,13 +180,14 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
 		__entry->vdev = vdev;
 		__entry->in_ctx = (unsigned long long) vdev->in_ctx->dma;
 		__entry->out_ctx = (unsigned long long) vdev->out_ctx->dma;
-		__entry->hcd_portnum = (int) vdev->rhub_port->hcd_portnum;
-		__entry->hw_portnum = (int) vdev->rhub_port->hw_portnum;
+		__entry->rhub_port = (struct xhci_port *) vdev->rhub_port;
 		__entry->current_mel = (u16) vdev->current_mel;
 		),
 	TP_printk("vdev %p ctx %llx | %llx hcd_portnum %d hw_portnum %d current_mel %d",
 		__entry->vdev, __entry->in_ctx, __entry->out_ctx,
-		__entry->hcd_portnum, __entry->hw_portnum, __entry->current_mel
+		__entry->rhub_port ? __entry->rhub_port->hcd_portnum : -1,
+		__entry->rhub_port ? __entry->rhub_port->hw_portnum : -1,
+		__entry->current_mel
 	)
 );


 You should see this line where the NULL deref happened in the attached
 log:

     kworker/0:1-8       [000] d..1.   250.442611: xhci_free_virt_device: vdev 00000000a84fbabe ctx 159dd6000 | 1a7d60000 hcd_portnum -1 hw_portnum -1 current_mel 0


Please review and help fix it.

Thanks,
Thinh

[-- Attachment #2: xhci_null_ptr.tar.gz --]
[-- Type: application/x-tar-gz, Size: 168244 bytes --]

Re: [PATCH 2/9] xhci: replace real & fake port with pointer to root hub port

[Mathias Nyman]

Hi Thinh

On 2.4.2024 3.50, Thinh Nguyen wrote:
> Hi Mathias,
> 
> We're getting a NULL pointer dereference bug for this patch. To
> reproduce this, just unload and reload the xhci driver while a device is
> connected. It may take a few times to hit the issue.
> 
> [ 1515.737623] BUG: kernel NULL pointer dereference, address: 000000000000000c
> [ 1515.737629] #PF: supervisor read access in kernel mode
> [ 1515.737631] #PF: error_code(0x0000) - not-present page
> [ 1515.737633] PGD 0 P4D 0
> [ 1515.737636] Oops: 0000 [#1] PREEMPT SMP NOPTI
> [ 1515.737639] CPU: 1 PID: 2905 Comm: kworker/1:0 Not tainted 6.9.0-rc2-snps-detached-HEAD-g5bab5dc780c9 #8
> [ 1515.737642] Hardware name: System manufacturer System Product Name/PRIME Q370M-C, BIOS 2605 10/16/2019
> [ 1515.737643] Workqueue: usb_hub_wq hub_event
> [ 1515.737648] RIP: 0010:trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd]
> [ 1515.737667] Code: 85 c0 74 59 48 89 58 08 48 8b 53 18 48 89 e7 48 8b 52 10 48 89 50 18 48 8b 53 10 48 8b 52 10 48 89 50 10 48 8b 93 90 11 00 00 <8b> 52 0c 89 50 20 48 8b 93 90 11 00 00 8b 52 08 89 50 24 8b 93 b0
> [ 1515.737669] RSP: 0018:ffffac30c0bdbce8 EFLAGS: 00010086
> [ 1515.737671] RAX: ffff99e0003309b8 RBX: ffff99e040fb4000 RCX: ffff99e0003309b4
> [ 1515.737673] RDX: 0000000000000000 RSI: 0000000000000616 RDI: ffffac30c0bdbce8
> [ 1515.737729] RBP: ffff99e007b58a20 R08: 0000000000000002 R09: 0000000000000010
> [ 1515.737731] R10: 000000014d33cf30 R11: 0000000000000020 R12: 0000000000000001
> [ 1515.737733] R13: ffff99e008c9a294 R14: ffff99e008c9a258 R15: ffff99e004ca5000
> [ 1515.737734] FS:  0000000000000000(0000) GS:ffff99e35bc40000(0000) knlGS:0000000000000000
> [ 1515.737736] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1515.737738] CR2: 000000000000000c CR3: 00000001024d6004 CR4: 00000000003706f0
> [ 1515.737740] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 1515.737741] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 1515.737743] Call Trace:
> [ 1515.737746]  <TASK>
> [ 1515.737748]  ? __die_body+0x1a/0x5c
> [ 1515.737751]  ? page_fault_oops+0x2ea/0x380
> [ 1515.737806]  ? sched_clock+0x10/0x23
> [ 1515.737808]  ? trace_clock_local+0x10/0x23
> [ 1515.737812]  ? exc_page_fault+0xfe/0x11e
> [ 1515.737815]  ? asm_exc_page_fault+0x26/0x30
> [ 1515.737818]  ? trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd]
> [ 1515.737832]  ? trace_event_raw_event_xhci_log_free_virt_dev+0x39/0xb7 [xhci_hcd]
> [ 1515.737844]  xhci_free_virt_device+0x76/0x1d8 [xhci_hcd]
> [ 1515.737856]  xhci_free_dev+0x111/0x12f [xhci_hcd]
> [ 1515.737867]  hub_event+0xca6/0x137e

Thanks for the report, and for debugging this.
I was able to reproduce it myself using your steps.

This triggers if xhci tracing is enabled and usb device is freed before it's addressed
as vdev->rhub_port is only set during addressing.

> 
> 
> To capture the tracepoints and avoid the NULL pointer, I made this
> change:
> 

I think we could skip printing the hcd_portnum and hw_portnum during
xhci_free_virt_dev() tracing. I haven't really found them useful.

It would make sense to print the slot_id instead.

how about this:

diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 1740000d54c2..5762564b9d73 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -172,8 +172,7 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
                 __field(void *, vdev)
                 __field(unsigned long long, out_ctx)
                 __field(unsigned long long, in_ctx)
-               __field(int, hcd_portnum)
-               __field(int, hw_portnum)
+               __field(int, slot_id)
                 __field(u16, current_mel)
  
         ),
@@ -181,13 +180,12 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
                 __entry->vdev = vdev;
                 __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma;
                 __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma;
-               __entry->hcd_portnum = (int) vdev->rhub_port->hcd_portnum;
-               __entry->hw_portnum = (int) vdev->rhub_port->hw_portnum;
+               __entry->slot_id = (int) vdev->slot_id;
                 __entry->current_mel = (u16) vdev->current_mel;
                 ),
-       TP_printk("vdev %p ctx %llx | %llx hcd_portnum %d hw_portnum %d current_mel %d",
-               __entry->vdev, __entry->in_ctx, __entry->out_ctx,
-               __entry->hcd_portnum, __entry->hw_portnum, __entry->current_mel
+       TP_printk("vdev %p slot %d ctx %llx | %llx current_mel %d",
+               __entry->vdev, __entry->slot_id, __entry->in_ctx,
+               __entry->out_ctx, __entry->current_mel
         )
  );

It works for me

Thanks
Mathias

Re: [PATCH 2/9] xhci: replace real & fake port with pointer to root hub port

[Thinh Nguyen]

On Tue, Apr 02, 2024, Mathias Nyman wrote:
> Hi Thinh
> 
> On 2.4.2024 3.50, Thinh Nguyen wrote:
> > Hi Mathias,
> > 
> > We're getting a NULL pointer dereference bug for this patch. To
> > reproduce this, just unload and reload the xhci driver while a device is
> > connected. It may take a few times to hit the issue.
> > 
> > [ 1515.737623] BUG: kernel NULL pointer dereference, address: 000000000000000c
> > [ 1515.737629] #PF: supervisor read access in kernel mode
> > [ 1515.737631] #PF: error_code(0x0000) - not-present page
> > [ 1515.737633] PGD 0 P4D 0
> > [ 1515.737636] Oops: 0000 [#1] PREEMPT SMP NOPTI
> > [ 1515.737639] CPU: 1 PID: 2905 Comm: kworker/1:0 Not tainted 6.9.0-rc2-snps-detached-HEAD-g5bab5dc780c9 #8
> > [ 1515.737642] Hardware name: System manufacturer System Product Name/PRIME Q370M-C, BIOS 2605 10/16/2019
> > [ 1515.737643] Workqueue: usb_hub_wq hub_event
> > [ 1515.737648] RIP: 0010:trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd]
> > [ 1515.737667] Code: 85 c0 74 59 48 89 58 08 48 8b 53 18 48 89 e7 48 8b 52 10 48 89 50 18 48 8b 53 10 48 8b 52 10 48 89 50 10 48 8b 93 90 11 00 00 <8b> 52 0c 89 50 20 48 8b 93 90 11 00 00 8b 52 08 89 50 24 8b 93 b0
> > [ 1515.737669] RSP: 0018:ffffac30c0bdbce8 EFLAGS: 00010086
> > [ 1515.737671] RAX: ffff99e0003309b8 RBX: ffff99e040fb4000 RCX: ffff99e0003309b4
> > [ 1515.737673] RDX: 0000000000000000 RSI: 0000000000000616 RDI: ffffac30c0bdbce8
> > [ 1515.737729] RBP: ffff99e007b58a20 R08: 0000000000000002 R09: 0000000000000010
> > [ 1515.737731] R10: 000000014d33cf30 R11: 0000000000000020 R12: 0000000000000001
> > [ 1515.737733] R13: ffff99e008c9a294 R14: ffff99e008c9a258 R15: ffff99e004ca5000
> > [ 1515.737734] FS:  0000000000000000(0000) GS:ffff99e35bc40000(0000) knlGS:0000000000000000
> > [ 1515.737736] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 1515.737738] CR2: 000000000000000c CR3: 00000001024d6004 CR4: 00000000003706f0
> > [ 1515.737740] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [ 1515.737741] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [ 1515.737743] Call Trace:
> > [ 1515.737746]  <TASK>
> > [ 1515.737748]  ? __die_body+0x1a/0x5c
> > [ 1515.737751]  ? page_fault_oops+0x2ea/0x380
> > [ 1515.737806]  ? sched_clock+0x10/0x23
> > [ 1515.737808]  ? trace_clock_local+0x10/0x23
> > [ 1515.737812]  ? exc_page_fault+0xfe/0x11e
> > [ 1515.737815]  ? asm_exc_page_fault+0x26/0x30
> > [ 1515.737818]  ? trace_event_raw_event_xhci_log_free_virt_dev+0x64/0xb7 [xhci_hcd]
> > [ 1515.737832]  ? trace_event_raw_event_xhci_log_free_virt_dev+0x39/0xb7 [xhci_hcd]
> > [ 1515.737844]  xhci_free_virt_device+0x76/0x1d8 [xhci_hcd]
> > [ 1515.737856]  xhci_free_dev+0x111/0x12f [xhci_hcd]
> > [ 1515.737867]  hub_event+0xca6/0x137e
> 
> Thanks for the report, and for debugging this.
> I was able to reproduce it myself using your steps.
> 
> This triggers if xhci tracing is enabled and usb device is freed before it's addressed
> as vdev->rhub_port is only set during addressing.
> 
> > 
> > 
> > To capture the tracepoints and avoid the NULL pointer, I made this
> > change:
> > 
> 
> I think we could skip printing the hcd_portnum and hw_portnum during
> xhci_free_virt_dev() tracing. I haven't really found them useful.
> 
> It would make sense to print the slot_id instead.
> 
> how about this:
> 
> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
> index 1740000d54c2..5762564b9d73 100644
> --- a/drivers/usb/host/xhci-trace.h
> +++ b/drivers/usb/host/xhci-trace.h
> @@ -172,8 +172,7 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
>                 __field(void *, vdev)
>                 __field(unsigned long long, out_ctx)
>                 __field(unsigned long long, in_ctx)
> -               __field(int, hcd_portnum)
> -               __field(int, hw_portnum)
> +               __field(int, slot_id)
>                 __field(u16, current_mel)
>         ),
> @@ -181,13 +180,12 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
>                 __entry->vdev = vdev;
>                 __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma;
>                 __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma;
> -               __entry->hcd_portnum = (int) vdev->rhub_port->hcd_portnum;
> -               __entry->hw_portnum = (int) vdev->rhub_port->hw_portnum;
> +               __entry->slot_id = (int) vdev->slot_id;
>                 __entry->current_mel = (u16) vdev->current_mel;
>                 ),
> -       TP_printk("vdev %p ctx %llx | %llx hcd_portnum %d hw_portnum %d current_mel %d",
> -               __entry->vdev, __entry->in_ctx, __entry->out_ctx,
> -               __entry->hcd_portnum, __entry->hw_portnum, __entry->current_mel
> +       TP_printk("vdev %p slot %d ctx %llx | %llx current_mel %d",
> +               __entry->vdev, __entry->slot_id, __entry->in_ctx,
> +               __entry->out_ctx, __entry->current_mel
>         )
>  );
> 
> It works for me
> 

That looks good to me. Can you submit the change?

On an unrelated note, often we have to debug xHCI driver on a system
with multiple xHCI controllers. I'm not sure if there's a good way to
filter the xHCI tracepoints to a specific controller? I needed to print
the devname for each tracepoint just to get around this, which doesn't
seem like a great solution. Any idea?

Thanks,
Thinh

Re: [PATCH 2/9] xhci: replace real & fake port with pointer to root hub port

[Mathias Nyman]

On 3.4.2024 2.13, Thinh Nguyen wrote:
> On Tue, Apr 02, 2024, Mathias Nyman wrote:
>> Hi Thinh
>>
>> On 2.4.2024 3.50, Thinh Nguyen wrote:
>>> Hi Mathias,
>>>
>>> We're getting a NULL pointer dereference bug for this patch. To
>>> reproduce this, just unload and reload the xhci driver while a device is
>>> connected. It may take a few times to hit the issue.
>>>
>> how about this:
>>
>> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
>> index 1740000d54c2..5762564b9d73 100644
>> --- a/drivers/usb/host/xhci-trace.h
>> +++ b/drivers/usb/host/xhci-trace.h
>> @@ -172,8 +172,7 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
>>                  __field(void *, vdev)
>>                  __field(unsigned long long, out_ctx)
>>                  __field(unsigned long long, in_ctx)
>> -               __field(int, hcd_portnum)
>> -               __field(int, hw_portnum)
>> +               __field(int, slot_id)
>>                  __field(u16, current_mel)
>>          ),
>> @@ -181,13 +180,12 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
>>                  __entry->vdev = vdev;
>>                  __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma;
>>                  __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma;
>> -               __entry->hcd_portnum = (int) vdev->rhub_port->hcd_portnum;
>> -               __entry->hw_portnum = (int) vdev->rhub_port->hw_portnum;
>> +               __entry->slot_id = (int) vdev->slot_id;
>>                  __entry->current_mel = (u16) vdev->current_mel;
>>                  ),
>> -       TP_printk("vdev %p ctx %llx | %llx hcd_portnum %d hw_portnum %d current_mel %d",
>> -               __entry->vdev, __entry->in_ctx, __entry->out_ctx,
>> -               __entry->hcd_portnum, __entry->hw_portnum, __entry->current_mel
>> +       TP_printk("vdev %p slot %d ctx %llx | %llx current_mel %d",
>> +               __entry->vdev, __entry->slot_id, __entry->in_ctx,
>> +               __entry->out_ctx, __entry->current_mel
>>          )
>>   );
> 
> That looks good to me. Can you submit the change?

yes, I'll submit the change

> 
> On an unrelated note, often we have to debug xHCI driver on a system
> with multiple xHCI controllers. I'm not sure if there's a good way to
> filter the xHCI tracepoints to a specific controller? I needed to print
> the devname for each tracepoint just to get around this, which doesn't
> seem like a great solution. Any idea?

I'm facing similar debugging issues, I'll look into it.

Thanks
Mathias