[RESEND][PATCH 0/7] Fix i.MX7D OCOTP write support

[RESEND][PATCH 0/7] Fix i.MX7D OCOTP write support

[Bryan O'Donoghue]

(Resend: I may have missed Philip Zabel on the first send - apologies if
that is the case Philip.)

The current OCOTP driver added support for i.MX7 read access and then added
support for i.MX6 write access. Between the two commits the fact that the
added write routine was only appropriate for i.MX6 was missed.

As a result its certain that attempting to write i.MX7 OTP fuses on Linux
would fail as the destination address on i.MX7 is different to i.MX6.

Without the update to the i.MX7 setup and hold timings it's not clear that
a write operation would actually do any writing which means the bad
addressing on i.MX7 might not actually destroy the wrong OTP fuses, it
probably would just fail to do anything, understandably I haven't
experimented with knowingly bad values for one-time-programmable fuses.

This series fixes the gap by:

1. Switching off OTP writing for i.MX7
2. Adding in support for the i.MX7 way of doing things
3. Switching OTP write support back on for i.MX7

There's an additional small fix for the naming of the module then to
indicate it works for i.MX7 as well as for i.MX6.

Bryan O'Donoghue (7):
  nvmem: imx-ocotp: Restrict OTP write to IMX6 processors
  nvmem: imx-ocotp: Pass parameters via a struct
  nvmem: imx-ocotp: Add support for banked OTP addressing
  nvmem: imx-ocotp: Move i.MX6 write clock setup to dedicated function
  nvmem: imx-ocotp: Add i.MX7D timing write clock setup support
  nvmem: imx-ocotp: Enable i.MX7D OTP write support
  nvmem: imx-ocotp: Update module description

 drivers/nvmem/imx-ocotp.c | 191 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 156 insertions(+), 35 deletions(-)

-- 
2.7.4

[RESEND][PATCH 1/7] nvmem: imx-ocotp: Restrict OTP write to IMX6 processors

[Bryan O'Donoghue]

i.MX7S/D have a different scheme for addressing the OTP registers inside
the OCOTP block. Currently it's possible to address the wrong OTP registers
given the disparity between IMX6 and IMX7 OTP addressing.

Since OTP programming is one-time destructive its important we restrict
this interface ASAP.

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

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

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 193ca8f..17d160f 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -347,6 +347,8 @@ static int imx_ocotp_probe(struct platform_device *pdev)
 	imx_ocotp_nvmem_config.dev = dev;
 	imx_ocotp_nvmem_config.priv = priv;
 	priv->config = &imx_ocotp_nvmem_config;
+	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx7d-ocotp"))
+		imx_ocotp_nvmem_config.read_only = true;
 	nvmem = nvmem_register(&imx_ocotp_nvmem_config);
 
 	if (IS_ERR(nvmem))
-- 
2.7.4

[RESEND][PATCH 1/7] nvmem: imx-ocotp: Restrict OTP write to IMX6 processors

[Philipp Zabel]

On Wed, 2017-10-04 at 23:25 +0100, Bryan O'Donoghue wrote:
> i.MX7S/D have a different scheme for addressing the OTP registers
> inside
> the OCOTP block. Currently it's possible to address the wrong OTP
> registers
> given the disparity between IMX6 and IMX7 OTP addressing.
> 
> Since OTP programming is one-time destructive its important we
> restrict
> this interface ASAP.
> 
> Fixes: 0642bac7da42 ("nvmem: imx-ocotp: add write support")
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

[RESEND][PATCH 2/7] nvmem: imx-ocotp: Pass parameters via a struct

[Bryan O'Donoghue]

It will be useful in later patches to know the register access mode and
bit-shift to apply to a given input offset.

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

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

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 17d160f..fed76a4 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -53,11 +53,15 @@
 
 static DEFINE_MUTEX(ocotp_mutex);
 
+struct octp_params {
+	unsigned int nregs;
+};
+
 struct ocotp_priv {
 	struct device *dev;
 	struct clk *clk;
 	void __iomem *base;
-	unsigned int nregs;
+	struct octp_params *params;
 	struct nvmem_config *config;
 };
 
@@ -121,8 +125,8 @@ static int imx_ocotp_read(void *context, unsigned int offset,
 	index = offset >> 2;
 	count = bytes >> 2;
 
-	if (count > (priv->nregs - index))
-		count = priv->nregs - index;
+	if (count > (priv->params->nregs - index))
+		count = priv->params->nregs - index;
 
 	mutex_lock(&ocotp_mutex);
 
@@ -308,12 +312,20 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
 	.reg_write = imx_ocotp_write,
 };
 
+static const struct octp_params params[] = {
+	{ .nregs = 128},
+	{ .nregs = 64},
+	{ .nregs = 128},
+	{ .nregs = 128},
+	{ .nregs = 64},
+};
+
 static const struct of_device_id imx_ocotp_dt_ids[] = {
-	{ .compatible = "fsl,imx6q-ocotp",  (void *)128 },
-	{ .compatible = "fsl,imx6sl-ocotp", (void *)64 },
-	{ .compatible = "fsl,imx6sx-ocotp", (void *)128 },
-	{ .compatible = "fsl,imx6ul-ocotp", (void *)128 },
-	{ .compatible = "fsl,imx7d-ocotp", (void *)64 },
+	{ .compatible = "fsl,imx6q-ocotp",  (void *)&params[0] },
+	{ .compatible = "fsl,imx6sl-ocotp", (void *)&params[1] },
+	{ .compatible = "fsl,imx6sx-ocotp", (void *)&params[2] },
+	{ .compatible = "fsl,imx6ul-ocotp", (void *)&params[3] },
+	{ .compatible = "fsl,imx7d-ocotp", (void *)&params[4] },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, imx_ocotp_dt_ids);
@@ -342,8 +354,8 @@ static int imx_ocotp_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->clk);
 
 	of_id = of_match_device(imx_ocotp_dt_ids, dev);
-	priv->nregs = (unsigned long)of_id->data;
-	imx_ocotp_nvmem_config.size = 4 * priv->nregs;
+	priv->params = (struct octp_params *)of_id->data;
+	imx_ocotp_nvmem_config.size = 4 * priv->params->nregs;
 	imx_ocotp_nvmem_config.dev = dev;
 	imx_ocotp_nvmem_config.priv = priv;
 	priv->config = &imx_ocotp_nvmem_config;
-- 
2.7.4

[RESEND][PATCH 2/7] nvmem: imx-ocotp: Pass parameters via a struct

[Philipp Zabel]

Hi Bryan,

a few small nitpicks below.

On Wed, 2017-10-04 at 23:25 +0100, Bryan O'Donoghue wrote:
> It will be useful in later patches to know the register access mode
> and bit-shift to apply to a given input offset.
> 
> Fixes: 0642bac7da42 ("nvmem: imx-ocotp: add write support")
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> ---
> ?drivers/nvmem/imx-ocotp.c | 32 ++++++++++++++++++++++----------
> ?1 file changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index 17d160f..fed76a4 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -53,11 +53,15 @@
> ?
> ?static DEFINE_MUTEX(ocotp_mutex);
> ?
> +struct octp_params {

Should this struct be called "ocotp_params"?

> +	unsigned int nregs;
> +};
> +
> ?struct ocotp_priv {
> ?	struct device *dev;
> ?	struct clk *clk;
> ?	void __iomem *base;
> -	unsigned int nregs;
> +	struct octp_params *params;
> ?	struct nvmem_config *config;
> ?};
> ?
> @@ -121,8 +125,8 @@ static int imx_ocotp_read(void *context, unsigned
> int offset,
> ?	index = offset >> 2;
> ?	count = bytes >> 2;
> ?
> -	if (count > (priv->nregs - index))
> -		count = priv->nregs - index;
> +	if (count > (priv->params->nregs - index))
> +		count = priv->params->nregs - index;
> ?
> ?	mutex_lock(&ocotp_mutex);
> ?
> @@ -308,12 +312,20 @@ static struct nvmem_config
> imx_ocotp_nvmem_config = {
> ?	.reg_write = imx_ocotp_write,
> ?};
> ?
> +static const struct octp_params params[] = {
> +	{ .nregs = 128},

Missing whitespace.

> +	{ .nregs = 64},
> +	{ .nregs = 128},
> +	{ .nregs = 128},
> +	{ .nregs = 64},
> +};
> +
> ?static const struct of_device_id imx_ocotp_dt_ids[] = {
> -	{ .compatible = "fsl,imx6q-ocotp",??(void *)128 },
> -	{ .compatible = "fsl,imx6sl-ocotp", (void *)64 },
> -	{ .compatible = "fsl,imx6sx-ocotp", (void *)128 },
> -	{ .compatible = "fsl,imx6ul-ocotp", (void *)128 },
> -	{ .compatible = "fsl,imx7d-ocotp", (void *)64 },
> +	{ .compatible = "fsl,imx6q-ocotp",??(void *)&params[0] },

The (void *) cast is superfluous with this change, I'd also add the
missing ".data =":

	{ .compatible = "fsl,imx6q-ocotp",??.data = &params[0] },


> +	{ .compatible = "fsl,imx6sl-ocotp", (void *)&params[1] },
> +	{ .compatible = "fsl,imx6sx-ocotp", (void *)&params[2] },
> +	{ .compatible = "fsl,imx6ul-ocotp", (void *)&params[3] },
> +	{ .compatible = "fsl,imx7d-ocotp", (void *)&params[4] },
> ?	{ },
> ?};
> ?MODULE_DEVICE_TABLE(of, imx_ocotp_dt_ids);
> @@ -342,8 +354,8 @@ static int imx_ocotp_probe(struct platform_device
> *pdev)
> ?		return PTR_ERR(priv->clk);
> ?
> ?	of_id = of_match_device(imx_ocotp_dt_ids, dev);
> -	priv->nregs = (unsigned long)of_id->data;
> -	imx_ocotp_nvmem_config.size = 4 * priv->nregs;
> +	priv->params = (struct octp_params *)of_id->data;
> +	imx_ocotp_nvmem_config.size = 4 * priv->params->nregs;

This would be a good opportunity to switch to of_device_get_match_data.

> ?	imx_ocotp_nvmem_config.dev = dev;
> ?	imx_ocotp_nvmem_config.priv = priv;
> ?	priv->config = &imx_ocotp_nvmem_config;

regards
Philipp

[RESEND][PATCH 3/7] nvmem: imx-ocotp: Add support for banked OTP addressing

[Bryan O'Donoghue]

The i.MX7S/D takes the bank address in the CTRLn.ADDR field and the data
value in one of the DATAx {0, 1, 2, 3} register fields. The current write
routine is based on writing the CTRLn.ADDR field and writing a single DATA
register only.

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

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

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index fed76a4..8034937 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -40,7 +40,10 @@
 #define IMX_OCOTP_ADDR_CTRL_SET		0x0004
 #define IMX_OCOTP_ADDR_CTRL_CLR		0x0008
 #define IMX_OCOTP_ADDR_TIMING		0x0010
-#define IMX_OCOTP_ADDR_DATA		0x0020
+#define IMX_OCOTP_ADDR_DATA0		0x0020
+#define IMX_OCOTP_ADDR_DATA1		0x0030
+#define IMX_OCOTP_ADDR_DATA2		0x0040
+#define IMX_OCOTP_ADDR_DATA3		0x0050
 
 #define IMX_OCOTP_BM_CTRL_ADDR		0x0000007F
 #define IMX_OCOTP_BM_CTRL_BUSY		0x00000100
@@ -55,6 +58,8 @@ static DEFINE_MUTEX(ocotp_mutex);
 
 struct octp_params {
 	unsigned int nregs;
+	bool banked;
+	unsigned int regs_per_bank;
 };
 
 struct ocotp_priv {
@@ -176,6 +181,7 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	u32 timing = 0;
 	u32 ctrl;
 	u8 waddr;
+	u8 word = 0;
 
 	/* allow only writing one complete OTP word at a time */
 	if ((bytes != priv->config->word_size) ||
@@ -228,8 +234,22 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	 * description. Both the unlock code and address can be written in the
 	 * same operation.
 	 */
-	/* OTP write/read address specifies one of 128 word address locations */
-	waddr = offset / 4;
+	if (priv->params->banked) {
+		/*
+		 * In banked mode the OTP register bank goes into waddr see
+		 * i.MX 7Solo Applications Processor Reference Manual, Rev. 0.1
+		 * 6.4.3.1
+		 */
+		offset = offset / priv->config->word_size;
+		waddr = offset / priv->params->regs_per_bank;
+		word  = offset & (priv->params->regs_per_bank - 1);
+	} else {
+		/*
+		 * OTP write/read address specifies one of 128 word address
+		 * locations
+		 */
+		waddr = offset / 4;
+	}
 
 	ctrl = readl(priv->base + IMX_OCOTP_ADDR_CTRL);
 	ctrl &= ~IMX_OCOTP_BM_CTRL_ADDR;
@@ -255,8 +275,41 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	 * shift right (with zero fill). This shifting is required to program
 	 * the OTP serially. During the write operation, HW_OCOTP_DATA cannot be
 	 * modified.
+	 * Note: on i.MX7 there are four data fields to write for banked write
+	 *       with the fuse blowing operation only taking place after data0
+	 *	 has been written. This is why data0 must always be the last
+	 *	 register written.
 	 */
-	writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA);
+	if (!priv->params->banked) {
+		writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA0);
+	} else {
+		switch (word) {
+		case 0:
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA1);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA2);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA3);
+			writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA0);
+			break;
+		case 1:
+			writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA1);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA2);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA3);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA0);
+			break;
+		case 2:
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA1);
+			writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA2);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA3);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA0);
+			break;
+		case 3:
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA1);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA2);
+			writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA3);
+			writel(0, priv->base + IMX_OCOTP_ADDR_DATA0);
+			break;
+		}
+	}
 
 	/* 47.4.1.4.5
 	 * Once complete, the controller will clear BUSY. A write request to a
@@ -313,11 +366,11 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
 };
 
 static const struct octp_params params[] = {
-	{ .nregs = 128},
-	{ .nregs = 64},
-	{ .nregs = 128},
-	{ .nregs = 128},
-	{ .nregs = 64},
+	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
+	{ .nregs = 64,  .banked = false, .regs_per_bank = 0},
+	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
+	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
+	{ .nregs = 64,  .banked = true, .regs_per_bank = 4},
 };
 
 static const struct of_device_id imx_ocotp_dt_ids[] = {
-- 
2.7.4

[RESEND][PATCH 3/7] nvmem: imx-ocotp: Add support for banked OTP addressing

[Philipp Zabel]

On Wed, 2017-10-04 at 23:25 +0100, Bryan O'Donoghue wrote:
> The i.MX7S/D takes the bank address in the CTRLn.ADDR field and the
> data
> value in one of the DATAx {0, 1, 2, 3} register fields. The current
> write
> routine is based on writing the CTRLn.ADDR field and writing a single
> DATA
> register only.
> 
> Fixes: 0642bac7da42 ("nvmem: imx-ocotp: add write support")
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> ---
> ?drivers/nvmem/imx-ocotp.c | 71
> +++++++++++++++++++++++++++++++++++++++++------
> ?1 file changed, 62 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index fed76a4..8034937 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -40,7 +40,10 @@
> ?#define IMX_OCOTP_ADDR_CTRL_SET		0x0004
> ?#define IMX_OCOTP_ADDR_CTRL_CLR		0x0008
> ?#define IMX_OCOTP_ADDR_TIMING		0x0010
> -#define IMX_OCOTP_ADDR_DATA		0x0020
> +#define IMX_OCOTP_ADDR_DATA0		0x0020
> +#define IMX_OCOTP_ADDR_DATA1		0x0030
> +#define IMX_OCOTP_ADDR_DATA2		0x0040
> +#define IMX_OCOTP_ADDR_DATA3		0x0050
> ?
> ?#define IMX_OCOTP_BM_CTRL_ADDR		0x0000007F
> ?#define IMX_OCOTP_BM_CTRL_BUSY		0x00000100
> @@ -55,6 +58,8 @@ static DEFINE_MUTEX(ocotp_mutex);
> ?
> ?struct octp_params {
> ?	unsigned int nregs;
> +	bool banked;
> +	unsigned int regs_per_bank;
> ?};

Instead of the separate banked parameter you could just use
(regs_per_bank != 0).

regards
Philipp

[RESEND][PATCH 4/7] nvmem: imx-ocotp: Move i.MX6 write clock setup to dedicated function

[Bryan O'Donoghue]

The i.MX7S/D has a different set of timing requirements, as a pre-cursor to
adding the i.MX7 timing parameters, move the i.MX6 stuff to a dedicated
function.

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

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

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 8034937..0cd7e03 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -169,6 +169,31 @@ static int imx_ocotp_read(void *context, unsigned int offset,
 	return ret;
 }
 
+static void imx_ocotp_set_imx6_timing(struct ocotp_priv *priv)
+{
+	unsigned long clk_rate = 0;
+	unsigned long strobe_read, relax, strobe_prog;
+	u32 timing = 0;
+
+	/* 47.3.1.3.1
+	 * Program HW_OCOTP_TIMING[STROBE_PROG] and HW_OCOTP_TIMING[RELAX]
+	 * 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.
+	 */
+	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;
+
+	timing = strobe_prog & 0x00000FFF;
+	timing |= (relax       << 12) & 0x0000F000;
+	timing |= (strobe_read << 16) & 0x003F0000;
+
+	writel(timing, priv->base + IMX_OCOTP_ADDR_TIMING);
+}
+
 static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 			   size_t bytes)
 {
@@ -176,9 +201,6 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	u32 *buf = val;
 	int ret;
 
-	unsigned long clk_rate = 0;
-	unsigned long strobe_read, relax, strobe_prog;
-	u32 timing = 0;
 	u32 ctrl;
 	u8 waddr;
 	u8 word = 0;
@@ -197,23 +219,8 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 		return ret;
 	}
 
-	/* 47.3.1.3.1
-	 * Program HW_OCOTP_TIMING[STROBE_PROG] and HW_OCOTP_TIMING[RELAX]
-	 * 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.
-	 */
-	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;
-
-	timing = strobe_prog & 0x00000FFF;
-	timing |= (relax       << 12) & 0x0000F000;
-	timing |= (strobe_read << 16) & 0x003F0000;
-
-	writel(timing, priv->base + IMX_OCOTP_ADDR_TIMING);
+	/* Setup the write timing values */
+	imx_ocotp_set_imx6_timing(priv);
 
 	/* 47.3.1.3.2
 	 * Check that HW_OCOTP_CTRL[BUSY] and HW_OCOTP_CTRL[ERROR] are clear.
-- 
2.7.4

[RESEND][PATCH 4/7] nvmem: imx-ocotp: Move i.MX6 write clock setup to dedicated function

[Philipp Zabel]

On Wed, 2017-10-04 at 23:25 +0100, Bryan O'Donoghue wrote:
> The i.MX7S/D has a different set of timing requirements, as a pre-
> cursor to
> adding the i.MX7 timing parameters, move the i.MX6 stuff to a
> dedicated
> function.
> 
> Fixes: 0642bac7da42 ("nvmem: imx-ocotp: add write support")
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

[RESEND][PATCH 5/7] nvmem: imx-ocotp: Add i.MX7D timing write clock setup support

[Bryan O'Donoghue]

This patch adds logic to correctly setup the write timing parameters
when blowing an OTP fuse for the i.MX7S/D.

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

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

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 0cd7e03..40a1669 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -51,6 +51,7 @@
 #define IMX_OCOTP_BM_CTRL_REL_SHADOWS	0x00000400
 
 #define DEF_RELAX			20 /* > 16.5ns */
+#define DEF_FSOURCE			1001
 #define IMX_OCOTP_WR_UNLOCK		0x3E770000
 #define IMX_OCOTP_READ_LOCKED_VAL	0xBADABADA
 
@@ -60,6 +61,7 @@ struct octp_params {
 	unsigned int nregs;
 	bool banked;
 	unsigned int regs_per_bank;
+	bool mx7_timing;
 };
 
 struct ocotp_priv {
@@ -194,6 +196,25 @@ static void imx_ocotp_set_imx6_timing(struct ocotp_priv *priv)
 	writel(timing, priv->base + IMX_OCOTP_ADDR_TIMING);
 }
 
+static void imx_ocotp_set_imx7_timing(struct ocotp_priv *priv)
+{
+	unsigned long clk_rate = 0;
+	unsigned long fsource, strobe_prog;
+	u32 timing = 0;
+
+	/* i.MX 7Solo Applications Processor Reference Manual, Rev. 0.1
+	 * 6.4.3.3
+	 */
+	clk_rate = clk_get_rate(priv->clk);
+	fsource = DIV_ROUND_UP(((clk_rate / 1000) * DEF_FSOURCE), 1000000) + 1;
+	strobe_prog = ((clk_rate * 10) / 1000000) + 1;
+
+	timing = strobe_prog & 0x00000FFF;
+	timing |= (fsource       << 12) & 0x000FF000;
+
+	writel(timing, priv->base + IMX_OCOTP_ADDR_TIMING);
+}
+
 static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 			   size_t bytes)
 {
@@ -220,7 +241,10 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	}
 
 	/* Setup the write timing values */
-	imx_ocotp_set_imx6_timing(priv);
+	if (priv->params->mx7_timing)
+		imx_ocotp_set_imx7_timing(priv);
+	else
+		imx_ocotp_set_imx6_timing(priv);
 
 	/* 47.3.1.3.2
 	 * Check that HW_OCOTP_CTRL[BUSY] and HW_OCOTP_CTRL[ERROR] are clear.
@@ -373,11 +397,36 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
 };
 
 static const struct octp_params params[] = {
-	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
-	{ .nregs = 64,  .banked = false, .regs_per_bank = 0},
-	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
-	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
-	{ .nregs = 64,  .banked = true, .regs_per_bank = 4},
+	{
+		.nregs = 128,
+		.banked = false,
+		.regs_per_bank = 0,
+		.mx7_timing = false
+	},
+	{
+		.nregs = 64,
+		.banked = false,
+		.regs_per_bank = 0,
+		.mx7_timing = false
+	},
+	{
+		.nregs = 128,
+		.banked = false,
+		.regs_per_bank = 0,
+		.mx7_timing = false
+	},
+	{
+		.nregs = 128,
+		.banked = false,
+		.regs_per_bank = 0,
+		.mx7_timing = false
+	},
+	{
+		.nregs = 64,
+		.banked = true,
+		.regs_per_bank = 4,
+		.mx7_timing = true
+	},
 };
 
 static const struct of_device_id imx_ocotp_dt_ids[] = {
-- 
2.7.4

[RESEND][PATCH 5/7] nvmem: imx-ocotp: Add i.MX7D timing write clock setup support

[Philipp Zabel]

On Wed, 2017-10-04 at 23:25 +0100, Bryan O'Donoghue wrote:
> This patch adds logic to correctly setup the write timing parameters
> when blowing an OTP fuse for the i.MX7S/D.
> 
> Fixes: 0642bac7da42 ("nvmem: imx-ocotp: add write support")
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> ---
> ?drivers/nvmem/imx-ocotp.c | 61
> ++++++++++++++++++++++++++++++++++++++++++-----
> ?1 file changed, 55 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index 0cd7e03..40a1669 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -51,6 +51,7 @@
> ?#define IMX_OCOTP_BM_CTRL_REL_SHADOWS	0x00000400
> ?
> ?#define DEF_RELAX			20 /* > 16.5ns */
> +#define DEF_FSOURCE			1001
> ?#define IMX_OCOTP_WR_UNLOCK		0x3E770000
> ?#define IMX_OCOTP_READ_LOCKED_VAL	0xBADABADA
> ?
> @@ -60,6 +61,7 @@ struct octp_params {
> ?	unsigned int nregs;
> ?	bool banked;
> ?	unsigned int regs_per_bank;
> +	bool mx7_timing;
> ?};
> ?
> ?struct ocotp_priv {
> @@ -194,6 +196,25 @@ static void imx_ocotp_set_imx6_timing(struct
> ocotp_priv *priv)
> ?	writel(timing, priv->base + IMX_OCOTP_ADDR_TIMING);
> ?}
> ?
> +static void imx_ocotp_set_imx7_timing(struct ocotp_priv *priv)
> +{
> +	unsigned long clk_rate = 0;
> +	unsigned long fsource, strobe_prog;
> +	u32 timing = 0;
> +
> +	/* i.MX 7Solo Applications Processor Reference Manual, Rev.
> 0.1
> +	?* 6.4.3.3
> +	?*/
> +	clk_rate = clk_get_rate(priv->clk);
> +	fsource = DIV_ROUND_UP(((clk_rate / 1000) * DEF_FSOURCE),
> 1000000) + 1;
> +	strobe_prog = ((clk_rate * 10) / 1000000) + 1;
> +
> +	timing = strobe_prog & 0x00000FFF;
> +	timing |= (fsource???????<< 12) & 0x000FF000;
> +
> +	writel(timing, priv->base + IMX_OCOTP_ADDR_TIMING);
> +}
> +
> ?static int imx_ocotp_write(void *context, unsigned int offset, void
> *val,
> ?			???size_t bytes)
> ?{
> @@ -220,7 +241,10 @@ static int imx_ocotp_write(void *context,
> unsigned int offset, void *val,
> ?	}
> ?
> ?	/* Setup the write timing values */
> -	imx_ocotp_set_imx6_timing(priv);
> +	if (priv->params->mx7_timing)
> +		imx_ocotp_set_imx7_timing(priv);
> +	else
> +		imx_ocotp_set_imx6_timing(priv);

Do we expect this to grow more different cases in the future?

Maybe it would be better to just store a .set_timing function pointer in
the params.

> ?
> ?	/* 47.3.1.3.2
> ?	?* Check that HW_OCOTP_CTRL[BUSY] and HW_OCOTP_CTRL[ERROR]
> are clear.
> @@ -373,11 +397,36 @@ static struct nvmem_config
> imx_ocotp_nvmem_config = {
> ?};
> ?
> ?static const struct octp_params params[] = {
> -	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
> -	{ .nregs = 64,??.banked = false, .regs_per_bank = 0},
> -	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
> -	{ .nregs = 128, .banked = false, .regs_per_bank = 0},
> -	{ .nregs = 64,??.banked = true, .regs_per_bank = 4},
> +	{
> +		.nregs = 128,
> +		.banked = false,
> +		.regs_per_bank = 0,
> +		.mx7_timing = false
> +	},
> +	{
> +		.nregs = 64,
> +		.banked = false,
> +		.regs_per_bank = 0,
> +		.mx7_timing = false
> +	},
> +	{
> +		.nregs = 128,
> +		.banked = false,
> +		.regs_per_bank = 0,
> +		.mx7_timing = false
> +	},
> +	{
> +		.nregs = 128,
> +		.banked = false,
> +		.regs_per_bank = 0,
> +		.mx7_timing = false
> +	},
> +	{
> +		.nregs = 64,
> +		.banked = true,
> +		.regs_per_bank = 4,
> +		.mx7_timing = true
> +	},
> ?};
> ?
> ?static const struct of_device_id imx_ocotp_dt_ids[] = {

regards
Philipp

[RESEND][PATCH 5/7] nvmem: imx-ocotp: Add i.MX7D timing write clock setup support

[Bryan O'Donoghue]

On 05/10/17 11:52, Philipp Zabel wrote:
> Do we expect this to grow more different cases in the future?

Grow - I'm not so sure. Change definitely there's at least one other 
type of MX7 (which I don't have) that doesn't require a clock setup here 
at all.

> 
> Maybe it would be better to just store a .set_timing function pointer in
> the params.

Good idea.

Thanks for the reviews.

---
bod

[RESEND][PATCH 6/7] nvmem: imx-ocotp: Enable i.MX7D OTP write support

[Bryan O'Donoghue]

After applying patches for both banked access and write timings we can
re-enable the OTP write interface on i.MX7D processors.

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

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

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 40a1669..5b0f7b5 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -468,8 +468,6 @@ static int imx_ocotp_probe(struct platform_device *pdev)
 	imx_ocotp_nvmem_config.dev = dev;
 	imx_ocotp_nvmem_config.priv = priv;
 	priv->config = &imx_ocotp_nvmem_config;
-	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx7d-ocotp"))
-		imx_ocotp_nvmem_config.read_only = true;
 	nvmem = nvmem_register(&imx_ocotp_nvmem_config);
 
 	if (IS_ERR(nvmem))
-- 
2.7.4

[RESEND][PATCH 7/7] nvmem: imx-ocotp: Update module description

[Bryan O'Donoghue]

This imx-ocotp driver encapsulates support for a subset of both i.MX6 and
i.MX7 processors. Update the module description to reflect.

Fixes: 711d45477931 ("nvmem: octop: Add i.MX7D support")

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
---
 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 5b0f7b5..a58a80b 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -496,5 +496,5 @@ static struct platform_driver imx_ocotp_driver = {
 module_platform_driver(imx_ocotp_driver);
 
 MODULE_AUTHOR("Philipp Zabel <p.zabel@pengutronix.de>");
-MODULE_DESCRIPTION("i.MX6 OCOTP fuse box driver");
+MODULE_DESCRIPTION("i.MX6/i.MX7 OCOTP fuse box driver");
 MODULE_LICENSE("GPL v2");
-- 
2.7.4