Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 14 Sep 2022 13:01:50 +0100
Message-ID: <87wna6kvv5.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Zhang Jianhua <chris.zjh@huawei.com>
Cc:     <tglx@linutronix.de>, <samuel@sholland.org>, <brgl@bgdev.pl>,
        <mark.rutland@arm.com>, <lvjianmin@loongson.cn>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH -next] genirq: Change can_request_irq() return value type to bool
In-Reply-To: <20220914110615.3570933-1-chris.zjh@huawei.com>
References: <20220914110615.3570933-1-chris.zjh@huawei.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Wed, 14 Sep 2022 12:06:15 +0100,
Zhang Jianhua <chris.zjh@huawei.com> wrote:
> 
> The function can_request_irq() is used to judge whether the irq can be
> allocated, so bool type would be more suitable for it.
> 
> Signed-off-by: Zhang Jianhua <chris.zjh@huawei.com>
> ---
>  include/linux/irq.h | 2 +-
>  kernel/irq/manage.c | 8 ++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index c3eb89606c2b..3a60c2313fb9 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -707,7 +707,7 @@ extern void note_interrupt(struct irq_desc *desc, irqreturn_t action_ret);
>  extern int noirqdebug_setup(char *str);
>  
>  /* Checks whether the interrupt can be requested by request_irq(): */
> -extern int can_request_irq(unsigned int irq, unsigned long irqflags);
> +extern bool can_request_irq(unsigned int irq, unsigned long irqflags);
>  
>  /* Dummy irq-chip implementations: */
>  extern struct irq_chip no_irq_chip;
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 40fe7806cc8c..d6940d15bf56 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -925,19 +925,19 @@ EXPORT_SYMBOL(irq_set_irq_wake);
>   * particular irq has been exclusively allocated or is available
>   * for driver use.
>   */
> -int can_request_irq(unsigned int irq, unsigned long irqflags)
> +bool can_request_irq(unsigned int irq, unsigned long irqflags)
>  {
>  	unsigned long flags;
>  	struct irq_desc *desc = irq_get_desc_lock(irq, &flags, 0);
> -	int canrequest = 0;
> +	bool canrequest = false;
>  
>  	if (!desc)
> -		return 0;
> +		return false;
>  
>  	if (irq_settings_can_request(desc)) {
>  		if (!desc->action ||
>  		    irqflags & desc->action->flags & IRQF_SHARED)
> -			canrequest = 1;
> +			canrequest = true;
>  	}
>  	irq_put_desc_unlock(desc, flags);
>  	return canrequest;

I'm sorry, but this is a very pointless change. Not only this doesn't
change anything for this particular code other than being cosmetic,
but it also doesn't help any of the callers which are still using an
int.

In general, this sort of patch only adds noise, and I'd like to see
less of them.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.