linux-kernel - Re: [PATCH 1/3] PCI: imx: encapsulate the clock enable into one standalone function

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210908151203.GA866207@bjorn-Precision-5520>
Date:   Wed, 8 Sep 2021 10:12:03 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Richard Zhu <hongxing.zhu@....com>
Cc:     l.stach@...gutronix.de, bhelgaas@...gle.com,
        lorenzo.pieralisi@....com, linux-pci@...r.kernel.org,
        linux-imx@....com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH 1/3] PCI: imx: encapsulate the clock enable into one
 standalone function

On Wed, Sep 08, 2021 at 02:59:24PM +0800, Richard Zhu wrote:
> No function changes, just encapsulate the i.MX PCIe clocks enable
> operations into one standalone function

When you update this,

  - it's helpful if you include a cover letter with a multi-patch
    series, with the patches being replies to the cover letter, and

  - please follow the sentence and formatting conventions for subject
    lines and commit logs (driver name should match, capitalize
    subject line, end sentences with periods, blank lines between
    paragraphs, remove useless information like timestamps from log
    messages, indent quoted material like logs by two spaces, etc).

> Signed-off-by: Richard Zhu <hongxing.zhu@....com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 82 +++++++++++++++++----------
>  1 file changed, 51 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 80fc98acf097..0264432e4c4a 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -143,6 +143,8 @@ struct imx6_pcie {
>  #define PHY_RX_OVRD_IN_LO_RX_DATA_EN		BIT(5)
>  #define PHY_RX_OVRD_IN_LO_RX_PLL_EN		BIT(3)
>  
> +static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie);
> +
>  static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, bool exp_val)
>  {
>  	struct dw_pcie *pci = imx6_pcie->pci;
> @@ -498,33 +500,12 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  		}
>  	}
>  
> -	ret = clk_prepare_enable(imx6_pcie->pcie_phy);
> -	if (ret) {
> -		dev_err(dev, "unable to enable pcie_phy clock\n");
> -		goto err_pcie_phy;
> -	}
> -
> -	ret = clk_prepare_enable(imx6_pcie->pcie_bus);
> +	ret = imx6_pcie_clk_enable(imx6_pcie);
>  	if (ret) {
> -		dev_err(dev, "unable to enable pcie_bus clock\n");
> -		goto err_pcie_bus;
> +		dev_err(dev, "unable to enable pcie clocks\n");
> +		goto err_clks;
>  	}
>  
> -	ret = clk_prepare_enable(imx6_pcie->pcie);
> -	if (ret) {
> -		dev_err(dev, "unable to enable pcie clock\n");
> -		goto err_pcie;
> -	}
> -
> -	ret = imx6_pcie_enable_ref_clk(imx6_pcie);
> -	if (ret) {
> -		dev_err(dev, "unable to enable pcie ref clock\n");
> -		goto err_ref_clk;
> -	}
> -
> -	/* allow the clocks to stabilize */
> -	usleep_range(200, 500);
> -
>  	/* Some boards don't have PCIe reset GPIO. */
>  	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
>  		gpio_set_value_cansleep(imx6_pcie->reset_gpio,
> @@ -578,13 +559,7 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  
>  	return;
>  
> -err_ref_clk:
> -	clk_disable_unprepare(imx6_pcie->pcie);
> -err_pcie:
> -	clk_disable_unprepare(imx6_pcie->pcie_bus);
> -err_pcie_bus:
> -	clk_disable_unprepare(imx6_pcie->pcie_phy);
> -err_pcie_phy:
> +err_clks:
>  	if (imx6_pcie->vpcie && regulator_is_enabled(imx6_pcie->vpcie) > 0) {
>  		ret = regulator_disable(imx6_pcie->vpcie);
>  		if (ret)
> @@ -914,6 +889,51 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
>  	usleep_range(1000, 10000);
>  }
>  
> +static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
> +{
> +	struct dw_pcie *pci = imx6_pcie->pci;
> +	struct device *dev = pci->dev;
> +	int ret;
> +
> +	ret = clk_prepare_enable(imx6_pcie->pcie_phy);
> +	if (ret) {
> +		dev_err(dev, "unable to enable pcie_phy clock\n");
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(imx6_pcie->pcie_bus);
> +	if (ret) {
> +		dev_err(dev, "unable to enable pcie_bus clock\n");
> +		goto err_pcie_bus;
> +	}
> +
> +	ret = clk_prepare_enable(imx6_pcie->pcie);
> +	if (ret) {
> +		dev_err(dev, "unable to enable pcie clock\n");
> +		goto err_pcie;
> +	}
> +
> +	ret = imx6_pcie_enable_ref_clk(imx6_pcie);
> +	if (ret) {
> +		dev_err(dev, "unable to enable pcie ref clock\n");
> +		goto err_ref_clk;
> +	}
> +
> +	/* allow the clocks to stabilize */
> +	usleep_range(200, 500);
> +	return 0;
> +
> +err_ref_clk:
> +	clk_disable_unprepare(imx6_pcie->pcie);
> +err_pcie:
> +	clk_disable_unprepare(imx6_pcie->pcie_bus);
> +err_pcie_bus:
> +	clk_disable_unprepare(imx6_pcie->pcie_phy);
> +
> +	return ret;
> +
> +}
> +
>  static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
>  {
>  	clk_disable_unprepare(imx6_pcie->pcie);
> -- 
> 2.25.1
>