* [PATCH] toradex: tdx-cfg-block: add 0068 i.mx 8m mini sku
@ 2022-06-21 11:49 Philippe Schenker
2022-07-01 5:29 ` Marcel Ziswiler
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Schenker @ 2022-06-21 11:49 UTC (permalink / raw)
To: u-boot
Cc: Philippe Schenker, Francesco Dolcini, Denys Drozdov,
Marcel Ziswiler, Simon Glass
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)",
};
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 {
--
2.36.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] toradex: tdx-cfg-block: add 0068 i.mx 8m mini sku
2022-06-21 11:49 [PATCH] toradex: tdx-cfg-block: add 0068 i.mx 8m mini sku Philippe Schenker
@ 2022-07-01 5:29 ` Marcel Ziswiler
2022-07-01 7:20 ` Philippe Schenker
2022-07-01 14:49 ` Philippe Schenker
0 siblings, 2 replies; 4+ messages in thread
From: Marcel Ziswiler @ 2022-07-01 5:29 UTC (permalink / raw)
To: dev, u-boot; +Cc: Philippe Schenker, sjg, Francesco Dolcini, Denys Drozdov
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?
> };
>
> 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 {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] toradex: tdx-cfg-block: add 0068 i.mx 8m mini sku
2022-07-01 5:29 ` Marcel Ziswiler
@ 2022-07-01 7:20 ` Philippe Schenker
2022-07-01 14:49 ` Philippe Schenker
1 sibling, 0 replies; 4+ messages in thread
From: Philippe Schenker @ 2022-07-01 7:20 UTC (permalink / raw)
To: Marcel Ziswiler, u-boot; +Cc: sjg, Francesco Dolcini, Denys Drozdov
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 {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] toradex: tdx-cfg-block: add 0068 i.mx 8m mini sku
2022-07-01 5:29 ` Marcel Ziswiler
2022-07-01 7:20 ` Philippe Schenker
@ 2022-07-01 14:49 ` Philippe Schenker
1 sibling, 0 replies; 4+ messages in thread
From: Philippe Schenker @ 2022-07-01 14:49 UTC (permalink / raw)
To: Marcel Ziswiler, u-boot; +Cc: sjg, Francesco Dolcini, Denys Drozdov
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?
Marcel, we just found an issue with that patch that made it through our
internal review. I will send a v2 of this one.
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 {
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-01 14:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-21 11:49 [PATCH] toradex: tdx-cfg-block: add 0068 i.mx 8m mini sku Philippe Schenker
2022-07-01 5:29 ` Marcel Ziswiler
2022-07-01 7:20 ` Philippe Schenker
2022-07-01 14:49 ` Philippe Schenker
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.