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 alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4D6CEC433EF for ; Wed, 12 Jan 2022 16:48:32 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id A44301B61; Wed, 12 Jan 2022 17:47:40 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz A44301B61 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1642006110; bh=LPfSAoUOtclsJGz3LzXEx6N6WlMYwykwtcwq/XxIZ/M=; h=Date:From:To:Subject:References:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=owOOs+6M6p2RxtG7/OE5riRtBsh2HvWfq/XI3vqJQ72nQvmupg25PtQI6zXq4ycqJ DB3yz4PUGH4Bpw5LpTS0WbIGUbdsUugPgRSRVK61WbvVYLgkW5apbsD88VuQTpi8iw jgHm3Pf7Jek3+TGXSn6jsqVTo1RX3ebbvLeM2JYc= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id DB149F8052D; Wed, 12 Jan 2022 17:45:10 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id E2EE4F8026A; Wed, 12 Jan 2022 14:39:09 +0100 (CET) Received: from vps0.lunn.ch (vps0.lunn.ch [185.16.172.187]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 2700AF80054 for ; Wed, 12 Jan 2022 14:38:57 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 2700AF80054 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b="REJii9uM" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=S/pA7bUX6e4VB1GHe3hlypsDdeFRt4oGL0831NNCTJM=; b=REJii9uMw/mNDMEDkaB6nMl41t HVia+zgOKIfzuMj09o6jQ+IaM7PowDfcCzSXN02NEw/KiBFCm/WUr1uWHbYcv6vi03Q5vZQPWTCZw kZaHOnCDnZ4glu9pEOOBCeugcwmvyPmDFDeJp1Ck0ttEnrICihUcMQo96DS7dmh4ziSA=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1n7dpl-001CDK-PQ; Wed, 12 Jan 2022 14:38:37 +0100 Date: Wed, 12 Jan 2022 14:38:37 +0100 From: Andrew Lunn To: Geert Uytterhoeven Subject: Re: [PATCH 1/2] platform: make platform_get_irq_optional() optional Message-ID: References: <20220110195449.12448-1-s.shtylyov@omp.ru> <20220110195449.12448-2-s.shtylyov@omp.ru> <20220110201014.mtajyrfcfznfhyqm@pengutronix.de> <20220112085009.dbasceh3obfok5dc@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailman-Approved-At: Wed, 12 Jan 2022 17:45:04 +0100 Cc: Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , ALSA Development Mailing List , Andy Shevchenko , Joakim Zhang , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , linux-phy@lists.infradead.org, netdev@vger.kernel.org, linux-spi , Jiri Slaby , openipmi-developer@lists.sourceforge.net, Khuong Dinh , Florian Fainelli , Matthias Schiffer , Kamal Dasu , Lee Jones , Bartosz Golaszewski , Daniel Lezcano , Kishon Vijay Abraham I , bcm-kernel-feedback-list , "open list:SERIAL DRIVERS" , Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Jakub Kicinski , Zhang Rui , Linux PWM List , Hans de Goede , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Mauro Carvalho Chehab , John Garry , Peter Korsgaard , William Breathitt Gray , Mark Gross , "open list:GPIO SUBSYSTEM" , Alex Williamson , Mark Brown , Borislav Petkov , Sebastian Reichel , Matthias Brugger , Takashi Iwai , platform-driver-x86@vger.kernel.org, Benson Leung , Linux ARM , linux-edac@vger.kernel.org, Tony Luck , Mun Yew Tham , Eric Auger , Greg Kroah-Hartman , Yoshihiro Shimoda , Cornelia Huck , Linux MMC List , Liam Girdwood , Linux Kernel Mailing List , Linux-Renesas , Sergey Shtylyov , Vinod Koul , James Morse , Zha Qipeng , Pengutronix Kernel Team , Richard Weinberger , Niklas =?iso-8859-1?Q?S=F6derlund?= , linux-mediatek@lists.infradead.org, Brian Norris , "David S. Miller" X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" > If an optional IRQ is not present, drivers either just ignore it (e.g. > for devices that can have multiple interrupts or a single muxed IRQ), > or they have to resort to polling. For the latter, fall-back handling > is needed elsewhere in the driver. > To me it sounds much more logical for the driver to check if an > optional irq is non-zero (available) or zero (not available), than to > sprinkle around checks for -ENXIO. In addition, you have to remember > that this one returns -ENXIO, while other APIs use -ENOENT or -ENOSYS > (or some other error code) to indicate absence. I thought not having > to care about the actual error code was the main reason behind the > introduction of the *_optional() APIs. The *_optional() functions return an error code if there has been a real error which should be reported up the call stack. This excludes whatever error code indicates the requested resource does not exist, which can be -ENODEV etc. If the device does not exist, a magic cookie is returned which appears to be a valid resources but in fact is not. So the users of these functions just need to check for an error code, and fail the probe if present. You seems to be suggesting in binary return value: non-zero (available) or zero (not available) This discards the error code when something goes wrong. That is useful information to have, so we should not be discarding it. IRQ don't currently have a magic cookie value. One option would be to add such a magic cookie to the subsystem. Otherwise, since 0 is invalid, return 0 to indicate the IRQ does not exist. The request for a script checking this then makes sense. However, i don't know how well coccinelle/sparse can track values across function calls. They probably can check for: ret = irq_get_optional() if (ret < 0) return ret; A missing if < 0 statement somewhere later is very likely to be an error. A comparison of <= 0 is also likely to be an error. A check for > 0 before calling any other IRQ functions would be good. I'm surprised such a check does not already existing in the IRQ API, but there are probably historical reasons for that. Andrew 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 1ACE8C433FE for ; Wed, 12 Jan 2022 13:40:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1353658AbiALNkR (ORCPT ); Wed, 12 Jan 2022 08:40:17 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:34014 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241154AbiALNkR (ORCPT ); Wed, 12 Jan 2022 08:40:17 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=S/pA7bUX6e4VB1GHe3hlypsDdeFRt4oGL0831NNCTJM=; b=REJii9uMw/mNDMEDkaB6nMl41t HVia+zgOKIfzuMj09o6jQ+IaM7PowDfcCzSXN02NEw/KiBFCm/WUr1uWHbYcv6vi03Q5vZQPWTCZw kZaHOnCDnZ4glu9pEOOBCeugcwmvyPmDFDeJp1Ck0ttEnrICihUcMQo96DS7dmh4ziSA=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1n7dpl-001CDK-PQ; Wed, 12 Jan 2022 14:38:37 +0100 Date: Wed, 12 Jan 2022 14:38:37 +0100 From: Andrew Lunn To: Geert Uytterhoeven Cc: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , ALSA Development Mailing List , Liam Girdwood , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , linux-phy@lists.infradead.org, Jiri Slaby , "David S. Miller" , Khuong Dinh , Florian Fainelli , Matthias Schiffer , Joakim Zhang , Kamal Dasu , Lee Jones , Bartosz Golaszewski , Daniel Lezcano , Tony Luck , Kishon Vijay Abraham I , bcm-kernel-feedback-list , "open list:SERIAL DRIVERS" , Jakub Kicinski , Zhang Rui , Matthias Brugger , platform-driver-x86@vger.kernel.org, Linux PWM List , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Mauro Carvalho Chehab , John Garry , Peter Korsgaard , William Breathitt Gray , Mark Gross , "open list:GPIO SUBSYSTEM" , Alex Williamson , Mark Brown , Borislav Petkov , Eric Auger , Takashi Iwai , Jaroslav Kysela , openipmi-developer@lists.sourceforge.net, Andy Shevchenko , Benson Leung , Pengutronix Kernel Team , Linux ARM , linux-edac@vger.kernel.org, Sergey Shtylyov , Richard Weinberger , Mun Yew Tham , Hans de Goede , Greg Kroah-Hartman , Yoshihiro Shimoda , Cornelia Huck , Linux MMC List , Linux Kernel Mailing List , linux-spi , Linux-Renesas , Vinod Koul , James Morse , Zha Qipeng , Sebastian Reichel , Niklas =?iso-8859-1?Q?S=F6derlund?= , linux-mediatek@lists.infradead.org, Brian Norris , netdev@vger.kernel.org Subject: Re: [PATCH 1/2] platform: make platform_get_irq_optional() optional Message-ID: References: <20220110195449.12448-1-s.shtylyov@omp.ru> <20220110195449.12448-2-s.shtylyov@omp.ru> <20220110201014.mtajyrfcfznfhyqm@pengutronix.de> <20220112085009.dbasceh3obfok5dc@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-edac@vger.kernel.org > If an optional IRQ is not present, drivers either just ignore it (e.g. > for devices that can have multiple interrupts or a single muxed IRQ), > or they have to resort to polling. For the latter, fall-back handling > is needed elsewhere in the driver. > To me it sounds much more logical for the driver to check if an > optional irq is non-zero (available) or zero (not available), than to > sprinkle around checks for -ENXIO. In addition, you have to remember > that this one returns -ENXIO, while other APIs use -ENOENT or -ENOSYS > (or some other error code) to indicate absence. I thought not having > to care about the actual error code was the main reason behind the > introduction of the *_optional() APIs. The *_optional() functions return an error code if there has been a real error which should be reported up the call stack. This excludes whatever error code indicates the requested resource does not exist, which can be -ENODEV etc. If the device does not exist, a magic cookie is returned which appears to be a valid resources but in fact is not. So the users of these functions just need to check for an error code, and fail the probe if present. You seems to be suggesting in binary return value: non-zero (available) or zero (not available) This discards the error code when something goes wrong. That is useful information to have, so we should not be discarding it. IRQ don't currently have a magic cookie value. One option would be to add such a magic cookie to the subsystem. Otherwise, since 0 is invalid, return 0 to indicate the IRQ does not exist. The request for a script checking this then makes sense. However, i don't know how well coccinelle/sparse can track values across function calls. They probably can check for: ret = irq_get_optional() if (ret < 0) return ret; A missing if < 0 statement somewhere later is very likely to be an error. A comparison of <= 0 is also likely to be an error. A check for > 0 before calling any other IRQ functions would be good. I'm surprised such a check does not already existing in the IRQ API, but there are probably historical reasons for that. Andrew 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 22A06C433EF for ; Wed, 12 Jan 2022 13:40:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=aio+qVxYxkl98Wk121/ohJHcbZ6m/kABp+kESviaN+w=; b=GvzzqbMntkitbL gVoXQrsZcGjpRl3MaQdME3nd3BuTzYSahRFuV8rkQlq913x4J99mgVjX98a7I5N1Reyh3+NaSyNcC mXZZJ2VmZV4T9a0wsNYe6nZHNpvOMYfc3CD9QCjoGWI8q/OdDHBtjJy8tx9Ti0eDr7Q+QS/uKefMP behQhNZXLvvvd76DxA2ModkWq3vb1HsuX9RYGXuIIRi2UTq+Ec72NhwWnKzpqp35t0VolZxvWZk/M K5GFa/K1zRjNf/sJS11q7f+I0zJIYMnQKEQwYeWL42ZQJxwFFuyMWqZsqgpzyTIKbdwqhkn3W/73v oILXfp0DT1mRny2XKAhQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n7drW-002bpf-SM; Wed, 12 Jan 2022 13:40:26 +0000 Received: from vps0.lunn.ch ([185.16.172.187]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n7drH-002bmu-U9; Wed, 12 Jan 2022 13:40:13 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=S/pA7bUX6e4VB1GHe3hlypsDdeFRt4oGL0831NNCTJM=; b=REJii9uMw/mNDMEDkaB6nMl41t HVia+zgOKIfzuMj09o6jQ+IaM7PowDfcCzSXN02NEw/KiBFCm/WUr1uWHbYcv6vi03Q5vZQPWTCZw kZaHOnCDnZ4glu9pEOOBCeugcwmvyPmDFDeJp1Ck0ttEnrICihUcMQo96DS7dmh4ziSA=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1n7dpl-001CDK-PQ; Wed, 12 Jan 2022 14:38:37 +0100 Date: Wed, 12 Jan 2022 14:38:37 +0100 From: Andrew Lunn To: Geert Uytterhoeven Cc: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , ALSA Development Mailing List , Liam Girdwood , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , linux-phy@lists.infradead.org, Jiri Slaby , "David S. Miller" , Khuong Dinh , Florian Fainelli , Matthias Schiffer , Joakim Zhang , Kamal Dasu , Lee Jones , Bartosz Golaszewski , Daniel Lezcano , Tony Luck , Kishon Vijay Abraham I , bcm-kernel-feedback-list , "open list:SERIAL DRIVERS" , Jakub Kicinski , Zhang Rui , Matthias Brugger , platform-driver-x86@vger.kernel.org, Linux PWM List , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Mauro Carvalho Chehab , John Garry , Peter Korsgaard , William Breathitt Gray , Mark Gross , "open list:GPIO SUBSYSTEM" , Alex Williamson , Mark Brown , Borislav Petkov , Eric Auger , Takashi Iwai , Jaroslav Kysela , openipmi-developer@lists.sourceforge.net, Andy Shevchenko , Benson Leung , Pengutronix Kernel Team , Linux ARM , linux-edac@vger.kernel.org, Sergey Shtylyov , Richard Weinberger , Mun Yew Tham , Hans de Goede , Greg Kroah-Hartman , Yoshihiro Shimoda , Cornelia Huck , Linux MMC List , Linux Kernel Mailing List , linux-spi , Linux-Renesas , Vinod Koul , James Morse , Zha Qipeng , Sebastian Reichel , Niklas =?iso-8859-1?Q?S=F6derlund?= , linux-mediatek@lists.infradead.org, Brian Norris , netdev@vger.kernel.org Subject: Re: [PATCH 1/2] platform: make platform_get_irq_optional() optional Message-ID: References: <20220110195449.12448-1-s.shtylyov@omp.ru> <20220110195449.12448-2-s.shtylyov@omp.ru> <20220110201014.mtajyrfcfznfhyqm@pengutronix.de> <20220112085009.dbasceh3obfok5dc@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220112_054012_007446_149338EB X-CRM114-Status: GOOD ( 25.52 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org > If an optional IRQ is not present, drivers either just ignore it (e.g. > for devices that can have multiple interrupts or a single muxed IRQ), > or they have to resort to polling. For the latter, fall-back handling > is needed elsewhere in the driver. > To me it sounds much more logical for the driver to check if an > optional irq is non-zero (available) or zero (not available), than to > sprinkle around checks for -ENXIO. In addition, you have to remember > that this one returns -ENXIO, while other APIs use -ENOENT or -ENOSYS > (or some other error code) to indicate absence. I thought not having > to care about the actual error code was the main reason behind the > introduction of the *_optional() APIs. The *_optional() functions return an error code if there has been a real error which should be reported up the call stack. This excludes whatever error code indicates the requested resource does not exist, which can be -ENODEV etc. If the device does not exist, a magic cookie is returned which appears to be a valid resources but in fact is not. So the users of these functions just need to check for an error code, and fail the probe if present. You seems to be suggesting in binary return value: non-zero (available) or zero (not available) This discards the error code when something goes wrong. That is useful information to have, so we should not be discarding it. IRQ don't currently have a magic cookie value. One option would be to add such a magic cookie to the subsystem. Otherwise, since 0 is invalid, return 0 to indicate the IRQ does not exist. The request for a script checking this then makes sense. However, i don't know how well coccinelle/sparse can track values across function calls. They probably can check for: ret = irq_get_optional() if (ret < 0) return ret; A missing if < 0 statement somewhere later is very likely to be an error. A comparison of <= 0 is also likely to be an error. A check for > 0 before calling any other IRQ functions would be good. I'm surprised such a check does not already existing in the IRQ API, but there are probably historical reasons for that. Andrew _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D9C5BC433F5 for ; Wed, 12 Jan 2022 13:41:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=jIz+aJBvWRkyrk7IDDdvY873aI2B58jshWbZmZq7swU=; b=u6rYhyvjYcYsT6 ryZ0lKuQYMb9gz6CH1I9a+YmrwqZZLEVl/epcCcNz2av5IsjdkK7ELnFPVGIQ5ri+qUKW2YSseEEj PFBfTC8b5rHW/uuBvUh39WgmBfeaNKuUlpoDU/xIb7PWZ0D+T8kCddDASogfDl5HP3cxjYz+sibdp euiCwg+MJHytIUTNBdq5Yoc07kxFVOSWamx71u8HhY0v2OVwZ6T3T3B+1aUpAE4fBoL/kqSzzr1m1 T2CsUEouqmdaTIp5K7Ve20K63LKa4ehD0q7c0InxdFgyyR8fljCHTAYuHYowHSEkZVpaStZJiMjbz I/08V7czLgaskcOzEXRQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n7drK-002bnX-L1; Wed, 12 Jan 2022 13:40:14 +0000 Received: from vps0.lunn.ch ([185.16.172.187]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n7drH-002bmu-U9; Wed, 12 Jan 2022 13:40:13 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=S/pA7bUX6e4VB1GHe3hlypsDdeFRt4oGL0831NNCTJM=; b=REJii9uMw/mNDMEDkaB6nMl41t HVia+zgOKIfzuMj09o6jQ+IaM7PowDfcCzSXN02NEw/KiBFCm/WUr1uWHbYcv6vi03Q5vZQPWTCZw kZaHOnCDnZ4glu9pEOOBCeugcwmvyPmDFDeJp1Ck0ttEnrICihUcMQo96DS7dmh4ziSA=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1n7dpl-001CDK-PQ; Wed, 12 Jan 2022 14:38:37 +0100 Date: Wed, 12 Jan 2022 14:38:37 +0100 From: Andrew Lunn To: Geert Uytterhoeven Cc: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , ALSA Development Mailing List , Liam Girdwood , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , linux-phy@lists.infradead.org, Jiri Slaby , "David S. Miller" , Khuong Dinh , Florian Fainelli , Matthias Schiffer , Joakim Zhang , Kamal Dasu , Lee Jones , Bartosz Golaszewski , Daniel Lezcano , Tony Luck , Kishon Vijay Abraham I , bcm-kernel-feedback-list , "open list:SERIAL DRIVERS" , Jakub Kicinski , Zhang Rui , Matthias Brugger , platform-driver-x86@vger.kernel.org, Linux PWM List , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Mauro Carvalho Chehab , John Garry , Peter Korsgaard , William Breathitt Gray , Mark Gross , "open list:GPIO SUBSYSTEM" , Alex Williamson , Mark Brown , Borislav Petkov , Eric Auger , Takashi Iwai , Jaroslav Kysela , openipmi-developer@lists.sourceforge.net, Andy Shevchenko , Benson Leung , Pengutronix Kernel Team , Linux ARM , linux-edac@vger.kernel.org, Sergey Shtylyov , Richard Weinberger , Mun Yew Tham , Hans de Goede , Greg Kroah-Hartman , Yoshihiro Shimoda , Cornelia Huck , Linux MMC List , Linux Kernel Mailing List , linux-spi , Linux-Renesas , Vinod Koul , James Morse , Zha Qipeng , Sebastian Reichel , Niklas =?iso-8859-1?Q?S=F6derlund?= , linux-mediatek@lists.infradead.org, Brian Norris , netdev@vger.kernel.org Subject: Re: [PATCH 1/2] platform: make platform_get_irq_optional() optional Message-ID: References: <20220110195449.12448-1-s.shtylyov@omp.ru> <20220110195449.12448-2-s.shtylyov@omp.ru> <20220110201014.mtajyrfcfznfhyqm@pengutronix.de> <20220112085009.dbasceh3obfok5dc@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220112_054012_007446_149338EB X-CRM114-Status: GOOD ( 25.52 ) X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org > If an optional IRQ is not present, drivers either just ignore it (e.g. > for devices that can have multiple interrupts or a single muxed IRQ), > or they have to resort to polling. For the latter, fall-back handling > is needed elsewhere in the driver. > To me it sounds much more logical for the driver to check if an > optional irq is non-zero (available) or zero (not available), than to > sprinkle around checks for -ENXIO. In addition, you have to remember > that this one returns -ENXIO, while other APIs use -ENOENT or -ENOSYS > (or some other error code) to indicate absence. I thought not having > to care about the actual error code was the main reason behind the > introduction of the *_optional() APIs. The *_optional() functions return an error code if there has been a real error which should be reported up the call stack. This excludes whatever error code indicates the requested resource does not exist, which can be -ENODEV etc. If the device does not exist, a magic cookie is returned which appears to be a valid resources but in fact is not. So the users of these functions just need to check for an error code, and fail the probe if present. You seems to be suggesting in binary return value: non-zero (available) or zero (not available) This discards the error code when something goes wrong. That is useful information to have, so we should not be discarding it. IRQ don't currently have a magic cookie value. One option would be to add such a magic cookie to the subsystem. Otherwise, since 0 is invalid, return 0 to indicate the IRQ does not exist. The request for a script checking this then makes sense. However, i don't know how well coccinelle/sparse can track values across function calls. They probably can check for: ret = irq_get_optional() if (ret < 0) return ret; A missing if < 0 statement somewhere later is very likely to be an error. A comparison of <= 0 is also likely to be an error. A check for > 0 before calling any other IRQ functions would be good. I'm surprised such a check does not already existing in the IRQ API, but there are probably historical reasons for that. Andrew ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5D3C5C433FE for ; Wed, 12 Jan 2022 13:40:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Tsp02k6C1C6kZpJYdAPx5LSDyNR70nGrkNGlDCxvXDc=; b=4BNqO86hvMXouj c5c0iNDLDoubSwsZ8WxxWrsKUspvbWHd4cT7AuQvxF4LOW1+9AbJJNAEPluYpCJWKGg5DPq6b3Hv/ o5IqLkyDbhNmaGs7lYt51lQP9rvOzQWz/luPHtOJR0omsgtaqesVjUo95h1Gvb3fSfRulRFAv6u9i 5a2FVs7aJXYC0SQptgBoiN7t92U8MhHeqSQXtkpRdujH7U30168kJQBYutexe7tM7SNjWo5EDVLEx MB93GpjHni8FGZ2a7ZejaSJj1m9ZUMJZdPGpErZGbfg37zj+G57QwfFQ8P/5vV04UXSf4VZCcNAK5 yrvnJ30q7WFe5MK91Q0w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n7drW-002bpa-P6; Wed, 12 Jan 2022 13:40:26 +0000 Received: from vps0.lunn.ch ([185.16.172.187]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n7drH-002bmu-U9; Wed, 12 Jan 2022 13:40:13 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=S/pA7bUX6e4VB1GHe3hlypsDdeFRt4oGL0831NNCTJM=; b=REJii9uMw/mNDMEDkaB6nMl41t HVia+zgOKIfzuMj09o6jQ+IaM7PowDfcCzSXN02NEw/KiBFCm/WUr1uWHbYcv6vi03Q5vZQPWTCZw kZaHOnCDnZ4glu9pEOOBCeugcwmvyPmDFDeJp1Ck0ttEnrICihUcMQo96DS7dmh4ziSA=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1n7dpl-001CDK-PQ; Wed, 12 Jan 2022 14:38:37 +0100 Date: Wed, 12 Jan 2022 14:38:37 +0100 From: Andrew Lunn To: Geert Uytterhoeven Cc: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , ALSA Development Mailing List , Liam Girdwood , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , linux-phy@lists.infradead.org, Jiri Slaby , "David S. Miller" , Khuong Dinh , Florian Fainelli , Matthias Schiffer , Joakim Zhang , Kamal Dasu , Lee Jones , Bartosz Golaszewski , Daniel Lezcano , Tony Luck , Kishon Vijay Abraham I , bcm-kernel-feedback-list , "open list:SERIAL DRIVERS" , Jakub Kicinski , Zhang Rui , Matthias Brugger , platform-driver-x86@vger.kernel.org, Linux PWM List , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Mauro Carvalho Chehab , John Garry , Peter Korsgaard , William Breathitt Gray , Mark Gross , "open list:GPIO SUBSYSTEM" , Alex Williamson , Mark Brown , Borislav Petkov , Eric Auger , Takashi Iwai , Jaroslav Kysela , openipmi-developer@lists.sourceforge.net, Andy Shevchenko , Benson Leung , Pengutronix Kernel Team , Linux ARM , linux-edac@vger.kernel.org, Sergey Shtylyov , Richard Weinberger , Mun Yew Tham , Hans de Goede , Greg Kroah-Hartman , Yoshihiro Shimoda , Cornelia Huck , Linux MMC List , Linux Kernel Mailing List , linux-spi , Linux-Renesas , Vinod Koul , James Morse , Zha Qipeng , Sebastian Reichel , Niklas =?iso-8859-1?Q?S=F6derlund?= , linux-mediatek@lists.infradead.org, Brian Norris , netdev@vger.kernel.org Subject: Re: [PATCH 1/2] platform: make platform_get_irq_optional() optional Message-ID: References: <20220110195449.12448-1-s.shtylyov@omp.ru> <20220110195449.12448-2-s.shtylyov@omp.ru> <20220110201014.mtajyrfcfznfhyqm@pengutronix.de> <20220112085009.dbasceh3obfok5dc@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220112_054012_007446_149338EB X-CRM114-Status: GOOD ( 25.52 ) X-BeenThere: linux-phy@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux Phy Mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-phy" Errors-To: linux-phy-bounces+linux-phy=archiver.kernel.org@lists.infradead.org > If an optional IRQ is not present, drivers either just ignore it (e.g. > for devices that can have multiple interrupts or a single muxed IRQ), > or they have to resort to polling. For the latter, fall-back handling > is needed elsewhere in the driver. > To me it sounds much more logical for the driver to check if an > optional irq is non-zero (available) or zero (not available), than to > sprinkle around checks for -ENXIO. In addition, you have to remember > that this one returns -ENXIO, while other APIs use -ENOENT or -ENOSYS > (or some other error code) to indicate absence. I thought not having > to care about the actual error code was the main reason behind the > introduction of the *_optional() APIs. The *_optional() functions return an error code if there has been a real error which should be reported up the call stack. This excludes whatever error code indicates the requested resource does not exist, which can be -ENODEV etc. If the device does not exist, a magic cookie is returned which appears to be a valid resources but in fact is not. So the users of these functions just need to check for an error code, and fail the probe if present. You seems to be suggesting in binary return value: non-zero (available) or zero (not available) This discards the error code when something goes wrong. That is useful information to have, so we should not be discarding it. IRQ don't currently have a magic cookie value. One option would be to add such a magic cookie to the subsystem. Otherwise, since 0 is invalid, return 0 to indicate the IRQ does not exist. The request for a script checking this then makes sense. However, i don't know how well coccinelle/sparse can track values across function calls. They probably can check for: ret = irq_get_optional() if (ret < 0) return ret; A missing if < 0 statement somewhere later is very likely to be an error. A comparison of <= 0 is also likely to be an error. A check for > 0 before calling any other IRQ functions would be good. I'm surprised such a check does not already existing in the IRQ API, but there are probably historical reasons for that. Andrew -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy