All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Add i.MX8MM support
@ 2019-04-19 17:19 Bryan O'Donoghue
  2019-04-19 17:19 ` [PATCH v3 1/5] nvmem: imx-ocotp: Elongate OCOTP_CTRL ADDR field to eight bits Bryan O'Donoghue
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Bryan O'Donoghue @ 2019-04-19 17:19 UTC (permalink / raw)
  To: l.stach, peng.fan, shawnguo, srinivas.kandagatla, leonard.crestez
  Cc: aisheng.dong, abel.vesa, anson.huang, linux-imx, kernel,
	fabio.estevam, Bryan O'Donoghue, linux-arm-kernel

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

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

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

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

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

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

Bryan O'Donoghue (5):
  nvmem: imx-ocotp: Elongate OCOTP_CTRL ADDR field to eight bits
  nvmem: imx-ocotp: Ensure WAIT bits are preserved when setting timing
  nvmem: imx-ocotp: Ensure the RELAX field is non-zero
  nvmem: imx-ocotp: Add i.MX8MM support
  dt-bindings: imx-ocotp: Add i.MX8MM compatible

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

-- 
2.20.1


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/5] nvmem: imx-ocotp: Elongate OCOTP_CTRL ADDR field to eight bits
  2019-04-19 17:19 [PATCH v3 0/5] Add i.MX8MM support Bryan O'Donoghue
@ 2019-04-19 17:19 ` Bryan O'Donoghue
  2019-04-19 20:59   ` Leonard Crestez
  2019-04-19 17:19 ` [PATCH v3 2/5] nvmem: imx-ocotp: Ensure WAIT bits are preserved when setting timing Bryan O'Donoghue
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Bryan O'Donoghue @ 2019-04-19 17:19 UTC (permalink / raw)
  To: l.stach, peng.fan, shawnguo, srinivas.kandagatla, leonard.crestez
  Cc: aisheng.dong, abel.vesa, anson.huang, linux-imx, kernel,
	fabio.estevam, Bryan O'Donoghue, linux-arm-kernel

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

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

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

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

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

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


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/5] nvmem: imx-ocotp: Ensure WAIT bits are preserved when setting timing
  2019-04-19 17:19 [PATCH v3 0/5] Add i.MX8MM support Bryan O'Donoghue
  2019-04-19 17:19 ` [PATCH v3 1/5] nvmem: imx-ocotp: Elongate OCOTP_CTRL ADDR field to eight bits Bryan O'Donoghue
@ 2019-04-19 17:19 ` Bryan O'Donoghue
  2019-04-19 22:32   ` Leonard Crestez
  2019-04-19 17:19 ` [PATCH v3 3/5] nvmem: imx-ocotp: Ensure the RELAX field is non-zero Bryan O'Donoghue
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Bryan O'Donoghue @ 2019-04-19 17:19 UTC (permalink / raw)
  To: l.stach, peng.fan, shawnguo, srinivas.kandagatla, leonard.crestez
  Cc: aisheng.dong, abel.vesa, anson.huang, linux-imx, kernel,
	fabio.estevam, Bryan O'Donoghue, linux-arm-kernel

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

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

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

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

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

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


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/5] nvmem: imx-ocotp: Ensure the RELAX field is non-zero
  2019-04-19 17:19 [PATCH v3 0/5] Add i.MX8MM support Bryan O'Donoghue
  2019-04-19 17:19 ` [PATCH v3 1/5] nvmem: imx-ocotp: Elongate OCOTP_CTRL ADDR field to eight bits Bryan O'Donoghue
  2019-04-19 17:19 ` [PATCH v3 2/5] nvmem: imx-ocotp: Ensure WAIT bits are preserved when setting timing Bryan O'Donoghue
@ 2019-04-19 17:19 ` Bryan O'Donoghue
  2019-04-22 10:28   ` Leonard Crestez
  2019-04-19 17:19 ` [PATCH v3 4/5] nvmem: imx-ocotp: Add i.MX8MM support Bryan O'Donoghue
  2019-04-19 17:19 ` [PATCH v3 5/5] dt-bindings: imx-ocotp: Add i.MX8MM compatible Bryan O'Donoghue
  4 siblings, 1 reply; 16+ messages in thread
From: Bryan O'Donoghue @ 2019-04-19 17:19 UTC (permalink / raw)
  To: l.stach, peng.fan, shawnguo, srinivas.kandagatla, leonard.crestez
  Cc: aisheng.dong, abel.vesa, anson.huang, linux-imx, kernel,
	fabio.estevam, Bryan O'Donoghue, linux-arm-kernel

The RELAX field of the OCOTP block quote "specifies the time to add to all
default timing parameters other than the Tpgm and Trd. It is given in
number of ipg_clk periods".

On the i.MX8MM the calculation for the RELAX value is turning out to be
zero which is not a problem for programming OTP values but, does
subsequently mess up reloading the OTP shadow registers.

This patch ensures the RELAX field is at least one ipg_clk cycle, which
seems like a pretty obvious floor to place on a value such as this.

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

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 85a7d0da3abb..5b625d61e433 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -186,6 +186,8 @@ static void imx_ocotp_set_imx6_timing(struct ocotp_priv *priv)
 	clk_rate = clk_get_rate(priv->clk);
 
 	relax = clk_rate / (1000000000 / DEF_RELAX) - 1;
+	if (!relax)
+		relax = 1;
 	strobe_prog = clk_rate / (1000000000 / 10000) + 2 * (DEF_RELAX + 1) - 1;
 	strobe_read = clk_rate / (1000000000 / 40) + 2 * (DEF_RELAX + 1) - 1;
 
-- 
2.20.1


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 4/5] nvmem: imx-ocotp: Add i.MX8MM support
  2019-04-19 17:19 [PATCH v3 0/5] Add i.MX8MM support Bryan O'Donoghue
                   ` (2 preceding siblings ...)
  2019-04-19 17:19 ` [PATCH v3 3/5] nvmem: imx-ocotp: Ensure the RELAX field is non-zero Bryan O'Donoghue
@ 2019-04-19 17:19 ` Bryan O'Donoghue
  2019-04-22 10:46   ` Leonard Crestez
  2019-04-19 17:19 ` [PATCH v3 5/5] dt-bindings: imx-ocotp: Add i.MX8MM compatible Bryan O'Donoghue
  4 siblings, 1 reply; 16+ messages in thread
From: Bryan O'Donoghue @ 2019-04-19 17:19 UTC (permalink / raw)
  To: l.stach, peng.fan, shawnguo, srinivas.kandagatla, leonard.crestez
  Cc: aisheng.dong, abel.vesa, anson.huang, linux-imx, kernel,
	fabio.estevam, Bryan O'Donoghue, linux-arm-kernel

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

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

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

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


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 5/5] dt-bindings: imx-ocotp: Add i.MX8MM compatible
  2019-04-19 17:19 [PATCH v3 0/5] Add i.MX8MM support Bryan O'Donoghue
                   ` (3 preceding siblings ...)
  2019-04-19 17:19 ` [PATCH v3 4/5] nvmem: imx-ocotp: Add i.MX8MM support Bryan O'Donoghue
@ 2019-04-19 17:19 ` Bryan O'Donoghue
  2019-04-22 10:35   ` Leonard Crestez
  4 siblings, 1 reply; 16+ messages in thread
From: Bryan O'Donoghue @ 2019-04-19 17:19 UTC (permalink / raw)
  To: l.stach, peng.fan, shawnguo, srinivas.kandagatla, leonard.crestez
  Cc: aisheng.dong, abel.vesa, anson.huang, linux-imx, kernel,
	fabio.estevam, Bryan O'Donoghue, linux-arm-kernel

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

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
---
 Documentation/devicetree/bindings/nvmem/imx-ocotp.txt | 1 +
 1 file changed, 1 insertion(+)

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


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/5] nvmem: imx-ocotp: Elongate OCOTP_CTRL ADDR field to eight bits
  2019-04-19 17:19 ` [PATCH v3 1/5] nvmem: imx-ocotp: Elongate OCOTP_CTRL ADDR field to eight bits Bryan O'Donoghue
@ 2019-04-19 20:59   ` Leonard Crestez
  0 siblings, 0 replies; 16+ messages in thread
From: Leonard Crestez @ 2019-04-19 20:59 UTC (permalink / raw)
  To: Bryan O'Donoghue, l.stach, Peng Fan, srinivas.kandagatla
  Cc: Aisheng Dong, Abel Vesa, Anson Huang, dl-linux-imx, kernel,
	Fabio Estevam, shawnguo, linux-arm-kernel

On 4/19/2019 8:19 PM, Bryan O'Donoghue wrote:
> i.MX6 defines OCOTP_CTRLn:ADDR as seven bit address-field with a one bit
> RSVD0 field, i.MX7 defines OCOTP_CTRLn:ADDR as a four bit address-field
> with a four bit RSVD0 field.
> 
> i.MX8 defines the OCOTP_CTRLn:ADDR bit-field as a full range eight bits.
> 
> i.MX6 and i.MX7 should return zero for their respective RSVD0 bits and
> ignore a write-back of zero where i.MX8 will make use of the full range.
> 
> This patch expands the bit-field definition for all users to eight bits,
> which is safe due to RSVD0 being a no-op for the i.MX6 and i.MX7.
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>

> ---
>   drivers/nvmem/imx-ocotp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index 4cf7b61e4bf5..6600c4ddeb51 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -45,7 +45,7 @@
>   #define IMX_OCOTP_ADDR_DATA2		0x0040
>   #define IMX_OCOTP_ADDR_DATA3		0x0050
>   
> -#define IMX_OCOTP_BM_CTRL_ADDR		0x0000007F
> +#define IMX_OCOTP_BM_CTRL_ADDR		0x000000FF
>   #define IMX_OCOTP_BM_CTRL_BUSY		0x00000100
>   #define IMX_OCOTP_BM_CTRL_ERROR		0x00000200
>   #define IMX_OCOTP_BM_CTRL_REL_SHADOWS	0x00000400
> 


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] nvmem: imx-ocotp: Ensure WAIT bits are preserved when setting timing
  2019-04-19 17:19 ` [PATCH v3 2/5] nvmem: imx-ocotp: Ensure WAIT bits are preserved when setting timing Bryan O'Donoghue
@ 2019-04-19 22:32   ` Leonard Crestez
  0 siblings, 0 replies; 16+ messages in thread
From: Leonard Crestez @ 2019-04-19 22:32 UTC (permalink / raw)
  To: Bryan O'Donoghue, l.stach, Peng Fan, shawnguo, srinivas.kandagatla
  Cc: Aisheng Dong, Abel Vesa, Anson Huang, dl-linux-imx, kernel,
	Fabio Estevam, linux-arm-kernel

On 4/19/19 8:19 PM, Bryan O'Donoghue wrote:
> The i.MX6 and i.MX8 both have a bit-field spanning bits 27:22 called the
> WAIT field.
> 
> The WAIT field according to the documentation for both parts "specifies
> time interval between auto read and write access in one time program. It is
> given in number of ipg_clk periods."
> 
> This patch ensures that the relevant field is read and written back to the
> timing register.
> 
> Fixes: 0642bac7da42 ("nvmem: imx-ocotp: add write support")
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] nvmem: imx-ocotp: Ensure the RELAX field is non-zero
  2019-04-19 17:19 ` [PATCH v3 3/5] nvmem: imx-ocotp: Ensure the RELAX field is non-zero Bryan O'Donoghue
@ 2019-04-22 10:28   ` Leonard Crestez
  2019-04-22 13:20     ` Bryan O'Donoghue
  2019-04-22 13:37     ` Bryan O'Donoghue
  0 siblings, 2 replies; 16+ messages in thread
From: Leonard Crestez @ 2019-04-22 10:28 UTC (permalink / raw)
  To: Bryan O'Donoghue, l.stach, Peng Fan
  Cc: Aisheng Dong, Abel Vesa, Anson Huang, srinivas.kandagatla,
	dl-linux-imx, kernel, Fabio Estevam, shawnguo, linux-arm-kernel

On 4/19/2019 8:19 PM, Bryan O'Donoghue wrote:
> The RELAX field of the OCOTP block quote "specifies the time to add to all
> default timing parameters other than the Tpgm and Trd. It is given in
> number of ipg_clk periods".
> 
> On the i.MX8MM the calculation for the RELAX value is turning out to be
> zero which is not a problem for programming OTP values but, does
> subsequently mess up reloading the OTP shadow registers.
> 
> This patch ensures the RELAX field is at least one ipg_clk cycle, which
> seems like a pretty obvious floor to place on a value such as this.
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> ---
>   drivers/nvmem/imx-ocotp.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index 85a7d0da3abb..5b625d61e433 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -186,6 +186,8 @@ static void imx_ocotp_set_imx6_timing(struct ocotp_priv *priv)
>   	clk_rate = clk_get_rate(priv->clk);
>   
>   	relax = clk_rate / (1000000000 / DEF_RELAX) - 1;
> +	if (!relax)
> +		relax = 1;

Math here hurts my head. Why is there a -1 above?

How about "relax = DIV_ROUND_UP(clk_rate, 1000000000 / DEF_RELAX);"? 
Rounding up seems safe and it should guarantee that the result is >= 1. 
As far as I understand a value which is slightly larger here shouldn't hurt.

Looking at uboot there DEF_RELAX is moved to the other side but that 
risks overflow at very high ipg rates.

Other chips have similar 66Mhz ipg rates; my guess is that this bug 
affects other chips as well.

--
Regards,
Leonard

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/5] dt-bindings: imx-ocotp: Add i.MX8MM compatible
  2019-04-19 17:19 ` [PATCH v3 5/5] dt-bindings: imx-ocotp: Add i.MX8MM compatible Bryan O'Donoghue
@ 2019-04-22 10:35   ` Leonard Crestez
  0 siblings, 0 replies; 16+ messages in thread
From: Leonard Crestez @ 2019-04-22 10:35 UTC (permalink / raw)
  To: Bryan O'Donoghue, l.stach, Peng Fan, shawnguo, srinivas.kandagatla
  Cc: Aisheng Dong, Abel Vesa, Anson Huang, dl-linux-imx, kernel,
	Fabio Estevam, linux-arm-kernel

On 4/19/2019 8:19 PM, Bryan O'Donoghue wrote:
> Add compatible for i.MX8MM as per arch/arm64/boot/dts/freescale/imx8mm.dtsi
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>

There are tiny differences between ocotp blocks so listing separate 
compats is useful.


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/5] nvmem: imx-ocotp: Add i.MX8MM support
  2019-04-19 17:19 ` [PATCH v3 4/5] nvmem: imx-ocotp: Add i.MX8MM support Bryan O'Donoghue
@ 2019-04-22 10:46   ` Leonard Crestez
  2019-04-22 13:22     ` Bryan O'Donoghue
  0 siblings, 1 reply; 16+ messages in thread
From: Leonard Crestez @ 2019-04-22 10:46 UTC (permalink / raw)
  To: Bryan O'Donoghue, l.stach, Peng Fan
  Cc: Aisheng Dong, Abel Vesa, Anson Huang, srinivas.kandagatla,
	dl-linux-imx, kernel, Fabio Estevam, shawnguo, linux-arm-kernel

On 4/19/2019 8:19 PM, Bryan O'Donoghue wrote:
> This patch adds support to burn the fuses on the i.MX8MM.
> 
> The i.MX8MM is similar to i.MX6 processors in terms of addressing and clock
> setup.
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> ---
>   drivers/nvmem/imx-ocotp.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index 5b625d61e433..08006b07657b 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -453,6 +453,12 @@ static const struct ocotp_params imx8mq_params = {
>   	.set_timing = imx_ocotp_set_imx7_timing,
>   };
>   
> +static const struct ocotp_params imx8mm_params = {
> +	.nregs = 60,
> +	.bank_address_words = 0,
> +	.set_timing = imx_ocotp_set_imx6_timing,
> +};

I'm not sure about that nregs. The reference manual documents registers 
up to 0x303507b0 in the "OCOTP" chapter while "fusemap" documents up to 
0x303513F0: that's where 60 vs 256 comes from.

This is from "i.MX 8M Mini Applications Processor Reference Manual" 
"Rev. 1 03/2019" from nxp.com: 
https://www.nxp.com/products/processors-and-microcontrollers/arm-based-processors-and-mcus/i.mx-applications-processors/i.mx-8-processors/i.mx-8m-mini-arm-cortex-a53-cortex-m4-audio-voice-video:i.MX8MMINI?tab=Documentation_Tab

Same situation for 8mq and for that part we have nregs=256. All the 
higher fuses seem to be "reserved" but I don't think there's any harm in 
allowing reads. At least driver should be consistent for 8mm/8mq.

--
Regards,
Leonard

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] nvmem: imx-ocotp: Ensure the RELAX field is non-zero
  2019-04-22 10:28   ` Leonard Crestez
@ 2019-04-22 13:20     ` Bryan O'Donoghue
  2019-04-22 13:37     ` Bryan O'Donoghue
  1 sibling, 0 replies; 16+ messages in thread
From: Bryan O'Donoghue @ 2019-04-22 13:20 UTC (permalink / raw)
  To: Leonard Crestez, l.stach, Peng Fan
  Cc: Aisheng Dong, Abel Vesa, Anson Huang, srinivas.kandagatla,
	dl-linux-imx, kernel, Fabio Estevam, shawnguo, linux-arm-kernel

On 22/04/2019 11:28, Leonard Crestez wrote:
> On 4/19/2019 8:19 PM, Bryan O'Donoghue wrote:
>> The RELAX field of the OCOTP block quote "specifies the time to add to all
>> default timing parameters other than the Tpgm and Trd. It is given in
>> number of ipg_clk periods".
>>
>> On the i.MX8MM the calculation for the RELAX value is turning out to be
>> zero which is not a problem for programming OTP values but, does
>> subsequently mess up reloading the OTP shadow registers.
>>
>> This patch ensures the RELAX field is at least one ipg_clk cycle, which
>> seems like a pretty obvious floor to place on a value such as this.
>>
>> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
>> ---
>>    drivers/nvmem/imx-ocotp.c | 2 ++
>>    1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
>> index 85a7d0da3abb..5b625d61e433 100644
>> --- a/drivers/nvmem/imx-ocotp.c
>> +++ b/drivers/nvmem/imx-ocotp.c
>> @@ -186,6 +186,8 @@ static void imx_ocotp_set_imx6_timing(struct ocotp_priv *priv)
>>    	clk_rate = clk_get_rate(priv->clk);
>>    
>>    	relax = clk_rate / (1000000000 / DEF_RELAX) - 1;
>> +	if (!relax)
>> +		relax = 1;
> 
> Math here hurts my head. Why is there a -1 above?

I thought of that, in theory it probably works, in practice I'm not so 
sure how it will affect the imx6, where the above code 'just works' 
right now.


> How about "relax = DIV_ROUND_UP(clk_rate, 1000000000 / DEF_RELAX);"?
> Rounding up seems safe and it should guarantee that the result is >= 1.
> As far as I understand a value which is slightly larger here shouldn't hurt.
> 
> Looking at uboot there DEF_RELAX is moved to the other side but that
> risks overflow at very high ipg rates.
> 
> Other chips have similar 66Mhz ipg rates; my guess is that this bug
> affects other chips as well.

Could be. I thought of converting this calculation to the u-boot version 
but... I don't want to break existing code for other users.

I have one i.MX6 board.

It seems safer to me to catch the corner-case for imx8mm.

But, I'm open to further convincing

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/5] nvmem: imx-ocotp: Add i.MX8MM support
  2019-04-22 10:46   ` Leonard Crestez
@ 2019-04-22 13:22     ` Bryan O'Donoghue
  0 siblings, 0 replies; 16+ messages in thread
From: Bryan O'Donoghue @ 2019-04-22 13:22 UTC (permalink / raw)
  To: Leonard Crestez, l.stach, Peng Fan
  Cc: Aisheng Dong, Abel Vesa, Anson Huang, srinivas.kandagatla,
	dl-linux-imx, kernel, Fabio Estevam, shawnguo, linux-arm-kernel

On 22/04/2019 11:46, Leonard Crestez wrote:
> Same situation for 8mq and for that part we have nregs=256. All the
> higher fuses seem to be "reserved" but I don't think there's any harm in
> allowing reads. At least driver should be consistent for 8mm/8mq.

OK, well you're the NXP guy.

I don't mind expanding the fusemap to undocumented registers in principle.

---
bod

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] nvmem: imx-ocotp: Ensure the RELAX field is non-zero
  2019-04-22 10:28   ` Leonard Crestez
  2019-04-22 13:20     ` Bryan O'Donoghue
@ 2019-04-22 13:37     ` Bryan O'Donoghue
  2019-04-22 14:35       ` Leonard Crestez
  1 sibling, 1 reply; 16+ messages in thread
From: Bryan O'Donoghue @ 2019-04-22 13:37 UTC (permalink / raw)
  To: Leonard Crestez, l.stach, Peng Fan
  Cc: Aisheng Dong, Abel Vesa, Anson Huang, srinivas.kandagatla,
	dl-linux-imx, kernel, Fabio Estevam, shawnguo, linux-arm-kernel

On 22/04/2019 11:28, Leonard Crestez wrote:
> How about "relax = DIV_ROUND_UP(clk_rate, 1000000000 / DEF_RELAX);"?
> Rounding up seems safe and it should guarantee that the result is >= 1.
> As far as I understand a value which is slightly larger here shouldn't hurt.

Actually re-reading your comment I think I'd be on for that.

Dropping the -1 for the RELAX field should be fine for all users - 
because DEF_RELAX != RELAX.

RELAX specifies a number of cycles to do nothing.
DEF_RELAX is a field in a calculation, they are not in fact the same thing.

So it is completely valid to vary RELAX w/r to the DEF_RELAX value we 
have right now and I'm probably being too conservative with the fix-up 
as-is.

---
bod

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] nvmem: imx-ocotp: Ensure the RELAX field is non-zero
  2019-04-22 13:37     ` Bryan O'Donoghue
@ 2019-04-22 14:35       ` Leonard Crestez
  2019-04-22 14:56         ` Bryan O'Donoghue
  0 siblings, 1 reply; 16+ messages in thread
From: Leonard Crestez @ 2019-04-22 14:35 UTC (permalink / raw)
  To: Bryan O'Donoghue, l.stach, Peng Fan
  Cc: Aisheng Dong, Abel Vesa, Anson Huang, srinivas.kandagatla,
	dl-linux-imx, kernel, Fabio Estevam, shawnguo, linux-arm-kernel,
	Richard Leitner

On 4/22/2019 4:37 PM, Bryan O'Donoghue wrote:
> On 22/04/2019 11:28, Leonard Crestez wrote:
>> How about "relax = DIV_ROUND_UP(clk_rate, 1000000000 / DEF_RELAX);"?
>> Rounding up seems safe and it should guarantee that the result is >= 1.
>> As far as I understand a value which is slightly larger here shouldn't hurt.
> 
> Actually re-reading your comment I think I'd be on for that.
> 
> Dropping the -1 for the RELAX field should be fine for all users -
> because DEF_RELAX != RELAX.
> 
> RELAX specifies a number of cycles to do nothing.
> DEF_RELAX is a field in a calculation, they are not in fact the same thing.
> 
> So it is completely valid to vary RELAX w/r to the DEF_RELAX value we
> have right now and I'm probably being too conservative with the fix-up
> as-is.

It seems that all imx6 have an ipg clk of 66Mhz and writes currently 
look like this:

clk_rate=66000000 relax=0 strobe_read=43 strobe_prog=701 timing=002b02bd

As far as I can tell the issue you reported for 8m should affect all imx6.

--
Regards,
Leonard

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] nvmem: imx-ocotp: Ensure the RELAX field is non-zero
  2019-04-22 14:35       ` Leonard Crestez
@ 2019-04-22 14:56         ` Bryan O'Donoghue
  0 siblings, 0 replies; 16+ messages in thread
From: Bryan O'Donoghue @ 2019-04-22 14:56 UTC (permalink / raw)
  To: Leonard Crestez, l.stach, Peng Fan
  Cc: Aisheng Dong, Abel Vesa, Anson Huang, srinivas.kandagatla,
	dl-linux-imx, kernel, Fabio Estevam, shawnguo, linux-arm-kernel,
	Richard Leitner

On 22/04/2019 15:35, Leonard Crestez wrote:
> clk_rate=66000000 relax=0 strobe_read=43 strobe_prog=701 timing=002b02bd
> 
> As far as I can tell the issue you reported for 8m should affect all imx6.

OK, I'll add a Fixes in that case

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-04-22 14:56 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-19 17:19 [PATCH v3 0/5] Add i.MX8MM support Bryan O'Donoghue
2019-04-19 17:19 ` [PATCH v3 1/5] nvmem: imx-ocotp: Elongate OCOTP_CTRL ADDR field to eight bits Bryan O'Donoghue
2019-04-19 20:59   ` Leonard Crestez
2019-04-19 17:19 ` [PATCH v3 2/5] nvmem: imx-ocotp: Ensure WAIT bits are preserved when setting timing Bryan O'Donoghue
2019-04-19 22:32   ` Leonard Crestez
2019-04-19 17:19 ` [PATCH v3 3/5] nvmem: imx-ocotp: Ensure the RELAX field is non-zero Bryan O'Donoghue
2019-04-22 10:28   ` Leonard Crestez
2019-04-22 13:20     ` Bryan O'Donoghue
2019-04-22 13:37     ` Bryan O'Donoghue
2019-04-22 14:35       ` Leonard Crestez
2019-04-22 14:56         ` Bryan O'Donoghue
2019-04-19 17:19 ` [PATCH v3 4/5] nvmem: imx-ocotp: Add i.MX8MM support Bryan O'Donoghue
2019-04-22 10:46   ` Leonard Crestez
2019-04-22 13:22     ` Bryan O'Donoghue
2019-04-19 17:19 ` [PATCH v3 5/5] dt-bindings: imx-ocotp: Add i.MX8MM compatible Bryan O'Donoghue
2019-04-22 10:35   ` Leonard Crestez

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.