[PATCH V1 0/4] irqchip/loongson: Fix some loongson irqchip drivers

[PATCH V1 0/4] irqchip/loongson: Fix some loongson irqchip drivers

[Jianmin Lv]

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

[PATCH V1 1/4] irqchip/loongson-pch-pic: Fix initialization of HT vector register

[Jianmin Lv]

In a dual-bridge system based ACPI, the IRQ on PCH PIC of
each bridge sent to CPU is always a zero-based number, which
means that the IRQ on PCH PIC of each bridge is mapped into
vector range from 0 to 63 of upstream irqchip(e.g. EIOINTC).

      EIOINTC N: [0 ... 63 | 64 ... 255]
                  --------   ----------
                      ^          ^
                      |          |
                  PCH PIC N      |
                             PCH MSI N

For example, the IRQ vector number of sata controller on
PCH PIC of each bridge is 16, which is sent to upstream
irqchip of EIOINTC when an interrupt occurs, which will set
bit 16 of EIOINTC. Since hwirq of 16 on EIOINTC has been
mapped to a irq_desc for sata controller during hierarchy
irq allocation, the related mapped IRQ will be found through
irq_resolve_mapping() in the IRQ domain of EIOINTC.

So, the IRQ number set in HT vector register should be fixed
to be a zero-based number.

Cc: stable@vger.kernel.org
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: liuyun <liuyun@loongson.cn>
---
 drivers/irqchip/irq-loongson-pch-pic.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index e5fe4d50be05..921c5c0190d1 100644
--- a/drivers/irqchip/irq-loongson-pch-pic.c
+++ b/drivers/irqchip/irq-loongson-pch-pic.c
@@ -401,14 +401,12 @@ static int __init acpi_cascade_irqdomain_init(void)
 int __init pch_pic_acpi_init(struct irq_domain *parent,
 					struct acpi_madt_bio_pic *acpi_pchpic)
 {
-	int ret, vec_base;
+	int ret;
 	struct fwnode_handle *domain_handle;
 
 	if (find_pch_pic(acpi_pchpic->gsi_base) >= 0)
 		return 0;
 
-	vec_base = acpi_pchpic->gsi_base - GSI_MIN_PCH_IRQ;
-
 	domain_handle = irq_domain_alloc_fwnode(&acpi_pchpic->address);
 	if (!domain_handle) {
 		pr_err("Unable to allocate domain handle\n");
@@ -416,7 +414,7 @@ int __init pch_pic_acpi_init(struct irq_domain *parent,
 	}
 
 	ret = pch_pic_init(acpi_pchpic->address, acpi_pchpic->size,
-				vec_base, parent, domain_handle, acpi_pchpic->gsi_base);
+				0, parent, domain_handle, acpi_pchpic->gsi_base);
 
 	if (ret < 0) {
 		irq_domain_free_fwnode(domain_handle);
-- 
2.31.1

Re: [PATCH V1 1/4] irqchip/loongson-pch-pic: Fix initialization of HT vector register

[WANG Xuerui]

Hi,

On 2023/5/20 14:38, Jianmin Lv wrote:
> In a dual-bridge system based ACPI, the IRQ on PCH PIC of

"In an ACPI-based dual-bridge system"?

Also what's "the IRQ on PCH PIC of each bridge", is it "IRQ of each 
bridge's PCH PIC"? Or did I misunderstand it?

> each bridge sent to CPU is always a zero-based number, which
> means that the IRQ on PCH PIC of each bridge is mapped into
> vector range from 0 to 63 of upstream irqchip(e.g. EIOINTC).
> 
>        EIOINTC N: [0 ... 63 | 64 ... 255]
>                    --------   ----------
>                        ^          ^
>                        |          |
>                    PCH PIC N      |
>                               PCH MSI N
> 
> For example, the IRQ vector number of sata controller on
> PCH PIC of each bridge is 16, which is sent to upstream
> irqchip of EIOINTC when an interrupt occurs, which will set
> bit 16 of EIOINTC. Since hwirq of 16 on EIOINTC has been
> mapped to a irq_desc for sata controller during hierarchy
> irq allocation, the related mapped IRQ will be found through
> irq_resolve_mapping() in the IRQ domain of EIOINTC.
> 
> So, the IRQ number set in HT vector register should be fixed
> to be a zero-based number.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> Signed-off-by: liuyun <liuyun@loongson.cn>

What's the patch's proper authorship, Yun Liu or you? Based on the 
ordering of the tags it seems you should be the author, but Yun should 
be submitting (because their tag comes later, the patch should be 
flowing from your tree to theirs); otherwise they should be the author 
and the two signoff lines should be reversed.

> ---
>   drivers/irqchip/irq-loongson-pch-pic.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
> index e5fe4d50be05..921c5c0190d1 100644
> --- a/drivers/irqchip/irq-loongson-pch-pic.c
> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
> @@ -401,14 +401,12 @@ static int __init acpi_cascade_irqdomain_init(void)
>   int __init pch_pic_acpi_init(struct irq_domain *parent,
>   					struct acpi_madt_bio_pic *acpi_pchpic)
>   {
> -	int ret, vec_base;
> +	int ret;
>   	struct fwnode_handle *domain_handle;
>   
>   	if (find_pch_pic(acpi_pchpic->gsi_base) >= 0)
>   		return 0;
>   
> -	vec_base = acpi_pchpic->gsi_base - GSI_MIN_PCH_IRQ;
> -
>   	domain_handle = irq_domain_alloc_fwnode(&acpi_pchpic->address);
>   	if (!domain_handle) {
>   		pr_err("Unable to allocate domain handle\n");
> @@ -416,7 +414,7 @@ int __init pch_pic_acpi_init(struct irq_domain *parent,
>   	}
>   
>   	ret = pch_pic_init(acpi_pchpic->address, acpi_pchpic->size,
> -				vec_base, parent, domain_handle, acpi_pchpic->gsi_base);
> +				0, parent, domain_handle, acpi_pchpic->gsi_base);
>   
>   	if (ret < 0) {
>   		irq_domain_free_fwnode(domain_handle);

-- 
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/

Re: [PATCH V1 1/4] irqchip/loongson-pch-pic: Fix initialization of HT vector register

[Jianmin Lv]

On 2023/5/21 下午6:29, WANG Xuerui wrote:
> Hi,
> 
> On 2023/5/20 14:38, Jianmin Lv wrote:
>> In a dual-bridge system based ACPI, the IRQ on PCH PIC of
> 
> "In an ACPI-based dual-bridge system"?
> 
> Also what's "the IRQ on PCH PIC of each bridge", is it "IRQ of each 
> bridge's PCH PIC"? Or did I misunderstand it?
> 

Thanks for your comment here, I'll change them in next version.

>> each bridge sent to CPU is always a zero-based number, which
>> means that the IRQ on PCH PIC of each bridge is mapped into
>> vector range from 0 to 63 of upstream irqchip(e.g. EIOINTC).
>>
>>        EIOINTC N: [0 ... 63 | 64 ... 255]
>>                    --------   ----------
>>                        ^          ^
>>                        |          |
>>                    PCH PIC N      |
>>                               PCH MSI N
>>
>> For example, the IRQ vector number of sata controller on
>> PCH PIC of each bridge is 16, which is sent to upstream
>> irqchip of EIOINTC when an interrupt occurs, which will set
>> bit 16 of EIOINTC. Since hwirq of 16 on EIOINTC has been
>> mapped to a irq_desc for sata controller during hierarchy
>> irq allocation, the related mapped IRQ will be found through
>> irq_resolve_mapping() in the IRQ domain of EIOINTC.
>>
>> So, the IRQ number set in HT vector register should be fixed
>> to be a zero-based number.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>> Signed-off-by: liuyun <liuyun@loongson.cn>
> 
> What's the patch's proper authorship, Yun Liu or you? Based on the 
> ordering of the tags it seems you should be the author, but Yun should 
> be submitting (because their tag comes later, the patch should be 
> flowing from your tree to theirs); otherwise they should be the author 
> and the two signoff lines should be reversed.
> 

Ok, I'll change them as following:
Co-developed-by: liuyun <liuyun@loongson.cn>
Signed-off-by: liuyun <liuyun@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>

Thanks.

>> ---
>>   drivers/irqchip/irq-loongson-pch-pic.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c 
>> b/drivers/irqchip/irq-loongson-pch-pic.c
>> index e5fe4d50be05..921c5c0190d1 100644
>> --- a/drivers/irqchip/irq-loongson-pch-pic.c
>> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
>> @@ -401,14 +401,12 @@ static int __init acpi_cascade_irqdomain_init(void)
>>   int __init pch_pic_acpi_init(struct irq_domain *parent,
>>                       struct acpi_madt_bio_pic *acpi_pchpic)
>>   {
>> -    int ret, vec_base;
>> +    int ret;
>>       struct fwnode_handle *domain_handle;
>>       if (find_pch_pic(acpi_pchpic->gsi_base) >= 0)
>>           return 0;
>> -    vec_base = acpi_pchpic->gsi_base - GSI_MIN_PCH_IRQ;
>> -
>>       domain_handle = irq_domain_alloc_fwnode(&acpi_pchpic->address);
>>       if (!domain_handle) {
>>           pr_err("Unable to allocate domain handle\n");
>> @@ -416,7 +414,7 @@ int __init pch_pic_acpi_init(struct irq_domain 
>> *parent,
>>       }
>>       ret = pch_pic_init(acpi_pchpic->address, acpi_pchpic->size,
>> -                vec_base, parent, domain_handle, acpi_pchpic->gsi_base);
>> +                0, parent, domain_handle, acpi_pchpic->gsi_base);
>>       if (ret < 0) {
>>           irq_domain_free_fwnode(domain_handle);
> 

[PATCH V1 2/4] irqchip/loongson-pch-pic: Fix potential incorrect hwirq assignment

[Jianmin Lv]

From: Liu Peibao <liupeibao@loongson.cn>

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: stable@vger.kernel.org
Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
 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

Re: [PATCH V1 2/4] irqchip/loongson-pch-pic: Fix potential incorrect hwirq assignment

[WANG Xuerui]

On 2023/5/20 14:38, Jianmin Lv wrote:
> From: Liu Peibao <liupeibao@loongson.cn>
> 
> 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: stable@vger.kernel.org
> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> ---
>   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/

Re: [PATCH V1 2/4] irqchip/loongson-pch-pic: Fix potential incorrect hwirq assignment

[Jianmin Lv]

On 2023/5/21 下午6:31, WANG Xuerui wrote:
> On 2023/5/20 14:38, Jianmin Lv wrote:
>> From: Liu Peibao <liupeibao@loongson.cn>
>>
>> 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: stable@vger.kernel.org
>> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>> ---
>>   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)
> 

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

[Jianmin Lv]

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

[WANG Xuerui]

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

[Jianmin Lv]

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);
> 

[PATCH V1 4/4] irqchip/loongson-liointc: Add IRQCHIP_SKIP_SET_WAKE flag

[Jianmin Lv]

From: Yinbo Zhu <zhuyinbo@loongson.cn>

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 <zhuyinbo@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
 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

Re: [PATCH V1 4/4] irqchip/loongson-liointc: Add IRQCHIP_SKIP_SET_WAKE flag

[Huacai Chen]

Hi, Jianmin,

On Sat, May 20, 2023 at 2:38 PM Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
> From: Yinbo Zhu <zhuyinbo@loongson.cn>
>
> 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 <chenhuacai@loongson.cn>

>
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> ---
>  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
>
>

Re: [PATCH V1 4/4] irqchip/loongson-liointc: Add IRQCHIP_SKIP_SET_WAKE flag

[Jianmin Lv]

On 2023/5/20 下午4:43, Huacai Chen wrote:
> Hi, Jianmin,
> 
> On Sat, May 20, 2023 at 2:38 PM Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>> From: Yinbo Zhu <zhuyinbo@loongson.cn>
>>
>> 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 <chenhuacai@loongson.cn>
> 
Ok, I'll change it, thanks.

>>
>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>> ---
>>   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 -- loongson-kernel@lists.loongnix.cn
> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn
> 

[PATCH V1 0/4] irqchip/loongson: Fix some loongson irqchip drivers

[Jianmin Lv]

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

[PATCH V1 2/4] irqchip/loongson-pch-pic: Fix potential incorrect hwirq assignment

[Jianmin Lv]

From: Liu Peibao <liupeibao@loongson.cn>

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: stable@vger.kernel.org
Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
 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