Messages in this thread | | | Subject | Re: [PATCH V1 3/4] irqchip/loongson-liointc: Fix IRQ trigger polarity | From | Jianmin Lv <> | Date | Mon, 22 May 2023 16:58:34 +0800 |
| |
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); >
| |