diff mbox

[V2] acpi, pci, irq: account for early penalty assignment

Message ID 56D7ABFB.3070302@codeaurora.org
State Superseded
Headers show

Commit Message

Sinan Kaya March 3, 2016, 3:14 a.m. UTC 
On 3/2/2016 1:31 PM, Sinan Kaya wrote:
> I don't think there's a restriction on what the SCI IRQ can be.  But
>> there is only one SCI IRQ, so all we have to do is keep track of what
>> it is, which only requires one word.

Taking a step back here and also some inspiration from your code, why don't we
fix the actual problem instead of redesigning the whole thing?

The code failed because we didn't account for the non-ISA SCI IRQs. Below code
does this.

Comments

Sinan Kaya March 3, 2016, 2:48 p.m. UTC | #1 
Taking another stab at it.

On 3/2/2016 10:14 PM, Sinan Kaya wrote:
> Taking a step back here and also some inspiration from your code, why don't we
> fix the actual problem instead of redesigning the whole thing?

I read your email multiple times. I think you want to move the x86 specific pieces
(ISA interrupts and its command line arguments) out of the drivers\acpi\pci_link.c.
Is this right?

- the legacy ISA IRQs, i.e., the contents of acpi_irq_isa_penalty[]
- acpi_irq_isa= from command line

 int pcibios_irq_penalty(int irq)
  {
    if (irq >= ACPI_MAX_ISA_IRQ)
      return 0;

    return acpi_irq_isa_penalty[irq] + acpi_irq_cmd_line_penalty[irq];
  }
Bjorn Helgaas March 3, 2016, 3:10 p.m. UTC | #2 
On Thu, Mar 03, 2016 at 09:48:09AM -0500, Sinan Kaya wrote:
> Taking another stab at it.
> 
> On 3/2/2016 10:14 PM, Sinan Kaya wrote:
> > Taking a step back here and also some inspiration from your code, why don't we
> > fix the actual problem instead of redesigning the whole thing?
> 
> I read your email multiple times. I think you want to move the x86 specific pieces
> (ISA interrupts and its command line arguments) out of the drivers\acpi\pci_link.c.
> Is this right?

That was my idea, but your minimal patch from last night looks awfully
attractive, and maybe it's not worth moving it to arch/x86.  I do think we
could simplify the code significantly by getting rid of the kzalloc and
acpi_irq_penalty_list from acpi_irq_set_penalty().  How about pushing on
that a little bit first, and see what it looks like then?

> - the legacy ISA IRQs, i.e., the contents of acpi_irq_isa_penalty[]
> - acpi_irq_isa= from command line
> 
>  int pcibios_irq_penalty(int irq)
>   {
>     if (irq >= ACPI_MAX_ISA_IRQ)
>       return 0;
> 
>     return acpi_irq_isa_penalty[irq] + acpi_irq_cmd_line_penalty[irq];
>   }
> 
> 
> -- 
> Sinan Kaya
> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sinan Kaya March 3, 2016, 3:12 p.m. UTC | #3 
On 3/3/2016 10:10 AM, Bjorn Helgaas wrote:
> That was my idea, but your minimal patch from last night looks awfully
> attractive, and maybe it's not worth moving it to arch/x86.  I do think we
> could simplify the code significantly by getting rid of the kzalloc and
> acpi_irq_penalty_list from acpi_irq_set_penalty().  How about pushing on
> that a little bit first, and see what it looks like then?

OK. Let me go that direction.
diff mbox

Patch

diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index fa28635..99d0716 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -87,6 +87,7 @@  struct acpi_pci_link {
 
 static LIST_HEAD(acpi_link_list);
 static DEFINE_MUTEX(acpi_link_lock);
+static int sci_irq, sci_irq_penalty;
 
 /* --------------------------------------------------------------------------
                             PCI Link Device Management
@@ -481,6 +482,9 @@  static int acpi_irq_get_penalty(int irq)
 	if (irq < ACPI_MAX_ISA_IRQ)
 		return acpi_irq_isa_penalty[irq];
 
+	if (irq == sci_irq)
+		return sci_penalty;
+
 	list_for_each_entry(irq_info, &acpi_irq_penalty_list, node) {
 		if (irq_info->irq == irq)
 			return irq_info->penalty;
@@ -507,6 +511,11 @@  static int acpi_irq_set_penalty(int irq, int new_penalty)
 		}
 	}
 
+	if (irq == sci_irq) {
+		sci_penalty = penalty;
+		return 0;
+	}
+
 	/* nope, let's allocate a slot for this IRQ */
 	irq_info = kzalloc(sizeof(*irq_info), GFP_KERNEL);
 	if (!irq_info)
@@ -900,6 +909,7 @@  void acpi_penalize_sci_irq(int irq, int trigger, int polarity)
 	if (irq < 0)
 		return;
 
+	sci_irq = irq;
 	if (trigger != ACPI_MADT_TRIGGER_LEVEL ||
 	    polarity != ACPI_MADT_POLARITY_ACTIVE_LOW)
 		penalty = PIRQ_PENALTY_ISA_ALWAYS;