| Message ID | 1492063800-17290-12-git-send-email-peng.fan@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
On Thu, Apr 13, 2017 at 3:10 AM, Peng Fan <peng.fan@nxp.com> wrote: > +#define BOARD_REV_C 0x300 > +#define BOARD_REV_B 0x200 > +#define BOARD_REV_A 0x100 > + > +static int mx7sabre_rev(void) > +{ > + /* > + * Get Board ID information from OCOTP_GP1[15:8] > + * i.MX7D SDB RevA: 0x41 > + * i.MX7D SDB RevB: 0x42 Isn't this versioning scheme shared with other NXP boards? If so, it would be better to put this in common code.
Hi Fabio, > -----Original Message----- > From: Fabio Estevam [mailto:festevam@gmail.com] > Sent: Monday, April 17, 2017 11:00 PM > To: Peng Fan <peng.fan@nxp.com> > Cc: Stefano Babic <sbabic@denx.de>; U-Boot-Denx <u-boot@lists.denx.de> > Subject: Re: [U-Boot] [PATCH V2 12/12] imx: mx7dsabresd: add board revision > check > > On Thu, Apr 13, 2017 at 3:10 AM, Peng Fan <peng.fan@nxp.com> wrote: > > > +#define BOARD_REV_C 0x300 > > +#define BOARD_REV_B 0x200 > > +#define BOARD_REV_A 0x100 > > + > > +static int mx7sabre_rev(void) > > +{ > > + /* > > + * Get Board ID information from OCOTP_GP1[15:8] > > + * i.MX7D SDB RevA: 0x41 > > + * i.MX7D SDB RevB: 0x42 > > Isn't this versioning scheme shared with other NXP boards? If so, it would be > better to put this in common code. I prefer to keep the code here. There are board revision fuse for the boards from NXP, but this is not always true, I think. Thanks, Peng.
Hi Peng, On 18/04/2017 02:54, Peng Fan wrote: > Hi Fabio, > >> -----Original Message----- >> From: Fabio Estevam [mailto:festevam@gmail.com] >> Sent: Monday, April 17, 2017 11:00 PM >> To: Peng Fan <peng.fan@nxp.com> >> Cc: Stefano Babic <sbabic@denx.de>; U-Boot-Denx <u-boot@lists.denx.de> >> Subject: Re: [U-Boot] [PATCH V2 12/12] imx: mx7dsabresd: add board revision >> check >> >> On Thu, Apr 13, 2017 at 3:10 AM, Peng Fan <peng.fan@nxp.com> wrote: >> >>> +#define BOARD_REV_C 0x300 >>> +#define BOARD_REV_B 0x200 >>> +#define BOARD_REV_A 0x100 >>> + >>> +static int mx7sabre_rev(void) >>> +{ >>> + /* >>> + * Get Board ID information from OCOTP_GP1[15:8] >>> + * i.MX7D SDB RevA: 0x41 >>> + * i.MX7D SDB RevB: 0x42 >> >> Isn't this versioning scheme shared with other NXP boards? If so, it would be >> better to put this in common code. > > I prefer to keep the code here. There are board revision fuse for the boards from NXP, but > this is not always true, I think. Anyway, there is "quite" same code for mx6 sabre: static int mx6sabre_rev(void) { /* * Get Board ID information from OCOTP_GP1[15:8] * i.MX6Q ARD RevA: 0x01 * i.MX6Q ARD RevB: 0x02 */ struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR; struct fuse_bank *bank = &ocotp->bank[4]; struct fuse_bank4_regs *fuse = (struct fuse_bank4_regs *)bank->fuse_regs; int reg = readl(&fuse->gp1); int ret; switch (reg >> 8 & 0x0F) { case 0x02: ret = BOARD_REV_B; break; case 0x01: default: And the version number is simple an integer and we do not need to add defines - if we simply returns the read value (1,2,3,..), the code works even with future versions. Are you sure that this is not at least "sabre common code " ? Regards, Stefano
Hi Stefano, > -----Original Message----- > From: Stefano Babic [mailto:sbabic@denx.de] > Sent: Thursday, May 11, 2017 7:33 PM > To: Peng Fan <peng.fan@nxp.com>; Fabio Estevam <festevam@gmail.com> > Cc: Stefano Babic <sbabic@denx.de>; U-Boot-Denx <u-boot@lists.denx.de> > Subject: Re: [U-Boot] [PATCH V2 12/12] imx: mx7dsabresd: add board revision > check > > Hi Peng, > > On 18/04/2017 02:54, Peng Fan wrote: > > Hi Fabio, > > > >> -----Original Message----- > >> From: Fabio Estevam [mailto:festevam@gmail.com] > >> Sent: Monday, April 17, 2017 11:00 PM > >> To: Peng Fan <peng.fan@nxp.com> > >> Cc: Stefano Babic <sbabic@denx.de>; U-Boot-Denx > >> <u-boot@lists.denx.de> > >> Subject: Re: [U-Boot] [PATCH V2 12/12] imx: mx7dsabresd: add board > >> revision check > >> > >> On Thu, Apr 13, 2017 at 3:10 AM, Peng Fan <peng.fan@nxp.com> wrote: > >> > >>> +#define BOARD_REV_C 0x300 > >>> +#define BOARD_REV_B 0x200 > >>> +#define BOARD_REV_A 0x100 > >>> + > >>> +static int mx7sabre_rev(void) > >>> +{ > >>> + /* > >>> + * Get Board ID information from OCOTP_GP1[15:8] > >>> + * i.MX7D SDB RevA: 0x41 > >>> + * i.MX7D SDB RevB: 0x42 > >> > >> Isn't this versioning scheme shared with other NXP boards? If so, it > >> would be better to put this in common code. > > > > I prefer to keep the code here. There are board revision fuse for the > > boards from NXP, but this is not always true, I think. > > Anyway, there is "quite" same code for mx6 sabre: > > static int mx6sabre_rev(void) > { > /* > * Get Board ID information from OCOTP_GP1[15:8] > * i.MX6Q ARD RevA: 0x01 > * i.MX6Q ARD RevB: 0x02 > */ > struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR; > struct fuse_bank *bank = &ocotp->bank[4]; > struct fuse_bank4_regs *fuse = > (struct fuse_bank4_regs *)bank->fuse_regs; > int reg = readl(&fuse->gp1); > int ret; > > switch (reg >> 8 & 0x0F) { > case 0x02: > ret = BOARD_REV_B; > break; > case 0x01: > default: > > And the version number is simple an integer and we do not need to add > defines - if we simply returns the read value (1,2,3,..), the code works even > with future versions. Are you sure that this is not at least "sabre common code > " ? Could I use a follow up patch to move the revision check to a common place? I would not send a whole V3 patch set if no more comments. Thanks, Peng. > > Regards, > Stefano > > -- > ============================================================ > ========= > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > ============================================================ > =========
Hi Peng, On 18/04/2017 02:54, Peng Fan wrote: > Hi Fabio, > >> -----Original Message----- >> From: Fabio Estevam [mailto:festevam@gmail.com] >> Sent: Monday, April 17, 2017 11:00 PM >> To: Peng Fan <peng.fan@nxp.com> >> Cc: Stefano Babic <sbabic@denx.de>; U-Boot-Denx <u-boot@lists.denx.de> >> Subject: Re: [U-Boot] [PATCH V2 12/12] imx: mx7dsabresd: add board revision >> check >> >> On Thu, Apr 13, 2017 at 3:10 AM, Peng Fan <peng.fan@nxp.com> wrote: >> >>> +#define BOARD_REV_C 0x300 >>> +#define BOARD_REV_B 0x200 >>> +#define BOARD_REV_A 0x100 >>> + >>> +static int mx7sabre_rev(void) >>> +{ >>> + /* >>> + * Get Board ID information from OCOTP_GP1[15:8] >>> + * i.MX7D SDB RevA: 0x41 >>> + * i.MX7D SDB RevB: 0x42 >> >> Isn't this versioning scheme shared with other NXP boards? If so, it would be >> better to put this in common code. > > I prefer to keep the code here. There are board revision fuse for the boards from NXP, but > this is not always true, I think. > Patches 1..11 are free of comments. I am merging them and I will send PR to Tom for inclusion after my build test. I will just let this last one out. Regards, Stefano
> -----Original Message----- > From: Stefano Babic [mailto:sbabic@denx.de] > Sent: Thursday, May 18, 2017 4:20 PM > To: Peng Fan <peng.fan@nxp.com>; Fabio Estevam <festevam@gmail.com> > Cc: Stefano Babic <sbabic@denx.de>; U-Boot-Denx <u-boot@lists.denx.de> > Subject: Re: [U-Boot] [PATCH V2 12/12] imx: mx7dsabresd: add board revision > check > > Hi Peng, Hi Stefano, > > On 18/04/2017 02:54, Peng Fan wrote: > > Hi Fabio, > > > >> -----Original Message----- > >> From: Fabio Estevam [mailto:festevam@gmail.com] > >> Sent: Monday, April 17, 2017 11:00 PM > >> To: Peng Fan <peng.fan@nxp.com> > >> Cc: Stefano Babic <sbabic@denx.de>; U-Boot-Denx > >> <u-boot@lists.denx.de> > >> Subject: Re: [U-Boot] [PATCH V2 12/12] imx: mx7dsabresd: add board > >> revision check > >> > >> On Thu, Apr 13, 2017 at 3:10 AM, Peng Fan <peng.fan@nxp.com> wrote: > >> > >>> +#define BOARD_REV_C 0x300 > >>> +#define BOARD_REV_B 0x200 > >>> +#define BOARD_REV_A 0x100 > >>> + > >>> +static int mx7sabre_rev(void) > >>> +{ > >>> + /* > >>> + * Get Board ID information from OCOTP_GP1[15:8] > >>> + * i.MX7D SDB RevA: 0x41 > >>> + * i.MX7D SDB RevB: 0x42 > >> > >> Isn't this versioning scheme shared with other NXP boards? If so, it > >> would be better to put this in common code. > > > > I prefer to keep the code here. There are board revision fuse for the > > boards from NXP, but this is not always true, I think. > > > > Patches 1..11 are free of comments. I am merging them and I will send PR to > Tom for inclusion after my build test. I will just let this last one out. It is ok for me to let this out. Thanks. Thanks, Peng. > > Regards, > Stefano > > > -- > ============================================================ > ========= > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > ============================================================ > =========
On 18/05/2017 10:51, Peng Fan wrote: >> Patches 1..11 are free of comments. I am merging them and I will send PR to >> Tom for inclusion after my build test. I will just let this last one out. > > It is ok for me to let this out. Thanks. ok - just check with patch I send today to fix the "secure" variant, then I could insert this to my PR. Thanks, Stefano
diff --git a/board/freescale/mx7dsabresd/mx7dsabresd.c b/board/freescale/mx7dsabresd/mx7dsabresd.c index ecea5a5..07392fa 100644 --- a/board/freescale/mx7dsabresd/mx7dsabresd.c +++ b/board/freescale/mx7dsabresd/mx7dsabresd.c @@ -82,6 +82,48 @@ static iomux_v3_cfg_t const uart1_pads[] = { MX7D_PAD_UART1_RX_DATA__UART1_DCE_RX | MUX_PAD_CTRL(UART_PAD_CTRL), }; +#define BOARD_REV_C 0x300 +#define BOARD_REV_B 0x200 +#define BOARD_REV_A 0x100 + +static int mx7sabre_rev(void) +{ + /* + * Get Board ID information from OCOTP_GP1[15:8] + * i.MX7D SDB RevA: 0x41 + * i.MX7D SDB RevB: 0x42 + */ + struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR; + struct fuse_bank *bank = &ocotp->bank[14]; + int reg = readl(&bank->fuse_regs[0]); + int ret; + + if (reg != 0) { + switch (reg >> 8 & 0x0F) { + case 0x3: + ret = BOARD_REV_C; + break; + case 0x02: + ret = BOARD_REV_B; + break; + case 0x01: + default: + ret = BOARD_REV_A; + break; + } + } else { + /* If the gp1 fuse is not burn, we have to use TO rev for the board rev */ + if (is_soc_rev(CHIP_REV_1_0)) + ret = BOARD_REV_A; + else if (is_soc_rev(CHIP_REV_1_1)) + ret = BOARD_REV_B; + else + ret = BOARD_REV_C; + } + + return ret; +} + #ifdef CONFIG_NAND_MXS static iomux_v3_cfg_t const gpmi_pads[] = { MX7D_PAD_SD3_DATA0__NAND_DATA00 | MUX_PAD_CTRL(NAND_PAD_CTRL), @@ -377,14 +419,29 @@ int board_late_init(void) int checkboard(void) { + int rev = mx7sabre_rev(); char *mode; + char *revname; if (IS_ENABLED(CONFIG_ARMV7_BOOT_SEC_DEFAULT)) mode = "secure"; else mode = "non-secure"; - printf("Board: i.MX7D SABRESD in %s mode\n", mode); + switch (rev) { + case BOARD_REV_C: + revname = "C"; + break; + case BOARD_REV_B: + revname = "B"; + break; + case BOARD_REV_A: + default: + revname = "A"; + break; + } + + printf("Board: i.MX7D SABRESD Rev%s in %s mode\n", revname, mode); return 0; }
Add board revision check Signed-off-by: Peng Fan <peng.fan@nxp.com> --- board/freescale/mx7dsabresd/mx7dsabresd.c | 59 ++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)