Hello Marek,
thanks for this.
On Tue, May 21, 2024 at 11:39:05AM +0200, Marek Vasut wrote:
> The older i.MX8M Mini Verdin SoMs may came with 20 MHz SPI CAN controller
> oscillator, the newer SoMs always use 40 MHz oscillator. Handle both by
> overriding the oscillator frequency just before booting the kernel.
>
> These are the known variants with 20 MHz oscillator:
> - 0055, V1.1A, V1.1B, V1.1C and V1.1D, use a 20MHz oscillator
> - 0059, V1.1A and V1.1B, use a 20MHz oscillator
>
> Signed-off-by: Marek Vasut <ma...@denx.de>
> ---
> Cc: "NXP i.MX U-Boot Team" <uboot-...@nxp.com>
> Cc: Fabio Estevam <feste...@gmail.com>
> Cc: Francesco Dolcini <francesco.dolc...@toradex.com>
> Cc: Marcel Ziswiler <marcel.ziswi...@toradex.com>
> Cc: Philippe Schenker <philippe.schen...@toradex.com>
> Cc: Martyn Welch <martyn.we...@collabora.com>
> Cc: Stefano Babic <sba...@denx.de>
> Cc: u-boot@lists.denx.de
> ---
> board/toradex/verdin-imx8mm/verdin-imx8mm.c | 30 +++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/board/toradex/verdin-imx8mm/verdin-imx8mm.c
> b/board/toradex/verdin-imx8mm/verdin-imx8mm.c
> index 55c02653da6..b4a443ebfb0 100644
> --- a/board/toradex/verdin-imx8mm/verdin-imx8mm.c
> +++ b/board/toradex/verdin-imx8mm/verdin-imx8mm.c
> @@ -125,6 +125,36 @@ 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 freq = 40000000; /* 40 MHz is used on most variants */
> + int canoscoff, ret;
> +
> + canoscoff = fdt_path_offset(blob, canoscpath);
> + if (canoscoff < 0) /* No CAN oscillator found. */
> + goto exit;
> +
> + /*
> + * The actual "prodid" (PID4 in Toradex naming) that have the CAN
> + * functionality are 0055 and 0059.
This is nor correct, we have more, they just always use 40MHz.
I would rephrase this:
/*
* 20MHz CAN oscillator product variants:
* - 0055, V1.1A, V1.1B, V1.1C and V1.1D
* - 0059, V1.1A and V1.1B
*/
or in a different way, if you prefer, but you got the point.
> + if ((tdx_hw_tag.ver_major == 1 && tdx_hw_tag.ver_minor == 1) &&
> + ((tdx_hw_tag.prodid == VERDIN_IMX8MMQ_IT &&
> + tdx_hw_tag.ver_assembly <= 1) || /* 0055 rev. A or B */
> + (tdx_hw_tag.prodid == VERDIN_IMX8MMQ_WIFI_BT_IT &&
> + tdx_hw_tag.ver_assembly <= 4))) { /* 0059 rev. A/B/C/D */
ver_assembly is the hardware revision - 'A', e.g. 'A' is 0, 'D' is 3.
VERDIN_IMX8MMQ_IT is 0059
VERDIN_IMX8MMQ_WIFI_BT_IT is 0055
with that said, it should be
if ((tdx_hw_tag.ver_major == 1 && tdx_hw_tag.ver_minor == 1) &&
((tdx_hw_tag.prodid == VERDIN_IMX8MMQ_WIFI_BT_IT &&
tdx_hw_tag.ver_assembly <= 1) || /* 0055 rev. A or B */
(tdx_hw_tag.prodid == VERDIN_IMX8MMQ_IT &&
tdx_hw_tag.ver_assembly <= 3))) { /* 0059 rev. A/B/C/D */
Francesco
