From: David Daney <david.s.daney@gmail.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Ralf Baechle <ralf@linux-mips.org>,
David Daney <ddaney@caviumnetworks.com>,
linux-mips@linux-mips.org
Subject: Re: [patch 3/5] MIPS: Octeon: Simplify irq_cpu_on/offline irq chip functions
Date: Sun, 27 Mar 2011 14:12:32 -0700 [thread overview]
Message-ID: <4D8FA840.2080108@gmail.com> (raw)
In-Reply-To: <20110327161118.737588559@linutronix.de>
On 03/27/2011 09:22 AM, Thomas Gleixner wrote:
> Make use of the IRQCHIP_ONOFFLINE_ENABLED flag and remove the
> wrappers. Use irqd_irq_disabled() instead of desc->status, which will
> go away.
>
I rewrote my patch set and was testing it. Interesting that I came up
with a function with almost the same name and purpose.
However my function told us if the irq was masked *or* disabled. The
idea being a function that returns true if the irq could fire. We
cannot be enabling the interrupt in the controller if it is masked.
For example I need to test this when adjusting affinity, and taking CPUs
on and off line.
I don't think your genirq changes can tell the me information I really
need in their current state. I think we need to consider how the masked
state interacts with IRQCHIP_ONOFFLINE_ENABLED and irqd_irq_disabled().
Since I have totally rewritten my interrupt code, I am a bit ambivalent
about applying these patches. It might make more sense that I adjust my
patch for your genirq changes and test it before committing it.
David Daney
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>
> Cc: David Daney<ddaney@caviumnetworks.com>
> ---
> arch/mips/cavium-octeon/octeon-irq.c | 71 ++++++++---------------------------
> 1 file changed, 17 insertions(+), 54 deletions(-)
>
> Index: linux-2.6-tip/arch/mips/cavium-octeon/octeon-irq.c
> ===================================================================
> --- linux-2.6-tip.orig/arch/mips/cavium-octeon/octeon-irq.c
> +++ linux-2.6-tip/arch/mips/cavium-octeon/octeon-irq.c
> @@ -152,19 +152,6 @@ static void octeon_irq_core_bus_sync_unl
> mutex_unlock(&cd->core_irq_mutex);
> }
>
> -
> -static void octeon_irq_core_cpu_online(struct irq_data *data)
> -{
> - if (irqd_irq_disabled(data))
> - octeon_irq_core_eoi(data);
> -}
> -
> -static void octeon_irq_core_cpu_offline(struct irq_data *data)
> -{
> - if (irqd_irq_disabled(data))
> - octeon_irq_core_ack(data);
> -}
> -
> static struct irq_chip octeon_irq_chip_core = {
> .name = "Core",
> .irq_enable = octeon_irq_core_enable,
> @@ -174,8 +161,9 @@ static struct irq_chip octeon_irq_chip_c
> .irq_bus_lock = octeon_irq_core_bus_lock,
> .irq_bus_sync_unlock = octeon_irq_core_bus_sync_unlock,
>
> - .irq_cpu_online = octeon_irq_core_cpu_online,
> - .irq_cpu_offline = octeon_irq_core_cpu_offline,
> + .irq_cpu_online = octeon_irq_core_eoi,
> + .irq_cpu_offline = octeon_irq_core_ack,
> + .flags = IRQCHIP_ONOFFLINE_ENABLED,
> };
>
> static void __init octeon_irq_init_core(void)
> @@ -517,30 +505,6 @@ static void octeon_irq_ciu_enable_all_v2
> }
> }
>
> -static void octeon_irq_cpu_online_mbox(struct irq_data *data)
> -{
> - if (irqd_irq_disabled(data))
> - octeon_irq_ciu_enable_local(data);
> -}
> -
> -static void octeon_irq_cpu_online_mbox_v2(struct irq_data *data)
> -{
> - if (irqd_irq_disabled(data))
> - octeon_irq_ciu_enable_local_v2(data);
> -}
> -
> -static void octeon_irq_cpu_offline_mbox(struct irq_data *data)
> -{
> - if (irqd_irq_disabled(data))
> - octeon_irq_ciu_disable_local(data);
> -}
> -
> -static void octeon_irq_cpu_offline_mbox_v2(struct irq_data *data)
> -{
> - if (irqd_irq_disabled(data))
> - octeon_irq_ciu_disable_local_v2(data);
> -}
> -
> #ifdef CONFIG_SMP
>
> static void octeon_irq_cpu_offline_ciu(struct irq_data *data)
> @@ -570,8 +534,7 @@ static int octeon_irq_ciu_set_affinity(s
> const struct cpumask *dest, bool force)
> {
> int cpu;
> - struct irq_desc *desc = irq_to_desc(data->irq);
> - int enable_one = (desc->status& IRQ_DISABLED) == 0;
> + bool enable_one = !irqd_irq_disabled(data);
> unsigned long flags;
> union octeon_ciu_chip_data cd;
>
> @@ -585,7 +548,7 @@ static int octeon_irq_ciu_set_affinity(s
> if (cpumask_weight(dest) != 1)
> return -EINVAL;
>
> - if (desc->status& IRQ_DISABLED)
> + if (!enable_one)
> return 0;
>
> if (cd.s.line == 0) {
> @@ -595,7 +558,7 @@ static int octeon_irq_ciu_set_affinity(s
> unsigned long *pen =&per_cpu(octeon_irq_ciu0_en_mirror, cpu);
>
> if (cpumask_test_cpu(cpu, dest)&& enable_one) {
> - enable_one = 0;
> + enable_one = false;
> set_bit(cd.s.bit, pen);
> } else {
> clear_bit(cd.s.bit, pen);
> @@ -610,7 +573,7 @@ static int octeon_irq_ciu_set_affinity(s
> unsigned long *pen =&per_cpu(octeon_irq_ciu1_en_mirror, cpu);
>
> if (cpumask_test_cpu(cpu, dest)&& enable_one) {
> - enable_one = 0;
> + enable_one = false;
> set_bit(cd.s.bit, pen);
> } else {
> clear_bit(cd.s.bit, pen);
> @@ -631,12 +594,11 @@ static int octeon_irq_ciu_set_affinity_v
> bool force)
> {
> int cpu;
> - struct irq_desc *desc = irq_to_desc(data->irq);
> - int enable_one = (desc->status& IRQ_DISABLED) == 0;
> + bool enable_one = !irqd_irq_disabled(data);
> u64 mask;
> union octeon_ciu_chip_data cd;
>
> - if (desc->status& IRQ_DISABLED)
> + if (!enable_one)
> return 0;
>
> cd.p = data->chip_data;
> @@ -647,7 +609,7 @@ static int octeon_irq_ciu_set_affinity_v
> unsigned long *pen =&per_cpu(octeon_irq_ciu0_en_mirror, cpu);
> int index = octeon_coreid_for_cpu(cpu) * 2;
> if (cpumask_test_cpu(cpu, dest)&& enable_one) {
> - enable_one = 0;
> + enable_one = false;
> set_bit(cd.s.bit, pen);
> cvmx_write_csr(CVMX_CIU_INTX_EN0_W1S(index), mask);
> } else {
> @@ -660,7 +622,7 @@ static int octeon_irq_ciu_set_affinity_v
> unsigned long *pen =&per_cpu(octeon_irq_ciu1_en_mirror, cpu);
> int index = octeon_coreid_for_cpu(cpu) * 2 + 1;
> if (cpumask_test_cpu(cpu, dest)&& enable_one) {
> - enable_one = 0;
> + enable_one = false;
> set_bit(cd.s.bit, pen);
> cvmx_write_csr(CVMX_CIU_INTX_EN1_W1S(index), mask);
> } else {
> @@ -679,7 +641,6 @@ static int octeon_irq_ciu_set_affinity_v
> */
> static void octeon_irq_dummy_mask(struct irq_data *data)
> {
> - return;
> }
>
> /*
> @@ -741,8 +702,9 @@ static struct irq_chip octeon_irq_chip_c
> .irq_ack = octeon_irq_ciu_disable_local_v2,
> .irq_eoi = octeon_irq_ciu_enable_local_v2,
>
> - .irq_cpu_online = octeon_irq_cpu_online_mbox_v2,
> - .irq_cpu_offline = octeon_irq_cpu_offline_mbox_v2,
> + .irq_cpu_online = octeon_irq_ciu_enable_local_v2,
> + .irq_cpu_offline = octeon_irq_ciu_disable_local_v2,
> + .flags = IRQCHIP_ONOFFLINE_ENABLED,
> };
>
> static struct irq_chip octeon_irq_chip_ciu_mbox = {
> @@ -750,8 +712,9 @@ static struct irq_chip octeon_irq_chip_c
> .irq_enable = octeon_irq_ciu_enable_all,
> .irq_disable = octeon_irq_ciu_disable_all,
>
> - .irq_cpu_online = octeon_irq_cpu_online_mbox,
> - .irq_cpu_offline = octeon_irq_cpu_offline_mbox,
> + .irq_cpu_online = octeon_irq_ciu_enable_local,
> + .irq_cpu_offline = octeon_irq_ciu_disable_local,
> + .flags = IRQCHIP_ONOFFLINE_ENABLED,
> };
>
> /*
>
>
>
next prev parent reply other threads:[~2011-03-27 21:12 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-27 16:22 [patch 0/5] MIPS: Final irq fixups Thomas Gleixner
2011-03-27 16:22 ` [patch 1/5] MIPS: Fix syncfs syscall copy and paste failure Thomas Gleixner
2011-03-28 11:55 ` Ralf Baechle
2011-03-27 16:22 ` [patch 2/5] MIPS: Octeon: Rewrite interrupt handling code Thomas Gleixner
2011-03-27 16:22 ` [patch 3/5] MIPS: Octeon: Simplify irq_cpu_on/offline irq chip functions Thomas Gleixner
2011-03-27 21:12 ` David Daney [this message]
2011-03-27 21:29 ` Thomas Gleixner
2011-03-27 21:41 ` Thomas Gleixner
2011-03-28 0:12 ` David Daney
2011-03-28 0:24 ` David Daney
2011-03-27 16:22 ` [patch 4/5] MIPS: alchemy: Use proper irq accessors Thomas Gleixner
2011-03-28 12:00 ` Ralf Baechle
2011-03-27 16:22 ` [patch 5/5] MIPS: Convert the irq functions to the new names Thomas Gleixner
2011-03-28 12:01 ` Ralf Baechle
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=4D8FA840.2080108@gmail.com \
--to=david.s.daney@gmail.com \
--cc=ddaney@caviumnetworks.com \
--cc=linux-mips@linux-mips.org \
--cc=ralf@linux-mips.org \
--cc=tglx@linutronix.de \
/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.