On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@xxxxxxx> wrote: > > Some boards do not have a PCIe reset GPIO. To avoid probe > failure on these boards, make the reset GPIO optional as > well. > > Signed-off-by: Marek Vasut <marex@xxxxxxx> > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > Cc: Frank Li <lznuaa@xxxxxxxxx> > Cc: Harro Haan <hrhaan@xxxxxxxxx> > Cc: Jingoo Han <jg1.han@xxxxxxxxxxx> > Cc: Mohit KUMAR <Mohit.KUMAR@xxxxxx> > Cc: Pratyush Anand <pratyush.anand@xxxxxx> > Cc: Richard Zhu <r65037@xxxxxxxxxxxxx> > Cc: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > Cc: Sean Cross <xobs@xxxxxxxxxx> > Cc: Shawn Guo <shawn.guo@xxxxxxxxxx> > Cc: Siva Reddy Kallam <siva.kallam@xxxxxxxxxxx> > Cc: Srikanth T Shivanand <ts.srikanth@xxxxxxxxxxx> > Cc: Tim Harvey <tharvey@xxxxxxxxxxxxx> > Cc: Troy Kisky <troy.kisky@xxxxxxxxxxxxxxxxxxx> > Cc: Yinghai Lu <yinghai@xxxxxxxxxx> > --- > .../devicetree/bindings/pci/designware-pcie.txt | 2 +- > drivers/pci/host/pci-imx6.c | 29 +++++++++++----------- > 2 files changed, 16 insertions(+), 15 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt > index d5d26d4..b7a2279 100644 > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt > @@ -19,9 +19,9 @@ Required properties: > to define the mapping of the PCIe interface to interrupt > numbers. > - num-lanes: number of lanes to use > -- reset-gpio: gpio pin number of power good signal > > Optional properties for fsl,imx6q-pcie > +- reset-gpio: gpio pin number of power good signal > - power-on-gpio: gpio pin number of power-enable signal > - wake-up-gpio: gpio pin number of incoming wakeup signal > - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable signal > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index bd70af8..52027ad 100644 > --- a/drivers/pci/host/pci-imx6.c > +++ b/drivers/pci/host/pci-imx6.c > @@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp) > regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16); > > - gpio_set_value(imx6_pcie->reset_gpio, 0); > - msleep(100); > - gpio_set_value(imx6_pcie->reset_gpio, 1); > + /* Some boards don't have PCIe reset GPIO. */ > + if (gpio_is_valid(imx6_pcie->reset_gpio)) { > + gpio_set_value(imx6_pcie->reset_gpio, 0); > + msleep(100); > + gpio_set_value(imx6_pcie->reset_gpio, 1); > + } > > return 0; > } Marek, Though not the fault of your patch, I noticed while looking at this that the PCI Express specification is not being properly met with regards to PERST# and the reference clock. The spec states that PERST# must be kept asserted until after the reference clock is stable (I'm not entirely clear how long of a delay is needed for the clock to become stable but I think the value is typically the 100ms). I see in the current pci-imx6.c code that imx6_pcie_host_init calls imx6_pcie_assert_core_reset first, then imx6_pcie_init_phy, followed by imx6_pcie_deassert_core_reset. Despite the function names, imx6_pcie_assert_core_reset as shown above asserts then de-asserts PERST# before the clock is enabled in imx6_pcie_deassert_core_reset. This seems to me to be a violation of the spec and I believe the msleep(100) and de-assertion of the option reset_gpio should be done in imx6_pcie_deassert_core reset after the clock is brought up. If you agree with my assessment, would you mind resolving this issue at the same time? If not, I'm happy to follow-up with a patch to resolve it after your patch is accepted. Thanks, Tim > @@ -439,17 +442,15 @@ static int __init imx6_pcie_probe(struct platform_device *pdev) > > /* Fetch GPIOs */ > imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0); > - if (!gpio_is_valid(imx6_pcie->reset_gpio)) { > - dev_err(&pdev->dev, "no reset-gpio defined\n"); > - ret = -ENODEV; > - } > - ret = devm_gpio_request_one(&pdev->dev, > - imx6_pcie->reset_gpio, > - GPIOF_OUT_INIT_LOW, > - "PCIe reset"); > - if (ret) { > - dev_err(&pdev->dev, "unable to get reset gpio\n"); > - goto err; > + if (gpio_is_valid(imx6_pcie->reset_gpio)) { > + ret = devm_gpio_request_one(&pdev->dev, > + imx6_pcie->reset_gpio, > + GPIOF_OUT_INIT_LOW, > + "PCIe reset"); > + if (ret) { > + dev_err(&pdev->dev, "unable to get reset gpio\n"); > + goto err; > + } > } > > imx6_pcie->power_on_gpio = of_get_named_gpio(np, "power-on-gpio", 0); > -- > 1.8.4.3 > -- 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