On Sat, Jun 12, 2021 at 01:57:31PM -0600, Clayton Casciato wrote:
> Fixed coding style issue.
I don't object to this, and in fact, I prefer the style you propose.
I don't know whether Rafael thinks it's worth the churn since it
doesn't actually fix anything.
But do take note of the commit log conventions. Run
$ git log --oneline drivers/acpi/pci_irq.c
and match the style. Probably something like this:
ACPI: PCI: Simplify acpi_reroute_boot_interrupt() coding style
And note that the commit log should use imperative mood and be a
little more specific about what the commit does. [1] has several
great hints.
More below.
> Signed-off-by: Clayton Casciato <majortomtosourcecontrol@xxxxxxxxx>
> ---
> drivers/acpi/pci_irq.c | 45 +++++++++++++++++++++---------------------
> 1 file changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
> index 08e15774fb9f..6eea3cf7b158 100644
> --- a/drivers/acpi/pci_irq.c
> +++ b/drivers/acpi/pci_irq.c
> @@ -260,30 +260,29 @@ static int bridge_has_boot_interrupt_variant(struct pci_bus *bus)
> static int acpi_reroute_boot_interrupt(struct pci_dev *dev,
> struct acpi_prt_entry *entry)
> {
> - if (noioapicquirk || noioapicreroute) {
> + if (noioapicquirk || noioapicreroute)
> return 0;
> - } else {
> - switch (bridge_has_boot_interrupt_variant(dev->bus)) {
> - case 0:
> - /* no rerouting necessary */
> - return 0;
> - case INTEL_IRQ_REROUTE_VARIANT:
> - /*
> - * Remap according to INTx routing table in 6700PXH
> - * specs, intel order number 302628-002, section
> - * 2.15.2. Other chipsets (80332, ...) have the same
> - * mapping and are handled here as well.
> - */
> - dev_info(&dev->dev, "PCI IRQ %d -> rerouted to legacy "
> - "IRQ %d\n", entry->index,
> - (entry->index % 4) + 16);
> - entry->index = (entry->index % 4) + 16;
> - return 1;
> - default:
> - dev_warn(&dev->dev, "Cannot reroute IRQ %d to legacy "
> - "IRQ: unknown mapping\n", entry->index);
> - return -1;
> - }
> +
> + switch (bridge_has_boot_interrupt_variant(dev->bus)) {
> + case 0:
> + /* no rerouting necessary */
> + return 0;
> + case INTEL_IRQ_REROUTE_VARIANT:
> + /*
> + * Remap according to INTx routing table in 6700PXH
> + * specs, intel order number 302628-002, section
> + * 2.15.2. Other chipsets (80332, ...) have the same
> + * mapping and are handled here as well.
> + */
> + dev_info(&dev->dev, "PCI IRQ %d -> rerouted to legacy "
> + "IRQ %d\n", entry->index,
> + (entry->index % 4) + 16);
> + entry->index = (entry->index % 4) + 16;
> + return 1;
> + default:
> + dev_warn(&dev->dev, "Cannot reroute IRQ %d to legacy "
> + "IRQ: unknown mapping\n", entry->index);
> + return -1;
Looking at this a little closer, this was added by e1d3a90846b4 ("pci,
acpi: reroute PCI interrupt to legacy boot interrupt equivalent") [2].
It looks like it might be a little over-engineered. It added
MAX_IRQ_REROUTE_VARIANTS, which is an indication that
irq_reroute_variant is a 2-bit bitfield. But in the last 13 years
we've never needed more than the single INTEL_IRQ_REROUTE_VARIANT bit.
So I would consider reworking it like this:
unsigned int intel_irq_reroute:1;
static void acpi_reroute_boot_interrupt(...)
{
if (noioapicquirk || noioapicreroute)
return;
if (bridge_has_intel_irq_reroute(dev->bus)) {
dev_info(..., "PCI IRQ %d -> rerouted ...");
entry->index = (entry->index % 4) + 16;
}
}
[1] https://chris.beams.io/posts/git-commit/
[2] https://git.kernel.org/linus/e1d3a90846b4
> }
> }
> #endif /* CONFIG_X86_IO_APIC */
> --
> 2.31.1
>
