| Message ID | 20230725164059.593455-1-afd@ti.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/2] configs: keystone2: Unwind KERNEL_MTD_PARTS definition | expand |
On Tue, Jul 25, 2023 at 11:40:58AM -0500, Andrew Davis wrote: > This is more complex than it needs to be and makes converting these > boards over to plain text env files more difficult. Simplify this > environment variable scheme. > > Signed-off-by: Andrew Davis <afd@ti.com> > --- > include/configs/k2e_evm.h | 5 +---- > include/configs/k2g_evm.h | 4 +--- > include/configs/k2hk_evm.h | 5 +---- > include/configs/k2l_evm.h | 5 +---- > include/configs/ti_armv7_keystone2.h | 6 +----- > include/environment/ti/spi.h | 14 -------------- > 6 files changed, 5 insertions(+), 34 deletions(-) > delete mode 100644 include/environment/ti/spi.h So this unwinding isn't quite enough as the defconfigs also set CONFIG_MTDPARTS_DEFAULT and that's the best way.
On 7/25/23 12:18 PM, Tom Rini wrote: > On Tue, Jul 25, 2023 at 11:40:58AM -0500, Andrew Davis wrote: > >> This is more complex than it needs to be and makes converting these >> boards over to plain text env files more difficult. Simplify this >> environment variable scheme. >> >> Signed-off-by: Andrew Davis <afd@ti.com> >> --- >> include/configs/k2e_evm.h | 5 +---- >> include/configs/k2g_evm.h | 4 +--- >> include/configs/k2hk_evm.h | 5 +---- >> include/configs/k2l_evm.h | 5 +---- >> include/configs/ti_armv7_keystone2.h | 6 +----- >> include/environment/ti/spi.h | 14 -------------- >> 6 files changed, 5 insertions(+), 34 deletions(-) >> delete mode 100644 include/environment/ti/spi.h > > So this unwinding isn't quite enough as the defconfigs also set > CONFIG_MTDPARTS_DEFAULT and that's the best way. > The one in CONFIG_MTDPARTS_DEFAULT is different from the one defined in k2*_evm.h, so switching would change the env. This patch was attempting to make no changes. Maybe a better question is do we need these anymore? They don't seem to do anything anymore and these partitions are board specific, so couldn't be set universally in defconfig anyway. Plus They are instead already defined (correctly too) in the DTS files for each board. Should we just drop mtdparts=? Andrew
On Tue, Jul 25, 2023 at 12:47:10PM -0500, Andrew Davis wrote: > On 7/25/23 12:18 PM, Tom Rini wrote: > > On Tue, Jul 25, 2023 at 11:40:58AM -0500, Andrew Davis wrote: > > > > > This is more complex than it needs to be and makes converting these > > > boards over to plain text env files more difficult. Simplify this > > > environment variable scheme. > > > > > > Signed-off-by: Andrew Davis <afd@ti.com> > > > --- > > > include/configs/k2e_evm.h | 5 +---- > > > include/configs/k2g_evm.h | 4 +--- > > > include/configs/k2hk_evm.h | 5 +---- > > > include/configs/k2l_evm.h | 5 +---- > > > include/configs/ti_armv7_keystone2.h | 6 +----- > > > include/environment/ti/spi.h | 14 -------------- > > > 6 files changed, 5 insertions(+), 34 deletions(-) > > > delete mode 100644 include/environment/ti/spi.h > > > > So this unwinding isn't quite enough as the defconfigs also set > > CONFIG_MTDPARTS_DEFAULT and that's the best way. > > The one in CONFIG_MTDPARTS_DEFAULT is different from the one defined in k2*_evm.h, > so switching would change the env. This patch was attempting to make no changes. > > Maybe a better question is do we need these anymore? They don't seem to do anything > anymore and these partitions are board specific, so couldn't be set universally in > defconfig anyway. Plus They are instead already defined (correctly too) in the DTS > files for each board. Should we just drop mtdparts=? So, CONFIG_MTDPARTS_DEFAULT (and related) largely just results in setting mtdparts= in the default environment. So yes, if they're correct in the DTS files the best thing to do is remove the CONFIG options and remove the environment settings and then there won't be confusion about which one is the source of truth.
diff --git a/include/configs/k2e_evm.h b/include/configs/k2e_evm.h index 630dfdc5b7f..46e33233650 100644 --- a/include/configs/k2e_evm.h +++ b/include/configs/k2e_evm.h @@ -9,8 +9,6 @@ #ifndef __CONFIG_K2E_EVM_H #define __CONFIG_K2E_EVM_H -#include <environment/ti/spi.h> - /* U-Boot general configuration */ #define ENV_KS2_BOARD_SETTINGS \ DEFAULT_FW_INITRAMFS_BOOT_ENV \ @@ -19,6 +17,7 @@ "boot=ubi\0" \ "args_ubi=setenv bootargs ${bootargs} rootfstype=ubifs " \ "root=ubi0:rootfs rootflags=sync rw ubi.mtd=ubifs,2048\0" \ + "mtdparts=spi0.0:1m(u-boot-spl)ro,-(misc)\0" \ "name_fdt=keystone-k2e-evm.dtb\0" \ "name_mon=skern-k2e.bin\0" \ "name_ubi=k2e-evm-ubifs.ubi\0" \ @@ -27,8 +26,6 @@ #include <configs/ti_armv7_keystone2.h> -#define SPI_MTD_PARTS KEYSTONE_SPI0_MTD_PARTS - /* Network */ #define CFG_KSNET_CPSW_NUM_PORTS 9 diff --git a/include/configs/k2g_evm.h b/include/configs/k2g_evm.h index d0634a99f49..744f1539a37 100644 --- a/include/configs/k2g_evm.h +++ b/include/configs/k2g_evm.h @@ -10,7 +10,6 @@ #define __CONFIG_K2G_EVM_H #include <environment/ti/mmc.h> -#include <environment/ti/spi.h> /* U-Boot general configuration */ #define ENV_KS2_BOARD_SETTINGS \ @@ -37,6 +36,7 @@ "else if test $name_fdt = undefined; then " \ "echo WARNING: Could not determine device tree to use;"\ "fi;fi;fi;fi; setenv fdtfile ${name_fdt}\0" \ + "mtdparts=spi1.0:1m(u-boot-spl)ro,-(misc)\0" \ "name_mon=skern-k2g.bin\0" \ "name_ubi=k2g-evm-ubifs.ubi\0" \ "name_uboot=u-boot-spi-k2g-evm.gph\0" \ @@ -54,8 +54,6 @@ #define CFG_KSNET_CPSW_NUM_PORTS 2 #define PHY_ANEG_TIMEOUT 10000 /* PHY needs longer aneg time */ -#define SPI_MTD_PARTS KEYSTONE_SPI1_MTD_PARTS - #include <configs/ti_armv7_keystone2.h> #endif /* __CONFIG_K2G_EVM_H */ diff --git a/include/configs/k2hk_evm.h b/include/configs/k2hk_evm.h index ed54be839ff..c688c4d738b 100644 --- a/include/configs/k2hk_evm.h +++ b/include/configs/k2hk_evm.h @@ -9,8 +9,6 @@ #ifndef __CONFIG_K2HK_EVM_H #define __CONFIG_K2HK_EVM_H -#include <environment/ti/spi.h> - /* U-Boot general configuration */ #define ENV_KS2_BOARD_SETTINGS \ DEFAULT_FW_INITRAMFS_BOOT_ENV \ @@ -19,6 +17,7 @@ "boot=ubi\0" \ "args_ubi=setenv bootargs ${bootargs} rootfstype=ubifs " \ "root=ubi0:rootfs rootflags=sync rw ubi.mtd=ubifs,2048\0" \ + "mtdparts=spi0.0:1m(u-boot-spl)ro,-(misc)\0" \ "name_fdt=keystone-k2hk-evm.dtb\0" \ "name_mon=skern-k2hk.bin\0" \ "name_ubi=k2hk-evm-ubifs.ubi\0" \ @@ -27,8 +26,6 @@ #include <configs/ti_armv7_keystone2.h> -#define SPI_MTD_PARTS KEYSTONE_SPI0_MTD_PARTS - /* Network */ #define CFG_KSNET_CPSW_NUM_PORTS 5 diff --git a/include/configs/k2l_evm.h b/include/configs/k2l_evm.h index 69d42eaf9f1..c0af0bc2fe5 100644 --- a/include/configs/k2l_evm.h +++ b/include/configs/k2l_evm.h @@ -9,8 +9,6 @@ #ifndef __CONFIG_K2L_EVM_H #define __CONFIG_K2L_EVM_H -#include <environment/ti/spi.h> - /* U-Boot general configuration */ #define ENV_KS2_BOARD_SETTINGS \ DEFAULT_FW_INITRAMFS_BOOT_ENV \ @@ -19,6 +17,7 @@ "boot=ubi\0" \ "args_ubi=setenv bootargs ${bootargs} rootfstype=ubifs " \ "root=ubi0:rootfs rootflags=sync rw ubi.mtd=ubifs,4096\0" \ + "mtdparts=spi0.0:1m(u-boot-spl)ro,-(misc)\0" \ "name_fdt=keystone-k2l-evm.dtb\0" \ "name_mon=skern-k2l.bin\0" \ "name_ubi=k2l-evm-ubifs.ubi\0" \ @@ -27,8 +26,6 @@ #include <configs/ti_armv7_keystone2.h> -#define SPI_MTD_PARTS KEYSTONE_SPI0_MTD_PARTS - /* Network */ #define CFG_KSNET_CPSW_NUM_PORTS 5 diff --git a/include/configs/ti_armv7_keystone2.h b/include/configs/ti_armv7_keystone2.h index a47f0902a26..06aa02a6d40 100644 --- a/include/configs/ti_armv7_keystone2.h +++ b/include/configs/ti_armv7_keystone2.h @@ -78,10 +78,6 @@ /* EDMA3 */ -#define KERNEL_MTD_PARTS \ - "mtdparts=" \ - SPI_MTD_PARTS - #define DEFAULT_FW_INITRAMFS_BOOT_ENV \ "name_fw_rd=k2-fw-initrd.cpio.gz\0" \ "set_rd_spec=setenv rd_spec ${rdaddr}:${filesize}\0" \ @@ -149,7 +145,7 @@ "burn_uboot_nand=nand erase 0 0x100000; " \ "nand write ${loadaddr} 0 ${filesize}\0" \ "args_all=setenv bootargs console=ttyS0,115200n8 rootwait " \ - KERNEL_MTD_PARTS \ + "mtdparts=${mtdparts}\0" \ "args_net=setenv bootargs ${bootargs} rootfstype=nfs " \ "root=/dev/nfs rw nfsroot=${serverip}:${nfs_root}," \ "${nfs_options} ip=dhcp\0" \ diff --git a/include/environment/ti/spi.h b/include/environment/ti/spi.h deleted file mode 100644 index 1681dc8635e..00000000000 --- a/include/environment/ti/spi.h +++ /dev/null @@ -1,14 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0+ */ -/* - * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com - * - * Environment variable definitions for SPI on TI boards. - */ - -#ifndef __TI_SPI_H -#define __TI_SPI_H - -#define KEYSTONE_SPI0_MTD_PARTS "spi0.0:1m(u-boot-spl)ro,-(misc);\0" -#define KEYSTONE_SPI1_MTD_PARTS "spi1.0:1m(u-boot-spl)ro,-(misc);\0" - -#endif
This is more complex than it needs to be and makes converting these boards over to plain text env files more difficult. Simplify this environment variable scheme. Signed-off-by: Andrew Davis <afd@ti.com> --- include/configs/k2e_evm.h | 5 +---- include/configs/k2g_evm.h | 4 +--- include/configs/k2hk_evm.h | 5 +---- include/configs/k2l_evm.h | 5 +---- include/configs/ti_armv7_keystone2.h | 6 +----- include/environment/ti/spi.h | 14 -------------- 6 files changed, 5 insertions(+), 34 deletions(-) delete mode 100644 include/environment/ti/spi.h