[RFC][PATCH] ARM: imx: verdin-imx8mm: Set CAN oscillator frequency based on model

[Francesco Dolcini]

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 at denx.de>
> > ---
> > Cc: "NXP i.MX U-Boot Team" <uboot-imx at nxp.com>
> > Cc: Fabio Estevam <festevam at gmail.com>
> > Cc: Francesco Dolcini <francesco.dolcini at toradex.com>
> > Cc: Marcel Ziswiler <marcel.ziswiler at toradex.com>
> > Cc: Philippe Schenker <philippe.schenker at toradex.com>
> > Cc: Stefano Babic <sbabic at denx.de>
> 
> Acked-by: Marcel Ziswiler <marcel.ziswiler at 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 at 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 at denx.de>
> 
> Applied, thanks.

Fabio, you were too fast ... please revert it.

Francesco