diff mbox series

[1/3] PCI: imx: encapsulate the clock enable into one standalone function

Message ID 1631084366-24785-1-git-send-email-hongxing.zhu@nxp.com
State New
Headers show
Series [1/3] PCI: imx: encapsulate the clock enable into one standalone function | expand

Commit Message

Richard Zhu Sept. 8, 2021, 6:59 a.m. UTC
No function changes, just encapsulate the i.MX PCIe clocks enable
operations into one standalone function

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 82 +++++++++++++++++----------
 1 file changed, 51 insertions(+), 31 deletions(-)

Comments

Lucas Stach Sept. 8, 2021, 8:47 a.m. UTC | #1
Am Mittwoch, dem 08.09.2021 um 14:59 +0800 schrieb Richard Zhu:
> No function changes, just encapsulate the i.MX PCIe clocks enable
> operations into one standalone function
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.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);
> +
I don't think this is strictly needed. Can you just move the placement
of the new imx6_pcie_clk_enable function in the file, such that we can
avoid the forward declaration?

>  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;
> +
Superfluous newline.

Regards,
Lucas
 
> +}
> +
>  static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
>  {
>  	clk_disable_unprepare(imx6_pcie->pcie);
Richard Zhu Sept. 8, 2021, 9:02 a.m. UTC | #2
> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: Wednesday, September 8, 2021 4:48 PM
> To: Richard Zhu <hongxing.zhu@nxp.com>; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com
> Cc: linux-pci@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de
> Subject: Re: [PATCH 1/3] PCI: imx: encapsulate the clock enable into one
> standalone function
> 
> Am Mittwoch, dem 08.09.2021 um 14:59 +0800 schrieb Richard Zhu:
> > No function changes, just encapsulate the i.MX PCIe clocks enable
> > operations into one standalone function
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.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);
> > +
> I don't think this is strictly needed. Can you just move the placement of the
> new imx6_pcie_clk_enable function in the file, such that we can avoid the
> forward declaration?
> 
[Richard Zhu] Thanks for your quickly review.
Okay, would move the placement to avoid he forward declaration.

> >  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;
> > +
> Superfluous newline.
[Richard Zhu]Got that. Blank line would be removed later.
Thanks.

> 
> Regards,
> Lucas
> 
> > +}
> > +
> >  static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)  {
> >  	clk_disable_unprepare(imx6_pcie->pcie);
>
kernel test robot Sept. 8, 2021, 1:41 p.m. UTC | #3
Hi Richard,

I love your patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on v5.14]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Richard-Zhu/PCI-imx-encapsulate-the-clock-enable-into-one-standalone-function/20210908-152610
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: ia64-allyesconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3872c55dc0f8d93641fd342d0524fbcac873de49
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Richard-Zhu/PCI-imx-encapsulate-the-clock-enable-into-one-standalone-function/20210908-152610
        git checkout 3872c55dc0f8d93641fd342d0524fbcac873de49
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ia64-linux-ld: drivers/pci/controller/dwc/pci-imx6.o: in function `imx6_pcie_deassert_core_reset':
>> pci-imx6.c:(.text+0x17c2): undefined reference to `imx6_pcie_clk_enable'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Bjorn Helgaas Sept. 8, 2021, 3:12 p.m. UTC | #4
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@nxp.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
>
Richard Zhu Sept. 9, 2021, 2:26 a.m. UTC | #5
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Wednesday, September 8, 2021 11:12 PM
> To: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; linux-pci@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; kernel@pengutronix.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).
> 
[Richard Zhu] Ok, got that. Thanks for your kindly reminder.
Would use the cover letter, and reformat the subject lines and commit logs later.

Best Regards
Richard Zhu
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.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
> >
diff mbox series

Patch

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);