[PATCH 1/3] configs: keystone2: Remove unused SPL_MALLOC_F_SIZE and KEYSTONE_SPL_STACK_SIZE
These are leftover definitions. While here cleanup some leftover comments.
Signed-off-by: Andrew Davis afd@ti.com --- include/configs/ti_armv7_keystone2.h | 22 ---------------------- 1 file changed, 22 deletions(-)
diff --git a/include/configs/ti_armv7_keystone2.h b/include/configs/ti_armv7_keystone2.h index 72c04d8a994..637e9e4369e 100644 --- a/include/configs/ti_armv7_keystone2.h +++ b/include/configs/ti_armv7_keystone2.h @@ -9,23 +9,10 @@ #ifndef __CONFIG_KS2_EVM_H #define __CONFIG_KS2_EVM_H
-/* U-Boot Build Configuration */ - -/* SoC Configuration */ - /* Memory Configuration */ #define CFG_SYS_LPAE_SDRAM_BASE 0x800000000 #define CFG_MAX_RAM_BANK_SIZE (2 << 30) /* 2GB */
-#ifdef CONFIG_SYS_MALLOC_F_LEN -#define SPL_MALLOC_F_SIZE CONFIG_SYS_MALLOC_F_LEN -#else -#define SPL_MALLOC_F_SIZE 0 -#endif - -/* SPL SPI Loader Configuration */ -#define KEYSTONE_SPL_STACK_SIZE (8 * 1024) - /* SRAM scratch space entries */ #define SRAM_SCRATCH_SPACE_ADDR 0xc0c23fc
@@ -53,8 +40,6 @@ #define CFG_KSNET_SERDES_SGMII2_BASE KS2_SGMII_SERDES2_BASE #define CFG_KSNET_SERDES_LANES_PER_SGMII KS2_LANES_PER_SGMII_SERDES
-/* EEPROM definitions */ - /* NAND Configuration */ #define CFG_SYS_NAND_MASK_CLE 0x4000 #define CFG_SYS_NAND_MASK_ALE 0x2000 @@ -63,13 +48,6 @@ #define CFG_SYS_NAND_LARGEPAGE #define CFG_SYS_NAND_BASE_LIST { 0x30000000, }
- - -/* U-Boot general configuration */ - -/* EDMA3 */ - - /* Now for the remaining common defines */ #include <configs/ti_armv7_common.h>
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>
diff --git a/include/configs/ti_armv7_keystone2.h b/include/configs/ti_armv7_keystone2.h index 637e9e4369e..b36207cb5d1 100644 --- a/include/configs/ti_armv7_keystone2.h +++ b/include/configs/ti_armv7_keystone2.h @@ -52,7 +52,6 @@ #include <configs/ti_armv7_common.h>
/* we may include files below only after all above definitions */ -#include <asm/arch/hardware.h> #include <asm/arch/clock.h> #ifndef CONFIG_SOC_K2G #define CFG_SYS_HZ_CLOCK ks_clk_get_rate(KS2_CLK1_6)
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.
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.
Andrew
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/
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.
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.
Regards, Simon
[1] https://patchwork.ozlabs.org/project/uboot/list/?series=193754&state=*
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.
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
Applied to u-boot/next, thanks!
Signed-off-by: Andrew Davis afd@ti.com --- arch/arm/mach-keystone/clock.c | 1 - arch/arm/mach-keystone/cmd_clock.c | 2 +- arch/arm/mach-keystone/cmd_mon.c | 1 - arch/arm/mach-keystone/cmd_poweroff.c | 1 - arch/arm/mach-keystone/ddr3.c | 2 +- arch/arm/mach-keystone/ddr3_spd.c | 2 +- arch/arm/mach-keystone/include/mach/mux-k2g.h | 1 - arch/arm/mach-keystone/init.c | 1 - arch/arm/mach-keystone/keystone.c | 1 - arch/arm/mach-keystone/mon.c | 1 - arch/arm/mach-keystone/msmc.c | 1 - arch/arm/mach-keystone/psc.c | 1 - 12 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-keystone/clock.c b/arch/arm/mach-keystone/clock.c index 0c59515d2eb..4f193794efb 100644 --- a/arch/arm/mach-keystone/clock.c +++ b/arch/arm/mach-keystone/clock.c @@ -6,7 +6,6 @@ * Texas Instruments Incorporated, <www.ti.com> */
-#include <common.h> #include <asm/arch/clock.h> #include <asm/arch/clock_defs.h> #include <linux/bitops.h> diff --git a/arch/arm/mach-keystone/cmd_clock.c b/arch/arm/mach-keystone/cmd_clock.c index 72dc394df5f..e9ecc05953a 100644 --- a/arch/arm/mach-keystone/cmd_clock.c +++ b/arch/arm/mach-keystone/cmd_clock.c @@ -6,7 +6,7 @@ * Texas Instruments Incorporated, <www.ti.com> */
-#include <common.h> +#include <vsprintf.h> #include <command.h> #include <asm/arch/hardware.h> #include <asm/arch/clock.h> diff --git a/arch/arm/mach-keystone/cmd_mon.c b/arch/arm/mach-keystone/cmd_mon.c index dc97bac8550..c6e7e2c3097 100644 --- a/arch/arm/mach-keystone/cmd_mon.c +++ b/arch/arm/mach-keystone/cmd_mon.c @@ -6,7 +6,6 @@ * Texas Instruments Incorporated, <www.ti.com> */
-#include <common.h> #include <command.h> #include <image.h> #include <mach/mon.h> diff --git a/arch/arm/mach-keystone/cmd_poweroff.c b/arch/arm/mach-keystone/cmd_poweroff.c index f0ad9173b96..0ad31ef4e28 100644 --- a/arch/arm/mach-keystone/cmd_poweroff.c +++ b/arch/arm/mach-keystone/cmd_poweroff.c @@ -6,7 +6,6 @@ * Texas Instruments Incorporated, <www.ti.com> */
-#include <common.h> #include <command.h> #include <asm/arch/mon.h> #include <asm/arch/psc_defs.h> diff --git a/arch/arm/mach-keystone/ddr3.c b/arch/arm/mach-keystone/ddr3.c index ea7d0b903cf..ca0fb702d54 100644 --- a/arch/arm/mach-keystone/ddr3.c +++ b/arch/arm/mach-keystone/ddr3.c @@ -9,7 +9,7 @@ #include <cpu_func.h> #include <env.h> #include <asm/io.h> -#include <common.h> +#include <vsprintf.h> #include <asm/arch/msmc.h> #include <asm/arch/ddr3.h> #include <asm/arch/psc_defs.h> diff --git a/arch/arm/mach-keystone/ddr3_spd.c b/arch/arm/mach-keystone/ddr3_spd.c index 6f7f8ab7b40..d4ff442175b 100644 --- a/arch/arm/mach-keystone/ddr3_spd.c +++ b/arch/arm/mach-keystone/ddr3_spd.c @@ -5,8 +5,8 @@ * (C) Copyright 2015-2016 Texas Instruments Incorporated, <www.ti.com> */
-#include <common.h> #include <log.h> +#include <string.h>
#include <i2c.h> #include <ddr_spd.h> diff --git a/arch/arm/mach-keystone/include/mach/mux-k2g.h b/arch/arm/mach-keystone/include/mach/mux-k2g.h index 67d47f81721..dfb5ad43506 100644 --- a/arch/arm/mach-keystone/include/mach/mux-k2g.h +++ b/arch/arm/mach-keystone/include/mach/mux-k2g.h @@ -9,7 +9,6 @@ #ifndef __ASM_ARCH_MUX_K2G_H #define __ASM_ARCH_MUX_K2G_H
-#include <common.h> #include <asm/io.h>
#define K2G_PADCFG_REG (KS2_DEVICE_STATE_CTRL_BASE + 0x1000) diff --git a/arch/arm/mach-keystone/init.c b/arch/arm/mach-keystone/init.c index 1954e69e9f0..39afaaa63d6 100644 --- a/arch/arm/mach-keystone/init.c +++ b/arch/arm/mach-keystone/init.c @@ -6,7 +6,6 @@ * Texas Instruments Incorporated, <www.ti.com> */
-#include <common.h> #include <cpu_func.h> #include <init.h> #include <ns16550.h> diff --git a/arch/arm/mach-keystone/keystone.c b/arch/arm/mach-keystone/keystone.c index efaabca5a7e..8846df3af48 100644 --- a/arch/arm/mach-keystone/keystone.c +++ b/arch/arm/mach-keystone/keystone.c @@ -6,7 +6,6 @@ * Texas Instruments Incorporated, <www.ti.com> */
-#include <common.h> #include <env.h> #include <init.h> #include <asm/io.h> diff --git a/arch/arm/mach-keystone/mon.c b/arch/arm/mach-keystone/mon.c index e91b0d68f4d..b945e19ec77 100644 --- a/arch/arm/mach-keystone/mon.c +++ b/arch/arm/mach-keystone/mon.c @@ -8,7 +8,6 @@ #include <hang.h> #include <image.h> #include <asm/unaligned.h> -#include <common.h> #include <command.h> #include <mach/mon.h> #include <spl.h> diff --git a/arch/arm/mach-keystone/msmc.c b/arch/arm/mach-keystone/msmc.c index f5cadfbf669..a20e0c98865 100644 --- a/arch/arm/mach-keystone/msmc.c +++ b/arch/arm/mach-keystone/msmc.c @@ -6,7 +6,6 @@ * Texas Instruments Incorporated, <www.ti.com> */
-#include <common.h> #include <asm/arch/msmc.h>
struct mpax { diff --git a/arch/arm/mach-keystone/psc.c b/arch/arm/mach-keystone/psc.c index 145aff8ac66..84d64f3bc40 100644 --- a/arch/arm/mach-keystone/psc.c +++ b/arch/arm/mach-keystone/psc.c @@ -6,7 +6,6 @@ * Texas Instruments Incorporated, <www.ti.com> */
-#include <common.h> #include <linux/delay.h> #include <linux/errno.h> #include <asm/io.h>
On Fri, 17 Nov 2023 at 15:38, Andrew Davis afd@ti.com wrote:
Signed-off-by: Andrew Davis afd@ti.com
arch/arm/mach-keystone/clock.c | 1 - arch/arm/mach-keystone/cmd_clock.c | 2 +- arch/arm/mach-keystone/cmd_mon.c | 1 - arch/arm/mach-keystone/cmd_poweroff.c | 1 - arch/arm/mach-keystone/ddr3.c | 2 +- arch/arm/mach-keystone/ddr3_spd.c | 2 +- arch/arm/mach-keystone/include/mach/mux-k2g.h | 1 - arch/arm/mach-keystone/init.c | 1 - arch/arm/mach-keystone/keystone.c | 1 - arch/arm/mach-keystone/mon.c | 1 - arch/arm/mach-keystone/msmc.c | 1 - arch/arm/mach-keystone/psc.c | 1 - 12 files changed, 3 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
missing commit message
On Fri, Nov 17, 2023 at 04:38:29PM -0600, Andrew Davis wrote:
Signed-off-by: Andrew Davis afd@ti.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!
On Fri, 17 Nov 2023 at 15:38, Andrew Davis afd@ti.com wrote:
These are leftover definitions. While here cleanup some leftover comments.
Signed-off-by: Andrew Davis afd@ti.com
include/configs/ti_armv7_keystone2.h | 22 ---------------------- 1 file changed, 22 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Fri, Nov 17, 2023 at 04:38:27PM -0600, Andrew Davis wrote:
These are leftover definitions. While here cleanup some leftover comments.
Signed-off-by: Andrew Davis afd@ti.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/next, thanks!
participants (3)
-
Andrew Davis -
Simon Glass -
Tom Rini