linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: simon.guinot@sequanux.org (Simon Guinot)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] genirq: move mask_cache into struct irq_chip_type
Date: Mon, 4 Mar 2013 16:44:54 +0100	[thread overview]
Message-ID: <20130304154454.GG21620@kw.sim.vm.gnt> (raw)
In-Reply-To: <5134AF7E.5010605@keymile.com>

On Mon, Mar 04, 2013 at 03:28:14PM +0100, Gerlando Falauto wrote:
> Hi everyone,
> 
> I know this issue is from two years ago...
> 
> On 07/27/2011 10:45 AM, saeed bishara wrote:
> >On Tue, Jul 26, 2011 at 6:39 PM, Simon Guinot <simon@sequanux.org> wrote:
> >>Hi Saeed,
> >>
> >>On Tue, Jul 26, 2011 at 05:35:50PM +0300, saeed bishara wrote:
> >>>On Fri, Jul 22, 2011 at 3:49 AM, Simon Guinot <simon@sequanux.org> wrote:
> >>>>From: Simon Guinot <sguinot@lacie.com>
> >>>>
> >>>>This fixes a regression introduced by e59347a
> >>>>"arm: orion: Use generic irq chip".
> >>>>
> >>>>The same interrupt mask cache (stored within struct irq_chip_generic)
> >>>>is shared between all the irq_chip_type instances. As each irq_chip_type
> >>>>can use a distinct mask register, share a single mask cache is not
> >>>>correct. This bug affects Orion SoCs, which have separate mask registers
> >>>>for edge and level interrupts.
> >>>>
> >>>>This patch move mask_cache from struct irq_chip_generic into struct
> >>>>irq_chip_type. Note that the interrupt support for Samsung SoCs is also
> >>>>slightly affected.
> >>>The patch looks to fix the issue with orion, but it seems that it
> >>>won't work for SoC with multiple irq_chip_type that use one mask
> >>>register.
> >>
> >>Yes indeed, but does such SoCs exists ? I mean that the only supported
> >>SoC using multiple irq_chip_type (for now) is Orion.
> >thats right, but as you code is generic, we should find some way to
> >fix it or to prevent such devices to use this generic code.
> >  What is the most
> >>generic case for edge/level interrupts ? shared or separated mask
> >>registers ?
> >orion gpios use separate mask.
> >>
> >>If Orion is the specific case, maybe we could provide a dedicated
> >>irq_mask() handler instead of using the generic one. It would be a
> >>little sad.
> >I personally prefer this method, along with getting rid off the
> >multiple irq_chip_types, my main consideration in this case is
> >performance.
> >the irq_gc_mask_clr_bit/irq_gc_ack_set_bit/irq_gc_unmask_enable_reg
> >are critical since it get called for every (level) interrupt, and it
> >would be great if you optimize those functions,
> >I think you better do the following:
> >1. use pre-calculated offsets instead using  gc->reg_base + cur_regs(d)->ack.
> >2. put all hot variables (lock, mask/ack/eoi register offset) in the
> >same cache line if possible.
> >
> >regards
> >saeed
> 
> I ran into the same problem (currently running 3.0.40)... but it
> looks like this patch was never applied anywhere.
> I also quickly scanned the log between now and then and could not
> find any reference to this problem. Was a fix ever committed or we
> still have this regression?

Hi Gerlando,

AFAIK this issue is still here. I include the current Orion maintainers
in the Cc list.

Regards,

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130304/0fabd282/attachment-0001.sig>

  reply	other threads:[~2013-03-04 15:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-19 19:32 plat-orion gpio regression for mixed level and edge sensitive IRQs Joey Oravec
2011-07-20 23:45 ` Simon Guinot
2011-07-22  0:49   ` [PATCH] genirq: move mask_cache into struct irq_chip_type Simon Guinot
2011-07-26 14:11     ` Simon Guinot
2011-07-26 14:35     ` saeed bishara
2011-07-26 15:39       ` Simon Guinot
2011-07-27  8:45         ` saeed bishara
2013-03-04 14:28           ` Gerlando Falauto
2013-03-04 15:44             ` Simon Guinot [this message]
2013-03-04 17:20               ` Gerlando Falauto
2013-03-05 10:15               ` Thomas Gleixner
2013-03-06 13:29                 ` Simon Guinot
2013-03-06 15:19                   ` Thomas Gleixner
2013-03-11 15:40                     ` Simon Guinot
2013-03-11 21:08                       ` Thomas Gleixner

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=20130304154454.GG21620@kw.sim.vm.gnt \
    --to=simon.guinot@sequanux.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).