From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1956724AbdDZIBR (ORCPT <rfc822;w@1wt.eu>);
        Wed, 26 Apr 2017 04:01:17 -0400
Received: from foss.arm.com ([217.140.101.70]:50810 "EHLO foss.arm.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1956692AbdDZIBH (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 26 Apr 2017 04:01:07 -0400
Subject: Re: [PATCH] irqchip/mbigen: Fix the clear register offset
To: Hanjun Guo <hanjun.guo@linaro.org>, Majun <majun258@huawei.com>,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        tglx@linutronix.de, dingtianhong@huawei.com, guohanjun@huawei.com
References: <1493086563-36396-1-git-send-email-majun258@huawei.com>
 <68214147-bc4d-c542-ce40-78f69a63e53d@linaro.org>
From: Marc Zyngier <marc.zyngier@arm.com>
Organization: ARM Ltd
Message-ID: <07081162-ca52-fa46-00e6-e72e1b4fd092@arm.com>
Date: Wed, 26 Apr 2017 09:01:03 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <68214147-bc4d-c542-ce40-78f69a63e53d@linaro.org>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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?

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

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