* [PATCH 1/3] PCI: apple: GPIO handling nitfixes [not found] <20220502093832.32778-1-marcan@marcan.st> @ 2022-05-02 9:38 ` Hector Martin 2022-05-02 10:20 ` Marc Zyngier 2022-05-02 11:39 ` Greg KH 2022-05-02 9:38 ` [PATCH 2/3] PCI: apple: Probe all GPIOs for availability first Hector Martin 1 sibling, 2 replies; 6+ messages in thread From: Hector Martin @ 2022-05-02 9:38 UTC (permalink / raw) To: Marc Zyngier Cc: Hector Martin, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Alyssa Rosenzweig, Sven Peter, linux-pci, linux-kernel, stable - Use devm managed GPIO getter - GPIO ops can sleep in this context Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up") Cc: stable@vger.kernel.org Signed-off-by: Hector Martin <marcan@marcan.st> --- drivers/pci/controller/pcie-apple.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c index a2c3c207a04b..e0c06c0ee731 100644 --- a/drivers/pci/controller/pcie-apple.c +++ b/drivers/pci/controller/pcie-apple.c @@ -516,8 +516,8 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie, u32 stat, idx; int ret, i; - reset = gpiod_get_from_of_node(np, "reset-gpios", 0, - GPIOD_OUT_LOW, "PERST#"); + reset = devm_gpiod_get_from_of_node(pcie->dev, np, "reset-gpios", 0, + GPIOD_OUT_LOW, "PERST#"); if (IS_ERR(reset)) return PTR_ERR(reset); @@ -541,7 +541,7 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie, rmw_set(PORT_APPCLK_EN, port->base + PORT_APPCLK); /* Assert PERST# before setting up the clock */ - gpiod_set_value(reset, 1); + gpiod_set_value_cansleep(reset, 1); ret = apple_pcie_setup_refclk(pcie, port); if (ret < 0) @@ -552,7 +552,7 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie, /* Deassert PERST# */ rmw_set(PORT_PERST_OFF, port->base + PORT_PERST); - gpiod_set_value(reset, 0); + gpiod_set_value_cansleep(reset, 0); /* Wait for 100ms after PERST# deassertion (PCIe r5.0, 6.6.1) */ msleep(100); -- 2.35.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] PCI: apple: GPIO handling nitfixes 2022-05-02 9:38 ` [PATCH 1/3] PCI: apple: GPIO handling nitfixes Hector Martin @ 2022-05-02 10:20 ` Marc Zyngier 2022-05-02 12:16 ` Hector Martin 2022-05-02 11:39 ` Greg KH 1 sibling, 1 reply; 6+ messages in thread From: Marc Zyngier @ 2022-05-02 10:20 UTC (permalink / raw) To: Hector Martin Cc: Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Alyssa Rosenzweig, Sven Peter, linux-pci, linux-kernel, stable On Mon, 02 May 2022 10:38:30 +0100, Hector Martin <marcan@marcan.st> wrote: > > - Use devm managed GPIO getter > - GPIO ops can sleep in this context > > Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up") > Cc: stable@vger.kernel.org Why the Cc: stable? I'd guess that at a push, the devm_*() usage help with potential memory leaks when the driver fails to probe, but it would be good to call that out in the commit message. > Signed-off-by: Hector Martin <marcan@marcan.st> > --- > drivers/pci/controller/pcie-apple.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c > index a2c3c207a04b..e0c06c0ee731 100644 > --- a/drivers/pci/controller/pcie-apple.c > +++ b/drivers/pci/controller/pcie-apple.c > @@ -516,8 +516,8 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie, > u32 stat, idx; > int ret, i; > > - reset = gpiod_get_from_of_node(np, "reset-gpios", 0, > - GPIOD_OUT_LOW, "PERST#"); > + reset = devm_gpiod_get_from_of_node(pcie->dev, np, "reset-gpios", 0, > + GPIOD_OUT_LOW, "PERST#"); > if (IS_ERR(reset)) > return PTR_ERR(reset); > > @@ -541,7 +541,7 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie, > rmw_set(PORT_APPCLK_EN, port->base + PORT_APPCLK); > > /* Assert PERST# before setting up the clock */ > - gpiod_set_value(reset, 1); > + gpiod_set_value_cansleep(reset, 1); > > ret = apple_pcie_setup_refclk(pcie, port); > if (ret < 0) > @@ -552,7 +552,7 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie, > > /* Deassert PERST# */ > rmw_set(PORT_PERST_OFF, port->base + PORT_PERST); > - gpiod_set_value(reset, 0); > + gpiod_set_value_cansleep(reset, 0); > > /* Wait for 100ms after PERST# deassertion (PCIe r5.0, 6.6.1) */ > msleep(100); Otherwise: Acked-by: Marc Zyngier <maz@kernel.org> M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] PCI: apple: GPIO handling nitfixes 2022-05-02 10:20 ` Marc Zyngier @ 2022-05-02 12:16 ` Hector Martin 0 siblings, 0 replies; 6+ messages in thread From: Hector Martin @ 2022-05-02 12:16 UTC (permalink / raw) To: Marc Zyngier Cc: Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Alyssa Rosenzweig, Sven Peter, linux-pci, linux-kernel, stable On 02/05/2022 19.20, Marc Zyngier wrote: > On Mon, 02 May 2022 10:38:30 +0100, > Hector Martin <marcan@marcan.st> wrote: >> >> - Use devm managed GPIO getter >> - GPIO ops can sleep in this context >> >> Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up") >> Cc: stable@vger.kernel.org > > Why the Cc: stable? I'd guess that at a push, the devm_*() usage help > with potential memory leaks when the driver fails to probe, but it > would be good to call that out in the commit message. Ack, I mentioned what this fixes for v2 and reworded the commit message. -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] PCI: apple: GPIO handling nitfixes 2022-05-02 9:38 ` [PATCH 1/3] PCI: apple: GPIO handling nitfixes Hector Martin 2022-05-02 10:20 ` Marc Zyngier @ 2022-05-02 11:39 ` Greg KH 1 sibling, 0 replies; 6+ messages in thread From: Greg KH @ 2022-05-02 11:39 UTC (permalink / raw) To: Hector Martin Cc: Marc Zyngier, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Alyssa Rosenzweig, Sven Peter, linux-pci, linux-kernel, stable On Mon, May 02, 2022 at 06:38:30PM +0900, Hector Martin wrote: > - Use devm managed GPIO getter > - GPIO ops can sleep in this context Please read the section entitled "The canonical patch format" in the kernel file, Documentation/SubmittingPatches for what is needed in order to properly describe the change. This text does not make any sense. Same for your subject, it needs to be reworked. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] PCI: apple: Probe all GPIOs for availability first [not found] <20220502093832.32778-1-marcan@marcan.st> 2022-05-02 9:38 ` [PATCH 1/3] PCI: apple: GPIO handling nitfixes Hector Martin @ 2022-05-02 9:38 ` Hector Martin 2022-05-02 10:23 ` Marc Zyngier 1 sibling, 1 reply; 6+ messages in thread From: Hector Martin @ 2022-05-02 9:38 UTC (permalink / raw) To: Marc Zyngier Cc: Hector Martin, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Alyssa Rosenzweig, Sven Peter, linux-pci, linux-kernel, stable If we're probing the PCI controller and some GPIOs are not available and cause a probe defer, we can end up leaving some ports initialized and not others and making a mess. Check for PERST# GPIOs for all ports first, and just return -EPROBE_DEFER if any are not ready yet, without bringing anything up. Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up") Cc: stable@vger.kernel.org Signed-off-by: Hector Martin <marcan@marcan.st> --- drivers/pci/controller/pcie-apple.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c index e0c06c0ee731..e3aa2d461739 100644 --- a/drivers/pci/controller/pcie-apple.c +++ b/drivers/pci/controller/pcie-apple.c @@ -507,6 +507,20 @@ static u32 apple_pcie_rid2sid_write(struct apple_pcie_port *port, return readl_relaxed(port->base + PORT_RID2SID(idx)); } +static int apple_pcie_probe_port(struct device_node *np) +{ + struct gpio_desc *gd; + + gd = gpiod_get_from_of_node(np, "reset-gpios", 0, + GPIOD_OUT_LOW, "PERST#"); + if (IS_ERR(gd)) { + return PTR_ERR(gd); + } + + gpiod_put(gd); + return 0; +} + static int apple_pcie_setup_port(struct apple_pcie *pcie, struct device_node *np) { @@ -797,8 +811,18 @@ static int apple_pcie_init(struct pci_config_window *cfg) static int apple_pcie_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; + struct device_node *of_port; int ret; + /* Check for probe dependencies for all ports first */ + for_each_child_of_node(dev->of_node, of_port) { + ret = apple_pcie_probe_port(of_port); + of_node_put(of_port); + if (ret) + return dev_err_probe(dev, ret, "Port %pOF probe fail\n", of_port); + } + ret = bus_register_notifier(&pci_bus_type, &apple_pcie_nb); if (ret) return ret; -- 2.35.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] PCI: apple: Probe all GPIOs for availability first 2022-05-02 9:38 ` [PATCH 2/3] PCI: apple: Probe all GPIOs for availability first Hector Martin @ 2022-05-02 10:23 ` Marc Zyngier 0 siblings, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2022-05-02 10:23 UTC (permalink / raw) To: Hector Martin Cc: Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Alyssa Rosenzweig, Sven Peter, linux-pci, linux-kernel, stable On Mon, 02 May 2022 10:38:31 +0100, Hector Martin <marcan@marcan.st> wrote: > > If we're probing the PCI controller and some GPIOs are not available and > cause a probe defer, we can end up leaving some ports initialized and > not others and making a mess. > > Check for PERST# GPIOs for all ports first, and just return > -EPROBE_DEFER if any are not ready yet, without bringing anything up. > > Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up") > Cc: stable@vger.kernel.org > Signed-off-by: Hector Martin <marcan@marcan.st> > --- > drivers/pci/controller/pcie-apple.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c > index e0c06c0ee731..e3aa2d461739 100644 > --- a/drivers/pci/controller/pcie-apple.c > +++ b/drivers/pci/controller/pcie-apple.c > @@ -507,6 +507,20 @@ static u32 apple_pcie_rid2sid_write(struct apple_pcie_port *port, > return readl_relaxed(port->base + PORT_RID2SID(idx)); > } > > +static int apple_pcie_probe_port(struct device_node *np) > +{ > + struct gpio_desc *gd; > + > + gd = gpiod_get_from_of_node(np, "reset-gpios", 0, > + GPIOD_OUT_LOW, "PERST#"); > + if (IS_ERR(gd)) { > + return PTR_ERR(gd); > + } > + > + gpiod_put(gd); > + return 0; > +} > + > static int apple_pcie_setup_port(struct apple_pcie *pcie, > struct device_node *np) > { > @@ -797,8 +811,18 @@ static int apple_pcie_init(struct pci_config_window *cfg) > > static int apple_pcie_probe(struct platform_device *pdev) > { > + struct device *dev = &pdev->dev; > + struct device_node *of_port; > int ret; > > + /* Check for probe dependencies for all ports first */ > + for_each_child_of_node(dev->of_node, of_port) { > + ret = apple_pcie_probe_port(of_port); > + of_node_put(of_port); > + if (ret) > + return dev_err_probe(dev, ret, "Port %pOF probe fail\n", of_port); > + } > + > ret = bus_register_notifier(&pci_bus_type, &apple_pcie_nb); > if (ret) > return ret; Acked-by: Marc Zyngier <maz@kernel.org> M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-02 12:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220502093832.32778-1-marcan@marcan.st>
2022-05-02 9:38 ` [PATCH 1/3] PCI: apple: GPIO handling nitfixes Hector Martin
2022-05-02 10:20 ` Marc Zyngier
2022-05-02 12:16 ` Hector Martin
2022-05-02 11:39 ` Greg KH
2022-05-02 9:38 ` [PATCH 2/3] PCI: apple: Probe all GPIOs for availability first Hector Martin
2022-05-02 10:23 ` Marc Zyngier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox