On Mon, Dec 09, 2024 at 12:23:21PM +0100, Thomas Richard wrote: > The reset_gpio is optional, so in j721e_pcie_suspend_noirq() check if it is > not NULL before to use it. If you have occasion to post a v2, update subject to: PCI: j721e: Check reset_gpio for NULL before using it s/before to use it/before using it/ Did you trip over a NULL pointer dereference here? Or maybe found via inspection? It looks like gpiod_set_value_cansleep(desc) *should* be a no-op if desc is NULL, based on this comment [1]: * This descriptor validation needs to be inserted verbatim into each * function taking a descriptor, so we need to use a preprocessor * macro to avoid endless duplication. If the desc is NULL it is an * optional GPIO and calls should just bail out. and the fact that the VALIDATE_DESC_VOID() macro looks like it would return early in that case. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?id=v6.12#n2316 > Fixes: c538d40f365b ("PCI: j721e: Add suspend and resume support") > Signed-off-by: Thomas Richard <thomas.richard@xxxxxxxxxxx> > --- > drivers/pci/controller/cadence/pci-j721e.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > index 0341d51d6aed..5bc14dd70811 100644 > --- a/drivers/pci/controller/cadence/pci-j721e.c > +++ b/drivers/pci/controller/cadence/pci-j721e.c > @@ -644,7 +644,9 @@ static int j721e_pcie_suspend_noirq(struct device *dev) > struct j721e_pcie *pcie = dev_get_drvdata(dev); > > if (pcie->mode == PCI_MODE_RC) { > - gpiod_set_value_cansleep(pcie->reset_gpio, 0); > + if (pcie->reset_gpio) > + gpiod_set_value_cansleep(pcie->reset_gpio, 0); > + > clk_disable_unprepare(pcie->refclk); > } > > -- > 2.39.5 >
