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

From: Marc Zyngier
Date: Wed Apr 26 2017 - 04:01:19 EST


On 26/04/17 04:10, Hanjun Guo wrote:
> Hi Majun,
>
> On 2017/4/25 10:16, Majun wrote:
>> From: MaJun <majun258@xxxxxxxxxx>
>>
>> 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?

>
>>
>> Signed-off-by: MaJun <majun258@xxxxxxxxxx>
>> ---
>> 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... ;-)

M.
--
Jazz is not dead. It just smells funny...