From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Babic Date: Tue, 1 Sep 2015 09:24:24 +0200 Subject: [U-Boot] [PATCH V2] mxc: ocotp fix hole in shadow registers In-Reply-To: <20150901010624.GB12983@shlinux2> References: <1440574847-22446-1-git-send-email-Peng.Fan@freescale.com> <55E4876B.1040701@denx.de> <55E48946.8000506@denx.de> <20150901010624.GB12983@shlinux2> Message-ID: <55E552A8.30200@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 01/09/2015 03:06, Peng Fan wrote: > Hi Stefano, > > On Mon, Aug 31, 2015 at 07:05:10PM +0200, Stefano Babic wrote: >> On 31/08/2015 18:57, Stefano Babic wrote: >>> On 26/08/2015 09:40, Peng Fan wrote: >>>> There is a hole in shadow registers address map of size 0x100 >>>> between bank 5 and bank 6 on iMX6QP, iMX6DQ, iMX6SDL, iMX6SX and iMX6UL. >>>> Bank 5 ends at 0x6F0 and Bank 6 starts at 0x800. When reading the fuses, >>>> we should account for this hole in address space. >>>> >>>> Similar hole exists between bank 14 and bank 15 of size >>>> 0x80 on iMX6QP, iMX6DQ, iMX6SDL and iMX6SX. >>>> Note: iMX6SL has only 0-7 banks and there is no hole. >>>> Note: iMX6UL doesn't have this one. >>>> >>>> When reading, we use register offset, so need to account for holes >>>> to get the correct address. >>>> When writing, we use bank/word index, there is no need to account >>>> for holes, always use bank/word index from fuse map. >>>> >>>> Signed-off-by: Peng Fan >>>> Cc: Stefano Babic >>>> Cc: Fabio Estevam >>>> --- >>>> >>>> Changes v2: >>>> Discard is_cpu_type(MXC_CPU_MX7D), since 7D cpu type is still not upstreamed. >>>> >>>> arch/arm/include/asm/arch-mx6/imx-regs.h | 3 +- >>>> drivers/misc/mxc_ocotp.c | 78 ++++++++++++++++++++++++++++++-- >>>> 2 files changed, 75 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h >>>> index 4d84a9b..a685ed2 100644 >>>> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h >>>> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h >>>> @@ -630,9 +630,10 @@ struct ocotp_regs { >>>> u32 version; >>>> u32 rsvd7[0xdb]; >>>> >>>> + /* fuse banks */ >>>> struct fuse_bank { >>>> u32 fuse_regs[0x20]; >>>> - } bank[16]; >>>> + } bank[0]; >>>> }; >>>> >>>> struct fuse_bank0_regs { >>>> diff --git a/drivers/misc/mxc_ocotp.c b/drivers/misc/mxc_ocotp.c >>>> index d9b02c7..65ff815 100644 >>>> --- a/drivers/misc/mxc_ocotp.c >>>> +++ b/drivers/misc/mxc_ocotp.c >>>> @@ -57,6 +57,68 @@ >>>> >>>> #define WRITE_POSTAMBLE_US 2 >>>> >>>> +#if defined(CONFIG_MX6) || defined(CONFIG_VF610) >>>> +#define FUSE_BANK_SIZE 0x80 >>>> +#ifdef CONFIG_MX6SL >>>> +#define FUSE_BANKS 8 >>>> +#else >>>> +#define FUSE_BANKS 16 >>>> +#endif >>>> +#elif defined CONFIG_MX7 >>>> +#define FUSE_BANK_SIZE 0x40 >>>> +#define FUSE_BANKS 16 >>>> +#else >>>> +#error "Unsupported architecture\n" >>>> +#endif >>>> + >>>> +#if defined(CONFIG_MX6) >>>> +#include >>>> + >>>> +/* >>>> + * There is a hole in shadow registers address map of size 0x100 >>>> + * between bank 5 and bank 6 on iMX6QP, iMX6DQ, iMX6SDL, iMX6SX and iMX6UL. >>>> + * Bank 5 ends at 0x6F0 and Bank 6 starts at 0x800. When reading the fuses, >>>> + * we should account for this hole in address space. >>>> + * >>>> + * Similar hole exists between bank 14 and bank 15 of size >>>> + * 0x80 on iMX6QP, iMX6DQ, iMX6SDL and iMX6SX. >>>> + * Note: iMX6SL has only 0-7 banks and there is no hole. >>>> + * Note: iMX6UL doesn't have this one. >>>> + * >>>> + * This function is to covert user input to physical bank index. >>>> + * Only needed when read fuse, because we use register offset, so >>>> + * need to calculate real register offset. >>>> + * When write, no need to consider hole, always use the bank/word >>>> + * index from fuse map. >>>> + */ >>>> +u32 fuse_bank_physical(int index) >>>> +{ >>>> + u32 phy_index; >>>> + >>>> + if (is_cpu_type(MXC_CPU_MX6SL)) { >>>> + phy_index = index; >>>> + } else if (is_cpu_type(MXC_CPU_MX6UL)) { >>>> + if (index >= 6) >>>> + phy_index = fuse_bank_physical(5) + (index - 6) + 3; >>>> + else >>>> + phy_index = index; >>>> + } else { >>>> + if (index >= 15) >>>> + phy_index = fuse_bank_physical(14) + (index - 15) + 2; >>>> + else if (index >= 6) >>>> + phy_index = fuse_bank_physical(5) + (index - 6) + 3; >>>> + else >>>> + phy_index = index; >>>> + } >>>> + return phy_index; >>>> +} >>>> +#else >>>> +u32 fuse_bank_physical(int index) >>>> +{ >>>> + return index; >>>> +} >>>> +#endif >>>> + >>>> static void wait_busy(struct ocotp_regs *regs, unsigned int delay_us) >>>> { >>>> while (readl(®s->ctrl) & BM_CTRL_BUSY) >>>> @@ -73,9 +135,9 @@ static int prepare_access(struct ocotp_regs **regs, u32 bank, u32 word, >>>> { >>>> *regs = (struct ocotp_regs *)OCOTP_BASE_ADDR; >>>> >>>> - if (bank >= ARRAY_SIZE((*regs)->bank) || >>>> - word >= ARRAY_SIZE((*regs)->bank[0].fuse_regs) >> 2 || >>>> - !assert) { >>>> + if (bank >= FUSE_BANKS || >>>> + word >= ARRAY_SIZE((*regs)->bank[0].fuse_regs) >> 2 || >>>> + !assert) { >>>> printf("mxc_ocotp %s(): Invalid argument\n", caller); >>>> return -EINVAL; >>>> } >>>> @@ -113,12 +175,15 @@ int fuse_read(u32 bank, u32 word, u32 *val) >>>> { >>>> struct ocotp_regs *regs; >>>> int ret; >>>> + u32 phy_bank; >>>> >>>> ret = prepare_read(®s, bank, word, val, __func__); >>>> if (ret) >>>> return ret; >>>> >>>> - *val = readl(®s->bank[bank].fuse_regs[word << 2]); >>>> + phy_bank = fuse_bank_physical(bank); >>>> + >>>> + *val = readl(®s->bank[phy_bank].fuse_regs[word << 2]); >>>> >>>> return finish_access(regspeng, __func__); >>>> } >>>> @@ -259,12 +324,15 @@ int fuse_override(u32 bank, u32 word, u32 val) >>>> { >>>> struct ocotp_regs *regs; >>>> int ret; >>>> + u32 phy_bank; >>>> >>>> ret = prepare_write(®s, bank, word, __func__); >>>> if (ret) >>>> return ret; >>>> >>>> - writel(val, ®s->bank[bank].fuse_regs[word << 2]); >>>> + phy_bank = fuse_bank_physical(bank); >>>> + >>>> + writel(val, ®s->bank[phy_bank].fuse_regs[word << 2]); >>>> >>>> return finish_access(regs, __func__); >>>> } >>>> >>> Applied to u-boot-imx, thanks ! >>> >> >> Sorry, it is not. It lokks like this breaks mx6 boards. Can you take a >> look, please ? > > The is_cpu_type only effects on mx6. > > I applied this following patch: > https://patchwork.ozlabs.org/patch/512669/ > > MAKEALL -s mx6 and MAKEALL -s vf610 does not trigger build errors. No - the error is not due to the missing get_cpu_rev() in VF610, but it has something to do with MX7. The reported error is: arm: + mx6dlarm2_lpddr2 +drivers/misc/mxc_ocotp.c: In function 'fuse_bank_physical': +drivers/misc/mxc_ocotp.c:99:6: error: 'MXC_CPU_MX7D' undeclared (first use in this function) +drivers/misc/mxc_ocotp.c:99:6: note: each undeclared identifier is reported only once for each function it appears in +make[3]: *** [drivers/misc/mxc_ocotp.o] Error 1 +make[2]: *** [drivers/misc] Error 2 +make[1]: *** [drivers] Error 2 +make: *** [sub-make] Error 2 arm: + nitrogen6s1g +drivers/misc/mxc_ocotp.c: In function 'fuse_bank_physical': +drivers/misc/mxc_ocotp.c:99:6: error: 'MXC_CPU_MX7D' undeclared (first use in this function) +drivers/misc/mxc_ocotp.c:99:6: note: each undeclared identifier is reported only once for each function it appears in +make[3]: *** [drivers/misc/mxc_ocotp.o] Error 1 +make[2]: *** [drivers/misc] Error 2 +make[1]: *** [drivers] Error 2 +make: *** [sub-make] Error 2 By the way, you should not use MAKEALL anymore. This was replaced by the more powerful Simon's tool "buildman". ./to0ls/buildman/buildman mx6 Best regards, Stefano Babic -- ===================================================================== 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 at denx.de =====================================================================