Re: [PATCH v2 3/3] leds: simatic-ipc-leds-gpio: introduce more Kconfig switches

From: Hans de Goede <hdegoede@redhat.com>
To: Henning Schild <henning.schild@siemens.com>,
	Pavel Machek <pavel@ucw.cz>, Lee Jones <lee@kernel.org>,
	Mark Gross <markgross@kernel.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org,
	platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH v2 3/3] leds: simatic-ipc-leds-gpio: introduce more Kconfig switches
Date: Wed, 1 Mar 2023 19:04:01 +0100	[thread overview]
Message-ID: <a6281345-95e1-3e8d-b6fb-146c1852373f@redhat.com> (raw)
In-Reply-To: <20230301170215.23382-4-henning.schild@siemens.com>

Hi,

On 3/1/23 18:02, Henning Schild wrote:
> To describe the dependency chain better and allow for potential
> fine-grained config tuning, introduce Kconfig switch for the individual
> GPIO based drivers.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  drivers/leds/simple/Kconfig  | 31 ++++++++++++++++++++++++++++---
>  drivers/leds/simple/Makefile |  7 +++----
>  2 files changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig
> index fd2b8225d926..44fa0f93cb3b 100644
> --- a/drivers/leds/simple/Kconfig
> +++ b/drivers/leds/simple/Kconfig
> @@ -1,11 +1,36 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  config LEDS_SIEMENS_SIMATIC_IPC
>  	tristate "LED driver for Siemens Simatic IPCs"
> -	depends on LEDS_GPIO

Since it is simatic-ipc-leds-gpio-core.c which actually registers
the LEDs GPIO platform device, this one should stay IMHO.

>  	depends on SIEMENS_SIMATIC_IPC
>  	help
>  	  This option enables support for the LEDs of several Industrial PCs
>  	  from Siemens.
>  
> -	  To compile this driver as a module, choose M here: the modules
> -	  will be called simatic-ipc-leds and simatic-ipc-leds-gpio.
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called simatic-ipc-leds.
> +
> +config LEDS_SIEMENS_SIMATIC_IPC_APOLLOLAKE
> +	tristate "LED driver for Siemens Simatic IPCs based on Intel Apollo Lake GPIO"
> +	depends on LEDS_GPIO

And then it can be dropped here.

> +	depends on PINCTRL_BROXTON

> +	depends on SIEMENS_SIMATIC_IPC

This should be "depends on LEDS_SIEMENS_SIMATIC_IPC" since it
actually uses symbol from that module.

> +	default LEDS_SIEMENS_SIMATIC_IPC
> +	help
> +	  This option enables support for the LEDs of several Industrial PCs
> +	  from Siemens based on Apollo Lake GPIO i.e. IPC127E.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called simatic-ipc-leds-gpio-apollolake.
> +
> +config LEDS_SIEMENS_SIMATIC_IPC_F7188X
> +	tristate "LED driver for Siemens Simatic IPCs based on Nuvoton GPIO"
> +	depends on LEDS_GPIO

Idem.

> +	depends on GPIO_F7188X
> +	depends on SIEMENS_SIMATIC_IPC

Idem.

> +	default LEDS_SIEMENS_SIMATIC_IPC
> +	help
> +	  This option enables support for the LEDs of several Industrial PCs
> +	  from Siemens based on Nuvoton GPIO i.e. IPC227G.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called simatic-ipc-leds-gpio-f7188x.
> diff --git a/drivers/leds/simple/Makefile b/drivers/leds/simple/Makefile
> index ed9057f7b6da..e3e840cea275 100644
> --- a/drivers/leds/simple/Makefile
> +++ b/drivers/leds/simple/Makefile
> @@ -1,5 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC)	+= simatic-ipc-leds.o
> -obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC)  += simatic-ipc-leds-gpio-core.o
> -obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC)	+= simatic-ipc-leds-gpio-apollolake.o
> -obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC)	+= simatic-ipc-leds-gpio-f7188x.o
> +obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC)			+= simatic-ipc-leds.o
> +obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC_APOLLOLAKE)	+= simatic-ipc-leds-gpio-core.o simatic-ipc-leds-gpio-apollolake.o
> +obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC_F7188X)		+= simatic-ipc-leds-gpio-core.o simatic-ipc-leds-gpio-f7188x.o

Regards,

Hans