[PATCH] irqchip/mbigen: Fix the clear register offset

Hanjun Guo
2017-04-27T01:59:45Z

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