Am Dienstag, den 05.04.2016, 09:29 -0700 schrieb Tim Harvey: > Freescale has stated [1] that the LVDS clock source of the IMX6 does not pass > the PCI Gen2 clock jitter test, therefore unless an external Gen2 compliant > external clock source is present and supplied back to the IMX6 PCIe core > via LVDS CLK1/CLK2 you can not claim Gen2 compliance. > > Add a dt property to specify gen1 vs gen2 and check this before allowing > a Gen2 link. > > We default to Gen1 if the property is not present because at this time there > are no IMX6 boards in mainline that 'input' a clock on LVDS CLK1/CLK2. > > In order to be Gen2 compliant on IMX6 you need to: > - have a Gen2 compliant external clock generator and route that clock back > to either LVDS CLK1 or LVDS CLK2 as an input. > (see IMX6SX-SabreSD reference design) > - specify this clock in the pcie node in the dt > (ie IMX6QDL_CLK_LVDS1_IN or IMX6QDL_CLK_LVDS2_IN instead of > IMX6QDL_CLK_LVDS1_GATE which configures it as a CLK output) > > [1] https://community.freescale.com/message/453209 > > Cc: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > Cc: Bjorn Helgaas <helgaas@xxxxxxxxxx> > Cc: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx> > Cc: Zhu Richard <Richard.Zhu@xxxxxxxxxxxxx> > Cc: Akshay Bhat <akshay.bhat@xxxxxxxxxxx> > Cc: Rob Herring <robh+dt@xxxxxxxxxx> > Cc: Shawn Guo <shawnguo@xxxxxxxxxx> > Signed-off-by: Tim Harvey <tharvey@xxxxxxxxxxxxx> One nit below. Maybe Bjorn can fix this up while applying, otherwise if you need to resend you may carry the blow tag. Reviewed-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > --- > v4: > - rebase against linux-pci/master > - add fsl vendor prefix to dt prop > > v3: > - added note in dt bindings doc that we limit to gen1 unless this is specified > as gen2 capable > - move property to imx6 pcie phy instead of designware core > - don't use &ret as temp storage as of_property_read_u32() doesn't change the > outval if property isn't found > v2: > - moved dt property to designware core > > Signed-off-by: Tim Harvey <tharvey@xxxxxxxxxxxxx> > --- > .../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 4 ++++ > drivers/pci/host/pci-imx6.c | 24 +++++++++++++++------- > 2 files changed, 21 insertions(+), 7 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt > index 3be80c6..b92fafb 100644 > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt > @@ -19,6 +19,10 @@ Optional properties: > - fsl,tx-deemph-gen2-6db: Gen2 (6db) De-emphasis value. Default: 20 > - fsl,tx-swing-full: Gen2 TX SWING FULL value. Default: 127 > - fsl,tx-swing-low: TX launch amplitude swing_low value. Default: 127 > +- fsl,max-link-speed: Specify PCI gen for link capability. Must be '2' for > + gen2, otherwise will default to gen1. Note that the IMX6 LVDS clock outputs > + do not meet gen2 jitter requirements and thus for gen2 capability a gen2 > + compliant clock generator should be used and configured. > > Example: > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index eb5a275..9656ac7 100644 > --- a/drivers/pci/host/pci-imx6.c > +++ b/drivers/pci/host/pci-imx6.c > @@ -44,6 +44,7 @@ struct imx6_pcie { > u32 tx_deemph_gen2_6db; > u32 tx_swing_full; > u32 tx_swing_low; > + int link_gen; > }; > > /* PCIe Root Complex registers (memory-mapped) */ > @@ -417,11 +418,15 @@ static int imx6_pcie_establish_link(struct pcie_port *pp) > goto err_reset_phy; > } > > - /* Allow Gen2 mode after the link is up. */ > - tmp = readl(pp->dbi_base + PCIE_RC_LCR); > - tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK; > - tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN2; > - writel(tmp, pp->dbi_base + PCIE_RC_LCR); > + if (imx6_pcie->link_gen == 2) { > + /* Allow Gen2 mode after the link is up. */ > + tmp = readl(pp->dbi_base + PCIE_RC_LCR); > + tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK; > + tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN2; > + writel(tmp, pp->dbi_base + PCIE_RC_LCR); > + } else { > + dev_info(pp->dev, "Link: Gen2 disabled\n"); > + } > > /* > * Start Directed Speed Change so the best possible speed both link > @@ -445,8 +450,7 @@ static int imx6_pcie_establish_link(struct pcie_port *pp) > } > > tmp = readl(pp->dbi_base + PCIE_RC_LCSR); > - dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf); > - > + dev_info(pp->dev, "Link up, Gen%i\n", (tmp >> 16) & 0xf); > return 0; > > err_reset_phy: > @@ -598,6 +602,12 @@ static int __init imx6_pcie_probe(struct platform_device *pdev) > &imx6_pcie->tx_swing_low)) > imx6_pcie->tx_swing_low = 127; > > + /* Limit link speed */ > + ret = of_property_read_u32(pp->dev->of_node, "fsl,max-link-speed", > + &imx6_pcie->link_gen); > + if (ret) > + imx6_pcie->link_gen = -1; The binding specifies that if the property is absent, it will default to Gen1, so I would like to see this reflected here by setting the link_gen to 1, not -1. > + > ret = imx6_add_pcie_port(pp, pdev); > if (ret < 0) > return ret; -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
