Re: [PATCH v3 2/2] phy: add a driver for T-Head TH1520 USB PHY

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] phy: state machine desync on reset_control_assert error in exit
--

commit ec20f3d159c3233e7ce6e058ab085a7384393e38
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

phy: add a driver for T-Head TH1520 USB PHY

This commit introduces a new USB PHY driver for the T-Head TH1520 SoC.
It manages basic PHY configuration through a regmap, along with handling
the required reference clock and reset controllers.

> diff --git a/drivers/phy/thead/phy-th1520-usb.c b/drivers/phy/thead/phy-th1520-usb.c
> index 0000000000000..6cb0e51401374 100644
> --- /dev/null
> +++ b/drivers/phy/thead/phy-th1520-usb.c

[ ... ]

> +static int th1520_usb_phy_exit(struct phy *phy)
> +{
> +	struct th1520_usb_phy *th1520_phy = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = reset_control_assert(th1520_phy->phy_reset);
> +	clk_disable_unprepare(th1520_phy->ref_clk);
> +
> +	return ret;
> +}

[Severity: Medium]
If reset_control_assert() fails and returns a negative error code, this
code still disables the reference clock but propagates the error to the
caller.

Looking at the PHY core in phy_exit(), if the driver's exit callback
returns an error, it skips decrementing the initialization counter:

drivers/phy/phy-core.c:phy_exit() {
    ...
	ret = phy->ops->exit(phy);
	if (ret < 0 && ret != -ENOTSUPP)
		goto out;

	--phy->init_count;
    ...
out:
	mutex_unlock(&phy->mutex);
	return ret;
}

This leaves the core's software state tracking the PHY as initialized
(init_count > 0), while the actual physical hardware has its reference
clock disabled. 

A subsequent call to phy_init() by a consumer would then see
init_count > 0, returning success immediately without re-enabling the clock
via the init callback, leaving the PHY permanently broken.

Could this create a state machine desync if the reset controller fails?
Would it be safer to return 0 and ignore the reset failure here to ensure
proper teardown accounting in the core?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009090444.3319517-1-zhengxingda@iscas.ac.cn?part=2

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