Re: [PATCH v10 2/7] phy: qcom: Introduce PCIe UNIPHY 28LP driver

[Varadarajan Narayanan]

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