On Fri, Jan 28, 2022 at 01:36:53PM -0800, Dave Hansen wrote: > On 1/28/22 12:48, Brent Spillner wrote: > > It seems like the multiline string literal is your main pain point--- would > > > > +#ifdef CONFIG_ACPI > > + if (acpi_noirq) > > + msg = "; consider removing acpi=noirq"; > > + else > > + msg = "; recommend verifying UEFI/BIOS > > IRQ options"; > > +#else > > + msg = "; recommend verifying UEFI/BIOS IRQ > > options or enabling ACPI"; > > +#endif > > > > be OK without going to IS_ENABLED()? (Personally, I think the #ifdef > > style is more readable.) > > I think that's _better_ than what was in the patch. But, even with it, > I still think the #ifdef mess borders on unreadable. > > But, if Bjorn likes it, then go for it. :) I was hoping to avoid commenting at all ;) I'm a little bit averse to suggesting *any* command-line options because users shouldn't have to use options like these. It would be better if we got a bug report and could fix the bug or add a quirk to work around a firmware issue automatically. If a user finds a command-line option that "works," the problem is solved as far as they are concerned, but it doesn't help the next person who trips over it. I think pci_acpi_init() should warn when "acpi=noirq" was used because the only reason to use it should be to work around a firmware defect, and ideally, we could make a quirk to do that. Maybe the doc should suggest a bug report. What does "verifying UEFI/BIOS IRQ options" mean? I could go look at the BIOS setup menu, but I would have no idea what to look for, so the only thing I could do would be to try randomly changing IRQ-related things. If that happens to hit on a working configuration, I might be happy, but it wouldn't help the next person at all. I'd rather see a bug report. Then we could at least try to make a quirk so no command line option would be needed. Bjorn
