| Message ID | 20230301170215.23382-4-henning.schild@siemens.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | leds: simatic-ipc-leds-gpio: split up | expand |
On Wed, Mar 01, 2023 at 06:02:15PM +0100, 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. ... > +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 Have you tried the configuration where all three are Y?
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
Am Wed, 1 Mar 2023 19:29:40 +0200 schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>: > On Wed, Mar 01, 2023 at 06:02:15PM +0100, 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. > > ... > > > +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 > > Have you tried the configuration where all three are Y? > I tried all m and will also try all Y. Why do you ask? Henning
Am Wed, 1 Mar 2023 19:04:01 +0100 schrieb Hans de Goede <hdegoede@redhat.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. No this one is now only for the port-IO based driver. The GPIO core is built under the two new switches only. > > > 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. Henning > > + 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 >
Hi, On 3/2/23 09:47, Henning Schild wrote: > Am Wed, 1 Mar 2023 19:04:01 +0100 > schrieb Hans de Goede <hdegoede@redhat.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. > > 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. >>> 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 >> >
Am Thu, 2 Mar 2023 10:02:51 +0100 schrieb Hans de Goede <hdegoede@redhat.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@redhat.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. > > > > 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 > >> > > >
Am Thu, 2 Mar 2023 10:02:51 +0100 schrieb Hans de Goede <hdegoede@redhat.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@redhat.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. > > > > 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. > > >>> 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. Hi Hans, > Ack. was that for that last patch or all three of them? I am preparing a v3 which will include ACKs, i will have to assume p3 only because it came in reply to that patch. regards, Henning > > 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 > >> > > >
Hi, On 5/15/23 12:41, Henning Schild wrote: > Am Thu, 2 Mar 2023 10:02:51 +0100 > schrieb Hans de Goede <hdegoede@redhat.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@redhat.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. >>> >>> 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. >> >>>>> 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. > > Hi Hans, > >> Ack. > > was that for that last patch or all three of them? I am preparing a v3 > which will include ACKs, i will have to assume p3 only because it came > in reply to that patch. You may add my: Acked-by: Hans de Goede <hdegoede@redhat.com> To all 3 patches in the series. 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 >>>> >>> >> >
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 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 + depends on PINCTRL_BROXTON + depends on SIEMENS_SIMATIC_IPC + 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 + depends on GPIO_F7188X + depends on SIEMENS_SIMATIC_IPC + 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
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(-)