Re: [PATCH] Fix J-core aic warning spam

Hi Rob,

On Tue, Apr 18, 2023 at 10:59 AM Rob Landley <[email protected]> wrote:
> On 4/18/23 03:10, John Paul Adrian Glaubitz wrote:
> > On Tue, 2023-04-18 at 03:09 -0500, Rob Landley wrote:
> >> On 4/18/23 02:18, Geert Uytterhoeven wrote:
> >> > On Tue, Apr 18, 2023 at 8:19 AM John Paul Adrian Glaubitz
> >> > <[email protected]> wrote:
> >> > > On Mon, 2023-04-17 at 23:23 -0500, Rob Landley wrote:
> >> > > > From: Rich Felker <[email protected]>
> >> > > > Signed-off-by: Rob Landley <[email protected]>
> >> > > >
> >> > > > Silence noisy boot messages (warning and stack dump for each IRQ) when booting
> >> > > > on J2 SOC.
> >> >
> >> > > > --- a/drivers/irqchip/irq-jcore-aic.c
> >> > > > +++ b/drivers/irqchip/irq-jcore-aic.c
> >> > > > @@ -68,6 +68,7 @@ static int __init aic_irq_of_init(struct device_node *node,
> >> > > >       unsigned min_irq = JCORE_AIC2_MIN_HWIRQ;
> >> > > >       unsigned dom_sz = JCORE_AIC_MAX_HWIRQ+1;
> >> > > >       struct irq_domain *domain;
> >> > > > +     int rc;
> >> > > >
> >> > > >       pr_info("Initializing J-Core AIC\n");
> >> > > >
> >> > > > @@ -100,6 +101,11 @@ static int __init aic_irq_of_init(struct device_node *node,
> >> > > >       jcore_aic.irq_unmask = noop;
> >> > > >       jcore_aic.name = "AIC";
> >> > > >
> >> > > > +     rc = irq_alloc_descs(min_irq, min_irq, dom_sz - min_irq,
> >> > > > +                          of_node_to_nid(node));
> >> > > > +     if (rc < 0)
> >> > > > +             pr_info("Cannot allocate irq_descs @ IRQ%d, assuming pre-allocated\n",
> >> > > > +                     min_irq);
> >> >
> >> > This is a fatal error, so please bail out, instead of continuing.
> >>
> >> If it can continue, it's not a fatal error. (Some pieces of hardware might not
> >> come up, but the board might still be usable.) If it can't continue, how does
> >> the _type_ of failure matter?
> >
> > I would still consider it fatal if any of the integral board components failed to
> > initialize. I don't think we want users to boot up their system into such an undefined
> > state.
>
> So if the network card doesn't work, kernel panic? If it's fatal, why does the
> function return? It could have called panic() instead. How does panicing _help_?
> (If the driver loads and the hardware works, we're good. If it doesn't, it won't
> work and they'll notice...)

I didn't suggest to call panic(), just return rc.

Diving deeper, irq_alloc_descs() can fail only when passing bad or severely
out-of-range values, so that's very unlikely.

BTW, what are the noisy boot messages? What's the call chain?

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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