Re: [PATCH 1/4] acpi,pci,irq: reduce resource requirements

[Sinan Kaya]

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