All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/7] Fix i.MX7D OCOTP write support
@ 2017-10-09 14:11 ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: p.zabel, richard.leitner, srinivas.kandagatla, axel.lin,
	ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel, Bryan O'Donoghue

V3:
- Fix compile bug caused by a rebase typo - kbuild test robot

V2:
- Added Reviewed-by and Acked-by as indicated by Philipp
- octp_params -> ocotp_params Philipp Zabel
- Added whitespace to aggregate initaliser - Philipp Zabel
- Dropped (void*) cast to data - Philipp Zabel
- Made use of of_device_get_match_data - Philipp Zabel
- Got rid of params->banked - Philipp Zabel
- Changed regs_per_bank to bank_address_words
  makes the intent of the parameter clearer - Bryan O'Donoghue

V1:
(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 | 182 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 147 insertions(+), 35 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH v3 0/7] Fix i.MX7D OCOTP write support
@ 2017-10-09 14:11 ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

V3:
- Fix compile bug caused by a rebase typo - kbuild test robot

V2:
- Added Reviewed-by and Acked-by as indicated by Philipp
- octp_params -> ocotp_params Philipp Zabel
- Added whitespace to aggregate initaliser - Philipp Zabel
- Dropped (void*) cast to data - Philipp Zabel
- Made use of of_device_get_match_data - Philipp Zabel
- Got rid of params->banked - Philipp Zabel
- Changed regs_per_bank to bank_address_words
  makes the intent of the parameter clearer - Bryan O'Donoghue

V1:
(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 | 182 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 147 insertions(+), 35 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH v3 1/7] nvmem: imx-ocotp: Restrict OTP write to IMX6 processors
  2017-10-09 14:11 ` Bryan O'Donoghue
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  -1 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: p.zabel, richard.leitner, srinivas.kandagatla, axel.lin,
	ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel, 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>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 1/7] nvmem: imx-ocotp: Restrict OTP write to IMX6 processors
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 2/7] nvmem: imx-ocotp: Pass parameters via a struct
  2017-10-09 14:11 ` Bryan O'Donoghue
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  -1 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: p.zabel, richard.leitner, srinivas.kandagatla, axel.lin,
	ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel, 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..7798571 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -53,11 +53,15 @@
 
 static DEFINE_MUTEX(ocotp_mutex);
 
+struct ocotp_params {
+	unsigned int nregs;
+};
+
 struct ocotp_priv {
 	struct device *dev;
 	struct clk *clk;
 	void __iomem *base;
-	unsigned int nregs;
+	const struct ocotp_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 ocotp_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",  .data = &params[0] },
+	{ .compatible = "fsl,imx6sl-ocotp", .data = &params[1] },
+	{ .compatible = "fsl,imx6sx-ocotp", .data = &params[2] },
+	{ .compatible = "fsl,imx6ul-ocotp", .data = &params[3] },
+	{ .compatible = "fsl,imx7d-ocotp",  .data = &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 = of_device_get_match_data(&pdev->dev);
+	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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 2/7] nvmem: imx-ocotp: Pass parameters via a struct
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

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..7798571 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -53,11 +53,15 @@
 
 static DEFINE_MUTEX(ocotp_mutex);
 
+struct ocotp_params {
+	unsigned int nregs;
+};
+
 struct ocotp_priv {
 	struct device *dev;
 	struct clk *clk;
 	void __iomem *base;
-	unsigned int nregs;
+	const struct ocotp_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 ocotp_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",  .data = &params[0] },
+	{ .compatible = "fsl,imx6sl-ocotp", .data = &params[1] },
+	{ .compatible = "fsl,imx6sx-ocotp", .data = &params[2] },
+	{ .compatible = "fsl,imx6ul-ocotp", .data = &params[3] },
+	{ .compatible = "fsl,imx7d-ocotp",  .data = &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 = of_device_get_match_data(&pdev->dev);
+	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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 3/7] nvmem: imx-ocotp: Add support for banked OTP addressing
  2017-10-09 14:11 ` Bryan O'Donoghue
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  -1 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: p.zabel, richard.leitner, srinivas.kandagatla, axel.lin,
	ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel, 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 | 70 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 61 insertions(+), 9 deletions(-)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 7798571..927d525 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,7 @@ static DEFINE_MUTEX(ocotp_mutex);
 
 struct ocotp_params {
 	unsigned int nregs;
+	unsigned int bank_address_words;
 };
 
 struct ocotp_priv {
@@ -176,6 +180,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 +233,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->bank_address_words != 0) {
+		/*
+		 * 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->bank_address_words;
+		word  = offset & (priv->params->bank_address_words - 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 +274,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->bank_address_words == 0) {
+		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 +365,11 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
 };
 
 static const struct ocotp_params params[] = {
-	{ .nregs = 128 },
-	{ .nregs = 64 },
-	{ .nregs = 128 },
-	{ .nregs = 128 },
-	{ .nregs = 64 },
+	{ .nregs = 128, .bank_address_words = 0 },
+	{ .nregs = 64,  .bank_address_words = 0 },
+	{ .nregs = 128, .bank_address_words = 0 },
+	{ .nregs = 128, .bank_address_words = 0 },
+	{ .nregs = 64,  .bank_address_words = 4 },
 };
 
 static const struct of_device_id imx_ocotp_dt_ids[] = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 3/7] nvmem: imx-ocotp: Add support for banked OTP addressing
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

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 | 70 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 61 insertions(+), 9 deletions(-)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 7798571..927d525 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,7 @@ static DEFINE_MUTEX(ocotp_mutex);
 
 struct ocotp_params {
 	unsigned int nregs;
+	unsigned int bank_address_words;
 };
 
 struct ocotp_priv {
@@ -176,6 +180,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 +233,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->bank_address_words != 0) {
+		/*
+		 * 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->bank_address_words;
+		word  = offset & (priv->params->bank_address_words - 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 +274,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->bank_address_words == 0) {
+		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 +365,11 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
 };
 
 static const struct ocotp_params params[] = {
-	{ .nregs = 128 },
-	{ .nregs = 64 },
-	{ .nregs = 128 },
-	{ .nregs = 128 },
-	{ .nregs = 64 },
+	{ .nregs = 128, .bank_address_words = 0 },
+	{ .nregs = 64,  .bank_address_words = 0 },
+	{ .nregs = 128, .bank_address_words = 0 },
+	{ .nregs = 128, .bank_address_words = 0 },
+	{ .nregs = 64,  .bank_address_words = 4 },
 };
 
 static const struct of_device_id imx_ocotp_dt_ids[] = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 4/7] nvmem: imx-ocotp: Move i.MX6 write clock setup to dedicated function
  2017-10-09 14:11 ` Bryan O'Donoghue
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  -1 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: p.zabel, richard.leitner, srinivas.kandagatla, axel.lin,
	ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel, 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>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 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 927d525..2645ee3 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -168,6 +168,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)
 {
@@ -175,9 +200,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;
@@ -196,23 +218,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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 4/7] nvmem: imx-ocotp: Move i.MX6 write clock setup to dedicated function
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

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>
---
 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 927d525..2645ee3 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -168,6 +168,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)
 {
@@ -175,9 +200,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;
@@ -196,23 +218,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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 5/7] nvmem: imx-ocotp: Add i.MX7D timing write clock setup support
  2017-10-09 14:11 ` Bryan O'Donoghue
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  -1 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: p.zabel, richard.leitner, srinivas.kandagatla, axel.lin,
	ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel, 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 | 63 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 52 insertions(+), 11 deletions(-)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 2645ee3..f80aee9 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -51,16 +51,12 @@
 #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
 
 static DEFINE_MUTEX(ocotp_mutex);
 
-struct ocotp_params {
-	unsigned int nregs;
-	unsigned int bank_address_words;
-};
-
 struct ocotp_priv {
 	struct device *dev;
 	struct clk *clk;
@@ -69,6 +65,12 @@ struct ocotp_priv {
 	struct nvmem_config *config;
 };
 
+struct ocotp_params {
+	unsigned int nregs;
+	unsigned int bank_address_words;
+	void (*set_timing)(struct ocotp_priv *priv);
+};
+
 static int imx_ocotp_wait_for_busy(void __iomem *base, u32 flags)
 {
 	int count;
@@ -193,6 +195,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)
 {
@@ -219,7 +240,7 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	}
 
 	/* Setup the write timing values */
-	imx_ocotp_set_imx6_timing(priv);
+	priv->params->set_timing(priv);
 
 	/* 47.3.1.3.2
 	 * Check that HW_OCOTP_CTRL[BUSY] and HW_OCOTP_CTRL[ERROR] are clear.
@@ -372,11 +393,31 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
 };
 
 static const struct ocotp_params params[] = {
-	{ .nregs = 128, .bank_address_words = 0 },
-	{ .nregs = 64,  .bank_address_words = 0 },
-	{ .nregs = 128, .bank_address_words = 0 },
-	{ .nregs = 128, .bank_address_words = 0 },
-	{ .nregs = 64,  .bank_address_words = 4 },
+	{
+		.nregs = 128,
+		.bank_address_words = 0,
+		.set_timing = imx_ocotp_set_imx6_timing,
+	},
+	{
+		.nregs = 64,
+		.bank_address_words = 0,
+		.set_timing = imx_ocotp_set_imx6_timing,
+	},
+	{
+		.nregs = 128,
+		.bank_address_words = 0,
+		.set_timing = imx_ocotp_set_imx6_timing,
+	},
+	{
+		.nregs = 128,
+		.bank_address_words = 0,
+		.set_timing = imx_ocotp_set_imx6_timing,
+	},
+	{
+		.nregs = 64,
+		.bank_address_words = 4,
+		.set_timing = imx_ocotp_set_imx7_timing,
+	},
 };
 
 static const struct of_device_id imx_ocotp_dt_ids[] = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 5/7] nvmem: imx-ocotp: Add i.MX7D timing write clock setup support
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

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 | 63 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 52 insertions(+), 11 deletions(-)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 2645ee3..f80aee9 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -51,16 +51,12 @@
 #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
 
 static DEFINE_MUTEX(ocotp_mutex);
 
-struct ocotp_params {
-	unsigned int nregs;
-	unsigned int bank_address_words;
-};
-
 struct ocotp_priv {
 	struct device *dev;
 	struct clk *clk;
@@ -69,6 +65,12 @@ struct ocotp_priv {
 	struct nvmem_config *config;
 };
 
+struct ocotp_params {
+	unsigned int nregs;
+	unsigned int bank_address_words;
+	void (*set_timing)(struct ocotp_priv *priv);
+};
+
 static int imx_ocotp_wait_for_busy(void __iomem *base, u32 flags)
 {
 	int count;
@@ -193,6 +195,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)
 {
@@ -219,7 +240,7 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	}
 
 	/* Setup the write timing values */
-	imx_ocotp_set_imx6_timing(priv);
+	priv->params->set_timing(priv);
 
 	/* 47.3.1.3.2
 	 * Check that HW_OCOTP_CTRL[BUSY] and HW_OCOTP_CTRL[ERROR] are clear.
@@ -372,11 +393,31 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
 };
 
 static const struct ocotp_params params[] = {
-	{ .nregs = 128, .bank_address_words = 0 },
-	{ .nregs = 64,  .bank_address_words = 0 },
-	{ .nregs = 128, .bank_address_words = 0 },
-	{ .nregs = 128, .bank_address_words = 0 },
-	{ .nregs = 64,  .bank_address_words = 4 },
+	{
+		.nregs = 128,
+		.bank_address_words = 0,
+		.set_timing = imx_ocotp_set_imx6_timing,
+	},
+	{
+		.nregs = 64,
+		.bank_address_words = 0,
+		.set_timing = imx_ocotp_set_imx6_timing,
+	},
+	{
+		.nregs = 128,
+		.bank_address_words = 0,
+		.set_timing = imx_ocotp_set_imx6_timing,
+	},
+	{
+		.nregs = 128,
+		.bank_address_words = 0,
+		.set_timing = imx_ocotp_set_imx6_timing,
+	},
+	{
+		.nregs = 64,
+		.bank_address_words = 4,
+		.set_timing = imx_ocotp_set_imx7_timing,
+	},
 };
 
 static const struct of_device_id imx_ocotp_dt_ids[] = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 6/7] nvmem: imx-ocotp: Enable i.MX7D OTP write support
  2017-10-09 14:11 ` Bryan O'Donoghue
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  -1 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: p.zabel, richard.leitner, srinivas.kandagatla, axel.lin,
	ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel, 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 f80aee9..8e8bc20 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -459,8 +459,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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 6/7] nvmem: imx-ocotp: Enable i.MX7D OTP write support
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

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 f80aee9..8e8bc20 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -459,8 +459,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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 7/7] nvmem: imx-ocotp: Update module description
  2017-10-09 14:11 ` Bryan O'Donoghue
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  -1 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: p.zabel, richard.leitner, srinivas.kandagatla, axel.lin,
	ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel, 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 8e8bc20..60e2617 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -487,5 +487,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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3 7/7] nvmem: imx-ocotp: Update module description
@ 2017-10-09 14:11   ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-09 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

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 8e8bc20..60e2617 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -487,5 +487,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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/7] nvmem: imx-ocotp: Pass parameters via a struct
  2017-10-09 14:11   ` Bryan O'Donoghue
@ 2017-10-11 15:28     ` Philipp Zabel
  -1 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2017-10-11 15:28 UTC (permalink / raw)
  To: Bryan O'Donoghue, richard.leitner, srinivas.kandagatla,
	axel.lin, ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel

Hi Bryan,

On Mon, 2017-10-09 at 15:11 +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..7798571 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -53,11 +53,15 @@
>  
>  static DEFINE_MUTEX(ocotp_mutex);
>  
> +struct ocotp_params {
> +	unsigned int nregs;
> +};
> +
>  struct ocotp_priv {
>  	struct device *dev;
>  	struct clk *clk;
>  	void __iomem *base;
> -	unsigned int nregs;
> +	const struct ocotp_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 ocotp_params params[] = {
> +	{ .nregs = 128 },
> +	{ .nregs = 64 },
> +	{ .nregs = 128 },
> +	{ .nregs = 128 },
> +	{ .nregs = 64 },
> +};

Could you either add an enum for the indices into this array or split
the array into separate named structs?

I find it already hard to see which parameter belongs to which
compatible, and this will grow worse with increasing number of entries
in the parameter set.

For example,

static const struct ocotp_params imx6q_params = {
	.nregs = 128,
};

static const struct ocotp_params imx6sl_params = {
	.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",  .data = &params[0] },
> +	{ .compatible = "fsl,imx6sl-ocotp", .data = &params[1] },
> +	{ .compatible = "fsl,imx6sx-ocotp", .data = &params[2] },
> +	{ .compatible = "fsl,imx6ul-ocotp", .data = &params[3] },
> +	{ .compatible = "fsl,imx7d-ocotp",  .data = &params[4] },

	{ .compatible = "fsl,imx6q-ocotp",  .data = &imx6q_params },
	{ .compatible = "fsl,imx6sl-ocotp",  .data = &imx6sl_params },
...

Apart from that,

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

>  	{ },
>  };
>  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 = of_device_get_match_data(&pdev->dev);
> +	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;

regards
Philipp

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH v3 2/7] nvmem: imx-ocotp: Pass parameters via a struct
@ 2017-10-11 15:28     ` Philipp Zabel
  0 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2017-10-11 15:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Bryan,

On Mon, 2017-10-09 at 15:11 +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..7798571 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -53,11 +53,15 @@
>  
>  static DEFINE_MUTEX(ocotp_mutex);
>  
> +struct ocotp_params {
> +	unsigned int nregs;
> +};
> +
>  struct ocotp_priv {
>  	struct device *dev;
>  	struct clk *clk;
>  	void __iomem *base;
> -	unsigned int nregs;
> +	const struct ocotp_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 ocotp_params params[] = {
> +	{ .nregs = 128 },
> +	{ .nregs = 64 },
> +	{ .nregs = 128 },
> +	{ .nregs = 128 },
> +	{ .nregs = 64 },
> +};

Could you either add an enum for the indices into this array or split
the array into separate named structs?

I find it already hard to see which parameter belongs to which
compatible, and this will grow worse with increasing number of entries
in the parameter set.

For example,

static const struct ocotp_params imx6q_params = {
	.nregs = 128,
};

static const struct ocotp_params imx6sl_params = {
	.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",  .data = &params[0] },
> +	{ .compatible = "fsl,imx6sl-ocotp", .data = &params[1] },
> +	{ .compatible = "fsl,imx6sx-ocotp", .data = &params[2] },
> +	{ .compatible = "fsl,imx6ul-ocotp", .data = &params[3] },
> +	{ .compatible = "fsl,imx7d-ocotp",  .data = &params[4] },

	{ .compatible = "fsl,imx6q-ocotp",  .data = &imx6q_params },
	{ .compatible = "fsl,imx6sl-ocotp",  .data = &imx6sl_params },
...

Apart from that,

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

>  	{ },
>  };
>  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 = of_device_get_match_data(&pdev->dev);
> +	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;

regards
Philipp

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3 3/7] nvmem: imx-ocotp: Add support for banked OTP addressing
  2017-10-09 14:11   ` Bryan O'Donoghue
@ 2017-10-11 15:32     ` Philipp Zabel
  -1 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2017-10-11 15:32 UTC (permalink / raw)
  To: Bryan O'Donoghue, richard.leitner, srinivas.kandagatla,
	axel.lin, ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel

On Mon, 2017-10-09 at 15:11 +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 | 70 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 61 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index 7798571..927d525 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,7 @@ static DEFINE_MUTEX(ocotp_mutex);
>  
>  struct ocotp_params {
>  	unsigned int nregs;
> +	unsigned int bank_address_words;
>  };
>  
>  struct ocotp_priv {
> @@ -176,6 +180,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 +233,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->bank_address_words != 0) {
> +		/*
> +		 * 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->bank_address_words;
> +		word  = offset & (priv->params->bank_address_words - 1);
> +	} else {
> +		/*
> +		 * OTP write/read address specifies one of 128 word address
> +		 * locations
> +		 */
> +		waddr = offset / 4;
> +	}

Smallest of nitpicks, here the order is:

	if (bank_address_words != 0) {
		/* MX7 */
	} else {
		/* MX6 */
	}

>  
>  	ctrl = readl(priv->base + IMX_OCOTP_ADDR_CTRL);
>  	ctrl &= ~IMX_OCOTP_BM_CTRL_ADDR;
> @@ -255,8 +274,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->bank_address_words == 0) {
> +		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;
> +		}
> +	}

But here the order is
	if (bank_address_words == 0) {
		/* MX6 */
	} else {
		/* MX7 */
	}

Reordering this for consistency would also move the MX7 code closer to
the comment.

>  
>  	/* 47.4.1.4.5
>  	 * Once complete, the controller will clear BUSY. A write request to a
> @@ -313,11 +365,11 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
>  };
>  
>  static const struct ocotp_params params[] = {
> -	{ .nregs = 128 },
> -	{ .nregs = 64 },
> -	{ .nregs = 128 },
> -	{ .nregs = 128 },
> -	{ .nregs = 64 },
> +	{ .nregs = 128, .bank_address_words = 0 },
> +	{ .nregs = 64,  .bank_address_words = 0 },
> +	{ .nregs = 128, .bank_address_words = 0 },
> +	{ .nregs = 128, .bank_address_words = 0 },
> +	{ .nregs = 64,  .bank_address_words = 4 },

See my comment on the last patch, I'd like to be able see which entry
corresponds to which SoC in this patch.

Otherwise,

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

regards
Philipp

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH v3 3/7] nvmem: imx-ocotp: Add support for banked OTP addressing
@ 2017-10-11 15:32     ` Philipp Zabel
  0 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2017-10-11 15:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2017-10-09 at 15:11 +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 | 70 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 61 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index 7798571..927d525 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,7 @@ static DEFINE_MUTEX(ocotp_mutex);
>  
>  struct ocotp_params {
>  	unsigned int nregs;
> +	unsigned int bank_address_words;
>  };
>  
>  struct ocotp_priv {
> @@ -176,6 +180,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 +233,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->bank_address_words != 0) {
> +		/*
> +		 * 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->bank_address_words;
> +		word  = offset & (priv->params->bank_address_words - 1);
> +	} else {
> +		/*
> +		 * OTP write/read address specifies one of 128 word address
> +		 * locations
> +		 */
> +		waddr = offset / 4;
> +	}

Smallest of nitpicks, here the order is:

	if (bank_address_words != 0) {
		/* MX7 */
	} else {
		/* MX6 */
	}

>  
>  	ctrl = readl(priv->base + IMX_OCOTP_ADDR_CTRL);
>  	ctrl &= ~IMX_OCOTP_BM_CTRL_ADDR;
> @@ -255,8 +274,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->bank_address_words == 0) {
> +		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;
> +		}
> +	}

But here the order is
	if (bank_address_words == 0) {
		/* MX6 */
	} else {
		/* MX7 */
	}

Reordering this for consistency would also move the MX7 code closer to
the comment.

>  
>  	/* 47.4.1.4.5
>  	 * Once complete, the controller will clear BUSY. A write request to a
> @@ -313,11 +365,11 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
>  };
>  
>  static const struct ocotp_params params[] = {
> -	{ .nregs = 128 },
> -	{ .nregs = 64 },
> -	{ .nregs = 128 },
> -	{ .nregs = 128 },
> -	{ .nregs = 64 },
> +	{ .nregs = 128, .bank_address_words = 0 },
> +	{ .nregs = 64,  .bank_address_words = 0 },
> +	{ .nregs = 128, .bank_address_words = 0 },
> +	{ .nregs = 128, .bank_address_words = 0 },
> +	{ .nregs = 64,  .bank_address_words = 4 },

See my comment on the last patch, I'd like to be able see which entry
corresponds to which SoC in this patch.

Otherwise,

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

regards
Philipp

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3 3/7] nvmem: imx-ocotp: Add support for banked OTP addressing
  2017-10-11 15:32     ` Philipp Zabel
@ 2017-10-11 15:54       ` Bryan O'Donoghue
  -1 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-11 15:54 UTC (permalink / raw)
  To: Philipp Zabel, richard.leitner, srinivas.kandagatla, axel.lin,
	ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel

On 11/10/17 16:32, Philipp Zabel wrote:
> On Mon, 2017-10-09 at 15:11 +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 | 70 +++++++++++++++++++++++++++++++++++++++++------
>>   1 file changed, 61 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
>> index 7798571..927d525 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,7 @@ static DEFINE_MUTEX(ocotp_mutex);
>>   
>>   struct ocotp_params {
>>   	unsigned int nregs;
>> +	unsigned int bank_address_words;
>>   };
>>   
>>   struct ocotp_priv {
>> @@ -176,6 +180,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 +233,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->bank_address_words != 0) {
>> +		/*
>> +		 * 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->bank_address_words;
>> +		word  = offset & (priv->params->bank_address_words - 1);
>> +	} else {
>> +		/*
>> +		 * OTP write/read address specifies one of 128 word address
>> +		 * locations
>> +		 */
>> +		waddr = offset / 4;
>> +	}
> 
> Smallest of nitpicks, here the order is:
> 
> 	if (bank_address_words != 0) {
> 		/* MX7 */
> 	} else {
> 		/* MX6 */
> 	}
> 
>>   
>>   	ctrl = readl(priv->base + IMX_OCOTP_ADDR_CTRL);
>>   	ctrl &= ~IMX_OCOTP_BM_CTRL_ADDR;
>> @@ -255,8 +274,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->bank_address_words == 0) {
>> +		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;
>> +		}
>> +	}
> 
> But here the order is
> 	if (bank_address_words == 0) {
> 		/* MX6 */
> 	} else {
> 		/* MX7 */
> 	}
> 
> Reordering this for consistency would also move the MX7 code closer to
> the comment.
> 
>>   
>>   	/* 47.4.1.4.5
>>   	 * Once complete, the controller will clear BUSY. A write request to a
>> @@ -313,11 +365,11 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
>>   };
>>   
>>   static const struct ocotp_params params[] = {
>> -	{ .nregs = 128 },
>> -	{ .nregs = 64 },
>> -	{ .nregs = 128 },
>> -	{ .nregs = 128 },
>> -	{ .nregs = 64 },
>> +	{ .nregs = 128, .bank_address_words = 0 },
>> +	{ .nregs = 64,  .bank_address_words = 0 },
>> +	{ .nregs = 128, .bank_address_words = 0 },
>> +	{ .nregs = 128, .bank_address_words = 0 },
>> +	{ .nregs = 64,  .bank_address_words = 4 },
> 
> See my comment on the last patch, I'd like to be able see which entry
> corresponds to which SoC in this patch.
> 
> Otherwise,
> 
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> regards
> Philipp
> 

ACK those changes Philipp.

Thanks for your suggestions/time

---
bod

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH v3 3/7] nvmem: imx-ocotp: Add support for banked OTP addressing
@ 2017-10-11 15:54       ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-11 15:54 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/10/17 16:32, Philipp Zabel wrote:
> On Mon, 2017-10-09 at 15:11 +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 | 70 +++++++++++++++++++++++++++++++++++++++++------
>>   1 file changed, 61 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
>> index 7798571..927d525 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,7 @@ static DEFINE_MUTEX(ocotp_mutex);
>>   
>>   struct ocotp_params {
>>   	unsigned int nregs;
>> +	unsigned int bank_address_words;
>>   };
>>   
>>   struct ocotp_priv {
>> @@ -176,6 +180,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 +233,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->bank_address_words != 0) {
>> +		/*
>> +		 * 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->bank_address_words;
>> +		word  = offset & (priv->params->bank_address_words - 1);
>> +	} else {
>> +		/*
>> +		 * OTP write/read address specifies one of 128 word address
>> +		 * locations
>> +		 */
>> +		waddr = offset / 4;
>> +	}
> 
> Smallest of nitpicks, here the order is:
> 
> 	if (bank_address_words != 0) {
> 		/* MX7 */
> 	} else {
> 		/* MX6 */
> 	}
> 
>>   
>>   	ctrl = readl(priv->base + IMX_OCOTP_ADDR_CTRL);
>>   	ctrl &= ~IMX_OCOTP_BM_CTRL_ADDR;
>> @@ -255,8 +274,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->bank_address_words == 0) {
>> +		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;
>> +		}
>> +	}
> 
> But here the order is
> 	if (bank_address_words == 0) {
> 		/* MX6 */
> 	} else {
> 		/* MX7 */
> 	}
> 
> Reordering this for consistency would also move the MX7 code closer to
> the comment.
> 
>>   
>>   	/* 47.4.1.4.5
>>   	 * Once complete, the controller will clear BUSY. A write request to a
>> @@ -313,11 +365,11 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
>>   };
>>   
>>   static const struct ocotp_params params[] = {
>> -	{ .nregs = 128 },
>> -	{ .nregs = 64 },
>> -	{ .nregs = 128 },
>> -	{ .nregs = 128 },
>> -	{ .nregs = 64 },
>> +	{ .nregs = 128, .bank_address_words = 0 },
>> +	{ .nregs = 64,  .bank_address_words = 0 },
>> +	{ .nregs = 128, .bank_address_words = 0 },
>> +	{ .nregs = 128, .bank_address_words = 0 },
>> +	{ .nregs = 64,  .bank_address_words = 4 },
> 
> See my comment on the last patch, I'd like to be able see which entry
> corresponds to which SoC in this patch.
> 
> Otherwise,
> 
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> regards
> Philipp
> 

ACK those changes Philipp.

Thanks for your suggestions/time

---
bod

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3 5/7] nvmem: imx-ocotp: Add i.MX7D timing write clock setup support
  2017-10-09 14:11   ` Bryan O'Donoghue
@ 2017-10-11 16:03     ` Philipp Zabel
  -1 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2017-10-11 16:03 UTC (permalink / raw)
  To: Bryan O'Donoghue, richard.leitner, srinivas.kandagatla,
	axel.lin, ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel

On Mon, 2017-10-09 at 15:11 +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 | 63 ++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 52 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index 2645ee3..f80aee9 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -51,16 +51,12 @@
>  #define IMX_OCOTP_BM_CTRL_REL_SHADOWS	0x00000400
>  
>  #define DEF_RELAX			20 /* > 16.5ns */
> +#define DEF_FSOURCE			1001

Maybe add a comment /* > 1000ns */ ?

Also, I get why there is no DEF_STROBE_PROG, but this is a bit
inconsistent. You could switch to 64-bit calculations and add a

#define DEF_STROBE_PROG			10000

here.

>  #define IMX_OCOTP_WR_UNLOCK		0x3E770000
>  #define IMX_OCOTP_READ_LOCKED_VAL	0xBADABADA
>  
>  static DEFINE_MUTEX(ocotp_mutex);
>  
> -struct ocotp_params {
> -	unsigned int nregs;
> -	unsigned int bank_address_words;
> -};
> -
>  struct ocotp_priv {
>  	struct device *dev;
>  	struct clk *clk;
> @@ -69,6 +65,12 @@ struct ocotp_priv {
>  	struct nvmem_config *config;
>  };
>  
> +struct ocotp_params {
> +	unsigned int nregs;
> +	unsigned int bank_address_words;
> +	void (*set_timing)(struct ocotp_priv *priv);
> +};
> +
>  static int imx_ocotp_wait_for_busy(void __iomem *base, u32 flags)
>  {
>  	int count;
> @@ -193,6 +195,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;

Would

	fsource = DIV_ROUND_UP_ULL((u64)clk_rate * DEF_FSOURCE,
				   NSEC_PER_SEC) + 1;
	strobe_prog = DIV_ROUND_CLOSEST_ULL((u64)clk_rate * DEF_STROBE_PROG,
					    NSEC_PER_SEC) + 1;

work instead?

> +
> +	timing = strobe_prog & 0x00000FFF;
> +	timing |= (fsource       << 12) & 0x000FF000;

Unnecessary whitespace.

> +
> +	writel(timing, priv->base + IMX_OCOTP_ADDR_TIMING);
> +}
> +
>  static int imx_ocotp_write(void *context, unsigned int offset, void *val,
>  			   size_t bytes)
>  {
> @@ -219,7 +240,7 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
>  	}
>  
>  	/* Setup the write timing values */
> -	imx_ocotp_set_imx6_timing(priv);
> +	priv->params->set_timing(priv);
>  
>  	/* 47.3.1.3.2
>  	 * Check that HW_OCOTP_CTRL[BUSY] and HW_OCOTP_CTRL[ERROR] are clear.
> @@ -372,11 +393,31 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
>  };
>  
>  static const struct ocotp_params params[] = {
> -	{ .nregs = 128, .bank_address_words = 0 },
> -	{ .nregs = 64,  .bank_address_words = 0 },
> -	{ .nregs = 128, .bank_address_words = 0 },
> -	{ .nregs = 128, .bank_address_words = 0 },
> -	{ .nregs = 64,  .bank_address_words = 4 },
> +	{
> +		.nregs = 128,
> +		.bank_address_words = 0,
> +		.set_timing = imx_ocotp_set_imx6_timing,
> +	},
> +	{
> +		.nregs = 64,
> +		.bank_address_words = 0,
> +		.set_timing = imx_ocotp_set_imx6_timing,
> +	},
> +	{
> +		.nregs = 128,
> +		.bank_address_words = 0,
> +		.set_timing = imx_ocotp_set_imx6_timing,
> +	},
> +	{
> +		.nregs = 128,
> +		.bank_address_words = 0,
> +		.set_timing = imx_ocotp_set_imx6_timing,
> +	},
> +	{
> +		.nregs = 64,
> +		.bank_address_words = 4,
> +		.set_timing = imx_ocotp_set_imx7_timing,
> +	},

See previous patches.

>  };
>  
>  static const struct of_device_id imx_ocotp_dt_ids[] = {

regards
Philipp

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH v3 5/7] nvmem: imx-ocotp: Add i.MX7D timing write clock setup support
@ 2017-10-11 16:03     ` Philipp Zabel
  0 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2017-10-11 16:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2017-10-09 at 15:11 +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 | 63 ++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 52 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index 2645ee3..f80aee9 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -51,16 +51,12 @@
>  #define IMX_OCOTP_BM_CTRL_REL_SHADOWS	0x00000400
>  
>  #define DEF_RELAX			20 /* > 16.5ns */
> +#define DEF_FSOURCE			1001

Maybe add a comment /* > 1000ns */ ?

Also, I get why there is no DEF_STROBE_PROG, but this is a bit
inconsistent. You could switch to 64-bit calculations and add a

#define DEF_STROBE_PROG			10000

here.

>  #define IMX_OCOTP_WR_UNLOCK		0x3E770000
>  #define IMX_OCOTP_READ_LOCKED_VAL	0xBADABADA
>  
>  static DEFINE_MUTEX(ocotp_mutex);
>  
> -struct ocotp_params {
> -	unsigned int nregs;
> -	unsigned int bank_address_words;
> -};
> -
>  struct ocotp_priv {
>  	struct device *dev;
>  	struct clk *clk;
> @@ -69,6 +65,12 @@ struct ocotp_priv {
>  	struct nvmem_config *config;
>  };
>  
> +struct ocotp_params {
> +	unsigned int nregs;
> +	unsigned int bank_address_words;
> +	void (*set_timing)(struct ocotp_priv *priv);
> +};
> +
>  static int imx_ocotp_wait_for_busy(void __iomem *base, u32 flags)
>  {
>  	int count;
> @@ -193,6 +195,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;

Would

	fsource = DIV_ROUND_UP_ULL((u64)clk_rate * DEF_FSOURCE,
				   NSEC_PER_SEC) + 1;
	strobe_prog = DIV_ROUND_CLOSEST_ULL((u64)clk_rate * DEF_STROBE_PROG,
					    NSEC_PER_SEC) + 1;

work instead?

> +
> +	timing = strobe_prog & 0x00000FFF;
> +	timing |= (fsource       << 12) & 0x000FF000;

Unnecessary whitespace.

> +
> +	writel(timing, priv->base + IMX_OCOTP_ADDR_TIMING);
> +}
> +
>  static int imx_ocotp_write(void *context, unsigned int offset, void *val,
>  			   size_t bytes)
>  {
> @@ -219,7 +240,7 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
>  	}
>  
>  	/* Setup the write timing values */
> -	imx_ocotp_set_imx6_timing(priv);
> +	priv->params->set_timing(priv);
>  
>  	/* 47.3.1.3.2
>  	 * Check that HW_OCOTP_CTRL[BUSY] and HW_OCOTP_CTRL[ERROR] are clear.
> @@ -372,11 +393,31 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
>  };
>  
>  static const struct ocotp_params params[] = {
> -	{ .nregs = 128, .bank_address_words = 0 },
> -	{ .nregs = 64,  .bank_address_words = 0 },
> -	{ .nregs = 128, .bank_address_words = 0 },
> -	{ .nregs = 128, .bank_address_words = 0 },
> -	{ .nregs = 64,  .bank_address_words = 4 },
> +	{
> +		.nregs = 128,
> +		.bank_address_words = 0,
> +		.set_timing = imx_ocotp_set_imx6_timing,
> +	},
> +	{
> +		.nregs = 64,
> +		.bank_address_words = 0,
> +		.set_timing = imx_ocotp_set_imx6_timing,
> +	},
> +	{
> +		.nregs = 128,
> +		.bank_address_words = 0,
> +		.set_timing = imx_ocotp_set_imx6_timing,
> +	},
> +	{
> +		.nregs = 128,
> +		.bank_address_words = 0,
> +		.set_timing = imx_ocotp_set_imx6_timing,
> +	},
> +	{
> +		.nregs = 64,
> +		.bank_address_words = 4,
> +		.set_timing = imx_ocotp_set_imx7_timing,
> +	},

See previous patches.

>  };
>  
>  static const struct of_device_id imx_ocotp_dt_ids[] = {

regards
Philipp

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3 5/7] nvmem: imx-ocotp: Add i.MX7D timing write clock setup support
  2017-10-11 16:03     ` Philipp Zabel
@ 2017-10-11 16:22       ` Bryan O'Donoghue
  -1 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-11 16:22 UTC (permalink / raw)
  To: Philipp Zabel, richard.leitner, srinivas.kandagatla, axel.lin,
	ping.bai, d.schultz, peng.fan, van.freenix
  Cc: linux-kernel, linux-arm-kernel

On 11/10/17 17:03, Philipp Zabel wrote:
> Would
> 
> 	fsource = DIV_ROUND_UP_ULL((u64)clk_rate * DEF_FSOURCE,
> 				   NSEC_PER_SEC) + 1;
> 	strobe_prog = DIV_ROUND_CLOSEST_ULL((u64)clk_rate * DEF_STROBE_PROG,
> 					    NSEC_PER_SEC) + 1;
> 
> work instead?

Might do, I'll see and change it if it works

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH v3 5/7] nvmem: imx-ocotp: Add i.MX7D timing write clock setup support
@ 2017-10-11 16:22       ` Bryan O'Donoghue
  0 siblings, 0 replies; 26+ messages in thread
From: Bryan O'Donoghue @ 2017-10-11 16:22 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/10/17 17:03, Philipp Zabel wrote:
> Would
> 
> 	fsource = DIV_ROUND_UP_ULL((u64)clk_rate * DEF_FSOURCE,
> 				   NSEC_PER_SEC) + 1;
> 	strobe_prog = DIV_ROUND_CLOSEST_ULL((u64)clk_rate * DEF_STROBE_PROG,
> 					    NSEC_PER_SEC) + 1;
> 
> work instead?

Might do, I'll see and change it if it works

^ permalink raw reply	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2017-10-11 16:22 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-09 14:11 [PATCH v3 0/7] Fix i.MX7D OCOTP write support Bryan O'Donoghue
2017-10-09 14:11 ` Bryan O'Donoghue
2017-10-09 14:11 ` [PATCH v3 1/7] nvmem: imx-ocotp: Restrict OTP write to IMX6 processors Bryan O'Donoghue
2017-10-09 14:11   ` Bryan O'Donoghue
2017-10-09 14:11 ` [PATCH v3 2/7] nvmem: imx-ocotp: Pass parameters via a struct Bryan O'Donoghue
2017-10-09 14:11   ` Bryan O'Donoghue
2017-10-11 15:28   ` Philipp Zabel
2017-10-11 15:28     ` Philipp Zabel
2017-10-09 14:11 ` [PATCH v3 3/7] nvmem: imx-ocotp: Add support for banked OTP addressing Bryan O'Donoghue
2017-10-09 14:11   ` Bryan O'Donoghue
2017-10-11 15:32   ` Philipp Zabel
2017-10-11 15:32     ` Philipp Zabel
2017-10-11 15:54     ` Bryan O'Donoghue
2017-10-11 15:54       ` Bryan O'Donoghue
2017-10-09 14:11 ` [PATCH v3 4/7] nvmem: imx-ocotp: Move i.MX6 write clock setup to dedicated function Bryan O'Donoghue
2017-10-09 14:11   ` Bryan O'Donoghue
2017-10-09 14:11 ` [PATCH v3 5/7] nvmem: imx-ocotp: Add i.MX7D timing write clock setup support Bryan O'Donoghue
2017-10-09 14:11   ` Bryan O'Donoghue
2017-10-11 16:03   ` Philipp Zabel
2017-10-11 16:03     ` Philipp Zabel
2017-10-11 16:22     ` Bryan O'Donoghue
2017-10-11 16:22       ` Bryan O'Donoghue
2017-10-09 14:11 ` [PATCH v3 6/7] nvmem: imx-ocotp: Enable i.MX7D OTP write support Bryan O'Donoghue
2017-10-09 14:11   ` Bryan O'Donoghue
2017-10-09 14:11 ` [PATCH v3 7/7] nvmem: imx-ocotp: Update module description Bryan O'Donoghue
2017-10-09 14:11   ` Bryan O'Donoghue

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.