[PATCH v5 0/5] Add i.MX8MM support

[PATCH v5 0/5] Add i.MX8MM support

[Bryan O'Donoghue]

V5:
- Adopt u-boot method of calculating timings.
  On the basis that the OTP registers have a programming time that is not
  related to the ipg_clk rate specify the various timing inputs to the
  RELAX, STROBE_READ and STROBE_PROG as-is done in u-boot.

  The wait time to burn a given OTP fuse is not documented anywhere except
  in code in u-boot.

  The ipg_clk then is used to clock the registers in the OCOTP block and to
  tell the OCOTP block how long to wait for programming to complete and how
  long to delay before doing an automatic re-read of the registers.

  Tested on the i.MX8MM-EVK

  relax = 1 strobe_read 6 strobe_prog 670

V4:
- Change the RELAX fix to drop subtraction of -1 for all users - Leonard
- Expand register definition from the 60 documented OTP registers to the
  entire 256 registers putatively in the address space*
- Add Reviewed-by as indicated - Leonard
- Added Suggested-by where it made sense - Bryan

* Dumping the expanded address space shows that there are indeed OTP values
  present that can be read back from registers that are not formally
  documented for i.MX8MM eg.

Bank 20
        0x55000801
        0x00014d14
        0xd503201f
        0x55000801
Bank 21
        0x00014d20
        0xd503201f
        0x00000000
        0x00000000

V3:
- Fix commit log for the expanding the ADDR field i.MX6 uses seven not four
  bits, which is why the existing define says 0x7F not 0x0F - bod

V2:
- Rebased to linux-next/master to align with i.8MQ work
- Two patches dropped as a result of rebase
- Added patch to expand OCOTP_CTRL_ADDR to 8 bits for all users - Leonard
- Makes sure nregs = 60 not 64 for i.MX8MM
- Tested imx8mm-evk, imx7s-warp7

V1:
This set adds support for the i.MX8MM.

When adding support for this processor there are two interesting gotchas to
watch for.

#1 We current do not preserve the WAIT field for i.MX6 and since we are
   reusing the i.MX6 set_timing() values, this would also affect i.MX8.
   On the face of it, it appears to be an inocuous error with no real side
   effects.

#2 Secondly the i.MX8MM will calculate a zero value for the RELAX bit-field
   when programming up OTP fuses.
   This is fine for programming the fuses but, it introduces a strange
   failure state with reloading the shadow registers subsequent to blowing
   an OTP fuse.
   The second important patch here then is ensuring the RELAX field is
   non-zero to avoid the failure state.

Bryan O'Donoghue (5):
  nvmem: imx-ocotp: Elongate OCOTP_CTRL ADDR field to eight bits
  nvmem: imx-ocotp: Ensure WAIT bits are preserved when setting timing
  nvmem: imx-ocotp: Change TIMING calculation to u-boot algorithm
  nvmem: imx-ocotp: Add i.MX8MM support
  dt-bindings: imx-ocotp: Add i.MX8MM compatible

 .../devicetree/bindings/nvmem/imx-ocotp.txt   |  1 +
 drivers/nvmem/imx-ocotp.c                     | 48 ++++++++++++++++---
 2 files changed, 43 insertions(+), 6 deletions(-)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 1/5] nvmem: imx-ocotp: Elongate OCOTP_CTRL ADDR field to eight bits

[Bryan O'Donoghue]

i.MX6 defines OCOTP_CTRLn:ADDR as seven bit address-field with a one bit
RSVD0 field, i.MX7 defines OCOTP_CTRLn:ADDR as a four bit address-field
with a four bit RSVD0 field.

i.MX8 defines the OCOTP_CTRLn:ADDR bit-field as a full range eight bits.

i.MX6 and i.MX7 should return zero for their respective RSVD0 bits and
ignore a write-back of zero where i.MX8 will make use of the full range.

This patch expands the bit-field definition for all users to eight bits,
which is safe due to RSVD0 being a no-op for the i.MX6 and i.MX7.

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/nvmem/imx-ocotp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 4cf7b61e4bf5..6600c4ddeb51 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -45,7 +45,7 @@
 #define IMX_OCOTP_ADDR_DATA2		0x0040
 #define IMX_OCOTP_ADDR_DATA3		0x0050
 
-#define IMX_OCOTP_BM_CTRL_ADDR		0x0000007F
+#define IMX_OCOTP_BM_CTRL_ADDR		0x000000FF
 #define IMX_OCOTP_BM_CTRL_BUSY		0x00000100
 #define IMX_OCOTP_BM_CTRL_ERROR		0x00000200
 #define IMX_OCOTP_BM_CTRL_REL_SHADOWS	0x00000400
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 2/5] nvmem: imx-ocotp: Ensure WAIT bits are preserved when setting timing

[Bryan O'Donoghue]

The i.MX6 and i.MX8 both have a bit-field spanning bits 27:22 called the
WAIT field.

The WAIT field according to the documentation for both parts "specifies
time interval between auto read and write access in one time program. It is
given in number of ipg_clk periods."

This patch ensures that the relevant field is read and written back to the
timing register.

Fixes: 0642bac7da42 ("nvmem: imx-ocotp: add write support")

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/nvmem/imx-ocotp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 6600c4ddeb51..85a7d0da3abb 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -189,7 +189,8 @@ static void imx_ocotp_set_imx6_timing(struct ocotp_priv *priv)
 	strobe_prog = clk_rate / (1000000000 / 10000) + 2 * (DEF_RELAX + 1) - 1;
 	strobe_read = clk_rate / (1000000000 / 40) + 2 * (DEF_RELAX + 1) - 1;
 
-	timing = strobe_prog & 0x00000FFF;
+	timing = readl(priv->base + IMX_OCOTP_ADDR_TIMING) & 0x0FC00000;
+	timing |= strobe_prog & 0x00000FFF;
 	timing |= (relax       << 12) & 0x0000F000;
 	timing |= (strobe_read << 16) & 0x003F0000;
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 3/5] nvmem: imx-ocotp: Change TIMING calculation to u-boot algorithm

[Bryan O'Donoghue]

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(-)

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;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 4/5] nvmem: imx-ocotp: Add i.MX8MM support

[Bryan O'Donoghue]

This patch adds support to burn the fuses on the i.MX8MM.
https://www.nxp.com/webapp/Download?colCode=IMX8MMRM

The i.MX8MM is similar to i.MX6 processors in terms of addressing and clock
setup.

The documentation specifies 60 discreet OTP registers but, the fusemap
address space encompasses up to 256 registers. We map the entire putative
256 OTP registers.

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
---
 drivers/nvmem/imx-ocotp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 826812d3332f..e8d3dcada6c6 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -479,6 +479,12 @@ static const struct ocotp_params imx8mq_params = {
 	.set_timing = imx_ocotp_set_imx7_timing,
 };
 
+static const struct ocotp_params imx8mm_params = {
+	.nregs = 256,
+	.bank_address_words = 0,
+	.set_timing = imx_ocotp_set_imx6_timing,
+};
+
 static const struct of_device_id imx_ocotp_dt_ids[] = {
 	{ .compatible = "fsl,imx6q-ocotp",  .data = &imx6q_params },
 	{ .compatible = "fsl,imx6sl-ocotp", .data = &imx6sl_params },
@@ -489,6 +495,7 @@ static const struct of_device_id imx_ocotp_dt_ids[] = {
 	{ .compatible = "fsl,imx6sll-ocotp", .data = &imx6sll_params },
 	{ .compatible = "fsl,imx7ulp-ocotp", .data = &imx7ulp_params },
 	{ .compatible = "fsl,imx8mq-ocotp", .data = &imx8mq_params },
+	{ .compatible = "fsl,imx8mm-ocotp", .data = &imx8mm_params },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, imx_ocotp_dt_ids);
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 5/5] dt-bindings: imx-ocotp: Add i.MX8MM compatible

[Bryan O'Donoghue]

Add compatible for i.MX8MM as per arch/arm64/boot/dts/freescale/imx8mm.dtsi

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Rob Herring <robh@kernel.org>
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 Documentation/devicetree/bindings/nvmem/imx-ocotp.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/nvmem/imx-ocotp.txt b/Documentation/devicetree/bindings/nvmem/imx-ocotp.txt
index 68f7d6fdd140..96ffd06d2ca8 100644
--- a/Documentation/devicetree/bindings/nvmem/imx-ocotp.txt
+++ b/Documentation/devicetree/bindings/nvmem/imx-ocotp.txt
@@ -15,6 +15,7 @@ Required properties:
 	"fsl,imx6sll-ocotp" (i.MX6SLL),
 	"fsl,imx7ulp-ocotp" (i.MX7ULP),
 	"fsl,imx8mq-ocotp" (i.MX8MQ),
+	"fsl,imx8mm-ocotp" (i.MX8MM),
 	followed by "syscon".
 - #address-cells : Should be 1
 - #size-cells : Should be 1
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 3/5] nvmem: imx-ocotp: Change TIMING calculation to u-boot algorithm

[Leonard Crestez]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel