Re: [PATCH] usb: ehci: skip companion handoff when no companion exists

[Michal Pecio <michal.pecio@gmail.com>]

On Sat, 14 Nov 2026 05:16:53 +0000, William A. Kennington III wrote:
> EHCI controllers without an integrated root-hub transaction translator
> unconditionally set PORT_OWNER whenever a root port sees low-speed line
> state before reset, finishes reset without enabling High-Speed mode, or
> is relinquished by usbcore after enumeration fails. When no companion
> controller is present or enabled, setting PORT_OWNER routes the port
> away from EHCI to a nonexistent or disabled controller. A single missed
> High-Speed chirp or transient D+/D- bounce during reset then strands
> downstream USB 2.0 hubs until EHCI is rebound, and connecting a USB 1.1
> device leaves the port dead to later USB 2.0 devices.
> 
> Check during ehci_setup() whether HCSPARAMS reports zero companions
> (HCS_N_CC == 0), both CONFIG_USB_OHCI_HCD and CONFIG_USB_UHCI_HCD are
> disabled in Kconfig (such as in aspeed_g4_defconfig,
> aspeed_g5_defconfig, and BMC builds without UHCI or OHCI), or a Device
> Tree companion phandle points to a disabled node. Runtime companion
> binding is intentionally not checked at setup time so modular or
> later-probing companion drivers still receive handoffs.
> 
> When no companion is available, set ehci->no_companion to skip
> PORT_OWNER handoffs and omit the companion sysfs attribute, matching
> the integrated-TT path so usbcore keeps port ownership on EHCI.

Hi,

That's for Alan to decide, but it seems that whenever you use
ehci->no_companion, you always add "|| ehci_is_TD(ehci)", and this
is unlikely to change since HCs with TT don't need companions.

So ehci_is_TDI() together with its associated comment could be
moved to ehci_has_companion(). Then you no longer need those funny
comments like "without companion there is no companion" and the
code becomes somewhat simpler too.

Regards,
Michal

> USB 1.1 full- and low-speed devices still cannot enumerate without an
> integrated TT or companion controller and will fail usbcore's normal
> reset retries instead of triggering a PORT_OWNER handoff.
> 
> Signed-off-by: William A. Kennington III <william@wkennington.com>
> ---
>  drivers/usb/host/ehci-hcd.c   | 31 +++++++++++++++++++++++++++++++
>  drivers/usb/host/ehci-hub.c   | 21 ++++++++++-----------
>  drivers/usb/host/ehci-sysfs.c |  8 ++++----
>  drivers/usb/host/ehci.h       |  1 +
>  4 files changed, 46 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 75f780ec3ba0..1e813ac5f0cb 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -26,6 +26,7 @@
>  #include <linux/moduleparam.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/debugfs.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> @@ -679,6 +680,33 @@ static int ehci_run (struct usb_hcd *hcd)
>  	return 0;
>  }
>  
> +static bool ehci_has_companion(struct ehci_hcd *ehci)
> +{
> +	struct device *dev = ehci_to_hcd(ehci)->self.controller;
> +
> +	if (HCS_N_CC(ehci->hcs_params) == 0)
> +		return false;
> +
> +	if (!IS_ENABLED(CONFIG_USB_OHCI_HCD) &&
> +	    !IS_ENABLED(CONFIG_USB_UHCI_HCD))
> +		return false;
> +
> +	if (dev && dev->of_node) {
> +		struct device_node *node;
> +
> +		node = of_parse_phandle(dev->of_node, "companion", 0);
> +		if (node) {
> +			bool avail = of_device_is_available(node);
> +
> +			of_node_put(node);
> +			if (!avail)
> +				return false;
> +		}
> +	}
> +
> +	return true;
> +}
> +
>  int ehci_setup(struct usb_hcd *hcd)
>  {
>  	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> @@ -691,6 +719,9 @@ int ehci_setup(struct usb_hcd *hcd)
>  
>  	/* cache this readonly data; minimize chip reads */
>  	ehci->hcs_params = ehci_readl(ehci, &ehci->caps->hcs_params);
> +	ehci->no_companion = !ehci_has_companion(ehci);
> +	if (ehci->no_companion)
> +		ehci_dbg(ehci, "no companion controller, disabling port handoff\n");
>  
>  	ehci->sbrn = HCD_USB2;
>  
> diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> index e676c98b6baa..1f69d9fc93c7 100644
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -618,18 +618,17 @@ static int check_reset_complete (
>  	/* if reset finished and it's still not enabled -- handoff */
>  	if (!(port_status & PORT_PE)) {
>  
> -		/* with integrated TT, there's nobody to hand it to! */
> -		if (ehci_is_TDI(ehci)) {
> -			ehci_dbg (ehci,
> -				"Failed to enable port %d on root hub TT\n",
> -				index+1);
> +		/* with integrated TT or no companion, there's nobody to hand it to! */
> +		if (ehci_is_TDI(ehci) || ehci->no_companion) {
> +			ehci_dbg(ehci, "Failed to enable port %d%s\n",
> +				 index + 1,
> +				 ehci_is_TDI(ehci) ? " on root hub TT" : "");
>  			return port_status;
>  		}
>  
>  		ehci_dbg (ehci, "port %d full speed --> companion\n",
>  			index + 1);
>  
> -		// what happens if HCS_N_CC(params) == 0 ?
>  		port_status |= PORT_OWNER;
>  		port_status &= ~PORT_RWC_BITS;
>  		ehci_writel(ehci, port_status, status_reg);
> @@ -1145,9 +1144,9 @@ int ehci_hub_control(
>  			 * which can be fine if this root hub has a
>  			 * transaction translator built in.
>  			 */
> -			if ((temp & (PORT_PE|PORT_CONNECT)) == PORT_CONNECT
> -					&& !ehci_is_TDI(ehci)
> -					&& PORT_USB11 (temp)) {
> +			if ((temp & (PORT_PE | PORT_CONNECT)) == PORT_CONNECT &&
> +			    !ehci_is_TDI(ehci) && !ehci->no_companion &&
> +			    PORT_USB11(temp)) {
>  				ehci_dbg (ehci,
>  					"port %d low speed --> companion\n",
>  					wIndex + 1);
> @@ -1244,7 +1243,7 @@ static void ehci_relinquish_port(struct usb_hcd *hcd, int portnum)
>  {
>  	struct ehci_hcd		*ehci = hcd_to_ehci(hcd);
>  
> -	if (ehci_is_TDI(ehci))
> +	if (ehci_is_TDI(ehci) || ehci->no_companion)
>  		return;
>  	set_owner(ehci, --portnum, PORT_OWNER);
>  }
> @@ -1254,7 +1253,7 @@ static int ehci_port_handed_over(struct usb_hcd *hcd, int portnum)
>  	struct ehci_hcd		*ehci = hcd_to_ehci(hcd);
>  	u32 __iomem		*reg;
>  
> -	if (ehci_is_TDI(ehci))
> +	if (ehci_is_TDI(ehci) || ehci->no_companion)
>  		return 0;
>  	reg = ehci_portsc(ehci, portnum - 1);
>  	return ehci_readl(ehci, reg) & PORT_OWNER;
> diff --git a/drivers/usb/host/ehci-sysfs.c b/drivers/usb/host/ehci-sysfs.c
> index 5e6b545c30e6..1dcca19b544b 100644
> --- a/drivers/usb/host/ehci-sysfs.c
> +++ b/drivers/usb/host/ehci-sysfs.c
> @@ -145,8 +145,8 @@ static inline int create_sysfs_files(struct ehci_hcd *ehci)
>  	struct device	*controller = ehci_to_hcd(ehci)->self.controller;
>  	int	i = 0;
>  
> -	/* with integrated TT there is no companion! */
> -	if (!ehci_is_TDI(ehci))
> +	/* with integrated TT or no_companion there is no companion! */
> +	if (!ehci_is_TDI(ehci) && !ehci->no_companion)
>  		i = device_create_file(controller, &dev_attr_companion);
>  	if (i)
>  		goto out;
> @@ -160,8 +160,8 @@ static inline void remove_sysfs_files(struct ehci_hcd *ehci)
>  {
>  	struct device	*controller = ehci_to_hcd(ehci)->self.controller;
>  
> -	/* with integrated TT there is no companion! */
> -	if (!ehci_is_TDI(ehci))
> +	/* with integrated TT or no_companion there is no companion! */
> +	if (!ehci_is_TDI(ehci) && !ehci->no_companion)
>  		device_remove_file(controller, &dev_attr_companion);
>  
>  	device_remove_file(controller, &dev_attr_uframe_periodic_max);
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index 855a9d08b019..8ab5c696717a 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -238,6 +238,7 @@ struct ehci_hcd {			/* one per controller */
>  	unsigned		no_configured_flag:1;
>  	unsigned		no_tdi_mode:1;
>  	unsigned		no_fsls_isoc:1;
> +	unsigned		no_companion:1;
>  
>  	/* required for usb32 quirk */
>  	#define OHCI_CTRL_HCFS          (3 << 6)
> -- 
> 2.56.0.611.g1030f0b940-goog
>