[PATCH v2 0/4] usb: chipidea: tegra: Add external control option

[PATCH v2 0/4] usb: chipidea: tegra: Add external control option

[Svyatoslav Ryhel]

Patchset adjusts the Tegra-specific portion of the Chipidea USB controller
to allow for the disabling of automatic PHY and USB controller
registration. This is achieved by adding the nvidia,external-control
property/flag. It does not affect any existing configurations, but it
allows the USB line to be registered or deregistered by an external
device — in this case, the modem.

Since Tegra USB PHY holds clock gating for controller PHY cannot be
handled by code Chipidea mechanism.

This patchset is a part of larger series aiming to bring XMM6260 modem
support for Tegra devices:
https://lore.kernel.org/lkml/20260511135703.62470-1-clamor95@gmail.com/

---
Changes in v2:
- separated Tegra USB into its own schema
- tegra_usb structure moved into tegra_usb_phy.h
---

Svyatoslav Ryhel (4):
  dt-bindings: usb: Move Tegra-specific Chipidea USB properties into a
    dedicated schema
  dt-bindings: usb: chipidea,usb2-tegra: Document
    nvidia,external-control property
  usb: chipidea: tegra: Avoid controller/PHY init if bus is externally
    controlled
  usb: chipidea: tegra: Expose tegra_usb structure

 .../bindings/usb/chipidea,usb2-tegra.yaml     | 95 +++++++++++++++++++
 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 22 -----
 drivers/usb/chipidea/ci_hdrc_tegra.c          | 43 ++++-----
 include/linux/usb/tegra_usb_phy.h             | 15 +++
 4 files changed, 131 insertions(+), 44 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml

-- 
2.51.0

[PATCH v2 1/4] dt-bindings: usb: Move Tegra-specific Chipidea USB properties into a dedicated schema

[Svyatoslav Ryhel]

Move all Tegra-specific Chipidea devices and their properties into a
dedicated schema file, by analogy with i.MX.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 .../bindings/usb/chipidea,usb2-tegra.yaml     | 88 +++++++++++++++++++
 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 22 -----
 2 files changed, 88 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml

diff --git a/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml b/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml
new file mode 100644
index 000000000000..78046f8a63ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/chipidea,usb2-tegra.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Tegra USB2 ChipIdea USB controller
+
+maintainers:
+  - Svyatoslav Ryhel <clamor95@gmail.com>
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - nvidia,tegra20-ehci
+          - nvidia,tegra20-udc
+          - nvidia,tegra30-ehci
+          - nvidia,tegra30-udc
+          - nvidia,tegra114-udc
+          - nvidia,tegra124-udc
+      - items:
+          - enum:
+              - nvidia,tegra114-ehci
+              - nvidia,tegra124-ehci
+              - nvidia,tegra210-ehci
+          - const: nvidia,tegra30-ehci
+
+  operating-points-v2:
+    description: A phandle to the OPP table containing the performance states.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  nvidia,phy:
+    description: phandle of usb phy that connects to the port. Use "phys" instead.
+    $ref: /schemas/types.yaml#/definitions/phandle
+    deprecated: true
+
+  nvidia,needs-double-reset:
+    description: Indicates double reset or not.
+    type: boolean
+    deprecated: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - resets
+
+allOf:
+  - $ref: chipidea,usb2-common.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/tegra30-car.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    usb@7d000000 {
+        compatible = "nvidia,tegra30-udc";
+        reg = <0x7d000000 0x4000>;
+        interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
+        phy_type = "utmi";
+        clocks = <&tegra_car TEGRA30_CLK_USBD>;
+        resets = <&tegra_car 22>;
+        reset-names = "usb";
+        phys = <&phy1>;
+        power-domains = <&pd_core>;
+        operating-points-v2 = <&usbd_dvfs_opp_table>;
+
+        dr_mode = "otg";
+
+        hnp-disable;
+        srp-disable;
+        adp-disable;
+
+        usb-role-switch;
+        extcon = <&charger>, <&extcon>; /* vbus, id */
+        vbus-supply = <&usb_otg_vbus>;
+
+        port {
+            usb_in: endpoint {
+                remote-endpoint = <&connector_out>;
+            };
+        };
+    };
+...
diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
index 691d6cf02c27..5abca0572714 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
@@ -17,20 +17,8 @@ properties:
           - chipidea,usb2
           - lsi,zevio-usb
           - nuvoton,npcm750-udc
-          - nvidia,tegra20-ehci
-          - nvidia,tegra20-udc
-          - nvidia,tegra30-ehci
-          - nvidia,tegra30-udc
-          - nvidia,tegra114-udc
-          - nvidia,tegra124-udc
           - nxp,s32g2-usb
           - qcom,ci-hdrc
-      - items:
-          - enum:
-              - nvidia,tegra114-ehci
-              - nvidia,tegra124-ehci
-              - nvidia,tegra210-ehci
-          - const: nvidia,tegra30-ehci
       - items:
           - const: xlnx,zynq-usb-2.20a
           - const: chipidea,usb2
@@ -65,16 +53,6 @@ properties:
       - description: register offset
       - description: phy index
 
-  nvidia,phy:
-    description: phandle of usb phy that connects to the port. Use "phys" instead.
-    $ref: /schemas/types.yaml#/definitions/phandle
-    deprecated: true
-
-  nvidia,needs-double-reset:
-    description: Indicates double reset or not.
-    type: boolean
-    deprecated: true
-
   ulpi:
     type: object
     additionalProperties: false
-- 
2.51.0

[PATCH v2 2/4] dt-bindings: usb: chipidea,usb2-tegra: Document nvidia,external-control property

[Svyatoslav Ryhel]

Document the nvidia,external-control property required, for example, for
USB lines in HSIC mode connected to a modem, where the modem requires
precise control over the USB bus to properly enumerate all its stages and
intermediate devices.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 .../devicetree/bindings/usb/chipidea,usb2-tegra.yaml       | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml b/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml
index 78046f8a63ad..d99a18900b5a 100644
--- a/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml
+++ b/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml
@@ -40,6 +40,13 @@ properties:
     type: boolean
     deprecated: true
 
+  nvidia,external-control:
+    description:
+      Indicates that the controller is configured externally and that the host
+      should not attempt to access it. This is usually used by a modem that
+      requires precise bus configuration.
+    type: boolean
+
 required:
   - compatible
   - reg
-- 
2.51.0

[PATCH v2 3/4] usb: chipidea: tegra: Avoid controller/PHY init if bus is externally controlled

[Svyatoslav Ryhel]

If the USB controller and PHY are externally controlled, then the
registration of the controller and the PHY initialization should be
skipped, since these configurations must be done by the device that
controls the bus to work correctly.

Since USB PHY in Tegra controls clock gates required by the controller
itself, Chipidea core PHY management is not suitable for Tegra.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/usb/chipidea/ci_hdrc_tegra.c | 32 ++++++++++++++++++----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c
index 372788f0f970..a19cb3b33bf3 100644
--- a/drivers/usb/chipidea/ci_hdrc_tegra.c
+++ b/drivers/usb/chipidea/ci_hdrc_tegra.c
@@ -32,6 +32,7 @@ struct tegra_usb {
 	struct clk *clk;
 
 	bool needs_double_reset;
+	bool externally_controlled;
 };
 
 struct tegra_usb_soc_info {
@@ -312,6 +313,9 @@ static int tegra_usb_probe(struct platform_device *pdev)
 	if (device_property_present(&pdev->dev, "nvidia,needs-double-reset"))
 		usb->needs_double_reset = true;
 
+	if (device_property_present(&pdev->dev, "nvidia,external-control"))
+		usb->externally_controlled = true;
+
 	err = tegra_usb_reset_controller(&pdev->dev);
 	if (err) {
 		dev_err_probe(&pdev->dev, err, "failed to reset controller");
@@ -323,9 +327,11 @@ static int tegra_usb_probe(struct platform_device *pdev)
 	 * initialized, otherwise CPU will hang because clocks are gated.
 	 * PHY driver controls gating of internal USB clocks on Tegra.
 	 */
-	err = usb_phy_init(usb->phy);
-	if (err)
-		goto fail_power_off;
+	if (!usb->externally_controlled) {
+		err = usb_phy_init(usb->phy);
+		if (err)
+			goto fail_power_off;
+	}
 
 	/* setup and register ChipIdea HDRC device */
 	usb->soc = soc;
@@ -342,12 +348,14 @@ static int tegra_usb_probe(struct platform_device *pdev)
 	if (of_usb_get_phy_mode(pdev->dev.of_node) == USBPHY_INTERFACE_MODE_ULPI)
 		usb->data.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM;
 
-	usb->dev = ci_hdrc_add_device(&pdev->dev, pdev->resource,
-				      pdev->num_resources, &usb->data);
-	if (IS_ERR(usb->dev)) {
-		err = dev_err_probe(&pdev->dev, PTR_ERR(usb->dev),
-				    "failed to add HDRC device");
-		goto phy_shutdown;
+	if (!usb->externally_controlled) {
+		usb->dev = ci_hdrc_add_device(&pdev->dev, pdev->resource,
+					      pdev->num_resources, &usb->data);
+		if (IS_ERR(usb->dev)) {
+			err = dev_err_probe(&pdev->dev, PTR_ERR(usb->dev),
+					    "failed to add HDRC device");
+			goto phy_shutdown;
+		}
 	}
 
 	return 0;
@@ -365,8 +373,10 @@ static void tegra_usb_remove(struct platform_device *pdev)
 {
 	struct tegra_usb *usb = platform_get_drvdata(pdev);
 
-	ci_hdrc_remove_device(usb->dev);
-	usb_phy_shutdown(usb->phy);
+	if (!usb->externally_controlled) {
+		ci_hdrc_remove_device(usb->dev);
+		usb_phy_shutdown(usb->phy);
+	}
 
 	pm_runtime_put_sync_suspend(&pdev->dev);
 	pm_runtime_force_suspend(&pdev->dev);
-- 
2.51.0

[PATCH v2 4/4] usb: chipidea: tegra: Expose tegra_usb structure

[Svyatoslav Ryhel]

The tegra_usb structure is platform data of USB controller and is required
for proper controller malipulation if its init/deinit is externally
controlled.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/usb/chipidea/ci_hdrc_tegra.c | 13 +------------
 include/linux/usb/tegra_usb_phy.h    | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c
index a19cb3b33bf3..109c4afdee4a 100644
--- a/drivers/usb/chipidea/ci_hdrc_tegra.c
+++ b/drivers/usb/chipidea/ci_hdrc_tegra.c
@@ -16,6 +16,7 @@
 #include <linux/usb/hcd.h>
 #include <linux/usb/of.h>
 #include <linux/usb/phy.h>
+#include <linux/usb/tegra_usb_phy.h>
 
 #include <soc/tegra/common.h>
 
@@ -23,18 +24,6 @@
 
 #include "ci.h"
 
-struct tegra_usb {
-	struct ci_hdrc_platform_data data;
-	struct platform_device *dev;
-
-	const struct tegra_usb_soc_info *soc;
-	struct usb_phy *phy;
-	struct clk *clk;
-
-	bool needs_double_reset;
-	bool externally_controlled;
-};
-
 struct tegra_usb_soc_info {
 	unsigned long flags;
 	unsigned int txfifothresh;
diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
index 6343f88df5de..ba74b7ebfba0 100644
--- a/include/linux/usb/tegra_usb_phy.h
+++ b/include/linux/usb/tegra_usb_phy.h
@@ -9,6 +9,7 @@
 #include <linux/clk.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
+#include <linux/usb/chipidea.h>
 #include <linux/usb/otg.h>
 
 struct gpio_desc;
@@ -62,6 +63,20 @@ enum tegra_usb_phy_port_speed {
 	TEGRA_USB_PHY_PORT_SPEED_HIGH,
 };
 
+struct tegra_usb_soc_info;
+
+struct tegra_usb {
+	struct ci_hdrc_platform_data data;
+	struct platform_device *dev;
+
+	const struct tegra_usb_soc_info *soc;
+	struct usb_phy *phy;
+	struct clk *clk;
+
+	bool needs_double_reset;
+	bool externally_controlled;
+};
+
 struct tegra_xtal_freq;
 
 struct tegra_usb_phy {
-- 
2.51.0

Re: [PATCH v2 2/4] dt-bindings: usb: chipidea,usb2-tegra: Document nvidia,external-control property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Adding `nvidia,external-control` to prevent the host from accessing the controller is an architectural anti-pattern that duplicates the purpose of the standard `status` property and misattributes resource ownership.
--

commit ba773e6bd558f2861c226bd39627e8f4363d1a0d
Author: Svyatoslav Ryhel <clamor95@gmail.com>

dt-bindings: usb: chipidea,usb2-tegra: Document nvidia,external-control property

This commit documents the nvidia,external-control property for USB controllers.
It indicates that the controller is configured externally, typically for a
modem requiring precise control over the USB bus in HSIC mode.

> diff --git a/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml b/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml
> index 78046f8a63ad5..d99a18900b5a8 100644
> --- a/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml
> +++ b/Documentation/devicetree/bindings/usb/chipidea,usb2-tegra.yaml
[ ... ]
> @@ -40,6 +40,13 @@ properties:
>      type: boolean
>      deprecated: true
>  
> +  nvidia,external-control:
> +    description:
> +      Indicates that the controller is configured externally and that the host
> +      should not attempt to access it. This is usually used by a modem that
> +      requires precise bus configuration.
> +    type: boolean
> +

[Severity: Medium]
Does this new property duplicate the purpose of the standard status property
and misattribute resource ownership?

Device tree is intended to describe hardware, not software workarounds. If
the hardware block is exclusively controlled by an external entity like a
modem and should not be accessed by the host OS, standard semantics generally
dictate using status = "disabled" or status = "reserved".

If the modem relies on the host to provide shared resources like clocks or
resets to bring the bus up before enumeration, could those resources be
modeled via a power-sequencer or assigned directly to the modem hardware
node? 

Modeling the resources this way might avoid using the USB controller node as a
silent resource enabler, which would help keep the hardware description
decoupled from software policy.

>  required:
>    - compatible
>    - reg

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523083013.46372-1-clamor95@gmail.com?part=2

Re: [PATCH v2 3/4] usb: chipidea: tegra: Avoid controller/PHY init if bus is externally controlled

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds I/O memory access in `tegra_ehci_hub_control()` due to unvalidated `wIndex`.
--

commit 60717c8d1b44f3d9bbb2d143c18add1a1d62f035
Author: Svyatoslav Ryhel <clamor95@gmail.com>

usb: chipidea: tegra: Avoid controller/PHY init if bus is externally controlled

This commit skips the registration of the USB controller and PHY
initialization if they are externally controlled. Since the USB PHY
in Tegra controls clock gates required by the controller itself, it
defers this configuration to the device that controls the bus.

> diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c
> index 372788f0f9701..a19cb3b33bf37 100644
> --- a/drivers/usb/chipidea/ci_hdrc_tegra.c
> +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c
[ ... ]
> @@ -342,12 +348,14 @@ static int tegra_usb_probe(struct platform_device *pdev)
>  	if (of_usb_get_phy_mode(pdev->dev.of_node) == USBPHY_INTERFACE_MODE_ULPI)
>  		usb->data.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM;
>  
> -	usb->dev = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> -				      pdev->num_resources, &usb->data);

[Severity: High]
This is a pre-existing issue, but does tegra_ehci_hub_control() have an
out-of-bounds I/O memory access?

When handling root hub control requests, the core USB code in
rh_call_control() passes the raw, user-supplied wIndex (e.g., from usbfs via
the USBDEVFS_CONTROL ioctl) to the HCD's hub_control callback.

In tegra_ehci_hub_control(), which is passed in the usb->data structure here,
the pointer to the port status register is calculated without bounds checking
on wIndex:

status_reg = &ehci->regs->port_status[(wIndex & 0xff) - 1]

If wIndex & 0xff evaluates to 0, the calculation produces an index of -1
(pointing to the configured_flag register). If it is a large number like 255,
it results in an index of 254, reading far past the port_status array which
has a size of HCS_N_PORTS_MAX (15).

Could sending a SetPortFeature request with wValue = USB_PORT_FEAT_RESET
and a crafted wIndex allow a user with access to the root hub device node to
force the driver to perform multiple reads and writes at arbitrary MMIO
offsets?

> +	if (!usb->externally_controlled) {
> +		usb->dev = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> +					      pdev->num_resources, &usb->data);
> +		if (IS_ERR(usb->dev)) {
> +			err = dev_err_probe(&pdev->dev, PTR_ERR(usb->dev),
> +					    "failed to add HDRC device");
> +			goto phy_shutdown;
> +		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523083013.46372-1-clamor95@gmail.com?part=3

Re: [PATCH v2 4/4] usb: chipidea: tegra: Expose tegra_usb structure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Exposing the private driver data structure `struct tegra_usb` in a public header violates driver encapsulation.
--

commit 458c2154b4c34aa542cc2d5d1ce84db0b5258a4b
Author: Svyatoslav Ryhel <clamor95@gmail.com>
usb: chipidea: tegra: Expose tegra_usb structure

This commit moves the tegra_usb structure from the tegra chipidea driver into
a public header file. The commit message states this is required for proper
manipulation when init and deinit are externally controlled.

> diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
> index 6343f88df5de8..ba74b7ebfba0e 100644
> --- a/include/linux/usb/tegra_usb_phy.h
> +++ b/include/linux/usb/tegra_usb_phy.h
[ ... ]
> @@ -62,6 +63,20 @@ enum tegra_usb_phy_port_speed {
>  	TEGRA_USB_PHY_PORT_SPEED_HIGH,
>  };
>  
> +struct tegra_usb_soc_info;
> +
> +struct tegra_usb {
> +	struct ci_hdrc_platform_data data;
> +	struct platform_device *dev;
> +
> +	const struct tegra_usb_soc_info *soc;
> +	struct usb_phy *phy;
> +	struct clk *clk;
> +
> +	bool needs_double_reset;
> +	bool externally_controlled;
> +};

[Severity: Low]
Does moving this private runtime state into a public header break driver
encapsulation?

Exposing the raw internal state structure allows external entities to
directly manipulate the controller's internal state, tightly coupling the
PHY and controller drivers.

Would it be better to provide explicit API functions, such as init and
deinit helpers, for cross-driver manipulation instead of exposing the
internal structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523083013.46372-1-clamor95@gmail.com?part=4