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

[Andrew Davis]

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