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 58902C4321E for ; Sun, 16 Jan 2022 18:15:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236000AbiAPSPi (ORCPT ); Sun, 16 Jan 2022 13:15:38 -0500 Received: from mxout03.lancloud.ru ([45.84.86.113]:59572 "EHLO mxout03.lancloud.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235965AbiAPSPd (ORCPT ); Sun, 16 Jan 2022 13:15:33 -0500 Received: from LanCloud DKIM-Filter: OpenDKIM Filter v2.11.0 mxout03.lancloud.ru 2173F206F618 Received: from LanCloud Received: from LanCloud Received: from LanCloud Subject: Re: [PATCH 1/2] platform: make platform_get_irq_optional() optional To: =?UTF-8?Q?Uwe_Kleine-K=c3=b6nig?= CC: Andrew Lunn , Ulf Hansson , Vignesh Raghavendra , KVM list , "Rafael J. Wysocki" , , "Linus Walleij" , Amit Kucheria , "ALSA Development Mailing List" , Andy Shevchenko , Jaroslav Kysela , Guenter Roeck , Thierry Reding , MTD Maling List , Linux I2C , Miquel Raynal , , Jiri Slaby , , "David S. Miller" , Khuong Dinh , Florian Fainelli , Matthias Schiffer , Joakim Zhang , Kamal Dasu , Lee Jones , "Bartosz Golaszewski" , Daniel Lezcano , Kishon Vijay Abraham I , Geert Uytterhoeven , "open list:SERIAL DRIVERS" , Yoshihiro Shimoda , bcm-kernel-feedback-list , Zhang Rui , Linux PWM List , Robert Richter , Saravanan Sekar , Corey Minyard , Linux PM list , Liam Girdwood , Mauro Carvalho Chehab , John Garry , Peter Korsgaard , "William Breathitt Gray" , Mark Gross , Hans de Goede , Alex Williamson , Mark Brown , "Borislav Petkov" , Sebastian Reichel , Jakub Kicinski , Matthias Brugger , Takashi Iwai , , Benson Leung , Linux ARM , , Tony Luck , Mun Yew Tham , Eric Auger , "Greg Kroah-Hartman" , "open list:GPIO SUBSYSTEM" , Cornelia Huck , "Linux MMC List" , Linux Kernel Mailing List , linux-spi , Linux-Renesas , Vinod Koul , James Morse , Zha Qipeng , Pengutronix Kernel Team , Richard Weinberger , =?UTF-8?Q?Niklas_S=c3=b6derlund?= , , "Brian Norris" , References: <20220110201014.mtajyrfcfznfhyqm@pengutronix.de> <20220112085009.dbasceh3obfok5dc@pengutronix.de> <20220112213121.5ruae5mxwj6t3qiy@pengutronix.de> <29f0c65d-77f2-e5b2-f6cc-422add8a707d@omp.ru> <20220114092557.jrkfx7ihg26ekzci@pengutronix.de> <61b80939-357d-14f5-df99-b8d102a4e1a1@omp.ru> <20220114202226.ugzklxv4wzr6egwj@pengutronix.de> From: Sergey Shtylyov Organization: Open Mobile Platform Message-ID: <57af1851-9341-985e-7b28-d2ba86770ecb@omp.ru> Date: Sun, 16 Jan 2022 21:15:20 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20220114202226.ugzklxv4wzr6egwj@pengutronix.de> Content-Type: text/plain; charset="windows-1252" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [192.168.11.198] X-ClientProxiedBy: LFEXT01.lancloud.ru (fd00:f066::141) To LFEX1907.lancloud.ru (fd00:f066::207) Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org Hello! On 1/14/22 11:22 PM, Uwe Kleine-König wrote: >>>>>>> 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. >>>>> >>>>>> No, the main benefit of gpiod_get_optional() (and clk_get_optional()) is >>>>>> that you can handle an absent GPIO (or clk) as if it were available. >>>> >>>> Hm, I've just looked at these and must note that they match 1:1 with >>>> platform_get_irq_optional(). Unfortunately, we can't however behave the >>>> same way in request_irq() -- because it has to support IRQ0 for the sake >>>> of i8253 drivers in arch/... >>> >>> Let me reformulate your statement to the IMHO equivalent: >>> >>> If you set aside the differences between >>> platform_get_irq_optional() and gpiod_get_optional(), >> >> Sorry, I should make it clear this is actually the diff between a would-be >> platform_get_irq_optional() after my patch, not the current code... > > The similarity is that with your patch both gpiod_get_optional() and > platform_get_irq_optional() return NULL and 0 on not-found. The relevant > difference however is that for a gpiod NULL is a dummy value, while for > irqs it's not. So the similarity is only syntactically, but not > semantically. I have noting to say here, rather than optional IRQ could well have a different meaning than for clk/gpio/etc. [...] >>> However for an interupt this cannot work. You will always have to check >>> if the irq is actually there or not because if it's not you cannot just >>> ignore that. So there is no benefit of an optional irq. >>> >>> Leaving error message reporting aside, the introduction of >>> platform_get_irq_optional() allows to change >>> >>> irq = platform_get_irq(...); >>> if (irq < 0 && irq != -ENXIO) { >>> return irq; >>> } else if (irq >= 0) { >> >> Rather (irq > 0) actually, IRQ0 is considered invalid (but still returned). > > This is a topic I don't feel strong for, so I'm sloppy here. If changing > this is all that is needed to convince you of my point ... Note that we should absolutely (and first of all) stop returning 0 from platform_get_irq() on a "real" IRQ0. Handling that "still good" zero absolutely doesn't scale e.g. for the subsystems (like libata) which take 0 as an indication that the polling mode should be used... We can't afford to be sloppy here. ;-) [...] > Best regards > Uwe MBR, Sergey