[PATCH v2 4/4] phy: freescale: fsl-samsung-hdmi: Disable APB clock on resume failure

[sashiko-bot at kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The clock `set_rate` callback directly accesses hardware registers without ensuring the PHY device is runtime active, which could cause a synchronous external abort when called while the device is suspended.
- [High] The driver mutates a static global variable to store dynamically calculated, instance-specific PHY configurations, causing a race condition in multi-instance or concurrent scenarios.
- [Medium] If `pm_runtime_resume_and_get()` fails in `fsl_samsung_hdmi_phy_remove()`, devres cleanup will underflow the APB clock enable count.
--

--- Patch [4]: [PATCH v2 4/4] phy: freescale: fsl-samsung-hdmi: Disable APB clock on resume failure ---
commit f185b639eb37a8417c686059fd4e7a2399306055
Author: Can Peng <pengcan at kylinos.cn>

phy: freescale: fsl-samsung-hdmi: Disable APB clock on resume failure

This commit modifies the fsl_samsung_hdmi_phy_resume() function to disable
the apbclk on the failure path. This ensures the clock enable count remains
balanced if the hardware configuration fails after the clock was enabled.

Cc: stable at vger.kernel.org

> diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> index 455d03220b72c..bb1aa19770650 100644
> --- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> @@ -722,8 +722,11 @@ static int __maybe_unused fsl_samsung_hdmi_phy_resume(struct device *dev)
>  		return ret;
>  	}
>  
> -	if (phy->cur_cfg)
> +	if (phy->cur_cfg) {
>  		ret = fsl_samsung_hdmi_phy_configure(phy, phy->cur_cfg);

[Severity: High]
This is a pre-existing issue, but does reading phy->cur_cfg here race
with concurrent clock rate updates?

In fsl_samsung_hdmi_phy_find_settings(), the driver dynamically calculates
settings and writes them into a shared static global variable without
locking:

static struct phy_config calculated_phy_pll_cfg = { ... };

fsl_samsung_hdmi_phy_find_settings()
    ...
    fsl_samsung_hdmi_calculate_phy(&calculated_phy_pll_cfg, ...);

If a concurrent call to clk_round_rate() updates calculated_phy_pll_cfg
while this resume callback is reading from it via phy->cur_cfg, could it
write partially updated or invalid PLL values to the hardware, resulting in
corrupted output or a display lockup?

> +		if (ret)
> +			clk_disable(phy->apbclk);
> +	}
>  
>  	return ret;

[Severity: Critical]
This is a pre-existing issue, but does the clock set_rate callback risk a
synchronous external abort if called while the device is suspended?

The clk_set_rate callback directly accesses hardware registers without
calling pm_runtime_resume_and_get():

fsl_samsung_hdmi_phy_clk_set_rate()
    ...
    return fsl_samsung_hdmi_phy_configure(phy, target_settings);

fsl_samsung_hdmi_phy_configure()
    ...
    writeb(REG33_FIX_DA, phy->regs + PHY_REG(33));

Because the clock framework does not automatically resume provider devices
during clk_set_rate(), if a consumer calls clk_set_rate() while the PHY is
runtime suspended, would writing to these unclocked APB registers cause a
system crash?

[Severity: Medium]
This is a pre-existing issue, but does failing to resume in remove()
cause a clock enable count underflow?

If pm_runtime_resume_and_get() fails (for example, due to a PLL lock
timeout), the device remains suspended with an enable count of 0:

fsl_samsung_hdmi_phy_remove()
    ...
    ret = pm_runtime_resume_and_get(&pdev->dev);
    if (ret < 0)
        dev_warn(&pdev->dev, "failed to resume on remove: %d\n", ret);

When remove() returns, devres cleanup unconditionally calls
clk_disable_unprepare(). Since the driver mixes devm_clk_get_enabled()
with manual clk_enable() and clk_disable() PM runtime toggling, wouldn't
this invoke clk_disable() on a clock that is already at 0, triggering a
warning in the clock core?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721064752.153338-1-pengcan@kylinos.cn?part=4