All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
To: "sjg@chromium.org" <sjg@chromium.org>,
	"u-boot@lists.denx.de" <u-boot@lists.denx.de>
Subject: REGRESSION: [PATCH 5/9] toradex: Use checkboard() instead of show_board_info()
Date: Wed, 24 Jan 2024 17:16:00 +0000	[thread overview]
Message-ID: <e40ed93bd8f371ec56b8fc451dcb458f3ce6dcba.camel@toradex.com> (raw)
In-Reply-To: <20231113030001.240020-6-sjg@chromium.org>

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@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	other threads:[~2024-01-24 17:16 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-13  2:58 [PATCH 0/9] sysinfo: Expand sysinfo with some more banner information Simon Glass
2023-11-13  2:58 ` [PATCH 1/9] board: Move show_board_info() comment to header file Simon Glass
2023-11-16 15:53   ` Tom Rini
2023-11-13  2:58 ` [PATCH 2/9] meson: Use checkboard() instead of show_board_info() Simon Glass
2023-11-13  8:01   ` Neil Armstrong
2023-11-13  2:58 ` [PATCH 3/9] turris: " Simon Glass
2023-11-13  2:58 ` [PATCH 4/9] solidrun: " Simon Glass
2023-11-13  2:58 ` [PATCH 5/9] toradex: " Simon Glass
2024-01-24 17:16   ` Marcel Ziswiler [this message]
2024-02-07 15:23     ` REGRESSION: " Marcel Ziswiler
2024-02-07 15:46       ` Tom Rini
2023-11-13  2:58 ` [PATCH 6/9] udoo: " Simon Glass
2023-11-13  2:58 ` [PATCH 7/9] Revert "generic-board: make show_board_info a weak function" Simon Glass
2023-11-16 15:53   ` Tom Rini
2023-11-13  2:58 ` [PATCH 8/9] sysinfo: Allow displaying more info on startup Simon Glass
2023-11-13  2:58 ` [PATCH 9/9] x86: coreboot: Add a sysinfo driver Simon Glass
2023-11-28 21:13 ` [PATCH 0/9] sysinfo: Expand sysinfo with some more banner information Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e40ed93bd8f371ec56b8fc451dcb458f3ce6dcba.camel@toradex.com \
    --to=marcel.ziswiler@toradex.com \
    --cc=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.