On Fri, Jul 03, 2026 at 07:03:09PM +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.

Also rewrite the existing set_bit() and clear_bit() branches with
assign_bit() to make it cleaner.

...

+static int usb_hub_set_port_pwrseq(struct usb_port *port, bool set)

Not sure 'set' in the name is a good choice as you have also parameter 'set',
can be confusing. So for the other one. I don't know enough about pwrseq to
suggest better naming of these parts, but I would like to see some consistency
and less oddity.

+{
+	int ret = 0;

Unneeded assignment.

+	if (set)
+		ret = pwrseq_power_on(port->pwrseq);
+	else
+		ret = pwrseq_power_off(port->pwrseq);
+
+	return ret;

Moreover, for now this can be written as

	if (set)
		return pwrseq_power_on(port->pwrseq);

	return pwrseq_power_off(port->pwrseq);

+}
+
+static int usb_hub_restore_port_pwrseq(struct usb_port *port, bool set)
+{
+	int ret = 0;

Ditto.

+	if (set)
+		ret = pwrseq_power_off(port->pwrseq);
+	else
+		ret = pwrseq_power_on(port->pwrseq);
+
+	return ret;
+}

...

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

		retval = dev_err_probe(&port_dev->dev, PTR_ERR(port_dev->pwrseq),
				       "failed to get power sequencing descriptor\n");

+		goto err_put_kn;
+	}

-- 
With Best Regards,
Andy Shevchenko