RE: [PATCH v3 3/3] ARCv2: MCIP: Deprecate setting of affinity in Device Tree

[Yuriy Kolerov]

> -----Original Message-----
> From: Vineet Gupta [mailto:vgupta@xxxxxxxxxxxx]
> Sent: Tuesday, January 03, 2017 9:11 PM
> To: Yuriy Kolerov <yuriy.kolerov@xxxxxxxxxxxx>; linux-snps-
> arc@xxxxxxxxxxxxxxxxxxx
> Cc: Alexey.Brodkin@xxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> marc.zyngier@xxxxxxx
> Subject: Re: [PATCH v3 3/3] ARCv2: MCIP: Deprecate setting of affinity in
> Device Tree
> 
> On 12/28/2016 12:47 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_enable() function is better place
> > for initialization of common interrupts.
> >
> > By default send all common interrupts to all available online CPUs.
> 
> Is this a departure from our original idea to route everything only to boot cpu
> ?
> Otherwise this looks OK to me !

I have come to the conclusion that now there is no way to route all interrupts to the boot CPU in a right manner. It may be implemented properly in the future.

> -Vineet
> 
> > 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.
> >
> > Signed-off-by: Yuriy Kolerov <yuriy.kolerov@xxxxxxxxxxxx>
> > ---
> >  .../interrupt-controller/snps,archs-idu-intc.txt   |  3 ++
> >  arch/arc/kernel/mcip.c                             | 52 +++++++++-------------
> >  2 files changed, 25 insertions(+), 30 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/interrupt-controller/snps,archs-id
> > u-intc.txt
> > b/Documentation/devicetree/bindings/interrupt-controller/snps,archs-id
> > u-intc.txt
> > index 0dcb7c7..9446576 100644
> > ---
> > a/Documentation/devicetree/bindings/interrupt-controller/snps,archs-id
> > u-intc.txt
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/snps,arch
> > +++ s-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
> > be131b2..d492a3c 100644
> > --- a/arch/arc/kernel/mcip.c
> > +++ b/arch/arc/kernel/mcip.c
> > @@ -175,7 +175,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)
> > @@ -205,12 +204,27 @@ idu_irq_set_affinity(struct irq_data *data,
> > const struct cpumask *cpumask,
> >
> >  	return IRQ_SET_MASK_OK;
> >  }
> > -#endif
> > +
> > +static void idu_irq_enable(struct irq_data *data) {
> > +	/*
> > +	 * By default send all common interrupts to all available online CPUs.
> > +	 * 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.
> > +	 */
> > +	idu_irq_set_affinity(data, cpu_online_mask, false);
> > +	idu_irq_unmask(data);
> > +}
> >
> >  static struct irq_chip idu_irq_chip = {
> >  	.name			= "MCIP IDU Intc",
> >  	.irq_mask		= idu_irq_mask,
> >  	.irq_unmask		= idu_irq_unmask,
> > +	.irq_enable		= idu_irq_enable,
> >  #ifdef CONFIG_SMP
> >  	.irq_set_affinity       = idu_irq_set_affinity,
> >  #endif
> > @@ -243,36 +257,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;
> >  }
> >