Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b510b97d-f613-66a6-711f-2b6dde72f884@redhat.com>
Date:   Mon, 14 Aug 2023 10:48:06 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH] watchdog: simatic: add PCI dependency
Content-Language: en-US, nl
To:     "xingtong.wu" <xingtong_wu@163.com>,
        Arnd Bergmann <arnd@kernel.org>,
        Wim Van Sebroeck <wim@linux-watchdog.org>,
        Guenter Roeck <linux@roeck-us.net>
Cc:     Arnd Bergmann <arnd@arndb.de>, linux-watchdog@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        "Haeussler, Gerd" <gerd.haeussler.ext@siemens.com>,
        "Schaffner, Tobias" <tobias.schaffner@siemens.com>,
        "Wu, Xing Tong" <XingTong.Wu@siemens.com>
References: <20230814073924.1066390-1-arnd@kernel.org>
 <a6c2f193-ef7c-54a0-dfbb-13915be259a1@163.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <a6c2f193-ef7c-54a0-dfbb-13915be259a1@163.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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 <arnd@arndb.de>
>>
>> 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 <arnd@arndb.de>
>> ---
>>  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