On 12/10/24 16:42, Bjorn Helgaas wrote: > 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? By 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 Oh yes you're right. In fact, the if statement in probe() and resume_noirq() is for msleep(), not really for gpiod_set_value_cansleep(). So this patch is useless. Regards, Thomas
