PCI: imx: Add PME_Turn_Off support
Subsystems (6)
cover 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 is 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.
---
Previously posted here: https://patchwork.kernel.org/cover/10565871/
Parts of that were already merged and available in linux-next so reposting as a focused series.
This is not very complex but needs to be split because it touches multiple trees. Merging out of order should be fine.
Some patches already carry acks for DT but somebody needs to ack the pci/reset parts.
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
.../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 1 + arch/arm/boot/dts/imx7d.dtsi | 5 +++-- drivers/pci/controller/dwc/pci-imx6.c | 15 +++++++++++++++ drivers/reset/reset-imx7.c | 1 + include/dt-bindings/reset/imx7-reset.h | 4 +++- 5 files changed, 23 insertions(+), 3 deletions(-)
Signature · 1 line
2.17.1
PATCH 1/4 reset: imx7: Add PCIE_CTRL_APPS_TURNOFF
This is required for the imx pci driver to send the PME_Turn_Off TLP.
--- drivers/reset/reset-imx7.c | 1 + include/dt-bindings/reset/imx7-reset.h | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-)
drivers/reset/reset-imx7.c +1 -0
@@ -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)
include/dt-bindings/reset/imx7-reset.h +3 -1
@@ -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
Signature · 1 line
2.17.1
PATCH 2/4 dt-bindings: imx6q-pcie: Add turnoff reset for imx7d
In reply to Leonard CrestezThis 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.
--- Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 1 + 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +1 -0
@@ -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";
Signature · 1 line
2.17.1
PATCH 3/4 ARM: dts: imx7d: Add turnoff reset
In reply to Leonard CrestezThis is required for the imx pci driver to send the PME_Turn_Off TLP.
--- arch/arm/boot/dts/imx7d.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm/boot/dts/imx7d.dtsi +3 -2
@@ -145,12 +145,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_ports {
Signature · 1 line
2.17.1
PATCH 4/4 PCI: imx: Add PME_Turn_Off support
In reply to Leonard CrestezWhen the root complex suspends it must send a PME_Turn_Off TLP. Implement this by asserting the "turnoff" reset.
On imx7d this is 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.
--- drivers/pci/controller/dwc/pci-imx6.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
drivers/pci/controller/dwc/pci-imx6.c +15 -0
@@ -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,16 @@ 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); +} +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 +839,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 +967,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" );
Signature · 1 line
2.17.1
Quoted 12 lines · Leonard Crestez
When the root complex suspends it must send a
PME_Turn_OffTLP. Implement this by asserting the "turnoff" reset.On imx7d this is 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.
Quoted 7 lines
For example imx8m (not yet supported) has the exact same
PCIE_CTRL_APPS_TURNOFFbit in the same location.Signed-off-by Leonard Crestez <[email protected]>--- drivers/pci/controller/dwc/pci-imx6.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
I need an ACK from Lucas on this patch to merge this series; also, if you can keep versioning your patch series please it would be easier for me to track them, I understand the patches content changed over time but that's for future postings too.
Thanks,
Lorenzo
Quoted 67 lines
drivers/pci/controller/dwc/pci-imx6.c +15
-0@@ -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,16 @@ 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); +} +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 +839,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 +967,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" );Signature · 1 line
2.17.1
Lucas Stach
maintainerAm Montag, den 01.10.2018, 22:53 +0300 schrieb Leonard Crestez:
Quoted 45 lines
When the root complex suspends it must send a
PME_Turn_OffTLP. Implement this by asserting the "turnoff" reset.On imx7d this is 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_TURNOFFbit in the same location.Quoted 1 line
Signed-off-by Leonard Crestez <[email protected]>---
drivers/pci/controller/dwc/pci-imx6.c | 15 +++++++++++++++1 file changed, 15 insertions(+)drivers/pci/controller/dwc/pci-imx6.c +0
-0@@ -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;Quoted 1 line
Quoted 1 line
u32
tx_swing_full;drivers/pci/controller/dwc/pci-imx6.c +0
-0@@ -812,10 +813,16 @@ static void imx6_pcie_ltssm_disable(struct device *dev)Quoted 3 lines
default:
dev_err(dev, "ltssm_disable not supported\n"); }}
drivers/pci/controller/dwc/pci-imx6.c +2
-0+static void imx6_pcie_pm_turnoff (struct imx6_pcie *imx6_pcie) +{Quoted 1 line
drivers/pci/controller/dwc/pci-imx6.c +1
-0+reset_control_assert (imx6_pcie->turnoff_reset);
drivers/pci/controller/dwc/pci-imx6.c static void imx6_pcie_ltssm_disable(struct device *dev) · line 821
Quoted 1 line
drivers/pci/controller/dwc/pci-imx6.c +1
-0+reset_control_deassert (imx6_pcie->turnoff_reset);
I'm a bit surprised to see no timing requirements here. I would have expected that there is a minimum time from asserting the reset, so the turnoff message gets transmitted to the EP before the clocks are stopped.
Regards,
Lucas
Quoted 37 lines
drivers/pci/controller/dwc/pci-imx6.c +2
-0+} +static void
imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie){Quoted 3 lines
clk_disable_unprepare(imx6_pcie->pcie);clk_disable_unprepare(imx6_pcie->pcie_phy);clk_disable_unprepare(imx6_pcie->pcie_bus);drivers/pci/controller/dwc/pci-imx6.c +0
-0@@ -832,10 +839,11 @@ static int imx6_pcie_suspend_noirq(struct device *dev) > struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);Quoted 2 lines
if (
imx6_pcie->variant!= IMX7D)
return 0;Quoted 3 lines
drivers/pci/controller/dwc/pci-imx6.c +1
-0+imx6_pcie_pm_turnoff (imx6_pcie);
imx6_pcie_clk_disable(imx6_pcie);
imx6_pcie_ltssm_disable(dev);Quoted 1 line
return 0;
}
drivers/pci/controller/dwc/pci-imx6.c +0
-0@@ -959,10 +967,17 @@ static int imx6_pcie_probe(struct platform_device *pdev) > break;Quoted 3 lines
default:
break;
}drivers/pci/controller/dwc/pci-imx6.c static int imx6_pcie_probe(struct platform_device *pdev) · line 977
Quoted 6 lines
drivers/pci/controller/dwc/pci-imx6.c +6
-0+/* 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); + }+
Quoted 5 lines
/* 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");
Quoted 4 lines · Lucas Stach
Am Montag, den 01.10.2018, 22:53 +0300 schrieb Leonard Crestez:
Quoted 3 lines
When the root complex suspends it must send a
PME_Turn_OffTLP. Implement this by asserting the "turnoff" reset.
Quoted 9 lines
drivers/pci/controller/dwc/pci-imx6.c static void imx6_pcie_ltssm_disable(struct device *dev) · line 821
Quoted 4 lines
drivers/pci/controller/dwc/pci-imx6.c +4
-0+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);I'm a bit surprised to see no timing requirements here. I would have expected that there is a minimum time from asserting the reset, so the turnoff message gets transmitted to the EP before the clocks are stopped.
According to the PCI standard after PME_Turn_Off is sent all components must respond with PME_TO_Ack. There doesn't seem to be any bit exposed to check if acks were received but the standard recommends a 1-10ms timeout after which you should proceed anyway.
It seems the NXP vendor tree has an udelay(1000) which I missed, it seems like an acceptable solution. Or maybe it should be msleep(10)?
--
Regards,
Leonard
Philipp Zabel
maintainerQuoted 15 lines · Leonard Crestez
Quoted 4 lines · Lucas Stach
Am Montag, den 01.10.2018, 22:53 +0300 schrieb Leonard Crestez:
Quoted 3 lines
When the root complex suspends it must send a
PME_Turn_OffTLP. Implement this by asserting the "turnoff" reset.Quoted 9 lines
drivers/pci/controller/dwc/pci-imx6.c static void imx6_pcie_ltssm_disable(struct device *dev) · line 821
Quoted 4 lines
drivers/pci/controller/dwc/pci-imx6.c +4
-0+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);I'm a bit surprised to see no timing requirements here. I would have expected that there is a minimum time from asserting the reset, so the turnoff message gets transmitted to the EP before the clocks are stopped.
Quoted 7 lines
According to the PCI standard after
PME_Turn_Offis sent all components must respond withPME_TO_Ack. There doesn't seem to be any bit exposed to check if acks were received but the standard recommends a 1-10ms timeout after which you should proceed anyway.It seems the NXP vendor tree has an udelay(1000) which I missed, it seems like an acceptable solution. Or maybe it should be msleep(10)?
Maybe usleep_range(1000, 10000) ?
regards
Philipp
Lucas Stach
maintainerAm Donnerstag, den 04.10.2018, 15:25 +0200 schrieb Philipp Zabel:
Quoted 21 lines · Leonard Crestez
Quoted 20 lines · Lucas Stach
Quoted 6 lines
Am Montag, den 01.10.2018, 22:53 +0300 schrieb Leonard Crestez:
Quoted 5 lines
When the root complex suspends it must send a
PME_Turn_OffTLP. Implement this by asserting the "turnoff" reset.drivers/pci/controller/dwc/pci-imx6.c +2
-0+static void imx6_pcie_pm_turnoff (struct imx6_pcie *imx6_pcie) +{Quoted 7 lines
drivers/pci/controller/dwc/pci-imx6.c static void imx6_pcie_ltssm_disable(struct device *dev) · line 821
Quoted 2 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
drivers/pci/controller/dwc/pci-imx6.c +1
-0+reset_control_assert (imx6_pcie->turnoff_reset);drivers/pci/controller/dwc/pci-imx6.c +1
-0+reset_control_deassert (imx6_pcie->turnoff_reset);I'm a bit surprised to see no timing requirements here. I would have expected that there is a minimum time from asserting the reset, so the turnoff message gets transmitted to the EP before the clocks are stopped.
According to the PCI standard after
PME_Turn_Offis sent all components must respond withPME_TO_Ack. There doesn't seem to be any bit exposed to check if acks were received but the standard recommends a 1-10ms timeout after which you should proceed anyway.
Quoted 4 lines
Quoted 2 lines
It seems the NXP vendor tree has an udelay(1000) which I missed, it seems like an acceptable solution. Or maybe it should be msleep(10)?
Maybe
usleep_range(1000, 10000)?
Yep, we aren't in atomic context here and the timeout is relatively short so usleep_range() looks like the correct thing to use.
Regards,
Lucas