Message ID | 20230125201020.10948-2-andriy.shevchenko@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp464712wrn; Wed, 25 Jan 2023 12:12:02 -0800 (PST) X-Google-Smtp-Source: AMrXdXtLvJjsvzpBlDwFqK2gKgMaZLvQXJr4IQqz1v6Iusd1kTVLWqmtKzTmdjYqhMe7+x7E+hJw X-Received: by 2002:a17:907:80cf:b0:7c1:962e:cf24 with SMTP id io15-20020a17090780cf00b007c1962ecf24mr37700751ejc.50.1674677522803; Wed, 25 Jan 2023 12:12:02 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1674677522; cv=none; d=google.com; s=arc-20160816; b=IFYXlzqeyeETealSSI9ggOFa7ottnYEBvPamn4kqXpXK7E76H/zmF8i0D9vQnVITgT 3rsN4PsR2ECtAan+4YoG3u+Q49LNpgMT3zx5qcBpC2mDj4mBlLfM2ZUUjUOEWHoKA5YK SEHdYmm/8IR41mVJSGC/Vv8pamerRjRjFUze7c1VoPed+6YxoGPtRU6QHNtF5M2Zclqo pWlMoVctWov3BuRb8qvwB0hEfa8nZS9FyxqWK1/Gf0XImuRuMdChdJG8urZtjAnWzm6H +O6EwrBTp/Twyui9quQm3X4InNpCqpsi8l31PG4IoX5bR7bDMPcmXsO7fP+1Fp1bw3OL ZkHw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=uUf2sV3BkGC+X3F0jQzcaytoPNQ/yUkIdTHWDQrNsvw=; b=cEzGkWavYgVSvyVzxd4cbZ/MI9s2o2xrPich9RG7yd7qeimfYg36LrT0qc/aJ8HQnE vY98hLatoGsoThTASr+TBKPT1BUnOApRJ3mLuklEhPgiKyMtVstmjxLGfZWMRN2FCc0V bdtbI8mgz5Aks8qdfggMEJfl3TAJO1tuaAxWvy3NEvktLUHg4QP2uaV5zpmzqyFW5weP 5muv9Y3G1Jf23M1AHHwlUL1y52FOCanU/hO9AMb+RwyQH4a2ZQI3aVPhFyWAx4Gfbc79 UE799vN7mjw3xNfbVNhDZ3sLaUIs24R5Cy0G7miCN1b99rQy9dkWXc/2N8r3jzaJjRHx g9Pg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=W2j+5NCu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id x16-20020a05640226d000b004a056941677si9789147edd.471.2023.01.25.12.11.39; Wed, 25 Jan 2023 12:12:02 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=W2j+5NCu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236304AbjAYUKv (ORCPT <rfc822;lekhanya01809@gmail.com> + 99 others); Wed, 25 Jan 2023 15:10:51 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45218 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236338AbjAYUKs (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 25 Jan 2023 15:10:48 -0500 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A3EC45D11A; Wed, 25 Jan 2023 12:10:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1674677418; x=1706213418; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=mWptUuAvNZS0kbLHRKn6oORyrZ25KQvA1dzBPjHWj5Y=; b=W2j+5NCuuX9pBpvjyjnweD+WaROJFjF8T0ZucJZB2nuFzVfah4nYli2y FPmk1UyN5rMTibZfXz7K+D0gWnLpf26Yo7fa/GwuYjU+KsnrmyDXsW0GW JyHz8mPoX3IR9+ZKGXEMo4aGxYYRXgcaVD9dHcYpfwaMulEfQdLSGPbrQ uVdqXRkC33Vo59KxgO0yEOOh+3iDItsHD79IPj1MuA5Ga5kSEu6cbfuss sOL7wYnOek1Q0ZbY4Du8LnDMN/j99/ARG3138/bvpsrHsT456z3yXPRHB NbD3Rjk94XR9hPO4yYSIjjeFtdpctugJt3N3E8RXu0z8BJBu9Ix11rVu2 A==; X-IronPort-AV: E=McAfee;i="6500,9779,10601"; a="326694750" X-IronPort-AV: E=Sophos;i="5.97,246,1669104000"; d="scan'208";a="326694750" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jan 2023 12:09:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10601"; a="770871658" X-IronPort-AV: E=Sophos;i="5.97,246,1669104000"; d="scan'208";a="770871658" Received: from black.fi.intel.com ([10.237.72.28]) by fmsmga002.fm.intel.com with ESMTP; 25 Jan 2023 12:09:49 -0800 Received: by black.fi.intel.com (Postfix, from userid 1003) id 3ABE992; Wed, 25 Jan 2023 22:10:25 +0200 (EET) From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Bartosz Golaszewski <bartosz.golaszewski@linaro.org>, Christophe Leroy <christophe.leroy@csgroup.eu>, Dmitry Torokhov <dmitry.torokhov@gmail.com>, linux-gpio@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Linus Walleij <linus.walleij@linaro.org>, Bartosz Golaszewski <brgl@bgdev.pl>, Arnd Bergmann <arnd@arndb.de>, Pierluigi Passaro <pierluigi.p@variscite.com>, kernel test robot <lkp@intel.com> Subject: [PATCH v1 1/5] gpiolib: fix linker errors when GPIOLIB is disabled Date: Wed, 25 Jan 2023 22:10:16 +0200 Message-Id: <20230125201020.10948-2-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230125201020.10948-1-andriy.shevchenko@linux.intel.com> References: <20230125201020.10948-1-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1756026657907074843?= X-GMAIL-MSGID: =?utf-8?q?1756026657907074843?= |
Series |
gpio: First attempt to clean up headers
|
|
Commit Message
Andy Shevchenko
Jan. 25, 2023, 8:10 p.m. UTC
From: Pierluigi Passaro <pierluigi.p@variscite.com> Both the functions gpiochip_request_own_desc and gpiochip_free_own_desc are exported from drivers/gpio/gpiolib.c but this file is compiled only when CONFIG_GPIOLIB is enabled. Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide reasonable definitions and includes in the "#else" branch. Fixes: 9091373ab7ea ("gpio: remove less important #ifdef around declarations") Signed-off-by: Pierluigi Passaro <pierluigi.p@variscite.com> Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- include/linux/gpio/driver.h | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)
Comments
Le 25/01/2023 à 21:10, Andy Shevchenko a écrit : > From: Pierluigi Passaro <pierluigi.p@variscite.com> > > Both the functions gpiochip_request_own_desc and > gpiochip_free_own_desc are exported from > drivers/gpio/gpiolib.c > but this file is compiled only when CONFIG_GPIOLIB is enabled. > Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide > reasonable definitions and includes in the "#else" branch. Can you give more details on when and why link fails ? You are adding a WARN(), I understand it mean the function should never ever be called. Shouldn't it be dropped completely by the compiler ? In that case, no call to gpiochip_request_own_desc() should be emitted and so link should be ok. If link fails, it means we still have unexpected calls to gpiochip_request_own_desc() or gpiochip_free_own_desc(), and we should fix the root cause instead of hiding it with a WARN(). Christophe > > Fixes: 9091373ab7ea ("gpio: remove less important #ifdef around declarations") > Signed-off-by: Pierluigi Passaro <pierluigi.p@variscite.com> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > include/linux/gpio/driver.h | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h > index 26a808fb8a25..67990908a040 100644 > --- a/include/linux/gpio/driver.h > +++ b/include/linux/gpio/driver.h > @@ -751,6 +751,8 @@ gpiochip_remove_pin_ranges(struct gpio_chip *gc) > > #endif /* CONFIG_PINCTRL */ > > +#ifdef CONFIG_GPIOLIB > + > struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, > unsigned int hwnum, > const char *label, > @@ -758,8 +760,6 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, > enum gpiod_flags dflags); > void gpiochip_free_own_desc(struct gpio_desc *desc); > > -#ifdef CONFIG_GPIOLIB > - > /* lock/unlock as IRQ */ > int gpiochip_lock_as_irq(struct gpio_chip *gc, unsigned int offset); > void gpiochip_unlock_as_irq(struct gpio_chip *gc, unsigned int offset); > @@ -769,6 +769,25 @@ struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc); > > #else /* CONFIG_GPIOLIB */ > > +#include <linux/gpio/machine.h> > +#include <linux/gpio/consumer.h> > + > +static inline struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, > + unsigned int hwnum, > + const char *label, > + enum gpio_lookup_flags lflags, > + enum gpiod_flags dflags) > +{ > + /* GPIO can never have been requested */ > + WARN_ON(1); > + return ERR_PTR(-ENODEV); > +} > + > +static inline void gpiochip_free_own_desc(struct gpio_desc *desc) > +{ > + WARN_ON(1); > +} > + > static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc) > { > /* GPIO can never have been requested */
On Thu, Jan 26, 2023, at 09:14, Christophe Leroy wrote: > Le 25/01/2023 à 21:10, Andy Shevchenko a écrit : >> From: Pierluigi Passaro <pierluigi.p@variscite.com> >> >> Both the functions gpiochip_request_own_desc and >> gpiochip_free_own_desc are exported from >> drivers/gpio/gpiolib.c >> but this file is compiled only when CONFIG_GPIOLIB is enabled. >> Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide >> reasonable definitions and includes in the "#else" branch. > > Can you give more details on when and why link fails ? > > You are adding a WARN(), I understand it mean the function should never > ever be called. Shouldn't it be dropped completely by the compiler ? In > that case, no call to gpiochip_request_own_desc() should be emitted and > so link should be ok. > > If link fails, it means we still have unexpected calls to > gpiochip_request_own_desc() or gpiochip_free_own_desc(), and we should > fix the root cause instead of hiding it with a WARN(). There are only a handful of files calling these functions: $ git grep -l gpiochip_request_own_desc Documentation/driver-api/gpio/driver.rst arch/arm/mach-omap1/ams-delta-fiq.c arch/arm/mach-omap1/board-ams-delta.c drivers/gpio/gpio-mvebu.c drivers/gpio/gpiolib-acpi.c drivers/gpio/gpiolib.c drivers/hid/hid-cp2112.c drivers/memory/omap-gpmc.c drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c drivers/power/supply/collie_battery.c drivers/spi/spi-bcm2835.c include/linux/gpio/driver.h All of these should already prevent the link failure through a Kconfig 'depends on GPIOLIB' for the driver, or 'select GPIOLIB' for the platform code. I checked all of the above and they seem fine. If anything else calls the function, I'd add the same dependency there. Arnd
On Thu, Jan 26, 2023 at 09:40:18AM +0100, Arnd Bergmann wrote: > On Thu, Jan 26, 2023, at 09:14, Christophe Leroy wrote: > > Le 25/01/2023 à 21:10, Andy Shevchenko a écrit : > >> From: Pierluigi Passaro <pierluigi.p@variscite.com> > >> > >> Both the functions gpiochip_request_own_desc and > >> gpiochip_free_own_desc are exported from > >> drivers/gpio/gpiolib.c > >> but this file is compiled only when CONFIG_GPIOLIB is enabled. > >> Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide > >> reasonable definitions and includes in the "#else" branch. > > > > Can you give more details on when and why link fails ? > > > > You are adding a WARN(), I understand it mean the function should never > > ever be called. Shouldn't it be dropped completely by the compiler ? In > > that case, no call to gpiochip_request_own_desc() should be emitted and > > so link should be ok. > > > > If link fails, it means we still have unexpected calls to > > gpiochip_request_own_desc() or gpiochip_free_own_desc(), and we should > > fix the root cause instead of hiding it with a WARN(). > > There are only a handful of files calling these functions: > > $ git grep -l gpiochip_request_own_desc > Documentation/driver-api/gpio/driver.rst > arch/arm/mach-omap1/ams-delta-fiq.c > arch/arm/mach-omap1/board-ams-delta.c > drivers/gpio/gpio-mvebu.c > drivers/gpio/gpiolib-acpi.c > drivers/gpio/gpiolib.c > drivers/hid/hid-cp2112.c > drivers/memory/omap-gpmc.c > drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c > drivers/power/supply/collie_battery.c > drivers/spi/spi-bcm2835.c > include/linux/gpio/driver.h > > All of these should already prevent the link failure through > a Kconfig 'depends on GPIOLIB' for the driver, or 'select GPIOLIB' > for the platform code. I checked all of the above and they seem fine. > If anything else calls the function, I'd add the same dependency > there. So, you think it's worth to send a few separate fixes as adding that dependency? But doesn't it feel like a papering over the issue with that APIs used in some of the drivers in the first place?
On Thu, Jan 26, 2023 at 08:14:49AM +0000, Christophe Leroy wrote: > Le 25/01/2023 à 21:10, Andy Shevchenko a écrit : > > From: Pierluigi Passaro <pierluigi.p@variscite.com> > > > > Both the functions gpiochip_request_own_desc and > > gpiochip_free_own_desc are exported from > > drivers/gpio/gpiolib.c > > but this file is compiled only when CONFIG_GPIOLIB is enabled. > > Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide > > reasonable definitions and includes in the "#else" branch. > > Can you give more details on when and why link fails ? > > You are adding a WARN(), I understand it mean the function should never > ever be called. Shouldn't it be dropped completely by the compiler ? In > that case, no call to gpiochip_request_own_desc() should be emitted and > so link should be ok. > > If link fails, it means we still have unexpected calls to > gpiochip_request_own_desc() or gpiochip_free_own_desc(), and we should > fix the root cause instead of hiding it with a WARN(). I agree, but what do you suggest exactly? I think the calls to that functions shouldn't be in the some drivers as it's layering violation (they are not a GPIO chips to begin with). Simply adding a dependency not better than this one.
On Thu, Jan 26, 2023, at 11:17, Andy Shevchenko wrote: > On Thu, Jan 26, 2023 at 09:40:18AM +0100, Arnd Bergmann wrote: >> On Thu, Jan 26, 2023, at 09:14, Christophe Leroy wrote: >> >> All of these should already prevent the link failure through >> a Kconfig 'depends on GPIOLIB' for the driver, or 'select GPIOLIB' >> for the platform code. I checked all of the above and they seem fine. >> If anything else calls the function, I'd add the same dependency >> there. > > So, you think it's worth to send a few separate fixes as adding that > dependency? But doesn't it feel like a papering over the issue with > that APIs used in some of the drivers in the first place? If there are drivers that use the interfaces but shouldn't then fixing those drivers is clearly better than adding a dependency, but we can decide that when someone sends a patch. Adding a stub helper that can never be used legitimately but turns a build time error into a run time warning seems counterproductive to me, as the CI systems are no longer able to report these automatically. Arnd
On Thu, Jan 26, 2023 at 11:27:51AM +0100, Arnd Bergmann wrote: > On Thu, Jan 26, 2023, at 11:17, Andy Shevchenko wrote: > > On Thu, Jan 26, 2023 at 09:40:18AM +0100, Arnd Bergmann wrote: > >> On Thu, Jan 26, 2023, at 09:14, Christophe Leroy wrote: > >> > >> All of these should already prevent the link failure through > >> a Kconfig 'depends on GPIOLIB' for the driver, or 'select GPIOLIB' > >> for the platform code. I checked all of the above and they seem fine. > >> If anything else calls the function, I'd add the same dependency > >> there. > > > > So, you think it's worth to send a few separate fixes as adding that > > dependency? But doesn't it feel like a papering over the issue with > > that APIs used in some of the drivers in the first place? > > If there are drivers that use the interfaces but shouldn't then > fixing those drivers is clearly better than adding a dependency, > but we can decide that when someone sends a patch. > > Adding a stub helper that can never be used legitimately > but turns a build time error into a run time warning seems > counterproductive to me, as the CI systems are no longer > able to report these automatically. What about adding ifdeffery in their code instead with a FIXME comment? So we will know that it's ugly and needs to be solved better sooner than later.
Le 26/01/2023 à 11:19, Andy Shevchenko a écrit : > On Thu, Jan 26, 2023 at 08:14:49AM +0000, Christophe Leroy wrote: >> Le 25/01/2023 à 21:10, Andy Shevchenko a écrit : >>> From: Pierluigi Passaro <pierluigi.p@variscite.com> >>> >>> Both the functions gpiochip_request_own_desc and >>> gpiochip_free_own_desc are exported from >>> drivers/gpio/gpiolib.c >>> but this file is compiled only when CONFIG_GPIOLIB is enabled. >>> Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide >>> reasonable definitions and includes in the "#else" branch. >> >> Can you give more details on when and why link fails ? >> >> You are adding a WARN(), I understand it mean the function should never >> ever be called. Shouldn't it be dropped completely by the compiler ? In >> that case, no call to gpiochip_request_own_desc() should be emitted and >> so link should be ok. >> >> If link fails, it means we still have unexpected calls to >> gpiochip_request_own_desc() or gpiochip_free_own_desc(), and we should >> fix the root cause instead of hiding it with a WARN(). > > I agree, but what do you suggest exactly? I think the calls to that functions > shouldn't be in the some drivers as it's layering violation (they are not a > GPIO chips to begin with). Simply adding a dependency not better than this one. > My suggestion is to go step by step. First step is to explicitely list drivers that call those functions without selecting GPIOLIB. Once we have this list we can see one by one how we solve it. And if we want to catch the problem before the final link, then I think we may use BUILD_BUG() but not WARN or WARN_ON. Christophe
On Thu, Jan 26, 2023, at 13:44, Christophe Leroy wrote: > Le 26/01/2023 à 11:19, Andy Shevchenko a écrit : >> On Thu, Jan 26, 2023 at 08:14:49AM +0000, Christophe Leroy wrote: >>> Le 25/01/2023 à 21:10, Andy Shevchenko a écrit : >>>> From: Pierluigi Passaro <pierluigi.p@variscite.com> >>>> >>>> Both the functions gpiochip_request_own_desc and >>>> gpiochip_free_own_desc are exported from >>>> drivers/gpio/gpiolib.c >>>> but this file is compiled only when CONFIG_GPIOLIB is enabled. >>>> Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide >>>> reasonable definitions and includes in the "#else" branch. >>> >>> Can you give more details on when and why link fails ? >>> >>> You are adding a WARN(), I understand it mean the function should never >>> ever be called. Shouldn't it be dropped completely by the compiler ? In >>> that case, no call to gpiochip_request_own_desc() should be emitted and >>> so link should be ok. >>> >>> If link fails, it means we still have unexpected calls to >>> gpiochip_request_own_desc() or gpiochip_free_own_desc(), and we should >>> fix the root cause instead of hiding it with a WARN(). >> >> I agree, but what do you suggest exactly? I think the calls to that functions >> shouldn't be in the some drivers as it's layering violation (they are not a >> GPIO chips to begin with). Simply adding a dependency not better than this one. >> > > My suggestion is to go step by step. First step is to explicitely list > drivers that call those functions without selecting GPIOLIB. I tried that and sent the list of the drivers that call these functions, but as I wrote, all of them already require GPIOLIB to be set. This means either I made a mistake in my search, or the problem has already been fixed. Either way, I think Andy should provide the exact build failure he observed so we know what caller caused the issue. Arnd
On Thu, Jan 26, 2023 at 01:56:26PM +0100, Arnd Bergmann wrote: > On Thu, Jan 26, 2023, at 13:44, Christophe Leroy wrote: > > Le 26/01/2023 à 11:19, Andy Shevchenko a écrit : > >> On Thu, Jan 26, 2023 at 08:14:49AM +0000, Christophe Leroy wrote: > >>> Le 25/01/2023 à 21:10, Andy Shevchenko a écrit : > >>>> From: Pierluigi Passaro <pierluigi.p@variscite.com> > >>>> > >>>> Both the functions gpiochip_request_own_desc and > >>>> gpiochip_free_own_desc are exported from > >>>> drivers/gpio/gpiolib.c > >>>> but this file is compiled only when CONFIG_GPIOLIB is enabled. > >>>> Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide > >>>> reasonable definitions and includes in the "#else" branch. > >>> > >>> Can you give more details on when and why link fails ? > >>> > >>> You are adding a WARN(), I understand it mean the function should never > >>> ever be called. Shouldn't it be dropped completely by the compiler ? In > >>> that case, no call to gpiochip_request_own_desc() should be emitted and > >>> so link should be ok. > >>> > >>> If link fails, it means we still have unexpected calls to > >>> gpiochip_request_own_desc() or gpiochip_free_own_desc(), and we should > >>> fix the root cause instead of hiding it with a WARN(). > >> > >> I agree, but what do you suggest exactly? I think the calls to that functions > >> shouldn't be in the some drivers as it's layering violation (they are not a > >> GPIO chips to begin with). Simply adding a dependency not better than this one. > >> > > > > My suggestion is to go step by step. First step is to explicitely list > > drivers that call those functions without selecting GPIOLIB. > > I tried that and sent the list of the drivers that call these functions, > but as I wrote, all of them already require GPIOLIB to be set. > > This means either I made a mistake in my search, or the problem > has already been fixed. Either way, I think Andy should provide > the exact build failure he observed so we know what caller caused > the issue. I believe it's not me, who first reported it. So, Pierluigi, can you point out to the LKP message that reported the issue? P.S> LKP sometimes finds a really twisted configurations to probe on.
On Thu, Jan 26, 2023 at 02:29PM +0100, Arnd Bergmann wrote: > On Thu, Jan 26, 2023 at 01:56:26PM +0100, Arnd Bergmann wrote: > > On Thu, Jan 26, 2023, at 13:44, Christophe Leroy wrote: > > > Le 26/01/2023 à 11:19, Andy Shevchenko a écrit : > > >> On Thu, Jan 26, 2023 at 08:14:49AM +0000, Christophe Leroy wrote: > > >>> Le 25/01/2023 à 21:10, Andy Shevchenko a écrit : > > >>>> From: Pierluigi Passaro <pierluigi.p@variscite.com> > > >>>> > > >>>> Both the functions gpiochip_request_own_desc and > > >>>> gpiochip_free_own_desc are exported from > > >>>> drivers/gpio/gpiolib.c > > >>>> but this file is compiled only when CONFIG_GPIOLIB is enabled. > > >>>> Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide > > >>>> reasonable definitions and includes in the "#else" branch. > > >>> > > >>> Can you give more details on when and why link fails ? > > >>> > > >>> You are adding a WARN(), I understand it mean the function should never > > >>> ever be called. Shouldn't it be dropped completely by the compiler ? In > > >>> that case, no call to gpiochip_request_own_desc() should be emitted and > > >>> so link should be ok. > > >>> > > >>> If link fails, it means we still have unexpected calls to > > >>> gpiochip_request_own_desc() or gpiochip_free_own_desc(), and we should > > >>> fix the root cause instead of hiding it with a WARN(). > > >> > > >> I agree, but what do you suggest exactly? I think the calls to that functions > > >> shouldn't be in the some drivers as it's layering violation (they are not a > > >> GPIO chips to begin with). Simply adding a dependency not better than this one. > > >> > > > > > > My suggestion is to go step by step. First step is to explicitely list > > > drivers that call those functions without selecting GPIOLIB. > > > > I tried that and sent the list of the drivers that call these functions, > > but as I wrote, all of them already require GPIOLIB to be set. > > > > This means either I made a mistake in my search, or the problem > > has already been fixed. Either way, I think Andy should provide > > the exact build failure he observed so we know what caller caused > > the issue. > > I believe it's not me, who first reported it. So, Pierluigi, can you point > out to the LKP message that reported the issue? > > P.S> LKP sometimes finds a really twisted configurations to probe on. > > -- > With Best Regards, > Andy Shevchenko > I've received the following messages: - https://lore.kernel.org/all/202301240409.tZdm0o0a-lkp@intel.com/ - https://lore.kernel.org/all/202301240439.wYz6uU0k-lkp@intel.com/ - https://lore.kernel.org/all/20230124075600.649bd7bb@canb.auug.org.au/ Please let me know if you need further details. Regards Pier
On Thu, Jan 26, 2023, at 14:41, Pierluigi Passaro wrote: > On Thu, Jan 26, 2023 at 02:29PM +0100, Arnd Bergmann wrote: >> > >>> >> > >>> If link fails, it means we still have unexpected calls to >> > >>> gpiochip_request_own_desc() or gpiochip_free_own_desc(), and we should >> > >>> fix the root cause instead of hiding it with a WARN(). >> > This means either I made a mistake in my search, or the problem >> > has already been fixed. Either way, I think Andy should provide >> > the exact build failure he observed so we know what caller caused >> > the issue. >> >> I believe it's not me, who first reported it. So, Pierluigi, can you point >> out to the LKP message that reported the issue? >> >> P.S> LKP sometimes finds a really twisted configurations to probe on. >> >> > I've received the following messages: > - https://lore.kernel.org/all/202301240409.tZdm0o0a-lkp@intel.com/ > - https://lore.kernel.org/all/202301240439.wYz6uU0k-lkp@intel.com/ > - https://lore.kernel.org/all/20230124075600.649bd7bb@canb.auug.org.au/ > Please let me know if you need further details. I think these three are all regressions that are caused by the patch in this thread, rather than the original problem that it was trying to fix. The one we're looking for is a randconfig bug that showed up as a link failure when referencing gpiochip_request_own_desc or gpiochip_free_own_desc. Arnd
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 26a808fb8a25..67990908a040 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -751,6 +751,8 @@ gpiochip_remove_pin_ranges(struct gpio_chip *gc) #endif /* CONFIG_PINCTRL */ +#ifdef CONFIG_GPIOLIB + struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, unsigned int hwnum, const char *label, @@ -758,8 +760,6 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, enum gpiod_flags dflags); void gpiochip_free_own_desc(struct gpio_desc *desc); -#ifdef CONFIG_GPIOLIB - /* lock/unlock as IRQ */ int gpiochip_lock_as_irq(struct gpio_chip *gc, unsigned int offset); void gpiochip_unlock_as_irq(struct gpio_chip *gc, unsigned int offset); @@ -769,6 +769,25 @@ struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc); #else /* CONFIG_GPIOLIB */ +#include <linux/gpio/machine.h> +#include <linux/gpio/consumer.h> + +static inline struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, + unsigned int hwnum, + const char *label, + enum gpio_lookup_flags lflags, + enum gpiod_flags dflags) +{ + /* GPIO can never have been requested */ + WARN_ON(1); + return ERR_PTR(-ENODEV); +} + +static inline void gpiochip_free_own_desc(struct gpio_desc *desc) +{ + WARN_ON(1); +} + static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc) { /* GPIO can never have been requested */