[U-Boot] [PATCH 1/1] arm: mx5: Add fuse supply gate enable in fsl_iim
Enable fuse supply gate before fuse programming and disable after.
Signed-off-by: Sergey Alyoshin alyoshin.s@gmail.com Tested-by: Sergey Alyoshin alyoshin.s@gmail.com --- arch/arm/cpu/armv7/mx5/clock.c | 12 ++++++++++++ arch/arm/include/asm/arch-mx5/clock.h | 1 + arch/arm/include/asm/arch-mx5/crm_regs.h | 3 +++ drivers/misc/fsl_iim.c | 18 +++++++++++++++++- 4 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/mx5/clock.c b/arch/arm/cpu/armv7/mx5/clock.c index fb3b128..10c1e34 100644 --- a/arch/arm/cpu/armv7/mx5/clock.c +++ b/arch/arm/cpu/armv7/mx5/clock.c @@ -749,6 +749,18 @@ void enable_nfc_clk(unsigned char enable) MXC_CCM_CCGR5_EMI_ENFC(cg)); }
+#ifdef CONFIG_FSL_IIM +void enable_efuse_prog_gate(unsigned char enable) +{ + if (enable) + setbits_le32(&mxc_ccm->cgpr, + MXC_CCM_CGPR_EFUSE_PROG_SUPPLY_GATE); + else + clrbits_le32(&mxc_ccm->cgpr, + MXC_CCM_CGPR_EFUSE_PROG_SUPPLY_GATE); +} +#endif + /* Config main_bus_clock for periphs */ static int config_periph_clk(u32 ref, u32 freq) { diff --git a/arch/arm/include/asm/arch-mx5/clock.h b/arch/arm/include/asm/arch-mx5/clock.h index 9ee79ae..5f3927a 100644 --- a/arch/arm/include/asm/arch-mx5/clock.h +++ b/arch/arm/include/asm/arch-mx5/clock.h @@ -53,5 +53,6 @@ void enable_usboh3_clk(bool enable); void mxc_set_sata_internal_clock(void); int enable_i2c_clk(unsigned char enable, unsigned i2c_num); void enable_nfc_clk(unsigned char enable); +void enable_efuse_prog_gate(unsigned char enable);
#endif /* __ASM_ARCH_CLOCK_H */ diff --git a/arch/arm/include/asm/arch-mx5/crm_regs.h b/arch/arm/include/asm/arch-mx5/crm_regs.h index 392881c..efe57e0 100644 --- a/arch/arm/include/asm/arch-mx5/crm_regs.h +++ b/arch/arm/include/asm/arch-mx5/crm_regs.h @@ -305,6 +305,9 @@ struct mxc_ccm_reg { /* Define the bits in register CCDR */ #define MXC_CCM_CCDR_IPU_HS_MASK (0x1 << 17)
+/* Define the bits in register CGPR */ +#define MXC_CCM_CGPR_EFUSE_PROG_SUPPLY_GATE (1 << 4) + /* Define the bits in register CCGRx */ #define MXC_CCM_CCGR_CG_MASK 0x3 #define MXC_CCM_CCGR_CG_OFF 0x0 diff --git a/drivers/misc/fsl_iim.c b/drivers/misc/fsl_iim.c index 44ae7b1..8529141 100644 --- a/drivers/misc/fsl_iim.c +++ b/drivers/misc/fsl_iim.c @@ -16,6 +16,9 @@ #ifndef CONFIG_MPC512X #include <asm/arch/imx-regs.h> #endif +#if defined(CONFIG_MX51) || defined(CONFIG_MX53) +#include <asm/arch/clock.h> +#endif
/* FSL IIM-specific constants */ #define STAT_BUSY 0x80 @@ -74,6 +77,15 @@ #error Endianess is not defined: please fix to continue #endif
+#if defined(CONFIG_MX51) || defined(CONFIG_MX53) +static inline void enable_fuse_prog(unsigned char enable) +{ + enable_efuse_prog_gate(enable); +} +#else +static inline void enable_fuse_prog(unsigned char enable) {} +#endif + /* IIM control registers */ struct fsl_iim { u32 stat; @@ -237,12 +249,16 @@ int fuse_prog(u32 bank, u32 word, u32 val) if (ret) return ret;
+ enable_fuse_prog(1); for (bit = 0; val; bit++, val >>= 1) if (val & 0x01) { ret = prog_bit(regs, bank, word, bit); - if (ret) + if (ret) { + enable_fuse_prog(0); return ret; + } } + enable_fuse_prog(0);
return 0; }
Dear Sergey Alyoshin,
On Thursday, December 12, 2013 5:46:21 PM, Sergey Alyoshin wrote:
Enable fuse supply gate before fuse programming and disable after.
Signed-off-by: Sergey Alyoshin alyoshin.s@gmail.com Tested-by: Sergey Alyoshin alyoshin.s@gmail.com
Have you also tested without this patch first too? On which SoC?
My tests had shown that this is not required on i.MX51. It's probably the same on i.MX53. I have not checked if i.MX25 and i.MX35 also have such a bit, but they also work fine as is. This is not even done by Freescale: http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/cha...
In the Reference Manual, nothing says in the IIM / fuse chapters that this bit is needed for proper programming. It is just described in the CCM register map, and nothing refers to it. It is perhaps only useful for test purposes. I'd vote for letting this bit untouched if possible.
Fabio, do you have more information from Freescale about this bit?
Best regards, Benoît
On Friday, December 13, 2013 1:52:19 AM, Benoît Thébaudeau wrote:
Dear Sergey Alyoshin,
On Thursday, December 12, 2013 5:46:21 PM, Sergey Alyoshin wrote:
Enable fuse supply gate before fuse programming and disable after.
Signed-off-by: Sergey Alyoshin alyoshin.s@gmail.com Tested-by: Sergey Alyoshin alyoshin.s@gmail.com
Have you also tested without this patch first too? On which SoC?
My tests had shown that this is not required on i.MX51. It's probably the same on i.MX53. I have not checked if i.MX25 and i.MX35 also have such a bit, but they also work fine as is. This is not even done by Freescale: http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/cha...
In the Reference Manual, nothing says in the IIM / fuse chapters that this bit is needed for proper programming. It is just described in the CCM register map, and nothing refers to it. It is perhaps only useful for test purposes. I'd vote for letting this bit untouched if possible.
Fabio, do you have more information from Freescale about this bit?
Also note that the data sheets of the i.MX51 and i.MX53 both mention a dedicated VDD_FUSE pin, with several notes saying that it should not be powered if programming is not required in order to avoid inadvertently blowing fuses in read mode. These notes would not be required if this bit gating this supply by default really did exactly that. I think that it is rather an internal feature for Freescale only, e.g. to blow Freescale bits like UID in die production.
Best regards, Benoît
On Fri, Dec 13, 2013 at 5:04 AM, Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
On Friday, December 13, 2013 1:52:19 AM, Benoît Thébaudeau wrote:
Dear Sergey Alyoshin,
On Thursday, December 12, 2013 5:46:21 PM, Sergey Alyoshin wrote:
Enable fuse supply gate before fuse programming and disable after.
Signed-off-by: Sergey Alyoshin alyoshin.s@gmail.com Tested-by: Sergey Alyoshin alyoshin.s@gmail.com
Have you also tested without this patch first too? On which SoC?
My tests had shown that this is not required on i.MX51. It's probably the same on i.MX53. I have not checked if i.MX25 and i.MX35 also have such a bit, but they also work fine as is. This is not even done by Freescale: http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/cha...
In the Reference Manual, nothing says in the IIM / fuse chapters that this bit is needed for proper programming. It is just described in the CCM register map, and nothing refers to it. It is perhaps only useful for test purposes. I'd vote for letting this bit untouched if possible.
Fabio, do you have more information from Freescale about this bit?
Also note that the data sheets of the i.MX51 and i.MX53 both mention a dedicated VDD_FUSE pin, with several notes saying that it should not be powered if programming is not required in order to avoid inadvertently blowing fuses in read mode. These notes would not be required if this bit gating this supply by default really did exactly that. I think that it is rather an internal feature for Freescale only, e.g. to blow Freescale bits like UID in die production.
I apply 3.3V on VDD_FUSE pin and still without setting EFUSE_PROG_SUPPLY_GATE bank 1 was not programmed on i.MX53.
Hello, Benoît,
On Fri, Dec 13, 2013 at 4:52 AM, Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
On Thursday, December 12, 2013 5:46:21 PM, Sergey Alyoshin wrote:
Enable fuse supply gate before fuse programming and disable after.
Signed-off-by: Sergey Alyoshin alyoshin.s@gmail.com Tested-by: Sergey Alyoshin alyoshin.s@gmail.com
Have you also tested without this patch first too? On which SoC?
I have tried to write MAC address in fuse (bank 1) on i.MX53 custom board and 'fuse sense' show no change, reboot show no change either.
I have not tested this on i.MX51, but this register and bit is the same on i.MX51 with exactly the same description. With this patch I have successfully written MAC addresses on several i.MX53 boards.
In Linux 2.6.35 from Freescale this bit is also set for fuse programming, e.g. in arch/arm/mach-mx5/mx51_babbage.c and arch/arm/mach-mx5/mx53_loco.c in mxc_iim_enable_fuse().
My tests had shown that this is not required on i.MX51. It's probably the same on i.MX53. I have not checked if i.MX25 and i.MX35 also have such a bit, but they also work fine as is. This is not even done by Freescale: http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/cha...
In the Reference Manual, nothing says in the IIM / fuse chapters that this bit is needed for proper programming. It is just described in the CCM register map, and nothing refers to it. It is perhaps only useful for test purposes. I'd vote for letting this bit untouched if possible.
Fabio, do you have more information from Freescale about this bit?
Dear Sergey Alyoshin,
On Thursday, December 12, 2013 5:46:21 PM, Sergey Alyoshin wrote:
Enable fuse supply gate before fuse programming and disable after.
Signed-off-by: Sergey Alyoshin alyoshin.s@gmail.com Tested-by: Sergey Alyoshin alyoshin.s@gmail.com
You convinced me. ;)
arch/arm/cpu/armv7/mx5/clock.c | 12 ++++++++++++ arch/arm/include/asm/arch-mx5/clock.h | 1 + arch/arm/include/asm/arch-mx5/crm_regs.h | 3 +++ drivers/misc/fsl_iim.c | 18 +++++++++++++++++- 4 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/mx5/clock.c b/arch/arm/cpu/armv7/mx5/clock.c index fb3b128..10c1e34 100644 --- a/arch/arm/cpu/armv7/mx5/clock.c +++ b/arch/arm/cpu/armv7/mx5/clock.c @@ -749,6 +749,18 @@ void enable_nfc_clk(unsigned char enable) MXC_CCM_CCGR5_EMI_ENFC(cg)); }
+#ifdef CONFIG_FSL_IIM +void enable_efuse_prog_gate(unsigned char enable)
Perhaps enable_efuse_prog_supply would be a better naming.
And bool would be more appropriate than unsigned char.
+{
- if (enable)
setbits_le32(&mxc_ccm->cgpr,MXC_CCM_CGPR_EFUSE_PROG_SUPPLY_GATE);- else
clrbits_le32(&mxc_ccm->cgpr,MXC_CCM_CGPR_EFUSE_PROG_SUPPLY_GATE);+} +#endif
/* Config main_bus_clock for periphs */ static int config_periph_clk(u32 ref, u32 freq) { diff --git a/arch/arm/include/asm/arch-mx5/clock.h b/arch/arm/include/asm/arch-mx5/clock.h index 9ee79ae..5f3927a 100644 --- a/arch/arm/include/asm/arch-mx5/clock.h +++ b/arch/arm/include/asm/arch-mx5/clock.h @@ -53,5 +53,6 @@ void enable_usboh3_clk(bool enable); void mxc_set_sata_internal_clock(void); int enable_i2c_clk(unsigned char enable, unsigned i2c_num); void enable_nfc_clk(unsigned char enable); +void enable_efuse_prog_gate(unsigned char enable);
Ditto for the function declaration.
#endif /* __ASM_ARCH_CLOCK_H */ diff --git a/arch/arm/include/asm/arch-mx5/crm_regs.h b/arch/arm/include/asm/arch-mx5/crm_regs.h index 392881c..efe57e0 100644 --- a/arch/arm/include/asm/arch-mx5/crm_regs.h +++ b/arch/arm/include/asm/arch-mx5/crm_regs.h @@ -305,6 +305,9 @@ struct mxc_ccm_reg { /* Define the bits in register CCDR */ #define MXC_CCM_CCDR_IPU_HS_MASK (0x1 << 17)
+/* Define the bits in register CGPR */ +#define MXC_CCM_CGPR_EFUSE_PROG_SUPPLY_GATE (1 << 4)
/* Define the bits in register CCGRx */ #define MXC_CCM_CCGR_CG_MASK 0x3 #define MXC_CCM_CCGR_CG_OFF 0x0 diff --git a/drivers/misc/fsl_iim.c b/drivers/misc/fsl_iim.c index 44ae7b1..8529141 100644 --- a/drivers/misc/fsl_iim.c +++ b/drivers/misc/fsl_iim.c @@ -16,6 +16,9 @@ #ifndef CONFIG_MPC512X #include <asm/arch/imx-regs.h> #endif +#if defined(CONFIG_MX51) || defined(CONFIG_MX53) +#include <asm/arch/clock.h> +#endif
/* FSL IIM-specific constants */ #define STAT_BUSY 0x80 @@ -74,6 +77,15 @@ #error Endianess is not defined: please fix to continue #endif
+#if defined(CONFIG_MX51) || defined(CONFIG_MX53) +static inline void enable_fuse_prog(unsigned char enable) +{
- enable_efuse_prog_gate(enable);
+} +#else +static inline void enable_fuse_prog(unsigned char enable) {} +#endif
Please drop the 'inline', and move this function after the struct fsl_iim type definition, i.e. right before prepare_access().
Also, you don't have to redefine a function, so you could just do: +#if !defined(CONFIG_MX51) && !defined(CONFIG_MX53) +static void enable_efuse_prog_supply(bool enable) {} +#endif +
/* IIM control registers */ struct fsl_iim { u32 stat; @@ -237,12 +249,16 @@ int fuse_prog(u32 bank, u32 word, u32 val) if (ret) return ret;
- enable_fuse_prog(1); for (bit = 0; val; bit++, val >>= 1) if (val & 0x01) { ret = prog_bit(regs, bank, word, bit);
if (ret)
if (ret) {enable_fuse_prog(0); return ret;}}
enable_fuse_prog(0);
return 0;
}
1.7.10.4
Best regards, Benoît
On Friday, December 13, 2013 1:42:10 PM, Benoît Thébaudeau wrote:
On Thursday, December 12, 2013 5:46:21 PM, Sergey Alyoshin wrote:
+#if defined(CONFIG_MX51) || defined(CONFIG_MX53) +static inline void enable_fuse_prog(unsigned char enable) +{
- enable_efuse_prog_gate(enable);
+} +#else +static inline void enable_fuse_prog(unsigned char enable) {} +#endif
Please drop the 'inline', and move this function after the struct fsl_iim type definition, i.e. right before prepare_access().
Also, you don't have to redefine a function, so you could just do: +#if !defined(CONFIG_MX51) && !defined(CONFIG_MX53) +static void enable_efuse_prog_supply(bool enable) {} +#endif
Or even better:
+#if !defined(CONFIG_MX51) && !defined(CONFIG_MX53) +#define enable_efuse_prog_supply(enable) +#endif
Best regards, Benoît
participants (2)
-
Benoît Thébaudeau -
Sergey Alyoshin