[RFC 1/2] ARM: dts: imx6qp-sabresd: Enable pcie support

[RFC 1/2] ARM: dts: imx6qp-sabresd: Enable pcie support

[Richard Zhu]

In the i.MX6QP sabresd board(sch-28857) design, one external oscillator
is used as the PCIe reference clock source by the endpoint device.

If RC uses this oscillator as reference clock too, PLL6(ENET PLL) would
has to be in bypass mode, and ENET clocks would be messed up.

To keep things simple, let RC use the internal PLL as reference clock
and always enable the external oscillator for endpoint device on
i.MX6QP sabresd board.

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

diff --git a/arch/arm/boot/dts/imx6qp-sabresd.dts b/arch/arm/boot/dts/imx6qp-sabresd.dts
index 480e73183f6b..083cf90bcab5 100644
--- a/arch/arm/boot/dts/imx6qp-sabresd.dts
+++ b/arch/arm/boot/dts/imx6qp-sabresd.dts
@@ -50,8 +50,14 @@ MX6QDL_PAD_SD3_DAT7__SD3_DATA7		0x17059
 	};
 };
 
+&vgen3_reg {
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-always-on;
+};
+
 &pcie {
-	status = "disabled";
+	status = "okay";
 };
 
 &sata {
-- 
2.25.1

[RFC 1/2] ARM: dts: imx6qp-sabresd: Enable pcie support

[Richard Zhu]

In the i.MX6QP sabresd board(sch-28857) design, one external oscillator
is used as the PCIe reference clock source by the endpoint device.

If RC uses this oscillator as reference clock too, PLL6(ENET PLL) would
has to be in bypass mode, and ENET clocks would be messed up.

To keep things simple, let RC use the internal PLL as reference clock
and always enable the external oscillator for endpoint device on
i.MX6QP sabresd board.

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

diff --git a/arch/arm/boot/dts/imx6qp-sabresd.dts b/arch/arm/boot/dts/imx6qp-sabresd.dts
index 480e73183f6b..083cf90bcab5 100644
--- a/arch/arm/boot/dts/imx6qp-sabresd.dts
+++ b/arch/arm/boot/dts/imx6qp-sabresd.dts
@@ -50,8 +50,14 @@ MX6QDL_PAD_SD3_DAT7__SD3_DATA7		0x17059
 	};
 };
 
+&vgen3_reg {
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-always-on;
+};
+
 &pcie {
-	status = "disabled";
+	status = "okay";
 };
 
 &sata {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Richard Zhu]

i.MX6QP PCIe supports the RESET logic, thus it can support
the L2 exit by the reset mechanism.
Enable the i.MX6QP PCIe suspend/resume operations support.

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

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 784801f2f9e6..62262483470a 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -995,6 +995,7 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
 	/* Others poke directly at IOMUXC registers */
 	switch (imx6_pcie->drvdata->variant) {
 	case IMX6SX:
+	case IMX6QP:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				IMX6SX_GPR12_PCIE_PM_TURN_OFF,
 				IMX6SX_GPR12_PCIE_PM_TURN_OFF);
@@ -1307,7 +1308,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 	[IMX6QP] = {
 		.variant = IMX6QP,
 		.flags = IMX6_PCIE_FLAG_IMX6_PHY |
-			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
+			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE |
+			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.dbi_length = 0x200,
 	},
 	[IMX7D] = {
-- 
2.25.1

[RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Richard Zhu]

i.MX6QP PCIe supports the RESET logic, thus it can support
the L2 exit by the reset mechanism.
Enable the i.MX6QP PCIe suspend/resume operations support.

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

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 784801f2f9e6..62262483470a 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -995,6 +995,7 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
 	/* Others poke directly at IOMUXC registers */
 	switch (imx6_pcie->drvdata->variant) {
 	case IMX6SX:
+	case IMX6QP:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				IMX6SX_GPR12_PCIE_PM_TURN_OFF,
 				IMX6SX_GPR12_PCIE_PM_TURN_OFF);
@@ -1307,7 +1308,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 	[IMX6QP] = {
 		.variant = IMX6QP,
 		.flags = IMX6_PCIE_FLAG_IMX6_PHY |
-			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
+			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE |
+			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.dbi_length = 0x200,
 	},
 	[IMX7D] = {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 1/2] ARM: dts: imx6qp-sabresd: Enable pcie support

[Lucas Stach]

Hi Richard,

Am Mittwoch, dem 09.02.2022 um 15:02 +0800 schrieb Richard Zhu:
> In the i.MX6QP sabresd board(sch-28857) design, one external oscillator
> is used as the PCIe reference clock source by the endpoint device.
> 
> If RC uses this oscillator as reference clock too, PLL6(ENET PLL) would
> has to be in bypass mode, and ENET clocks would be messed up.
> 
> To keep things simple, let RC use the internal PLL as reference clock
> and always enable the external oscillator for endpoint device on
> i.MX6QP sabresd board.
> 
The commit message doesn't really match what's being done in the patch.
Maybe you meant to say that even though the HW design is different you
are enabling the PCIe controller in the same way as on the 6Q sabresd?

Also, is this configuration stable for you? We've had some issues with
this kind of split clocking setup in a customer design, where it was
enabled by accident, due to PLL6 no being bypassed. In this design it
caused the link to randomly drop under load and causing aborts on the
CPU side, due to completion timeouts. I think it at least warrants a
comment somewhere that this isn't a recommended setup.

Regards,
Lucas

> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  arch/arm/boot/dts/imx6qp-sabresd.dts | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/imx6qp-sabresd.dts b/arch/arm/boot/dts/imx6qp-sabresd.dts
> index 480e73183f6b..083cf90bcab5 100644
> --- a/arch/arm/boot/dts/imx6qp-sabresd.dts
> +++ b/arch/arm/boot/dts/imx6qp-sabresd.dts
> @@ -50,8 +50,14 @@ MX6QDL_PAD_SD3_DAT7__SD3_DATA7		0x17059
>  	};
>  };
>  
> +&vgen3_reg {
> +	regulator-min-microvolt = <1800000>;
> +	regulator-max-microvolt = <3300000>;
> +	regulator-always-on;
> +};
> +
>  &pcie {
> -	status = "disabled";
> +	status = "okay";
>  };
>  
>  &sata {

Re: [RFC 1/2] ARM: dts: imx6qp-sabresd: Enable pcie support

[Lucas Stach]

Hi Richard,

Am Mittwoch, dem 09.02.2022 um 15:02 +0800 schrieb Richard Zhu:
> In the i.MX6QP sabresd board(sch-28857) design, one external oscillator
> is used as the PCIe reference clock source by the endpoint device.
> 
> If RC uses this oscillator as reference clock too, PLL6(ENET PLL) would
> has to be in bypass mode, and ENET clocks would be messed up.
> 
> To keep things simple, let RC use the internal PLL as reference clock
> and always enable the external oscillator for endpoint device on
> i.MX6QP sabresd board.
> 
The commit message doesn't really match what's being done in the patch.
Maybe you meant to say that even though the HW design is different you
are enabling the PCIe controller in the same way as on the 6Q sabresd?

Also, is this configuration stable for you? We've had some issues with
this kind of split clocking setup in a customer design, where it was
enabled by accident, due to PLL6 no being bypassed. In this design it
caused the link to randomly drop under load and causing aborts on the
CPU side, due to completion timeouts. I think it at least warrants a
comment somewhere that this isn't a recommended setup.

Regards,
Lucas

> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  arch/arm/boot/dts/imx6qp-sabresd.dts | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/imx6qp-sabresd.dts b/arch/arm/boot/dts/imx6qp-sabresd.dts
> index 480e73183f6b..083cf90bcab5 100644
> --- a/arch/arm/boot/dts/imx6qp-sabresd.dts
> +++ b/arch/arm/boot/dts/imx6qp-sabresd.dts
> @@ -50,8 +50,14 @@ MX6QDL_PAD_SD3_DAT7__SD3_DATA7		0x17059
>  	};
>  };
>  
> +&vgen3_reg {
> +	regulator-min-microvolt = <1800000>;
> +	regulator-max-microvolt = <3300000>;
> +	regulator-always-on;
> +};
> +
>  &pcie {
> -	status = "disabled";
> +	status = "okay";
>  };
>  
>  &sata {



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Lucas Stach]

s/pcie/PCIe/ and maybe s/imx6qp/i.MX6QP/ in the subject.

Am Mittwoch, dem 09.02.2022 um 15:02 +0800 schrieb Richard Zhu:
> i.MX6QP PCIe supports the RESET logic, thus it can support
> the L2 exit by the reset mechanism.
> Enable the i.MX6QP PCIe suspend/resume operations support.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>

Other than the nitpick:
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
 
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 784801f2f9e6..62262483470a 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -995,6 +995,7 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
>  	/* Others poke directly at IOMUXC registers */
>  	switch (imx6_pcie->drvdata->variant) {
>  	case IMX6SX:
> +	case IMX6QP:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				IMX6SX_GPR12_PCIE_PM_TURN_OFF,
>  				IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> @@ -1307,7 +1308,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
>  	[IMX6QP] = {
>  		.variant = IMX6QP,
>  		.flags = IMX6_PCIE_FLAG_IMX6_PHY |
> -			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
> +			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE |
> +			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.dbi_length = 0x200,
>  	},
>  	[IMX7D] = {

Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Lucas Stach]

s/pcie/PCIe/ and maybe s/imx6qp/i.MX6QP/ in the subject.

Am Mittwoch, dem 09.02.2022 um 15:02 +0800 schrieb Richard Zhu:
> i.MX6QP PCIe supports the RESET logic, thus it can support
> the L2 exit by the reset mechanism.
> Enable the i.MX6QP PCIe suspend/resume operations support.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>

Other than the nitpick:
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
 
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 784801f2f9e6..62262483470a 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -995,6 +995,7 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
>  	/* Others poke directly at IOMUXC registers */
>  	switch (imx6_pcie->drvdata->variant) {
>  	case IMX6SX:
> +	case IMX6QP:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				IMX6SX_GPR12_PCIE_PM_TURN_OFF,
>  				IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> @@ -1307,7 +1308,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
>  	[IMX6QP] = {
>  		.variant = IMX6QP,
>  		.flags = IMX6_PCIE_FLAG_IMX6_PHY |
> -			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
> +			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE |
> +			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.dbi_length = 0x200,
>  	},
>  	[IMX7D] = {



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Bjorn Helgaas]

In subject, s/pcie/PCIe/ or remove it altogether, since I don't think
it adds useful information.

On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> i.MX6QP PCIe supports the RESET logic, thus it can support
> the L2 exit by the reset mechanism.
> Enable the i.MX6QP PCIe suspend/resume operations support.

Add blank line between paragraphs or rewrap into a single paragraph.

Rewrap to fill 75 columns.

What does "L2 exit by reset mechanism" mean?  Is this an
i.MX6-specific thing?  If not, can you point me to the relevant part
of the PCIe spec?

Bjorn

Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Bjorn Helgaas]

In subject, s/pcie/PCIe/ or remove it altogether, since I don't think
it adds useful information.

On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> i.MX6QP PCIe supports the RESET logic, thus it can support
> the L2 exit by the reset mechanism.
> Enable the i.MX6QP PCIe suspend/resume operations support.

Add blank line between paragraphs or rewrap into a single paragraph.

Rewrap to fill 75 columns.

What does "L2 exit by reset mechanism" mean?  Is this an
i.MX6-specific thing?  If not, can you point me to the relevant part
of the PCIe spec?

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [RFC 1/2] ARM: dts: imx6qp-sabresd: Enable pcie support

[Hongxing Zhu]

> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: 2022年2月9日 17:05
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org
> Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [RFC 1/2] ARM: dts: imx6qp-sabresd: Enable pcie support
> 
> Hi Richard,
> 
> Am Mittwoch, dem 09.02.2022 um 15:02 +0800 schrieb Richard Zhu:
> > In the i.MX6QP sabresd board(sch-28857) design, one external
> > oscillator is used as the PCIe reference clock source by the endpoint device.
> >
> > If RC uses this oscillator as reference clock too, PLL6(ENET PLL)
> > would has to be in bypass mode, and ENET clocks would be messed up.
> >
> > To keep things simple, let RC use the internal PLL as reference clock
> > and always enable the external oscillator for endpoint device on
> > i.MX6QP sabresd board.
> >
> The commit message doesn't really match what's being done in the patch.
> Maybe you meant to say that even though the HW design is different you are
> enabling the PCIe controller in the same way as on the 6Q sabresd?
> 
> Also, is this configuration stable for you? We've had some issues with this kind
> of split clocking setup in a customer design, where it was enabled by accident,
> due to PLL6 no being bypassed. In this design it caused the link to randomly
> drop under load and causing aborts on the CPU side, due to completion
> timeouts. I think it at least warrants a comment somewhere that this isn't a
> recommended setup.
Hi Lucas:
Thanks for your review.
There is a difference between i.MX6Q/DL sabresd and i.MX6QP sabresd board.

On i.MX6Q/DL sabresd board design, the PCIe clock used by EP device is
 output from internal PLL by CLK_N/P pads. This clock has some jitter
 problems, and can't pass the GEN2 TX compliance tests either.

To let remote EP device use qualified reference clock, and let PCIe
 hardware design pass the PCIe GEN2 TX compliance tests, one external
 oscillator is populated on the i.MX6QP sabresd board.

This patch is applied to i.MX6QP sabersd board, and enable PCIe port.
Yes, the PCIe is stable and pass the GEN2 TX compliance tests on i.MX6QP
 sabresd board

Thanks.
Best Regards
Richard
> 
> Regards,
> Lucas
> 
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  arch/arm/boot/dts/imx6qp-sabresd.dts | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/imx6qp-sabresd.dts
> > b/arch/arm/boot/dts/imx6qp-sabresd.dts
> > index 480e73183f6b..083cf90bcab5 100644
> > --- a/arch/arm/boot/dts/imx6qp-sabresd.dts
> > +++ b/arch/arm/boot/dts/imx6qp-sabresd.dts
> > @@ -50,8 +50,14 @@ MX6QDL_PAD_SD3_DAT7__SD3_DATA7
> 	0x17059
> >  	};
> >  };
> >
> > +&vgen3_reg {
> > +	regulator-min-microvolt = <1800000>;
> > +	regulator-max-microvolt = <3300000>;
> > +	regulator-always-on;
> > +};
> > +
> >  &pcie {
> > -	status = "disabled";
> > +	status = "okay";
> >  };
> >
> >  &sata {
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [RFC 1/2] ARM: dts: imx6qp-sabresd: Enable pcie support

[Hongxing Zhu]

> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: 2022年2月9日 17:05
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org
> Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [RFC 1/2] ARM: dts: imx6qp-sabresd: Enable pcie support
> 
> Hi Richard,
> 
> Am Mittwoch, dem 09.02.2022 um 15:02 +0800 schrieb Richard Zhu:
> > In the i.MX6QP sabresd board(sch-28857) design, one external
> > oscillator is used as the PCIe reference clock source by the endpoint device.
> >
> > If RC uses this oscillator as reference clock too, PLL6(ENET PLL)
> > would has to be in bypass mode, and ENET clocks would be messed up.
> >
> > To keep things simple, let RC use the internal PLL as reference clock
> > and always enable the external oscillator for endpoint device on
> > i.MX6QP sabresd board.
> >
> The commit message doesn't really match what's being done in the patch.
> Maybe you meant to say that even though the HW design is different you are
> enabling the PCIe controller in the same way as on the 6Q sabresd?
> 
> Also, is this configuration stable for you? We've had some issues with this kind
> of split clocking setup in a customer design, where it was enabled by accident,
> due to PLL6 no being bypassed. In this design it caused the link to randomly
> drop under load and causing aborts on the CPU side, due to completion
> timeouts. I think it at least warrants a comment somewhere that this isn't a
> recommended setup.
Hi Lucas:
Thanks for your review.
There is a difference between i.MX6Q/DL sabresd and i.MX6QP sabresd board.

On i.MX6Q/DL sabresd board design, the PCIe clock used by EP device is
 output from internal PLL by CLK_N/P pads. This clock has some jitter
 problems, and can't pass the GEN2 TX compliance tests either.

To let remote EP device use qualified reference clock, and let PCIe
 hardware design pass the PCIe GEN2 TX compliance tests, one external
 oscillator is populated on the i.MX6QP sabresd board.

This patch is applied to i.MX6QP sabersd board, and enable PCIe port.
Yes, the PCIe is stable and pass the GEN2 TX compliance tests on i.MX6QP
 sabresd board

Thanks.
Best Regards
Richard
> 
> Regards,
> Lucas
> 
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  arch/arm/boot/dts/imx6qp-sabresd.dts | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/imx6qp-sabresd.dts
> > b/arch/arm/boot/dts/imx6qp-sabresd.dts
> > index 480e73183f6b..083cf90bcab5 100644
> > --- a/arch/arm/boot/dts/imx6qp-sabresd.dts
> > +++ b/arch/arm/boot/dts/imx6qp-sabresd.dts
> > @@ -50,8 +50,14 @@ MX6QDL_PAD_SD3_DAT7__SD3_DATA7
> 	0x17059
> >  	};
> >  };
> >
> > +&vgen3_reg {
> > +	regulator-min-microvolt = <1800000>;
> > +	regulator-max-microvolt = <3300000>;
> > +	regulator-always-on;
> > +};
> > +
> >  &pcie {
> > -	status = "disabled";
> > +	status = "okay";
> >  };
> >
> >  &sata {
> 

RE: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Hongxing Zhu]

> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: 2022年2月9日 17:08
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org
> Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> support
> 
> s/pcie/PCIe/ and maybe s/imx6qp/i.MX6QP/ in the subject.
Thanks, would be changed later.

> 
> Am Mittwoch, dem 09.02.2022 um 15:02 +0800 schrieb Richard Zhu:
> > i.MX6QP PCIe supports the RESET logic, thus it can support the L2 exit
> > by the reset mechanism.
> > Enable the i.MX6QP PCIe suspend/resume operations support.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> 
> Other than the nitpick:
> Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

Thanks.
Best Regards
Richard Zhu> 
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 784801f2f9e6..62262483470a 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -995,6 +995,7 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie
> *imx6_pcie)
> >  	/* Others poke directly at IOMUXC registers */
> >  	switch (imx6_pcie->drvdata->variant) {
> >  	case IMX6SX:
> > +	case IMX6QP:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				IMX6SX_GPR12_PCIE_PM_TURN_OFF,
> >  				IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> > @@ -1307,7 +1308,8 @@ static const struct imx6_pcie_drvdata drvdata[] =
> {
> >  	[IMX6QP] = {
> >  		.variant = IMX6QP,
> >  		.flags = IMX6_PCIE_FLAG_IMX6_PHY |
> > -			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
> > +			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE |
> > +			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> >  		.dbi_length = 0x200,
> >  	},
> >  	[IMX7D] = {
> 

RE: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Hongxing Zhu]

> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: 2022年2月9日 17:08
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org
> Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> support
> 
> s/pcie/PCIe/ and maybe s/imx6qp/i.MX6QP/ in the subject.
Thanks, would be changed later.

> 
> Am Mittwoch, dem 09.02.2022 um 15:02 +0800 schrieb Richard Zhu:
> > i.MX6QP PCIe supports the RESET logic, thus it can support the L2 exit
> > by the reset mechanism.
> > Enable the i.MX6QP PCIe suspend/resume operations support.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> 
> Other than the nitpick:
> Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

Thanks.
Best Regards
Richard Zhu> 
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 784801f2f9e6..62262483470a 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -995,6 +995,7 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie
> *imx6_pcie)
> >  	/* Others poke directly at IOMUXC registers */
> >  	switch (imx6_pcie->drvdata->variant) {
> >  	case IMX6SX:
> > +	case IMX6QP:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				IMX6SX_GPR12_PCIE_PM_TURN_OFF,
> >  				IMX6SX_GPR12_PCIE_PM_TURN_OFF);
> > @@ -1307,7 +1308,8 @@ static const struct imx6_pcie_drvdata drvdata[] =
> {
> >  	[IMX6QP] = {
> >  		.variant = IMX6QP,
> >  		.flags = IMX6_PCIE_FLAG_IMX6_PHY |
> > -			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
> > +			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE |
> > +			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> >  		.dbi_length = 0x200,
> >  	},
> >  	[IMX7D] = {
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2022年2月9日 23:37
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> support
> 
> In subject, s/pcie/PCIe/ or remove it altogether, since I don't think it adds
> useful information.
> 
> On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> > i.MX6QP PCIe supports the RESET logic, thus it can support the L2 exit
> > by the reset mechanism.
> > Enable the i.MX6QP PCIe suspend/resume operations support.
> 
> Add blank line between paragraphs or rewrap into a single paragraph.
Got that, thanks.

> 
> Rewrap to fill 75 columns.
Okay, would be changed.

> 
> What does "L2 exit by reset mechanism" mean?  Is this an i.MX6-specific
> thing?  If not, can you point me to the relevant part of the PCIe spec?
No, it's not i.MX6 specific thing. i.MX6Q/DL doesn't have the self-reset mechanism.
Thus, it can't reset itself to an initialized stat when link exit from the L2
or L3 stats.
i.MX6QP PCIe has the self-reset mechanism, and it can reset itself when link
exit from L2 or L3 stats.
The commit description might not accurate.
How about change them to "i.MX6QP PCIe supports the RESET logic, thus it can
 reset itself to the initialized stat when exit from L2 or L3 stats."

Best Regards
Richard Zhu
> 
> Bjorn

RE: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2022年2月9日 23:37
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> support
> 
> In subject, s/pcie/PCIe/ or remove it altogether, since I don't think it adds
> useful information.
> 
> On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> > i.MX6QP PCIe supports the RESET logic, thus it can support the L2 exit
> > by the reset mechanism.
> > Enable the i.MX6QP PCIe suspend/resume operations support.
> 
> Add blank line between paragraphs or rewrap into a single paragraph.
Got that, thanks.

> 
> Rewrap to fill 75 columns.
Okay, would be changed.

> 
> What does "L2 exit by reset mechanism" mean?  Is this an i.MX6-specific
> thing?  If not, can you point me to the relevant part of the PCIe spec?
No, it's not i.MX6 specific thing. i.MX6Q/DL doesn't have the self-reset mechanism.
Thus, it can't reset itself to an initialized stat when link exit from the L2
or L3 stats.
i.MX6QP PCIe has the self-reset mechanism, and it can reset itself when link
exit from L2 or L3 stats.
The commit description might not accurate.
How about change them to "i.MX6QP PCIe supports the RESET logic, thus it can
 reset itself to the initialized stat when exit from L2 or L3 stats."

Best Regards
Richard Zhu
> 
> Bjorn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Bjorn Helgaas]

On Thu, Feb 10, 2022 at 03:23:19AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: 2022年2月9日 23:37
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > lorenzo.pieralisi@arm.com; shawnguo@kernel.org; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>
> > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> > support
> > 
> > On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> > > i.MX6QP PCIe supports the RESET logic, thus it can support the L2 exit
> > > by the reset mechanism.
> > > Enable the i.MX6QP PCIe suspend/resume operations support.

> > What does "L2 exit by reset mechanism" mean?  Is this an
> > i.MX6-specific thing?  If not, can you point me to the relevant
> > part of the PCIe spec?
>
> No, it's not i.MX6 specific thing. i.MX6Q/DL doesn't have the
> self-reset mechanism.  Thus, it can't reset itself to an initialized
> stat when link exit from the L2 or L3 stats.  i.MX6QP PCIe has the
> self-reset mechanism, and it can reset itself when link exit from L2
> or L3 stats.  The commit description might not accurate.  How about
> change them to "i.MX6QP PCIe supports the RESET logic, thus it can
> reset itself to the initialized stat when exit from L2 or L3 stats."

s/stat/state/

Ugh, I have all sorts of questions now, but I don't think I want to
know much more about this ;)

Seems like this device requires software assist when bringing the link
out of L2 or L3.  Is that allowed per PCIe spec, or is this an
erratum?

Does this mean the driver needs to be involved when we take a device
out of D3 (where the link was in L2 or L3)?

Bjorn

Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Bjorn Helgaas]

On Thu, Feb 10, 2022 at 03:23:19AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: 2022年2月9日 23:37
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > lorenzo.pieralisi@arm.com; shawnguo@kernel.org; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>
> > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> > support
> > 
> > On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> > > i.MX6QP PCIe supports the RESET logic, thus it can support the L2 exit
> > > by the reset mechanism.
> > > Enable the i.MX6QP PCIe suspend/resume operations support.

> > What does "L2 exit by reset mechanism" mean?  Is this an
> > i.MX6-specific thing?  If not, can you point me to the relevant
> > part of the PCIe spec?
>
> No, it's not i.MX6 specific thing. i.MX6Q/DL doesn't have the
> self-reset mechanism.  Thus, it can't reset itself to an initialized
> stat when link exit from the L2 or L3 stats.  i.MX6QP PCIe has the
> self-reset mechanism, and it can reset itself when link exit from L2
> or L3 stats.  The commit description might not accurate.  How about
> change them to "i.MX6QP PCIe supports the RESET logic, thus it can
> reset itself to the initialized stat when exit from L2 or L3 stats."

s/stat/state/

Ugh, I have all sorts of questions now, but I don't think I want to
know much more about this ;)

Seems like this device requires software assist when bringing the link
out of L2 or L3.  Is that allowed per PCIe spec, or is this an
erratum?

Does this mean the driver needs to be involved when we take a device
out of D3 (where the link was in L2 or L3)?

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2022年2月11日 6:05
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> support
> 
> On Thu, Feb 10, 2022 at 03:23:19AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > Sent: 2022年2月9日 23:37
> > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > > lorenzo.pieralisi@arm.com; shawnguo@kernel.org;
> > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> > > <linux-imx@nxp.com>
> > > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power
> > > management support
> > >
> > > On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> > > > i.MX6QP PCIe supports the RESET logic, thus it can support the L2
> > > > exit by the reset mechanism.
> > > > Enable the i.MX6QP PCIe suspend/resume operations support.
> 
> > > What does "L2 exit by reset mechanism" mean?  Is this an
> > > i.MX6-specific thing?  If not, can you point me to the relevant part
> > > of the PCIe spec?
> >
> > No, it's not i.MX6 specific thing. i.MX6Q/DL doesn't have the
> > self-reset mechanism.  Thus, it can't reset itself to an initialized
> > stat when link exit from the L2 or L3 stats.  i.MX6QP PCIe has the
> > self-reset mechanism, and it can reset itself when link exit from L2
> > or L3 stats.  The commit description might not accurate.  How about
> > change them to "i.MX6QP PCIe supports the RESET logic, thus it can
> > reset itself to the initialized stat when exit from L2 or L3 stats."
> 
> s/stat/state/
Thanks for your quickly reply.
Got that, would be changed later.
> 
> Ugh, I have all sorts of questions now, but I don't think I want to know much
> more about this ;)
> 
> Seems like this device requires software assist when bringing the link out of L2
> or L3.  Is that allowed per PCIe spec, or is this an erratum?
> 
> Does this mean the driver needs to be involved when we take a device out of
> D3 (where the link was in L2 or L3)?

Yes, the SW should be involved when bringing the link out of L2 or L3.
I looked through the SPEC, didn't find that they are forbidden by SPEC.
It might be a design limitation, I think.

Best Regards
Richard Zhu

> 
> Bjorn

RE: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2022年2月11日 6:05
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> support
> 
> On Thu, Feb 10, 2022 at 03:23:19AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > Sent: 2022年2月9日 23:37
> > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > > lorenzo.pieralisi@arm.com; shawnguo@kernel.org;
> > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> > > <linux-imx@nxp.com>
> > > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power
> > > management support
> > >
> > > On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> > > > i.MX6QP PCIe supports the RESET logic, thus it can support the L2
> > > > exit by the reset mechanism.
> > > > Enable the i.MX6QP PCIe suspend/resume operations support.
> 
> > > What does "L2 exit by reset mechanism" mean?  Is this an
> > > i.MX6-specific thing?  If not, can you point me to the relevant part
> > > of the PCIe spec?
> >
> > No, it's not i.MX6 specific thing. i.MX6Q/DL doesn't have the
> > self-reset mechanism.  Thus, it can't reset itself to an initialized
> > stat when link exit from the L2 or L3 stats.  i.MX6QP PCIe has the
> > self-reset mechanism, and it can reset itself when link exit from L2
> > or L3 stats.  The commit description might not accurate.  How about
> > change them to "i.MX6QP PCIe supports the RESET logic, thus it can
> > reset itself to the initialized stat when exit from L2 or L3 stats."
> 
> s/stat/state/
Thanks for your quickly reply.
Got that, would be changed later.
> 
> Ugh, I have all sorts of questions now, but I don't think I want to know much
> more about this ;)
> 
> Seems like this device requires software assist when bringing the link out of L2
> or L3.  Is that allowed per PCIe spec, or is this an erratum?
> 
> Does this mean the driver needs to be involved when we take a device out of
> D3 (where the link was in L2 or L3)?

Yes, the SW should be involved when bringing the link out of L2 or L3.
I looked through the SPEC, didn't find that they are forbidden by SPEC.
It might be a design limitation, I think.

Best Regards
Richard Zhu

> 
> Bjorn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Bjorn Helgaas]

[+cc Rafael, beginning of thread:
https://lore.kernel.org/r/1644390156-5940-2-git-send-email-hongxing.zhu@nxp.com]

On Fri, Feb 11, 2022 at 02:05:24AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: 2022年2月11日 6:05
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > lorenzo.pieralisi@arm.com; shawnguo@kernel.org; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>
> > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> > support
> > 
> > On Thu, Feb 10, 2022 at 03:23:19AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > > Sent: 2022年2月9日 23:37
> > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > > > lorenzo.pieralisi@arm.com; shawnguo@kernel.org;
> > > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> > > > <linux-imx@nxp.com>
> > > > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power
> > > > management support
> > > >
> > > > On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> > > > > i.MX6QP PCIe supports the RESET logic, thus it can support the L2
> > > > > exit by the reset mechanism.
> > > > > Enable the i.MX6QP PCIe suspend/resume operations support.
> > 
> > > > What does "L2 exit by reset mechanism" mean?  Is this an
> > > > i.MX6-specific thing?  If not, can you point me to the relevant part
> > > > of the PCIe spec?
> > >
> > > No, it's not i.MX6 specific thing. i.MX6Q/DL doesn't have the
> > > self-reset mechanism.  Thus, it can't reset itself to an initialized
> > > stat when link exit from the L2 or L3 stats.  i.MX6QP PCIe has the
> > > self-reset mechanism, and it can reset itself when link exit from L2
> > > or L3 stats.  The commit description might not accurate.  How about
> > > change them to "i.MX6QP PCIe supports the RESET logic, thus it can
> > > reset itself to the initialized stat when exit from L2 or L3 stats."
> > 
> > Ugh, I have all sorts of questions now, but I don't think I want
> > to know much more about this ;)
> > 
> > Seems like this device requires software assist when bringing the
> > link out of L2 or L3.  Is that allowed per PCIe spec, or is this
> > an erratum?
> > 
> > Does this mean the driver needs to be involved when we take a
> > device out of D3 (where the link was in L2 or L3)?
> 
> Yes, the SW should be involved when bringing the link out of L2 or
> L3.  I looked through the SPEC, didn't find that they are forbidden
> by SPEC.  It might be a design limitation, I think.

OK.  I don't understand all the details of L2 and L3, so I'll take
your word for it that this is allowed by spec.

The case I was wondering about is when software puts a device in
D3hot, which can be done via pci_set_power_state(), without any help
from the imx6 driver.  What state will the link be in, and can we also
put the device back in D0 without help from imx6?

Sec 5.3.2 says L1 is a permissible link state for D3hot, so maybe L2
and L3 are not involved in this case.

Bjorn

Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Bjorn Helgaas]

[+cc Rafael, beginning of thread:
https://lore.kernel.org/r/1644390156-5940-2-git-send-email-hongxing.zhu@nxp.com]

On Fri, Feb 11, 2022 at 02:05:24AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: 2022年2月11日 6:05
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > lorenzo.pieralisi@arm.com; shawnguo@kernel.org; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>
> > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> > support
> > 
> > On Thu, Feb 10, 2022 at 03:23:19AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > > Sent: 2022年2月9日 23:37
> > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > > > lorenzo.pieralisi@arm.com; shawnguo@kernel.org;
> > > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> > > > <linux-imx@nxp.com>
> > > > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power
> > > > management support
> > > >
> > > > On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> > > > > i.MX6QP PCIe supports the RESET logic, thus it can support the L2
> > > > > exit by the reset mechanism.
> > > > > Enable the i.MX6QP PCIe suspend/resume operations support.
> > 
> > > > What does "L2 exit by reset mechanism" mean?  Is this an
> > > > i.MX6-specific thing?  If not, can you point me to the relevant part
> > > > of the PCIe spec?
> > >
> > > No, it's not i.MX6 specific thing. i.MX6Q/DL doesn't have the
> > > self-reset mechanism.  Thus, it can't reset itself to an initialized
> > > stat when link exit from the L2 or L3 stats.  i.MX6QP PCIe has the
> > > self-reset mechanism, and it can reset itself when link exit from L2
> > > or L3 stats.  The commit description might not accurate.  How about
> > > change them to "i.MX6QP PCIe supports the RESET logic, thus it can
> > > reset itself to the initialized stat when exit from L2 or L3 stats."
> > 
> > Ugh, I have all sorts of questions now, but I don't think I want
> > to know much more about this ;)
> > 
> > Seems like this device requires software assist when bringing the
> > link out of L2 or L3.  Is that allowed per PCIe spec, or is this
> > an erratum?
> > 
> > Does this mean the driver needs to be involved when we take a
> > device out of D3 (where the link was in L2 or L3)?
> 
> Yes, the SW should be involved when bringing the link out of L2 or
> L3.  I looked through the SPEC, didn't find that they are forbidden
> by SPEC.  It might be a design limitation, I think.

OK.  I don't understand all the details of L2 and L3, so I'll take
your word for it that this is allowed by spec.

The case I was wondering about is when software puts a device in
D3hot, which can be done via pci_set_power_state(), without any help
from the imx6 driver.  What state will the link be in, and can we also
put the device back in D0 without help from imx6?

Sec 5.3.2 says L1 is a permissible link state for D3hot, so maybe L2
and L3 are not involved in this case.

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2022年2月12日 0:53
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki
> <rjw@rjwysocki.net>
> Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> support
> 
> [+cc Rafael, beginning of thread:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Fr%2F1644390156-5940-2-git-send-email-hongxing.zhu%40nxp.com
> &amp;data=04%7C01%7Chongxing.zhu%40nxp.com%7Ca40c2a3d7c03497f8c
> f208d9ed7f0463%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> 7801952076367918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=
> BnzoSbb92HEw%2BQAMspyCT72787aFUhmPiZX2ZVAztJo%3D&amp;reserved
> =0]
> 
> On Fri, Feb 11, 2022 at 02:05:24AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > Sent: 2022年2月11日 6:05
> > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > > lorenzo.pieralisi@arm.com; shawnguo@kernel.org;
> > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> > > <linux-imx@nxp.com>
> > > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power
> > > management support
> > >
> > > On Thu, Feb 10, 2022 at 03:23:19AM +0000, Hongxing Zhu wrote:
> > > > > -----Original Message-----
> > > > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > > > Sent: 2022年2月9日 23:37
> > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > > > > lorenzo.pieralisi@arm.com; shawnguo@kernel.org;
> > > > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de;
> > > > > dl-linux-imx <linux-imx@nxp.com>
> > > > > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power
> > > > > management support
> > > > >
> > > > > On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> > > > > > i.MX6QP PCIe supports the RESET logic, thus it can support the
> > > > > > L2 exit by the reset mechanism.
> > > > > > Enable the i.MX6QP PCIe suspend/resume operations support.
> > >
> > > > > What does "L2 exit by reset mechanism" mean?  Is this an
> > > > > i.MX6-specific thing?  If not, can you point me to the relevant
> > > > > part of the PCIe spec?
> > > >
> > > > No, it's not i.MX6 specific thing. i.MX6Q/DL doesn't have the
> > > > self-reset mechanism.  Thus, it can't reset itself to an
> > > > initialized stat when link exit from the L2 or L3 stats.  i.MX6QP
> > > > PCIe has the self-reset mechanism, and it can reset itself when
> > > > link exit from L2 or L3 stats.  The commit description might not
> > > > accurate.  How about change them to "i.MX6QP PCIe supports the
> > > > RESET logic, thus it can reset itself to the initialized stat when exit from L2
> or L3 stats."
> > >
> > > Ugh, I have all sorts of questions now, but I don't think I want to
> > > know much more about this ;)
> > >
> > > Seems like this device requires software assist when bringing the
> > > link out of L2 or L3.  Is that allowed per PCIe spec, or is this an
> > > erratum?
> > >
> > > Does this mean the driver needs to be involved when we take a device
> > > out of D3 (where the link was in L2 or L3)?
> >
> > Yes, the SW should be involved when bringing the link out of L2 or L3.
> > I looked through the SPEC, didn't find that they are forbidden by
> > SPEC.  It might be a design limitation, I think.
> 
> OK.  I don't understand all the details of L2 and L3, so I'll take your word for it
> that this is allowed by spec.
> 
> The case I was wondering about is when software puts a device in D3hot,
> which can be done via pci_set_power_state(), without any help from the imx6
> driver.  What state will the link be in, and can we also put the device back in
> D0 without help from imx6?
Hi Bjorn:
Regarding my understand, i.MX6QP PCIe would broadcast the PME message,
 handshake with remote EP, let link state to be L1 or L2/L3. Then, the EP
 device driver would put the device into D3 mode accordingly.

Since i.MX PCIe doesn't support the L2 and the beacon to exit the L2 link
 power state. The reset and link retraining would be done to re-setup the
 link when exit the L3 link power state.

Best Regards
Richard
> 
> Sec 5.3.2 says L1 is a permissible link state for D3hot, so maybe L2 and L3 are
> not involved in this case.
> 
> Bjorn

RE: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management support

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2022年2月12日 0:53
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; shawnguo@kernel.org; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki
> <rjw@rjwysocki.net>
> Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power management
> support
> 
> [+cc Rafael, beginning of thread:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Fr%2F1644390156-5940-2-git-send-email-hongxing.zhu%40nxp.com
> &amp;data=04%7C01%7Chongxing.zhu%40nxp.com%7Ca40c2a3d7c03497f8c
> f208d9ed7f0463%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> 7801952076367918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=
> BnzoSbb92HEw%2BQAMspyCT72787aFUhmPiZX2ZVAztJo%3D&amp;reserved
> =0]
> 
> On Fri, Feb 11, 2022 at 02:05:24AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > Sent: 2022年2月11日 6:05
> > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > > lorenzo.pieralisi@arm.com; shawnguo@kernel.org;
> > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> > > <linux-imx@nxp.com>
> > > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power
> > > management support
> > >
> > > On Thu, Feb 10, 2022 at 03:23:19AM +0000, Hongxing Zhu wrote:
> > > > > -----Original Message-----
> > > > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > > > Sent: 2022年2月9日 23:37
> > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > > > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > > > > lorenzo.pieralisi@arm.com; shawnguo@kernel.org;
> > > > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > > linux-kernel@vger.kernel.org; kernel@pengutronix.de;
> > > > > dl-linux-imx <linux-imx@nxp.com>
> > > > > Subject: Re: [RFC 2/2] PCI: imx6: Enable imx6qp pcie power
> > > > > management support
> > > > >
> > > > > On Wed, Feb 09, 2022 at 03:02:36PM +0800, Richard Zhu wrote:
> > > > > > i.MX6QP PCIe supports the RESET logic, thus it can support the
> > > > > > L2 exit by the reset mechanism.
> > > > > > Enable the i.MX6QP PCIe suspend/resume operations support.
> > >
> > > > > What does "L2 exit by reset mechanism" mean?  Is this an
> > > > > i.MX6-specific thing?  If not, can you point me to the relevant
> > > > > part of the PCIe spec?
> > > >
> > > > No, it's not i.MX6 specific thing. i.MX6Q/DL doesn't have the
> > > > self-reset mechanism.  Thus, it can't reset itself to an
> > > > initialized stat when link exit from the L2 or L3 stats.  i.MX6QP
> > > > PCIe has the self-reset mechanism, and it can reset itself when
> > > > link exit from L2 or L3 stats.  The commit description might not
> > > > accurate.  How about change them to "i.MX6QP PCIe supports the
> > > > RESET logic, thus it can reset itself to the initialized stat when exit from L2
> or L3 stats."
> > >
> > > Ugh, I have all sorts of questions now, but I don't think I want to
> > > know much more about this ;)
> > >
> > > Seems like this device requires software assist when bringing the
> > > link out of L2 or L3.  Is that allowed per PCIe spec, or is this an
> > > erratum?
> > >
> > > Does this mean the driver needs to be involved when we take a device
> > > out of D3 (where the link was in L2 or L3)?
> >
> > Yes, the SW should be involved when bringing the link out of L2 or L3.
> > I looked through the SPEC, didn't find that they are forbidden by
> > SPEC.  It might be a design limitation, I think.
> 
> OK.  I don't understand all the details of L2 and L3, so I'll take your word for it
> that this is allowed by spec.
> 
> The case I was wondering about is when software puts a device in D3hot,
> which can be done via pci_set_power_state(), without any help from the imx6
> driver.  What state will the link be in, and can we also put the device back in
> D0 without help from imx6?
Hi Bjorn:
Regarding my understand, i.MX6QP PCIe would broadcast the PME message,
 handshake with remote EP, let link state to be L1 or L2/L3. Then, the EP
 device driver would put the device into D3 mode accordingly.

Since i.MX PCIe doesn't support the L2 and the beacon to exit the L2 link
 power state. The reset and link retraining would be done to re-setup the
 link when exit the L3 link power state.

Best Regards
Richard
> 
> Sec 5.3.2 says L1 is a permissible link state for D3hot, so maybe L2 and L3 are
> not involved in this case.
> 
> Bjorn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel