Messages in this thread | | | Subject | Re: [PATCH] irqchip/mbigen: Fix the clear register offset | From | Hanjun Guo <> | Date | Thu, 27 Apr 2017 09:59:45 +0800 |
| |
On 2017/4/27 9:27, majun (Euler7) wrote: > Hi Marc: > > 在 2017/4/26 16:01, Marc Zyngier 写道: >> On 26/04/17 04:10, Hanjun Guo wrote: >>> Hi Majun, >>> >>> On 2017/4/25 10:16, Majun wrote: >>>> From: MaJun <majun258@huawei.com> >>>> >>>> Don't minus reserved interrupts (64) when get the clear register offset,because >>>> the clear register space includes the space of these 64 interrupts. >>> >>> Could you mention the background that there is a timeout mechanism >>> to clear the register in the mbigen to make the code work even we clear >>> the wrong (and noneffective) register? that will help for review I >>> think. >> >> A timeout? So if you don't clear the interrupt in a timely manner, it >> will still bypass the masking? That feels very wrong. How is this >> timeout configured? Can it be entirely disabled? > > You are right. The timeout should be turn off usually, it's just a debug or > testing function in our chip. Because of configuration mistake in BIOS, this function > is turned on now and covered the bug in clear offset calculate. > Now, it's time to fix this.
Thanks for the clarify, that means it can be entirely disabled in the firmware.
>> >>> >>>> >>>> Signed-off-by: MaJun <majun258@huawei.com> >>>> --- >>>> drivers/irqchip/irq-mbigen.c | 1 - >>>> 1 file changed, 1 deletion(-) >>>> >>>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c >>>> index 061cdb8..75818a5 100644 >>>> --- a/drivers/irqchip/irq-mbigen.c >>>> +++ b/drivers/irqchip/irq-mbigen.c >>>> @@ -108,7 +108,6 @@ static inline void get_mbigen_clear_reg(irq_hw_number_t hwirq, >>>> { >>>> unsigned int ofst; >>>> >>>> - hwirq -= RESERVED_IRQ_PER_MBIGEN_CHIP; >>>> ofst = hwirq / 32 * 4; >>>> >>>> *mask = 1 << (hwirq % 32); >>> >>> How about following to save more lines of code: >>> >>> --- a/drivers/irqchip/irq-mbigen.c >>> +++ b/drivers/irqchip/irq-mbigen.c >>> @@ -106,10 +106,7 @@ static inline void >>> get_mbigen_type_reg(irq_hw_number_t hwirq, >>> static inline void get_mbigen_clear_reg(irq_hw_number_t hwirq, >>> u32 *mask, u32 *addr) >>> { >>> - unsigned int ofst; >>> - >>> - hwirq -= RESERVED_IRQ_PER_MBIGEN_CHIP; >>> - ofst = hwirq / 32 * 4; >>> + unsigned int ofst = hwirq / 32 * 4; >>> >>> *mask = 1 << (hwirq % 32); >>> *addr = ofst + REG_MBIGEN_CLEAR_OFFSET; >> >> Well, this is not a code deletion contest... ;-)
That's just my personal addiction :)
Thanks Hanjun
| |