[PATCH v2 0/5] usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports

[PATCH v2 0/5] usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports

[Heiner Kallweit]

I have a system with a low-cost Amlogic S905W SoC that supports USB3
but has a USB3 root hub with no ports. This results in an error
message and a hcd that is good for nothing. The USB2 root hub has
ports and works normally.
I think we can do better and omit creating a shared hcd if either of
the root hubs has no ports. This series is based on discussion [0].

The series works as intended for me. What I couldn't test is the case
of the USB2 root hub having no ports.

Follow-up to this series could be applying the xhci-plat extension
to other xhci drivers.

[0] https://www.spinics.net/lists/linux-usb/msg223416.html

v2:
- reordered patches
- create new helper xhci_has_one_roothub() instead of using
  xhci_hcd->needs_shared_hcd (proposed by Mathias)

Heiner Kallweit (5):
  xhci: factor out parts of xhci_gen_setup()
  xhci: prepare for operation w/o shared hcd
  usb: host: xhci-plat: create shared hcd after having added main hcd
  usb: host: xhci-plat: prepare operation w/o shared hcd
  usb: host: xhci-plat: omit shared hcd if either root hub has no ports

 drivers/usb/host/xhci-hub.c  |   3 +-
 drivers/usb/host/xhci-mem.c  |  11 +--
 drivers/usb/host/xhci-plat.c |  46 +++++++----
 drivers/usb/host/xhci.c      | 155 ++++++++++++++++++++---------------
 drivers/usb/host/xhci.h      |  26 ++++++
 5 files changed, 149 insertions(+), 92 deletions(-)

-- 
2.35.1

[PATCH v2 0/5] usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports

[Heiner Kallweit]

I have a system with a low-cost Amlogic S905W SoC that supports USB3
but has a USB3 root hub with no ports. This results in an error
message and a hcd that is good for nothing. The USB2 root hub has
ports and works normally.
I think we can do better and omit creating a shared hcd if either of
the root hubs has no ports. This series is based on discussion [0].

The series works as intended for me. What I couldn't test is the case
of the USB2 root hub having no ports.

Follow-up to this series could be applying the xhci-plat extension
to other xhci drivers.

[0] https://www.spinics.net/lists/linux-usb/msg223416.html

v2:
- reordered patches
- create new helper xhci_has_one_roothub() instead of using
  xhci_hcd->needs_shared_hcd (proposed by Mathias)

Heiner Kallweit (5):
  xhci: factor out parts of xhci_gen_setup()
  xhci: prepare for operation w/o shared hcd
  usb: host: xhci-plat: create shared hcd after having added main hcd
  usb: host: xhci-plat: prepare operation w/o shared hcd
  usb: host: xhci-plat: omit shared hcd if either root hub has no ports

 drivers/usb/host/xhci-hub.c  |   3 +-
 drivers/usb/host/xhci-mem.c  |  11 +--
 drivers/usb/host/xhci-plat.c |  46 +++++++----
 drivers/usb/host/xhci.c      | 155 ++++++++++++++++++++---------------
 drivers/usb/host/xhci.h      |  26 ++++++
 5 files changed, 149 insertions(+), 92 deletions(-)

-- 
2.35.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v2 1/5] xhci: factor out parts of xhci_gen_setup()

[Heiner Kallweit]

Factoring out parts of xhci_gen_setup() has two motivations:
- When adding functionaliy to omit shared hcd if not needed in a
  subsequent patch, we'll have to call xhci_hcd_init_usb3_data()
  from two places.
- It reduces size of xhci_gen_setup() and makes it better readable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/usb/host/xhci.c | 104 +++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 50 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 642610c78..4949de71a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5196,6 +5196,57 @@ static int xhci_get_frame(struct usb_hcd *hcd)
 	return readl(&xhci->run_regs->microframe_index) >> 3;
 }
 
+static void xhci_hcd_init_usb2_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
+{
+	xhci->usb2_rhub.hcd = hcd;
+	hcd->speed = HCD_USB2;
+	hcd->self.root_hub->speed = USB_SPEED_HIGH;
+	/*
+	 * USB 2.0 roothub under xHCI has an integrated TT,
+	 * (rate matching hub) as opposed to having an OHCI/UHCI
+	 * companion controller.
+	 */
+	hcd->has_tt = 1;
+}
+
+static void xhci_hcd_init_usb3_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
+{
+	unsigned int minor_rev;
+
+	/*
+	 * Early xHCI 1.1 spec did not mention USB 3.1 capable hosts
+	 * should return 0x31 for sbrn, or that the minor revision
+	 * is a two digit BCD containig minor and sub-minor numbers.
+	 * This was later clarified in xHCI 1.2.
+	 *
+	 * Some USB 3.1 capable hosts therefore have sbrn 0x30, and
+	 * minor revision set to 0x1 instead of 0x10.
+	 */
+	if (xhci->usb3_rhub.min_rev == 0x1)
+		minor_rev = 1;
+	else
+		minor_rev = xhci->usb3_rhub.min_rev / 0x10;
+
+	switch (minor_rev) {
+	case 2:
+		hcd->speed = HCD_USB32;
+		hcd->self.root_hub->speed = USB_SPEED_SUPER_PLUS;
+		hcd->self.root_hub->rx_lanes = 2;
+		hcd->self.root_hub->tx_lanes = 2;
+		hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x2;
+		break;
+	case 1:
+		hcd->speed = HCD_USB31;
+		hcd->self.root_hub->speed = USB_SPEED_SUPER_PLUS;
+		hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x1;
+		break;
+	}
+	xhci_info(xhci, "Host supports USB 3.%x %sSuperSpeed\n",
+		  minor_rev, minor_rev ? "Enhanced " : "");
+
+	xhci->usb3_rhub.hcd = hcd;
+}
+
 int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 {
 	struct xhci_hcd		*xhci;
@@ -5204,7 +5255,6 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	 * quirks
 	 */
 	struct device		*dev = hcd->self.sysdev;
-	unsigned int		minor_rev;
 	int			retval;
 
 	/* Accept arbitrarily long scatter-gather lists */
@@ -5219,60 +5269,14 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	xhci = hcd_to_xhci(hcd);
 
 	if (usb_hcd_is_primary_hcd(hcd)) {
-		xhci->main_hcd = hcd;
-		xhci->usb2_rhub.hcd = hcd;
-		/* Mark the first roothub as being USB 2.0.
-		 * The xHCI driver will register the USB 3.0 roothub.
-		 */
-		hcd->speed = HCD_USB2;
-		hcd->self.root_hub->speed = USB_SPEED_HIGH;
-		/*
-		 * USB 2.0 roothub under xHCI has an integrated TT,
-		 * (rate matching hub) as opposed to having an OHCI/UHCI
-		 * companion controller.
-		 */
-		hcd->has_tt = 1;
+		xhci_hcd_init_usb2_data(xhci, hcd);
 	} else {
-		/*
-		 * Early xHCI 1.1 spec did not mention USB 3.1 capable hosts
-		 * should return 0x31 for sbrn, or that the minor revision
-		 * is a two digit BCD containig minor and sub-minor numbers.
-		 * This was later clarified in xHCI 1.2.
-		 *
-		 * Some USB 3.1 capable hosts therefore have sbrn 0x30, and
-		 * minor revision set to 0x1 instead of 0x10.
-		 */
-		if (xhci->usb3_rhub.min_rev == 0x1)
-			minor_rev = 1;
-		else
-			minor_rev = xhci->usb3_rhub.min_rev / 0x10;
-
-		switch (minor_rev) {
-		case 2:
-			hcd->speed = HCD_USB32;
-			hcd->self.root_hub->speed = USB_SPEED_SUPER_PLUS;
-			hcd->self.root_hub->rx_lanes = 2;
-			hcd->self.root_hub->tx_lanes = 2;
-			hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x2;
-			break;
-		case 1:
-			hcd->speed = HCD_USB31;
-			hcd->self.root_hub->speed = USB_SPEED_SUPER_PLUS;
-			hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x1;
-			break;
-		}
-		xhci_info(xhci, "Host supports USB 3.%x %sSuperSpeed\n",
-			  minor_rev,
-			  minor_rev ? "Enhanced " : "");
-
-		xhci->usb3_rhub.hcd = hcd;
-		/* xHCI private pointer was set in xhci_pci_probe for the second
-		 * registered roothub.
-		 */
+		xhci_hcd_init_usb3_data(xhci, hcd);
 		return 0;
 	}
 
 	mutex_init(&xhci->mutex);
+	xhci->main_hcd = hcd;
 	xhci->cap_regs = hcd->regs;
 	xhci->op_regs = hcd->regs +
 		HC_LENGTH(readl(&xhci->cap_regs->hc_capbase));
-- 
2.35.1

[PATCH v2 1/5] xhci: factor out parts of xhci_gen_setup()

[Heiner Kallweit]

Factoring out parts of xhci_gen_setup() has two motivations:
- When adding functionaliy to omit shared hcd if not needed in a
  subsequent patch, we'll have to call xhci_hcd_init_usb3_data()
  from two places.
- It reduces size of xhci_gen_setup() and makes it better readable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/usb/host/xhci.c | 104 +++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 50 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 642610c78..4949de71a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5196,6 +5196,57 @@ static int xhci_get_frame(struct usb_hcd *hcd)
 	return readl(&xhci->run_regs->microframe_index) >> 3;
 }
 
+static void xhci_hcd_init_usb2_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
+{
+	xhci->usb2_rhub.hcd = hcd;
+	hcd->speed = HCD_USB2;
+	hcd->self.root_hub->speed = USB_SPEED_HIGH;
+	/*
+	 * USB 2.0 roothub under xHCI has an integrated TT,
+	 * (rate matching hub) as opposed to having an OHCI/UHCI
+	 * companion controller.
+	 */
+	hcd->has_tt = 1;
+}
+
+static void xhci_hcd_init_usb3_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
+{
+	unsigned int minor_rev;
+
+	/*
+	 * Early xHCI 1.1 spec did not mention USB 3.1 capable hosts
+	 * should return 0x31 for sbrn, or that the minor revision
+	 * is a two digit BCD containig minor and sub-minor numbers.
+	 * This was later clarified in xHCI 1.2.
+	 *
+	 * Some USB 3.1 capable hosts therefore have sbrn 0x30, and
+	 * minor revision set to 0x1 instead of 0x10.
+	 */
+	if (xhci->usb3_rhub.min_rev == 0x1)
+		minor_rev = 1;
+	else
+		minor_rev = xhci->usb3_rhub.min_rev / 0x10;
+
+	switch (minor_rev) {
+	case 2:
+		hcd->speed = HCD_USB32;
+		hcd->self.root_hub->speed = USB_SPEED_SUPER_PLUS;
+		hcd->self.root_hub->rx_lanes = 2;
+		hcd->self.root_hub->tx_lanes = 2;
+		hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x2;
+		break;
+	case 1:
+		hcd->speed = HCD_USB31;
+		hcd->self.root_hub->speed = USB_SPEED_SUPER_PLUS;
+		hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x1;
+		break;
+	}
+	xhci_info(xhci, "Host supports USB 3.%x %sSuperSpeed\n",
+		  minor_rev, minor_rev ? "Enhanced " : "");
+
+	xhci->usb3_rhub.hcd = hcd;
+}
+
 int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 {
 	struct xhci_hcd		*xhci;
@@ -5204,7 +5255,6 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	 * quirks
 	 */
 	struct device		*dev = hcd->self.sysdev;
-	unsigned int		minor_rev;
 	int			retval;
 
 	/* Accept arbitrarily long scatter-gather lists */
@@ -5219,60 +5269,14 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	xhci = hcd_to_xhci(hcd);
 
 	if (usb_hcd_is_primary_hcd(hcd)) {
-		xhci->main_hcd = hcd;
-		xhci->usb2_rhub.hcd = hcd;
-		/* Mark the first roothub as being USB 2.0.
-		 * The xHCI driver will register the USB 3.0 roothub.
-		 */
-		hcd->speed = HCD_USB2;
-		hcd->self.root_hub->speed = USB_SPEED_HIGH;
-		/*
-		 * USB 2.0 roothub under xHCI has an integrated TT,
-		 * (rate matching hub) as opposed to having an OHCI/UHCI
-		 * companion controller.
-		 */
-		hcd->has_tt = 1;
+		xhci_hcd_init_usb2_data(xhci, hcd);
 	} else {
-		/*
-		 * Early xHCI 1.1 spec did not mention USB 3.1 capable hosts
-		 * should return 0x31 for sbrn, or that the minor revision
-		 * is a two digit BCD containig minor and sub-minor numbers.
-		 * This was later clarified in xHCI 1.2.
-		 *
-		 * Some USB 3.1 capable hosts therefore have sbrn 0x30, and
-		 * minor revision set to 0x1 instead of 0x10.
-		 */
-		if (xhci->usb3_rhub.min_rev == 0x1)
-			minor_rev = 1;
-		else
-			minor_rev = xhci->usb3_rhub.min_rev / 0x10;
-
-		switch (minor_rev) {
-		case 2:
-			hcd->speed = HCD_USB32;
-			hcd->self.root_hub->speed = USB_SPEED_SUPER_PLUS;
-			hcd->self.root_hub->rx_lanes = 2;
-			hcd->self.root_hub->tx_lanes = 2;
-			hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x2;
-			break;
-		case 1:
-			hcd->speed = HCD_USB31;
-			hcd->self.root_hub->speed = USB_SPEED_SUPER_PLUS;
-			hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x1;
-			break;
-		}
-		xhci_info(xhci, "Host supports USB 3.%x %sSuperSpeed\n",
-			  minor_rev,
-			  minor_rev ? "Enhanced " : "");
-
-		xhci->usb3_rhub.hcd = hcd;
-		/* xHCI private pointer was set in xhci_pci_probe for the second
-		 * registered roothub.
-		 */
+		xhci_hcd_init_usb3_data(xhci, hcd);
 		return 0;
 	}
 
 	mutex_init(&xhci->mutex);
+	xhci->main_hcd = hcd;
 	xhci->cap_regs = hcd->regs;
 	xhci->op_regs = hcd->regs +
 		HC_LENGTH(readl(&xhci->cap_regs->hc_capbase));
-- 
2.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v2 2/5] xhci: prepare for operation w/o shared hcd

[Heiner Kallweit]

This patch prepares xhci for the following scenario:
- If either of the root hubs has no ports, then omit shared hcd
- Main hcd can be USB3 if there are no USB2 ports

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/usb/host/xhci-hub.c |  3 ++-
 drivers/usb/host/xhci-mem.c | 11 ++++----
 drivers/usb/host/xhci.c     | 53 ++++++++++++++++++++++++-------------
 drivers/usb/host/xhci.h     | 26 ++++++++++++++++++
 4 files changed, 68 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 1e7dc130c..9e835fdfe 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -707,6 +707,7 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
 				u16 test_mode, u16 wIndex, unsigned long *flags)
 	__must_hold(&xhci->lock)
 {
+	struct usb_hcd *usb3_hcd = xhci_get_usb3_hcd(xhci);
 	int i, retval;
 
 	/* Disable all Device Slots */
@@ -727,7 +728,7 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
 	xhci_dbg(xhci, "Disable all port (PP = 0)\n");
 	/* Power off USB3 ports*/
 	for (i = 0; i < xhci->usb3_rhub.num_ports; i++)
-		xhci_set_port_power(xhci, xhci->shared_hcd, i, false, flags);
+		xhci_set_port_power(xhci, usb3_hcd, i, false, flags);
 	/* Power off USB2 ports*/
 	for (i = 0; i < xhci->usb2_rhub.num_ports; i++)
 		xhci_set_port_power(xhci, xhci->main_hcd, i, false, flags);
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index bbb27ee2c..50bf64dcb 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1072,7 +1072,7 @@ static u32 xhci_find_real_port_number(struct xhci_hcd *xhci,
 	struct usb_hcd *hcd;
 
 	if (udev->speed >= USB_SPEED_SUPER)
-		hcd = xhci->shared_hcd;
+		hcd = xhci_get_usb3_hcd(xhci);
 	else
 		hcd = xhci->main_hcd;
 
@@ -2362,10 +2362,11 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
 		xhci->usb2_rhub.num_ports = USB_MAXCHILDREN;
 	}
 
-	/*
-	 * Note we could have all USB 3.0 ports, or all USB 2.0 ports.
-	 * Not sure how the USB core will handle a hub with no ports...
-	 */
+	if (!xhci->usb2_rhub.num_ports)
+		xhci_info(xhci, "USB2 root hub has no ports\n");
+
+	if (!xhci->usb3_rhub.num_ports)
+		xhci_info(xhci, "USB3 root hub has no ports\n");
 
 	xhci_create_rhub_port_array(xhci, &xhci->usb2_rhub, flags);
 	xhci_create_rhub_port_array(xhci, &xhci->usb3_rhub, flags);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4949de71a..5d1576a7b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -486,6 +486,10 @@ static void compliance_mode_recovery(struct timer_list *t)
 
 	xhci = from_timer(xhci, t, comp_mode_recovery_timer);
 	rhub = &xhci->usb3_rhub;
+	hcd = rhub->hcd;
+
+	if (!hcd)
+		return;
 
 	for (i = 0; i < rhub->num_ports; i++) {
 		temp = readl(rhub->ports[i]->addr);
@@ -499,7 +503,6 @@ static void compliance_mode_recovery(struct timer_list *t)
 					i + 1);
 			xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
 					"Attempting compliance mode recovery");
-			hcd = xhci->shared_hcd;
 
 			if (hcd->state == HC_STATE_SUSPENDED)
 				usb_hcd_resume_root_hub(hcd);
@@ -612,14 +615,11 @@ static int xhci_run_finished(struct xhci_hcd *xhci)
 		xhci_halt(xhci);
 		return -ENODEV;
 	}
-	xhci->shared_hcd->state = HC_STATE_RUNNING;
 	xhci->cmd_ring_state = CMD_RING_STATE_RUNNING;
 
 	if (xhci->quirks & XHCI_NEC_HOST)
 		xhci_ring_cmd_db(xhci);
 
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"Finished xhci_run for USB3 roothub");
 	return 0;
 }
 
@@ -694,12 +694,15 @@ int xhci_run(struct usb_hcd *hcd)
 			xhci_free_command(xhci, command);
 	}
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"Finished xhci_run for USB2 roothub");
+			"Finished %s for main hcd", __func__);
 
 	xhci_create_dbc_dev(xhci);
 
 	xhci_debugfs_init(xhci);
 
+	if (xhci_has_one_roothub(xhci))
+		return xhci_run_finished(xhci);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(xhci_run);
@@ -981,7 +984,7 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
 		return 0;
 
 	if (hcd->state != HC_STATE_SUSPENDED ||
-			xhci->shared_hcd->state != HC_STATE_SUSPENDED)
+	    (xhci->shared_hcd && xhci->shared_hcd->state != HC_STATE_SUSPENDED))
 		return -EINVAL;
 
 	/* Clear root port wake on bits if wakeup not allowed. */
@@ -998,15 +1001,18 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
 		 __func__, hcd->self.busnum);
 	clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);
 	del_timer_sync(&hcd->rh_timer);
-	clear_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
-	del_timer_sync(&xhci->shared_hcd->rh_timer);
+	if (xhci->shared_hcd) {
+		clear_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
+		del_timer_sync(&xhci->shared_hcd->rh_timer);
+	}
 
 	if (xhci->quirks & XHCI_SUSPEND_DELAY)
 		usleep_range(1000, 1500);
 
 	spin_lock_irq(&xhci->lock);
 	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
-	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
+	if (xhci->shared_hcd)
+		clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
 	/* step 1: stop endpoint */
 	/* skipped assuming that port suspend has done */
 
@@ -1106,7 +1112,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 		msleep(100);
 
 	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
-	set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
+	if (xhci->shared_hcd)
+		set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
 
 	spin_lock_irq(&xhci->lock);
 
@@ -1166,7 +1173,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 
 		/* Let the USB core know _both_ roothubs lost power. */
 		usb_root_hub_lost_power(xhci->main_hcd->self.root_hub);
-		usb_root_hub_lost_power(xhci->shared_hcd->self.root_hub);
+		if (xhci->shared_hcd)
+			usb_root_hub_lost_power(xhci->shared_hcd->self.root_hub);
 
 		xhci_dbg(xhci, "Stop HCD\n");
 		xhci_halt(xhci);
@@ -1206,12 +1214,13 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 
 		xhci_dbg(xhci, "Start the primary HCD\n");
 		retval = xhci_run(hcd->primary_hcd);
-		if (!retval) {
+		if (!retval && secondary_hcd) {
 			xhci_dbg(xhci, "Start the secondary HCD\n");
 			retval = xhci_run(secondary_hcd);
 		}
 		hcd->state = HC_STATE_SUSPENDED;
-		xhci->shared_hcd->state = HC_STATE_SUSPENDED;
+		if (xhci->shared_hcd)
+			xhci->shared_hcd->state = HC_STATE_SUSPENDED;
 		goto done;
 	}
 
@@ -1249,7 +1258,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 		}
 
 		if (pending_portevent) {
-			usb_hcd_resume_root_hub(xhci->shared_hcd);
+			if (xhci->shared_hcd)
+				usb_hcd_resume_root_hub(xhci->shared_hcd);
 			usb_hcd_resume_root_hub(hcd);
 		}
 	}
@@ -1268,8 +1278,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 	/* Re-enable port polling. */
 	xhci_dbg(xhci, "%s: starting usb%d port polling.\n",
 		 __func__, hcd->self.busnum);
-	set_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
-	usb_hcd_poll_rh_status(xhci->shared_hcd);
+	if (xhci->shared_hcd) {
+		set_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
+		usb_hcd_poll_rh_status(xhci->shared_hcd);
+	}
 	set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
 	usb_hcd_poll_rh_status(hcd);
 
@@ -5268,9 +5280,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 
 	xhci = hcd_to_xhci(hcd);
 
-	if (usb_hcd_is_primary_hcd(hcd)) {
-		xhci_hcd_init_usb2_data(xhci, hcd);
-	} else {
+	if (!usb_hcd_is_primary_hcd(hcd)) {
 		xhci_hcd_init_usb3_data(xhci, hcd);
 		return 0;
 	}
@@ -5351,6 +5361,11 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 		return retval;
 	xhci_dbg(xhci, "Called HCD init\n");
 
+	if (xhci_hcd_is_usb3(hcd))
+		xhci_hcd_init_usb3_data(xhci, hcd);
+	else
+		xhci_hcd_init_usb2_data(xhci, hcd);
+
 	xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%016llx\n",
 		  xhci->hcc_params, xhci->hci_version, xhci->quirks);
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 473a33ce2..c792a3148 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1911,6 +1911,8 @@ struct xhci_hcd {
 	unsigned		hw_lpm_support:1;
 	/* Broken Suspend flag for SNPS Suspend resume issue */
 	unsigned		broken_suspend:1;
+	/* Indicates that omitting hcd is supported if root hub has no ports */
+	unsigned		allow_single_roothub:1;
 	/* cached usb2 extened protocol capabilites */
 	u32                     *ext_caps;
 	unsigned int            num_ext_caps;
@@ -1966,6 +1968,30 @@ static inline struct usb_hcd *xhci_to_hcd(struct xhci_hcd *xhci)
 	return xhci->main_hcd;
 }
 
+static inline struct usb_hcd *xhci_get_usb3_hcd(struct xhci_hcd *xhci)
+{
+	if (xhci->shared_hcd)
+		return xhci->shared_hcd;
+
+	if (!xhci->usb2_rhub.num_ports)
+		return xhci->main_hcd;
+
+	return NULL;
+}
+
+static inline bool xhci_hcd_is_usb3(struct usb_hcd *hcd)
+{
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+	return hcd == xhci_get_usb3_hcd(xhci);
+}
+
+static inline bool xhci_has_one_roothub(struct xhci_hcd *xhci)
+{
+	return xhci->allow_single_roothub &&
+	       (!xhci->usb2_rhub.num_ports || !xhci->usb3_rhub.num_ports);
+}
+
 #define xhci_dbg(xhci, fmt, args...) \
 	dev_dbg(xhci_to_hcd(xhci)->self.controller , fmt , ## args)
 #define xhci_err(xhci, fmt, args...) \
-- 
2.35.1

[PATCH v2 2/5] xhci: prepare for operation w/o shared hcd

[Heiner Kallweit]

This patch prepares xhci for the following scenario:
- If either of the root hubs has no ports, then omit shared hcd
- Main hcd can be USB3 if there are no USB2 ports

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/usb/host/xhci-hub.c |  3 ++-
 drivers/usb/host/xhci-mem.c | 11 ++++----
 drivers/usb/host/xhci.c     | 53 ++++++++++++++++++++++++-------------
 drivers/usb/host/xhci.h     | 26 ++++++++++++++++++
 4 files changed, 68 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 1e7dc130c..9e835fdfe 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -707,6 +707,7 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
 				u16 test_mode, u16 wIndex, unsigned long *flags)
 	__must_hold(&xhci->lock)
 {
+	struct usb_hcd *usb3_hcd = xhci_get_usb3_hcd(xhci);
 	int i, retval;
 
 	/* Disable all Device Slots */
@@ -727,7 +728,7 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
 	xhci_dbg(xhci, "Disable all port (PP = 0)\n");
 	/* Power off USB3 ports*/
 	for (i = 0; i < xhci->usb3_rhub.num_ports; i++)
-		xhci_set_port_power(xhci, xhci->shared_hcd, i, false, flags);
+		xhci_set_port_power(xhci, usb3_hcd, i, false, flags);
 	/* Power off USB2 ports*/
 	for (i = 0; i < xhci->usb2_rhub.num_ports; i++)
 		xhci_set_port_power(xhci, xhci->main_hcd, i, false, flags);
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index bbb27ee2c..50bf64dcb 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1072,7 +1072,7 @@ static u32 xhci_find_real_port_number(struct xhci_hcd *xhci,
 	struct usb_hcd *hcd;
 
 	if (udev->speed >= USB_SPEED_SUPER)
-		hcd = xhci->shared_hcd;
+		hcd = xhci_get_usb3_hcd(xhci);
 	else
 		hcd = xhci->main_hcd;
 
@@ -2362,10 +2362,11 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
 		xhci->usb2_rhub.num_ports = USB_MAXCHILDREN;
 	}
 
-	/*
-	 * Note we could have all USB 3.0 ports, or all USB 2.0 ports.
-	 * Not sure how the USB core will handle a hub with no ports...
-	 */
+	if (!xhci->usb2_rhub.num_ports)
+		xhci_info(xhci, "USB2 root hub has no ports\n");
+
+	if (!xhci->usb3_rhub.num_ports)
+		xhci_info(xhci, "USB3 root hub has no ports\n");
 
 	xhci_create_rhub_port_array(xhci, &xhci->usb2_rhub, flags);
 	xhci_create_rhub_port_array(xhci, &xhci->usb3_rhub, flags);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4949de71a..5d1576a7b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -486,6 +486,10 @@ static void compliance_mode_recovery(struct timer_list *t)
 
 	xhci = from_timer(xhci, t, comp_mode_recovery_timer);
 	rhub = &xhci->usb3_rhub;
+	hcd = rhub->hcd;
+
+	if (!hcd)
+		return;
 
 	for (i = 0; i < rhub->num_ports; i++) {
 		temp = readl(rhub->ports[i]->addr);
@@ -499,7 +503,6 @@ static void compliance_mode_recovery(struct timer_list *t)
 					i + 1);
 			xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
 					"Attempting compliance mode recovery");
-			hcd = xhci->shared_hcd;
 
 			if (hcd->state == HC_STATE_SUSPENDED)
 				usb_hcd_resume_root_hub(hcd);
@@ -612,14 +615,11 @@ static int xhci_run_finished(struct xhci_hcd *xhci)
 		xhci_halt(xhci);
 		return -ENODEV;
 	}
-	xhci->shared_hcd->state = HC_STATE_RUNNING;
 	xhci->cmd_ring_state = CMD_RING_STATE_RUNNING;
 
 	if (xhci->quirks & XHCI_NEC_HOST)
 		xhci_ring_cmd_db(xhci);
 
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"Finished xhci_run for USB3 roothub");
 	return 0;
 }
 
@@ -694,12 +694,15 @@ int xhci_run(struct usb_hcd *hcd)
 			xhci_free_command(xhci, command);
 	}
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"Finished xhci_run for USB2 roothub");
+			"Finished %s for main hcd", __func__);
 
 	xhci_create_dbc_dev(xhci);
 
 	xhci_debugfs_init(xhci);
 
+	if (xhci_has_one_roothub(xhci))
+		return xhci_run_finished(xhci);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(xhci_run);
@@ -981,7 +984,7 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
 		return 0;
 
 	if (hcd->state != HC_STATE_SUSPENDED ||
-			xhci->shared_hcd->state != HC_STATE_SUSPENDED)
+	    (xhci->shared_hcd && xhci->shared_hcd->state != HC_STATE_SUSPENDED))
 		return -EINVAL;
 
 	/* Clear root port wake on bits if wakeup not allowed. */
@@ -998,15 +1001,18 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
 		 __func__, hcd->self.busnum);
 	clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);
 	del_timer_sync(&hcd->rh_timer);
-	clear_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
-	del_timer_sync(&xhci->shared_hcd->rh_timer);
+	if (xhci->shared_hcd) {
+		clear_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
+		del_timer_sync(&xhci->shared_hcd->rh_timer);
+	}
 
 	if (xhci->quirks & XHCI_SUSPEND_DELAY)
 		usleep_range(1000, 1500);
 
 	spin_lock_irq(&xhci->lock);
 	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
-	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
+	if (xhci->shared_hcd)
+		clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
 	/* step 1: stop endpoint */
 	/* skipped assuming that port suspend has done */
 
@@ -1106,7 +1112,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 		msleep(100);
 
 	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
-	set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
+	if (xhci->shared_hcd)
+		set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
 
 	spin_lock_irq(&xhci->lock);
 
@@ -1166,7 +1173,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 
 		/* Let the USB core know _both_ roothubs lost power. */
 		usb_root_hub_lost_power(xhci->main_hcd->self.root_hub);
-		usb_root_hub_lost_power(xhci->shared_hcd->self.root_hub);
+		if (xhci->shared_hcd)
+			usb_root_hub_lost_power(xhci->shared_hcd->self.root_hub);
 
 		xhci_dbg(xhci, "Stop HCD\n");
 		xhci_halt(xhci);
@@ -1206,12 +1214,13 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 
 		xhci_dbg(xhci, "Start the primary HCD\n");
 		retval = xhci_run(hcd->primary_hcd);
-		if (!retval) {
+		if (!retval && secondary_hcd) {
 			xhci_dbg(xhci, "Start the secondary HCD\n");
 			retval = xhci_run(secondary_hcd);
 		}
 		hcd->state = HC_STATE_SUSPENDED;
-		xhci->shared_hcd->state = HC_STATE_SUSPENDED;
+		if (xhci->shared_hcd)
+			xhci->shared_hcd->state = HC_STATE_SUSPENDED;
 		goto done;
 	}
 
@@ -1249,7 +1258,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 		}
 
 		if (pending_portevent) {
-			usb_hcd_resume_root_hub(xhci->shared_hcd);
+			if (xhci->shared_hcd)
+				usb_hcd_resume_root_hub(xhci->shared_hcd);
 			usb_hcd_resume_root_hub(hcd);
 		}
 	}
@@ -1268,8 +1278,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 	/* Re-enable port polling. */
 	xhci_dbg(xhci, "%s: starting usb%d port polling.\n",
 		 __func__, hcd->self.busnum);
-	set_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
-	usb_hcd_poll_rh_status(xhci->shared_hcd);
+	if (xhci->shared_hcd) {
+		set_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
+		usb_hcd_poll_rh_status(xhci->shared_hcd);
+	}
 	set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
 	usb_hcd_poll_rh_status(hcd);
 
@@ -5268,9 +5280,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 
 	xhci = hcd_to_xhci(hcd);
 
-	if (usb_hcd_is_primary_hcd(hcd)) {
-		xhci_hcd_init_usb2_data(xhci, hcd);
-	} else {
+	if (!usb_hcd_is_primary_hcd(hcd)) {
 		xhci_hcd_init_usb3_data(xhci, hcd);
 		return 0;
 	}
@@ -5351,6 +5361,11 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 		return retval;
 	xhci_dbg(xhci, "Called HCD init\n");
 
+	if (xhci_hcd_is_usb3(hcd))
+		xhci_hcd_init_usb3_data(xhci, hcd);
+	else
+		xhci_hcd_init_usb2_data(xhci, hcd);
+
 	xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%016llx\n",
 		  xhci->hcc_params, xhci->hci_version, xhci->quirks);
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 473a33ce2..c792a3148 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1911,6 +1911,8 @@ struct xhci_hcd {
 	unsigned		hw_lpm_support:1;
 	/* Broken Suspend flag for SNPS Suspend resume issue */
 	unsigned		broken_suspend:1;
+	/* Indicates that omitting hcd is supported if root hub has no ports */
+	unsigned		allow_single_roothub:1;
 	/* cached usb2 extened protocol capabilites */
 	u32                     *ext_caps;
 	unsigned int            num_ext_caps;
@@ -1966,6 +1968,30 @@ static inline struct usb_hcd *xhci_to_hcd(struct xhci_hcd *xhci)
 	return xhci->main_hcd;
 }
 
+static inline struct usb_hcd *xhci_get_usb3_hcd(struct xhci_hcd *xhci)
+{
+	if (xhci->shared_hcd)
+		return xhci->shared_hcd;
+
+	if (!xhci->usb2_rhub.num_ports)
+		return xhci->main_hcd;
+
+	return NULL;
+}
+
+static inline bool xhci_hcd_is_usb3(struct usb_hcd *hcd)
+{
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+	return hcd == xhci_get_usb3_hcd(xhci);
+}
+
+static inline bool xhci_has_one_roothub(struct xhci_hcd *xhci)
+{
+	return xhci->allow_single_roothub &&
+	       (!xhci->usb2_rhub.num_ports || !xhci->usb3_rhub.num_ports);
+}
+
 #define xhci_dbg(xhci, fmt, args...) \
 	dev_dbg(xhci_to_hcd(xhci)->self.controller , fmt , ## args)
 #define xhci_err(xhci, fmt, args...) \
-- 
2.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v2 3/5] usb: host: xhci-plat: create shared hcd after having added main hcd

[Heiner Kallweit]

This patch is in preparation of an extension where in case of a
root hub with no ports no shared hcd will be created.
Whether one of the root hubs has no ports we figure our in
usb_add_hcd() for the primary hcd. Therefore create the shared hcd
only after this call.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/usb/host/xhci-plat.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 649ffd861..5d752b384 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -283,12 +283,6 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	device_set_wakeup_capable(&pdev->dev, true);
 
 	xhci->main_hcd = hcd;
-	xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
-			dev_name(&pdev->dev), hcd);
-	if (!xhci->shared_hcd) {
-		ret = -ENOMEM;
-		goto disable_clk;
-	}
 
 	/* imod_interval is the interrupt moderation value in nanoseconds. */
 	xhci->imod_interval = 40000;
@@ -313,16 +307,16 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (IS_ERR(hcd->usb_phy)) {
 		ret = PTR_ERR(hcd->usb_phy);
 		if (ret == -EPROBE_DEFER)
-			goto put_usb3_hcd;
+			goto disable_clk;
 		hcd->usb_phy = NULL;
 	} else {
 		ret = usb_phy_init(hcd->usb_phy);
 		if (ret)
-			goto put_usb3_hcd;
+			goto disable_clk;
 	}
 
 	hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node);
-	xhci->shared_hcd->tpl_support = hcd->tpl_support;
+
 	if (priv && (priv->quirks & XHCI_SKIP_PHY_INIT))
 		hcd->skip_phy_initialization = 1;
 
@@ -333,12 +327,21 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_usb_phy;
 
+	xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
+			dev_name(&pdev->dev), hcd);
+	if (!xhci->shared_hcd) {
+		ret = -ENOMEM;
+		goto dealloc_usb2_hcd;
+	}
+
+	xhci->shared_hcd->tpl_support = hcd->tpl_support;
+
 	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
 
 	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
 	if (ret)
-		goto dealloc_usb2_hcd;
+		goto put_usb3_hcd;
 
 	device_enable_async_suspend(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
@@ -352,15 +355,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	return 0;
 
 
+put_usb3_hcd:
+	usb_put_hcd(xhci->shared_hcd);
+
 dealloc_usb2_hcd:
 	usb_remove_hcd(hcd);
 
 disable_usb_phy:
 	usb_phy_shutdown(hcd->usb_phy);
 
-put_usb3_hcd:
-	usb_put_hcd(xhci->shared_hcd);
-
 disable_clk:
 	clk_disable_unprepare(xhci->clk);
 
-- 
2.35.1

[PATCH v2 3/5] usb: host: xhci-plat: create shared hcd after having added main hcd

[Heiner Kallweit]

This patch is in preparation of an extension where in case of a
root hub with no ports no shared hcd will be created.
Whether one of the root hubs has no ports we figure our in
usb_add_hcd() for the primary hcd. Therefore create the shared hcd
only after this call.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/usb/host/xhci-plat.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 649ffd861..5d752b384 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -283,12 +283,6 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	device_set_wakeup_capable(&pdev->dev, true);
 
 	xhci->main_hcd = hcd;
-	xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
-			dev_name(&pdev->dev), hcd);
-	if (!xhci->shared_hcd) {
-		ret = -ENOMEM;
-		goto disable_clk;
-	}
 
 	/* imod_interval is the interrupt moderation value in nanoseconds. */
 	xhci->imod_interval = 40000;
@@ -313,16 +307,16 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (IS_ERR(hcd->usb_phy)) {
 		ret = PTR_ERR(hcd->usb_phy);
 		if (ret == -EPROBE_DEFER)
-			goto put_usb3_hcd;
+			goto disable_clk;
 		hcd->usb_phy = NULL;
 	} else {
 		ret = usb_phy_init(hcd->usb_phy);
 		if (ret)
-			goto put_usb3_hcd;
+			goto disable_clk;
 	}
 
 	hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node);
-	xhci->shared_hcd->tpl_support = hcd->tpl_support;
+
 	if (priv && (priv->quirks & XHCI_SKIP_PHY_INIT))
 		hcd->skip_phy_initialization = 1;
 
@@ -333,12 +327,21 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_usb_phy;
 
+	xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
+			dev_name(&pdev->dev), hcd);
+	if (!xhci->shared_hcd) {
+		ret = -ENOMEM;
+		goto dealloc_usb2_hcd;
+	}
+
+	xhci->shared_hcd->tpl_support = hcd->tpl_support;
+
 	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
 
 	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
 	if (ret)
-		goto dealloc_usb2_hcd;
+		goto put_usb3_hcd;
 
 	device_enable_async_suspend(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
@@ -352,15 +355,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	return 0;
 
 
+put_usb3_hcd:
+	usb_put_hcd(xhci->shared_hcd);
+
 dealloc_usb2_hcd:
 	usb_remove_hcd(hcd);
 
 disable_usb_phy:
 	usb_phy_shutdown(hcd->usb_phy);
 
-put_usb3_hcd:
-	usb_put_hcd(xhci->shared_hcd);
-
 disable_clk:
 	clk_disable_unprepare(xhci->clk);
 
-- 
2.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v2 4/5] usb: host: xhci-plat: prepare operation w/o shared hcd

[Heiner Kallweit]

This patch prepares xhci-plat for the following scenario
- If either of the root hubs has no ports, then omit shared hcd
- Main hcd can be USB3 if there are no USB2 ports

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/usb/host/xhci-plat.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 5d752b384..c512ec214 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -180,7 +180,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	struct device		*sysdev, *tmpdev;
 	struct xhci_hcd		*xhci;
 	struct resource         *res;
-	struct usb_hcd		*hcd;
+	struct usb_hcd		*hcd, *usb3_hcd;
 	int			ret;
 	int			irq;
 	struct xhci_plat_priv	*priv = NULL;
@@ -327,21 +327,26 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_usb_phy;
 
-	xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
-			dev_name(&pdev->dev), hcd);
-	if (!xhci->shared_hcd) {
-		ret = -ENOMEM;
-		goto dealloc_usb2_hcd;
-	}
+	if (!xhci_has_one_roothub(xhci)) {
+		xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
+						    dev_name(&pdev->dev), hcd);
+		if (!xhci->shared_hcd) {
+			ret = -ENOMEM;
+			goto dealloc_usb2_hcd;
+		}
 
-	xhci->shared_hcd->tpl_support = hcd->tpl_support;
+		xhci->shared_hcd->tpl_support = hcd->tpl_support;
+	}
 
-	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
-		xhci->shared_hcd->can_do_streams = 1;
+	usb3_hcd = xhci_get_usb3_hcd(xhci);
+	if (usb3_hcd && HCC_MAX_PSA(xhci->hcc_params) >= 4)
+		usb3_hcd->can_do_streams = 1;
 
-	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
-	if (ret)
-		goto put_usb3_hcd;
+	if (xhci->shared_hcd) {
+		ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
+		if (ret)
+			goto put_usb3_hcd;
+	}
 
 	device_enable_async_suspend(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
-- 
2.35.1

[PATCH v2 4/5] usb: host: xhci-plat: prepare operation w/o shared hcd

[Heiner Kallweit]

This patch prepares xhci-plat for the following scenario
- If either of the root hubs has no ports, then omit shared hcd
- Main hcd can be USB3 if there are no USB2 ports

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/usb/host/xhci-plat.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 5d752b384..c512ec214 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -180,7 +180,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	struct device		*sysdev, *tmpdev;
 	struct xhci_hcd		*xhci;
 	struct resource         *res;
-	struct usb_hcd		*hcd;
+	struct usb_hcd		*hcd, *usb3_hcd;
 	int			ret;
 	int			irq;
 	struct xhci_plat_priv	*priv = NULL;
@@ -327,21 +327,26 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_usb_phy;
 
-	xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
-			dev_name(&pdev->dev), hcd);
-	if (!xhci->shared_hcd) {
-		ret = -ENOMEM;
-		goto dealloc_usb2_hcd;
-	}
+	if (!xhci_has_one_roothub(xhci)) {
+		xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
+						    dev_name(&pdev->dev), hcd);
+		if (!xhci->shared_hcd) {
+			ret = -ENOMEM;
+			goto dealloc_usb2_hcd;
+		}
 
-	xhci->shared_hcd->tpl_support = hcd->tpl_support;
+		xhci->shared_hcd->tpl_support = hcd->tpl_support;
+	}
 
-	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
-		xhci->shared_hcd->can_do_streams = 1;
+	usb3_hcd = xhci_get_usb3_hcd(xhci);
+	if (usb3_hcd && HCC_MAX_PSA(xhci->hcc_params) >= 4)
+		usb3_hcd->can_do_streams = 1;
 
-	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
-	if (ret)
-		goto put_usb3_hcd;
+	if (xhci->shared_hcd) {
+		ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
+		if (ret)
+			goto put_usb3_hcd;
+	}
 
 	device_enable_async_suspend(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
-- 
2.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v2 5/5] usb: host: xhci-plat: omit shared hcd if either root hub has no ports

[Heiner Kallweit]

Activate the just added extension for xhci-plat and omit the shared
hcd if either of the root hubs has no ports.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/usb/host/xhci-plat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index c512ec214..044855818 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -245,6 +245,8 @@ static int xhci_plat_probe(struct platform_device *pdev)
 
 	xhci = hcd_to_xhci(hcd);
 
+	xhci->allow_single_roothub = 1;
+
 	/*
 	 * Not all platforms have clks so it is not an error if the
 	 * clock do not exist.
-- 
2.35.1

[PATCH v2 5/5] usb: host: xhci-plat: omit shared hcd if either root hub has no ports

[Heiner Kallweit]

Activate the just added extension for xhci-plat and omit the shared
hcd if either of the root hubs has no ports.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/usb/host/xhci-plat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index c512ec214..044855818 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -245,6 +245,8 @@ static int xhci_plat_probe(struct platform_device *pdev)
 
 	xhci = hcd_to_xhci(hcd);
 
+	xhci->allow_single_roothub = 1;
+
 	/*
 	 * Not all platforms have clks so it is not an error if the
 	 * clock do not exist.
-- 
2.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 0/5] usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports

[Mathias Nyman]

On 17.3.2022 0.08, Heiner Kallweit wrote:
> I have a system with a low-cost Amlogic S905W SoC that supports USB3
> but has a USB3 root hub with no ports. This results in an error
> message and a hcd that is good for nothing. The USB2 root hub has
> ports and works normally.
> I think we can do better and omit creating a shared hcd if either of
> the root hubs has no ports. This series is based on discussion [0].
> 
> The series works as intended for me. What I couldn't test is the case
> of the USB2 root hub having no ports.
> 
> Follow-up to this series could be applying the xhci-plat extension
> to other xhci drivers.
> 

Thanks,
Patches look good but something must have changed since you submitted them.
I can't apply them neatly on usb-next, usb-linus, 5.17, or 5.17-rc8


On top of usb-next I get:
Applying: usb: host: xhci-plat: create shared hcd after having added main hcd
error: patch failed: drivers/usb/host/xhci-plat.c:313
error: drivers/usb/host/xhci-plat.c: patch does not apply
Patch failed at 0003 usb: host: xhci-plat: create shared hcd after having added main hcd

-Mathias

Re: [PATCH v2 0/5] usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports

[Mathias Nyman]

On 17.3.2022 0.08, Heiner Kallweit wrote:
> I have a system with a low-cost Amlogic S905W SoC that supports USB3
> but has a USB3 root hub with no ports. This results in an error
> message and a hcd that is good for nothing. The USB2 root hub has
> ports and works normally.
> I think we can do better and omit creating a shared hcd if either of
> the root hubs has no ports. This series is based on discussion [0].
> 
> The series works as intended for me. What I couldn't test is the case
> of the USB2 root hub having no ports.
> 
> Follow-up to this series could be applying the xhci-plat extension
> to other xhci drivers.
> 

Thanks,
Patches look good but something must have changed since you submitted them.
I can't apply them neatly on usb-next, usb-linus, 5.17, or 5.17-rc8


On top of usb-next I get:
Applying: usb: host: xhci-plat: create shared hcd after having added main hcd
error: patch failed: drivers/usb/host/xhci-plat.c:313
error: drivers/usb/host/xhci-plat.c: patch does not apply
Patch failed at 0003 usb: host: xhci-plat: create shared hcd after having added main hcd

-Mathias

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 0/5] usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports

[Heiner Kallweit]

On 24.03.2022 10:23, Mathias Nyman wrote:
> On 17.3.2022 0.08, Heiner Kallweit wrote:
>> I have a system with a low-cost Amlogic S905W SoC that supports USB3
>> but has a USB3 root hub with no ports. This results in an error
>> message and a hcd that is good for nothing. The USB2 root hub has
>> ports and works normally.
>> I think we can do better and omit creating a shared hcd if either of
>> the root hubs has no ports. This series is based on discussion [0].
>>
>> The series works as intended for me. What I couldn't test is the case
>> of the USB2 root hub having no ports.
>>
>> Follow-up to this series could be applying the xhci-plat extension
>> to other xhci drivers.
>>
> 
> Thanks,
> Patches look good but something must have changed since you submitted them.
> I can't apply them neatly on usb-next, usb-linus, 5.17, or 5.17-rc8
> 
> 
> On top of usb-next I get:
> Applying: usb: host: xhci-plat: create shared hcd after having added main hcd
> error: patch failed: drivers/usb/host/xhci-plat.c:313
> error: drivers/usb/host/xhci-plat.c: patch does not apply
> Patch failed at 0003 usb: host: xhci-plat: create shared hcd after having added main hcd
> 
The series was based on linux-next. Seems you're missing 8e10548f7f48
("Revert "usb: host: xhci: mvebu: make USB 3.0 PHY optional for Armada 3720"").

> -Mathias

Heiner

Re: [PATCH v2 0/5] usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports

[Heiner Kallweit]

On 24.03.2022 10:23, Mathias Nyman wrote:
> On 17.3.2022 0.08, Heiner Kallweit wrote:
>> I have a system with a low-cost Amlogic S905W SoC that supports USB3
>> but has a USB3 root hub with no ports. This results in an error
>> message and a hcd that is good for nothing. The USB2 root hub has
>> ports and works normally.
>> I think we can do better and omit creating a shared hcd if either of
>> the root hubs has no ports. This series is based on discussion [0].
>>
>> The series works as intended for me. What I couldn't test is the case
>> of the USB2 root hub having no ports.
>>
>> Follow-up to this series could be applying the xhci-plat extension
>> to other xhci drivers.
>>
> 
> Thanks,
> Patches look good but something must have changed since you submitted them.
> I can't apply them neatly on usb-next, usb-linus, 5.17, or 5.17-rc8
> 
> 
> On top of usb-next I get:
> Applying: usb: host: xhci-plat: create shared hcd after having added main hcd
> error: patch failed: drivers/usb/host/xhci-plat.c:313
> error: drivers/usb/host/xhci-plat.c: patch does not apply
> Patch failed at 0003 usb: host: xhci-plat: create shared hcd after having added main hcd
> 
The series was based on linux-next. Seems you're missing 8e10548f7f48
("Revert "usb: host: xhci: mvebu: make USB 3.0 PHY optional for Armada 3720"").

> -Mathias

Heiner

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 0/5] usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports

[Mathias Nyman]

On 24.3.2022 21.38, Heiner Kallweit wrote:
> On 24.03.2022 10:23, Mathias Nyman wrote:
>> On 17.3.2022 0.08, Heiner Kallweit wrote:
>>> I have a system with a low-cost Amlogic S905W SoC that supports USB3
>>> but has a USB3 root hub with no ports. This results in an error
>>> message and a hcd that is good for nothing. The USB2 root hub has
>>> ports and works normally.
>>> I think we can do better and omit creating a shared hcd if either of
>>> the root hubs has no ports. This series is based on discussion [0].
>>>
>>> The series works as intended for me. What I couldn't test is the case
>>> of the USB2 root hub having no ports.
>>>
>>> Follow-up to this series could be applying the xhci-plat extension
>>> to other xhci drivers.
>>>
>>
>> Thanks,
>> Patches look good but something must have changed since you submitted them.
>> I can't apply them neatly on usb-next, usb-linus, 5.17, or 5.17-rc8
>>
>>
>> On top of usb-next I get:
>> Applying: usb: host: xhci-plat: create shared hcd after having added main hcd
>> error: patch failed: drivers/usb/host/xhci-plat.c:313
>> error: drivers/usb/host/xhci-plat.c: patch does not apply
>> Patch failed at 0003 usb: host: xhci-plat: create shared hcd after having added main hcd
>>
> The series was based on linux-next. Seems you're missing 8e10548f7f48
> ("Revert "usb: host: xhci: mvebu: make USB 3.0 PHY optional for Armada 3720"").
> 
>> -Mathias
> 
> Heiner
> 

Thanks, applies fine on linux-next.

I'll add these after 5.18-rc1 to my own tree, and try them out by faking a one roothub
PCI xhci device. Patches should end up in 5.19 if all goes well.

-Mathias 

Re: [PATCH v2 0/5] usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports

[Mathias Nyman]

On 24.3.2022 21.38, Heiner Kallweit wrote:
> On 24.03.2022 10:23, Mathias Nyman wrote:
>> On 17.3.2022 0.08, Heiner Kallweit wrote:
>>> I have a system with a low-cost Amlogic S905W SoC that supports USB3
>>> but has a USB3 root hub with no ports. This results in an error
>>> message and a hcd that is good for nothing. The USB2 root hub has
>>> ports and works normally.
>>> I think we can do better and omit creating a shared hcd if either of
>>> the root hubs has no ports. This series is based on discussion [0].
>>>
>>> The series works as intended for me. What I couldn't test is the case
>>> of the USB2 root hub having no ports.
>>>
>>> Follow-up to this series could be applying the xhci-plat extension
>>> to other xhci drivers.
>>>
>>
>> Thanks,
>> Patches look good but something must have changed since you submitted them.
>> I can't apply them neatly on usb-next, usb-linus, 5.17, or 5.17-rc8
>>
>>
>> On top of usb-next I get:
>> Applying: usb: host: xhci-plat: create shared hcd after having added main hcd
>> error: patch failed: drivers/usb/host/xhci-plat.c:313
>> error: drivers/usb/host/xhci-plat.c: patch does not apply
>> Patch failed at 0003 usb: host: xhci-plat: create shared hcd after having added main hcd
>>
> The series was based on linux-next. Seems you're missing 8e10548f7f48
> ("Revert "usb: host: xhci: mvebu: make USB 3.0 PHY optional for Armada 3720"").
> 
>> -Mathias
> 
> Heiner
> 

Thanks, applies fine on linux-next.

I'll add these after 5.18-rc1 to my own tree, and try them out by faking a one roothub
PCI xhci device. Patches should end up in 5.19 if all goes well.

-Mathias 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 4/5] usb: host: xhci-plat: prepare operation w/o shared hcd

[Matthias Kaehlcke]

Hi,

with v5.19-rc1 (which includes this patch) I encounter a NULL pointer
exception during system resume on a SC7280 board, which has an
USB2-only HCD:

[   40.490107] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[   40.490121] coresight-dynamic-funnel 6b04000.funnel: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.495836] Modules linked in: veth rfcomm algif_hash algif_skcipher af_alg uinput venus_enc venus_dec videobuf2_dma_contig videobuf2_memops cros_ec_typec xt_MASQUERADE typec hci_uart btqca venus_core v4l2_mem2mem videobuf2_v4l2 qcom_q6v5_mss videobuf2_common qcom_pil_info qcom_q6v5 ipa qcom_common rmtfs_mem
[   40.506420] coresight-dynamic-funnel 6b04000.funnel: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs
[   40.506427]  ip6table_nat fuse 8021q cdc_ether usbnet cfg80211 bluetooth ecdh_generic ecc r8152 mii joydev
[   40.534424] CPU: 4 PID: 68 Comm: kworker/u16:2 Not tainted 5.19.0-rc1 #160 01dfc77dff686f7aa36c93a01f531f57c578e1d9
[   40.534433] Hardware name: Google Herobrine (rev1+) (DT)
[   40.544539] coresight-tmc 6b05000.etf: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.554421] Workqueue: events_unbound async_run_entry_fn
[   40.554435] pstate: 00400009 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   40.554441] pc : usb_hcd_is_primary_hcd+0x10/0x30
[   40.556483] usb usb2: PM: usb_dev_resume+0x0/0x2c returned 0 after 123459 usecs
[   40.556698] usb 2-1: PM: calling usb_dev_resume+0x0/0x2c @ 3480, parent: usb2
[   40.565157] coresight-tmc 6b05000.etf: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs
[   40.570596] lr : xhci_init+0x24/0xf8
[   40.570604] sp : ffffffc008873c30
[   40.570606] x29: ffffffc008873c30 x28: ffffffdb82157000 x27: 0000000000000402
[   40.570615] x26: ffffff8080040838 x25: ffffff8080906a10 x24: 0000000000000002
[   40.579923] coresight-etm4x 7040000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.585363]
[   40.585365] x23: ffffffdb82157000 x22: 0000000000000000 x21: 0000000000000000
[   40.585372] x20: 0000000000000000 x19: ffffff8081f9c000 x18: 0000000000000800
[   40.585378] x17: 0000000000000354 x16: ffffffdb82162cf8 x15: fffffffe020377c8
[   40.592549] coresight-etm4x 7040000.etm: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs
[   40.597370] x14: 0000000000000000 x13: 0000000062a0f74e x12: 0000000000000018
[   40.597378] x11: 0000000080200000 x10: fffffffe02037820 x9 : ffffffdb817e4490
[   40.597385] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 000000000000003f
[   40.604934] coresight-etm4x 7140000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.612230] x5 : 0000000080200018 x4 : fffffffe02037820 x3 : 0000000080200018
[   40.612237] x2 : ffffff8080de0400 x1 : 0000000000000000 x0 : 0000000000000000
[   40.612244] Call trace:
[   40.612247]  usb_hcd_is_primary_hcd+0x10/0x30
[   40.621107] coresight-etm4x 7140000.etm: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs
[   40.624769]  xhci_resume+0x378/0x5a8
[   40.624775]  xhci_plat_resume+0x70/0xac
[   40.624783]  platform_pm_resume+0x44/0x58
[   40.628215] coresight-etm4x 7240000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.635529]  dpm_run_callback+0x54/0x1a0
[   40.635538]  device_resume+0x220/0x23c
[   40.635545]  async_resume+0x34/0x84
[   40.642885] coresight-etm4x 7240000.etm: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs
[   40.644448] usb usb3: PM: usb_dev_resume+0x0/0x2c returned 0 after 211490 usecs
[   40.644660] usb 3-1: PM: calling usb_dev_resume+0x0/0x2c @ 3486, parent: usb3
[   40.652340]  async_run_entry_fn+0x30/0xd8
[   40.652349]  process_one_work+0x190/0x3d0
[   40.652356]  worker_thread+0x230/0x3d4
[   40.652361]  kthread+0x104/0x2d0
[   40.653923] coresight-etm4x 7340000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.661229]  ret_from_fork+0x10/0x20
[   40.661243] Code: d503245f aa1e03e9 d503201f d503233f (f9413808)
[   40.661247] ---[ end trace 0000000000000000 ]---
[   40.681525] Kernel panic - not syncing: Oops: Fatal exception
[   40.681529] SMP: stopping secondary CPUs
[   40.684977] Kernel Offset: 0x1b79000000 from 0xffffffc008000000
[   40.684980] PHYS_OFFSET: 0x80000000
[   40.684982] CPU features: 0x800,0002e015,19801c82
[   40.684987] Memory Limit: none

In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
however this field isn't initialized by __usb_create_hcd() for a HCD
without secondary controller.

On Wed, Mar 16, 2022 at 11:11:33PM +0100, Heiner Kallweit wrote:
> This patch prepares xhci-plat for the following scenario
> - If either of the root hubs has no ports, then omit shared hcd
> - Main hcd can be USB3 if there are no USB2 ports
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/usb/host/xhci-plat.c | 31 ++++++++++++++++++-------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 5d752b384..c512ec214 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -180,7 +180,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	struct device		*sysdev, *tmpdev;
>  	struct xhci_hcd		*xhci;
>  	struct resource         *res;
> -	struct usb_hcd		*hcd;
> +	struct usb_hcd		*hcd, *usb3_hcd;
>  	int			ret;
>  	int			irq;
>  	struct xhci_plat_priv	*priv = NULL;
> @@ -327,21 +327,26 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto disable_usb_phy;
>  
> -	xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
> -			dev_name(&pdev->dev), hcd);
> -	if (!xhci->shared_hcd) {
> -		ret = -ENOMEM;
> -		goto dealloc_usb2_hcd;
> -	}
> +	if (!xhci_has_one_roothub(xhci)) {
> +		xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
> +						    dev_name(&pdev->dev), hcd);
> +		if (!xhci->shared_hcd) {
> +			ret = -ENOMEM;
> +			goto dealloc_usb2_hcd;
> +		}
>  
> -	xhci->shared_hcd->tpl_support = hcd->tpl_support;
> +		xhci->shared_hcd->tpl_support = hcd->tpl_support;
> +	}
>  
> -	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> -		xhci->shared_hcd->can_do_streams = 1;
> +	usb3_hcd = xhci_get_usb3_hcd(xhci);
> +	if (usb3_hcd && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> +		usb3_hcd->can_do_streams = 1;
>  
> -	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> -	if (ret)
> -		goto put_usb3_hcd;
> +	if (xhci->shared_hcd) {
> +		ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> +		if (ret)
> +			goto put_usb3_hcd;
> +	}
>  
>  	device_enable_async_suspend(&pdev->dev);
>  	pm_runtime_put_noidle(&pdev->dev);

Re: [PATCH v2 4/5] usb: host: xhci-plat: prepare operation w/o shared hcd

[Matthias Kaehlcke]

Hi,

with v5.19-rc1 (which includes this patch) I encounter a NULL pointer
exception during system resume on a SC7280 board, which has an
USB2-only HCD:

[   40.490107] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[   40.490121] coresight-dynamic-funnel 6b04000.funnel: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.495836] Modules linked in: veth rfcomm algif_hash algif_skcipher af_alg uinput venus_enc venus_dec videobuf2_dma_contig videobuf2_memops cros_ec_typec xt_MASQUERADE typec hci_uart btqca venus_core v4l2_mem2mem videobuf2_v4l2 qcom_q6v5_mss videobuf2_common qcom_pil_info qcom_q6v5 ipa qcom_common rmtfs_mem
[   40.506420] coresight-dynamic-funnel 6b04000.funnel: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs
[   40.506427]  ip6table_nat fuse 8021q cdc_ether usbnet cfg80211 bluetooth ecdh_generic ecc r8152 mii joydev
[   40.534424] CPU: 4 PID: 68 Comm: kworker/u16:2 Not tainted 5.19.0-rc1 #160 01dfc77dff686f7aa36c93a01f531f57c578e1d9
[   40.534433] Hardware name: Google Herobrine (rev1+) (DT)
[   40.544539] coresight-tmc 6b05000.etf: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.554421] Workqueue: events_unbound async_run_entry_fn
[   40.554435] pstate: 00400009 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   40.554441] pc : usb_hcd_is_primary_hcd+0x10/0x30
[   40.556483] usb usb2: PM: usb_dev_resume+0x0/0x2c returned 0 after 123459 usecs
[   40.556698] usb 2-1: PM: calling usb_dev_resume+0x0/0x2c @ 3480, parent: usb2
[   40.565157] coresight-tmc 6b05000.etf: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs
[   40.570596] lr : xhci_init+0x24/0xf8
[   40.570604] sp : ffffffc008873c30
[   40.570606] x29: ffffffc008873c30 x28: ffffffdb82157000 x27: 0000000000000402
[   40.570615] x26: ffffff8080040838 x25: ffffff8080906a10 x24: 0000000000000002
[   40.579923] coresight-etm4x 7040000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.585363]
[   40.585365] x23: ffffffdb82157000 x22: 0000000000000000 x21: 0000000000000000
[   40.585372] x20: 0000000000000000 x19: ffffff8081f9c000 x18: 0000000000000800
[   40.585378] x17: 0000000000000354 x16: ffffffdb82162cf8 x15: fffffffe020377c8
[   40.592549] coresight-etm4x 7040000.etm: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs
[   40.597370] x14: 0000000000000000 x13: 0000000062a0f74e x12: 0000000000000018
[   40.597378] x11: 0000000080200000 x10: fffffffe02037820 x9 : ffffffdb817e4490
[   40.597385] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 000000000000003f
[   40.604934] coresight-etm4x 7140000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.612230] x5 : 0000000080200018 x4 : fffffffe02037820 x3 : 0000000080200018
[   40.612237] x2 : ffffff8080de0400 x1 : 0000000000000000 x0 : 0000000000000000
[   40.612244] Call trace:
[   40.612247]  usb_hcd_is_primary_hcd+0x10/0x30
[   40.621107] coresight-etm4x 7140000.etm: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs
[   40.624769]  xhci_resume+0x378/0x5a8
[   40.624775]  xhci_plat_resume+0x70/0xac
[   40.624783]  platform_pm_resume+0x44/0x58
[   40.628215] coresight-etm4x 7240000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.635529]  dpm_run_callback+0x54/0x1a0
[   40.635538]  device_resume+0x220/0x23c
[   40.635545]  async_resume+0x34/0x84
[   40.642885] coresight-etm4x 7240000.etm: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs
[   40.644448] usb usb3: PM: usb_dev_resume+0x0/0x2c returned 0 after 211490 usecs
[   40.644660] usb 3-1: PM: calling usb_dev_resume+0x0/0x2c @ 3486, parent: usb3
[   40.652340]  async_run_entry_fn+0x30/0xd8
[   40.652349]  process_one_work+0x190/0x3d0
[   40.652356]  worker_thread+0x230/0x3d4
[   40.652361]  kthread+0x104/0x2d0
[   40.653923] coresight-etm4x 7340000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0
[   40.661229]  ret_from_fork+0x10/0x20
[   40.661243] Code: d503245f aa1e03e9 d503201f d503233f (f9413808)
[   40.661247] ---[ end trace 0000000000000000 ]---
[   40.681525] Kernel panic - not syncing: Oops: Fatal exception
[   40.681529] SMP: stopping secondary CPUs
[   40.684977] Kernel Offset: 0x1b79000000 from 0xffffffc008000000
[   40.684980] PHYS_OFFSET: 0x80000000
[   40.684982] CPU features: 0x800,0002e015,19801c82
[   40.684987] Memory Limit: none

In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
however this field isn't initialized by __usb_create_hcd() for a HCD
without secondary controller.

On Wed, Mar 16, 2022 at 11:11:33PM +0100, Heiner Kallweit wrote:
> This patch prepares xhci-plat for the following scenario
> - If either of the root hubs has no ports, then omit shared hcd
> - Main hcd can be USB3 if there are no USB2 ports
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/usb/host/xhci-plat.c | 31 ++++++++++++++++++-------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 5d752b384..c512ec214 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -180,7 +180,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	struct device		*sysdev, *tmpdev;
>  	struct xhci_hcd		*xhci;
>  	struct resource         *res;
> -	struct usb_hcd		*hcd;
> +	struct usb_hcd		*hcd, *usb3_hcd;
>  	int			ret;
>  	int			irq;
>  	struct xhci_plat_priv	*priv = NULL;
> @@ -327,21 +327,26 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto disable_usb_phy;
>  
> -	xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
> -			dev_name(&pdev->dev), hcd);
> -	if (!xhci->shared_hcd) {
> -		ret = -ENOMEM;
> -		goto dealloc_usb2_hcd;
> -	}
> +	if (!xhci_has_one_roothub(xhci)) {
> +		xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
> +						    dev_name(&pdev->dev), hcd);
> +		if (!xhci->shared_hcd) {
> +			ret = -ENOMEM;
> +			goto dealloc_usb2_hcd;
> +		}
>  
> -	xhci->shared_hcd->tpl_support = hcd->tpl_support;
> +		xhci->shared_hcd->tpl_support = hcd->tpl_support;
> +	}
>  
> -	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> -		xhci->shared_hcd->can_do_streams = 1;
> +	usb3_hcd = xhci_get_usb3_hcd(xhci);
> +	if (usb3_hcd && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> +		usb3_hcd->can_do_streams = 1;
>  
> -	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> -	if (ret)
> -		goto put_usb3_hcd;
> +	if (xhci->shared_hcd) {
> +		ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> +		if (ret)
> +			goto put_usb3_hcd;
> +	}
>  
>  	device_enable_async_suspend(&pdev->dev);
>  	pm_runtime_put_noidle(&pdev->dev);

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 4/5] usb: host: xhci-plat: prepare operation w/o shared hcd

[Mathias Nyman]

On 8.6.2022 23.55, Matthias Kaehlcke wrote:
> Hi,
> 
> with v5.19-rc1 (which includes this patch) I encounter a NULL pointer
> exception during system resume on a SC7280 board, which has an
> USB2-only HCD:
> 
...
> 
> In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
> however this field isn't initialized by __usb_create_hcd() for a HCD
> without secondary controller.

Thanks for debugging this, I'll write a patch for it.
Can you try it out?

-Mathias

Re: [PATCH v2 4/5] usb: host: xhci-plat: prepare operation w/o shared hcd

[Mathias Nyman]

On 8.6.2022 23.55, Matthias Kaehlcke wrote:
> Hi,
> 
> with v5.19-rc1 (which includes this patch) I encounter a NULL pointer
> exception during system resume on a SC7280 board, which has an
> USB2-only HCD:
> 
...
> 
> In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
> however this field isn't initialized by __usb_create_hcd() for a HCD
> without secondary controller.

Thanks for debugging this, I'll write a patch for it.
Can you try it out?

-Mathias

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[RFT PATCH] xhci: Fix null pointer dereference in resume if xhci has only one roothub

[Mathias Nyman]

In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
however this field isn't initialized by __usb_create_hcd() for a HCD
without secondary controller.

xhci_resume() is called once per xHC device, not per hcd, so the extra
checking for primary hcd can be removed.

Fixes: e0fe986972f5 ("usb: host: xhci-plat: prepare operation w/o shared hcd")
Reported-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f0ab63138016..9ac56e9ffc64 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1107,7 +1107,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 {
 	u32			command, temp = 0;
 	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
-	struct usb_hcd		*secondary_hcd;
 	int			retval = 0;
 	bool			comp_timer_running = false;
 	bool			pending_portevent = false;
@@ -1214,23 +1213,19 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 		 * first with the primary HCD, and then with the secondary HCD.
 		 * If we don't do the same, the host will never be started.
 		 */
-		if (!usb_hcd_is_primary_hcd(hcd))
-			secondary_hcd = hcd;
-		else
-			secondary_hcd = xhci->shared_hcd;
-
 		xhci_dbg(xhci, "Initialize the xhci_hcd\n");
-		retval = xhci_init(hcd->primary_hcd);
+		retval = xhci_init(hcd);
 		if (retval)
 			return retval;
 		comp_timer_running = true;
 
 		xhci_dbg(xhci, "Start the primary HCD\n");
-		retval = xhci_run(hcd->primary_hcd);
-		if (!retval && secondary_hcd) {
+		retval = xhci_run(hcd);
+		if (!retval && xhci->shared_hcd) {
 			xhci_dbg(xhci, "Start the secondary HCD\n");
-			retval = xhci_run(secondary_hcd);
+			retval = xhci_run(xhci->shared_hcd);
 		}
+
 		hcd->state = HC_STATE_SUSPENDED;
 		if (xhci->shared_hcd)
 			xhci->shared_hcd->state = HC_STATE_SUSPENDED;
-- 
2.25.1

[RFT PATCH] xhci: Fix null pointer dereference in resume if xhci has only one roothub

[Mathias Nyman]

In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
however this field isn't initialized by __usb_create_hcd() for a HCD
without secondary controller.

xhci_resume() is called once per xHC device, not per hcd, so the extra
checking for primary hcd can be removed.

Fixes: e0fe986972f5 ("usb: host: xhci-plat: prepare operation w/o shared hcd")
Reported-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f0ab63138016..9ac56e9ffc64 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1107,7 +1107,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 {
 	u32			command, temp = 0;
 	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
-	struct usb_hcd		*secondary_hcd;
 	int			retval = 0;
 	bool			comp_timer_running = false;
 	bool			pending_portevent = false;
@@ -1214,23 +1213,19 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 		 * first with the primary HCD, and then with the secondary HCD.
 		 * If we don't do the same, the host will never be started.
 		 */
-		if (!usb_hcd_is_primary_hcd(hcd))
-			secondary_hcd = hcd;
-		else
-			secondary_hcd = xhci->shared_hcd;
-
 		xhci_dbg(xhci, "Initialize the xhci_hcd\n");
-		retval = xhci_init(hcd->primary_hcd);
+		retval = xhci_init(hcd);
 		if (retval)
 			return retval;
 		comp_timer_running = true;
 
 		xhci_dbg(xhci, "Start the primary HCD\n");
-		retval = xhci_run(hcd->primary_hcd);
-		if (!retval && secondary_hcd) {
+		retval = xhci_run(hcd);
+		if (!retval && xhci->shared_hcd) {
 			xhci_dbg(xhci, "Start the secondary HCD\n");
-			retval = xhci_run(secondary_hcd);
+			retval = xhci_run(xhci->shared_hcd);
 		}
+
 		hcd->state = HC_STATE_SUSPENDED;
 		if (xhci->shared_hcd)
 			xhci->shared_hcd->state = HC_STATE_SUSPENDED;
-- 
2.25.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 4/5] usb: host: xhci-plat: prepare operation w/o shared hcd

[Matthias Kaehlcke]

On Thu, Jun 09, 2022 at 02:38:45PM +0300, Mathias Nyman wrote:
> On 8.6.2022 23.55, Matthias Kaehlcke wrote:
> > Hi,
> > 
> > with v5.19-rc1 (which includes this patch) I encounter a NULL pointer
> > exception during system resume on a SC7280 board, which has an
> > USB2-only HCD:
> > 
> ...
> > 
> > In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
> > however this field isn't initialized by __usb_create_hcd() for a HCD
> > without secondary controller.
> 
> Thanks for debugging this, I'll write a patch for it.
> Can you try it out?

Sure!

Re: [PATCH v2 4/5] usb: host: xhci-plat: prepare operation w/o shared hcd

[Matthias Kaehlcke]

On Thu, Jun 09, 2022 at 02:38:45PM +0300, Mathias Nyman wrote:
> On 8.6.2022 23.55, Matthias Kaehlcke wrote:
> > Hi,
> > 
> > with v5.19-rc1 (which includes this patch) I encounter a NULL pointer
> > exception during system resume on a SC7280 board, which has an
> > USB2-only HCD:
> > 
> ...
> > 
> > In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
> > however this field isn't initialized by __usb_create_hcd() for a HCD
> > without secondary controller.
> 
> Thanks for debugging this, I'll write a patch for it.
> Can you try it out?

Sure!

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [RFT PATCH] xhci: Fix null pointer dereference in resume if xhci has only one roothub

[Matthias Kaehlcke]

On Thu, Jun 09, 2022 at 03:03:36PM +0300, Mathias Nyman wrote:
> In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
> however this field isn't initialized by __usb_create_hcd() for a HCD
> without secondary controller.
> 
> xhci_resume() is called once per xHC device, not per hcd, so the extra
> checking for primary hcd can be removed.
> 
> Fixes: e0fe986972f5 ("usb: host: xhci-plat: prepare operation w/o shared hcd")
> Reported-by: Matthias Kaehlcke <mka@chromium.org>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/usb/host/xhci.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index f0ab63138016..9ac56e9ffc64 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1107,7 +1107,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>  {
>  	u32			command, temp = 0;
>  	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
> -	struct usb_hcd		*secondary_hcd;
>  	int			retval = 0;
>  	bool			comp_timer_running = false;
>  	bool			pending_portevent = false;
> @@ -1214,23 +1213,19 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>  		 * first with the primary HCD, and then with the secondary HCD.
>  		 * If we don't do the same, the host will never be started.
>  		 */
> -		if (!usb_hcd_is_primary_hcd(hcd))
> -			secondary_hcd = hcd;
> -		else
> -			secondary_hcd = xhci->shared_hcd;
> -
>  		xhci_dbg(xhci, "Initialize the xhci_hcd\n");
> -		retval = xhci_init(hcd->primary_hcd);
> +		retval = xhci_init(hcd);
>  		if (retval)
>  			return retval;
>  		comp_timer_running = true;
>  
>  		xhci_dbg(xhci, "Start the primary HCD\n");

Is the log still correct? IIUC this now isn't necessarily the primary HCD.

> -		retval = xhci_run(hcd->primary_hcd);
> -		if (!retval && secondary_hcd) {
> +		retval = xhci_run(hcd);
> +		if (!retval && xhci->shared_hcd) {
>  			xhci_dbg(xhci, "Start the secondary HCD\n");

ditto

> -			retval = xhci_run(secondary_hcd);
> +			retval = xhci_run(xhci->shared_hcd);
>  		}
> +
>  		hcd->state = HC_STATE_SUSPENDED;
>  		if (xhci->shared_hcd)
>  			xhci->shared_hcd->state = HC_STATE_SUSPENDED;

Tested-by: Matthias Kaehlcke <mka@chromium.org>

Re: [RFT PATCH] xhci: Fix null pointer dereference in resume if xhci has only one roothub

[Matthias Kaehlcke]

On Thu, Jun 09, 2022 at 03:03:36PM +0300, Mathias Nyman wrote:
> In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
> however this field isn't initialized by __usb_create_hcd() for a HCD
> without secondary controller.
> 
> xhci_resume() is called once per xHC device, not per hcd, so the extra
> checking for primary hcd can be removed.
> 
> Fixes: e0fe986972f5 ("usb: host: xhci-plat: prepare operation w/o shared hcd")
> Reported-by: Matthias Kaehlcke <mka@chromium.org>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/usb/host/xhci.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index f0ab63138016..9ac56e9ffc64 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1107,7 +1107,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>  {
>  	u32			command, temp = 0;
>  	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
> -	struct usb_hcd		*secondary_hcd;
>  	int			retval = 0;
>  	bool			comp_timer_running = false;
>  	bool			pending_portevent = false;
> @@ -1214,23 +1213,19 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>  		 * first with the primary HCD, and then with the secondary HCD.
>  		 * If we don't do the same, the host will never be started.
>  		 */
> -		if (!usb_hcd_is_primary_hcd(hcd))
> -			secondary_hcd = hcd;
> -		else
> -			secondary_hcd = xhci->shared_hcd;
> -
>  		xhci_dbg(xhci, "Initialize the xhci_hcd\n");
> -		retval = xhci_init(hcd->primary_hcd);
> +		retval = xhci_init(hcd);
>  		if (retval)
>  			return retval;
>  		comp_timer_running = true;
>  
>  		xhci_dbg(xhci, "Start the primary HCD\n");

Is the log still correct? IIUC this now isn't necessarily the primary HCD.

> -		retval = xhci_run(hcd->primary_hcd);
> -		if (!retval && secondary_hcd) {
> +		retval = xhci_run(hcd);
> +		if (!retval && xhci->shared_hcd) {
>  			xhci_dbg(xhci, "Start the secondary HCD\n");

ditto

> -			retval = xhci_run(secondary_hcd);
> +			retval = xhci_run(xhci->shared_hcd);
>  		}
> +
>  		hcd->state = HC_STATE_SUSPENDED;
>  		if (xhci->shared_hcd)
>  			xhci->shared_hcd->state = HC_STATE_SUSPENDED;

Tested-by: Matthias Kaehlcke <mka@chromium.org>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [RFT PATCH] xhci: Fix null pointer dereference in resume if xhci has only one roothub

[Mathias Nyman]

*On 9.6.2022 18.41, Matthias Kaehlcke wrote:
> On Thu, Jun 09, 2022 at 03:03:36PM +0300, Mathias Nyman wrote:
>> In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
>> however this field isn't initialized by __usb_create_hcd() for a HCD
>> without secondary controller.
>>
>> xhci_resume() is called once per xHC device, not per hcd, so the extra
>> checking for primary hcd can be removed.
>>
>> Fixes: e0fe986972f5 ("usb: host: xhci-plat: prepare operation w/o shared hcd")
>> Reported-by: Matthias Kaehlcke <mka@chromium.org>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> ---
>>   drivers/usb/host/xhci.c | 15 +++++----------
>>   1 file changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index f0ab63138016..9ac56e9ffc64 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -1107,7 +1107,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>>   {
>>   	u32			command, temp = 0;
>>   	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
>> -	struct usb_hcd		*secondary_hcd;
>>   	int			retval = 0;
>>   	bool			comp_timer_running = false;
>>   	bool			pending_portevent = false;
>> @@ -1214,23 +1213,19 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>>   		 * first with the primary HCD, and then with the secondary HCD.
>>   		 * If we don't do the same, the host will never be started.
>>   		 */
>> -		if (!usb_hcd_is_primary_hcd(hcd))
>> -			secondary_hcd = hcd;
>> -		else
>> -			secondary_hcd = xhci->shared_hcd;
>> -
>>   		xhci_dbg(xhci, "Initialize the xhci_hcd\n");
>> -		retval = xhci_init(hcd->primary_hcd);
>> +		retval = xhci_init(hcd);
>>   		if (retval)
>>   			return retval;
>>   		comp_timer_running = true;
>>   
>>   		xhci_dbg(xhci, "Start the primary HCD\n");
> 
> Is the log still correct? IIUC this now isn't necessarily the primary HCD.

It's still correct as this is always the xhci->main_hcd, the one that is created first.
There could be a better word than "primary", but my brain is accustomed to seeing this
line while debugging.


> 
>> -		retval = xhci_run(hcd->primary_hcd);
>> -		if (!retval && secondary_hcd) {
>> +		retval = xhci_run(hcd);
>> +		if (!retval && xhci->shared_hcd) {
>>   			xhci_dbg(xhci, "Start the secondary HCD\n");
> 
> ditto

same, always xhci->shared_hcd, the one that is created second.

> 
>> -			retval = xhci_run(secondary_hcd);
>> +			retval = xhci_run(xhci->shared_hcd);
>>   		}
>> +
>>   		hcd->state = HC_STATE_SUSPENDED;
>>   		if (xhci->shared_hcd)
>>   			xhci->shared_hcd->state = HC_STATE_SUSPENDED;
> 
> Tested-by: Matthias Kaehlcke <mka@chromium.org>

Thanks for testing
-Mathias

Re: [RFT PATCH] xhci: Fix null pointer dereference in resume if xhci has only one roothub

[Mathias Nyman]

*On 9.6.2022 18.41, Matthias Kaehlcke wrote:
> On Thu, Jun 09, 2022 at 03:03:36PM +0300, Mathias Nyman wrote:
>> In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(),
>> however this field isn't initialized by __usb_create_hcd() for a HCD
>> without secondary controller.
>>
>> xhci_resume() is called once per xHC device, not per hcd, so the extra
>> checking for primary hcd can be removed.
>>
>> Fixes: e0fe986972f5 ("usb: host: xhci-plat: prepare operation w/o shared hcd")
>> Reported-by: Matthias Kaehlcke <mka@chromium.org>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> ---
>>   drivers/usb/host/xhci.c | 15 +++++----------
>>   1 file changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index f0ab63138016..9ac56e9ffc64 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -1107,7 +1107,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>>   {
>>   	u32			command, temp = 0;
>>   	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
>> -	struct usb_hcd		*secondary_hcd;
>>   	int			retval = 0;
>>   	bool			comp_timer_running = false;
>>   	bool			pending_portevent = false;
>> @@ -1214,23 +1213,19 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>>   		 * first with the primary HCD, and then with the secondary HCD.
>>   		 * If we don't do the same, the host will never be started.
>>   		 */
>> -		if (!usb_hcd_is_primary_hcd(hcd))
>> -			secondary_hcd = hcd;
>> -		else
>> -			secondary_hcd = xhci->shared_hcd;
>> -
>>   		xhci_dbg(xhci, "Initialize the xhci_hcd\n");
>> -		retval = xhci_init(hcd->primary_hcd);
>> +		retval = xhci_init(hcd);
>>   		if (retval)
>>   			return retval;
>>   		comp_timer_running = true;
>>   
>>   		xhci_dbg(xhci, "Start the primary HCD\n");
> 
> Is the log still correct? IIUC this now isn't necessarily the primary HCD.

It's still correct as this is always the xhci->main_hcd, the one that is created first.
There could be a better word than "primary", but my brain is accustomed to seeing this
line while debugging.


> 
>> -		retval = xhci_run(hcd->primary_hcd);
>> -		if (!retval && secondary_hcd) {
>> +		retval = xhci_run(hcd);
>> +		if (!retval && xhci->shared_hcd) {
>>   			xhci_dbg(xhci, "Start the secondary HCD\n");
> 
> ditto

same, always xhci->shared_hcd, the one that is created second.

> 
>> -			retval = xhci_run(secondary_hcd);
>> +			retval = xhci_run(xhci->shared_hcd);
>>   		}
>> +
>>   		hcd->state = HC_STATE_SUSPENDED;
>>   		if (xhci->shared_hcd)
>>   			xhci->shared_hcd->state = HC_STATE_SUSPENDED;
> 
> Tested-by: Matthias Kaehlcke <mka@chromium.org>

Thanks for testing
-Mathias

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic