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

[Marcel Ziswiler]

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 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.

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 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

Cheers

Marcel