linux-kernel - Re: [PATCH 6/8] gpiolib: split linux/gpio/driver.h out of linux/gpio.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: <Y9KIeXq866C+VKV+@smile.fi.intel.com>
Date:   Thu, 26 Jan 2023 16:04:41 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     linux-gpio@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
        Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/8] gpiolib: split linux/gpio/driver.h out of
 linux/gpio.h

On Thu, Jan 26, 2023 at 02:27:59PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
> 
> Almost all gpio drivers include linux/gpio/driver.h, and other
> files should not rely on includes from this header.
> 
> Remove the indirect include from here and include the correct
> headers directly from where they are used.

Some minor nit-picks below, otherwise
Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>

> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  arch/arm/mach-omap1/irq.c                              | 1 +
>  arch/arm/mach-orion5x/board-rd88f5182.c                | 1 +
>  arch/arm/mach-s3c/s3c64xx.c                            | 1 +
>  arch/arm/mach-sa1100/assabet.c                         | 1 +
>  arch/arm/plat-orion/gpio.c                             | 1 +
>  drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c | 1 +
>  include/linux/gpio.h                                   | 2 --
>  include/linux/mfd/ucb1x00.h                            | 1 +
>  8 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/irq.c b/arch/arm/mach-omap1/irq.c
> index 9ccc784fd614..bfc7ab010ae2 100644
> --- a/arch/arm/mach-omap1/irq.c
> +++ b/arch/arm/mach-omap1/irq.c
> @@ -41,6 +41,7 @@
>  #include <linux/sched.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> +#include <linux/irqdomain.h>
>  
>  #include <asm/irq.h>
>  #include <asm/exception.h>
> diff --git a/arch/arm/mach-orion5x/board-rd88f5182.c b/arch/arm/mach-orion5x/board-rd88f5182.c
> index 596601367989..1c14e49a90a6 100644
> --- a/arch/arm/mach-orion5x/board-rd88f5182.c
> +++ b/arch/arm/mach-orion5x/board-rd88f5182.c
> @@ -9,6 +9,7 @@
>  #include <linux/gpio.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/pci.h>
>  #include <linux/irq.h>
> diff --git a/arch/arm/mach-s3c/s3c64xx.c b/arch/arm/mach-s3c/s3c64xx.c
> index c20163e6a8b4..c5aa4d8ed8bd 100644
> --- a/arch/arm/mach-s3c/s3c64xx.c
> +++ b/arch/arm/mach-s3c/s3c64xx.c
> @@ -24,6 +24,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/reboot.h>
>  #include <linux/io.h>

> +#include <linux/of.h>

Taking the limited context into account, I think this makes more sense to be
placed before platform_device.h as to keep longer _sorted_ chain.

>  #include <linux/clk/samsung.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/irq.h>
> diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
> index 04f8a1680bc1..06e7b1358226 100644
> --- a/arch/arm/mach-sa1100/assabet.c
> +++ b/arch/arm/mach-sa1100/assabet.c
> @@ -12,6 +12,7 @@
>  #include <linux/errno.h>
>  #include <linux/gpio/gpio-reg.h>
>  #include <linux/gpio/machine.h>
> +#include <linux/gpio/driver.h>

Similar. Keep it more sorted.

>  #include <linux/gpio_keys.h>
>  #include <linux/ioport.h>
>  #include <linux/platform_data/sa11x0-serial.h>
> diff --git a/arch/arm/plat-orion/gpio.c b/arch/arm/plat-orion/gpio.c
> index 3ef9ecdd6343..4946d8066f6a 100644
> --- a/arch/arm/plat-orion/gpio.c
> +++ b/arch/arm/plat-orion/gpio.c
> @@ -19,6 +19,7 @@
>  #include <linux/bitops.h>
>  #include <linux/io.h>
>  #include <linux/gpio.h>
> +#include <linux/gpio/driver.h>
>  #include <linux/leds.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c
> index 9540a05247c2..89c8829528c2 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include <net/mac80211.h>
>  #include <linux/bcma/bcma_driver_chipcommon.h>
> +#include <linux/gpio.h>
>  #include <linux/gpio/driver.h>
>  #include <linux/gpio/machine.h>
>  #include <linux/gpio/consumer.h>
> diff --git a/include/linux/gpio.h b/include/linux/gpio.h
> index 57ec3975b656..06a33339994e 100644
> --- a/include/linux/gpio.h
> +++ b/include/linux/gpio.h
> @@ -54,8 +54,6 @@ struct gpio {
>  };
>  
>  #ifdef CONFIG_GPIOLIB
> -#include <linux/compiler.h>
> -#include <linux/gpio/driver.h>
>  #include <linux/gpio/consumer.h>
>  
>  /*
> diff --git a/include/linux/mfd/ucb1x00.h b/include/linux/mfd/ucb1x00.h
> index 9aed2797d3d5..2e65354fc7b8 100644
> --- a/include/linux/mfd/ucb1x00.h
> +++ b/include/linux/mfd/ucb1x00.h
> @@ -9,6 +9,7 @@
>  
>  #include <linux/device.h>
>  #include <linux/mfd/mcp.h>

> +#include <linux/gpio/driver.h>
>  #include <linux/gpio.h>

Here is the opposite ordering to the above similar cases. Swap?

>  #include <linux/mutex.h>
>  
> -- 
> 2.39.0
> 

-- 
With Best Regards,
Andy Shevchenko