devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Rework secure-monitor driver
@ 2019-07-29 18:39 Carlo Caione
  2019-07-29 18:39 ` [PATCH 1/5] nvmem: meson-efuse: Move data to a container struct Carlo Caione
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Carlo Caione @ 2019-07-29 18:39 UTC (permalink / raw)
  To: srinivas.kandagatla, khilman, narmstrong, robh+dt, tglx, jbrunet,
	linux-arm-kernel, linux-amlogic, devicetree
  Cc: Carlo Caione

The secure-monitor driver is currently in really bad shape, not my 
proudest piece of code (thanks Jerome for pointing that out ;). I tried 
to rework it a bit to make it a bit more tolerable.

I needed to change a bit the APIs and consequently adapt the only user 
we have, that is the nvmem/efuses driver. To not break bisectability I 
added one single commit to change both the drivers.

The remaining commits are cosmetic and DTS/docs fixes.

Carlo Caione (5):
  nvmem: meson-efuse: Move data to a container struct
  firmware: meson_sm: Mark chip struct as static const
  nvmem: meson-efuse: bindings: Add secure-monitor phandle
  firmware: meson_sm: Rework driver as a proper platform driver
  arm64: dts: meson: Link nvmem and secure-monitor nodes

 .../bindings/nvmem/amlogic-efuse.txt          |  6 ++
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi    |  1 +
 arch/arm64/boot/dts/amlogic/meson-g12a.dtsi   |  1 +
 drivers/firmware/meson/meson_sm.c             | 96 +++++++++++++------
 drivers/nvmem/meson-efuse.c                   | 70 +++++++++-----
 include/linux/firmware/meson/meson_sm.h       | 15 +--
 6 files changed, 128 insertions(+), 61 deletions(-)

-- 
2.20.1

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

* [PATCH 1/5] nvmem: meson-efuse: Move data to a container struct
  2019-07-29 18:39 [PATCH 0/5] Rework secure-monitor driver Carlo Caione
@ 2019-07-29 18:39 ` Carlo Caione
  2019-07-30  9:04   ` Jerome Brunet
  2019-07-29 18:39 ` [PATCH 2/5] firmware: meson_sm: Mark chip struct as static const Carlo Caione
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Carlo Caione @ 2019-07-29 18:39 UTC (permalink / raw)
  To: srinivas.kandagatla, khilman, narmstrong, robh+dt, tglx, jbrunet,
	linux-arm-kernel, linux-amlogic, devicetree
  Cc: Carlo Caione

No functional changes, just a cleanup commit to tidy up a bit. Move the
nvmem_* and clk structures in a container struct.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 drivers/nvmem/meson-efuse.c | 47 ++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/nvmem/meson-efuse.c b/drivers/nvmem/meson-efuse.c
index 39bd76306033..9924b98db772 100644
--- a/drivers/nvmem/meson-efuse.c
+++ b/drivers/nvmem/meson-efuse.c
@@ -14,6 +14,12 @@
 
 #include <linux/firmware/meson/meson_sm.h>
 
+struct meson_efuse {
+	struct nvmem_device *nvmem;
+	struct nvmem_config config;
+	struct clk *clk;
+};
+
 static int meson_efuse_read(void *context, unsigned int offset,
 			    void *val, size_t bytes)
 {
@@ -37,21 +43,23 @@ MODULE_DEVICE_TABLE(of, meson_efuse_match);
 static int meson_efuse_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct nvmem_device *nvmem;
-	struct nvmem_config *econfig;
-	struct clk *clk;
+	struct meson_efuse *efuse;
 	unsigned int size;
 	int ret;
 
-	clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(clk)) {
-		ret = PTR_ERR(clk);
+	efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL);
+	if (!efuse)
+		return -ENOMEM;
+
+	efuse->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(efuse->clk)) {
+		ret = PTR_ERR(efuse->clk);
 		if (ret != -EPROBE_DEFER)
 			dev_err(dev, "failed to get efuse gate");
 		return ret;
 	}
 
-	ret = clk_prepare_enable(clk);
+	ret = clk_prepare_enable(efuse->clk);
 	if (ret) {
 		dev_err(dev, "failed to enable gate");
 		return ret;
@@ -59,7 +67,7 @@ static int meson_efuse_probe(struct platform_device *pdev)
 
 	ret = devm_add_action_or_reset(dev,
 				       (void(*)(void *))clk_disable_unprepare,
-				       clk);
+				       efuse->clk);
 	if (ret) {
 		dev_err(dev, "failed to add disable callback");
 		return ret;
@@ -70,21 +78,18 @@ static int meson_efuse_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
-	if (!econfig)
-		return -ENOMEM;
-
-	econfig->dev = dev;
-	econfig->name = dev_name(dev);
-	econfig->stride = 1;
-	econfig->word_size = 1;
-	econfig->reg_read = meson_efuse_read;
-	econfig->reg_write = meson_efuse_write;
-	econfig->size = size;
+	efuse->config.dev = dev;
+	efuse->config.name = dev_name(dev);
+	efuse->config.stride = 1;
+	efuse->config.word_size = 1;
+	efuse->config.reg_read = meson_efuse_read;
+	efuse->config.reg_write = meson_efuse_write;
+	efuse->config.size = size;
+	efuse->config.priv = efuse;
 
-	nvmem = devm_nvmem_register(&pdev->dev, econfig);
+	efuse->nvmem = devm_nvmem_register(&pdev->dev, &efuse->config);
 
-	return PTR_ERR_OR_ZERO(nvmem);
+	return PTR_ERR_OR_ZERO(efuse->nvmem);
 }
 
 static struct platform_driver meson_efuse_driver = {
-- 
2.20.1

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

* [PATCH 2/5] firmware: meson_sm: Mark chip struct as static const
  2019-07-29 18:39 [PATCH 0/5] Rework secure-monitor driver Carlo Caione
  2019-07-29 18:39 ` [PATCH 1/5] nvmem: meson-efuse: Move data to a container struct Carlo Caione
@ 2019-07-29 18:39 ` Carlo Caione
  2019-07-30  9:05   ` Jerome Brunet
  2019-07-29 18:39 ` [PATCH 3/5] nvmem: meson-efuse: bindings: Add secure-monitor phandle Carlo Caione
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Carlo Caione @ 2019-07-29 18:39 UTC (permalink / raw)
  To: srinivas.kandagatla, khilman, narmstrong, robh+dt, tglx, jbrunet,
	linux-arm-kernel, linux-amlogic, devicetree
  Cc: Carlo Caione

No need to be a global struct.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 drivers/firmware/meson/meson_sm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index 8d908a8e0d20..772ca6726e7b 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -35,7 +35,7 @@ struct meson_sm_chip {
 	struct meson_sm_cmd cmd[];
 };
 
-struct meson_sm_chip gxbb_chip = {
+static const struct meson_sm_chip gxbb_chip = {
 	.shmem_size		= SZ_4K,
 	.cmd_shmem_in_base	= 0x82000020,
 	.cmd_shmem_out_base	= 0x82000021,
-- 
2.20.1

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

* [PATCH 3/5] nvmem: meson-efuse: bindings: Add secure-monitor phandle
  2019-07-29 18:39 [PATCH 0/5] Rework secure-monitor driver Carlo Caione
  2019-07-29 18:39 ` [PATCH 1/5] nvmem: meson-efuse: Move data to a container struct Carlo Caione
  2019-07-29 18:39 ` [PATCH 2/5] firmware: meson_sm: Mark chip struct as static const Carlo Caione
@ 2019-07-29 18:39 ` Carlo Caione
  2019-07-29 18:39 ` [PATCH 4/5] firmware: meson_sm: Rework driver as a proper platform driver Carlo Caione
  2019-07-29 18:39 ` [PATCH 5/5] arm64: dts: meson: Link nvmem and secure-monitor nodes Carlo Caione
  4 siblings, 0 replies; 12+ messages in thread
From: Carlo Caione @ 2019-07-29 18:39 UTC (permalink / raw)
  To: srinivas.kandagatla, khilman, narmstrong, robh+dt, tglx, jbrunet,
	linux-arm-kernel, linux-amlogic, devicetree
  Cc: Carlo Caione

Add a new property to link the nvmem driver to the secure-monitor. The
nvmem driver needs to access the secure-monitor to be able to access the
fuses.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt b/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
index 2e0723ab3384..f7b3ed74db54 100644
--- a/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
+++ b/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
@@ -4,6 +4,7 @@ Required properties:
 - compatible: should be "amlogic,meson-gxbb-efuse"
 - clocks: phandle to the efuse peripheral clock provided by the
 	  clock controller.
+- secure-monitor: phandle to the secure-monitor node
 
 = Data cells =
 Are child nodes of eFuse, bindings of which as described in
@@ -16,6 +17,7 @@ Example:
 		clocks = <&clkc CLKID_EFUSE>;
 		#address-cells = <1>;
 		#size-cells = <1>;
+		secure-monitor = <&sm>;
 
 		sn: sn@14 {
 			reg = <0x14 0x10>;
@@ -30,6 +32,10 @@ Example:
 		};
 	};
 
+	sm: secure-monitor {
+		compatible = "amlogic,meson-gxbb-sm";
+	};
+
 = Data consumers =
 Are device nodes which consume nvmem data cells.
 
-- 
2.20.1

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

* [PATCH 4/5] firmware: meson_sm: Rework driver as a proper platform driver
  2019-07-29 18:39 [PATCH 0/5] Rework secure-monitor driver Carlo Caione
                   ` (2 preceding siblings ...)
  2019-07-29 18:39 ` [PATCH 3/5] nvmem: meson-efuse: bindings: Add secure-monitor phandle Carlo Caione
@ 2019-07-29 18:39 ` Carlo Caione
  2019-07-30  9:18   ` Jerome Brunet
  2019-07-29 18:39 ` [PATCH 5/5] arm64: dts: meson: Link nvmem and secure-monitor nodes Carlo Caione
  4 siblings, 1 reply; 12+ messages in thread
From: Carlo Caione @ 2019-07-29 18:39 UTC (permalink / raw)
  To: srinivas.kandagatla, khilman, narmstrong, robh+dt, tglx, jbrunet,
	linux-arm-kernel, linux-amlogic, devicetree
  Cc: Carlo Caione

The secure monitor driver is currently a frankenstein driver which is
registered as a platform driver but its functionality goes through a
global struct accessed by the consumer drivers using exported helper
functions.

Try to tidy up the driver moving the firmware struct into the driver
data and make the consumer drivers referencing the secure-monitor using
a new property in the DT.

Currently only the nvmem driver is using this API so we can fix it in
the same commit.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 drivers/firmware/meson/meson_sm.c       | 94 +++++++++++++++++--------
 drivers/nvmem/meson-efuse.c             | 23 +++++-
 include/linux/firmware/meson/meson_sm.h | 15 ++--
 3 files changed, 93 insertions(+), 39 deletions(-)

diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index 772ca6726e7b..2e36a2aa274c 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -54,8 +54,6 @@ struct meson_sm_firmware {
 	void __iomem *sm_shmem_out_base;
 };
 
-static struct meson_sm_firmware fw;
-
 static u32 meson_sm_get_cmd(const struct meson_sm_chip *chip,
 			    unsigned int cmd_index)
 {
@@ -90,6 +88,7 @@ static void __iomem *meson_sm_map_shmem(u32 cmd_shmem, unsigned int size)
 /**
  * meson_sm_call - generic SMC32 call to the secure-monitor
  *
+ * @fw:		Pointer to secure-monitor firmware
  * @cmd_index:	Index of the SMC32 function ID
  * @ret:	Returned value
  * @arg0:	SMC32 Argument 0
@@ -100,15 +99,15 @@ static void __iomem *meson_sm_map_shmem(u32 cmd_shmem, unsigned int size)
  *
  * Return:	0 on success, a negative value on error
  */
-int meson_sm_call(unsigned int cmd_index, u32 *ret, u32 arg0,
-		  u32 arg1, u32 arg2, u32 arg3, u32 arg4)
+int meson_sm_call(struct meson_sm_firmware *fw, unsigned int cmd_index,
+		  u32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4)
 {
 	u32 cmd, lret;
 
-	if (!fw.chip)
+	if (!fw->chip)
 		return -ENOENT;
 
-	cmd = meson_sm_get_cmd(fw.chip, cmd_index);
+	cmd = meson_sm_get_cmd(fw->chip, cmd_index);
 	if (!cmd)
 		return -EINVAL;
 
@@ -124,6 +123,7 @@ EXPORT_SYMBOL(meson_sm_call);
 /**
  * meson_sm_call_read - retrieve data from secure-monitor
  *
+ * @fw:		Pointer to secure-monitor firmware
  * @buffer:	Buffer to store the retrieved data
  * @bsize:	Size of the buffer
  * @cmd_index:	Index of the SMC32 function ID
@@ -137,22 +137,23 @@ EXPORT_SYMBOL(meson_sm_call);
  *		When 0 is returned there is no guarantee about the amount of
  *		data read and bsize bytes are copied in buffer.
  */
-int meson_sm_call_read(void *buffer, unsigned int bsize, unsigned int cmd_index,
-		       u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4)
+int meson_sm_call_read(struct meson_sm_firmware *fw, void *buffer,
+		       unsigned int bsize, unsigned int cmd_index, u32 arg0,
+		       u32 arg1, u32 arg2, u32 arg3, u32 arg4)
 {
 	u32 size;
 	int ret;
 
-	if (!fw.chip)
+	if (!fw->chip)
 		return -ENOENT;
 
-	if (!fw.chip->cmd_shmem_out_base)
+	if (!fw->chip->cmd_shmem_out_base)
 		return -EINVAL;
 
-	if (bsize > fw.chip->shmem_size)
+	if (bsize > fw->chip->shmem_size)
 		return -EINVAL;
 
-	if (meson_sm_call(cmd_index, &size, arg0, arg1, arg2, arg3, arg4) < 0)
+	if (meson_sm_call(fw, cmd_index, &size, arg0, arg1, arg2, arg3, arg4) < 0)
 		return -EINVAL;
 
 	if (size > bsize)
@@ -164,7 +165,7 @@ int meson_sm_call_read(void *buffer, unsigned int bsize, unsigned int cmd_index,
 		size = bsize;
 
 	if (buffer)
-		memcpy(buffer, fw.sm_shmem_out_base, size);
+		memcpy(buffer, fw->sm_shmem_out_base, size);
 
 	return ret;
 }
@@ -173,6 +174,7 @@ EXPORT_SYMBOL(meson_sm_call_read);
 /**
  * meson_sm_call_write - send data to secure-monitor
  *
+ * @fw:		Pointer to secure-monitor firmware
  * @buffer:	Buffer containing data to send
  * @size:	Size of the data to send
  * @cmd_index:	Index of the SMC32 function ID
@@ -184,23 +186,24 @@ EXPORT_SYMBOL(meson_sm_call_read);
  *
  * Return:	size of sent data on success, a negative value on error
  */
-int meson_sm_call_write(void *buffer, unsigned int size, unsigned int cmd_index,
-			u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4)
+int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer,
+			unsigned int size, unsigned int cmd_index, u32 arg0,
+			u32 arg1, u32 arg2, u32 arg3, u32 arg4)
 {
 	u32 written;
 
-	if (!fw.chip)
+	if (!fw->chip)
 		return -ENOENT;
 
-	if (size > fw.chip->shmem_size)
+	if (size > fw->chip->shmem_size)
 		return -EINVAL;
 
-	if (!fw.chip->cmd_shmem_in_base)
+	if (!fw->chip->cmd_shmem_in_base)
 		return -EINVAL;
 
-	memcpy(fw.sm_shmem_in_base, buffer, size);
+	memcpy(fw->sm_shmem_in_base, buffer, size);
 
-	if (meson_sm_call(cmd_index, &written, arg0, arg1, arg2, arg3, arg4) < 0)
+	if (meson_sm_call(fw, cmd_index, &written, arg0, arg1, arg2, arg3, arg4) < 0)
 		return -EINVAL;
 
 	if (!written)
@@ -210,6 +213,24 @@ int meson_sm_call_write(void *buffer, unsigned int size, unsigned int cmd_index,
 }
 EXPORT_SYMBOL(meson_sm_call_write);
 
+/**
+ * meson_sm_get - get pointer to meson_sm_firmware structure.
+ *
+ * @sm_node:		Pointer to the secure-monitor Device Tree node.
+ *
+ * Return:		NULL is the secure-monitor device is not ready.
+ */
+struct meson_sm_firmware *meson_sm_get(struct device_node *sm_node)
+{
+	struct platform_device *pdev = of_find_device_by_node(sm_node);
+
+	if (!pdev)
+		return NULL;
+
+	return platform_get_drvdata(pdev);
+}
+EXPORT_SYMBOL_GPL(meson_sm_get);
+
 #define SM_CHIP_ID_LENGTH	119
 #define SM_CHIP_ID_OFFSET	4
 #define SM_CHIP_ID_SIZE		12
@@ -217,14 +238,18 @@ EXPORT_SYMBOL(meson_sm_call_write);
 static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
+	struct platform_device *pdev = to_platform_device(dev);
+	struct meson_sm_firmware *fw;
 	uint8_t *id_buf;
 	int ret;
 
+	fw = platform_get_drvdata(pdev);
+
 	id_buf = kmalloc(SM_CHIP_ID_LENGTH, GFP_KERNEL);
 	if (!id_buf)
 		return -ENOMEM;
 
-	ret = meson_sm_call_read(id_buf, SM_CHIP_ID_LENGTH, SM_GET_CHIP_ID,
+	ret = meson_sm_call_read(fw, id_buf, SM_CHIP_ID_LENGTH, SM_GET_CHIP_ID,
 				 0, 0, 0, 0, 0);
 	if (ret < 0) {
 		kfree(id_buf);
@@ -268,25 +293,34 @@ static const struct of_device_id meson_sm_ids[] = {
 
 static int __init meson_sm_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	const struct meson_sm_chip *chip;
+	struct meson_sm_firmware *fw;
+
+	fw = devm_kzalloc(dev, sizeof(*fw), GFP_KERNEL);
+	if (!fw)
+		return -ENOMEM;
 
-	chip = of_match_device(meson_sm_ids, &pdev->dev)->data;
+	chip = of_match_device(meson_sm_ids, dev)->data;
 
 	if (chip->cmd_shmem_in_base) {
-		fw.sm_shmem_in_base = meson_sm_map_shmem(chip->cmd_shmem_in_base,
-							 chip->shmem_size);
-		if (WARN_ON(!fw.sm_shmem_in_base))
+		fw->sm_shmem_in_base = meson_sm_map_shmem(chip->cmd_shmem_in_base,
+							  chip->shmem_size);
+		if (WARN_ON(!fw->sm_shmem_in_base))
 			goto out;
 	}
 
 	if (chip->cmd_shmem_out_base) {
-		fw.sm_shmem_out_base = meson_sm_map_shmem(chip->cmd_shmem_out_base,
-							  chip->shmem_size);
-		if (WARN_ON(!fw.sm_shmem_out_base))
+		fw->sm_shmem_out_base = meson_sm_map_shmem(chip->cmd_shmem_out_base,
+							   chip->shmem_size);
+		if (WARN_ON(!fw->sm_shmem_out_base))
 			goto out_in_base;
 	}
 
-	fw.chip = chip;
+	fw->chip = chip;
+
+	platform_set_drvdata(pdev, fw);
+
 	pr_info("secure-monitor enabled\n");
 
 	if (sysfs_create_group(&pdev->dev.kobj, &meson_sm_sysfs_attr_group))
@@ -295,7 +329,7 @@ static int __init meson_sm_probe(struct platform_device *pdev)
 	return 0;
 
 out_in_base:
-	iounmap(fw.sm_shmem_in_base);
+	iounmap(fw->sm_shmem_in_base);
 out:
 	return -EINVAL;
 }
diff --git a/drivers/nvmem/meson-efuse.c b/drivers/nvmem/meson-efuse.c
index 9924b98db772..669d20d73877 100644
--- a/drivers/nvmem/meson-efuse.c
+++ b/drivers/nvmem/meson-efuse.c
@@ -17,20 +17,25 @@
 struct meson_efuse {
 	struct nvmem_device *nvmem;
 	struct nvmem_config config;
+	struct meson_sm_firmware *fw;
 	struct clk *clk;
 };
 
 static int meson_efuse_read(void *context, unsigned int offset,
 			    void *val, size_t bytes)
 {
-	return meson_sm_call_read((u8 *)val, bytes, SM_EFUSE_READ, offset,
+	struct meson_efuse *efuse = context;
+
+	return meson_sm_call_read(efuse->fw, (u8 *)val, bytes, SM_EFUSE_READ, offset,
 				  bytes, 0, 0, 0);
 }
 
 static int meson_efuse_write(void *context, unsigned int offset,
 			     void *val, size_t bytes)
 {
-	return meson_sm_call_write((u8 *)val, bytes, SM_EFUSE_WRITE, offset,
+	struct meson_efuse *efuse = context;
+
+	return meson_sm_call_write(efuse->fw, (u8 *)val, bytes, SM_EFUSE_WRITE, offset,
 				   bytes, 0, 0, 0);
 }
 
@@ -43,6 +48,7 @@ MODULE_DEVICE_TABLE(of, meson_efuse_match);
 static int meson_efuse_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *sm_np;
 	struct meson_efuse *efuse;
 	unsigned int size;
 	int ret;
@@ -51,6 +57,17 @@ static int meson_efuse_probe(struct platform_device *pdev)
 	if (!efuse)
 		return -ENOMEM;
 
+	sm_np = of_parse_phandle(pdev->dev.of_node, "secure-monitor", 0);
+	if (!sm_np) {
+		dev_err(&pdev->dev, "no secure-monitor node\n");
+		return -ENODEV;
+	}
+
+	efuse->fw = meson_sm_get(sm_np);
+	of_node_put(sm_np);
+	if (!efuse->fw)
+		return -EPROBE_DEFER;
+
 	efuse->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(efuse->clk)) {
 		ret = PTR_ERR(efuse->clk);
@@ -73,7 +90,7 @@ static int meson_efuse_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (meson_sm_call(SM_EFUSE_USER_MAX, &size, 0, 0, 0, 0, 0) < 0) {
+	if (meson_sm_call(efuse->fw, SM_EFUSE_USER_MAX, &size, 0, 0, 0, 0, 0) < 0) {
 		dev_err(dev, "failed to get max user");
 		return -EINVAL;
 	}
diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
index 7613bf7c9442..6669e2a1d5fd 100644
--- a/include/linux/firmware/meson/meson_sm.h
+++ b/include/linux/firmware/meson/meson_sm.h
@@ -16,11 +16,14 @@ enum {
 
 struct meson_sm_firmware;
 
-int meson_sm_call(unsigned int cmd_index, u32 *ret, u32 arg0, u32 arg1,
-		  u32 arg2, u32 arg3, u32 arg4);
-int meson_sm_call_write(void *buffer, unsigned int b_size, unsigned int cmd_index,
-			u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4);
-int meson_sm_call_read(void *buffer, unsigned int bsize, unsigned int cmd_index,
-		       u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4);
+int meson_sm_call(struct meson_sm_firmware *fw, unsigned int cmd_index,
+		  u32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4);
+int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer,
+			unsigned int b_size, unsigned int cmd_index, u32 arg0,
+			u32 arg1, u32 arg2, u32 arg3, u32 arg4);
+int meson_sm_call_read(struct meson_sm_firmware *fw, void *buffer,
+		       unsigned int bsize, unsigned int cmd_index, u32 arg0,
+		       u32 arg1, u32 arg2, u32 arg3, u32 arg4);
+struct meson_sm_firmware *meson_sm_get(struct device_node *firmware_node);
 
 #endif /* _MESON_SM_FW_H_ */
-- 
2.20.1

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

* [PATCH 5/5] arm64: dts: meson: Link nvmem and secure-monitor nodes
  2019-07-29 18:39 [PATCH 0/5] Rework secure-monitor driver Carlo Caione
                   ` (3 preceding siblings ...)
  2019-07-29 18:39 ` [PATCH 4/5] firmware: meson_sm: Rework driver as a proper platform driver Carlo Caione
@ 2019-07-29 18:39 ` Carlo Caione
  2019-07-30  9:23   ` Jerome Brunet
  4 siblings, 1 reply; 12+ messages in thread
From: Carlo Caione @ 2019-07-29 18:39 UTC (permalink / raw)
  To: srinivas.kandagatla, khilman, narmstrong, robh+dt, tglx, jbrunet,
	linux-arm-kernel, linux-amlogic, devicetree
  Cc: Carlo Caione

The former is going to use the latter to retrieve the efuses data.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi  | 1 +
 arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 6219337033a0..b8244efb85fa 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -117,6 +117,7 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		read-only;
+		secure-monitor = <&sm>;
 	};
 
 	psci {
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
index f8d43e3dcf20..2b07752e034f 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
@@ -100,6 +100,7 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		read-only;
+		secure-monitor = <&sm>;
 	};
 
 	psci {
-- 
2.20.1

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

* Re: [PATCH 1/5] nvmem: meson-efuse: Move data to a container struct
  2019-07-29 18:39 ` [PATCH 1/5] nvmem: meson-efuse: Move data to a container struct Carlo Caione
@ 2019-07-30  9:04   ` Jerome Brunet
  2019-07-30  9:29     ` Carlo Caione
  0 siblings, 1 reply; 12+ messages in thread
From: Jerome Brunet @ 2019-07-30  9:04 UTC (permalink / raw)
  To: srinivas.kandagatla, khilman, narmstrong, robh+dt, tglx,
	linux-arm-kernel, linux-amlogic, devicetree
  Cc: Carlo Caione

On Mon 29 Jul 2019 at 19:39, Carlo Caione <ccaione@baylibre.com> wrote:

> No functional changes, just a cleanup commit to tidy up a bit. Move the
> nvmem_* and clk structures in a container struct.
>
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
> ---
>  drivers/nvmem/meson-efuse.c | 47 ++++++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/nvmem/meson-efuse.c b/drivers/nvmem/meson-efuse.c
> index 39bd76306033..9924b98db772 100644
> --- a/drivers/nvmem/meson-efuse.c
> +++ b/drivers/nvmem/meson-efuse.c
> @@ -14,6 +14,12 @@
>  
>  #include <linux/firmware/meson/meson_sm.h>
>  
> +struct meson_efuse {
> +	struct nvmem_device *nvmem;
> +	struct nvmem_config config;
> +	struct clk *clk;

since this driver uses devm_add_action_or_reset, I don't think you need
to keep the clk pointer around, unless you plan to do something with it
later on ?

It is kind of the same the other structure members, do we need to keep
them around ? We could just let devm deal with it ?

If you have a plan, could you share it ?

> +};
> +
>  static int meson_efuse_read(void *context, unsigned int offset,
>  			    void *val, size_t bytes)
>  {
> @@ -37,21 +43,23 @@ MODULE_DEVICE_TABLE(of, meson_efuse_match);
>  static int meson_efuse_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct nvmem_device *nvmem;
> -	struct nvmem_config *econfig;
> -	struct clk *clk;
> +	struct meson_efuse *efuse;
>  	unsigned int size;
>  	int ret;
>  
> -	clk = devm_clk_get(dev, NULL);
> -	if (IS_ERR(clk)) {
> -		ret = PTR_ERR(clk);
> +	efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL);
> +	if (!efuse)
> +		return -ENOMEM;
> +
> +	efuse->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(efuse->clk)) {
> +		ret = PTR_ERR(efuse->clk);
>  		if (ret != -EPROBE_DEFER)
>  			dev_err(dev, "failed to get efuse gate");
>  		return ret;
>  	}
>  
> -	ret = clk_prepare_enable(clk);
> +	ret = clk_prepare_enable(efuse->clk);
>  	if (ret) {
>  		dev_err(dev, "failed to enable gate");
>  		return ret;
> @@ -59,7 +67,7 @@ static int meson_efuse_probe(struct platform_device *pdev)
>  
>  	ret = devm_add_action_or_reset(dev,
>  				       (void(*)(void *))clk_disable_unprepare,
> -				       clk);
> +				       efuse->clk);
>  	if (ret) {
>  		dev_err(dev, "failed to add disable callback");
>  		return ret;
> @@ -70,21 +78,18 @@ static int meson_efuse_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
> -	if (!econfig)
> -		return -ENOMEM;
> -
> -	econfig->dev = dev;
> -	econfig->name = dev_name(dev);
> -	econfig->stride = 1;
> -	econfig->word_size = 1;
> -	econfig->reg_read = meson_efuse_read;
> -	econfig->reg_write = meson_efuse_write;
> -	econfig->size = size;
> +	efuse->config.dev = dev;
> +	efuse->config.name = dev_name(dev);
> +	efuse->config.stride = 1;
> +	efuse->config.word_size = 1;
> +	efuse->config.reg_read = meson_efuse_read;
> +	efuse->config.reg_write = meson_efuse_write;
> +	efuse->config.size = size;
> +	efuse->config.priv = efuse;
>  
> -	nvmem = devm_nvmem_register(&pdev->dev, econfig);
> +	efuse->nvmem = devm_nvmem_register(&pdev->dev, &efuse->config);
>  
> -	return PTR_ERR_OR_ZERO(nvmem);
> +	return PTR_ERR_OR_ZERO(efuse->nvmem);
>  }
>  
>  static struct platform_driver meson_efuse_driver = {
> -- 
> 2.20.1

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

* Re: [PATCH 2/5] firmware: meson_sm: Mark chip struct as static const
  2019-07-29 18:39 ` [PATCH 2/5] firmware: meson_sm: Mark chip struct as static const Carlo Caione
@ 2019-07-30  9:05   ` Jerome Brunet
  0 siblings, 0 replies; 12+ messages in thread
From: Jerome Brunet @ 2019-07-30  9:05 UTC (permalink / raw)
  To: srinivas.kandagatla, khilman, narmstrong, robh+dt, tglx,
	linux-arm-kernel, linux-amlogic, devicetree
  Cc: Carlo Caione

On Mon 29 Jul 2019 at 19:39, Carlo Caione <ccaione@baylibre.com> wrote:

> No need to be a global struct.
>
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>

Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>

> ---
>  drivers/firmware/meson/meson_sm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
> index 8d908a8e0d20..772ca6726e7b 100644
> --- a/drivers/firmware/meson/meson_sm.c
> +++ b/drivers/firmware/meson/meson_sm.c
> @@ -35,7 +35,7 @@ struct meson_sm_chip {
>  	struct meson_sm_cmd cmd[];
>  };
>  
> -struct meson_sm_chip gxbb_chip = {
> +static const struct meson_sm_chip gxbb_chip = {
>  	.shmem_size		= SZ_4K,
>  	.cmd_shmem_in_base	= 0x82000020,
>  	.cmd_shmem_out_base	= 0x82000021,
> -- 
> 2.20.1

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

* Re: [PATCH 4/5] firmware: meson_sm: Rework driver as a proper platform driver
  2019-07-29 18:39 ` [PATCH 4/5] firmware: meson_sm: Rework driver as a proper platform driver Carlo Caione
@ 2019-07-30  9:18   ` Jerome Brunet
  0 siblings, 0 replies; 12+ messages in thread
From: Jerome Brunet @ 2019-07-30  9:18 UTC (permalink / raw)
  To: srinivas.kandagatla, khilman, narmstrong, robh+dt, tglx,
	linux-arm-kernel, linux-amlogic, devicetree
  Cc: Carlo Caione

On Mon 29 Jul 2019 at 19:39, Carlo Caione <ccaione@baylibre.com> wrote:

> The secure monitor driver is currently a frankenstein driver which is
> registered as a platform driver but its functionality goes through a
> global struct accessed by the consumer drivers using exported helper
> functions.
>
> Try to tidy up the driver moving the firmware struct into the driver
> data and make the consumer drivers referencing the secure-monitor using
> a new property in the DT.
>
> Currently only the nvmem driver is using this API so we can fix it in
> the same commit.

Indeed, I don't have another idea to deal with it without breaking bisect

Thanks for this !

Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>

>
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
> ---
>  drivers/firmware/meson/meson_sm.c       | 94 +++++++++++++++++--------
>  drivers/nvmem/meson-efuse.c             | 23 +++++-
>  include/linux/firmware/meson/meson_sm.h | 15 ++--
>  3 files changed, 93 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
> index 772ca6726e7b..2e36a2aa274c 100644
> --- a/drivers/firmware/meson/meson_sm.c
> +++ b/drivers/firmware/meson/meson_sm.c
> @@ -54,8 +54,6 @@ struct meson_sm_firmware {
>  	void __iomem *sm_shmem_out_base;
>  };
>  
> -static struct meson_sm_firmware fw;
> -
>  static u32 meson_sm_get_cmd(const struct meson_sm_chip *chip,
>  			    unsigned int cmd_index)
>  {
> @@ -90,6 +88,7 @@ static void __iomem *meson_sm_map_shmem(u32 cmd_shmem, unsigned int size)
>  /**
>   * meson_sm_call - generic SMC32 call to the secure-monitor
>   *
> + * @fw:		Pointer to secure-monitor firmware
>   * @cmd_index:	Index of the SMC32 function ID
>   * @ret:	Returned value
>   * @arg0:	SMC32 Argument 0
> @@ -100,15 +99,15 @@ static void __iomem *meson_sm_map_shmem(u32 cmd_shmem, unsigned int size)
>   *
>   * Return:	0 on success, a negative value on error
>   */
> -int meson_sm_call(unsigned int cmd_index, u32 *ret, u32 arg0,
> -		  u32 arg1, u32 arg2, u32 arg3, u32 arg4)
> +int meson_sm_call(struct meson_sm_firmware *fw, unsigned int cmd_index,
> +		  u32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4)
>  {
>  	u32 cmd, lret;
>  
> -	if (!fw.chip)
> +	if (!fw->chip)
>  		return -ENOENT;
>  
> -	cmd = meson_sm_get_cmd(fw.chip, cmd_index);
> +	cmd = meson_sm_get_cmd(fw->chip, cmd_index);
>  	if (!cmd)
>  		return -EINVAL;
>  
> @@ -124,6 +123,7 @@ EXPORT_SYMBOL(meson_sm_call);
>  /**
>   * meson_sm_call_read - retrieve data from secure-monitor
>   *
> + * @fw:		Pointer to secure-monitor firmware
>   * @buffer:	Buffer to store the retrieved data
>   * @bsize:	Size of the buffer
>   * @cmd_index:	Index of the SMC32 function ID
> @@ -137,22 +137,23 @@ EXPORT_SYMBOL(meson_sm_call);
>   *		When 0 is returned there is no guarantee about the amount of
>   *		data read and bsize bytes are copied in buffer.
>   */
> -int meson_sm_call_read(void *buffer, unsigned int bsize, unsigned int cmd_index,
> -		       u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4)
> +int meson_sm_call_read(struct meson_sm_firmware *fw, void *buffer,
> +		       unsigned int bsize, unsigned int cmd_index, u32 arg0,
> +		       u32 arg1, u32 arg2, u32 arg3, u32 arg4)
>  {
>  	u32 size;
>  	int ret;
>  
> -	if (!fw.chip)
> +	if (!fw->chip)
>  		return -ENOENT;
>  
> -	if (!fw.chip->cmd_shmem_out_base)
> +	if (!fw->chip->cmd_shmem_out_base)
>  		return -EINVAL;
>  
> -	if (bsize > fw.chip->shmem_size)
> +	if (bsize > fw->chip->shmem_size)
>  		return -EINVAL;
>  
> -	if (meson_sm_call(cmd_index, &size, arg0, arg1, arg2, arg3, arg4) < 0)
> +	if (meson_sm_call(fw, cmd_index, &size, arg0, arg1, arg2, arg3, arg4) < 0)
>  		return -EINVAL;
>  
>  	if (size > bsize)
> @@ -164,7 +165,7 @@ int meson_sm_call_read(void *buffer, unsigned int bsize, unsigned int cmd_index,
>  		size = bsize;
>  
>  	if (buffer)
> -		memcpy(buffer, fw.sm_shmem_out_base, size);
> +		memcpy(buffer, fw->sm_shmem_out_base, size);
>  
>  	return ret;
>  }
> @@ -173,6 +174,7 @@ EXPORT_SYMBOL(meson_sm_call_read);
>  /**
>   * meson_sm_call_write - send data to secure-monitor
>   *
> + * @fw:		Pointer to secure-monitor firmware
>   * @buffer:	Buffer containing data to send
>   * @size:	Size of the data to send
>   * @cmd_index:	Index of the SMC32 function ID
> @@ -184,23 +186,24 @@ EXPORT_SYMBOL(meson_sm_call_read);
>   *
>   * Return:	size of sent data on success, a negative value on error
>   */
> -int meson_sm_call_write(void *buffer, unsigned int size, unsigned int cmd_index,
> -			u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4)
> +int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer,
> +			unsigned int size, unsigned int cmd_index, u32 arg0,
> +			u32 arg1, u32 arg2, u32 arg3, u32 arg4)
>  {
>  	u32 written;
>  
> -	if (!fw.chip)
> +	if (!fw->chip)
>  		return -ENOENT;
>  
> -	if (size > fw.chip->shmem_size)
> +	if (size > fw->chip->shmem_size)
>  		return -EINVAL;
>  
> -	if (!fw.chip->cmd_shmem_in_base)
> +	if (!fw->chip->cmd_shmem_in_base)
>  		return -EINVAL;
>  
> -	memcpy(fw.sm_shmem_in_base, buffer, size);
> +	memcpy(fw->sm_shmem_in_base, buffer, size);
>  
> -	if (meson_sm_call(cmd_index, &written, arg0, arg1, arg2, arg3, arg4) < 0)
> +	if (meson_sm_call(fw, cmd_index, &written, arg0, arg1, arg2, arg3, arg4) < 0)
>  		return -EINVAL;
>  
>  	if (!written)
> @@ -210,6 +213,24 @@ int meson_sm_call_write(void *buffer, unsigned int size, unsigned int cmd_index,
>  }
>  EXPORT_SYMBOL(meson_sm_call_write);
>  
> +/**
> + * meson_sm_get - get pointer to meson_sm_firmware structure.
> + *
> + * @sm_node:		Pointer to the secure-monitor Device Tree node.
> + *
> + * Return:		NULL is the secure-monitor device is not ready.
> + */
> +struct meson_sm_firmware *meson_sm_get(struct device_node *sm_node)
> +{
> +	struct platform_device *pdev = of_find_device_by_node(sm_node);
> +
> +	if (!pdev)
> +		return NULL;
> +
> +	return platform_get_drvdata(pdev);
> +}
> +EXPORT_SYMBOL_GPL(meson_sm_get);
> +
>  #define SM_CHIP_ID_LENGTH	119
>  #define SM_CHIP_ID_OFFSET	4
>  #define SM_CHIP_ID_SIZE		12
> @@ -217,14 +238,18 @@ EXPORT_SYMBOL(meson_sm_call_write);
>  static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
>  			 char *buf)
>  {
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct meson_sm_firmware *fw;
>  	uint8_t *id_buf;
>  	int ret;
>  
> +	fw = platform_get_drvdata(pdev);
> +
>  	id_buf = kmalloc(SM_CHIP_ID_LENGTH, GFP_KERNEL);
>  	if (!id_buf)
>  		return -ENOMEM;
>  
> -	ret = meson_sm_call_read(id_buf, SM_CHIP_ID_LENGTH, SM_GET_CHIP_ID,
> +	ret = meson_sm_call_read(fw, id_buf, SM_CHIP_ID_LENGTH, SM_GET_CHIP_ID,
>  				 0, 0, 0, 0, 0);
>  	if (ret < 0) {
>  		kfree(id_buf);
> @@ -268,25 +293,34 @@ static const struct of_device_id meson_sm_ids[] = {
>  
>  static int __init meson_sm_probe(struct platform_device *pdev)
>  {
> +	struct device *dev = &pdev->dev;
>  	const struct meson_sm_chip *chip;
> +	struct meson_sm_firmware *fw;
> +
> +	fw = devm_kzalloc(dev, sizeof(*fw), GFP_KERNEL);
> +	if (!fw)
> +		return -ENOMEM;
>  
> -	chip = of_match_device(meson_sm_ids, &pdev->dev)->data;
> +	chip = of_match_device(meson_sm_ids, dev)->data;
>  
>  	if (chip->cmd_shmem_in_base) {
> -		fw.sm_shmem_in_base = meson_sm_map_shmem(chip->cmd_shmem_in_base,
> -							 chip->shmem_size);
> -		if (WARN_ON(!fw.sm_shmem_in_base))
> +		fw->sm_shmem_in_base = meson_sm_map_shmem(chip->cmd_shmem_in_base,
> +							  chip->shmem_size);
> +		if (WARN_ON(!fw->sm_shmem_in_base))
>  			goto out;
>  	}
>  
>  	if (chip->cmd_shmem_out_base) {
> -		fw.sm_shmem_out_base = meson_sm_map_shmem(chip->cmd_shmem_out_base,
> -							  chip->shmem_size);
> -		if (WARN_ON(!fw.sm_shmem_out_base))
> +		fw->sm_shmem_out_base = meson_sm_map_shmem(chip->cmd_shmem_out_base,
> +							   chip->shmem_size);
> +		if (WARN_ON(!fw->sm_shmem_out_base))
>  			goto out_in_base;
>  	}
>  
> -	fw.chip = chip;
> +	fw->chip = chip;
> +
> +	platform_set_drvdata(pdev, fw);
> +
>  	pr_info("secure-monitor enabled\n");
>  
>  	if (sysfs_create_group(&pdev->dev.kobj, &meson_sm_sysfs_attr_group))
> @@ -295,7 +329,7 @@ static int __init meson_sm_probe(struct platform_device *pdev)
>  	return 0;
>  
>  out_in_base:
> -	iounmap(fw.sm_shmem_in_base);
> +	iounmap(fw->sm_shmem_in_base);
>  out:
>  	return -EINVAL;
>  }
> diff --git a/drivers/nvmem/meson-efuse.c b/drivers/nvmem/meson-efuse.c
> index 9924b98db772..669d20d73877 100644
> --- a/drivers/nvmem/meson-efuse.c
> +++ b/drivers/nvmem/meson-efuse.c
> @@ -17,20 +17,25 @@
>  struct meson_efuse {
>  	struct nvmem_device *nvmem;
>  	struct nvmem_config config;
> +	struct meson_sm_firmware *fw;
>  	struct clk *clk;
>  };
>  
>  static int meson_efuse_read(void *context, unsigned int offset,
>  			    void *val, size_t bytes)
>  {
> -	return meson_sm_call_read((u8 *)val, bytes, SM_EFUSE_READ, offset,
> +	struct meson_efuse *efuse = context;
> +
> +	return meson_sm_call_read(efuse->fw, (u8 *)val, bytes, SM_EFUSE_READ, offset,
>  				  bytes, 0, 0, 0);
>  }
>  
>  static int meson_efuse_write(void *context, unsigned int offset,
>  			     void *val, size_t bytes)
>  {
> -	return meson_sm_call_write((u8 *)val, bytes, SM_EFUSE_WRITE, offset,
> +	struct meson_efuse *efuse = context;
> +
> +	return meson_sm_call_write(efuse->fw, (u8 *)val, bytes, SM_EFUSE_WRITE, offset,
>  				   bytes, 0, 0, 0);
>  }
>  
> @@ -43,6 +48,7 @@ MODULE_DEVICE_TABLE(of, meson_efuse_match);
>  static int meson_efuse_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	struct device_node *sm_np;
>  	struct meson_efuse *efuse;
>  	unsigned int size;
>  	int ret;
> @@ -51,6 +57,17 @@ static int meson_efuse_probe(struct platform_device *pdev)
>  	if (!efuse)
>  		return -ENOMEM;
>  
> +	sm_np = of_parse_phandle(pdev->dev.of_node, "secure-monitor", 0);
> +	if (!sm_np) {
> +		dev_err(&pdev->dev, "no secure-monitor node\n");
> +		return -ENODEV;
> +	}
> +
> +	efuse->fw = meson_sm_get(sm_np);
> +	of_node_put(sm_np);
> +	if (!efuse->fw)
> +		return -EPROBE_DEFER;
> +
>  	efuse->clk = devm_clk_get(dev, NULL);
>  	if (IS_ERR(efuse->clk)) {
>  		ret = PTR_ERR(efuse->clk);
> @@ -73,7 +90,7 @@ static int meson_efuse_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	if (meson_sm_call(SM_EFUSE_USER_MAX, &size, 0, 0, 0, 0, 0) < 0) {
> +	if (meson_sm_call(efuse->fw, SM_EFUSE_USER_MAX, &size, 0, 0, 0, 0, 0) < 0) {
>  		dev_err(dev, "failed to get max user");
>  		return -EINVAL;
>  	}
> diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
> index 7613bf7c9442..6669e2a1d5fd 100644
> --- a/include/linux/firmware/meson/meson_sm.h
> +++ b/include/linux/firmware/meson/meson_sm.h
> @@ -16,11 +16,14 @@ enum {
>  
>  struct meson_sm_firmware;
>  
> -int meson_sm_call(unsigned int cmd_index, u32 *ret, u32 arg0, u32 arg1,
> -		  u32 arg2, u32 arg3, u32 arg4);
> -int meson_sm_call_write(void *buffer, unsigned int b_size, unsigned int cmd_index,
> -			u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> -int meson_sm_call_read(void *buffer, unsigned int bsize, unsigned int cmd_index,
> -		       u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> +int meson_sm_call(struct meson_sm_firmware *fw, unsigned int cmd_index,
> +		  u32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> +int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer,
> +			unsigned int b_size, unsigned int cmd_index, u32 arg0,
> +			u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> +int meson_sm_call_read(struct meson_sm_firmware *fw, void *buffer,
> +		       unsigned int bsize, unsigned int cmd_index, u32 arg0,
> +		       u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> +struct meson_sm_firmware *meson_sm_get(struct device_node *firmware_node);
>  
>  #endif /* _MESON_SM_FW_H_ */
> -- 
> 2.20.1

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

* Re: [PATCH 5/5] arm64: dts: meson: Link nvmem and secure-monitor nodes
  2019-07-29 18:39 ` [PATCH 5/5] arm64: dts: meson: Link nvmem and secure-monitor nodes Carlo Caione
@ 2019-07-30  9:23   ` Jerome Brunet
  2019-07-30  9:36     ` Carlo Caione
  0 siblings, 1 reply; 12+ messages in thread
From: Jerome Brunet @ 2019-07-30  9:23 UTC (permalink / raw)
  To: srinivas.kandagatla, khilman, narmstrong, robh+dt, tglx,
	linux-arm-kernel, linux-amlogic, devicetree
  Cc: Carlo Caione

On Mon 29 Jul 2019 at 19:39, Carlo Caione <ccaione@baylibre.com> wrote:

> The former is going to use the latter to retrieve the efuses data.

Actually, if you really want to not break bisect, this change must be
merged before the driver change (patch 4).

I'm a bit surpised to see only the axg and g12a here ?
Doesn't it apply to gxbb/gxl as well ?

>
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi  | 1 +
>  arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> index 6219337033a0..b8244efb85fa 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> @@ -117,6 +117,7 @@
>  		#address-cells = <1>;
>  		#size-cells = <1>;
>  		read-only;
> +		secure-monitor = <&sm>;
>  	};
>  
>  	psci {
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
> index f8d43e3dcf20..2b07752e034f 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
> @@ -100,6 +100,7 @@
>  		#address-cells = <1>;
>  		#size-cells = <1>;
>  		read-only;
> +		secure-monitor = <&sm>;
>  	};
>  
>  	psci {
> -- 
> 2.20.1

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

* Re: [PATCH 1/5] nvmem: meson-efuse: Move data to a container struct
  2019-07-30  9:04   ` Jerome Brunet
@ 2019-07-30  9:29     ` Carlo Caione
  0 siblings, 0 replies; 12+ messages in thread
From: Carlo Caione @ 2019-07-30  9:29 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: devicetree, narmstrong, khilman, robh+dt, srinivas.kandagatla,
	linux-amlogic, tglx, linux-arm-kernel

On 30/07/2019 10:04, Jerome Brunet wrote:
> On Mon 29 Jul 2019 at 19:39, Carlo Caione <ccaione@baylibre.com> wrote:

/cut
>> +struct meson_efuse {
>> +	struct nvmem_device *nvmem;
>> +	struct nvmem_config config;
>> +	struct clk *clk;
> 
> since this driver uses devm_add_action_or_reset, I don't think you need
> to keep the clk pointer around, unless you plan to do something with it
> later on ?

Good point about the clk pointer.

> It is kind of the same the other structure members, do we need to keep
> them around ? We could just let devm deal with it ?
> 
> If you have a plan, could you share it ?

No plan. In the PATCH 4/5 I was going to add struct meson_sm_firmware to 
the bundle and to me it's a bit more elegant to have all the auxiliary 
structs together in a single container.

For the sake of keeping the diff size low I'm going to rework this in V2.

--
Carlo Caione

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

* Re: [PATCH 5/5] arm64: dts: meson: Link nvmem and secure-monitor nodes
  2019-07-30  9:23   ` Jerome Brunet
@ 2019-07-30  9:36     ` Carlo Caione
  0 siblings, 0 replies; 12+ messages in thread
From: Carlo Caione @ 2019-07-30  9:36 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: devicetree, narmstrong, khilman, robh+dt, srinivas.kandagatla,
	linux-amlogic, tglx, linux-arm-kernel

On 30/07/2019 10:23, Jerome Brunet wrote:
> On Mon 29 Jul 2019 at 19:39, Carlo Caione <ccaione@baylibre.com> wrote:
> 
>> The former is going to use the latter to retrieve the efuses data.
> 
> Actually, if you really want to not break bisect, this change must be
> merged before the driver change (patch 4).
> 
> I'm a bit surpised to see only the axg and g12a here ?
> Doesn't it apply to gxbb/gxl as well ?

Ah, it does indeed. Fix coming, thanks.

--
Carlo Caione

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

end of thread, other threads:[~2019-07-30  9:36 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-29 18:39 [PATCH 0/5] Rework secure-monitor driver Carlo Caione
2019-07-29 18:39 ` [PATCH 1/5] nvmem: meson-efuse: Move data to a container struct Carlo Caione
2019-07-30  9:04   ` Jerome Brunet
2019-07-30  9:29     ` Carlo Caione
2019-07-29 18:39 ` [PATCH 2/5] firmware: meson_sm: Mark chip struct as static const Carlo Caione
2019-07-30  9:05   ` Jerome Brunet
2019-07-29 18:39 ` [PATCH 3/5] nvmem: meson-efuse: bindings: Add secure-monitor phandle Carlo Caione
2019-07-29 18:39 ` [PATCH 4/5] firmware: meson_sm: Rework driver as a proper platform driver Carlo Caione
2019-07-30  9:18   ` Jerome Brunet
2019-07-29 18:39 ` [PATCH 5/5] arm64: dts: meson: Link nvmem and secure-monitor nodes Carlo Caione
2019-07-30  9:23   ` Jerome Brunet
2019-07-30  9:36     ` Carlo Caione

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).