From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C8E56C7EE2E for ; Sun, 28 May 2023 17:09:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229566AbjE1RJa (ORCPT ); Sun, 28 May 2023 13:09:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38368 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229453AbjE1RJ3 (ORCPT ); Sun, 28 May 2023 13:09:29 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9B620AD; Sun, 28 May 2023 10:09:28 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 3830760C43; Sun, 28 May 2023 17:09:28 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9E930C433D2; Sun, 28 May 2023 17:09:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685293767; bh=2HpDsz16RbryZeSwk70hXDJ4gla3IAE1NdBPN6l1aP8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Gr7QOQt2l6vjbf3VM5hswjKooNeJfYxzibw4EBvFMzr6n1/GoC6PpJxvrJ/OSbiyc +mKF1bnLp+ges7WvYmqyL7W2abCU90qn/i9l0fP4xb7qKF6wfRSh7saGoj6JTORp9v 5KsEMMULspxFSEltwBYRnArSLyi6T3YXQbVvAbU+6E0Cn1O3mZhj6AOa+Re9io82A0 bAL+LUT9h9y7xq1qEBrVNKXVTFEKyN/Ex25o8am3ApyPZLEcER9wLiabH1WIqw3ztq cftIDCy+iZtZOBNoONnZOKsvPH2uQUDTw19nFruZATKPF/sytCKpatxgMXRSmXqHVk gIqRX+UivGbDQ== Date: Sun, 28 May 2023 18:25:43 +0100 From: Jonathan Cameron To: Matti Vaittinen Cc: Matti Vaittinen , Andy Shevchenko , Daniel Scally , Heikki Krogerus , Sakari Ailus , Greg Kroah-Hartman , "Rafael J. Wysocki" , Wolfram Sang , Lars-Peter Clausen , Michael Hennerich , Andreas Klinger , Marcin Wojtas , Russell King , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Jonathan =?UTF-8?B?TmV1c2Now6RmZXI=?= , Linus Walleij , Paul Cercueil , Akhil R , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org, linux-iio@vger.kernel.org, netdev@vger.kernel.org, openbmc@lists.ozlabs.org, linux-gpio@vger.kernel.org, linux-mips@vger.kernel.org Subject: Re: [PATCH v6 1/8] drivers: fwnode: fix fwnode_irq_get[_byname]() Message-ID: <20230528182543.656da0d1@jic23-huawei> In-Reply-To: References: X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Fri, 26 May 2023 09:35:30 +0300 Matti Vaittinen wrote: > The fwnode_irq_get() and the fwnode_irq_get_byname() return 0 upon > device-tree IRQ mapping failure. This is contradicting the > fwnode_irq_get_byname() function documentation and can potentially be a > source of errors like: > > int probe(...) { > ... > > irq = fwnode_irq_get_byname(); > if (irq <= 0) > return irq; > > ... > } > > Here we do correctly check the return value from fwnode_irq_get_byname() > but the driver probe will now return success. (There was already one > such user in-tree). > > Change the fwnode_irq_get_byname() to work as documented and make also the > fwnode_irq_get() follow same common convention returning a negative errno > upon failure. > > Fixes: ca0acb511c21 ("device property: Add fwnode_irq_get_byname") > Suggested-by: Sakari Ailus > Suggested-by: Jonathan Cameron > Signed-off-by: Matti Vaittinen > Reviewed-by: Andy Shevchenko > This bothers me a little because there may be drivers that haven't been caught yet that assume the zero value. Still this is more consistent with what I'd expect to happen, so fair enough Reviewed-by: Jonathan Cameron > --- > I dropped the existing reviewed-by tags because change to > fwnode_irq_get() was added. > > Revision history: > v4 =>: > - No Changes > v3 => v4: > - Change also the fwnode_irq_get() > --- > drivers/base/property.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/property.c b/drivers/base/property.c > index f6117ec9805c..8c40abed7852 100644 > --- a/drivers/base/property.c > +++ b/drivers/base/property.c > @@ -987,12 +987,18 @@ EXPORT_SYMBOL(fwnode_iomap); > * @fwnode: Pointer to the firmware node > * @index: Zero-based index of the IRQ > * > - * Return: Linux IRQ number on success. Other values are determined > - * according to acpi_irq_get() or of_irq_get() operation. > + * Return: Linux IRQ number on success. Negative errno on failure. > */ > int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index) > { > - return fwnode_call_int_op(fwnode, irq_get, index); > + int ret; > + > + ret = fwnode_call_int_op(fwnode, irq_get, index); > + /* We treat mapping errors as invalid case */ > + if (ret == 0) > + return -EINVAL; > + > + return ret; > } > EXPORT_SYMBOL(fwnode_irq_get); >