| Message ID | 20240521093923.8771-1-marex@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Fabio Estevam |
| Headers | show |
| Series | ARM: imx: verdin-imx8mm: Set CAN oscillator frequency based on model | expand |
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 <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: Martyn Welch <martyn.welch@collabora.com> > Cc: Stefano Babic <sbabic@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
On Tue, May 21, 2024 at 01:19:24PM +0200, Francesco Dolcini wrote: > 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 <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: Martyn Welch <martyn.welch@collabora.com> > > Cc: Stefano Babic <sbabic@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 */ whoops. I confused myself. What is correct is the table you have in the comment and the commit message. What is wrong is the comment in this if branch, and this confused me (and probably also you, sorry). 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) || /* 0059 rev. A or B */ (tdx_hw_tag.prodid == VERDIN_IMX8MMQ_WIFI_BT_IT && tdx_hw_tag.ver_assembly <= 3))) { /* 0055 rev. A/B/C/D */ Francesco
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. Special case 20 MHz variant + * here: + * - 0055, V1.1A, V1.1B, V1.1C and V1.1D, use a 20MHz oscillator + * - 0059, V1.1A and V1.1B, use a 20MHz oscillator + */ + 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 */ + freq = 20000000; + } + + 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
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 <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: Martyn Welch <martyn.welch@collabora.com> Cc: Stefano Babic <sbabic@denx.de> Cc: u-boot@lists.denx.de --- board/toradex/verdin-imx8mm/verdin-imx8mm.c | 30 +++++++++++++++++++++ 1 file changed, 30 insertions(+)