REGRESSION: [PATCH 5/9] toradex: Use checkboard() instead of show_board_info()

[Marcel Ziswiler]

Gentle ping on this. Any opinion on that matter much appreciated. Thanks!

On Wed, 2024-01-24 at 18:16 +0100, Marcel Ziswiler wrote:
> Hi Simon
> 
> Sorry, I missed this one, also due to a longer Xmas/New Year and later skiing vacation.
> 
> On Sun, 2023-11-12 at 19:58 -0700, Simon Glass wrote:
> > Boards can use a sysinfo driver if a particular model name is needed.
> 
> Okay, but so far we did print more than just a model name:
> 
> On Apalis/Colibri:
> 
> Model: Toradex 0028 Apalis iMX6Q 2GB IT V1.1D
> Serial#: 10867499
> 
> On Verdin:
> 
> Model: Toradex 0058 Verdin iMX8M Plus Quad 4GB WB IT V1.1A
> Serial#: 14772913
> Carrier: Toradex Dahlia V1.1A, Serial# 10870316
> 
> Optionally there would even be display adapters with potentially more model (serial) information.
> 
> Now with your change we get the following:
> 
> On Apalis/Colibri:
> 
> Model: Toradex Apalis iMX6Q/D Module on Apalis Evaluation Board
> Model: Toradex Apalis iMX6 Quad 2GB IT
> Model: Toradex 0028 Apalis iMX6Q 2GB IT V1.1D
> Serial#: 11211073
> 
> The first line gets printed from the information in the device tree, the second Line from the fall-back in
> our
> board file (which so far was only used for the case when we failed reading the ConfigBlock) and the third and
> fourth lines are the previous information. Ugly, but so far so good.
> 
> On Verdin:
> 
> Model: Toradex Verdin iMX8M Plus WB on Verdin Development Board
> 
> Here only the device tree information gets printed and the ConfigBlock is not even read at all which
> subsequently fails detecting the variant (e.g. Wi-Fi vs. non-Wi-Fi) and later Ethernet fails due to an
> invalid
> MAC address. This does not look good...
> 
> Anyway, I don't propose to just revert your work but instead looked into converting our previous
> show_board_info (now tdx_checkboard) to a proper sysinfo driver. The basics actually worked quite smoothly
> but
> we would need more than just SYSINFO_ID_BOARD_MODEL, just like you do with CONFIG_SYSINFO_EXTRA. Of course, I
> could just do a CONFIG_SYSINFO_TORADEX, with e.g. SYSINFO_ID_BOARD_SERIAL and optionally
> SYSINFO_ID_BOARD_CARRIER or something but maybe a more generic way of extending sysinfo would make more
> sense.
> 
> What do you think?
> 
> > Update this board to use checkboard() directly, rather than having a
> > weak function laid on top of a weak function.
> 
> Unfortunately, as mentioned above, this does not quite lead to any desired behaviour.
> 
> > Make all the checkboard() functions call the new tdx_checkboard() so
> > that the same information is displayed.
> 
> Not quite.
> 
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> > 
> >  board/toradex/apalis-imx8/apalis-imx8.c         | 2 +-
> >  board/toradex/apalis-tk1/apalis-tk1.c           | 2 +-
> >  board/toradex/apalis_imx6/apalis_imx6.c         | 3 ++-
> >  board/toradex/apalis_t30/apalis_t30.c           | 2 +-
> >  board/toradex/colibri-imx6ull/colibri-imx6ull.c | 2 +-
> >  board/toradex/colibri-imx8x/colibri-imx8x.c     | 2 +-
> >  board/toradex/colibri_imx6/colibri_imx6.c       | 3 ++-
> >  board/toradex/colibri_imx7/colibri_imx7.c       | 2 +-
> >  board/toradex/colibri_t20/colibri_t20.c         | 2 +-
> >  board/toradex/colibri_t30/colibri_t30.c         | 2 +-
> >  board/toradex/colibri_vf/colibri_vf.c           | 2 +-
> >  board/toradex/common/tdx-common.c               | 2 +-
> >  board/toradex/common/tdx-common.h               | 1 +
> >  13 files changed, 15 insertions(+), 12 deletions(-)
> 
> [snip]
> 
> 
> Cheers
> 
> Marcel