* [PATCH] acpi: pci_irq: Fixed a control flow style issue
@ 2021-06-12 19:57 Clayton Casciato
2021-06-16 20:56 ` Bjorn Helgaas
0 siblings, 1 reply; 2+ messages in thread
From: Clayton Casciato @ 2021-06-12 19:57 UTC (permalink / raw)
To: bhelgaas; +Cc: rjw, lenb, linux-pci, Clayton Casciato
Fixed coding style issue.
Signed-off-by: Clayton Casciato <majortomtosourcecontrol@gmail.com>
---
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;
}
}
#endif /* CONFIG_X86_IO_APIC */
--
2.31.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] acpi: pci_irq: Fixed a control flow style issue
2021-06-12 19:57 [PATCH] acpi: pci_irq: Fixed a control flow style issue Clayton Casciato
@ 2021-06-16 20:56 ` Bjorn Helgaas
0 siblings, 0 replies; 2+ messages in thread
From: Bjorn Helgaas @ 2021-06-16 20:56 UTC (permalink / raw)
To: Clayton Casciato; +Cc: bhelgaas, rjw, lenb, linux-pci
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@gmail.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-06-16 20:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-12 19:57 [PATCH] acpi: pci_irq: Fixed a control flow style issue Clayton Casciato
2021-06-16 20:56 ` Bjorn Helgaas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.