1. Patchew
  2. linux
  3. [PATCH V1 0/4] irqchip/loongson: Fix some loongson irqchip drivers
  4. View patch

[PATCH V1 3/4] irqchip/loongson-liointc: Fix IRQ trigger polarity

Jianmin Lv posted 4 patches 2 years, 8 months ago
There is a newer version of this series
[PATCH V1 3/4] irqchip/loongson-liointc: Fix IRQ trigger polarity
Posted by Jianmin Lv 2 years, 8 months ago 
For IRQ controller INT_POLARITY regitser of Loongson-2K CPU
series, '0' indicates high level or rising edge triggered IRQ,
'1' indicates low level or falling edge triggered IRQ.

For Loongson-3A CPU series, setting INT_POLARITY register is not
supported and writting it has no effect.

So trigger polarity setting shouled be fixed for Loongson-2K CPU
series.

Fixes: 17343d0b4039 ("irqchip/loongson-liointc: Support to set IRQ type for ACPI path")
Cc: stable@vger.kernel.org
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
 drivers/irqchip/irq-loongson-liointc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
index 8d00a9ad5b00..9a9c2bf048a3 100644
--- a/drivers/irqchip/irq-loongson-liointc.c
+++ b/drivers/irqchip/irq-loongson-liointc.c
@@ -116,19 +116,19 @@ static int liointc_set_type(struct irq_data *data, unsigned int type)
 	switch (type) {
 	case IRQ_TYPE_LEVEL_HIGH:
 		liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, false);
-		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
+		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
 		break;
 	case IRQ_TYPE_LEVEL_LOW:
 		liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, false);
-		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
+		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
 		break;
 	case IRQ_TYPE_EDGE_RISING:
 		liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, true);
-		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
+		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
 		break;
 	case IRQ_TYPE_EDGE_FALLING:
 		liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, true);
-		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
+		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
 		break;
 	default:
 		irq_gc_unlock_irqrestore(gc, flags);
-- 
2.31.1
Re: [PATCH V1 3/4] irqchip/loongson-liointc: Fix IRQ trigger polarity
Posted by WANG Xuerui 2 years, 8 months ago 
On 2023/5/20 14:38, Jianmin Lv wrote:
> For IRQ controller INT_POLARITY regitser of Loongson-2K CPU

"For the INT_POLARITY register of Loongson-2K series IRQ controller"?

> series, '0' indicates high level or rising edge triggered IRQ,
> '1' indicates low level or falling edge triggered IRQ.

Remove the two "IRQ"s; the topic is "polarity", not "IRQs".

Also please mention the source of this information; I've checked the 
Loongson 2K1000LA User Manual v1.0 and it seems a similar description is 
found in Table 9-2, Section 9.3 (中断寄存器描述 / Description of the Interrupt 
Registers). It mentioned "Intpol_0" and "Intpol_1" but the description 
is consistent with the wording here.

> 
> For Loongson-3A CPU series, setting INT_POLARITY register is not
> supported and writting it has no effect.

Only 3A and not the whole Loongson-3 series?

Also typo: "writing".

> 
> So trigger polarity setting shouled be fixed for Loongson-2K CPU
> series.

The changes seem to be just inversion of the polarity flags. It should 
be correct given your description, and not affect Loongson-3 series 
because it's supposed to behave as noops; it may be better to move the 
explanation regarding Loongson-3 behavior to code comment (e.g. 
somewhere near the definition of LIOINTC_REG_INTC_POL) so it's 
immediately visible to drive-by readers not familiar with LoongArch 
internals, without them having to dig through commit history to see this.

> 
> Fixes: 17343d0b4039 ("irqchip/loongson-liointc: Support to set IRQ type for ACPI path")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>

Again, who's the proper author for this patch? Given the tags it seems 
the author should be Chong Qiao, but I didn't see an Author: line at the 
beginning.

> ---
>   drivers/irqchip/irq-loongson-liointc.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
> index 8d00a9ad5b00..9a9c2bf048a3 100644
> --- a/drivers/irqchip/irq-loongson-liointc.c
> +++ b/drivers/irqchip/irq-loongson-liointc.c
> @@ -116,19 +116,19 @@ static int liointc_set_type(struct irq_data *data, unsigned int type)
>   	switch (type) {
>   	case IRQ_TYPE_LEVEL_HIGH:
>   		liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, false);
> -		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
> +		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
>   		break;
>   	case IRQ_TYPE_LEVEL_LOW:
>   		liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, false);
> -		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
> +		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
>   		break;
>   	case IRQ_TYPE_EDGE_RISING:
>   		liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, true);
> -		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
> +		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
>   		break;
>   	case IRQ_TYPE_EDGE_FALLING:
>   		liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, true);
> -		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
> +		liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
>   		break;
>   	default:
>   		irq_gc_unlock_irqrestore(gc, flags);

-- 
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
Re: [PATCH V1 3/4] irqchip/loongson-liointc: Fix IRQ trigger polarity
Posted by Jianmin Lv 2 years, 8 months ago 

On 2023/5/21 下午6:46, WANG Xuerui wrote:
> On 2023/5/20 14:38, Jianmin Lv wrote:
>> For IRQ controller INT_POLARITY regitser of Loongson-2K CPU
> 
> "For the INT_POLARITY register of Loongson-2K series IRQ controller"?
> 
>> series, '0' indicates high level or rising edge triggered IRQ,
>> '1' indicates low level or falling edge triggered IRQ.
> 
> Remove the two "IRQ"s; the topic is "polarity", not "IRQs".
> 
> Also please mention the source of this information; I've checked the 
> Loongson 2K1000LA User Manual v1.0 and it seems a similar description is 
> found in Table 9-2, Section 9.3 (中断寄存器描述 / Description of the 
> Interrupt Registers). It mentioned "Intpol_0" and "Intpol_1" but the 
> description is consistent with the wording here.
> 
>>
>> For Loongson-3A CPU series, setting INT_POLARITY register is not
>> supported and writting it has no effect.
> 
> Only 3A and not the whole Loongson-3 series?
> 
> Also typo: "writing".
> 

Ok, I'll adjust the commit as your suggestion above, thanks.

>>
>> So trigger polarity setting shouled be fixed for Loongson-2K CPU
>> series.
> 
> The changes seem to be just inversion of the polarity flags. It should 
> be correct given your description, and not affect Loongson-3 series 
> because it's supposed to behave as noops; it may be better to move the 
> explanation regarding Loongson-3 behavior to code comment (e.g. 
> somewhere near the definition of LIOINTC_REG_INTC_POL) so it's 
> immediately visible to drive-by readers not familiar with LoongArch 
> internals, without them having to dig through commit history to see this.
> 
Good suggestion, I'll add the information near the definition of 
LIOINTC_REG_INTC_POL.

>>
>> Fixes: 17343d0b4039 ("irqchip/loongson-liointc: Support to set IRQ 
>> type for ACPI path")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> 
> Again, who's the proper author for this patch? Given the tags it seems 
> the author should be Chong Qiao, but I didn't see an Author: line at the 
> beginning.
> 

Again, I'll adjust them as following:
Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>

Thanks.

>> ---
>>   drivers/irqchip/irq-loongson-liointc.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-liointc.c 
>> b/drivers/irqchip/irq-loongson-liointc.c
>> index 8d00a9ad5b00..9a9c2bf048a3 100644
>> --- a/drivers/irqchip/irq-loongson-liointc.c
>> +++ b/drivers/irqchip/irq-loongson-liointc.c
>> @@ -116,19 +116,19 @@ static int liointc_set_type(struct irq_data 
>> *data, unsigned int type)
>>       switch (type) {
>>       case IRQ_TYPE_LEVEL_HIGH:
>>           liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, false);
>> -        liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
>> +        liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
>>           break;
>>       case IRQ_TYPE_LEVEL_LOW:
>>           liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, false);
>> -        liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
>> +        liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
>>           break;
>>       case IRQ_TYPE_EDGE_RISING:
>>           liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, true);
>> -        liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
>> +        liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
>>           break;
>>       case IRQ_TYPE_EDGE_FALLING:
>>           liointc_set_bit(gc, LIOINTC_REG_INTC_EDGE, mask, true);
>> -        liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, false);
>> +        liointc_set_bit(gc, LIOINTC_REG_INTC_POL, mask, true);
>>           break;
>>       default:
>>           irq_gc_unlock_irqrestore(gc, flags);
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.