[PATCH v6 0/2] Make i.MX9 OCOTP work as accessing controller

[PATCH v6 0/2] Make i.MX9 OCOTP work as accessing controller

[Peng Fan (OSS)]

Previously there was discussion on using "feature-controller" or
adding a specific node to group the fuse information for U-Boot to
parse and update node. But rejected by DT maintainer(Rob).

Since the STM32 engineers added access controller concept and landed
the support in Linux Kernel, reuse the concept for i.MX.

i.MX OCOTP eFuse has some bits to indicate a specific periphral or
function is avaiable for current chip, for example i.MX93 has variants
that not have NPU. So introduce '#access-controller-cells' for OCOTP,
and add "#access-controllers = <&ocotp [gate_index]>" for periphrals
that needs accessing control checking.

In OCOTP driver, the nodes under "/" will be iterated, and checked
if property "#access-controllers" exsits. If not allow access, detach
the node.

In the end, cpu nodes should still be handled by bootloader.
And I may also update i.MX8M to use same method to support variants
if this patchset is good.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v6:
- Rebased to next-2025-1-20
- Update Kconfig per Alexander
- Link to v5: https://lore.kernel.org/r/20250108-imx-ocotp-v5-0-a6d90e18ebe9@nxp.com

Changes in v5:
- Rebased to next-20250107, but seems Srinivas's for-fixes patches not
included, so I picked the for-fixes patches and rebase the patchset.
- make gates pointer and using ARRAY_SIZE
- Use of_parse_phandle_by_args
- Read fuse when needed, not cache them in the very beginning
- Use %pOF for node name
- Check controller node name in imx_ele_ocotp_grant_access
- Use dev_info when detach node
- Use priv as function parameter to replace pdev.
- Link to v4: https://lore.kernel.org/r/20241223-imx-ocotp-v4-0-f504c1420640@nxp.com

Changes in v4:
- Add missing of_node_put(provider)
- Typo fix in commit log
- Link to v3: https://lore.kernel.org/r/20241219-imx-ocotp-v3-0-b22371098071@nxp.com

Changes in v3:
- Add A-b for patch 1
- Use scoped API when iterate nodes and address build warning in patch 2
- I am also thinking that to drop patch 2 to let
 bootloader to iterate the nodes that have access-controllers property and
 update the nodes before booting the kernel. Then just need patch 1 for
 the binding updates. Anyway, comments are welcomed.
- Link to v2: https://lore.kernel.org/r/20241217-imx-ocotp-v2-0-3faa6cbff41b@nxp.com

Changes in v2:
- Add R-b for patch 1
- Use '/' root node instead of '/soc' or '/soc@0' for patch 2
- Link to v1: https://lore.kernel.org/r/20241212-imx-ocotp-v1-0-198bb0af86a0@nxp.com

---
Peng Fan (2):
      dt-bindings: nvmem: imx-ocotp: Introduce #access-controller-cells
      nvmem: imx-ocotp-ele: Support accessing controller for i.MX9

 .../devicetree/bindings/nvmem/imx-ocotp.yaml       |   5 +
 drivers/nvmem/Kconfig                              |   3 +
 drivers/nvmem/imx-ocotp-ele.c                      | 172 ++++++++++++++++++++-
 include/dt-bindings/nvmem/fsl,imx93-ocotp.h        |  24 +++
 include/dt-bindings/nvmem/fsl,imx95-ocotp.h        |  43 ++++++
 5 files changed, 246 insertions(+), 1 deletion(-)
---
base-commit: 8c50c358a94b6f6d7e6185966830b5bc3afa5524
change-id: 20241212-imx-ocotp-cbef304140f5

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH v6 1/2] dt-bindings: nvmem: imx-ocotp: Introduce #access-controller-cells

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Introduce "#access-controller-cells" to make OCOTP be an accessing
controller, because i.MX Family OCOTP supports a specific peripheral
or function being fused which means being disabled.

Add the i.MX[95,93] OCOTP gate index.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/nvmem/imx-ocotp.yaml       |  5 +++
 include/dt-bindings/nvmem/fsl,imx93-ocotp.h        | 24 ++++++++++++
 include/dt-bindings/nvmem/fsl,imx95-ocotp.h        | 43 ++++++++++++++++++++++
 3 files changed, 72 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml b/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml
index b2cb76cf9053a883a158acaf5eaa108895818afc..c78e202ced22f1c278f7be827b71ba434832d2a7 100644
--- a/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml
+++ b/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml
@@ -54,6 +54,11 @@ properties:
   clocks:
     maxItems: 1
 
+  "#access-controller-cells":
+    const: 1
+    description:
+      Contains the gate ID associated to the peripheral.
+
 required:
   - "#address-cells"
   - "#size-cells"
diff --git a/include/dt-bindings/nvmem/fsl,imx93-ocotp.h b/include/dt-bindings/nvmem/fsl,imx93-ocotp.h
new file mode 100644
index 0000000000000000000000000000000000000000..6ef525173845fd4ee0e847cf5a17e53a14f71362
--- /dev/null
+++ b/include/dt-bindings/nvmem/fsl,imx93-ocotp.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+
+#ifndef _DT_BINDINGS_NVMEM_IMX93_OTPC_H
+#define _DT_BINDINGS_NVMEM_IMX93_OTPC_H
+
+#define IMX93_OCOTP_NPU_GATE		0
+#define IMX93_OCOTP_A550_GATE		1
+#define IMX93_OCOTP_A551_GATE		2
+#define IMX93_OCOTP_M33_GATE		3
+#define IMX93_OCOTP_CAN1_FD_GATE	4
+#define IMX93_OCOTP_CAN2_FD_GATE	5
+#define IMX93_OCOTP_CAN1_GATE		6
+#define IMX93_OCOTP_CAN2_GATE		7
+#define IMX93_OCOTP_USB1_GATE		8
+#define IMX93_OCOTP_USB2_GATE		9
+#define IMX93_OCOTP_ENET1_GATE		10
+#define IMX93_OCOTP_ENET2_GATE		11
+#define IMX93_OCOTP_PXP_GATE		12
+#define IMX93_OCOTP_MIPI_CSI1_GATE	13
+#define IMX93_OCOTP_MIPI_DSI1_GATE	14
+#define IMX93_OCOTP_LVDS1_GATE		15
+#define IMX93_OCOTP_ADC1_GATE		16
+
+#endif
diff --git a/include/dt-bindings/nvmem/fsl,imx95-ocotp.h b/include/dt-bindings/nvmem/fsl,imx95-ocotp.h
new file mode 100644
index 0000000000000000000000000000000000000000..2d21d1f690974d0215c71352168378a150f489af
--- /dev/null
+++ b/include/dt-bindings/nvmem/fsl,imx95-ocotp.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+
+#ifndef _DT_BINDINGS_NVMEM_IMX95_OTPC_H
+#define _DT_BINDINGS_NVMEM_IMX95_OTPC_H
+
+#define IMX95_OCOTP_CANFD1_GATE		0
+#define IMX95_OCOTP_CANFD2_GATE		1
+#define IMX95_OCOTP_CANFD3_GATE		2
+#define IMX95_OCOTP_CANFD4_GATE		3
+#define IMX95_OCOTP_CANFD5_GATE		4
+#define IMX95_OCOTP_CAN1_GATE		5
+#define IMX95_OCOTP_CAN2_GATE		6
+#define IMX95_OCOTP_CAN3_GATE		7
+#define IMX95_OCOTP_CAN4_GATE		8
+#define IMX95_OCOTP_CAN5_GATE		9
+#define IMX95_OCOTP_NPU_GATE		10
+#define IMX95_OCOTP_A550_GATE		11
+#define IMX95_OCOTP_A551_GATE		12
+#define IMX95_OCOTP_A552_GATE		13
+#define IMX95_OCOTP_A553_GATE		14
+#define IMX95_OCOTP_A554_GATE		15
+#define IMX95_OCOTP_A555_GATE		16
+#define IMX95_OCOTP_M7_GATE		17
+#define IMX95_OCOTP_DCSS_GATE		18
+#define IMX95_OCOTP_LVDS1_GATE		19
+#define IMX95_OCOTP_ISP_GATE		20
+#define IMX95_OCOTP_USB1_GATE		21
+#define IMX95_OCOTP_USB2_GATE		22
+#define IMX95_OCOTP_NETC_GATE		23
+#define IMX95_OCOTP_PCIE1_GATE		24
+#define IMX95_OCOTP_PCIE2_GATE		25
+#define IMX95_OCOTP_ADC1_GATE		26
+#define IMX95_OCOTP_EARC_RX_GATE	27
+#define IMX95_OCOTP_GPU3D_GATE		28
+#define IMX95_OCOTP_VPU_GATE		29
+#define IMX95_OCOTP_JPEG_ENC_GATE	30
+#define IMX95_OCOTP_JPEG_DEC_GATE	31
+#define IMX95_OCOTP_MIPI_CSI1_GATE	32
+#define IMX95_OCOTP_MIPI_CSI2_GATE	33
+#define IMX95_OCOTP_MIPI_DSI1_GATE	34
+#define IMX95_OCOTP_V2X_GATE		35
+
+#endif

-- 
2.37.1

Re: [PATCH v6 1/2] dt-bindings: nvmem: imx-ocotp: Introduce #access-controller-cells

[Alexander Stein]

Hi,

Am Dienstag, 21. Januar 2025, 16:05:31 CET schrieb Peng Fan (OSS):
> From: Peng Fan <peng.fan@nxp.com>
> 
> Introduce "#access-controller-cells" to make OCOTP be an accessing
> controller, because i.MX Family OCOTP supports a specific peripheral
> or function being fused which means being disabled.
> 
> Add the i.MX[95,93] OCOTP gate index.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  .../devicetree/bindings/nvmem/imx-ocotp.yaml       |  5 +++
>  include/dt-bindings/nvmem/fsl,imx93-ocotp.h        | 24 ++++++++++++
>  include/dt-bindings/nvmem/fsl,imx95-ocotp.h        | 43 ++++++++++++++++++++++
>  3 files changed, 72 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml b/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml
> index b2cb76cf9053a883a158acaf5eaa108895818afc..c78e202ced22f1c278f7be827b71ba434832d2a7 100644
> --- a/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml
> @@ -54,6 +54,11 @@ properties:
>    clocks:
>      maxItems: 1
>  
> +  "#access-controller-cells":
> +    const: 1
> +    description:
> +      Contains the gate ID associated to the peripheral.
> +
>  required:
>    - "#address-cells"
>    - "#size-cells"
> diff --git a/include/dt-bindings/nvmem/fsl,imx93-ocotp.h b/include/dt-bindings/nvmem/fsl,imx93-ocotp.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..6ef525173845fd4ee0e847cf5a17e53a14f71362
> --- /dev/null
> +++ b/include/dt-bindings/nvmem/fsl,imx93-ocotp.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +
> +#ifndef _DT_BINDINGS_NVMEM_IMX93_OTPC_H
> +#define _DT_BINDINGS_NVMEM_IMX93_OTPC_H

This doesn't match the filename.

> +
> +#define IMX93_OCOTP_NPU_GATE		0
> +#define IMX93_OCOTP_A550_GATE		1
> +#define IMX93_OCOTP_A551_GATE		2
> +#define IMX93_OCOTP_M33_GATE		3
> +#define IMX93_OCOTP_CAN1_FD_GATE	4
> +#define IMX93_OCOTP_CAN2_FD_GATE	5
> +#define IMX93_OCOTP_CAN1_GATE		6
> +#define IMX93_OCOTP_CAN2_GATE		7
> +#define IMX93_OCOTP_USB1_GATE		8
> +#define IMX93_OCOTP_USB2_GATE		9
> +#define IMX93_OCOTP_ENET1_GATE		10
> +#define IMX93_OCOTP_ENET2_GATE		11
> +#define IMX93_OCOTP_PXP_GATE		12
> +#define IMX93_OCOTP_MIPI_CSI1_GATE	13
> +#define IMX93_OCOTP_MIPI_DSI1_GATE	14
> +#define IMX93_OCOTP_LVDS1_GATE		15
> +#define IMX93_OCOTP_ADC1_GATE		16
> +
> +#endif
> diff --git a/include/dt-bindings/nvmem/fsl,imx95-ocotp.h b/include/dt-bindings/nvmem/fsl,imx95-ocotp.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..2d21d1f690974d0215c71352168378a150f489af
> --- /dev/null
> +++ b/include/dt-bindings/nvmem/fsl,imx95-ocotp.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +
> +#ifndef _DT_BINDINGS_NVMEM_IMX95_OTPC_H
> +#define _DT_BINDINGS_NVMEM_IMX95_OTPC_H

This doesn't match the filename.

Best regards,
Alexander

> +
> +#define IMX95_OCOTP_CANFD1_GATE		0
> +#define IMX95_OCOTP_CANFD2_GATE		1
> +#define IMX95_OCOTP_CANFD3_GATE		2
> +#define IMX95_OCOTP_CANFD4_GATE		3
> +#define IMX95_OCOTP_CANFD5_GATE		4
> +#define IMX95_OCOTP_CAN1_GATE		5
> +#define IMX95_OCOTP_CAN2_GATE		6
> +#define IMX95_OCOTP_CAN3_GATE		7
> +#define IMX95_OCOTP_CAN4_GATE		8
> +#define IMX95_OCOTP_CAN5_GATE		9
> +#define IMX95_OCOTP_NPU_GATE		10
> +#define IMX95_OCOTP_A550_GATE		11
> +#define IMX95_OCOTP_A551_GATE		12
> +#define IMX95_OCOTP_A552_GATE		13
> +#define IMX95_OCOTP_A553_GATE		14
> +#define IMX95_OCOTP_A554_GATE		15
> +#define IMX95_OCOTP_A555_GATE		16
> +#define IMX95_OCOTP_M7_GATE		17
> +#define IMX95_OCOTP_DCSS_GATE		18
> +#define IMX95_OCOTP_LVDS1_GATE		19
> +#define IMX95_OCOTP_ISP_GATE		20
> +#define IMX95_OCOTP_USB1_GATE		21
> +#define IMX95_OCOTP_USB2_GATE		22
> +#define IMX95_OCOTP_NETC_GATE		23
> +#define IMX95_OCOTP_PCIE1_GATE		24
> +#define IMX95_OCOTP_PCIE2_GATE		25
> +#define IMX95_OCOTP_ADC1_GATE		26
> +#define IMX95_OCOTP_EARC_RX_GATE	27
> +#define IMX95_OCOTP_GPU3D_GATE		28
> +#define IMX95_OCOTP_VPU_GATE		29
> +#define IMX95_OCOTP_JPEG_ENC_GATE	30
> +#define IMX95_OCOTP_JPEG_DEC_GATE	31
> +#define IMX95_OCOTP_MIPI_CSI1_GATE	32
> +#define IMX95_OCOTP_MIPI_CSI2_GATE	33
> +#define IMX95_OCOTP_MIPI_DSI1_GATE	34
> +#define IMX95_OCOTP_V2X_GATE		35
> +
> +#endif
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

[PATCH v6 2/2] nvmem: imx-ocotp-ele: Support accessing controller for i.MX9

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX9 OCOTP supports a specific peripheral or function being fused
which means disabled, so
 - Introduce ocotp_access_gates to be container of efuse gate info
 - Iterate all nodes to check accessing permission. If not
   allowed to be accessed, detach the node

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/nvmem/Kconfig         |   3 +
 drivers/nvmem/imx-ocotp-ele.c | 172 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 174 insertions(+), 1 deletion(-)

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 8671b7c974b933e147154bb40b5d41b5730518d2..77cc496fd5e0e1afd753534b56fe1f5ef3e3ec55 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -93,6 +93,9 @@ config NVMEM_IMX_OCOTP_ELE
 	  This is a driver for the On-Chip OTP Controller (OCOTP)
 	  available on i.MX SoCs which has ELE.
 
+	  If built as modules, any other driver relying on this working
+	  as access controller also needs to be a module as well.
+
 config NVMEM_IMX_OCOTP_SCU
 	tristate "i.MX8 SCU On-Chip OTP Controller support"
 	depends on IMX_SCU
diff --git a/drivers/nvmem/imx-ocotp-ele.c b/drivers/nvmem/imx-ocotp-ele.c
index ca6dd71d8a2e29888c6e556aaea116c1a967cb5f..5ea6d959ce38760eeed44a989992fb35c462c0b4 100644
--- a/drivers/nvmem/imx-ocotp-ele.c
+++ b/drivers/nvmem/imx-ocotp-ele.c
@@ -5,6 +5,8 @@
  * Copyright 2023 NXP
  */
 
+#include <dt-bindings/nvmem/fsl,imx93-ocotp.h>
+#include <dt-bindings/nvmem/fsl,imx95-ocotp.h>
 #include <linux/device.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -27,6 +29,7 @@ struct ocotp_map_entry {
 };
 
 struct ocotp_devtype_data {
+	const struct ocotp_access_gates *access_gates;
 	u32 reg_off;
 	char *name;
 	u32 size;
@@ -36,6 +39,20 @@ struct ocotp_devtype_data {
 	struct ocotp_map_entry entry[];
 };
 
+#define OCOTP_MAX_NUM_GATE_WORDS 4
+
+struct access_gate {
+	u32 word;
+	u32 mask;
+};
+
+struct ocotp_access_gates {
+	u32 num_words;
+	u32 words[OCOTP_MAX_NUM_GATE_WORDS];
+	u32 num_gates;
+	struct access_gate *gates;
+};
+
 struct imx_ocotp_priv {
 	struct device *dev;
 	void __iomem *base;
@@ -131,6 +148,82 @@ static void imx_ocotp_fixup_dt_cell_info(struct nvmem_device *nvmem,
 	cell->read_post_process = imx_ocotp_cell_pp;
 }
 
+static int imx_ele_ocotp_check_access(struct imx_ocotp_priv *priv, u32 id)
+{
+	const struct ocotp_access_gates *access_gates = priv->data->access_gates;
+	void __iomem *reg = priv->base + priv->data->reg_off;
+	u32 word, mask, val;
+
+	if (id >= access_gates->num_gates) {
+		dev_err(priv->config.dev, "Index %d too large\n", id);
+		return -EACCES;
+	}
+
+	word = access_gates->gates[id].word;
+	mask = access_gates->gates[id].mask;
+
+	reg = priv->base + priv->data->reg_off + (word << 2);
+	val = readl(reg);
+
+	dev_dbg(priv->config.dev, "id:%d word:%d mask:0x%08x\n", id, word, mask);
+	/* true means not allow access */
+	if (val & mask)
+		return -EACCES;
+
+	return 0;
+}
+
+static int imx_ele_ocotp_grant_access(struct imx_ocotp_priv *priv, struct device_node *parent)
+{
+	struct device *dev = priv->config.dev;
+
+	for_each_available_child_of_node_scoped(parent, child) {
+		struct of_phandle_args args;
+		u32 id, idx = 0;
+
+		while (!of_parse_phandle_with_args(child, "access-controllers",
+						   "#access-controller-cells",
+						   idx++, &args)) {
+			of_node_put(args.np);
+			if (args.np != dev->of_node)
+				continue;
+
+			/* Only support one cell */
+			if (args.args_count != 1) {
+				dev_err(dev, "wrong args count\n");
+				continue;
+			}
+
+			id = args.args[0];
+
+			dev_dbg(dev, "Checking node: %pOF gate: %d\n", child, id);
+
+			if (imx_ele_ocotp_check_access(priv, id)) {
+				of_detach_node(child);
+				dev_info(dev, "%pOF: Not granted, device driver will not be probed\n",
+					 child);
+			}
+		}
+
+		imx_ele_ocotp_grant_access(priv, child);
+	}
+
+	return 0;
+}
+
+static int imx_ele_ocotp_access_control(struct imx_ocotp_priv *priv)
+{
+	struct device_node *root __free(device_node) = of_find_node_by_path("/");
+
+	if (!priv->data->access_gates)
+		return 0;
+
+	/* This should never happen */
+	WARN_ON(!root);
+
+	return imx_ele_ocotp_grant_access(priv, root);
+}
+
 static int imx_ele_ocotp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -161,14 +254,45 @@ static int imx_ele_ocotp_probe(struct platform_device *pdev)
 	priv->config.fixup_dt_cell_info = imx_ocotp_fixup_dt_cell_info;
 	mutex_init(&priv->lock);
 
+	platform_set_drvdata(pdev, priv);
+
 	nvmem = devm_nvmem_register(dev, &priv->config);
 	if (IS_ERR(nvmem))
 		return PTR_ERR(nvmem);
 
-	return 0;
+
+	return imx_ele_ocotp_access_control(priv);
 }
 
+struct access_gate imx93_access_gate[] = {
+		[IMX93_OCOTP_NPU_GATE]		= { .word = 19, .mask = BIT(13) },
+		[IMX93_OCOTP_A550_GATE]		= { .word = 19, .mask = BIT(14) },
+		[IMX93_OCOTP_A551_GATE]		= { .word = 19, .mask = BIT(15) },
+		[IMX93_OCOTP_M33_GATE]		= { .word = 19, .mask = BIT(24) },
+		[IMX93_OCOTP_CAN1_FD_GATE]	= { .word = 19, .mask = BIT(28) },
+		[IMX93_OCOTP_CAN2_FD_GATE]	= { .word = 19, .mask = BIT(29) },
+		[IMX93_OCOTP_CAN1_GATE]		= { .word = 19, .mask = BIT(30) },
+		[IMX93_OCOTP_CAN2_GATE]		= { .word = 19, .mask = BIT(31) },
+		[IMX93_OCOTP_USB1_GATE]		= { .word = 20, .mask = BIT(3) },
+		[IMX93_OCOTP_USB2_GATE]		= { .word = 20, .mask = BIT(4) },
+		[IMX93_OCOTP_ENET1_GATE]	= { .word = 20, .mask = BIT(5) },
+		[IMX93_OCOTP_ENET2_GATE]	= { .word = 20, .mask = BIT(6) },
+		[IMX93_OCOTP_PXP_GATE]		= { .word = 20, .mask = BIT(10) },
+		[IMX93_OCOTP_MIPI_CSI1_GATE]	= { .word = 20, .mask = BIT(17) },
+		[IMX93_OCOTP_MIPI_DSI1_GATE]	= { .word = 20, .mask = BIT(19) },
+		[IMX93_OCOTP_LVDS1_GATE]	= { .word = 20, .mask = BIT(24) },
+		[IMX93_OCOTP_ADC1_GATE]		= { .word = 21, .mask = BIT(7) },
+};
+
+static const struct ocotp_access_gates imx93_access_gates_info = {
+	.num_words = 3,
+	.words = {19, 20, 21},
+	.num_gates = ARRAY_SIZE(imx93_access_gate),
+	.gates = imx93_access_gate,
+};
+
 static const struct ocotp_devtype_data imx93_ocotp_data = {
+	.access_gates = &imx93_access_gates_info,
 	.reg_off = 0x8000,
 	.reg_read = imx_ocotp_reg_read,
 	.size = 2048,
@@ -183,7 +307,53 @@ static const struct ocotp_devtype_data imx93_ocotp_data = {
 	},
 };
 
+struct access_gate imx95_access_gate[] = {
+		[IMX95_OCOTP_CANFD1_GATE]	= { .word = 17, .mask = BIT(20) },
+		[IMX95_OCOTP_CANFD2_GATE]	= { .word = 17, .mask = BIT(21) },
+		[IMX95_OCOTP_CANFD3_GATE]	= { .word = 17, .mask = BIT(22) },
+		[IMX95_OCOTP_CANFD4_GATE]	= { .word = 17, .mask = BIT(23) },
+		[IMX95_OCOTP_CANFD5_GATE]	= { .word = 17, .mask = BIT(24) },
+		[IMX95_OCOTP_CAN1_GATE]		= { .word = 17, .mask = BIT(25) },
+		[IMX95_OCOTP_CAN2_GATE]		= { .word = 17, .mask = BIT(26) },
+		[IMX95_OCOTP_CAN3_GATE]		= { .word = 17, .mask = BIT(27) },
+		[IMX95_OCOTP_CAN4_GATE]		= { .word = 17, .mask = BIT(28) },
+		[IMX95_OCOTP_CAN5_GATE]		= { .word = 17, .mask = BIT(29) },
+		[IMX95_OCOTP_NPU_GATE]		= { .word = 18, .mask = BIT(0) },
+		[IMX95_OCOTP_A550_GATE]		= { .word = 18, .mask = BIT(1) },
+		[IMX95_OCOTP_A551_GATE]		= { .word = 18, .mask = BIT(2) },
+		[IMX95_OCOTP_A552_GATE]		= { .word = 18, .mask = BIT(3) },
+		[IMX95_OCOTP_A553_GATE]		= { .word = 18, .mask = BIT(4) },
+		[IMX95_OCOTP_A554_GATE]		= { .word = 18, .mask = BIT(5) },
+		[IMX95_OCOTP_A555_GATE]		= { .word = 18, .mask = BIT(6) },
+		[IMX95_OCOTP_M7_GATE]		= { .word = 18, .mask = BIT(9) },
+		[IMX95_OCOTP_DCSS_GATE]		= { .word = 18, .mask = BIT(22) },
+		[IMX95_OCOTP_LVDS1_GATE]	= { .word = 18, .mask = BIT(27) },
+		[IMX95_OCOTP_ISP_GATE]		= { .word = 18, .mask = BIT(29) },
+		[IMX95_OCOTP_USB1_GATE]		= { .word = 19, .mask = BIT(2) },
+		[IMX95_OCOTP_USB2_GATE]		= { .word = 19, .mask = BIT(3) },
+		[IMX95_OCOTP_NETC_GATE]		= { .word = 19, .mask = BIT(4) },
+		[IMX95_OCOTP_PCIE1_GATE]	= { .word = 19, .mask = BIT(6) },
+		[IMX95_OCOTP_PCIE2_GATE]	= { .word = 19, .mask = BIT(7) },
+		[IMX95_OCOTP_ADC1_GATE]		= { .word = 19, .mask = BIT(8) },
+		[IMX95_OCOTP_EARC_RX_GATE]	= { .word = 19, .mask = BIT(11) },
+		[IMX95_OCOTP_GPU3D_GATE]	= { .word = 19, .mask = BIT(16) },
+		[IMX95_OCOTP_VPU_GATE]		= { .word = 19, .mask = BIT(17) },
+		[IMX95_OCOTP_JPEG_ENC_GATE]	= { .word = 19, .mask = BIT(18) },
+		[IMX95_OCOTP_JPEG_DEC_GATE]	= { .word = 19, .mask = BIT(19) },
+		[IMX95_OCOTP_MIPI_CSI1_GATE]	= { .word = 19, .mask = BIT(21) },
+		[IMX95_OCOTP_MIPI_CSI2_GATE]	= { .word = 19, .mask = BIT(22) },
+		[IMX95_OCOTP_MIPI_DSI1_GATE]	= { .word = 19, .mask = BIT(23) },
+};
+
+static const struct ocotp_access_gates imx95_access_gates_info = {
+	.num_words = 3,
+	.words = {17, 18, 19},
+	.num_gates = ARRAY_SIZE(imx95_access_gate),
+	.gates = imx95_access_gate,
+};
+
 static const struct ocotp_devtype_data imx95_ocotp_data = {
+	.access_gates = &imx95_access_gates_info,
 	.reg_off = 0x8000,
 	.reg_read = imx_ocotp_reg_read,
 	.size = 2048,

-- 
2.37.1

Re: [PATCH v6 2/2] nvmem: imx-ocotp-ele: Support accessing controller for i.MX9

[Alexander Stein]

Hi,

Am Dienstag, 21. Januar 2025, 16:05:32 CET schrieb Peng Fan (OSS):
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX9 OCOTP supports a specific peripheral or function being fused
> which means disabled, so
>  - Introduce ocotp_access_gates to be container of efuse gate info
>  - Iterate all nodes to check accessing permission. If not
>    allowed to be accessed, detach the node
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/nvmem/Kconfig         |   3 +
>  drivers/nvmem/imx-ocotp-ele.c | 172 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 174 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 8671b7c974b933e147154bb40b5d41b5730518d2..77cc496fd5e0e1afd753534b56fe1f5ef3e3ec55 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -93,6 +93,9 @@ config NVMEM_IMX_OCOTP_ELE
>  	  This is a driver for the On-Chip OTP Controller (OCOTP)
>  	  available on i.MX SoCs which has ELE.
>  
> +	  If built as modules, any other driver relying on this working
> +	  as access controller also needs to be a module as well.
> +
>  config NVMEM_IMX_OCOTP_SCU
>  	tristate "i.MX8 SCU On-Chip OTP Controller support"
>  	depends on IMX_SCU
> diff --git a/drivers/nvmem/imx-ocotp-ele.c b/drivers/nvmem/imx-ocotp-ele.c
> index ca6dd71d8a2e29888c6e556aaea116c1a967cb5f..5ea6d959ce38760eeed44a989992fb35c462c0b4 100644
> --- a/drivers/nvmem/imx-ocotp-ele.c
> +++ b/drivers/nvmem/imx-ocotp-ele.c
> @@ -5,6 +5,8 @@
>   * Copyright 2023 NXP
>   */
>  
> +#include <dt-bindings/nvmem/fsl,imx93-ocotp.h>
> +#include <dt-bindings/nvmem/fsl,imx95-ocotp.h>
>  #include <linux/device.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> @@ -27,6 +29,7 @@ struct ocotp_map_entry {
>  };
>  
>  struct ocotp_devtype_data {
> +	const struct ocotp_access_gates *access_gates;
>  	u32 reg_off;
>  	char *name;
>  	u32 size;
> @@ -36,6 +39,20 @@ struct ocotp_devtype_data {
>  	struct ocotp_map_entry entry[];
>  };
>  
> +#define OCOTP_MAX_NUM_GATE_WORDS 4
> +
> +struct access_gate {
> +	u32 word;
> +	u32 mask;
> +};
> +
> +struct ocotp_access_gates {
> +	u32 num_words;
> +	u32 words[OCOTP_MAX_NUM_GATE_WORDS];
> +	u32 num_gates;
> +	struct access_gate *gates;
> +};
> +
>  struct imx_ocotp_priv {
>  	struct device *dev;
>  	void __iomem *base;
> @@ -131,6 +148,82 @@ static void imx_ocotp_fixup_dt_cell_info(struct nvmem_device *nvmem,
>  	cell->read_post_process = imx_ocotp_cell_pp;
>  }
>  
> +static int imx_ele_ocotp_check_access(struct imx_ocotp_priv *priv, u32 id)
> +{
> +	const struct ocotp_access_gates *access_gates = priv->data->access_gates;
> +	void __iomem *reg = priv->base + priv->data->reg_off;
> +	u32 word, mask, val;
> +
> +	if (id >= access_gates->num_gates) {
> +		dev_err(priv->config.dev, "Index %d too large\n", id);
> +		return -EACCES;
> +	}
> +
> +	word = access_gates->gates[id].word;
> +	mask = access_gates->gates[id].mask;
> +
> +	reg = priv->base + priv->data->reg_off + (word << 2);
> +	val = readl(reg);
> +
> +	dev_dbg(priv->config.dev, "id:%d word:%d mask:0x%08x\n", id, word, mask);
> +	/* true means not allow access */
> +	if (val & mask)
> +		return -EACCES;
> +
> +	return 0;
> +}
> +
> +static int imx_ele_ocotp_grant_access(struct imx_ocotp_priv *priv, struct device_node *parent)
> +{
> +	struct device *dev = priv->config.dev;
> +
> +	for_each_available_child_of_node_scoped(parent, child) {
> +		struct of_phandle_args args;
> +		u32 id, idx = 0;
> +
> +		while (!of_parse_phandle_with_args(child, "access-controllers",
> +						   "#access-controller-cells",
> +						   idx++, &args)) {
> +			of_node_put(args.np);
> +			if (args.np != dev->of_node)
> +				continue;
> +
> +			/* Only support one cell */
> +			if (args.args_count != 1) {
> +				dev_err(dev, "wrong args count\n");
> +				continue;
> +			}
> +
> +			id = args.args[0];
> +
> +			dev_dbg(dev, "Checking node: %pOF gate: %d\n", child, id);
> +
> +			if (imx_ele_ocotp_check_access(priv, id)) {
> +				of_detach_node(child);
> +				dev_info(dev, "%pOF: Not granted, device driver will not be probed\n",
> +					 child);
> +			}
> +		}
> +
> +		imx_ele_ocotp_grant_access(priv, child);
> +	}
> +
> +	return 0;
> +}
> +
> +static int imx_ele_ocotp_access_control(struct imx_ocotp_priv *priv)
> +{
> +	struct device_node *root __free(device_node) = of_find_node_by_path("/");
> +
> +	if (!priv->data->access_gates)
> +		return 0;
> +
> +	/* This should never happen */
> +	WARN_ON(!root);

Even if you warning something is wrong, aka root == NULL, you are still
using it on imx_ele_ocotp_grant_access(). Just return early.

if (WARN_ON(!))
	return -EINVAL;

Best regards
Alexander

> +
> +	return imx_ele_ocotp_grant_access(priv, root);
> +}
> +
>  static int imx_ele_ocotp_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -161,14 +254,45 @@ static int imx_ele_ocotp_probe(struct platform_device *pdev)
>  	priv->config.fixup_dt_cell_info = imx_ocotp_fixup_dt_cell_info;
>  	mutex_init(&priv->lock);
>  
> +	platform_set_drvdata(pdev, priv);
> +
>  	nvmem = devm_nvmem_register(dev, &priv->config);
>  	if (IS_ERR(nvmem))
>  		return PTR_ERR(nvmem);
>  
> -	return 0;
> +
> +	return imx_ele_ocotp_access_control(priv);
>  }



-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

RE: [PATCH v6 2/2] nvmem: imx-ocotp-ele: Support accessing controller for i.MX9

[Peng Fan]

> Subject: Re: [PATCH v6 2/2] nvmem: imx-ocotp-ele: Support accessing
> controller for i.MX9
> 
> Hi,
> 
> Am Dienstag, 21. Januar 2025, 16:05:32 CET schrieb Peng Fan (OSS):
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > i.MX9 OCOTP supports a specific peripheral or function being fused
> > which means disabled, so
> >  - Introduce ocotp_access_gates to be container of efuse gate info
> >  - Iterate all nodes to check accessing permission. If not
> >    allowed to be accessed, detach the node
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/nvmem/Kconfig         |   3 +
> >  drivers/nvmem/imx-ocotp-ele.c | 172
> > +++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 174 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index
> >
> 8671b7c974b933e147154bb40b5d41b5730518d2..77cc496fd5e0e1af
> d753534b56fe
> > 1f5ef3e3ec55 100644
> > --- a/drivers/nvmem/Kconfig
> > +++ b/drivers/nvmem/Kconfig
> > @@ -93,6 +93,9 @@ config NVMEM_IMX_OCOTP_ELE
> >  	  This is a driver for the On-Chip OTP Controller (OCOTP)
> >  	  available on i.MX SoCs which has ELE.
> >
> > +	  If built as modules, any other driver relying on this working
> > +	  as access controller also needs to be a module as well.
> > +
> >  config NVMEM_IMX_OCOTP_SCU
> >  	tristate "i.MX8 SCU On-Chip OTP Controller support"
> >  	depends on IMX_SCU
> > diff --git a/drivers/nvmem/imx-ocotp-ele.c
> > b/drivers/nvmem/imx-ocotp-ele.c index
> >
> ca6dd71d8a2e29888c6e556aaea116c1a967cb5f..5ea6d959ce38760ee
> ed44a989992
> > fb35c462c0b4 100644
> > --- a/drivers/nvmem/imx-ocotp-ele.c
> > +++ b/drivers/nvmem/imx-ocotp-ele.c
> > @@ -5,6 +5,8 @@
> >   * Copyright 2023 NXP
> >   */
> >
> > +#include <dt-bindings/nvmem/fsl,imx93-ocotp.h>
> > +#include <dt-bindings/nvmem/fsl,imx95-ocotp.h>
> >  #include <linux/device.h>
> >  #include <linux/io.h>
> >  #include <linux/module.h>
> > @@ -27,6 +29,7 @@ struct ocotp_map_entry {  };
> >
> >  struct ocotp_devtype_data {
> > +	const struct ocotp_access_gates *access_gates;
> >  	u32 reg_off;
> >  	char *name;
> >  	u32 size;
> > @@ -36,6 +39,20 @@ struct ocotp_devtype_data {
> >  	struct ocotp_map_entry entry[];
> >  };
> >
> > +#define OCOTP_MAX_NUM_GATE_WORDS 4
> > +
> > +struct access_gate {
> > +	u32 word;
> > +	u32 mask;
> > +};
> > +
> > +struct ocotp_access_gates {
> > +	u32 num_words;
> > +	u32 words[OCOTP_MAX_NUM_GATE_WORDS];
> > +	u32 num_gates;
> > +	struct access_gate *gates;
> > +};
> > +
> >  struct imx_ocotp_priv {
> >  	struct device *dev;
> >  	void __iomem *base;
> > @@ -131,6 +148,82 @@ static void
> imx_ocotp_fixup_dt_cell_info(struct nvmem_device *nvmem,
> >  	cell->read_post_process = imx_ocotp_cell_pp;  }
> >
> > +static int imx_ele_ocotp_check_access(struct imx_ocotp_priv *priv,
> > +u32 id) {
> > +	const struct ocotp_access_gates *access_gates = priv->data-
> >access_gates;
> > +	void __iomem *reg = priv->base + priv->data->reg_off;
> > +	u32 word, mask, val;
> > +
> > +	if (id >= access_gates->num_gates) {
> > +		dev_err(priv->config.dev, "Index %d too large\n", id);
> > +		return -EACCES;
> > +	}
> > +
> > +	word = access_gates->gates[id].word;
> > +	mask = access_gates->gates[id].mask;
> > +
> > +	reg = priv->base + priv->data->reg_off + (word << 2);
> > +	val = readl(reg);
> > +
> > +	dev_dbg(priv->config.dev, "id:%d word:%d mask:0x%08x\n",
> id, word, mask);
> > +	/* true means not allow access */
> > +	if (val & mask)
> > +		return -EACCES;
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx_ele_ocotp_grant_access(struct imx_ocotp_priv *priv,
> > +struct device_node *parent) {
> > +	struct device *dev = priv->config.dev;
> > +
> > +	for_each_available_child_of_node_scoped(parent, child) {
> > +		struct of_phandle_args args;
> > +		u32 id, idx = 0;
> > +
> > +		while (!of_parse_phandle_with_args(child, "access-
> controllers",
> > +						   "#access-
> controller-cells",
> > +						   idx++, &args)) {
> > +			of_node_put(args.np);
> > +			if (args.np != dev->of_node)
> > +				continue;
> > +
> > +			/* Only support one cell */
> > +			if (args.args_count != 1) {
> > +				dev_err(dev, "wrong args count\n");
> > +				continue;
> > +			}
> > +
> > +			id = args.args[0];
> > +
> > +			dev_dbg(dev, "Checking node: %pOF
> gate: %d\n", child, id);
> > +
> > +			if (imx_ele_ocotp_check_access(priv, id)) {
> > +				of_detach_node(child);
> > +				dev_info(dev, "%pOF: Not granted,
> device driver will not be probed\n",
> > +					 child);
> > +			}
> > +		}
> > +
> > +		imx_ele_ocotp_grant_access(priv, child);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx_ele_ocotp_access_control(struct imx_ocotp_priv *priv)
> > +{
> > +	struct device_node *root __free(device_node) =
> > +of_find_node_by_path("/");
> > +
> > +	if (!priv->data->access_gates)
> > +		return 0;
> > +
> > +	/* This should never happen */
> > +	WARN_ON(!root);
> 
> Even if you warning something is wrong, aka root == NULL, you are still
> using it on imx_ele_ocotp_grant_access(). Just return early.
> 
> if (WARN_ON(!))
> 	return -EINVAL;

Hmm, If this really happens, return early or not does not make much sense.
Does it really matter here?

Regards,
Peng.
> 
> Best regards
> Alexander
> 
> > +
> > +	return imx_ele_ocotp_grant_access(priv, root); }
> > +
> >  static int imx_ele_ocotp_probe(struct platform_device *pdev)  {
> >  	struct device *dev = &pdev->dev;
> > @@ -161,14 +254,45 @@ static int imx_ele_ocotp_probe(struct
> platform_device *pdev)
> >  	priv->config.fixup_dt_cell_info = imx_ocotp_fixup_dt_cell_info;
> >  	mutex_init(&priv->lock);
> >
> > +	platform_set_drvdata(pdev, priv);
> > +
> >  	nvmem = devm_nvmem_register(dev, &priv->config);
> >  	if (IS_ERR(nvmem))
> >  		return PTR_ERR(nvmem);
> >
> > -	return 0;
> > +
> > +	return imx_ele_ocotp_access_control(priv);
> >  }
> 
> 
> 
> --
> 

Re: [PATCH v6 2/2] nvmem: imx-ocotp-ele: Support accessing controller for i.MX9

[Alexander Stein]

Am Mittwoch, 22. Januar 2025, 04:36:33 CET schrieb Peng Fan:
> > Subject: Re: [PATCH v6 2/2] nvmem: imx-ocotp-ele: Support accessing
> > controller for i.MX9
> > 
> > Hi,
> > 
> > Am Dienstag, 21. Januar 2025, 16:05:32 CET schrieb Peng Fan (OSS):
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > i.MX9 OCOTP supports a specific peripheral or function being fused
> > > which means disabled, so
> > >  - Introduce ocotp_access_gates to be container of efuse gate info
> > >  - Iterate all nodes to check accessing permission. If not
> > >    allowed to be accessed, detach the node
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  drivers/nvmem/Kconfig         |   3 +
> > >  drivers/nvmem/imx-ocotp-ele.c | 172
> > > +++++++++++++++++++++++++++++++++++++++++-
> > >  2 files changed, 174 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index
> > >
> > 8671b7c974b933e147154bb40b5d41b5730518d2..77cc496fd5e0e1af
> > d753534b56fe
> > > 1f5ef3e3ec55 100644
> > > --- a/drivers/nvmem/Kconfig
> > > +++ b/drivers/nvmem/Kconfig
> > > @@ -93,6 +93,9 @@ config NVMEM_IMX_OCOTP_ELE
> > >  	  This is a driver for the On-Chip OTP Controller (OCOTP)
> > >  	  available on i.MX SoCs which has ELE.
> > >
> > > +	  If built as modules, any other driver relying on this working
> > > +	  as access controller also needs to be a module as well.
> > > +
> > >  config NVMEM_IMX_OCOTP_SCU
> > >  	tristate "i.MX8 SCU On-Chip OTP Controller support"
> > >  	depends on IMX_SCU
> > > diff --git a/drivers/nvmem/imx-ocotp-ele.c
> > > b/drivers/nvmem/imx-ocotp-ele.c index
> > >
> > ca6dd71d8a2e29888c6e556aaea116c1a967cb5f..5ea6d959ce38760ee
> > ed44a989992
> > > fb35c462c0b4 100644
> > > --- a/drivers/nvmem/imx-ocotp-ele.c
> > > +++ b/drivers/nvmem/imx-ocotp-ele.c
> > > @@ -5,6 +5,8 @@
> > >   * Copyright 2023 NXP
> > >   */
> > >
> > > +#include <dt-bindings/nvmem/fsl,imx93-ocotp.h>
> > > +#include <dt-bindings/nvmem/fsl,imx95-ocotp.h>
> > >  #include <linux/device.h>
> > >  #include <linux/io.h>
> > >  #include <linux/module.h>
> > > @@ -27,6 +29,7 @@ struct ocotp_map_entry {  };
> > >
> > >  struct ocotp_devtype_data {
> > > +	const struct ocotp_access_gates *access_gates;
> > >  	u32 reg_off;
> > >  	char *name;
> > >  	u32 size;
> > > @@ -36,6 +39,20 @@ struct ocotp_devtype_data {
> > >  	struct ocotp_map_entry entry[];
> > >  };
> > >
> > > +#define OCOTP_MAX_NUM_GATE_WORDS 4
> > > +
> > > +struct access_gate {
> > > +	u32 word;
> > > +	u32 mask;
> > > +};
> > > +
> > > +struct ocotp_access_gates {
> > > +	u32 num_words;
> > > +	u32 words[OCOTP_MAX_NUM_GATE_WORDS];
> > > +	u32 num_gates;
> > > +	struct access_gate *gates;
> > > +};
> > > +
> > >  struct imx_ocotp_priv {
> > >  	struct device *dev;
> > >  	void __iomem *base;
> > > @@ -131,6 +148,82 @@ static void
> > imx_ocotp_fixup_dt_cell_info(struct nvmem_device *nvmem,
> > >  	cell->read_post_process = imx_ocotp_cell_pp;  }
> > >
> > > +static int imx_ele_ocotp_check_access(struct imx_ocotp_priv *priv,
> > > +u32 id) {
> > > +	const struct ocotp_access_gates *access_gates = priv->data-
> > >access_gates;
> > > +	void __iomem *reg = priv->base + priv->data->reg_off;
> > > +	u32 word, mask, val;
> > > +
> > > +	if (id >= access_gates->num_gates) {
> > > +		dev_err(priv->config.dev, "Index %d too large\n", id);
> > > +		return -EACCES;
> > > +	}
> > > +
> > > +	word = access_gates->gates[id].word;
> > > +	mask = access_gates->gates[id].mask;
> > > +
> > > +	reg = priv->base + priv->data->reg_off + (word << 2);
> > > +	val = readl(reg);
> > > +
> > > +	dev_dbg(priv->config.dev, "id:%d word:%d mask:0x%08x\n",
> > id, word, mask);
> > > +	/* true means not allow access */
> > > +	if (val & mask)
> > > +		return -EACCES;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int imx_ele_ocotp_grant_access(struct imx_ocotp_priv *priv,
> > > +struct device_node *parent) {
> > > +	struct device *dev = priv->config.dev;
> > > +
> > > +	for_each_available_child_of_node_scoped(parent, child) {
> > > +		struct of_phandle_args args;
> > > +		u32 id, idx = 0;
> > > +
> > > +		while (!of_parse_phandle_with_args(child, "access-
> > controllers",
> > > +						   "#access-
> > controller-cells",
> > > +						   idx++, &args)) {
> > > +			of_node_put(args.np);
> > > +			if (args.np != dev->of_node)
> > > +				continue;
> > > +
> > > +			/* Only support one cell */
> > > +			if (args.args_count != 1) {
> > > +				dev_err(dev, "wrong args count\n");
> > > +				continue;
> > > +			}
> > > +
> > > +			id = args.args[0];
> > > +
> > > +			dev_dbg(dev, "Checking node: %pOF
> > gate: %d\n", child, id);
> > > +
> > > +			if (imx_ele_ocotp_check_access(priv, id)) {
> > > +				of_detach_node(child);
> > > +				dev_info(dev, "%pOF: Not granted,
> > device driver will not be probed\n",
> > > +					 child);
> > > +			}
> > > +		}
> > > +
> > > +		imx_ele_ocotp_grant_access(priv, child);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int imx_ele_ocotp_access_control(struct imx_ocotp_priv *priv)
> > > +{
> > > +	struct device_node *root __free(device_node) =
> > > +of_find_node_by_path("/");
> > > +
> > > +	if (!priv->data->access_gates)
> > > +		return 0;
> > > +
> > > +	/* This should never happen */
> > > +	WARN_ON(!root);
> > 
> > Even if you warning something is wrong, aka root == NULL, you are still
> > using it on imx_ele_ocotp_grant_access(). Just return early.
> > 
> > if (WARN_ON(!))
> > 	return -EINVAL;
> 
> Hmm, If this really happens, return early or not does not make much sense.
> Does it really matter here?

Why does it not make much sense? You already know something is wrong, aka
you have a NULL pointer, so it makes even less sense to continue.
I've skipped through the sources and looked for 'WARN_ON(!<pointer>)',
most of the times it is actually checked for early returns.

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

RE: [PATCH v6 2/2] nvmem: imx-ocotp-ele: Support accessing controller for i.MX9

[Peng Fan]

> Subject: Re: [PATCH v6 2/2] nvmem: imx-ocotp-ele: Support accessing
> controller for i.MX9
> 
> Am Mittwoch, 22. Januar 2025, 04:36:33 CET schrieb Peng Fan:
> > > Subject: Re: [PATCH v6 2/2] nvmem: imx-ocotp-ele: Support
> accessing
> > > controller for i.MX9
> > >
> > > Hi,
> > >
> > > Am Dienstag, 21. Januar 2025, 16:05:32 CET schrieb Peng Fan (OSS):
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > i.MX9 OCOTP supports a specific peripheral or function being
> fused
> > > > which means disabled, so
> > > >  - Introduce ocotp_access_gates to be container of efuse gate info
> > > >  - Iterate all nodes to check accessing permission. If not
> > > >    allowed to be accessed, detach the node
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >  drivers/nvmem/Kconfig         |   3 +
> > > >  drivers/nvmem/imx-ocotp-ele.c | 172
> > > > +++++++++++++++++++++++++++++++++++++++++-
> > > >  2 files changed, 174 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index
> > > >
> > >
> 8671b7c974b933e147154bb40b5d41b5730518d2..77cc496fd5e0e1af
> > > d753534b56fe
> > > > 1f5ef3e3ec55 100644
> > > > --- a/drivers/nvmem/Kconfig
> > > > +++ b/drivers/nvmem/Kconfig
> > > > @@ -93,6 +93,9 @@ config NVMEM_IMX_OCOTP_ELE
> > > >  	  This is a driver for the On-Chip OTP Controller (OCOTP)
> > > >  	  available on i.MX SoCs which has ELE.
> > > >
> > > > +	  If built as modules, any other driver relying on this working
> > > > +	  as access controller also needs to be a module as well.
> > > > +
> > > >  config NVMEM_IMX_OCOTP_SCU
> > > >  	tristate "i.MX8 SCU On-Chip OTP Controller support"
> > > >  	depends on IMX_SCU
> > > > diff --git a/drivers/nvmem/imx-ocotp-ele.c
> > > > b/drivers/nvmem/imx-ocotp-ele.c index
> > > >
> > >
> ca6dd71d8a2e29888c6e556aaea116c1a967cb5f..5ea6d959ce38760ee
> > > ed44a989992
> > > > fb35c462c0b4 100644
> > > > --- a/drivers/nvmem/imx-ocotp-ele.c
> > > > +++ b/drivers/nvmem/imx-ocotp-ele.c
> > > > @@ -5,6 +5,8 @@
> > > >   * Copyright 2023 NXP
> > > >   */
> > > >
> > > > +#include <dt-bindings/nvmem/fsl,imx93-ocotp.h>
> > > > +#include <dt-bindings/nvmem/fsl,imx95-ocotp.h>
> > > >  #include <linux/device.h>
> > > >  #include <linux/io.h>
> > > >  #include <linux/module.h>
> > > > @@ -27,6 +29,7 @@ struct ocotp_map_entry {  };
> > > >
> > > >  struct ocotp_devtype_data {
> > > > +	const struct ocotp_access_gates *access_gates;
> > > >  	u32 reg_off;
> > > >  	char *name;
> > > >  	u32 size;
> > > > @@ -36,6 +39,20 @@ struct ocotp_devtype_data {
> > > >  	struct ocotp_map_entry entry[];
> > > >  };
> > > >
> > > > +#define OCOTP_MAX_NUM_GATE_WORDS 4
> > > > +
> > > > +struct access_gate {
> > > > +	u32 word;
> > > > +	u32 mask;
> > > > +};
> > > > +
> > > > +struct ocotp_access_gates {
> > > > +	u32 num_words;
> > > > +	u32 words[OCOTP_MAX_NUM_GATE_WORDS];
> > > > +	u32 num_gates;
> > > > +	struct access_gate *gates;
> > > > +};
> > > > +
> > > >  struct imx_ocotp_priv {
> > > >  	struct device *dev;
> > > >  	void __iomem *base;
> > > > @@ -131,6 +148,82 @@ static void
> > > imx_ocotp_fixup_dt_cell_info(struct nvmem_device *nvmem,
> > > >  	cell->read_post_process = imx_ocotp_cell_pp;  }
> > > >
> > > > +static int imx_ele_ocotp_check_access(struct imx_ocotp_priv
> > > > +*priv,
> > > > +u32 id) {
> > > > +	const struct ocotp_access_gates *access_gates = priv->data-
> > > >access_gates;
> > > > +	void __iomem *reg = priv->base + priv->data->reg_off;
> > > > +	u32 word, mask, val;
> > > > +
> > > > +	if (id >= access_gates->num_gates) {
> > > > +		dev_err(priv->config.dev, "Index %d too large\n", id);
> > > > +		return -EACCES;
> > > > +	}
> > > > +
> > > > +	word = access_gates->gates[id].word;
> > > > +	mask = access_gates->gates[id].mask;
> > > > +
> > > > +	reg = priv->base + priv->data->reg_off + (word << 2);
> > > > +	val = readl(reg);
> > > > +
> > > > +	dev_dbg(priv->config.dev, "id:%d word:%d mask:0x%08x\n",
> > > id, word, mask);
> > > > +	/* true means not allow access */
> > > > +	if (val & mask)
> > > > +		return -EACCES;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int imx_ele_ocotp_grant_access(struct imx_ocotp_priv
> > > > +*priv, struct device_node *parent) {
> > > > +	struct device *dev = priv->config.dev;
> > > > +
> > > > +	for_each_available_child_of_node_scoped(parent, child) {
> > > > +		struct of_phandle_args args;
> > > > +		u32 id, idx = 0;
> > > > +
> > > > +		while (!of_parse_phandle_with_args(child, "access-
> > > controllers",
> > > > +						   "#access-
> > > controller-cells",
> > > > +						   idx++, &args)) {
> > > > +			of_node_put(args.np);
> > > > +			if (args.np != dev->of_node)
> > > > +				continue;
> > > > +
> > > > +			/* Only support one cell */
> > > > +			if (args.args_count != 1) {
> > > > +				dev_err(dev, "wrong args count\n");
> > > > +				continue;
> > > > +			}
> > > > +
> > > > +			id = args.args[0];
> > > > +
> > > > +			dev_dbg(dev, "Checking node: %pOF
> > > gate: %d\n", child, id);
> > > > +
> > > > +			if (imx_ele_ocotp_check_access(priv, id)) {
> > > > +				of_detach_node(child);
> > > > +				dev_info(dev, "%pOF: Not granted,
> > > device driver will not be probed\n",
> > > > +					 child);
> > > > +			}
> > > > +		}
> > > > +
> > > > +		imx_ele_ocotp_grant_access(priv, child);
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int imx_ele_ocotp_access_control(struct imx_ocotp_priv
> > > > +*priv) {
> > > > +	struct device_node *root __free(device_node) =
> > > > +of_find_node_by_path("/");
> > > > +
> > > > +	if (!priv->data->access_gates)
> > > > +		return 0;
> > > > +
> > > > +	/* This should never happen */
> > > > +	WARN_ON(!root);
> > >
> > > Even if you warning something is wrong, aka root == NULL, you are
> > > still using it on imx_ele_ocotp_grant_access(). Just return early.
> > >
> > > if (WARN_ON(!))
> > > 	return -EINVAL;
> >
> > Hmm, If this really happens, return early or not does not make much
> sense.
> > Does it really matter here?
> 
> Why does it not make much sense? You already know something is
> wrong, aka you have a NULL pointer, so it makes even less sense to
> continue.
> I've skipped through the sources and looked for
> 'WARN_ON(!<pointer>)', most of the times it is actually checked for
> early returns.
> 
I will include the fail return in V7. Please help check if other parts are
good for you.

Thanks,
Peng.