| Message ID | 20220502093832.32778-2-marcan@marcan.st (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: apple: PWREN GPIO support & related fixes | expand |
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.
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
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.
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);
- 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(-)