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

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

[Andy Shevchenko]

Update header inclusions to follow IWYU (Include What You Use)
principle.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 sound/soc/mediatek/mt8365/mt8365-mt6357.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/sound/soc/mediatek/mt8365/mt8365-mt6357.c b/sound/soc/mediatek/mt8365/mt8365-mt6357.c
index d398e83ea052..9f28d6bf0323 100644
--- a/sound/soc/mediatek/mt8365/mt8365-mt6357.c
+++ b/sound/soc/mediatek/mt8365/mt8365-mt6357.c
@@ -6,12 +6,19 @@
  * Authors: Nicolas Belin <nbelin@baylibre.com>
  */
 
+#include <linux/array_size.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
 #include <sound/soc.h>
 #include <sound/pcm_params.h>
+
 #include "mt8365-afe-common.h"
-#include <linux/pinctrl/consumer.h>
 #include "../common/mtk-soc-card.h"
 #include "../common/mtk-soundcard-driver.h"
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

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

[Andy Shevchenko]

On Thu, Oct 31, 2024 at 12:27:25PM +0200, Andy Shevchenko wrote:
> Update header inclusions to follow IWYU (Include What You Use)
> principle.

Hmm... I think we are waiting for somebody to Ack / review this change?

-- 
With Best Regards,
Andy Shevchenko

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

On Mon, Nov 04, 2024 at 09:06:26PM +0200, Andy Shevchenko wrote:
> On Thu, Oct 31, 2024 at 12:27:25PM +0200, Andy Shevchenko wrote:
> > Update header inclusions to follow IWYU (Include What You Use)
> > principle.

> Hmm... I think we are waiting for somebody to Ack / review this change?

Yes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Alexandre Mergnat]

Hello Andy.

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.


On 04/11/2024 22:12, Mark Brown wrote:
> On Mon, Nov 04, 2024 at 09:06:26PM +0200, Andy Shevchenko wrote:
>> On Thu, Oct 31, 2024 at 12:27:25PM +0200, Andy Shevchenko wrote:
>>> Update header inclusions to follow IWYU (Include What You Use)
>>> principle.
> 
>> Hmm... I think we are waiting for somebody to Ack / review this change?
> 
> Yes.

-- 
Regards,
Alexandre

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

[Andy Shevchenko]

On Mon, Nov 25, 2024 at 12:32:13PM +0100, Alexandre Mergnat wrote:
> Hello Andy.
> 
> 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.

> On 04/11/2024 22:12, Mark Brown wrote:
> > On Mon, Nov 04, 2024 at 09:06:26PM +0200, Andy Shevchenko wrote:
> > > On Thu, Oct 31, 2024 at 12:27:25PM +0200, Andy Shevchenko wrote:
> > > > Update header inclusions to follow IWYU (Include What You Use)
> > > > principle.
> > 
> > > Hmm... I think we are waiting for somebody to Ack / review this change?
> > 
> > Yes.

-- 
With Best Regards,
Andy Shevchenko

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

[Alexandre Mergnat]

Hi Andy

On 25/11/2024 12:50, Andy Shevchenko wrote:
> On Mon, Nov 25, 2024 at 12:32:13PM +0100, Alexandre Mergnat wrote:
>> Hello Andy.
>>
>> 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 ?
Maybe my test isn't consistent because it isn't possible to clear all proxy ?

If that's the case, consider my review-by.
I've validated some include manually. Are you using a script to parse the file and raise all 
necessary "linux/*" include ?

-- 
Regards,
Alexandre

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

[Alexandre Mergnat]

On 26/11/2024 11:29, 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:
>>> Hello Andy.
>>>
>>> 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 ?
> Maybe my test isn't consistent because it isn't possible to clear all proxy ?
> 
> If that's the case, consider my review-by.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

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

-- 
Regards,
Alexandre

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

[Andy Shevchenko]

On Tue, Nov 26, 2024 at 11:37:26AM +0100, Alexandre Mergnat wrote:
> On 26/11/2024 11:29, 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 ?
> > Maybe my test isn't consistent because it isn't possible to clear all proxy ?
> > 
> > If that's the case, consider my review-by.

> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

Thank you!
Mark, would be enough to apply the change?

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

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

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

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

[Mark Brown]

On Thu, 31 Oct 2024 12:27:25 +0200, Andy Shevchenko wrote:
> Update header inclusions to follow IWYU (Include What You Use)
> principle.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoc: mediatek: mt8365: Don't use "proxy" headers
      commit: 509544d1b6dbffe7bc1849593ff217f965ec0419

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark