[PATCH] phy: broadcom: brcm-usb: unwind clocks on probe failure
Manivannan Sadhasivam
mani at kernel.org
Mon Oct 12 06:28:03 PDT 2026
On Sat, Sep 12, 2026 at 09:29:56PM -0400, Myeonghun Pak wrote:
> brcm_usb_phy_dvr_init() enables the optional USB2 and USB3 clocks before
> creating all PHYs and acquiring the remaining probe resources. Several
> later failures return without disabling clocks that were already enabled,
> and deferred probe retries can keep increasing their enable counts.
>
> Track each successful clock enable and unwind only those clocks, in reverse
> order, when initialization fails. Keep the existing successful probe path
> unchanged. This is limited to the BCM4908 and Broadcom STB USB PHY driver.
>
> This issue was identified during our ongoing static-analysis research while
> reviewing kernel code.
>
> Fixes: 49859e55e364 ("phy: usb: phy-brcm-usb: Add Broadcom STB USB phy driver")
> Assisted-by: OpenAI:GPT-5.6
> Co-developed-by: Ijae Kim <ae878000 at gmail.com>
> Signed-off-by: Ijae Kim <ae878000 at gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512 at gmail.com>
> ---
> drivers/phy/broadcom/phy-brcm-usb.c | 48 +++++++++++++++++++++--------
> 1 file changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/phy/broadcom/phy-brcm-usb.c b/drivers/phy/broadcom/phy-brcm-usb.c
> index 59d756a10..273f7b586 100644
> --- a/drivers/phy/broadcom/phy-brcm-usb.c
> +++ b/drivers/phy/broadcom/phy-brcm-usb.c
> @@ -404,6 +404,8 @@ static int brcm_usb_phy_dvr_init(struct platform_device *pdev,
> {
> struct device *dev = &pdev->dev;
> struct phy *gphy = NULL;
> + bool usb_20_clk_enabled = false;
> + bool usb_30_clk_enabled = false;
> int err;
>
> priv->usb_20_clk = of_clk_get_by_name(dn, "sw_usb");
> @@ -413,15 +415,19 @@ static int brcm_usb_phy_dvr_init(struct platform_device *pdev,
> dev_info(dev, "Clock not found in Device Tree\n");
> priv->usb_20_clk = NULL;
> }
> - err = clk_prepare_enable(priv->usb_20_clk);
> - if (err)
> - return err;
> + if (priv->usb_20_clk) {
> + err = clk_prepare_enable(priv->usb_20_clk);
> + if (err)
> + return err;
> + usb_20_clk_enabled = true;
This looks ugly :( Either use two err labels or just club both
clk_disable_unprepare() calls in one err label as clk framework will bail out
safely if the clk pointer is NULL.
- Mani
--
மணிவண்ணன் சதாசிவம்
More information about the linux-phy
mailing list