[PATCH 00/10] Add new feature controller framework

[PATCH 00/10] Add new feature controller framework

[Ahmad Fatoum]

The i.MX8MM exists in a Lite variant with no VPUs as well as Solo and
Dual variants with one or two cores respectively instead of the default
four. For i.MX6, we had a manual fixup taking care of deleting the
excess CPUs, but for e.g. the i.MX8MP, we have fuses for the M7, VPUs, CAN,
CAN-FD, ISPs, NPU, ... etc. Describing all that in the DT is overly verbose
as we need to take care not to rely on specific device node names that
should be disabled. There has been an upstream attempt to get a binding
for U-Boot to act on:

  https://lore.kernel.org/all/20220324042024.26813-1-peng.fan@oss.nxp.com/

This was refused by the DT maintainer, because any solution to this
problem should also be flexible enough to cover the case of partitioning
devices between the secure and normal world.

There's a patch series upstream to describe a domain-controller binding
that allows a hypervisor to partition devices into domains. With the
naming generalized, this fits nicely the use case of gating devices
behind specific features:

https://lore.kernel.org/all/3ca7cd75-4b62-2380-adb0-646bbeb647a2@pengutronix.de/

This series does that. See the first two commit messages for details.
We use a barebox, prefix as the naming isn't set in stone, but the intention
is to drop the prefix and potentially rename once an upstream binding is
approved.

Ahmad Fatoum (10):
  driver: add feature controller framework
  driver: consult feature controller prior to device probe
  driver: featctrl: fixup kernel device tree
  dt-bindings: add i.MX8M feature controller bindings
  soc: imx: add i.MX8M feature controller driver
  nvmem: import Linux nvmem_cell_read_variable_le_u32
  nvmem: ocotp: add i.MX8M[MN] feature controller support
  ARM: dts: i.MX8MN: describe feature controller
  RFC: soc: imx: imx8m-featctrl: add i.MX8M[MN] stand-alone driver
  RFC: ARM: dts: i.MX8MM: describe standlone feature controller

 arch/arm/dts/imx8mm.dtsi             |  61 ++++++++++
 arch/arm/dts/imx8mn.dtsi             |  32 ++++++
 drivers/base/Kconfig                 |   3 +
 drivers/base/Makefile                |   1 +
 drivers/base/driver.c                |   9 ++
 drivers/base/featctrl.c              | 160 +++++++++++++++++++++++++++
 drivers/nvmem/core.c                 |  33 ++++++
 drivers/nvmem/ocotp.c                |  62 +++++++++--
 drivers/of/Kconfig                   |  12 ++
 drivers/soc/imx/Kconfig              |   6 +
 drivers/soc/imx/Makefile             |   1 +
 drivers/soc/imx/imx8m-featctrl.c     | 101 +++++++++++++++++
 include/dt-bindings/features/imx8m.h |  14 +++
 include/featctrl.h                   |  29 +++++
 include/linux/nvmem-consumer.h       |   9 ++
 include/soc/imx8m/featctrl.h         |  25 +++++
 16 files changed, 550 insertions(+), 8 deletions(-)
 create mode 100644 drivers/base/featctrl.c
 create mode 100644 drivers/soc/imx/imx8m-featctrl.c
 create mode 100644 include/dt-bindings/features/imx8m.h
 create mode 100644 include/featctrl.h
 create mode 100644 include/soc/imx8m/featctrl.h

-- 
2.30.2

[PATCH 01/10] driver: add feature controller framework

[Ahmad Fatoum]

Many SoCs feature hardware that controls or reports access restrictions
to specific devices, e.g. a TrustZone firewall controller can limit
which devices are accessible to the non-secure world and a fusebank
can report that some peripherals are gated and unusable.

A feature controller is an abstraction that covers both cases.
Nodes that are dependent on a feature (e.g. exception level or
SoC type) get a barebox,feature-gates property that references
a feature controller with a feature index. The feature controller
registers a callback with the framework that checks whether
a device is accessible.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/base/Kconfig    |   3 +
 drivers/base/Makefile   |   1 +
 drivers/base/featctrl.c | 120 ++++++++++++++++++++++++++++++++++++++++
 include/featctrl.h      |  29 ++++++++++
 4 files changed, 153 insertions(+)
 create mode 100644 drivers/base/featctrl.c
 create mode 100644 include/featctrl.h

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 5bc70aa1e525..eebb60ce9193 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -2,3 +2,6 @@
 
 config PM_GENERIC_DOMAINS
 	bool
+
+config FEATURE_CONTROLLER
+	bool "Feature controller support" if COMPILE_TEST
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 59645c6f5359..e8e354cdaabc 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -6,3 +6,4 @@ obj-y	+= resource.o
 obj-y	+= regmap/
 
 obj-$(CONFIG_PM_GENERIC_DOMAINS) += power.o
+obj-$(CONFIG_FEATURE_CONTROLLER) += featctrl.o
diff --git a/drivers/base/featctrl.c b/drivers/base/featctrl.c
new file mode 100644
index 000000000000..a5d06323c221
--- /dev/null
+++ b/drivers/base/featctrl.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// SPDX-FileCopyrightText: 2022 Ahmad Fatoum, Pengutronix
+
+#define pr_fmt(fmt) "featctrl: " fmt
+
+#include <common.h>
+#include <driver.h>
+#include <errno.h>
+#include <of.h>
+
+#include <featctrl.h>
+
+/* List of registered feature controllers */
+static LIST_HEAD(of_feature_controllers);
+
+/**
+ * feature_controller_register() - Register a feature controller
+ * @feat: Pointer to feature controller
+ */
+int feature_controller_register(struct feature_controller *feat)
+{
+	struct device_node *np = dev_of_node(feat->dev);
+
+	if (!np)
+		return -EINVAL;
+
+	list_add(&feat->list, &of_feature_controllers);
+	dev_dbg(feat->dev, "Registering feature controller\n");
+	return 0;
+}
+EXPORT_SYMBOL_GPL(feature_controller_register);
+
+/**
+ * featctrl_get_from_provider() - Look-up feature gate
+ * @spec: OF phandle args to use for look-up
+ * @gateid: ID of feature controller gate populated on successful lookup
+ *
+ * Looks for a feature controller under the node specified by @spec.
+ *
+ * Returns a valid pointer to struct feature_controller on success or ERR_PTR()
+ * on failure.
+ */
+static struct feature_controller *featctrl_get_from_provider(struct of_phandle_args *spec,
+							     unsigned *gateid)
+{
+	struct feature_controller *featctrl;
+	int ret;
+
+	if (!spec)
+		return ERR_PTR(-EINVAL);
+
+	ret = of_device_ensure_probed(spec->np);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	/* Check if we have such a controller in our array */
+	list_for_each_entry(featctrl, &of_feature_controllers, list) {
+		if (dev_of_node(featctrl->dev) == spec->np) {
+			*gateid = spec->args[0];
+			return featctrl;
+		}
+	}
+
+	return ERR_PTR(-ENOENT);
+}
+
+/**
+ * of_feature_controller_check - Check whether a feature controller gates the device
+ * @np: Device node to check
+ *
+ * Parse device's OF node to find a feature controller specifier. If such is
+ * found, checks it to determine whether device is gated.
+ *
+ * Returns FEATCTRL_GATED if a specified feature controller gates the device
+ * and FEATCTRL_OKAY if none do. On error a negative error code is returned.
+ */
+int of_feature_controller_check(struct device_node *np)
+{
+	struct of_phandle_args featctrl_args;
+	struct feature_controller *featctrl;
+	int ret, err = 0, i, ngates;
+
+	ngates = of_count_phandle_with_args(np, "barebox,feature-gates",
+					    "#feature-cells");
+	if (ngates <= 0)
+		return FEATCTRL_OKAY;
+
+	for (i = 0; i < ngates; i++) {
+		unsigned gateid = 0;
+
+		ret = of_parse_phandle_with_args(np, "barebox,feature-gates",
+						 "#feature-cells", i, &featctrl_args);
+		if (ret < 0)
+			return ret;
+
+		featctrl = featctrl_get_from_provider(&featctrl_args, &gateid);
+		if (IS_ERR(featctrl)) {
+			ret = PTR_ERR(featctrl);
+			pr_debug("%s() failed to find feature controller: %pe\n",
+				 __func__, ERR_PTR(ret));
+			/*
+			 * Assume that missing featctrls are unresolved
+			 * dependency are report them as deferred
+			 */
+			return (ret == -ENOENT) ? -EPROBE_DEFER : ret;
+		}
+
+		ret = featctrl->check(featctrl, gateid);
+
+		dev_dbg(featctrl->dev, "checking %s: %d\n", np->full_name, ret);
+
+		if (ret == FEATCTRL_OKAY)
+			return FEATCTRL_OKAY;
+		if (ret != FEATCTRL_GATED)
+			err = ret;
+	}
+
+	return err ?: FEATCTRL_GATED;
+}
+EXPORT_SYMBOL_GPL(of_feature_controller_check);
diff --git a/include/featctrl.h b/include/featctrl.h
new file mode 100644
index 000000000000..fb9e4156bdc2
--- /dev/null
+++ b/include/featctrl.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __FEATCTRL_H_
+#define __FEATCTRL_H_
+
+#include <linux/list.h>
+
+struct feature_controller;
+struct device_node;
+
+struct feature_controller {
+	struct device_d *dev;
+	int (*check)(struct feature_controller *, int idx);
+	struct list_head list;
+};
+
+enum { FEATCTRL_GATED = 0, FEATCTRL_OKAY = 1 };
+
+int feature_controller_register(struct feature_controller *);
+
+#ifdef CONFIG_FEATURE_CONTROLLER
+int of_feature_controller_check(struct device_node *np);
+#else
+static inline int of_feature_controller_check(struct device_node *np)
+{
+	return FEATCTRL_OKAY;
+}
+#endif
+
+#endif /* PINCTRL_H */
-- 
2.30.2

[PATCH 02/10] driver: consult feature controller prior to device probe

[Ahmad Fatoum]

The newly added feature controller framework has two goals: Avoid
probing device in barebox that aren't indeed available and fixing
up the kernel tree, so the same devices aren't probed either.

The first one is easily done, by checking whether a feature is gated
prior to probe.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/base/driver.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index e7288f6a61cc..072870bea444 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -27,6 +27,7 @@
 #include <linux/err.h>
 #include <complete.h>
 #include <pinctrl.h>
+#include <featctrl.h>
 #include <linux/clk/clk-conf.h>
 
 #ifdef CONFIG_DEBUG_PROBES
@@ -85,6 +86,14 @@ int device_probe(struct device_d *dev)
 	static int depth = 0;
 	int ret;
 
+	ret = of_feature_controller_check(dev->device_node);
+	if (ret < 0)
+		return ret;
+	if (ret == FEATCTRL_GATED) {
+		dev_dbg(dev, "feature gated, skipping probe\n");
+		return -ENODEV;
+	}
+
 	depth++;
 
 	pr_report_probe("%*sprobe-> %s\n", depth * 4, "", dev_name(dev));
-- 
2.30.2

Re: [PATCH 02/10] driver: consult feature controller prior to device probe

[Philipp Zabel]

Hi Ahmad,

On Do, 2022-08-18 at 07:19 +0200, Ahmad Fatoum wrote:
> The newly added feature controller framework has two goals: Avoid
> probing device in barebox that aren't indeed available

This specific wording makes me wonder why this isn't implemented inside
of_device_is_available().

This would also take care of other devices querying device node
availability, for example via of_graph_port_is_available(), if that
ever happens.

regards
Philipp

Re: [PATCH 02/10] driver: consult feature controller prior to device probe

[Ahmad Fatoum]

Hello Philipp,

On 18.08.22 10:19, Philipp Zabel wrote:
> Hi Ahmad,
> 
> On Do, 2022-08-18 at 07:19 +0200, Ahmad Fatoum wrote:
>> The newly added feature controller framework has two goals: Avoid
>> probing device in barebox that aren't indeed available
> 
> This specific wording makes me wonder why this isn't implemented inside
> of_device_is_available().
> 
> This would also take care of other devices querying device node
> availability, for example via of_graph_port_is_available(), if that
> ever happens.

I admit that restricting the barebox-side of feature controllers to device
probe can be limiting. For kernel fixup, any node may be disabled, unlike
with barebox, where only device probes can be skipped.

Placing the feature controller check in of_device_is_available is not
trivial. We currently do a single pass through the device tree and create
devices that are of_device_is_available(). If we start to look up the
feature controller there, we will need to do device creation in multiple
passes. While of_probe() can be called multiple times, I think there are
quite a few cornercase we will need to take into consideration when
doing this and are thus better addressed separately when the need arises.

Thanks,
Ahmad

> 
> regards
> Philipp
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 03/10] driver: featctrl: fixup kernel device tree

[Ahmad Fatoum]

barebox not proving feature gated devices is one aspect of the feature
controller framework. The more important one is that Linux doesn't probe
them as these tend to be units like VPUs and GPUs or extra CPU cores, which
barebox usually has no use for anyway. Add a fixup that runs for every
DT and evaluates barebox,feature-gates properties in the barebox device
tree and fixes up the result into the kernel device tree using
reproducible names to match nodes between the two.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/base/featctrl.c | 40 ++++++++++++++++++++++++++++++++++++++++
 drivers/of/Kconfig      | 12 ++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/base/featctrl.c b/drivers/base/featctrl.c
index a5d06323c221..abe21698ede7 100644
--- a/drivers/base/featctrl.c
+++ b/drivers/base/featctrl.c
@@ -118,3 +118,43 @@ int of_feature_controller_check(struct device_node *np)
 	return err ?: FEATCTRL_GATED;
 }
 EXPORT_SYMBOL_GPL(of_feature_controller_check);
+
+static int of_featctrl_fixup(struct device_node *root, void *context)
+{
+	struct device_node *srcnp, *dstnp;
+	int err = 0;
+
+	for_each_node_with_property(srcnp, "barebox,feature-gates") {
+		int ret;
+
+		ret = of_feature_controller_check(srcnp);
+		if (ret < 0)
+			err = ret;
+		if (ret != FEATCTRL_GATED)
+			continue;
+
+		dstnp = of_get_node_by_reproducible_name(root, srcnp);
+		if (!dstnp) {
+			pr_debug("gated node %s not in fixup DT\n",
+				 srcnp->full_name);
+			continue;
+		}
+
+		pr_debug("fixing up gating of node %s\n", dstnp->full_name);
+		/* Convention is deleting non-existing CPUs, not disable them. */
+		if (of_property_match_string(srcnp, "device_type", "cpu") >= 0)
+			of_delete_node(dstnp);
+		else
+			of_device_disable(dstnp);
+	}
+
+	return err;
+}
+
+static __maybe_unused int of_featctrl_fixup_register(void)
+{
+	return of_register_fixup(of_featctrl_fixup, NULL);
+}
+#ifdef CONFIG_FEATURE_CONTROLLER_FIXUP
+device_initcall(of_featctrl_fixup_register);
+#endif
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index f3d2cc00cc86..7283331ba9ce 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -16,6 +16,18 @@ config OFDEVICE
 	select DTC
 	bool "Enable probing of devices from the devicetree"
 
+config FEATURE_CONTROLLER_FIXUP
+	bool "Fix up DT nodes gated by feature controller"
+	depends on FEATURE_CONTROLLER
+	default y
+	help
+	  When specified, barebox feature controller drivers are consulted
+	  prior to probing nodes to detect whether the device may not
+	  be available (e.g. because support is fused out).
+	  This option additionally fixes up the kernel device tree,
+	  so it doesn't attempt probing these devices either.
+	  If unsure, say y.
+
 config OF_ADDRESS_PCI
 	bool
 
-- 
2.30.2

[PATCH 04/10] dt-bindings: add i.MX8M feature controller bindings

[Ahmad Fatoum]

As a first step, we'll add feature controller support for i.MX8MM and
i.MX8MN. Both can exist in a dual and a solo CPU core variant and the
i.MX8MNL lacks a GPU, while the i.MX8MML lacks a GPU. Define a
dt-bindings header with the associated feature IDs.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/dt-bindings/features/imx8m.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 include/dt-bindings/features/imx8m.h

diff --git a/include/dt-bindings/features/imx8m.h b/include/dt-bindings/features/imx8m.h
new file mode 100644
index 000000000000..4dd85aa5c8fb
--- /dev/null
+++ b/include/dt-bindings/features/imx8m.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: (GPL-2.0-or-later OR MIT) */
+#ifndef __DT_BINDINGS_FEATURES_IMX8M_
+#define __DT_BINDINGS_FEATURES_IMX8M_
+
+#define IMX8M_FEAT_DUMMY	0
+
+#define IMX8M_FEAT_CPU_DUAL	1
+#define IMX8M_FEAT_CPU_QUAD	2
+#define IMX8M_FEAT_VPU		4
+#define IMX8M_FEAT_GPU		4
+
+#define IMX8M_FEAT_END		5
+
+#endif
-- 
2.30.2

[PATCH 05/10] soc: imx: add i.MX8M feature controller driver

[Ahmad Fatoum]

The tester4 fuse bank of the i.MX8M is a 32-bit collection of fuses,
apaprently fused during test, which contains information about the
available IPs: How many cores are available and whether a VPU and GPU
is available an usable. Add a imx8m_feat_ctrl_init() function
that initializes a bitmap of supported features using tester4's value
and registers a feature controller with a check callback that just
looks up the relevant bit.

This function can then be called from a standalone driver or from the
fuse bank (ocotp) driver itself.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/soc/imx/Kconfig          |  6 +++
 drivers/soc/imx/Makefile         |  1 +
 drivers/soc/imx/imx8m-featctrl.c | 64 ++++++++++++++++++++++++++++++++
 include/soc/imx8m/featctrl.h     | 25 +++++++++++++
 4 files changed, 96 insertions(+)
 create mode 100644 drivers/soc/imx/imx8m-featctrl.c
 create mode 100644 include/soc/imx8m/featctrl.h

diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig
index 742d13c4f5d7..eabe4a06d22c 100644
--- a/drivers/soc/imx/Kconfig
+++ b/drivers/soc/imx/Kconfig
@@ -7,4 +7,10 @@ config IMX_GPCV2_PM_DOMAINS
 	select PM_GENERIC_DOMAINS
 	default y if ARCH_IMX7 || ARCH_IMX8MQ
 
+config IMX8M_FEATCTRL
+        bool "i.MX8M feature controller"
+	depends on ARCH_IMX8M
+	select FEATURE_CONTROLLER
+	default y
+
 endmenu
diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
index 3a8a8d0b00db..bd1717b03883 100644
--- a/drivers/soc/imx/Makefile
+++ b/drivers/soc/imx/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
+obj-$(CONFIG_IMX8M_FEATCTRL) += imx8m-featctrl.o
diff --git a/drivers/soc/imx/imx8m-featctrl.c b/drivers/soc/imx/imx8m-featctrl.c
new file mode 100644
index 000000000000..480c80e6c1d9
--- /dev/null
+++ b/drivers/soc/imx/imx8m-featctrl.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// SPDX-FileCopyrightText: 2022 Ahmad Fatoum, Pengutronix
+
+#include <common.h>
+#include <linux/bitmap.h>
+#include <featctrl.h>
+#include <soc/imx8m/featctrl.h>
+
+#include <dt-bindings/features/imx8m.h>
+
+struct imx_feat {
+	struct feature_controller feat;
+	unsigned long features[BITS_TO_LONGS(IMX8M_FEAT_END)];
+};
+
+static struct imx_feat *to_imx_feat(struct feature_controller *feat)
+{
+	return container_of(feat, struct imx_feat, feat);
+}
+
+static int imx8m_feat_check(struct feature_controller *feat, int idx)
+{
+	struct imx_feat *priv = to_imx_feat(feat);
+
+	if (idx > IMX8M_FEAT_END)
+		return -EINVAL;
+
+	return test_bit(idx, priv->features) ? FEATCTRL_OKAY : FEATCTRL_GATED;
+}
+
+int imx8m_feat_ctrl_init(struct device_d *dev, u32 tester4,
+			 const struct imx8m_featctrl_data *data)
+{
+	unsigned long *features;
+	struct imx_feat *priv;
+
+	if (!dev || !data)
+		return -ENODEV;
+
+	dev_dbg(dev, "tester4 = 0x%08x\n", tester4);
+
+	priv = xzalloc(sizeof(*priv));
+	features = priv->features;
+
+	bitmap_fill(features, IMX8M_FEAT_END);
+
+	if (tester4 & data->vpu_bitmask)
+		clear_bit(IMX8M_FEAT_VPU, features);
+	if (tester4 & data->gpu_bitmask)
+		clear_bit(IMX8M_FEAT_GPU, features);
+
+	switch (tester4 & 3) {
+	case 0b11:
+		clear_bit(IMX8M_FEAT_CPU_DUAL, features);
+		fallthrough;
+	case 0b10:
+		clear_bit(IMX8M_FEAT_CPU_QUAD, features);
+	}
+
+	priv->feat.dev = dev;
+	priv->feat.check = imx8m_feat_check;
+
+	return feature_controller_register(&priv->feat);
+}
diff --git a/include/soc/imx8m/featctrl.h b/include/soc/imx8m/featctrl.h
new file mode 100644
index 000000000000..af94995a916a
--- /dev/null
+++ b/include/soc/imx8m/featctrl.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* SPDX-FileCopyrightText: 2022 Ahmad Fatoum, Pengutronix */
+
+#ifndef __IMX8M_FEATCTRL_H_
+#define __IMX8M_FEATCTRL_H_
+
+#include <linux/types.h>
+
+struct imx8m_featctrl_data {
+	u32 vpu_bitmask;
+	u32 gpu_bitmask;
+};
+
+#ifdef CONFIG_IMX8M_FEATCTRL
+int imx8m_feat_ctrl_init(struct device_d *dev, u32 tester4,
+			 const struct imx8m_featctrl_data *data);
+#else
+static inline int imx8m_feat_ctrl_init(struct device_d *dev, u32 tester4,
+				       const struct imx8m_featctrl_data *data)
+{
+	return -ENODEV;
+}
+#endif
+
+#endif
-- 
2.30.2

[PATCH 06/10] nvmem: import Linux nvmem_cell_read_variable_le_u32

[Ahmad Fatoum]

We already have nvmem_cell_get_and_read(), so add its Linux sibling
nvmem_cell_read_variable_le_u32 as well, which additionally takes care
of conversion to little-endian.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/nvmem/core.c           | 33 +++++++++++++++++++++++++++++++++
 include/linux/nvmem-consumer.h |  9 +++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index fed387c43a26..c5fe5f6b767a 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -803,3 +803,36 @@ void *nvmem_cell_get_and_read(struct device_node *np, const char *cell_name,
 	return value;
 }
 EXPORT_SYMBOL_GPL(nvmem_cell_get_and_read);
+
+/**
+ * nvmem_cell_read_variable_le_u32() - Read up to 32-bits of data as a little endian number.
+ *
+ * @dev: Device that requests the nvmem cell.
+ * @cell_id: Name of nvmem cell to read.
+ * @val: pointer to output value.
+ *
+ * Return: 0 on success or negative errno.
+ */
+int nvmem_cell_read_variable_le_u32(struct device_d *dev, const char *cell_id,
+				    u32 *val)
+{
+	size_t len;
+	const u8 *buf;
+	int i;
+
+	len = sizeof(*val);
+
+	buf = nvmem_cell_get_and_read(dev->device_node, cell_id, len);
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	/* Copy w/ implicit endian conversion */
+	*val = 0;
+	for (i = 0; i < len; i++)
+		*val |= buf[i] << (8 * i);
+
+	kfree(buf);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(nvmem_cell_read_variable_le_u32);
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index b979f23372a6..b461f840957e 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -34,6 +34,8 @@ void nvmem_cell_put(struct nvmem_cell *cell);
 void *nvmem_cell_read(struct nvmem_cell *cell, size_t *len);
 void *nvmem_cell_get_and_read(struct device_node *np, const char *cell_name,
 			      size_t bytes);
+int nvmem_cell_read_variable_le_u32(struct device_d *dev, const char *cell_id,
+				    u32 *val);
 
 int nvmem_cell_write(struct nvmem_cell *cell, void *buf, size_t len);
 
@@ -75,6 +77,13 @@ static inline void *nvmem_cell_get_and_read(struct device_node *np,
 	return ERR_PTR(-EOPNOTSUPP);
 }
 
+static inline int nvmem_cell_read_variable_le_u32(struct device_d *dev,
+						  const char *cell_id,
+						  u32 *val)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
 static inline int nvmem_cell_write(struct nvmem_cell *cell,
 				    void *buf, size_t len)
 {
-- 
2.30.2

[PATCH 07/10] nvmem: ocotp: add i.MX8M[MN] feature controller support

[Ahmad Fatoum]

Devices with a feature-controller property can be specified as provider
in a barebox,feature-gates property to announce that the bus should
consult them before instantiating or probing devices.

Check for this property in the OCOTP driver and register a feature
controller for the i.MX8MM and i.MX8MN.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/nvmem/ocotp.c | 62 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 8 deletions(-)

diff --git a/drivers/nvmem/ocotp.c b/drivers/nvmem/ocotp.c
index 1f99a8012f7a..9fcbd1a6a414 100644
--- a/drivers/nvmem/ocotp.c
+++ b/drivers/nvmem/ocotp.c
@@ -28,6 +28,7 @@
 #include <mach/ocotp.h>
 #include <machine_id.h>
 #include <mach/ocotp-fusemap.h>
+#include <soc/imx8m/featctrl.h>
 #include <linux/nvmem-provider.h>
 
 /*
@@ -108,6 +109,7 @@ struct imx_ocotp_data {
 	int (*fuse_blow)(struct ocotp_priv *priv, u32 addr, u32 value);
 	u8  mac_offsets[MAX_MAC_OFFSETS];
 	u8  mac_offsets_num;
+	struct imx8m_featctrl_data *feat;
 };
 
 struct ocotp_priv_ethaddr {
@@ -638,19 +640,20 @@ static struct regmap_bus imx_ocotp_regmap_bus = {
 	.reg_read = imx_ocotp_reg_read,
 };
 
-static void imx_ocotp_init_dt(struct ocotp_priv *priv)
+static int imx_ocotp_init_dt(struct ocotp_priv *priv)
 {
 	char mac[MAC_BYTES];
 	const __be32 *prop;
 	struct device_node *node = priv->dev.parent->device_node;
-	int len;
+	u32 tester4;
+	int ret, len;
 
 	if (!node)
-		return;
+		return 0;
 
 	prop = of_get_property(node, "barebox,provide-mac-address", &len);
 	if (!prop)
-		return;
+		return 0;
 
 	for (; len >= MAC_ADDRESS_PROPLEN; len -= MAC_ADDRESS_PROPLEN) {
 		struct device_node *rnode;
@@ -668,6 +671,15 @@ static void imx_ocotp_init_dt(struct ocotp_priv *priv)
 
 		of_eth_register_ethaddr(rnode, mac);
 	}
+
+	if (!of_property_read_bool(node, "barebox,feature-controller"))
+		return 0;
+
+	ret = regmap_read(priv->map, OCOTP_OFFSET_TO_ADDR(0x450), &tester4);
+	if (ret != 0)
+		return ret;
+
+	return imx8m_feat_ctrl_init(priv->dev.parent, tester4, priv->data->feat);
 }
 
 static int imx_ocotp_write(void *ctx, unsigned offset, const void *val, size_t bytes)
@@ -785,10 +797,12 @@ static int imx_ocotp_probe(struct device_d *dev)
 	if (IS_ENABLED(CONFIG_MACHINE_ID))
 		imx_ocotp_set_unique_machine_id();
 
-	imx_ocotp_init_dt(priv);
+	ret = imx_ocotp_init_dt(priv);
+	if (ret)
+		dev_warn(dev, "feature controller registration failed: %pe\n",
+			 ERR_PTR(ret));
 
 	dev_add_param_bool(&(priv->dev), "sense_enable", NULL, NULL, &priv->sense_enable, priv);
-
 	return 0;
 }
 
@@ -895,6 +909,38 @@ static struct imx_ocotp_data imx8mq_ocotp_data = {
 	.fuse_read = imx6_fuse_read_addr,
 };
 
+static struct imx8m_featctrl_data imx8mm_featctrl_data = {
+	.vpu_bitmask = 0x1c0000,
+};
+
+static struct imx_ocotp_data imx8mm_ocotp_data = {
+	.num_regs = 2048,
+	.addr_to_offset = imx6sl_addr_to_offset,
+	.mac_offsets_num = 1,
+	.mac_offsets = { 0x90 },
+	.format_mac = imx_ocotp_format_mac,
+	.set_timing = imx6_ocotp_set_timing,
+	.fuse_blow = imx6_fuse_blow_addr,
+	.fuse_read = imx6_fuse_read_addr,
+	.feat = &imx8mm_featctrl_data,
+};
+
+static struct imx8m_featctrl_data imx8mn_featctrl_data = {
+	.gpu_bitmask = 0x1000000,
+};
+
+static struct imx_ocotp_data imx8mn_ocotp_data = {
+	.num_regs = 2048,
+	.addr_to_offset = imx6sl_addr_to_offset,
+	.mac_offsets_num = 1,
+	.mac_offsets = { 0x90 },
+	.format_mac = imx_ocotp_format_mac,
+	.set_timing = imx6_ocotp_set_timing,
+	.fuse_blow = imx6_fuse_blow_addr,
+	.fuse_read = imx6_fuse_read_addr,
+	.feat = &imx8mn_featctrl_data,
+};
+
 static struct imx_ocotp_data imx7d_ocotp_data = {
 	.num_regs = 2048,
 	.addr_to_offset = imx6sl_addr_to_offset,
@@ -933,10 +979,10 @@ static __maybe_unused struct of_device_id imx_ocotp_dt_ids[] = {
 		.data = &imx8mq_ocotp_data,
 	}, {
 		.compatible = "fsl,imx8mm-ocotp",
-		.data = &imx8mq_ocotp_data,
+		.data = &imx8mm_ocotp_data,
 	}, {
 		.compatible = "fsl,imx8mn-ocotp",
-		.data = &imx8mq_ocotp_data,
+		.data = &imx8mn_ocotp_data,
 	}, {
 		.compatible = "fsl,vf610-ocotp",
 		.data = &vf610_ocotp_data,
-- 
2.30.2

[PATCH 08/10] ARM: dts: i.MX8MN: describe feature controller

[Ahmad Fatoum]

Now with i.MX8M feature controller driver support available, have the
OCOTP provide feature control on the i.MX8MN to ensure the kernel DT does
not attempt accessing the GPU and its power domain if barebox knows
them to be unavailable.

This is needed because the upstream kernel imx8mm.dtsi only describes
the full-featured SoC, which can lead to hangs when instantiating
drivers for hardware that's unavailable in a less-featureful variant
of the SoC.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/dts/imx8mn.dtsi | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/dts/imx8mn.dtsi b/arch/arm/dts/imx8mn.dtsi
index 176125e73bce..c8ea034e9553 100644
--- a/arch/arm/dts/imx8mn.dtsi
+++ b/arch/arm/dts/imx8mn.dtsi
@@ -1,5 +1,37 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
 
+#include <dt-bindings/features/imx8m.h>
+
 &pgc_otg1 {
 	barebox,allow-dummy;
 };
+
+
+feat: &ocotp {
+	#feature-cells = <1>;
+	barebox,feature-controller;
+};
+
+&A53_1 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_CPU_DUAL>;
+};
+
+&A53_2 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_CPU_QUAD>;
+};
+
+&A53_3 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_CPU_QUAD>;
+};
+
+&gpc {
+	barebox,feature-gates = <&feat 0>;
+};
+
+&gpu {
+	barebox,feature-gates = <&feat IMX8M_FEAT_GPU>;
+};
+
+&pgc_gpumix {
+	barebox,feature-gates = <&feat IMX8M_FEAT_GPU>;
+};
-- 
2.30.2

[PATCH 09/10] RFC: soc: imx: imx8m-featctrl: add i.MX8M[MN] stand-alone driver

[Ahmad Fatoum]

Previous commit added imx8m_feat_ctrl_init() which can be used to
initialize a feature controller for i.MX8M. Add a standalone driver with
new compatibles that can be described in the device tree to control
feature gating of peripherals.

[afa: This is a RFC. I lean towards the OCOTP driver calling
 imx8m_feat_ctrl_init(). See previous commits]

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/soc/imx/imx8m-featctrl.c | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/soc/imx/imx8m-featctrl.c b/drivers/soc/imx/imx8m-featctrl.c
index 480c80e6c1d9..c9fedfde1488 100644
--- a/drivers/soc/imx/imx8m-featctrl.c
+++ b/drivers/soc/imx/imx8m-featctrl.c
@@ -5,6 +5,10 @@
 #include <linux/bitmap.h>
 #include <featctrl.h>
 #include <soc/imx8m/featctrl.h>
+#include <asm/unaligned.h>
+#include <of.h>
+#include <init.h>
+#include <linux/nvmem-consumer.h>
 
 #include <dt-bindings/features/imx8m.h>
 
@@ -62,3 +66,36 @@ int imx8m_feat_ctrl_init(struct device_d *dev, u32 tester4,
 
 	return feature_controller_register(&priv->feat);
 }
+
+static int imx8m_featctrl_probe(struct device_d *dev)
+{
+	const struct imx8m_featctrl_data *data = device_get_match_data(dev);
+	u32 tester4;
+	int ret;
+
+	ret = nvmem_cell_read_variable_le_u32(dev, "tester4", &tester4);
+	if (ret)
+		return ret;
+
+	return imx8m_feat_ctrl_init(dev, tester4, data);
+}
+
+static const struct imx8m_featctrl_data imx8mm_featctrl_data = {
+	.vpu_bitmask = 0x1c0000
+};
+static const struct imx8m_featctrl_data imx8mn_featctrl_data = {
+	.gpu_bitmask = 0x1000000
+};
+
+static const struct of_device_id imx8m_featctrl_dt_ids[] = {
+	{ .compatible = "barebox,imx8mm-featctrl", &imx8mm_featctrl_data },
+	{ .compatible = "barebox,imx8mn-featctrl", &imx8mn_featctrl_data },
+	{ /* sentinel */ }
+};
+
+static struct driver_d imx8m_featctrl_driver = {
+	.name = "imx8m-featctrl",
+	.probe = imx8m_featctrl_probe,
+	.of_compatible = imx8m_featctrl_dt_ids,
+};
+coredevice_platform_driver(imx8m_featctrl_driver);
-- 
2.30.2

[PATCH 10/10] RFC: ARM: dts: i.MX8MM: describe standlone feature controller

[Ahmad Fatoum]

Now with i.MX8M feature controller driver support available, define it
for the i.MX8MM to ensure the kernel DT does not attempt accessing VPUs
and their power domain/blkctrl if barebox knows them to be unavailable.

This is needed because the upstream kernel imx8mm.dtsi only describes
the full-featured SoC, which can lead to hangs when instantiating
drivers for hardware that's unavailable in a less-featureful variant of
the SoC.

[afa: This is one possibility for the binding. My favourite would
 be having the ocotp be the feature-controller, see previous commits]

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/dts/imx8mm.dtsi | 61 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm/dts/imx8mm.dtsi b/arch/arm/dts/imx8mm.dtsi
index cdf212820594..bef791739e72 100644
--- a/arch/arm/dts/imx8mm.dtsi
+++ b/arch/arm/dts/imx8mm.dtsi
@@ -1,8 +1,25 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+#include <dt-bindings/features/imx8m.h>
+
 / {
 	aliases {
 		gpr.reboot_mode = &reboot_mode_gpr;
 	};
+
+	feat: imx8m-feature-controller {
+		compatible = "barebox,imx8mm-featctrl";
+		#feature-cells = <1>;
+		barebox,feature-controller;
+		nvmem-cell-names = "tester4";
+		nvmem-cells = <&soc_tester4>;
+	};
+};
+
+&ocotp {
+	soc_tester4: tester4@14 {
+		reg = <0x14 4>;
+	};
 };
 
 &pgc_otg1 {
@@ -24,3 +41,47 @@
 		mode-serial = <0x00000010>, <0x40000000>;
 	};
 };
+
+&A53_1 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_CPU_DUAL>;
+};
+
+&A53_2 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_CPU_QUAD>;
+};
+
+&A53_3 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_CPU_QUAD>;
+};
+
+&gpc {
+	barebox,feature-gates = <&feat 0>;
+};
+
+&vpu_g1 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_VPU>;
+};
+
+&vpu_g2 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_VPU>;
+};
+
+&vpu_blk_ctrl {
+	barebox,feature-gates = <&feat IMX8M_FEAT_VPU>;
+};
+
+&pgc_vpumix {
+	barebox,feature-gates = <&feat IMX8M_FEAT_VPU>;
+};
+
+&pgc_vpu_g1 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_VPU>;
+};
+
+&pgc_vpu_g2 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_VPU>;
+};
+
+&pgc_vpu_h1 {
+	barebox,feature-gates = <&feat IMX8M_FEAT_VPU>;
+};
-- 
2.30.2

Re: [PATCH 00/10] Add new feature controller framework

[Sascha Hauer]

Hi Ahmad,

On Thu, Aug 18, 2022 at 07:19:45AM +0200, Ahmad Fatoum wrote:
> The i.MX8MM exists in a Lite variant with no VPUs as well as Solo and
> Dual variants with one or two cores respectively instead of the default
> four. For i.MX6, we had a manual fixup taking care of deleting the
> excess CPUs, but for e.g. the i.MX8MP, we have fuses for the M7, VPUs, CAN,
> CAN-FD, ISPs, NPU, ... etc. Describing all that in the DT is overly verbose
> as we need to take care not to rely on specific device node names that
> should be disabled. There has been an upstream attempt to get a binding
> for U-Boot to act on:
> 
>   https://lore.kernel.org/all/20220324042024.26813-1-peng.fan@oss.nxp.com/
> 
> This was refused by the DT maintainer, because any solution to this
> problem should also be flexible enough to cover the case of partitioning
> devices between the secure and normal world.
> 
> There's a patch series upstream to describe a domain-controller binding
> that allows a hypervisor to partition devices into domains. With the
> naming generalized, this fits nicely the use case of gating devices
> behind specific features:
> 
> https://lore.kernel.org/all/3ca7cd75-4b62-2380-adb0-646bbeb647a2@pengutronix.de/
> 
> This series does that. See the first two commit messages for details.
> We use a barebox, prefix as the naming isn't set in stone, but the intention
> is to drop the prefix and potentially rename once an upstream binding is
> approved.

I am fine with this series. Is this still the way you want to have it
merged or do you have an update?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 00/10] Add new feature controller framework

[Ahmad Fatoum]

Hello Sascha,

On 30.08.22 09:32, Sascha Hauer wrote:
> Hi Ahmad,
> 
> On Thu, Aug 18, 2022 at 07:19:45AM +0200, Ahmad Fatoum wrote:
>> The i.MX8MM exists in a Lite variant with no VPUs as well as Solo and
>> Dual variants with one or two cores respectively instead of the default
>> four. For i.MX6, we had a manual fixup taking care of deleting the
>> excess CPUs, but for e.g. the i.MX8MP, we have fuses for the M7, VPUs, CAN,
>> CAN-FD, ISPs, NPU, ... etc. Describing all that in the DT is overly verbose
>> as we need to take care not to rely on specific device node names that
>> should be disabled. There has been an upstream attempt to get a binding
>> for U-Boot to act on:
>>
>>   https://lore.kernel.org/all/20220324042024.26813-1-peng.fan@oss.nxp.com/
>>
>> This was refused by the DT maintainer, because any solution to this
>> problem should also be flexible enough to cover the case of partitioning
>> devices between the secure and normal world.
>>
>> There's a patch series upstream to describe a domain-controller binding
>> that allows a hypervisor to partition devices into domains. With the
>> naming generalized, this fits nicely the use case of gating devices
>> behind specific features:
>>
>> https://lore.kernel.org/all/3ca7cd75-4b62-2380-adb0-646bbeb647a2@pengutronix.de/
>>
>> This series does that. See the first two commit messages for details.
>> We use a barebox, prefix as the naming isn't set in stone, but the intention
>> is to drop the prefix and potentially rename once an upstream binding is
>> approved.
> 
> I am fine with this series. Is this still the way you want to have it
> merged or do you have an update?

No updates, please apply series, except for last two RFC commits.
I'll revise the naming once there is an upstream binding,
but we can do with the barebox, prefixed binding until then.

Cheers,
Ahmad


> 
> Sascha
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 00/10] Add new feature controller framework

[Sascha Hauer]

On Tue, Aug 30, 2022 at 09:38:57AM +0200, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 30.08.22 09:32, Sascha Hauer wrote:
> > Hi Ahmad,
> > 
> > On Thu, Aug 18, 2022 at 07:19:45AM +0200, Ahmad Fatoum wrote:
> >> The i.MX8MM exists in a Lite variant with no VPUs as well as Solo and
> >> Dual variants with one or two cores respectively instead of the default
> >> four. For i.MX6, we had a manual fixup taking care of deleting the
> >> excess CPUs, but for e.g. the i.MX8MP, we have fuses for the M7, VPUs, CAN,
> >> CAN-FD, ISPs, NPU, ... etc. Describing all that in the DT is overly verbose
> >> as we need to take care not to rely on specific device node names that
> >> should be disabled. There has been an upstream attempt to get a binding
> >> for U-Boot to act on:
> >>
> >>   https://lore.kernel.org/all/20220324042024.26813-1-peng.fan@oss.nxp.com/
> >>
> >> This was refused by the DT maintainer, because any solution to this
> >> problem should also be flexible enough to cover the case of partitioning
> >> devices between the secure and normal world.
> >>
> >> There's a patch series upstream to describe a domain-controller binding
> >> that allows a hypervisor to partition devices into domains. With the
> >> naming generalized, this fits nicely the use case of gating devices
> >> behind specific features:
> >>
> >> https://lore.kernel.org/all/3ca7cd75-4b62-2380-adb0-646bbeb647a2@pengutronix.de/
> >>
> >> This series does that. See the first two commit messages for details.
> >> We use a barebox, prefix as the naming isn't set in stone, but the intention
> >> is to drop the prefix and potentially rename once an upstream binding is
> >> approved.
> > 
> > I am fine with this series. Is this still the way you want to have it
> > merged or do you have an update?
> 
> No updates, please apply series, except for last two RFC commits.

Did that.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |