Add Amlogic secure monitor driver

From: Carlo Caione <[email protected]>

Introduce a driver to provide calls into secure monitor mode.

In the Amlogic SoCs these calls are used for multiple reasons: access to NVMEM, set USB boot, enable JTAG, etc...

Signed-off-by Carlo Caione <[email protected]>

drivers/firmware/meson/meson_sm.c new file +10 -0

+static int meson_sm_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct meson_sm_firmware *fw;
+	u32 sm_phy_in_base, sm_phy_out_base;
+	int cmd_in, cmd_out;
+
+	fw = devm_kzalloc(&pdev->dev, sizeof(*fw), GFP_KERNEL);
+	if (!fw)
+		return -ENOMEM;

drivers/firmware/meson/meson_sm.c line 131

Quoted 7 lines

drivers/firmware/meson/meson_sm.c new file +7 -0

+	if (of_property_read_u32(np, "amlogic,sm-cmd-input-base", &cmd_in))
+		return -EINVAL;
+
+	if (of_property_read_u32(np, "amlogic,sm-cmd-output-base", &cmd_out))
+		return -EINVAL;
+
+	sm_phy_in_base = meson_sm_call(cmd_in, 0, 0, 0, 0, 0);

Should there be any error checking here? Do we have any info on the return values here in case of error, or in case of missing firmware, etc.

From: Carlo Caione <[email protected]>

Add the binding documentation for the Amlogic secure monitor driver.

Signed-off-by Carlo Caione <[email protected]>

---
 .../bindings/firmware/meson/meson_sm.txt           | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt new file +1 -0

new file mode 100644
index 0000000..2b71716
@@ -0,0 +1,51 @@
+* Amlogic Secure Monitor

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt new file +24 -0

+
+Example:
+
+	#include <dt-bindings/firmware/meson-gxbb-sm.h>
+
+	...
+
+	firmware {
+		compatible = "simple-bus";
+
+		sm: secure-monitor {
+			compatible = "amlogic,meson-sm";
+			amlogic,sm-cmd-input-base = <SM_GET_SHARE_MEM_INPUT_BASE>;
+			amlogic,sm-cmd-output-base = <SM_GET_SHARE_MEM_OUTPUT_BASE>;
+		};
+	};
+
+Node of a device using the secure monitor must have a custom property to
+specify the SMC32 function the driver is going to use. The values of all the
+SMC function identifiers are listed in a SoC-specific header file:
+
+"include/dt-bindings/firmware/meson-gxbb-sm.h" - for the Meson GXBB secure monitor.
+
+Example of the node using the secure monitor:

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt new file +6 -0

+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		...
+	};
+

Signature · 6 lines

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

Minor nit, but please put the binding earlier in the series than the code implementing it, as per Documentation/devicetree/bindings/submitting-patches.txt

This makes it possible to review a series in a linear fashion.

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt line 11

Quoted 7 lines

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt new file +7 -0

+
+Required properties for the secure monitor node:
+- compatible: Should be "amlogic,meson-sm"
+- amlogic,sm-cmd-input-base: SMC32 function identifier to read the physical
+                             address of the input buffer
+- amlogic,sm-cmd-output-base: SMC32 function identifier to read the physical
+                              address of the output buffer

Do the IDs for these actually differ per board?

Are some functions simply not implemented on some boards?

Quoted 6 lines

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt new file +6 -0

+
+Example of the node using the secure monitor:
+
+	#include <dt-bindings/firmware/meson-gxbb-sm.h>
+
+	...

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt line 45

Quoted 6 lines

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt new file +6 -0

+
+	efuse: efuse {
+		compatible = "amlogic,meson-gxbb-efuse";
+		secure-monitor = <&sm>;
+		amlogic,sm-cmd-read-efuse = <SM_EFUSE_READ>;
+		amlogic,sm-cmd-max-efuse = <SM_EFUSE_USER_MAX>;

Likewise for these?

Quoted 8 lines · Mark Rutland

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt line 11

Quoted 6 lines

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt new file +6 -0

+Required properties for the secure monitor node:
+- compatible: Should be "amlogic,meson-sm"
+- amlogic,sm-cmd-input-base: SMC32 function identifier to read the physical
+                             address of the input buffer
+- amlogic,sm-cmd-output-base: SMC32 function identifier to read the physical
+                              address of the output buffer

Do the IDs for these actually differ per board?

I expect these to differ per SoC (GXBB in this case), not per board. The driver is generic enough to be (hopefully) used for several SoCs just changing the related header file that defines the SCM commands.

Quoted 1 line

Are some functions simply not implemented on some boards?

I don't think this is possible.

Quoted 14 lines · Carlo Caione

Quoted 8 lines · Mark Rutland

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt line 11

Quoted 6 lines

Documentation/devicetree/bindings/firmware/meson/meson_sm.txt new file +6 -0

+Required properties for the secure monitor node:
+- compatible: Should be "amlogic,meson-sm"
+- amlogic,sm-cmd-input-base: SMC32 function identifier to read the physical
+                             address of the input buffer
+- amlogic,sm-cmd-output-base: SMC32 function identifier to read the physical
+                              address of the output buffer

Do the IDs for these actually differ per board?

I expect these to differ per SoC (GXBB in this case), not per board. The driver is generic enough to be (hopefully) used for several SoCs just changing the related header file that defines the SCM commands.

Quoted 3 lines

Quoted 1 line

Are some functions simply not implemented on some boards?

I don't think this is possible.

Given that, I think it may be better to just have a "amlogic,meson-gxbb-sm" compatible string, and derive the set of functions and associated IDs from that.

That is, unless you know that future revisions have functions with the exact same semantics but differing IDs.

Regardless, it would make sense to have a GXBB-specific compatible string prepended to the list. That way in the future we can handle anything specific to the GXBB variant of the secure monitor if necessary.

From: Carlo Caione <[email protected]>

In the Amlogic SoCs the secure monitor calls are used for a lot of reasons:
interact with the NVMEM (efuses), check the reboot reason, set USB boot, ...

This driver defines a generic interface towards the secure monitor that can be used by more specialized drivers to interact with the secure monitor itself without worrying about bounce buffers managing.

Changelog:

v3:
* Moved driver to drivers/firmware
* Added EXPORT_SYMBOL macros
* Changed '_' in '-'
* Introduced 'struct meson_sm_firmware' and 'meson_sm_get_fw()' to be used by drivers using the secure-monitor to enforce probe ordering

v2:
* All the SMC function identifiers are now in a SoC-specific header file to be included by the DTS files so the SMC commands are now defined in the DT instead to be hardcoded into the driver
* Patchset is no longer an RFC
* Better error management and boundary checking
* s/unsigned int/u32/
* SMC call not only on CPU 0
* Fix memory leaking
* s/amlogic/meson/ in the directory names

Carlo Caione (4):
firmware: Amlogic: Add secure monitor driver
firmware: dt-bindings: Add secure monitor header file for GXBB
ARM64: dts: amlogic: gxbb: Enable secure monitor
documentation: Add secure monitor binding documentation

  .../bindings/firmware/meson/meson_sm.txt           |  51 ++++++
  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |  11 ++
  drivers/firmware/Kconfig                           |   1 +
  drivers/firmware/Makefile                          |   1 +
  drivers/firmware/meson/Kconfig                     |   8 +
  drivers/firmware/meson/Makefile                    |   1 +
  drivers/firmware/meson/meson_sm.c                  | 179 +++++++++++++++++++++
  include/dt-bindings/firmware/meson-gxbb-sm.h       |  44 +++++
  include/linux/firmware/meson/meson_sm.h            |  38 +++++
  9 files changed, 334 insertions(+)

create mode 100644 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt create mode 100644 drivers/firmware/meson/Kconfig
create mode 100644 drivers/firmware/meson/Makefile
create mode 100644 drivers/firmware/meson/meson_sm.c
create mode 100644 include/dt-bindings/firmware/meson-gxbb-sm.h create mode 100644 include/linux/firmware/meson/meson_sm.h