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

[Bjorn Helgaas <helgaas@kernel.org>]

On Mon, Mar 14, 2016 at 04:37:51PM -0400, Sinan Kaya wrote:
> Hi Bjorn,
> 
> On 3/14/2016 2:52 PM, Bjorn Helgaas wrote:
> >> bool acpi_isa_irq_available(int irq)
> >> > @@ -840,13 +881,6 @@ bool acpi_isa_irq_available(int irq)
> >> >   */
> >> >  void acpi_penalize_sci_irq(int irq, int trigger, int polarity)
> >> >  {
> >> > -	if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty)) {
> >> > -		if (trigger != ACPI_MADT_TRIGGER_LEVEL ||
> >> > -		    polarity != ACPI_MADT_POLARITY_ACTIVE_LOW)
> >> > -			acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_ALWAYS;
> >> > -		else
> >> > -			acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING;
> >> > -	}
> > I think we lost the validation of trigger mode and polarity, didn't
> > we?
> > 
> 
> This function gets called to inform ACPI that this is the SCI interrupt
> and, trigger and polarity are their attributes.
> 
> The return value is void and the caller is not interested in what ACPI thinks
> about. 
> 
> This function adjusts the SCI penalty based on correct attributes passed
> (ISA_ALWAYS vs. PCI_USING).
> 
> I agree that we lost this validation. 
> 
> I can keep sci_trigger/sci_polarity somewhere and keep that into the calculation
> in get function.
> 
> Like this for instance,
> 
> 	if (irq == acpi_gbl_FADT.sci_interrupt) {
> +		if (sci_trigger != ACPI_MADT_TRIGGER_LEVEL ||
> +		    sci_polarity != ACPI_MADT_POLARITY_ACTIVE_LOW)
> +			penalty += PIRQ_PENALTY_ISA_ALWAYS;
> +		else
> 			penalty += PIRQ_PENALTY_PCI_USING; 
> 	}
> 
> Then, we can't get rid of the function just we can reduce the contents.

I think it's important to keep that check.

I raised the possibility of using irq_get_trigger_type() for all IRQs
(not just the SCI).  Did you have a chance to look into that at all?

Bjorn