[PATCH v2 1/5] ARC: SMP: Use "unsigned virq" in smp_ipi_irq_setup instead of "signed irq"

[PATCH v2 1/5] ARC: SMP: Use "unsigned virq" in smp_ipi_irq_setup instead of "signed irq"

[Yuriy Kolerov]

This function takes a cpu number and a virq number and registers an
appropriate handler per cpu. However smp_ipi_irq_setup is incorrectly
used in several places of ARC platform code - hwirq is passed instead
of virq. This patch is intended to clarify declaration of virq argument
to prevent passing of hwirq instead of virq in future.

Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
---
 arch/arc/include/asm/smp.h | 4 ++--
 arch/arc/kernel/smp.c      | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arc/include/asm/smp.h b/arch/arc/include/asm/smp.h
index 89fdd1b..3ebebbc 100644
--- a/arch/arc/include/asm/smp.h
+++ b/arch/arc/include/asm/smp.h
@@ -37,9 +37,9 @@ extern const char *arc_platform_smp_cpuinfo(void);
  * API expected BY platform smp code (FROM arch smp code)
  *
  * smp_ipi_irq_setup:
- *	Takes @cpu and @irq to which the arch-common ISR is hooked up
+ *	Takes @cpu and @virq to which the arch-common ISR is hooked up
  */
-extern int smp_ipi_irq_setup(int cpu, int irq);
+extern int smp_ipi_irq_setup(int cpu, unsigned int virq);
 
 /*
  * struct plat_smp_ops	- SMP callbacks provided by platform to ARC SMP
diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
index f183cc6..ec4d956 100644
--- a/arch/arc/kernel/smp.c
+++ b/arch/arc/kernel/smp.c
@@ -351,7 +351,7 @@ irqreturn_t do_IPI(int irq, void *dev_id)
  */
 static DEFINE_PER_CPU(int, ipi_dev);
 
-int smp_ipi_irq_setup(int cpu, int irq)
+int smp_ipi_irq_setup(int cpu, unsigned int virq)
 {
 	int *dev = per_cpu_ptr(&ipi_dev, cpu);
 
@@ -359,12 +359,12 @@ int smp_ipi_irq_setup(int cpu, int irq)
 	if (!cpu) {
 		int rc;
 
-		rc = request_percpu_irq(irq, do_IPI, "IPI Interrupt", dev);
+		rc = request_percpu_irq(virq, do_IPI, "IPI Interrupt", dev);
 		if (rc)
-			panic("Percpu IRQ request failed for %d\n", irq);
+			panic("Percpu IRQ request failed for %d\n", virq);
 	}
 
-	enable_percpu_irq(irq, 0);
+	enable_percpu_irq(virq, 0);
 
 	return 0;
 }
-- 
2.7.4

[PATCH v2 2/5] ARC: SMP: Pass virq to smp_ipi_irq_setup instead of hwirq

[Yuriy Kolerov]

This function takes a cpu number and a virq number and registers an
appropriate handler per cpu. However smp_ipi_irq_setup is incorrectly
used in several places of ARC platform code - hwirq is passed instead of
virq. There is a code with an example of inccorect usage of smp_ipi_irq_setup:

    smp_ipi_irq_setup(cpu, IPI_IRQ);
    smp_ipi_irq_setup(cpu, SOFTIRQ_IRQ);

where IPI_IRQ and SOFTIRQ_IRQ are hwirq numbers for per cpu interrupts.
Since smp_ipi_irq_setup must be taken a virq number insdead of a hwirq
number then it is necessary to fix usage of this function this way:

    smp_ipi_irq_setup(cpu, irq_find_mapping(NULL, IPI_IRQ));
    smp_ipi_irq_setup(cpu, irq_find_mapping(NULL, SOFTIRQ_IRQ));

The idea is to find an appropriate mapping of <domain, hwirq> to virq
by using irq_find_mapping function. NULL value is used as a domain
argument (a default domain) since smp_ipi_irq_setup is used for
registering an IRQ handler for a root interrupt controller (a default
IRQ domain must be used).

Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
---
 arch/arc/kernel/mcip.c    | 5 +++--
 arch/arc/plat-eznps/smp.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
index 72f9179..28ff766 100644
--- a/arch/arc/kernel/mcip.c
+++ b/arch/arc/kernel/mcip.c
@@ -11,6 +11,7 @@
 #include <linux/smp.h>
 #include <linux/irq.h>
 #include <linux/spinlock.h>
+#include <linux/irqdomain.h>
 #include <asm/irqflags-arcv2.h>
 #include <asm/mcip.h>
 #include <asm/setup.h>
@@ -22,8 +23,8 @@ static DEFINE_RAW_SPINLOCK(mcip_lock);
 
 static void mcip_setup_per_cpu(int cpu)
 {
-	smp_ipi_irq_setup(cpu, IPI_IRQ);
-	smp_ipi_irq_setup(cpu, SOFTIRQ_IRQ);
+	smp_ipi_irq_setup(cpu, irq_find_mapping(NULL, IPI_IRQ));
+	smp_ipi_irq_setup(cpu, irq_find_mapping(NULL, SOFTIRQ_IRQ));
 }
 
 static void mcip_ipi_send(int cpu)
diff --git a/arch/arc/plat-eznps/smp.c b/arch/arc/plat-eznps/smp.c
index 5e901f8..dd996e0 100644
--- a/arch/arc/plat-eznps/smp.c
+++ b/arch/arc/plat-eznps/smp.c
@@ -134,7 +134,7 @@ static void eznps_ipi_send(int cpu)
 
 static void eznps_init_per_cpu(int cpu)
 {
-	smp_ipi_irq_setup(cpu, NPS_IPI_IRQ);
+	smp_ipi_irq_setup(cpu, irq_find_mapping(NULL, NPS_IPI_IRQ));
 
 	eznps_init_core(cpu);
 	mtm_enable_core(cpu);
-- 
2.7.4

[PATCH v2 3/5] ARC: MCIP: Use hwirq instead of virq for resolution of IDU IRQ handlers

[Yuriy Kolerov]

Multicore ARC configurations use IDU (Interrupt Distribution Unit) for
distributing of common interrupts. In fact IDU is a interrupt controller
on top of main per core interrupt controller.

All common IRQs are physically and linearly mapped to per core
interrupts. E.g. <0, 1, 2, 3> common IDU interrupts may be mapped to per
core <24, 25, 26, 27> interrupts. An initialization code of IDU
controller (idu_of_init) creates mappings for all parent interrupts
<24, 25, ...> and sets a chained IDU handler for them. In the same
time idu_of_init must save the first met parent hwirq (idu_first_irq)
thus in future it is possible to figure out what common hwirq has come
by subtracting of idu_first_irq from the current parent hwirq (see
idu_cascade_isr).

The problem is that idu_of_init and idu_cascade_isr use parent virtual
IRQs as hardware IRQs and perform all the above-described manipulations
on virtual IRQs. But it is wrong and hardware IRQs must be used instead.

Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
---
 arch/arc/kernel/mcip.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
index 28ff766..51a218c 100644
--- a/arch/arc/kernel/mcip.c
+++ b/arch/arc/kernel/mcip.c
@@ -220,16 +220,14 @@ static struct irq_chip idu_irq_chip = {
 
 };
 
-static int idu_first_irq;
+static irq_hw_number_t idu_first_hwirq;
 
 static void idu_cascade_isr(struct irq_desc *desc)
 {
-	struct irq_domain *domain = irq_desc_get_handler_data(desc);
-	unsigned int core_irq = irq_desc_get_irq(desc);
-	unsigned int idu_irq;
-
-	idu_irq = core_irq - idu_first_irq;
-	generic_handle_irq(irq_find_mapping(domain, idu_irq));
+	struct irq_domain *idu_domain = irq_desc_get_handler_data(desc);
+	irq_hw_number_t core_hwirq = irqd_to_hwirq(irq_desc_get_irq_data(desc));
+	irq_hw_number_t idu_hwirq = core_hwirq - idu_first_hwirq;
+	generic_handle_irq(irq_find_mapping(idu_domain, idu_hwirq));
 }
 
 static int idu_irq_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t hwirq)
@@ -295,7 +293,7 @@ idu_of_init(struct device_node *intc, struct device_node *parent)
 	struct irq_domain *domain;
 	/* Read IDU BCR to confirm nr_irqs */
 	int nr_irqs = of_irq_count(intc);
-	int i, irq;
+	int i, virq;
 
 	if (!idu_detected)
 		panic("IDU not detected, but DeviceTree using it");
@@ -313,11 +311,11 @@ idu_of_init(struct device_node *intc, struct device_node *parent)
 		 * however we need it to get the parent virq and set IDU handler
 		 * as first level isr
 		 */
-		irq = irq_of_parse_and_map(intc, i);
+		virq = irq_of_parse_and_map(intc, i);
 		if (!i)
-			idu_first_irq = irq;
+			idu_first_hwirq = irqd_to_hwirq(irq_get_irq_data(virq));
 
-		irq_set_chained_handler_and_data(irq, idu_cascade_isr, domain);
+		irq_set_chained_handler_and_data(virq, idu_cascade_isr, domain);
 	}
 
 	__mcip_cmd(CMD_IDU_ENABLE, 0);
-- 
2.7.4

[PATCH v2 4/5] ARC: MCIP: Set an initial affinity value in idu_irq_map

[Yuriy Kolerov]

Originally an initial distribution mode (its value resides in Device Tree)
for each common interrupt is set in idu_irq_xlate. This leads to the
following problems. idu_irq_xlate may be called several times during parsing
of the Device Tree and it is semantically wrong to configure an initial
distribution mode here. Also a value of affinity (CPUs bitmap) is not saved
to irq_desc structure for the virq - later (after parsing of the DT) kernel
sees that affinity is not set and sets a default value of affinity (all
cores in round robin mode). As a result a value of affinity from Device
Tree is ignored.

To fix it I created a buffer for initial CPUs bitmaps from Device Tree.
In idu_irq_xlate those bitmaps are saved to the buffer. Then affinity
for virq is set manually in idu_irq_map. It works because idu_irq_xlate
is always called before idu_irq_map.

Despite the fact that it works I think that it must be rewritten to
eliminate usage of the buffer and move all logic to idu_irq_map but
I do not know how to do it correctly.

Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
---
 arch/arc/kernel/mcip.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
index 51a218c..090f0a1 100644
--- a/arch/arc/kernel/mcip.c
+++ b/arch/arc/kernel/mcip.c
@@ -12,12 +12,15 @@
 #include <linux/irq.h>
 #include <linux/spinlock.h>
 #include <linux/irqdomain.h>
+#include <linux/interrupt.h>
+#include <linux/cpumask.h>
 #include <asm/irqflags-arcv2.h>
 #include <asm/mcip.h>
 #include <asm/setup.h>
 
 static char smp_cpuinfo_buf[128];
 static int idu_detected;
+static unsigned long idu_cirq_to_dest[CONFIG_ARC_NUMBER_OF_INTERRUPTS];
 
 static DEFINE_RAW_SPINLOCK(mcip_lock);
 
@@ -232,9 +235,15 @@ 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)
 {
+	cpumask_t mask;
+
 	irq_set_chip_and_handler(virq, &idu_irq_chip, handle_level_irq);
 	irq_set_status_flags(virq, IRQ_MOVE_PCNTXT);
 
+	cpumask_clear(&mask);
+	cpumask_bits(&mask)[0] |= idu_cirq_to_dest[hwirq];
+	irq_set_affinity(virq, &mask);
+
 	return 0;
 }
 
@@ -252,8 +261,7 @@ static int idu_irq_xlate(struct irq_domain *d, struct device_node *n,
 	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);
+		idu_cirq_to_dest[hwirq] = BIT(num_online_cpus()) - 1;
 		raw_spin_unlock_irqrestore(&mcip_lock, flags);
 	} else {
 		/*
@@ -267,8 +275,7 @@ static int idu_irq_xlate(struct irq_domain *d, struct device_node *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);
+		idu_cirq_to_dest[hwirq] = cpu;
 		raw_spin_unlock_irqrestore(&mcip_lock, flags);
 	}
 
-- 
2.7.4

[PATCH v2 5/5] ARC: MCIP: Use IDU_M_DISTRI_DEST mode if there is only 1 destination core

[Yuriy Kolerov]

ARC linux uses 2 distribution modes for common interrupts: round robin
mode (IDU_M_DISTRI_RR) and a simple destination mode (IDU_M_DISTRI_DEST).
The first one is used when more than 1 cores may handle a common interrupt
and the second one is used when only 1 core may handle a common interrupt.

However idu_irq_set_affinity always sets IDU_M_DISTRI_RR for all affinity
values. But there is no sense in setting of such mode if only 1 core must
handle a common interrupt.

Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
---
 arch/arc/kernel/mcip.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
index 090f0a1..75e6d73 100644
--- a/arch/arc/kernel/mcip.c
+++ b/arch/arc/kernel/mcip.c
@@ -197,6 +197,7 @@ idu_irq_set_affinity(struct irq_data *data, const struct cpumask *cpumask,
 {
 	unsigned long flags;
 	cpumask_t online;
+	unsigned long dest_bits;
 
 	/* errout if no online cpu per @cpumask */
 	if (!cpumask_and(&online, cpumask, cpu_online_mask))
@@ -204,8 +205,14 @@ idu_irq_set_affinity(struct irq_data *data, const struct cpumask *cpumask,
 
 	raw_spin_lock_irqsave(&mcip_lock, flags);
 
-	idu_set_dest(data->hwirq, cpumask_bits(&online)[0]);
-	idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_RR);
+	dest_bits = cpumask_bits(&online)[0];
+	idu_set_dest(data->hwirq, dest_bits);
+
+	if (ffs(dest_bits) == fls(dest_bits)) {
+		idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_DEST);
+	} else {
+		idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_RR);
+	}
 
 	raw_spin_unlock_irqrestore(&mcip_lock, flags);
 
-- 
2.7.4

Re: [PATCH v2 1/5] ARC: SMP: Use "unsigned virq" in smp_ipi_irq_setup instead of "signed irq"

[Vineet Gupta]

Hi Yuriy,

On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
> This function takes a cpu number and a virq number and registers an
> appropriate handler per cpu. However smp_ipi_irq_setup is incorrectly
> used in several places of ARC platform code - hwirq is passed instead
> of virq. This patch is intended to clarify declaration of virq argument
> to prevent passing of hwirq instead of virq in future.
> 
> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>

So this series is missing the cover letter which makes it a bit hard to gauge the
end goal.
It seems patch [1-3]/5 are fixes which can be applied right away.
And [4-5]/5 help fix the irq affinity setting. What this doesn't say is which
platforms does it fix. And do we not need the hierarchical irq domains, specially
for the AXS platform which has 3 interrupt controllers stacked up.

> ---
>  arch/arc/include/asm/smp.h | 4 ++--
>  arch/arc/kernel/smp.c      | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arc/include/asm/smp.h b/arch/arc/include/asm/smp.h
> index 89fdd1b..3ebebbc 100644
> --- a/arch/arc/include/asm/smp.h
> +++ b/arch/arc/include/asm/smp.h
> @@ -37,9 +37,9 @@ extern const char *arc_platform_smp_cpuinfo(void);
>   * API expected BY platform smp code (FROM arch smp code)
>   *
>   * smp_ipi_irq_setup:
> - *	Takes @cpu and @irq to which the arch-common ISR is hooked up
> + *	Takes @cpu and @virq to which the arch-common ISR is hooked up
>   */
> -extern int smp_ipi_irq_setup(int cpu, int irq);
> +extern int smp_ipi_irq_setup(int cpu, unsigned int virq);
>  
>  /*
>   * struct plat_smp_ops	- SMP callbacks provided by platform to ARC SMP
> diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> index f183cc6..ec4d956 100644
> --- a/arch/arc/kernel/smp.c
> +++ b/arch/arc/kernel/smp.c
> @@ -351,7 +351,7 @@ irqreturn_t do_IPI(int irq, void *dev_id)
>   */
>  static DEFINE_PER_CPU(int, ipi_dev);
>  
> -int smp_ipi_irq_setup(int cpu, int irq)
> +int smp_ipi_irq_setup(int cpu, unsigned int virq)
>  {
>  	int *dev = per_cpu_ptr(&ipi_dev, cpu);
>  
> @@ -359,12 +359,12 @@ int smp_ipi_irq_setup(int cpu, int irq)
>  	if (!cpu) {
>  		int rc;
>  
> -		rc = request_percpu_irq(irq, do_IPI, "IPI Interrupt", dev);
> +		rc = request_percpu_irq(virq, do_IPI, "IPI Interrupt", dev);
>  		if (rc)
> -			panic("Percpu IRQ request failed for %d\n", irq);
> +			panic("Percpu IRQ request failed for %d\n", virq);
>  	}
>  
> -	enable_percpu_irq(irq, 0);
> +	enable_percpu_irq(virq, 0);
>  
>  	return 0;
>  }
> 

Re: [PATCH v2 2/5] ARC: SMP: Pass virq to smp_ipi_irq_setup instead of hwirq

[Vineet Gupta]

On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
> This function takes a cpu number and a virq number and registers an
> appropriate handler per cpu. However smp_ipi_irq_setup is incorrectly
> used in several places of ARC platform code - hwirq is passed instead of
> virq. There is a code with an example of inccorect usage of smp_ipi_irq_setup:
> 
>     smp_ipi_irq_setup(cpu, IPI_IRQ);
>     smp_ipi_irq_setup(cpu, SOFTIRQ_IRQ);
> 
> where IPI_IRQ and SOFTIRQ_IRQ are hwirq numbers for per cpu interrupts.
> Since smp_ipi_irq_setup must be taken a virq number insdead of a hwirq
> number then it is necessary to fix usage of this function this way:
> 
>     smp_ipi_irq_setup(cpu, irq_find_mapping(NULL, IPI_IRQ));
>     smp_ipi_irq_setup(cpu, irq_find_mapping(NULL, SOFTIRQ_IRQ));
> 
> The idea is to find an appropriate mapping of <domain, hwirq> to virq
> by using irq_find_mapping function. NULL value is used as a domain
> argument (a default domain) since smp_ipi_irq_setup is used for
> registering an IRQ handler for a root interrupt controller (a default
> IRQ domain must be used).
> 
> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> ---
>  arch/arc/kernel/mcip.c    | 5 +++--
>  arch/arc/plat-eznps/smp.c | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
> index 72f9179..28ff766 100644
> --- a/arch/arc/kernel/mcip.c
> +++ b/arch/arc/kernel/mcip.c
> @@ -11,6 +11,7 @@
>  #include <linux/smp.h>
>  #include <linux/irq.h>
>  #include <linux/spinlock.h>
> +#include <linux/irqdomain.h>
>  #include <asm/irqflags-arcv2.h>
>  #include <asm/mcip.h>
>  #include <asm/setup.h>
> @@ -22,8 +23,8 @@ static DEFINE_RAW_SPINLOCK(mcip_lock);
>  
>  static void mcip_setup_per_cpu(int cpu)
>  {
> -	smp_ipi_irq_setup(cpu, IPI_IRQ);
> -	smp_ipi_irq_setup(cpu, SOFTIRQ_IRQ);
> +	smp_ipi_irq_setup(cpu, irq_find_mapping(NULL, IPI_IRQ));
> +	smp_ipi_irq_setup(cpu, irq_find_mapping(NULL, SOFTIRQ_IRQ));

Can we do this slightly differently. smp_ipi_irq_setup() is just a helper anyways,
can we keep the interface the same (maybe explicit'ify that it takes hwirq). And
call irq_find_mapping() inside it - this will reduce the code churn !


>  }
>  
>  static void mcip_ipi_send(int cpu)
> diff --git a/arch/arc/plat-eznps/smp.c b/arch/arc/plat-eznps/smp.c
> index 5e901f8..dd996e0 100644
> --- a/arch/arc/plat-eznps/smp.c
> +++ b/arch/arc/plat-eznps/smp.c
> @@ -134,7 +134,7 @@ static void eznps_ipi_send(int cpu)
>  
>  static void eznps_init_per_cpu(int cpu)
>  {
> -	smp_ipi_irq_setup(cpu, NPS_IPI_IRQ);
> +	smp_ipi_irq_setup(cpu, irq_find_mapping(NULL, NPS_IPI_IRQ));
>  
>  	eznps_init_core(cpu);
>  	mtm_enable_core(cpu);
> 

Re: [PATCH v2 1/5] ARC: SMP: Use "unsigned virq" in smp_ipi_irq_setup instead of "signed irq"

[Vineet Gupta]

On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
> This function takes a cpu number and a virq number and registers an
> appropriate handler per cpu. However smp_ipi_irq_setup is incorrectly
> used in several places of ARC platform code - hwirq is passed instead
> of virq. This patch is intended to clarify declaration of virq argument
> to prevent passing of hwirq instead of virq in future.
> 
> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> ---
>  arch/arc/include/asm/smp.h | 4 ++--
>  arch/arc/kernel/smp.c      | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arc/include/asm/smp.h b/arch/arc/include/asm/smp.h
> index 89fdd1b..3ebebbc 100644
> --- a/arch/arc/include/asm/smp.h
> +++ b/arch/arc/include/asm/smp.h
> @@ -37,9 +37,9 @@ extern const char *arc_platform_smp_cpuinfo(void);
>   * API expected BY platform smp code (FROM arch smp code)
>   *
>   * smp_ipi_irq_setup:
> - *	Takes @cpu and @irq to which the arch-common ISR is hooked up
> + *	Takes @cpu and @virq to which the arch-common ISR is hooked up

If you agree to my comment on 2/5, then yeah change here to say hwirq, but IMO
this can be squashed with 2/5.

>   */
> -extern int smp_ipi_irq_setup(int cpu, int irq);
> +extern int smp_ipi_irq_setup(int cpu, unsigned int virq);
>  
>  /*
>   * struct plat_smp_ops	- SMP callbacks provided by platform to ARC SMP
> diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> index f183cc6..ec4d956 100644
> --- a/arch/arc/kernel/smp.c
> +++ b/arch/arc/kernel/smp.c
> @@ -351,7 +351,7 @@ irqreturn_t do_IPI(int irq, void *dev_id)
>   */
>  static DEFINE_PER_CPU(int, ipi_dev);
>  
> -int smp_ipi_irq_setup(int cpu, int irq)
> +int smp_ipi_irq_setup(int cpu, unsigned int virq)
>  {
>  	int *dev = per_cpu_ptr(&ipi_dev, cpu);
>  
> @@ -359,12 +359,12 @@ int smp_ipi_irq_setup(int cpu, int irq)
>  	if (!cpu) {
>  		int rc;
>  
> -		rc = request_percpu_irq(irq, do_IPI, "IPI Interrupt", dev);
> +		rc = request_percpu_irq(virq, do_IPI, "IPI Interrupt", dev);
>  		if (rc)
> -			panic("Percpu IRQ request failed for %d\n", irq);
> +			panic("Percpu IRQ request failed for %d\n", virq);
>  	}
>  
> -	enable_percpu_irq(irq, 0);
> +	enable_percpu_irq(virq, 0);
>  
>  	return 0;
>  }
> 

RE: [PATCH v2 1/5] ARC: SMP: Use "unsigned virq" in smp_ipi_irq_setup instead of "signed irq"

[Yuriy Kolerov]

Hi Vineet,

Yes, I agree with you and I will squash these patches.

> -----Original Message-----
> From: Vineet Gupta [mailto:vgupta@synopsys.com]
> Sent: Tuesday, October 25, 2016 5:29 AM
> To: Yuriy Kolerov <yuriy.kolerov@synopsys.com>; linux-snps-
> arc@lists.infradead.org
> Cc: Alexey.Brodkin@synopsys.com; marc.zyngier@arm.com;
> tglx@linutronix.de; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 1/5] ARC: SMP: Use "unsigned virq" in
> smp_ipi_irq_setup instead of "signed irq"
> 
> On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
> > This function takes a cpu number and a virq number and registers an
> > appropriate handler per cpu. However smp_ipi_irq_setup is incorrectly
> > used in several places of ARC platform code - hwirq is passed instead
> > of virq. This patch is intended to clarify declaration of virq
> > argument to prevent passing of hwirq instead of virq in future.
> >
> > Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> > ---
> >  arch/arc/include/asm/smp.h | 4 ++--
> >  arch/arc/kernel/smp.c      | 8 ++++----
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arc/include/asm/smp.h b/arch/arc/include/asm/smp.h
> > index 89fdd1b..3ebebbc 100644
> > --- a/arch/arc/include/asm/smp.h
> > +++ b/arch/arc/include/asm/smp.h
> > @@ -37,9 +37,9 @@ extern const char *arc_platform_smp_cpuinfo(void);
> >   * API expected BY platform smp code (FROM arch smp code)
> >   *
> >   * smp_ipi_irq_setup:
> > - *	Takes @cpu and @irq to which the arch-common ISR is hooked up
> > + *	Takes @cpu and @virq to which the arch-common ISR is hooked up
> 
> If you agree to my comment on 2/5, then yeah change here to say hwirq, but
> IMO this can be squashed with 2/5.
> 
> >   */
> > -extern int smp_ipi_irq_setup(int cpu, int irq);
> > +extern int smp_ipi_irq_setup(int cpu, unsigned int virq);
> >
> >  /*
> >   * struct plat_smp_ops	- SMP callbacks provided by platform to ARC
> SMP
> > diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c index
> > f183cc6..ec4d956 100644
> > --- a/arch/arc/kernel/smp.c
> > +++ b/arch/arc/kernel/smp.c
> > @@ -351,7 +351,7 @@ irqreturn_t do_IPI(int irq, void *dev_id)
> >   */
> >  static DEFINE_PER_CPU(int, ipi_dev);
> >
> > -int smp_ipi_irq_setup(int cpu, int irq)
> > +int smp_ipi_irq_setup(int cpu, unsigned int virq)
> >  {
> >  	int *dev = per_cpu_ptr(&ipi_dev, cpu);
> >
> > @@ -359,12 +359,12 @@ int smp_ipi_irq_setup(int cpu, int irq)
> >  	if (!cpu) {
> >  		int rc;
> >
> > -		rc = request_percpu_irq(irq, do_IPI, "IPI Interrupt", dev);
> > +		rc = request_percpu_irq(virq, do_IPI, "IPI Interrupt", dev);
> >  		if (rc)
> > -			panic("Percpu IRQ request failed for %d\n", irq);
> > +			panic("Percpu IRQ request failed for %d\n", virq);
> >  	}
> >
> > -	enable_percpu_irq(irq, 0);
> > +	enable_percpu_irq(virq, 0);
> >
> >  	return 0;
> >  }
> >

Re: [PATCH v2 5/5] ARC: MCIP: Use IDU_M_DISTRI_DEST mode if there is only 1 destination core

[Vineet Gupta]

On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
> ARC linux uses 2 distribution modes for common interrupts: round robin
> mode (IDU_M_DISTRI_RR) and a simple destination mode (IDU_M_DISTRI_DEST).
> The first one is used when more than 1 cores may handle a common interrupt
> and the second one is used when only 1 core may handle a common interrupt.
> 
> However idu_irq_set_affinity always sets IDU_M_DISTRI_RR for all affinity
> values. But there is no sense in setting of such mode if only 1 core must
> handle a common interrupt.
> 
> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> ---
>  arch/arc/kernel/mcip.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
> index 090f0a1..75e6d73 100644
> --- a/arch/arc/kernel/mcip.c
> +++ b/arch/arc/kernel/mcip.c
> @@ -197,6 +197,7 @@ idu_irq_set_affinity(struct irq_data *data, const struct cpumask *cpumask,
>  {
>  	unsigned long flags;
>  	cpumask_t online;
> +	unsigned long dest_bits;
>  
>  	/* errout if no online cpu per @cpumask */
>  	if (!cpumask_and(&online, cpumask, cpu_online_mask))
> @@ -204,8 +205,14 @@ idu_irq_set_affinity(struct irq_data *data, const struct cpumask *cpumask,
>  
>  	raw_spin_lock_irqsave(&mcip_lock, flags);
>  
> -	idu_set_dest(data->hwirq, cpumask_bits(&online)[0]);
> -	idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_RR);
> +	dest_bits = cpumask_bits(&online)[0];
> +	idu_set_dest(data->hwirq, dest_bits);
> +
> +	if (ffs(dest_bits) == fls(dest_bits)) {
> +		idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_DEST);
> +	} else {
> +		idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_RR);
> +	}

Better to use a local variable to assign IDU_M_xxx and then call idu_set_mode()
outside the if. I know the compiler would do that anyways, but that looks simpler
to read !

But on the other hand, adding all of this here - isn't there some sort of
duplication of code now between here and in the idu_irq_xlate() ?
Do we need the same stuff in 2 places ?

>  
>  	raw_spin_unlock_irqrestore(&mcip_lock, flags);
>  
> 

RE: [PATCH v2 5/5] ARC: MCIP: Use IDU_M_DISTRI_DEST mode if there is only 1 destination core

[Yuriy Kolerov]

> -----Original Message-----
> From: Vineet Gupta [mailto:vgupta@synopsys.com]
> Sent: Tuesday, October 25, 2016 8:53 PM
> To: Yuriy Kolerov <yuriy.kolerov@synopsys.com>; linux-snps-
> arc@lists.infradead.org
> Cc: marc.zyngier@arm.com; Vineet.Gupta1@synopsys.com;
> Alexey.Brodkin@synopsys.com; linux-kernel@vger.kernel.org;
> tglx@linutronix.de
> Subject: Re: [PATCH v2 5/5] ARC: MCIP: Use IDU_M_DISTRI_DEST mode if
> there is only 1 destination core
> 
> On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
> > ARC linux uses 2 distribution modes for common interrupts: round robin
> > mode (IDU_M_DISTRI_RR) and a simple destination mode
> (IDU_M_DISTRI_DEST).
> > The first one is used when more than 1 cores may handle a common
> > interrupt and the second one is used when only 1 core may handle a
> common interrupt.
> >
> > However idu_irq_set_affinity always sets IDU_M_DISTRI_RR for all
> > affinity values. But there is no sense in setting of such mode if only
> > 1 core must handle a common interrupt.
> >
> > Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> > ---
> >  arch/arc/kernel/mcip.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c index
> > 090f0a1..75e6d73 100644
> > --- a/arch/arc/kernel/mcip.c
> > +++ b/arch/arc/kernel/mcip.c
> > @@ -197,6 +197,7 @@ idu_irq_set_affinity(struct irq_data *data, const
> > struct cpumask *cpumask,  {
> >  	unsigned long flags;
> >  	cpumask_t online;
> > +	unsigned long dest_bits;
> >
> >  	/* errout if no online cpu per @cpumask */
> >  	if (!cpumask_and(&online, cpumask, cpu_online_mask)) @@ -204,8
> > +205,14 @@ idu_irq_set_affinity(struct irq_data *data, const struct
> > cpumask *cpumask,
> >
> >  	raw_spin_lock_irqsave(&mcip_lock, flags);
> >
> > -	idu_set_dest(data->hwirq, cpumask_bits(&online)[0]);
> > -	idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL,
> IDU_M_DISTRI_RR);
> > +	dest_bits = cpumask_bits(&online)[0];
> > +	idu_set_dest(data->hwirq, dest_bits);
> > +
> > +	if (ffs(dest_bits) == fls(dest_bits)) {
> > +		idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL,
> IDU_M_DISTRI_DEST);
> > +	} else {
> > +		idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL,
> IDU_M_DISTRI_RR);
> > +	}
> 
> Better to use a local variable to assign IDU_M_xxx and then call
> idu_set_mode() outside the if. I know the compiler would do that anyways,
> but that looks simpler to read !

Yep.

> But on the other hand, adding all of this here - isn't there some sort of
> duplication of code now between here and in the idu_irq_xlate() ?
> Do we need the same stuff in 2 places ?

I tried to explain in in [PATCH v2 4/5]. In short I moved logic for setting affinity to idu_irq_set_affinity because xlate function is not the best place where it must be done (see commit message for that patch).

> >
> >  	raw_spin_unlock_irqrestore(&mcip_lock, flags);
> >
> >

Re: [PATCH v2 4/5] ARC: MCIP: Set an initial affinity value in idu_irq_map

[Vineet Gupta]

On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
> Originally an initial distribution mode (its value resides in Device Tree)
> for each common interrupt is set in idu_irq_xlate. This leads to the
> following problems. idu_irq_xlate may be called several times during parsing
> of the Device Tree and it is semantically wrong to configure an initial
> distribution mode here. Also a value of affinity (CPUs bitmap) is not saved
> to irq_desc structure for the virq - later (after parsing of the DT) kernel
> sees that affinity is not set and sets a default value of affinity (all
> cores in round robin mode). As a result a value of affinity from Device
> Tree is ignored.
> 
> To fix it I created a buffer for initial CPUs bitmaps from Device Tree.
> In idu_irq_xlate those bitmaps are saved to the buffer. Then affinity
> for virq is set manually in idu_irq_map. It works because idu_irq_xlate
> is always called before idu_irq_map.
> 
> Despite the fact that it works I think that it must be rewritten to
> eliminate usage of the buffer and move all logic to idu_irq_map but
> I do not know how to do it correctly.
> 
> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> ---
>  arch/arc/kernel/mcip.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
> index 51a218c..090f0a1 100644
> --- a/arch/arc/kernel/mcip.c
> +++ b/arch/arc/kernel/mcip.c
> @@ -12,12 +12,15 @@
>  #include <linux/irq.h>
>  #include <linux/spinlock.h>
>  #include <linux/irqdomain.h>
> +#include <linux/interrupt.h>
> +#include <linux/cpumask.h>
>  #include <asm/irqflags-arcv2.h>
>  #include <asm/mcip.h>
>  #include <asm/setup.h>
>  
>  static char smp_cpuinfo_buf[128];
>  static int idu_detected;
> +static unsigned long idu_cirq_to_dest[CONFIG_ARC_NUMBER_OF_INTERRUPTS];
>  
>  static DEFINE_RAW_SPINLOCK(mcip_lock);
>  
> @@ -232,9 +235,15 @@ 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)
>  {
> +	cpumask_t mask;
> +
>  	irq_set_chip_and_handler(virq, &idu_irq_chip, handle_level_irq);
>  	irq_set_status_flags(virq, IRQ_MOVE_PCNTXT);
>  
> +	cpumask_clear(&mask);
> +	cpumask_bits(&mask)[0] |= idu_cirq_to_dest[hwirq];
> +	irq_set_affinity(virq, &mask);
> +
>  	return 0;
>  }
>  
> @@ -252,8 +261,7 @@ static int idu_irq_xlate(struct irq_domain *d, struct device_node *n,
>  	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);
> +		idu_cirq_to_dest[hwirq] = BIT(num_online_cpus()) - 1;
>  		raw_spin_unlock_irqrestore(&mcip_lock, flags);
>  	} else {
>  		/*
> @@ -267,8 +275,7 @@ static int idu_irq_xlate(struct irq_domain *d, struct device_node *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);
> +		idu_cirq_to_dest[hwirq] = cpu;
>  		raw_spin_unlock_irqrestore(&mcip_lock, flags);
>  	}
> 

So I missed this part - you are not touching the hardware here at all - and so we
don't really need the spin lock check. Nevertheless, as you said off list, this
patch is more of a hack and we really need to find a saner way of doing this !

@Marc, @tglx any guidance here - changelog at the top has the motivation for this
hack !

Thx,
-Vineet

Re: [PATCH v2 4/5] ARC: MCIP: Set an initial affinity value in idu_irq_map

[Marc Zyngier]

On 25/10/16 19:28, Vineet Gupta wrote:
> On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
>> Originally an initial distribution mode (its value resides in Device Tree)
>> for each common interrupt is set in idu_irq_xlate. This leads to the
>> following problems. idu_irq_xlate may be called several times during parsing
>> of the Device Tree and it is semantically wrong to configure an initial
>> distribution mode here. Also a value of affinity (CPUs bitmap) is not saved
>> to irq_desc structure for the virq - later (after parsing of the DT) kernel
>> sees that affinity is not set and sets a default value of affinity (all
>> cores in round robin mode). As a result a value of affinity from Device
>> Tree is ignored.
>>
>> To fix it I created a buffer for initial CPUs bitmaps from Device Tree.
>> In idu_irq_xlate those bitmaps are saved to the buffer. Then affinity
>> for virq is set manually in idu_irq_map. It works because idu_irq_xlate
>> is always called before idu_irq_map.
>>
>> Despite the fact that it works I think that it must be rewritten to
>> eliminate usage of the buffer and move all logic to idu_irq_map but
>> I do not know how to do it correctly.
>>
>> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
>> ---
>>  arch/arc/kernel/mcip.c | 15 +++++++++++----
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
>> index 51a218c..090f0a1 100644
>> --- a/arch/arc/kernel/mcip.c
>> +++ b/arch/arc/kernel/mcip.c
>> @@ -12,12 +12,15 @@
>>  #include <linux/irq.h>
>>  #include <linux/spinlock.h>
>>  #include <linux/irqdomain.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/cpumask.h>
>>  #include <asm/irqflags-arcv2.h>
>>  #include <asm/mcip.h>
>>  #include <asm/setup.h>
>>  
>>  static char smp_cpuinfo_buf[128];
>>  static int idu_detected;
>> +static unsigned long idu_cirq_to_dest[CONFIG_ARC_NUMBER_OF_INTERRUPTS];
>>  
>>  static DEFINE_RAW_SPINLOCK(mcip_lock);
>>  
>> @@ -232,9 +235,15 @@ 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)
>>  {
>> +	cpumask_t mask;
>> +
>>  	irq_set_chip_and_handler(virq, &idu_irq_chip, handle_level_irq);
>>  	irq_set_status_flags(virq, IRQ_MOVE_PCNTXT);
>>  
>> +	cpumask_clear(&mask);
>> +	cpumask_bits(&mask)[0] |= idu_cirq_to_dest[hwirq];
>> +	irq_set_affinity(virq, &mask);
>> +
>>  	return 0;
>>  }
>>  
>> @@ -252,8 +261,7 @@ static int idu_irq_xlate(struct irq_domain *d, struct device_node *n,
>>  	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);
>> +		idu_cirq_to_dest[hwirq] = BIT(num_online_cpus()) - 1;
>>  		raw_spin_unlock_irqrestore(&mcip_lock, flags);
>>  	} else {
>>  		/*
>> @@ -267,8 +275,7 @@ static int idu_irq_xlate(struct irq_domain *d, struct device_node *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);
>> +		idu_cirq_to_dest[hwirq] = cpu;
>>  		raw_spin_unlock_irqrestore(&mcip_lock, flags);
>>  	}
>>
> 
> So I missed this part - you are not touching the hardware here at all - and so we
> don't really need the spin lock check. Nevertheless, as you said off list, this
> patch is more of a hack and we really need to find a saner way of doing this !
> 
> @Marc, @tglx any guidance here - changelog at the top has the motivation for this
> hack !

It definitely feels weird to encode the interrupt affinity in the DT
(the kernel and possible userspace usually know much better than the
firmware). What is the actual reason for storing the affinity there?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 4/5] ARC: MCIP: Set an initial affinity value in idu_irq_map

[Vineet Gupta]

On 10/26/2016 07:05 AM, Marc Zyngier wrote:
> It definitely feels weird to encode the interrupt affinity in the DT
> (the kernel and possible userspace usually know much better than the
> firmware). What is the actual reason for storing the affinity there?

The IDU intc supports various interrupt distribution modes (Round Robin, send to
one cpu only etc) whcih in turn map to affinity setting. When doing the DT
binding, we decided to add that this to DT to get the "seed" value for affinity -
which user could optionally changed after boot. This seemed like a benign design
choice at the time.

Thx,
-Vineet

Re: [PATCH v2 4/5] ARC: MCIP: Set an initial affinity value in idu_irq_map

[Marc Zyngier]

On Wed, Oct 26 2016 at 05:17:34 PM, Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote:
> On 10/26/2016 07:05 AM, Marc Zyngier wrote:
>> It definitely feels weird to encode the interrupt affinity in the DT
>> (the kernel and possible userspace usually know much better than the
>> firmware). What is the actual reason for storing the affinity there?
>
> The IDU intc supports various interrupt distribution modes (Round
> Robin, send to one cpu only etc) whcih in turn map to affinity
> setting. When doing the DT binding, we decided to add that this to DT
> to get the "seed" value for affinity - which user could optionally
> changed after boot. This seemed like a benign design choice at the
> time.

Right. But is this initial setting something that the kernel has to
absolutely honor? The usual behavior is to let kernel pick something
sensible, and let the user mess with it afterwards.

Is there any part of the kernel that would otherwise depend on this
affinity being set to a particular mode? If the answer is "none", then I
believe we can safely ignore that part of the binding (and maybe
deprecate it in the documentation).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH v2 4/5] ARC: MCIP: Set an initial affinity value in idu_irq_map

[Vineet Gupta]

On 10/26/2016 09:36 AM, Marc Zyngier wrote:
> On Wed, Oct 26 2016 at 05:17:34 PM, Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote:
>> On 10/26/2016 07:05 AM, Marc Zyngier wrote:
>>> It definitely feels weird to encode the interrupt affinity in the DT
>>> (the kernel and possible userspace usually know much better than the
>>> firmware). What is the actual reason for storing the affinity there?
>>
>> The IDU intc supports various interrupt distribution modes (Round
>> Robin, send to one cpu only etc) whcih in turn map to affinity
>> setting. When doing the DT binding, we decided to add that this to DT
>> to get the "seed" value for affinity - which user could optionally
>> changed after boot. This seemed like a benign design choice at the
>> time.
> 
> Right. But is this initial setting something that the kernel has to
> absolutely honor? 

Not necessarily.

> The usual behavior is to let kernel pick something
> sensible, and let the user mess with it afterwards.

Right !


> Is there any part of the kernel that would otherwise depend on this
> affinity being set to a particular mode? If the answer is "none", then I
> believe we can safely ignore that part of the binding (and maybe
> deprecate it in the documentation).

Not really. It was relevant for initial bring up of IDU software and hardware.
e.g. checking that uart behind idu works fine in both modes for very first user
mode prints, which is before you could make the init script cmds to change the
affinity etc. But that bridge has long been crossed.

So agree that we will ignore the affinity settings from DT and deprecate the binding.

Thx for steering us in the right direction. Much appreciated.

-Vineet