Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] phy: nuvoton: missing MFD_SYSCON select and unchecked
regmap_read()
--
commit ce87e45e509dca5386021bb0dd4e3bb91e3935d8
Author: Joey Lu [off-list ref]
phy: nuvoton: phy-ma35d1-usb2: extend to dual-port with OTG support
This commit extends the Nuvoton MA35D1 USB 2.0 PHY driver to support both
PHY0 and PHY1, updates clock-stability polling in phy_init(), registers a
read-only USB role switch for PHY0, and adds resistor calibration and
over-current polarity device-tree properties.
quoted hunk ↗ jump to hunk
diff --git a/drivers/phy/nuvoton/Kconfig b/drivers/phy/nuvoton/Kconfig
index d02cae2db315a..1f37e860de507 100644
--- a/drivers/phy/nuvoton/Kconfig
+++ b/drivers/phy/nuvoton/Kconfig
@@ -6,6 +6,7 @@ config PHY_MA35_USB
tristate "Nuvoton MA35 USB2.0 PHY driver"
depends on ARCH_MA35 || COMPILE_TEST
depends on OF
+ depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH
select GENERIC_PHY
help
Enable this to support the USB2.0 PHY on the Nuvoton MA35
diff --git a/drivers/phy/nuvoton/phy-ma35d1-usb2.c b/drivers/phy/nuvoton/phy-ma35d1-usb2.c
index 0b9976fb15c17..bc706356c4341 100644
--- a/drivers/phy/nuvoton/phy-ma35d1-usb2.c
+++ b/drivers/phy/nuvoton/phy-ma35d1-usb2.c
[ ... ]
quoted hunk ↗ jump to hunk
+static int ma35_usb_phy_init(struct phy *phy)
+{
+ struct ma35_phy_port *port = phy_get_drvdata(phy);
+ struct ma35_usb_phy *p = container_of(port - port->idx,
+ struct ma35_usb_phy, port[0]);
+ unsigned int n = port->idx;
unsigned int val;
int ret;
- ret = clk_prepare_enable(p_phy->clk);
- if (ret < 0) {
- dev_err(p_phy->dev, "Failed to enable PHY clock: %d\n", ret);
- return ret;
- }
+ ret = clk_prepare_enable(p->clk);
+ if (ret)
+ return dev_err_probe(p->dev, ret, "failed to enable clock\n");
- regmap_read(p_phy->sysreg, MA35_SYS_REG_USBPMISCR, &val);
- if (val & PHY0SUSPEND) {
- /*
- * USB PHY0 is in operation mode already
- * make sure USB PHY 60 MHz UTMI Interface Clock ready
- */
- ret = regmap_read_poll_timeout(p_phy->sysreg, MA35_SYS_REG_USBPMISCR, val,
- val & PHY0DEVCKSTB, 10, 1000);
- if (ret == 0)
- return 0;
- }
+ regmap_read(p->sysreg, MA35_SYS_USBPMISCR, &val);
- /*
- * reset USB PHY0.
- * wait until USB PHY0 60 MHz UTMI Interface Clock ready
- */
- regmap_update_bits(p_phy->sysreg, MA35_SYS_REG_USBPMISCR, 0x7, (PHY0POR | PHY0SUSPEND));
+ if ((val & USBPMISCR_PHY_SUSPEND(n)) &&
+ !ma35_usb_phy_wait_ready(p, n, &val))
+ goto out;
[ ... ]
quoted hunk ↗ jump to hunk
+static enum usb_role ma35_role_sw_get(struct usb_role_switch *sw)
+{
+ struct ma35_usb_phy *p = usb_role_switch_get_drvdata(sw);
+ u32 val;
+
+ regmap_read(p->sysreg, MA35_SYS_PWRONOTP, &val);
+
+ return (val & PWRONOTP_USBP0ID) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+}
[Severity: Medium]
Can val be used uninitialized in ma35_usb_phy_init() and
ma35_role_sw_get() when regmap_read() fails or is stubbed out?
Neither PHY_MA35_USB nor ARCH_MA35 selects MFD_SYSCON, so PHY_MA35_USB can
be built with CONFIG_MFD_SYSCON=n and CONFIG_REGMAP=n. In that
configuration, syscon_regmap_lookup_by_phandle() always fails with
-ENOTSUPP, and the regmap_read() stub returns -EINVAL without writing val,
causing the compiler to flag val as uninitialized in ma35_usb_phy_init()
and ma35_role_sw_get().
Selecting MFD_SYSCON in PHY_MA35_USB and checking the regmap_read() return
value (or initializing val) would avoid both issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261109065606.62641-1-a0987203069@gmail.com?part=4