Re: [PATCH v5] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition in `google_usb_set_orientation` due to an unsafe Runtime PM state check.
--

--- Patch [1]: [PATCH v5] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver ---
commit 52452ad1a696ea08726cabf09141d6fdf6a5b7a9
Author: RD Babiera <rdbabiera@google.com>

phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver

This commit adds USB SuperSpeed support to the Google Tensor G5 USB PHY
driver. It implements USB3 phy_ops, refactors the probe sequence, and
modifies google_usb_set_orientation() to reprogram the TCA when the PHY
is ready while holding a pm_runtime reference.

> diff --git a/drivers/phy/phy-google-usb.c b/drivers/phy/phy-google-usb.c
> index ab20bc20f19ea..60581ce815b27 100644
> --- a/drivers/phy/phy-google-usb.c
> +++ b/drivers/phy/phy-google-usb.c

[ ... ]

> @@ -46,53 +77,201 @@ struct google_usb_phy_instance {
>  static int google_usb_set_orientation(struct typec_switch_dev *sw,
>  				      enum typec_orientation orientation)
>  {
>  	struct google_usb_phy *gphy = typec_switch_get_drvdata(sw);
> +	int ret = 0;
>  
>  	dev_dbg(gphy->dev, "set orientation %d\n", orientation);
>  
> +	guard(mutex)(&gphy->phy_mutex);
> +
>  	gphy->orientation = orientation;
>  
> -	if (pm_runtime_suspended(gphy->dev))
> -		return 0;
> +	pm_runtime_get_noresume(gphy->dev);
>  
> -	guard(mutex)(&gphy->phy_mutex);
> +	if (pm_runtime_suspended(gphy->dev)) {
> +		pm_runtime_put_noidle(gphy->dev);
> +		return 0;
> +	}

[Severity: High]
Can this result in a race condition during device suspend?

pm_runtime_get_noresume() increments the usage counter but doesn't check
if a suspend is already in progress. If the device status is currently
RPM_SUSPENDING, pm_runtime_suspended() will return false.

This would allow the driver to proceed to access hardware via
set_vbus_valid() and program_tca_locked() concurrently with the suspend
callback disabling clocks and power domains. On ARM platforms, this
typically triggers an asynchronous external abort or kernel panic.

Would it be safer to use pm_runtime_get_if_active(gphy->dev) <= 0 instead,
which atomically increments the counter only if the device is fully active?

>  
>  	set_vbus_valid(gphy);
>  
> -	return 0;
> +	if (gphy->phy_state == COMBO_PHY_TCA_READY && orientation != TYPEC_ORIENTATION_NONE)
> +		ret = program_tca_locked(gphy);
> +
> +	pm_runtime_put_autosuspend(gphy->dev);
> +
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709163112.3902148-2-rdbabiera@google.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy