| Message ID | 2e0f1e9d250b6ee1b3039362affc411751a3cd4a.1547382494.git.baruch@tkos.co.il (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Revert "PCI: armada8k: Add support for gpio controlled reset signal" | expand |
On Sun, Jan 13, 2019 at 02:28:14PM +0200, Baruch Siach wrote: > This reverts commit 3d71746c420c1c1c27cf5c4e48f8fa0a6cfdc185. > > That commit breaks boot on Macchiatobin when a Mellanox NIC is in the > PCIe slot. > > It turns out that full reset cycle requires first comphy serdes > initialization. Reset signal toggle without comphy initialization makes > access to PCI configuration registers stall indefinitely. U-Boot toggles > the Macchiatobin PCIe reset line already at boot, after initializing the > comphy serdes. > > So while commit 3d71746c42 enables PCIe on platforms that U-Boot does > not (yet) touch the reset line (like Clearfog GT-8K), it breaks PCIe > (and boot) on the Macchiatobin. > > Revert commit 3d71746c42 entirely for now to fix the Macchaitobin > regression. Proper PCIe reset support would require PCIe comphy setup > support. That must wait for another kernel release. > > Reported-by: Sven Auhagen <sven.auhagen@voleatech.de> > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > drivers/pci/controller/dwc/pcie-armada8k.c | 16 ---------------- > 1 file changed, 16 deletions(-) Read: https://lore.kernel.org/linux-pci/20171026223701.GA25649@bhelgaas-glaptop.roam.corp.google.com/ reformat the patch, repost it and I will pick it up. Thanks, Lorenzo > diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c > index b171b6bc15c8..0c389a30ef5d 100644 > --- a/drivers/pci/controller/dwc/pcie-armada8k.c > +++ b/drivers/pci/controller/dwc/pcie-armada8k.c > @@ -22,7 +22,6 @@ > #include <linux/resource.h> > #include <linux/of_pci.h> > #include <linux/of_irq.h> > -#include <linux/gpio/consumer.h> > > #include "pcie-designware.h" > > @@ -30,7 +29,6 @@ struct armada8k_pcie { > struct dw_pcie *pci; > struct clk *clk; > struct clk *clk_reg; > - struct gpio_desc *reset_gpio; > }; > > #define PCIE_VENDOR_REGS_OFFSET 0x8000 > @@ -139,12 +137,6 @@ static int armada8k_pcie_host_init(struct pcie_port *pp) > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > struct armada8k_pcie *pcie = to_armada8k_pcie(pci); > > - if (pcie->reset_gpio) { > - /* assert and then deassert the reset signal */ > - gpiod_set_value_cansleep(pcie->reset_gpio, 1); > - msleep(100); > - gpiod_set_value_cansleep(pcie->reset_gpio, 0); > - } > dw_pcie_setup_rc(pp); > armada8k_pcie_establish_link(pcie); > > @@ -257,14 +249,6 @@ static int armada8k_pcie_probe(struct platform_device *pdev) > goto fail_clkreg; > } > > - /* Get reset gpio signal and hold asserted (logically high) */ > - pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset", > - GPIOD_OUT_HIGH); > - if (IS_ERR(pcie->reset_gpio)) { > - ret = PTR_ERR(pcie->reset_gpio); > - goto fail_clkreg; > - } > - > platform_set_drvdata(pdev, pcie); > > ret = armada8k_add_pcie_port(pcie, pdev); > -- > 2.20.1 >
diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c index b171b6bc15c8..0c389a30ef5d 100644 --- a/drivers/pci/controller/dwc/pcie-armada8k.c +++ b/drivers/pci/controller/dwc/pcie-armada8k.c @@ -22,7 +22,6 @@ #include <linux/resource.h> #include <linux/of_pci.h> #include <linux/of_irq.h> -#include <linux/gpio/consumer.h> #include "pcie-designware.h" @@ -30,7 +29,6 @@ struct armada8k_pcie { struct dw_pcie *pci; struct clk *clk; struct clk *clk_reg; - struct gpio_desc *reset_gpio; }; #define PCIE_VENDOR_REGS_OFFSET 0x8000 @@ -139,12 +137,6 @@ static int armada8k_pcie_host_init(struct pcie_port *pp) struct dw_pcie *pci = to_dw_pcie_from_pp(pp); struct armada8k_pcie *pcie = to_armada8k_pcie(pci); - if (pcie->reset_gpio) { - /* assert and then deassert the reset signal */ - gpiod_set_value_cansleep(pcie->reset_gpio, 1); - msleep(100); - gpiod_set_value_cansleep(pcie->reset_gpio, 0); - } dw_pcie_setup_rc(pp); armada8k_pcie_establish_link(pcie); @@ -257,14 +249,6 @@ static int armada8k_pcie_probe(struct platform_device *pdev) goto fail_clkreg; } - /* Get reset gpio signal and hold asserted (logically high) */ - pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset", - GPIOD_OUT_HIGH); - if (IS_ERR(pcie->reset_gpio)) { - ret = PTR_ERR(pcie->reset_gpio); - goto fail_clkreg; - } - platform_set_drvdata(pdev, pcie); ret = armada8k_add_pcie_port(pcie, pdev);
This reverts commit 3d71746c420c1c1c27cf5c4e48f8fa0a6cfdc185. That commit breaks boot on Macchiatobin when a Mellanox NIC is in the PCIe slot. It turns out that full reset cycle requires first comphy serdes initialization. Reset signal toggle without comphy initialization makes access to PCI configuration registers stall indefinitely. U-Boot toggles the Macchiatobin PCIe reset line already at boot, after initializing the comphy serdes. So while commit 3d71746c42 enables PCIe on platforms that U-Boot does not (yet) touch the reset line (like Clearfog GT-8K), it breaks PCIe (and boot) on the Macchiatobin. Revert commit 3d71746c42 entirely for now to fix the Macchaitobin regression. Proper PCIe reset support would require PCIe comphy setup support. That must wait for another kernel release. Reported-by: Sven Auhagen <sven.auhagen@voleatech.de> Signed-off-by: Baruch Siach <baruch@tkos.co.il> --- drivers/pci/controller/dwc/pcie-armada8k.c | 16 ---------------- 1 file changed, 16 deletions(-)