linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: helgaas@kernel.org (Bjorn Helgaas)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2 1/3] Revert "ACPI,PCI,IRQ: reduce static IRQ array size to 16"
Date: Thu, 13 Oct 2016 15:02:39 -0500	[thread overview]
Message-ID: <20161013200238.GE21529@localhost> (raw)
In-Reply-To: <498097a5-f75c-d2dd-f134-148b3fb8e1ed@codeaurora.org>

On Thu, Oct 13, 2016 at 03:36:11PM -0400, Sinan Kaya wrote:
> On 10/13/2016 2:15 PM, Bjorn Helgaas wrote:
> > It seems like the problem is that we removed acpi_penalize_sci_irq(),
> > which told us the polarity and trigger mode.  We tried to get that
> > information via irq_get_trigger_type(), but that didn't work in this
> > case because we use the acpi_irq_get_penalty() path before the SCI is
> > registered.
> > 
> > It makes sense to me to add acpi_penalize_sci_irq() back in, which is
> > what patch [3/3] does.
> > 
> > I don't understand how *this* patch, which basically just increases
> > the penalty array size from 16 to 256, helps fix the problem.  It
> > seems like this patch should only matter if the SCI were some IRQ
> > between 16 and 255.
> 
> 
> I see your point. The original code supported 256 interrupts. 
> 
> The machine where we had the problem had an SCI interrupt of 11. So,
> this patch does not necessarily fix anything for this machine alone.
> However, to be safe; I wanted to go back to the old behavior to fix
> the SCI issue for all existing platforms.

I saw a previous email that said the SCI interrupt could not be
greater than 256, but I don't know where that restriction is.  I'm
pretty sure the FADT field is 2 bytes, which would mean it could be up
to 65535.

To fix this problem, I think we only need to fix the penalty for the
SCI interrupt.  It seems better to add a single "sci_penalty"
variable, set it to PIRQ_PENALTY_PCI_USING if it's level/low or
PIRQ_PENALTY_ISA_ALWAYS otherwise, and add "sci_penalty" in when
appropriate.  That should fix it for *any* SCI IRQ, not just those
less than 256, and we don't have to add these extra penalty table
entries that are all unused (except possibly for one entry if we have
an SCI in the 16-255 range).

Something like this:

  static int sci_irq, sci_penalty;

  void acpi_penalize_sci_irq(int irq, int trigger, int polarity)
  {
    sci = irq;
    if (trigger == ACPI_MADT_TRIGGER_LEVEL &&
        polarity == ACPI_MADT_POLARITY_ACTIVE_LOW)
      sci_penalty = PIRQ_PENALTY_PCI_USING;
    else
      sci_penalty = PIRQ_PENALTY_ISA_ALWAYS;
  }

  static int acpi_irq_get_penalty(int irq)
  {
    int penalty = 0;

    if (irq == sci_irq)
      penalty += sci_penalty;
    ...
  }

One could argue that ACPI devices can use IRQs above 15, and we should
handle penalties for them, too.  But the table is the wrong mechanism
for that, because it handles penalties for IRQs < 256, but IRQs above
that would mysteriously be handled differently.

Bjorn

  reply	other threads:[~2016-10-13 20:02 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-04 21:15 [PATCH V2 0/3] ACPI,PCI,IRQ: revert penalty calculation for SCI Sinan Kaya
2016-10-04 21:15 ` [PATCH V2 1/3] Revert "ACPI, PCI, IRQ: reduce static IRQ array size to 16" Sinan Kaya
2016-10-12 22:13   ` [PATCH V2 1/3] Revert "ACPI,PCI,IRQ: " Bjorn Helgaas
2016-10-12 22:46     ` [PATCH V2 1/3] Revert "ACPI, PCI, IRQ: " Sinan Kaya
2016-10-13 18:15       ` [PATCH V2 1/3] Revert "ACPI,PCI,IRQ: " Bjorn Helgaas
2016-10-13 19:36         ` [PATCH V2 1/3] Revert "ACPI, PCI, IRQ: " Sinan Kaya
2016-10-13 20:02           ` Bjorn Helgaas [this message]
2016-10-13 20:16             ` Sinan Kaya
2016-10-13 21:01               ` Rafael J. Wysocki
2016-10-15  3:50       ` Sinan Kaya
2016-10-04 21:15 ` [PATCH V2 2/3] ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts Sinan Kaya
2016-10-12 19:07   ` Bjorn Helgaas
2016-10-04 21:15 ` [PATCH V2 3/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" Sinan Kaya
2016-10-05 13:38   ` Thomas Gleixner
2016-10-13 18:25   ` Bjorn Helgaas
2016-10-11 21:55 ` [PATCH V2 0/3] ACPI,PCI,IRQ: revert penalty calculation for SCI Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161013200238.GE21529@localhost \
    --to=helgaas@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).