[PATCH] toradex: tdx-cfg-block: add 0068 i.mx 8m mini sku

[Philippe Schenker]

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 at 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 at toradex.com>
> > Reviewed-by: Francesco Dolcini <francesco.dolcini at 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 {