From: Arnd Bergmann <[email protected]>
The simatic-ipc driver no longer depends on PCI, but its watchdog portion
still needs it, otherwise P2SB runs into a build failure:
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:121:3: error: call to undeclared function 'pci_bus_write_config_dword'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
pci_bus_write_config_dword(bus, devfn_p2sb, P2SBC, 0);
Add back the minimum dependendency to make it build in random configurations
again.
Fixes: b72da71ce24b0 ("platform/x86: simatic-ipc: drop PCI runtime depends and header")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/watchdog/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 04e9b40cf7d53..09452384221a4 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1680,7 +1680,7 @@ config NIC7018_WDT
config SIEMENS_SIMATIC_IPC_WDT
tristate "Siemens Simatic IPC Watchdog"
- depends on SIEMENS_SIMATIC_IPC
+ depends on SIEMENS_SIMATIC_IPC && PCI
default y
select WATCHDOG_CORE
select P2SB
--
2.39.2
Hi,
On 8/14/23 09:38, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The simatic-ipc driver no longer depends on PCI, but its watchdog portion
> still needs it, otherwise P2SB runs into a build failure:
>
> 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:121:3: error: call to undeclared function 'pci_bus_write_config_dword'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> pci_bus_write_config_dword(bus, devfn_p2sb, P2SBC, 0);
>
> Add back the minimum dependendency to make it build in random configurations
> again.
>
> Fixes: b72da71ce24b0 ("platform/x86: simatic-ipc: drop PCI runtime depends and header")
> Signed-off-by: Arnd Bergmann <[email protected]>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <[email protected]>
Wachdog maintainers the commit causing the fixed issue is part of my
platform-drivers-x86-simatic-ipc branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=platform-drivers-x86-simatic-ipc
And more importantly that branch already contains a conflicting commit to the SIEMENS_SIMATIC_IPC_WDT Kconfig entry, which was merged through that branch with
Guenter's ack.
So I'll also pick up this fix and merge it through that branch to avoid conflicts.
Regards,
Hans
> ---
> drivers/watchdog/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 04e9b40cf7d53..09452384221a4 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1680,7 +1680,7 @@ config NIC7018_WDT
>
> config SIEMENS_SIMATIC_IPC_WDT
> tristate "Siemens Simatic IPC Watchdog"
> - depends on SIEMENS_SIMATIC_IPC
> + depends on SIEMENS_SIMATIC_IPC && PCI
> default y
> select WATCHDOG_CORE
> select P2SB
Hi,
On 8/14/23 09:38, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The simatic-ipc driver no longer depends on PCI, but its watchdog portion
> still needs it, otherwise P2SB runs into a build failure:
>
> 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:121:3: error: call to undeclared function 'pci_bus_write_config_dword'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> pci_bus_write_config_dword(bus, devfn_p2sb, P2SBC, 0);
>
> Add back the minimum dependendency to make it build in random configurations
> again.
>
> Fixes: b72da71ce24b0 ("platform/x86: simatic-ipc: drop PCI runtime depends and header")
> Signed-off-by: Arnd Bergmann <[email protected]>
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> drivers/watchdog/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 04e9b40cf7d53..09452384221a4 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1680,7 +1680,7 @@ config NIC7018_WDT
>
> config SIEMENS_SIMATIC_IPC_WDT
> tristate "Siemens Simatic IPC Watchdog"
> - depends on SIEMENS_SIMATIC_IPC
> + depends on SIEMENS_SIMATIC_IPC && PCI
> default y
> select WATCHDOG_CORE
> select P2SB
Hi Xing Tong,
On 8/14/23 10:27, xingtong.wu wrote:
> Hi
>
> On 2023/8/14 15:38, Arnd Bergmann wrote:
>> From: Arnd Bergmann <[email protected]>
>>
>> The simatic-ipc driver no longer depends on PCI, but its watchdog portion
>> still needs it, otherwise P2SB runs into a build failure:
>
> If the simatic-ipc driver no longer depends on PCI, you can
> delete the dependency here:
> https://elixir.bootlin.com/linux/v6.5-rc6/source/drivers/platform/x86/Kconfig#L1079
That has already been done, the latest code of the
simatic-ipc support lives here:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=platform-drivers-x86-simatic-ipc
and there that depends on has been dropped, which is actually what is causing the issue this patch addresses.
>> 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:121:3: error: call to undeclared function 'pci_bus_write_config_dword'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>> pci_bus_write_config_dword(bus, devfn_p2sb, P2SBC, 0);
>>
>> Add back the minimum dependendency to make it build in random configurations
>> again.
>>
>> Fixes: b72da71ce24b0 ("platform/x86: simatic-ipc: drop PCI runtime depends and header")
>> Signed-off-by: Arnd Bergmann <[email protected]>
>> ---
>> drivers/watchdog/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 04e9b40cf7d53..09452384221a4 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -1680,7 +1680,7 @@ config NIC7018_WDT
>>
>> config SIEMENS_SIMATIC_IPC_WDT
>> tristate "Siemens Simatic IPC Watchdog"
>> - depends on SIEMENS_SIMATIC_IPC
>> + depends on SIEMENS_SIMATIC_IPC && PCI
>
> The SIEMENS_SIMATIC_IPC_WDT does not depends on PCI directly,
> it should depends on P2SB, then P2SB depends on PCI
P2SB is a helper library and as such it is selected by all code
which needs it and when selecting the Kconfig entry doing
the selecting Kconfig must either depend on or select all
the Kconfig dependencies of the Kconfig entry it is selecting.
>
>> default y
>> select WATCHDOG_CORE
>> select P2SB
>
> You can replace the "select" to "depends on" instead.
No that would mean that we start mixing select and depends on
for P2SB, where as currently all users use select.
Mixing select and depends on is know to cause dependency
cycle issues and thus should be avoided.
Arnd's patch is correct as is (and I have merged it
this morning).
Regards,
Hans
On Mon, Aug 14, 2023, at 10:27, xingtong.wu wrote:
>>
>> config SIEMENS_SIMATIC_IPC_WDT
>> tristate "Siemens Simatic IPC Watchdog"
>> - depends on SIEMENS_SIMATIC_IPC
>> + depends on SIEMENS_SIMATIC_IPC && PCI
>
> The SIEMENS_SIMATIC_IPC_WDT does not depends on PCI directly,
> it should depends on P2SB, then P2SB depends on PCI
>
>> default y
>> select WATCHDOG_CORE
>> select P2SB
>
> You can replace the "select" to "depends on" instead.
That would also work, but it's generally best not to mix
'depends on' and 'select' for the same symbol. Changing
all three 'select P2SB' to 'depends on P2SB', or removing
the prompt to turn P2SB itself into a hidden symbol is
probably a good follow-up patch, but I'd keep that separate
from the regression fix.
Arnd
Hi,
On 8/14/23 10:48, Arnd Bergmann wrote:
> On Mon, Aug 14, 2023, at 10:27, xingtong.wu wrote:
>
>>>
>>> config SIEMENS_SIMATIC_IPC_WDT
>>> tristate "Siemens Simatic IPC Watchdog"
>>> - depends on SIEMENS_SIMATIC_IPC
>>> + depends on SIEMENS_SIMATIC_IPC && PCI
>>
>> The SIEMENS_SIMATIC_IPC_WDT does not depends on PCI directly,
>> it should depends on P2SB, then P2SB depends on PCI
>>
>>> default y
>>> select WATCHDOG_CORE
>>> select P2SB
>>
>> You can replace the "select" to "depends on" instead.
>
> That would also work, but it's generally best not to mix
> 'depends on' and 'select' for the same symbol. Changing
> all three 'select P2SB' to 'depends on P2SB', or removing
> the prompt to turn P2SB itself into a hidden symbol is
> probably a good follow-up patch, but I'd keep that separate
> from the regression fix.
Right P2SB would benefit from being changed into a hidden
symbol.
Andy, any comments on making P2SB a hidden symbol ?
Regards,
Hans
>
> Arnd
>
Hi
On 2023/8/14 15:38, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The simatic-ipc driver no longer depends on PCI, but its watchdog portion
> still needs it, otherwise P2SB runs into a build failure:
If the simatic-ipc driver no longer depends on PCI, you can
delete the dependency here:
https://elixir.bootlin.com/linux/v6.5-rc6/source/drivers/platform/x86/Kconfig#L1079
>
> 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:121:3: error: call to undeclared function 'pci_bus_write_config_dword'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> pci_bus_write_config_dword(bus, devfn_p2sb, P2SBC, 0);
>
> Add back the minimum dependendency to make it build in random configurations
> again.
>
> Fixes: b72da71ce24b0 ("platform/x86: simatic-ipc: drop PCI runtime depends and header")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/watchdog/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 04e9b40cf7d53..09452384221a4 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1680,7 +1680,7 @@ config NIC7018_WDT
>
> config SIEMENS_SIMATIC_IPC_WDT
> tristate "Siemens Simatic IPC Watchdog"
> - depends on SIEMENS_SIMATIC_IPC
> + depends on SIEMENS_SIMATIC_IPC && PCI
The SIEMENS_SIMATIC_IPC_WDT does not depends on PCI directly,
it should depends on P2SB, then P2SB depends on PCI
> default y
> select WATCHDOG_CORE
> select P2SB
You can replace the "select" to "depends on" instead.
Reviewed-by: Xing Tong Wu <[email protected]>
--
Xing Tong
On Mon, Aug 14, 2023 at 09:38:50AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The simatic-ipc driver no longer depends on PCI, but its watchdog portion
> still needs it, otherwise P2SB runs into a build failure:
>
> 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:121:3: error: call to undeclared function 'pci_bus_write_config_dword'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> pci_bus_write_config_dword(bus, devfn_p2sb, P2SBC, 0);
>
> Add back the minimum dependendency to make it build in random configurations
> again.
>
> Fixes: b72da71ce24b0 ("platform/x86: simatic-ipc: drop PCI runtime depends and header")
> Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 04e9b40cf7d53..09452384221a4 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1680,7 +1680,7 @@ config NIC7018_WDT
>
> config SIEMENS_SIMATIC_IPC_WDT
> tristate "Siemens Simatic IPC Watchdog"
> - depends on SIEMENS_SIMATIC_IPC
> + depends on SIEMENS_SIMATIC_IPC && PCI
> default y
> select WATCHDOG_CORE
> select P2SB
> --
> 2.39.2
>
On Mon, Aug 14, 2023 at 11:51 AM Hans de Goede <[email protected]> wrote:
> On 8/14/23 10:48, Arnd Bergmann wrote:
...
> Right P2SB would benefit from being changed into a hidden
> symbol.
Yes, I dunno why it wasn't that way from day 1. Perhaps due to the
original contribution done by somebody else...
> Andy, any comments on making P2SB a hidden symbol ?
I'm fine with that.
--
With Best Regards,
Andy Shevchenko