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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox