From: Thomas Gleixner <tglx@linutronix.de>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: Eric Anholt <eric@anholt.net>,
linux-arm-kernel@lists.infradead.org,
linux-rpi-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Lee Jones <lee@kernel.org>,
devicetree@vger.kernel.org, Jason Cooper <jason@lakedaemon.net>,
Andrea Merello <andrea.merello@gmail.com>
Subject: Re: [PATCH 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.
Date: Sat, 11 Jul 2015 09:51:08 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.11.1507110940420.17353@nanos> (raw)
In-Reply-To: <55A0A5EB.4090007@wwwdotorg.org>
On Fri, 10 Jul 2015, Stephen Warren wrote:
> On 07/07/2015 03:13 PM, Eric Anholt wrote:
> > +static struct arm_local_intc intc __read_mostly;
>
> It'd be nice to give everything (types, functions, variables) a
> consistent symbol prefix; bcm2836_arm_irqchip_ sounds like a good
> bikeshed to me, but perhaps just propagating the above arm_local_ to the
> functions too would be good, although that seems to risk symbol name
> collisions with other ARM SoCs.
Which is irrelevant because its static.
> > +static void bcm2836_mask_per_cpu_irq(unsigned int reg, unsigned int bit)
> > +{
> > + void __iomem *reg_base = intc.base + reg;
> > + unsigned int i;
> > +
> > + for (i = 0; i < 4; i++)
>
> Is "4" there the CPU count? Perhaps this should use one of the Linux
> APIs to query the CPU count rather than hard-coding it?
>
> Should per-CPU IRQs automatically be masked on all CPUs at once, or only
> on the current CPU? A very quick look at the ARM GIC driver implies it
> doesn't iterate over all CPUs when masking per-CPU IRQs.
Usually per cpu interrupts are only masked on the cpu which is calling
the function. The whole reason why per cpu interrupts exist is that
you can share the same interrupt number for all cores.
So masking all interrupts is not a good idea. In this case if a cpu is
hot unplugged, then all other cpus would not longer get timer
interrupts. Not what you really want, right?
> > +static void bcm2836_mask_gpu_irq(struct irq_data *d)
> > +{
> > +}
> > +
> > +static void bcm2836_unmask_gpu_irq(struct irq_data *d)
> > +{
> > +}
>
> If the IRQs can't be masked, should these functions actually be implemented?
We have a few places in the core which expect at least mask/unmask to
be implemented.
Thanks,
tglx
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
To: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>,
Andrea Merello
<andrea.merello-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.
Date: Sat, 11 Jul 2015 09:51:08 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.11.1507110940420.17353@nanos> (raw)
In-Reply-To: <55A0A5EB.4090007-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
On Fri, 10 Jul 2015, Stephen Warren wrote:
> On 07/07/2015 03:13 PM, Eric Anholt wrote:
> > +static struct arm_local_intc intc __read_mostly;
>
> It'd be nice to give everything (types, functions, variables) a
> consistent symbol prefix; bcm2836_arm_irqchip_ sounds like a good
> bikeshed to me, but perhaps just propagating the above arm_local_ to the
> functions too would be good, although that seems to risk symbol name
> collisions with other ARM SoCs.
Which is irrelevant because its static.
> > +static void bcm2836_mask_per_cpu_irq(unsigned int reg, unsigned int bit)
> > +{
> > + void __iomem *reg_base = intc.base + reg;
> > + unsigned int i;
> > +
> > + for (i = 0; i < 4; i++)
>
> Is "4" there the CPU count? Perhaps this should use one of the Linux
> APIs to query the CPU count rather than hard-coding it?
>
> Should per-CPU IRQs automatically be masked on all CPUs at once, or only
> on the current CPU? A very quick look at the ARM GIC driver implies it
> doesn't iterate over all CPUs when masking per-CPU IRQs.
Usually per cpu interrupts are only masked on the cpu which is calling
the function. The whole reason why per cpu interrupts exist is that
you can share the same interrupt number for all cores.
So masking all interrupts is not a good idea. In this case if a cpu is
hot unplugged, then all other cpus would not longer get timer
interrupts. Not what you really want, right?
> > +static void bcm2836_mask_gpu_irq(struct irq_data *d)
> > +{
> > +}
> > +
> > +static void bcm2836_unmask_gpu_irq(struct irq_data *d)
> > +{
> > +}
>
> If the IRQs can't be masked, should these functions actually be implemented?
We have a few places in the core which expect at least mask/unmask to
be implemented.
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: tglx@linutronix.de (Thomas Gleixner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.
Date: Sat, 11 Jul 2015 09:51:08 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.11.1507110940420.17353@nanos> (raw)
In-Reply-To: <55A0A5EB.4090007@wwwdotorg.org>
On Fri, 10 Jul 2015, Stephen Warren wrote:
> On 07/07/2015 03:13 PM, Eric Anholt wrote:
> > +static struct arm_local_intc intc __read_mostly;
>
> It'd be nice to give everything (types, functions, variables) a
> consistent symbol prefix; bcm2836_arm_irqchip_ sounds like a good
> bikeshed to me, but perhaps just propagating the above arm_local_ to the
> functions too would be good, although that seems to risk symbol name
> collisions with other ARM SoCs.
Which is irrelevant because its static.
> > +static void bcm2836_mask_per_cpu_irq(unsigned int reg, unsigned int bit)
> > +{
> > + void __iomem *reg_base = intc.base + reg;
> > + unsigned int i;
> > +
> > + for (i = 0; i < 4; i++)
>
> Is "4" there the CPU count? Perhaps this should use one of the Linux
> APIs to query the CPU count rather than hard-coding it?
>
> Should per-CPU IRQs automatically be masked on all CPUs at once, or only
> on the current CPU? A very quick look at the ARM GIC driver implies it
> doesn't iterate over all CPUs when masking per-CPU IRQs.
Usually per cpu interrupts are only masked on the cpu which is calling
the function. The whole reason why per cpu interrupts exist is that
you can share the same interrupt number for all cores.
So masking all interrupts is not a good idea. In this case if a cpu is
hot unplugged, then all other cpus would not longer get timer
interrupts. Not what you really want, right?
> > +static void bcm2836_mask_gpu_irq(struct irq_data *d)
> > +{
> > +}
> > +
> > +static void bcm2836_unmask_gpu_irq(struct irq_data *d)
> > +{
> > +}
>
> If the IRQs can't be masked, should these functions actually be implemented?
We have a few places in the core which expect at least mask/unmask to
be implemented.
Thanks,
tglx
next prev parent reply other threads:[~2015-07-11 7:51 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-07 21:13 Raspberry Pi 2 support, part 1: Interrupt controller Eric Anholt
2015-07-07 21:13 ` Eric Anholt
2015-07-07 21:13 ` Eric Anholt
2015-07-07 21:13 ` [PATCH 1/4] irqchip: bcm2835: Refactor handle_IRQ() calls out of MAKE_HWIRQ Eric Anholt
2015-07-07 21:13 ` Eric Anholt
2015-07-07 21:13 ` Eric Anholt
2015-07-07 21:13 ` [PATCH 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it Eric Anholt
2015-07-07 21:13 ` Eric Anholt
2015-07-07 21:13 ` [PATCH 3/4] irqchip: Add documentation for the bcm2836 interrupt controller Eric Anholt
2015-07-07 21:13 ` Eric Anholt
2015-07-07 21:13 ` Eric Anholt
2015-07-11 4:57 ` Stephen Warren
2015-07-11 4:57 ` Stephen Warren
2015-07-11 4:57 ` Stephen Warren
2015-07-11 6:01 ` Eric Anholt
2015-07-11 6:01 ` Eric Anholt
2015-07-11 6:01 ` Eric Anholt
2015-07-14 5:08 ` Stephen Warren
2015-07-14 5:08 ` Stephen Warren
2015-07-14 5:08 ` Stephen Warren
2015-07-07 21:13 ` [PATCH 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2 Eric Anholt
2015-07-07 21:13 ` Eric Anholt
2015-07-07 21:13 ` Eric Anholt
2015-07-11 5:13 ` Stephen Warren
2015-07-11 5:13 ` Stephen Warren
2015-07-11 5:13 ` Stephen Warren
2015-07-11 7:51 ` Thomas Gleixner [this message]
2015-07-11 7:51 ` Thomas Gleixner
2015-07-11 7:51 ` Thomas Gleixner
2015-07-13 16:06 ` Eric Anholt
2015-07-13 16:06 ` Eric Anholt
2015-07-14 5:09 ` Stephen Warren
2015-07-14 5:09 ` Stephen Warren
2015-07-14 5:09 ` Stephen Warren
2015-07-13 18:48 ` Eric Anholt
2015-07-13 18:48 ` Eric Anholt
2015-07-13 18:48 ` Eric Anholt
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=alpine.DEB.2.11.1507110940420.17353@nanos \
--to=tglx@linutronix.de \
--cc=andrea.merello@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=eric@anholt.net \
--cc=jason@lakedaemon.net \
--cc=lee@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rpi-kernel@lists.infradead.org \
--cc=swarren@wwwdotorg.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.