Re: [PATCH] of/irq: improve error message on irq discovery process failure

[Mark Rutland <mark.rutland@arm.com>]

On Fri, Nov 11, 2016 at 08:30:43AM +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2016-11-09 at 19:04 +0000, Mark Rutland wrote:
> > 
> > If we don't have an interrupt-map on a PCI controller, why don't we
> > instead log a message regarding that being missing, and give up
> > early?
> 
> Why ? It's legit to not support LSIs.

Sure; I had envisioned a message like:

	pr_info("%s: no interrupt-map, INTx interrupts not possible\n",
		pci_controller_name);

... Which tells the user exaclty what we know, and doesn't imply either
an error or the actual absence of HW support.

> > That sounds like a more generically useful error message; it's also
> > possible that a DT author simply forgot to add the map, and the
> > platform has suitable interrupts wired up.
> 
> But it's not necessarily an error...

Sure, "error" was a misnomer.

> > > This patch introduces a different message for this specific case,
> > > and it also reduces the level of the message from error to warning.
> > > Before this patch, when an adapter was plugged in a slot without
> > Level
> > > interrupts capabilities, we saw generic error messages like this:
> > > 
> > >     [54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-
> > 22
> > > 
> > > Now, with this applied, we see the following specific message:
> > > 
> > >     [19.947] pci 0014:60:00.0: of_irq_parse_pci() gave up. The slot
> > of this
> > >     device has no Level-triggered Interrupts capability.
> > 
> > Following my above example, this has gone from opaque to potentially
> > misleading
> 
> I'm not sure. At least for some of our platforms this is the correct
> message :-) Our Hypervisor doesn't allow LSIs on some slots.
> 
> I think it's not that misleading. It's obvious something is wrong with
> LSIs, which you can easily figure out from there.

As above, I think it's clearer to log that there's no interrupt-map for
the controller. 

Orthogonal to that, "INTx" is a more generally understood name for the
legacy wired PCI interrupts, and is probably preferable in generic code.

Thanks,
Mark.