[PATCH 1/2] ARM: dts: imx6qp-sabresd: enable pcie

[PATCH 1/2] ARM: dts: imx6qp-sabresd: enable pcie

[Richard Zhu]

Add one regulator, used to power up the external oscillator,
and enable PCIe on iMX6QP SABRESD board.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 arch/arm/boot/dts/imx6qp-sabresd.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qp-sabresd.dts b/arch/arm/boot/dts/imx6qp-sabresd.dts
index 480e73183f6b..cd8a1f610427 100644
--- a/arch/arm/boot/dts/imx6qp-sabresd.dts
+++ b/arch/arm/boot/dts/imx6qp-sabresd.dts
@@ -51,7 +51,8 @@
 };
 
 &pcie {
-	status = "disabled";
+	vepdev-supply = <&vgen3_reg>;
+	status = "okay";
 };
 
 &sata {
-- 
2.17.1

[PATCH 2/2] PCI: imx: add another regulator for imx pcie

[Richard Zhu]

One more regulator is required to turn on the external oscillator
populated on the iMX6QP SABRESD board.
Add another regulator to enable PCIe on iMX6QP SABRESD board.

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

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 8f08ae53f53e..9e1563601835 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -81,6 +81,7 @@ struct imx6_pcie {
 	u32			tx_swing_low;
 	int			link_gen;
 	struct regulator	*vpcie;
+	struct regulator	*vepdev;
 	void __iomem		*phy_base;
 
 	/* power domain for pcie */
@@ -507,6 +508,14 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 			return;
 		}
 	}
+	if (imx6_pcie->vepdev && !regulator_is_enabled(imx6_pcie->vepdev)) {
+		ret = regulator_enable(imx6_pcie->vepdev);
+		if (ret) {
+			dev_err(dev, "failed to enable vepdev regulator: %d\n",
+				ret);
+			goto err_pcie_vepdev;
+		}
+	}
 
 	ret = clk_prepare_enable(imx6_pcie->pcie_phy);
 	if (ret) {
@@ -595,6 +604,13 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 err_pcie_bus:
 	clk_disable_unprepare(imx6_pcie->pcie_phy);
 err_pcie_phy:
+	if (imx6_pcie->vepdev && regulator_is_enabled(imx6_pcie->vepdev) > 0) {
+		ret = regulator_disable(imx6_pcie->vepdev);
+		if (ret)
+			dev_err(dev, "failed to disable vepdev regulator: %d\n",
+				ret);
+	}
+err_pcie_vepdev:
 	if (imx6_pcie->vpcie && regulator_is_enabled(imx6_pcie->vpcie) > 0) {
 		ret = regulator_disable(imx6_pcie->vpcie);
 		if (ret)
@@ -1178,6 +1194,12 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 			return PTR_ERR(imx6_pcie->vpcie);
 		imx6_pcie->vpcie = NULL;
 	}
+	imx6_pcie->vepdev = devm_regulator_get_optional(&pdev->dev, "vepdev");
+	if (IS_ERR(imx6_pcie->vepdev)) {
+		if (PTR_ERR(imx6_pcie->vepdev) != -ENODEV)
+			return PTR_ERR(imx6_pcie->vepdev);
+		imx6_pcie->vepdev = NULL;
+	}
 
 	platform_set_drvdata(pdev, imx6_pcie);
 
-- 
2.17.1

Re: [PATCH 1/2] ARM: dts: imx6qp-sabresd: enable pcie

[Lucas Stach]

Am Dienstag, den 21.07.2020, 15:44 +0800 schrieb Richard Zhu:
> Add one regulator, used to power up the external oscillator,
> and enable PCIe on iMX6QP SABRESD board.

That's not the right thing to do. If there is an external oscillator,
which requires a power supply then the oscillator should have its own
clock DT node (it's a separate device after all) and this node needs to
control the regulator.

This has nothing to do with the PCIe controller, which only cares about
the clock being provided.

Regards,
Lucas

> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  arch/arm/boot/dts/imx6qp-sabresd.dts | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/imx6qp-sabresd.dts
> b/arch/arm/boot/dts/imx6qp-sabresd.dts
> index 480e73183f6b..cd8a1f610427 100644
> --- a/arch/arm/boot/dts/imx6qp-sabresd.dts
> +++ b/arch/arm/boot/dts/imx6qp-sabresd.dts
> @@ -51,7 +51,8 @@
>  };
>  
>  &pcie {
> -	status = "disabled";
> +	vepdev-supply = <&vgen3_reg>;
> +	status = "okay";
>  };
>  
>  &sata {

RE: [EXT] Re: [PATCH 1/2] ARM: dts: imx6qp-sabresd: enable pcie

[Richard Zhu]

> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: 2020年7月21日 16:16
> To: Richard Zhu <hongxing.zhu@nxp.com>; bhelgaas@google.com;
> shawnguo@kernel.org; festevam@gmail.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: [EXT] Re: [PATCH 1/2] ARM: dts: imx6qp-sabresd: enable pcie
> Am Dienstag, den 21.07.2020, 15:44 +0800 schrieb Richard Zhu:
> > Add one regulator, used to power up the external oscillator, and
> > enable PCIe on iMX6QP SABRESD board.
> 
> That's not the right thing to do. If there is an external oscillator, which
> requires a power supply then the oscillator should have its own clock DT
> node (it's a separate device after all) and this node needs to control the
> regulator.
> 
> This has nothing to do with the PCIe controller, which only cares about the
> clock being provided.
> 
Hi Lucas:
Thanks for your comments. 
Okay, I would integrate the regulator into one clock node later.

Best Regards
Richard Zhu

> Regards,
> Lucas
> 
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  arch/arm/boot/dts/imx6qp-sabresd.dts | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/imx6qp-sabresd.dts
> > b/arch/arm/boot/dts/imx6qp-sabresd.dts
> > index 480e73183f6b..cd8a1f610427 100644
> > --- a/arch/arm/boot/dts/imx6qp-sabresd.dts
> > +++ b/arch/arm/boot/dts/imx6qp-sabresd.dts
> > @@ -51,7 +51,8 @@
> >  };
> >
> >  &pcie {
> > -     status = "disabled";
> > +     vepdev-supply = <&vgen3_reg>;
> > +     status = "okay";
> >  };
> >
> >  &sata {