diff mbox series

[2/2] nvmem: imx-ocotp: Support multiple word writes

Message ID 20190709183016.4789-2-tpiepho@impinj.com (mailing list archive)
State New, archived
Headers show
Series [1/2] nvmem: imx-ocotp: Set type to OTP | expand

Commit Message

Trent Piepho July 9, 2019, 6:30 p.m. UTC
All the other nvmem drivers here support multiple words being read, and
for writable memory, written in one call.  This driver appears to be the
only one with a single word write restriction.  It makes the driver fail
with generic userspace nvmem tools.

It's easy to support multiple words to write so do that.

The nvmem core verifies the write length against the word size, so that
can be removed from the driver.  But offset still needs to be checked.

Also simplify the bank write code for imx7 to avoid a lot of
duplication.

Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Signed-off-by: Trent Piepho <tpiepho@impinj.com>
---
 drivers/nvmem/imx-ocotp.c | 108 +++++++++++++++++++++++-----------------------
 1 file changed, 54 insertions(+), 54 deletions(-)

Comments

Srinivas Kandagatla Aug. 6, 2019, 9:57 a.m. UTC | #1
On 09/07/2019 19:30, Trent Piepho wrote:
> All the other nvmem drivers here support multiple words being read, and
> for writable memory, written in one call.  This driver appears to be the
> only one with a single word write restriction.  It makes the driver fail
> with generic userspace nvmem tools.
> 
> It's easy to support multiple words to write so do that.
> 
> The nvmem core verifies the write length against the word size, so that
> can be removed from the driver.  But offset still needs to be checked.
> 
> Also simplify the bank write code for imx7 to avoid a lot of
> duplication.
> 
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Trent Piepho <tpiepho@impinj.com>


Any Acks or Tested-by before I can push this would be really appreciated.

--srini
diff mbox series

Patch

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index d45e71e9b73a..a8b5ea553c12 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -240,33 +240,13 @@  static void imx_ocotp_set_imx7_timing(struct ocotp_priv *priv)
 	writel(timing, priv->base + IMX_OCOTP_ADDR_TIMING);
 }
 
-static int imx_ocotp_write(void *context, unsigned int offset, void *val,
-			   size_t bytes)
+/* Write a single word to ocotp */
+static int imx_ocotp_write_word(struct ocotp_priv *priv, unsigned int offset,
+				u32 val)
 {
-	struct ocotp_priv *priv = context;
-	u32 *buf = val;
 	int ret;
-
 	u32 ctrl;
-	u8 waddr;
-	u8 word = 0;
-
-	/* allow only writing one complete OTP word at a time */
-	if ((bytes != priv->config->word_size) ||
-	    (offset % priv->config->word_size))
-		return -EINVAL;
-
-	mutex_lock(&ocotp_mutex);
-
-	ret = clk_prepare_enable(priv->clk);
-	if (ret < 0) {
-		mutex_unlock(&ocotp_mutex);
-		dev_err(priv->dev, "failed to prepare/enable ocotp clk\n");
-		return ret;
-	}
-
-	/* Setup the write timing values */
-	priv->params->set_timing(priv);
+	u8 word, waddr;
 
 	/* 47.3.1.3.2
 	 * Check that HW_OCOTP_CTRL[BUSY] and HW_OCOTP_CTRL[ERROR] are clear.
@@ -277,7 +257,7 @@  static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	ret = imx_ocotp_wait_for_busy(priv->base, 0);
 	if (ret < 0) {
 		dev_err(priv->dev, "timeout during timing setup\n");
-		goto write_end;
+		return ret;
 	}
 
 	/* 47.3.1.3.3
@@ -303,6 +283,7 @@  static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 		 * locations
 		 */
 		waddr = offset / 4;
+		word = 0;
 	}
 
 	ctrl = readl(priv->base + IMX_OCOTP_ADDR_CTRL);
@@ -335,36 +316,19 @@  static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	 *	 register written.
 	 */
 	if (priv->params->bank_address_words != 0) {
-		/* Banked/i.MX7 mode */
-		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;
-		}
+		/* Banked/i.MX7 mode. All four data words are zero ... */
+		u32 data[4] = { 0, 0, 0, 0 };
+		/* ... except the word we are programming. */
+		data[word] = val;
+
+		/* Always write in order 1,2,3,0: 0 being last is critical */
+		writel(data[1], priv->base + IMX_OCOTP_ADDR_DATA1);
+		writel(data[2], priv->base + IMX_OCOTP_ADDR_DATA2);
+		writel(data[3], priv->base + IMX_OCOTP_ADDR_DATA3);
+		writel(data[0], priv->base + IMX_OCOTP_ADDR_DATA0);
 	} else {
 		/* Non-banked i.MX6 mode */
-		writel(*buf, priv->base + IMX_OCOTP_ADDR_DATA0);
+		writel(val, priv->base + IMX_OCOTP_ADDR_DATA0);
 	}
 
 	/* 47.4.1.4.5
@@ -381,8 +345,9 @@  static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 			imx_ocotp_clr_err_if_set(priv->base);
 		} else {
 			dev_err(priv->dev, "timeout during data write\n");
+			ret = -ETIMEDOUT;
 		}
-		goto write_end;
+		return ret;
 	}
 
 	/* 47.3.1.4
@@ -393,6 +358,41 @@  static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	 */
 	udelay(2);
 
+	return 0;
+}
+
+static int imx_ocotp_write(void *context, unsigned int offset, void *val,
+			   size_t bytes)
+{
+	struct ocotp_priv *priv = context;
+	u32 *buf = val;
+	int ret;
+
+	/* While the nvmem core will check that bytes is a multiple of the word
+	 * size, it does not check that offset is aligned to the word size or
+	 * the stride.
+	 */
+	if (offset % priv->config->word_size)
+		return -EINVAL;
+
+	mutex_lock(&ocotp_mutex);
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret < 0) {
+		mutex_unlock(&ocotp_mutex);
+		dev_err(priv->dev, "failed to prepare/enable ocotp clk\n");
+		return ret;
+	}
+
+	/* Setup the write timing values */
+	priv->params->set_timing(priv);
+
+	for (; bytes > 0; bytes -= 4, offset += 4, buf++) {
+		ret = imx_ocotp_write_word(priv, offset, *buf);
+		if (ret)
+			goto write_end;
+	}
+
 	/* reload all shadow registers */
 	writel(IMX_OCOTP_BM_CTRL_REL_SHADOWS,
 	       priv->base + IMX_OCOTP_ADDR_CTRL_SET);