From: khalasa@piap.pl (Krzysztof Hałasa)
To: "Petr Štetiar" <ynezz@true.cz>,
"Richard Zhu" <Richard.Zhu@freescale.com>,
"Lucas Stach" <l.stach@pengutronix.de>,
"Bjorn Helgaas" <bhelgaas@google.com>,
linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: [PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity
Date: Fri, 25 Mar 2016 14:32:35 +0100 [thread overview]
Message-ID: <m3h9fuk7ik.fsf@t19.piap.pl> (raw)
A recent commit 5c5fb40de8f14391a1238db05cef88754faf9229 stated:
Follows: linus/v4.4-rc2
Precedes: linus/v4.5-rc1
PCI: imx6: Add support for active-low reset GPIO
We previously used of_get_named_gpio(), which ignores the OF flags cell, so
the reset GPIO defaulted to "active high." This doesn't work on the Toradex
Apalis SoM with Ixora base board, which has an active-low reset GPIO.
Use devm_gpiod_get_optional() instead so we pay attention to the active
high/low flag. This also adds support for GPIOs described via ACPI.
The (now replaced) code doesn't support the above:
@@ -287,10 +287,10 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
usleep_range(200, 500);
/* Some boards don't have PCIe reset GPIO. */
- if (gpio_is_valid(imx6_pcie->reset_gpio)) {
- gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
+ if (imx6_pcie->reset_gpio) {
+ gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
msleep(100);
- gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
+ gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
}
return 0;
If the reset_gpio setup code had ignored the flags (haven't checked
that), then clearly the resets were active-low (most reset lines are,
because they can be then driven with open-drain/collector output).
The gpiod_set_value*(0) activates reset, gpiod_set_value(1) -
deactivates.
Now we're told the setup code is now level-aware, but the above sequence
thus _deactivates_ reset for 100 ms, then _activates_ it again. It has
no chance to work, unless a board has a broken DTS file. A quick grep
shows that about half the IMX6 boards specify an active-low PCIe reset,
4 ask for active-high, and another 4 don't bother.
I wonder if all boards (except maybe that Toradex set) use an active-low
PCIe reset and are now broken. Perhaps Toradex uses active-high and thus
works.
I'm not fixing individual DTS files because I don't really know, though
perhaps we should change them all to "active-low", since it would work
the same as before the 5c5fb40de8f14391a1238db05cef88754faf9229 change.
Confirmed to fix Gateworks Laguna GW54xx.
Without the patch, the following happens (as expected):
PCI host bridge /soc/pcie@0x01000000 ranges:
No bus range found for /soc/pcie@0x01000000, using [bus 00-ff]
IO 0x01f80000..0x01f8ffff -> 0x00000000
MEM 0x01000000..0x01efffff -> 0x01000000
imx6q-pcie 1ffc000.pcie: phy link never came up
Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index fe60096..f17fb02 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -288,9 +288,9 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
/* Some boards don't have PCIe reset GPIO. */
if (imx6_pcie->reset_gpio) {
- gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
- msleep(100);
gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
+ msleep(100);
+ gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
}
return 0;
WARNING: multiple messages have this Message-ID (diff)
From: khalasa@piap.pl (Krzysztof Hałasa)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity
Date: Fri, 25 Mar 2016 14:32:35 +0100 [thread overview]
Message-ID: <m3h9fuk7ik.fsf@t19.piap.pl> (raw)
A recent commit 5c5fb40de8f14391a1238db05cef88754faf9229 stated:
Follows: linus/v4.4-rc2
Precedes: linus/v4.5-rc1
PCI: imx6: Add support for active-low reset GPIO
We previously used of_get_named_gpio(), which ignores the OF flags cell, so
the reset GPIO defaulted to "active high." This doesn't work on the Toradex
Apalis SoM with Ixora base board, which has an active-low reset GPIO.
Use devm_gpiod_get_optional() instead so we pay attention to the active
high/low flag. This also adds support for GPIOs described via ACPI.
The (now replaced) code doesn't support the above:
@@ -287,10 +287,10 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
usleep_range(200, 500);
/* Some boards don't have PCIe reset GPIO. */
- if (gpio_is_valid(imx6_pcie->reset_gpio)) {
- gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
+ if (imx6_pcie->reset_gpio) {
+ gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
msleep(100);
- gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
+ gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
}
return 0;
If the reset_gpio setup code had ignored the flags (haven't checked
that), then clearly the resets were active-low (most reset lines are,
because they can be then driven with open-drain/collector output).
The gpiod_set_value*(0) activates reset, gpiod_set_value(1) -
deactivates.
Now we're told the setup code is now level-aware, but the above sequence
thus _deactivates_ reset for 100 ms, then _activates_ it again. It has
no chance to work, unless a board has a broken DTS file. A quick grep
shows that about half the IMX6 boards specify an active-low PCIe reset,
4 ask for active-high, and another 4 don't bother.
I wonder if all boards (except maybe that Toradex set) use an active-low
PCIe reset and are now broken. Perhaps Toradex uses active-high and thus
works.
I'm not fixing individual DTS files because I don't really know, though
perhaps we should change them all to "active-low", since it would work
the same as before the 5c5fb40de8f14391a1238db05cef88754faf9229 change.
Confirmed to fix Gateworks Laguna GW54xx.
Without the patch, the following happens (as expected):
PCI host bridge /soc/pcie at 0x01000000 ranges:
No bus range found for /soc/pcie at 0x01000000, using [bus 00-ff]
IO 0x01f80000..0x01f8ffff -> 0x00000000
MEM 0x01000000..0x01efffff -> 0x01000000
imx6q-pcie 1ffc000.pcie: phy link never came up
Signed-off-by: Krzysztof Ha?asa <khalasa@piap.pl>
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index fe60096..f17fb02 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -288,9 +288,9 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
/* Some boards don't have PCIe reset GPIO. */
if (imx6_pcie->reset_gpio) {
- gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
- msleep(100);
gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
+ msleep(100);
+ gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
}
return 0;
next reply other threads:[~2016-03-25 13:32 UTC|newest]
Thread overview: 85+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-25 13:32 Krzysztof Hałasa [this message]
2016-03-25 13:32 ` [PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity Krzysztof Hałasa
2016-03-27 14:44 ` Fabio Estevam
2016-03-27 14:44 ` Fabio Estevam
2016-03-28 0:26 ` Fabio Estevam
2016-03-28 0:26 ` Fabio Estevam
2016-03-28 19:59 ` Tim Harvey
2016-03-28 19:59 ` Tim Harvey
2016-03-28 20:13 ` Fabio Estevam
2016-03-28 20:13 ` Fabio Estevam
2016-03-28 20:42 ` Tim Harvey
2016-03-28 20:42 ` Tim Harvey
2016-03-28 21:30 ` Fabio Estevam
2016-03-28 21:30 ` Fabio Estevam
2016-03-28 22:06 ` Tim Harvey
2016-03-28 22:06 ` Tim Harvey
2016-03-28 22:13 ` Fabio Estevam
2016-03-28 22:13 ` Fabio Estevam
2016-03-29 5:40 ` Krzysztof Hałasa
2016-03-29 5:40 ` Krzysztof Hałasa
2016-03-29 5:43 ` Krzysztof Hałasa
2016-03-29 5:43 ` Krzysztof Hałasa
2016-03-29 5:29 ` Krzysztof Hałasa
2016-03-29 5:29 ` Krzysztof Hałasa
2016-03-29 8:55 ` Lucas Stach
2016-03-29 8:55 ` Lucas Stach
2016-03-29 10:39 ` Krzysztof Hałasa
2016-03-29 10:39 ` Krzysztof Hałasa
2016-03-29 10:55 ` Lucas Stach
2016-03-29 10:55 ` Lucas Stach
2016-03-29 13:12 ` Arnd Bergmann
2016-03-29 13:12 ` Arnd Bergmann
2016-03-29 13:32 ` Tim Harvey
2016-03-29 13:32 ` Tim Harvey
2016-03-29 13:52 ` Arnd Bergmann
2016-03-29 13:52 ` Arnd Bergmann
2016-03-29 14:29 ` Tim Harvey
2016-03-29 14:29 ` Tim Harvey
2016-03-29 14:50 ` Arnd Bergmann
2016-03-29 14:50 ` Arnd Bergmann
2016-03-29 15:10 ` Tim Harvey
2016-03-29 15:10 ` Tim Harvey
2016-03-29 15:24 ` Arnd Bergmann
2016-03-29 15:24 ` Arnd Bergmann
2016-03-29 17:38 ` Tim Harvey
2016-03-29 17:38 ` Tim Harvey
2016-03-29 19:39 ` Arnd Bergmann
2016-03-29 19:39 ` Arnd Bergmann
2016-03-29 17:56 ` Marc Zyngier
2016-03-29 17:56 ` Marc Zyngier
2016-03-29 16:13 ` Roberto Fichera
2016-03-29 16:13 ` Roberto Fichera
2016-03-29 16:40 ` Tim Harvey
2016-03-29 16:40 ` Tim Harvey
2016-03-29 16:44 ` Roberto Fichera
2016-03-29 16:44 ` Roberto Fichera
2016-03-29 17:31 ` Tim Harvey
2016-03-29 17:31 ` Tim Harvey
2016-03-30 8:00 ` Roberto Fichera
2016-03-30 8:00 ` Roberto Fichera
2016-03-30 10:10 ` Arnd Bergmann
2016-03-30 10:10 ` Arnd Bergmann
2016-03-30 12:50 ` Roberto Fichera
2016-03-30 12:50 ` Roberto Fichera
2016-03-30 13:38 ` Tim Harvey
2016-03-30 13:38 ` Tim Harvey
2016-03-30 15:20 ` Roberto Fichera
2016-03-30 15:20 ` Roberto Fichera
2016-03-30 8:10 ` Krzysztof Hałasa
2016-03-30 8:10 ` Krzysztof Hałasa
2016-03-31 16:19 ` Tim Harvey
2016-03-31 16:19 ` Tim Harvey
2016-04-04 10:37 ` Krzysztof Hałasa
2016-04-04 10:37 ` Krzysztof Hałasa
2016-03-29 14:14 ` Fabio Estevam
2016-03-29 14:14 ` Fabio Estevam
2016-03-29 5:21 ` Krzysztof Hałasa
2016-03-29 5:21 ` Krzysztof Hałasa
2016-03-30 12:06 ` Petr Štetiar
2016-03-30 12:06 ` Petr Štetiar
2016-03-30 12:45 ` Fabio Estevam
2016-03-30 12:45 ` Fabio Estevam
2016-03-30 14:38 ` Marcel Ziswiler
2016-03-30 14:38 ` Marcel Ziswiler
2016-03-30 14:38 ` Marcel Ziswiler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m3h9fuk7ik.fsf@t19.piap.pl \
--to=khalasa@piap.pl \
--cc=Richard.Zhu@freescale.com \
--cc=bhelgaas@google.com \
--cc=l.stach@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=ynezz@true.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.