Re: [PATCH] can: rockchip_canfd: Drop obsolete dependency on COMPILE_TEST

[Jean Delvare <jdelvare@suse.de>]

Hi Vincent,

On Tue, 22 Oct 2024 22:22:05 +0900, Vincent MAILHOL wrote:
> On Tue. 22 Oct. 2024 at 20:06, Jean Delvare <jdelvare@suse.de> wrote:
> > Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), OF
> > can be enabled on all architectures. Therefore depending on
> > COMPILE_TEST as an alternative is no longer needed.  
> 
> I understand the motivation behind this patch, but for me, as a
> maintainer, it becomes more work when I want to do a compile test.
> Before I would have needed to only select COMPILE_TEST but now, I
> would need to remember to also select OF for that driver to appear in
> the menuconfig.

Not sure what your typical process looks like, but if you need to
test-build drivers on a regular basis, why don't you have
CONFIG_COMPILE_TEST always enabled? Switching back and forth must be a
hassle.

About CONFIG_OF, well you probably want it enabled regardless. For one
thing, there are at least 940 drivers in the kernel tree which depend on
OF, so you are already missing a lot of test build coverage if you have
it disabled.

For another, if test-building a driver which would normally depend on
OF, but with OF disabled (as you do for rockchip_canfd at the moment,
as I understand it), then you may not be building the full driver. Part
of the code could be stripped out at build time because the compiler
notices that it is unreachable. Apparently this isn't the case of the
rockchip_canfd driver though.

> Well, I am not strongly against this simplification, but, wouldn't it
> be good to make COMPILE_TEST automatically select OF then? Looking at
> the description of COMPILE_TEST, I read:
> 
>  If you are a developer and want to build everything available, say Y here.

As a side note, I don't think this is a proper description of how to
use this option. If you "want to build everything available" then what
you want to do is "make allmodconfig", not manually enabling all
drivers in "make menuconfig". The purpose of manually selecting
COMPILE_TEST would presumably be to test-build a specific driver or set
of drivers.

> So having COMPILE_TEST automatically select OF looks sane to me as it
> goes in the direction of "building everything". If this makes sense, I
> can send a patch for this. Thoughts?

My initial reaction was that this would be an artificial dependency,
which seems wrong.

However, considering that the purpose of COMPILE_TEST is essentially to
let the user build drivers from other architectures, and that enabling
OF on an architecture which doesn't have it enabled by default
essentially lets you select a whole lot of drivers from other
architectures... I must admit that both options indeed share a
purpose.

But I still have a concern. Some drivers can be built with or without
OF (they support both OF and non-OF devices). If someone wants to
test-build such a driver on a different architecture (and therefore
must enable COMPILE_TEST) and wants to ensure that it builds fine with
and without OF, then if COMPILE_TEST selects OF as you suggested, they
would no longer be able to test the CONFIG_OF=n case.

So I'm afraid we can't actually do that.

-- 
Jean Delvare
SUSE L3 Support