| Message ID | 20230301170215.23382-4-henning.schild@siemens.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | leds: simatic-ipc-leds-gpio: split up | expand |
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(-)