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

[Hongxing Zhu <hongxing.zhu@nxp.com>]

> -----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 {
>