On Wed, Nov 09, 2016 at 12:05:08PM -0200, Guilherme G. Piccoli wrote:
On PowerPC machines some PCI slots might not have Level-triggered
interrupts capability (also know as Level Signaled Interrupts - LSI),
leading of_irq_parse_pci() to complain by presenting error messages
on the kernel log - in this case, the properties "interrupt-map" and
"interrupt-map-mask" are not present on the device's node on device
tree.
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?
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.
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.
Thanks,
Mark.
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.
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...
quoted
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
quoted
interrupts capabilities, we saw generic error messages like this:
[54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-
22
quoted
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
quoted
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.
Cheers,
Ben.
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:
quoted
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.
quoted
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.
quoted
quoted
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
quoted
interrupts capabilities, we saw generic error messages like this:
[54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-
22
quoted
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
quoted
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.
On Fri, 2016-11-11 at 16:32 +0000, Mark Rutland wrote:
On Fri, Nov 11, 2016 at 08:30:43AM +1100, Benjamin Herrenschmidt
wrote:
quoted
On Wed, 2016-11-09 at 19:04 +0000, Mark Rutland wrote:
quoted
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.
Works for me.
Cheers,
Ben.