From:   Thomas Gleixner <tglx@linutronix.de>
To:     Shanker Donthineni <sdonthineni@nvidia.com>,
        Marc Zyngier <maz@kernel.org>, Michael Walle <michael@walle.cc>
Cc:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        Hans de Goede <hdegoede@redhat.com>,
        Wolfram Sang <wsa+renesas@sang-engineering.com>,
        Shanker Donthineni <sdonthineni@nvidia.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] genirq: Use the common function irq_expand_nr_irqs()
In-Reply-To: <20230130005725.3517597-5-sdonthineni@nvidia.com>
References: <20230130005725.3517597-1-sdonthineni@nvidia.com>
 <20230130005725.3517597-5-sdonthineni@nvidia.com>
Date:   Tue, 31 Jan 2023 10:35:23 +0100
Message-ID: <87h6w711yc.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Sun, Jan 29 2023 at 18:57, Shanker Donthineni wrote:

> Subject: genirq: Use the common function ...

  genirq: Unify irq_expand_nr_irqs()

  irq_expand_nr_irqs() is implemented as a stub function for !SPARSEIRQ
  builds. That's not necessary as the SPARSEIRQ version returns -ENOMEM
  correctly even for the !SPARSEIRQ case as the ....


But this common function is non-obvious for the !SPARSEIRQ case. It at
least needs a comment

> +static int irq_expand_nr_irqs(unsigned int nr)
> +{
> +	if (nr > MAX_SPARSE_IRQS)
> +		return -ENOMEM;
> +	nr_irqs = nr;
> +	return 0;
> +}

or preferrably something like this:

	if (!IS_ENABLED(CONFIG_SPARSEIRQ) || nr > MAX_SPARSE_IRQS)
		return -ENOMEM;

which makes it entirely clear and also allows the compiler to optimize
is down to a 'return -ENOMEM'.

Thanks,

        tglx