Re: [PATCH] ipmi: si: Use platform_get_irq() to retrieve interrupt

[Corey Minyard <corey@minyard.net>]

On Wed, Jun 03, 2026 at 04:18:57PM -0700, Rosen Penev wrote:
> On Wed, Jun 3, 2026 at 4:17 PM Corey Minyard <corey@minyard.net> wrote:
> >
> > On Wed, Jun 03, 2026 at 04:05:54PM -0700, Rosen Penev wrote:
> > > On Wed, Jun 3, 2026 at 3:57 PM Corey Minyard <corey@minyard.net> wrote:
> > > >
> > > > On Wed, Jun 03, 2026 at 05:53:56PM -0500, Corey Minyard wrote:
> > > > > On Wed, Jun 03, 2026 at 12:25:11PM -0700, Rosen Penev wrote:
> > > > > > Use platform_get_irq() to retrieve the interrupt resource instead of
> > > > > > directly parsing and mapping the OF node via irq_of_parse_and_map().
> > > > > > This is the standard pattern for platform devices.
> > > > > > irq_of_parse_and_map() requires ire_dispose_mapping(), which is missing.
> > > > > >
> > > > > > Assisted-by: Antigravity:Gemini-3.5-Flash
> > > > > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > > > > ---
> > > > > >  drivers/char/ipmi/ipmi_si_platform.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c
> > > > > > index fb6e359ae494..e10b5d8af092 100644
> > > > > > --- a/drivers/char/ipmi/ipmi_si_platform.c
> > > > > > +++ b/drivers/char/ipmi/ipmi_si_platform.c
> > > > > > @@ -276,7 +276,7 @@ static int of_ipmi_probe(struct platform_device *pdev)
> > > > > >     io.regspacing   = regspacing ? be32_to_cpup(regspacing) : DEFAULT_REGSPACING;
> > > > > >     io.regshift     = regshift ? be32_to_cpup(regshift) : 0;
> > > > > >
> > > > > > -   io.irq          = irq_of_parse_and_map(pdev->dev.of_node, 0);
> > > > > > +   io.irq          = platform_get_irq(pdev, 0);
> > > > >
> > > > > This should be something like:
> > > > >
> > > > >       io.irq = platform_get_irq_optional(pdev, 0);
> > > > >       if (io.irq > 0)
> > > > >               io.irq_setup = ipmi_std_irq_setup;
> > > > >       else
> > > > >               io.irq = 0;
> > > > >
> > > > > right?
> > > >
> > > > Oops, cut and paste error, try:
> > > >
> > > >         io.irq = platform_get_irq_optional(pdev, 0);
> > > >         if (io.irq < 0)
> > > >                 io.irq = 0;
> > > I don't think that if is appropriate.
> >
> > Can platform_get_irq[_optional]() return < 0?  The driver still has to
> > work even if there is no interrupt specified.
> It can return -EPROBE_DEFER.
> 
> No idea how likely in this driver.

Well, I have to handle a negative return value here, the comments above
platform_get_irq_optional() say so.  The value is optional.  So it has
to be the above.

I'll modify the patch to be what I posted above.

> >
> > -corey
> >
> > > >
> > > > This just disables the interrupt if it can't get it.
> > > >
> > > > >
> > > > > -corey
> > > > >
> > > > > >     io.dev          = &pdev->dev;
> > > > > >
> > > > > >     dev_dbg(&pdev->dev, "addr 0x%lx regsize %d spacing %d irq %d\n",
> > > > > > --
> > > > > > 2.54.0
> > > > > >