[v1] nfc: nxp-nci: allow GPIOs to sleep

[PATCH] nfc: nxp-nci: allow GPIOs to sleep

Posted by Ian Ray 1 month, 1 week ago

Allow the firmware and enable GPIOs to sleep.

This fixes a `WARN_ON' and allows the driver to operate GPIOs which are
connected to I2C GPIO expanders.

-- >8 --
kernel: WARNING: CPU: 3 PID: 2636 at drivers/gpio/gpiolib.c:3880 gpiod_set_value+0x88/0x98
-- >8 --

Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
---
 drivers/nfc/nxp-nci/i2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 6a5ce8ff91f0..b3d34433bd14 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -47,8 +47,8 @@ static int nxp_nci_i2c_set_mode(void *phy_id,
 {
 	struct nxp_nci_i2c_phy *phy = (struct nxp_nci_i2c_phy *) phy_id;
 
-	gpiod_set_value(phy->gpiod_fw, (mode == NXP_NCI_MODE_FW) ? 1 : 0);
-	gpiod_set_value(phy->gpiod_en, (mode != NXP_NCI_MODE_COLD) ? 1 : 0);
+	gpiod_set_value_cansleep(phy->gpiod_fw, (mode == NXP_NCI_MODE_FW) ? 1 : 0);
+	gpiod_set_value_cansleep(phy->gpiod_en, (mode != NXP_NCI_MODE_COLD) ? 1 : 0);
 	usleep_range(10000, 15000);
 
 	if (mode == NXP_NCI_MODE_COLD)
-- 
2.49.0

Re: [PATCH] nfc: nxp-nci: allow GPIOs to sleep

Posted by Andrew Lunn 1 month, 1 week ago

On Fri, Feb 20, 2026 at 01:59:05PM +0200, Ian Ray wrote:
> Allow the firmware and enable GPIOs to sleep.
> 
> This fixes a `WARN_ON' and allows the driver to operate GPIOs which are
> connected to I2C GPIO expanders.
> 
> -- >8 --
> kernel: WARNING: CPU: 3 PID: 2636 at drivers/gpio/gpiolib.c:3880 gpiod_set_value+0x88/0x98
> -- >8 --

Are there in tree boards which have this problem? Should this be part
of stable?

https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

Did you use ./scripts/get_maintainer.pl to get the To: and Cc: list?
It is very unusual to see an empty To:.

	Andrew

Re: [PATCH] nfc: nxp-nci: allow GPIOs to sleep

Posted by Ian Ray 1 month, 1 week ago

On Fri, Feb 20, 2026 at 02:19:34PM +0100, Andrew Lunn wrote:
> CAUTION: This email originated from outside of GE HealthCare. Only open links or attachments if you trust the sender. Report suspicious emails using Outlook’s “Report” button.
> 
> On Fri, Feb 20, 2026 at 01:59:05PM +0200, Ian Ray wrote:
> > Allow the firmware and enable GPIOs to sleep.
> >
> > This fixes a `WARN_ON' and allows the driver to operate GPIOs which are
> > connected to I2C GPIO expanders.
> >
> > -- >8 --
> > kernel: WARNING: CPU: 3 PID: 2636 at drivers/gpio/gpiolib.c:3880 gpiod_set_value+0x88/0x98
> > -- >8 --
> 
> Are there in tree boards which have this problem? Should this be part
> of stable?

I could not find any (I am working on a board which I have not submitted
upstream as yet).

> 
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> 
> Did you use ./scripts/get_maintainer.pl to get the To: and Cc: list?
> It is very unusual to see an empty To:.
> 

I did.  This driver is marked as an orphan.

>         Andrew

Regards,
Ian

Re: [PATCH] nfc: nxp-nci: allow GPIOs to sleep

Posted by Ian Ray 1 month, 1 week ago

On Fri, Feb 20, 2026 at 03:28:54PM +0200, Ian Ray wrote:
> On Fri, Feb 20, 2026 at 02:19:34PM +0100, Andrew Lunn wrote:
> > CAUTION: This email originated from outside of GE HealthCare. Only open links or attachments if you trust the sender. Report suspicious emails using Outlook’s “Report” button.
> > 
> > On Fri, Feb 20, 2026 at 01:59:05PM +0200, Ian Ray wrote:
> > > Allow the firmware and enable GPIOs to sleep.
> > >
> > > This fixes a `WARN_ON' and allows the driver to operate GPIOs which are
> > > connected to I2C GPIO expanders.
> > >
> > > -- >8 --
> > > kernel: WARNING: CPU: 3 PID: 2636 at drivers/gpio/gpiolib.c:3880 gpiod_set_value+0x88/0x98
> > > -- >8 --
> > 
> > Are there in tree boards which have this problem? Should this be part
> > of stable?
> 
> I could not find any (I am working on a board which I have not submitted
> upstream as yet).

Argh.  Apologies, failed search-fu.  There are indeed a few boards with
compatible nxp,nxp-nci-i2c.

> 
> > 
> > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > 
> > Did you use ./scripts/get_maintainer.pl to get the To: and Cc: list?
> > It is very unusual to see an empty To:.
> > 
> 
> I did.  This driver is marked as an orphan.
> 
> >         Andrew
> 
> Regards,
> Ian

Re: [PATCH] nfc: nxp-nci: allow GPIOs to sleep

Posted by Charalampos Mitrodimas 1 month, 1 week ago

Ian Ray <ian.ray@gehealthcare.com> writes:

> Allow the firmware and enable GPIOs to sleep.
>
> This fixes a `WARN_ON' and allows the driver to operate GPIOs which are
> connected to I2C GPIO expanders.
>
> -- >8 --
> kernel: WARNING: CPU: 3 PID: 2636 at drivers/gpio/gpiolib.c:3880 gpiod_set_value+0x88/0x98
> -- >8 --
>
> Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
> ---
>  drivers/nfc/nxp-nci/i2c.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
> index 6a5ce8ff91f0..b3d34433bd14 100644
> --- a/drivers/nfc/nxp-nci/i2c.c
> +++ b/drivers/nfc/nxp-nci/i2c.c
> @@ -47,8 +47,8 @@ static int nxp_nci_i2c_set_mode(void *phy_id,
>  {
>  	struct nxp_nci_i2c_phy *phy = (struct nxp_nci_i2c_phy *) phy_id;
>  
> -	gpiod_set_value(phy->gpiod_fw, (mode == NXP_NCI_MODE_FW) ? 1 : 0);
> -	gpiod_set_value(phy->gpiod_en, (mode != NXP_NCI_MODE_COLD) ? 1 : 0);
> +	gpiod_set_value_cansleep(phy->gpiod_fw, (mode == NXP_NCI_MODE_FW) ? 1 : 0);
> +	gpiod_set_value_cansleep(phy->gpiod_en, (mode != NXP_NCI_MODE_COLD) ? 1 : 0);

Do we need a Fixes tag here?

AFAICT this was introduced in

  6be88670fc59 ("NFC: nxp-nci_i2c: Add I2C support to NXP NCI driver")

which used gpio_set_value() instead of gpio_set_value_cansleep() from
the beginning (that is before transitioning to gpiod_* calls in
43201767b44c ("NFC: nxp-nci: Convert to use GPIO descriptor").

This piece of code is present on active stable trees. So you also need
to Cc stable?
  
Cheers,
C. Mitrodimas

>  	usleep_range(10000, 15000);
>  
>  	if (mode == NXP_NCI_MODE_COLD)

Re: [PATCH] nfc: nxp-nci: allow GPIOs to sleep

Posted by Ian Ray 1 month, 1 week ago

On Fri, Feb 20, 2026 at 01:16:56PM +0000, Charalampos Mitrodimas wrote:
> CAUTION: This email originated from outside of GE HealthCare. Only open links or attachments if you trust the sender. Report suspicious emails using Outlook’s “Report” button.
> 
> Ian Ray <ian.ray@gehealthcare.com> writes:
> 
> > Allow the firmware and enable GPIOs to sleep.
> >
> > This fixes a `WARN_ON' and allows the driver to operate GPIOs which are
> > connected to I2C GPIO expanders.
> >
> > -- >8 --
> > kernel: WARNING: CPU: 3 PID: 2636 at drivers/gpio/gpiolib.c:3880 gpiod_set_value+0x88/0x98
> > -- >8 --
> >
> > Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
> > ---
> >  drivers/nfc/nxp-nci/i2c.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
> > index 6a5ce8ff91f0..b3d34433bd14 100644
> > --- a/drivers/nfc/nxp-nci/i2c.c
> > +++ b/drivers/nfc/nxp-nci/i2c.c
> > @@ -47,8 +47,8 @@ static int nxp_nci_i2c_set_mode(void *phy_id,
> >  {
> >       struct nxp_nci_i2c_phy *phy = (struct nxp_nci_i2c_phy *) phy_id;
> >
> > -     gpiod_set_value(phy->gpiod_fw, (mode == NXP_NCI_MODE_FW) ? 1 : 0);
> > -     gpiod_set_value(phy->gpiod_en, (mode != NXP_NCI_MODE_COLD) ? 1 : 0);
> > +     gpiod_set_value_cansleep(phy->gpiod_fw, (mode == NXP_NCI_MODE_FW) ? 1 : 0);
> > +     gpiod_set_value_cansleep(phy->gpiod_en, (mode != NXP_NCI_MODE_COLD) ? 1 : 0);
> 
> Do we need a Fixes tag here?

I think we should, yes; sorry for overlooking that.

> 
> AFAICT this was introduced in
> 
>   6be88670fc59 ("NFC: nxp-nci_i2c: Add I2C support to NXP NCI driver")
> 
> which used gpio_set_value() instead of gpio_set_value_cansleep() from
> the beginning (that is before transitioning to gpiod_* calls in
> 43201767b44c ("NFC: nxp-nci: Convert to use GPIO descriptor").
> 
> This piece of code is present on active stable trees. So you also need
> to Cc stable?

Yes, you're right, thank you.

> 
> Cheers,
> C. Mitrodimas
> 
> >       usleep_range(10000, 15000);
> >
> >       if (mode == NXP_NCI_MODE_COLD)