[PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Lad Prabhakar]

platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
allocation of IRQ resources in DT core code, this causes an issue
when using hierarchical interrupt domains using "interrupts" property
in the node as this bypassed the hierarchical setup and messed up the
irq chaining.

In preparation for removal of static setup of IRQ resource from DT core
code use platform_get_irq_optional().

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
Hi,

Usage of platform_get_irq/_optional was agreed based on
the discussion [0].

[0] https://patchwork.kernel.org/project/linux-renesas-soc/
patch/20211209001056.29774-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar
---
 drivers/irqchip/irq-renesas-irqc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
index 07a6d8b42b63..c460f3c228d0 100644
--- a/drivers/irqchip/irq-renesas-irqc.c
+++ b/drivers/irqchip/irq-renesas-irqc.c
@@ -126,7 +126,6 @@ static int irqc_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	const char *name = dev_name(dev);
 	struct irqc_priv *p;
-	struct resource *irq;
 	int ret;
 	int k;
 
@@ -142,13 +141,15 @@ static int irqc_probe(struct platform_device *pdev)
 
 	/* allow any number of IRQs between 1 and IRQC_IRQ_MAX */
 	for (k = 0; k < IRQC_IRQ_MAX; k++) {
-		irq = platform_get_resource(pdev, IORESOURCE_IRQ, k);
-		if (!irq)
+		ret = platform_get_irq_optional(pdev, k);
+		if (ret == -EPROBE_DEFER)
+			goto err_runtime_pm_disable;
+		if (ret < 0)
 			break;
 
 		p->irq[k].p = p;
 		p->irq[k].hw_irq = k;
-		p->irq[k].requested_irq = irq->start;
+		p->irq[k].requested_irq = ret;
 	}
 
 	p->number_of_irqs = k;
-- 
2.17.1

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Geert Uytterhoeven]

Hi Prabhakar,

On Thu, Dec 16, 2021 at 12:50 AM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
> allocation of IRQ resources in DT core code, this causes an issue
> when using hierarchical interrupt domains using "interrupts" property
> in the node as this bypassed the hierarchical setup and messed up the
> irq chaining.
>
> In preparation for removal of static setup of IRQ resource from DT core
> code use platform_get_irq_optional().
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/irqchip/irq-renesas-irqc.c
> +++ b/drivers/irqchip/irq-renesas-irqc.c

> @@ -142,13 +141,15 @@ static int irqc_probe(struct platform_device *pdev)
>
>         /* allow any number of IRQs between 1 and IRQC_IRQ_MAX */
>         for (k = 0; k < IRQC_IRQ_MAX; k++) {
> -               irq = platform_get_resource(pdev, IORESOURCE_IRQ, k);
> -               if (!irq)
> +               ret = platform_get_irq_optional(pdev, k);
> +               if (ret == -EPROBE_DEFER)
> +                       goto err_runtime_pm_disable;
> +               if (ret < 0)
>                         break;

Same comment as for the other patch: shouldn't all errors be
considered fatal, except for -ENXIO (no interrupt)?
>
>                 p->irq[k].p = p;
>                 p->irq[k].hw_irq = k;
> -               p->irq[k].requested_irq = irq->start;
> +               p->irq[k].requested_irq = ret;
>         }
>
>         p->number_of_irqs = k;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Andy Shevchenko]

On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
> allocation of IRQ resources in DT core code, this causes an issue
> when using hierarchical interrupt domains using "interrupts" property
> in the node as this bypassed the hierarchical setup and messed up the
> irq chaining.
>
> In preparation for removal of static setup of IRQ resource from DT core
> code use platform_get_irq_optional().


> +               ret = platform_get_irq_optional(pdev, k);
> +               if (ret == -EPROBE_DEFER)
> +                       goto err_runtime_pm_disable;
> +               if (ret < 0)
>                         break;

The best approach is

ret = platform_get_irq_optional(...);
if (ret < 0 && ret != -ENXIO)
  return ret;
if (ret > 0)
  ...we got it...

It will allow the future API fix of platform_get_irq_optional() to be
really optional.


--
With Best Regards,
Andy Shevchenko

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Lad, Prabhakar]

Hi Andy,

Thank you for the review.

On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> >
> > platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
> > allocation of IRQ resources in DT core code, this causes an issue
> > when using hierarchical interrupt domains using "interrupts" property
> > in the node as this bypassed the hierarchical setup and messed up the
> > irq chaining.
> >
> > In preparation for removal of static setup of IRQ resource from DT core
> > code use platform_get_irq_optional().
>
>
> > +               ret = platform_get_irq_optional(pdev, k);
> > +               if (ret == -EPROBE_DEFER)
> > +                       goto err_runtime_pm_disable;
> > +               if (ret < 0)
> >                         break;
>
> The best approach is
>
> ret = platform_get_irq_optional(...);
> if (ret < 0 && ret != -ENXIO)
>   return ret;
> if (ret > 0)
>   ...we got it...
>
> It will allow the future API fix of platform_get_irq_optional() to be
> really optional.
>
Later patch [0] (merged into -next) does check for -ENXIO first.

[0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/

Cheers,
Prabhakar

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Andy Shevchenko]

On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:

> > ret = platform_get_irq_optional(...);
> > if (ret < 0 && ret != -ENXIO)
> >   return ret;
> > if (ret > 0)
> >   ...we got it...
> >
> > It will allow the future API fix of platform_get_irq_optional() to be
> > really optional.
> >
> Later patch [0] (merged into -next) does check for -ENXIO first.
>
> [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/

The problem is that it doesn't consider 0 as no IRQ.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Lad, Prabhakar]

Hi Andy,

On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> > > ret = platform_get_irq_optional(...);
> > > if (ret < 0 && ret != -ENXIO)
> > >   return ret;
> > > if (ret > 0)
> > >   ...we got it...
> > >
> > > It will allow the future API fix of platform_get_irq_optional() to be
> > > really optional.
> > >
> > Later patch [0] (merged into -next) does check for -ENXIO first.
> >
> > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
>
> The problem is that it doesn't consider 0 as no IRQ.
>
Can you please point me to the discussion/patch where this API change
is considered/discussed. Just to clarify now the new API for
platform_get_irq_optional() will return "0" in case there is no
interrupt and not not -ENXIO anymore?

When will this patch be merged for the new api, so that I can base my
patches on top of it to avoid more changes?

Cheers,
Prabhakar

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Andy Shevchenko]

On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> > <prabhakar.csengg@gmail.com> wrote:
> > > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> >
> > > > ret = platform_get_irq_optional(...);
> > > > if (ret < 0 && ret != -ENXIO)
> > > >   return ret;
> > > > if (ret > 0)
> > > >   ...we got it...
> > > >
> > > > It will allow the future API fix of platform_get_irq_optional() to be
> > > > really optional.
> > > >
> > > Later patch [0] (merged into -next) does check for -ENXIO first.
> > >
> > > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
> >
> > The problem is that it doesn't consider 0 as no IRQ.
> >
> Can you please point me to the discussion/patch where this API change
> is considered/discussed. Just to clarify now the new API for
> platform_get_irq_optional() will return "0" in case there is no
> interrupt and not not -ENXIO anymore?

The longest one happened here:
https://lore.kernel.org/linux-ide/20211209145937.77719-1-andriy.shevchenko@linux.intel.com/T/#u

It has links to some other discussions on the topic.

> When will this patch be merged for the new api, so that I can base my
> patches on top of it to avoid more changes?

You can simply imply that, I dunno when it gets merged (from my point
of view the users should be fixed first, and since you are adding
users, the burden is increasing).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Geert Uytterhoeven]

Hi Andy,

On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> > > <prabhakar.csengg@gmail.com> wrote:
> > > > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > >
> > > > > ret = platform_get_irq_optional(...);
> > > > > if (ret < 0 && ret != -ENXIO)
> > > > >   return ret;
> > > > > if (ret > 0)
> > > > >   ...we got it...
> > > > >
> > > > > It will allow the future API fix of platform_get_irq_optional() to be
> > > > > really optional.
> > > > >
> > > > Later patch [0] (merged into -next) does check for -ENXIO first.
> > > >
> > > > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
> > >
> > > The problem is that it doesn't consider 0 as no IRQ.
> > >
> > Can you please point me to the discussion/patch where this API change
> > is considered/discussed. Just to clarify now the new API for
> > platform_get_irq_optional() will return "0" in case there is no
> > interrupt and not not -ENXIO anymore?
>
> The longest one happened here:
> https://lore.kernel.org/linux-ide/20211209145937.77719-1-andriy.shevchenko@linux.intel.com/T/#u
>
> It has links to some other discussions on the topic.
>
> > When will this patch be merged for the new api, so that I can base my
> > patches on top of it to avoid more changes?
>
> You can simply imply that, I dunno when it gets merged (from my point
> of view the users should be fixed first, and since you are adding
> users, the burden is increasing).

Not only users (drivers), but also providers (architecture-specific code).
IRQ zero is still valid on some architectures, e.g. on SH[1].

[1] https://lore.kernel.org/linux-renesas-soc/CAMuHMdUg3=q7gyaVHP0XcYUOo3PQUUv8Hc8wp5faVQ+bTBpg4A@mail.gmail.com/

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Andy Shevchenko]

On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Andy,
>
> On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > <prabhakar.csengg@gmail.com> wrote:
> > > On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > >
> > > > > > ret = platform_get_irq_optional(...);
> > > > > > if (ret < 0 && ret != -ENXIO)
> > > > > >   return ret;
> > > > > > if (ret > 0)
> > > > > >   ...we got it...
> > > > > >
> > > > > > It will allow the future API fix of platform_get_irq_optional() to be
> > > > > > really optional.
> > > > > >
> > > > > Later patch [0] (merged into -next) does check for -ENXIO first.
> > > > >
> > > > > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
> > > >
> > > > The problem is that it doesn't consider 0 as no IRQ.
> > > >
> > > Can you please point me to the discussion/patch where this API change
> > > is considered/discussed. Just to clarify now the new API for
> > > platform_get_irq_optional() will return "0" in case there is no
> > > interrupt and not not -ENXIO anymore?
> >
> > The longest one happened here:
> > https://lore.kernel.org/linux-ide/20211209145937.77719-1-andriy.shevchenko@linux.intel.com/T/#u
> >
> > It has links to some other discussions on the topic.
> >
> > > When will this patch be merged for the new api, so that I can base my
> > > patches on top of it to avoid more changes?
> >
> > You can simply imply that, I dunno when it gets merged (from my point
> > of view the users should be fixed first, and since you are adding
> > users, the burden is increasing).
>
> Not only users (drivers), but also providers (architecture-specific code).
> IRQ zero is still valid on some architectures, e.g. on SH[1].

Are we talking about vIRQ?
And users are fine with a big warning?

My understanding is that the architecture code there is broken. It
needs to be fixed to use IRQ domains and all that machinery instead of
what it does.

0 is "no IRQ" in Linux.

> [1] https://lore.kernel.org/linux-renesas-soc/CAMuHMdUg3=q7gyaVHP0XcYUOo3PQUUv8Hc8wp5faVQ+bTBpg4A@mail.gmail.com/


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Geert Uytterhoeven]

Hi Andy,

On Mon, Dec 27, 2021 at 10:57 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > <prabhakar.csengg@gmail.com> wrote:
> > > > On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > > On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > >
> > > > > > > ret = platform_get_irq_optional(...);
> > > > > > > if (ret < 0 && ret != -ENXIO)
> > > > > > >   return ret;
> > > > > > > if (ret > 0)
> > > > > > >   ...we got it...
> > > > > > >
> > > > > > > It will allow the future API fix of platform_get_irq_optional() to be
> > > > > > > really optional.
> > > > > > >
> > > > > > Later patch [0] (merged into -next) does check for -ENXIO first.
> > > > > >
> > > > > > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
> > > > >
> > > > > The problem is that it doesn't consider 0 as no IRQ.
> > > > >
> > > > Can you please point me to the discussion/patch where this API change
> > > > is considered/discussed. Just to clarify now the new API for
> > > > platform_get_irq_optional() will return "0" in case there is no
> > > > interrupt and not not -ENXIO anymore?
> > >
> > > The longest one happened here:
> > > https://lore.kernel.org/linux-ide/20211209145937.77719-1-andriy.shevchenko@linux.intel.com/T/#u
> > >
> > > It has links to some other discussions on the topic.
> > >
> > > > When will this patch be merged for the new api, so that I can base my
> > > > patches on top of it to avoid more changes?
> > >
> > > You can simply imply that, I dunno when it gets merged (from my point
> > > of view the users should be fixed first, and since you are adding
> > > users, the burden is increasing).
> >
> > Not only users (drivers), but also providers (architecture-specific code).
> > IRQ zero is still valid on some architectures, e.g. on SH[1].
>
> Are we talking about vIRQ?
> And users are fine with a big warning?

The warning is only seen when a driver uses platorm_get_irq{,_optional}().
There are several other ways to obtain interrupts, avoiding the
big warning.

> My understanding is that the architecture code there is broken. It
> needs to be fixed to use IRQ domains and all that machinery instead of
> what it does.
>
> 0 is "no IRQ" in Linux.
>
> > [1] https://lore.kernel.org/linux-renesas-soc/CAMuHMdUg3=q7gyaVHP0XcYUOo3PQUUv8Hc8wp5faVQ+bTBpg4A@mail.gmail.com/

BTW, perhaps the IRQ subsystem should switch from integer IRQ numbers
to an opaque object, like was done for GPIO before? The IRQ subsystem
is one of the few (only?) subsystems still using plain integers. That
way we don't need this two-step platform_get_irq_optional() conversion
(step 1: check for both -ENXIO and zero, step 2: drop the check for
-ENXIO).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Andy Shevchenko]

On Mon, Dec 27, 2021 at 11:56 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> >
> > Hi Andy,
> >
> > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > <prabhakar.csengg@gmail.com> wrote:
> > > > On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > > On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > >
> > > > > > > ret = platform_get_irq_optional(...);
> > > > > > > if (ret < 0 && ret != -ENXIO)
> > > > > > >   return ret;
> > > > > > > if (ret > 0)
> > > > > > >   ...we got it...
> > > > > > >
> > > > > > > It will allow the future API fix of platform_get_irq_optional() to be
> > > > > > > really optional.
> > > > > > >
> > > > > > Later patch [0] (merged into -next) does check for -ENXIO first.
> > > > > >
> > > > > > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
> > > > >
> > > > > The problem is that it doesn't consider 0 as no IRQ.
> > > > >
> > > > Can you please point me to the discussion/patch where this API change
> > > > is considered/discussed. Just to clarify now the new API for
> > > > platform_get_irq_optional() will return "0" in case there is no
> > > > interrupt and not not -ENXIO anymore?
> > >
> > > The longest one happened here:
> > > https://lore.kernel.org/linux-ide/20211209145937.77719-1-andriy.shevchenko@linux.intel.com/T/#u
> > >
> > > It has links to some other discussions on the topic.
> > >
> > > > When will this patch be merged for the new api, so that I can base my
> > > > patches on top of it to avoid more changes?
> > >
> > > You can simply imply that, I dunno when it gets merged (from my point
> > > of view the users should be fixed first, and since you are adding
> > > users, the burden is increasing).
> >
> > Not only users (drivers), but also providers (architecture-specific code).
> > IRQ zero is still valid on some architectures, e.g. on SH[1].
>
> Are we talking about vIRQ?
> And users are fine with a big warning?
>
> My understanding is that the architecture code there is broken. It
> needs to be fixed to use IRQ domains and all that machinery instead of
> what it does.
>
> 0 is "no IRQ" in Linux.
>
> > [1] https://lore.kernel.org/linux-renesas-soc/CAMuHMdUg3=q7gyaVHP0XcYUOo3PQUUv8Hc8wp5faVQ+bTBpg4A@mail.gmail.com/

And to the point of the scope of this change, why should we obfuscate
the code in the case we know that it's not the case? You pointed out
to the ethernet driver. How does it related here?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Andy Shevchenko]

On Mon, Dec 27, 2021 at 12:02 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Dec 27, 2021 at 10:57 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
> > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> > > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > > > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > > >
> > > > > > > > ret = platform_get_irq_optional(...);
> > > > > > > > if (ret < 0 && ret != -ENXIO)
> > > > > > > >   return ret;
> > > > > > > > if (ret > 0)
> > > > > > > >   ...we got it...
> > > > > > > >
> > > > > > > > It will allow the future API fix of platform_get_irq_optional() to be
> > > > > > > > really optional.
> > > > > > > >
> > > > > > > Later patch [0] (merged into -next) does check for -ENXIO first.
> > > > > > >
> > > > > > > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
> > > > > >
> > > > > > The problem is that it doesn't consider 0 as no IRQ.
> > > > > >
> > > > > Can you please point me to the discussion/patch where this API change
> > > > > is considered/discussed. Just to clarify now the new API for
> > > > > platform_get_irq_optional() will return "0" in case there is no
> > > > > interrupt and not not -ENXIO anymore?
> > > >
> > > > The longest one happened here:
> > > > https://lore.kernel.org/linux-ide/20211209145937.77719-1-andriy.shevchenko@linux.intel.com/T/#u
> > > >
> > > > It has links to some other discussions on the topic.
> > > >
> > > > > When will this patch be merged for the new api, so that I can base my
> > > > > patches on top of it to avoid more changes?
> > > >
> > > > You can simply imply that, I dunno when it gets merged (from my point
> > > > of view the users should be fixed first, and since you are adding
> > > > users, the burden is increasing).
> > >
> > > Not only users (drivers), but also providers (architecture-specific code).
> > > IRQ zero is still valid on some architectures, e.g. on SH[1].
> >
> > Are we talking about vIRQ?
> > And users are fine with a big warning?
>
> The warning is only seen when a driver uses platorm_get_irq{,_optional}().
> There are several other ways to obtain interrupts, avoiding the
> big warning.
>
> > My understanding is that the architecture code there is broken. It
> > needs to be fixed to use IRQ domains and all that machinery instead of
> > what it does.
> >
> > 0 is "no IRQ" in Linux.
> >
> > > [1] https://lore.kernel.org/linux-renesas-soc/CAMuHMdUg3=q7gyaVHP0XcYUOo3PQUUv8Hc8wp5faVQ+bTBpg4A@mail.gmail.com/
>
> BTW, perhaps the IRQ subsystem should switch from integer IRQ numbers
> to an opaque object, like was done for GPIO before?

Hasn't it been done a long time ago?
IIRC somewhere in ~2014 timeframe.

> The IRQ subsystem
> is one of the few (only?) subsystems still using plain integers.

It uses opaque objects for which the plain integer is a unique key.

> That
> way we don't need this two-step platform_get_irq_optional() conversion
> (step 1: check for both -ENXIO and zero, step 2: drop the check for
> -ENXIO).


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Andy Shevchenko]

On Mon, Dec 27, 2021 at 12:02 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Andy,
>
> On Mon, Dec 27, 2021 at 10:57 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
> > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> > > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > > > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > > >
> > > > > > > > ret = platform_get_irq_optional(...);
> > > > > > > > if (ret < 0 && ret != -ENXIO)
> > > > > > > >   return ret;
> > > > > > > > if (ret > 0)
> > > > > > > >   ...we got it...
> > > > > > > >
> > > > > > > > It will allow the future API fix of platform_get_irq_optional() to be
> > > > > > > > really optional.
> > > > > > > >
> > > > > > > Later patch [0] (merged into -next) does check for -ENXIO first.
> > > > > > >
> > > > > > > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
> > > > > >
> > > > > > The problem is that it doesn't consider 0 as no IRQ.
> > > > > >
> > > > > Can you please point me to the discussion/patch where this API change
> > > > > is considered/discussed. Just to clarify now the new API for
> > > > > platform_get_irq_optional() will return "0" in case there is no
> > > > > interrupt and not not -ENXIO anymore?
> > > >
> > > > The longest one happened here:
> > > > https://lore.kernel.org/linux-ide/20211209145937.77719-1-andriy.shevchenko@linux.intel.com/T/#u
> > > >
> > > > It has links to some other discussions on the topic.
> > > >
> > > > > When will this patch be merged for the new api, so that I can base my
> > > > > patches on top of it to avoid more changes?
> > > >
> > > > You can simply imply that, I dunno when it gets merged (from my point
> > > > of view the users should be fixed first, and since you are adding
> > > > users, the burden is increasing).
> > >
> > > Not only users (drivers), but also providers (architecture-specific code).
> > > IRQ zero is still valid on some architectures, e.g. on SH[1].
> >
> > Are we talking about vIRQ?
> > And users are fine with a big warning?
>
> The warning is only seen when a driver uses platorm_get_irq{,_optional}().
> There are several other ways to obtain interrupts, avoiding the
> big warning.

Forgot to comment on this, then why is it a problem to allow
platfiorm_get_irq_optional() use 0 for no IRQ?
So, it seems you gave me a good justification for my way :-)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Geert Uytterhoeven]

Hi Andy,

On Mon, Dec 27, 2021 at 11:06 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Dec 27, 2021 at 12:02 PM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Mon, Dec 27, 2021 at 10:57 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> > > <geert@linux-m68k.org> wrote:
> > > > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
> > > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > > On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> > > > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > > > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > > > > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > > > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > > > >
> > > > > > > > > ret = platform_get_irq_optional(...);
> > > > > > > > > if (ret < 0 && ret != -ENXIO)
> > > > > > > > >   return ret;
> > > > > > > > > if (ret > 0)
> > > > > > > > >   ...we got it...
> > > > > > > > >
> > > > > > > > > It will allow the future API fix of platform_get_irq_optional() to be
> > > > > > > > > really optional.
> > > > > > > > >
> > > > > > > > Later patch [0] (merged into -next) does check for -ENXIO first.
> > > > > > > >
> > > > > > > > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
> > > > > > >
> > > > > > > The problem is that it doesn't consider 0 as no IRQ.
> > > > > > >
> > > > > > Can you please point me to the discussion/patch where this API change
> > > > > > is considered/discussed. Just to clarify now the new API for
> > > > > > platform_get_irq_optional() will return "0" in case there is no
> > > > > > interrupt and not not -ENXIO anymore?
> > > > >
> > > > > The longest one happened here:
> > > > > https://lore.kernel.org/linux-ide/20211209145937.77719-1-andriy.shevchenko@linux.intel.com/T/#u
> > > > >
> > > > > It has links to some other discussions on the topic.
> > > > >
> > > > > > When will this patch be merged for the new api, so that I can base my
> > > > > > patches on top of it to avoid more changes?
> > > > >
> > > > > You can simply imply that, I dunno when it gets merged (from my point
> > > > > of view the users should be fixed first, and since you are adding
> > > > > users, the burden is increasing).
> > > >
> > > > Not only users (drivers), but also providers (architecture-specific code).
> > > > IRQ zero is still valid on some architectures, e.g. on SH[1].
> > >
> > > Are we talking about vIRQ?
> > > And users are fine with a big warning?
> >
> > The warning is only seen when a driver uses platorm_get_irq{,_optional}().
> > There are several other ways to obtain interrupts, avoiding the
> > big warning.
> >
> > > My understanding is that the architecture code there is broken. It
> > > needs to be fixed to use IRQ domains and all that machinery instead of
> > > what it does.
> > >
> > > 0 is "no IRQ" in Linux.
> > >
> > > > [1] https://lore.kernel.org/linux-renesas-soc/CAMuHMdUg3=q7gyaVHP0XcYUOo3PQUUv8Hc8wp5faVQ+bTBpg4A@mail.gmail.com/
> >
> > BTW, perhaps the IRQ subsystem should switch from integer IRQ numbers
> > to an opaque object, like was done for GPIO before?
>
> Hasn't it been done a long time ago?
> IIRC somewhere in ~2014 timeframe.
>
> > The IRQ subsystem
> > is one of the few (only?) subsystems still using plain integers.
>
> It uses opaque objects for which the plain integer is a unique key.

And the key "cannot" be zero.

"cannot": that was the end goal, which is AFAIK still not reached, as we
still have IRQ zero is struct resource in platform files.

>
> > That
> > way we don't need this two-step platform_get_irq_optional() conversion
> > (step 1: check for both -ENXIO and zero, step 2: drop the check for
> > -ENXIO).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Geert Uytterhoeven]

Hi Andy,

On Mon, Dec 27, 2021 at 11:10 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Dec 27, 2021 at 12:02 PM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Mon, Dec 27, 2021 at 10:57 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> > > <geert@linux-m68k.org> wrote:
> > > > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > When will this patch be merged for the new api, so that I can base my
> > > > > > patches on top of it to avoid more changes?
> > > > >
> > > > > You can simply imply that, I dunno when it gets merged (from my point
> > > > > of view the users should be fixed first, and since you are adding
> > > > > users, the burden is increasing).
> > > >
> > > > Not only users (drivers), but also providers (architecture-specific code).
> > > > IRQ zero is still valid on some architectures, e.g. on SH[1].
> > >
> > > Are we talking about vIRQ?
> > > And users are fine with a big warning?
> >
> > The warning is only seen when a driver uses platorm_get_irq{,_optional}().
> > There are several other ways to obtain interrupts, avoiding the
> > big warning.
>
> Forgot to comment on this, then why is it a problem to allow
> platfiorm_get_irq_optional() use 0 for no IRQ?
> So, it seems you gave me a good justification for my way :-)

In se that is not a problem, assumed by now everybody should have
seen the warning, right?  Unfortunately that assumption is probably
not true, as people may not upgrade their kernel, cfr. my SH Ethernet
example.

Apart from that, any new conversion to platfiorm_get_irq_optional()
might cause a regression on an obscure platform still using IRQ0.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Andy Shevchenko]

On Mon, Dec 27, 2021 at 12:19 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Andy,
>
> On Mon, Dec 27, 2021 at 11:06 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Dec 27, 2021 at 12:02 PM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Mon, Dec 27, 2021 at 10:57 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> > > > <geert@linux-m68k.org> wrote:
> > > > > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > > On Sat, Dec 25, 2021 at 5:40 PM Andy Shevchenko
> > > > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > > > On Sat, Dec 25, 2021 at 7:28 PM Lad, Prabhakar
> > > > > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > > > > On Sat, Dec 25, 2021 at 4:46 PM Andy Shevchenko
> > > > > > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > > > > > On Thu, Dec 16, 2021 at 9:52 AM Lad Prabhakar
> > > > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > > > > >
> > > > > > > > > > ret = platform_get_irq_optional(...);
> > > > > > > > > > if (ret < 0 && ret != -ENXIO)
> > > > > > > > > >   return ret;
> > > > > > > > > > if (ret > 0)
> > > > > > > > > >   ...we got it...
> > > > > > > > > >
> > > > > > > > > > It will allow the future API fix of platform_get_irq_optional() to be
> > > > > > > > > > really optional.
> > > > > > > > > >
> > > > > > > > > Later patch [0] (merged into -next) does check for -ENXIO first.
> > > > > > > > >
> > > > > > > > > [0] https://lore.kernel.org/lkml/20211216182121.5323-1-prabhakar.mahadev-lad.rj@bp.renesas.com/t/
> > > > > > > >
> > > > > > > > The problem is that it doesn't consider 0 as no IRQ.
> > > > > > > >
> > > > > > > Can you please point me to the discussion/patch where this API change
> > > > > > > is considered/discussed. Just to clarify now the new API for
> > > > > > > platform_get_irq_optional() will return "0" in case there is no
> > > > > > > interrupt and not not -ENXIO anymore?
> > > > > >
> > > > > > The longest one happened here:
> > > > > > https://lore.kernel.org/linux-ide/20211209145937.77719-1-andriy.shevchenko@linux.intel.com/T/#u
> > > > > >
> > > > > > It has links to some other discussions on the topic.
> > > > > >
> > > > > > > When will this patch be merged for the new api, so that I can base my
> > > > > > > patches on top of it to avoid more changes?
> > > > > >
> > > > > > You can simply imply that, I dunno when it gets merged (from my point
> > > > > > of view the users should be fixed first, and since you are adding
> > > > > > users, the burden is increasing).
> > > > >
> > > > > Not only users (drivers), but also providers (architecture-specific code).
> > > > > IRQ zero is still valid on some architectures, e.g. on SH[1].
> > > >
> > > > Are we talking about vIRQ?
> > > > And users are fine with a big warning?
> > >
> > > The warning is only seen when a driver uses platorm_get_irq{,_optional}().
> > > There are several other ways to obtain interrupts, avoiding the
> > > big warning.
> > >
> > > > My understanding is that the architecture code there is broken. It
> > > > needs to be fixed to use IRQ domains and all that machinery instead of
> > > > what it does.
> > > >
> > > > 0 is "no IRQ" in Linux.
> > > >
> > > > > [1] https://lore.kernel.org/linux-renesas-soc/CAMuHMdUg3=q7gyaVHP0XcYUOo3PQUUv8Hc8wp5faVQ+bTBpg4A@mail.gmail.com/
> > >
> > > BTW, perhaps the IRQ subsystem should switch from integer IRQ numbers
> > > to an opaque object, like was done for GPIO before?
> >
> > Hasn't it been done a long time ago?
> > IIRC somewhere in ~2014 timeframe.
> >
> > > The IRQ subsystem
> > > is one of the few (only?) subsystems still using plain integers.
> >
> > It uses opaque objects for which the plain integer is a unique key.
>
> And the key "cannot" be zero.
>
> "cannot": that was the end goal, which is AFAIK still not reached,

What do you mean by that? The IRQ 0 is no IRQ in Linux, but the goal is reached.

> as we
> still have IRQ zero is struct resource in platform files.

This is not related to the above. The problem is that some (broken)
code is still in use. Maybe instead of obfuscating platform_get_irq*()
APIs we start fixing the root cause?

> > > That
> > > way we don't need this two-step platform_get_irq_optional() conversion
> > > (step 1: check for both -ENXIO and zero, step 2: drop the check for
> > > -ENXIO).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Andy Shevchenko]

On Mon, Dec 27, 2021 at 12:24 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Dec 27, 2021 at 11:10 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Dec 27, 2021 at 12:02 PM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Mon, Dec 27, 2021 at 10:57 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> > > > <geert@linux-m68k.org> wrote:
> > > > > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > > When will this patch be merged for the new api, so that I can base my
> > > > > > > patches on top of it to avoid more changes?
> > > > > >
> > > > > > You can simply imply that, I dunno when it gets merged (from my point
> > > > > > of view the users should be fixed first, and since you are adding
> > > > > > users, the burden is increasing).
> > > > >
> > > > > Not only users (drivers), but also providers (architecture-specific code).
> > > > > IRQ zero is still valid on some architectures, e.g. on SH[1].
> > > >
> > > > Are we talking about vIRQ?
> > > > And users are fine with a big warning?
> > >
> > > The warning is only seen when a driver uses platorm_get_irq{,_optional}().
> > > There are several other ways to obtain interrupts, avoiding the
> > > big warning.
> >
> > Forgot to comment on this, then why is it a problem to allow
> > platfiorm_get_irq_optional() use 0 for no IRQ?
> > So, it seems you gave me a good justification for my way :-)
>
> In se that is not a problem, assumed by now everybody should have
> seen the warning, right?  Unfortunately that assumption is probably
> not true, as people may not upgrade their kernel, cfr. my SH Ethernet
> example.
>
> Apart from that, any new conversion to platfiorm_get_irq_optional()
> might cause a regression on an obscure platform still using IRQ0.

What architectures?
Are there any examples besides ethernet drivers on SH?

Let's start  a list:
SH: only few cases related to smc911 Ethernet driver
x86: Legacy APIC 1:1 mapping, where 0 is used by timer which doesn't
involve platform API
...???...

And what about "getting IRQ without big warning"? What did you have in
mind when you put it?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Geert Uytterhoeven]

Hi Andy,

On Mon, Dec 27, 2021 at 11:49 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Dec 27, 2021 at 12:24 PM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Mon, Dec 27, 2021 at 11:10 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Mon, Dec 27, 2021 at 12:02 PM Geert Uytterhoeven
> > > <geert@linux-m68k.org> wrote:
> > > > On Mon, Dec 27, 2021 at 10:57 AM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > > On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> > > > > <geert@linux-m68k.org> wrote:
> > > > > > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > > > When will this patch be merged for the new api, so that I can base my
> > > > > > > > patches on top of it to avoid more changes?
> > > > > > >
> > > > > > > You can simply imply that, I dunno when it gets merged (from my point
> > > > > > > of view the users should be fixed first, and since you are adding
> > > > > > > users, the burden is increasing).
> > > > > >
> > > > > > Not only users (drivers), but also providers (architecture-specific code).
> > > > > > IRQ zero is still valid on some architectures, e.g. on SH[1].
> > > > >
> > > > > Are we talking about vIRQ?
> > > > > And users are fine with a big warning?
> > > >
> > > > The warning is only seen when a driver uses platorm_get_irq{,_optional}().
> > > > There are several other ways to obtain interrupts, avoiding the
> > > > big warning.
> > >
> > > Forgot to comment on this, then why is it a problem to allow
> > > platfiorm_get_irq_optional() use 0 for no IRQ?
> > > So, it seems you gave me a good justification for my way :-)
> >
> > In se that is not a problem, assumed by now everybody should have
> > seen the warning, right?  Unfortunately that assumption is probably
> > not true, as people may not upgrade their kernel, cfr. my SH Ethernet
> > example.
> >
> > Apart from that, any new conversion to platfiorm_get_irq_optional()
> > might cause a regression on an obscure platform still using IRQ0.
>
> What architectures?
> Are there any examples besides ethernet drivers on SH?

Sorry, I don't know.

> Let's start  a list:
> SH: only few cases related to smc911 Ethernet driver

Time to get rid of SH ;-)

> x86: Legacy APIC 1:1 mapping, where 0 is used by timer which doesn't
> involve platform API

Time to get rid of x86 ;-)

> ...???...
>
> And what about "getting IRQ without big warning"? What did you have in
> mind when you put it?

If the driver uses platform_get_resource(..., IORESOURCE_IRQ, ...) to
get the IRQ number, the warning in platform_get_irq_optional()
doesn't trigger, as the latter is not called?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] irqchip/renesas-irqc: Use platform_get_irq_optional() to get the interrupt

[Andy Shevchenko]

On Mon, Dec 27, 2021 at 12:58 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Dec 27, 2021 at 11:49 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Dec 27, 2021 at 12:24 PM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Mon, Dec 27, 2021 at 11:10 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Mon, Dec 27, 2021 at 12:02 PM Geert Uytterhoeven
> > > > <geert@linux-m68k.org> wrote:
> > > > > On Mon, Dec 27, 2021 at 10:57 AM Andy Shevchenko
> > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > On Mon, Dec 27, 2021 at 11:45 AM Geert Uytterhoeven
> > > > > > <geert@linux-m68k.org> wrote:
> > > > > > > On Sun, Dec 26, 2021 at 9:49 AM Andy Shevchenko
> > > > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > > > On Sun, Dec 26, 2021 at 1:59 AM Lad, Prabhakar
> > > > > > > > <prabhakar.csengg@gmail.com> wrote:
> > > > > > > > > When will this patch be merged for the new api, so that I can base my
> > > > > > > > > patches on top of it to avoid more changes?
> > > > > > > >
> > > > > > > > You can simply imply that, I dunno when it gets merged (from my point
> > > > > > > > of view the users should be fixed first, and since you are adding
> > > > > > > > users, the burden is increasing).
> > > > > > >
> > > > > > > Not only users (drivers), but also providers (architecture-specific code).
> > > > > > > IRQ zero is still valid on some architectures, e.g. on SH[1].
> > > > > >
> > > > > > Are we talking about vIRQ?
> > > > > > And users are fine with a big warning?
> > > > >
> > > > > The warning is only seen when a driver uses platorm_get_irq{,_optional}().
> > > > > There are several other ways to obtain interrupts, avoiding the
> > > > > big warning.
> > > >
> > > > Forgot to comment on this, then why is it a problem to allow
> > > > platfiorm_get_irq_optional() use 0 for no IRQ?
> > > > So, it seems you gave me a good justification for my way :-)
> > >
> > > In se that is not a problem, assumed by now everybody should have
> > > seen the warning, right?  Unfortunately that assumption is probably
> > > not true, as people may not upgrade their kernel, cfr. my SH Ethernet
> > > example.
> > >
> > > Apart from that, any new conversion to platfiorm_get_irq_optional()
> > > might cause a regression on an obscure platform still using IRQ0.
> >
> > What architectures?
> > Are there any examples besides ethernet drivers on SH?
>
> Sorry, I don't know.
>
> > Let's start  a list:
> > SH: only few cases related to smc911 Ethernet driver
>
> Time to get rid of SH ;-)
>
> > x86: Legacy APIC 1:1 mapping, where 0 is used by timer which doesn't
> > involve platform API
>
> Time to get rid of x86 ;-)

Why (in both cases)? I have mentioned that x86 doesn't use vIRQ0 via
platform APIs, SH can be fixed, if you are interested in fixing, of
course, by switching to IRQ domains.

> > ...???...
> >
> > And what about "getting IRQ without big warning"? What did you have in
> > mind when you put it?
>
> If the driver uses platform_get_resource(..., IORESOURCE_IRQ, ...) to
> get the IRQ number, the warning in platform_get_irq_optional()
> doesn't trigger, as the latter is not called?

So, again, let's put the comment to those drivers then and avoid
obfuscation of the rest of the kernel, would it be feasible?

-- 
With Best Regards,
Andy Shevchenko