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;
Date:   Wed, 27 Jul 2022 20:50:06 +0200
From:   Jean Delvare <jdelvare@suse.de>
To:     Mark Brown <broonie@kernel.org>
Cc:     linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
        Liam Girdwood <lgirdwood@gmail.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Chenglin Xu <chenglin.xu@mediatek.com>,
        Sean Wang <sean.wang@mediatek.com>
Subject: Re: [PATCH RFC] regulator: mt6380: Fix unused array warning
Message-ID: <20220727205006.0ff42274@endymion.delvare>
In-Reply-To: <YuEug/OV9H0DKnmZ@sirena.org.uk>
References: <20220727132637.76d6073f@endymion.delvare>
        <YuEpGTD/DbC1T9Z0@sirena.org.uk>
        <20220727140809.0951b72d@endymion.delvare>
        <YuEug/OV9H0DKnmZ@sirena.org.uk>
Organization: SUSE Linux
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Mark,

On Wed, 27 Jul 2022 13:24:35 +0100, Mark Brown wrote:
> On Wed, Jul 27, 2022 at 02:08:09PM +0200, Jean Delvare wrote:
> > On Wed, 27 Jul 2022 13:01:29 +0100, Mark Brown wrote:  
> 
> > > It's helpful to keep the build coverage high.  
> 
> > OF does not depend on the architecture, anyone can enable it. So I
> > can't see any problem of coverage with making drivers depend on OF.  
> 
> It still reduces a barrier to entry.

Can't see how that would be a goal. By allowing randomconfig to pick
configuration option combinations which do not make sense, we are not
increasing coverage. That's quite the opposite. We are limited by the
overall power of the build farm, so every test build of such a useless
combination is a waste of resources. We'd rather use that machine time
to test a configuration option combination which real people would be
using, as these are the ones we care about.

> It's also that it's easier to just
> prefer the conditional compilation pattern rather than either check to
> see which cases is needed or have people copy an example that doesn't
> use it when they should.

In my experience, there's always a very easy way to silent a warning,
but in most cases, that easy way is wrong because it hides the warning
instead of fixing its cause.

Very much to the point, the build farm pointed us to a combination of
options which triggers warnings which developers had apparently never
noticed before, which is a hint that maybe this combination is not
something we should support in the first place. We can of course silent
all such warnings with __maybe_unused, but that should not be our first
choice (else we might as well stop building with -Wunused).

Not only that, but in this case it might also be that we have kernel
code that can be removed because it isn't needed. Not much, but that
would still be a gain, methinks.

I also don't think that one goal of the kernel code is to be easy to
copy and paste without understanding what you are doing.

Anyway, this is your subsystem, so the decision is yours. My patch
removes the warning, if you are happy with it, feel free to apply it.

Thanks,
-- 
Jean Delvare
SUSE L3 Support