On Mon, Nov 20, 2023 at 07:16:03PM -0700, Simon Glass wrote: > Hi Tom, > > On Mon, 20 Nov 2023 at 08:26, Tom Rini <tr...@konsulko.com> wrote: > > > > On Mon, Nov 20, 2023 at 08:49:32AM -0600, Andrew Davis wrote: > > > On 11/17/23 4:40 PM, Tom Rini wrote: > > > > On Fri, Nov 17, 2023 at 04:38:28PM -0600, Andrew Davis wrote: > > > > > > > > > This is a hacky way to have this file included in all source files > > > > > that > > > > > include common.h, instead just include from the files that need it. > > > > > > > > > > Signed-off-by: Andrew Davis <a...@ti.com> > > > > > --- > > > > > drivers/memory/ti-aemif.c | 1 + > > > > > drivers/soc/ti/keystone_serdes.c | 1 + > > > > > include/configs/ti_armv7_keystone2.h | 1 - > > > > > 3 files changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c > > > > > index c4bc88c1510..41325eb0f94 100644 > > > > > --- a/drivers/memory/ti-aemif.c > > > > > +++ b/drivers/memory/ti-aemif.c > > > > > @@ -7,6 +7,7 @@ > > > > > */ > > > > > #include <common.h> > > > > > +#include <asm/arch/hardware.h> > > > > > #include <asm/ti-common/ti-aemif.h> > > > > > #define AEMIF_WAITCYCLE_CONFIG (KS2_AEMIF_CNTRL_BASE + 0x4) > > > > > diff --git a/drivers/soc/ti/keystone_serdes.c > > > > > b/drivers/soc/ti/keystone_serdes.c > > > > > index 2ece1a8f647..0e1bf8ff39d 100644 > > > > > --- a/drivers/soc/ti/keystone_serdes.c > > > > > +++ b/drivers/soc/ti/keystone_serdes.c > > > > > @@ -8,6 +8,7 @@ > > > > > #include <errno.h> > > > > > #include <common.h> > > > > > +#include <asm/io.h> > > > > > #include <asm/ti-common/keystone_serdes.h> > > > > > #include <linux/bitops.h> > > > > > > > > Is there going to be a follow-up to remove common.h from these files > > > > then? Thanks. > > > > > > Yes I can take a look at that next. > > > > > > Wasn't there some automation around this? Dropping headers from common.h > > > one at a time until it disappears IIRC. > > > > Simon has posted a python tool he wrote that partially did this, but I > > wasn't quite happy with the results at the time. It might work out > > better now, or on top of the series I posted to remove it from include/ > > My tool inserts #includes based on the symbols referenced in a file. > For example, reference to strcpy() or memcmp() would result in the > script adding '#include <linux/string.h>' to the top of the file.
But that's not how we typically work, nor is it how the kernel works. In
a semi-current kernel tree:
$ git grep -l '<linux/string.h>' *.c | wc -l
2177
$ git grep -l '<linux/string.h>' *.h | wc -l
146
$ git grep -l 'memset(' *.c | wc -l
6957
So most C files that call memset(..) get <linux/string.h> indirectly.
Which is why I'm trying to clean up the header files first.
> I still believe this is a reasonable approach. My original series to
> handle this was posted at [1] over 3 years ago. It foundered in part
> due to the number of files it touched, but I still believe the
> principle of 'include what you use' makes sense in U-Boot.
>
> Other problems are that in a few cases it results in some ugly code,
> or misordering of includes
>
> On the plus side, it gets us past this problem. We shouldn't make
> perfect an enemy of good and I am disappointed that we are still
> trying to get this over the line.
What makes this hard to get over the line is the less clear examples.
Dealing with <config.h> and CFG symbols gets pretty odd. And the failure
in https://source.denx.de/u-boot/u-boot/-/jobs/739662 was just really
odd.
--
Tom
signature.asc
Description: PGP signature
