From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9D6AC10E8 for ; Sun, 21 May 2023 17:13:26 +0000 (UTC) Received: from fgw20-7.mail.saunalahti.fi (fgw20-7.mail.saunalahti.fi [62.142.5.81]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5B81910E for ; Sun, 21 May 2023 10:13:23 -0700 (PDT) Received: from localhost (88-113-26-95.elisa-laajakaista.fi [88.113.26.95]) by fgw20.mail.saunalahti.fi (Halon) with ESMTP id c6755f98-f7fa-11ed-b3cf-005056bd6ce9; Sun, 21 May 2023 20:13:18 +0300 (EEST) From: andy.shevchenko@gmail.com Date: Sun, 21 May 2023 20:13:16 +0300 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 , Jonathan Cameron , Andreas Klinger , Marcin Wojtas , Russell King , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Jonathan =?iso-8859-1?Q?Neusch=E4fer?= , 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 v5 1/8] drivers: fwnode: fix fwnode_irq_get[_byname]() Message-ID: References: <339cc23ccae4580d5551cc2b6b9b4afdde48f25e.1684493615.git.mazziesaccount@gmail.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <339cc23ccae4580d5551cc2b6b9b4afdde48f25e.1684493615.git.mazziesaccount@gmail.com> X-Spam-Status: No, score=0.7 required=5.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FORGED_GMAIL_RCVD,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED,RCVD_IN_MSPIKE_H2, SPF_HELO_NONE,SPF_SOFTFAIL,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Fri, May 19, 2023 at 02:00:54PM +0300, Matti Vaittinen kirjoitti: > 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. Reviewed-by: Andy Shevchenko > Fixes: ca0acb511c21 ("device property: Add fwnode_irq_get_byname") > Suggested-by: Sakari Ailus > Suggested-by: Jonathan Cameron > Signed-off-by: Matti Vaittinen > > --- > I dropped the existing reviewed-by tags because change to > fwnode_irq_get() was added. > > Revision history: > 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; Not sure if this is the best choice, perhaps -EEXIST or -ENOENT might be better, but it's just a spoken up thought. > + return ret; > } > EXPORT_SYMBOL(fwnode_irq_get); -- With Best Regards, Andy Shevchenko