Re: [PATCH v10 5/5] phy: airoha: Add support for Airoha AN7581 USB PHY

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incorrect usage of `regmap_set_bits` with `FIELD_PREP` corrupts multi-bit register field
- [Medium] Missing PHY Lifecycle Management for SerDes PHY
- [Medium] Computed slew rate calibration value is not clamped, risking silent wraparound
--

Note: The format of this review has been altered to use free-form summaries
instead of inline diff quoting due to strict recitation restrictions.

commit 4d1e2d05354e7c1aad9463afa3c9c7ffd853f726
Author: Christian Marangi <ansuelsmth@gmail.com>
phy: airoha: Add support for Airoha AN7581 USB PHY

This commit adds support for the Airoha AN7581 USB PHY driver, which supports
up to two USB ports. USB 2.0 is always supported, while USB 3.0 requires the
SerDes port to be configured. It also introduces USB 2.0 slew rate calibration
using a mandatory monitor clock.

[Severity: Medium]
In an7581_usb_phy_init() and an7581_usb_phy_power_on(), the driver sets the
mode of the SerDes PHY but does not call phy_init(), phy_power_on(),
phy_power_off(), or phy_exit() for the priv->serdes_phy. Is it safe to omit
these generic PHY lifecycle calls?

Even if the current SerDes PHY driver for this SoC only implements the
set_mode callback, omitting these lifecycle management functions might cause
failures if the SerDes PHY driver is updated to require hardware
initialization, or if this driver is used on a SoC variant where the
underlying SerDes PHY requires power management to function.

[Severity: High]
In an7581_usb_phy_u3_init(), regmap_set_bits() is used alongside FIELD_PREP()
to write a 16-bit value into the AIROHA_USB_PHY_SSUSB_XTAL_TOP_RESERVE
register. Does this unintentionally leave old bits set in the multi-bit field?

Since regmap_set_bits() performs a bitwise OR and does not clear previously
set bits like regmap_update_bits() does, any pre-existing overlapping bits
from hardware defaults or bootloader initialization will remain set. This
could result in a corrupted XTAL/PLL configuration and cause an unstable
USB 3.0 clock or probe failures.

[Severity: Medium]
In an7581_usb_phy_u2_slew_rate_calibration(), the dynamically computed srctrl
value is passed directly to FIELD_PREP() for the 3-bit
AIROHA_USB_PHY_USB20_HSTX_SRCTRL field. Could this silently wrap around if
fm_out is unexpectedly small?

If a transient clock issue causes fm_out to drop below expected values, the
computed srctrl can exceed the maximum value of 7. Because FIELD_PREP()
masks the value, an srctrl of 8 would silently wrap around to 0. This would
convert what should be a maximum slew rate calibration into the minimum
calibration, risking severe USB 2.0 signal degradation. Should the srctrl
value be clamped before insertion?

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