* [PATCH v5 1/9] dt-bindings: soc: qcom: eud: Add per-path child nodes for UTMI routing
2026-08-03 3:15 [PATCH v5 0/9] Improve Qualcomm EUD driver and platform support Elson Serrao
@ 2026-08-03 3:15 ` Elson Serrao
2026-08-03 3:23 ` sashiko-bot
2026-08-03 3:15 ` [PATCH v5 2/9] usb: misc: qcom_eud: add sysfs attribute for port selection Elson Serrao
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Elson Serrao @ 2026-08-03 3:15 UTC (permalink / raw)
To: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Peter Chen, Rob Herring,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel
The Qualcomm Embedded USB Debugger (EUD) can intercept one or two
High-Speed UTMI paths depending on the SoC. For example, SC7280 supports
EUD on the primary UTMI path only, while SM8350 supports EUD on both
primary and secondary UTMI paths.
Each UTMI path connects an independent USB controller/connector pair and
forms its own role-switch domain. EUD routes traffic based on USB role.
In device role the debug hub is inserted, while in host role it is
bypassed.
The existing binding models EUD using a flattened ports representation.
While sufficient for fixed device-role configurations, it cannot naturally
associate a controller, connector, and role switch with a specific
EUD-routable path. This is problematic for role-switch capable ports and
becomes more pronounced on SoCs with multiple paths. In such cases, each
path forms an independent role-switch domain and requires per-path role
awareness.
Model each UTMI path as a separate child node with its own OF graph. Add
an SM8350 compatible for dual-path EUD topologies while retaining the
legacy ports representation for backward compatibility.
Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
---
.../bindings/soc/qcom/qcom,eud.yaml | 128 +++++++++++++++++-
1 file changed, 127 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
index 84218636c0d8..6fef958487c8 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
@@ -18,6 +18,7 @@ properties:
items:
- enum:
- qcom,sc7280-eud
+ - qcom,sm8350-eud
- const: qcom,eud
reg:
@@ -45,10 +46,82 @@ properties:
$ref: /schemas/graph.yaml#/properties/port
description: This port is to be attached to the type C connector.
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+patternProperties:
+ "^eud-path@[0-1]$":
+ type: object
+ description:
+ Represents one High-Speed UTMI path that EUD intercepts. Use eud-path nodes
+ to associate role-switching behavior with specific port connections, allowing
+ EUD to manage role transitions independently for each UTMI path.
+
+ properties:
+ reg:
+ maxItems: 1
+ description: Path number
+
+ usb-role-switch:
+ type: boolean
+ description:
+ Indicates that EUD relays USB role-switch events for this path and
+ performs role-dependent UTMI routing accordingly. In device role,
+ the EUD hub is inserted into this path to enable debug
+ functionality. In host role, the EUD hub is bypassed and UTMI
+ traffic flows directly between the PHY and the USB controller.
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+ description:
+ These ports are to be attached to the endpoint of the USB controller node
+ and USB connector node.
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: This port is to be attached to the USB controller.
+
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: This port is to be attached to the USB connector.
+
+ required:
+ - reg
+ - ports
+
+ additionalProperties: false
+
required:
- compatible
- reg
- - ports
+
+oneOf:
+ - required:
+ - ports
+ - allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,sc7280-eud
+ then:
+ required:
+ - eud-path@0
+ properties:
+ eud-path@1: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,sm8350-eud
+ then:
+ required:
+ - eud-path@0
+ - eud-path@1
additionalProperties: false
@@ -77,3 +150,56 @@ examples:
};
};
};
+ - |
+ eud@88e0000 {
+ compatible = "qcom,sm8350-eud", "qcom,eud";
+ reg = <0x88e0000 0x2000>,
+ <0x88e2000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ eud-path@0 {
+ reg = <0>;
+ usb-role-switch;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ eud_usb0: endpoint {
+ remote-endpoint = <&usb_1_dwc3_hs>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ eud_con0: endpoint {
+ remote-endpoint = <&pmic_glink_hs_in>;
+ };
+ };
+ };
+ };
+
+ eud-path@1 {
+ reg = <1>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ eud_usb1: endpoint {
+ remote-endpoint = <&usb_2_dwc3_hs>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ eud_con1: endpoint {
+ remote-endpoint = <&conn_2_hs_in>;
+ };
+ };
+ };
+ };
+ };
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 1/9] dt-bindings: soc: qcom: eud: Add per-path child nodes for UTMI routing
2026-08-03 3:15 ` [PATCH v5 1/9] dt-bindings: soc: qcom: eud: Add per-path child nodes for UTMI routing Elson Serrao
@ 2026-08-03 3:23 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 3:23 UTC (permalink / raw)
To: Elson Serrao; +Cc: devicetree, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `oneOf` block fails to strictly enforce mutual exclusivity and compatible-specific requirements, allowing mixed and invalid configurations.
--
--- Patch [1]: [PATCH v5 1/9] dt-bindings: soc: qcom: eud: Add per-path child nodes for UTMI routing ---
commit d07ffdab4981374b8db10b3e58b93f23b98e8107
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>
dt-bindings: soc: qcom: eud: Add per-path child nodes for UTMI routing
The Qualcomm Embedded USB Debugger (EUD) can intercept one or two
High-Speed UTMI paths depending on the SoC. For example, SC7280 supports
EUD on the primary UTMI path only, while SM8350 supports EUD on both
primary and secondary UTMI paths.
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
> index 84218636c0d8d..6fef958487c8d 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
[ ... ]
> required:
> - compatible
> - reg
> - - ports
> +
> +oneOf:
> + - required:
> + - ports
> + - allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: qcom,sc7280-eud
> + then:
> + required:
> + - eud-path@0
> + properties:
> + eud-path@1: false
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: qcom,sm8350-eud
> + then:
> + required:
> + - eud-path@0
> + - eud-path@1
[Severity: Medium]
Does this oneOf block introduce a regression by allowing mixed or invalid
configurations to pass schema validation?
Since oneOf validates if exactly one of its branches passes, a device tree
could provide both the legacy ports property and invalid eud-path nodes
(such as eud-path@1 on qcom,sc7280-eud).
The input would satisfy the first branch (required: ports) but fail the
second branch's constraints. Because exactly one branch passes, the
validation would incorrectly succeed.
Could we restructure this to strictly enforce mutual exclusivity between the
legacy and new representations?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803031534.2942628-1-elson.serrao@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 2/9] usb: misc: qcom_eud: add sysfs attribute for port selection
2026-08-03 3:15 [PATCH v5 0/9] Improve Qualcomm EUD driver and platform support Elson Serrao
2026-08-03 3:15 ` [PATCH v5 1/9] dt-bindings: soc: qcom: eud: Add per-path child nodes for UTMI routing Elson Serrao
@ 2026-08-03 3:15 ` Elson Serrao
2026-08-03 3:25 ` sashiko-bot
2026-08-03 3:15 ` [PATCH v5 3/9] usb: misc: qcom_eud: add per-path High-Speed PHY control Elson Serrao
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Elson Serrao @ 2026-08-03 3:15 UTC (permalink / raw)
To: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Peter Chen, Rob Herring,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel, Konrad Dybcio
EUD can be mapped to either the primary USB port or the secondary USB port
depending on the value of the EUD_PORT_SEL register. Add a 'port' sysfs
attribute to allow userspace to select which port EUD should operate on
and update the ABI documentation. This is needed for systems with dual
USB ports where EUD needs to be accessible on either port depending on the
system configuration and use case.
Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
Reviewed-by: Peter Chen <peter.chen@kernel.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Documentation/ABI/testing/sysfs-driver-eud | 18 ++++++++++
drivers/usb/misc/qcom_eud.c | 39 ++++++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
index 2bab0db2d2f0..fb927a003596 100644
--- a/Documentation/ABI/testing/sysfs-driver-eud
+++ b/Documentation/ABI/testing/sysfs-driver-eud
@@ -7,3 +7,21 @@ Description:
EUD based on a 1 or a 0 value. By enabling EUD,
the user is able to activate the mini-usb hub of
EUD for debug and trace capabilities.
+
+What: /sys/bus/platform/drivers/qcom_eud/.../port
+Date: January 2026
+Contact: Elson Serrao <elson.serrao@oss.qualcomm.com>
+Description:
+ Selects which USB port the Embedded USB Debugger (EUD)
+ is mapped to on platforms providing multiple High-Speed
+ USB ports.
+
+ Valid values:
+ primary - Primary USB port
+ secondary - Secondary USB port
+
+ Writing any other value returns -EINVAL.
+
+ The attribute is writable only while EUD is disabled.
+ Writing it while EUD is enabled returns -EBUSY. Reading
+ the attribute returns the currently selected USB port.
diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 926419ca560f..25a6a8f110ec 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -23,14 +23,22 @@
#define EUD_REG_VBUS_INT_CLR 0x0080
#define EUD_REG_CSR_EUD_EN 0x1014
#define EUD_REG_SW_ATTACH_DET 0x1018
+#define EUD_REG_PORT_SEL 0x1028
#define EUD_REG_EUD_EN2 0x0000
+#define EUD_MAX_PORTS 2
+
#define EUD_ENABLE BIT(0)
#define EUD_INT_PET_EUD BIT(0)
#define EUD_INT_VBUS BIT(2)
#define EUD_INT_SAFE_MODE BIT(4)
#define EUD_INT_ALL (EUD_INT_VBUS | EUD_INT_SAFE_MODE)
+static const char * const eud_port_names[] = {
+ "primary",
+ "secondary",
+};
+
struct eud_chip {
struct device *dev;
struct usb_role_switch *role_sw;
@@ -40,6 +48,7 @@ struct eud_chip {
int irq;
bool enabled;
bool usb_attached;
+ u8 port_idx;
};
static int enable_eud(struct eud_chip *priv)
@@ -104,8 +113,38 @@ static ssize_t enable_store(struct device *dev,
static DEVICE_ATTR_RW(enable);
+static ssize_t port_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct eud_chip *chip = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%s\n", eud_port_names[chip->port_idx]);
+}
+
+static ssize_t port_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct eud_chip *chip = dev_get_drvdata(dev);
+ int port;
+
+ port = sysfs_match_string(eud_port_names, buf);
+ if (port < 0)
+ return port;
+
+ /* Port selection must be done before enabling EUD */
+ if (chip->enabled)
+ return -EBUSY;
+
+ writel(port, chip->base + EUD_REG_PORT_SEL);
+ chip->port_idx = port;
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(port);
+
static struct attribute *eud_attrs[] = {
&dev_attr_enable.attr,
+ &dev_attr_port.attr,
NULL,
};
ATTRIBUTE_GROUPS(eud);
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 2/9] usb: misc: qcom_eud: add sysfs attribute for port selection
2026-08-03 3:15 ` [PATCH v5 2/9] usb: misc: qcom_eud: add sysfs attribute for port selection Elson Serrao
@ 2026-08-03 3:25 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 3:25 UTC (permalink / raw)
To: Elson Serrao; +Cc: conor+dt, robh, devicetree
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Lockless race condition between `enable_store` and `port_store` allows unsafe concurrent hardware register mutation.
- [Medium] The initial software state for the USB port is out of sync with the uninitialized hardware register at probe time.
Pre-existing issues:
- [High] The pre-existing failure to clear `chip->enabled` when disabling the EUD permanently locks the newly added `port` sysfs attribute.
- [High] Calling `readl_poll_timeout()` from hardirq context causes a 'scheduling while atomic' panic.
--
--- Patch [2]: [PATCH v5 2/9] usb: misc: qcom_eud: add sysfs attribute for port selection ---
commit d4f0438c23f8813f8cdd1221aa0f9078442b7817
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>
usb: misc: qcom_eud: add sysfs attribute for port selection
EUD can be mapped to either the primary USB port or the secondary USB port
depending on the value of the EUD_PORT_SEL register. Add a 'port' sysfs
attribute to allow userspace to select which port EUD should operate on
and update the ABI documentation.
[ ... ]
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 926419ca560fc..25a6a8f110ec8 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
> @@ -40,6 +48,7 @@ struct eud_chip {
> int irq;
> bool enabled;
> bool usb_attached;
> + u8 port_idx;
[Severity: Medium]
Does the initial software state of the USB port get out of sync with the
uninitialized hardware register at probe time?
During eud_probe(), devm_kzalloc() implicitly zeroes chip->port_idx, which
corresponds to 'primary':
drivers/usb/misc/qcom_eud.c:eud_probe() {
...
chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
...
}
However, the driver's probe sequence never writes to EUD_REG_PORT_SEL to
enforce this default, nor does it read the register to populate chip->port_idx.
If the hardware defaults to or is left in the 'secondary' state by a bootloader,
the sysfs attribute will incorrectly report the 'primary' port is selected.
> };
[ ... ]
> @@ -104,8 +113,38 @@ static ssize_t enable_store(struct device *dev,
>
> static DEVICE_ATTR_RW(enable);
>
> +static ssize_t port_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct eud_chip *chip = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%s\n", eud_port_names[chip->port_idx]);
> +}
> +
> +static ssize_t port_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct eud_chip *chip = dev_get_drvdata(dev);
> + int port;
> +
> + port = sysfs_match_string(eud_port_names, buf);
> + if (port < 0)
> + return port;
> +
> + /* Port selection must be done before enabling EUD */
> + if (chip->enabled)
> + return -EBUSY;
[Severity: High]
This is a pre-existing issue, but because enable_store() never sets
chip->enabled to false when disabling the device, will this check
permanently lock out port changes once the EUD has been enabled?
If a user writes '1' to /sys/.../enable, and then writes '0', the device
successfully disables. However, because chip->enabled is never updated to false:
drivers/usb/misc/qcom_eud.c:enable_store() {
...
} else {
ret = disable_eud(chip);
/* chip->enabled = false; is missing */
}
...
}
Any subsequent write to /sys/.../port will evaluate chip->enabled as true and
reject the operation with -EBUSY indefinitely.
[Severity: High]
Does this lockless check allow unsafe concurrent hardware register mutation?
If there are concurrent writes to the enable and port sysfs attributes,
port_store() validates chip->enabled without taking a lock. Concurrently,
enable_store() could call enable_eud(), which turns on the hardware and sets
chip->enabled to true:
drivers/usb/misc/qcom_eud.c:enable_store() {
...
if (enable) {
ret = enable_eud(chip);
if (!ret)
chip->enabled = enable;
...
}
This would allow port_store() to overwrite the EUD_REG_PORT_SEL register while
the EUD is actively being enabled, leading to undefined hardware behavior.
> +
> + writel(port, chip->base + EUD_REG_PORT_SEL);
> + chip->port_idx = port;
> +
> + return count;
> +}
[ ... ]
[Severity: High]
This is a pre-existing issue, but does calling readl_poll_timeout() from
hardirq context cause a 'scheduling while atomic' panic?
The hardware triggers an EUD_INT_SAFE_MODE interrupt, and the primary
interrupt handler handle_eud_irq() invokes pet_eud():
drivers/usb/misc/qcom_eud.c:handle_eud_irq() {
...
case EUD_INT_SAFE_MODE:
pet_eud(chip);
return IRQ_HANDLED;
...
}
Inside pet_eud(), readl_poll_timeout() is called with a 1-microsecond delay:
drivers/usb/misc/qcom_eud.c:pet_eud() {
...
ret = readl_poll_timeout(chip->base + EUD_REG_SW_ATTACH_DET,
reg, (reg == 0), 1, 100);
...
}
Since readl_poll_timeout() internally expands to usleep_range(), it attempts
to sleep while interrupts are disabled, which triggers a panic.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803031534.2942628-1-elson.serrao@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 3/9] usb: misc: qcom_eud: add per-path High-Speed PHY control
2026-08-03 3:15 [PATCH v5 0/9] Improve Qualcomm EUD driver and platform support Elson Serrao
2026-08-03 3:15 ` [PATCH v5 1/9] dt-bindings: soc: qcom: eud: Add per-path child nodes for UTMI routing Elson Serrao
2026-08-03 3:15 ` [PATCH v5 2/9] usb: misc: qcom_eud: add sysfs attribute for port selection Elson Serrao
@ 2026-08-03 3:15 ` Elson Serrao
2026-08-03 3:28 ` sashiko-bot
2026-08-03 3:15 ` [PATCH v5 4/9] usb: misc: qcom_eud: add per-path role switch support Elson Serrao
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Elson Serrao @ 2026-08-03 3:15 UTC (permalink / raw)
To: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Peter Chen, Rob Herring,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel
EUD hardware can support multiple High-Speed USB paths, each routed
through its own PHY. The active path is selected in hardware via the
EUD_PORT_SEL register. As a High-Speed hub, EUD requires access to
the High-Speed PHY associated with the active path. To support this
multi-path capability, the driver must manage PHY resources on a
per-path basis, ensuring that the PHY for the currently selected
path is properly initialized and powered.
This patch restructures the driver to implement per-path PHY management.
The driver now powers the appropriate PHY based on the selected and
enabled UTMI path, ensuring correct operation when EUD is enabled.
Historically, EUD appeared to work on single-path systems because the
USB controller kept the PHY initialized. However, EUD is designed to
operate independently of the USB controller and therefore requires
explicit PHY control for proper operation.
Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
Reviewed-by: Peter Chen <peter.chen@kernel.org>
---
Documentation/ABI/testing/sysfs-driver-eud | 3 +-
drivers/usb/misc/qcom_eud.c | 124 ++++++++++++++++++++-
2 files changed, 125 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
index fb927a003596..f20b82b5a38a 100644
--- a/Documentation/ABI/testing/sysfs-driver-eud
+++ b/Documentation/ABI/testing/sysfs-driver-eud
@@ -20,7 +20,8 @@ Description:
primary - Primary USB port
secondary - Secondary USB port
- Writing any other value returns -EINVAL.
+ Writing any other value returns -EINVAL. Writing a port
+ not supported on the current platform returns -EOPNOTSUPP.
The attribute is writable only while EUD is disabled.
Writing it while EUD is enabled returns -EBUSY. Reading
diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 25a6a8f110ec..03d1c200658c 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -11,6 +11,8 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/sysfs.h>
@@ -39,26 +41,84 @@ static const char * const eud_port_names[] = {
"secondary",
};
+struct eud_path {
+ struct eud_chip *chip;
+ struct phy *phy;
+ u8 num;
+};
+
struct eud_chip {
struct device *dev;
struct usb_role_switch *role_sw;
void __iomem *base;
+ struct eud_path *paths[EUD_MAX_PORTS];
phys_addr_t mode_mgr;
unsigned int int_status;
int irq;
bool enabled;
bool usb_attached;
+ bool phy_enabled;
u8 port_idx;
};
+static int eud_phy_enable(struct eud_chip *chip)
+{
+ struct phy *phy;
+ int ret;
+
+ if (chip->phy_enabled)
+ return 0;
+
+ phy = chip->paths[chip->port_idx]->phy;
+
+ ret = phy_init(phy);
+ if (ret) {
+ dev_err(chip->dev, "Failed to initialize USB2 PHY for port %u: %d\n",
+ chip->port_idx, ret);
+ return ret;
+ }
+
+ ret = phy_power_on(phy);
+ if (ret) {
+ dev_err(chip->dev, "Failed to power on USB2 PHY for port %u: %d\n",
+ chip->port_idx, ret);
+ phy_exit(phy);
+ return ret;
+ }
+
+ chip->phy_enabled = true;
+
+ return 0;
+}
+
+static void eud_phy_disable(struct eud_chip *chip)
+{
+ struct phy *phy;
+
+ if (!chip->phy_enabled)
+ return;
+
+ phy = chip->paths[chip->port_idx]->phy;
+
+ phy_power_off(phy);
+ phy_exit(phy);
+ chip->phy_enabled = false;
+}
+
static int enable_eud(struct eud_chip *priv)
{
int ret;
- ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 1);
+ ret = eud_phy_enable(priv);
if (ret)
return ret;
+ ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 1);
+ if (ret) {
+ eud_phy_disable(priv);
+ return ret;
+ }
+
writel(EUD_ENABLE, priv->base + EUD_REG_CSR_EUD_EN);
writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
priv->base + EUD_REG_INT1_EN_MASK);
@@ -75,6 +135,8 @@ static int disable_eud(struct eud_chip *priv)
return ret;
writel(0, priv->base + EUD_REG_CSR_EUD_EN);
+ eud_phy_disable(priv);
+
return 0;
}
@@ -130,6 +192,10 @@ static ssize_t port_store(struct device *dev, struct device_attribute *attr,
if (port < 0)
return port;
+ /* Check if the corresponding path is available */
+ if (!chip->paths[port])
+ return -EOPNOTSUPP;
+
/* Port selection must be done before enabling EUD */
if (chip->enabled)
return -EBUSY;
@@ -227,8 +293,54 @@ static void eud_role_switch_release(void *data)
usb_role_switch_put(chip->role_sw);
}
+static int eud_init_path(struct eud_chip *chip, struct device_node *np)
+{
+ struct device_node *controller_node;
+ struct eud_path *path;
+ u32 path_num;
+ int ret;
+
+ ret = of_property_read_u32(np, "reg", &path_num);
+ if (ret) {
+ /* Legacy DT uses 'ports' node without 'reg' property; treat as path 0 */
+ if (of_node_name_eq(np, "ports"))
+ path_num = 0;
+ else
+ return dev_err_probe(chip->dev, ret, "unexpected child node '%s'\n",
+ np->name);
+ }
+
+ if (path_num >= EUD_MAX_PORTS)
+ return dev_err_probe(chip->dev, -EINVAL, "invalid path number: %u (max %d)\n",
+ path_num, EUD_MAX_PORTS - 1);
+
+ path = devm_kzalloc(chip->dev, sizeof(*path), GFP_KERNEL);
+ if (!path)
+ return -ENOMEM;
+
+ path->chip = chip;
+ path->num = path_num;
+
+ controller_node = of_graph_get_remote_node(np, 0, -1);
+ if (!controller_node)
+ return dev_err_probe(chip->dev, -ENODEV,
+ "failed to get controller node for path %u\n", path_num);
+
+ path->phy = devm_of_phy_get_by_index(chip->dev, controller_node, 0);
+ of_node_put(controller_node);
+
+ if (IS_ERR(path->phy))
+ return dev_err_probe(chip->dev, PTR_ERR(path->phy),
+ "failed to get PHY for path %d\n", path_num);
+
+ chip->paths[path_num] = path;
+
+ return 0;
+}
+
static int eud_probe(struct platform_device *pdev)
{
+ struct device_node *np = pdev->dev.of_node;
struct eud_chip *chip;
struct resource *res;
int ret;
@@ -239,6 +351,16 @@ static int eud_probe(struct platform_device *pdev)
chip->dev = &pdev->dev;
+ for_each_child_of_node_scoped(np, child) {
+ ret = eud_init_path(chip, child);
+ if (ret)
+ return ret;
+ }
+
+ /* Primary path is mandatory. Secondary is optional */
+ if (!chip->paths[0])
+ return dev_err_probe(chip->dev, -ENODEV, "primary path not found\n");
+
chip->role_sw = usb_role_switch_get(&pdev->dev);
if (IS_ERR(chip->role_sw))
return dev_err_probe(chip->dev, PTR_ERR(chip->role_sw),
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 3/9] usb: misc: qcom_eud: add per-path High-Speed PHY control
2026-08-03 3:15 ` [PATCH v5 3/9] usb: misc: qcom_eud: add per-path High-Speed PHY control Elson Serrao
@ 2026-08-03 3:28 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 3:28 UTC (permalink / raw)
To: Elson Serrao; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] A race condition between `enable_store` and `port_store` leads to unbalanced PHY power operations and resource leaks due to lockless state modification.
- [High] Concurrent writes to the `enable` sysfs attribute lack serialization, causing double-initialization and leakage of PHY resources.
Pre-existing issues:
- [High] The sysfs `enable_store` callback fails to clear the `chip->enabled` flag when disabling the EUD, permanently locking the device state.
--
--- Patch [3]: [PATCH v5 3/9] usb: misc: qcom_eud: add per-path High-Speed PHY control ---
commit 577fea81d5ebf5fe65bcee04faa29252dcef08d7
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>
usb: misc: qcom_eud: add per-path High-Speed PHY control
EUD hardware can support multiple High-Speed USB paths, each routed
through its own PHY. The active path is selected in hardware via the
EUD_PORT_SEL register. As a High-Speed hub, EUD requires access to
the High-Speed PHY associated with the active path. To support this
multi-path capability, the driver must manage PHY resources on a
per-path basis, ensuring that the PHY for the currently selected
path is properly initialized and powered.
[ ... ]
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 25a6a8f110ec8..03d1c200658c8 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
[ ... ]
> @@ -39,26 +41,84 @@ static const char * const eud_port_names[] = {
> "secondary",
> };
>
> +struct eud_path {
> + struct eud_chip *chip;
> + struct phy *phy;
> + u8 num;
> +};
> +
> struct eud_chip {
> struct device *dev;
> struct usb_role_switch *role_sw;
> void __iomem *base;
> + struct eud_path *paths[EUD_MAX_PORTS];
> phys_addr_t mode_mgr;
> unsigned int int_status;
> int irq;
> bool enabled;
> bool usb_attached;
> + bool phy_enabled;
> u8 port_idx;
> };
>
> +static int eud_phy_enable(struct eud_chip *chip)
> +{
> + struct phy *phy;
> + int ret;
> +
> + if (chip->phy_enabled)
> + return 0;
[Severity: High]
Does this lack serialization for concurrent sysfs writes? If two threads
concurrently write '1' to the 'enable' attribute, both could invoke
enable_eud() and reach here simultaneously.
Since there is no locking, both threads could evaluate chip->phy_enabled as
false at the same time and proceed to call phy_init() and phy_power_on().
Would this double-increment the PHY's reference counts, meaning a later call
to eud_phy_disable() would only execute once and leave the PHY references
permanently leaked?
> +
> + phy = chip->paths[chip->port_idx]->phy;
> +
> + ret = phy_init(phy);
[ ... ]
> @@ -130,6 +192,10 @@ static ssize_t port_store(struct device *dev, struct device_attribute *attr,
> if (port < 0)
> return port;
>
> + /* Check if the corresponding path is available */
> + if (!chip->paths[port])
> + return -EOPNOTSUPP;
> +
> /* Port selection must be done before enabling EUD */
> if (chip->enabled)
> return -EBUSY;
> writel(port, chip->base + EUD_REG_PORT_SEL);
> chip->port_idx = port;
[Severity: High]
Can this race with enable_store()? If thread A writes to the 'enable'
attribute and calls enable_eud(), which enables the PHY for the current
port_idx, could thread B write to 'port' before thread A sets
chip->enabled = true?
Thread A in enable_store():
ret = enable_eud(chip);
if (!ret)
chip->enabled = enable;
Thread B in port_store():
if (chip->enabled)
return -EBUSY;
writel(port, chip->base + EUD_REG_PORT_SEL);
chip->port_idx = port;
If thread B updates chip->port_idx locklessly during this window, wouldn't a
subsequent disable operation in eud_phy_disable() use the new port_idx,
disabling a PHY that was never enabled and leaking the one that was?
[Severity: High]
This is a pre-existing issue, but does enable_store() fail to clear the
chip->enabled flag when disabling the EUD?
Looking at enable_store() in drivers/usb/misc/qcom_eud.c:
if (enable) {
ret = enable_eud(chip);
} else {
ret = disable_eud(chip);
}
If the else branch handles disabling but omits clearing chip->enabled,
wouldn't writing '0' to 'enable' disable the hardware while leaving
chip->enabled true? This would cause subsequent attempts to write to
'port' to permanently fail with -EBUSY, breaking the new port switching
functionality introduced here.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803031534.2942628-1-elson.serrao@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 4/9] usb: misc: qcom_eud: add per-path role switch support
2026-08-03 3:15 [PATCH v5 0/9] Improve Qualcomm EUD driver and platform support Elson Serrao
` (2 preceding siblings ...)
2026-08-03 3:15 ` [PATCH v5 3/9] usb: misc: qcom_eud: add per-path High-Speed PHY control Elson Serrao
@ 2026-08-03 3:15 ` Elson Serrao
2026-08-03 3:30 ` sashiko-bot
2026-08-03 9:37 ` Peter Chen
2026-08-03 3:15 ` [PATCH v5 5/9] usb: misc: qcom_eud: improve enable_store API Elson Serrao
` (4 subsequent siblings)
8 siblings, 2 replies; 20+ messages in thread
From: Elson Serrao @ 2026-08-03 3:15 UTC (permalink / raw)
To: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Peter Chen, Rob Herring,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel
The EUD hardware can support multiple High-Speed USB paths, each connected
to different USB controllers. The current implementation uses a single
chip-level role switch, which cannot properly handle multi-path
configurations where each path needs independent role management. Since
EUD is physically present between the USB connector and the controller,
it should also relay the role change requests from the connector.
Restructure the driver to support per-path role switches and remove the
chip-level role switch. Additionally, as EUD need not modify the USB
role upon enabling, remove the unnecessary role switch call from
enable_eud().
Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
---
drivers/usb/misc/qcom_eud.c | 100 +++++++++++++++++++++++++++++-------
1 file changed, 81 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 03d1c200658c..48a4441b0c9c 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -44,12 +44,15 @@ static const char * const eud_port_names[] = {
struct eud_path {
struct eud_chip *chip;
struct phy *phy;
+ struct usb_role_switch *controller_sw;
+ struct usb_role_switch *eud_sw;
+ enum usb_role curr_role;
+ char name[16];
u8 num;
};
struct eud_chip {
struct device *dev;
- struct usb_role_switch *role_sw;
void __iomem *base;
struct eud_path *paths[EUD_MAX_PORTS];
phys_addr_t mode_mgr;
@@ -123,7 +126,7 @@ static int enable_eud(struct eud_chip *priv)
writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
priv->base + EUD_REG_INT1_EN_MASK);
- return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
+ return 0;
}
static int disable_eud(struct eud_chip *priv)
@@ -270,12 +273,15 @@ static irqreturn_t handle_eud_irq(int irq, void *data)
static irqreturn_t handle_eud_irq_thread(int irq, void *data)
{
struct eud_chip *chip = data;
+ struct eud_path *path;
int ret;
+ path = chip->paths[chip->port_idx];
+
if (chip->usb_attached)
- ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
+ ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_DEVICE);
else
- ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);
+ ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_HOST);
if (ret)
dev_err(chip->dev, "failed to set role switch\n");
@@ -286,17 +292,39 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
return IRQ_HANDLED;
}
-static void eud_role_switch_release(void *data)
+static int eud_role_switch_set(struct usb_role_switch *sw, enum usb_role role)
{
- struct eud_chip *chip = data;
+ struct eud_path *path = usb_role_switch_get_drvdata(sw);
+ int ret;
+
+ /* Forward the role request to the USB controller */
+ ret = usb_role_switch_set_role(path->controller_sw, role);
+ if (ret) {
+ dev_err(path->chip->dev, "failed to set role %s for port %u: %d\n",
+ usb_role_string(role), path->num, ret);
+ return ret;
+ }
+
+ path->curr_role = role;
- usb_role_switch_put(chip->role_sw);
+ return 0;
+}
+
+static void eud_path_role_switch_release(void *data)
+{
+ struct eud_path *path = data;
+
+ usb_role_switch_unregister(path->eud_sw);
+ usb_role_switch_put(path->controller_sw);
}
static int eud_init_path(struct eud_chip *chip, struct device_node *np)
{
+ struct usb_role_switch_desc role_sw_desc = {NULL};
+ struct usb_role_switch *sw;
struct device_node *controller_node;
struct eud_path *path;
+ const char *dr_mode = "";
u32 path_num;
int ret;
@@ -327,14 +355,57 @@ static int eud_init_path(struct eud_chip *chip, struct device_node *np)
"failed to get controller node for path %u\n", path_num);
path->phy = devm_of_phy_get_by_index(chip->dev, controller_node, 0);
- of_node_put(controller_node);
-
- if (IS_ERR(path->phy))
+ if (IS_ERR(path->phy)) {
+ of_node_put(controller_node);
return dev_err_probe(chip->dev, PTR_ERR(path->phy),
"failed to get PHY for path %d\n", path_num);
+ }
chip->paths[path_num] = path;
+ of_property_read_string(controller_node, "dr_mode", &dr_mode);
+
+ if (!strcmp(dr_mode, "host"))
+ path->curr_role = USB_ROLE_HOST;
+ else if (!strcmp(dr_mode, "peripheral"))
+ path->curr_role = USB_ROLE_DEVICE;
+ else
+ path->curr_role = USB_ROLE_NONE;
+
+ /* No role switch needed for fixed role controllers */
+ if (path->curr_role != USB_ROLE_NONE) {
+ of_node_put(controller_node);
+ return 0;
+ }
+
+ sw = usb_role_switch_find_by_fwnode(of_fwnode_handle(controller_node));
+ if (!sw) {
+ of_node_put(controller_node);
+ return dev_err_probe(chip->dev, -EPROBE_DEFER,
+ "Failed to get controller role switch for path %d\n",
+ path_num);
+ }
+ path->controller_sw = sw;
+
+ of_node_put(controller_node);
+
+ role_sw_desc.fwnode = of_fwnode_handle(np);
+ role_sw_desc.set = eud_role_switch_set;
+ role_sw_desc.driver_data = path;
+ snprintf(path->name, sizeof(path->name), "eud-path%u", path_num);
+ role_sw_desc.name = path->name;
+
+ path->eud_sw = usb_role_switch_register(chip->dev, &role_sw_desc);
+ if (IS_ERR(path->eud_sw)) {
+ usb_role_switch_put(path->controller_sw);
+ return dev_err_probe(chip->dev, PTR_ERR(path->eud_sw),
+ "Failed to register EUD role switch for path %d\n", path_num);
+ }
+
+ ret = devm_add_action_or_reset(chip->dev, eud_path_role_switch_release, path);
+ if (ret)
+ return ret;
+
return 0;
}
@@ -361,15 +432,6 @@ static int eud_probe(struct platform_device *pdev)
if (!chip->paths[0])
return dev_err_probe(chip->dev, -ENODEV, "primary path not found\n");
- chip->role_sw = usb_role_switch_get(&pdev->dev);
- if (IS_ERR(chip->role_sw))
- return dev_err_probe(chip->dev, PTR_ERR(chip->role_sw),
- "failed to get role switch\n");
-
- ret = devm_add_action_or_reset(chip->dev, eud_role_switch_release, chip);
- if (ret)
- return ret;
-
chip->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(chip->base))
return PTR_ERR(chip->base);
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 4/9] usb: misc: qcom_eud: add per-path role switch support
2026-08-03 3:15 ` [PATCH v5 4/9] usb: misc: qcom_eud: add per-path role switch support Elson Serrao
@ 2026-08-03 3:30 ` sashiko-bot
2026-08-03 9:37 ` Peter Chen
1 sibling, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 3:30 UTC (permalink / raw)
To: Elson Serrao; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Missing synchronization in sysfs handlers enable_store and port_store causes PHY reference count corruption and hardware desynchronization.
- [High] EUD remains physically enabled when the role switches to HOST, breaking host mode.
Pre-existing issues:
- [High] Sysfs enable_store fails to clear chip->enabled on disable, permanently locking port_store.
--
--- Patch [4]: [PATCH v5 4/9] usb: misc: qcom_eud: add per-path role switch support ---
commit b7772664d1b8d3bcc67e85e648539118fa47a9cd
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>
usb: misc: qcom_eud: add per-path role switch support
This commit updates the EUD driver to support per-path role switches instead
of a single chip-level switch. It allows each path to handle independent role
management by relaying requests from the connector to the controller. The
unnecessary role switch call from enable_eud() is also removed.
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 03d1c200658c8..48a4441b0c9c0 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
[ ... ]
> @@ -286,17 +292,39 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -static void eud_role_switch_release(void *data)
> +static int eud_role_switch_set(struct usb_role_switch *sw, enum usb_role role)
> {
> - struct eud_chip *chip = data;
> + struct eud_path *path = usb_role_switch_get_drvdata(sw);
> + int ret;
> +
> + /* Forward the role request to the USB controller */
> + ret = usb_role_switch_set_role(path->controller_sw, role);
> + if (ret) {
> + dev_err(path->chip->dev, "failed to set role %s for port %u: %d\n",
> + usb_role_string(role), path->num, ret);
> + return ret;
> + }
> +
> + path->curr_role = role;
> +
> + return 0;
> +}
[Severity: High]
Does the EUD remain physically enabled when the role switches to USB_ROLE_HOST?
If a role switch to USB_ROLE_HOST occurs, eud_role_switch_set() forwards the
request to the USB controller but doesn't appear to dynamically disable the
EUD hardware itself. Since EUD is only usable in device role, would keeping it
enabled during host role disrupt the UTMI lines and break the host connection?
[Severity: High]
Could the missing synchronization between the sysfs handlers enable_store()
and port_store() cause PHY reference count corruption?
Looking at drivers/usb/misc/qcom_eud.c, if Thread A executes enable_store()
while Thread B concurrently executes port_store(), port_store() can modify
chip->port_idx locklessly:
Thread B in port_store():
chip->port_idx = port;
Thread A in enable_store() would then execute enable_eud() without seeing the
updated port_idx or while it is changing:
Thread A in enable_store():
ret = enable_eud(chip);
if (!ret)
chip->enabled = enable;
Because neither function holds a lock, port_store() can bypass the
chip->enabled check. This might cause eud_phy_enable() to power on the PHY
for the new port. When disable_eud() is later called, could it disable the
wrong PHY and leak the power-on state for the old PHY?
[Severity: High]
This is a pre-existing issue, but does enable_store() fail to clear the
enabled flag on disable?
Looking at enable_store() in drivers/usb/misc/qcom_eud.c:
enable_store() {
...
} else {
ret = disable_eud(chip);
}
return ret < 0 ? ret : count;
}
If a user writes 0 to the enable sysfs attribute, this calls disable_eud(chip)
but doesn't set chip->enabled = false. Could this permanently lock
port_store(), since subsequent writes will unconditionally fail with -EBUSY
when checking if (chip->enabled)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803031534.2942628-1-elson.serrao@oss.qualcomm.com?part=4
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 4/9] usb: misc: qcom_eud: add per-path role switch support
2026-08-03 3:15 ` [PATCH v5 4/9] usb: misc: qcom_eud: add per-path role switch support Elson Serrao
2026-08-03 3:30 ` sashiko-bot
@ 2026-08-03 9:37 ` Peter Chen
1 sibling, 0 replies; 20+ messages in thread
From: Peter Chen @ 2026-08-03 9:37 UTC (permalink / raw)
To: Elson Serrao
Cc: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
linux-arm-msm, devicetree, linux-usb, linux-kernel
On 26-08-02 20:15:29, Elson Serrao wrote:
> The EUD hardware can support multiple High-Speed USB paths, each connected
> to different USB controllers. The current implementation uses a single
> chip-level role switch, which cannot properly handle multi-path
> configurations where each path needs independent role management. Since
> EUD is physically present between the USB connector and the controller,
> it should also relay the role change requests from the connector.
>
> Restructure the driver to support per-path role switches and remove the
> chip-level role switch. Additionally, as EUD need not modify the USB
> role upon enabling, remove the unnecessary role switch call from
> enable_eud().
>
> Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
Reviewed-by: Peter Chen <peter.chen@kernel.org>
Peter
> ---
> drivers/usb/misc/qcom_eud.c | 100 +++++++++++++++++++++++++++++-------
> 1 file changed, 81 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 03d1c200658c..48a4441b0c9c 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
> @@ -44,12 +44,15 @@ static const char * const eud_port_names[] = {
> struct eud_path {
> struct eud_chip *chip;
> struct phy *phy;
> + struct usb_role_switch *controller_sw;
> + struct usb_role_switch *eud_sw;
> + enum usb_role curr_role;
> + char name[16];
> u8 num;
> };
>
> struct eud_chip {
> struct device *dev;
> - struct usb_role_switch *role_sw;
> void __iomem *base;
> struct eud_path *paths[EUD_MAX_PORTS];
> phys_addr_t mode_mgr;
> @@ -123,7 +126,7 @@ static int enable_eud(struct eud_chip *priv)
> writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
> priv->base + EUD_REG_INT1_EN_MASK);
>
> - return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
> + return 0;
> }
>
> static int disable_eud(struct eud_chip *priv)
> @@ -270,12 +273,15 @@ static irqreturn_t handle_eud_irq(int irq, void *data)
> static irqreturn_t handle_eud_irq_thread(int irq, void *data)
> {
> struct eud_chip *chip = data;
> + struct eud_path *path;
> int ret;
>
> + path = chip->paths[chip->port_idx];
> +
> if (chip->usb_attached)
> - ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
> + ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_DEVICE);
> else
> - ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);
> + ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_HOST);
> if (ret)
> dev_err(chip->dev, "failed to set role switch\n");
>
> @@ -286,17 +292,39 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -static void eud_role_switch_release(void *data)
> +static int eud_role_switch_set(struct usb_role_switch *sw, enum usb_role role)
> {
> - struct eud_chip *chip = data;
> + struct eud_path *path = usb_role_switch_get_drvdata(sw);
> + int ret;
> +
> + /* Forward the role request to the USB controller */
> + ret = usb_role_switch_set_role(path->controller_sw, role);
> + if (ret) {
> + dev_err(path->chip->dev, "failed to set role %s for port %u: %d\n",
> + usb_role_string(role), path->num, ret);
> + return ret;
> + }
> +
> + path->curr_role = role;
>
> - usb_role_switch_put(chip->role_sw);
> + return 0;
> +}
> +
> +static void eud_path_role_switch_release(void *data)
> +{
> + struct eud_path *path = data;
> +
> + usb_role_switch_unregister(path->eud_sw);
> + usb_role_switch_put(path->controller_sw);
> }
>
> static int eud_init_path(struct eud_chip *chip, struct device_node *np)
> {
> + struct usb_role_switch_desc role_sw_desc = {NULL};
> + struct usb_role_switch *sw;
> struct device_node *controller_node;
> struct eud_path *path;
> + const char *dr_mode = "";
> u32 path_num;
> int ret;
>
> @@ -327,14 +355,57 @@ static int eud_init_path(struct eud_chip *chip, struct device_node *np)
> "failed to get controller node for path %u\n", path_num);
>
> path->phy = devm_of_phy_get_by_index(chip->dev, controller_node, 0);
> - of_node_put(controller_node);
> -
> - if (IS_ERR(path->phy))
> + if (IS_ERR(path->phy)) {
> + of_node_put(controller_node);
> return dev_err_probe(chip->dev, PTR_ERR(path->phy),
> "failed to get PHY for path %d\n", path_num);
> + }
>
> chip->paths[path_num] = path;
>
> + of_property_read_string(controller_node, "dr_mode", &dr_mode);
> +
> + if (!strcmp(dr_mode, "host"))
> + path->curr_role = USB_ROLE_HOST;
> + else if (!strcmp(dr_mode, "peripheral"))
> + path->curr_role = USB_ROLE_DEVICE;
> + else
> + path->curr_role = USB_ROLE_NONE;
> +
> + /* No role switch needed for fixed role controllers */
> + if (path->curr_role != USB_ROLE_NONE) {
> + of_node_put(controller_node);
> + return 0;
> + }
> +
> + sw = usb_role_switch_find_by_fwnode(of_fwnode_handle(controller_node));
> + if (!sw) {
> + of_node_put(controller_node);
> + return dev_err_probe(chip->dev, -EPROBE_DEFER,
> + "Failed to get controller role switch for path %d\n",
> + path_num);
> + }
> + path->controller_sw = sw;
> +
> + of_node_put(controller_node);
> +
> + role_sw_desc.fwnode = of_fwnode_handle(np);
> + role_sw_desc.set = eud_role_switch_set;
> + role_sw_desc.driver_data = path;
> + snprintf(path->name, sizeof(path->name), "eud-path%u", path_num);
> + role_sw_desc.name = path->name;
> +
> + path->eud_sw = usb_role_switch_register(chip->dev, &role_sw_desc);
> + if (IS_ERR(path->eud_sw)) {
> + usb_role_switch_put(path->controller_sw);
> + return dev_err_probe(chip->dev, PTR_ERR(path->eud_sw),
> + "Failed to register EUD role switch for path %d\n", path_num);
> + }
> +
> + ret = devm_add_action_or_reset(chip->dev, eud_path_role_switch_release, path);
> + if (ret)
> + return ret;
> +
> return 0;
> }
>
> @@ -361,15 +432,6 @@ static int eud_probe(struct platform_device *pdev)
> if (!chip->paths[0])
> return dev_err_probe(chip->dev, -ENODEV, "primary path not found\n");
>
> - chip->role_sw = usb_role_switch_get(&pdev->dev);
> - if (IS_ERR(chip->role_sw))
> - return dev_err_probe(chip->dev, PTR_ERR(chip->role_sw),
> - "failed to get role switch\n");
> -
> - ret = devm_add_action_or_reset(chip->dev, eud_role_switch_release, chip);
> - if (ret)
> - return ret;
> -
> chip->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(chip->base))
> return PTR_ERR(chip->base);
> --
> 2.34.1
>
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 5/9] usb: misc: qcom_eud: improve enable_store API
2026-08-03 3:15 [PATCH v5 0/9] Improve Qualcomm EUD driver and platform support Elson Serrao
` (3 preceding siblings ...)
2026-08-03 3:15 ` [PATCH v5 4/9] usb: misc: qcom_eud: add per-path role switch support Elson Serrao
@ 2026-08-03 3:15 ` Elson Serrao
2026-08-03 3:25 ` sashiko-bot
2026-08-03 3:15 ` [PATCH v5 6/9] usb: misc: qcom_eud: add role-based EUD control Elson Serrao
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Elson Serrao @ 2026-08-03 3:15 UTC (permalink / raw)
To: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Peter Chen, Rob Herring,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel, Konrad Dybcio
Currently enable_store() allows operations irrespective of the EUD state,
which can result in redundant operations. Avoid this by adding duplicate
state checks to skip requests when EUD is already in the desired state.
Additionally, improve error handling with explicit logging to provide
better feedback.
Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Peter Chen <peter.chen@kernel.org>
---
drivers/usb/misc/qcom_eud.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 48a4441b0c9c..e83a4402e4b8 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -162,18 +162,27 @@ static ssize_t enable_store(struct device *dev,
if (kstrtobool(buf, &enable))
return -EINVAL;
+ /* Skip operation if already in desired state */
+ if (chip->enabled == enable)
+ return count;
+
if (enable) {
ret = enable_eud(chip);
- if (!ret)
- chip->enabled = enable;
- else
- disable_eud(chip);
-
+ if (ret) {
+ dev_err(chip->dev, "failed to enable eud\n");
+ return ret;
+ }
} else {
ret = disable_eud(chip);
+ if (ret) {
+ dev_err(chip->dev, "failed to disable eud\n");
+ return ret;
+ }
}
- return ret < 0 ? ret : count;
+ chip->enabled = enable;
+
+ return count;
}
static DEVICE_ATTR_RW(enable);
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 5/9] usb: misc: qcom_eud: improve enable_store API
2026-08-03 3:15 ` [PATCH v5 5/9] usb: misc: qcom_eud: improve enable_store API Elson Serrao
@ 2026-08-03 3:25 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 3:25 UTC (permalink / raw)
To: Elson Serrao; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the sysfs store handlers (`enable_store` and `port_store`) lack synchronization, exposing a critical TOCTOU race and hardware state corruption. While this patch attempts to avoid redundant hardware operations by adding a check `if (chip->enabled == enable)`, the check is performed without a lock. If multiple threads concurrently write '1' to the `enable` attribute, both can observe `chip->enabled == false`, bypass the check, and invoke `enable_eud()` simultaneously. This results in duplicate unsynchronized MMIO writes and unbalanced calls to `phy_init()` and `phy_power_on()`, permanently leaking PHY internal power/init reference counts. Furthermore, because `port_store` is also unprotected, a concurrent write to `port` can modify `chip->port_idx` while `enable_eud()` is executing. This can cause the driver to enable the PHY on one port but later disable the PHY on the other port, leading to permanent hardware resource leaks and corrupted PHY state.
--
--- Patch [5]: [PATCH v5 5/9] usb: misc: qcom_eud: improve enable_store API ---
commit daa70bf1af36bae927820d34ca275135da4718d1
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>
usb: misc: qcom_eud: improve enable_store API
This commit adds a state check to enable_store() to skip redundant
operations when the EUD is already in the desired state. It also
improves error handling by explicitly logging failures.
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
> @@ -162,18 +162,27 @@ static ssize_t enable_store(struct device *dev,
> if (kstrtobool(buf, &enable))
> return -EINVAL;
>
> + /* Skip operation if already in desired state */
> + if (chip->enabled == enable)
> + return count;
> +
> if (enable) {
> ret = enable_eud(chip);
[Severity: High]
This isn't a bug introduced by this patch, but does the lack of
synchronization in this sysfs store handler expose a race condition?
If multiple threads concurrently write to the enable attribute, could both
observe chip->enabled == false, bypass this new check, and invoke
enable_eud() simultaneously?
Would this result in duplicate unsynchronized MMIO writes and unbalanced
calls to phy_init() and phy_power_on(), permanently leaking PHY internal
power and initialization reference counts?
Furthermore, because port_store() is also unprotected, can a concurrent
write to port modify chip->port_idx while enable_eud() is executing?
drivers/usb/misc/qcom_eud.c:port_store() {
...
/* Port selection must be done before enabling EUD */
if (chip->enabled)
return -EBUSY;
writel(port, chip->base + EUD_REG_PORT_SEL);
chip->port_idx = port;
...
}
Could this interleaving cause the driver to enable the PHY on one port but
later disable the PHY on the other port, leading to corrupted hardware state?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803031534.2942628-1-elson.serrao@oss.qualcomm.com?part=5
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 6/9] usb: misc: qcom_eud: add role-based EUD control
2026-08-03 3:15 [PATCH v5 0/9] Improve Qualcomm EUD driver and platform support Elson Serrao
` (4 preceding siblings ...)
2026-08-03 3:15 ` [PATCH v5 5/9] usb: misc: qcom_eud: improve enable_store API Elson Serrao
@ 2026-08-03 3:15 ` Elson Serrao
2026-08-03 3:27 ` sashiko-bot
2026-08-03 9:50 ` Peter Chen
2026-08-03 3:15 ` [PATCH v5 7/9] usb: misc: qcom_eud: fix virtual attach/detach event handling Elson Serrao
` (2 subsequent siblings)
8 siblings, 2 replies; 20+ messages in thread
From: Elson Serrao @ 2026-08-03 3:15 UTC (permalink / raw)
To: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Peter Chen, Rob Herring,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel
EUD functions by presenting itself as a USB device to the host PC and is
therefore only applicable when the port operates in device role. Keeping
EUD enabled in other roles provides no benefit and unnecessarily keeps
the associated PHY resources enabled. Furthermore, enabling EUD in host
role can cause the USB controller to misbehave because the EUD hub
supports only a single upstream-facing port.
Address this by treating EUD as usable only in device role. The
role-switch callback enables or disables EUD as the port enters or
leaves device role. A sysfs enable request made outside device role is
recorded and honored when the port subsequently switches to device role.
Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
---
drivers/usb/misc/qcom_eud.c | 46 ++++++++++++++++++++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index e83a4402e4b8..538e070cc140 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -15,6 +15,7 @@
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
+#include <linux/string_choices.h>
#include <linux/sysfs.h>
#include <linux/usb/role.h>
#include <linux/firmware/qcom/qcom_scm.h>
@@ -55,6 +56,8 @@ struct eud_chip {
struct device *dev;
void __iomem *base;
struct eud_path *paths[EUD_MAX_PORTS];
+ /* serializes EUD control operations */
+ struct mutex state_lock;
phys_addr_t mode_mgr;
unsigned int int_status;
int irq;
@@ -156,17 +159,33 @@ static ssize_t enable_store(struct device *dev,
const char *buf, size_t count)
{
struct eud_chip *chip = dev_get_drvdata(dev);
+ struct eud_path *path;
bool enable;
int ret;
if (kstrtobool(buf, &enable))
return -EINVAL;
+ guard(mutex)(&chip->state_lock);
+
/* Skip operation if already in desired state */
if (chip->enabled == enable)
return count;
if (enable) {
+ path = chip->paths[chip->port_idx];
+
+ /*
+ * If not yet in device role, honor the userspace request and defer
+ * EUD enablement until the port transitions to device role in the
+ * set_role callback.
+ */
+ if (path->curr_role != USB_ROLE_DEVICE) {
+ dev_info(chip->dev, "Deferring EUD enable until port enters device mode\n");
+ chip->enabled = enable;
+ return count;
+ }
+
ret = enable_eud(chip);
if (ret) {
dev_err(chip->dev, "failed to enable eud\n");
@@ -304,7 +323,30 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
static int eud_role_switch_set(struct usb_role_switch *sw, enum usb_role role)
{
struct eud_path *path = usb_role_switch_get_drvdata(sw);
- int ret;
+ struct eud_chip *chip = path->chip;
+ int ret = 0;
+
+ guard(mutex)(&chip->state_lock);
+
+ /*
+ * EUD is usable only in device role. Power it down for every other
+ * role to avoid keeping an unusable module 'ON'. chip->enabled
+ * preserves user's sysfs configuration and is not modified across
+ * role transitions.
+ */
+ if (chip->enabled && path->num == chip->port_idx && role != path->curr_role) {
+ if (role == USB_ROLE_DEVICE)
+ ret = enable_eud(chip);
+ else if (path->curr_role == USB_ROLE_DEVICE)
+ ret = disable_eud(chip);
+
+ if (ret) {
+ dev_err(chip->dev, "failed to %s EUD for role %s: %d\n",
+ str_enable_disable(role == USB_ROLE_DEVICE),
+ usb_role_string(role), ret);
+ return ret;
+ }
+ }
/* Forward the role request to the USB controller */
ret = usb_role_switch_set_role(path->controller_sw, role);
@@ -431,6 +473,8 @@ static int eud_probe(struct platform_device *pdev)
chip->dev = &pdev->dev;
+ mutex_init(&chip->state_lock);
+
for_each_child_of_node_scoped(np, child) {
ret = eud_init_path(chip, child);
if (ret)
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 6/9] usb: misc: qcom_eud: add role-based EUD control
2026-08-03 3:15 ` [PATCH v5 6/9] usb: misc: qcom_eud: add role-based EUD control Elson Serrao
@ 2026-08-03 3:27 ` sashiko-bot
2026-08-03 9:50 ` Peter Chen
1 sibling, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 3:27 UTC (permalink / raw)
To: Elson Serrao; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Missing `state_lock` in `port_store()` allows a concurrent TOCTOU race with `enable_store()`, leading to unbalanced PHY operations and resource leaks.
- [High] Error handling in `eud_role_switch_set()` causes state desynchronization and blocks core USB role switches.
- [High] `eud_remove()` lacks `state_lock` and does not set `chip->enabled = false`, causing a concurrent driver teardown race.
Pre-existing issues:
- [High] Simultaneous VBUS and SAFE_MODE interrupts cause an unacknowledged IRQ storm.
--
--- Patch [6]: [PATCH v5 6/9] usb: misc: qcom_eud: add role-based EUD control ---
commit e9c3a517ab0e4301ef49204c0ab1c8da2ba44fa8
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>
Subject: usb: misc: qcom_eud: add role-based EUD control
This commit modifies the EUD driver to treat EUD as usable only in device
role, disabling it in host mode. It uses a role-switch callback to enable or
disable EUD across role transitions and defers sysfs enable requests made
outside device role until the port switches.
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index e83a4402e4b84..538e070cc140a 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
[ ... ]
> @@ -55,6 +56,8 @@ struct eud_chip {
> struct device *dev;
> void __iomem *base;
> struct eud_path *paths[EUD_MAX_PORTS];
> + /* serializes EUD control operations */
> + struct mutex state_lock;
[Severity: High]
Does the introduction of state_lock serialize EUD operations entirely, or
could this leave a regression with unbalanced PHY operations?
Looking at port_store() and handle_eud_irq_thread(), they appear to access
chip->port_idx and chip->enabled without acquiring state_lock. For example,
in port_store():
/* Port selection must be done before enabling EUD */
if (chip->enabled)
return -EBUSY;
writel(port, chip->base + EUD_REG_PORT_SEL);
chip->port_idx = port;
If Thread A writes to the port sysfs attribute and Thread B writes to the
enable sysfs attribute concurrently, could Thread A read chip->enabled as
false, while Thread B acquires state_lock in enable_store() and enables
the PHY for the current port?
If Thread A then locklessly updates chip->port_idx, future disable operations
would target the new port's PHY instead of the old one. Could this lead to
a regression by leaving a permanent PHY power leak?
[ ... ]
> @@ -304,7 +323,30 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
> static int eud_role_switch_set(struct usb_role_switch *sw, enum usb_role role)
> {
> struct eud_path *path = usb_role_switch_get_drvdata(sw);
> - int ret;
> + struct eud_chip *chip = path->chip;
> + int ret = 0;
> +
> + guard(mutex)(&chip->state_lock);
> +
> + /*
> + * EUD is usable only in device role. Power it down for every other
> + * role to avoid keeping an unusable module 'ON'. chip->enabled
> + * preserves user's sysfs configuration and is not modified across
> + * role transitions.
> + */
> + if (chip->enabled && path->num == chip->port_idx && role != path->curr_role) {
> + if (role == USB_ROLE_DEVICE)
> + ret = enable_eud(chip);
> + else if (path->curr_role == USB_ROLE_DEVICE)
> + ret = disable_eud(chip);
[Severity: High]
If a Type-C role switch occurs concurrently during or after module unload,
could this introduce a regression by corrupting the hardware state?
Looking at eud_remove(), it calls disable_eud() without holding state_lock
and without setting chip->enabled to false:
static void eud_remove(struct platform_device *pdev)
{
struct eud_chip *chip = platform_get_drvdata(pdev);
if (chip->enabled)
disable_eud(chip);
Since the EUD role switch and IRQs are managed by devm, they remain active
during eud_remove(). If eud_role_switch_set() executes concurrently and
observes chip->enabled is still true, could it re-enable the hardware just
before devres teardown, leading to PHY imbalances?
> +
> + if (ret) {
> + dev_err(chip->dev, "failed to %s EUD for role %s: %d\n",
> + str_enable_disable(role == USB_ROLE_DEVICE),
> + usb_role_string(role), ret);
> + return ret;
> + }
> + }
[Severity: High]
Does this early return introduce a regression where core USB role switches
fail if EUD encounters an error?
If enable_eud() fails, the function returns early and completely bypasses
the downstream usb_role_switch_set_role() call, which might block core
USB role switch functionality.
Additionally, if enable_eud() succeeds but the downstream controller switch
fails:
ret = usb_role_switch_set_role(path->controller_sw, role);
if (ret) {
dev_err(path->chip->dev, "failed to set role %s for port %u: %d\n",
usb_role_string(role), path->num, ret);
return ret;
}
path->curr_role = role;
The function returns without rolling back the EUD state and without updating
path->curr_role. When the port later switches back to USB_ROLE_HOST,
role != path->curr_role would evaluate to false, skipping the disable step
and permanently desynchronizing the state.
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but does handle_eud_irq()
correctly handle the hardware state where multiple interrupt bits are asserted
at once?
Looking at handle_eud_irq():
switch (reg & EUD_INT_ALL) {
case EUD_INT_VBUS:
usb_attach_detach(chip);
return IRQ_WAKE_THREAD;
case EUD_INT_SAFE_MODE:
pet_eud(chip);
return IRQ_HANDLED;
default:
return IRQ_NONE;
}
If both EUD_INT_VBUS and EUD_INT_SAFE_MODE are asserted simultaneously,
the bitwise AND results in both bits being set. This combined value falls
through to the default block, returning IRQ_NONE without clearing the
interrupts. Could this cause an unacknowledged IRQ storm and force the kernel
to disable the EUD IRQ line?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803031534.2942628-1-elson.serrao@oss.qualcomm.com?part=6
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 6/9] usb: misc: qcom_eud: add role-based EUD control
2026-08-03 3:15 ` [PATCH v5 6/9] usb: misc: qcom_eud: add role-based EUD control Elson Serrao
2026-08-03 3:27 ` sashiko-bot
@ 2026-08-03 9:50 ` Peter Chen
1 sibling, 0 replies; 20+ messages in thread
From: Peter Chen @ 2026-08-03 9:50 UTC (permalink / raw)
To: Elson Serrao
Cc: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
linux-arm-msm, devicetree, linux-usb, linux-kernel
On 26-08-02 20:15:31, Elson Serrao wrote:
> EUD functions by presenting itself as a USB device to the host PC and is
> therefore only applicable when the port operates in device role. Keeping
> EUD enabled in other roles provides no benefit and unnecessarily keeps
> the associated PHY resources enabled. Furthermore, enabling EUD in host
> role can cause the USB controller to misbehave because the EUD hub
> supports only a single upstream-facing port.
>
> Address this by treating EUD as usable only in device role. The
> role-switch callback enables or disables EUD as the port enters or
> leaves device role. A sysfs enable request made outside device role is
> recorded and honored when the port subsequently switches to device role.
>
> Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
Reviewed-by: Peter Chen <peter.chen@kernel.org>
Peter
> ---
> drivers/usb/misc/qcom_eud.c | 46 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index e83a4402e4b8..538e070cc140 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
> @@ -15,6 +15,7 @@
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> +#include <linux/string_choices.h>
> #include <linux/sysfs.h>
> #include <linux/usb/role.h>
> #include <linux/firmware/qcom/qcom_scm.h>
> @@ -55,6 +56,8 @@ struct eud_chip {
> struct device *dev;
> void __iomem *base;
> struct eud_path *paths[EUD_MAX_PORTS];
> + /* serializes EUD control operations */
> + struct mutex state_lock;
> phys_addr_t mode_mgr;
> unsigned int int_status;
> int irq;
> @@ -156,17 +159,33 @@ static ssize_t enable_store(struct device *dev,
> const char *buf, size_t count)
> {
> struct eud_chip *chip = dev_get_drvdata(dev);
> + struct eud_path *path;
> bool enable;
> int ret;
>
> if (kstrtobool(buf, &enable))
> return -EINVAL;
>
> + guard(mutex)(&chip->state_lock);
> +
> /* Skip operation if already in desired state */
> if (chip->enabled == enable)
> return count;
>
> if (enable) {
> + path = chip->paths[chip->port_idx];
> +
> + /*
> + * If not yet in device role, honor the userspace request and defer
> + * EUD enablement until the port transitions to device role in the
> + * set_role callback.
> + */
> + if (path->curr_role != USB_ROLE_DEVICE) {
> + dev_info(chip->dev, "Deferring EUD enable until port enters device mode\n");
> + chip->enabled = enable;
> + return count;
> + }
> +
> ret = enable_eud(chip);
> if (ret) {
> dev_err(chip->dev, "failed to enable eud\n");
> @@ -304,7 +323,30 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
> static int eud_role_switch_set(struct usb_role_switch *sw, enum usb_role role)
> {
> struct eud_path *path = usb_role_switch_get_drvdata(sw);
> - int ret;
> + struct eud_chip *chip = path->chip;
> + int ret = 0;
> +
> + guard(mutex)(&chip->state_lock);
> +
> + /*
> + * EUD is usable only in device role. Power it down for every other
> + * role to avoid keeping an unusable module 'ON'. chip->enabled
> + * preserves user's sysfs configuration and is not modified across
> + * role transitions.
> + */
> + if (chip->enabled && path->num == chip->port_idx && role != path->curr_role) {
> + if (role == USB_ROLE_DEVICE)
> + ret = enable_eud(chip);
> + else if (path->curr_role == USB_ROLE_DEVICE)
> + ret = disable_eud(chip);
> +
> + if (ret) {
> + dev_err(chip->dev, "failed to %s EUD for role %s: %d\n",
> + str_enable_disable(role == USB_ROLE_DEVICE),
> + usb_role_string(role), ret);
> + return ret;
> + }
> + }
>
> /* Forward the role request to the USB controller */
> ret = usb_role_switch_set_role(path->controller_sw, role);
> @@ -431,6 +473,8 @@ static int eud_probe(struct platform_device *pdev)
>
> chip->dev = &pdev->dev;
>
> + mutex_init(&chip->state_lock);
> +
> for_each_child_of_node_scoped(np, child) {
> ret = eud_init_path(chip, child);
> if (ret)
> --
> 2.34.1
>
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 7/9] usb: misc: qcom_eud: fix virtual attach/detach event handling
2026-08-03 3:15 [PATCH v5 0/9] Improve Qualcomm EUD driver and platform support Elson Serrao
` (5 preceding siblings ...)
2026-08-03 3:15 ` [PATCH v5 6/9] usb: misc: qcom_eud: add role-based EUD control Elson Serrao
@ 2026-08-03 3:15 ` Elson Serrao
2026-08-03 3:26 ` sashiko-bot
2026-08-03 3:15 ` [PATCH v5 8/9] arm64: dts: qcom: kodiak: Describe EUD UTMI path using child node Elson Serrao
2026-08-03 3:15 ` [PATCH v5 9/9] arm64: dts: qcom: Map USB connector to EUD on Kodiak boards Elson Serrao
8 siblings, 1 reply; 20+ messages in thread
From: Elson Serrao @ 2026-08-03 3:15 UTC (permalink / raw)
To: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Peter Chen, Rob Herring,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel, Konrad Dybcio
EUD provides virtual USB attach/detach events to simulate cable
plug/unplug while maintaining the physical debug connection. However,
the current implementation incorrectly sets the USB role to HOST on
virtual detach, which doesn't represent the disconnected state.
Fix the virtual detach handling by setting the USB role to NONE
instead of HOST, correctly representing the disconnected state.
Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Peter Chen <peter.chen@kernel.org>
---
drivers/usb/misc/qcom_eud.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 538e070cc140..a02f3535956d 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -306,10 +306,26 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
path = chip->paths[chip->port_idx];
+ /*
+ * EUD virtual attach/detach event handling for low power debugging:
+ *
+ * When EUD is enabled in debug mode, the device remains physically
+ * connected to the PC throughout the debug session, keeping the USB
+ * controller active. This prevents testing of low power scenarios that
+ * require USB disconnection.
+ *
+ * EUD solves this by providing virtual USB attach/detach events while
+ * maintaining the physical connection. These events are triggered from
+ * the Host PC via the enumerated EUD control interface and delivered
+ * to the EUD driver as interrupts.
+ *
+ * These notifications are forwarded to the USB controller through role
+ * switch framework.
+ */
if (chip->usb_attached)
ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_DEVICE);
else
- ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_HOST);
+ ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_NONE);
if (ret)
dev_err(chip->dev, "failed to set role switch\n");
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 7/9] usb: misc: qcom_eud: fix virtual attach/detach event handling
2026-08-03 3:15 ` [PATCH v5 7/9] usb: misc: qcom_eud: fix virtual attach/detach event handling Elson Serrao
@ 2026-08-03 3:26 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 3:26 UTC (permalink / raw)
To: Elson Serrao; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Race condition in `port_store` allowing unsafe concurrent modification of `port_idx`.
- [High] Threaded IRQ handler modifies controller role without synchronization, racing with Type-C physical detachment.
- [High] The EUD driver fails to restore the physical USB role when disabled while in a virtually detached state.
--
--- Patch [7]: [PATCH v5 7/9] usb: misc: qcom_eud: fix virtual attach/detach event handling ---
commit ce50a52a3d67527a1dac91d09de56e8f3206cf1c
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>
usb: misc: qcom_eud: fix virtual attach/detach event handling
EUD provides virtual USB attach/detach events to simulate cable plug/unplug
while maintaining the physical debug connection. This commit fixes the virtual
detach handling by setting the USB role to NONE instead of HOST, which
correctly represents the disconnected state.
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 538e070cc140a..a02f3535956db 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
> @@ -306,10 +306,26 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data)
[ ... ]
> if (chip->usb_attached)
> ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_DEVICE);
> else
> - ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_HOST);
> + ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_NONE);
[Severity: High]
This isn't a bug introduced by this patch, but does this threaded IRQ handler
race with Type-C physical detachment?
The role switch is performed locklessly and without verifying if EUD is still
logically active or physically attached.
If the physical Type-C cable is unplugged before this threaded IRQ handler runs,
eud_role_switch_set() could disable EUD and set path->controller_sw to
USB_ROLE_NONE.
When handle_eud_irq_thread() runs concurrently, if it sees chip->usb_attached
as true, it would blindly call usb_role_switch_set_role() and set the role to
USB_ROLE_DEVICE after the PHY has been powered off. Can this cause hardware
inconsistency or controller hangs?
> if (ret)
> dev_err(chip->dev, "failed to set role switch\n");
[Severity: High]
This is a pre-existing issue, but does the EUD driver fail to restore the
physical USB role when disabled while in a virtually detached state?
If the physical Type-C cable is connected (where path->curr_role is
USB_ROLE_DEVICE) and a virtual detach triggers handle_eud_irq_thread(), the
controller role is overridden to USB_ROLE_NONE.
If the user then disables EUD via sysfs:
drivers/usb/misc/qcom_eud.c:enable_store() {
...
ret = disable_eud(chip);
if (ret) {
dev_err(chip->dev, "failed to disable eud\n");
return ret;
}
...
}
The driver does not appear to restore path->controller_sw to the actual
physical role (USB_ROLE_DEVICE).
Since the physical cable was never unplugged, the Type-C driver will not issue
a new attach event. Could this leave the device controller stuck in
USB_ROLE_NONE and permanently prevent USB functionality until a physical replug
occurs?
[Severity: High]
This is a pre-existing issue, but is there a race condition in port_store()
that allows unsafe concurrent modification of chip->port_idx?
If thread A starts enable_store() and acquires chip->state_lock, and thread B
executes port_store() concurrently, thread B reads chip->enabled locklessly.
Since chip->enabled is still false, thread B bypasses the -EBUSY check and
updates chip->port_idx:
drivers/usb/misc/qcom_eud.c:port_store() {
...
/* Port selection must be done before enabling EUD */
if (chip->enabled)
return -EBUSY;
writel(port, chip->base + EUD_REG_PORT_SEL);
chip->port_idx = port;
...
}
Thread A then finishes enable_store(), initializing the old port's PHY and
setting chip->enabled = true.
When EUD is subsequently disabled, disable_eud() calls eud_phy_disable(chip)
using the new chip->port_idx, attempting to power off an uninitialized PHY.
Does this leave the originally initialized PHY powered on indefinitely and risk
causing a kernel warning or crash?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803031534.2942628-1-elson.serrao@oss.qualcomm.com?part=7
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 8/9] arm64: dts: qcom: kodiak: Describe EUD UTMI path using child node
2026-08-03 3:15 [PATCH v5 0/9] Improve Qualcomm EUD driver and platform support Elson Serrao
` (6 preceding siblings ...)
2026-08-03 3:15 ` [PATCH v5 7/9] usb: misc: qcom_eud: fix virtual attach/detach event handling Elson Serrao
@ 2026-08-03 3:15 ` Elson Serrao
2026-08-03 3:54 ` sashiko-bot
2026-08-03 3:15 ` [PATCH v5 9/9] arm64: dts: qcom: Map USB connector to EUD on Kodiak boards Elson Serrao
8 siblings, 1 reply; 20+ messages in thread
From: Elson Serrao @ 2026-08-03 3:15 UTC (permalink / raw)
To: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Peter Chen, Rob Herring,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel
The existing EUD description uses a legacy single-path representation
that assumes uniform UTMI routing. This was sufficient for minimal
configurations where the USB port operated only in device mode and role
switching was not considered.
Update the description to explicitly model the topology using eud path
child node and add a role-switch declaration to reflect role-dependent
UTMI routing. Also on this SoC, the EUD hardware intercepts only the
primary UTMI path. So remove the USB endpoint mapping to the secondary
controller and associate it with the primary controller.
With the UTMI path now accurately described, enable the EUD by default.
The connector side of the path is left unspecified, allowing board-level
DTS files to describe the physical connector mapping.
Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/kodiak.dtsi | 35 +++++++++++++++++-----------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
index fa540d8c2615..be9d02760a98 100644
--- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
+++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
@@ -4404,12 +4404,6 @@ usb_2: usb@8c00000 {
phy-names = "usb2-phy";
maximum-speed = "high-speed";
usb-role-switch;
-
- port {
- usb2_role_switch: endpoint {
- remote-endpoint = <&eud_ep>;
- };
- };
};
qspi: spi@88dc000 {
@@ -4736,16 +4730,28 @@ eud: eud@88e0000 {
<0 0x88e2000 0 0x1000>;
interrupts-extended = <&pdc 11 IRQ_TYPE_LEVEL_HIGH>;
- status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
- ports {
- #address-cells = <1>;
- #size-cells = <0>;
+ eud0: eud-path@0 {
+ reg = <0>;
+ usb-role-switch;
- port@0 {
- reg = <0>;
- eud_ep: endpoint {
- remote-endpoint = <&usb2_role_switch>;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ eud_usb: endpoint {
+ remote-endpoint = <&usb_1_dwc3_hs>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ eud_con: endpoint {
+ };
};
};
};
@@ -4972,6 +4978,7 @@ port@0 {
reg = <0>;
usb_1_dwc3_hs: endpoint {
+ remote-endpoint = <&eud_usb>;
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 8/9] arm64: dts: qcom: kodiak: Describe EUD UTMI path using child node
2026-08-03 3:15 ` [PATCH v5 8/9] arm64: dts: qcom: kodiak: Describe EUD UTMI path using child node Elson Serrao
@ 2026-08-03 3:54 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 3:54 UTC (permalink / raw)
To: Elson Serrao; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch creates a broken (asymmetric) OF graph connection by modifying `kodiak.dtsi` without updating dependent board files, which breaks USB role switching for EUD.
- [Medium] Missing mutex lock in `port_store` allows a race condition with `enable_store` and role switch callbacks, potentially corrupting EUD hardware state.
--
--- Patch [8]: [PATCH v5 8/9] arm64: dts: qcom: kodiak: Describe EUD UTMI path using child node ---
commit 6a20db16eb123d18691f3748fc7fc0dc057b3d4c
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>
arm64: dts: qcom: kodiak: Describe EUD UTMI path using child node
This patch updates the EUD description to explicitly model the topology
using a child node and adds a role-switch declaration. It removes the
USB endpoint mapping to the secondary controller, associating it with
the primary controller, and enables the EUD by default.
> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
> index fa540d8c2615d..be9d02760a985 100644
> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
[ ... ]
> @@ -4736,16 +4730,28 @@ eud: eud@88e0000 {
[ ... ]
> + eud0: eud-path@0 {
> + reg = <0>;
> + usb-role-switch;
>
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + eud_usb: endpoint {
> + remote-endpoint = <&usb_1_dwc3_hs>;
> + };
> + };
[Severity: High]
Does this create an asymmetric OF graph connection?
If downstream board files (like qcs6490-rb3gen2.dts) override usb_1_dwc3_hs
to point directly to their Type-C connectors (e.g., pmic_glink_hs_in),
wouldn't this leave eud_usb as a dangling one-way link?
This would violate the OF graph specification which strictly requires
symmetric bidirectional links.
[ ... ]
> @@ -4972,6 +4978,7 @@ port@0 {
> reg = <0>;
>
> usb_1_dwc3_hs: endpoint {
> + remote-endpoint = <&eud_usb>;
> };
> };
[Severity: High]
Will Type-C role switch events bypass the EUD driver entirely on boards
that override this endpoint?
If the OF graph connection is broken by downstream overrides, could this
leave the enabled EUD hardware permanently waiting for role change callbacks
that never arrive?
Additionally, looking at drivers/usb/misc/qcom_eud.c:port_store():
/* Port selection must be done before enabling EUD */
if (chip->enabled)
return -EBUSY;
writel(port, chip->base + EUD_REG_PORT_SEL);
chip->port_idx = port;
[Severity: Medium]
Does this missing mutex lock in port_store() allow a race condition with
enable_store()?
If a concurrent sysfs write to enable and port occurs, port_store() can
interleave with enable_store() since it doesn't hold chip->state_lock.
Could this allow enable_store() to perform role checks using one port but
configure and enable the PHY for another port, misaligning the internal
driver state with the hardware?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803031534.2942628-1-elson.serrao@oss.qualcomm.com?part=8
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 9/9] arm64: dts: qcom: Map USB connector to EUD on Kodiak boards
2026-08-03 3:15 [PATCH v5 0/9] Improve Qualcomm EUD driver and platform support Elson Serrao
` (7 preceding siblings ...)
2026-08-03 3:15 ` [PATCH v5 8/9] arm64: dts: qcom: kodiak: Describe EUD UTMI path using child node Elson Serrao
@ 2026-08-03 3:15 ` Elson Serrao
8 siblings, 0 replies; 20+ messages in thread
From: Elson Serrao @ 2026-08-03 3:15 UTC (permalink / raw)
To: Bjorn Andersson, Dmitry Baryshkov, Greg Kroah-Hartman,
Konrad Dybcio, Krzysztof Kozlowski, Peter Chen, Rob Herring,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel
On Kodiak-based boards, the primary USB connector is connected through
the EUD on the High-Speed path. Update the board-level descriptions to
map the USB connector endpoint to EUD, reflecting the intended hardware
topology.
Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 10 +++++-----
arch/arm64/boot/dts/qcom/qcm6490-particle-tachyon.dts | 10 +++++-----
arch/arm64/boot/dts/qcom/qcm6490-shift-otter.dts | 10 +++++-----
arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 10 +++++-----
.../boot/dts/qcom/qcs6490-thundercomm-minipc-g1iot.dts | 4 ++--
.../boot/dts/qcom/qcs6490-thundercomm-rubikpi3.dts | 10 +++++-----
arch/arm64/boot/dts/qcom/sm7325-motorola-dubai.dts | 4 ++--
arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts | 10 +++++-----
8 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
index 04cb9230d29f..c213e06a4428 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -98,7 +98,7 @@ port@0 {
reg = <0>;
pmic_glink_hs_in: endpoint {
- remote-endpoint = <&usb_1_dwc3_hs>;
+ remote-endpoint = <&eud_con>;
};
};
@@ -1478,10 +1478,6 @@ &usb_1 {
status = "okay";
};
-&usb_1_dwc3_hs {
- remote-endpoint = <&pmic_glink_hs_in>;
-};
-
&usb_1_hsphy {
vdda-pll-supply = <&vreg_l10c>;
vdda18-supply = <&vreg_l1c>;
@@ -1521,3 +1517,7 @@ &wifi {
qcom,calibration-variant = "Fairphone_5";
status = "okay";
};
+
+&eud_con {
+ remote-endpoint = <&pmic_glink_hs_in>;
+};
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-particle-tachyon.dts b/arch/arm64/boot/dts/qcom/qcm6490-particle-tachyon.dts
index bf18c4852081..e9c0463daa53 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-particle-tachyon.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-particle-tachyon.dts
@@ -74,7 +74,7 @@ port@0 {
reg = <0>;
pmic_glink_hs_in: endpoint {
- remote-endpoint = <&usb_1_dwc3_hs>;
+ remote-endpoint = <&eud_con>;
};
};
@@ -826,10 +826,6 @@ &usb_1 {
status = "okay";
};
-&usb_1_dwc3_hs {
- remote-endpoint = <&pmic_glink_hs_in>;
-};
-
&usb_1_hsphy {
vdda-pll-supply = <&vreg_l10c_0p88>;
vdda33-supply = <&vreg_l2b_3p072>;
@@ -862,3 +858,7 @@ &usb_2_hsphy {
&usb_dp_qmpphy_out {
remote-endpoint = <&pmic_glink_ss_in>;
};
+
+&eud_con {
+ remote-endpoint = <&pmic_glink_hs_in>;
+};
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-shift-otter.dts b/arch/arm64/boot/dts/qcom/qcm6490-shift-otter.dts
index 797f37596bf1..650c4fbc1606 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-shift-otter.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-shift-otter.dts
@@ -84,7 +84,7 @@ port@0 {
reg = <0>;
pmic_glink_hs_in: endpoint {
- remote-endpoint = <&usb_1_dwc3_hs>;
+ remote-endpoint = <&eud_con>;
};
};
@@ -952,10 +952,6 @@ &usb_1 {
status = "okay";
};
-&usb_1_dwc3_hs {
- remote-endpoint = <&pmic_glink_hs_in>;
-};
-
&usb_1_hsphy {
vdda-pll-supply = <&vreg_l10c>;
vdda18-supply = <&vreg_l1c>;
@@ -986,6 +982,10 @@ &usb_dp_qmpphy_out {
remote-endpoint = <&pmic_glink_ss_in>;
};
+&eud_con {
+ remote-endpoint = <&pmic_glink_hs_in>;
+};
+
&venus {
firmware-name = "qcom/qcm6490/SHIFT/otter/venus.mbn";
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 37a3b51323ce..514d6d7fedb5 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -211,7 +211,7 @@ port@0 {
reg = <0>;
pmic_glink_hs_in: endpoint {
- remote-endpoint = <&usb_1_dwc3_hs>;
+ remote-endpoint = <&eud_con>;
};
};
@@ -1402,14 +1402,14 @@ &usb_1 {
status = "okay";
};
-&usb_1_dwc3_hs {
- remote-endpoint = <&pmic_glink_hs_in>;
-};
-
&usb_1_dwc3_ss {
remote-endpoint = <&usb_dp_qmpphy_usb_ss_in>;
};
+&eud_con {
+ remote-endpoint = <&pmic_glink_hs_in>;
+};
+
&usb_1_hsphy {
vdda-pll-supply = <&vreg_l10c_0p88>;
vdda33-supply = <&vreg_l2b_3p072>;
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-minipc-g1iot.dts b/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-minipc-g1iot.dts
index a5ad796cb65d..1d992dcc89ed 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-minipc-g1iot.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-minipc-g1iot.dts
@@ -77,7 +77,7 @@ port@0 {
reg = <0>;
pmic_glink_hs_in: endpoint {
- remote-endpoint = <&usb_1_dwc3_hs>;
+ remote-endpoint = <&eud_con>;
};
};
@@ -1046,7 +1046,7 @@ &usb_1 {
status = "okay";
};
-&usb_1_dwc3_hs {
+&eud_con {
remote-endpoint = <&pmic_glink_hs_in>;
};
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-rubikpi3.dts b/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-rubikpi3.dts
index f47efca42d48..387c38aa87d2 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-rubikpi3.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-rubikpi3.dts
@@ -93,7 +93,7 @@ port@0 {
reg = <0>;
pmic_glink_hs_in: endpoint {
- remote-endpoint = <&usb_1_dwc3_hs>;
+ remote-endpoint = <&eud_con>;
};
};
@@ -1090,10 +1090,6 @@ &usb_1 {
status = "okay";
};
-&usb_1_dwc3_hs {
- remote-endpoint = <&pmic_glink_hs_in>;
-};
-
&usb_1_hsphy {
vdda-pll-supply = <&vreg_l10c_0p88>;
vdda33-supply = <&vreg_l2b_3p072>;
@@ -1127,6 +1123,10 @@ &usb_dp_qmpphy_out {
remote-endpoint = <&pmic_glink_ss_in>;
};
+&eud_con {
+ remote-endpoint = <&pmic_glink_hs_in>;
+};
+
&ufs_mem_hc {
reset-gpios = <&tlmm 175 GPIO_ACTIVE_LOW>;
vcc-supply = <&vreg_l7b_2p952>;
diff --git a/arch/arm64/boot/dts/qcom/sm7325-motorola-dubai.dts b/arch/arm64/boot/dts/qcom/sm7325-motorola-dubai.dts
index 3c836e196b19..99913ba41d55 100644
--- a/arch/arm64/boot/dts/qcom/sm7325-motorola-dubai.dts
+++ b/arch/arm64/boot/dts/qcom/sm7325-motorola-dubai.dts
@@ -98,7 +98,7 @@ port@0 {
reg = <0>;
pmic_glink_hs_in: endpoint {
- remote-endpoint = <&usb_1_dwc3_hs>;
+ remote-endpoint = <&eud_con>;
};
};
@@ -1431,7 +1431,7 @@ &usb_1 {
status = "okay";
};
-&usb_1_dwc3_hs {
+&eud_con {
remote-endpoint = <&pmic_glink_hs_in>;
};
diff --git a/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts b/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
index cb59c122f6f6..a2e160f3f46e 100644
--- a/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
+++ b/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
@@ -99,7 +99,7 @@ port@0 {
reg = <0>;
pmic_glink_hs_in: endpoint {
- remote-endpoint = <&usb_1_dwc3_hs>;
+ remote-endpoint = <&eud_con>;
};
};
@@ -1440,10 +1440,6 @@ &usb_1 {
status = "okay";
};
-&usb_1_dwc3_hs {
- remote-endpoint = <&pmic_glink_hs_in>;
-};
-
&usb_1_hsphy {
vdda-pll-supply = <&vdd_a_usbhs_core>;
vdda18-supply = <&vdd_a_usbhs_1p8>;
@@ -1459,3 +1455,7 @@ &venus {
&wifi {
status = "okay";
};
+
+&eud_con {
+ remote-endpoint = <&pmic_glink_hs_in>;
+};
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread