Hi Bjorn,

>>>
>>> Device(LN0A){
>>>     Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link
>>>     Name(_UID, 1)
>>>     Name(_PRS, ResourceTemplate(){
>>>     Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive, , ,) {0x123}
>>>     })
>>
>> I forgot that the link already include the trigger mode in it.  Maybe we
>> can check for that instead of assuming level/low.
>>
> 
> Let me explore this area.
> 
>> Bjorn

I have been implementing this. 

I see that placing the check in allocate function is not right. The interrupts 
get
registered with ACPI subsystem after locating the interrupt in register_gsi 
function.
Trying to find the IRQ type with get_trigger_type() for an unregistered 
interrupt
didn't look right to me.

I think it is best if we move these checks into get_penalty function.

I also see that the code is currently allowing EDGE type PCI interrupts 
assuming they are
exclusive only. Let me if this looks alright. I'm also curious about the PCI 
checks.

static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)

I know you want to validate that all PCI interrupts are level, it looks like 
the code
is allowing other combinations.

diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index 85be579e..5056a9c 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -495,6 +495,45 @@ static int acpi_irq_pci_sharing_penalty(int irq)
        return penalty;
 }
 
+static int acpi_link_trigger(int irq, u8 *polarity, u8 *triggering)
+{
+       struct acpi_pci_link *link;
+       int link_trigger = 0;
+
+       list_for_each_entry(link, &acpi_link_list, list) {
+               int i;
+
+               if (link->irq.active && link->irq.active == irq) {
+                       *polarity = link->polarity;
+                       *triggering = link->triggering;
+                       return 0;
+               }
+
+               for (i = 0; i < link->irq.possible_count; i++)
+                       if (link->irq.possible[i] == irq) {
+                               *polarity = link->polarity;
+                               *triggering = link->triggering;
+                               return 0;
+                       }
+       }
+
+       return -EINVAL;
+}
+
+static bool acpi_pci_compatible_trigger(int irq)
+{
+       u8 polarity;
+       u8 triggering;
+       int type = irq_get_trigger_type(irq);
+       int rc;
+
+       rc = acpi_link_trigger(irq, &polarity, &triggering, &sharing)
+       if (rc)
+               return false;
+
+       return (type == IRQ_TYPE_LEVEL_LOW || type == IRQ_TYPE_NONE);
+}
+
 static int acpi_irq_get_penalty(int irq)
 {
        int penalty = 0;
@@ -502,8 +541,21 @@ static int acpi_irq_get_penalty(int irq)
        if (irq < ACPI_MAX_ISA_IRQ)
                penalty += acpi_irq_isa_penalty[irq];
 
-       if (irq == acpi_gbl_FADT.sci_interrupt)
-               penalty += PIRQ_PENALTY_PCI_USING;
+       if (irq == acpi_gbl_FADT.sci_interrupt) {
+               u8 sci_polarity;
+               u8 sci_trigger;
+
+               rc = acpi_link_trigger(irq, &polarity, &sci_trigger);
+               if (!rc) {
+                       if (sci_trigger != ACPI_LEVEL_SENSITIVE ||
+                           sci_polarity != ACPI_ACTIVE_LOW)
+                               penalty += PIRQ_PENALTY_ISA_ALWAYS;
+                       else
+                               penalty += PIRQ_PENALTY_PCI_USING;
+               } else {
+                       return ~0;
+               }
+       }
 
        penalty += acpi_irq_pci_sharing_penalty(irq);
        return penalty; 


-- 
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

Reply via email to