linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] STM32 ETZPC bus controller
@ 2020-05-05  7:33 Benjamin Gaignard
  2020-05-05  7:33 ` [PATCH v3 1/5] dt-bindings: bus: Add firewall bindings Benjamin Gaignard
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Benjamin Gaignard @ 2020-05-05  7:33 UTC (permalink / raw)
  To: robh+dt, mcoquelin.stm32, alexandre.torgue, gregkh,
	loic.pallardy, linus.walleij
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
	Benjamin Gaignard

STM32 Extended TrustZone Protection controller act like a firewall on the
platform bus. Depending of its configuration devices could be accessible
by the TrustZone, the co-processor or the non-secure world. ETZPC
configuration could evolve at runtime for example to switch a device from
non-secure world to co-processor.

The series introduce 'firewall' helpers to handle the new devices-tree
properties. These properties are not dedicated to ETZPC and will be reused
for STM32 next generation of bus controller.

version 3:
- add description in firewall consumer bindings
- add Linus reviewed-by tag

version 2:
- fix unit name into st,stm32-etzpc.yaml example and DT


Benjamin Gaignard (5):
  dt-bindings: bus: Add firewall bindings
  bus: stm32: Introduce firewall controller helpers
  dt-bindings: bus: Add STM32 ETZPC firewall controller
  bus: stm32: Add stm32 ETZPC firewall bus controller
  ARM: dts: stm32: Use ETZPC firewall bus

 .../bindings/bus/stm32/firewall-consumer.yaml      |  36 +++
 .../bindings/bus/stm32/firewall-provider.yaml      |  18 ++
 .../bindings/bus/stm32/st,stm32-etzpc.yaml         |  46 ++++
 arch/arm/boot/dts/stm32mp151.dtsi                  |   7 +-
 drivers/bus/Kconfig                                |   2 +
 drivers/bus/Makefile                               |   2 +
 drivers/bus/stm32/Kconfig                          |  11 +
 drivers/bus/stm32/Makefile                         |   2 +
 drivers/bus/stm32/firewall.c                       | 266 +++++++++++++++++++++
 drivers/bus/stm32/firewall.h                       |  75 ++++++
 drivers/bus/stm32/stm32-etzpc.c                    | 160 +++++++++++++
 include/dt-bindings/bus/stm32/stm32-etzpc.h        |  90 +++++++
 12 files changed, 713 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/bus/stm32/firewall-consumer.yaml
 create mode 100644 Documentation/devicetree/bindings/bus/stm32/firewall-provider.yaml
 create mode 100644 Documentation/devicetree/bindings/bus/stm32/st,stm32-etzpc.yaml
 create mode 100644 drivers/bus/stm32/Kconfig
 create mode 100644 drivers/bus/stm32/Makefile
 create mode 100644 drivers/bus/stm32/firewall.c
 create mode 100644 drivers/bus/stm32/firewall.h
 create mode 100644 drivers/bus/stm32/stm32-etzpc.c
 create mode 100644 include/dt-bindings/bus/stm32/stm32-etzpc.h

-- 
2.15.0


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

* [PATCH v3 1/5] dt-bindings: bus: Add firewall bindings
  2020-05-05  7:33 [PATCH v3 0/5] STM32 ETZPC bus controller Benjamin Gaignard
@ 2020-05-05  7:33 ` Benjamin Gaignard
  2020-05-05  7:33 ` [PATCH v3 2/5] bus: stm32: Introduce firewall controller helpers Benjamin Gaignard
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Benjamin Gaignard @ 2020-05-05  7:33 UTC (permalink / raw)
  To: robh+dt, mcoquelin.stm32, alexandre.torgue, gregkh,
	loic.pallardy, linus.walleij
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
	Benjamin Gaignard

Add schemas for firewall consumer and provider.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
version 3:
- add description in firewall consumer bindings
- add Linus reviewed-by tag

 .../bindings/bus/stm32/firewall-consumer.yaml      | 36 ++++++++++++++++++++++
 .../bindings/bus/stm32/firewall-provider.yaml      | 18 +++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/stm32/firewall-consumer.yaml
 create mode 100644 Documentation/devicetree/bindings/bus/stm32/firewall-provider.yaml

diff --git a/Documentation/devicetree/bindings/bus/stm32/firewall-consumer.yaml b/Documentation/devicetree/bindings/bus/stm32/firewall-consumer.yaml
new file mode 100644
index 000000000000..d3d76f99b38d
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/stm32/firewall-consumer.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/stm32/firewall-consumer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common Bus Firewall consumer binding
+
+description: |
+  Firewall properties provide the possible firewall bus controller
+  configurations for a device.
+  Bus firewall controllers are typically used to control if a hardware
+  block can perform read or write operations on bus.
+  The contents of the firewall bus configuration properties are defined by
+  the binding for the individual firewall controller device.
+
+  The first configuration 'firewall-0' or the one named 'default' is
+  applied before probing the device itself.
+
+maintainers:
+  - Benjamin Gaignard <benjamin.gaignard@st.com>
+
+# always select the core schema
+select: true
+
+properties:
+  firewall-0: true
+
+  firewall-names: true
+
+patternProperties:
+  "firewall-[0-9]":
+    $ref: "/schemas/types.yaml#/definitions/phandle-array"
+
+dependencies:
+  firewall-names: [ firewall-0 ]
diff --git a/Documentation/devicetree/bindings/bus/stm32/firewall-provider.yaml b/Documentation/devicetree/bindings/bus/stm32/firewall-provider.yaml
new file mode 100644
index 000000000000..299824c620ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/stm32/firewall-provider.yaml
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/stm32/firewall-provider.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common Bus Firewall provider binding
+
+maintainers:
+  - Benjamin Gaignard <benjamin.gaignard@st.com>
+
+properties:
+  '#firewall-cells':
+     $ref: /schemas/types.yaml#/definitions/uint32
+     description: Number of cells in a bus firewall specifier
+
+required:
+  - '#firewall-cells'
-- 
2.15.0


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

* [PATCH v3 2/5] bus: stm32: Introduce firewall controller helpers
  2020-05-05  7:33 [PATCH v3 0/5] STM32 ETZPC bus controller Benjamin Gaignard
  2020-05-05  7:33 ` [PATCH v3 1/5] dt-bindings: bus: Add firewall bindings Benjamin Gaignard
@ 2020-05-05  7:33 ` Benjamin Gaignard
  2020-05-05 14:40   ` Greg KH
  2020-05-05  7:33 ` [PATCH v3 3/5] dt-bindings: bus: Add STM32 ETZPC firewall controller Benjamin Gaignard
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Benjamin Gaignard @ 2020-05-05  7:33 UTC (permalink / raw)
  To: robh+dt, mcoquelin.stm32, alexandre.torgue, gregkh,
	loic.pallardy, linus.walleij
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
	Benjamin Gaignard

The goal of these helpers are to offer an interface for the
hardware blocks controlling bus accesses rights.

Bus firewall controllers are typically used to control if a
hardware block can perform read or write operations on bus.

Smarter firewall controllers could be able to define accesses
rights per hardware blocks to control where they can read
or write.

Firewall controller configurations are provided in device node,
parsed by the helpers and send to the driver to apply them.
Each controller may need different number and type of inputs
to configure the firewall so device-tree properties size have to
be define by using "#firewall-cells".
Firewall configurations properties have to be named "firewall-X"
on device node.
"firewall-names" keyword can also be used to give a name to
a specific configuration.

Example of device-tree:
ctrl0: firewall@0 {
	#firewall-cells = <2>;
      };

foo: foo@0 {
	firewall-names = "default", "setting1";
	firewall-0 = <&ctrl0 1 2>;
	firewall-1 = <&ctrl0 3 4>;
};

Configurations could be applied with functions like
firewall_set_config_by_index() or firewall_set_config_by_name().

firewall_set_default_config() function will apply the
configuration named "default" (if existing) or the configuration
with index 0 (i.e. firewall-0).

Drivers could register/unregister themselves be calling
firewall_register/firewall_unregister functions.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 drivers/bus/Kconfig          |   2 +
 drivers/bus/Makefile         |   2 +
 drivers/bus/stm32/Kconfig    |   3 +
 drivers/bus/stm32/Makefile   |   1 +
 drivers/bus/stm32/firewall.c | 266 +++++++++++++++++++++++++++++++++++++++++++
 drivers/bus/stm32/firewall.h |  75 ++++++++++++
 6 files changed, 349 insertions(+)
 create mode 100644 drivers/bus/stm32/Kconfig
 create mode 100644 drivers/bus/stm32/Makefile
 create mode 100644 drivers/bus/stm32/firewall.c
 create mode 100644 drivers/bus/stm32/firewall.h

diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 6d4e4497b59b..843b356322d9 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -203,4 +203,6 @@ config DA8XX_MSTPRI
 source "drivers/bus/fsl-mc/Kconfig"
 source "drivers/bus/mhi/Kconfig"
 
+source "drivers/bus/stm32/Kconfig"
+
 endmenu
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index 05f32cd694a4..5e0e34b10235 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -37,3 +37,5 @@ obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
 
 # MHI
 obj-$(CONFIG_MHI_BUS)		+= mhi/
+
+obj-$(CONFIG_MACH_STM32MP157) 	+= stm32/
\ No newline at end of file
diff --git a/drivers/bus/stm32/Kconfig b/drivers/bus/stm32/Kconfig
new file mode 100644
index 000000000000..57221e833e2d
--- /dev/null
+++ b/drivers/bus/stm32/Kconfig
@@ -0,0 +1,3 @@
+config FIREWALL_CONTROLLERS
+	bool "Support of bus firewall controllers"
+	depends on OF
diff --git a/drivers/bus/stm32/Makefile b/drivers/bus/stm32/Makefile
new file mode 100644
index 000000000000..eb6b978d6450
--- /dev/null
+++ b/drivers/bus/stm32/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_FIREWALL_CONTROLLERS) += firewall.o
diff --git a/drivers/bus/stm32/firewall.c b/drivers/bus/stm32/firewall.c
new file mode 100644
index 000000000000..95f716cf926f
--- /dev/null
+++ b/drivers/bus/stm32/firewall.c
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+#include "firewall.h"
+
+/* Mutex taken to protect firewall_list */
+static DEFINE_MUTEX(firewall_list_mutex);
+
+/* Global list of firewall control devices */
+static LIST_HEAD(firewall_list);
+
+struct firewall_ctrl {
+	struct list_head list;
+	struct firewall_ops *ops;
+	struct device_node *np;
+	void *priv;
+};
+
+static struct firewall_ctrl *get_firewallctrl_from_node(struct device_node *np)
+{
+	struct firewall_ctrl *ctrl;
+
+	mutex_lock(&firewall_list_mutex);
+
+	list_for_each_entry(ctrl, &firewall_list, list) {
+		if (ctrl->np == np) {
+			mutex_unlock(&firewall_list_mutex);
+			return ctrl;
+		}
+	}
+
+	mutex_unlock(&firewall_list_mutex);
+
+	return NULL;
+}
+
+/**
+ * firewall_dt_has_default
+ *
+ * Check if the device node provide firewall configuration
+ *
+ * @np: device node with possible firewall configuration
+ *
+ * Return: true is firewall-0 property exist in the device node
+ */
+static bool firewall_dt_has_default(struct device_node *np)
+{
+	struct property *prop;
+	int size;
+
+	if (!np)
+		return false;
+
+	prop = of_find_property(np, "firewall-0", &size);
+
+	return prop ? true : false;
+}
+
+/**
+ * firewall_set_config_by_index
+ *
+ * Set a firewall controller configuration based on given index.
+ *
+ * @np: device node with firewall configuration to apply.
+ * @index: the index of the configuration in device node.
+ *
+ * Return: 0 if OK, -EPROBE_DEFER if waiting for firewall controller to be
+ * registered or negative value on other errors.
+ */
+int firewall_set_config_by_index(struct device_node *np, int index)
+{
+	char *propname;
+	int configs, i, err = 0;
+
+	if (!np)
+		return 0;
+
+	propname = kasprintf(GFP_KERNEL, "firewall-%d", index);
+	configs = of_count_phandle_with_args(np, propname, "#firewall-cells");
+	if (configs < 0) {
+		err = -EINVAL;
+		goto error;
+	}
+
+	for (i = 0; i < configs; i++) {
+		struct firewall_ctrl *ctrl;
+		struct of_phandle_args args;
+
+		err = of_parse_phandle_with_args(np, propname,
+						 "#firewall-cells",
+						 i, &args);
+		if (err)
+			goto error;
+
+		/* Test if the controller is (or will be) available */
+		if (!of_device_is_available(args.np)) {
+			of_node_put(args.np);
+			continue;
+		}
+
+		ctrl = get_firewallctrl_from_node(args.np);
+		of_node_put(args.np);
+
+		/* Controller is not yet registered */
+		if (!ctrl) {
+			err = -EPROBE_DEFER;
+			goto error;
+		}
+
+		err = ctrl->ops->set_config(ctrl->priv, &args);
+		if (err)
+			goto error;
+	}
+
+error:
+	kfree(propname);
+	return err;
+}
+EXPORT_SYMBOL_GPL(firewall_set_config_by_index);
+
+/**
+ * firewall_set_config_by_name
+ *
+ * Set a firwall controller configuration based on given name.
+ *
+ * @np: device node with firewall configuration to apply.
+ * @name: the name of the configuration in device node.
+ *
+ * Return: 0 if OK, -EPROBE_DEFER if waiting for firewall controller to be
+ * registered or negative value on other errors.
+ */
+int firewall_set_config_by_name(struct device_node *np, char *name)
+{
+	const char *configname;
+	int count, i;
+
+	count = of_property_count_strings(np, "firewall-names");
+	for (i = 0; i < count; i++) {
+		int err;
+
+		err = of_property_read_string_index(np,
+						    "firewall-names",
+						    i, &configname);
+		if (err)
+			return err;
+
+		if (strcmp(name, configname))
+			continue;
+
+		return firewall_set_config_by_index(np, i);
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(firewall_set_config_by_name);
+
+/**
+ * firewall_set_default_config
+ *
+ * Set the default configuration for device.
+ * First try to apply configuration named "default", if it fails
+ * or doesn't exist, try to apply firewall-0 configuration.
+ *
+ * @np: device node with firewall configuration to apply.
+ *
+ * Return: 0 if OK, -EPROBE_DEFER if waiting for firewall controller to be
+ * registered or negative value on other errors.
+ */
+int firewall_set_default_config(struct device_node *np)
+{
+	int ret;
+
+	/* Nothing to do if device node doesn't contain at least
+	 * one configuration
+	 */
+	if (!firewall_dt_has_default(np))
+		return 0;
+
+	ret = firewall_set_config_by_name(np, "default");
+	if (!ret || (ret == -EPROBE_DEFER))
+		return ret;
+
+	return firewall_set_config_by_index(np, 0);
+}
+EXPORT_SYMBOL_GPL(firewall_set_default_config);
+
+/**
+ * firewall_register
+ *
+ * Register a firewall controller.
+ *
+ * @np: node implementing firewall controller.
+ * @ops: firewall controller operations.
+ * @priv: caller private data.
+ *
+ * Return: 0 if OK or negative value on error.
+ */
+int firewall_register(struct device_node *np,
+		      struct firewall_ops *ops,
+		      void *priv)
+{
+	struct firewall_ctrl *ctrl;
+
+	if (!np || !ops || !ops->set_config)
+		return -EINVAL;
+
+	ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&ctrl->list);
+
+	ctrl->np = np;
+	ctrl->ops = ops;
+	ctrl->priv = priv;
+
+	mutex_lock(&firewall_list_mutex);
+	list_add_tail(&ctrl->list, &firewall_list);
+	mutex_unlock(&firewall_list_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(firewall_register);
+
+/**
+ * firewall_unregister
+ *
+ * Unregister a firewall controller.
+ *
+ * @np: node implementing firewall controller.
+ */
+void firewall_unregister(struct device_node *np)
+{
+	struct firewall_ctrl *ctrl;
+
+	ctrl = get_firewallctrl_from_node(np);
+	if (!ctrl)
+		return;
+
+	mutex_lock(&firewall_list_mutex);
+	list_del(&ctrl->list);
+	mutex_unlock(&firewall_list_mutex);
+
+	kfree(ctrl);
+}
+EXPORT_SYMBOL_GPL(firewall_unregister);
+
+static int __init firewall_init(void)
+{
+	pr_info("initialized bus firewall controller subsystem\n");
+	return 0;
+}
+
+/* Init early since drivers really need to configure firewall early */
+core_initcall(firewall_init);
diff --git a/drivers/bus/stm32/firewall.h b/drivers/bus/stm32/firewall.h
new file mode 100644
index 000000000000..d6fd720a229e
--- /dev/null
+++ b/drivers/bus/stm32/firewall.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ */
+
+#ifndef _FIREWALL_H_
+#define _FIREWALL_H_
+
+#include <linux/device.h>
+#include <linux/of.h>
+
+/**
+ * struct firewall_ops
+ *
+ * Firewall controller operations structure to be filled by drivers.
+ */
+struct firewall_ops {
+	/**
+	 * @set_config:
+	 *
+	 * Driver callback to set a firewall configuration on a controller.
+	 * Configuration arguments are provided in out_args parameter.
+	 *
+	 * Return: 0 on success, a negative error code on failure.
+	 */
+	int (*set_config)(void *priv, struct of_phandle_args *out_args);
+};
+
+#ifdef CONFIG_FIREWALL_CONTROLLERS
+
+int firewall_set_config_by_index(struct device_node *np, int index);
+int firewall_set_config_by_name(struct device_node *np, char *name);
+int firewall_set_default_config(struct device_node *np);
+
+int firewall_register(struct device_node *np,
+		      struct firewall_ops *ops,
+		      void *priv);
+
+void firewall_unregister(struct device_node *np);
+
+#else
+
+static inline int firewall_set_config_by_index(struct device_node *np,
+					       int index)
+{
+	return 0;
+}
+
+static inline int firewall_set_config_by_name(struct device_node *np,
+					      char *name)
+{
+	return 0;
+}
+
+static inline int firewall_set_default_config(struct device_node *np)
+{
+	return 0;
+}
+
+static inline int firewall_register(struct device_node *np,
+				    struct firewall_ops *ops,
+				    void *priv)
+{
+	return 0;
+}
+
+static inline void firewall_unregister(struct device_node *np)
+{
+	/* Empty */
+}
+
+#endif
+
+#endif /* _FIREWALL_H_ */
-- 
2.15.0


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

* [PATCH v3 3/5] dt-bindings: bus: Add STM32 ETZPC firewall controller
  2020-05-05  7:33 [PATCH v3 0/5] STM32 ETZPC bus controller Benjamin Gaignard
  2020-05-05  7:33 ` [PATCH v3 1/5] dt-bindings: bus: Add firewall bindings Benjamin Gaignard
  2020-05-05  7:33 ` [PATCH v3 2/5] bus: stm32: Introduce firewall controller helpers Benjamin Gaignard
@ 2020-05-05  7:33 ` Benjamin Gaignard
  2020-05-05  7:33 ` [PATCH v3 4/5] bus: stm32: Add stm32 ETZPC firewall bus controller Benjamin Gaignard
  2020-05-05  7:33 ` [PATCH v3 5/5] ARM: dts: stm32: Use ETZPC firewall bus Benjamin Gaignard
  4 siblings, 0 replies; 9+ messages in thread
From: Benjamin Gaignard @ 2020-05-05  7:33 UTC (permalink / raw)
  To: robh+dt, mcoquelin.stm32, alexandre.torgue, gregkh,
	loic.pallardy, linus.walleij
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
	Benjamin Gaignard

Document STM32 ETZPC firewall controller bindings

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 .../bindings/bus/stm32/st,stm32-etzpc.yaml         | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/stm32/st,stm32-etzpc.yaml

diff --git a/Documentation/devicetree/bindings/bus/stm32/st,stm32-etzpc.yaml b/Documentation/devicetree/bindings/bus/stm32/st,stm32-etzpc.yaml
new file mode 100644
index 000000000000..d92865fda40c
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/stm32/st,stm32-etzpc.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/stm32/st,stm32-etzpc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STM32 Extended TrustZone Protection controller
+
+maintainers:
+  - Benjamin Gaignard <benjamin.gaignard@st.com>
+
+description: STMicroelectronics's STM32 firewall bus controller implementation
+
+allOf:
+  - $ref: "firewall-provider.yaml#"
+  - $ref: /schemas/simple-bus.yaml#
+
+properties:
+  compatible:
+    contains:
+      enum:
+        - st,stm32-etzpc-bus
+
+  reg:
+   maxItems: 1
+
+  '#firewall-cells':
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - '#firewall-cells'
+
+examples:
+  - |
+    soc@5c007000 {
+      compatible = "st,stm32-etzpc-bus", "simple-bus";
+      reg = <0x5c007000 0x400>;
+      #firewall-cells = <2>;
+      #address-cells = <1>;
+      #size-cells = <1>;
+      ranges;
+    };
+
+...
-- 
2.15.0


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

* [PATCH v3 4/5] bus: stm32: Add stm32 ETZPC firewall bus controller
  2020-05-05  7:33 [PATCH v3 0/5] STM32 ETZPC bus controller Benjamin Gaignard
                   ` (2 preceding siblings ...)
  2020-05-05  7:33 ` [PATCH v3 3/5] dt-bindings: bus: Add STM32 ETZPC firewall controller Benjamin Gaignard
@ 2020-05-05  7:33 ` Benjamin Gaignard
  2020-05-05  7:33 ` [PATCH v3 5/5] ARM: dts: stm32: Use ETZPC firewall bus Benjamin Gaignard
  4 siblings, 0 replies; 9+ messages in thread
From: Benjamin Gaignard @ 2020-05-05  7:33 UTC (permalink / raw)
  To: robh+dt, mcoquelin.stm32, alexandre.torgue, gregkh,
	loic.pallardy, linus.walleij
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
	Benjamin Gaignard

Add STM32 Extended TrustZone Protection bus controller.
For each of device-tree nodes it will check and apply
firewall configuration. If it doesn't match the device
will not be probed by platform bus.

A device could be configured to be accessible by trusted world,
co-processor or non-secure world.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 drivers/bus/stm32/Kconfig                   |   8 ++
 drivers/bus/stm32/Makefile                  |   1 +
 drivers/bus/stm32/stm32-etzpc.c             | 160 ++++++++++++++++++++++++++++
 include/dt-bindings/bus/stm32/stm32-etzpc.h |  90 ++++++++++++++++
 4 files changed, 259 insertions(+)
 create mode 100644 drivers/bus/stm32/stm32-etzpc.c
 create mode 100644 include/dt-bindings/bus/stm32/stm32-etzpc.h

diff --git a/drivers/bus/stm32/Kconfig b/drivers/bus/stm32/Kconfig
index 57221e833e2d..5dc6e2504de5 100644
--- a/drivers/bus/stm32/Kconfig
+++ b/drivers/bus/stm32/Kconfig
@@ -1,3 +1,11 @@
 config FIREWALL_CONTROLLERS
 	bool "Support of bus firewall controllers"
 	depends on OF
+
+config STM32_ETZPC
+	bool "STM32 ETZPC bus controller"
+	depends on MACH_STM32MP157
+	select FIREWALL_CONTROLLERS
+	help
+	  Select y to enable STM32 Extended TrustZone Protection
+	  Controller (ETZPC)
diff --git a/drivers/bus/stm32/Makefile b/drivers/bus/stm32/Makefile
index eb6b978d6450..d42e99b5865e 100644
--- a/drivers/bus/stm32/Makefile
+++ b/drivers/bus/stm32/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_FIREWALL_CONTROLLERS) += firewall.o
+obj-$(CONFIG_STM32_ETZPC) += stm32-etzpc.o
diff --git a/drivers/bus/stm32/stm32-etzpc.c b/drivers/bus/stm32/stm32-etzpc.c
new file mode 100644
index 000000000000..c216c71448ed
--- /dev/null
+++ b/drivers/bus/stm32/stm32-etzpc.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/bus/stm32/stm32-etzpc.h>
+
+#include "firewall.h"
+
+#define ETZPC_DECPROT	0x010
+#define ETZPC_NUM_LOCKS	94
+
+struct stm32_etzpc {
+	struct regmap_field *fields[ETZPC_NUM_LOCKS];
+};
+
+static int stm32_etzpc_set_config(void *priv, struct of_phandle_args *out_args)
+{
+	struct stm32_etzpc *etzpc = dev_get_drvdata((struct device *)priv);
+	int index = out_args->args[0];
+	unsigned int value = out_args->args[1];
+	u32 status;
+
+	if (out_args->args_count != 2)
+		return -EINVAL;
+
+	if (index >= ETZPC_NUM_LOCKS)
+		return -EINVAL;
+
+	if (value > STM32_ETZPC_NON_SECURE)
+		return -EINVAL;
+
+	regmap_field_force_write(etzpc->fields[index], value);
+
+	/* Hardware could denied the new value, read it back to check it */
+	regmap_field_read(etzpc->fields[index], &status);
+
+	if (value != status) {
+		pr_info("failed to set configuration: index %d, value %d\n",
+			index, value);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static struct firewall_ops stm32_etzpc_ops = {
+	.set_config = stm32_etzpc_set_config,
+};
+
+static const struct regmap_config stm32_etzpc_regmap_cfg = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = sizeof(u32),
+	.max_register = 0x3FF,
+};
+
+static void stm32_etzpc_populate(struct device *parent)
+{
+	struct device_node *child;
+
+	if (!parent)
+		return;
+
+	for_each_available_child_of_node(dev_of_node(parent), child) {
+		if (firewall_set_default_config(child)) {
+			/*
+			 * Failed to set firewall configuration mark the node
+			 * as populated so platform bus won't probe it
+			 */
+			of_node_set_flag(child, OF_POPULATED);
+			dev_info(parent, "%s: Bad firewall configuration\n",
+				 child->name);
+		}
+	}
+}
+
+static int stm32_etzpc_probe(struct platform_device *pdev)
+{
+	struct stm32_etzpc *etzpc;
+	struct regmap *regmap;
+	struct resource *res;
+	void __iomem *mmio;
+	int i, ret;
+
+	etzpc = devm_kzalloc(&pdev->dev, sizeof(*etzpc), GFP_KERNEL);
+	if (!etzpc)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mmio = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mmio))
+		return PTR_ERR(mmio);
+
+	regmap = devm_regmap_init_mmio(&pdev->dev, mmio,
+				       &stm32_etzpc_regmap_cfg);
+
+	for (i = 0; i < ETZPC_NUM_LOCKS; i++) {
+		struct reg_field field;
+
+		/*
+		 * Each hardware block status is defined by
+		 * a 2 bits field and all of them are packed into
+		 * 32 bits registers. Do some computation to get
+		 * register offset and the shift.
+		 */
+		field.reg = ETZPC_DECPROT + (i >> 4) * sizeof(u32);
+		field.lsb = (i % 0x10) << 1;
+		field.msb = field.lsb + 1;
+
+		etzpc->fields[i] = devm_regmap_field_alloc(&pdev->dev,
+							   regmap, field);
+	}
+
+	platform_set_drvdata(pdev, etzpc);
+
+	ret = firewall_register(dev_of_node(&pdev->dev),
+				&stm32_etzpc_ops,
+				&pdev->dev);
+	if (ret)
+		return ret;
+
+	stm32_etzpc_populate(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id stm32_etzpc_of_match[] = {
+	{ .compatible = "st,stm32-etzpc-bus" },
+	{ /* end node */ }
+};
+MODULE_DEVICE_TABLE(of, stm32_etzpc_of_match);
+
+static struct platform_driver stm32_etzpc_driver = {
+	.probe  = stm32_etzpc_probe,
+	.driver = {
+		.name = "stm32-etzpc",
+		.of_match_table = stm32_etzpc_of_match,
+	},
+};
+
+static int __init stm32_etzpc_init(void)
+{
+	return platform_driver_register(&stm32_etzpc_driver);
+}
+arch_initcall(stm32_etzpc_init);
+
+MODULE_AUTHOR("Benjamin Gaignard <benjamin.gaignard@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32 Bus Firewall Controller");
diff --git a/include/dt-bindings/bus/stm32/stm32-etzpc.h b/include/dt-bindings/bus/stm32/stm32-etzpc.h
new file mode 100644
index 000000000000..9c4783b9783c
--- /dev/null
+++ b/include/dt-bindings/bus/stm32/stm32-etzpc.h
@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ */
+
+#ifndef _STM32_ETZPC_H_
+#define _STM32_ETZPC_H_
+
+/* ETZPC configurations: trust-zone, non-secure or coprocessor*/
+#define STM32_ETZPC_TRUST	1
+#define STM32_ETPCZ_COPRO	2
+#define STM32_ETZPC_NON_SECURE	3
+
+/* ETZPC hard blocks index */
+#define STM32_ETZPC_USART1	3
+#define STM32_ETZPC_SPI6	4
+#define STM32_ETZPC_I2C4	5
+#define STM32_ETZPC_RNG1	7
+#define STM32_ETZPC_HASH1	8
+#define STM32_ETZPC_CRYP1	9
+#define STM32_ETZPC_I2C6	12
+#define STM32_ETZPC_TIM2	16
+#define STM32_ETZPC_TIM3	17
+#define STM32_ETZPC_TIM4	18
+#define STM32_ETZPC_TIM5	19
+#define STM32_ETZPC_TIM6	20
+#define STM32_ETZPC_TIM7	21
+#define STM32_ETZPC_TIM12	22
+#define STM32_ETZPC_TIM13	23
+#define STM32_ETZPC_TIM14	24
+#define STM32_ETZPC_LPTIM1	25
+#define STM32_ETZPC_SPI2	27
+#define STM32_ETZPC_SPI3	28
+#define STM32_ETZPC_USART2	30
+#define STM32_ETZPC_USART3	31
+#define STM32_ETZPC_USART4	32
+#define STM32_ETZPC_USART5	33
+#define STM32_ETZPC_I2C1	34
+#define STM32_ETZPC_I2C2	35
+#define STM32_ETZPC_I2C3	36
+#define STM32_ETZPC_I2C5	37
+#define STM32_ETZPC_CEC		38
+#define STM32_ETZPC_DAC		39
+#define STM32_ETZPC_UART7	40
+#define STM32_ETZPC_UART8	41
+#define STM32_ETZPC_MDIOS	44
+#define STM32_ETZPC_TIM1	48
+#define STM32_ETZPC_TIM8	49
+#define STM32_ETZPC_USART6	51
+#define STM32_ETZPC_SPI1	52
+#define STM32_ETZPC_SPI4	53
+#define STM32_ETZPC_TIM15	54
+#define STM32_ETZPC_TIM16	55
+#define STM32_ETZPC_TIM17	56
+#define STM32_ETZPC_SPI5	57
+#define STM32_ETZPC_SAI1	58
+#define STM32_ETZPC_SAI2	59
+#define STM32_ETZPC_SAI3	60
+#define STM32_ETZPC_DFSDM	61
+#define STM32_ETZPC_TT_FDCAN	62
+#define STM32_ETZPC_LPTIM2	64
+#define STM32_ETZPC_LPTIM3	65
+#define STM32_ETZPC_LPTIM4	66
+#define STM32_ETZPC_LPTIM5	67
+#define STM32_ETZPC_SAI4	68
+#define STM32_ETZPC_VREFBUF	69
+#define STM32_ETZPC_DCMI	70
+#define STM32_ETZPC_CRC2	71
+#define STM32_ETZPC_ADC		72
+#define STM32_ETZPC_HASH2	73
+#define STM32_ETZPC_RNG2	74
+#define STM32_ETZPC_CRYP2	75
+#define STM32_ETZPC_SRAM1	80
+#define STM32_ETZPC_SRAM2	81
+#define STM32_ETZPC_SRAM3	82
+#define STM32_ETZPC_SRAM4	83
+#define STM32_ETZPC_RETRAM	84
+#define STM32_ETZPC_OTG		85
+#define STM32_ETZPC_SDMMC3	86
+#define STM32_ETZPC_DLYBSD3	87
+#define STM32_ETZPC_DMA1	88
+#define STM32_ETZPC_DMA2	89
+#define STM32_ETZPC_DMAMUX	90
+#define STM32_ETZPC_FMC		91
+#define STM32_ETZPC_QSPI	92
+#define STM32_ETZPC_DLYBQ	93
+#define STM32_ETZPC_ETH1	94
+
+#endif /* _STM32_ETZPC_H_ */
-- 
2.15.0


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

* [PATCH v3 5/5] ARM: dts: stm32: Use ETZPC firewall bus
  2020-05-05  7:33 [PATCH v3 0/5] STM32 ETZPC bus controller Benjamin Gaignard
                   ` (3 preceding siblings ...)
  2020-05-05  7:33 ` [PATCH v3 4/5] bus: stm32: Add stm32 ETZPC firewall bus controller Benjamin Gaignard
@ 2020-05-05  7:33 ` Benjamin Gaignard
  4 siblings, 0 replies; 9+ messages in thread
From: Benjamin Gaignard @ 2020-05-05  7:33 UTC (permalink / raw)
  To: robh+dt, mcoquelin.stm32, alexandre.torgue, gregkh,
	loic.pallardy, linus.walleij
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
	Benjamin Gaignard

Allow STM32 ETZPC to check firewall configuration before populating
the platform bus.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 arch/arm/boot/dts/stm32mp151.dtsi | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi
index 3ea05ba48215..0290eb6f3c35 100644
--- a/arch/arm/boot/dts/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/stm32mp151.dtsi
@@ -4,6 +4,7 @@
  * Author: Ludovic Barre <ludovic.barre@st.com> for STMicroelectronics.
  */
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/bus/stm32/stm32-etzpc.h>
 #include <dt-bindings/clock/stm32mp1-clks.h>
 #include <dt-bindings/reset/stm32mp1-resets.h>
 
@@ -110,8 +111,10 @@
 		status = "disabled";
 	};
 
-	soc {
-		compatible = "simple-bus";
+	etzpc_bus: soc@5c007000 {
+		compatible = "st,stm32-etzpc-bus", "simple-bus";
+		reg = <0x5c007000 0x400>;
+		#firewall-cells = <2>;
 		#address-cells = <1>;
 		#size-cells = <1>;
 		interrupt-parent = <&intc>;
-- 
2.15.0


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

* Re: [PATCH v3 2/5] bus: stm32: Introduce firewall controller helpers
  2020-05-05  7:33 ` [PATCH v3 2/5] bus: stm32: Introduce firewall controller helpers Benjamin Gaignard
@ 2020-05-05 14:40   ` Greg KH
  2020-05-05 15:00     ` Benjamin GAIGNARD
  0 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2020-05-05 14:40 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: robh+dt, mcoquelin.stm32, alexandre.torgue, loic.pallardy,
	linus.walleij, devicetree, linux-stm32, linux-arm-kernel,
	linux-kernel

On Tue, May 05, 2020 at 09:33:05AM +0200, Benjamin Gaignard wrote:
> The goal of these helpers are to offer an interface for the
> hardware blocks controlling bus accesses rights.
> 
> Bus firewall controllers are typically used to control if a
> hardware block can perform read or write operations on bus.
> 
> Smarter firewall controllers could be able to define accesses
> rights per hardware blocks to control where they can read
> or write.
> 
> Firewall controller configurations are provided in device node,
> parsed by the helpers and send to the driver to apply them.
> Each controller may need different number and type of inputs
> to configure the firewall so device-tree properties size have to
> be define by using "#firewall-cells".
> Firewall configurations properties have to be named "firewall-X"
> on device node.
> "firewall-names" keyword can also be used to give a name to
> a specific configuration.
> 
> Example of device-tree:
> ctrl0: firewall@0 {
> 	#firewall-cells = <2>;
>       };
> 
> foo: foo@0 {
> 	firewall-names = "default", "setting1";
> 	firewall-0 = <&ctrl0 1 2>;
> 	firewall-1 = <&ctrl0 3 4>;
> };
> 
> Configurations could be applied with functions like
> firewall_set_config_by_index() or firewall_set_config_by_name().
> 
> firewall_set_default_config() function will apply the
> configuration named "default" (if existing) or the configuration
> with index 0 (i.e. firewall-0).
> 
> Drivers could register/unregister themselves be calling
> firewall_register/firewall_unregister functions.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  drivers/bus/Kconfig          |   2 +
>  drivers/bus/Makefile         |   2 +
>  drivers/bus/stm32/Kconfig    |   3 +
>  drivers/bus/stm32/Makefile   |   1 +
>  drivers/bus/stm32/firewall.c | 266 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/bus/stm32/firewall.h |  75 ++++++++++++
>  6 files changed, 349 insertions(+)
>  create mode 100644 drivers/bus/stm32/Kconfig
>  create mode 100644 drivers/bus/stm32/Makefile
>  create mode 100644 drivers/bus/stm32/firewall.c
>  create mode 100644 drivers/bus/stm32/firewall.h
> 
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index 6d4e4497b59b..843b356322d9 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -203,4 +203,6 @@ config DA8XX_MSTPRI
>  source "drivers/bus/fsl-mc/Kconfig"
>  source "drivers/bus/mhi/Kconfig"
>  
> +source "drivers/bus/stm32/Kconfig"
> +
>  endmenu
> diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
> index 05f32cd694a4..5e0e34b10235 100644
> --- a/drivers/bus/Makefile
> +++ b/drivers/bus/Makefile
> @@ -37,3 +37,5 @@ obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
>  
>  # MHI
>  obj-$(CONFIG_MHI_BUS)		+= mhi/
> +
> +obj-$(CONFIG_MACH_STM32MP157) 	+= stm32/
> \ No newline at end of file
> diff --git a/drivers/bus/stm32/Kconfig b/drivers/bus/stm32/Kconfig
> new file mode 100644
> index 000000000000..57221e833e2d
> --- /dev/null
> +++ b/drivers/bus/stm32/Kconfig
> @@ -0,0 +1,3 @@
> +config FIREWALL_CONTROLLERS
> +	bool "Support of bus firewall controllers"
> +	depends on OF
> diff --git a/drivers/bus/stm32/Makefile b/drivers/bus/stm32/Makefile
> new file mode 100644
> index 000000000000..eb6b978d6450
> --- /dev/null
> +++ b/drivers/bus/stm32/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_FIREWALL_CONTROLLERS) += firewall.o
> diff --git a/drivers/bus/stm32/firewall.c b/drivers/bus/stm32/firewall.c
> new file mode 100644
> index 000000000000..95f716cf926f
> --- /dev/null
> +++ b/drivers/bus/stm32/firewall.c
> @@ -0,0 +1,266 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +
> +#include "firewall.h"
> +
> +/* Mutex taken to protect firewall_list */
> +static DEFINE_MUTEX(firewall_list_mutex);
> +
> +/* Global list of firewall control devices */
> +static LIST_HEAD(firewall_list);

Why is that needed?  Why can't you just walk the list of devices on this
"bus/class" if you really wanted to?

Along those lines, why is this going around the driver model and
ignoring it?  Shouldn't this be a bus and you have devices attached to
it of the specific type?


greg k-h

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

* Re: [PATCH v3 2/5] bus: stm32: Introduce firewall controller helpers
  2020-05-05 14:40   ` Greg KH
@ 2020-05-05 15:00     ` Benjamin GAIGNARD
  2020-05-05 18:20       ` Greg KH
  0 siblings, 1 reply; 9+ messages in thread
From: Benjamin GAIGNARD @ 2020-05-05 15:00 UTC (permalink / raw)
  To: Greg KH
  Cc: robh+dt, mcoquelin.stm32, Alexandre TORGUE, Loic PALLARDY,
	linus.walleij, devicetree, linux-stm32, linux-arm-kernel,
	linux-kernel



On 5/5/20 4:40 PM, Greg KH wrote:
> On Tue, May 05, 2020 at 09:33:05AM +0200, Benjamin Gaignard wrote:
>> The goal of these helpers are to offer an interface for the
>> hardware blocks controlling bus accesses rights.
>>
>> Bus firewall controllers are typically used to control if a
>> hardware block can perform read or write operations on bus.
>>
>> Smarter firewall controllers could be able to define accesses
>> rights per hardware blocks to control where they can read
>> or write.
>>
>> Firewall controller configurations are provided in device node,
>> parsed by the helpers and send to the driver to apply them.
>> Each controller may need different number and type of inputs
>> to configure the firewall so device-tree properties size have to
>> be define by using "#firewall-cells".
>> Firewall configurations properties have to be named "firewall-X"
>> on device node.
>> "firewall-names" keyword can also be used to give a name to
>> a specific configuration.
>>
>> Example of device-tree:
>> ctrl0: firewall@0 {
>> 	#firewall-cells = <2>;
>>        };
>>
>> foo: foo@0 {
>> 	firewall-names = "default", "setting1";
>> 	firewall-0 = <&ctrl0 1 2>;
>> 	firewall-1 = <&ctrl0 3 4>;
>> };
>>
>> Configurations could be applied with functions like
>> firewall_set_config_by_index() or firewall_set_config_by_name().
>>
>> firewall_set_default_config() function will apply the
>> configuration named "default" (if existing) or the configuration
>> with index 0 (i.e. firewall-0).
>>
>> Drivers could register/unregister themselves be calling
>> firewall_register/firewall_unregister functions.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>>   drivers/bus/Kconfig          |   2 +
>>   drivers/bus/Makefile         |   2 +
>>   drivers/bus/stm32/Kconfig    |   3 +
>>   drivers/bus/stm32/Makefile   |   1 +
>>   drivers/bus/stm32/firewall.c | 266 +++++++++++++++++++++++++++++++++++++++++++
>>   drivers/bus/stm32/firewall.h |  75 ++++++++++++
>>   6 files changed, 349 insertions(+)
>>   create mode 100644 drivers/bus/stm32/Kconfig
>>   create mode 100644 drivers/bus/stm32/Makefile
>>   create mode 100644 drivers/bus/stm32/firewall.c
>>   create mode 100644 drivers/bus/stm32/firewall.h
>>
>> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
>> index 6d4e4497b59b..843b356322d9 100644
>> --- a/drivers/bus/Kconfig
>> +++ b/drivers/bus/Kconfig
>> @@ -203,4 +203,6 @@ config DA8XX_MSTPRI
>>   source "drivers/bus/fsl-mc/Kconfig"
>>   source "drivers/bus/mhi/Kconfig"
>>   
>> +source "drivers/bus/stm32/Kconfig"
>> +
>>   endmenu
>> diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
>> index 05f32cd694a4..5e0e34b10235 100644
>> --- a/drivers/bus/Makefile
>> +++ b/drivers/bus/Makefile
>> @@ -37,3 +37,5 @@ obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
>>   
>>   # MHI
>>   obj-$(CONFIG_MHI_BUS)		+= mhi/
>> +
>> +obj-$(CONFIG_MACH_STM32MP157) 	+= stm32/
>> \ No newline at end of file
>> diff --git a/drivers/bus/stm32/Kconfig b/drivers/bus/stm32/Kconfig
>> new file mode 100644
>> index 000000000000..57221e833e2d
>> --- /dev/null
>> +++ b/drivers/bus/stm32/Kconfig
>> @@ -0,0 +1,3 @@
>> +config FIREWALL_CONTROLLERS
>> +	bool "Support of bus firewall controllers"
>> +	depends on OF
>> diff --git a/drivers/bus/stm32/Makefile b/drivers/bus/stm32/Makefile
>> new file mode 100644
>> index 000000000000..eb6b978d6450
>> --- /dev/null
>> +++ b/drivers/bus/stm32/Makefile
>> @@ -0,0 +1 @@
>> +obj-$(CONFIG_FIREWALL_CONTROLLERS) += firewall.o
>> diff --git a/drivers/bus/stm32/firewall.c b/drivers/bus/stm32/firewall.c
>> new file mode 100644
>> index 000000000000..95f716cf926f
>> --- /dev/null
>> +++ b/drivers/bus/stm32/firewall.c
>> @@ -0,0 +1,266 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/list.h>
>> +#include <linux/of.h>
>> +#include <linux/slab.h>
>> +
>> +#include "firewall.h"
>> +
>> +/* Mutex taken to protect firewall_list */
>> +static DEFINE_MUTEX(firewall_list_mutex);
>> +
>> +/* Global list of firewall control devices */
>> +static LIST_HEAD(firewall_list);
> Why is that needed?  Why can't you just walk the list of devices on this
> "bus/class" if you really wanted to?
>
> Along those lines, why is this going around the driver model and
> ignoring it?  Shouldn't this be a bus and you have devices attached to
> it of the specific type?
This part of the series is only a a set of common functions and bindings
that I plan to reuse for futur STM32 SoCs.
The 'real' bus implementation is in patch 4.

Benjamin
>
>
> greg k-h

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

* Re: [PATCH v3 2/5] bus: stm32: Introduce firewall controller helpers
  2020-05-05 15:00     ` Benjamin GAIGNARD
@ 2020-05-05 18:20       ` Greg KH
  0 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2020-05-05 18:20 UTC (permalink / raw)
  To: Benjamin GAIGNARD
  Cc: robh+dt, mcoquelin.stm32, Alexandre TORGUE, Loic PALLARDY,
	linus.walleij, devicetree, linux-stm32, linux-arm-kernel,
	linux-kernel

On Tue, May 05, 2020 at 03:00:53PM +0000, Benjamin GAIGNARD wrote:
> 
> 
> On 5/5/20 4:40 PM, Greg KH wrote:
> > On Tue, May 05, 2020 at 09:33:05AM +0200, Benjamin Gaignard wrote:
> >> The goal of these helpers are to offer an interface for the
> >> hardware blocks controlling bus accesses rights.
> >>
> >> Bus firewall controllers are typically used to control if a
> >> hardware block can perform read or write operations on bus.
> >>
> >> Smarter firewall controllers could be able to define accesses
> >> rights per hardware blocks to control where they can read
> >> or write.
> >>
> >> Firewall controller configurations are provided in device node,
> >> parsed by the helpers and send to the driver to apply them.
> >> Each controller may need different number and type of inputs
> >> to configure the firewall so device-tree properties size have to
> >> be define by using "#firewall-cells".
> >> Firewall configurations properties have to be named "firewall-X"
> >> on device node.
> >> "firewall-names" keyword can also be used to give a name to
> >> a specific configuration.
> >>
> >> Example of device-tree:
> >> ctrl0: firewall@0 {
> >> 	#firewall-cells = <2>;
> >>        };
> >>
> >> foo: foo@0 {
> >> 	firewall-names = "default", "setting1";
> >> 	firewall-0 = <&ctrl0 1 2>;
> >> 	firewall-1 = <&ctrl0 3 4>;
> >> };
> >>
> >> Configurations could be applied with functions like
> >> firewall_set_config_by_index() or firewall_set_config_by_name().
> >>
> >> firewall_set_default_config() function will apply the
> >> configuration named "default" (if existing) or the configuration
> >> with index 0 (i.e. firewall-0).
> >>
> >> Drivers could register/unregister themselves be calling
> >> firewall_register/firewall_unregister functions.
> >>
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> ---
> >>   drivers/bus/Kconfig          |   2 +
> >>   drivers/bus/Makefile         |   2 +
> >>   drivers/bus/stm32/Kconfig    |   3 +
> >>   drivers/bus/stm32/Makefile   |   1 +
> >>   drivers/bus/stm32/firewall.c | 266 +++++++++++++++++++++++++++++++++++++++++++
> >>   drivers/bus/stm32/firewall.h |  75 ++++++++++++
> >>   6 files changed, 349 insertions(+)
> >>   create mode 100644 drivers/bus/stm32/Kconfig
> >>   create mode 100644 drivers/bus/stm32/Makefile
> >>   create mode 100644 drivers/bus/stm32/firewall.c
> >>   create mode 100644 drivers/bus/stm32/firewall.h
> >>
> >> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> >> index 6d4e4497b59b..843b356322d9 100644
> >> --- a/drivers/bus/Kconfig
> >> +++ b/drivers/bus/Kconfig
> >> @@ -203,4 +203,6 @@ config DA8XX_MSTPRI
> >>   source "drivers/bus/fsl-mc/Kconfig"
> >>   source "drivers/bus/mhi/Kconfig"
> >>   
> >> +source "drivers/bus/stm32/Kconfig"
> >> +
> >>   endmenu
> >> diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
> >> index 05f32cd694a4..5e0e34b10235 100644
> >> --- a/drivers/bus/Makefile
> >> +++ b/drivers/bus/Makefile
> >> @@ -37,3 +37,5 @@ obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
> >>   
> >>   # MHI
> >>   obj-$(CONFIG_MHI_BUS)		+= mhi/
> >> +
> >> +obj-$(CONFIG_MACH_STM32MP157) 	+= stm32/
> >> \ No newline at end of file
> >> diff --git a/drivers/bus/stm32/Kconfig b/drivers/bus/stm32/Kconfig
> >> new file mode 100644
> >> index 000000000000..57221e833e2d
> >> --- /dev/null
> >> +++ b/drivers/bus/stm32/Kconfig
> >> @@ -0,0 +1,3 @@
> >> +config FIREWALL_CONTROLLERS
> >> +	bool "Support of bus firewall controllers"
> >> +	depends on OF
> >> diff --git a/drivers/bus/stm32/Makefile b/drivers/bus/stm32/Makefile
> >> new file mode 100644
> >> index 000000000000..eb6b978d6450
> >> --- /dev/null
> >> +++ b/drivers/bus/stm32/Makefile
> >> @@ -0,0 +1 @@
> >> +obj-$(CONFIG_FIREWALL_CONTROLLERS) += firewall.o
> >> diff --git a/drivers/bus/stm32/firewall.c b/drivers/bus/stm32/firewall.c
> >> new file mode 100644
> >> index 000000000000..95f716cf926f
> >> --- /dev/null
> >> +++ b/drivers/bus/stm32/firewall.c
> >> @@ -0,0 +1,266 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
> >> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
> >> + */
> >> +
> >> +#include <linux/device.h>
> >> +#include <linux/err.h>
> >> +#include <linux/init.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/list.h>
> >> +#include <linux/of.h>
> >> +#include <linux/slab.h>
> >> +
> >> +#include "firewall.h"
> >> +
> >> +/* Mutex taken to protect firewall_list */
> >> +static DEFINE_MUTEX(firewall_list_mutex);
> >> +
> >> +/* Global list of firewall control devices */
> >> +static LIST_HEAD(firewall_list);
> > Why is that needed?  Why can't you just walk the list of devices on this
> > "bus/class" if you really wanted to?
> >
> > Along those lines, why is this going around the driver model and
> > ignoring it?  Shouldn't this be a bus and you have devices attached to
> > it of the specific type?
> This part of the series is only a a set of common functions and bindings
> that I plan to reuse for futur STM32 SoCs.
> The 'real' bus implementation is in patch 4.

Then you don't need a "fake" list of devices in this patch, do you?  Why
not just create the real bus and then have people use it, otherwise this
sequence of review is really complicated as you must be deleting this
code.  Right?  :)

Do it correct the first time please.

thanks,

greg k-h

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

end of thread, other threads:[~2020-05-05 18:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-05  7:33 [PATCH v3 0/5] STM32 ETZPC bus controller Benjamin Gaignard
2020-05-05  7:33 ` [PATCH v3 1/5] dt-bindings: bus: Add firewall bindings Benjamin Gaignard
2020-05-05  7:33 ` [PATCH v3 2/5] bus: stm32: Introduce firewall controller helpers Benjamin Gaignard
2020-05-05 14:40   ` Greg KH
2020-05-05 15:00     ` Benjamin GAIGNARD
2020-05-05 18:20       ` Greg KH
2020-05-05  7:33 ` [PATCH v3 3/5] dt-bindings: bus: Add STM32 ETZPC firewall controller Benjamin Gaignard
2020-05-05  7:33 ` [PATCH v3 4/5] bus: stm32: Add stm32 ETZPC firewall bus controller Benjamin Gaignard
2020-05-05  7:33 ` [PATCH v3 5/5] ARM: dts: stm32: Use ETZPC firewall bus Benjamin Gaignard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).