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

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:
>> > Hi Adrian, Rob,
>> > 
>> > 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...)

*shrug* You're the arch maintainer, you're welcome to change it. You can define
it to be as brittle as possible if you like. Me, I read "worse is better" long
ago (https://dreamsongs.com/RiseOfWorseIsBetter.html) and The Unix Philosophy by
Mike Gancarz (when it breaks, you get to keep the pieces) and generally try to
leave it up to the user what to do about things. I've also used a LOT of
secondhand hardware over the years that got repurposed after it stopped being
useful for its original task. (Right to repair and all that.)

But it's not my call...

Rob