[PATCH] platform/x86/siemens: simatic-ipc: fix nonsensical condition

[PATCH] platform/x86/siemens: simatic-ipc: fix nonsensical condition

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The condition checking for a constant SIMATIC_IPC_DEVICE_BX_59A value
clearly makes no sense, as clang warns:

drivers/platform/x86/siemens/simatic-ipc.c:132:42: error: use of logical '||' with constant operand [-Werror,-Wconstant-logical-operand]
                if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
                                                       ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/platform/x86/siemens/simatic-ipc-batt.c:197:49: error: use of logical '||' with constant operand [-Werror,-Wconstant-logical-operand]
                if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
                                                              ^  ~~~~~~~~~~~~~~~~~~~~~~~~~

Most likely, this was meant to check ledmode to be one of the two values,
so change it to that.

Fixes: b8af77951941e ("platform/x86/siemens: simatic-ipc: add new models BX-56A/BX-59A")
Fixes: c56beff203754 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
 drivers/platform/x86/siemens/simatic-ipc.c      | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt.c b/drivers/platform/x86/siemens/simatic-ipc-batt.c
index d66b9969234bf..e6c12c52843ca 100644
--- a/drivers/platform/x86/siemens/simatic-ipc-batt.c
+++ b/drivers/platform/x86/siemens/simatic-ipc-batt.c
@@ -194,7 +194,8 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
 
 	if (table->table[2].key) {
 		flags = GPIOD_OUT_HIGH;
-		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
+		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A ||
+		    priv.devmode == SIMATIC_IPC_DEVICE_BX_59A)
 			flags = GPIOD_OUT_LOW;
 		priv.gpios[2] = devm_gpiod_get_index(dev, "CMOSBattery meter", 2, flags);
 		if (IS_ERR(priv.gpios[2])) {
diff --git a/drivers/platform/x86/siemens/simatic-ipc.c b/drivers/platform/x86/siemens/simatic-ipc.c
index 02c540cf40702..e11d28ffac604 100644
--- a/drivers/platform/x86/siemens/simatic-ipc.c
+++ b/drivers/platform/x86/siemens/simatic-ipc.c
@@ -129,7 +129,8 @@ static int register_platform_devices(u32 station_id)
 		pdevname = KBUILD_MODNAME "_leds";
 		if (ledmode == SIMATIC_IPC_DEVICE_127E)
 			pdevname = KBUILD_MODNAME "_leds_gpio_apollolake";
-		if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
+		if (ledmode == SIMATIC_IPC_DEVICE_227G ||
+		    ledmode == SIMATIC_IPC_DEVICE_BX_59A)
 			pdevname = KBUILD_MODNAME "_leds_gpio_f7188x";
 		if (ledmode == SIMATIC_IPC_DEVICE_BX_21A)
 			pdevname = KBUILD_MODNAME "_leds_gpio_elkhartlake";
-- 
2.39.2

Re: [PATCH] platform/x86/siemens: simatic-ipc: fix nonsensical condition

[Ilpo Järvinen]

On Fri, 11 Aug 2023, Arnd Bergmann wrote:

> From: Arnd Bergmann <arnd@arndb.de>
> 
> The condition checking for a constant SIMATIC_IPC_DEVICE_BX_59A value
> clearly makes no sense, as clang warns:
> 
> drivers/platform/x86/siemens/simatic-ipc.c:132:42: error: use of logical '||' with constant operand [-Werror,-Wconstant-logical-operand]
>                 if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
>                                                        ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/platform/x86/siemens/simatic-ipc-batt.c:197:49: error: use of logical '||' with constant operand [-Werror,-Wconstant-logical-operand]
>                 if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
>                                                               ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Most likely, this was meant to check ledmode to be one of the two values,
> so change it to that.
> 
> Fixes: b8af77951941e ("platform/x86/siemens: simatic-ipc: add new models BX-56A/BX-59A")
> Fixes: c56beff203754 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
>  drivers/platform/x86/siemens/simatic-ipc.c      | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt.c b/drivers/platform/x86/siemens/simatic-ipc-batt.c
> index d66b9969234bf..e6c12c52843ca 100644
> --- a/drivers/platform/x86/siemens/simatic-ipc-batt.c
> +++ b/drivers/platform/x86/siemens/simatic-ipc-batt.c
> @@ -194,7 +194,8 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
>  
>  	if (table->table[2].key) {
>  		flags = GPIOD_OUT_HIGH;
> -		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
> +		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A ||
> +		    priv.devmode == SIMATIC_IPC_DEVICE_BX_59A)
>  			flags = GPIOD_OUT_LOW;
>  		priv.gpios[2] = devm_gpiod_get_index(dev, "CMOSBattery meter", 2, flags);
>  		if (IS_ERR(priv.gpios[2])) {
> diff --git a/drivers/platform/x86/siemens/simatic-ipc.c b/drivers/platform/x86/siemens/simatic-ipc.c
> index 02c540cf40702..e11d28ffac604 100644
> --- a/drivers/platform/x86/siemens/simatic-ipc.c
> +++ b/drivers/platform/x86/siemens/simatic-ipc.c
> @@ -129,7 +129,8 @@ static int register_platform_devices(u32 station_id)
>  		pdevname = KBUILD_MODNAME "_leds";
>  		if (ledmode == SIMATIC_IPC_DEVICE_127E)
>  			pdevname = KBUILD_MODNAME "_leds_gpio_apollolake";
> -		if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
> +		if (ledmode == SIMATIC_IPC_DEVICE_227G ||
> +		    ledmode == SIMATIC_IPC_DEVICE_BX_59A)
>  			pdevname = KBUILD_MODNAME "_leds_gpio_f7188x";
>  		if (ledmode == SIMATIC_IPC_DEVICE_BX_21A)
>  			pdevname = KBUILD_MODNAME "_leds_gpio_elkhartlake";

Thank you for the patch but these are already fixed by commits:

7abf253afa5c ("platform/x86/siemens: simatic-ipc-batt: fix logical error for BX-59A")
b01c1e022f7f ("platform/x86/siemens: simatic-ipc: fix logical error for BX-59A")


-- 
 i.

Re: [PATCH] platform/x86/siemens: simatic-ipc: fix nonsensical condition

[Arnd Bergmann]

On Fri, Aug 11, 2023, at 15:17, Ilpo Järvinen wrote:
> On Fri, 11 Aug 2023, Arnd Bergmann wrote:
>
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> The condition checking for a constant SIMATIC_IPC_DEVICE_BX_59A value
>> clearly makes no sense, as clang warns:
>> 
>> drivers/platform/x86/siemens/simatic-ipc.c:132:42: error: use of logical '||' with constant operand [-Werror,-Wconstant-logical-operand]
>>                 if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
>>                                                        ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/platform/x86/siemens/simatic-ipc-batt.c:197:49: error: use of logical '||' with constant operand [-Werror,-Wconstant-logical-operand]
>>                 if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
>>                                                               ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
>> 
>> Most likely, this was meant to check ledmode to be one of the two values,
>> so change it to that.
>> 
>> Fixes: b8af77951941e ("platform/x86/siemens: simatic-ipc: add new models BX-56A/BX-59A")
>> Fixes: c56beff203754 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Ok, I see. I missed those as there is hasn't been a new linux-next in
a few days.

I suppose this one is also fixed then?

WARNING: unmet direct dependencies detected for P2SB
  Depends on [n]: PCI [=n] && X86 [=y]
  Selected by [m]:
  - SIEMENS_SIMATIC_IPC_WDT [=m] && WATCHDOG [=y] && SIEMENS_SIMATIC_IPC [=y]
drivers/platform/x86/p2sb.c:68:9: error: call to undeclared function 'pci_scan_single_device'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]

     Arnd

Re: [PATCH] platform/x86/siemens: simatic-ipc: fix nonsensical condition

[Hans de Goede]

Hi,

On 8/11/23 17:02, Arnd Bergmann wrote:
> On Fri, Aug 11, 2023, at 15:17, Ilpo Järvinen wrote:
>> On Fri, 11 Aug 2023, Arnd Bergmann wrote:
>>
>>> From: Arnd Bergmann <arnd@arndb.de>
>>>
>>> The condition checking for a constant SIMATIC_IPC_DEVICE_BX_59A value
>>> clearly makes no sense, as clang warns:
>>>
>>> drivers/platform/x86/siemens/simatic-ipc.c:132:42: error: use of logical '||' with constant operand [-Werror,-Wconstant-logical-operand]
>>>                 if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
>>>                                                        ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/platform/x86/siemens/simatic-ipc-batt.c:197:49: error: use of logical '||' with constant operand [-Werror,-Wconstant-logical-operand]
>>>                 if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
>>>                                                               ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Most likely, this was meant to check ledmode to be one of the two values,
>>> so change it to that.
>>>
>>> Fixes: b8af77951941e ("platform/x86/siemens: simatic-ipc: add new models BX-56A/BX-59A")
>>> Fixes: c56beff203754 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Ok, I see. I missed those as there is hasn't been a new linux-next in
> a few days.
> 
> I suppose this one is also fixed then?
> 
> WARNING: unmet direct dependencies detected for P2SB
>   Depends on [n]: PCI [=n] && X86 [=y]
>   Selected by [m]:
>   - SIEMENS_SIMATIC_IPC_WDT [=m] && WATCHDOG [=y] && SIEMENS_SIMATIC_IPC [=y]
> drivers/platform/x86/p2sb.c:68:9: error: call to undeclared function 'pci_scan_single_device'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]

No that one has not been fixed yet. This is the first time I've heard of this one. It seems to not have been caught by the LKP bot.

Regards,

Hans

Re: FW: [PATCH] platform/x86/siemens: simatic-ipc: fix nonsensical condition

[Arnd Bergmann]

On Mon, Aug 14, 2023, at 05:40, xingtong.wu wrote:
>>From: Arnd Bergmann <arnd@arndb.de> 
>
>>Sent: Friday, August 11, 2023 11:02 PM
>>
>>Ok, I see. I missed those as there is hasn't been a new linux-next in a few days.
>>
>>I suppose this one is also fixed then?
>>
>>WARNING: unmet direct dependencies detected for P2SB
>>  Depends on [n]: PCI [=n] && X86 [=y]
>>  Selected by [m]:
>>  - SIEMENS_SIMATIC_IPC_WDT [=m] && WATCHDOG [=y] && SIEMENS_SIMATIC_IPC [=y]
>>drivers/platform/x86/p2sb.c:68:9: error: call to undeclared function 'pci_scan_single_device'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>
> I’m pretty sure your .config file is error to compile the kernel,
> you must have changed the .config file manually, and there will be no
> possibility that option SIEMENS_SIMATIC_IPC is [y] while PCI is [n].
> reason:
> https://elixir.bootlin.com/linux/v6.5-rc6/source/drivers/platform/x86/Kconfig#L1079
>
> if the PCI option is [n], the "p2sb.c" should never compile pass.
>
> I suggest you make menuconfig to build kernel and check your .config
> carefully.

It was broken in linux-next by commit b72da71ce24b0 ("platform/x86:
simatic-ipc: drop PCI runtime depends and header"), I sent a fix now.

    Arnd