From: Bjorn Helgaas <helgaas@kernel.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Thomas Richard <thomas.richard@bootlin.com>,
vigneshr@ti.com, s-vadapalli@ti.com, lpieralisi@kernel.org,
kw@linux.com, robh@kernel.org, bhelgaas@google.com,
theo.lebrun@bootlin.com, thomas.petazzoni@bootlin.com,
kwilczynski@kernel.org, linux-omap@vger.kernel.org,
linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, gregory.clement@bootlin.com,
u-kumar1@ti.com, Linus Walleij <linus.walleij@linaro.org>,
Bartosz Golaszewski <brgl@bgdev.pl>,
linux-gpio@vger.kernel.org
Subject: Re: [PATCH] PCI: j721e: In j721e_pcie_suspend_noirq() check reset_gpio before to use it
Date: Wed, 11 Dec 2024 14:24:09 -0600 [thread overview]
Message-ID: <20241211202409.GA3305505@bhelgaas> (raw)
In-Reply-To: <20241211091421.4empou7mbm35ynxq@thinkpad>
[+cc GPIO folks in case they think it's worthwhile to document that
it's safe to pass NULL pointers to gpiod_*() interfaces]
On Wed, Dec 11, 2024 at 02:44:21PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Dec 11, 2024 at 09:59:30AM +0100, Thomas Richard wrote:
> > 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.
> > >> +++ 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);
> > >> }
> > > 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
> Yes. Almost all of the GPIO APIs accepting desc (except few) use
> VALIDATE_DESC() to check for NULL descriptor. So explicit check is
> not needed.
I think it would be nice if the kernel-doc for these functions
mentioned this somewhere. It's kind of a pain for every user to have
to deduce this.
Bjorn
parent reply other threads:[~2024-12-11 20:24 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <20241211091421.4empou7mbm35ynxq@thinkpad>]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241211202409.GA3305505@bhelgaas \
--to=helgaas@kernel.org \
--cc=bhelgaas@google.com \
--cc=brgl@bgdev.pl \
--cc=gregory.clement@bootlin.com \
--cc=kw@linux.com \
--cc=kwilczynski@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=manivannan.sadhasivam@linaro.org \
--cc=robh@kernel.org \
--cc=s-vadapalli@ti.com \
--cc=theo.lebrun@bootlin.com \
--cc=thomas.petazzoni@bootlin.com \
--cc=thomas.richard@bootlin.com \
--cc=u-kumar1@ti.com \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox