From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933956AbcLMRpn (ORCPT ); Tue, 13 Dec 2016 12:45:43 -0500 Received: from us01smtprelay-2.synopsys.com ([198.182.47.9]:52410 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753425AbcLMRpf (ORCPT ); Tue, 13 Dec 2016 12:45:35 -0500 Subject: Re: [PATCH v2 2/2] ARCv2: MCIP: Deprecate setting of affinity in Device Tree To: Yuriy Kolerov , References: <1481277572-18283-1-git-send-email-yuriy.kolerov@synopsys.com> <1481277572-18283-2-git-send-email-yuriy.kolerov@synopsys.com> CC: Marc Zyngier , , , Newsgroups: gmane.linux.kernel,gmane.linux.kernel.arc From: Vineet Gupta Message-ID: Date: Tue, 13 Dec 2016 09:45:24 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <1481277572-18283-2-git-send-email-yuriy.kolerov@synopsys.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.10.161.49] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > .../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; > } > >