The patch series provide some fixes for loongson-liointc and loongson-pch-pic driver.
Jianmin Lv (2):
irqchip/loongson-pch-pic: Fix initialization of HT vector register
irqchip/loongson-liointc: Fix IRQ trigger polarity
Liu Peibao (1):
irqchip/loongson-pch-pic: Fix potential incorrect hwirq assignment
Yinbo Zhu (1):
irqchip/loongson-liointc: Add IRQCHIP_SKIP_SET_WAKE flag
drivers/irqchip/irq-loongson-liointc.c | 9 +++++----
drivers/irqchip/irq-loongson-pch-pic.c | 10 ++++------
2 files changed, 9 insertions(+), 10 deletions(-)
--
2.31.1
From: Liu Peibao <[email protected]>
In DeviceTree path, when ht_vec_base is not zero, the hwirq of PCH PIC will
be assigned incorrectly. Because when pch_pic_domain_translate() adds the
ht_vec_base to hwirq, the hwirq dose not subtract the ht_vec_base when
calling irq_domain_set_info().
The ht_vec_base is designed for the parent irq chip/domain of the PCH PIC.
It seems not proper to deal this in callbacks of the PCH PIC domain and
let's put this back like the initial commit ef8c01eb64ca ("irqchip: Add
Loongson PCH PIC controller").
Fixes: bcdd75c596c8 ("irqchip/loongson-pch-pic: Add ACPI init support")
Cc: [email protected]
Signed-off-by: Liu Peibao <[email protected]>
Signed-off-by: Jianmin Lv <[email protected]>
---
drivers/irqchip/irq-loongson-pch-pic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index 921c5c0190d1..93a71f66efeb 100644
--- a/drivers/irqchip/irq-loongson-pch-pic.c
+++ b/drivers/irqchip/irq-loongson-pch-pic.c
@@ -164,7 +164,7 @@ static int pch_pic_domain_translate(struct irq_domain *d,
if (fwspec->param_count < 2)
return -EINVAL;
- *hwirq = fwspec->param[0] + priv->ht_vec_base;
+ *hwirq = fwspec->param[0];
*type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
} else {
if (fwspec->param_count < 1)
@@ -196,7 +196,7 @@ static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
parent_fwspec.fwnode = domain->parent->fwnode;
parent_fwspec.param_count = 1;
- parent_fwspec.param[0] = hwirq;
+ parent_fwspec.param[0] = hwirq + priv->ht_vec_base;
err = irq_domain_alloc_irqs_parent(domain, virq, 1, &parent_fwspec);
if (err)
--
2.31.1
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: [email protected]
Signed-off-by: Chong Qiao <[email protected]>
Signed-off-by: Jianmin Lv <[email protected]>
---
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
From: Yinbo Zhu <[email protected]>
Liointc doesn't require specific logic to work with wakeup IRQs,
and no irq_set_wake callback is needed. To allow registered IRQs
from liointc to be used as a wakeup-source, and ensure irq_set_irq_wake()
works well, the flag IRQCHIP_SKIP_SET_WAKE should be added.
Signed-off-by: Yinbo Zhu <[email protected]>
Signed-off-by: Jianmin Lv <[email protected]>
---
drivers/irqchip/irq-loongson-liointc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
index 9a9c2bf048a3..dbd1ccce0fb2 100644
--- a/drivers/irqchip/irq-loongson-liointc.c
+++ b/drivers/irqchip/irq-loongson-liointc.c
@@ -291,6 +291,7 @@ static int liointc_init(phys_addr_t addr, unsigned long size, int revision,
ct->chip.irq_mask = irq_gc_mask_disable_reg;
ct->chip.irq_mask_ack = irq_gc_mask_disable_reg;
ct->chip.irq_set_type = liointc_set_type;
+ ct->chip.flags = IRQCHIP_SKIP_SET_WAKE;
gc->mask_cache = 0;
priv->gc = gc;
--
2.31.1
Hi, Jianmin,
On Sat, May 20, 2023 at 2:38 PM Jianmin Lv <[email protected]> wrote:
>
> From: Yinbo Zhu <[email protected]>
>
> Liointc doesn't require specific logic to work with wakeup IRQs,
> and no irq_set_wake callback is needed. To allow registered IRQs
> from liointc to be used as a wakeup-source, and ensure irq_set_irq_wake()
> works well, the flag IRQCHIP_SKIP_SET_WAKE should be added.
Maybe using LIOINTC instead of Liointc/liointc is better in commit
message. Except this small issue, for the whole series,
Reviewed-by: Huacai Chen <[email protected]>
>
> Signed-off-by: Yinbo Zhu <[email protected]>
> Signed-off-by: Jianmin Lv <[email protected]>
> ---
> drivers/irqchip/irq-loongson-liointc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
> index 9a9c2bf048a3..dbd1ccce0fb2 100644
> --- a/drivers/irqchip/irq-loongson-liointc.c
> +++ b/drivers/irqchip/irq-loongson-liointc.c
> @@ -291,6 +291,7 @@ static int liointc_init(phys_addr_t addr, unsigned long size, int revision,
> ct->chip.irq_mask = irq_gc_mask_disable_reg;
> ct->chip.irq_mask_ack = irq_gc_mask_disable_reg;
> ct->chip.irq_set_type = liointc_set_type;
> + ct->chip.flags = IRQCHIP_SKIP_SET_WAKE;
>
> gc->mask_cache = 0;
> priv->gc = gc;
> --
> 2.31.1
>
>
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: [email protected]
> Signed-off-by: Chong Qiao <[email protected]>
> Signed-off-by: Jianmin Lv <[email protected]>
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/
On 2023/5/20 14:38, Jianmin Lv wrote:
> From: Liu Peibao <[email protected]>
>
> In DeviceTree path, when ht_vec_base is not zero, the hwirq of PCH PIC will
> be assigned incorrectly. Because when pch_pic_domain_translate() adds the
> ht_vec_base to hwirq, the hwirq dose not subtract the ht_vec_base when
"does not have the ht_vec_base subtracted"?
> calling irq_domain_set_info().
>
> The ht_vec_base is designed for the parent irq chip/domain of the PCH PIC.
> It seems not proper to deal this in callbacks of the PCH PIC domain and
> let's put this back like the initial commit ef8c01eb64ca ("irqchip: Add
> Loongson PCH PIC controller").
>
> Fixes: bcdd75c596c8 ("irqchip/loongson-pch-pic: Add ACPI init support")
> Cc: [email protected]
> Signed-off-by: Liu Peibao <[email protected]>
> Signed-off-by: Jianmin Lv <[email protected]>
> ---
> drivers/irqchip/irq-loongson-pch-pic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
> index 921c5c0190d1..93a71f66efeb 100644
> --- a/drivers/irqchip/irq-loongson-pch-pic.c
> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
> @@ -164,7 +164,7 @@ static int pch_pic_domain_translate(struct irq_domain *d,
> if (fwspec->param_count < 2)
> return -EINVAL;
>
> - *hwirq = fwspec->param[0] + priv->ht_vec_base;
> + *hwirq = fwspec->param[0];
> *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
> } else {
> if (fwspec->param_count < 1)
> @@ -196,7 +196,7 @@ static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
>
> parent_fwspec.fwnode = domain->parent->fwnode;
> parent_fwspec.param_count = 1;
> - parent_fwspec.param[0] = hwirq;
> + parent_fwspec.param[0] = hwirq + priv->ht_vec_base;
>
> err = irq_domain_alloc_irqs_parent(domain, virq, 1, &parent_fwspec);
> if (err)
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
On 2023/5/21 下午6:31, WANG Xuerui wrote:
> On 2023/5/20 14:38, Jianmin Lv wrote:
>> From: Liu Peibao <[email protected]>
>>
>> In DeviceTree path, when ht_vec_base is not zero, the hwirq of PCH PIC
>> will
>> be assigned incorrectly. Because when pch_pic_domain_translate() adds the
>> ht_vec_base to hwirq, the hwirq dose not subtract the ht_vec_base when
>
> "does not have the ht_vec_base subtracted"?
>
Ok, I'll change it as your suggestion, thanks.
>> calling irq_domain_set_info().
>>
>> The ht_vec_base is designed for the parent irq chip/domain of the PCH
>> PIC.
>> It seems not proper to deal this in callbacks of the PCH PIC domain and
>> let's put this back like the initial commit ef8c01eb64ca ("irqchip: Add
>> Loongson PCH PIC controller").
>>
>> Fixes: bcdd75c596c8 ("irqchip/loongson-pch-pic: Add ACPI init support")
>> Cc: [email protected]
>> Signed-off-by: Liu Peibao <[email protected]>
>> Signed-off-by: Jianmin Lv <[email protected]>
>> ---
>> drivers/irqchip/irq-loongson-pch-pic.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c
>> b/drivers/irqchip/irq-loongson-pch-pic.c
>> index 921c5c0190d1..93a71f66efeb 100644
>> --- a/drivers/irqchip/irq-loongson-pch-pic.c
>> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
>> @@ -164,7 +164,7 @@ static int pch_pic_domain_translate(struct
>> irq_domain *d,
>> if (fwspec->param_count < 2)
>> return -EINVAL;
>> - *hwirq = fwspec->param[0] + priv->ht_vec_base;
>> + *hwirq = fwspec->param[0];
>> *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
>> } else {
>> if (fwspec->param_count < 1)
>> @@ -196,7 +196,7 @@ static int pch_pic_alloc(struct irq_domain
>> *domain, unsigned int virq,
>> parent_fwspec.fwnode = domain->parent->fwnode;
>> parent_fwspec.param_count = 1;
>> - parent_fwspec.param[0] = hwirq;
>> + parent_fwspec.param[0] = hwirq + priv->ht_vec_base;
>> err = irq_domain_alloc_irqs_parent(domain, virq, 1,
>> &parent_fwspec);
>> if (err)
>
On 2023/5/20 下午4:43, Huacai Chen wrote:
> Hi, Jianmin,
>
> On Sat, May 20, 2023 at 2:38 PM Jianmin Lv <[email protected]> wrote:
>>
>> From: Yinbo Zhu <[email protected]>
>>
>> Liointc doesn't require specific logic to work with wakeup IRQs,
>> and no irq_set_wake callback is needed. To allow registered IRQs
>> from liointc to be used as a wakeup-source, and ensure irq_set_irq_wake()
>> works well, the flag IRQCHIP_SKIP_SET_WAKE should be added.
> Maybe using LIOINTC instead of Liointc/liointc is better in commit
> message. Except this small issue, for the whole series,
> Reviewed-by: Huacai Chen <[email protected]>
>
Ok, I'll change it, thanks.
>>
>> Signed-off-by: Yinbo Zhu <[email protected]>
>> Signed-off-by: Jianmin Lv <[email protected]>
>> ---
>> drivers/irqchip/irq-loongson-liointc.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
>> index 9a9c2bf048a3..dbd1ccce0fb2 100644
>> --- a/drivers/irqchip/irq-loongson-liointc.c
>> +++ b/drivers/irqchip/irq-loongson-liointc.c
>> @@ -291,6 +291,7 @@ static int liointc_init(phys_addr_t addr, unsigned long size, int revision,
>> ct->chip.irq_mask = irq_gc_mask_disable_reg;
>> ct->chip.irq_mask_ack = irq_gc_mask_disable_reg;
>> ct->chip.irq_set_type = liointc_set_type;
>> + ct->chip.flags = IRQCHIP_SKIP_SET_WAKE;
>>
>> gc->mask_cache = 0;
>> priv->gc = gc;
>> --
>> 2.31.1
>>
>>
> _______________________________________________
> Loongson-kernel mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
>
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: [email protected]
>> Signed-off-by: Chong Qiao <[email protected]>
>> Signed-off-by: Jianmin Lv <[email protected]>
>
> 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 <[email protected]>
Signed-off-by: Chong Qiao <[email protected]>
Signed-off-by: Jianmin Lv <[email protected]>
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);
>