On Thu, Feb 27, 2025 at 11:30:24AM +0530, Vinod Koul wrote:
[ . . .]
> > > should we not unroll the pipe clk registration here?
> >
> > Since it is a 'devm_' clk_hw_register_fixed_rate, wouldn't the devm
> > framework do the unregister?
> >
> > $ git diff
> > diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
> > index 6b4f76b9c4da..3fd1a12cc163 100644
> > --- a/drivers/clk/clk-fixed-rate.c
> > +++ b/drivers/clk/clk-fixed-rate.c
> > @@ -58,6 +58,7 @@ static void
> > devm_clk_hw_register_fixed_rate_release(struct device *dev, void *re
> > * the hw, resulting in double free. Just unregister the hw and
> > * let
> > * devres code kfree() it.
> > */
> > + printk("--> %s: %s\n", __func__, __clk_get_name(fix->hw.clk));
> > clk_hw_unregister(&fix->hw);
> > }
> >
> > diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c
> > b/drivers/phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c
> > index 311f98181177..9a8d8d9a7c2b 100644
> > --- a/drivers/phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c
> > +++ b/drivers/phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c
> > @@ -267,6 +268,7 @@ static int qcom_uniphy_pcie_probe(struct
> > platform_device *pdev)
> >
> > phy_provider = devm_of_phy_provider_register(phy->dev,
> > of_phy_simple_xlate);
> > + phy_provider = ERR_PTR(-EINVAL);
> > if (IS_ERR(phy_provider))
> > return PTR_ERR(phy_provider);
> >
> > I forced an error here and saw that devm_clk_hw_register_fixed_rate_release
> > is getting called, which in turn calls clk_hw_unregister. Is that sufficient?
> > Or am i missing something.
>
> I missed that internally this is devm_, this is fine
Thanks for the clarification. Have posted V11, please review.
-Varada
