| Message ID | 20190424202607.23522-4-pure.logic@nexus-software.ie (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add i.MX8MM support | expand |
On 24.04.2019 23:26, Bryan O'Donoghue wrote: > The RELAX field of the OCOTP block is turning out as a zero on i.MX8MM. > This messes up the subsequent re-load of the fuse shadow registers. > > After some discussion with people @ NXP its clear we have missed a trick > here in Linux. > > The OCOTP fuse programming time has a physical minimum 'burn time' that is > not related to the ipg_clk. > > We need to define the RELAX, STROBE_READ and STROBE_PROG fields in terms of > desired timings to allow for the burn-in to safely complete. Right now only > the RELAX field is calculated in terms of an absolute time and we are > ending up with a value of zero. > > This patch inherits the u-boot timings for the OCOTP_TIMING calculation on > the i.MX6 and i.MX8. Those timings are known to work and critically specify > values such as STROBE_PROG as a minimum timing. > > Fixes: 0642bac7da42 ("nvmem: imx-ocotp: add write support") > > Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie> > -#define DEF_RELAX 20 /* > 16.5ns */ > +#define BV_TIMING_STROBE_PROG_US 10 /* Min time to blow a fuse */ > +#define BV_TIMING_STROBE_READ_NS 37 /* Min time before read */ > +#define BV_TIMING_RELAX_NS 17 I don't know what the BV_ prefix from uboot means exactly. It's combined with some bit field macros so maybe "Bit Value"? This prefix seems meaningless in kernel. > @@ -182,12 +184,38 @@ static void imx_ocotp_set_imx6_timing(struct ocotp_priv *priv) > * fields with timing values to match the current frequency of the > * ipg_clk. OTP writes will work at maximum bus frequencies as long > * as the HW_OCOTP_TIMING parameters are set correctly. > + * > + * Note: there are minimum timings required to ensure an OTP fuse burns > + * correctly that are independent of the ipg_clk. Those values are not > + * formally documented anywhere however, working from the minimum > + * timings given in u-boot we can say: > + * > + * - Minimum STROBE_PROG time is 10 microseconds. Intuitively 10 > + * microseconds feels about right as representative of a minimum time > + * to physically burn out a fuse. > + * > + * - Minimum STROBE_READ i.e. the time to wait post OTP fuse burn before > + * performing another read is 37 nanoseconds > + * > + * - Minimum RELAX timing is 17 nanoseconds. This final RELAX minimum > + * timing is not entirely clear the documentation says "This > + * count value specifies the time to add to all default timing > + * parameters other than the Tpgm and Trd. It is given in number > + * of ipg_clk periods." where Tpgm and Trd refer to STROBE_PROG > + * and STROBE_READ respectively. What the other timing parameters > + * are though, is not specified. Experience shows a zero RELAX > + * value will mess up a re-load of the shadow registers post OTP > + * burn. > */ > clk_rate = clk_get_rate(priv->clk); > > - relax = clk_rate / (1000000000 / DEF_RELAX) - 1; > - strobe_prog = clk_rate / (1000000000 / 10000) + 2 * (DEF_RELAX + 1) - 1; > - strobe_read = clk_rate / (1000000000 / 40) + 2 * (DEF_RELAX + 1) - 1; > + relax = DIV_ROUND_UP(clk_rate * BV_TIMING_RELAX_NS, 1000000000) - 1; > + strobe_read = DIV_ROUND_UP(clk_rate * BV_TIMING_STROBE_READ_NS, > + 1000000000); > + strobe_read += 2 * (relax + 1) - 1; > + strobe_prog = DIV_ROUND_CLOSEST(clk_rate * BV_TIMING_STROBE_PROG_US, > + 1000000); > + strobe_prog += 2 * (relax + 1) - 1; Other than constant naming series whole looks good to me: Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com> -- Regards, Leonard
diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c index 85a7d0da3abb..826812d3332f 100644 --- a/drivers/nvmem/imx-ocotp.c +++ b/drivers/nvmem/imx-ocotp.c @@ -50,7 +50,9 @@ #define IMX_OCOTP_BM_CTRL_ERROR 0x00000200 #define IMX_OCOTP_BM_CTRL_REL_SHADOWS 0x00000400 -#define DEF_RELAX 20 /* > 16.5ns */ +#define BV_TIMING_STROBE_PROG_US 10 /* Min time to blow a fuse */ +#define BV_TIMING_STROBE_READ_NS 37 /* Min time before read */ +#define BV_TIMING_RELAX_NS 17 #define DEF_FSOURCE 1001 /* > 1000 ns */ #define DEF_STROBE_PROG 10000 /* IPG clocks */ #define IMX_OCOTP_WR_UNLOCK 0x3E770000 @@ -182,12 +184,38 @@ static void imx_ocotp_set_imx6_timing(struct ocotp_priv *priv) * fields with timing values to match the current frequency of the * ipg_clk. OTP writes will work at maximum bus frequencies as long * as the HW_OCOTP_TIMING parameters are set correctly. + * + * Note: there are minimum timings required to ensure an OTP fuse burns + * correctly that are independent of the ipg_clk. Those values are not + * formally documented anywhere however, working from the minimum + * timings given in u-boot we can say: + * + * - Minimum STROBE_PROG time is 10 microseconds. Intuitively 10 + * microseconds feels about right as representative of a minimum time + * to physically burn out a fuse. + * + * - Minimum STROBE_READ i.e. the time to wait post OTP fuse burn before + * performing another read is 37 nanoseconds + * + * - Minimum RELAX timing is 17 nanoseconds. This final RELAX minimum + * timing is not entirely clear the documentation says "This + * count value specifies the time to add to all default timing + * parameters other than the Tpgm and Trd. It is given in number + * of ipg_clk periods." where Tpgm and Trd refer to STROBE_PROG + * and STROBE_READ respectively. What the other timing parameters + * are though, is not specified. Experience shows a zero RELAX + * value will mess up a re-load of the shadow registers post OTP + * burn. */ clk_rate = clk_get_rate(priv->clk); - relax = clk_rate / (1000000000 / DEF_RELAX) - 1; - strobe_prog = clk_rate / (1000000000 / 10000) + 2 * (DEF_RELAX + 1) - 1; - strobe_read = clk_rate / (1000000000 / 40) + 2 * (DEF_RELAX + 1) - 1; + relax = DIV_ROUND_UP(clk_rate * BV_TIMING_RELAX_NS, 1000000000) - 1; + strobe_read = DIV_ROUND_UP(clk_rate * BV_TIMING_STROBE_READ_NS, + 1000000000); + strobe_read += 2 * (relax + 1) - 1; + strobe_prog = DIV_ROUND_CLOSEST(clk_rate * BV_TIMING_STROBE_PROG_US, + 1000000); + strobe_prog += 2 * (relax + 1) - 1; timing = readl(priv->base + IMX_OCOTP_ADDR_TIMING) & 0x0FC00000; timing |= strobe_prog & 0x00000FFF;
The RELAX field of the OCOTP block is turning out as a zero on i.MX8MM. This messes up the subsequent re-load of the fuse shadow registers. After some discussion with people @ NXP its clear we have missed a trick here in Linux. The OCOTP fuse programming time has a physical minimum 'burn time' that is not related to the ipg_clk. We need to define the RELAX, STROBE_READ and STROBE_PROG fields in terms of desired timings to allow for the burn-in to safely complete. Right now only the RELAX field is calculated in terms of an absolute time and we are ending up with a value of zero. This patch inherits the u-boot timings for the OCOTP_TIMING calculation on the i.MX6 and i.MX8. Those timings are known to work and critically specify values such as STROBE_PROG as a minimum timing. Fixes: 0642bac7da42 ("nvmem: imx-ocotp: add write support") Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie> Suggested-by: Leonard Crestez <leonard.crestez@nxp.com> --- drivers/nvmem/imx-ocotp.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-)