* Re: [PATCH] drivers: acpi: fix GIC irq model default PCI IRQ polarity
2016-09-05 14:12 [PATCH] drivers: acpi: fix GIC irq model default PCI IRQ polarity Lorenzo Pieralisi
@ 2016-09-05 14:32 ` Marc Zyngier
2016-09-05 20:37 ` Rafael J. Wysocki
2016-09-06 6:16 ` Duc Dang
2 siblings, 0 replies; 5+ messages in thread
From: Marc Zyngier @ 2016-09-05 14:32 UTC (permalink / raw)
To: Lorenzo Pieralisi, linux-pci, linux-acpi
Cc: linux-arm-kernel, Punit Agrawal, Duc Dang, Bjorn Helgaas,
Sinan Kaya, Rafael J. Wysocki
On 05/09/16 15:12, Lorenzo Pieralisi wrote:
> On ACPI ARM based systems the GIC interrupt controller
> and corresponding interrupt model permit only the high
> polarity for level interrupts.
>
> ACPI firmware describes PCI legacy IRQs through entries
> in the _PRT objects. Entries in the _PRT can be of two types:
>
> - Static: not configurable, trigger/polarity default to level-low,
> _PRT entry defines the global GSI interrupt number
> - Configurable: _PRT interrupt entry contains a reference to the
> corresponding PCI interrupt link device (that in turn provides the
> interrupt descriptor through its _CRS/_PRS methods)
>
> Configurable IRQ entries are not currently allowed by the ACPI
> specification on ARM, since they can only be used for interrupt
> pins that are routable, and ARM platforms GIC configurations
> do not allow dynamic IRQ routing, routing is statically laid out
> at synthesis time; therefore PCI interrupt links cannot be used
> for PCI legacy IRQ descriptions in the _PRT on ARM systems.
>
> On the other hand, current core ACPI code handling PCI legacy IRQs
> consider IRQ trigger/polarity for static _PRT entries as level-low.
>
> On ARM systems with a GIC interrupt controller and corresponding
> ACPI interrupt model this does not work in that GIC interrupt
> controller is only capable of handling level interrupts whose
> polarity is high (for PCI legacy IRQs - that are level-low by
> specification - this means that the legacy IRQs are inverted before
> reaching the interrupt controller pin), resulting in IRQ allocation
> failures such as:
>
> genirq: Setting trigger mode 8 for irq 18 failed (gic_set_type+0x0/0x48)
>
> Change the default polarity for PCI legacy IRQs to high on systems
> booting wth ACPI on platforms with a GIC interrupt controller model,
> fixing the discrepancy between specification and HW behaviour.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Duc Dang <dhdang@apm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Sinan Kaya <okaya@codeaurora.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
> drivers/acpi/pci_irq.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
> index 2c45dd3..c576a6f 100644
> --- a/drivers/acpi/pci_irq.c
> +++ b/drivers/acpi/pci_irq.c
> @@ -411,7 +411,15 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
> int gsi;
> u8 pin;
> int triggering = ACPI_LEVEL_SENSITIVE;
> - int polarity = ACPI_ACTIVE_LOW;
> + /*
> + * On ARM systems with the GIC interrupt model, level interrupts
> + * are always polarity high by specification; PCI legacy
> + * IRQs lines are inverted before reaching the interrupt
> + * controller and must therefore be considered active high
> + * as default.
> + */
> + int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC ?
> + ACPI_ACTIVE_HIGH : ACPI_ACTIVE_LOW;
> char *link = NULL;
> char link_desc[16];
> int rc;
>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: acpi: fix GIC irq model default PCI IRQ polarity
2016-09-05 14:12 [PATCH] drivers: acpi: fix GIC irq model default PCI IRQ polarity Lorenzo Pieralisi
2016-09-05 14:32 ` Marc Zyngier
@ 2016-09-05 20:37 ` Rafael J. Wysocki
2016-09-06 15:11 ` Bjorn Helgaas
2016-09-06 6:16 ` Duc Dang
2 siblings, 1 reply; 5+ messages in thread
From: Rafael J. Wysocki @ 2016-09-05 20:37 UTC (permalink / raw)
To: Lorenzo Pieralisi, Bjorn Helgaas
Cc: Punit Agrawal, Marc Zyngier, Linux PCI, Duc Dang,
Rafael J. Wysocki, Sinan Kaya, ACPI Devel Maling List,
linux-arm-kernel@lists.infradead.org
On Mon, Sep 5, 2016 at 4:12 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On ACPI ARM based systems the GIC interrupt controller
> and corresponding interrupt model permit only the high
> polarity for level interrupts.
>
> ACPI firmware describes PCI legacy IRQs through entries
> in the _PRT objects. Entries in the _PRT can be of two types:
>
> - Static: not configurable, trigger/polarity default to level-low,
> _PRT entry defines the global GSI interrupt number
> - Configurable: _PRT interrupt entry contains a reference to the
> corresponding PCI interrupt link device (that in turn provides the
> interrupt descriptor through its _CRS/_PRS methods)
>
> Configurable IRQ entries are not currently allowed by the ACPI
> specification on ARM, since they can only be used for interrupt
> pins that are routable, and ARM platforms GIC configurations
> do not allow dynamic IRQ routing, routing is statically laid out
> at synthesis time; therefore PCI interrupt links cannot be used
> for PCI legacy IRQ descriptions in the _PRT on ARM systems.
>
> On the other hand, current core ACPI code handling PCI legacy IRQs
> consider IRQ trigger/polarity for static _PRT entries as level-low.
>
> On ARM systems with a GIC interrupt controller and corresponding
> ACPI interrupt model this does not work in that GIC interrupt
> controller is only capable of handling level interrupts whose
> polarity is high (for PCI legacy IRQs - that are level-low by
> specification - this means that the legacy IRQs are inverted before
> reaching the interrupt controller pin), resulting in IRQ allocation
> failures such as:
>
> genirq: Setting trigger mode 8 for irq 18 failed (gic_set_type+0x0/0x48)
>
> Change the default polarity for PCI legacy IRQs to high on systems
> booting wth ACPI on platforms with a GIC interrupt controller model,
> fixing the discrepancy between specification and HW behaviour.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Duc Dang <dhdang@apm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Sinan Kaya <okaya@codeaurora.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
> drivers/acpi/pci_irq.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
> index 2c45dd3..c576a6f 100644
> --- a/drivers/acpi/pci_irq.c
> +++ b/drivers/acpi/pci_irq.c
> @@ -411,7 +411,15 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
> int gsi;
> u8 pin;
> int triggering = ACPI_LEVEL_SENSITIVE;
> - int polarity = ACPI_ACTIVE_LOW;
> + /*
> + * On ARM systems with the GIC interrupt model, level interrupts
> + * are always polarity high by specification; PCI legacy
> + * IRQs lines are inverted before reaching the interrupt
> + * controller and must therefore be considered active high
> + * as default.
> + */
> + int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC ?
> + ACPI_ACTIVE_HIGH : ACPI_ACTIVE_LOW;
> char *link = NULL;
> char link_desc[16];
> int rc;
> --
Bjorn, any objections?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: acpi: fix GIC irq model default PCI IRQ polarity
2016-09-05 20:37 ` Rafael J. Wysocki
@ 2016-09-06 15:11 ` Bjorn Helgaas
0 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2016-09-06 15:11 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Lorenzo Pieralisi, Bjorn Helgaas, Punit Agrawal, Marc Zyngier,
Linux PCI, Duc Dang, Rafael J. Wysocki, Sinan Kaya,
ACPI Devel Maling List, linux-arm-kernel@lists.infradead.org
On Mon, Sep 05, 2016 at 10:37:29PM +0200, Rafael J. Wysocki wrote:
> On Mon, Sep 5, 2016 at 4:12 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On ACPI ARM based systems the GIC interrupt controller
> > and corresponding interrupt model permit only the high
> > polarity for level interrupts.
> >
> > ACPI firmware describes PCI legacy IRQs through entries
> > in the _PRT objects. Entries in the _PRT can be of two types:
> >
> > - Static: not configurable, trigger/polarity default to level-low,
> > _PRT entry defines the global GSI interrupt number
> > - Configurable: _PRT interrupt entry contains a reference to the
> > corresponding PCI interrupt link device (that in turn provides the
> > interrupt descriptor through its _CRS/_PRS methods)
> >
> > Configurable IRQ entries are not currently allowed by the ACPI
> > specification on ARM, since they can only be used for interrupt
> > pins that are routable, and ARM platforms GIC configurations
> > do not allow dynamic IRQ routing, routing is statically laid out
> > at synthesis time; therefore PCI interrupt links cannot be used
> > for PCI legacy IRQ descriptions in the _PRT on ARM systems.
> >
> > On the other hand, current core ACPI code handling PCI legacy IRQs
> > consider IRQ trigger/polarity for static _PRT entries as level-low.
> >
> > On ARM systems with a GIC interrupt controller and corresponding
> > ACPI interrupt model this does not work in that GIC interrupt
> > controller is only capable of handling level interrupts whose
> > polarity is high (for PCI legacy IRQs - that are level-low by
> > specification - this means that the legacy IRQs are inverted before
> > reaching the interrupt controller pin), resulting in IRQ allocation
> > failures such as:
> >
> > genirq: Setting trigger mode 8 for irq 18 failed (gic_set_type+0x0/0x48)
> >
> > Change the default polarity for PCI legacy IRQs to high on systems
> > booting wth ACPI on platforms with a GIC interrupt controller model,
> > fixing the discrepancy between specification and HW behaviour.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Punit Agrawal <punit.agrawal@arm.com>
> > Cc: Duc Dang <dhdang@apm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Sinan Kaya <okaya@codeaurora.org>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> > drivers/acpi/pci_irq.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
> > index 2c45dd3..c576a6f 100644
> > --- a/drivers/acpi/pci_irq.c
> > +++ b/drivers/acpi/pci_irq.c
> > @@ -411,7 +411,15 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
> > int gsi;
> > u8 pin;
> > int triggering = ACPI_LEVEL_SENSITIVE;
> > - int polarity = ACPI_ACTIVE_LOW;
> > + /*
> > + * On ARM systems with the GIC interrupt model, level interrupts
> > + * are always polarity high by specification; PCI legacy
> > + * IRQs lines are inverted before reaching the interrupt
> > + * controller and must therefore be considered active high
> > + * as default.
> > + */
> > + int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC ?
> > + ACPI_ACTIVE_HIGH : ACPI_ACTIVE_LOW;
> > char *link = NULL;
> > char link_desc[16];
> > int rc;
> > --
>
> Bjorn, any objections?
Looks OK to me.
I never did figure out exactly what in the ACPI spec prohibited the
use of interrupt links, so a spec reference there would be nice.
Bjorn
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: acpi: fix GIC irq model default PCI IRQ polarity
2016-09-05 14:12 [PATCH] drivers: acpi: fix GIC irq model default PCI IRQ polarity Lorenzo Pieralisi
2016-09-05 14:32 ` Marc Zyngier
2016-09-05 20:37 ` Rafael J. Wysocki
@ 2016-09-06 6:16 ` Duc Dang
2 siblings, 0 replies; 5+ messages in thread
From: Duc Dang @ 2016-09-06 6:16 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-pci, linux-acpi, linux-arm, Punit Agrawal, Bjorn Helgaas,
Sinan Kaya, Rafael J. Wysocki, Marc Zyngier
On Mon, Sep 5, 2016 at 7:12 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On ACPI ARM based systems the GIC interrupt controller
> and corresponding interrupt model permit only the high
> polarity for level interrupts.
>
> ACPI firmware describes PCI legacy IRQs through entries
> in the _PRT objects. Entries in the _PRT can be of two types:
>
> - Static: not configurable, trigger/polarity default to level-low,
> _PRT entry defines the global GSI interrupt number
> - Configurable: _PRT interrupt entry contains a reference to the
> corresponding PCI interrupt link device (that in turn provides the
> interrupt descriptor through its _CRS/_PRS methods)
>
> Configurable IRQ entries are not currently allowed by the ACPI
> specification on ARM, since they can only be used for interrupt
> pins that are routable, and ARM platforms GIC configurations
> do not allow dynamic IRQ routing, routing is statically laid out
> at synthesis time; therefore PCI interrupt links cannot be used
> for PCI legacy IRQ descriptions in the _PRT on ARM systems.
>
> On the other hand, current core ACPI code handling PCI legacy IRQs
> consider IRQ trigger/polarity for static _PRT entries as level-low.
>
> On ARM systems with a GIC interrupt controller and corresponding
> ACPI interrupt model this does not work in that GIC interrupt
> controller is only capable of handling level interrupts whose
> polarity is high (for PCI legacy IRQs - that are level-low by
> specification - this means that the legacy IRQs are inverted before
> reaching the interrupt controller pin), resulting in IRQ allocation
> failures such as:
>
> genirq: Setting trigger mode 8 for irq 18 failed (gic_set_type+0x0/0x48)
>
> Change the default polarity for PCI legacy IRQs to high on systems
> booting wth ACPI on platforms with a GIC interrupt controller model,
> fixing the discrepancy between specification and HW behaviour.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Duc Dang <dhdang@apm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Sinan Kaya <okaya@codeaurora.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
> drivers/acpi/pci_irq.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
> index 2c45dd3..c576a6f 100644
> --- a/drivers/acpi/pci_irq.c
> +++ b/drivers/acpi/pci_irq.c
> @@ -411,7 +411,15 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
> int gsi;
> u8 pin;
> int triggering = ACPI_LEVEL_SENSITIVE;
> - int polarity = ACPI_ACTIVE_LOW;
> + /*
> + * On ARM systems with the GIC interrupt model, level interrupts
> + * are always polarity high by specification; PCI legacy
> + * IRQs lines are inverted before reaching the interrupt
> + * controller and must therefore be considered active high
> + * as default.
> + */
> + int polarity = acpi_irq_model == ACPI_IRQ_MODEL_GIC ?
> + ACPI_ACTIVE_HIGH : ACPI_ACTIVE_LOW;
> char *link = NULL;
> char link_desc[16];
> int rc;
Tested-by: Duc Dang <dhdang@apm.com>
> --
> 2.6.4
>
Regards,
Duc Dang.
^ permalink raw reply [flat|nested] 5+ messages in thread