[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230302111427.47ba9f64@md1za8fc.ad001.siemens.net>
Date: Thu, 2 Mar 2023 11:14:27 +0100
From: Henning Schild <henning.schild@...mens.com>
To: Hans de Goede <hdegoede@...hat.com>
Cc: Pavel Machek <pavel@....cz>, Lee Jones <lee@...nel.org>,
Mark Gross <markgross@...nel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org,
platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v2 3/3] leds: simatic-ipc-leds-gpio: introduce more
Kconfig switches
Am Thu, 2 Mar 2023 10:02:51 +0100
schrieb Hans de Goede <hdegoede@...hat.com>:
> Hi,
>
> On 3/2/23 09:47, Henning Schild wrote:
> > Am Wed, 1 Mar 2023 19:04:01 +0100
> > schrieb Hans de Goede <hdegoede@...hat.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@...mens.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.
> >
> > No this one is now only for the port-IO based driver. The GPIO core
> > is built under the two new switches only.
>
> You are right, I thought this would enable building
> simatic-ipc-leds-gpio-core.o into its own .ko which would
> then be used by both other gpio LED drivers. But upon a closer
> look at the Makefile changes I see that is not the case.
>
> Note that with your current solution you are linking that into
> the kernel twice.
>
> As long this is build into modules that is fine. But if both are
> builtin I *think* you may get linker errors because of duplicate
> symbols?
>
> I believe this is why Andy asked to try a build with all 3 options
> set to Y.
Thanks. There are no linker errors so i think there is no need for yet
another CONFIG especially for the core. I would try to keep it simple
as the name of the directory suggests, so go with what we have.
Henning
>
> >>> 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.
> >
> > Same as above, the GPIO based drivers do not depend on the port-IO
> > based driver.
>
> Ack.
>
> Regards,
>
> Hans
>
>
>
>
>
>
> >>> + 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
> >>
> >
>
Powered by blists - more mailing lists