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 45BEFC3525B for ; Wed, 12 Jan 2022 13:55:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1353796AbiALNzG (ORCPT ); Wed, 12 Jan 2022 08:55:06 -0500 Received: from mail-ua1-f49.google.com ([209.85.222.49]:41851 "EHLO mail-ua1-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1353747AbiALNzA (ORCPT ); Wed, 12 Jan 2022 08:55:00 -0500 Received: by mail-ua1-f49.google.com with SMTP id p37so4867312uae.8; Wed, 12 Jan 2022 05:54:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=liYNFdlsWo55pmV9ntmO9rGWUh5k4VTb3DRABECHgIo=; b=Ce9z+0RhpEuDwV49z9nysFRxmar8MkdptVAlCU6Y1S6jDEdZJrirfhY0HMauE36KcG Kne+ZSRn8NJAc80uvWUtgx86qozGED/YeDYsJkye7Q2CzapDAFaZ4r7rw9sgK0wUHpNI Xg2+KPaE7rWILdGcFo2asnJy67cbHa6WppoJONwlvmWAtx0ugj/YQr4U8bCVvMNEm377 eQOr7tvZqOeMR68KOmpFA2V9xrBRefmQ6L+FANi8r1EguceteAD6dgw0ChQ3YaKmaBuD eb6Zb+ZBoMylYBxtwaq5lI/h98jLnOTgKvfLuLbTmXdYBNOQxGumAyIkEdiEOVhuNFlE 5N6A== X-Gm-Message-State: AOAM5311zYeLx9/x9lUBAcsekjlNLX25NBaoUmOFqW9iria8gwaHsJKR CiLgNacN3wmxfsC6r0z+4vdU+H5VKQiRzGNr X-Google-Smtp-Source: ABdhPJwnR+euX9sbAHp/A8jaywC6gBn9cqzMbafXfQIkM8VHORFCZAgtddrIuQoBO+XtvbsKhHh1Dw== X-Received: by 2002:a67:24c3:: with SMTP id k186mr4141265vsk.74.1641995697787; Wed, 12 Jan 2022 05:54:57 -0800 (PST) Received: from mail-vk1-f179.google.com (mail-vk1-f179.google.com. [209.85.221.179]) by smtp.gmail.com with ESMTPSA id w62sm7361401vkd.47.2022.01.12.05.54.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 12 Jan 2022 05:54:57 -0800 (PST) Received: by mail-vk1-f179.google.com with SMTP id w206so1693955vkd.10; Wed, 12 Jan 2022 05:54:57 -0800 (PST) X-Received: by 2002:ac5:c967:: with SMTP id t7mr4740789vkm.20.1641995696856; Wed, 12 Jan 2022 05:54:56 -0800 (PST) MIME-Version: 1.0 References: <20220110195449.12448-1-s.shtylyov@omp.ru> <20220110195449.12448-2-s.shtylyov@omp.ru> <20220110201014.mtajyrfcfznfhyqm@pengutronix.de> <20220112085009.dbasceh3obfok5dc@pengutronix.de> In-Reply-To: From: Geert Uytterhoeven Date: Wed, 12 Jan 2022 14:54:44 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/2] platform: make platform_get_irq_optional() optional To: Andrew Lunn Cc: =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , 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 , =?UTF-8?Q?Niklas_S=C3=B6derlund?= , linux-mediatek@lists.infradead.org, Brian Norris , netdev@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi Andrew, On Wed, Jan 12, 2022 at 2:38 PM Andrew Lunn wrote: > > 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. Agreed. Note that in most (all?) other cases, the return type is a pointer (e.g. to struct clk), and NULL is the magic cookie. > You seems to be suggesting in binary return value: non-zero > (available) or zero (not available) Only in case of success. In case of a real failure, an error code must be returned. > This discards the error code when something goes wrong. That is useful > information to have, so we should not be discarding it. No, the error code must be retained in case of failure. > 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. Exactly. And using 0 means the similar code can be used as for other subsystems, where NULL would be returned. The only remaining difference is the "dummy cookie can be passed to other functions" behavior. Which is IMHO a valid difference, as unlike with e.g. clk_prepare_enable(), you do pass extra data to request_irq(), and sometimes you do need to handle the absence of the interrupt using e.g. polling. > 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. There are still a few platforms where IRQ 0 does exist. 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