[PATCH v3 0/2] irqchip/loongson-eiointc: Add simple irq routing method

[PATCH v3 0/2] irqchip/loongson-eiointc: Add simple irq routing method

[Bibo Mao]

Fix return value checking of eiointc_index where int type
is converted uint32_t and check smaller than 0.

Add simple irq route support on system with only one eioi node,
rather than use anysend method.

---
Changes in v3:
  Modify some spell checking problems.

Changes in v2:
  Use the simple irq routing on embeded board like 2K0500 and 2K2000
board, since there is only one eio node.

---
Bibo Mao (2):
  irqchip/loongson-eiointc: Fix return value checking of eiointc_index
  irqchip/loongson-eiointc: Simplify irq routing on some platforms

 drivers/irqchip/irq-loongson-eiointc.c | 91 ++++++++++++++++++++++----
 1 file changed, 80 insertions(+), 11 deletions(-)

-- 
2.27.0

[PATCH v3 1/2] irqchip/loongson-eiointc: Fix return value checking of eiointc_index

[Bibo Mao]

Return value of function eiointc_index is int, however it is converted
into uint32_t and then compared smaller than zero. This causes logic
problem. There is eioint initialization problem on qemu virt-machine
where there is only one eioint node and more than 4 vcpus. Nodemap of
eioint is 1, and external device intr can only be routed to vcpu 0-3, the
other vcpus can not response any external device interrupts and only local
processor interrupts like ipi/timer can work.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 drivers/irqchip/irq-loongson-eiointc.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
index 92d8aa28bdf5..1c5a5b59f199 100644
--- a/drivers/irqchip/irq-loongson-eiointc.c
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -144,12 +144,14 @@ static int eiointc_router_init(unsigned int cpu)
 	int i, bit;
 	uint32_t data;
 	uint32_t node = cpu_to_eio_node(cpu);
-	uint32_t index = eiointc_index(node);
+	int index = eiointc_index(node);
 
-	if (index < 0) {
-		pr_err("Error: invalid nodemap!\n");
-		return -1;
-	}
+	/*
+	 * qemu virt-machine has only one eio intc and more than four cpus
+	 * irq from eio can only be routed to cpu 0-3 on virt machine
+	 */
+	if (index < 0)
+		return 0;
 
 	if ((cpu_logical_map(cpu) % CORES_PER_EIO_NODE) == 0) {
 		eiointc_enable();
-- 
2.27.0

Re: [PATCH v3 1/2] irqchip/loongson-eiointc: Fix return value checking of eiointc_index

[Philippe Mathieu-Daudé]

On 11/7/23 14:08, Bibo Mao wrote:
> Return value of function eiointc_index is int, however it is converted
> into uint32_t and then compared smaller than zero. This causes logic
> problem. There is eioint initialization problem on qemu virt-machine
> where there is only one eioint node and more than 4 vcpus. Nodemap of
> eioint is 1, and external device intr can only be routed to vcpu 0-3, the
> other vcpus can not response any external device interrupts and only local
> processor interrupts like ipi/timer can work.
> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>   drivers/irqchip/irq-loongson-eiointc.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)

Fixes: dd281e1a1a93 ("irqchip: Add Loongson Extended I/O interrupt 
controller support")

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 1/2] irqchip/loongson-eiointc: Fix return value checking of eiointc_index

[Huacai Chen]

On Tue, Jul 11, 2023 at 8:08 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
> Return value of function eiointc_index is int, however it is converted
> into uint32_t and then compared smaller than zero. This causes logic
> problem. There is eioint initialization problem on qemu virt-machine
> where there is only one eioint node and more than 4 vcpus. Nodemap of
> eioint is 1, and external device intr can only be routed to vcpu 0-3, the
> other vcpus can not response any external device interrupts and only local
> processor interrupts like ipi/timer can work.
I'm sorry but there are still spelling problems...
"eio", "eio intc", "eioint" should all be "eiointc".

Huacai

>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  drivers/irqchip/irq-loongson-eiointc.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
> index 92d8aa28bdf5..1c5a5b59f199 100644
> --- a/drivers/irqchip/irq-loongson-eiointc.c
> +++ b/drivers/irqchip/irq-loongson-eiointc.c
> @@ -144,12 +144,14 @@ static int eiointc_router_init(unsigned int cpu)
>         int i, bit;
>         uint32_t data;
>         uint32_t node = cpu_to_eio_node(cpu);
> -       uint32_t index = eiointc_index(node);
> +       int index = eiointc_index(node);
>
> -       if (index < 0) {
> -               pr_err("Error: invalid nodemap!\n");
> -               return -1;
> -       }
> +       /*
> +        * qemu virt-machine has only one eio intc and more than four cpus
> +        * irq from eio can only be routed to cpu 0-3 on virt machine
> +        */
> +       if (index < 0)
> +               return 0;
>
>         if ((cpu_logical_map(cpu) % CORES_PER_EIO_NODE) == 0) {
>                 eiointc_enable();
> --
> 2.27.0
>

Re: [PATCH v3 1/2] irqchip/loongson-eiointc: Fix return value checking of eiointc_index

[bibo mao]

在 2023/7/11 23:06, Huacai Chen 写道:
> On Tue, Jul 11, 2023 at 8:08 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> Return value of function eiointc_index is int, however it is converted
>> into uint32_t and then compared smaller than zero. This causes logic
>> problem. There is eioint initialization problem on qemu virt-machine
>> where there is only one eioint node and more than 4 vcpus. Nodemap of
>> eioint is 1, and external device intr can only be routed to vcpu 0-3, the
>> other vcpus can not response any external device interrupts and only local
>> processor interrupts like ipi/timer can work.
> I'm sorry but there are still spelling problems...
> "eio", "eio intc", "eioint" should all be "eiointc".
Sure, thanks for pointing it out, will modify in next version.

Regards
Bibo Mao
> 
> Huacai
> 
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>  drivers/irqchip/irq-loongson-eiointc.c | 12 +++++++-----
>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
>> index 92d8aa28bdf5..1c5a5b59f199 100644
>> --- a/drivers/irqchip/irq-loongson-eiointc.c
>> +++ b/drivers/irqchip/irq-loongson-eiointc.c
>> @@ -144,12 +144,14 @@ static int eiointc_router_init(unsigned int cpu)
>>         int i, bit;
>>         uint32_t data;
>>         uint32_t node = cpu_to_eio_node(cpu);
>> -       uint32_t index = eiointc_index(node);
>> +       int index = eiointc_index(node);
>>
>> -       if (index < 0) {
>> -               pr_err("Error: invalid nodemap!\n");
>> -               return -1;
>> -       }
>> +       /*
>> +        * qemu virt-machine has only one eio intc and more than four cpus
>> +        * irq from eio can only be routed to cpu 0-3 on virt machine
>> +        */
>> +       if (index < 0)
>> +               return 0;
>>
>>         if ((cpu_logical_map(cpu) % CORES_PER_EIO_NODE) == 0) {
>>                 eiointc_enable();
>> --
>> 2.27.0
>>

Re: [PATCH v3 1/2] irqchip/loongson-eiointc: Fix return value checking of eiointc_index

[Marc Zyngier]

On Tue, 11 Jul 2023 13:08:06 +0100,
Bibo Mao <maobibo@loongson.cn> wrote:
> 
> Return value of function eiointc_index is int, however it is converted
> into uint32_t and then compared smaller than zero. This causes logic
> problem. There is eioint initialization problem on qemu virt-machine
> where there is only one eioint node and more than 4 vcpus. Nodemap of
> eioint is 1, and external device intr can only be routed to vcpu 0-3, the
> other vcpus can not response any external device interrupts and only local
> processor interrupts like ipi/timer can work.
> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  drivers/irqchip/irq-loongson-eiointc.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
> index 92d8aa28bdf5..1c5a5b59f199 100644
> --- a/drivers/irqchip/irq-loongson-eiointc.c
> +++ b/drivers/irqchip/irq-loongson-eiointc.c
> @@ -144,12 +144,14 @@ static int eiointc_router_init(unsigned int cpu)
>  	int i, bit;
>  	uint32_t data;
>  	uint32_t node = cpu_to_eio_node(cpu);
> -	uint32_t index = eiointc_index(node);
> +	int index = eiointc_index(node);
>  
> -	if (index < 0) {
> -		pr_err("Error: invalid nodemap!\n");
> -		return -1;
> -	}
> +	/*
> +	 * qemu virt-machine has only one eio intc and more than four cpus
> +	 * irq from eio can only be routed to cpu 0-3 on virt machine
> +	 */
> +	if (index < 0)
> +		return 0;
>  
>  	if ((cpu_logical_map(cpu) % CORES_PER_EIO_NODE) == 0) {
>  		eiointc_enable();

This sort of thing really needs a Fixes: tag.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v3 1/2] irqchip/loongson-eiointc: Fix return value checking of eiointc_index

[bibo mao]

在 2023/7/12 16:56, Marc Zyngier 写道:
> On Tue, 11 Jul 2023 13:08:06 +0100,
> Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> Return value of function eiointc_index is int, however it is converted
>> into uint32_t and then compared smaller than zero. This causes logic
>> problem. There is eioint initialization problem on qemu virt-machine
>> where there is only one eioint node and more than 4 vcpus. Nodemap of
>> eioint is 1, and external device intr can only be routed to vcpu 0-3, the
>> other vcpus can not response any external device interrupts and only local
>> processor interrupts like ipi/timer can work.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>  drivers/irqchip/irq-loongson-eiointc.c | 12 +++++++-----
>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
>> index 92d8aa28bdf5..1c5a5b59f199 100644
>> --- a/drivers/irqchip/irq-loongson-eiointc.c
>> +++ b/drivers/irqchip/irq-loongson-eiointc.c
>> @@ -144,12 +144,14 @@ static int eiointc_router_init(unsigned int cpu)
>>  	int i, bit;
>>  	uint32_t data;
>>  	uint32_t node = cpu_to_eio_node(cpu);
>> -	uint32_t index = eiointc_index(node);
>> +	int index = eiointc_index(node);
>>  
>> -	if (index < 0) {
>> -		pr_err("Error: invalid nodemap!\n");
>> -		return -1;
>> -	}
>> +	/*
>> +	 * qemu virt-machine has only one eio intc and more than four cpus
>> +	 * irq from eio can only be routed to cpu 0-3 on virt machine
>> +	 */
>> +	if (index < 0)
>> +		return 0;
>>  
>>  	if ((cpu_logical_map(cpu) % CORES_PER_EIO_NODE) == 0) {
>>  		eiointc_enable();
> 
> This sort of thing really needs a Fixes: tag.
Sure, will add Fixes tag.

I am a newbie to lkml, thanks for kindly pointing it out :)

Regards
Bibo Mao

> 
> Thanks,
> 
> 	M.
> 

[PATCH v3 2/2] irqchip/loongson-eiointc: Simplify irq routing on some platforms

[Bibo Mao]

Some LoongArch systems have only one eiointc node such as 3A5000/2K2000
and qemu virt-machine. If there is only one eiointc node, all cpus can
access eiointc registers directly; if there is multiple eiointc nodes, each
cpu can only access eiointc belonging to specified node group, so anysend
or ipi needs to be used to configure irq routing. IRQ routing is simple on
such systems with one node, hacking method like anysend is not necessary.

This patch provides simpile IRQ routing method for systems with one eiointc
node, and is tested on 3A5000 board and qemu virt-machine.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 drivers/irqchip/irq-loongson-eiointc.c | 79 ++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
index 1c5a5b59f199..5c15a582075d 100644
--- a/drivers/irqchip/irq-loongson-eiointc.c
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -127,6 +127,48 @@ static int eiointc_set_irq_affinity(struct irq_data *d, const struct cpumask *af
 	return IRQ_SET_MASK_OK;
 }
 
+static int eiointc_single_set_irq_affinity(struct irq_data *d,
+				const struct cpumask *affinity, bool force)
+{
+	unsigned int cpu;
+	unsigned long flags;
+	uint32_t vector, regaddr, data, coremap;
+	struct cpumask mask;
+	struct eiointc_priv *priv = d->domain->host_data;
+
+	cpumask_and(&mask, affinity, cpu_online_mask);
+	cpumask_and(&mask, &mask, &priv->cpuspan_map);
+	if (cpumask_empty(&mask))
+		return -EINVAL;
+
+	cpu = cpumask_first(&mask);
+	vector = d->hwirq;
+	regaddr = EIOINTC_REG_ENABLE + ((vector >> 5) << 2);
+	data = ~BIT(vector & 0x1F);
+	coremap = BIT(cpu_logical_map(cpu) % CORES_PER_EIO_NODE);
+
+	/*
+	 * simplify for only one eio node
+	 * access eio registers directly rather than
+	 * use any_send hack method here
+	 */
+	raw_spin_lock_irqsave(&affinity_lock, flags);
+	iocsr_write32(EIOINTC_ALL_ENABLE & data, regaddr);
+	/*
+	 * get irq route info for continuous 4 vectors
+	 * and set affinity for specified vector
+	 */
+	data = iocsr_read32(EIOINTC_REG_ROUTE + (vector & ~3));
+	data &=  ~(0xff << ((vector & 3) * 8));
+	data |= coremap << ((vector & 3) * 8);
+	iocsr_write32(data, EIOINTC_REG_ROUTE + (vector & ~3));
+	iocsr_write32(EIOINTC_ALL_ENABLE, regaddr);
+	raw_spin_unlock_irqrestore(&affinity_lock, flags);
+
+	irq_data_update_effective_affinity(d, cpumask_of(cpu));
+	return IRQ_SET_MASK_OK;
+}
+
 static int eiointc_index(int node)
 {
 	int i;
@@ -237,22 +279,39 @@ static struct irq_chip eiointc_irq_chip = {
 	.irq_set_affinity	= eiointc_set_irq_affinity,
 };
 
+static struct irq_chip eiointc_irq_chipi_single = {
+	.name			= "EIOINTC-S",
+	.irq_ack		= eiointc_ack_irq,
+	.irq_mask		= eiointc_mask_irq,
+	.irq_unmask		= eiointc_unmask_irq,
+#ifdef CONFIG_SMP
+	.irq_set_affinity       = eiointc_single_set_irq_affinity,
+#endif
+};
+
 static int eiointc_domain_alloc(struct irq_domain *domain, unsigned int virq,
 				unsigned int nr_irqs, void *arg)
 {
 	int ret;
 	unsigned int i, type;
 	unsigned long hwirq = 0;
-	struct eiointc *priv = domain->host_data;
+	struct eiointc_priv *priv = domain->host_data;
+	struct irq_chip *chip;
 
 	ret = irq_domain_translate_onecell(domain, arg, &hwirq, &type);
 	if (ret)
 		return ret;
 
-	for (i = 0; i < nr_irqs; i++) {
-		irq_domain_set_info(domain, virq + i, hwirq + i, &eiointc_irq_chip,
+	/*
+	 * use simple irq route method for single node eiointc
+	 */
+	if ((nr_pics == 1) && (nodes_weight(priv->node_map) == 1))
+		chip = &eiointc_irq_chipi_single;
+	else
+		chip = &eiointc_irq_chip;
+	for (i = 0; i < nr_irqs; i++)
+		irq_domain_set_info(domain, virq + i, hwirq + i, chip,
 					priv, handle_edge_irq, NULL, NULL);
-	}
 
 	return 0;
 }
@@ -309,6 +368,7 @@ static void eiointc_resume(void)
 	int i, j;
 	struct irq_desc *desc;
 	struct irq_data *irq_data;
+	struct irq_chip *chip;
 
 	eiointc_router_init(0);
 
@@ -318,7 +378,8 @@ static void eiointc_resume(void)
 			if (desc && desc->handle_irq && desc->handle_irq != handle_bad_irq) {
 				raw_spin_lock(&desc->lock);
 				irq_data = irq_domain_get_irq_data(eiointc_priv[i]->eiointc_domain, irq_desc_get_irq(desc));
-				eiointc_set_irq_affinity(irq_data, irq_data->common->affinity, 0);
+				chip = irq_data_get_irq_chip(irq_data);
+				chip->irq_set_affinity(irq_data, irq_data->common->affinity, 0);
 				raw_spin_unlock(&desc->lock);
 			}
 		}
@@ -496,7 +557,13 @@ static int __init eiointc_of_init(struct device_node *of_node,
 	priv->node = 0;
 	priv->domain_handle = of_node_to_fwnode(of_node);
 
-	ret = eiointc_init(priv, parent_irq, 0);
+	/*
+	 * 2k0500 and 2k2000 has only one eio node
+	 * set nodemap as 1 for simple irq routing
+	 * what about for future embedded board more than 4 cpus??
+	 * nodemap and node need be added in dts like acpi table
+	 */
+	ret = eiointc_init(priv, parent_irq, 1);
 	if (ret < 0)
 		goto out_free_priv;
 
-- 
2.27.0