[PATCH 0/4] arm64: qcom: QCM2290: add the Embedded USB Debugger

[PATCH 0/4] arm64: qcom: QCM2290: add the Embedded USB Debugger

[Igor Opaniuk]

The Embedded USB Debugger of the QCM2290 is the block the SC7280 has,
with one difference: its mode manager ignores the enable (EUD_EN2)
unless the TCSR register TCSR_APSS_SPARE_REG1 is set first. XBL_SEC,
the secure part of the boot loader, does exactly that when it enables
the EUD itself: it sets TCSR_APSS_SPARE_REG1, then EUD_EN2.

Patch 1 adds the compatible and describes that register as a third
reg entry, patch 2 has qcom_eud write it through the secure IO call
like EUD_EN2, patch 3 adds the EUD to agatti.dtsi and patch 4 enables
it on the Arduino UNO Q.

Why Linux sets the bit and not the firmware:
- XBL_SEC runs that sequence only when the device's debug policy (an
  optional signed image that enables JTAG) asks for it. Without one,
  as on the Arduino UNO Q, the board boots with both registers clear.
- The stock TEE (QTEE) does not set the bit either. It allow-lists
  TCSR_APSS_SPARE_REG1 for the HLOS secure IO calls next to EUD_EN2,
  with the same policy (write allowed when secure boot is off or
  debug is not disabled), i.e. it expects the OS to set it before
  EUD_EN2.
- TF-A does the same on Agatti [1][2]: TCSR_APSS_SPARE_REG1 is open to
  the normal world in all builds ("this register only permits EUD to
  be enabled"), EUD_EN2 in debug builds only.
So with either TEE only the driver can complete the sequence, while
the secure firmware keeps deciding whether the EUD may be enabled at
all. Where EUD_EN2 is refused, enabling fails with -EINVAL as before.

Tested on the Arduino UNO Q (QRB2210) with next-20261112 plus the board
support it needs. The A/B runs use the same kernel, with and without
the third reg in the EUD node; without it the driver only writes
EUD_EN2, as today:
- TF-A, built to allow EUD_EN2: with the TCSR write the host enumerates
  the EUD hub (05c6:9500) and its control function (05c6:9501) in each
  of three enable/disable cycles; without it the writes are accepted
  and the enable reports success, but nothing enumerates in three
  cycles.
- Stock QTEE + Gunyah (secure boot off): with the TCSR write the first
  enable after a fresh boot attaches (two boots); without it nothing
  attaches (two boots). Reading both registers back through the secure
  IO call shows them taking every write. The USB controller node was
  disabled in this setup (Linux cannot program the SMMU under Gunyah
  with this DT), and there only the first enable per boot attached;
  re-enabling after a disable was not looked into further.
- OpenOCD's EUD adapter attached over SWD on the TF-A setup (earlier
  build, same enable sequence). Not tested: RB1.

dt_binding_check, dtbs_check (qrb2210-arduino-imola, qrb2210-rb1) and
W=1 are clean. Elson Serrao's pending EUD rework touches the same
binding and driver; this series does not depend on it, whichever lands
second needs a small rebase.

[1] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/55733
[2] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/55964

---
Igor Opaniuk (4):
      dt-bindings: soc: qcom: eud: add the QCM2290 EUD
      usb: misc: qcom_eud: set the TCSR check register before enabling on QCM2290
      arm64: dts: qcom: agatti: add the Embedded USB Debugger
      arm64: dts: qcom: qrb2210-arduino-imola: enable the EUD

 .../devicetree/bindings/soc/qcom/qcom,eud.yaml     | 19 +++++++++++++
 arch/arm64/boot/dts/qcom/agatti.dtsi               | 33 +++++++++++++++++++++-
 arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts |  4 +++
 drivers/usb/misc/qcom_eud.c                        | 19 +++++++++++++
 4 files changed, 74 insertions(+), 1 deletion(-)
---
base-commit: 84abb955b1015477aa11a96e2f3ac1b938b77f48
change-id: 20261114-qcm2290-eud-81e10f3a8e5f

Best regards,
-- 
Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>

[PATCH 1/4] dt-bindings: soc: qcom: eud: add the QCM2290 EUD

[Igor Opaniuk]

The QCM2290 has the same Embedded USB Debugger as the SC7280: the EUD
CSRs and the mode manager register that the secure world writes on
behalf of the driver. On QCM2290 the mode manager additionally ignores
the enable unless the TCSR register TCSR_APSS_SPARE_REG1 is set first;
the boot firmware does the same before enabling the EUD, and the
register is written through the same secure IO service.

Add the compatible and describe that register as a third reg entry,
required on QCM2290 and not allowed on the other EUDs.

Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
---
 .../devicetree/bindings/soc/qcom/qcom,eud.yaml        | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
index 84218636c0d8..14503527cdf2 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
@@ -17,13 +17,17 @@ properties:
   compatible:
     items:
       - enum:
+          - qcom,qcm2290-eud
           - qcom,sc7280-eud
       - const: qcom,eud
 
   reg:
+    minItems: 2
     items:
       - description: EUD Base Register Region
       - description: EUD Mode Manager Register
+      - description: TCSR register that has to be set before the mode
+          manager accepts the enable
 
   interrupts:
     description: EUD interrupt
@@ -50,6 +54,21 @@ required:
   - reg
   - ports
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,qcm2290-eud
+    then:
+      properties:
+        reg:
+          minItems: 3
+    else:
+      properties:
+        reg:
+          maxItems: 2
+
 additionalProperties: false
 
 examples:

-- 
2.53.0

Re: [PATCH 1/4] dt-bindings: soc: qcom: eud: add the QCM2290 EUD

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] dt-bindings: soc: qcom: eud: TCSR register modeled as EUD reg
  entry
--

commit 37264f054b9e512075c1f86ca8168f643e3b2fd7
Author: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>

dt-bindings: soc: qcom: eud: add the QCM2290 EUD

This commit adds the qcom,qcm2290-eud compatible string to the Qualcomm EUD
device tree binding schema and defines a third reg entry for the TCSR
register required on QCM2290.

> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
> index 84218636c0d8d..14503527cdf2e 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
> @@ -17,13 +17,17 @@ properties:
>    compatible:
>      items:
>        - enum:
> +          - qcom,qcm2290-eud
>            - qcom,sc7280-eud
>        - const: qcom,eud
>  
>    reg:
> +    minItems: 2
>      items:
>        - description: EUD Base Register Region
>        - description: EUD Mode Manager Register
> +      - description: TCSR register that has to be set before the mode
> +          manager accepts the enable
>  
>    interrupts:
>      description: EUD interrupt
> @@ -50,6 +54,21 @@ required:
>    - reg
>    - ports
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,qcm2290-eud
> +    then:
> +      properties:
> +        reg:
> +          minItems: 3
> +    else:
> +      properties:
> +        reg:
> +          maxItems: 2

[Severity: Medium]
Should the TCSR register be referenced via a phandle and offset to the TCSR
syscon node rather than modeled as a third reg entry on the EUD node?

TCSR_APSS_SPARE_REG1 (0x003e5018) belongs to the separate TCSR hardware
block, which is already described on QCM2290 as a syscon node covering that
address range:

arch/arm64/boot/dts/qcom/agatti.dtsi {
    ...
		tcsr_regs: syscon@3c0000 {
			compatible = "qcom,qcm2290-tcsr", "syscon";
			reg = <0x0 0x003c0000 0x0 0x40000>;
		};
    ...
}

Putting this register into the EUD node's reg property represents a TCSR
register as part of the EUD address space and creates overlapping reg ranges
between sibling nodes on the SoC bus.  Other Qualcomm bindings that access
TCSR registers (including via qcom_scm_io_writel(), such as qcom,dload-mode
in Documentation/devicetree/bindings/firmware/qcom,scm.yaml) use a
phandle-and-offset property pointing to tcsr_regs instead.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-qcm2290-eud-v1-0-b6157b817377@oss.qualcomm.com?part=1

[PATCH 2/4] usb: misc: qcom_eud: set the TCSR check register before enabling on QCM2290

[Igor Opaniuk]

On QCM2290 the EUD mode manager ignores EUD_EN2 unless the TCSR
register TCSR_APSS_SPARE_REG1 is set first. The boot firmware does
exactly that when it enables the EUD itself (TCSR_APSS_SPARE_REG1 = 1,
then EUD_EN2 = 1), but only when the device's debug policy asks for
it. Otherwise EUD_EN2 is accepted, the driver reports success, and the
EUD stays in bypass.

Take the register from an optional third reg entry and, like EUD_EN2,
write it through qcom_scm_io_writel(), right before EUD_EN2 on enable
and back to 0 after the disable. Whether the writes are allowed stays
the secure firmware's decision: QTEE accepts both when secure boot is
off or debug is not disabled; TF-A accepts TCSR_APSS_SPARE_REG1 always
and EUD_EN2 in debug builds only.

Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
---
 drivers/usb/misc/qcom_eud.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 73f9fac22e19..217f6c1791f0 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -36,6 +36,7 @@ struct eud_chip {
 	struct usb_role_switch		*role_sw;
 	void __iomem			*base;
 	phys_addr_t			mode_mgr;
+	phys_addr_t			tcsr_check;
 	unsigned int			int_status;
 	int				irq;
 	bool				enabled;
@@ -46,6 +47,16 @@ static int enable_eud(struct eud_chip *priv)
 {
 	int ret;
 
+	/*
+	 * On QCM2290 the boot firmware sets the TCSR check register before
+	 * EUD_EN2; without it the mode manager ignores the enable.
+	 */
+	if (priv->tcsr_check) {
+		ret = qcom_scm_io_writel(priv->tcsr_check, 1);
+		if (ret)
+			return ret;
+	}
+
 	ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 1);
 	if (ret)
 		return ret;
@@ -65,6 +76,9 @@ static int disable_eud(struct eud_chip *priv)
 	if (ret)
 		return ret;
 
+	if (priv->tcsr_check)
+		qcom_scm_io_writel(priv->tcsr_check, 0);
+
 	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
 	return 0;
 }
@@ -218,6 +232,11 @@ static int eud_probe(struct platform_device *pdev)
 		return -ENODEV;
 	chip->mode_mgr = res->start;
 
+	/* Optional TCSR check register, written through the secure IO SMC */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (res)
+		chip->tcsr_check = res->start;
+
 	chip->irq = platform_get_irq(pdev, 0);
 	if (chip->irq < 0)
 		return chip->irq;

-- 
2.53.0

Re: [PATCH 2/4] usb: misc: qcom_eud: set the TCSR check register before enabling on QCM2290

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: misc: qcom_eud: tcsr_check left set when EUD_EN2 write
  fails
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] usb: misc: qcom_eud: missing state update in enable_store()
  [1]
--

commit c35f742b781c2fbb7146624bfd77bf246348e228
Author: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>

usb: misc: qcom_eud: set the TCSR check register before enabling on QCM2290

This commit adds support for an optional third memory resource for the
TCSR check register on QCM2290 and writes to it via qcom_scm_io_writel()
when enabling and disabling the EUD.

> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 73f9fac22e19a..217f6c1791f01 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c

[ ... ]

> @@ -46,6 +47,16 @@ static int enable_eud(struct eud_chip *priv)
>  {
>  	int ret;
>  
> +	/*
> +	 * On QCM2290 the boot firmware sets the TCSR check register before
> +	 * EUD_EN2; without it the mode manager ignores the enable.
> +	 */
> +	if (priv->tcsr_check) {
> +		ret = qcom_scm_io_writel(priv->tcsr_check, 1);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 1);
>  	if (ret)
>  		return ret;

[Severity: Medium]
Does this leave priv->tcsr_check set to 1 when the write to
EUD_REG_EUD_EN2 fails?

On non-debug TF-A builds, the write to priv->tcsr_check succeeds while the
write to EUD_REG_EUD_EN2 is rejected.  When enable_store() calls
disable_eud() to roll back after enable_eud() fails, disable_eud() also
writes to EUD_REG_EUD_EN2 first and returns on error before clearing
priv->tcsr_check:

drivers/usb/misc/qcom_eud.c:disable_eud() {
	ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 0);
	if (ret)
		return ret;

	if (priv->tcsr_check)
		qcom_scm_io_writel(priv->tcsr_check, 0);
    ...
}

As a result, the TCSR check register remains set to 1 even though enabling
the EUD failed.

> @@ -65,6 +76,9 @@ static int disable_eud(struct eud_chip *priv)
>  	if (ret)
>  		return ret;
>  
> +	if (priv->tcsr_check)
> +		qcom_scm_io_writel(priv->tcsr_check, 0);
> +
>  	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
>  	return 0;
>  }

[Severity: Medium]
This problem wasn't introduced by this patch, but does enable_store()
leave chip->enabled stuck as true [1] after disable_eud() succeeds?

drivers/usb/misc/qcom_eud.c:enable_store() {
    ...
	if (enable) {
		ret = enable_eud(chip);
		if (!ret)
			chip->enabled = enable;
		else
			disable_eud(chip);

	} else {
		ret = disable_eud(chip);
	}
    ...
}

Because the disable branch never clears chip->enabled, subsequent reads of
the enable sysfs attribute still report 1 after the EUD has been disabled.

[1] https://sashiko.dev/#/bug/linux-99d39974-0e5d-4e78-9df1-4c5cd22f0990

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-qcm2290-eud-v1-0-b6157b817377@oss.qualcomm.com?part=2

[PATCH 3/4] arm64: dts: qcom: agatti: add the Embedded USB Debugger

[Igor Opaniuk]

Describe the EUD: its CSRs at 0x1610000, the mode manager register at
0x1612000 and TCSR_APSS_SPARE_REG1 at 0x3e5018, which the secure world
writes on behalf of the driver, and its interrupt (eud_int[1], GIC SPI
189). Hook its port to a second endpoint of the dwc3 high-speed port so
the driver finds the role switch, while the board keeps the first
endpoint for its connector.

Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/agatti.dtsi | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/agatti.dtsi b/arch/arm64/boot/dts/qcom/agatti.dtsi
index ea9b39b1ed43..a77a5bf87838 100644
--- a/arch/arm64/boot/dts/qcom/agatti.dtsi
+++ b/arch/arm64/boot/dts/qcom/agatti.dtsi
@@ -826,6 +826,29 @@ gcc: clock-controller@1400000 {
 			#power-domain-cells = <1>;
 		};
 
+		eud: eud@1610000 {
+			compatible = "qcom,qcm2290-eud", "qcom,eud";
+			reg = <0x0 0x01610000 0x0 0x2000>,
+			      <0x0 0x01612000 0x0 0x1000>,
+			      <0x0 0x003e5018 0x0 0x4>;
+			interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>;
+
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+
+					eud_ep: endpoint {
+						remote-endpoint = <&usb_dwc3_hs_eud>;
+					};
+				};
+			};
+		};
+
 		usb_hsphy: phy@1613000 {
 			compatible = "qcom,qcm2290-qusb2-phy";
 			reg = <0x0 0x01613000 0x0 0x180>;
@@ -1680,8 +1703,16 @@ ports {
 
 				port@0 {
 					reg = <0>;
+					#address-cells = <1>;
+					#size-cells = <0>;
 
-					usb_dwc3_hs: endpoint {
+					usb_dwc3_hs: endpoint@0 {
+						reg = <0>;
+					};
+
+					usb_dwc3_hs_eud: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&eud_ep>;
 					};
 				};
 

-- 
2.53.0

Re: [PATCH 3/4] arm64: dts: qcom: agatti: add the Embedded USB Debugger

[sashiko-bot]

> Describe the EUD: its CSRs at 0x1610000, the mode manager register at
> 0x1612000 and TCSR_APSS_SPARE_REG1 at 0x3e5018, which the secure world
> writes on behalf of the driver, and its interrupt (eud_int[1], GIC SPI
> 189). Hook its port to a second endpoint of the dwc3 high-speed port so
> the driver finds the role switch, while the board keeps the first
> endpoint for its connector.
> 
> Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-qcm2290-eud-v1-0-b6157b817377@oss.qualcomm.com?part=3

[PATCH 4/4] arm64: dts: qcom: qrb2210-arduino-imola: enable the EUD

[Igor Opaniuk]

The UNO Q exposes the EUD on its USB-C port: with it enabled, a host
sees the EUD hub and its SWD/trace functions, which OpenOCD's EUD
adapter uses to debug the SoC.

Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
index e99d2a56dc28..a1af2caa5bfc 100644
--- a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
+++ b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
@@ -226,6 +226,10 @@ vph_pwr: regulator-vph-pwr {
 	};
 };
 
+&eud {
+	status = "okay";
+};
+
 &gpi_dma0 {
 	status = "okay";
 };

-- 
2.53.0

Re: [PATCH 4/4] arm64: dts: qcom: qrb2210-arduino-imola: enable the EUD

[sashiko-bot]

> The UNO Q exposes the EUD on its USB-C port: with it enabled, a host
> sees the EUD hub and its SWD/trace functions, which OpenOCD's EUD
> adapter uses to debug the SoC.
> 
> Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-qcm2290-eud-v1-0-b6157b817377@oss.qualcomm.com?part=4