[RFC][PATCH] ARM: imx: verdin-imx8mm: Set CAN oscillator frequency based on model
The older i.MX8M Mini Verdin SoMs before rev. 1.1C came with 20 MHz SPI CAN controller oscillator, the newer SoMs use 40 MHz oscillator. Handle both by overriding the oscillator frequency just before booting the kernel.
Signed-off-by: Marek Vasut marex@denx.de --- Cc: "NXP i.MX U-Boot Team" uboot-imx@nxp.com Cc: Fabio Estevam festevam@gmail.com Cc: Francesco Dolcini francesco.dolcini@toradex.com Cc: Marcel Ziswiler marcel.ziswiler@toradex.com Cc: Philippe Schenker philippe.schenker@toradex.com Cc: Stefano Babic sbabic@denx.de --- board/toradex/verdin-imx8mm/verdin-imx8mm.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/board/toradex/verdin-imx8mm/verdin-imx8mm.c b/board/toradex/verdin-imx8mm/verdin-imx8mm.c index b2781b51d6a..8bc1a51eeb1 100644 --- a/board/toradex/verdin-imx8mm/verdin-imx8mm.c +++ b/board/toradex/verdin-imx8mm/verdin-imx8mm.c @@ -142,6 +142,25 @@ int board_phys_sdram_size(phys_size_t *size) #if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) int ft_board_setup(void *blob, struct bd_info *bd) { + const char *canoscpath = "/oscillator"; + int canoscoff, freq, ret; + + canoscoff = fdt_path_offset(blob, canoscpath); + if (canoscoff < 0) /* No CAN oscillator found. */ + goto exit; + + if (tdx_hw_tag.ver_assembly < 2) /* rev. A or B */ + freq = 20000000; + else /* rev. C or newer */ + freq = 40000000; + + ret = fdt_setprop_u32(blob, canoscoff, "clock-frequency", freq); + if (ret < 0) { + printf("Failed to set CAN oscillator clock-frequency, ret=%d\n", + ret); + } + +exit: return ft_common_board_setup(blob, bd); } #endif
Hi Marek
Thanks, seems like a decent idea. I guess we just did not do it as those previous revisions were just samples but with this change can still be used much easier (e.g. without requiring manual intervention).
On Sat, 2024-01-13 at 19:33 +0100, Marek Vasut wrote:
The older i.MX8M Mini Verdin SoMs before rev. 1.1C came with 20 MHz SPI CAN controller oscillator, the newer SoMs use 40 MHz oscillator. Handle both by overriding the oscillator frequency just before booting the kernel.
Signed-off-by: Marek Vasut marex@denx.de
Cc: "NXP i.MX U-Boot Team" uboot-imx@nxp.com Cc: Fabio Estevam festevam@gmail.com Cc: Francesco Dolcini francesco.dolcini@toradex.com Cc: Marcel Ziswiler marcel.ziswiler@toradex.com Cc: Philippe Schenker philippe.schenker@toradex.com Cc: Stefano Babic sbabic@denx.de
Acked-by: Marcel Ziswiler marcel.ziswiler@toradex.com
board/toradex/verdin-imx8mm/verdin-imx8mm.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/board/toradex/verdin-imx8mm/verdin-imx8mm.c b/board/toradex/verdin-imx8mm/verdin-imx8mm.c index b2781b51d6a..8bc1a51eeb1 100644 --- a/board/toradex/verdin-imx8mm/verdin-imx8mm.c +++ b/board/toradex/verdin-imx8mm/verdin-imx8mm.c @@ -142,6 +142,25 @@ int board_phys_sdram_size(phys_size_t *size) #if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) int ft_board_setup(void *blob, struct bd_info *bd) {
- const char *canoscpath = "/oscillator";
- int canoscoff, freq, ret;
- canoscoff = fdt_path_offset(blob, canoscpath);
- if (canoscoff < 0) /* No CAN oscillator found. */
goto exit;- if (tdx_hw_tag.ver_assembly < 2) /* rev. A or B */
freq = 20000000;- else /* rev. C or newer */
freq = 40000000;- ret = fdt_setprop_u32(blob, canoscoff, "clock-frequency", freq);
- if (ret < 0) {
printf("Failed to set CAN oscillator clock-frequency, ret=%d\n",ret);- }
+exit: return ft_common_board_setup(blob, bd); } #endif
Cheers
Marcel
On Sat, Jan 13, 2024 at 3:34 PM Marek Vasut marex@denx.de wrote:
The older i.MX8M Mini Verdin SoMs before rev. 1.1C came with 20 MHz SPI CAN controller oscillator, the newer SoMs use 40 MHz oscillator. Handle both by overriding the oscillator frequency just before booting the kernel.
Signed-off-by: Marek Vasut marex@denx.de
Applied, thanks.
On Sat, Jan 13, 2024 at 06:40:19PM +0000, Marcel Ziswiler wrote:
Hi Marek
Thanks, seems like a decent idea. I guess we just did not do it as those previous revisions were just samples but with this change can still be used much easier (e.g. without requiring manual intervention).
On Sat, 2024-01-13 at 19:33 +0100, Marek Vasut wrote:
The older i.MX8M Mini Verdin SoMs before rev. 1.1C came with 20 MHz SPI CAN controller oscillator, the newer SoMs use 40 MHz oscillator. Handle both by overriding the oscillator frequency just before booting the kernel.
Signed-off-by: Marek Vasut marex@denx.de
Cc: "NXP i.MX U-Boot Team" uboot-imx@nxp.com Cc: Fabio Estevam festevam@gmail.com Cc: Francesco Dolcini francesco.dolcini@toradex.com Cc: Marcel Ziswiler marcel.ziswiler@toradex.com Cc: Philippe Schenker philippe.schenker@toradex.com Cc: Stefano Babic sbabic@denx.de
Acked-by: Marcel Ziswiler marcel.ziswiler@toradex.com
Marcel, that patch is buggy.
The assembly version makes sense only for a specific major/minor hardware version.
So what you need to check if revision < 1.1C, while the code from Marek check just for the assembly version C.
board/toradex/verdin-imx8mm/verdin-imx8mm.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/board/toradex/verdin-imx8mm/verdin-imx8mm.c b/board/toradex/verdin-imx8mm/verdin-imx8mm.c index b2781b51d6a..8bc1a51eeb1 100644 --- a/board/toradex/verdin-imx8mm/verdin-imx8mm.c +++ b/board/toradex/verdin-imx8mm/verdin-imx8mm.c @@ -142,6 +142,25 @@ int board_phys_sdram_size(phys_size_t *size) #if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) int ft_board_setup(void *blob, struct bd_info *bd) {
- const char *canoscpath = "/oscillator";
- int canoscoff, freq, ret;
- canoscoff = fdt_path_offset(blob, canoscpath);
- if (canoscoff < 0) /* No CAN oscillator found. */
goto exit;- if (tdx_hw_tag.ver_assembly < 2) /* rev. A or B */
I would suggest a slightly different approach.
First you need to check also for major/minor version. The revision restart from A every time there is a change in a version.
Second I would set the 20MHz oscillator only for older version and do nothing, not set anything, for the newer one.
On Sat, Jan 13, 2024 at 04:52:57PM -0300, Fabio Estevam wrote:
On Sat, Jan 13, 2024 at 3:34 PM Marek Vasut marex@denx.de wrote:
The older i.MX8M Mini Verdin SoMs before rev. 1.1C came with 20 MHz SPI CAN controller oscillator, the newer SoMs use 40 MHz oscillator. Handle both by overriding the oscillator frequency just before booting the kernel.
Signed-off-by: Marek Vasut marex@denx.de
Applied, thanks.
Fabio, you were too fast ... please revert it.
Francesco
On Sat, Jan 13, 2024 at 09:45:42PM +0100, Francesco Dolcini wrote:
On Sat, Jan 13, 2024 at 06:40:19PM +0000, Marcel Ziswiler wrote:
Hi Marek
Thanks, seems like a decent idea. I guess we just did not do it as those previous revisions were just samples but with this change can still be used much easier (e.g. without requiring manual intervention).
On Sat, 2024-01-13 at 19:33 +0100, Marek Vasut wrote:
The older i.MX8M Mini Verdin SoMs before rev. 1.1C came with 20 MHz SPI CAN controller oscillator, the newer SoMs use 40 MHz oscillator. Handle both by overriding the oscillator frequency just before booting the kernel.
Signed-off-by: Marek Vasut marex@denx.de
Cc: "NXP i.MX U-Boot Team" uboot-imx@nxp.com Cc: Fabio Estevam festevam@gmail.com Cc: Francesco Dolcini francesco.dolcini@toradex.com Cc: Marcel Ziswiler marcel.ziswiler@toradex.com Cc: Philippe Schenker philippe.schenker@toradex.com Cc: Stefano Babic sbabic@denx.de
Acked-by: Marcel Ziswiler marcel.ziswiler@toradex.com
Marcel, that patch is buggy.
The assembly version makes sense only for a specific major/minor hardware version.
So what you need to check if revision < 1.1C, while the code from Marek check just for the assembly version C.
And in the general case even my suggested test is not enough. We started having assembly version that are not linearly increasing since some time.
I am fine on finding a solution to dynamically handle the different oscillator frequency, but let's do it right.
I will reply Monday with some more details so we can have a working solution.
Francesco
On Sat, Jan 13, 2024 at 5:45 PM Francesco Dolcini francesco@dolcini.it wrote:
Fabio, you were too fast ... please revert it.
I have dropped this patch from u-boot-imx master-next.
Hi Guys
On Sat, 2024-01-13 at 21:45 +0100, Francesco Dolcini wrote:
On Sat, Jan 13, 2024 at 06:40:19PM +0000, Marcel Ziswiler wrote:
Hi Marek
Thanks, seems like a decent idea. I guess we just did not do it as those previous revisions were just samples but with this change can still be used much easier (e.g. without requiring manual intervention).
On Sat, 2024-01-13 at 19:33 +0100, Marek Vasut wrote:
The older i.MX8M Mini Verdin SoMs before rev. 1.1C came with 20 MHz SPI CAN controller oscillator, the newer SoMs use 40 MHz oscillator. Handle both by overriding the oscillator frequency just before booting the kernel.
Signed-off-by: Marek Vasut marex@denx.de
Cc: "NXP i.MX U-Boot Team" uboot-imx@nxp.com Cc: Fabio Estevam festevam@gmail.com Cc: Francesco Dolcini francesco.dolcini@toradex.com Cc: Marcel Ziswiler marcel.ziswiler@toradex.com Cc: Philippe Schenker philippe.schenker@toradex.com Cc: Stefano Babic sbabic@denx.de
Acked-by: Marcel Ziswiler marcel.ziswiler@toradex.com
Marcel, that patch is buggy.
The assembly version makes sense only for a specific major/minor hardware version.
Yes, sorry, I did not look at it careful enough.
So what you need to check if revision < 1.1C, while the code from Marek check just for the assembly version C.
board/toradex/verdin-imx8mm/verdin-imx8mm.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/board/toradex/verdin-imx8mm/verdin-imx8mm.c b/board/toradex/verdin-imx8mm/verdin-imx8mm.c index b2781b51d6a..8bc1a51eeb1 100644 --- a/board/toradex/verdin-imx8mm/verdin-imx8mm.c +++ b/board/toradex/verdin-imx8mm/verdin-imx8mm.c @@ -142,6 +142,25 @@ int board_phys_sdram_size(phys_size_t *size) #if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) int ft_board_setup(void *blob, struct bd_info *bd) {
- const char *canoscpath = "/oscillator";
- int canoscoff, freq, ret;
- canoscoff = fdt_path_offset(blob, canoscpath);
- if (canoscoff < 0) /* No CAN oscillator found. */
goto exit;- if (tdx_hw_tag.ver_assembly < 2) /* rev. A or B */
I would suggest a slightly different approach.
First you need to check also for major/minor version. The revision restart from A every time there is a change in a version.
Second I would set the 20MHz oscillator only for older version and do nothing, not set anything, for the newer one.
Yes, I agree. So worst case it will always use the new/fixed setting.
On Sat, Jan 13, 2024 at 04:52:57PM -0300, Fabio Estevam wrote:
On Sat, Jan 13, 2024 at 3:34 PM Marek Vasut marex@denx.de wrote:
The older i.MX8M Mini Verdin SoMs before rev. 1.1C came with 20 MHz SPI CAN controller oscillator, the newer SoMs use 40 MHz oscillator. Handle both by overriding the oscillator frequency just before booting the kernel.
Signed-off-by: Marek Vasut marex@denx.de
Applied, thanks.
Fabio, you were too fast ... please revert it.
Francesco
Cheers
Marcel
On Sat, Jan 13, 2024 at 07:33:17PM +0100, Marek Vasut wrote:
The older i.MX8M Mini Verdin SoMs before rev. 1.1C came with 20 MHz SPI CAN controller oscillator, the newer SoMs use 40 MHz oscillator. Handle both by overriding the oscillator frequency just before booting the kernel.
Signed-off-by: Marek Vasut marex@denx.de
Cc: "NXP i.MX U-Boot Team" uboot-imx@nxp.com Cc: Fabio Estevam festevam@gmail.com Cc: Francesco Dolcini francesco.dolcini@toradex.com Cc: Marcel Ziswiler marcel.ziswiler@toradex.com Cc: Philippe Schenker philippe.schenker@toradex.com
Philippe is no longer in toradex, removed from cc list.
Cc: Stefano Babic sbabic@denx.de
board/toradex/verdin-imx8mm/verdin-imx8mm.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/board/toradex/verdin-imx8mm/verdin-imx8mm.c b/board/toradex/verdin-imx8mm/verdin-imx8mm.c index b2781b51d6a..8bc1a51eeb1 100644 --- a/board/toradex/verdin-imx8mm/verdin-imx8mm.c +++ b/board/toradex/verdin-imx8mm/verdin-imx8mm.c @@ -142,6 +142,25 @@ int board_phys_sdram_size(phys_size_t *size) #if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) int ft_board_setup(void *blob, struct bd_info *bd) {
- const char *canoscpath = "/oscillator";
- int canoscoff, freq, ret;
- canoscoff = fdt_path_offset(blob, canoscpath);
- if (canoscoff < 0) /* No CAN oscillator found. */
goto exit;- if (tdx_hw_tag.ver_assembly < 2) /* rev. A or B */
freq = 20000000;- else /* rev. C or newer */
freq = 40000000;
So, the situation is more complex than what I thought initially.
The actual frequency of the oscillator depends on the whole `struct toradex_hw`, you would need to check prodid, ver_major, ver_minor and ver_assembly. In addition to that code should not expect `ver_assembly` to be an increasing number, this is just a number that is matching the BOM.
The actual `prodid` (PID4 in Toradex naming) that have the CAN functionality are 0055 and 0059.
0059, V1.1A and V1.1B, use a 20MHz oscillator 0055, V1.1A, V1.1B, V1.1C and V1.1D, use a 20Mhz oscillator
Verdin iMX8MM V1.0* is something I would not consider, from what I know they were all scrapped (not even myself has any of those anymore).
With that said what I would do is something like
if ((tdx_hw_tag.ver_major == 1 && tdx_hw_tag.ver_minor == 1) && ((tdx_hw_tag.prodid == VERDIN_IMX8MMQ_IT && ver_assembly <= 1) || (tdx_hw_tag.prodid == VERDIN_IMX8MMQ_WIFI_BT_IT &&. ....))
ft_fixup_can_oscillator_freq();
or any other more readable variant of your choice ;-)
And I would not have the code guess anything apart this, just setting 20MHz for the well known configuration.
If you are interested the details on the versions they are public, see https://www.toradex.com/computer-on-modules/verdin-arm-family/nxp-imx-8m-min... and https://developer.toradex.com/hardware/verdin-som-family/modules/verdin-imx8...
Francesco
On 1/15/24 16:44, Francesco Dolcini wrote:
On Sat, Jan 13, 2024 at 07:33:17PM +0100, Marek Vasut wrote:
The older i.MX8M Mini Verdin SoMs before rev. 1.1C came with 20 MHz SPI CAN controller oscillator, the newer SoMs use 40 MHz oscillator. Handle both by overriding the oscillator frequency just before booting the kernel.
Signed-off-by: Marek Vasut marex@denx.de
Cc: "NXP i.MX U-Boot Team" uboot-imx@nxp.com Cc: Fabio Estevam festevam@gmail.com Cc: Francesco Dolcini francesco.dolcini@toradex.com Cc: Marcel Ziswiler marcel.ziswiler@toradex.com Cc: Philippe Schenker philippe.schenker@toradex.com
Philippe is no longer in toradex, removed from cc list.
Cc: Stefano Babic sbabic@denx.de
board/toradex/verdin-imx8mm/verdin-imx8mm.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/board/toradex/verdin-imx8mm/verdin-imx8mm.c b/board/toradex/verdin-imx8mm/verdin-imx8mm.c index b2781b51d6a..8bc1a51eeb1 100644 --- a/board/toradex/verdin-imx8mm/verdin-imx8mm.c +++ b/board/toradex/verdin-imx8mm/verdin-imx8mm.c @@ -142,6 +142,25 @@ int board_phys_sdram_size(phys_size_t *size) #if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) int ft_board_setup(void *blob, struct bd_info *bd) {
- const char *canoscpath = "/oscillator";
- int canoscoff, freq, ret;
- canoscoff = fdt_path_offset(blob, canoscpath);
- if (canoscoff < 0) /* No CAN oscillator found. */
goto exit;- if (tdx_hw_tag.ver_assembly < 2) /* rev. A or B */
freq = 20000000;- else /* rev. C or newer */
freq = 40000000;So, the situation is more complex than what I thought initially.
The actual frequency of the oscillator depends on the whole `struct toradex_hw`, you would need to check prodid, ver_major, ver_minor and ver_assembly.
In addition to that code should not expect `ver_assembly` to be an increasing number, this is just a number that is matching the BOM.
The actual `prodid` (PID4 in Toradex naming) that have the CAN functionality are 0055 and 0059.
0059, V1.1A and V1.1B, use a 20MHz oscillator 0055, V1.1A, V1.1B, V1.1C and V1.1D, use a 20Mhz oscillator
Do you happen to have a table of what was populated with which oscillator, so I can fill all the 20 MHz parts in right away ?
On Tue, Jan 16, 2024 at 02:01:02AM +0100, Marek Vasut wrote:
On 1/15/24 16:44, Francesco Dolcini wrote:
On Sat, Jan 13, 2024 at 07:33:17PM +0100, Marek Vasut wrote: 0059, V1.1A and V1.1B, use a 20MHz oscillator 0055, V1.1A, V1.1B, V1.1C and V1.1D, use a 20Mhz oscillator
Do you happen to have a table of what was populated with which oscillator, so I can fill all the 20 MHz parts in right away ?
I guess that one quoted here from my previous email is "the table", but maybe I have not understood your question.
Francesco
participants (4)
-
Fabio Estevam -
Francesco Dolcini -
Marcel Ziswiler -
Marek Vasut