2018-10-04 16:35:57

by Leonard Crestez

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

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



2018-10-04 16:35:06

by Leonard Crestez

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

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

Signed-off-by: Leonard Crestez <[email protected]>
Acked-by: Shawn Guo <[email protected]>
---
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


2018-10-04 16:35:14

by Leonard Crestez

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

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

Signed-off-by: Leonard Crestez <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
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


2018-10-04 16:35:21

by Leonard Crestez

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

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 <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
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


2018-10-04 16:35:29

by Leonard Crestez

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

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 <[email protected]>
---
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


2018-10-04 16:45:02

by Lucas Stach

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

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 <[email protected]>

Reviewed-by: Lucas Stach <[email protected]>

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

2018-10-04 16:47:45

by Philipp Zabel

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

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 <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> ---
> 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 <[email protected]>

Let me know if I should pick them up instead.

regards
Philipp

2018-10-04 17:08:39

by Lorenzo Pieralisi

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

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 <[email protected]>
> > Acked-by: Rob Herring <[email protected]>
> > ---
> > 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 <[email protected]>
>
> 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

2018-10-04 20:41:56

by Leonard Crestez

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

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 <[email protected]>
> >
> > 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

2018-10-05 10:28:49

by Lorenzo Pieralisi

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

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 <[email protected]>
> ---
> 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
>

2018-10-05 11:01:28

by Philipp Zabel

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

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

2018-10-05 11:09:21

by Lorenzo Pieralisi

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

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