Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Wed, 29 Nov 2023 09:41:31 -0600
From:   Rob Herring <robh@kernel.org>
To:     Dan Carpenter <dan.carpenter@linaro.org>
Cc:     James Tai <james.tai@realtek.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Marc Zyngier <maz@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        kernel test robot <lkp@intel.com>,
        Dan Carpenter <error27@gmail.com>
Subject: Re: [PATCH v3 2/6] irqchip: Add interrupt controller support for
 Realtek DHC SoCs
Message-ID: <20231129154131.GA2492847-robh@kernel.org>
References: <20231129054339.3054202-1-james.tai@realtek.com>
 <20231129054339.3054202-3-james.tai@realtek.com>
 <d94c79bf-04c4-4e87-bd7e-a8755508ac89@suswa.mountain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d94c79bf-04c4-4e87-bd7e-a8755508ac89@suswa.mountain>
Precedence: bulk

On Wed, Nov 29, 2023 at 11:21:06AM +0300, Dan Carpenter wrote:
> On Wed, Nov 29, 2023 at 01:43:35PM +0800, James Tai wrote:
> > +static int realtek_intc_subset(struct device_node *node, struct realtek_intc_data *data, int index)
> > +{
> > +	struct realtek_intc_subset_data *subset_data = &data->subset_data[index];
> > +	const struct realtek_intc_subset_cfg *cfg = &data->info->cfg[index];
> > +	int irq;
> > +
> > +	irq = irq_of_parse_and_map(node, index);
> > +	if (irq <= 0)
> > +		return irq;
> 
> I don't think irq_of_parse_and_map() can return negatives.  Only zero
> on error.  Returning zero on error is a historical artifact with IRQ
> functions and a constant source of bugs.  But here returning zero is
> success.  See my blog for more details:
> https://staticthinking.wordpress.com/2023/08/07/writing-a-check-for-zero-irq-error-codes/

It's worse than that. The irq functions historically returned NO_IRQ on 
error, but that could be 0 or -1 depending on the arch.

Use of_irq_get() instead. It's a bit better in that it returns an error 
code for most cases. But still returns 0 on mapping failure.

Rob