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

[Marc Zyngier]

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...