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 <s...@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

Reply via email to