Re: [PATCH] arm: mach-k3: am6_init: Use CONFIG_TI_I2C_BOARD_DETECT
Hi Bryan. Am Mo., 14. Feb. 2022 um 23:25 Uhr schrieb Bryan Brattlof : > > 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 > > > > --- > > > > 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. -- greets -- Christian
Re: [PATCH] arm: mach-k3: am6_init: Use CONFIG_TI_I2C_BOARD_DETECT
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 > > > --- > > > 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
Re: [PATCH] arm: mach-k3: am6_init: Use CONFIG_TI_I2C_BOARD_DETECT
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 > --- > 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
Re: [PATCH] arm: mach-k3: am6_init: Use CONFIG_TI_I2C_BOARD_DETECT
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 > > --- > > 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. -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy