From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
To: Yuriy Kolerov <yuriy.kolerov@synopsys.com>,
<linux-snps-arc@lists.infradead.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>,
<Alexey.Brodkin@synopsys.com>, <linux-kernel@vger.kernel.org>,
<tglx@linutronix.de>
Subject: Re: [PATCH v2 2/2] ARCv2: MCIP: Deprecate setting of affinity in Device Tree
Date: Tue, 13 Dec 2016 09:45:24 -0800 [thread overview]
Message-ID: <da69cdf3-0413-3b5a-900f-fe3a99fa52c9@synopsys.com> (raw)
In-Reply-To: <1481277572-18283-2-git-send-email-yuriy.kolerov@synopsys.com>
On 12/09/2016 01:59 AM, Yuriy Kolerov wrote:
> Ignore value of interrupt distribution mode for common interrupts in
> IDU since setting of affinity using value from Device Tree is deprecated
> in ARC. Originally it is done in idu_irq_xlate() function and it is
> semantically wrong and does not guaranty that an affinity value will be
> set properly. idu_irq_map() function is better place since it is called
> once for each IRQ.
>
> The affinity of common interrupts in IDU must be set manually since
> in some cases the kernel will not call irq_set_affinity() by itself:
>
> 1. When the kernel is not configured with support of SMP.
> 2. When the kernel is configured with support of SMP but upper
> interrupt controllers does not support setting of the affinity
> and cannot propagate it to IDU.
>
> By default send all common interrupts to the boot CPU. If the kernel
> has support of SMP then use irq_default_affinity as the affinity
> for IDU common interrupts since it is set to the boot CPU by default.
> If the kernel is configured without support of SMP then use
> cpu_online_mask since in this case it must always contain a valid
> bitmap with only 1 online CPU.
>
> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> ---
> .../interrupt-controller/snps,archs-idu-intc.txt | 3 ++
> arch/arc/kernel/mcip.c | 60 +++++++++++-----------
> 2 files changed, 33 insertions(+), 30 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
> index 0dcb7c7..9446576 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
> @@ -15,6 +15,9 @@ Properties:
> Second cell specifies the irq distribution mode to cores
> 0=Round Robin; 1=cpu0, 2=cpu1, 4=cpu2, 8=cpu3
>
> + The second cell in interrupts property is deprecated and may be ignored by
> + the kernel.
> +
> intc accessed via the special ARC AUX register interface, hence "reg" property
> is not specified.
>
> diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
> index f39142a..5b36f7b 100644
> --- a/arch/arc/kernel/mcip.c
> +++ b/arch/arc/kernel/mcip.c
> @@ -174,7 +174,6 @@ static void idu_irq_unmask(struct irq_data *data)
> raw_spin_unlock_irqrestore(&mcip_lock, flags);
> }
>
> -#ifdef CONFIG_SMP
> static int
> idu_irq_set_affinity(struct irq_data *data, const struct cpumask *cpumask,
> bool force)
> @@ -204,7 +203,6 @@ idu_irq_set_affinity(struct irq_data *data, const struct cpumask *cpumask,
>
> return IRQ_SET_MASK_OK;
> }
> -#endif
>
> static struct irq_chip idu_irq_chip = {
> .name = "MCIP IDU Intc",
> @@ -229,9 +227,33 @@ static void idu_cascade_isr(struct irq_desc *desc)
>
> static int idu_irq_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t hwirq)
> {
> + struct irq_data *irqd = irq_get_irq_data(virq);
> +
> irq_set_chip_and_handler(virq, &idu_irq_chip, handle_level_irq);
> irq_set_status_flags(virq, IRQ_MOVE_PCNTXT);
>
> + /*
> + * The affinity of common interrupts in IDU must be set manually since
> + * in some cases the kernel will not call irq_set_affinity() by itself:
> + *
> + * 1. When the kernel is not configured with support of SMP.
> + * 2. When the kernel is configured with support of SMP but upper
> + * interrupt controllers does not support setting of the affinity
> + * and cannot propagate it to IDU.
> + *
> + * By default send all common interrupts to the boot CPU. If the kernel
> + * has support of SMP then use irq_default_affinity as the affinity
> + * for IDU common interrupts since it is set to the boot CPU by default.
> + * If the kernel is configured without support of SMP then use
> + * cpu_online_mask since in this case it must always contain a valid
> + * bitmap with only 1 online CPU.
> + */
> +#ifdef CONFIG_SMP
> + idu_irq_set_affinity(irqd, irq_default_affinity, false);
> +#else
> + idu_irq_set_affinity(irqd, cpu_online_mask, false);
> +#endif
Why even bother with using a genirq global. Just create a new cpu bitmap with
smp_processor_id() and use it for both UP/SMP. Isn't that simpler and leave genirq
alone.
> +
> return 0;
> }
>
> @@ -239,36 +261,14 @@ static int idu_irq_xlate(struct irq_domain *d, struct device_node *n,
> const u32 *intspec, unsigned int intsize,
> irq_hw_number_t *out_hwirq, unsigned int *out_type)
> {
> - irq_hw_number_t hwirq = *out_hwirq = intspec[0];
> - int distri = intspec[1];
> - unsigned long flags;
> -
> + /*
> + * Ignore value of interrupt distribution mode for common interrupts in
> + * IDU which resides in intspec[1] since setting an affinity using value
> + * from Device Tree is deprecated in ARC.
> + */
> + *out_hwirq = intspec[0];
> *out_type = IRQ_TYPE_NONE;
>
> - /* XXX: validate distribution scheme again online cpu mask */
> - if (distri == 0) {
> - /* 0 - Round Robin to all cpus, otherwise 1 bit per core */
> - raw_spin_lock_irqsave(&mcip_lock, flags);
> - idu_set_dest(hwirq, BIT(num_online_cpus()) - 1);
> - idu_set_mode(hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_RR);
> - raw_spin_unlock_irqrestore(&mcip_lock, flags);
> - } else {
> - /*
> - * DEST based distribution for Level Triggered intr can only
> - * have 1 CPU, so generalize it to always contain 1 cpu
> - */
> - int cpu = ffs(distri);
> -
> - if (cpu != fls(distri))
> - pr_warn("IDU irq %lx distri mode set to cpu %x\n",
> - hwirq, cpu);
> -
> - raw_spin_lock_irqsave(&mcip_lock, flags);
> - idu_set_dest(hwirq, cpu);
> - idu_set_mode(hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_DEST);
> - raw_spin_unlock_irqrestore(&mcip_lock, flags);
> - }
> -
> return 0;
> }
>
>
next prev parent reply other threads:[~2016-12-13 17:45 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-09 9:59 [PATCH v2 1/2] ARC: SMP: Set the default affinity to the boot cpu Yuriy Kolerov
2016-12-09 9:59 ` [PATCH v2 2/2] ARCv2: MCIP: Deprecate setting of affinity in Device Tree Yuriy Kolerov
2016-12-13 17:45 ` Vineet Gupta [this message]
2016-12-13 17:40 ` [PATCH v2 1/2] ARC: SMP: Set the default affinity to the boot cpu Vineet Gupta
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=da69cdf3-0413-3b5a-900f-fe3a99fa52c9@synopsys.com \
--to=vineet.gupta1@synopsys.com \
--cc=Alexey.Brodkin@synopsys.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-snps-arc@lists.infradead.org \
--cc=marc.zyngier@arm.com \
--cc=tglx@linutronix.de \
--cc=yuriy.kolerov@synopsys.com \
/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).