[PATCH v2 0/4] PCI: imx: Add PME_Turn_Off support

[PATCH v2 0/4] PCI: imx: Add PME_Turn_Off support

[Leonard Crestez]

When the root complex suspends it must send a PME_Turn_Off TLP.
Implement this by asserting the "turnoff" reset.

On imx7d this functionality is part of the SRC and exposed through the
linux reset-controller subsystem. On imx6 equivalent bits are in the
IOMUXC GPR area which the imx6-pcie driver accesses directly.

This is only for imx7d right now but it's deliberately implemented as an
optional reset, ignoring the chip variant:
* Older dtbs won't have this reset so it will be ignored.
* Future chips might also expose this as a reset controller.

For example imx8m (not yet supported) has the exact same
PCIE_CTRL_APPS_TURNOFF bit in the same location.

---
Changes since v1:
 * Add 1-10ms sleep after PME_Turn_Off, with explanation
 * Slight fix in commit msg (this is functionality is)
 * Link: https://lore.kernel.org/patchwork/cover/994076/

Leonard Crestez (4):
  reset: imx7: Add PCIE_CTRL_APPS_TURNOFF
  dt-bindings: imx6q-pcie: Add turnoff reset for imx7d
  ARM: dts: imx7d: Add turnoff reset
  PCI: imx: Add PME_Turn_Off support

 .../bindings/pci/fsl,imx6q-pcie.txt           |  1 +
 arch/arm/boot/dts/imx7d.dtsi                  |  5 ++--
 drivers/pci/controller/dwc/pci-imx6.c         | 24 +++++++++++++++++++
 drivers/reset/reset-imx7.c                    |  1 +
 include/dt-bindings/reset/imx7-reset.h        |  4 +++-
 5 files changed, 32 insertions(+), 3 deletions(-)

-- 
2.17.1

[PATCH v2 1/4] reset: imx7: Add PCIE_CTRL_APPS_TURNOFF

[Leonard Crestez]

This is required for the imx pci driver to send the PME_Turn_Off TLP.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 drivers/reset/reset-imx7.c             | 1 +
 include/dt-bindings/reset/imx7-reset.h | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
index 97d9f08271c5..77911fa8f31d 100644
--- a/drivers/reset/reset-imx7.c
+++ b/drivers/reset/reset-imx7.c
@@ -65,10 +65,11 @@ static const struct imx7_src_signal imx7_src_signals[IMX7_RESET_NUM] = {
 	[IMX7_RESET_MIPI_PHY_MRST]	= { SRC_MIPIPHY_RCR, BIT(1) },
 	[IMX7_RESET_MIPI_PHY_SRST]	= { SRC_MIPIPHY_RCR, BIT(2) },
 	[IMX7_RESET_PCIEPHY]		= { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
 	[IMX7_RESET_PCIEPHY_PERST]	= { SRC_PCIEPHY_RCR, BIT(3) },
 	[IMX7_RESET_PCIE_CTRL_APPS_EN]	= { SRC_PCIEPHY_RCR, BIT(6) },
+	[IMX7_RESET_PCIE_CTRL_APPS_TURNOFF] = { SRC_PCIEPHY_RCR, BIT(11) },
 	[IMX7_RESET_DDRC_PRST]		= { SRC_DDRC_RCR, BIT(0) },
 	[IMX7_RESET_DDRC_CORE_RST]	= { SRC_DDRC_RCR, BIT(1) },
 };
 
 static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev)
diff --git a/include/dt-bindings/reset/imx7-reset.h b/include/dt-bindings/reset/imx7-reset.h
index 63948170c7b2..31b3f87dde9a 100644
--- a/include/dt-bindings/reset/imx7-reset.h
+++ b/include/dt-bindings/reset/imx7-reset.h
@@ -54,9 +54,11 @@
  */
 #define IMX7_RESET_PCIE_CTRL_APPS_EN	22
 #define IMX7_RESET_DDRC_PRST		23
 #define IMX7_RESET_DDRC_CORE_RST	24
 
-#define IMX7_RESET_NUM			25
+#define IMX7_RESET_PCIE_CTRL_APPS_TURNOFF 25
+
+#define IMX7_RESET_NUM			26
 
 #endif
 
-- 
2.17.1

[PATCH v2 2/4] dt-bindings: imx6q-pcie: Add turnoff reset for imx7d

[Leonard Crestez]

This is documented as "required" but won't be present in old dtbs.

These resets are also present on other imx chips but right now only
imx7d implements them through the reset controller subsystem.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index cb33421184a0..f37494d5a7be 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -48,10 +48,11 @@ Additional required properties for imx7d-pcie:
 - resets: Must contain phandles to PCIe-related reset lines exposed by SRC
   IP block
 - reset-names: Must contain the following entires:
 	       - "pciephy"
 	       - "apps"
+	       - "turnoff"
 
 Example:
 
 	pcie@01000000 {
 		compatible = "fsl,imx6q-pcie", "snps,dw-pcie";
-- 
2.17.1

[PATCH v2 3/4] ARM: dts: imx7d: Add turnoff reset

[Leonard Crestez]

This is required for the imx pci driver to send the PME_Turn_Off TLP.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Acked-by: Shawn Guo <shawnguo@kernel.org>
---
 arch/arm/boot/dts/imx7d.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 73c35939e07c..826224bf7f4f 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -147,12 +147,13 @@
 					 <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>;
 
 		fsl,max-link-speed = <2>;
 		power-domains = <&pgc_pcie_phy>;
 		resets = <&src IMX7_RESET_PCIEPHY>,
-			 <&src IMX7_RESET_PCIE_CTRL_APPS_EN>;
-		reset-names = "pciephy", "apps";
+			 <&src IMX7_RESET_PCIE_CTRL_APPS_EN>,
+			 <&src IMX7_RESET_PCIE_CTRL_APPS_TURNOFF>;
+		reset-names = "pciephy", "apps", "turnoff";
 		status = "disabled";
 	};
 };
 
 &ca_funnel_in_ports {
-- 
2.17.1

[PATCH v2 4/4] PCI: imx: Add PME_Turn_Off support

[Leonard Crestez]

When the root complex suspends it must send a PME_Turn_Off TLP.
Implement this by asserting the "turnoff" reset.

On imx7d this functionality is part of the SRC and exposed through the
linux reset-controller subsystem. On imx6 equivalent bits are in the
IOMUXC GPR area which the imx6-pcie driver accesses directly.

This is only for imx7d right now but it's deliberately implemented as an
optional reset, ignoring the chip variant:
* Older dtbs won't have this reset so it will be ignored.
* Future chips might also expose this as a reset controller.

For example imx8m (not yet supported) has the exact same
PCIE_CTRL_APPS_TURNOFF bit in the same location.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 6ba16fd1373c..2bf80f1ad852 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -50,10 +50,11 @@ struct imx6_pcie {
 	struct clk		*pcie_inbound_axi;
 	struct clk		*pcie;
 	struct regmap		*iomuxc_gpr;
 	struct reset_control	*pciephy_reset;
 	struct reset_control	*apps_reset;
+	struct reset_control	*turnoff_reset;
 	enum imx6_pcie_variants variant;
 	u32			tx_deemph_gen1;
 	u32			tx_deemph_gen2_3p5db;
 	u32			tx_deemph_gen2_6db;
 	u32			tx_swing_full;
@@ -812,10 +813,25 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
 	default:
 		dev_err(dev, "ltssm_disable not supported\n");
 	}
 }
 
+static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
+{
+	reset_control_assert(imx6_pcie->turnoff_reset);
+	reset_control_deassert(imx6_pcie->turnoff_reset);
+
+	/*
+	 * Components with an upstream port must respond to
+	 * PME_Turn_Off with PME_TO_Ack but we can't check.
+	 *
+	 * The standard recommends a 1-10ms timeout after which to
+	 * proceed anyway as if acks were received.
+	 */
+	usleep_range(1000, 10000);
+}
+
 static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
 {
 	clk_disable_unprepare(imx6_pcie->pcie);
 	clk_disable_unprepare(imx6_pcie->pcie_phy);
 	clk_disable_unprepare(imx6_pcie->pcie_bus);
@@ -832,10 +848,11 @@ static int imx6_pcie_suspend_noirq(struct device *dev)
 	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
 
 	if (imx6_pcie->variant != IMX7D)
 		return 0;
 
+	imx6_pcie_pm_turnoff(imx6_pcie);
 	imx6_pcie_clk_disable(imx6_pcie);
 	imx6_pcie_ltssm_disable(dev);
 
 	return 0;
 }
@@ -959,10 +976,17 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 		break;
 	default:
 		break;
 	}
 
+	/* Grab turnoff reset */
+	imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");
+	if (IS_ERR(imx6_pcie->turnoff_reset)) {
+		dev_err(dev, "Failed to get TURNOFF reset control\n");
+		return PTR_ERR(imx6_pcie->turnoff_reset);
+	}
+
 	/* Grab GPR config register range */
 	imx6_pcie->iomuxc_gpr =
 		 syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
 	if (IS_ERR(imx6_pcie->iomuxc_gpr)) {
 		dev_err(dev, "unable to find iomuxc registers\n");
-- 
2.17.1

Re: [PATCH v2 4/4] PCI: imx: Add PME_Turn_Off support

[Lucas Stach]

Am Donnerstag, den 04.10.2018, 16:34 +0000 schrieb Leonard Crestez:
> When the root complex suspends it must send a PME_Turn_Off TLP.
> Implement this by asserting the "turnoff" reset.
> 
> On imx7d this functionality is part of the SRC and exposed through the
> linux reset-controller subsystem. On imx6 equivalent bits are in the
> IOMUXC GPR area which the imx6-pcie driver accesses directly.
> 
> This is only for imx7d right now but it's deliberately implemented as an
> optional reset, ignoring the chip variant:
> * Older dtbs won't have this reset so it will be ignored.
> * Future chips might also expose this as a reset controller.
> 
> For example imx8m (not yet supported) has the exact same
> PCIE_CTRL_APPS_TURNOFF bit in the same location.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 6ba16fd1373c..2bf80f1ad852 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -50,10 +50,11 @@ struct imx6_pcie {
> > >  	struct clk		*pcie_inbound_axi;
> > >  	struct clk		*pcie;
> > >  	struct regmap		*iomuxc_gpr;
> > >  	struct reset_control	*pciephy_reset;
> > >  	struct reset_control	*apps_reset;
> > > +	struct reset_control	*turnoff_reset;
> >  	enum imx6_pcie_variants variant;
> > >  	u32			tx_deemph_gen1;
> > >  	u32			tx_deemph_gen2_3p5db;
> > >  	u32			tx_deemph_gen2_6db;
> > >  	u32			tx_swing_full;
> @@ -812,10 +813,25 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
> >  	default:
> >  		dev_err(dev, "ltssm_disable not supported\n");
> >  	}
>  }
>  
> +static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
> +{
> > +	reset_control_assert(imx6_pcie->turnoff_reset);
> > +	reset_control_deassert(imx6_pcie->turnoff_reset);
> +
> > +	/*
> > +	 * Components with an upstream port must respond to
> > +	 * PME_Turn_Off with PME_TO_Ack but we can't check.
> > +	 *
> > +	 * The standard recommends a 1-10ms timeout after which to
> > +	 * proceed anyway as if acks were received.
> > +	 */
> > +	usleep_range(1000, 10000);
> +}
> +
>  static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
>  {
> >  	clk_disable_unprepare(imx6_pcie->pcie);
> >  	clk_disable_unprepare(imx6_pcie->pcie_phy);
> >  	clk_disable_unprepare(imx6_pcie->pcie_bus);
> @@ -832,10 +848,11 @@ static int imx6_pcie_suspend_noirq(struct device *dev)
> >  	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
>  
> >  	if (imx6_pcie->variant != IMX7D)
> >  		return 0;
>  
> > +	imx6_pcie_pm_turnoff(imx6_pcie);
> >  	imx6_pcie_clk_disable(imx6_pcie);
> >  	imx6_pcie_ltssm_disable(dev);
>  
> >  	return 0;
>  }
> @@ -959,10 +976,17 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> >  		break;
> >  	default:
> >  		break;
> >  	}
>  
> > +	/* Grab turnoff reset */
> > +	imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");
> > +	if (IS_ERR(imx6_pcie->turnoff_reset)) {
> > +		dev_err(dev, "Failed to get TURNOFF reset control\n");
> > +		return PTR_ERR(imx6_pcie->turnoff_reset);
> > +	}
> +
> >  	/* Grab GPR config register range */
> >  	imx6_pcie->iomuxc_gpr =
> >  		 syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
> >  	if (IS_ERR(imx6_pcie->iomuxc_gpr)) {
> >  		dev_err(dev, "unable to find iomuxc registers\n");

Re: [PATCH v2 1/4] reset: imx7: Add PCIE_CTRL_APPS_TURNOFF

[Philipp Zabel]

Hi Leonard,

On Thu, 2018-10-04 at 16:34 +0000, Leonard Crestez wrote:
> This is required for the imx pci driver to send the PME_Turn_Off TLP.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/reset/reset-imx7.c             | 1 +
>  include/dt-bindings/reset/imx7-reset.h | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> index 97d9f08271c5..77911fa8f31d 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -65,10 +65,11 @@ static const struct imx7_src_signal imx7_src_signals[IMX7_RESET_NUM] = {
>  	[IMX7_RESET_MIPI_PHY_MRST]	= { SRC_MIPIPHY_RCR, BIT(1) },
>  	[IMX7_RESET_MIPI_PHY_SRST]	= { SRC_MIPIPHY_RCR, BIT(2) },
>  	[IMX7_RESET_PCIEPHY]		= { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
>  	[IMX7_RESET_PCIEPHY_PERST]	= { SRC_PCIEPHY_RCR, BIT(3) },
>  	[IMX7_RESET_PCIE_CTRL_APPS_EN]	= { SRC_PCIEPHY_RCR, BIT(6) },
> +	[IMX7_RESET_PCIE_CTRL_APPS_TURNOFF] = { SRC_PCIEPHY_RCR, BIT(11) },
>  	[IMX7_RESET_DDRC_PRST]		= { SRC_DDRC_RCR, BIT(0) },
>  	[IMX7_RESET_DDRC_CORE_RST]	= { SRC_DDRC_RCR, BIT(1) },
>  };
>  
>  static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev)
> diff --git a/include/dt-bindings/reset/imx7-reset.h b/include/dt-bindings/reset/imx7-reset.h
> index 63948170c7b2..31b3f87dde9a 100644
> --- a/include/dt-bindings/reset/imx7-reset.h
> +++ b/include/dt-bindings/reset/imx7-reset.h
> @@ -54,9 +54,11 @@
>   */
>  #define IMX7_RESET_PCIE_CTRL_APPS_EN	22
>  #define IMX7_RESET_DDRC_PRST		23
>  #define IMX7_RESET_DDRC_CORE_RST	24
>  
> -#define IMX7_RESET_NUM			25
> +#define IMX7_RESET_PCIE_CTRL_APPS_TURNOFF 25
> +
> +#define IMX7_RESET_NUM			26
>  
>  #endif

This is contained enough to be merged with the rest of the series,
patches 1 and 2:

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

Let me know if I should pick them up instead.

regards
Philipp

Re: [PATCH v2 1/4] reset: imx7: Add PCIE_CTRL_APPS_TURNOFF

[Lorenzo Pieralisi]

On Thu, Oct 04, 2018 at 06:47:01PM +0200, Philipp Zabel wrote:
> Hi Leonard,
> 
> On Thu, 2018-10-04 at 16:34 +0000, Leonard Crestez wrote:
> > This is required for the imx pci driver to send the PME_Turn_Off TLP.
> > 
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > Acked-by: Rob Herring <robh@kernel.org>
> > ---
> >  drivers/reset/reset-imx7.c             | 1 +
> >  include/dt-bindings/reset/imx7-reset.h | 4 +++-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> > index 97d9f08271c5..77911fa8f31d 100644
> > --- a/drivers/reset/reset-imx7.c
> > +++ b/drivers/reset/reset-imx7.c
> > @@ -65,10 +65,11 @@ static const struct imx7_src_signal imx7_src_signals[IMX7_RESET_NUM] = {
> >  	[IMX7_RESET_MIPI_PHY_MRST]	= { SRC_MIPIPHY_RCR, BIT(1) },
> >  	[IMX7_RESET_MIPI_PHY_SRST]	= { SRC_MIPIPHY_RCR, BIT(2) },
> >  	[IMX7_RESET_PCIEPHY]		= { SRC_PCIEPHY_RCR, BIT(2) | BIT(1) },
> >  	[IMX7_RESET_PCIEPHY_PERST]	= { SRC_PCIEPHY_RCR, BIT(3) },
> >  	[IMX7_RESET_PCIE_CTRL_APPS_EN]	= { SRC_PCIEPHY_RCR, BIT(6) },
> > +	[IMX7_RESET_PCIE_CTRL_APPS_TURNOFF] = { SRC_PCIEPHY_RCR, BIT(11) },
> >  	[IMX7_RESET_DDRC_PRST]		= { SRC_DDRC_RCR, BIT(0) },
> >  	[IMX7_RESET_DDRC_CORE_RST]	= { SRC_DDRC_RCR, BIT(1) },
> >  };
> >  
> >  static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev)
> > diff --git a/include/dt-bindings/reset/imx7-reset.h b/include/dt-bindings/reset/imx7-reset.h
> > index 63948170c7b2..31b3f87dde9a 100644
> > --- a/include/dt-bindings/reset/imx7-reset.h
> > +++ b/include/dt-bindings/reset/imx7-reset.h
> > @@ -54,9 +54,11 @@
> >   */
> >  #define IMX7_RESET_PCIE_CTRL_APPS_EN	22
> >  #define IMX7_RESET_DDRC_PRST		23
> >  #define IMX7_RESET_DDRC_CORE_RST	24
> >  
> > -#define IMX7_RESET_NUM			25
> > +#define IMX7_RESET_PCIE_CTRL_APPS_TURNOFF 25
> > +
> > +#define IMX7_RESET_NUM			26
> >  
> >  #endif
> 
> This is contained enough to be merged with the rest of the series,
> patches 1 and 2:
> 
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> Let me know if I should pick them up instead.

I can take the whole series but I would like to have it rebased against
v4.19-rc4 please since it does not apply as it is, thanks.

Lorenzo

Re: [PATCH v2 1/4] reset: imx7: Add PCIE_CTRL_APPS_TURNOFF

[Leonard Crestez]

On Thu, 2018-10-04 at 18:08 +0100, Lorenzo Pieralisi wrote:
> On Thu, Oct 04, 2018 at 06:47:01PM +0200, Philipp Zabel wrote:

> > This is contained enough to be merged with the rest of the series,
> > patches 1 and 2:
> > 
> > Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
> > 
> > Let me know if I should pick them up instead.
> 
> I can take the whole series but I would like to have it rebased against
> v4.19-rc4 please since it does not apply as it is, thanks.

Series is from a branch on top of next-20181004 + a few other changes
but it applied it on top of your pci/dwc branch with no conflicts.

Pushed the branch here:

https://github.com/cdleonard/linux/commits/imx_pci_for_lorenzo

Assuming you want on top of this:

https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/log/?h=pci/dwc

--
Regards,
Leonard

Re: [PATCH v2 4/4] PCI: imx: Add PME_Turn_Off support

[Lorenzo Pieralisi]

On Thu, Oct 04, 2018 at 04:34:30PM +0000, Leonard Crestez wrote:
> When the root complex suspends it must send a PME_Turn_Off TLP.
> Implement this by asserting the "turnoff" reset.
> 
> On imx7d this functionality is part of the SRC and exposed through the
> linux reset-controller subsystem. On imx6 equivalent bits are in the
> IOMUXC GPR area which the imx6-pcie driver accesses directly.

Nit: I am merging the series, please define what SRC and GPR are so
that I can update the commit log accordingly, it must be clear to
anyone reading it.

Thanks,
Lorenzo

> This is only for imx7d right now but it's deliberately implemented as an
> optional reset, ignoring the chip variant:
> * Older dtbs won't have this reset so it will be ignored.
> * Future chips might also expose this as a reset controller.
> 
> For example imx8m (not yet supported) has the exact same
> PCIE_CTRL_APPS_TURNOFF bit in the same location.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 6ba16fd1373c..2bf80f1ad852 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -50,10 +50,11 @@ struct imx6_pcie {
>  	struct clk		*pcie_inbound_axi;
>  	struct clk		*pcie;
>  	struct regmap		*iomuxc_gpr;
>  	struct reset_control	*pciephy_reset;
>  	struct reset_control	*apps_reset;
> +	struct reset_control	*turnoff_reset;
>  	enum imx6_pcie_variants variant;
>  	u32			tx_deemph_gen1;
>  	u32			tx_deemph_gen2_3p5db;
>  	u32			tx_deemph_gen2_6db;
>  	u32			tx_swing_full;
> @@ -812,10 +813,25 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
>  	default:
>  		dev_err(dev, "ltssm_disable not supported\n");
>  	}
>  }
>  
> +static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
> +{
> +	reset_control_assert(imx6_pcie->turnoff_reset);
> +	reset_control_deassert(imx6_pcie->turnoff_reset);
> +
> +	/*
> +	 * Components with an upstream port must respond to
> +	 * PME_Turn_Off with PME_TO_Ack but we can't check.
> +	 *
> +	 * The standard recommends a 1-10ms timeout after which to
> +	 * proceed anyway as if acks were received.
> +	 */
> +	usleep_range(1000, 10000);
> +}
> +
>  static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
>  {
>  	clk_disable_unprepare(imx6_pcie->pcie);
>  	clk_disable_unprepare(imx6_pcie->pcie_phy);
>  	clk_disable_unprepare(imx6_pcie->pcie_bus);
> @@ -832,10 +848,11 @@ static int imx6_pcie_suspend_noirq(struct device *dev)
>  	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
>  
>  	if (imx6_pcie->variant != IMX7D)
>  		return 0;
>  
> +	imx6_pcie_pm_turnoff(imx6_pcie);
>  	imx6_pcie_clk_disable(imx6_pcie);
>  	imx6_pcie_ltssm_disable(dev);
>  
>  	return 0;
>  }
> @@ -959,10 +976,17 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		break;
>  	default:
>  		break;
>  	}
>  
> +	/* Grab turnoff reset */
> +	imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");
> +	if (IS_ERR(imx6_pcie->turnoff_reset)) {
> +		dev_err(dev, "Failed to get TURNOFF reset control\n");
> +		return PTR_ERR(imx6_pcie->turnoff_reset);
> +	}
> +
>  	/* Grab GPR config register range */
>  	imx6_pcie->iomuxc_gpr =
>  		 syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
>  	if (IS_ERR(imx6_pcie->iomuxc_gpr)) {
>  		dev_err(dev, "unable to find iomuxc registers\n");
> -- 
> 2.17.1
> 

Re: [PATCH v2 4/4] PCI: imx: Add PME_Turn_Off support

[Philipp Zabel]

Hi Lorenzo,

On Fri, 2018-10-05 at 11:29 +0100, Lorenzo Pieralisi wrote:
> On Thu, Oct 04, 2018 at 04:34:30PM +0000, Leonard Crestez wrote:
> > When the root complex suspends it must send a PME_Turn_Off TLP.
> > Implement this by asserting the "turnoff" reset.
> > 
> > On imx7d this functionality is part of the SRC and exposed through the
> > linux reset-controller subsystem. On imx6 equivalent bits are in the
> > IOMUXC GPR area which the imx6-pcie driver accesses directly.
> 
> Nit: I am merging the series, please define what SRC and GPR are so
> that I can update the commit log accordingly, it must be clear to
> anyone reading it.

SRC is the System Reset Controller, which controls reset signals to all
kinds of devices, among them the PCIe PHY.

IOMUXC GPR is a General Purpose Register range in the IOMUXC (pinmux
controller) that is used to control various signals all over the SoC.

regards
Philipp

Re: [PATCH v2 4/4] PCI: imx: Add PME_Turn_Off support

[Lorenzo Pieralisi]

On Fri, Oct 05, 2018 at 12:59:16PM +0200, Philipp Zabel wrote:
> Hi Lorenzo,
> 
> On Fri, 2018-10-05 at 11:29 +0100, Lorenzo Pieralisi wrote:
> > On Thu, Oct 04, 2018 at 04:34:30PM +0000, Leonard Crestez wrote:
> > > When the root complex suspends it must send a PME_Turn_Off TLP.
> > > Implement this by asserting the "turnoff" reset.
> > > 
> > > On imx7d this functionality is part of the SRC and exposed through the
> > > linux reset-controller subsystem. On imx6 equivalent bits are in the
> > > IOMUXC GPR area which the imx6-pcie driver accesses directly.
> > 
> > Nit: I am merging the series, please define what SRC and GPR are so
> > that I can update the commit log accordingly, it must be clear to
> > anyone reading it.
> 
> SRC is the System Reset Controller, which controls reset signals to all
> kinds of devices, among them the PCIe PHY.
> 
> IOMUXC GPR is a General Purpose Register range in the IOMUXC (pinmux
> controller) that is used to control various signals all over the SoC.

Thanks Philipp. Updated the commit log and pushed out the series
for v4.20 (branch pci/dwc - I have just updated this patch log).

Thanks,
Lorenzo