[PATCH 0/2] phy: nuvoton: Add MA35D1 USB2 OTG PHY driver

[PATCH 0/2] phy: nuvoton: Add MA35D1 USB2 OTG PHY driver

[Joey Lu]

The Nuvoton MA35D1 SoC has two USB 2.0 ports:

  USB0 is an OTG-capable port.  Its physical signals are routed by a
  hardware mux to either a DWC2 gadget controller or the EHCI0/OHCI0
  host controllers, depending on the USB ID pin state.  The DWC2 IP is
  device-only in hardware, so all host-mode operation on USB0 is
  handled by EHCI0/OHCI0.

  USB1 is a dedicated host-only port served by EHCI1/OHCI1.

About this driver:

  - Runs the PHY Power-On Reset sequence, with a guard that skips
    re-initialization if the PHY is already operational.

  - Supports optional resistor calibration trim (nuvoton,rcalcode) and
    over-current detect polarity configuration (nuvoton,oc-active-high).

  - For PHY0 (USB0) only: registers a USB role switch that reads the
    hardware ID pin state from PWRONOTP[16] on every query.

Joey Lu (2):
  dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY  binding
  phy: nuvoton: Add MA35D1 USB2 OTG PHY driver

 .../phy/nuvoton,ma35d1-usb2-phy-otg.yaml      |  79 ++++++
 drivers/phy/nuvoton/Kconfig                   |  15 +
 drivers/phy/nuvoton/Makefile                  |   1 +
 drivers/phy/nuvoton/phy-ma35d1-otg.c          | 264 ++++++++++++++++++
 4 files changed, 359 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
 create mode 100644 drivers/phy/nuvoton/phy-ma35d1-otg.c

-- 
2.43.0

[PATCH 1/2] dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding

[Joey Lu]

Add device tree binding documentation for the Nuvoton MA35D1 USB 2.0
OTG PHY driver (nuvoton,ma35d1-usb2-phy-otg).

PHY index 0 (USB0) is an OTG port whose signals are routed by a hardware
mux to either the DWC2 device controller or the EHCI0/OHCI0 host
controllers depending on the USB ID pin state.  PHY index 1 (USB1) is a
dedicated host-only port.

Optional properties allow board-specific resistor calibration trim
(nuvoton,rcalcode) and over-current detect polarity configuration
(nuvoton,oc-active-high).

Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
 .../phy/nuvoton,ma35d1-usb2-phy-otg.yaml      | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml

diff --git a/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
new file mode 100644
index 000000000000..19f074565cc6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/nuvoton,ma35d1-usb2-phy-otg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 USB 2.0 host PHY
+
+maintainers:
+  - Joey Lu <yclu4@nuvoton.com>
+
+description:
+  USB 2.0 PHY driver for the Nuvoton MA35D1 SoC, used by the EHCI and
+  OHCI host controllers.
+
+  USB0 (PHY index 0) is an OTG port whose physical signals are routed to
+  either the DWC2 device controller or the EHCI0/OHCI0 host controller by
+  a hardware mux that follows the USB ID pin.
+
+  USB1 (PHY index 1) is a dedicated host port with no OTG capability.
+
+properties:
+  compatible:
+    const: nuvoton,ma35d1-usb2-phy-otg
+
+  clocks:
+    maxItems: 1
+
+  nuvoton,sys:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to the system management syscon.
+          - description: PHY instance index.
+            enum:
+              - 0   # USB0, OTG port (shared with DWC2 gadget controller)
+              - 1   # USB1, host-only port
+    description:
+      A phandle to the syscon node covering the SYS register block, with
+      one argument selecting the PHY instance. Index 0 selects the OTG
+      port PHY (USB0) and index 1 selects the host-only PHY (USB1).
+
+  "#phy-cells":
+    const: 0
+
+  nuvoton,rcalcode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 15
+    description:
+      Resistor calibration trim code written to the RCALCODE field in
+      USBPMISCR. The 4-bit value adjusts the PHY's internal termination
+      resistance. When absent the hardware reset default is used.
+
+  nuvoton,oc-active-high:
+    type: boolean
+    description:
+      When present, the over-current detect input from the VBUS power
+      switch is treated as active-high. The default (property absent) is
+      active-low. This setting is shared by both USB host ports.
+
+required:
+  - compatible
+  - clocks
+  - nuvoton,sys
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+
+    usb_hphy0: usb-host-phy {
+        compatible = "nuvoton,ma35d1-usb2-phy-otg";
+        clocks = <&clk HUSBH0_GATE>;
+        nuvoton,sys = <&sys 0>;
+        #phy-cells = <0>;
+    };
-- 
2.43.0

[PATCH 2/2] phy: nuvoton: Add MA35D1 USB2 OTG PHY driver

[Joey Lu]

Add a PHY driver for the USB 2.0 PHYs in the Nuvoton MA35D1 SoC,
intended for use with the EHCI and OHCI host controllers.

The MA35D1 SoC has two USB ports:

  - USB0: an OTG port shared between a DWC2 gadget controller and
    EHCI0/OHCI0 host controllers.  A hardware mux automatically routes
    the physical USB0 signals to the appropriate controller based on the
    USB ID pin state.  The DWC2 IP is device-only in hardware,
    so host-mode operation on USB0 is handled entirely by EHCI0/OHCI0.

  - USB1: a dedicated host-only port served by EHCI1/OHCI1.

The driver implements:
  - Power-On Reset sequence with a guard that skips re-initialization if
    the PHY is already operational.  This protects PHY0 when the DWC2
    gadget driver has already run its own init before EHCI0 probes.
  - Optional resistor calibration trim via nuvoton,rcalcode.
  - Optional over-current detect polarity via nuvoton,oc-active-high.
  - For PHY0 only: a USB role switch that exposes the hardware ID pin
    state (PWRONOTP[16]).

Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
 drivers/phy/nuvoton/Kconfig          |  15 ++
 drivers/phy/nuvoton/Makefile         |   1 +
 drivers/phy/nuvoton/phy-ma35d1-otg.c | 264 +++++++++++++++++++++++++++
 3 files changed, 280 insertions(+)
 create mode 100644 drivers/phy/nuvoton/phy-ma35d1-otg.c

diff --git a/drivers/phy/nuvoton/Kconfig b/drivers/phy/nuvoton/Kconfig
index d02cae2db315..5fdd13f841e7 100644
--- a/drivers/phy/nuvoton/Kconfig
+++ b/drivers/phy/nuvoton/Kconfig
@@ -10,3 +10,18 @@ config PHY_MA35_USB
 	help
 	  Enable this to support the USB2.0 PHY on the Nuvoton MA35
 	  series SoCs.
+
+config PHY_MA35_USB_OTG
+	tristate "Nuvoton MA35 USB2.0 OTG PHY driver"
+	depends on ARCH_MA35 || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+	select MFD_SYSCON
+	select USB_ROLE_SWITCH
+	help
+	  Enable this to support the USB2.0 OTG PHY on the Nuvoton MA35
+	  series SoCs.  This driver handles PHY initialization for the
+	  EHCI/OHCI host controllers, including per-PHY power-on reset,
+	  resistor calibration trim, and over-current polarity
+	  configuration.  For the OTG port (PHY0), it also monitors the
+	  USB ID pin and registers a USB role switch.
diff --git a/drivers/phy/nuvoton/Makefile b/drivers/phy/nuvoton/Makefile
index 2937e3921898..3ecd76f35d7c 100644
--- a/drivers/phy/nuvoton/Makefile
+++ b/drivers/phy/nuvoton/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_PHY_MA35_USB)		+= phy-ma35d1-usb2.o
+obj-$(CONFIG_PHY_MA35_USB_OTG)		+= phy-ma35d1-otg.o
diff --git a/drivers/phy/nuvoton/phy-ma35d1-otg.c b/drivers/phy/nuvoton/phy-ma35d1-otg.c
new file mode 100644
index 000000000000..53bc6ddf755e
--- /dev/null
+++ b/drivers/phy/nuvoton/phy-ma35d1-otg.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nuvoton MA35D1 USB 2.0 OTG PHY driver
+ *
+ * PHY0 (USB0) is shared between DWC2 gadget and EHCI0/OHCI0 host
+ * controllers. The hardware mux switches automatically via the USB
+ * ID pin. PHY1 (USB1) is host-only.
+ *
+ * Copyright (C) 2026 Nuvoton Technology Corp.
+ */
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/usb/role.h>
+
+#define MA35_SYS_PWRONOTP		0x04
+#define PWRONOTP_USBP0ID		BIT(16)
+
+#define MA35_SYS_USBPMISCR		0x60
+#define USBPMISCR_PHY_POR(n)		BIT(0 + (n) * 16)
+#define USBPMISCR_PHY_SUSPEND(n)	BIT(1 + (n) * 16)
+#define USBPMISCR_PHY_COMN(n)		BIT(2 + (n) * 16)
+#define USBPMISCR_PHY_HSTCKSTB(n)	BIT(8 + (n) * 16)
+#define USBPMISCR_PHY_CK12MSTB(n)	BIT(9 + (n) * 16)
+/* Mask for control bits (POR, SUSPEND, COMN) of one PHY */
+#define USBPMISCR_PHY_CTL_MASK(n)	(0x7 << ((n) * 16))
+/* Host-mode ready: SUSPEND + HSTCKSTB + CK12MSTB */
+#define USBPMISCR_PHY_HOST_READY(n)	(USBPMISCR_PHY_SUSPEND(n)  | \
+					 USBPMISCR_PHY_HSTCKSTB(n) | \
+					 USBPMISCR_PHY_CK12MSTB(n))
+/* RCALCODE: 4-bit resistor trim at bits [15:12] (PHY0) or [31:28] (PHY1) */
+#define USBPMISCR_RCAL_SHIFT(n)		(12 + (n) * 16)
+#define USBPMISCR_RCAL_MASK(n)		GENMASK(USBPMISCR_RCAL_SHIFT(n) + 3, \
+						USBPMISCR_RCAL_SHIFT(n))
+
+#define MA35_SYS_MISCFCR0		0x70
+/* MISCFCR0[12]: USB host over-current detect polarity (shared, both ports) */
+#define MISCFCR0_UHOVRCURH		BIT(12)
+
+struct ma35_otg_phy {
+	struct clk *clk;
+	struct device *dev;
+	struct regmap *sysreg;
+	unsigned int phy_idx;
+	struct usb_role_switch *role_sw;
+	enum usb_role cur_role;
+};
+
+static int ma35_otg_phy_init(struct phy *phy)
+{
+	struct ma35_otg_phy *p = phy_get_drvdata(phy);
+	unsigned int n = p->phy_idx;
+	u32 ready_mask = USBPMISCR_PHY_HOST_READY(n);
+	unsigned int val;
+	int ret;
+
+	regmap_read(p->sysreg, MA35_SYS_USBPMISCR, &val);
+	if ((val & ready_mask) == ready_mask)
+		return 0;
+
+	regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
+			   USBPMISCR_PHY_CTL_MASK(n),
+			   USBPMISCR_PHY_POR(n) | USBPMISCR_PHY_SUSPEND(n));
+	msleep(20);
+
+	regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
+			   USBPMISCR_PHY_CTL_MASK(n),
+			   USBPMISCR_PHY_SUSPEND(n));
+
+	ret = regmap_read_poll_timeout(p->sysreg, MA35_SYS_USBPMISCR, val,
+				       (val & ready_mask) == ready_mask,
+				       10, 1000);
+	if (ret) {
+		dev_err(p->dev, "USB PHY%u clock not stable (USBPMISCR=0x%08x)\n",
+			n, val);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ma35_otg_phy_power_on(struct phy *phy)
+{
+	struct ma35_otg_phy *p = phy_get_drvdata(phy);
+
+	return clk_prepare_enable(p->clk);
+}
+
+static int ma35_otg_phy_power_off(struct phy *phy)
+{
+	struct ma35_otg_phy *p = phy_get_drvdata(phy);
+
+	clk_disable_unprepare(p->clk);
+	return 0;
+}
+
+static const struct phy_ops ma35_otg_phy_ops = {
+	.init = ma35_otg_phy_init,
+	.power_on = ma35_otg_phy_power_on,
+	.power_off = ma35_otg_phy_power_off,
+	.owner = THIS_MODULE,
+};
+
+static enum usb_role ma35_otg_read_id(struct ma35_otg_phy *p)
+{
+	unsigned int val;
+
+	regmap_read(p->sysreg, MA35_SYS_PWRONOTP, &val);
+	return (val & PWRONOTP_USBP0ID) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+}
+
+static int ma35_otg_role_sw_set(struct usb_role_switch *sw,
+				enum usb_role role)
+{
+	struct ma35_otg_phy *p = usb_role_switch_get_drvdata(sw);
+
+	p->cur_role = role;
+
+	return 0;
+}
+
+static enum usb_role ma35_otg_role_sw_get(struct usb_role_switch *sw)
+{
+	struct ma35_otg_phy *p = usb_role_switch_get_drvdata(sw);
+
+	return ma35_otg_read_id(p);
+}
+
+static int ma35_otg_role_switch_init(struct platform_device *pdev,
+				     struct ma35_otg_phy *p)
+{
+	struct usb_role_switch_desc sw_desc = { };
+
+	p->cur_role = ma35_otg_read_id(p);
+
+	sw_desc.set = ma35_otg_role_sw_set;
+	sw_desc.get = ma35_otg_role_sw_get;
+	sw_desc.allow_userspace_control = true;
+	sw_desc.driver_data = p;
+	sw_desc.fwnode = dev_fwnode(&pdev->dev);
+
+	p->role_sw = usb_role_switch_register(&pdev->dev, &sw_desc);
+	if (IS_ERR(p->role_sw))
+		return dev_err_probe(&pdev->dev, PTR_ERR(p->role_sw),
+				     "failed to register role switch\n");
+
+	return 0;
+}
+
+static void ma35_otg_role_switch_exit(struct ma35_otg_phy *p)
+{
+	if (!p->role_sw)
+		return;
+
+	usb_role_switch_unregister(p->role_sw);
+	p->role_sw = NULL;
+}
+
+static int ma35_otg_phy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *provider;
+	struct ma35_otg_phy *p;
+	unsigned int sys_args[1];
+	struct phy *phy;
+	u32 rcalcode;
+	int ret;
+
+	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	p->dev = &pdev->dev;
+	platform_set_drvdata(pdev, p);
+
+	p->sysreg = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
+							 "nuvoton,sys",
+							 1, sys_args);
+	if (IS_ERR(p->sysreg))
+		return dev_err_probe(&pdev->dev, PTR_ERR(p->sysreg),
+				     "Failed to get SYS regmap\n");
+
+	p->phy_idx = sys_args[0];
+
+	if (p->phy_idx > 1)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "invalid PHY index %u (must be 0 or 1)\n",
+				     p->phy_idx);
+
+	p->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(p->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(p->clk),
+				     "failed to get PHY clock\n");
+
+	if (!of_property_read_u32(pdev->dev.of_node, "nuvoton,rcalcode",
+				  &rcalcode)) {
+		if (rcalcode > 15)
+			return dev_err_probe(&pdev->dev, -EINVAL,
+					     "rcalcode %u out of range (0-15)\n",
+					     rcalcode);
+		regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
+				   USBPMISCR_RCAL_MASK(p->phy_idx),
+				   rcalcode << USBPMISCR_RCAL_SHIFT(p->phy_idx));
+	}
+
+	if (of_property_read_bool(pdev->dev.of_node, "nuvoton,oc-active-high"))
+		regmap_update_bits(p->sysreg, MA35_SYS_MISCFCR0,
+				   MISCFCR0_UHOVRCURH, MISCFCR0_UHOVRCURH);
+
+	phy = devm_phy_create(&pdev->dev, pdev->dev.of_node, &ma35_otg_phy_ops);
+	if (IS_ERR(phy))
+		return dev_err_probe(&pdev->dev, PTR_ERR(phy),
+				     "Failed to create PHY\n");
+
+	phy_set_drvdata(phy, p);
+
+	provider = devm_of_phy_provider_register(&pdev->dev,
+						 of_phy_simple_xlate);
+	if (IS_ERR(provider))
+		return dev_err_probe(&pdev->dev, PTR_ERR(provider),
+				     "Failed to register PHY provider\n");
+
+	if (p->phy_idx == 0) {
+		ret = ma35_otg_role_switch_init(pdev, p);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static void ma35_otg_phy_remove(struct platform_device *pdev)
+{
+	struct ma35_otg_phy *p = platform_get_drvdata(pdev);
+
+	ma35_otg_role_switch_exit(p);
+}
+
+static const struct of_device_id ma35_otg_phy_of_match[] = {
+	{ .compatible = "nuvoton,ma35d1-usb2-phy-otg" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ma35_otg_phy_of_match);
+
+static struct platform_driver ma35_otg_phy_driver = {
+	.probe	= ma35_otg_phy_probe,
+	.remove	= ma35_otg_phy_remove,
+	.driver	= {
+		.name		= "ma35d1-usb2-phy-otg",
+		.of_match_table	= ma35_otg_phy_of_match,
+	},
+};
+module_platform_driver(ma35_otg_phy_driver);
+
+MODULE_DESCRIPTION("Nuvoton MA35D1 USB 2.0 OTG PHY driver");
+MODULE_AUTHOR("Joey Lu <a0987203069@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH 1/2] dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The binding incorrectly represents a multi-port PHY controller as multiple independent nodes instead of a single provider with `#phy-cells = <1>`.
--

commit edfd20765d416e895bc152da5eefd3bf59f431bd
Author: Joey Lu <a0987203069@gmail.com>

dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding

Adds the device tree binding documentation for the Nuvoton MA35D1 USB 2.0
OTG PHY driver. The PHY supports two ports (OTG and host-only) and allows
board-specific resistor calibration and over-current detect polarity.

> diff --git a/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml

[ ... ]

> +  nuvoton,sys:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to the system management syscon.
> +          - description: PHY instance index.
> +            enum:
> +              - 0   # USB0, OTG port (shared with DWC2 gadget controller)
> +              - 1   # USB1, host-only port
> +    description:
> +      A phandle to the syscon node covering the SYS register block, with
> +      one argument selecting the PHY instance. Index 0 selects the OTG
> +      port PHY (USB0) and index 1 selects the host-only PHY (USB1).
> +
> +  "#phy-cells":
> +    const: 0

[Severity: Medium]
Should this multi-port PHY controller be represented as a single provider
node with #phy-cells = <1> instead of embedding the index in nuvoton,sys?

Using independent nodes for each port with #phy-cells = <0> sidesteps the
standard consumer argument pattern and forces non-standard node names to
avoid collisions at the same tree level.

[ ... ]

> +  nuvoton,oc-active-high:
> +    type: boolean
> +    description:
> +      When present, the over-current detect input from the VBUS power
> +      switch is treated as active-high. The default (property absent) is
> +      active-low. This setting is shared by both USB host ports.

Since this property is explicitly described as shared by both ports, does
instantiating separate nodes per port risk conflicting states if the nodes
configure this property differently?

A single PHY provider node would allow shared properties to be defined once,
while consumers could specify their port index via standard phys arguments.

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

Re: [PATCH 2/2] phy: nuvoton: Add MA35D1 USB2 OTG PHY driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The driver implements a dummy USB role switch that falsely reports success without modifying hardware state.
- [High] Race condition exposing the PHY provider before initialization completes leads to a Use-After-Free.
- [High] PHY initialization fails due to polling clock stability bits while the PHY clock is disabled.
- [Medium] Ignoring the return value of `regmap_read()` results in the use of uninitialized stack memory.
--

commit 9e57cc2572b36a8f75f7ca9bc25a14c367c18b1a
Author: Joey Lu <a0987203069@gmail.com>
Subject: phy: nuvoton: Add MA35D1 USB2 OTG PHY driver

Add a PHY driver for the USB 2.0 PHYs in the Nuvoton MA35D1 SoC. It 
implements Power-On Reset sequence, optional resistor calibration trim, 
and a USB role switch.

> diff --git a/drivers/phy/nuvoton/phy-ma35d1-otg.c b/drivers/phy/nuvoton/phy-ma35d1-otg.c
> new file mode 100644
> index 0000000000000..53bc6ddf755e6
> --- /dev/null
> +++ b/drivers/phy/nuvoton/phy-ma35d1-otg.c
[ ... ]
> +static int ma35_otg_phy_init(struct phy *phy)
> +{
> +	struct ma35_otg_phy *p = phy_get_drvdata(phy);
> +	unsigned int n = p->phy_idx;
> +	u32 ready_mask = USBPMISCR_PHY_HOST_READY(n);
> +	unsigned int val;
> +	int ret;
> +
> +	regmap_read(p->sysreg, MA35_SYS_USBPMISCR, &val);
> +	if ((val & ready_mask) == ready_mask)
> +		return 0;

[Severity: Medium]
Does ignoring the return value of regmap_read() risk evaluating uninitialized
stack memory here?

If the read fails, val remains uninitialized, and using it in the bitwise
operation could cause non-deterministic behavior, such as incorrectly skipping
the PHY power-on reset sequence.

> +
> +	regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
> +			   USBPMISCR_PHY_CTL_MASK(n),
> +			   USBPMISCR_PHY_POR(n) | USBPMISCR_PHY_SUSPEND(n));
> +	msleep(20);
> +
> +	regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
> +			   USBPMISCR_PHY_CTL_MASK(n),
> +			   USBPMISCR_PHY_SUSPEND(n));
> +
> +	ret = regmap_read_poll_timeout(p->sysreg, MA35_SYS_USBPMISCR, val,
> +				       (val & ready_mask) == ready_mask,
> +				       10, 1000);

[Severity: High]
Is it possible for this polling to time out because the PHY clock isn't
enabled yet?

The polling expects the PHY clock to be stable, but the PHY clock (p->clk) is
only enabled later in ma35_otg_phy_power_on(). Because the Linux PHY framework
guarantees that phy_init() is called before phy_power_on(), the PHY reference
clock remains disabled during this stability polling.

> +	if (ret) {
> +		dev_err(p->dev, "USB PHY%u clock not stable (USBPMISCR=0x%08x)\n",
> +			n, val);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
[ ... ]
> +static enum usb_role ma35_otg_read_id(struct ma35_otg_phy *p)
> +{
> +	unsigned int val;
> +
> +	regmap_read(p->sysreg, MA35_SYS_PWRONOTP, &val);
> +	return (val & PWRONOTP_USBP0ID) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
> +}

[Severity: Medium]
Can this return an arbitrary USB role if regmap_read() fails?

If the read fails, the uninitialized stack variable val is evaluated, leading
to a potentially incorrect role being reported.

> +
> +static int ma35_otg_role_sw_set(struct usb_role_switch *sw,
> +				enum usb_role role)
> +{
> +	struct ma35_otg_phy *p = usb_role_switch_get_drvdata(sw);
> +
> +	p->cur_role = role;
> +
> +	return 0;
> +}

[Severity: High]
Since the hardware mux is entirely automatic based on the physical USB ID pin,
should this .set callback return an error instead of falsely reporting success?

Writing to p->cur_role doesn't configure any hardware register, and the .get
callback correctly queries the hardware directly. Returning 0 here defeats the
subsystem's -EOPNOTSUPP rejection for read-only switches, as writes from
userspace will appear to succeed but will not change the actual hardware state.

> +
> +static enum usb_role ma35_otg_role_sw_get(struct usb_role_switch *sw)
> +{
> +	struct ma35_otg_phy *p = usb_role_switch_get_drvdata(sw);
> +
> +	return ma35_otg_read_id(p);
> +}
> +
> +static int ma35_otg_role_switch_init(struct platform_device *pdev,
> +				     struct ma35_otg_phy *p)
> +{
> +	struct usb_role_switch_desc sw_desc = { };
> +
> +	p->cur_role = ma35_otg_read_id(p);
> +
> +	sw_desc.set = ma35_otg_role_sw_set;
> +	sw_desc.get = ma35_otg_role_sw_get;
> +	sw_desc.allow_userspace_control = true;

[Severity: High]
Is it appropriate to set allow_userspace_control to true given that the switch
strictly reflects an automatic hardware state?

> +	sw_desc.driver_data = p;
> +	sw_desc.fwnode = dev_fwnode(&pdev->dev);
[ ... ]
> +static int ma35_otg_phy_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	provider = devm_of_phy_provider_register(&pdev->dev,
> +						 of_phy_simple_xlate);
> +	if (IS_ERR(provider))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(provider),
> +				     "Failed to register PHY provider\n");
> +
> +	if (p->phy_idx == 0) {
> +		ret = ma35_otg_role_switch_init(pdev, p);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}

[Severity: High]
Could registering the PHY provider before role switch initialization risk a
use-after-free?

If devm_of_phy_provider_register() exposes the PHY globally, a consumer on
another CPU could look up the PHY and call phy_init(), acquiring a reference.

If ma35_otg_role_switch_init() subsequently fails, probe returns an error,
triggering the devm cleanup which frees the struct ma35_otg_phy *p memory.
Because the consumer already holds a valid reference, subsequent calls to
PHY APIs will dereference the freed pointer.

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