Re: [PATCH v5 10/16] usb: hub: Power on connected M.2 E-key connectors with power sequencing API

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Wed, Jul 15, 2026 at 04:53:40PM +0800, Chen-Yu Tsai wrote:
> The new M.2 E-key connector can have a USB connection. For the USB device
> on this connector to work, its power must be enabled and the W_DISABLE2#
> signal deasserted. The connector driver handles this and provides a
> toggle over the power sequencing API.
> 
> This feature currently only supports a directly connected (no mux in
> between) M.2 E-key connector. Existing USB connector types are not
> covered. The USB A connector was recently added to the onboard devices
> driver. USB B connectors have historically been managed by the USB
> gadget or dual-role device controller drivers. USB C connectors are
> handled by TCPM drivers.
> 
> The power sequencing API does not know whether a power sequence provider
> is not needed or not available yet, so we only request it for connectors
> that we know need it, which at this time is just the E-key connector.
> 
> On the USB side, the port firmware node (if present) is tied to the
> usb_port device. This device is used to acquire the power sequencing
> descriptor. This allows the provider to tell the different ports on one
> hub apart.
> 
> This feature is not implemented in the onboard USB devices driver. The
> power sequencing API expects the consumer device to make the request,
> but there is no device node to instantiate a platform device to tie
> the driver to. The connector is not a child node of the USB host or
> hub, and the graph connection is from a USB port to the connector.
> And the connector itself already has a driver.
> 
> Power sequencing is not directly enabled in the connector driver as
> that would completely decouple the timing of it from the USB subsystem.
> It would not be possible for the USB subsystem to toggle the power
> for a power cycle or to disable the port.

...

> static void usb_port_device_release(struct device *dev)

>  	fwnode_handle_put(dev_fwnode(dev));
> +	/* usb_hub_create_port_device() could leave an error value */
> +	if (!IS_ERR(port_dev->pwrseq))
> +		pwrseq_put(port_dev->pwrseq);

Hmm... I would rather make pwrseq_put() NULL and error pointer-aware, so
it will be no-op in such cases. But I think Bart has his own opinion about
this.

>  	kfree(port_dev->req);
>  	kfree(port_dev);
>  }

>  		put_device(&port_dev->dev);
> +		hub->ports[port1 - 1] = NULL;
>  		return retval;
>  	}

...

> int usb_hub_create_port_device(struct usb_hub *hub, int port1)

> +	port_dev->pwrseq = usb_hub_port_pwrseq_get(port_dev);
> +	if (IS_ERR(port_dev->pwrseq)) {
> +		retval = dev_err_probe(&port_dev->dev, PTR_ERR(port_dev->pwrseq),
> +				       "failed to get power sequencing descriptor\n");
> +		goto err_put_kn;

OTOH, how pwrseq can be non-NULL and at the same time port be created?

> +	}

-- 
With Best Regards,
Andy Shevchenko