[PATCH net-next v1 1/1] nfc: mrvl: Don't use "proxy" headers

[PATCH net-next v1 1/1] nfc: mrvl: 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>
---
 drivers/nfc/nfcmrvl/uart.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/nfcmrvl/uart.c b/drivers/nfc/nfcmrvl/uart.c
index 956ae92f7573..2037cd6d4f4f 100644
--- a/drivers/nfc/nfcmrvl/uart.c
+++ b/drivers/nfc/nfcmrvl/uart.c
@@ -5,11 +5,16 @@
  * Copyright (C) 2015, Marvell International Ltd.
  */
 
-#include <linux/module.h>
 #include <linux/delay.h>
-#include <linux/of_gpio.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/printk.h>
+
 #include <net/nfc/nci.h>
 #include <net/nfc/nci_core.h>
+
 #include "nfcmrvl.h"
 
 static unsigned int hci_muxed;
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH net-next v1 1/1] nfc: mrvl: Don't use "proxy" headers

[Krzysztof Kozlowski]

On 01/11/2024 09:39, Andy Shevchenko wrote:
> Update header inclusions to follow IWYU (Include What You Use)
> principle.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/nfc/nfcmrvl/uart.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nfc/nfcmrvl/uart.c b/drivers/nfc/nfcmrvl/uart.c
> index 956ae92f7573..2037cd6d4f4f 100644
> --- a/drivers/nfc/nfcmrvl/uart.c
> +++ b/drivers/nfc/nfcmrvl/uart.c
> @@ -5,11 +5,16 @@
>   * Copyright (C) 2015, Marvell International Ltd.
>   */
>  
> -#include <linux/module.h>
>  #include <linux/delay.h>
> -#include <linux/of_gpio.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/printk.h>

Do we really include printk? It's almost everywhere and pulled by kernel.h.

I assume you checked rest of the nfcmrvl files for similar cleanups, so
anyway:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH net-next v1 1/1] nfc: mrvl: Don't use "proxy" headers

[Jakub Kicinski]

On Fri,  1 Nov 2024 10:39:10 +0200 Andy Shevchenko wrote:
> Subject: [PATCH net-next v1 1/1] nfc: mrvl: Don't use "proxy" headers

What is a "proxy" header? Guessing by the two patches you posted - 
are you trying to get rid of of_gpio.h?

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

I'm definitely on board with cleaning this up, but would prefer
to make sure we can validate new patches against introducing
regressions. Otherwise the stream of patches will be never ending.
What tooling do you use? Is it easy to integrate into a CI system?

Re: [PATCH net-next v1 1/1] nfc: mrvl: Don't use "proxy" headers

[Andy Shevchenko]

On Sat, Nov 02, 2024 at 09:00:04AM +0100, Krzysztof Kozlowski wrote:
> On 01/11/2024 09:39, Andy Shevchenko wrote:
> > Update header inclusions to follow IWYU (Include What You Use)
> > principle.

...

> > -#include <linux/module.h>
> >  #include <linux/delay.h>
> > -#include <linux/of_gpio.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/printk.h>
> 
> Do we really include printk?

Yes, we use it here.

> It's almost everywhere and pulled by kernel.h.

kernel.h shouldn't be considered at all, it's a mess which is on undergoing
cleaning up process.


> I assume you checked rest of the nfcmrvl files for similar cleanups, so
> anyway:
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thank you!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v1 1/1] nfc: mrvl: Don't use "proxy" headers

[Andy Shevchenko]

On Sun, Nov 03, 2024 at 08:17:40AM -0800, Jakub Kicinski wrote:
> On Fri,  1 Nov 2024 10:39:10 +0200 Andy Shevchenko wrote:
> > Subject: [PATCH net-next v1 1/1] nfc: mrvl: Don't use "proxy" headers
> 
> What is a "proxy" header?

The "proxy" header is any header that the code rely on to include other(s)
*not related* to that header headers. Hopefully, despite too many words
"header" you got the idea.

> Guessing by the two patches you posted - are you trying to get rid of
> of_gpio.h?

In this particular case:
1) I want to get rid of deprecated of_gpio.h;
2) but at the same time it *is* a "proxy" header in this case.

> > Update header inclusions to follow IWYU (Include What You Use)
> > principle.
> 
> I'm definitely on board with cleaning this up, but would prefer
> to make sure we can validate new patches against introducing
> regressions.

0-day LKP quite likely will notice any issues with this (it's quite good
at it), and so far it reported A-OK.

> Otherwise the stream of patches will be never ending.

I know, and this is pity. And there was an attempt to make clang based tool
for that, but no move forward as far as I can see. So, you are welcome to help
developing such a tool.

> What tooling do you use?

My brains and my expertise in Linux Kernel project for the dependency hell
we have in the headers.

> Is it easy to integrate into a CI system?

I don't think so.

Can we have this being applied meanwhile, please? It's a showstopper for
getting rid of of_gpio.h rather sooner than later.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v1 1/1] nfc: mrvl: Don't use "proxy" headers

[Krzysztof Kozlowski]

On 04/11/2024 09:07, Andy Shevchenko wrote:
> On Sat, Nov 02, 2024 at 09:00:04AM +0100, Krzysztof Kozlowski wrote:
>> On 01/11/2024 09:39, Andy Shevchenko wrote:
>>> Update header inclusions to follow IWYU (Include What You Use)
>>> principle.
> 
> ...
> 
>>> -#include <linux/module.h>
>>>  #include <linux/delay.h>
>>> -#include <linux/of_gpio.h>
>>> +#include <linux/device.h>
>>> +#include <linux/err.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/printk.h>
>>
>> Do we really include printk?
> 
> Yes, we use it here.

Yes, I know, I meant if this is actual practice and coding style,
regardless of IWYU.

Best regards,
Krzysztof

Re: [PATCH net-next v1 1/1] nfc: mrvl: Don't use "proxy" headers

[Jakub Kicinski]

On Mon, 4 Nov 2024 10:15:02 +0200 Andy Shevchenko wrote:
> So, you are welcome to help developing such a tool.

FTR I find saying such things to maintains very, very rude.

> Can we have this being applied meanwhile, please? It's a showstopper for
> getting rid of of_gpio.h rather sooner than later.

Doing this cleanup as part of deleting a deprecated header seems legit.
But you need to say that in the commit message.

Re: [PATCH net-next v1 1/1] nfc: mrvl: Don't use "proxy" headers

[Andy Shevchenko]

On Mon, Nov 04, 2024 at 06:29:41PM -0800, Jakub Kicinski wrote:
> On Mon, 4 Nov 2024 10:15:02 +0200 Andy Shevchenko wrote:
> > So, you are welcome to help developing such a tool.
> 
> FTR I find saying such things to maintains very, very rude.

Sorry, it wasn't meant to be that. Let me elaborate. The tool is really
what we need, but seems nobody is ready to invest time into its appearance.
It would great and appreciated to have a consolidated work towards that.

> > Can we have this being applied meanwhile, please? It's a showstopper for
> > getting rid of of_gpio.h rather sooner than later.
> 
> Doing this cleanup as part of deleting a deprecated header seems legit.
> But you need to say that in the commit message.

Thanks for review, I will address then in next version.

-- 
With Best Regards,
Andy Shevchenko