Re: [U-Boot] [PATCH V2 12/12] imx: mx7dsabresd: add board revision check

11 May 2017


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