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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z8AoCHxD4IlFeXv9@hu-varada-blr.qualcomm.com>
Date: Thu, 27 Feb 2025 14:23:28 +0530
From: Varadarajan Narayanan <quic_varada@...cinc.com>
To: Vinod Koul <vkoul@...nel.org>
CC: <lpieralisi@...nel.org>, <kw@...ux.com>,
        <manivannan.sadhasivam@...aro.org>, <robh@...nel.org>,
        <bhelgaas@...gle.com>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
        <kishon@...nel.org>, <andersson@...nel.org>, <konradybcio@...nel.org>,
        <p.zabel@...gutronix.de>, <quic_nsekar@...cinc.com>,
        <dmitry.baryshkov@...aro.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-pci@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-phy@...ts.infradead.org>
Subject: Re: [PATCH v10 2/7] phy: qcom: Introduce PCIe UNIPHY 28LP driver

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