Re: [GIT PULL] Backlight for v6.1

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Hans de Goede <hdegoede@redhat.com>
Cc: Lee Jones <lee@kernel.org>, Mark Brown <broonie@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] Backlight for v6.1
Date: Sat, 8 Oct 2022 21:30:52 +0300	[thread overview]
Message-ID: <Y0HB3K8IRVhX5IvT@smile.fi.intel.com> (raw)
In-Reply-To: <CAHk-=wg=hh8xkPjiySnjAyR66AG64eyZ1Y9gHw+MCs8uuSZReA@mail.gmail.com>

On Fri, Oct 07, 2022 at 11:45:27AM -0700, Linus Torvalds wrote:
> On Fri, Oct 7, 2022 at 6:16 AM Lee Jones <lee@kernel.org> wrote:
> >
> > PR satisfying this dependency was submitted the following day:
> 
> .. you ignored the whole "another driver hit v6.0 without ever getting
> the dependency".
> 
> So this whole thing with "dead driver because the config option it
> depended on didn't even exist" wasn't some little temporary thing.
> It's literally there in a released kernel, which has a whole driver in
> it that cannot actually even be enabled.
> 
> And now that I *did* get the MFD update, I notice that the build
> coverage of *that* is pitiful too.
> 
> In particular, there was a silent semantic conflict in the Crystal
> Cove (intel PMIC) driver with the i2c changes. I noticed it because
> there were other things going on, and I went and looked.
> 
> But most notably I *didn't* notice it due to any build coverage,
> because the Kconfig for that thing seems designed to hide the driver.
> It does have
> 
>         depends on (X86 && ACPI) || COMPILE_TEST
> 
> so that it *looks* like it should get compile test coverage even
> outside of x86, but in reality,  even on x86 it's actually really hard
> to test, because it also has
> 
>         depends on I2C_DESIGNWARE_PLATFORM=y

The Intel PMICs are the beasts when we want to run the code on the real
hardware.  The above mentioned dependency is required due to ACPI wants
to poke some PMIC registers via OpRegion in the AML. And since it may
happen quite early at the boot time, we can't wait for I2C host controller
driver to be loaded later on. I don't remember all the details of what will
go bad for the user in such cases, perhaps Hans can clarify these bits.

> so if you do a regular "allmodconfig" build, that driver doesn't
> actually get any build testing at all, because while that platform is
> indeed enabled, it's only a module.
> 
> So I caught this particular issue, but I really think that code that
> cannot be enabled at all even for build testing - or code that is
> quite hard to enable either intentionally or by mistake - is a
> problem. And right now MFD was involved in both of the issues I've
> noticed this merge window.
> 
> That's not a great look.

-- 
With Best Regards,
Andy Shevchenko