[PATCH] arm: mach-k3: am6_init: Use CONFIG_TI_I2C_BOARD_DETECT

[Christian Gmeiner]

Hi Bryan.

Am Mo., 14. Feb. 2022 um 23:25 Uhr schrieb Bryan Brattlof <bb at 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 at 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.

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy