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 trini@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 afd@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.