| Message ID | 20220207151305.126696-1-christian.gmeiner@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | arm: mach-k3: am6_init: Use CONFIG_TI_I2C_BOARD_DETECT | expand |
Hi Christian! On this day, February 7, 2022, thus sayeth Christian Gmeiner: > We only want to call bo_board_detect() if CONFIG_TI_I2C_BOARD_DETECT s/bo_board_detect/do_board_detect/ > is set. Same as done for am64. > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > --- > arch/arm/mach-k3/am6_init.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-k3/am6_init.c b/arch/arm/mach-k3/am6_init.c > index ffb7aaded2..8a6b1de764 100644 > --- a/arch/arm/mach-k3/am6_init.c > +++ b/arch/arm/mach-k3/am6_init.c > @@ -251,7 +251,8 @@ void board_init_f(ulong dummy) > k3_sysfw_print_ver(); > > /* Perform EEPROM-based board detection */ > - do_board_detect(); > + if (IS_ENABLED(CONFIG_TI_I2C_BOARD_DETECT)) > + do_board_detect(); > Unless I'm mistaken the AM65 and AM64 use different do_board_detect()s. This one being defined inside board/ti/am65x/evm.c At least that's why I think I get this nasty error when I try to build with CONFIG_TI_I2C_BOARD_DETECT turned off and your patch applied. ard/ti/am65x/evm.c: In function ‘do_board_detect’: board/ti/am65x/evm.c:136:35: error: ‘CONFIG_EEPROM_BUS_ADDRESS’ undeclared (first use in this function) 136 | ret = ti_i2c_eeprom_am6_get_base(CONFIG_EEPROM_BUS_ADDRESS, | ^~~~~~~~~~~~~~~~~~~~~~~~~ board/ti/am65x/evm.c:136:35: note: each undeclared identifier is reported only once for each function it appears in board/ti/am65x/evm.c:137:7: error: ‘CONFIG_EEPROM_CHIP_ADDRESS’ undeclared (first use in this function) 137 | CONFIG_EEPROM_CHIP_ADDRESS); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ board/ti/am65x/evm.c: In function ‘setup_board_eeprom_env’: board/ti/am65x/evm.c:172:2: warning: implicit declaration of function ‘set_board_info_env_am6’ [-Wimplicit-function-declaration] 172 | set_board_info_env_am6(name); | ^~~~~~~~~~~~~~~~~~~~~~ board/ti/am65x/evm.c: In function ‘probe_daughtercards’: board/ti/am65x/evm.c:283:31: error: ‘CONFIG_EEPROM_BUS_ADDRESS’ undeclared (first use in this function) 283 | ret = ti_i2c_eeprom_am6_get(CONFIG_EEPROM_BUS_ADDRESS, i2c_addr, | ^~~~~~~~~~~~~~~~~~~~~~~~~ board/ti/am65x/evm.c: In function ‘board_late_init’: board/ti/am65x/evm.c:369:2: warning: implicit declaration of function ‘board_ti_am6_set_ethaddr’ [-Wimplicit-function-declaration] 369 | board_ti_am6_set_ethaddr(1, ep->mac_addr_cnt); | ^~~~~~~~~~~~~~~~~~~~~~~~ I will say I just turned CONFIG_TI_I2C_BOARD_DETECT off and chose the default options defconfig offered me. The chances are high I goofed on something. Thanks for the patch! ~Bryan
Hi Bryan. > > On this day, February 7, 2022, thus sayeth Christian Gmeiner: > > We only want to call bo_board_detect() if CONFIG_TI_I2C_BOARD_DETECT > > s/bo_board_detect/do_board_detect/ Upps.. > > > is set. Same as done for am64. > > > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > > --- > > arch/arm/mach-k3/am6_init.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/mach-k3/am6_init.c b/arch/arm/mach-k3/am6_init.c > > index ffb7aaded2..8a6b1de764 100644 > > --- a/arch/arm/mach-k3/am6_init.c > > +++ b/arch/arm/mach-k3/am6_init.c > > @@ -251,7 +251,8 @@ void board_init_f(ulong dummy) > > k3_sysfw_print_ver(); > > > > /* Perform EEPROM-based board detection */ > > - do_board_detect(); > > + if (IS_ENABLED(CONFIG_TI_I2C_BOARD_DETECT)) > > + do_board_detect(); > > > > Unless I'm mistaken the AM65 and AM64 use different do_board_detect()s. They use the same mechanism (read a i2c eeprom and interpret these values) > This one being defined inside board/ti/am65x/evm.c > > At least that's why I think I get this nasty error when I try to build > with CONFIG_TI_I2C_BOARD_DETECT turned off and your patch applied. > config TARGET_AM654_A53_EVM and TARGET_AM654_R5_EVM both imply TI_I2C_BOARD_DETECT so manually disabling CONFIG_TI_I2C_BOARD_DETECT will hurt. > > ard/ti/am65x/evm.c: In function ‘do_board_detect’: > board/ti/am65x/evm.c:136:35: error: ‘CONFIG_EEPROM_BUS_ADDRESS’ undeclared (first use in this function) > 136 | ret = ti_i2c_eeprom_am6_get_base(CONFIG_EEPROM_BUS_ADDRESS, > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > board/ti/am65x/evm.c:136:35: note: each undeclared identifier is reported only once for each function it appears in > board/ti/am65x/evm.c:137:7: error: ‘CONFIG_EEPROM_CHIP_ADDRESS’ undeclared (first use in this function) > 137 | CONFIG_EEPROM_CHIP_ADDRESS); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > board/ti/am65x/evm.c: In function ‘setup_board_eeprom_env’: > board/ti/am65x/evm.c:172:2: warning: implicit declaration of function ‘set_board_info_env_am6’ [-Wimplicit-function-declaration] > 172 | set_board_info_env_am6(name); > | ^~~~~~~~~~~~~~~~~~~~~~ > board/ti/am65x/evm.c: In function ‘probe_daughtercards’: > board/ti/am65x/evm.c:283:31: error: ‘CONFIG_EEPROM_BUS_ADDRESS’ undeclared (first use in this function) > 283 | ret = ti_i2c_eeprom_am6_get(CONFIG_EEPROM_BUS_ADDRESS, i2c_addr, > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > board/ti/am65x/evm.c: In function ‘board_late_init’: > board/ti/am65x/evm.c:369:2: warning: implicit declaration of function ‘board_ti_am6_set_ethaddr’ [-Wimplicit-function-declaration] > 369 | board_ti_am6_set_ethaddr(1, ep->mac_addr_cnt); > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > Okay.. that was expected as CONFIG_EEPROM_BUS_ADDRESS depends on CONFIG_TI_I2C_BOARD_DETECT see board/ti/common/Kconfig > I will say I just turned CONFIG_TI_I2C_BOARD_DETECT off and chose the > default options defconfig offered me. The chances are high I goofed on > something. > Okay.. so you have not used something like am65x_evm_a53_defconfig? I am working on a custom am65 based board where I do not have an i2c eeprom and do not want to use TI's board detection thing. AFAICT am642 does it the same way and there are no problems.
On this day, February 14, 2022, thus sayeth Christian Gmeiner: > Hi Bryan. > > > > > On this day, February 7, 2022, thus sayeth Christian Gmeiner: > > > We only want to call bo_board_detect() if CONFIG_TI_I2C_BOARD_DETECT > > > > s/bo_board_detect/do_board_detect/ > > Upps.. > > > > > > is set. Same as done for am64. > > > > > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > > > --- > > > arch/arm/mach-k3/am6_init.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/arm/mach-k3/am6_init.c b/arch/arm/mach-k3/am6_init.c > > > index ffb7aaded2..8a6b1de764 100644 > > > --- a/arch/arm/mach-k3/am6_init.c > > > +++ b/arch/arm/mach-k3/am6_init.c > > > @@ -251,7 +251,8 @@ void board_init_f(ulong dummy) > > > k3_sysfw_print_ver(); > > > > > > /* Perform EEPROM-based board detection */ > > > - do_board_detect(); > > > + if (IS_ENABLED(CONFIG_TI_I2C_BOARD_DETECT)) > > > + do_board_detect(); > > > > > > > Unless I'm mistaken the AM65 and AM64 use different do_board_detect()s. > > They use the same mechanism (read a i2c eeprom and interpret these values) > > > This one being defined inside board/ti/am65x/evm.c > > > > At least that's why I think I get this nasty error when I try to build > > with CONFIG_TI_I2C_BOARD_DETECT turned off and your patch applied. > > > > config TARGET_AM654_A53_EVM and TARGET_AM654_R5_EVM both imply > TI_I2C_BOARD_DETECT so > manually disabling CONFIG_TI_I2C_BOARD_DETECT will hurt. > > > > > ard/ti/am65x/evm.c: In function ‘do_board_detect’: > > board/ti/am65x/evm.c:136:35: error: ‘CONFIG_EEPROM_BUS_ADDRESS’ undeclared (first use in this function) > > 136 | ret = ti_i2c_eeprom_am6_get_base(CONFIG_EEPROM_BUS_ADDRESS, > > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > > board/ti/am65x/evm.c:136:35: note: each undeclared identifier is reported only once for each function it appears in > > board/ti/am65x/evm.c:137:7: error: ‘CONFIG_EEPROM_CHIP_ADDRESS’ undeclared (first use in this function) > > 137 | CONFIG_EEPROM_CHIP_ADDRESS); > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > > board/ti/am65x/evm.c: In function ‘setup_board_eeprom_env’: > > board/ti/am65x/evm.c:172:2: warning: implicit declaration of function ‘set_board_info_env_am6’ [-Wimplicit-function-declaration] > > 172 | set_board_info_env_am6(name); > > | ^~~~~~~~~~~~~~~~~~~~~~ > > board/ti/am65x/evm.c: In function ‘probe_daughtercards’: > > board/ti/am65x/evm.c:283:31: error: ‘CONFIG_EEPROM_BUS_ADDRESS’ undeclared (first use in this function) > > 283 | ret = ti_i2c_eeprom_am6_get(CONFIG_EEPROM_BUS_ADDRESS, i2c_addr, > > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > > board/ti/am65x/evm.c: In function ‘board_late_init’: > > board/ti/am65x/evm.c:369:2: warning: implicit declaration of function ‘board_ti_am6_set_ethaddr’ [-Wimplicit-function-declaration] > > 369 | board_ti_am6_set_ethaddr(1, ep->mac_addr_cnt); > > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > Okay.. that was expected as CONFIG_EEPROM_BUS_ADDRESS depends on > CONFIG_TI_I2C_BOARD_DETECT > see board/ti/common/Kconfig > > > I will say I just turned CONFIG_TI_I2C_BOARD_DETECT off and chose the > > default options defconfig offered me. The chances are high I goofed on > > something. > > > > Okay.. so you have not used something like am65x_evm_a53_defconfig? > > I am working on a custom am65 based board where I do not have an i2c > eeprom and do not want to use TI's > board detection thing. > > AFAICT am642 does it the same way and there are no problems. > I'm currently building the .config like so: $ make ARCH=arm CROSS_COMPILE= ... am65x_evm_a53_defconfig $ cp .config .config.default $ make ARCH=arm CROSS_COMPILE= ... menuconfig ARM architecture -> Support for Board detection for TI platforms Disabling CONFIG_TI_I2C_BOARD_DETECT and producing this diff $ diff .config.default .config 180,182c180 < CONFIG_TI_I2C_BOARD_DETECT=y < CONFIG_EEPROM_BUS_ADDRESS=0 < CONFIG_EEPROM_CHIP_ADDRESS=0x50 --- > # CONFIG_TI_I2C_BOARD_DETECT is not set My thinking is this do_board_detect() is not protected with an #ifdef like the one in board/ti/am64x/evm.c which is causing the disappearance of CONFIG_EEPROM_BUS_ADDRESS and CONFIG_EEPROM_CHIP_ADDRESS to break my build. I haven't look much further, but it seems like we need to do more to stop the compiler from worrying about dead code. ~Bryan
Hi Bryan. Am Mo., 14. Feb. 2022 um 23:25 Uhr schrieb Bryan Brattlof <bb@ti.com>: > > On this day, February 14, 2022, thus sayeth Christian Gmeiner: > > Hi Bryan. > > > > > > > > On this day, February 7, 2022, thus sayeth Christian Gmeiner: > > > > We only want to call bo_board_detect() if CONFIG_TI_I2C_BOARD_DETECT > > > > > > s/bo_board_detect/do_board_detect/ > > > > Upps.. > > > > > > > > > is set. Same as done for am64. > > > > > > > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > > > > --- > > > > arch/arm/mach-k3/am6_init.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/arm/mach-k3/am6_init.c b/arch/arm/mach-k3/am6_init.c > > > > index ffb7aaded2..8a6b1de764 100644 > > > > --- a/arch/arm/mach-k3/am6_init.c > > > > +++ b/arch/arm/mach-k3/am6_init.c > > > > @@ -251,7 +251,8 @@ void board_init_f(ulong dummy) > > > > k3_sysfw_print_ver(); > > > > > > > > /* Perform EEPROM-based board detection */ > > > > - do_board_detect(); > > > > + if (IS_ENABLED(CONFIG_TI_I2C_BOARD_DETECT)) > > > > + do_board_detect(); > > > > > > > > > > Unless I'm mistaken the AM65 and AM64 use different do_board_detect()s. > > > > They use the same mechanism (read a i2c eeprom and interpret these values) > > > > > This one being defined inside board/ti/am65x/evm.c > > > > > > At least that's why I think I get this nasty error when I try to build > > > with CONFIG_TI_I2C_BOARD_DETECT turned off and your patch applied. > > > > > > > config TARGET_AM654_A53_EVM and TARGET_AM654_R5_EVM both imply > > TI_I2C_BOARD_DETECT so > > manually disabling CONFIG_TI_I2C_BOARD_DETECT will hurt. > > > > > > > > ard/ti/am65x/evm.c: In function ‘do_board_detect’: > > > board/ti/am65x/evm.c:136:35: error: ‘CONFIG_EEPROM_BUS_ADDRESS’ undeclared (first use in this function) > > > 136 | ret = ti_i2c_eeprom_am6_get_base(CONFIG_EEPROM_BUS_ADDRESS, > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > > > board/ti/am65x/evm.c:136:35: note: each undeclared identifier is reported only once for each function it appears in > > > board/ti/am65x/evm.c:137:7: error: ‘CONFIG_EEPROM_CHIP_ADDRESS’ undeclared (first use in this function) > > > 137 | CONFIG_EEPROM_CHIP_ADDRESS); > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > > > board/ti/am65x/evm.c: In function ‘setup_board_eeprom_env’: > > > board/ti/am65x/evm.c:172:2: warning: implicit declaration of function ‘set_board_info_env_am6’ [-Wimplicit-function-declaration] > > > 172 | set_board_info_env_am6(name); > > > | ^~~~~~~~~~~~~~~~~~~~~~ > > > board/ti/am65x/evm.c: In function ‘probe_daughtercards’: > > > board/ti/am65x/evm.c:283:31: error: ‘CONFIG_EEPROM_BUS_ADDRESS’ undeclared (first use in this function) > > > 283 | ret = ti_i2c_eeprom_am6_get(CONFIG_EEPROM_BUS_ADDRESS, i2c_addr, > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > > > board/ti/am65x/evm.c: In function ‘board_late_init’: > > > board/ti/am65x/evm.c:369:2: warning: implicit declaration of function ‘board_ti_am6_set_ethaddr’ [-Wimplicit-function-declaration] > > > 369 | board_ti_am6_set_ethaddr(1, ep->mac_addr_cnt); > > > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > > > > > Okay.. that was expected as CONFIG_EEPROM_BUS_ADDRESS depends on > > CONFIG_TI_I2C_BOARD_DETECT > > see board/ti/common/Kconfig > > > > > I will say I just turned CONFIG_TI_I2C_BOARD_DETECT off and chose the > > > default options defconfig offered me. The chances are high I goofed on > > > something. > > > > > > > Okay.. so you have not used something like am65x_evm_a53_defconfig? > > > > I am working on a custom am65 based board where I do not have an i2c > > eeprom and do not want to use TI's > > board detection thing. > > > > AFAICT am642 does it the same way and there are no problems. > > > > I'm currently building the .config like so: > > $ make ARCH=arm CROSS_COMPILE= ... am65x_evm_a53_defconfig > $ cp .config .config.default > $ make ARCH=arm CROSS_COMPILE= ... menuconfig > > ARM architecture -> Support for Board detection for TI platforms > > Disabling CONFIG_TI_I2C_BOARD_DETECT and producing this diff > Yes.. my patch did not take into account that somebody would turn off CONFIG_TI_I2C_BOARD_DETECT when it is implied by the board :) > $ diff .config.default .config > 180,182c180 > < CONFIG_TI_I2C_BOARD_DETECT=y > < CONFIG_EEPROM_BUS_ADDRESS=0 > < CONFIG_EEPROM_CHIP_ADDRESS=0x50 > --- > > # CONFIG_TI_I2C_BOARD_DETECT is not set > > My thinking is this do_board_detect() is not protected with an #ifdef > like the one in board/ti/am64x/evm.c which is causing the disappearance > of CONFIG_EEPROM_BUS_ADDRESS and CONFIG_EEPROM_CHIP_ADDRESS to break my > build. > > I haven't look much further, but it seems like we need to do more to > stop the compiler from worrying about dead code. > I have sent out a V2 of this patch which should address all issues.
diff --git a/arch/arm/mach-k3/am6_init.c b/arch/arm/mach-k3/am6_init.c index ffb7aaded2..8a6b1de764 100644 --- a/arch/arm/mach-k3/am6_init.c +++ b/arch/arm/mach-k3/am6_init.c @@ -251,7 +251,8 @@ void board_init_f(ulong dummy) k3_sysfw_print_ver(); /* Perform EEPROM-based board detection */ - do_board_detect(); + if (IS_ENABLED(CONFIG_TI_I2C_BOARD_DETECT)) + do_board_detect(); #if defined(CONFIG_CPU_V7R) && defined(CONFIG_K3_AVS0) ret = uclass_get_device_by_driver(UCLASS_MISC, DM_DRIVER_GET(k3_avs),
We only want to call bo_board_detect() if CONFIG_TI_I2C_BOARD_DETECT is set. Same as done for am64. Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> --- arch/arm/mach-k3/am6_init.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)