Re: [PATCH v1 1/1] ASoc: mediatek: mt8365: Don't use "proxy" headers

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, Nov 26, 2024 at 11:29:28AM +0100, Alexandre Mergnat wrote:
> On 25/11/2024 12:50, Andy Shevchenko wrote:
> > On Mon, Nov 25, 2024 at 12:32:13PM +0100, Alexandre Mergnat wrote:

...

> > > Actually, after test it, "linux/of_gpio.h" isn't needed at all anymore.
> > > 
> > > That mean all added include in this patch aren't required.
> > Do you mean the driver doesn't not use types from types.h or dev_*() macros
> > from dev_printk.h? I don't believe this, sorry.
> 
> > Basically what you are trying to say is "let's move of_gpio.h implicit
> > includes to become something else's problem". It's not what this patch
> > intended to do.
> 
> I'm just saying that I've test a build/boot with "linux/of_gpio.h" removed and without all
> include added in you patch. My understand is "linux/of_gpio.h" act as proxy
> for the includes added in your patch, my first idea was "if I remove it, build should fail cause
> of lack of other includes". I can understand these missing includes are mandatory, that
> probably means there is another proxy header ?

Exactly. The compilation can't be done without respective headers to be included,
so one way or another they are somehow got included, meaning that other(s) header(s)
are "proxying" them.

The idea is to have less of the dependency hell (you may find this term used
many times in the relation to Linux Kernel project), so the (leaf)
modules/drivers should follow the IWYU principle (Include What You Use). With
that in place it's much better to clean up the headers in order to decrease
the hellish dependency tree.

> Maybe my test isn't consistent because it isn't possible to clear all proxy ?

Theoretically it's possible and even practically, just requires a lot of time.
(Or a lot of resources to make a flag day or so, but seems not many are interested
 to take the job)

> If that's the case, consider my review-by.

Thank you!

> I've validated some include manually. Are you using a script to parse the
> file and raise all necessary "linux/*" include ?

Nope, it comes just from my experience in the project, but I love to see
such a tool to exist!

-- 
With Best Regards,
Andy Shevchenko