[PATCH 0/2] nvmem: imx-ocotp: allow reads with arbitrary size and offset

[PATCH 0/2] nvmem: imx-ocotp: allow reads with arbitrary size and offset

[Jose Diaz de Grenu]

Currently the imx-ocotp driver does only allow reading complete OTP words
correcty aligned.

Usually OTP memory is limited, so the fields are stored using as few bits as
possible. This means that a given value rarely uses 32 bits and happens to be
aligned.

Even though the NVMEM API offers a way to define offset and size of each cell
(at bit level) this is not currently usable iwth the imx-ocotp driver, which
forces consumers to read complete words and then hardcode the necessary
shifting and masking in the driver code. 

As an example take the nvmem consumer imx_thermal.c, which reads nvmem cells
as uint32_t words:

	ret = nvmem_cell_read_u32(&pdev->dev, "calib", &val);
	if (ret)
		return ret;

	ret = imx_init_calib(pdev, val);
	if (ret)
		return ret;

	ret = nvmem_cell_read_u32(&pdev->dev, "temp_grade", &val);
	if (ret)
		return ret;
	imx_init_temp_grade(pdev, val);
	
but needs to later adjust the values in code:

	// Inside imx_init_calib()
	data->c1 = (ocotp_ana1 >> 9) & 0x1ff;

	// Inside imx_init_temp_grade()
	switch ((ocotp_mem0 >> 6) & 0x3) {
	
This patch adjusts the driver so that reads can be requested using any size
and offset. Then, for example the nvmem cell "calib" could use the 'bits'
property to specify size and offset in bits, removing the need to mask and
shift in the driver code.

This is specially useful when several drivers use the same nvmem cell and when
the specific size and offset of a OTP value depends on a hardware version.

Jose Diaz de Grenu (2):
  nvmem: imx-ocotp: use constant for write restriction
  nvmem: imx-ocotp: allow reads with arbitrary size and offset

 drivers/nvmem/imx-ocotp.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)


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

[PATCH 1/2] nvmem: imx-ocotp: use constant for write restriction

[Jose Diaz de Grenu]

Use a new constant instead of reusing config->word_size, which applies
both to read and writes. This allows to change config->word_size without
affecting to the write size restriction.

Signed-off-by: Jose Diaz de Grenu <Jose.DiazdeGrenu@digi.com>
---
 drivers/nvmem/imx-ocotp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 42d4451e7d67..dc86d863563a 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -29,6 +29,7 @@
 #define IMX_OCOTP_OFFSET_PER_WORD	0x10  /* Offset between the start addr
 					       * of two consecutive OTP words.
 					       */
+#define IMX_OTP_WORD_SIZE		4
 
 #define IMX_OCOTP_ADDR_CTRL		0x0000
 #define IMX_OCOTP_ADDR_CTRL_SET		0x0004
@@ -252,8 +253,8 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 	u8 word = 0;
 
 	/* allow only writing one complete OTP word at a time */
-	if ((bytes != priv->config->word_size) ||
-	    (offset % priv->config->word_size))
+	if ((bytes != IMX_OTP_WORD_SIZE) ||
+	    (offset % IMX_OTP_WORD_SIZE))
 		return -EINVAL;
 
 	mutex_lock(&ocotp_mutex);
@@ -293,7 +294,7 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 		 * see i.MX 7Solo Applications Processor Reference Manual, Rev.
 		 * 0.1 section 6.4.3.1
 		 */
-		offset = offset / priv->config->word_size;
+		offset = offset / IMX_OTP_WORD_SIZE;
 		waddr = offset / priv->params->bank_address_words;
 		word  = offset & (priv->params->bank_address_words - 1);
 	} else {

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

[PATCH 2/2] nvmem: imx-ocotp: allow reads with arbitrary size and offset

[Jose Diaz de Grenu]

Currently the driver only allows to read 32 aligned 32-bit chunks. This
is inconvenient when defining nvmem-cells, as they are rarely a multiple
of 32 bits in size, and that makes the nvmem-consumer hardcode offsets and
masks to extract the real value.

Remove the limitation but keep reading in 32-bit chunks from the hardware
to ensure there is no change in the behaviour.

Signed-off-by: Jose Diaz de Grenu <Jose.DiazdeGrenu@digi.com>
---
 drivers/nvmem/imx-ocotp.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index dc86d863563a..9590eeab85d8 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -121,16 +121,8 @@ static int imx_ocotp_read(void *context, unsigned int offset,
 			  void *val, size_t bytes)
 {
 	struct ocotp_priv *priv = context;
-	unsigned int count;
-	u32 *buf = val;
+	u8 *buf = val;
 	int i, ret;
-	u32 index;
-
-	index = offset >> 2;
-	count = bytes >> 2;
-
-	if (count > (priv->params->nregs - index))
-		count = priv->params->nregs - index;
 
 	mutex_lock(&ocotp_mutex);
 
@@ -147,9 +139,9 @@ static int imx_ocotp_read(void *context, unsigned int offset,
 		goto read_end;
 	}
 
-	for (i = index; i < (index + count); i++) {
-		*buf++ = readl(priv->base + IMX_OCOTP_OFFSET_B0W0 +
-			       i * IMX_OCOTP_OFFSET_PER_WORD);
+	for (i = offset; i < (bytes + offset); i++) {
+		u32 word_val = readl(priv->base + IMX_OCOTP_OFFSET_B0W0 +
+				     (i >> 2) * IMX_OCOTP_OFFSET_PER_WORD);
 
 		/* 47.3.1.2
 		 * For "read locked" registers 0xBADABADA will be returned and
@@ -157,9 +149,14 @@ static int imx_ocotp_read(void *context, unsigned int offset,
 		 * software before any new write, read or reload access can be
 		 * issued
 		 */
-		if (*(buf - 1) == IMX_OCOTP_READ_LOCKED_VAL)
+		if (word_val == IMX_OCOTP_READ_LOCKED_VAL)
 			imx_ocotp_clr_err_if_set(priv->base);
+
+		word_val >>= (i % 4) * 8;
+
+		*buf++ = (u8) (word_val & 0xFF);
 	}
+
 	ret = 0;
 
 read_end:
@@ -415,8 +412,8 @@ static int imx_ocotp_write(void *context, unsigned int offset, void *val,
 static struct nvmem_config imx_ocotp_nvmem_config = {
 	.name = "imx-ocotp",
 	.read_only = false,
-	.word_size = 4,
-	.stride = 4,
+	.word_size = 1,
+	.stride = 1,
 	.reg_read = imx_ocotp_read,
 	.reg_write = imx_ocotp_write,
 };

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

Re: [PATCH 0/2] nvmem: imx-ocotp: allow reads with arbitrary size and offset

[Srinivas Kandagatla]

On 23/07/2019 16:32, Jose Diaz de Grenu wrote:
> Currently the imx-ocotp driver does only allow reading complete OTP words
> correcty aligned.
> 
> Usually OTP memory is limited, so the fields are stored using as few bits as
> possible. This means that a given value rarely uses 32 bits and happens to be
> aligned.
> 
> Even though the NVMEM API offers a way to define offset and size of each cell
> (at bit level) this is not currently usable iwth the imx-ocotp driver, which
> forces consumers to read complete words and then hardcode the necessary
> shifting and masking in the driver code.
> 
> As an example take the nvmem consumer imx_thermal.c, which reads nvmem cells
> as uint32_t words:
> 
> 	ret = nvmem_cell_read_u32(&pdev->dev, "calib", &val);
> 	if (ret)
> 		return ret;
> 
> 	ret = imx_init_calib(pdev, val);
> 	if (ret)
> 		return ret;
> 
> 	ret = nvmem_cell_read_u32(&pdev->dev, "temp_grade", &val);
> 	if (ret)
> 		return ret;
> 	imx_init_temp_grade(pdev, val);
> 	
> but needs to later adjust the values in code:
> 
> 	// Inside imx_init_calib()
> 	data->c1 = (ocotp_ana1 >> 9) & 0x1ff;
> 
> 	// Inside imx_init_temp_grade()
> 	switch ((ocotp_mem0 >> 6) & 0x3) {
> 	
> This patch adjusts the driver so that reads can be requested using any size
> and offset. Then, for example the nvmem cell "calib" could use the 'bits'
> property to specify size and offset in bits, removing the need to mask and
> shift in the driver code.
> 
> This is specially useful when several drivers use the same nvmem cell and when
> the specific size and offset of a OTP value depends on a hardware version.
> 
> Jose Diaz de Grenu (2):
>    nvmem: imx-ocotp: use constant for write restriction
>    nvmem: imx-ocotp: allow reads with arbitrary size and offset
> 
>   drivers/nvmem/imx-ocotp.c | 34 ++++++++++++++++------------------
>   1 file changed, 16 insertions(+), 18 deletions(-)

Anyone form IMX can test this patchset before I push this out?

Thanks,
srini
> 

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

RE: [PATCH 0/2] nvmem: imx-ocotp: allow reads with arbitrary size and offset

[Diaz de Grenu, Jose]

On 06/08/2019 12:06 Srinivas Kandagatla wrote:
> Anyone form IMX can test this patchset before I push this out?
>
> Thanks,
> srini

Just for the record, I tested this on an i.MX6UL based board.

Let me know if there is something I can do to facilitate the testing to anyone from IMX.

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