* [PATCH v3 01/13] device property: Add fwnode_graph_get_port_by_id()
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 11:03 ` [PATCH v3 02/13] device property: Add fwnode_graph_get_next_port_endpoint() Chen-Yu Tsai
` (12 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Bartosz Golaszewski
In some cases the driver needs a reference to the port firmware node.
Once such case is the upcoming USB power sequencing integration. The
USB hub port is tied to the corresponding port firmware node if it
exists.
Provide a helper for this.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Moved "Return:" kernel-doc section to the end. (Andy)
Changes since v1:
- New patch
---
drivers/base/property.c | 22 ++++++++++++++++++++++
include/linux/property.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 9387bb83eb54..3e3e19ef66a9 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1346,6 +1346,28 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
}
EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
+/**
+ * fwnode_graph_get_port_by_id - get the port matching a given id
+ * @fwnode: parent fwnode_handle containing the graph
+ * @id: id of the port
+ *
+ * The caller is responsible for calling fwnode_handle_put() on the returned
+ * fwnode pointer.
+ *
+ * Return: A 'port' firmware node pointer with refcount incremented.
+ */
+struct fwnode_handle *fwnode_graph_get_port_by_id(struct fwnode_handle *fwnode, u32 id)
+{
+ struct fwnode_handle *ep;
+
+ ep = fwnode_graph_get_endpoint_by_id(fwnode, id, 0, FWNODE_GRAPH_ENDPOINT_NEXT);
+ if (!ep)
+ return NULL;
+
+ return fwnode_get_next_parent(ep);
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_get_port_by_id);
+
const void *device_get_match_data(const struct device *dev)
{
return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
diff --git a/include/linux/property.h b/include/linux/property.h
index 14c304db4664..e04901c0bd8f 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -505,6 +505,7 @@ int fwnode_get_phy_mode(const struct fwnode_handle *fwnode);
void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index);
+struct fwnode_handle *fwnode_graph_get_port_by_id(struct fwnode_handle *fwnode, u32 id);
struct fwnode_handle *fwnode_graph_get_next_endpoint(
const struct fwnode_handle *fwnode, struct fwnode_handle *prev);
struct fwnode_handle *
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 02/13] device property: Add fwnode_graph_get_next_port_endpoint()
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
2026-07-03 11:03 ` [PATCH v3 01/13] device property: Add fwnode_graph_get_port_by_id() Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 12:04 ` Bartosz Golaszewski
2026-07-03 12:46 ` Andy Shevchenko
2026-07-03 11:03 ` [PATCH v3 03/13] power: sequencing: Add pwrseq_power_is_on() Chen-Yu Tsai
` (11 subsequent siblings)
13 siblings, 2 replies; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern
Due to design constraints of the power sequencing API, the consumer
must first be sure that the other side is actually a provider, or it
will continually get -EPROBE_DEFER when requesting the power
sequencing descriptor.
In the upcoming USB power sequencing integration, the USB hub driver
first needs to check whether a graph connection exists, and whether
the other side of the connection is a supported connector type. The
USB port is tied to a "port" firmware node, and this new helper will
be used to get the endpoint under the known "port" firmware node.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Dropped unused |ep| variable
- Rewrote as do {} while()
- Dropped WARN() use
---
drivers/base/property.c | 25 +++++++++++++++++++++++++
include/linux/property.h | 2 ++
2 files changed, 27 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 3e3e19ef66a9..7e7ad3635806 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1099,6 +1099,31 @@ int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name)
}
EXPORT_SYMBOL(fwnode_irq_get_byname);
+/**
+ * fwnode_graph_get_next_port_endpoint - Get next endpoint firmware node in port
+ * @port: Pointer to the target port firmware node
+ * @prev: Previous endpoint node or %NULL to get the first
+ *
+ * The caller is responsible for calling fwnode_handle_put() on the returned
+ * fwnode pointer. Note that this function also puts a reference to @prev
+ * unconditionally.
+ *
+ * Return: an endpoint firmware node pointer or %NULL if no more endpoints
+ * are available.
+ */
+struct fwnode_handle *fwnode_graph_get_next_port_endpoint(const struct fwnode_handle *port,
+ struct fwnode_handle *prev)
+{
+ do {
+ prev = fwnode_get_next_child_node(port, prev);
+ if (fwnode_name_eq(prev, "endpoint"))
+ break;
+ } while (prev);
+
+ return prev;
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_get_next_port_endpoint);
+
/**
* fwnode_graph_get_next_endpoint - Get next endpoint firmware node
* @fwnode: Pointer to the parent firmware node
diff --git a/include/linux/property.h b/include/linux/property.h
index e04901c0bd8f..931e703393cb 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -506,6 +506,8 @@ int fwnode_get_phy_mode(const struct fwnode_handle *fwnode);
void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index);
struct fwnode_handle *fwnode_graph_get_port_by_id(struct fwnode_handle *fwnode, u32 id);
+struct fwnode_handle *fwnode_graph_get_next_port_endpoint(
+ const struct fwnode_handle *port, struct fwnode_handle *prev);
struct fwnode_handle *fwnode_graph_get_next_endpoint(
const struct fwnode_handle *fwnode, struct fwnode_handle *prev);
struct fwnode_handle *
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 02/13] device property: Add fwnode_graph_get_next_port_endpoint()
2026-07-03 11:03 ` [PATCH v3 02/13] device property: Add fwnode_graph_get_next_port_endpoint() Chen-Yu Tsai
@ 2026-07-03 12:04 ` Bartosz Golaszewski
2026-07-03 12:46 ` Andy Shevchenko
1 sibling, 0 replies; 35+ messages in thread
From: Bartosz Golaszewski @ 2026-07-03 12:04 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Bartosz Golaszewski,
Greg Kroah-Hartman, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
On Fri, 3 Jul 2026 13:03:03 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> Due to design constraints of the power sequencing API, the consumer
> must first be sure that the other side is actually a provider, or it
> will continually get -EPROBE_DEFER when requesting the power
> sequencing descriptor.
>
> In the upcoming USB power sequencing integration, the USB hub driver
> first needs to check whether a graph connection exists, and whether
> the other side of the connection is a supported connector type. The
> USB port is tied to a "port" firmware node, and this new helper will
> be used to get the endpoint under the known "port" firmware node.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> Changes since v2:
> - Dropped unused |ep| variable
> - Rewrote as do {} while()
> - Dropped WARN() use
> ---
> drivers/base/property.c | 25 +++++++++++++++++++++++++
> include/linux/property.h | 2 ++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 3e3e19ef66a9..7e7ad3635806 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1099,6 +1099,31 @@ int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name)
> }
> EXPORT_SYMBOL(fwnode_irq_get_byname);
>
> +/**
> + * fwnode_graph_get_next_port_endpoint - Get next endpoint firmware node in port
> + * @port: Pointer to the target port firmware node
> + * @prev: Previous endpoint node or %NULL to get the first
> + *
> + * The caller is responsible for calling fwnode_handle_put() on the returned
> + * fwnode pointer. Note that this function also puts a reference to @prev
> + * unconditionally.
> + *
> + * Return: an endpoint firmware node pointer or %NULL if no more endpoints
I think the correct keyword is Returns: followed by a newline.
> + * are available.
> + */
> +struct fwnode_handle *fwnode_graph_get_next_port_endpoint(const struct fwnode_handle *port,
> + struct fwnode_handle *prev)
> +{
> + do {
> + prev = fwnode_get_next_child_node(port, prev);
> + if (fwnode_name_eq(prev, "endpoint"))
> + break;
> + } while (prev);
> +
> + return prev;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_graph_get_next_port_endpoint);
> +
> /**
> * fwnode_graph_get_next_endpoint - Get next endpoint firmware node
> * @fwnode: Pointer to the parent firmware node
> diff --git a/include/linux/property.h b/include/linux/property.h
> index e04901c0bd8f..931e703393cb 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -506,6 +506,8 @@ int fwnode_get_phy_mode(const struct fwnode_handle *fwnode);
> void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index);
>
> struct fwnode_handle *fwnode_graph_get_port_by_id(struct fwnode_handle *fwnode, u32 id);
> +struct fwnode_handle *fwnode_graph_get_next_port_endpoint(
> + const struct fwnode_handle *port, struct fwnode_handle *prev);
> struct fwnode_handle *fwnode_graph_get_next_endpoint(
> const struct fwnode_handle *fwnode, struct fwnode_handle *prev);
> struct fwnode_handle *
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
>
>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v3 02/13] device property: Add fwnode_graph_get_next_port_endpoint()
2026-07-03 11:03 ` [PATCH v3 02/13] device property: Add fwnode_graph_get_next_port_endpoint() Chen-Yu Tsai
2026-07-03 12:04 ` Bartosz Golaszewski
@ 2026-07-03 12:46 ` Andy Shevchenko
1 sibling, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2026-07-03 12:46 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, linux-acpi,
driver-core, linux-pm, linux-usb, devicetree, linux-mediatek,
linux-arm-kernel, linux-kernel, Manivannan Sadhasivam, Alan Stern
On Fri, Jul 03, 2026 at 07:03:03PM +0800, Chen-Yu Tsai wrote:
> Due to design constraints of the power sequencing API, the consumer
> must first be sure that the other side is actually a provider, or it
> will continually get -EPROBE_DEFER when requesting the power
> sequencing descriptor.
>
> In the upcoming USB power sequencing integration, the USB hub driver
> first needs to check whether a graph connection exists, and whether
> the other side of the connection is a supported connector type. The
> USB port is tied to a "port" firmware node, and this new helper will
> be used to get the endpoint under the known "port" firmware node.
This version is good.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 03/13] power: sequencing: Add pwrseq_power_is_on()
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
2026-07-03 11:03 ` [PATCH v3 01/13] device property: Add fwnode_graph_get_port_by_id() Chen-Yu Tsai
2026-07-03 11:03 ` [PATCH v3 02/13] device property: Add fwnode_graph_get_next_port_endpoint() Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 12:07 ` Bartosz Golaszewski
2026-07-03 11:03 ` [PATCH v3 04/13] usb: hub: Return actual error from hub_configure() in hub_probe() Chen-Yu Tsai
` (10 subsequent siblings)
13 siblings, 1 reply; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern
The power sequencing consumer API already does power on state tracking
internally. Expose the state to consumers through pwrseq_power_is_on()
so that they don't have to reimplement it locally.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- New patch
Needs to go in with "usb: hub: Power on connected M.2 E-key connectors"
as it is a build time dependency.
---
drivers/power/sequencing/core.c | 18 ++++++++++++++++++
include/linux/pwrseq/consumer.h | 1 +
2 files changed, 19 insertions(+)
diff --git a/drivers/power/sequencing/core.c b/drivers/power/sequencing/core.c
index 02f42da91598..72b96d36920e 100644
--- a/drivers/power/sequencing/core.c
+++ b/drivers/power/sequencing/core.c
@@ -968,6 +968,24 @@ int pwrseq_power_off(struct pwrseq_desc *desc)
}
EXPORT_SYMBOL_GPL(pwrseq_power_off);
+/**
+ * pwrseq_power_is_on() - Queries the last requested state of the power sequencer.
+ * @desc: Descriptor referencing the power sequencer.
+ *
+ * This returns the last requested state of the power sequencer.
+ *
+ * Returns:
+ * On success, 1 for on and 0 for off; negative error number on failure.
+ */
+int pwrseq_power_is_on(struct pwrseq_desc *desc)
+{
+ if (!desc)
+ return -EINVAL;
+
+ return desc->powered_on;
+}
+EXPORT_SYMBOL_GPL(pwrseq_power_is_on);
+
/**
* pwrseq_to_device() - Get the pwrseq device pointer from a descriptor.
* @desc: Descriptor referencing the power sequencer.
diff --git a/include/linux/pwrseq/consumer.h b/include/linux/pwrseq/consumer.h
index 3c907c9e1885..5a5eaf85d5db 100644
--- a/include/linux/pwrseq/consumer.h
+++ b/include/linux/pwrseq/consumer.h
@@ -22,6 +22,7 @@ devm_pwrseq_get(struct device *dev, const char *target);
int pwrseq_power_on(struct pwrseq_desc *desc);
int pwrseq_power_off(struct pwrseq_desc *desc);
+int pwrseq_power_is_on(struct pwrseq_desc *desc);
struct device *pwrseq_to_device(struct pwrseq_desc *desc);
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 03/13] power: sequencing: Add pwrseq_power_is_on()
2026-07-03 11:03 ` [PATCH v3 03/13] power: sequencing: Add pwrseq_power_is_on() Chen-Yu Tsai
@ 2026-07-03 12:07 ` Bartosz Golaszewski
2026-07-03 12:17 ` Chen-Yu Tsai
0 siblings, 1 reply; 35+ messages in thread
From: Bartosz Golaszewski @ 2026-07-03 12:07 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Bartosz Golaszewski,
Greg Kroah-Hartman, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
On Fri, 3 Jul 2026 13:03:04 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> The power sequencing consumer API already does power on state tracking
> internally. Expose the state to consumers through pwrseq_power_is_on()
> so that they don't have to reimplement it locally.
>
They wouldn't be able to do it as the field is private to pwrseq core anyway.
In what situation would consumers need this? Typically you know what state the
handle is in if you control it.
Bart
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 03/13] power: sequencing: Add pwrseq_power_is_on()
2026-07-03 12:07 ` Bartosz Golaszewski
@ 2026-07-03 12:17 ` Chen-Yu Tsai
2026-07-03 13:18 ` Bartosz Golaszewski
0 siblings, 1 reply; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 12:17 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Greg Kroah-Hartman,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Rafael J. Wysocki, Danilo Krummrich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno
On Fri, Jul 3, 2026 at 8:07 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Fri, 3 Jul 2026 13:03:04 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> > The power sequencing consumer API already does power on state tracking
> > internally. Expose the state to consumers through pwrseq_power_is_on()
> > so that they don't have to reimplement it locally.
> >
>
> They wouldn't be able to do it as the field is private to pwrseq core anyway.
Which is why a helper is added? Note this is returning the requested state
on the consumer side, not the actual state.
> In what situation would consumers need this? Typically you know what state the
> handle is in if you control it.
Mostly just to avoid extra tracking in the USB port device. For the hub's
port power feature flag, the hub driver actually queries the hub for the
current state; no in-kernel tracking is involved, i.e. the consumer doesn't
actually know what state it is currently in.
ChenYu
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 03/13] power: sequencing: Add pwrseq_power_is_on()
2026-07-03 12:17 ` Chen-Yu Tsai
@ 2026-07-03 13:18 ` Bartosz Golaszewski
0 siblings, 0 replies; 35+ messages in thread
From: Bartosz Golaszewski @ 2026-07-03 13:18 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Greg Kroah-Hartman,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Rafael J. Wysocki, Danilo Krummrich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Bartosz Golaszewski
On Fri, 3 Jul 2026 14:17:19 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> On Fri, Jul 3, 2026 at 8:07 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>>
>> On Fri, 3 Jul 2026 13:03:04 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
>> > The power sequencing consumer API already does power on state tracking
>> > internally. Expose the state to consumers through pwrseq_power_is_on()
>> > so that they don't have to reimplement it locally.
>> >
>>
>> They wouldn't be able to do it as the field is private to pwrseq core anyway.
>
> Which is why a helper is added? Note this is returning the requested state
> on the consumer side, not the actual state.
>
>> In what situation would consumers need this? Typically you know what state the
>> handle is in if you control it.
>
> Mostly just to avoid extra tracking in the USB port device. For the hub's
> port power feature flag, the hub driver actually queries the hub for the
> current state; no in-kernel tracking is involved, i.e. the consumer doesn't
> actually know what state it is currently in.
>
Fair enough, thanks.
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 04/13] usb: hub: Return actual error from hub_configure() in hub_probe()
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (2 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 03/13] power: sequencing: Add pwrseq_power_is_on() Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 11:03 ` [PATCH v3 05/13] usb: hub: Associate port@ fwnode with USB port device Chen-Yu Tsai
` (9 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Bartosz Golaszewski
The addition of power sequencing descriptor handling in the USB hub code
requires dealing with deferred probing from pwrseq_get(). The power
sequencing provider may not yet be available when the USB hub probes.
Return the actual error code from hub_configure() when it fails, so that
the driver core can notice the deferred probe request.
Also rewrite this section into the standard error handling pattern:
if (error) {
# handle error
return error;
}
# do more work
return 0;
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Rewrite into standard error handling pattern
Changes since v1:
- Moved "int ret" declaration in hub_configure() over here from the next
patch
---
drivers/usb/core/hub.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 13264e86bc6d..fe10d72ef39d 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1874,6 +1874,7 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
struct usb_host_interface *desc;
struct usb_device *hdev;
struct usb_hub *hub;
+ int ret;
desc = intf->cur_altsetting;
hdev = interface_to_usbdev(intf);
@@ -2005,14 +2006,15 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
usb_set_interface(hdev, 0, 0);
}
- if (hub_configure(hub, &desc->endpoint[0].desc) >= 0) {
- onboard_dev_create_pdevs(hdev, &hub->onboard_devs);
-
- return 0;
+ ret = hub_configure(hub, &desc->endpoint[0].desc);
+ if (ret < 0) {
+ hub_disconnect(intf);
+ return ret;
}
- hub_disconnect(intf);
- return -ENODEV;
+ onboard_dev_create_pdevs(hdev, &hub->onboard_devs);
+
+ return 0;
}
static int
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 05/13] usb: hub: Associate port@ fwnode with USB port device
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (3 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 04/13] usb: hub: Return actual error from hub_configure() in hub_probe() Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 12:09 ` Bartosz Golaszewski
2026-07-03 13:01 ` Andy Shevchenko
2026-07-03 11:03 ` [PATCH v3 06/13] usb: hub: Pass |struct usb_port*| to usb_port_is_power_on() Chen-Yu Tsai
` (8 subsequent siblings)
13 siblings, 2 replies; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern
When a USB hub port is connected to a connector in a firmware node
graph, the port itself has a node in the graph.
Associate the port's firmware node with the USB port's device,
usb_port::dev. This is used in later changes for the M.2 slot power
sequencing provider to match against the requesting port.
To avoid potential conflicts with ACPI firmware nodes and then causing
power management issues, only assign the firmware node if the hub's
firmware node is not an ACPI firmware node.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Skip assignment if hub firmware node is ACPI node
drivers/usb/core/port.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index b1364f0c384c..1088776ef750 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -7,6 +7,7 @@
* Author: Lan Tianyu <tianyu.lan@intel.com>
*/
+#include <linux/acpi.h>
#include <linux/kstrtox.h>
#include <linux/slab.h>
#include <linux/string_choices.h>
@@ -780,6 +781,13 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
port_dev->dev.driver = &usb_port_driver;
dev_set_name(&port_dev->dev, "%s-port%d", dev_name(&hub->hdev->dev),
port1);
+ /*
+ * ACPI FW nodes are associated later when device_register() happens.
+ * Skip assigning one here to avoid potential conflicts.
+ */
+ if (!is_acpi_node(dev_fwnode(&hdev->dev)))
+ device_set_node(&port_dev->dev,
+ fwnode_graph_get_port_by_id(dev_fwnode(&hdev->dev), port1));
mutex_init(&port_dev->status_lock);
retval = device_register(&port_dev->dev);
if (retval) {
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 05/13] usb: hub: Associate port@ fwnode with USB port device
2026-07-03 11:03 ` [PATCH v3 05/13] usb: hub: Associate port@ fwnode with USB port device Chen-Yu Tsai
@ 2026-07-03 12:09 ` Bartosz Golaszewski
2026-07-03 13:01 ` Andy Shevchenko
1 sibling, 0 replies; 35+ messages in thread
From: Bartosz Golaszewski @ 2026-07-03 12:09 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Bartosz Golaszewski,
Greg Kroah-Hartman, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
On Fri, 3 Jul 2026 13:03:06 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> When a USB hub port is connected to a connector in a firmware node
> graph, the port itself has a node in the graph.
>
> Associate the port's firmware node with the USB port's device,
> usb_port::dev. This is used in later changes for the M.2 slot power
> sequencing provider to match against the requesting port.
>
> To avoid potential conflicts with ACPI firmware nodes and then causing
> power management issues, only assign the firmware node if the hub's
> firmware node is not an ACPI firmware node.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 05/13] usb: hub: Associate port@ fwnode with USB port device
2026-07-03 11:03 ` [PATCH v3 05/13] usb: hub: Associate port@ fwnode with USB port device Chen-Yu Tsai
2026-07-03 12:09 ` Bartosz Golaszewski
@ 2026-07-03 13:01 ` Andy Shevchenko
2026-07-03 13:25 ` Chen-Yu Tsai
1 sibling, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2026-07-03 13:01 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, linux-acpi,
driver-core, linux-pm, linux-usb, devicetree, linux-mediatek,
linux-arm-kernel, linux-kernel, Manivannan Sadhasivam, Alan Stern
On Fri, Jul 03, 2026 at 07:03:06PM +0800, Chen-Yu Tsai wrote:
> When a USB hub port is connected to a connector in a firmware node
> graph, the port itself has a node in the graph.
>
> Associate the port's firmware node with the USB port's device,
> usb_port::dev. This is used in later changes for the M.2 slot power
> sequencing provider to match against the requesting port.
>
> To avoid potential conflicts with ACPI firmware nodes and then causing
> power management issues, only assign the firmware node if the hub's
> firmware node is not an ACPI firmware node.
Now I'm more confident that it does not mess up with ACPI case.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 05/13] usb: hub: Associate port@ fwnode with USB port device
2026-07-03 13:01 ` Andy Shevchenko
@ 2026-07-03 13:25 ` Chen-Yu Tsai
2026-07-03 13:34 ` Andy Shevchenko
0 siblings, 1 reply; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 13:25 UTC (permalink / raw)
To: Andy Shevchenko, Bartosz Golaszewski
Cc: Greg Kroah-Hartman, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Rafael J. Wysocki, Danilo Krummrich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, linux-acpi, driver-core, linux-pm,
linux-usb, devicetree, linux-mediatek, linux-arm-kernel,
linux-kernel, Manivannan Sadhasivam, Alan Stern
On Fri, Jul 3, 2026 at 9:02 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Jul 03, 2026 at 07:03:06PM +0800, Chen-Yu Tsai wrote:
> > When a USB hub port is connected to a connector in a firmware node
> > graph, the port itself has a node in the graph.
> >
> > Associate the port's firmware node with the USB port's device,
> > usb_port::dev. This is used in later changes for the M.2 slot power
> > sequencing provider to match against the requesting port.
> >
> > To avoid potential conflicts with ACPI firmware nodes and then causing
> > power management issues, only assign the firmware node if the hub's
> > firmware node is not an ACPI firmware node.
>
> Now I'm more confident that it does not mess up with ACPI case.
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thank you and Bartosz for the reviews.
FTR Sashiko pointed out that this is likely leaking a fwnode reference.
I will add a fwnode_handle_put() call to usb_hub_remove_port_device()
in the next version.
ChenYu
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 05/13] usb: hub: Associate port@ fwnode with USB port device
2026-07-03 13:25 ` Chen-Yu Tsai
@ 2026-07-03 13:34 ` Andy Shevchenko
0 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2026-07-03 13:34 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, linux-acpi,
driver-core, linux-pm, linux-usb, devicetree, linux-mediatek,
linux-arm-kernel, linux-kernel, Manivannan Sadhasivam, Alan Stern
On Fri, Jul 03, 2026 at 09:25:56PM +0800, Chen-Yu Tsai wrote:
> On Fri, Jul 3, 2026 at 9:02 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Fri, Jul 03, 2026 at 07:03:06PM +0800, Chen-Yu Tsai wrote:
> > > When a USB hub port is connected to a connector in a firmware node
> > > graph, the port itself has a node in the graph.
> > >
> > > Associate the port's firmware node with the USB port's device,
> > > usb_port::dev. This is used in later changes for the M.2 slot power
> > > sequencing provider to match against the requesting port.
> > >
> > > To avoid potential conflicts with ACPI firmware nodes and then causing
> > > power management issues, only assign the firmware node if the hub's
> > > firmware node is not an ACPI firmware node.
> >
> > Now I'm more confident that it does not mess up with ACPI case.
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Thank you and Bartosz for the reviews.
>
> FTR Sashiko pointed out that this is likely leaking a fwnode reference.
> I will add a fwnode_handle_put() call to usb_hub_remove_port_device()
> in the next version.
Ah, indeed. The _get call has to have its _put counterpart.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 06/13] usb: hub: Pass |struct usb_port*| to usb_port_is_power_on()
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (4 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 05/13] usb: hub: Associate port@ fwnode with USB port device Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 13:11 ` Andy Shevchenko
2026-07-03 11:03 ` [PATCH v3 07/13] usb: hub: Use usb_hub_set_port_power() to control port power everywhere Chen-Yu Tsai
` (7 subsequent siblings)
13 siblings, 1 reply; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Bartosz Golaszewski
usb_port_is_power_on() currently takes |struct usb_hub*|, but only needs
it to tell if the hub/port is SuperSpeed or not.
In a subsequent change, usb_port_is_power_on() needs access to a pwrseq
state tracking field in |struct usb_port|. Either structure can be used
to identify whether a port/hub is SuperSpeed or not, as the field in
|struct usb_port| is inherited from the hub:
port->is_superspeed = hub_is_superspeed(hub)
Replace usb_port_is_power_on()'s |struct usb_hub*| parameter with
|struct usb_port*| so a subsequent change can use it.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
drivers/usb/core/hub.c | 11 ++++++-----
drivers/usb/core/hub.h | 2 +-
drivers/usb/core/port.c | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index fe10d72ef39d..da0a4cc8e15a 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3240,11 +3240,11 @@ static bool hub_port_stop_enumerate(struct usb_hub *hub, int port1, int retries)
}
/* Check if a port is power on */
-int usb_port_is_power_on(struct usb_hub *hub, unsigned int portstatus)
+int usb_port_is_power_on(struct usb_port *port, unsigned int portstatus)
{
int ret = 0;
- if (hub_is_superspeed(hub->hdev)) {
+ if (port->is_superspeed) {
if (portstatus & USB_SS_PORT_STAT_POWER)
ret = 1;
} else {
@@ -3306,7 +3306,7 @@ static int check_port_resume_type(struct usb_device *udev,
}
/* Is the device still present? */
else if (status || port_is_suspended(hub, portstatus) ||
- !usb_port_is_power_on(hub, portstatus)) {
+ !usb_port_is_power_on(port_dev, portstatus)) {
if (status >= 0)
status = -ENODEV;
} else if (!(portstatus & USB_PORT_STAT_CONNECTION)) {
@@ -3748,12 +3748,13 @@ static int wait_for_connected(struct usb_device *udev,
struct usb_hub *hub, int port1,
u16 *portchange, u16 *portstatus)
{
+ struct usb_port *port_dev = hub->ports[port1 - 1];
int status = 0, delay_ms = 0;
while (delay_ms < 2000) {
if (status || *portstatus & USB_PORT_STAT_CONNECTION)
break;
- if (!usb_port_is_power_on(hub, *portstatus)) {
+ if (!usb_port_is_power_on(port_dev, *portstatus)) {
status = -ENODEV;
break;
}
@@ -5449,7 +5450,7 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
* but only if the port isn't owned by someone else.
*/
if (hub_is_port_power_switchable(hub)
- && !usb_port_is_power_on(hub, portstatus)
+ && !usb_port_is_power_on(port_dev, portstatus)
&& !port_dev->port_owner)
set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 9ebc5ef54a32..b65d9192379d 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -138,7 +138,7 @@ extern int usb_clear_port_feature(struct usb_device *hdev,
int port1, int feature);
extern int usb_hub_port_status(struct usb_hub *hub, int port1,
u16 *status, u16 *change);
-extern int usb_port_is_power_on(struct usb_hub *hub, unsigned int portstatus);
+extern int usb_port_is_power_on(struct usb_port *port, unsigned int portstatus);
static inline bool hub_is_port_power_switchable(struct usb_hub *hub)
{
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 1088776ef750..77dbf51f1760 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -98,7 +98,7 @@ static ssize_t disable_show(struct device *dev,
}
usb_hub_port_status(hub, port1, &portstatus, &unused);
- disabled = !usb_port_is_power_on(hub, portstatus);
+ disabled = !usb_port_is_power_on(port_dev, portstatus);
out_hdev_lock:
usb_unlock_device(hdev);
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 06/13] usb: hub: Pass |struct usb_port*| to usb_port_is_power_on()
2026-07-03 11:03 ` [PATCH v3 06/13] usb: hub: Pass |struct usb_port*| to usb_port_is_power_on() Chen-Yu Tsai
@ 2026-07-03 13:11 ` Andy Shevchenko
2026-07-03 13:17 ` Chen-Yu Tsai
0 siblings, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2026-07-03 13:11 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, linux-acpi,
driver-core, linux-pm, linux-usb, devicetree, linux-mediatek,
linux-arm-kernel, linux-kernel, Manivannan Sadhasivam, Alan Stern,
Bartosz Golaszewski
On Fri, Jul 03, 2026 at 07:03:07PM +0800, Chen-Yu Tsai wrote:
> usb_port_is_power_on() currently takes |struct usb_hub*|, but only needs
> it to tell if the hub/port is SuperSpeed or not.
>
> In a subsequent change, usb_port_is_power_on() needs access to a pwrseq
> state tracking field in |struct usb_port|. Either structure can be used
> to identify whether a port/hub is SuperSpeed or not, as the field in
> |struct usb_port| is inherited from the hub:
>
> port->is_superspeed = hub_is_superspeed(hub)
>
> Replace usb_port_is_power_on()'s |struct usb_hub*| parameter with
> |struct usb_port*| so a subsequent change can use it.
At a brief look this will be the only function that takes usb_port
instead of usb_hub in the entire hub.h (I don't count container_of()
as a function). With that being said I would rather see it to be moved
to port.c altogether (yes, it's more invasive change, but looks more
consistent). I would even dare to move struct usb_port (and container_of()
accompanied with that) and this function to port.h. This might require
a separate patch, though.
Perhaps something like: 1) "move struct usb_port and associated APIs to port.h";
2) "...this patch...".
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 06/13] usb: hub: Pass |struct usb_port*| to usb_port_is_power_on()
2026-07-03 13:11 ` Andy Shevchenko
@ 2026-07-03 13:17 ` Chen-Yu Tsai
2026-07-03 13:33 ` Andy Shevchenko
0 siblings, 1 reply; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 13:17 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, linux-acpi,
driver-core, linux-pm, linux-usb, devicetree, linux-mediatek,
linux-arm-kernel, linux-kernel, Manivannan Sadhasivam, Alan Stern,
Bartosz Golaszewski
On Fri, Jul 3, 2026 at 9:11 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Jul 03, 2026 at 07:03:07PM +0800, Chen-Yu Tsai wrote:
> > usb_port_is_power_on() currently takes |struct usb_hub*|, but only needs
> > it to tell if the hub/port is SuperSpeed or not.
> >
> > In a subsequent change, usb_port_is_power_on() needs access to a pwrseq
> > state tracking field in |struct usb_port|. Either structure can be used
> > to identify whether a port/hub is SuperSpeed or not, as the field in
> > |struct usb_port| is inherited from the hub:
> >
> > port->is_superspeed = hub_is_superspeed(hub)
> >
> > Replace usb_port_is_power_on()'s |struct usb_hub*| parameter with
> > |struct usb_port*| so a subsequent change can use it.
>
> At a brief look this will be the only function that takes usb_port
> instead of usb_hub in the entire hub.h (I don't count container_of()
> as a function). With that being said I would rather see it to be moved
> to port.c altogether (yes, it's more invasive change, but looks more
> consistent). I would even dare to move struct usb_port (and container_of()
> accompanied with that) and this function to port.h. This might require
> a separate patch, though.
I agree with the reasoning, especially given the function name. However
I wonder if it would cause problems given the linking order. I'll give
it a try nevertheless and report back.
ChenYu
> Perhaps something like: 1) "move struct usb_port and associated APIs to port.h";
> 2) "...this patch...".
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 06/13] usb: hub: Pass |struct usb_port*| to usb_port_is_power_on()
2026-07-03 13:17 ` Chen-Yu Tsai
@ 2026-07-03 13:33 ` Andy Shevchenko
0 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2026-07-03 13:33 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, linux-acpi,
driver-core, linux-pm, linux-usb, devicetree, linux-mediatek,
linux-arm-kernel, linux-kernel, Manivannan Sadhasivam, Alan Stern,
Bartosz Golaszewski
On Fri, Jul 03, 2026 at 09:17:16PM +0800, Chen-Yu Tsai wrote:
> On Fri, Jul 3, 2026 at 9:11 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Fri, Jul 03, 2026 at 07:03:07PM +0800, Chen-Yu Tsai wrote:
> > > usb_port_is_power_on() currently takes |struct usb_hub*|, but only needs
> > > it to tell if the hub/port is SuperSpeed or not.
> > >
> > > In a subsequent change, usb_port_is_power_on() needs access to a pwrseq
> > > state tracking field in |struct usb_port|. Either structure can be used
> > > to identify whether a port/hub is SuperSpeed or not, as the field in
> > > |struct usb_port| is inherited from the hub:
> > >
> > > port->is_superspeed = hub_is_superspeed(hub)
> > >
> > > Replace usb_port_is_power_on()'s |struct usb_hub*| parameter with
> > > |struct usb_port*| so a subsequent change can use it.
> >
> > At a brief look this will be the only function that takes usb_port
> > instead of usb_hub in the entire hub.h (I don't count container_of()
> > as a function). With that being said I would rather see it to be moved
> > to port.c altogether (yes, it's more invasive change, but looks more
> > consistent). I would even dare to move struct usb_port (and container_of()
> > accompanied with that) and this function to port.h. This might require
> > a separate patch, though.
>
> I agree with the reasoning, especially given the function name. However
> I wonder if it would cause problems given the linking order. I'll give
> it a try nevertheless and report back.
Thanks!
In case it won't fly (but I still think it's better to split), can you at least
group usb_port APIs and struct? Means moving the proto closer to that struct
usb_port followed by container_of().
> > Perhaps something like: 1) "move struct usb_port and associated APIs to port.h";
> > 2) "...this patch...".
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 07/13] usb: hub: Use usb_hub_set_port_power() to control port power everywhere
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (5 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 06/13] usb: hub: Pass |struct usb_port*| to usb_port_is_power_on() Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 12:12 ` Bartosz Golaszewski
2026-07-03 11:03 ` [PATCH v3 08/13] usb: hub: Power on connected M.2 E-key connectors with power sequencing API Chen-Yu Tsai
` (6 subsequent siblings)
13 siblings, 1 reply; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern
There are still some instances in the USB hub driver where port power is
directly controlled by toggling the USB_PORT_FEAT_POWER feature flag.
Switch these instances over to usb_hub_set_port_power() so that only one
unified function to do this exists. This makes adding external power
control with the power sequencing API easier and consistently applied.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- New patch
---
drivers/usb/core/hub.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index da0a4cc8e15a..8ae97e8c26aa 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -975,11 +975,8 @@ static void hub_power_on(struct usb_hub *hub, bool do_delay)
dev_dbg(hub->intfdev, "trying to enable port power on "
"non-switchable hub\n");
for (port1 = 1; port1 <= hub->hdev->maxchild; port1++)
- if (test_bit(port1, hub->power_bits))
- set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
- else
- usb_clear_port_feature(hub->hdev, port1,
- USB_PORT_FEAT_POWER);
+ usb_hub_set_port_power(hub->hdev, hub, port1,
+ test_bit(port1, hub->power_bits));
if (do_delay)
msleep(hub_power_on_good_delay(hub));
}
@@ -5452,7 +5449,7 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
if (hub_is_port_power_switchable(hub)
&& !usb_port_is_power_on(port_dev, portstatus)
&& !port_dev->port_owner)
- set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
+ usb_hub_set_port_power(hdev, hub, port1, true);
if (portstatus & USB_PORT_STAT_ENABLE)
goto done;
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 07/13] usb: hub: Use usb_hub_set_port_power() to control port power everywhere
2026-07-03 11:03 ` [PATCH v3 07/13] usb: hub: Use usb_hub_set_port_power() to control port power everywhere Chen-Yu Tsai
@ 2026-07-03 12:12 ` Bartosz Golaszewski
0 siblings, 0 replies; 35+ messages in thread
From: Bartosz Golaszewski @ 2026-07-03 12:12 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, linux-acpi,
driver-core, linux-pm, linux-usb, devicetree, linux-mediatek,
linux-arm-kernel, linux-kernel, Manivannan Sadhasivam, Alan Stern
On Fri, 3 Jul 2026 13:03:08 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> There are still some instances in the USB hub driver where port power is
> directly controlled by toggling the USB_PORT_FEAT_POWER feature flag.
>
> Switch these instances over to usb_hub_set_port_power() so that only one
> unified function to do this exists. This makes adding external power
> control with the power sequencing API easier and consistently applied.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 08/13] usb: hub: Power on connected M.2 E-key connectors with power sequencing API
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (6 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 07/13] usb: hub: Use usb_hub_set_port_power() to control port power everywhere Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 12:41 ` Bartosz Golaszewski
2026-07-03 13:19 ` Andy Shevchenko
2026-07-03 11:03 ` [PATCH v3 09/13] dt-bindings: usb: mediatek,mtk-xhci: Switch to ports for USB connections Chen-Yu Tsai
` (5 subsequent siblings)
13 siblings, 2 replies; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern
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.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Expanded subject to mention power sequencing API
- Dropped commit message bit about power sequencing Kconfig symbol change
to bool
- Added optional dependency on POWER_SEQUENCING to USB
- Split out pwrseq_power_*() calls into separate helpers
- Rewrote set_bit() and clear_bit() branches with assign_bit()
- Dropped the pwrseq_power_off() before pwrseq_put(): pwrseq_put() does it
automatically.
- Removed pwrseq_power_on() from usb_hub_create_port_device(); it will
get called through usb_hub_set_port_power() in hub_activate().
- Added checks for port->pwrseq in hub_is_port_power_switchable()
- Use separate pwrseq descriptors for HighSpeed and SuperSpeed ports.
This makes things simpler. On the other hand to power cycle a port
userspace needs to toggle it on both the HS and SS ports together.
- Dropped pwrseq state tracking again
The power sequencing consumer API already tracks the state internally;
doing it again in |struct usb_port| is not necessary especially now
that the descriptors aren't shared.
It's unclear to me how actual hubs reconcile USB_PORT_FEAT_POWER settings
from the HS side and SS side. One hub chip vendor said that VBUS_EN for
a port is on if the flag is set on either side; however actually testing
on one of their hubs showed that VBUS was cut as soon as the flag is
cleared on the HS port. Maybe it could be different if a SS device was
connected? That scenario was not tested. Testing on another retail
bought hub seemed to work exactly as described though: USB_PORT_FEAT_POWER
needed to be clear on both HS and SS ports to turn off VBUS.
Under this scheme, I'm not sure how the power cycle in hub_port_connect()
would work correctly.
- Link to v2:
https://lore.kernel.org/all/20260610084053.2059858-1-wenst@chromium.org/
Changes since v1:
- Switch to fwnode instead of OF
- Tie port@ fwnode to usb_port device
- Move remote node compatible checking to separate helper
- Use usb_port device to request power sequencing descriptor
- Drop "index" parameter from pwrseq_get()
- Do not get pwrseq descriptor for SuperSpeed port; share one for one
physical port
- Add pwrseq state tracking
- Link to v1:
https://lore.kernel.org/all/20260515090149.3169406-1-wenst@chromium.org/
---
drivers/usb/Kconfig | 1 +
drivers/usb/core/hub.c | 44 +++++++++++++++++++++++++++++-----
drivers/usb/core/hub.h | 10 +++++++-
drivers/usb/core/port.c | 52 ++++++++++++++++++++++++++++++++++++++++-
4 files changed, 99 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index abf8c6cdea9e..ef1959363fb1 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -44,6 +44,7 @@ config USB_ARCH_HAS_HCD
config USB
tristate "Support for Host-side USB"
depends on USB_ARCH_HAS_HCD
+ depends on POWER_SEQUENCING if POWER_SEQUENCING
select GENERIC_ALLOCATOR
select USB_COMMON
select NLS # for UTF-8 strings
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 8ae97e8c26aa..dfa0f5dd75e8 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -32,6 +32,7 @@
#include <linux/mutex.h>
#include <linux/random.h>
#include <linux/pm_qos.h>
+#include <linux/pwrseq/consumer.h>
#include <linux/kobject.h>
#include <linux/bitfield.h>
@@ -871,6 +872,30 @@ static void hub_tt_work(struct work_struct *work)
spin_unlock_irqrestore(&hub->tt.lock, flags);
}
+static int usb_hub_set_port_pwrseq(struct usb_port *port, bool set)
+{
+ int ret = 0;
+
+ if (set)
+ ret = pwrseq_power_on(port->pwrseq);
+ else
+ ret = pwrseq_power_off(port->pwrseq);
+
+ return ret;
+}
+
+static int usb_hub_restore_port_pwrseq(struct usb_port *port, bool set)
+{
+ int ret = 0;
+
+ if (set)
+ ret = pwrseq_power_off(port->pwrseq);
+ else
+ ret = pwrseq_power_on(port->pwrseq);
+
+ return ret;
+}
+
/**
* usb_hub_set_port_power - control hub port's power state
* @hdev: USB device belonging to the usb hub
@@ -886,20 +911,24 @@ static void hub_tt_work(struct work_struct *work)
int usb_hub_set_port_power(struct usb_device *hdev, struct usb_hub *hub,
int port1, bool set)
{
+ struct usb_port *pwrseq_port = hub->ports[port1 - 1];
int ret;
+ ret = usb_hub_set_port_pwrseq(pwrseq_port, set);
+ if (ret)
+ return ret;
+
if (set)
ret = set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
else
ret = usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
- if (ret)
+ if (ret) {
+ usb_hub_restore_port_pwrseq(pwrseq_port, set);
return ret;
+ }
- if (set)
- set_bit(port1, hub->power_bits);
- else
- clear_bit(port1, hub->power_bits);
+ assign_bit(port1, hub->power_bits, set);
return 0;
}
@@ -3249,7 +3278,10 @@ int usb_port_is_power_on(struct usb_port *port, unsigned int portstatus)
ret = 1;
}
- return ret;
+ if (!port->pwrseq)
+ return ret;
+
+ return ret && pwrseq_power_is_on(port->pwrseq);
}
static void usb_lock_port(struct usb_port *port_dev)
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index b65d9192379d..99bae6ace4da 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -85,6 +85,7 @@ struct usb_hub {
* @port_owner: port's owner
* @peer: related usb2 and usb3 ports (share the same connector)
* @connector: USB Type-C connector
+ * @pwrseq: power sequencing descriptor for the port
* @req: default pm qos request for hubs without port power control
* @connect_type: port's connect type
* @state: device state of the usb device attached to the port
@@ -104,6 +105,7 @@ struct usb_port {
struct usb_dev_state *port_owner;
struct usb_port *peer;
struct typec_connector *connector;
+ struct pwrseq_desc *pwrseq;
struct dev_pm_qos_request *req;
enum usb_port_connect_type connect_type;
enum usb_device_state state;
@@ -147,7 +149,13 @@ static inline bool hub_is_port_power_switchable(struct usb_hub *hub)
if (!hub)
return false;
hcs = hub->descriptor->wHubCharacteristics;
- return (le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM;
+ if ((le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM)
+ return true;
+ /* check for controllable external power sequencers */
+ for (unsigned int i = 1; i <= hub->hdev->maxchild; i++)
+ if (hub->ports[i] && hub->ports[i]->pwrseq)
+ return true;
+ return false;
}
static inline int hub_is_superspeed(struct usb_device *hdev)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 77dbf51f1760..87b8e4f90be6 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -8,11 +8,14 @@
*/
#include <linux/acpi.h>
+#include <linux/cleanup.h>
#include <linux/kstrtox.h>
#include <linux/slab.h>
#include <linux/string_choices.h>
#include <linux/sysfs.h>
#include <linux/pm_qos.h>
+#include <linux/property.h>
+#include <linux/pwrseq/consumer.h>
#include <linux/component.h>
#include <linux/usb/of.h>
@@ -29,6 +32,9 @@ static bool usb_port_allow_power_off(struct usb_device *hdev,
if (hub_is_port_power_switchable(hub))
return true;
+ if (port_dev->pwrseq)
+ return true;
+
if (!IS_ENABLED(CONFIG_ACPI))
return false;
@@ -749,6 +755,39 @@ static const struct component_ops connector_ops = {
.unbind = connector_unbind,
};
+static bool port_pwrseq_is_supported(struct usb_port *port_dev)
+{
+ struct device *dev = &port_dev->dev;
+ struct fwnode_handle *port = dev->fwnode;
+ struct fwnode_handle *ep __free(fwnode_handle) =
+ fwnode_graph_get_next_port_endpoint(port, NULL);
+ if (!ep)
+ return false;
+
+ struct fwnode_handle *remote __free(fwnode_handle) =
+ fwnode_graph_get_remote_port_parent(ep);
+ if (!remote)
+ return false;
+
+ if (!fwnode_device_is_compatible(remote, "pcie-m2-e-connector")) {
+ dev_dbg(dev, "remote endpoint %pfw is not a supported connector", remote);
+ return false;
+ }
+
+ return true;
+}
+
+static struct pwrseq_desc *usb_hub_port_pwrseq_get(struct usb_port *port_dev)
+{
+ if (!IS_ENABLED(CONFIG_POWER_SEQUENCING))
+ return NULL;
+
+ if (!port_pwrseq_is_supported(port_dev))
+ return NULL;
+
+ return pwrseq_get(&port_dev->dev, "usb");
+}
+
int usb_hub_create_port_device(struct usb_hub *hub, int port1)
{
struct usb_port *port_dev;
@@ -809,10 +848,18 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
goto err_put_kn;
}
+ 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");
+ goto err_put_kn;
+ }
+
retval = component_add(&port_dev->dev, &connector_ops);
if (retval) {
dev_warn(&port_dev->dev, "failed to add component\n");
- goto err_put_kn;
+ goto err_put_pwrseq;
}
find_and_link_peer(hub, port1);
@@ -850,6 +897,8 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
}
return 0;
+err_put_pwrseq:
+ pwrseq_put(port_dev->pwrseq);
err_put_kn:
sysfs_put(port_dev->state_kn);
err_unregister:
@@ -866,6 +915,7 @@ void usb_hub_remove_port_device(struct usb_hub *hub, int port1)
peer = port_dev->peer;
if (peer)
unlink_peers(port_dev, peer);
+ pwrseq_put(port_dev->pwrseq);
component_del(&port_dev->dev, &connector_ops);
sysfs_put(port_dev->state_kn);
device_unregister(&port_dev->dev);
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 08/13] usb: hub: Power on connected M.2 E-key connectors with power sequencing API
2026-07-03 11:03 ` [PATCH v3 08/13] usb: hub: Power on connected M.2 E-key connectors with power sequencing API Chen-Yu Tsai
@ 2026-07-03 12:41 ` Bartosz Golaszewski
2026-07-03 12:58 ` Chen-Yu Tsai
2026-07-03 13:19 ` Andy Shevchenko
1 sibling, 1 reply; 35+ messages in thread
From: Bartosz Golaszewski @ 2026-07-03 12:41 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Bartosz Golaszewski,
Greg Kroah-Hartman, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
On Fri, 3 Jul 2026 13:03:09 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> 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.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> Changes since v2:
> - Expanded subject to mention power sequencing API
> - Dropped commit message bit about power sequencing Kconfig symbol change
> to bool
> - Added optional dependency on POWER_SEQUENCING to USB
> - Split out pwrseq_power_*() calls into separate helpers
> - Rewrote set_bit() and clear_bit() branches with assign_bit()
> - Dropped the pwrseq_power_off() before pwrseq_put(): pwrseq_put() does it
> automatically.
> - Removed pwrseq_power_on() from usb_hub_create_port_device(); it will
> get called through usb_hub_set_port_power() in hub_activate().
> - Added checks for port->pwrseq in hub_is_port_power_switchable()
>
> - Use separate pwrseq descriptors for HighSpeed and SuperSpeed ports.
> This makes things simpler. On the other hand to power cycle a port
> userspace needs to toggle it on both the HS and SS ports together.
> - Dropped pwrseq state tracking again
> The power sequencing consumer API already tracks the state internally;
> doing it again in |struct usb_port| is not necessary especially now
> that the descriptors aren't shared.
>
> It's unclear to me how actual hubs reconcile USB_PORT_FEAT_POWER settings
> from the HS side and SS side. One hub chip vendor said that VBUS_EN for
> a port is on if the flag is set on either side; however actually testing
> on one of their hubs showed that VBUS was cut as soon as the flag is
> cleared on the HS port. Maybe it could be different if a SS device was
> connected? That scenario was not tested. Testing on another retail
> bought hub seemed to work exactly as described though: USB_PORT_FEAT_POWER
> needed to be clear on both HS and SS ports to turn off VBUS.
>
> Under this scheme, I'm not sure how the power cycle in hub_port_connect()
> would work correctly.
>
> - Link to v2:
> https://lore.kernel.org/all/20260610084053.2059858-1-wenst@chromium.org/
>
> Changes since v1:
> - Switch to fwnode instead of OF
> - Tie port@ fwnode to usb_port device
> - Move remote node compatible checking to separate helper
> - Use usb_port device to request power sequencing descriptor
> - Drop "index" parameter from pwrseq_get()
> - Do not get pwrseq descriptor for SuperSpeed port; share one for one
> physical port
> - Add pwrseq state tracking
> - Link to v1:
> https://lore.kernel.org/all/20260515090149.3169406-1-wenst@chromium.org/
> ---
> drivers/usb/Kconfig | 1 +
> drivers/usb/core/hub.c | 44 +++++++++++++++++++++++++++++-----
> drivers/usb/core/hub.h | 10 +++++++-
> drivers/usb/core/port.c | 52 ++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 99 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index abf8c6cdea9e..ef1959363fb1 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -44,6 +44,7 @@ config USB_ARCH_HAS_HCD
> config USB
> tristate "Support for Host-side USB"
> depends on USB_ARCH_HAS_HCD
> + depends on POWER_SEQUENCING if POWER_SEQUENCING
> select GENERIC_ALLOCATOR
> select USB_COMMON
> select NLS # for UTF-8 strings
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 8ae97e8c26aa..dfa0f5dd75e8 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -32,6 +32,7 @@
> #include <linux/mutex.h>
> #include <linux/random.h>
> #include <linux/pm_qos.h>
> +#include <linux/pwrseq/consumer.h>
> #include <linux/kobject.h>
>
> #include <linux/bitfield.h>
> @@ -871,6 +872,30 @@ static void hub_tt_work(struct work_struct *work)
> spin_unlock_irqrestore(&hub->tt.lock, flags);
> }
>
> +static int usb_hub_set_port_pwrseq(struct usb_port *port, bool set)
> +{
> + int ret = 0;
> +
> + if (set)
> + ret = pwrseq_power_on(port->pwrseq);
> + else
> + ret = pwrseq_power_off(port->pwrseq);
> +
> + return ret;
> +}
> +
> +static int usb_hub_restore_port_pwrseq(struct usb_port *port, bool set)
> +{
> + int ret = 0;
> +
> + if (set)
> + ret = pwrseq_power_off(port->pwrseq);
> + else
> + ret = pwrseq_power_on(port->pwrseq);
> +
> + return ret;
> +}
> +
> /**
> * usb_hub_set_port_power - control hub port's power state
> * @hdev: USB device belonging to the usb hub
> @@ -886,20 +911,24 @@ static void hub_tt_work(struct work_struct *work)
> int usb_hub_set_port_power(struct usb_device *hdev, struct usb_hub *hub,
> int port1, bool set)
> {
> + struct usb_port *pwrseq_port = hub->ports[port1 - 1];
> int ret;
>
> + ret = usb_hub_set_port_pwrseq(pwrseq_port, set);
> + if (ret)
> + return ret;
> +
> if (set)
> ret = set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
> else
> ret = usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
>
> - if (ret)
> + if (ret) {
> + usb_hub_restore_port_pwrseq(pwrseq_port, set);
> return ret;
> + }
>
> - if (set)
> - set_bit(port1, hub->power_bits);
> - else
> - clear_bit(port1, hub->power_bits);
> + assign_bit(port1, hub->power_bits, set);
> return 0;
> }
>
> @@ -3249,7 +3278,10 @@ int usb_port_is_power_on(struct usb_port *port, unsigned int portstatus)
> ret = 1;
> }
>
> - return ret;
> + if (!port->pwrseq)
> + return ret;
> +
> + return ret && pwrseq_power_is_on(port->pwrseq);
So this is only needed to not have to track the state in a separate field?
> }
>
> static void usb_lock_port(struct usb_port *port_dev)
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index b65d9192379d..99bae6ace4da 100644
> --- a/drivers/usb/core/hub.h
> +++ b/drivers/usb/core/hub.h
> @@ -85,6 +85,7 @@ struct usb_hub {
> * @port_owner: port's owner
> * @peer: related usb2 and usb3 ports (share the same connector)
> * @connector: USB Type-C connector
> + * @pwrseq: power sequencing descriptor for the port
> * @req: default pm qos request for hubs without port power control
> * @connect_type: port's connect type
> * @state: device state of the usb device attached to the port
> @@ -104,6 +105,7 @@ struct usb_port {
> struct usb_dev_state *port_owner;
> struct usb_port *peer;
> struct typec_connector *connector;
> + struct pwrseq_desc *pwrseq;
> struct dev_pm_qos_request *req;
> enum usb_port_connect_type connect_type;
> enum usb_device_state state;
> @@ -147,7 +149,13 @@ static inline bool hub_is_port_power_switchable(struct usb_hub *hub)
> if (!hub)
> return false;
> hcs = hub->descriptor->wHubCharacteristics;
> - return (le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM;
> + if ((le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM)
> + return true;
> + /* check for controllable external power sequencers */
> + for (unsigned int i = 1; i <= hub->hdev->maxchild; i++)
> + if (hub->ports[i] && hub->ports[i]->pwrseq)
> + return true;
> + return false;
> }
>
> static inline int hub_is_superspeed(struct usb_device *hdev)
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index 77dbf51f1760..87b8e4f90be6 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -8,11 +8,14 @@
> */
>
> #include <linux/acpi.h>
> +#include <linux/cleanup.h>
> #include <linux/kstrtox.h>
> #include <linux/slab.h>
> #include <linux/string_choices.h>
> #include <linux/sysfs.h>
> #include <linux/pm_qos.h>
> +#include <linux/property.h>
> +#include <linux/pwrseq/consumer.h>
> #include <linux/component.h>
> #include <linux/usb/of.h>
>
> @@ -29,6 +32,9 @@ static bool usb_port_allow_power_off(struct usb_device *hdev,
> if (hub_is_port_power_switchable(hub))
> return true;
>
> + if (port_dev->pwrseq)
> + return true;
> +
> if (!IS_ENABLED(CONFIG_ACPI))
> return false;
>
> @@ -749,6 +755,39 @@ static const struct component_ops connector_ops = {
> .unbind = connector_unbind,
> };
>
> +static bool port_pwrseq_is_supported(struct usb_port *port_dev)
> +{
> + struct device *dev = &port_dev->dev;
> + struct fwnode_handle *port = dev->fwnode;
> + struct fwnode_handle *ep __free(fwnode_handle) =
> + fwnode_graph_get_next_port_endpoint(port, NULL);
> + if (!ep)
> + return false;
> +
> + struct fwnode_handle *remote __free(fwnode_handle) =
> + fwnode_graph_get_remote_port_parent(ep);
> + if (!remote)
> + return false;
> +
> + if (!fwnode_device_is_compatible(remote, "pcie-m2-e-connector")) {
> + dev_dbg(dev, "remote endpoint %pfw is not a supported connector", remote);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static struct pwrseq_desc *usb_hub_port_pwrseq_get(struct usb_port *port_dev)
> +{
> + if (!IS_ENABLED(CONFIG_POWER_SEQUENCING))
> + return NULL;
> +
> + if (!port_pwrseq_is_supported(port_dev))
> + return NULL;
> +
> + return pwrseq_get(&port_dev->dev, "usb");
> +}
> +
> int usb_hub_create_port_device(struct usb_hub *hub, int port1)
> {
> struct usb_port *port_dev;
> @@ -809,10 +848,18 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
> goto err_put_kn;
> }
>
> + 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");
> + goto err_put_kn;
> + }
> +
> retval = component_add(&port_dev->dev, &connector_ops);
> if (retval) {
> dev_warn(&port_dev->dev, "failed to add component\n");
> - goto err_put_kn;
> + goto err_put_pwrseq;
> }
>
> find_and_link_peer(hub, port1);
> @@ -850,6 +897,8 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
> }
> return 0;
>
> +err_put_pwrseq:
> + pwrseq_put(port_dev->pwrseq);
> err_put_kn:
> sysfs_put(port_dev->state_kn);
> err_unregister:
> @@ -866,6 +915,7 @@ void usb_hub_remove_port_device(struct usb_hub *hub, int port1)
> peer = port_dev->peer;
> if (peer)
> unlink_peers(port_dev, peer);
> + pwrseq_put(port_dev->pwrseq);
> component_del(&port_dev->dev, &connector_ops);
> sysfs_put(port_dev->state_kn);
> device_unregister(&port_dev->dev);
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
>
>
Looks good to me.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Bart
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v3 08/13] usb: hub: Power on connected M.2 E-key connectors with power sequencing API
2026-07-03 12:41 ` Bartosz Golaszewski
@ 2026-07-03 12:58 ` Chen-Yu Tsai
2026-07-03 13:16 ` Bartosz Golaszewski
0 siblings, 1 reply; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 12:58 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Greg Kroah-Hartman,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Rafael J. Wysocki, Danilo Krummrich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno
On Fri, Jul 3, 2026 at 8:41 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Fri, 3 Jul 2026 13:03:09 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> > 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.
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> > Changes since v2:
> > - Expanded subject to mention power sequencing API
> > - Dropped commit message bit about power sequencing Kconfig symbol change
> > to bool
> > - Added optional dependency on POWER_SEQUENCING to USB
> > - Split out pwrseq_power_*() calls into separate helpers
> > - Rewrote set_bit() and clear_bit() branches with assign_bit()
> > - Dropped the pwrseq_power_off() before pwrseq_put(): pwrseq_put() does it
> > automatically.
> > - Removed pwrseq_power_on() from usb_hub_create_port_device(); it will
> > get called through usb_hub_set_port_power() in hub_activate().
> > - Added checks for port->pwrseq in hub_is_port_power_switchable()
> >
> > - Use separate pwrseq descriptors for HighSpeed and SuperSpeed ports.
> > This makes things simpler. On the other hand to power cycle a port
> > userspace needs to toggle it on both the HS and SS ports together.
> > - Dropped pwrseq state tracking again
> > The power sequencing consumer API already tracks the state internally;
> > doing it again in |struct usb_port| is not necessary especially now
> > that the descriptors aren't shared.
> >
> > It's unclear to me how actual hubs reconcile USB_PORT_FEAT_POWER settings
> > from the HS side and SS side. One hub chip vendor said that VBUS_EN for
> > a port is on if the flag is set on either side; however actually testing
> > on one of their hubs showed that VBUS was cut as soon as the flag is
> > cleared on the HS port. Maybe it could be different if a SS device was
> > connected? That scenario was not tested. Testing on another retail
> > bought hub seemed to work exactly as described though: USB_PORT_FEAT_POWER
> > needed to be clear on both HS and SS ports to turn off VBUS.
> >
> > Under this scheme, I'm not sure how the power cycle in hub_port_connect()
> > would work correctly.
> >
> > - Link to v2:
> > https://lore.kernel.org/all/20260610084053.2059858-1-wenst@chromium.org/
> >
> > Changes since v1:
> > - Switch to fwnode instead of OF
> > - Tie port@ fwnode to usb_port device
> > - Move remote node compatible checking to separate helper
> > - Use usb_port device to request power sequencing descriptor
> > - Drop "index" parameter from pwrseq_get()
> > - Do not get pwrseq descriptor for SuperSpeed port; share one for one
> > physical port
> > - Add pwrseq state tracking
> > - Link to v1:
> > https://lore.kernel.org/all/20260515090149.3169406-1-wenst@chromium.org/
> > ---
> > drivers/usb/Kconfig | 1 +
> > drivers/usb/core/hub.c | 44 +++++++++++++++++++++++++++++-----
> > drivers/usb/core/hub.h | 10 +++++++-
> > drivers/usb/core/port.c | 52 ++++++++++++++++++++++++++++++++++++++++-
> > 4 files changed, 99 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> > index abf8c6cdea9e..ef1959363fb1 100644
> > --- a/drivers/usb/Kconfig
> > +++ b/drivers/usb/Kconfig
> > @@ -44,6 +44,7 @@ config USB_ARCH_HAS_HCD
> > config USB
> > tristate "Support for Host-side USB"
> > depends on USB_ARCH_HAS_HCD
> > + depends on POWER_SEQUENCING if POWER_SEQUENCING
> > select GENERIC_ALLOCATOR
> > select USB_COMMON
> > select NLS # for UTF-8 strings
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 8ae97e8c26aa..dfa0f5dd75e8 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -32,6 +32,7 @@
> > #include <linux/mutex.h>
> > #include <linux/random.h>
> > #include <linux/pm_qos.h>
> > +#include <linux/pwrseq/consumer.h>
> > #include <linux/kobject.h>
> >
> > #include <linux/bitfield.h>
> > @@ -871,6 +872,30 @@ static void hub_tt_work(struct work_struct *work)
> > spin_unlock_irqrestore(&hub->tt.lock, flags);
> > }
> >
> > +static int usb_hub_set_port_pwrseq(struct usb_port *port, bool set)
> > +{
> > + int ret = 0;
> > +
> > + if (set)
> > + ret = pwrseq_power_on(port->pwrseq);
> > + else
> > + ret = pwrseq_power_off(port->pwrseq);
> > +
> > + return ret;
> > +}
> > +
> > +static int usb_hub_restore_port_pwrseq(struct usb_port *port, bool set)
> > +{
> > + int ret = 0;
> > +
> > + if (set)
> > + ret = pwrseq_power_off(port->pwrseq);
> > + else
> > + ret = pwrseq_power_on(port->pwrseq);
> > +
> > + return ret;
> > +}
> > +
> > /**
> > * usb_hub_set_port_power - control hub port's power state
> > * @hdev: USB device belonging to the usb hub
> > @@ -886,20 +911,24 @@ static void hub_tt_work(struct work_struct *work)
> > int usb_hub_set_port_power(struct usb_device *hdev, struct usb_hub *hub,
> > int port1, bool set)
> > {
> > + struct usb_port *pwrseq_port = hub->ports[port1 - 1];
> > int ret;
> >
> > + ret = usb_hub_set_port_pwrseq(pwrseq_port, set);
> > + if (ret)
> > + return ret;
> > +
As Sashiko pointed out, the stub function returns -ENOSYS. I need to add
something to handle that case properly in the helper functions above.
if (!IS_ENABLED(POWER_SEQUENCING))
return 0;
seems to be more straightforward.
> > if (set)
> > ret = set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
> > else
> > ret = usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
> >
> > - if (ret)
> > + if (ret) {
> > + usb_hub_restore_port_pwrseq(pwrseq_port, set);
> > return ret;
> > + }
> >
> > - if (set)
> > - set_bit(port1, hub->power_bits);
> > - else
> > - clear_bit(port1, hub->power_bits);
> > + assign_bit(port1, hub->power_bits, set);
> > return 0;
> > }
> >
> > @@ -3249,7 +3278,10 @@ int usb_port_is_power_on(struct usb_port *port, unsigned int portstatus)
> > ret = 1;
> > }
> >
> > - return ret;
> > + if (!port->pwrseq)
> > + return ret;
> > +
> > + return ret && pwrseq_power_is_on(port->pwrseq);
>
> So this is only needed to not have to track the state in a separate field?
Correct. Reading the field is not the problem. Updating it is, as the
fields are packed bitfields, and Sashiko suggested that locking would
be needed, though it may have been for the peer port, which has since
been removed.
[...]
> Looks good to me.
>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Thanks
ChenYu
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v3 08/13] usb: hub: Power on connected M.2 E-key connectors with power sequencing API
2026-07-03 12:58 ` Chen-Yu Tsai
@ 2026-07-03 13:16 ` Bartosz Golaszewski
2026-07-03 13:20 ` Chen-Yu Tsai
0 siblings, 1 reply; 35+ messages in thread
From: Bartosz Golaszewski @ 2026-07-03 13:16 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Greg Kroah-Hartman,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Rafael J. Wysocki, Danilo Krummrich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Bartosz Golaszewski
On Fri, 3 Jul 2026 14:58:56 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> On Fri, Jul 3, 2026 at 8:41 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>>
>> So this is only needed to not have to track the state in a separate field?
>
> Correct. Reading the field is not the problem. Updating it is, as the
> fields are packed bitfields, and Sashiko suggested that locking would
> be needed, though it may have been for the peer port, which has since
> been removed.
>
I'm not sure if it is. The user controlls the handle. We could argue, it's up
to them to provide synchroniztion. We only assign the target and pwrseq fields
once in pwrseq_match_device() and only change the powered_on field later with
pwrseq_power_on/off(). Unless the user does something weird, we should be
alright. Maybe a small note in the kernel doc would be in order.
Thanks,
Bartosz
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 08/13] usb: hub: Power on connected M.2 E-key connectors with power sequencing API
2026-07-03 13:16 ` Bartosz Golaszewski
@ 2026-07-03 13:20 ` Chen-Yu Tsai
0 siblings, 0 replies; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 13:20 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Greg Kroah-Hartman,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Rafael J. Wysocki, Danilo Krummrich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno
On Fri, Jul 3, 2026 at 9:16 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Fri, 3 Jul 2026 14:58:56 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> > On Fri, Jul 3, 2026 at 8:41 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
> >>
> >> So this is only needed to not have to track the state in a separate field?
> >
> > Correct. Reading the field is not the problem. Updating it is, as the
> > fields are packed bitfields, and Sashiko suggested that locking would
> > be needed, though it may have been for the peer port, which has since
> > been removed.
> >
>
> I'm not sure if it is. The user controlls the handle. We could argue, it's up
> to them to provide synchroniztion. We only assign the target and pwrseq fields
> once in pwrseq_match_device() and only change the powered_on field later with
> pwrseq_power_on/off(). Unless the user does something weird, we should be
> alright. Maybe a small note in the kernel doc would be in order.
I was refering to the status bitfields in |struct usb_port|. As you already
explained I think the pwrseq stuff is fine.
ChenYu
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 08/13] usb: hub: Power on connected M.2 E-key connectors with power sequencing API
2026-07-03 11:03 ` [PATCH v3 08/13] usb: hub: Power on connected M.2 E-key connectors with power sequencing API Chen-Yu Tsai
2026-07-03 12:41 ` Bartosz Golaszewski
@ 2026-07-03 13:19 ` Andy Shevchenko
1 sibling, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2026-07-03 13:19 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, linux-acpi,
driver-core, linux-pm, linux-usb, devicetree, linux-mediatek,
linux-arm-kernel, linux-kernel, Manivannan Sadhasivam, Alan Stern
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
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 09/13] dt-bindings: usb: mediatek,mtk-xhci: Switch to ports for USB connections
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (7 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 08/13] usb: hub: Power on connected M.2 E-key connectors with power sequencing API Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-05 9:43 ` Krzysztof Kozlowski
2026-07-03 11:03 ` [PATCH v3 10/13] power: sequencing: pcie-m2: support matching on remote "port" node Chen-Yu Tsai
` (4 subsequent siblings)
13 siblings, 1 reply; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern
This reverts commit 454a1e3cd36c113341d7b71e8e691c6e47ab4a8a.
MediaTek's XHCI implementation supports both USB 2.0 High Speed (HS)
and USB 3.x Super Speed (SS). The block can also be synthesized with
either HS-only capability or HS+SS capability. The SSUSB controller
handles the device or gadget mode. Saying that SSUSB handles the HS
portion is wrong.
For example, on the MT8195, the first two instances support both HS and
SS, while the latter two instances support only HS.
Switch to a "ports" sub-node for describing USB connections. Port 1 is
Super Speed if the controller is SS-capable, otherwise it is High Speed.
Port 2 is High Speed if SS-capable. This port mapping scheme directly
matches what the hardware returns in its capability registers.
Fixes: 454a1e3cd36c ("dt-bindings: usb: mediatek,mtk-xhci: Add port for SuperSpeed EP")
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v1:
- Squashed DT binding revert and addition together
- Dropped reviewed-by from Bartosz
---
.../bindings/usb/mediatek,mtk-xhci.yaml | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index 231e6f35a986..d6c75bd20b78 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -107,10 +107,6 @@ properties:
- description: USB3/SS(P) PHY
- description: USB2/HS PHY
- port:
- $ref: /schemas/graph.yaml#/properties/port
- description: Super Speed (SS) Output endpoint to a Type-C connector
-
vusb33-supply:
description: Regulator of USB AVDD3.3v
@@ -188,6 +184,19 @@ properties:
"#size-cells":
const: 0
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Super Speed (SS) data bus if SS-capable;
+ otherwise High Speed (HS) data bus.
+
+ port@2:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: High Speed (HS) data bus if controller is SS-capable.
+
patternProperties:
"@[0-9a-f]{1}$":
type: object
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 09/13] dt-bindings: usb: mediatek,mtk-xhci: Switch to ports for USB connections
2026-07-03 11:03 ` [PATCH v3 09/13] dt-bindings: usb: mediatek,mtk-xhci: Switch to ports for USB connections Chen-Yu Tsai
@ 2026-07-05 9:43 ` Krzysztof Kozlowski
0 siblings, 0 replies; 35+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-05 9:43 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, linux-acpi,
driver-core, linux-pm, linux-usb, devicetree, linux-mediatek,
linux-arm-kernel, linux-kernel, Manivannan Sadhasivam, Alan Stern
On Fri, Jul 03, 2026 at 07:03:10PM +0800, Chen-Yu Tsai wrote:
> This reverts commit 454a1e3cd36c113341d7b71e8e691c6e47ab4a8a.
>
> MediaTek's XHCI implementation supports both USB 2.0 High Speed (HS)
> and USB 3.x Super Speed (SS). The block can also be synthesized with
> either HS-only capability or HS+SS capability. The SSUSB controller
> handles the device or gadget mode. Saying that SSUSB handles the HS
> portion is wrong.
>
> For example, on the MT8195, the first two instances support both HS and
> SS, while the latter two instances support only HS.
>
> Switch to a "ports" sub-node for describing USB connections. Port 1 is
> Super Speed if the controller is SS-capable, otherwise it is High Speed.
> Port 2 is High Speed if SS-capable. This port mapping scheme directly
> matches what the hardware returns in its capability registers.
>
> Fixes: 454a1e3cd36c ("dt-bindings: usb: mediatek,mtk-xhci: Add port for SuperSpeed EP")
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> Changes since v1:
> - Squashed DT binding revert and addition together
> - Dropped reviewed-by from Bartosz
> ---
> .../bindings/usb/mediatek,mtk-xhci.yaml | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 10/13] power: sequencing: pcie-m2: support matching on remote "port" node
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (8 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 09/13] dt-bindings: usb: mediatek,mtk-xhci: Switch to ports for USB connections Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 12:57 ` Andy Shevchenko
2026-07-03 11:03 ` [PATCH v3 11/13] power: sequencing: pcie-m2: Add usb and sdio targets for E-key connector Chen-Yu Tsai
` (3 subsequent siblings)
13 siblings, 1 reply; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern
A USB hub can have multiple ports, and this driver needs to
differentiate which port is being matched to. The USB hub driver now
associates the "port" node with the usb_port device, so here we can
use the remote "port" node to check for a match. Then fall back to
the remote device node for the other connection types.
Also rewrite the existing "remote == dev_of_node(dev)" with
device_match_of_node() for consistency.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Use device_match_of_node()
---
drivers/power/sequencing/pwrseq-pcie-m2.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
index c8ee6df42b81..16a47f76e7ce 100644
--- a/drivers/power/sequencing/pwrseq-pcie-m2.c
+++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
@@ -175,9 +175,18 @@ static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
* parent matches the OF node of 'dev'.
*/
for_each_endpoint_of_node(ctx->of_node, endpoint) {
+ /* USB port devices are tied to the port nodes. */
+ struct device_node *remote_port __free(device_node) =
+ of_graph_get_remote_port(endpoint);
+
+ if (remote_port && device_match_of_node(dev, remote_port))
+ return PWRSEQ_MATCH_OK;
+
+ /* Try the remote port parent for other types. */
struct device_node *remote __free(device_node) =
of_graph_get_remote_port_parent(endpoint);
- if (remote && (remote == dev_of_node(dev)))
+
+ if (remote && device_match_of_node(dev, remote))
return PWRSEQ_MATCH_OK;
}
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 10/13] power: sequencing: pcie-m2: support matching on remote "port" node
2026-07-03 11:03 ` [PATCH v3 10/13] power: sequencing: pcie-m2: support matching on remote "port" node Chen-Yu Tsai
@ 2026-07-03 12:57 ` Andy Shevchenko
0 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2026-07-03 12:57 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, linux-acpi,
driver-core, linux-pm, linux-usb, devicetree, linux-mediatek,
linux-arm-kernel, linux-kernel, Manivannan Sadhasivam, Alan Stern
On Fri, Jul 03, 2026 at 07:03:11PM +0800, Chen-Yu Tsai wrote:
> A USB hub can have multiple ports, and this driver needs to
> differentiate which port is being matched to. The USB hub driver now
> associates the "port" node with the usb_port device, so here we can
> use the remote "port" node to check for a match. Then fall back to
> the remote device node for the other connection types.
>
> Also rewrite the existing "remote == dev_of_node(dev)" with
> device_match_of_node() for consistency.
...
> for_each_endpoint_of_node(ctx->of_node, endpoint) {
> + /* USB port devices are tied to the port nodes. */
> + struct device_node *remote_port __free(device_node) =
> + of_graph_get_remote_port(endpoint);
> +
> + if (remote_port && device_match_of_node(dev, remote_port))
Dup NULL check. _match_of_node() already does that and it seems follows
the same logic here. So dropping it should not change how it functions.
> + return PWRSEQ_MATCH_OK;
> +
> + /* Try the remote port parent for other types. */
> struct device_node *remote __free(device_node) =
> of_graph_get_remote_port_parent(endpoint);
> - if (remote && (remote == dev_of_node(dev)))
> +
> + if (remote && device_match_of_node(dev, remote))
Ditto.
> return PWRSEQ_MATCH_OK;
> }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 11/13] power: sequencing: pcie-m2: Add usb and sdio targets for E-key connector
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (9 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 10/13] power: sequencing: pcie-m2: support matching on remote "port" node Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 11:03 ` [PATCH v3 12/13] arm64: dts: mediatek: mt8195-cherry: Add M.2 E-key slot Chen-Yu Tsai
` (2 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern
The M.2 E-key connector allows either PCIe or SDIO for WiFi and USB or
UART for BT. Currently the driver only supports PCIe and UART.
Add power sequencing targets for SDIO and USB. To avoid adding a
complicated dependency tree, rename the existing power sequencing units
"pcie" and "uart" to "wifi" and "bt". The existing target names are left
untouched. The new "sdio" and "usb" targets just point to the renamed
"wifi" and "bt" units.
The "unit" names are internal to the power sequencing framework, and
should be confined to a single provider. The names are only
informational. Dependencies are tracked with pointers to other units.
The "target" names are the strings that the consumer uses to acquire a
descriptor with. As these remain the same, existing users will continue
to work.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Expand commit message
---
drivers/power/sequencing/pwrseq-pcie-m2.c | 41 +++++++++++++++--------
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
index 16a47f76e7ce..029ae39f6e3d 100644
--- a/drivers/power/sequencing/pwrseq-pcie-m2.c
+++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
@@ -69,46 +69,46 @@ static const struct pwrseq_unit_data *pwrseq_pcie_m2_unit_deps[] = {
NULL
};
-static int pwrseq_pci_m2_e_uart_enable(struct pwrseq_device *pwrseq)
+static int pwrseq_pci_m2_e_bt_enable(struct pwrseq_device *pwrseq)
{
struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 0);
}
-static int pwrseq_pci_m2_e_uart_disable(struct pwrseq_device *pwrseq)
+static int pwrseq_pci_m2_e_bt_disable(struct pwrseq_device *pwrseq)
{
struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 1);
}
-static const struct pwrseq_unit_data pwrseq_pcie_m2_e_uart_unit_data = {
- .name = "uart-enable",
+static const struct pwrseq_unit_data pwrseq_pcie_m2_e_bt_unit_data = {
+ .name = "bt-enable",
.deps = pwrseq_pcie_m2_unit_deps,
- .enable = pwrseq_pci_m2_e_uart_enable,
- .disable = pwrseq_pci_m2_e_uart_disable,
+ .enable = pwrseq_pci_m2_e_bt_enable,
+ .disable = pwrseq_pci_m2_e_bt_disable,
};
-static int pwrseq_pci_m2_e_pcie_enable(struct pwrseq_device *pwrseq)
+static int pwrseq_pci_m2_e_wifi_enable(struct pwrseq_device *pwrseq)
{
struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
return gpiod_set_value_cansleep(ctx->w_disable1_gpio, 0);
}
-static int pwrseq_pci_m2_e_pcie_disable(struct pwrseq_device *pwrseq)
+static int pwrseq_pci_m2_e_wifi_disable(struct pwrseq_device *pwrseq)
{
struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
return gpiod_set_value_cansleep(ctx->w_disable1_gpio, 1);
}
-static const struct pwrseq_unit_data pwrseq_pcie_m2_e_pcie_unit_data = {
- .name = "pcie-enable",
+static const struct pwrseq_unit_data pwrseq_pcie_m2_e_wifi_unit_data = {
+ .name = "wifi-enable",
.deps = pwrseq_pcie_m2_unit_deps,
- .enable = pwrseq_pci_m2_e_pcie_enable,
- .disable = pwrseq_pci_m2_e_pcie_disable,
+ .enable = pwrseq_pci_m2_e_wifi_enable,
+ .disable = pwrseq_pci_m2_e_wifi_disable,
};
static const struct pwrseq_unit_data pwrseq_pcie_m2_m_pcie_unit_data = {
@@ -130,13 +130,24 @@ static int pwrseq_pcie_m2_e_pwup_delay(struct pwrseq_device *pwrseq)
static const struct pwrseq_target_data pwrseq_pcie_m2_e_uart_target_data = {
.name = "uart",
- .unit = &pwrseq_pcie_m2_e_uart_unit_data,
+ .unit = &pwrseq_pcie_m2_e_bt_unit_data,
.post_enable = pwrseq_pcie_m2_e_pwup_delay,
};
+static const struct pwrseq_target_data pwrseq_pcie_m2_e_usb_target_data = {
+ .name = "usb",
+ .unit = &pwrseq_pcie_m2_e_bt_unit_data,
+};
+
static const struct pwrseq_target_data pwrseq_pcie_m2_e_pcie_target_data = {
.name = "pcie",
- .unit = &pwrseq_pcie_m2_e_pcie_unit_data,
+ .unit = &pwrseq_pcie_m2_e_wifi_unit_data,
+ .post_enable = pwrseq_pcie_m2_e_pwup_delay,
+};
+
+static const struct pwrseq_target_data pwrseq_pcie_m2_e_sdio_target_data = {
+ .name = "sdio",
+ .unit = &pwrseq_pcie_m2_e_wifi_unit_data,
.post_enable = pwrseq_pcie_m2_e_pwup_delay,
};
@@ -147,7 +158,9 @@ static const struct pwrseq_target_data pwrseq_pcie_m2_m_pcie_target_data = {
static const struct pwrseq_target_data *pwrseq_pcie_m2_e_targets[] = {
&pwrseq_pcie_m2_e_pcie_target_data,
+ &pwrseq_pcie_m2_e_sdio_target_data,
&pwrseq_pcie_m2_e_uart_target_data,
+ &pwrseq_pcie_m2_e_usb_target_data,
NULL
};
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 12/13] arm64: dts: mediatek: mt8195-cherry: Add M.2 E-key slot
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (10 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 11/13] power: sequencing: pcie-m2: Add usb and sdio targets for E-key connector Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 11:03 ` [PATCH v3 13/13] arm64: dts: mediatek: mt8188-geralt: Add WiFi/BT as " Chen-Yu Tsai
2026-07-03 13:19 ` [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Bartosz Golaszewski
13 siblings, 0 replies; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern
The Mt8195 Cherry design features an M.2 E-key slot for WiFi/BT combo
cards. Only PCIe and USB are wired from the SoC to the slot, along with
some auxiliary signals.
Add the proper representation for it, replacing the PCIe wifi node and
vpcie3v3-supply property under the PCIe controller, and the vbus-supply
property under the xhci3 node.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Drop default GPIO output state from kill pins pinconfig
---
.../boot/dts/mediatek/mt8195-cherry.dtsi | 73 +++++++++++++++++--
1 file changed, 68 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
index ef7afc436aef..8d4cc30d91e4 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
@@ -266,6 +266,47 @@ tboard_thermistor2: thermal-sensor-t2 {
120000 51
125000 44>;
};
+
+ wifi-bt-connector {
+ compatible = "pcie-m2-e-connector";
+ pinctrl-names = "default";
+ pinctrl-0 = <&m2_e_key_kill_pins>;
+ vpcie3v3-supply = <&pp3300_wlan>;
+ w-disable1-gpios = <&pio 61 GPIO_ACTIVE_LOW>;
+ w-disable2-gpios = <&pio 59 GPIO_ACTIVE_LOW>;
+ /* PCIe auxiliary signals wired to controller. */
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* PCIe for WiFi */
+ port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ wifi_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&pcie1_ep>;
+ };
+ };
+
+ /* USB for Bluetooth */
+ port@2 {
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ bt_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&usb3_ep>;
+ };
+ };
+
+ /* SDIO, UART and I2S not implemented */
+ };
+ };
};
&adsp {
@@ -791,14 +832,14 @@ pcie@0 {
reg = <0 0 0 0 0>;
device_type = "pci";
num-lanes = <1>;
- vpcie3v3-supply = <&pp3300_wlan>;
#address-cells = <3>;
#size-cells = <2>;
ranges;
- wifi@0 {
- reg = <0 0 0 0 0>;
- wakeup-source;
+ port {
+ pcie1_ep: endpoint {
+ remote-endpoint = <&wifi_ep>;
+ };
};
};
};
@@ -1085,6 +1126,13 @@ pins-bus {
};
};
+ m2_e_key_kill_pins: m2-e-key-kill-pins {
+ pins-kill {
+ pinmux = <PINMUX_GPIO61__FUNC_GPIO61>,
+ <PINMUX_GPIO59__FUNC_GPIO59>;
+ };
+ };
+
mmc0_pins_default: mmc0-default-pins {
pins-cmd-dat {
pinmux = <PINMUX_GPIO126__FUNC_MSDC0_DAT0>,
@@ -1637,9 +1685,24 @@ &xhci2 {
&xhci3 {
/* MT7921's USB Bluetooth has issues with USB2 LPM */
usb2-lpm-disable;
- vbus-supply = <&pp3300_wlan>;
vusb33-supply = <&mt6359_vusb_ldo_reg>;
status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb3_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&bt_ep>;
+ };
+ };
+ };
};
#include <arm/cros-ec-keyboard.dtsi>
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 13/13] arm64: dts: mediatek: mt8188-geralt: Add WiFi/BT as M.2 E-key slot
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (11 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 12/13] arm64: dts: mediatek: mt8195-cherry: Add M.2 E-key slot Chen-Yu Tsai
@ 2026-07-03 11:03 ` Chen-Yu Tsai
2026-07-03 13:19 ` [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Bartosz Golaszewski
13 siblings, 0 replies; 35+ messages in thread
From: Chen-Yu Tsai @ 2026-07-03 11:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Greg Kroah-Hartman, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-acpi, driver-core, linux-pm, linux-usb,
devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern
The MT8188 Geralt design features a chip-on-board WiFi/BT solution. This
is a M.2 E-key WiFi/BT board layout directly inserted into the mainboard
design. The connections to the rest of the board are almost the same as
if it were a separate M.2 card. The only addition is the PMU_EN pin on
the chip; on M.2 cards this would be tied to the primary power source.
Model the chip-on-board WiFi/BT solution as a M.2 E-key slot with PCIe,
USB and auxiliary signals. The PMU_EN pin, which enables the internal
power controls and regulators, is modeled as a regulator fed by the
pp3300_wlan regulator. Since power sequencing is now correctly modeled
using the M.2 E-key slot, drop the "regulator-always-on" property one
pp3300_wlan regulator. Also drop the comment in xhci2 saying "MT7921's
power is controlled by PCIe".
Also drop the voltage range on the pp3300_wlan regulator. This
"regulator" is just a load switch and does not provide any regulation.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v2:
- Drop default GPIO output state from kill pins pinconfig
---
.../boot/dts/mediatek/mt8188-geralt.dtsi | 92 ++++++++++++++++++-
1 file changed, 88 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi
index f382f90c48f5..8316dd41ba23 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi
@@ -86,13 +86,11 @@ pp3300_z1: regulator-pp3300-z1 {
pp3300_wlan: regulator-pp3300-wlan {
compatible = "regulator-fixed";
regulator-name = "pp3300_wlan";
- regulator-always-on;
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3300000>;
enable-active-high;
gpio = <&pio 12 GPIO_ACTIVE_HIGH>;
pinctrl-0 = <&wlan_en>;
pinctrl-names = "default";
+ /* load switch */
vin-supply = <&pp3300_z1>;
};
@@ -159,6 +157,17 @@ ppvar_mipi_disp_avee: regulator-ppvar-mipi-disp-avee {
vin-supply = <&pp5000_z1>;
};
+ /* PMU_EN pin controls internal regulators and power sequence */
+ wlan_pmu: regulator-wlan-pmu {
+ compatible = "regulator-fixed";
+ regulator-name = "wlan-pmu";
+ enable-active-high;
+ gpio = <&pio 145 GPIO_ACTIVE_HIGH>;
+ pinctrl-0 = <&wlan_pmu_en>;
+ pinctrl-names = "default";
+ vin-supply = <&pp3300_wlan>;
+ };
+
reserved_memory: reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
@@ -193,6 +202,39 @@ adsp_dma_mem: memory@61000000 {
no-map;
};
};
+
+ wifi-bt-connector {
+ compatible = "pcie-m2-e-connector";
+ pinctrl-names = "default";
+ pinctrl-0 = <&m2_e_key_kill_pins>;
+ vpcie1v8-supply = <&mt6359_vcn18_ldo_reg>;
+ vpcie3v3-supply = <&wlan_pmu>;
+ w-disable1-gpios = <&pio 13 GPIO_ACTIVE_LOW>;
+ w-disable2-gpios = <&pio 14 GPIO_ACTIVE_LOW>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* PCIe for WiFi */
+ port@0 {
+ reg = <0>;
+
+ wifi_ep: endpoint {
+ remote-endpoint = <&pcie_ep>;
+ };
+ };
+
+ /* USB for Bluetooth */
+ port@2 {
+ reg = <2>;
+
+ bt_ep: endpoint {
+ remote-endpoint = <&usb2_ep>;
+ };
+ };
+ };
+ };
};
&adsp {
@@ -657,6 +699,22 @@ &pcie {
pinctrl-names = "default";
pinctrl-0 = <&pcie_pins>;
status = "okay";
+
+ pcie@0 {
+ compatible = "pciclass,0604";
+ reg = <0 0 0 0 0>;
+ device_type = "pci";
+ num-lanes = <1>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+
+ port {
+ pcie_ep: endpoint {
+ remote-endpoint = <&wifi_ep>;
+ };
+ };
+ };
};
&pciephy {
@@ -1000,6 +1058,13 @@ pins-bus {
};
};
+ m2_e_key_kill_pins: m2-e-key-kill-pins {
+ pins-kill {
+ pinmux = <PINMUX_GPIO13__FUNC_B_GPIO13>,
+ <PINMUX_GPIO14__FUNC_B_GPIO14>;
+ };
+ };
+
mipi_disp_avdd_en: mipi-disp-avdd-en-pins {
pins-en-ppvar-mipi-disp {
pinmux = <PINMUX_GPIO3__FUNC_B_GPIO3>;
@@ -1164,6 +1229,13 @@ pins-bus {
};
};
+ wlan_pmu_en: wlan-pmu-en-pins {
+ pins-wlan-pmu-en {
+ pinmux = <PINMUX_GPIO145__FUNC_B_GPIO145>;
+ output-low;
+ };
+ };
+
wlan_en: wlan-en-pins {
pins-en-pp3300-wlan {
pinmux = <PINMUX_GPIO12__FUNC_B_GPIO12>;
@@ -1343,10 +1415,22 @@ vdosys1_ep_ext: endpoint@1 {
};
&xhci2 {
- /* no power supply since MT7921's power is controlled by PCIe */
/* MT7921's USB BT has issues with USB2 LPM */
usb2-lpm-disable;
status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <1>;
+
+ usb2_ep: endpoint {
+ remote-endpoint = <&bt_ep>;
+ };
+ };
+ };
};
#include <arm/cros-ec-keyboard.dtsi>
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks
2026-07-03 11:03 [PATCH v3 00/13] arm64: mediatek: Add M.2 E-key slot on Chromebooks Chen-Yu Tsai
` (12 preceding siblings ...)
2026-07-03 11:03 ` [PATCH v3 13/13] arm64: dts: mediatek: mt8188-geralt: Add WiFi/BT as " Chen-Yu Tsai
@ 2026-07-03 13:19 ` Bartosz Golaszewski
13 siblings, 0 replies; 35+ messages in thread
From: Bartosz Golaszewski @ 2026-07-03 13:19 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam, Alan Stern, Bartosz Golaszewski,
Greg Kroah-Hartman, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno
On Fri, 3 Jul 2026 13:03:01 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
>
> This series unfortunately spans multiple trees. The way I see it:
>
> - Patch 1 and 2 go through the driver core, and an immutable tag is
> provided to be merged together with the USB patches.
>
> - Patch 3 gets an ack from Bartosz, and goes through the USB tree.
>
I gave my ack but I would prefer to have this in my tree as well, so an
immutable branch would be appreciated.
Bartosz
> - Patch 4 through 9 (all the USB related ones) go through the USB
> tree, along with the dependencies above.
>
> - Patch 10 and 11 go through the power sequencing tree.
>
> - Patch 12 and 13 (device tree only) go through the soc tree via the
> mediatek tree.
>
^ permalink raw reply [flat|nested] 35+ messages in thread