[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1782110.RW4F1b4ZIY@en-pc05>
Date: Tue, 13 Jun 2017 05:55:09 +0000
From: Schöfegger Stefan
<Stefan.Schoefegger@...zinger.com>
To: Richard Zhu <hongxing.zhu@....com>
CC: Bjorn Helgaas <helgaas@...nel.org>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
Arnd Bergmann <arnd@...db.de>,
open list <linux-kernel@...r.kernel.org>,
Kishon Vijay Abraham I <kishon@...com>,
Jingoo Han <jingoohan1@...il.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
"moderated list:PCI DRIVER FOR IMX6"
<linux-arm-kernel@...ts.infradead.org>,
Lucas Stach <l.stach@...gutronix.de>
Subject: Re: [PATCH v2 1/1] PCI: imx6: Add pcie compliance test option
On Tuesday, June 13, 2017 2:00:16 AM CEST Richard Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas [mailto:helgaas@...nel.org]
> > Sent: Tuesday, June 13, 2017 7:49 AM
> > To: Stefan Schoefegger <stefan.schoefegger@...zinger.com>
> > Cc: linux-pci@...r.kernel.org; Richard Zhu <hongxing.zhu@....com>; Arnd
> > Bergmann <arnd@...db.de>; open list <linux-kernel@...r.kernel.org>;
> > Kishon Vijay Abraham I <kishon@...com>; Jingoo Han
> > <jingoohan1@...il.com>; Bjorn Helgaas <bhelgaas@...gle.com>;
> > moderated list:PCI DRIVER FOR IMX6 <linux-arm-kernel@...ts.infradead.org>;
> > Lucas Stach <l.stach@...gutronix.de>
> > Subject: Re: [PATCH v2 1/1] PCI: imx6: Add pcie compliance test option
> >
> > On Wed, Jun 07, 2017 at 01:36:11PM +0200, Stefan Schoefegger wrote:
> > > Link speed must not be limited to gen1 during link test for compliance
> > > tests
> > >
> > > Signed-off-by: Stefan Schoefegger <stefan.schoefegger@...zinger.com>
> > > ---
> > >
> > > Changes since v1:
> > > - pci-imx6.c moved to dwc directory
> > >
> > > drivers/pci/dwc/Kconfig | 10 ++++++++++
> > > drivers/pci/dwc/pci-imx6.c | 21 ++++++++++++---------
> > > 2 files changed, 22 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig index
> > > b7e15526d676..b6e9ced5a45d 100644
> > > --- a/drivers/pci/dwc/Kconfig
> > > +++ b/drivers/pci/dwc/Kconfig
> > > @@ -77,6 +77,16 @@ config PCI_IMX6
> > >
> > > select PCIEPORTBUS
> > > select PCIE_DW_HOST
> > >
> > > +config PCI_IMX6_COMPLIANCE_TEST
> > > + bool "Enable pcie compliance tests on imx6"
> > > + depends on PCI_IMX6
> > > + default n
> > > + help
> > > + Enables support for pcie compliance test on FSL iMX SoCs.
> > > + The link speed wouldn't be limited to gen1 when enabled.
> > > + Enable only during compliance tests, otherwise
> > > + link detection will fail on some peripherals.
> >
> > I'm puzzled about why we would want to merge this patch. It looks like
> > we're trying to game the system to make the device pass compliance testing
> > when it isn't really compliant. Is this config option useful to users, or
> > is it only useful during internal development of iMX SoCs?
>
> [Zhu hongxing] Agree with Bjorn. These modifications shouldn't be merged.
> They are used for the boards used to pass the PCIe RC certification, when
> one Standalone external OSC is used as PCIe reference clock source in the
> boards design.
Yes, passing gen2 compliance test is only possible with external clk. But this
is a errata of imx6 pci phy of PCIe clk jitter. Why should we not be able to
do gen2 tests? I think it is useful to do other gen2 tests (signal integrity)
during board design.
> > > config PCIE_SPEAR13XX
> > >
> > > bool "STMicroelectronics SPEAr PCIe controller"
> > > depends on PCI
> > >
> > > diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> > > index 19a289b8cc94..b0fbe52e25b0 100644
> > > --- a/drivers/pci/dwc/pci-imx6.c
> > > +++ b/drivers/pci/dwc/pci-imx6.c
> > > @@ -533,15 +533,18 @@ static int imx6_pcie_establish_link(struct
> >
> > imx6_pcie *imx6_pcie)
> >
> > > u32 tmp;
> > > int ret;
> > >
> > > - /*
> > > - * Force Gen1 operation when starting the link. In case the link is
> > > - * started in Gen2 mode, there is a possibility the devices on the
> > > - * bus will not be detected at all. This happens with PCIe switches.
> > > - */
> > > - tmp = dw_pcie_readl_dbi(pci, PCIE_RC_LCR);
> > > - tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
> > > - tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1;
> > > - dw_pcie_writel_dbi(pci, PCIE_RC_LCR, tmp);
> > > + if (!IS_ENABLED(CONFIG_PCI_IMX6_COMPLIANCE_TEST)) {
> > > + /*
> > > + * Force Gen1 operation when starting the link. In case the
> > > + * link is started in Gen2 mode, there is a possibility the
> > > + * devices on the bus will not be detected at all. This
> > > + * happens with PCIe switches.
> > > + */
> > > + tmp = dw_pcie_readl_dbi(pci, PCIE_RC_LCR);
> > > + tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
> > > + tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1;
> > > + dw_pcie_writel_dbi(pci, PCIE_RC_LCR, tmp);
> > > + }
> > >
> > > /* Start LTSSM. */
> > > if (imx6_pcie->variant == IMX7D)
> > >
> > > --
> > > 2.11.0
> > >
> > >
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@...ts.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Powered by blists - more mailing lists