diff mbox

[1/7] PCI: imx6: Make reset-gpio optional

Message ID 1386757818-5154-1-git-send-email-marex@denx.de
State Superseded
Headers show

Commit Message

Marek Vasut Dec. 11, 2013, 10:30 a.m. UTC
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@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
 drivers/pci/host/pci-imx6.c                        | 29 +++++++++++-----------
 2 files changed, 16 insertions(+), 15 deletions(-)

Comments

Tim Harvey Dec. 12, 2013, 5:10 a.m. UTC | #1
On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> 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@denx.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Frank Li <lznuaa@gmail.com>
> Cc: Harro Haan <hrhaan@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> Cc: Pratyush Anand <pratyush.anand@st.com>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Yinghai Lu <yinghai@kernel.org>
> ---
>  .../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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut Dec. 12, 2013, 10:22 a.m. UTC | #2
On Thursday, December 12, 2013 at 06:10:31 AM, Tim Harvey wrote:
> On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> 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@denx.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Frank Li <lznuaa@gmail.com>
> > Cc: Harro Haan <hrhaan@gmail.com>
> > Cc: Jingoo Han <jg1.han@samsung.com>
> > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> > Cc: Pratyush Anand <pratyush.anand@st.com>
> > Cc: Richard Zhu <r65037@freescale.com>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Sean Cross <xobs@kosagi.com>
> > Cc: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> > Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> > Cc: Tim Harvey <tharvey@gateworks.com>
> > Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> > Cc: Yinghai Lu <yinghai@kernel.org>
> > ---
> > 
> >  .../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.

Is this not resolved by patch 0006 in this series please?
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tim Harvey Dec. 12, 2013, 6:16 p.m. UTC | #3
On Thu, Dec 12, 2013 at 2:22 AM, Marek Vasut <marex@denx.de> wrote:
>
> On Thursday, December 12, 2013 at 06:10:31 AM, Tim Harvey wrote:
> > On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> 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@denx.de>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: Frank Li <lznuaa@gmail.com>
> > > Cc: Harro Haan <hrhaan@gmail.com>
> > > Cc: Jingoo Han <jg1.han@samsung.com>
> > > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> > > Cc: Pratyush Anand <pratyush.anand@st.com>
> > > Cc: Richard Zhu <r65037@freescale.com>
> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Sean Cross <xobs@kosagi.com>
> > > Cc: Shawn Guo <shawn.guo@linaro.org>
> > > Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> > > Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> > > Cc: Tim Harvey <tharvey@gateworks.com>
> > > Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> > > Cc: Yinghai Lu <yinghai@kernel.org>
> > > ---
> > >
> > >  .../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.
>
> Is this not resolved by patch 0006 in this series please?

Marek,

Yes it is addressed there.  Sorry - I missed that.

Tim
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut Dec. 12, 2013, 6:25 p.m. UTC | #4
On Thursday, December 12, 2013 at 07:16:18 PM, Tim Harvey wrote:
> On Thu, Dec 12, 2013 at 2:22 AM, Marek Vasut <marex@denx.de> wrote:
> > On Thursday, December 12, 2013 at 06:10:31 AM, Tim Harvey wrote:
> > > On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> 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@denx.de>
> > > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > > Cc: Frank Li <lznuaa@gmail.com>
> > > > Cc: Harro Haan <hrhaan@gmail.com>
> > > > Cc: Jingoo Han <jg1.han@samsung.com>
> > > > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> > > > Cc: Pratyush Anand <pratyush.anand@st.com>
> > > > Cc: Richard Zhu <r65037@freescale.com>
> > > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Cc: Sean Cross <xobs@kosagi.com>
> > > > Cc: Shawn Guo <shawn.guo@linaro.org>
> > > > Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> > > > Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> > > > Cc: Tim Harvey <tharvey@gateworks.com>
> > > > Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> > > > Cc: Yinghai Lu <yinghai@kernel.org>
> > > > ---
> > > > 
> > > >  .../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.
> > 
> > Is this not resolved by patch 0006 in this series please?
> 
> Marek,
> 
> Yes it is addressed there.  Sorry - I missed that.

No problem, thanks for checking it :)

Bjorn, would you mind picking 2/7...6/7 of this series so we are done with 
those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact 
on the rest of the series.

Shawn, can you please pick 7/7 ?
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas Dec. 12, 2013, 9:07 p.m. UTC | #5
On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
> Bjorn, would you mind picking 2/7...6/7 of this series so we are done with
> those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact
> on the rest of the series.

Sure.  I can't really evaluate these myself, so I'm looking for acks
from Richard or Shawn.

I noticed Pratyush, Tim, and Harro had some questions the first time
around, and I don't know whether they got resolved.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas Dec. 12, 2013, 9:20 p.m. UTC | #6
On Thu, Dec 12, 2013 at 2:07 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
>> Bjorn, would you mind picking 2/7...6/7 of this series so we are done with
>> those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact
>> on the rest of the series.
>
> Sure.  I can't really evaluate these myself, so I'm looking for acks
> from Richard or Shawn.
>
> I noticed Pratyush, Tim, and Harro had some questions the first time
> around, and I don't know whether they got resolved.

By the way, I tried to apply patches 2-6 to my pci/host-imx6 branch,
but patch 6 ("PCI: imx6: Fix bugs in PCIe startup code") didn't apply
cleanly.

I pushed the branch to
http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-imx6
if you want to try it yourself.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut Dec. 12, 2013, 9:38 p.m. UTC | #7
On Thursday, December 12, 2013 at 10:20:30 PM, Bjorn Helgaas wrote:
> On Thu, Dec 12, 2013 at 2:07 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
> >> Bjorn, would you mind picking 2/7...6/7 of this series so we are done
> >> with those? I will re-post 1/7 as a self-standing patch afterwards, it
> >> has no impact on the rest of the series.
> > 
> > Sure.  I can't really evaluate these myself, so I'm looking for acks
> > from Richard or Shawn.
> > 
> > I noticed Pratyush, Tim, and Harro had some questions the first time
> > around, and I don't know whether they got resolved.
> 
> By the way, I tried to apply patches 2-6 to my pci/host-imx6 branch,
> but patch 6 ("PCI: imx6: Fix bugs in PCIe startup code") didn't apply
> cleanly.
> 
> I pushed the branch to
> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host
> -imx6 if you want to try it yourself.

That's because of Fabio's patch. I will rebase and repost, thanks.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Harro Haan Dec. 12, 2013, 10:12 p.m. UTC | #8
On 12 December 2013 22:07, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
>> Bjorn, would you mind picking 2/7...6/7 of this series so we are done with
>> those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact
>> on the rest of the series.
>
> Sure.  I can't really evaluate these myself, so I'm looking for acks
> from Richard or Shawn.
>
> I noticed Pratyush, Tim, and Harro had some questions the first time
> around, and I don't know whether they got resolved.

Yes, Marek resolved the build warning comment in [PATCH V2 3/7]. Regards, Harro
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;
 }
@@ -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);