On Fri, 2022-07-01 at 05:29 +0000, Marcel Ziswiler wrote:
On Tue, 2022-06-21 at 13:49 +0200, Philippe Schenker wrote:
From: Philippe Schenker philippe.schenker@toradex.com
Add new i.MX 8M Mini SKU to ConfigBlock handling.
0068: Verdin iMX8M Mini Quad 2GB Wi-Fi / BT IT (No CAN)
This SKU is identical to 0055 but without CAN. Mention this in brackets so those modules can be distinguished.
Signed-off-by: Philippe Schenker philippe.schenker@toradex.com Reviewed-by: Francesco Dolcini francesco.dolcini@toradex.com
board/toradex/common/tdx-cfg-block.c | 13 ++++++++++++- board/toradex/common/tdx-cfg-block.h | 1 + 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/board/toradex/common/tdx-cfg-block.c b/board/toradex/common/tdx-cfg-block.c index 6c8cf4592d..fa54208ce9 100644 --- a/board/toradex/common/tdx-cfg-block.c +++ b/board/toradex/common/tdx-cfg-block.c @@ -145,6 +145,7 @@ const char * const toradex_modules[] = { [65] = "Verdin iMX8M Plus QuadLite 1GB IT", [66] = "Verdin iMX8M Plus Quad 8GB Wi-Fi / BT", [67] = "Apalis iMX8 QuadMax 8GB Wi-Fi / BT IT", + [68] = "Verdin iMX8M Mini Quad 2GB Wi-Fi / BT IT (No CAN)",
That one seems overly long meaning even without that new V0.0#26 notation it will be way longer than an 80 character line in my serial terminal. Anyway, I recommend calling it something like:
+ [68] = "Verdin iMX8M Mini Quad 2GB WB IT (- CAN)",
What do you guys think?
I think the dash (-) to substitute "No" only saves one char but introduces a whole lot of interpretation headroom. What I also dislike is that we do not stick to our form of naming the Wi-Fi.
The string that will be printed will be in that form:
Model: Toradex Verdin iMX8M Mini Quad 2GB Wi-Fi / BT IT (No CAN) V0.0#26, Serial# 01234567
You're right, these are 90 chars. Which is not ideal, I agree. As far as I understand we now use a mixed form of naming of what we use in our webshop and main website (e.g. Verdin iMX8M Plus Quad 4GB Wi-Fi / Bluetooth IT). We shorten Bluetooth but not Wi-Fi. This sort of name does only exist in config-block code.
Since we're planning to refactor this code anyway soonish it was brought up that it probably would be wise to use the SKU number along with the name to prevent mixing modules, since it seems we will get even more such slightly different modules in the future. With that we could completely get rid of "(No CAN)" in the example of the module added by this commit (0068). While at that I proposed to switch to defined Toradex names, either we could use the short-name used on stickers on the modules itself (e.g. Verdin iMX8MP Q 4GB WB IT) or the slightly longer ones (e.g. Verdin iMX8M Plus Quad 4GB WB IT).
Further we could think of taking the serial# to a new line to have a bit more headroom twoards the 80chars in the future.
With that I suggest that we stay consistent now within the code and revisit this topic once we refactor the code.
Philippe
}; const char * const toradex_carrier_boards[] = { @@ -361,6 +362,7 @@ static int get_cfgblock_interactive(void) char it = 'n'; char wb = 'n'; char mem8g = 'n'; + char can = 'y'; int len = 0; /* Unknown module by default */ @@ -387,6 +389,13 @@ static int get_cfgblock_interactive(void) mem8g = console_buffer[0]; } #endif +#if defined(CONFIG_TARGET_VERDIN_IMX8MM) + if (is_cpu_type(MXC_CPU_IMX8MM) && (wb == 'y' || wb == 'Y')) { + sprintf(message, "Does your module have CAN? [y/N] "); + len = cli_readline(message); + can = console_buffer[0]; + } +#endif #endif soc = env_get("soc"); @@ -474,7 +483,9 @@ static int get_cfgblock_interactive(void) else tdx_hw_tag.prodid = VERDIN_IMX8MMDL; } else if (is_cpu_type(MXC_CPU_IMX8MM)) { - if (wb == 'y' || wb == 'Y') + if (can == 'n' || can == 'N') + tdx_hw_tag.prodid = VERDIN_IMX8MMQ_WIFI_BT_IT_NO_CAN; + else if (wb == 'y' || wb == 'Y') tdx_hw_tag.prodid = VERDIN_IMX8MMQ_WIFI_BT_IT; else tdx_hw_tag.prodid = VERDIN_IMX8MMQ_IT; diff --git a/board/toradex/common/tdx-cfg-block.h b/board/toradex/common/tdx-cfg-block.h index 43e662e41d..9697447158 100644 --- a/board/toradex/common/tdx-cfg-block.h +++ b/board/toradex/common/tdx-cfg-block.h @@ -88,6 +88,7 @@ enum { VERDIN_IMX8MPQL_IT, /* 65 */ VERDIN_IMX8MPQ_8GB_WIFI_BT, APALIS_IMX8QM_8GB_WIFI_BT_IT, + VERDIN_IMX8MMQ_WIFI_BT_IT_NO_CAN, }; enum {