linux-kernel - Re: [PATCH v4 1/2] gpiolib: move all includes to the top of gpio/consumer.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMRc=Mf6HLoORvth1O=DLGXcSvq75-mbmPR0zXg6cm6VV-LVWA@mail.gmail.com>
Date: Wed, 19 Feb 2025 06:35:59 -0800
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Dipen Patel <dipenp@...dia.com>, Linus Walleij <linus.walleij@...aro.org>, 
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org, 
	timestamp@...ts.linux.dev, 
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>
Subject: Re: [PATCH v4 1/2] gpiolib: move all includes to the top of gpio/consumer.h

On Wed, 19 Feb 2025 13:32:49 +0100, Andy Shevchenko
<andriy.shevchenko@...el.com> said:
> On Mon, Feb 17, 2025 at 11:39:21AM +0100, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
>>
>> We have several conditional includes depending on !CONFIG_GPIOLIB. This
>> is supposed to reduce compilation time with CONFIG_GPIOLIB=y but in
>> practice there's no difference on modern machines.
>
> It's not about modern machines. If every maintainer will think this way,
> we end up in the complete and utter dead end with the headers.
>
> I believe you at least had read the cover letter for the infamous Ingo's series
> about headers and how it speeds up the build (in some cases up to 70% on as you
> said "modern machines").
>
>> It makes adding new stubs that depend on more than just GPIOLIB harder so
>> move them all to the top, unduplicate them and replace asm/ with preferred
>> linux/ alternatives.
>
> NAK.
>
> This makes dependency hell things much worse and this is a step back on the
> untangling the current situation along with the slowing down the speed of the
> build. Please. consider to revert or discard this patch.
>
> ...
>
>>  #include <linux/bits.h>
>> +#include <linux/bug.h>
>
> Okay to replace, but not okay to move.
>
>>  #include <linux/err.h>
>> +#include <linux/errno.h>
>
> Please, double check that it uses error codes from it, otherwise err.h includes
> asm/errno.h with basic codes already.
>
>> +#include <linux/kernel.h>
>
> This is definitely no. Please, read what's written in the top of that file and
> here is just a proxy for should come in the future a kind of might_sleep.h.
> Do not move this one at all, please.
>
>>  #include <linux/types.h>
>

Fair enough. Does this look right to you?

diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 0b2b56199c36..38e313fd0e9a 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -3,10 +3,6 @@
 #define __LINUX_GPIO_CONSUMER_H

 #include <linux/bits.h>
-#include <linux/bug.h>
-#include <linux/err.h>
-#include <linux/errno.h>
-#include <linux/kernel.h>
 #include <linux/types.h>

 struct acpi_device;
@@ -185,6 +181,10 @@ struct gpio_desc
*devm_fwnode_gpiod_get_index(struct device *dev,

 #else /* CONFIG_GPIOLIB */

+#include <linux/bug.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+
 static inline int gpiod_count(struct device *dev, const char *con_id)
 {
	return 0;
@@ -549,6 +549,10 @@ struct gpio_desc
*devm_fwnode_gpiod_get_index(struct device *dev,
 int gpiod_enable_hw_timestamp_ns(struct gpio_desc *desc, unsigned long flags);
 int gpiod_disable_hw_timestamp_ns(struct gpio_desc *desc, unsigned long flags);
 #else
+
+#include <linux/bug.h>
+#include <linux/err.h>
+
 static inline int gpiod_enable_hw_timestamp_ns(struct gpio_desc *desc,
					       unsigned long flags)
 {
@@ -615,6 +619,8 @@ int devm_acpi_dev_add_driver_gpios(struct device *dev,

 #else  /* CONFIG_GPIOLIB && CONFIG_ACPI */

+#include <linux/err.h>
+
 static inline int acpi_dev_add_driver_gpios(struct acpi_device *adev,
			      const struct acpi_gpio_mapping *gpios)
 {
@@ -640,6 +646,8 @@ void gpiod_unexport(struct gpio_desc *desc);

 #else  /* CONFIG_GPIOLIB && CONFIG_GPIO_SYSFS */

+#include <linux/err.h>
+
 static inline int gpiod_export(struct gpio_desc *desc,
			       bool direction_may_change)
 {

Bart