linux-kernel - Re: [PATCH] mailbox, remoteproc: k3-m4+: fix compile testing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <585e5471-8a49-43cd-afba-055855be8e75@ti.com>
Date: Mon, 14 Oct 2024 12:26:22 -0500
From: Andrew Davis <afd@...com>
To: Nishanth Menon <nm@...com>, Arnd Bergmann <arnd@...db.de>
CC: Arnd Bergmann <arnd@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Martyn Welch
	<martyn.welch@...labora.com>,
        Hari Nagalla <hnagalla@...com>, Jassi Brar
	<jassisinghbrar@...il.com>,
        <linux-remoteproc@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mailbox, remoteproc: k3-m4+: fix compile testing

On 10/14/24 11:52 AM, Nishanth Menon wrote:
> On 15:43-20241014, Arnd Bergmann wrote:
>> On Mon, Oct 14, 2024, at 14:56, Andrew Davis wrote:
>>> On 10/7/24 8:23 AM, Arnd Bergmann wrote:
>>>>    config TI_K3_M4_REMOTEPROC
>>>>    	tristate "TI K3 M4 remoteproc support"
>>>> -	depends on ARCH_OMAP2PLUS || ARCH_K3
>>>> -	select MAILBOX
>>>> -	select OMAP2PLUS_MBOX
>>>> +	depends on ARCH_K3 || COMPILE_TEST
>>>> +	depends on TI_SCI_PROTOCOL || (COMPILE_TEST && TI_SCI_PROTOCOL=n)
>>>
>>> This line is odd. IMHO "COMPILE_TEST" should only be added to ARCH_*
>>> dependencies, as often only one ARCH can be selected which prevents
>>> compile testing drivers with various multiple architecture deps in
>>> one compile test.
>>
>> I generally agree, but the TI_SCI_PROTOCOL interface
>> definitions that were added in aa276781a64a ("firmware: Add basic
>> support for TI System Control Interface (TI-SCI) protocol")
>> appear to explicitly support the case of compile-testing.
>>
>> See also 13678f3feb30 ("reset: ti-sci: honor TI_SCI_PROTOCOL
>> setting when not COMPILE_TEST").
>>
>>> Normal dependencies, on the other hand, can simply be enabled if one
>>> wants to compile test its dependent drivers. In this case, TI_SCI_PROTOCOL
>>> cannot be enabled as it has a dependency up the chain that doesn't
>>> allow selecting when not on a TI platform. We can fix that as I posted
>>> here[0]. With that fix in, this line can be simply become:
>>>
>>> depends on TI_SCI_PROTOCOL
>>
>> That's certainly fine with me, but if we do this, I would suggest
>> also removing the stub functions from
>> include/linux/soc/ti/ti_sci_protocol.h, and the dependency in the
>> reset driver.
>>
>> Adding Nishanth Menon to Cc, to see if he has a preference.
>>
>>       Arnd
> 
> While I am OK to do the cleanups and make the drivers independent,
> I am also looking to make TISCI stuff independent and be a module
> itself. So, dropping the stubs will probably get in the way as we move
> ahead with those plans.
> 

Not sure how having the stubs would help with that. TI_SCI_PROTOCOL
can already be built as a module. The stubs are what cause this
issue here. When built as a module, the stubs are not active so any
driver that depends on this module that is built-in will cause a
link error.

Stubs only make sense for optional components, like GPIOLIB, where
the driver using that component can continue when it is not available.
For all drivers that depend on TI-SCI it is not optional. The dependent
driver *will* fail to probe and error out. These stubs do nothing, and
I'd like to just remove them.

Andrew