Re: [EXT] Re: [PATCH] imx: Revert "imx: mx6ull: fix REFTOP_VBGADJ setting" and fix comments

[Ye Li <ye.li@nxp.com>]

Hi Michael,

On Fri, 2022-03-11 at 15:01 +0100, Dario Binacchi wrote:
> Caution: EXT Email
> 
> Hi,
> 
> On Fri, Mar 11, 2022 at 1:26 PM Ye Li <ye.li@nxp.com> wrote:
> > 
> > 
> > Hi Michael,
> > 
> > On Fri, 2022-03-11 at 08:57 +0100, Michael Nazzareno Trimarchi
> > wrote:
> > > 
> > > Caution: EXT Email
> > > 
> > > Hi
> > > 
> > > On Tue, Mar 8, 2022 at 7:42 AM Ye Li <ye.li@nxp.com> wrote:
> > > > 
> > > > 
> > > > 
> > > > The change in commit c1af358 (imx: mx6ull: fix REFTOP_VBGADJ
> > > > setting)
> > > > is not correct. According to latest i.MX6ULL fuse table,
> > > > the REFTOP_TRIM[2:0] fuse is defined as below. So the previous
> > > > codes
> > > > are correct, only need to fix the comments.
> > > > 
> > > > Set the REFTOP_VBGADJ field in PMU_MISC0 register based on the
> > > > REFTOP_TRIM fuse:
> > > > '000" - set REFTOP_VBGADJ[2:0] to 3'b000
> > > > '001" - set REFTOP_VBGADJ[2:0] to 3'b001
> > > > '010" - set REFTOP_VBGADJ[2:0] to 3'b010
> > > > '011" - set REFTOP_VBGADJ[2:0] to 3'b011
> > > > '100" - set REFTOP_VBGADJ[2:0] to 3'b100
> > > > '101" - set REFTOP_VBGADJ[2:0] to 3'b101
> > > > '110" - set REFTOP_VBGADJ[2:0] to 3'b110
> > > > '111" - set REFTOP_VBGADJ[2:0] to 3'b111
> > > > 
> > > > Signed-off-by: Ye Li <ye.li@nxp.com>
> > > > ---
> > > >  arch/arm/mach-imx/mx6/soc.c | 20 +++++++++-----------
> > > >  1 file changed, 9 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-
> > > > imx/mx6/soc.c
> > > > index 03d8fd7..bb7dc42 100644
> > > > --- a/arch/arm/mach-imx/mx6/soc.c
> > > > +++ b/arch/arm/mach-imx/mx6/soc.c
> > > > @@ -394,23 +394,21 @@ static void init_bandgap(void)
> > > >         /*
> > > >          * On i.MX6ULL,we need to set VBGADJ bits according to
> > > > the
> > > >          * REFTOP_TRIM[3:0] in fuse table
> > > > -        *      000 - set REFTOP_VBGADJ[2:0] to 3b'110,
> > > > -        *      110 - set REFTOP_VBGADJ[2:0] to 3b'000,
> > > > -        *      001 - set REFTOP_VBGADJ[2:0] to 3b'001,
> > > > -        *      010 - set REFTOP_VBGADJ[2:0] to 3b'010,
> > > > -        *      011 - set REFTOP_VBGADJ[2:0] to 3b'011,
> > > > -        *      100 - set REFTOP_VBGADJ[2:0] to 3b'100,
> > > > -        *      101 - set REFTOP_VBGADJ[2:0] to 3b'101,
> > > > -        *      111 - set REFTOP_VBGADJ[2:0] to 3b'111,
> > > > +        *      000 - set REFTOP_VBGADJ[2:0] to 3'b000
> > > > +        *      001 - set REFTOP_VBGADJ[2:0] to 3'b001
> > > > +        *      010 - set REFTOP_VBGADJ[2:0] to 3'b010
> > > > +        *      011 - set REFTOP_VBGADJ[2:0] to 3'b011
> > > > +        *      100 - set REFTOP_VBGADJ[2:0] to 3'b100
> > > > +        *      101 - set REFTOP_VBGADJ[2:0] to 3'b101
> > > > +        *      110 - set REFTOP_VBGADJ[2:0] to 3'b110
> > > > +        *      111 - set REFTOP_VBGADJ[2:0] to 3'b111
> > > >          */
> > > >         if (is_mx6ull()) {
> > > > -               static const u32 map[] = {6, 1, 2, 3, 4, 5, 0,
> > > > 7};
> > > > -
> > > >                 val = readl(&fuse->mem0);
> > > >                 val >>= OCOTP_MEM0_REFTOP_TRIM_SHIFT;
> > > >                 val &= 0x7;
> > > > 
> > > > -               writel(map[val] <<
> > > > BM_ANADIG_ANA_MISC0_REFTOP_VBGADJ_SHIFT,
> > > > +               writel(val <<
> > > > BM_ANADIG_ANA_MISC0_REFTOP_VBGADJ_SHIFT,
> > > >                        &anatop->ana_misc0_set);
> > > >         }
> > > If this is the case. I have the impression there are a lot of
> > > cpus
> > > that have this fuse not set during manufacturing.
> > Most of iMX6 series don't need to program this fuse, but 6ull is a
> > exception due to different process used. if your 6ull don't have
> > the
> > fuse programmed, please contact NXP support team.
> > 
> > > 
> > > If this can happen if we have devices in the field already
> > > programmed
> > > that they do any temperature evaluation for thermal
> > > framework the calculation start to be different and they can have
> > > impact on their device
> > I don't understand this.
> The imx6ul reference manual states that "the bandgap is necessary for
> correct operation of most of the LDO, PLL, and other analog functions
> on
> the die." Applying the 97c16dc8bf098 ("imx: mx6ull: update the
> REFTOP_VBGADJ setting")
> patch caused a 7/8 degree variation in the temperature provided by
> the
> TEMPMON module on devices in the field. The patch you are reverting
> tried to make backward compatible the update  of REFTOP_VBGADJ
> from fuse. Your patch, re-introduces non-backward compatibility.
> 
> Thanks and regards,
> Dario
Without patch 97c16dc8bf098 ("imx: mx6ull: update the REFTOP_VBGADJ
setting"), the previous codes set 0x6 to REFTOP_VBGADJ, but this
actually generates wrong temperature. 
So it does not make sense to align with the wrong setting. 


Best regards,
Ye Li
> 
> > 
> > Anyway, both NXP manufacturing and SW need to follow the fuse value
> > definition.
> > 
> > Best regards,
> > Ye Li
> > > 
> > > 
> > > Michael
> > > 
> > > > 
> > > > 
> > > >  }
> > > > --
> > > > 2.7.4
> > > > 
> > > --
> > > Michael Nazzareno Trimarchi
> > > Co-Founder & Chief Executive Officer
> > > M. +39 347 913 2170
> > > michael@amarulasolutions.com
> > > __________________________________
> > > 
> > > Amarula Solutions BV
> > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > > T. +31 (0)85 111 9172
> > > info@amarulasolutions.com
> > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> > > www.
> > > amarulasolutions.com%2F&amp;data=04%7C01%7Cye.li%40nxp.com%7C1837
> > > 3707
> > > 3bca4b167e1808da0334d5fc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> > > C0%7
> > > C637825822708088241%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL
> > > CJQI
> > > joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=jZ0z1f
> > > FhG9
> > > %2BmQ7pB94C4RPEEwCg2IxMvhRluxs7HcQc%3D&amp;reserved=0
> 
> 
> --
> 
> Dario Binacchi
> 
> Embedded Linux Developer
> 
> dario.binacchi@amarulasolutions.com
> 
> __________________________________
> 
> 
> Amarula Solutions SRL
> 
> Via Le Canevare 30, 31100 Treviso, Veneto, IT
> 
> T. +39 042 243 5310
> info@amarulasolutions.com
> 
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> amarulasolutions.com%2F&amp;data=04%7C01%7Cye.li%40nxp.com%7C29efc280
> 7a614e78614308da0367b070%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C637826041117804146%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQI
> joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=TOgDwYOSYc
> N%2Ff6a9BzGWwilAnI1%2FOPw3XXBBXJYc2OI%3D&amp;reserved=0