Re: Defining polarity and trigger mode for static interrupts in _PRT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Marc Zyngier <marc.zyngier@xxxxxxx> writes:

> On Wed, 24 Aug 2016 14:30:00 -0500
> Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>
>> On Wed, Aug 24, 2016 at 03:27:23PM +0100, Lorenzo Pieralisi wrote:
>> > [ +Bjorn, Punit]
>> > 
>> > On Wed, Aug 24, 2016 at 04:06:13AM -0700, Duc Dang wrote:  
>> > > [Resend in plain text mode]
>> > > 
>> > > Hi Lorenzo, Rafael,
>> > > 
>> > > ACPI 6.1 spec does not specify how to set interrupt polarity and
>> > > trigger mode in _PRT when the interrupts are static (hardwired to
>> > > specific interrupt inputs in interrupt controller). In current
>> > > acpi_pci_irq_enable (drivers/acpi/pci_irq.c) implementation, by
>> > > default the trigger mode is set to LEVEL_SENSITIVE, polarity is set to
>> > > ACTIVE_LOW. This default setting won't work for ARM64 GICv2, GICv2m,
>> > > GICv3 controllers and will cause failures in PCIe AER, PME services
>> > > (on X-Gene platforms).  
>> 
>> PCI (not PCIe) r3.0, sec 2.2.6, says "Interrupts on PCI are optional
>> and defined as 'level sensitive,' asserted low."
>> 
>> I've heard before that ARM64 does this differently, but I still don't
>> understand the difference.  Obviously if you plug a legacy PCI card
>> into an ARM64 system, it's still going to pull INTA# low to assert an
>> interrupt.  So is there something special about ARM64 that inverts
>> that, or what?
>
> There is certainly an inverter somewhere on the interrupt path, because
> the GIC triggers on level high, not level low. But I don't think that's
> the issue Duc is trying to outline here, because that's not something
> SW can fix. I'm worried that in his system, the interrupt is edge
> triggered instead. 
>
>> 
>> > > Is there any way to specify polarity and trigger mode for static
>> > > interrupts in _PRT?   
>> 
>> There is no way I'm aware of in _PRT to specify polarity and trigger
>> mode.  I don't know the history, but my guess is that it would be seen
>> as superfluous given that the PCI spec requires level, active low.
>> 
>> Obviously I'm missing something important.
>
> Same here, unless the HW is not PCI compliant...

I had faced this issue on Juno r2[0] a few months back - though AFAICS,
it wasn't preventing anything to work but printed an annoying message on
boot.

[ 1.353696] genirq: Setting trigger mode 8 for irq 9 failed (gic_set_type+0x0/0x5c)
[ 1.478286] genirq: Setting trigger mode 8 for irq 17 failed (gic_set_type+0x0/0x5c)

The ACPI code in the kernel (drivers/acpi/pci_irq.c) is behaving as per
spec, so nothing to be done there IMHO. The problem arises due to the
integration of two mismatched components - PCI is level low and the GIC
supports only level high - making it necessary to introduce glue
elements like the inverter.

This would all be OK if ACPI had a mechanism to specify the interrupt
type (trigger and polarity). As an alternative, for Juno I created a
link device (as Lorenzo suggests) to provide this information to the
kernel.

With this fix the warnings went away and I suspect this will address
Duc's issue as well. But that is playing naughty with the spec (ACPI 6.1
Section. 6.2.13).

If there are no good reason to restrict using link devices to
configurable interrupts, perhaps the spec needs an update.

Perhaps Rafael knows why is there a restriction on using link devices
for fixed interrupts in the ACPI spec...

[0] https://lists.linaro.org/pipermail/linaro-acpi/2016-May/006880.html

>
> Thanks,
>
> 	M.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux