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

[Francesco Dolcini]

On Wed, May 22, 2024 at 03:23:51PM +0200, Marek Vasut wrote:
> On 5/22/24 8:39 AM, Francesco Dolcini wrote:
> > > diff --git a/board/toradex/verdin-imx8mm/verdin-imx8mm.c b/board/toradex/verdin-imx8mm/verdin-imx8mm.c
> > > index 55c02653da6..ef632d95f0a 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
> > > +	 */
> > 
> > Any reason why you ignored my suggestion here? The variants you list
> > here are the only one with a 20MHz oscillator, and this is correct.
> > 
> > What is not correct is that 0055/0059 are the only variant with CAN
> > functionality. We have other "prodid" with CAN functionality.
> > 
> > With that said, the code is correct, thanks. I appreciate you taking care
> > of this.
> 
> So ... what should I change for V3 ?
> 
> Maybe just create me a diff I can squash into the patch before resend ? (I
> think I am a bit confused, I thought I addressed all the V1 feedback)

Sorry for the confusion, here the diff.
With that please add

Reviewed-by: Francesco Dolcini <francesco.dolcini at toradex.com>


diff --git a/board/toradex/verdin-imx8mm/verdin-imx8mm.c b/board/toradex/verdin-imx8mm/verdin-imx8mm.c
index ef632d95f0a5..59cc28f652f1 100644
--- a/board/toradex/verdin-imx8mm/verdin-imx8mm.c
+++ b/board/toradex/verdin-imx8mm/verdin-imx8mm.c
@@ -134,11 +134,10 @@ int ft_board_setup(void *blob, struct bd_info *bd)
                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
+        * The followings "prodid" (PID4 in Toradex naming) use
+        * a 20MHz CAN oscillator:
+        * - 0055, V1.1A, V1.1B, V1.1C and V1.1D
+        * - 0059, V1.1A and V1.1B
         */
        if ((tdx_hw_tag.ver_major == 1 && tdx_hw_tag.ver_minor == 1) &&
            ((tdx_hw_tag.prodid == VERDIN_IMX8MMQ_IT &&