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 22267C433F5 for ; Tue, 25 Jan 2022 12:56:25 +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:MIME-Version:References:In-Reply-To: Date:Cc:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=NGkYOlhR7D9mbSOs3ISo7H7eo+Hjl/Le18STgD8AXg8=; b=NAA/0oLZgHlHUh GEb40jAMV3o4q9gY3HbCjhqkckSobjOpx1yUE3/OvmDP63waKSkRqM1JCrpmUY0a6hozMuVHHvsS2 k5sTQOedcQ8DWbNUizZ4+qDDDHdtqLPZyKQ0DlAOqDE17TqTcOR2L74x+09pPdMULPJAAUyYNf3Hv F98pNdk6ke6y1mHqzAK5Dk3mFbOiXF0R0MCIRZXZSuHbbgszI0I2y1z7qseoDs87uV2F2tDQxMQhG g6Kqw2AxvF+CxGKJUJILJbk/mgwKxuIVrrWRFL4a60Sb3LbnnsN4/h9ul7e8XS5NJ2XWFZ2VKl6sh U57ybvOqsaMzFMk6fCRg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nCLMv-007psm-RE; Tue, 25 Jan 2022 12:56:17 +0000 Received: from mx1.tq-group.com ([93.104.207.81]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nCLMs-007pqE-3G; Tue, 25 Jan 2022 12:56:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1643115374; x=1674651374; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=qmHH9G8TVCwWdo3uTby79rF+j0/3SailZLcBZpByLtg=; b=pS4eRpcnTie77mX5mWeVBdVGEz3IQSKBSx5WXTrJZa8cJGgVNcTYBApz hOS2JodreXZquasstBG9oPNGTJmXX/EkFsKQsmJmPUJ2iZ51fVLGuIZK8 B/g2ukX0I2FSXyF48W1tglAE0Zf9O5G6pcuLvVXBxcm9pnIDjgjHvEqeM P2jDTdmpssmmPHR1w6tQ8jbFCAyVQGb0vkGpJp5iTZ6eLDwGdeqHTB9cJ OrHIdluxFQmCYgnXKxPThAcKWXBlYwlNUW+fxXXxbNv3P64xx9rlZVCDr dpJn8UDuVSi4bayTEAnJvfu2t4WBbnsuk1rsrp+oKFlVzrAHoCGjlH917 w==; X-IronPort-AV: E=Sophos;i="5.88,315,1635199200"; d="scan'208";a="21697222" Received: from unknown (HELO tq-pgp-pr1.tq-net.de) ([192.168.6.15]) by mx1-pgp.tq-group.com with ESMTP; 25 Jan 2022 13:56:09 +0100 Received: from mx1.tq-group.com ([192.168.6.7]) by tq-pgp-pr1.tq-net.de (PGP Universal service); Tue, 25 Jan 2022 13:56:09 +0100 X-PGP-Universal: processed; by tq-pgp-pr1.tq-net.de on Tue, 25 Jan 2022 13:56:09 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1643115369; x=1674651369; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=qmHH9G8TVCwWdo3uTby79rF+j0/3SailZLcBZpByLtg=; b=qzYLcPSMgGM/jDgybQQC0hKCR3aRrdymevnaT4ydaFVxIjMrnrBcV4T6 C5NEX/Uz1J9BcAy/7VV5kAdwi/kxjCsKhTL988mrL1Mf+41YL82U19pTm UIK7b/78zU00Wx1VC2U+5oXhxLzKUsxejHCf55h8+VcSSbzQy6uRt/Qgn jvT61jUGnXXTaU/jyQn1pkY79EM0wk4g7hgaDex//2xlHHq2lIMsC8h0H 2HrYsEYbYjeLAYBA23nQl139MTwAqHxltuCAUqBGA/+5++FvzqsMq6v5y 4URwO2uqbIVjwWBSkS4vRvG/pq3OGi9pHvaUWYVGDg0PfEAcXZz0W9Ise w==; X-IronPort-AV: E=Sophos;i="5.88,315,1635199200"; d="scan'208";a="21697219" Received: from vtuxmail01.tq-net.de ([10.115.0.20]) by mx1.tq-group.com with ESMTP; 25 Jan 2022 13:56:08 +0100 Received: from schifferm-ubuntu (SCHIFFERM-M2.tq-net.de [10.121.201.138]) by vtuxmail01.tq-net.de (Postfix) with ESMTPA id 966B4280065; Tue, 25 Jan 2022 13:56:07 +0100 (CET) Message-ID: <33e55c4c0a637b23d76db5d33872378ad04121bd.camel@ew.tq-group.com> Subject: Re: [PATCH] driver core: platform: Rename platform_get_irq_optional() to platform_get_irq_silent() From: Matthias Schiffer To: Geert Uytterhoeven , Sergey Shtylyov Cc: Andy Shevchenko , Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= , Andrew Lunn , Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , ALSA Development Mailing List , Jaroslav Kysela , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , linux-phy@lists.infradead.org, Jiri Slaby , openipmi-developer@lists.sourceforge.net, Khuong Dinh , Florian Fainelli , Joakim Zhang , Kamal Dasu , Greg Kroah-Hartman , Lee Jones , Bartosz Golaszewski , Daniel Lezcano , Kishon Vijay Abraham I , "open list:SERIAL DRIVERS" , bcm-kernel-feedback-list , Zhang Rui , Linux PWM List , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Linux Kernel Mailing 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 , Eric Auger , Jakub Kicinski , Matthias Brugger , Takashi Iwai , platform-driver-x86@vger.kernel.org, Benson Leung , Linux ARM , linux-edac@vger.kernel.org, Tony Luck , Mun Yew Tham , Hans de Goede , netdev@vger.kernel.org, Yoshihiro Shimoda , Cornelia Huck , Linux MMC List , Liam Girdwood , linux-spi , Linux-Renesas , 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" Date: Tue, 25 Jan 2022 13:56:05 +0100 In-Reply-To: References: <20220112213121.5ruae5mxwj6t3qiy@pengutronix.de> <20220113110831.wvwbm75hbfysbn2d@pengutronix.de> <20220113194358.xnnbhsoyetihterb@pengutronix.de> <20220115154539.j3tsz5ioqexq2yuu@pengutronix.de> <20220120075718.5qtrpc543kkykaow@pengutronix.de> <15796e57-f7d4-9c66-3b53-0b026eaf31d8@omp.ru> User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220125_045614_571208_92F9503D X-CRM114-Status: GOOD ( 66.13 ) 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 On Tue, 2022-01-25 at 09:25 +0100, Geert Uytterhoeven wrote: > Hi Sergey, > > On Mon, Jan 24, 2022 at 10:02 PM Sergey Shtylyov > wrote: > > On 1/24/22 6:01 PM, Andy Shevchenko wrote: > > > > > > > > > It'd certainly be good to name anything that doesn't > > > > > > > > > correspond to one > > > > > > > > > of the existing semantics for the API (!) something > > > > > > > > > different rather > > > > > > > > > than adding yet another potentially overloaded > > > > > > > > > meaning. > > > > > > > > > > > > > > > > It seems we're (at least) three who agree about this. > > > > > > > > Here is a patch > > > > > > > > fixing the name. > > > > > > > > > > > > > > And similar number of people are on the other side. > > > > > > > > > > > > If someone already opposed to the renaming (and not only > > > > > > the name) I > > > > > > must have missed that. > > > > > > > > > > > > So you think it's a good idea to keep the name > > > > > > platform_get_irq_optional() despite the "not found" value > > > > > > returned by it > > > > > > isn't usable as if it were a normal irq number? > > > > > > > > > > I meant that on the other side people who are in favour of > > > > > Sergey's patch. > > > > > Since that I commented already that I opposed the renaming > > > > > being a standalone > > > > > change. > > > > > > > > > > Do you agree that we have several issues with > > > > > platform_get_irq*() APIs? > > [...] > > > > > 2. The vIRQ0 handling: a) WARN() followed by b) returned > > > > > value 0 > > > > > > > > I'm happy with the vIRQ0 handling. Today platform_get_irq() and > > > > it's > > > > silent variant returns either a valid and usuable irq number or > > > > a > > > > negative error value. That's totally fine. > > > > > > It might return 0. > > > Actually it seems that the WARN() can only be issued in two > > > cases: > > > - SPARC with vIRQ0 in one of the array member > > > - fallback to ACPI for GPIO IRQ resource with index 0 > > > > You have probably missed the recent discovery that > > arch/sh/boards/board-aps4*.c > > causes IRQ0 to be passed as a direct IRQ resource? > > So far no one reported seeing the big fat warning ;-) FWIW, we had a similar issue with an IRQ resource passed from the tqmx86 MFD driver do the GPIO driver, which we noticed due to this warning, and which was fixed in a946506c48f3bd09363c9d2b0a178e55733bcbb6 and 9b87f43537acfa24b95c236beba0f45901356eb2. I believe these changes are what promted this whole discussion and led to my "Reported-by" on the patch? It is not entirely clear to me when IRQ 0 is valid and when it isn't, but the warning seems useful to me. Maybe it would make more sense to warn when such an IRQ resource is registered for a platform device, and not when it is looked up? My opinion is that it would be very confusing if there are any places in the kernel (on some platforms) where IRQ 0 is valid, but for platform_get_irq() it would suddenly mean "not found". Keeping a negative return value seems preferable to me for this reason. (An alternative, more involved idea would be to add 1 to all IRQ "cookies", so IRQ 0 would return 1, leaving 0 as a special value. I have absolutely no idea how big the API surface is that would need changes, and it is likely not worth the effort at all.) > > > > The bottom line here is the SPARC case. Anybody familiar with the > > > platform > > > can shed a light on this. If there is no such case, we may remove > > > warning > > > along with ret = 0 case from platfrom_get_irq(). > > > > I'm afraid you're too fast here... :-) > > We'll have a really hard time if we continue to allow IRQ0 to be > > returned by > > platform_get_irq() -- we'll have oto fileter it out in the callers > > then... > > So far no one reported seeing the big fat warning? > > > > > > 3. The specific cookie for "IRQ not found, while no error > > > > > happened" case > > > > > > > > Not sure what you mean here. I have no problem that a situation > > > > I can > > > > cope with is called an error for the query function. I just do > > > > error > > > > handling and continue happily. So the part "while no error > > > > happened" is > > > > irrelevant to me. > > > > > > I meant that instead of using special error code, 0 is very much > > > good for > > > the cases when IRQ is not found. It allows to distinguish -ENXIO > > > from the > > > low layer from -ENXIO with this magic meaning. > > > > I don't see how -ENXIO can trickle from the lower layers, > > frankly... > > It might one day, leading to very hard to track bugs. As gregkh noted, changing the return value without also making the compile fail will be a huge PITA whenever driver patches are back- or forward-ported, as it would require subtle changes in error paths, which can easily slip through unnoticed, in particular with half- automated stable backports. Even if another return value like -ENODEV might be better aligned with ...regulator_get_optional() and similar functions, or we even find a way to make 0 usable for this, none of the proposed changes strike me as big enough a win to outweigh the churn caused by making such a change at all. Kind regards, Matthias > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > geert@linux-m68k.org > > In personal conversations with technical people, I call myself a > hacker. But > when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds _______________________________________________ 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 EC2ABC433EF for ; Tue, 25 Jan 2022 12:56:34 +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:MIME-Version:References:In-Reply-To: Date:Cc:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=KxVB6Z/vj3IjKb0SmjFokmBAse8qaixE9NX7Ng03bgg=; b=zKMaX1ZbNNaSjX x4SFaMQpdSB4Rq3qNDb9Qmmzi7IloTX/lcovc2zyDwKb1rp+Lcun7a5vKvQ0jipJGkfibGqEQR/wd 59i0b+MPP+a0AQfkVrD9PvDw939uaEscq2or5AUd10JDAjbK7J5m5HWf6UXq/PYnYD9lJf62KpH0R MVGsHSBca5i7gz1crBQzc64CVVTvDGAnzKwML75NHl1kEn33v/+X+ptw3YcAVmgIfCeWBTHFeBylH 7Aa1Y57pjZDwLk9JomDu9LS84HuvRPhWOO0ZJ54/guJ7DaIPXBVLrO3s9kDeTkwvGcx96vuiWYXb/ BeTZt90iw/w7SWUy9bAg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nCLNC-007pyq-DX; Tue, 25 Jan 2022 12:56:34 +0000 Received: from mx1.tq-group.com ([93.104.207.81]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nCLMs-007pqE-3G; Tue, 25 Jan 2022 12:56:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1643115374; x=1674651374; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=qmHH9G8TVCwWdo3uTby79rF+j0/3SailZLcBZpByLtg=; b=pS4eRpcnTie77mX5mWeVBdVGEz3IQSKBSx5WXTrJZa8cJGgVNcTYBApz hOS2JodreXZquasstBG9oPNGTJmXX/EkFsKQsmJmPUJ2iZ51fVLGuIZK8 B/g2ukX0I2FSXyF48W1tglAE0Zf9O5G6pcuLvVXBxcm9pnIDjgjHvEqeM P2jDTdmpssmmPHR1w6tQ8jbFCAyVQGb0vkGpJp5iTZ6eLDwGdeqHTB9cJ OrHIdluxFQmCYgnXKxPThAcKWXBlYwlNUW+fxXXxbNv3P64xx9rlZVCDr dpJn8UDuVSi4bayTEAnJvfu2t4WBbnsuk1rsrp+oKFlVzrAHoCGjlH917 w==; X-IronPort-AV: E=Sophos;i="5.88,315,1635199200"; d="scan'208";a="21697222" Received: from unknown (HELO tq-pgp-pr1.tq-net.de) ([192.168.6.15]) by mx1-pgp.tq-group.com with ESMTP; 25 Jan 2022 13:56:09 +0100 Received: from mx1.tq-group.com ([192.168.6.7]) by tq-pgp-pr1.tq-net.de (PGP Universal service); Tue, 25 Jan 2022 13:56:09 +0100 X-PGP-Universal: processed; by tq-pgp-pr1.tq-net.de on Tue, 25 Jan 2022 13:56:09 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1643115369; x=1674651369; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=qmHH9G8TVCwWdo3uTby79rF+j0/3SailZLcBZpByLtg=; b=qzYLcPSMgGM/jDgybQQC0hKCR3aRrdymevnaT4ydaFVxIjMrnrBcV4T6 C5NEX/Uz1J9BcAy/7VV5kAdwi/kxjCsKhTL988mrL1Mf+41YL82U19pTm UIK7b/78zU00Wx1VC2U+5oXhxLzKUsxejHCf55h8+VcSSbzQy6uRt/Qgn jvT61jUGnXXTaU/jyQn1pkY79EM0wk4g7hgaDex//2xlHHq2lIMsC8h0H 2HrYsEYbYjeLAYBA23nQl139MTwAqHxltuCAUqBGA/+5++FvzqsMq6v5y 4URwO2uqbIVjwWBSkS4vRvG/pq3OGi9pHvaUWYVGDg0PfEAcXZz0W9Ise w==; X-IronPort-AV: E=Sophos;i="5.88,315,1635199200"; d="scan'208";a="21697219" Received: from vtuxmail01.tq-net.de ([10.115.0.20]) by mx1.tq-group.com with ESMTP; 25 Jan 2022 13:56:08 +0100 Received: from schifferm-ubuntu (SCHIFFERM-M2.tq-net.de [10.121.201.138]) by vtuxmail01.tq-net.de (Postfix) with ESMTPA id 966B4280065; Tue, 25 Jan 2022 13:56:07 +0100 (CET) Message-ID: <33e55c4c0a637b23d76db5d33872378ad04121bd.camel@ew.tq-group.com> Subject: Re: [PATCH] driver core: platform: Rename platform_get_irq_optional() to platform_get_irq_silent() From: Matthias Schiffer To: Geert Uytterhoeven , Sergey Shtylyov Cc: Andy Shevchenko , Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= , Andrew Lunn , Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , ALSA Development Mailing List , Jaroslav Kysela , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , linux-phy@lists.infradead.org, Jiri Slaby , openipmi-developer@lists.sourceforge.net, Khuong Dinh , Florian Fainelli , Joakim Zhang , Kamal Dasu , Greg Kroah-Hartman , Lee Jones , Bartosz Golaszewski , Daniel Lezcano , Kishon Vijay Abraham I , "open list:SERIAL DRIVERS" , bcm-kernel-feedback-list , Zhang Rui , Linux PWM List , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Linux Kernel Mailing 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 , Eric Auger , Jakub Kicinski , Matthias Brugger , Takashi Iwai , platform-driver-x86@vger.kernel.org, Benson Leung , Linux ARM , linux-edac@vger.kernel.org, Tony Luck , Mun Yew Tham , Hans de Goede , netdev@vger.kernel.org, Yoshihiro Shimoda , Cornelia Huck , Linux MMC List , Liam Girdwood , linux-spi , Linux-Renesas , 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" Date: Tue, 25 Jan 2022 13:56:05 +0100 In-Reply-To: References: <20220112213121.5ruae5mxwj6t3qiy@pengutronix.de> <20220113110831.wvwbm75hbfysbn2d@pengutronix.de> <20220113194358.xnnbhsoyetihterb@pengutronix.de> <20220115154539.j3tsz5ioqexq2yuu@pengutronix.de> <20220120075718.5qtrpc543kkykaow@pengutronix.de> <15796e57-f7d4-9c66-3b53-0b026eaf31d8@omp.ru> User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220125_045614_571208_92F9503D X-CRM114-Status: GOOD ( 66.13 ) 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 On Tue, 2022-01-25 at 09:25 +0100, Geert Uytterhoeven wrote: > Hi Sergey, > > On Mon, Jan 24, 2022 at 10:02 PM Sergey Shtylyov > wrote: > > On 1/24/22 6:01 PM, Andy Shevchenko wrote: > > > > > > > > > It'd certainly be good to name anything that doesn't > > > > > > > > > correspond to one > > > > > > > > > of the existing semantics for the API (!) something > > > > > > > > > different rather > > > > > > > > > than adding yet another potentially overloaded > > > > > > > > > meaning. > > > > > > > > > > > > > > > > It seems we're (at least) three who agree about this. > > > > > > > > Here is a patch > > > > > > > > fixing the name. > > > > > > > > > > > > > > And similar number of people are on the other side. > > > > > > > > > > > > If someone already opposed to the renaming (and not only > > > > > > the name) I > > > > > > must have missed that. > > > > > > > > > > > > So you think it's a good idea to keep the name > > > > > > platform_get_irq_optional() despite the "not found" value > > > > > > returned by it > > > > > > isn't usable as if it were a normal irq number? > > > > > > > > > > I meant that on the other side people who are in favour of > > > > > Sergey's patch. > > > > > Since that I commented already that I opposed the renaming > > > > > being a standalone > > > > > change. > > > > > > > > > > Do you agree that we have several issues with > > > > > platform_get_irq*() APIs? > > [...] > > > > > 2. The vIRQ0 handling: a) WARN() followed by b) returned > > > > > value 0 > > > > > > > > I'm happy with the vIRQ0 handling. Today platform_get_irq() and > > > > it's > > > > silent variant returns either a valid and usuable irq number or > > > > a > > > > negative error value. That's totally fine. > > > > > > It might return 0. > > > Actually it seems that the WARN() can only be issued in two > > > cases: > > > - SPARC with vIRQ0 in one of the array member > > > - fallback to ACPI for GPIO IRQ resource with index 0 > > > > You have probably missed the recent discovery that > > arch/sh/boards/board-aps4*.c > > causes IRQ0 to be passed as a direct IRQ resource? > > So far no one reported seeing the big fat warning ;-) FWIW, we had a similar issue with an IRQ resource passed from the tqmx86 MFD driver do the GPIO driver, which we noticed due to this warning, and which was fixed in a946506c48f3bd09363c9d2b0a178e55733bcbb6 and 9b87f43537acfa24b95c236beba0f45901356eb2. I believe these changes are what promted this whole discussion and led to my "Reported-by" on the patch? It is not entirely clear to me when IRQ 0 is valid and when it isn't, but the warning seems useful to me. Maybe it would make more sense to warn when such an IRQ resource is registered for a platform device, and not when it is looked up? My opinion is that it would be very confusing if there are any places in the kernel (on some platforms) where IRQ 0 is valid, but for platform_get_irq() it would suddenly mean "not found". Keeping a negative return value seems preferable to me for this reason. (An alternative, more involved idea would be to add 1 to all IRQ "cookies", so IRQ 0 would return 1, leaving 0 as a special value. I have absolutely no idea how big the API surface is that would need changes, and it is likely not worth the effort at all.) > > > > The bottom line here is the SPARC case. Anybody familiar with the > > > platform > > > can shed a light on this. If there is no such case, we may remove > > > warning > > > along with ret = 0 case from platfrom_get_irq(). > > > > I'm afraid you're too fast here... :-) > > We'll have a really hard time if we continue to allow IRQ0 to be > > returned by > > platform_get_irq() -- we'll have oto fileter it out in the callers > > then... > > So far no one reported seeing the big fat warning? > > > > > > 3. The specific cookie for "IRQ not found, while no error > > > > > happened" case > > > > > > > > Not sure what you mean here. I have no problem that a situation > > > > I can > > > > cope with is called an error for the query function. I just do > > > > error > > > > handling and continue happily. So the part "while no error > > > > happened" is > > > > irrelevant to me. > > > > > > I meant that instead of using special error code, 0 is very much > > > good for > > > the cases when IRQ is not found. It allows to distinguish -ENXIO > > > from the > > > low layer from -ENXIO with this magic meaning. > > > > I don't see how -ENXIO can trickle from the lower layers, > > frankly... > > It might one day, leading to very hard to track bugs. As gregkh noted, changing the return value without also making the compile fail will be a huge PITA whenever driver patches are back- or forward-ported, as it would require subtle changes in error paths, which can easily slip through unnoticed, in particular with half- automated stable backports. Even if another return value like -ENODEV might be better aligned with ...regulator_get_optional() and similar functions, or we even find a way to make 0 usable for this, none of the proposed changes strike me as big enough a win to outweigh the churn caused by making such a change at all. Kind regards, Matthias > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > geert@linux-m68k.org > > In personal conversations with technical people, I call myself a > hacker. But > when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy 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 2B34BC433EF for ; Tue, 25 Jan 2022 12:56:49 +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:MIME-Version:References:In-Reply-To: Date:Cc:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=U3Dk50QIc2v4pX38tvJ3iLbBxAMFoxNi5MpbruW2CWA=; b=fiM0etfyDNixyi Eoz8Q7bdIP2P3aFQOF24CEo67eE6lgawBtN+BLARoDMKronAnm9uaOhoQFgrbbjLr9untjDU2ag3z DpBXmzhJTIIKQQ47uqSzli9eKIddxXOEIUBHs47qOQwc5FT2QN69JB6Snf/Qz9GTKfp6pQUrGh0qi CFpvoDhHjHG2oxFBMkeL7BjwQspSsbz1MFEZX7DXci4qWSKh9h0JVtXFL1Nv9M4c7SCCXAKP/d4tA HAbUCHFrc+Puu8nk9kZvsCKImT2mlEdSptR6OIPE8Rz09Vfndy+SjPwIeTT8T/CQ4rIMM8ZM/o+ic HfWc7aZlFSB1gWSaW11w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nCLMx-007ptS-Nr; Tue, 25 Jan 2022 12:56:19 +0000 Received: from mx1.tq-group.com ([93.104.207.81]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nCLMs-007pqE-3G; Tue, 25 Jan 2022 12:56:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1643115374; x=1674651374; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=qmHH9G8TVCwWdo3uTby79rF+j0/3SailZLcBZpByLtg=; b=pS4eRpcnTie77mX5mWeVBdVGEz3IQSKBSx5WXTrJZa8cJGgVNcTYBApz hOS2JodreXZquasstBG9oPNGTJmXX/EkFsKQsmJmPUJ2iZ51fVLGuIZK8 B/g2ukX0I2FSXyF48W1tglAE0Zf9O5G6pcuLvVXBxcm9pnIDjgjHvEqeM P2jDTdmpssmmPHR1w6tQ8jbFCAyVQGb0vkGpJp5iTZ6eLDwGdeqHTB9cJ OrHIdluxFQmCYgnXKxPThAcKWXBlYwlNUW+fxXXxbNv3P64xx9rlZVCDr dpJn8UDuVSi4bayTEAnJvfu2t4WBbnsuk1rsrp+oKFlVzrAHoCGjlH917 w==; X-IronPort-AV: E=Sophos;i="5.88,315,1635199200"; d="scan'208";a="21697222" Received: from unknown (HELO tq-pgp-pr1.tq-net.de) ([192.168.6.15]) by mx1-pgp.tq-group.com with ESMTP; 25 Jan 2022 13:56:09 +0100 Received: from mx1.tq-group.com ([192.168.6.7]) by tq-pgp-pr1.tq-net.de (PGP Universal service); Tue, 25 Jan 2022 13:56:09 +0100 X-PGP-Universal: processed; by tq-pgp-pr1.tq-net.de on Tue, 25 Jan 2022 13:56:09 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1643115369; x=1674651369; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=qmHH9G8TVCwWdo3uTby79rF+j0/3SailZLcBZpByLtg=; b=qzYLcPSMgGM/jDgybQQC0hKCR3aRrdymevnaT4ydaFVxIjMrnrBcV4T6 C5NEX/Uz1J9BcAy/7VV5kAdwi/kxjCsKhTL988mrL1Mf+41YL82U19pTm UIK7b/78zU00Wx1VC2U+5oXhxLzKUsxejHCf55h8+VcSSbzQy6uRt/Qgn jvT61jUGnXXTaU/jyQn1pkY79EM0wk4g7hgaDex//2xlHHq2lIMsC8h0H 2HrYsEYbYjeLAYBA23nQl139MTwAqHxltuCAUqBGA/+5++FvzqsMq6v5y 4URwO2uqbIVjwWBSkS4vRvG/pq3OGi9pHvaUWYVGDg0PfEAcXZz0W9Ise w==; X-IronPort-AV: E=Sophos;i="5.88,315,1635199200"; d="scan'208";a="21697219" Received: from vtuxmail01.tq-net.de ([10.115.0.20]) by mx1.tq-group.com with ESMTP; 25 Jan 2022 13:56:08 +0100 Received: from schifferm-ubuntu (SCHIFFERM-M2.tq-net.de [10.121.201.138]) by vtuxmail01.tq-net.de (Postfix) with ESMTPA id 966B4280065; Tue, 25 Jan 2022 13:56:07 +0100 (CET) Message-ID: <33e55c4c0a637b23d76db5d33872378ad04121bd.camel@ew.tq-group.com> Subject: Re: [PATCH] driver core: platform: Rename platform_get_irq_optional() to platform_get_irq_silent() From: Matthias Schiffer To: Geert Uytterhoeven , Sergey Shtylyov Cc: Andy Shevchenko , Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= , Andrew Lunn , Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , ALSA Development Mailing List , Jaroslav Kysela , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , linux-phy@lists.infradead.org, Jiri Slaby , openipmi-developer@lists.sourceforge.net, Khuong Dinh , Florian Fainelli , Joakim Zhang , Kamal Dasu , Greg Kroah-Hartman , Lee Jones , Bartosz Golaszewski , Daniel Lezcano , Kishon Vijay Abraham I , "open list:SERIAL DRIVERS" , bcm-kernel-feedback-list , Zhang Rui , Linux PWM List , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Linux Kernel Mailing 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 , Eric Auger , Jakub Kicinski , Matthias Brugger , Takashi Iwai , platform-driver-x86@vger.kernel.org, Benson Leung , Linux ARM , linux-edac@vger.kernel.org, Tony Luck , Mun Yew Tham , Hans de Goede , netdev@vger.kernel.org, Yoshihiro Shimoda , Cornelia Huck , Linux MMC List , Liam Girdwood , linux-spi , Linux-Renesas , 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" Date: Tue, 25 Jan 2022 13:56:05 +0100 In-Reply-To: References: <20220112213121.5ruae5mxwj6t3qiy@pengutronix.de> <20220113110831.wvwbm75hbfysbn2d@pengutronix.de> <20220113194358.xnnbhsoyetihterb@pengutronix.de> <20220115154539.j3tsz5ioqexq2yuu@pengutronix.de> <20220120075718.5qtrpc543kkykaow@pengutronix.de> <15796e57-f7d4-9c66-3b53-0b026eaf31d8@omp.ru> User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220125_045614_571208_92F9503D X-CRM114-Status: GOOD ( 66.13 ) 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 On Tue, 2022-01-25 at 09:25 +0100, Geert Uytterhoeven wrote: > Hi Sergey, > > On Mon, Jan 24, 2022 at 10:02 PM Sergey Shtylyov > wrote: > > On 1/24/22 6:01 PM, Andy Shevchenko wrote: > > > > > > > > > It'd certainly be good to name anything that doesn't > > > > > > > > > correspond to one > > > > > > > > > of the existing semantics for the API (!) something > > > > > > > > > different rather > > > > > > > > > than adding yet another potentially overloaded > > > > > > > > > meaning. > > > > > > > > > > > > > > > > It seems we're (at least) three who agree about this. > > > > > > > > Here is a patch > > > > > > > > fixing the name. > > > > > > > > > > > > > > And similar number of people are on the other side. > > > > > > > > > > > > If someone already opposed to the renaming (and not only > > > > > > the name) I > > > > > > must have missed that. > > > > > > > > > > > > So you think it's a good idea to keep the name > > > > > > platform_get_irq_optional() despite the "not found" value > > > > > > returned by it > > > > > > isn't usable as if it were a normal irq number? > > > > > > > > > > I meant that on the other side people who are in favour of > > > > > Sergey's patch. > > > > > Since that I commented already that I opposed the renaming > > > > > being a standalone > > > > > change. > > > > > > > > > > Do you agree that we have several issues with > > > > > platform_get_irq*() APIs? > > [...] > > > > > 2. The vIRQ0 handling: a) WARN() followed by b) returned > > > > > value 0 > > > > > > > > I'm happy with the vIRQ0 handling. Today platform_get_irq() and > > > > it's > > > > silent variant returns either a valid and usuable irq number or > > > > a > > > > negative error value. That's totally fine. > > > > > > It might return 0. > > > Actually it seems that the WARN() can only be issued in two > > > cases: > > > - SPARC with vIRQ0 in one of the array member > > > - fallback to ACPI for GPIO IRQ resource with index 0 > > > > You have probably missed the recent discovery that > > arch/sh/boards/board-aps4*.c > > causes IRQ0 to be passed as a direct IRQ resource? > > So far no one reported seeing the big fat warning ;-) FWIW, we had a similar issue with an IRQ resource passed from the tqmx86 MFD driver do the GPIO driver, which we noticed due to this warning, and which was fixed in a946506c48f3bd09363c9d2b0a178e55733bcbb6 and 9b87f43537acfa24b95c236beba0f45901356eb2. I believe these changes are what promted this whole discussion and led to my "Reported-by" on the patch? It is not entirely clear to me when IRQ 0 is valid and when it isn't, but the warning seems useful to me. Maybe it would make more sense to warn when such an IRQ resource is registered for a platform device, and not when it is looked up? My opinion is that it would be very confusing if there are any places in the kernel (on some platforms) where IRQ 0 is valid, but for platform_get_irq() it would suddenly mean "not found". Keeping a negative return value seems preferable to me for this reason. (An alternative, more involved idea would be to add 1 to all IRQ "cookies", so IRQ 0 would return 1, leaving 0 as a special value. I have absolutely no idea how big the API surface is that would need changes, and it is likely not worth the effort at all.) > > > > The bottom line here is the SPARC case. Anybody familiar with the > > > platform > > > can shed a light on this. If there is no such case, we may remove > > > warning > > > along with ret = 0 case from platfrom_get_irq(). > > > > I'm afraid you're too fast here... :-) > > We'll have a really hard time if we continue to allow IRQ0 to be > > returned by > > platform_get_irq() -- we'll have oto fileter it out in the callers > > then... > > So far no one reported seeing the big fat warning? > > > > > > 3. The specific cookie for "IRQ not found, while no error > > > > > happened" case > > > > > > > > Not sure what you mean here. I have no problem that a situation > > > > I can > > > > cope with is called an error for the query function. I just do > > > > error > > > > handling and continue happily. So the part "while no error > > > > happened" is > > > > irrelevant to me. > > > > > > I meant that instead of using special error code, 0 is very much > > > good for > > > the cases when IRQ is not found. It allows to distinguish -ENXIO > > > from the > > > low layer from -ENXIO with this magic meaning. > > > > I don't see how -ENXIO can trickle from the lower layers, > > frankly... > > It might one day, leading to very hard to track bugs. As gregkh noted, changing the return value without also making the compile fail will be a huge PITA whenever driver patches are back- or forward-ported, as it would require subtle changes in error paths, which can easily slip through unnoticed, in particular with half- automated stable backports. Even if another return value like -ENODEV might be better aligned with ...regulator_get_optional() and similar functions, or we even find a way to make 0 usable for this, none of the proposed changes strike me as big enough a win to outweigh the churn caused by making such a change at all. Kind regards, Matthias > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > geert@linux-m68k.org > > In personal conversations with technical people, I call myself a > hacker. But > when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds ______________________________________________________ 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C37C0C4167E for ; Tue, 25 Jan 2022 13:00:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347805AbiAYM7b (ORCPT ); Tue, 25 Jan 2022 07:59:31 -0500 Received: from mx1.tq-group.com ([93.104.207.81]:23582 "EHLO mx1.tq-group.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1359066AbiAYM4T (ORCPT ); Tue, 25 Jan 2022 07:56:19 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1643115378; x=1674651378; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=qmHH9G8TVCwWdo3uTby79rF+j0/3SailZLcBZpByLtg=; b=MB4CtmBY/kKUZUkbx858aLxhxtieEwNP3ArjS4boMb6N8oV/9x7+oe+7 vBRLZ4YVEvZ/1JO2JM7klGULc8cKXlCkrMAODKUDhoRxB28SicAlb3MwA z4BaKpNCiXG7OZsbwPxYNCX20Nvck+6x4Hz/LZ+ldX2fSU+OXD7ABJ1L3 qhCGVEW1kcKUCt7niPCL5EASKNk6v9jLS3splPrSApHD9OyMA3IK+xWnD qyyDv4o+OVOldOqDDmEWH7qIkVMd4VmaPCCCi8zZkUsMRkMvEb3Tf3C3R UUuaUcupgO11DYD/qAb/ED5oLYw/xNWULPE2SGkeHr9g/mXySY1kfHonr w==; X-IronPort-AV: E=Sophos;i="5.88,315,1635199200"; d="scan'208";a="21697221" Received: from unknown (HELO tq-pgp-pr1.tq-net.de) ([192.168.6.15]) by mx1-pgp.tq-group.com with ESMTP; 25 Jan 2022 13:56:08 +0100 Received: from mx1.tq-group.com ([192.168.6.7]) by tq-pgp-pr1.tq-net.de (PGP Universal service); Tue, 25 Jan 2022 13:56:09 +0100 X-PGP-Universal: processed; by tq-pgp-pr1.tq-net.de on Tue, 25 Jan 2022 13:56:09 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1643115368; x=1674651368; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=qmHH9G8TVCwWdo3uTby79rF+j0/3SailZLcBZpByLtg=; b=DZazWES6Z7qX74NIrM+i69skWo5ObJhdpFykKEns/urrKSV7FmjkeM2t YnBYyu8w987PyL5eZzlJ5LqVC1ihXP16gqLHPxLAtK9NLfyfbbnxMJjnE xkheSFbGqZGupY07RAOmoaOCOMZDwMnZDGO5NYbNANthuHRau78001vK2 zJddEGsKEXtkeB0hUhuWkr3kF7tYn69GziEH85Y5mupr+z4GI7q8lpN8I tHnvDwxI2K5PUlNNRZSdV0LyBsYg2DAs8tg6Mfpsr/QsP6qCBNv225Vhv 9E6GQFdFpwV+AAWYlFUvOpxFqY72lFs4EOW/dDH0eW9QyDbiOHxD6zCA3 Q==; X-IronPort-AV: E=Sophos;i="5.88,315,1635199200"; d="scan'208";a="21697220" Received: from vtuxmail01.tq-net.de ([10.115.0.20]) by mx1.tq-group.com with ESMTP; 25 Jan 2022 13:56:08 +0100 Received: from schifferm-ubuntu (SCHIFFERM-M2.tq-net.de [10.121.201.138]) by vtuxmail01.tq-net.de (Postfix) with ESMTPA id 966B4280065; Tue, 25 Jan 2022 13:56:07 +0100 (CET) Message-ID: <33e55c4c0a637b23d76db5d33872378ad04121bd.camel@ew.tq-group.com> Subject: Re: [PATCH] driver core: platform: Rename platform_get_irq_optional() to platform_get_irq_silent() From: Matthias Schiffer To: Geert Uytterhoeven , Sergey Shtylyov Cc: Andy Shevchenko , Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= , Andrew Lunn , Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , ALSA Development Mailing List , Jaroslav Kysela , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , linux-phy@lists.infradead.org, Jiri Slaby , openipmi-developer@lists.sourceforge.net, Khuong Dinh , Florian Fainelli , Joakim Zhang , Kamal Dasu , Greg Kroah-Hartman , Lee Jones , Bartosz Golaszewski , Daniel Lezcano , Kishon Vijay Abraham I , "open list:SERIAL DRIVERS" , bcm-kernel-feedback-list , Zhang Rui , Linux PWM List , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Linux Kernel Mailing 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 , Eric Auger , Jakub Kicinski , Matthias Brugger , Takashi Iwai , platform-driver-x86@vger.kernel.org, Benson Leung , Linux ARM , linux-edac@vger.kernel.org, Tony Luck , Mun Yew Tham , Hans de Goede , netdev@vger.kernel.org, Yoshihiro Shimoda , Cornelia Huck , Linux MMC List , Liam Girdwood , linux-spi , Linux-Renesas , 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" Date: Tue, 25 Jan 2022 13:56:05 +0100 In-Reply-To: References: <20220112213121.5ruae5mxwj6t3qiy@pengutronix.de> <20220113110831.wvwbm75hbfysbn2d@pengutronix.de> <20220113194358.xnnbhsoyetihterb@pengutronix.de> <20220115154539.j3tsz5ioqexq2yuu@pengutronix.de> <20220120075718.5qtrpc543kkykaow@pengutronix.de> <15796e57-f7d4-9c66-3b53-0b026eaf31d8@omp.ru> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org On Tue, 2022-01-25 at 09:25 +0100, Geert Uytterhoeven wrote: > Hi Sergey, > > On Mon, Jan 24, 2022 at 10:02 PM Sergey Shtylyov > wrote: > > On 1/24/22 6:01 PM, Andy Shevchenko wrote: > > > > > > > > > It'd certainly be good to name anything that doesn't > > > > > > > > > correspond to one > > > > > > > > > of the existing semantics for the API (!) something > > > > > > > > > different rather > > > > > > > > > than adding yet another potentially overloaded > > > > > > > > > meaning. > > > > > > > > > > > > > > > > It seems we're (at least) three who agree about this. > > > > > > > > Here is a patch > > > > > > > > fixing the name. > > > > > > > > > > > > > > And similar number of people are on the other side. > > > > > > > > > > > > If someone already opposed to the renaming (and not only > > > > > > the name) I > > > > > > must have missed that. > > > > > > > > > > > > So you think it's a good idea to keep the name > > > > > > platform_get_irq_optional() despite the "not found" value > > > > > > returned by it > > > > > > isn't usable as if it were a normal irq number? > > > > > > > > > > I meant that on the other side people who are in favour of > > > > > Sergey's patch. > > > > > Since that I commented already that I opposed the renaming > > > > > being a standalone > > > > > change. > > > > > > > > > > Do you agree that we have several issues with > > > > > platform_get_irq*() APIs? > > [...] > > > > > 2. The vIRQ0 handling: a) WARN() followed by b) returned > > > > > value 0 > > > > > > > > I'm happy with the vIRQ0 handling. Today platform_get_irq() and > > > > it's > > > > silent variant returns either a valid and usuable irq number or > > > > a > > > > negative error value. That's totally fine. > > > > > > It might return 0. > > > Actually it seems that the WARN() can only be issued in two > > > cases: > > > - SPARC with vIRQ0 in one of the array member > > > - fallback to ACPI for GPIO IRQ resource with index 0 > > > > You have probably missed the recent discovery that > > arch/sh/boards/board-aps4*.c > > causes IRQ0 to be passed as a direct IRQ resource? > > So far no one reported seeing the big fat warning ;-) FWIW, we had a similar issue with an IRQ resource passed from the tqmx86 MFD driver do the GPIO driver, which we noticed due to this warning, and which was fixed in a946506c48f3bd09363c9d2b0a178e55733bcbb6 and 9b87f43537acfa24b95c236beba0f45901356eb2. I believe these changes are what promted this whole discussion and led to my "Reported-by" on the patch? It is not entirely clear to me when IRQ 0 is valid and when it isn't, but the warning seems useful to me. Maybe it would make more sense to warn when such an IRQ resource is registered for a platform device, and not when it is looked up? My opinion is that it would be very confusing if there are any places in the kernel (on some platforms) where IRQ 0 is valid, but for platform_get_irq() it would suddenly mean "not found". Keeping a negative return value seems preferable to me for this reason. (An alternative, more involved idea would be to add 1 to all IRQ "cookies", so IRQ 0 would return 1, leaving 0 as a special value. I have absolutely no idea how big the API surface is that would need changes, and it is likely not worth the effort at all.) > > > > The bottom line here is the SPARC case. Anybody familiar with the > > > platform > > > can shed a light on this. If there is no such case, we may remove > > > warning > > > along with ret = 0 case from platfrom_get_irq(). > > > > I'm afraid you're too fast here... :-) > > We'll have a really hard time if we continue to allow IRQ0 to be > > returned by > > platform_get_irq() -- we'll have oto fileter it out in the callers > > then... > > So far no one reported seeing the big fat warning? > > > > > > 3. The specific cookie for "IRQ not found, while no error > > > > > happened" case > > > > > > > > Not sure what you mean here. I have no problem that a situation > > > > I can > > > > cope with is called an error for the query function. I just do > > > > error > > > > handling and continue happily. So the part "while no error > > > > happened" is > > > > irrelevant to me. > > > > > > I meant that instead of using special error code, 0 is very much > > > good for > > > the cases when IRQ is not found. It allows to distinguish -ENXIO > > > from the > > > low layer from -ENXIO with this magic meaning. > > > > I don't see how -ENXIO can trickle from the lower layers, > > frankly... > > It might one day, leading to very hard to track bugs. As gregkh noted, changing the return value without also making the compile fail will be a huge PITA whenever driver patches are back- or forward-ported, as it would require subtle changes in error paths, which can easily slip through unnoticed, in particular with half- automated stable backports. Even if another return value like -ENODEV might be better aligned with ...regulator_get_optional() and similar functions, or we even find a way to make 0 usable for this, none of the proposed changes strike me as big enough a win to outweigh the churn caused by making such a change at all. Kind regards, Matthias > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > geert@linux-m68k.org > > In personal conversations with technical people, I call myself a > hacker. But > when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds 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 46A76C433EF for ; Fri, 28 Jan 2022 09:29:28 +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 7F34E16F7; Fri, 28 Jan 2022 10:28:36 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 7F34E16F7 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1643362166; bh=GW9NXKkVZtzOIUjsLFCS/Wmg8aDF8wjV62rkPX5gQfg=; h=Subject:From:To:Date:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=gMEdsfVsm1x9by3Esj9wrpgY0/KdvASTYsr7ZsAZY8HrXMXK6bI2y6troauPkftsk BFFrxtH9CKYPimDSZGyUrsV8zM2OXDHUsNT1iIeFQ84hYGCrmJTvJxpllGvwg92lUp AaFgTUScaPGkkgvipAgPoEWuZEXdDKHWv6q0LIQ4= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 56A7BF80551; Fri, 28 Jan 2022 10:24:53 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 26061F80118; Tue, 25 Jan 2022 13:56:16 +0100 (CET) Received: from mx1.tq-group.com (mx1.tq-group.com [93.104.207.81]) (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 18E21F80118 for ; Tue, 25 Jan 2022 13:56:09 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 18E21F80118 Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=tq-group.com header.i=@tq-group.com header.b="ZDZx6MmB"; dkim=pass (2048-bit key) header.d=tq-group.com header.i=@tq-group.com header.b="qzYLcPSM" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1643115371; x=1674651371; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=qmHH9G8TVCwWdo3uTby79rF+j0/3SailZLcBZpByLtg=; b=ZDZx6MmBN+X9n0OsDqi0hnHetrNOJDf3a6HjT5utWU5L//7i7pBOdBCE KAdcwmdYAKL67r2p+Zyx+457BtaFslYFLMgIOYIsVCc/wtozp9lELMuw7 FJgVc53kBdLVC2jytydPGxK4LgxWypQs9Opg4B24iNOUHVA4SeTBO8tWM vO9wS2FR4kUvJwfd4EkbvxAxfHlpZGydNwzj0y9+9vy/WfZsU+AV+rF21 gemAuFmBjmb7GnqcqXnuzurCCUsRZHjV8WR/1XeNzNRgPRIF34ytg39iu NAuKornid8ERI7Bvzhoyaxc1Oeb/w6HL0vqs2IsttdZshWQRIoL56x9J0 g==; X-IronPort-AV: E=Sophos;i="5.88,315,1635199200"; d="scan'208";a="21697222" Received: from unknown (HELO tq-pgp-pr1.tq-net.de) ([192.168.6.15]) by mx1-pgp.tq-group.com with ESMTP; 25 Jan 2022 13:56:09 +0100 Received: from mx1.tq-group.com ([192.168.6.7]) by tq-pgp-pr1.tq-net.de (PGP Universal service); Tue, 25 Jan 2022 13:56:09 +0100 X-PGP-Universal: processed; by tq-pgp-pr1.tq-net.de on Tue, 25 Jan 2022 13:56:09 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1643115369; x=1674651369; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=qmHH9G8TVCwWdo3uTby79rF+j0/3SailZLcBZpByLtg=; b=qzYLcPSMgGM/jDgybQQC0hKCR3aRrdymevnaT4ydaFVxIjMrnrBcV4T6 C5NEX/Uz1J9BcAy/7VV5kAdwi/kxjCsKhTL988mrL1Mf+41YL82U19pTm UIK7b/78zU00Wx1VC2U+5oXhxLzKUsxejHCf55h8+VcSSbzQy6uRt/Qgn jvT61jUGnXXTaU/jyQn1pkY79EM0wk4g7hgaDex//2xlHHq2lIMsC8h0H 2HrYsEYbYjeLAYBA23nQl139MTwAqHxltuCAUqBGA/+5++FvzqsMq6v5y 4URwO2uqbIVjwWBSkS4vRvG/pq3OGi9pHvaUWYVGDg0PfEAcXZz0W9Ise w==; X-IronPort-AV: E=Sophos;i="5.88,315,1635199200"; d="scan'208";a="21697219" Received: from vtuxmail01.tq-net.de ([10.115.0.20]) by mx1.tq-group.com with ESMTP; 25 Jan 2022 13:56:08 +0100 Received: from schifferm-ubuntu (SCHIFFERM-M2.tq-net.de [10.121.201.138]) by vtuxmail01.tq-net.de (Postfix) with ESMTPA id 966B4280065; Tue, 25 Jan 2022 13:56:07 +0100 (CET) Message-ID: <33e55c4c0a637b23d76db5d33872378ad04121bd.camel@ew.tq-group.com> Subject: Re: [PATCH] driver core: platform: Rename platform_get_irq_optional() to platform_get_irq_silent() From: Matthias Schiffer To: Geert Uytterhoeven , Sergey Shtylyov Date: Tue, 25 Jan 2022 13:56:05 +0100 In-Reply-To: References: <20220112213121.5ruae5mxwj6t3qiy@pengutronix.de> <20220113110831.wvwbm75hbfysbn2d@pengutronix.de> <20220113194358.xnnbhsoyetihterb@pengutronix.de> <20220115154539.j3tsz5ioqexq2yuu@pengutronix.de> <20220120075718.5qtrpc543kkykaow@pengutronix.de> <15796e57-f7d4-9c66-3b53-0b026eaf31d8@omp.ru> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Mailman-Approved-At: Fri, 28 Jan 2022 10:24:41 +0100 Cc: Andrew Lunn , Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , linux-iio@vger.kernel.org, Linus Walleij , Amit Kucheria , ALSA Development Mailing List , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , linux-phy@lists.infradead.org, linux-spi , Jiri Slaby , "David S. Miller" , Khuong Dinh , Florian Fainelli , 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 , platform-driver-x86@vger.kernel.org, Linux PWM List , Hans de Goede , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Liam Girdwood , Mauro Carvalho Chehab , John Garry , Peter Korsgaard , William Breathitt Gray , Mark Gross , "open list:GPIO SUBSYSTEM" , Alex Williamson , Mark Brown , Borislav Petkov , Takashi Iwai , Matthias Brugger , openipmi-developer@lists.sourceforge.net, Andy Shevchenko , Benson Leung , Pengutronix Kernel Team , Linux ARM , linux-edac@vger.kernel.org, Tony Luck , Richard Weinberger , Mun Yew Tham , Eric Auger , Greg Kroah-Hartman , Yoshihiro Shimoda , Cornelia Huck , Linux MMC List , Joakim Zhang , Linux Kernel Mailing List , 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 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" On Tue, 2022-01-25 at 09:25 +0100, Geert Uytterhoeven wrote: > Hi Sergey, > > On Mon, Jan 24, 2022 at 10:02 PM Sergey Shtylyov > wrote: > > On 1/24/22 6:01 PM, Andy Shevchenko wrote: > > > > > > > > > It'd certainly be good to name anything that doesn't > > > > > > > > > correspond to one > > > > > > > > > of the existing semantics for the API (!) something > > > > > > > > > different rather > > > > > > > > > than adding yet another potentially overloaded > > > > > > > > > meaning. > > > > > > > > > > > > > > > > It seems we're (at least) three who agree about this. > > > > > > > > Here is a patch > > > > > > > > fixing the name. > > > > > > > > > > > > > > And similar number of people are on the other side. > > > > > > > > > > > > If someone already opposed to the renaming (and not only > > > > > > the name) I > > > > > > must have missed that. > > > > > > > > > > > > So you think it's a good idea to keep the name > > > > > > platform_get_irq_optional() despite the "not found" value > > > > > > returned by it > > > > > > isn't usable as if it were a normal irq number? > > > > > > > > > > I meant that on the other side people who are in favour of > > > > > Sergey's patch. > > > > > Since that I commented already that I opposed the renaming > > > > > being a standalone > > > > > change. > > > > > > > > > > Do you agree that we have several issues with > > > > > platform_get_irq*() APIs? > > [...] > > > > > 2. The vIRQ0 handling: a) WARN() followed by b) returned > > > > > value 0 > > > > > > > > I'm happy with the vIRQ0 handling. Today platform_get_irq() and > > > > it's > > > > silent variant returns either a valid and usuable irq number or > > > > a > > > > negative error value. That's totally fine. > > > > > > It might return 0. > > > Actually it seems that the WARN() can only be issued in two > > > cases: > > > - SPARC with vIRQ0 in one of the array member > > > - fallback to ACPI for GPIO IRQ resource with index 0 > > > > You have probably missed the recent discovery that > > arch/sh/boards/board-aps4*.c > > causes IRQ0 to be passed as a direct IRQ resource? > > So far no one reported seeing the big fat warning ;-) FWIW, we had a similar issue with an IRQ resource passed from the tqmx86 MFD driver do the GPIO driver, which we noticed due to this warning, and which was fixed in a946506c48f3bd09363c9d2b0a178e55733bcbb6 and 9b87f43537acfa24b95c236beba0f45901356eb2. I believe these changes are what promted this whole discussion and led to my "Reported-by" on the patch? It is not entirely clear to me when IRQ 0 is valid and when it isn't, but the warning seems useful to me. Maybe it would make more sense to warn when such an IRQ resource is registered for a platform device, and not when it is looked up? My opinion is that it would be very confusing if there are any places in the kernel (on some platforms) where IRQ 0 is valid, but for platform_get_irq() it would suddenly mean "not found". Keeping a negative return value seems preferable to me for this reason. (An alternative, more involved idea would be to add 1 to all IRQ "cookies", so IRQ 0 would return 1, leaving 0 as a special value. I have absolutely no idea how big the API surface is that would need changes, and it is likely not worth the effort at all.) > > > > The bottom line here is the SPARC case. Anybody familiar with the > > > platform > > > can shed a light on this. If there is no such case, we may remove > > > warning > > > along with ret = 0 case from platfrom_get_irq(). > > > > I'm afraid you're too fast here... :-) > > We'll have a really hard time if we continue to allow IRQ0 to be > > returned by > > platform_get_irq() -- we'll have oto fileter it out in the callers > > then... > > So far no one reported seeing the big fat warning? > > > > > > 3. The specific cookie for "IRQ not found, while no error > > > > > happened" case > > > > > > > > Not sure what you mean here. I have no problem that a situation > > > > I can > > > > cope with is called an error for the query function. I just do > > > > error > > > > handling and continue happily. So the part "while no error > > > > happened" is > > > > irrelevant to me. > > > > > > I meant that instead of using special error code, 0 is very much > > > good for > > > the cases when IRQ is not found. It allows to distinguish -ENXIO > > > from the > > > low layer from -ENXIO with this magic meaning. > > > > I don't see how -ENXIO can trickle from the lower layers, > > frankly... > > It might one day, leading to very hard to track bugs. As gregkh noted, changing the return value without also making the compile fail will be a huge PITA whenever driver patches are back- or forward-ported, as it would require subtle changes in error paths, which can easily slip through unnoticed, in particular with half- automated stable backports. Even if another return value like -ENODEV might be better aligned with ...regulator_get_optional() and similar functions, or we even find a way to make 0 usable for this, none of the proposed changes strike me as big enough a win to outweigh the churn caused by making such a change at all. Kind regards, Matthias > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > geert@linux-m68k.org > > In personal conversations with technical people, I call myself a > hacker. But > when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds