[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0549face-4b03-03e4-6da3-28f59fff94d1@redhat.com>
Date: Mon, 15 May 2023 13:24:18 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Henning Schild <henning.schild@...mens.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
Hi,
On 5/15/23 12:41, Henning Schild wrote:
> 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.
>>
>>>>> 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@...hat.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
>>>>
>>>
>>
>
Powered by blists - more mailing lists