linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] gpio-mlxbf2: only get IRQ for device instances 0 and 3
@ 2025-07-21 16:22 David Thompson
  2025-07-22 14:52 ` Bartosz Golaszewski
  0 siblings, 1 reply; 4+ messages in thread
From: David Thompson @ 2025-07-21 16:22 UTC (permalink / raw)
  To: linus.walleij, brgl, davem, asmaa
  Cc: linux-gpio, linux-kernel, stable, David Thompson,
	Shravan Kumar Ramani

The gpio-mlxbf2 driver interfaces with four GPIO controllers,
device instances 0-3. There are two IRQ resources shared between
the four controllers, and they are found in the ACPI table for
device instances 0 and 3.  The driver should not attempt to get
an IRQ resource when probing device instance 1 or 2, otherwise
the following error is logged:
  mlxbf2_gpio MLNXBF22:01: error -ENXIO: IRQ index 0 not found

Fixes: 2b725265cb08 ("gpio: mlxbf2: Introduce IRQ support")
Cc: stable@vger.kernel.org
Signed-off-by: David Thompson <davthompson@nvidia.com>
Reviewed-by: Shravan Kumar Ramani <shravankr@nvidia.com>
---
v3: added version history
v2: added tag "Cc: stable@vger.kernel.org"

 drivers/gpio/gpio-mlxbf2.c | 56 ++++++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/drivers/gpio/gpio-mlxbf2.c b/drivers/gpio/gpio-mlxbf2.c
index 6f3dda6b635f..fc56ac81e344 100644
--- a/drivers/gpio/gpio-mlxbf2.c
+++ b/drivers/gpio/gpio-mlxbf2.c
@@ -353,7 +353,9 @@ mlxbf2_gpio_probe(struct platform_device *pdev)
 	struct gpio_chip *gc;
 	unsigned int npins;
 	const char *name;
+	char *colon_ptr;
 	int ret, irq;
+	long num;
 
 	name = dev_name(dev);
 
@@ -397,26 +399,40 @@ mlxbf2_gpio_probe(struct platform_device *pdev)
 	gc->ngpio = npins;
 	gc->owner = THIS_MODULE;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq >= 0) {
-		girq = &gs->gc.irq;
-		gpio_irq_chip_set_chip(girq, &mlxbf2_gpio_irq_chip);
-		girq->handler = handle_simple_irq;
-		girq->default_type = IRQ_TYPE_NONE;
-		/* This will let us handle the parent IRQ in the driver */
-		girq->num_parents = 0;
-		girq->parents = NULL;
-		girq->parent_handler = NULL;
-
-		/*
-		 * Directly request the irq here instead of passing
-		 * a flow-handler because the irq is shared.
-		 */
-		ret = devm_request_irq(dev, irq, mlxbf2_gpio_irq_handler,
-				       IRQF_SHARED, name, gs);
-		if (ret) {
-			dev_err(dev, "failed to request IRQ");
-			return ret;
+	colon_ptr = strchr(dev_name(dev), ':');
+	if (!colon_ptr) {
+		dev_err(dev, "invalid device name format\n");
+		return -EINVAL;
+	}
+
+	ret = kstrtol(++colon_ptr, 16, &num);
+	if (ret) {
+		dev_err(dev, "invalid device instance\n");
+		return ret;
+	}
+
+	if (!num || num == 3) {
+		irq = platform_get_irq(pdev, 0);
+		if (irq >= 0) {
+			girq = &gs->gc.irq;
+			gpio_irq_chip_set_chip(girq, &mlxbf2_gpio_irq_chip);
+			girq->handler = handle_simple_irq;
+			girq->default_type = IRQ_TYPE_NONE;
+			/* This will let us handle the parent IRQ in the driver */
+			girq->num_parents = 0;
+			girq->parents = NULL;
+			girq->parent_handler = NULL;
+
+			/*
+			 * Directly request the irq here instead of passing
+			 * a flow-handler because the irq is shared.
+			 */
+			ret = devm_request_irq(dev, irq, mlxbf2_gpio_irq_handler,
+					       IRQF_SHARED, name, gs);
+			if (ret) {
+				dev_err(dev, "failed to request IRQ");
+				return ret;
+			}
 		}
 	}
 
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] gpio-mlxbf2: only get IRQ for device instances 0 and 3
  2025-07-21 16:22 [PATCH v3] gpio-mlxbf2: only get IRQ for device instances 0 and 3 David Thompson
@ 2025-07-22 14:52 ` Bartosz Golaszewski
  2025-07-23 13:57   ` Andy Shevchenko
  0 siblings, 1 reply; 4+ messages in thread
From: Bartosz Golaszewski @ 2025-07-22 14:52 UTC (permalink / raw)
  To: David Thompson, Andy Shevchenko, Mika Westerberg
  Cc: linus.walleij, davem, asmaa, linux-gpio, linux-kernel, stable,
	Shravan Kumar Ramani

On Mon, Jul 21, 2025 at 6:22 PM David Thompson <davthompson@nvidia.com> wrote:
>
> The gpio-mlxbf2 driver interfaces with four GPIO controllers,
> device instances 0-3. There are two IRQ resources shared between
> the four controllers, and they are found in the ACPI table for
> device instances 0 and 3.  The driver should not attempt to get
> an IRQ resource when probing device instance 1 or 2, otherwise
> the following error is logged:
>   mlxbf2_gpio MLNXBF22:01: error -ENXIO: IRQ index 0 not found
>
> Fixes: 2b725265cb08 ("gpio: mlxbf2: Introduce IRQ support")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Thompson <davthompson@nvidia.com>
> Reviewed-by: Shravan Kumar Ramani <shravankr@nvidia.com>
> ---
> v3: added version history
> v2: added tag "Cc: stable@vger.kernel.org"
>
>  drivers/gpio/gpio-mlxbf2.c | 56 ++++++++++++++++++++++++--------------
>  1 file changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mlxbf2.c b/drivers/gpio/gpio-mlxbf2.c
> index 6f3dda6b635f..fc56ac81e344 100644
> --- a/drivers/gpio/gpio-mlxbf2.c
> +++ b/drivers/gpio/gpio-mlxbf2.c
> @@ -353,7 +353,9 @@ mlxbf2_gpio_probe(struct platform_device *pdev)
>         struct gpio_chip *gc;
>         unsigned int npins;
>         const char *name;
> +       char *colon_ptr;
>         int ret, irq;
> +       long num;
>
>         name = dev_name(dev);
>
> @@ -397,26 +399,40 @@ mlxbf2_gpio_probe(struct platform_device *pdev)
>         gc->ngpio = npins;
>         gc->owner = THIS_MODULE;
>
> -       irq = platform_get_irq(pdev, 0);
> -       if (irq >= 0) {
> -               girq = &gs->gc.irq;
> -               gpio_irq_chip_set_chip(girq, &mlxbf2_gpio_irq_chip);
> -               girq->handler = handle_simple_irq;
> -               girq->default_type = IRQ_TYPE_NONE;
> -               /* This will let us handle the parent IRQ in the driver */
> -               girq->num_parents = 0;
> -               girq->parents = NULL;
> -               girq->parent_handler = NULL;
> -
> -               /*
> -                * Directly request the irq here instead of passing
> -                * a flow-handler because the irq is shared.
> -                */
> -               ret = devm_request_irq(dev, irq, mlxbf2_gpio_irq_handler,
> -                                      IRQF_SHARED, name, gs);
> -               if (ret) {
> -                       dev_err(dev, "failed to request IRQ");
> -                       return ret;
> +       colon_ptr = strchr(dev_name(dev), ':');
> +       if (!colon_ptr) {
> +               dev_err(dev, "invalid device name format\n");
> +               return -EINVAL;
> +       }
> +
> +       ret = kstrtol(++colon_ptr, 16, &num);
> +       if (ret) {
> +               dev_err(dev, "invalid device instance\n");
> +               return ret;
> +       }
> +

That is *really* fragile. Andy, Mika: does this look remotely correct
to you? I don't know much about ACPI systems.

Bart

> +       if (!num || num == 3) {
> +               irq = platform_get_irq(pdev, 0);
> +               if (irq >= 0) {
> +                       girq = &gs->gc.irq;
> +                       gpio_irq_chip_set_chip(girq, &mlxbf2_gpio_irq_chip);
> +                       girq->handler = handle_simple_irq;
> +                       girq->default_type = IRQ_TYPE_NONE;
> +                       /* This will let us handle the parent IRQ in the driver */
> +                       girq->num_parents = 0;
> +                       girq->parents = NULL;
> +                       girq->parent_handler = NULL;
> +
> +                       /*
> +                        * Directly request the irq here instead of passing
> +                        * a flow-handler because the irq is shared.
> +                        */
> +                       ret = devm_request_irq(dev, irq, mlxbf2_gpio_irq_handler,
> +                                              IRQF_SHARED, name, gs);
> +                       if (ret) {
> +                               dev_err(dev, "failed to request IRQ");
> +                               return ret;
> +                       }
>                 }
>         }
>
> --
> 2.30.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] gpio-mlxbf2: only get IRQ for device instances 0 and 3
  2025-07-22 14:52 ` Bartosz Golaszewski
@ 2025-07-23 13:57   ` Andy Shevchenko
  2025-07-28 20:24     ` David Thompson
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2025-07-23 13:57 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: David Thompson, Mika Westerberg, linus.walleij, davem, asmaa,
	linux-gpio, linux-kernel, stable, Shravan Kumar Ramani

On Tue, Jul 22, 2025 at 04:52:02PM +0200, Bartosz Golaszewski wrote:
> On Mon, Jul 21, 2025 at 6:22 PM David Thompson <davthompson@nvidia.com> wrote:
> >
> > The gpio-mlxbf2 driver interfaces with four GPIO controllers,
> > device instances 0-3. There are two IRQ resources shared between
> > the four controllers, and they are found in the ACPI table for
> > device instances 0 and 3.  The driver should not attempt to get
> > an IRQ resource when probing device instance 1 or 2, otherwise
> > the following error is logged:
> >   mlxbf2_gpio MLNXBF22:01: error -ENXIO: IRQ index 0 not found

...

> > -       irq = platform_get_irq(pdev, 0);

So, why not simply change this to the _optional() call?

> > -       if (irq >= 0) {
> > -               girq = &gs->gc.irq;
> > -               gpio_irq_chip_set_chip(girq, &mlxbf2_gpio_irq_chip);
> > -               girq->handler = handle_simple_irq;
> > -               girq->default_type = IRQ_TYPE_NONE;
> > -               /* This will let us handle the parent IRQ in the driver */
> > -               girq->num_parents = 0;
> > -               girq->parents = NULL;
> > -               girq->parent_handler = NULL;
> > -
> > -               /*
> > -                * Directly request the irq here instead of passing
> > -                * a flow-handler because the irq is shared.
> > -                */
> > -               ret = devm_request_irq(dev, irq, mlxbf2_gpio_irq_handler,
> > -                                      IRQF_SHARED, name, gs);
> > -               if (ret) {
> > -                       dev_err(dev, "failed to request IRQ");
> > -                       return ret;
> > +       colon_ptr = strchr(dev_name(dev), ':');
> > +       if (!colon_ptr) {
> > +               dev_err(dev, "invalid device name format\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       ret = kstrtol(++colon_ptr, 16, &num);
> > +       if (ret) {
> > +               dev_err(dev, "invalid device instance\n");
> > +               return ret;
> > +       }
> > +
> 
> That is *really* fragile. Andy, Mika: does this look remotely correct
> to you? I don't know much about ACPI systems.

I totally agree with you. This is an ugly hack and here is formal NAK from me.
The ACPI tables that doesn't provide an IRQ resources (in any of its possible
type) can be simply ignored by not requesting that IRQ. The message above
AFAICT is harmless. Above I proposed the better fix.

> > +       if (!num || num == 3) {
> > +               irq = platform_get_irq(pdev, 0);
> > +               if (irq >= 0) {
> > +                       girq = &gs->gc.irq;
> > +                       gpio_irq_chip_set_chip(girq, &mlxbf2_gpio_irq_chip);
> > +                       girq->handler = handle_simple_irq;
> > +                       girq->default_type = IRQ_TYPE_NONE;
> > +                       /* This will let us handle the parent IRQ in the driver */
> > +                       girq->num_parents = 0;
> > +                       girq->parents = NULL;
> > +                       girq->parent_handler = NULL;
> > +
> > +                       /*
> > +                        * Directly request the irq here instead of passing
> > +                        * a flow-handler because the irq is shared.
> > +                        */
> > +                       ret = devm_request_irq(dev, irq, mlxbf2_gpio_irq_handler,
> > +                                              IRQF_SHARED, name, gs);
> > +                       if (ret) {
> > +                               dev_err(dev, "failed to request IRQ");
> > +                               return ret;
> > +                       }
> >                 }
> >         }

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v3] gpio-mlxbf2: only get IRQ for device instances 0 and 3
  2025-07-23 13:57   ` Andy Shevchenko
@ 2025-07-28 20:24     ` David Thompson
  0 siblings, 0 replies; 4+ messages in thread
From: David Thompson @ 2025-07-28 20:24 UTC (permalink / raw)
  To: Andy Shevchenko, Bartosz Golaszewski
  Cc: Mika Westerberg, linus.walleij@linaro.org, davem@davemloft.net,
	Asmaa Mnebhi, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Shravan Ramani

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: Wednesday, July 23, 2025 9:58 AM
> To: Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: David Thompson <davthompson@nvidia.com>; Mika Westerberg
> <mika.westerberg@linux.intel.com>; linus.walleij@linaro.org;
> davem@davemloft.net; Asmaa Mnebhi <asmaa@nvidia.com>; linux-
> gpio@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org;
> Shravan Ramani <shravankr@nvidia.com>
> Subject: Re: [PATCH v3] gpio-mlxbf2: only get IRQ for device instances 0 and 3
> 
> On Tue, Jul 22, 2025 at 04:52:02PM +0200, Bartosz Golaszewski wrote:
> > On Mon, Jul 21, 2025 at 6:22 PM David Thompson <davthompson@nvidia.com>
> wrote:
> > >
> > > The gpio-mlxbf2 driver interfaces with four GPIO controllers, device
> > > instances 0-3. There are two IRQ resources shared between the four
> > > controllers, and they are found in the ACPI table for device
> > > instances 0 and 3.  The driver should not attempt to get an IRQ
> > > resource when probing device instance 1 or 2, otherwise the
> > > following error is logged:
> > >   mlxbf2_gpio MLNXBF22:01: error -ENXIO: IRQ index 0 not found
> 
> ...
> 
> > > -       irq = platform_get_irq(pdev, 0);
> 
> So, why not simply change this to the _optional() call?
> 

Thanks for the tip, Andy.  

I've sent a v4 patch that uses this approach.

Thanks, 
- Dave

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-07-28 20:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-21 16:22 [PATCH v3] gpio-mlxbf2: only get IRQ for device instances 0 and 3 David Thompson
2025-07-22 14:52 ` Bartosz Golaszewski
2025-07-23 13:57   ` Andy Shevchenko
2025-07-28 20:24     ` David Thompson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).