[PATCH v3 0/6] hwrng: exynos: Add support for Exynos850

[PATCH v3 0/6] hwrng: exynos: Add support for Exynos850

[Sam Protsenko]

Exynos850 has True Random Number Generator (TRNG) block which is very
similar to Exynos5250 for which the driver already exists
(exynos-trng.c). There are two differences though:
  1. Additional SSS PCLK clock has to be enabled to make TRNG registers
     accessible.
  2. All SSS registers (including TRNG area) are protected with
     TrustZone and can only be accessed from EL3 monitor. So the
     corresponding SMC calls have to be used instead to interact with
     TRNG block.

This patch series enables TRNG support on Exynos850 SoC. It was tested
on the E850-96 board running Debian rootfs like this:

    8<-------------------------------------------------------------->8
    # cat /sys/devices/virtual/misc/hw_random/rng_current
    12081400.rng

    # dd if=/dev/hwrng bs=100000 count=1 > /dev/null
    ...
    122KB/s

    # apt install rng-tools5
    # rngtest -c 1000 < /dev/hwrng
    ...
    rngtest: starting FIPS tests...
    rngtest: bits received from input: 20000032
    rngtest: FIPS 140-2 successes: 1000
    rngtest: FIPS 140-2 failures: 0
    rngtest: FIPS 140-2(2001-10-10) Monobit: 0
    rngtest: FIPS 140-2(2001-10-10) Poker: 0
    rngtest: FIPS 140-2(2001-10-10) Runs: 0
    rngtest: FIPS 140-2(2001-10-10) Long run: 0
    rngtest: FIPS 140-2(2001-10-10) Continuous run: 0
    rngtest: input channel speed: (min=941.855; avg=965.515;
             max=968.236)Kibits/s
    rngtest: FIPS tests speed: (min=49.542; avg=52.886;
             max=53.577)Mibits/s
    rngtest: Program run time: 20590194 microseconds
    8<-------------------------------------------------------------->8

SMC commands added in this series require LDFW (Loadable Firmware) to be
loaded by the bootloader. In case of E850-96 board, at the moment only
the LittleKernel based bootloader [1] is able to load LDFW. It is
expected to be added into U-Boot port soon as well. See [2] for more
details.

[1] https://gitlab.com/Linaro/96boards/e850-96/lk
[2] https://docs.u-boot.org/en/latest/board/samsung/e850-96.html

Changes in v3:
  - Rebased on top of the most recent linux-next
  - Removed dts patch (7/7) from the series, as suggested by Krzysztof
  - Addressed all review comments for v2 series

Changes in v2:
  - Removed the patch for renaming the dt-bindings doc file
  - Added the patch for using devm_clk_get_enabled() to get the clock
  - Addressed all review comments for v1 series

Sam Protsenko (6):
  dt-bindings: rng: Add Exynos850 support to exynos-trng
  hwrng: exynos: Improve coding style
  hwrng: exynos: Use devm_clk_get_enabled() to get the clock
  hwrng: exynos: Implement bus clock control
  hwrng: exynos: Add SMC based TRNG operation
  hwrng: exynos: Enable Exynos850 support

 .../bindings/rng/samsung,exynos5250-trng.yaml |  40 +++-
 drivers/char/hw_random/exynos-trng.c          | 225 +++++++++++++-----
 2 files changed, 206 insertions(+), 59 deletions(-)

-- 
2.39.2

[PATCH v3 1/6] dt-bindings: rng: Add Exynos850 support to exynos-trng

[Sam Protsenko]

The TRNG block in Exynos850 is pretty much the same as in Exynos5250,
but there are two clocks that has to be controlled to make it work:
  1. Functional (operating) clock: called ACLK in Exynos850, the same as
     "secss" clock in Exynos5250
  2. Interface (bus) clock: called PCLK in Exynos850. It has to be
     enabled in order to access TRNG registers

Document Exynos850 compatible and the related clock changes.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v3:
  - Added R-b tag from Krzysztof

Changes in v2:
  - Removed example added in v1

 .../bindings/rng/samsung,exynos5250-trng.yaml | 40 +++++++++++++++++--
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.yaml b/Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.yaml
index 765d9f9edd6e..1a71935d8a19 100644
--- a/Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.yaml
+++ b/Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.yaml
@@ -12,14 +12,17 @@ maintainers:
 
 properties:
   compatible:
-    const: samsung,exynos5250-trng
+    enum:
+      - samsung,exynos5250-trng
+      - samsung,exynos850-trng
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   clock-names:
-    items:
-      - const: secss
+    minItems: 1
+    maxItems: 2
 
   reg:
     maxItems: 1
@@ -30,6 +33,35 @@ required:
   - clock-names
   - reg
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: samsung,exynos850-trng
+
+    then:
+      properties:
+        clocks:
+          items:
+            - description: SSS (Security Sub System) operating clock
+            - description: SSS (Security Sub System) bus clock
+
+        clock-names:
+          items:
+            - const: secss
+            - const: pclk
+
+    else:
+      properties:
+        clocks:
+          items:
+            - description: SSS (Security Sub System) operating clock
+
+        clock-names:
+          items:
+            - const: secss
+
 additionalProperties: false
 
 examples:
-- 
2.39.2

[PATCH v3 2/6] hwrng: exynos: Improve coding style

[Sam Protsenko]

Fix obvious style issues. Some of those were found with checkpatch, and
some just contradict the kernel coding style guide.

No functional change.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Łukasz Stelmach <l.stelmach@samsung.com>
---
Changes in v3:
  - Added '\n' in dev_err("clock divider too large")
  - Added A-b tag from Łukasz

Changes in v2:
  - Added Krzysztof's R-b tag

 drivers/char/hw_random/exynos-trng.c | 63 +++++++++++++---------------
 1 file changed, 30 insertions(+), 33 deletions(-)

diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
index 0ed5d22fe667..266bdad84f3c 100644
--- a/drivers/char/hw_random/exynos-trng.c
+++ b/drivers/char/hw_random/exynos-trng.c
@@ -23,45 +23,41 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 
-#define EXYNOS_TRNG_CLKDIV         (0x0)
-
-#define EXYNOS_TRNG_CTRL           (0x20)
-#define EXYNOS_TRNG_CTRL_RNGEN     BIT(31)
-
-#define EXYNOS_TRNG_POST_CTRL      (0x30)
-#define EXYNOS_TRNG_ONLINE_CTRL    (0x40)
-#define EXYNOS_TRNG_ONLINE_STAT    (0x44)
-#define EXYNOS_TRNG_ONLINE_MAXCHI2 (0x48)
-#define EXYNOS_TRNG_FIFO_CTRL      (0x50)
-#define EXYNOS_TRNG_FIFO_0         (0x80)
-#define EXYNOS_TRNG_FIFO_1         (0x84)
-#define EXYNOS_TRNG_FIFO_2         (0x88)
-#define EXYNOS_TRNG_FIFO_3         (0x8c)
-#define EXYNOS_TRNG_FIFO_4         (0x90)
-#define EXYNOS_TRNG_FIFO_5         (0x94)
-#define EXYNOS_TRNG_FIFO_6         (0x98)
-#define EXYNOS_TRNG_FIFO_7         (0x9c)
-#define EXYNOS_TRNG_FIFO_LEN       (8)
-#define EXYNOS_TRNG_CLOCK_RATE     (500000)
-
+#define EXYNOS_TRNG_CLKDIV		0x0
+
+#define EXYNOS_TRNG_CTRL		0x20
+#define EXYNOS_TRNG_CTRL_RNGEN		BIT(31)
+
+#define EXYNOS_TRNG_POST_CTRL		0x30
+#define EXYNOS_TRNG_ONLINE_CTRL		0x40
+#define EXYNOS_TRNG_ONLINE_STAT		0x44
+#define EXYNOS_TRNG_ONLINE_MAXCHI2	0x48
+#define EXYNOS_TRNG_FIFO_CTRL		0x50
+#define EXYNOS_TRNG_FIFO_0		0x80
+#define EXYNOS_TRNG_FIFO_1		0x84
+#define EXYNOS_TRNG_FIFO_2		0x88
+#define EXYNOS_TRNG_FIFO_3		0x8c
+#define EXYNOS_TRNG_FIFO_4		0x90
+#define EXYNOS_TRNG_FIFO_5		0x94
+#define EXYNOS_TRNG_FIFO_6		0x98
+#define EXYNOS_TRNG_FIFO_7		0x9c
+#define EXYNOS_TRNG_FIFO_LEN		8
+#define EXYNOS_TRNG_CLOCK_RATE		500000
 
 struct exynos_trng_dev {
-	struct device    *dev;
-	void __iomem     *mem;
-	struct clk       *clk;
-	struct hwrng rng;
+	struct device	*dev;
+	void __iomem	*mem;
+	struct clk	*clk;
+	struct hwrng	rng;
 };
 
 static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
 			       bool wait)
 {
-	struct exynos_trng_dev *trng;
+	struct exynos_trng_dev *trng = (struct exynos_trng_dev *)rng->priv;
 	int val;
 
 	max = min_t(size_t, max, (EXYNOS_TRNG_FIFO_LEN * 4));
-
-	trng = (struct exynos_trng_dev *)rng->priv;
-
 	writel_relaxed(max * 8, trng->mem + EXYNOS_TRNG_FIFO_CTRL);
 	val = readl_poll_timeout(trng->mem + EXYNOS_TRNG_FIFO_CTRL, val,
 				 val == 0, 200, 1000000);
@@ -87,7 +83,7 @@ static int exynos_trng_init(struct hwrng *rng)
 	 */
 	val = sss_rate / (EXYNOS_TRNG_CLOCK_RATE * 2);
 	if (val > 0x7fff) {
-		dev_err(trng->dev, "clock divider too large: %d", val);
+		dev_err(trng->dev, "clock divider too large: %d\n", val);
 		return -ERANGE;
 	}
 	val = val << 1;
@@ -122,7 +118,7 @@ static int exynos_trng_probe(struct platform_device *pdev)
 
 	trng->rng.init = exynos_trng_init;
 	trng->rng.read = exynos_trng_do_read;
-	trng->rng.priv = (unsigned long) trng;
+	trng->rng.priv = (unsigned long)trng;
 
 	platform_set_drvdata(pdev, trng);
 	trng->dev = &pdev->dev;
@@ -175,7 +171,7 @@ static int exynos_trng_probe(struct platform_device *pdev)
 
 static void exynos_trng_remove(struct platform_device *pdev)
 {
-	struct exynos_trng_dev *trng =  platform_get_drvdata(pdev);
+	struct exynos_trng_dev *trng = platform_get_drvdata(pdev);
 
 	clk_disable_unprepare(trng->clk);
 
@@ -204,7 +200,7 @@ static int exynos_trng_resume(struct device *dev)
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(exynos_trng_pm_ops, exynos_trng_suspend,
-			 exynos_trng_resume);
+				exynos_trng_resume);
 
 static const struct of_device_id exynos_trng_dt_match[] = {
 	{
@@ -225,6 +221,7 @@ static struct platform_driver exynos_trng_driver = {
 };
 
 module_platform_driver(exynos_trng_driver);
+
 MODULE_AUTHOR("Łukasz Stelmach");
 MODULE_DESCRIPTION("H/W TRNG driver for Exynos chips");
 MODULE_LICENSE("GPL v2");
-- 
2.39.2

[PATCH v3 3/6] hwrng: exynos: Use devm_clk_get_enabled() to get the clock

[Sam Protsenko]

Use devm_clk_get_enabled() helper instead of calling devm_clk_get() and
then clk_prepare_enable(). It simplifies the error handling and makes
the code more compact. Also use dev_err_probe() to handle possible
-EPROBE_DEFER errors if the clock is not available yet.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Anand Moon <linux.amoon@gmail.com>
---
Changes in v3:
  - Added missing '\n' in dev_err_probe()
  - Added R-b tag from Krzysztof
  - Added R-b tag from Anand

Changes in v2:
  - No changes (it's a new patch added in v2)

 drivers/char/hw_random/exynos-trng.c | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
index 266bdad84f3c..997bd22f4498 100644
--- a/drivers/char/hw_random/exynos-trng.c
+++ b/drivers/char/hw_random/exynos-trng.c
@@ -134,32 +134,23 @@ static int exynos_trng_probe(struct platform_device *pdev)
 		goto err_pm_get;
 	}
 
-	trng->clk = devm_clk_get(&pdev->dev, "secss");
+	trng->clk = devm_clk_get_enabled(&pdev->dev, "secss");
 	if (IS_ERR(trng->clk)) {
-		ret = PTR_ERR(trng->clk);
-		dev_err(&pdev->dev, "Could not get clock.\n");
-		goto err_clock;
-	}
-
-	ret = clk_prepare_enable(trng->clk);
-	if (ret) {
-		dev_err(&pdev->dev, "Could not enable the clk.\n");
+		ret = dev_err_probe(&pdev->dev, PTR_ERR(trng->clk),
+				    "Could not get clock\n");
 		goto err_clock;
 	}
 
 	ret = devm_hwrng_register(&pdev->dev, &trng->rng);
 	if (ret) {
 		dev_err(&pdev->dev, "Could not register hwrng device.\n");
-		goto err_register;
+		goto err_clock;
 	}
 
 	dev_info(&pdev->dev, "Exynos True Random Number Generator.\n");
 
 	return 0;
 
-err_register:
-	clk_disable_unprepare(trng->clk);
-
 err_clock:
 	pm_runtime_put_noidle(&pdev->dev);
 
@@ -171,10 +162,6 @@ static int exynos_trng_probe(struct platform_device *pdev)
 
 static void exynos_trng_remove(struct platform_device *pdev)
 {
-	struct exynos_trng_dev *trng = platform_get_drvdata(pdev);
-
-	clk_disable_unprepare(trng->clk);
-
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 }
-- 
2.39.2

[PATCH v3 4/6] hwrng: exynos: Implement bus clock control

[Sam Protsenko]

Some SoCs like Exynos850 might require the SSS bus clock (PCLK) to be
enabled in order to access TRNG registers. Add and handle the optional
PCLK clock accordingly to make it possible.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Anand Moon <linux.amoon@gmail.com>
---
Changes in v3:
  - Added missing '\n' in dev_err_probe()
  - Added R-b tag from Krzysztof
  - Added R-b tag from Anand

Changes in v2:
  - Used devm_clk_get_optional_enabled() to avoid calling
    clk_prepare_enable() for PCLK

 drivers/char/hw_random/exynos-trng.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
index 997bd22f4498..6ef2ee6c9804 100644
--- a/drivers/char/hw_random/exynos-trng.c
+++ b/drivers/char/hw_random/exynos-trng.c
@@ -47,7 +47,8 @@
 struct exynos_trng_dev {
 	struct device	*dev;
 	void __iomem	*mem;
-	struct clk	*clk;
+	struct clk	*clk;	/* operating clock */
+	struct clk	*pclk;	/* bus clock */
 	struct hwrng	rng;
 };
 
@@ -141,6 +142,13 @@ static int exynos_trng_probe(struct platform_device *pdev)
 		goto err_clock;
 	}
 
+	trng->pclk = devm_clk_get_optional_enabled(&pdev->dev, "pclk");
+	if (IS_ERR(trng->pclk)) {
+		ret = dev_err_probe(&pdev->dev, PTR_ERR(trng->pclk),
+				    "Could not get pclk\n");
+		goto err_clock;
+	}
+
 	ret = devm_hwrng_register(&pdev->dev, &trng->rng);
 	if (ret) {
 		dev_err(&pdev->dev, "Could not register hwrng device.\n");
-- 
2.39.2

[PATCH v3 5/6] hwrng: exynos: Add SMC based TRNG operation

[Sam Protsenko]

On some Exynos chips like Exynos850 the access to Security Sub System
(SSS) registers is protected with TrustZone, and therefore only possible
from EL3 monitor software. The Linux kernel is running in EL1, so the
only way for the driver to obtain TRNG data is via SMC calls to EL3
monitor. Implement such SMC operation and use it when EXYNOS_SMC flag is
set in the corresponding chip driver data.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v3:
  - Added appropriate error messages for the case when init SMC command fails

Changes in v2:
  - Used the "reversed Christmas tree" style in the variable declaration
    block in exynos_trng_do_read_smc()
  - Renamed .quirks to .flags in the driver structure
  - Added Krzysztof's R-b tag

 drivers/char/hw_random/exynos-trng.c | 140 +++++++++++++++++++++++++--
 1 file changed, 130 insertions(+), 10 deletions(-)

diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
index 6ef2ee6c9804..9fa30583cc86 100644
--- a/drivers/char/hw_random/exynos-trng.c
+++ b/drivers/char/hw_random/exynos-trng.c
@@ -10,6 +10,7 @@
  * Krzysztof Kozłowski <krzk@kernel.org>
  */
 
+#include <linux/arm-smccc.h>
 #include <linux/clk.h>
 #include <linux/crypto.h>
 #include <linux/delay.h>
@@ -22,6 +23,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 
 #define EXYNOS_TRNG_CLKDIV		0x0
 
@@ -44,16 +46,41 @@
 #define EXYNOS_TRNG_FIFO_LEN		8
 #define EXYNOS_TRNG_CLOCK_RATE		500000
 
+/* Driver feature flags */
+#define EXYNOS_SMC			BIT(0)
+
+#define EXYNOS_SMC_CALL_VAL(func_num)			\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
+			   ARM_SMCCC_SMC_32,		\
+			   ARM_SMCCC_OWNER_SIP,		\
+			   func_num)
+
+/* SMC command for DTRNG access */
+#define SMC_CMD_RANDOM			EXYNOS_SMC_CALL_VAL(0x1012)
+
+/* SMC_CMD_RANDOM: arguments */
+#define HWRNG_INIT			0x0
+#define HWRNG_EXIT			0x1
+#define HWRNG_GET_DATA			0x2
+#define HWRNG_RESUME			0x3
+
+/* SMC_CMD_RANDOM: return values */
+#define HWRNG_RET_OK			0x0
+#define HWRNG_RET_RETRY_ERROR		0x2
+
+#define HWRNG_MAX_TRIES			100
+
 struct exynos_trng_dev {
 	struct device	*dev;
 	void __iomem	*mem;
 	struct clk	*clk;	/* operating clock */
 	struct clk	*pclk;	/* bus clock */
 	struct hwrng	rng;
+	unsigned long	flags;
 };
 
-static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
-			       bool wait)
+static int exynos_trng_do_read_reg(struct hwrng *rng, void *data, size_t max,
+				   bool wait)
 {
 	struct exynos_trng_dev *trng = (struct exynos_trng_dev *)rng->priv;
 	int val;
@@ -70,7 +97,40 @@ static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
 	return max;
 }
 
-static int exynos_trng_init(struct hwrng *rng)
+static int exynos_trng_do_read_smc(struct hwrng *rng, void *data, size_t max,
+				   bool wait)
+{
+	struct arm_smccc_res res;
+	unsigned int copied = 0;
+	u32 *buf = data;
+	int tries = 0;
+
+	while (copied < max) {
+		arm_smccc_smc(SMC_CMD_RANDOM, HWRNG_GET_DATA, 0, 0, 0, 0, 0, 0,
+			      &res);
+		switch (res.a0) {
+		case HWRNG_RET_OK:
+			*buf++ = res.a2;
+			*buf++ = res.a3;
+			copied += 8;
+			tries = 0;
+			break;
+		case HWRNG_RET_RETRY_ERROR:
+			if (!wait)
+				return copied;
+			if (++tries >= HWRNG_MAX_TRIES)
+				return copied;
+			cond_resched();
+			break;
+		default:
+			return -EIO;
+		}
+	}
+
+	return copied;
+}
+
+static int exynos_trng_init_reg(struct hwrng *rng)
 {
 	struct exynos_trng_dev *trng = (struct exynos_trng_dev *)rng->priv;
 	unsigned long sss_rate;
@@ -103,6 +163,24 @@ static int exynos_trng_init(struct hwrng *rng)
 	return 0;
 }
 
+static int exynos_trng_init_smc(struct hwrng *rng)
+{
+	struct exynos_trng_dev *trng = (struct exynos_trng_dev *)rng->priv;
+	struct arm_smccc_res res;
+	int ret = 0;
+
+	arm_smccc_smc(SMC_CMD_RANDOM, HWRNG_INIT, 0, 0, 0, 0, 0, 0, &res);
+	if (res.a0 != HWRNG_RET_OK) {
+		dev_err(trng->dev, "SMC command for TRNG init failed (%d)\n",
+			(int)res.a0);
+		ret = -EIO;
+	}
+	if ((int)res.a0 == -1)
+		dev_info(trng->dev, "Make sure LDFW is loaded by your BL\n");
+
+	return ret;
+}
+
 static int exynos_trng_probe(struct platform_device *pdev)
 {
 	struct exynos_trng_dev *trng;
@@ -112,21 +190,29 @@ static int exynos_trng_probe(struct platform_device *pdev)
 	if (!trng)
 		return ret;
 
+	platform_set_drvdata(pdev, trng);
+	trng->dev = &pdev->dev;
+
+	trng->flags = (unsigned long)device_get_match_data(&pdev->dev);
+
 	trng->rng.name = devm_kstrdup(&pdev->dev, dev_name(&pdev->dev),
 				      GFP_KERNEL);
 	if (!trng->rng.name)
 		return ret;
 
-	trng->rng.init = exynos_trng_init;
-	trng->rng.read = exynos_trng_do_read;
 	trng->rng.priv = (unsigned long)trng;
 
-	platform_set_drvdata(pdev, trng);
-	trng->dev = &pdev->dev;
+	if (trng->flags & EXYNOS_SMC) {
+		trng->rng.init = exynos_trng_init_smc;
+		trng->rng.read = exynos_trng_do_read_smc;
+	} else {
+		trng->rng.init = exynos_trng_init_reg;
+		trng->rng.read = exynos_trng_do_read_reg;
 
-	trng->mem = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(trng->mem))
-		return PTR_ERR(trng->mem);
+		trng->mem = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(trng->mem))
+			return PTR_ERR(trng->mem);
+	}
 
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_resume_and_get(&pdev->dev);
@@ -170,12 +256,31 @@ static int exynos_trng_probe(struct platform_device *pdev)
 
 static void exynos_trng_remove(struct platform_device *pdev)
 {
+	struct exynos_trng_dev *trng = platform_get_drvdata(pdev);
+
+	if (trng->flags & EXYNOS_SMC) {
+		struct arm_smccc_res res;
+
+		arm_smccc_smc(SMC_CMD_RANDOM, HWRNG_EXIT, 0, 0, 0, 0, 0, 0,
+			      &res);
+	}
+
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 }
 
 static int exynos_trng_suspend(struct device *dev)
 {
+	struct exynos_trng_dev *trng = dev_get_drvdata(dev);
+	struct arm_smccc_res res;
+
+	if (trng->flags & EXYNOS_SMC) {
+		arm_smccc_smc(SMC_CMD_RANDOM, HWRNG_EXIT, 0, 0, 0, 0, 0, 0,
+			      &res);
+		if (res.a0 != HWRNG_RET_OK)
+			return -EIO;
+	}
+
 	pm_runtime_put_sync(dev);
 
 	return 0;
@@ -183,6 +288,7 @@ static int exynos_trng_suspend(struct device *dev)
 
 static int exynos_trng_resume(struct device *dev)
 {
+	struct exynos_trng_dev *trng = dev_get_drvdata(dev);
 	int ret;
 
 	ret = pm_runtime_resume_and_get(dev);
@@ -191,6 +297,20 @@ static int exynos_trng_resume(struct device *dev)
 		return ret;
 	}
 
+	if (trng->flags & EXYNOS_SMC) {
+		struct arm_smccc_res res;
+
+		arm_smccc_smc(SMC_CMD_RANDOM, HWRNG_RESUME, 0, 0, 0, 0, 0, 0,
+			      &res);
+		if (res.a0 != HWRNG_RET_OK)
+			return -EIO;
+
+		arm_smccc_smc(SMC_CMD_RANDOM, HWRNG_INIT, 0, 0, 0, 0, 0, 0,
+			      &res);
+		if (res.a0 != HWRNG_RET_OK)
+			return -EIO;
+	}
+
 	return 0;
 }
 
-- 
2.39.2

Re: [PATCH v3 5/6] hwrng: exynos: Add SMC based TRNG operation

[Lukasz Stelmach]

[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]

It was <2024-06-20 czw 18:13>, when Sam Protsenko wrote:
> On some Exynos chips like Exynos850 the access to Security Sub System
> (SSS) registers is protected with TrustZone, and therefore only possible
> from EL3 monitor software. The Linux kernel is running in EL1, so the
> only way for the driver to obtain TRNG data is via SMC calls to EL3
> monitor. Implement such SMC operation and use it when EXYNOS_SMC flag is
> set in the corresponding chip driver data.
>
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> Changes in v3:
>   - Added appropriate error messages for the case when init SMC command fails
>
> Changes in v2:
>   - Used the "reversed Christmas tree" style in the variable declaration
>     block in exynos_trng_do_read_smc()
>   - Renamed .quirks to .flags in the driver structure
>   - Added Krzysztof's R-b tag
>
>  drivers/char/hw_random/exynos-trng.c | 140 +++++++++++++++++++++++++--
>  1 file changed, 130 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
> index 6ef2ee6c9804..9fa30583cc86 100644
> --- a/drivers/char/hw_random/exynos-trng.c
> +++ b/drivers/char/hw_random/exynos-trng.c

[...]


> @@ -103,6 +163,24 @@ static int exynos_trng_init(struct hwrng *rng)
>  	return 0;
>  }
>  
> +static int exynos_trng_init_smc(struct hwrng *rng)
> +{
> +	struct exynos_trng_dev *trng = (struct exynos_trng_dev *)rng->priv;
> +	struct arm_smccc_res res;
> +	int ret = 0;
> +
> +	arm_smccc_smc(SMC_CMD_RANDOM, HWRNG_INIT, 0, 0, 0, 0, 0, 0, &res);
> +	if (res.a0 != HWRNG_RET_OK) {
> +		dev_err(trng->dev, "SMC command for TRNG init failed (%d)\n",
> +			(int)res.a0);
> +		ret = -EIO;
> +	}
> +	if ((int)res.a0 == -1)
> +		dev_info(trng->dev, "Make sure LDFW is loaded by your BL\n");

This is good, thank you for adding it. It can be even better though, if
you don't skimp on message length (-; I mean, I know what BL is, I can
fingure what LDFW is because you have explained to me and I can see the
source code, but somewone who sees it for the first time will be only
slightly less surprised than with v2 error message only. Come on, you
can make this message twice as long and it will still fit in 80 characters (-;

Don't change it if v3 is the last. If not, please, make it more verbose.

> +
> +	return ret;
> +}
> +


[...]


Kind regards,
-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [PATCH v3 5/6] hwrng: exynos: Add SMC based TRNG operation

[Sam Protsenko]

On Fri, Jun 21, 2024 at 2:00 PM Lukasz Stelmach <l.stelmach@samsung.com> wrote:
>
> It was <2024-06-20 czw 18:13>, when Sam Protsenko wrote:
> > On some Exynos chips like Exynos850 the access to Security Sub System
> > (SSS) registers is protected with TrustZone, and therefore only possible
> > from EL3 monitor software. The Linux kernel is running in EL1, so the
> > only way for the driver to obtain TRNG data is via SMC calls to EL3
> > monitor. Implement such SMC operation and use it when EXYNOS_SMC flag is
> > set in the corresponding chip driver data.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > ---
> > Changes in v3:
> >   - Added appropriate error messages for the case when init SMC command fails
> >
> > Changes in v2:
> >   - Used the "reversed Christmas tree" style in the variable declaration
> >     block in exynos_trng_do_read_smc()
> >   - Renamed .quirks to .flags in the driver structure
> >   - Added Krzysztof's R-b tag
> >
> >  drivers/char/hw_random/exynos-trng.c | 140 +++++++++++++++++++++++++--
> >  1 file changed, 130 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
> > index 6ef2ee6c9804..9fa30583cc86 100644
> > --- a/drivers/char/hw_random/exynos-trng.c
> > +++ b/drivers/char/hw_random/exynos-trng.c
>
> [...]
>
>
> > @@ -103,6 +163,24 @@ static int exynos_trng_init(struct hwrng *rng)
> >       return 0;
> >  }
> >
> > +static int exynos_trng_init_smc(struct hwrng *rng)
> > +{
> > +     struct exynos_trng_dev *trng = (struct exynos_trng_dev *)rng->priv;
> > +     struct arm_smccc_res res;
> > +     int ret = 0;
> > +
> > +     arm_smccc_smc(SMC_CMD_RANDOM, HWRNG_INIT, 0, 0, 0, 0, 0, 0, &res);
> > +     if (res.a0 != HWRNG_RET_OK) {
> > +             dev_err(trng->dev, "SMC command for TRNG init failed (%d)\n",
> > +                     (int)res.a0);
> > +             ret = -EIO;
> > +     }
> > +     if ((int)res.a0 == -1)
> > +             dev_info(trng->dev, "Make sure LDFW is loaded by your BL\n");
>
> This is good, thank you for adding it. It can be even better though, if
> you don't skimp on message length (-; I mean, I know what BL is, I can
> fingure what LDFW is because you have explained to me and I can see the
> source code, but somewone who sees it for the first time will be only
> slightly less surprised than with v2 error message only. Come on, you
> can make this message twice as long and it will still fit in 80 characters (-;
>

Guess my OCD got in the way and I just didn't want to break the line
:) But yeah, LDFW = Loadable Firmware, and BL = bootloader. There is
an "ldfw" partition on eMMC, and I noticed Samsung usually uses LDFW
term, so I figured it was not a big deal to throw that abbreviation at
the user. But I totally agree on BL part, it might be confusing. I
don't have any strong opinion on this one. If you are going to apply
v3, can I kindly ask you to change that message the way you want it to
be?

> Don't change it if v3 is the last. If not, please, make it more verbose.
>
> > +
> > +     return ret;
> > +}
> > +
>
>
> [...]
>
>
> Kind regards,
> --
> Łukasz Stelmach
> Samsung R&D Institute Poland
> Samsung Electronics

Re: [PATCH v3 5/6] hwrng: exynos: Add SMC based TRNG operation

[Lukasz Stelmach]

[-- Attachment #1: Type: text/plain, Size: 3764 bytes --]

It was <2024-06-21 pią 14:40>, when Sam Protsenko wrote:
> On Fri, Jun 21, 2024 at 2:00 PM Lukasz Stelmach <l.stelmach@samsung.com> wrote:
>>
>> It was <2024-06-20 czw 18:13>, when Sam Protsenko wrote:
>> > On some Exynos chips like Exynos850 the access to Security Sub System
>> > (SSS) registers is protected with TrustZone, and therefore only possible
>> > from EL3 monitor software. The Linux kernel is running in EL1, so the
>> > only way for the driver to obtain TRNG data is via SMC calls to EL3
>> > monitor. Implement such SMC operation and use it when EXYNOS_SMC flag is
>> > set in the corresponding chip driver data.
>> >
>> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
>> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> > ---
>> > Changes in v3:
>> >   - Added appropriate error messages for the case when init SMC command fails
>> >
>> > Changes in v2:
>> >   - Used the "reversed Christmas tree" style in the variable declaration
>> >     block in exynos_trng_do_read_smc()
>> >   - Renamed .quirks to .flags in the driver structure
>> >   - Added Krzysztof's R-b tag
>> >
>> >  drivers/char/hw_random/exynos-trng.c | 140 +++++++++++++++++++++++++--
>> >  1 file changed, 130 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
>> > index 6ef2ee6c9804..9fa30583cc86 100644
>> > --- a/drivers/char/hw_random/exynos-trng.c
>> > +++ b/drivers/char/hw_random/exynos-trng.c
>>
>> [...]
>>
>>
>> > @@ -103,6 +163,24 @@ static int exynos_trng_init(struct hwrng *rng)
>> >       return 0;
>> >  }
>> >
>> > +static int exynos_trng_init_smc(struct hwrng *rng)
>> > +{
>> > +     struct exynos_trng_dev *trng = (struct exynos_trng_dev *)rng->priv;
>> > +     struct arm_smccc_res res;
>> > +     int ret = 0;
>> > +
>> > +     arm_smccc_smc(SMC_CMD_RANDOM, HWRNG_INIT, 0, 0, 0, 0, 0, 0, &res);
>> > +     if (res.a0 != HWRNG_RET_OK) {
>> > +             dev_err(trng->dev, "SMC command for TRNG init failed (%d)\n",
>> > +                     (int)res.a0);
>> > +             ret = -EIO;
>> > +     }
>> > +     if ((int)res.a0 == -1)
>> > +             dev_info(trng->dev, "Make sure LDFW is loaded by your BL\n");
>>
>> This is good, thank you for adding it. It can be even better though, if
>> you don't skimp on message length (-; I mean, I know what BL is, I can
>> fingure what LDFW is because you have explained to me and I can see the
>> source code, but somewone who sees it for the first time will be only
>> slightly less surprised than with v2 error message only. Come on, you
>> can make this message twice as long and it will still fit in 80 characters (-;
>>
>
> Guess my OCD got in the way and I just didn't want to break the line
> :) But yeah, LDFW = Loadable Firmware, and BL = bootloader. There is
> an "ldfw" partition on eMMC, and I noticed Samsung usually uses LDFW
> term, so I figured it was not a big deal to throw that abbreviation at
> the user. But I totally agree on BL part, it might be confusing. I
> don't have any strong opinion on this one. If you are going to apply
> v3, can I kindly ask you to change that message the way you want it to
> be?

I guess Olivia or Herbert will be applying it. Let me try… How about:

"Check if your bootloader loads the firmware (SMC) part of the driver."

>> Don't change it if v3 is the last. If not, please, make it more verbose.
>>
>> > +
>> > +     return ret;
>> > +}
>> > +
>>
>>
>> [...]
>>
>>
>> Kind regards,
>> --
>> Łukasz Stelmach
>> Samsung R&D Institute Poland
>> Samsung Electronics
>

-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [PATCH v3 5/6] hwrng: exynos: Add SMC based TRNG operation

[Sam Protsenko]

On Fri, Jun 21, 2024 at 5:11 PM Lukasz Stelmach <l.stelmach@samsung.com> wrote:

[snip]

> >> This is good, thank you for adding it. It can be even better though, if
> >> you don't skimp on message length (-; I mean, I know what BL is, I can
> >> fingure what LDFW is because you have explained to me and I can see the
> >> source code, but somewone who sees it for the first time will be only
> >> slightly less surprised than with v2 error message only. Come on, you
> >> can make this message twice as long and it will still fit in 80 characters (-;
> >>
> >
> > Guess my OCD got in the way and I just didn't want to break the line
> > :) But yeah, LDFW = Loadable Firmware, and BL = bootloader. There is
> > an "ldfw" partition on eMMC, and I noticed Samsung usually uses LDFW
> > term, so I figured it was not a big deal to throw that abbreviation at
> > the user. But I totally agree on BL part, it might be confusing. I
> > don't have any strong opinion on this one. If you are going to apply
> > v3, can I kindly ask you to change that message the way you want it to
> > be?
>
> I guess Olivia or Herbert will be applying it. Let me try… How about:
>
> "Check if your bootloader loads the firmware (SMC) part of the driver."
>

Much better. Thanks, Lukasz!

> >> Don't change it if v3 is the last. If not, please, make it more verbose.
> >>

[PATCH v3 6/6] hwrng: exynos: Enable Exynos850 support

[Sam Protsenko]

Add Exynos850 compatible and its driver data. It's only possible to
access TRNG block via SMC calls in Exynos850, so specify that fact using
EXYNOS_SMC flag in the driver data.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Łukasz Stelmach <l.stelmach@samsung.com>
---
Changes in v3:
  - Added R-b tag from Krzysztof
  - Added A-b tag from Łukasz

Changes in v2:
  - Changed QUIRK_SMC to EXYNOS_SMC to reflect the name change in the
    previous patch

 drivers/char/hw_random/exynos-trng.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
index 9fa30583cc86..9f039fddaee3 100644
--- a/drivers/char/hw_random/exynos-trng.c
+++ b/drivers/char/hw_random/exynos-trng.c
@@ -320,6 +320,9 @@ static DEFINE_SIMPLE_DEV_PM_OPS(exynos_trng_pm_ops, exynos_trng_suspend,
 static const struct of_device_id exynos_trng_dt_match[] = {
 	{
 		.compatible = "samsung,exynos5250-trng",
+	}, {
+		.compatible = "samsung,exynos850-trng",
+		.data = (void *)EXYNOS_SMC,
 	},
 	{ },
 };
-- 
2.39.2

Re: [PATCH v3 0/6] hwrng: exynos: Add support for Exynos850

[Herbert Xu]

On Thu, Jun 20, 2024 at 06:13:33PM -0500, Sam Protsenko wrote:
> Exynos850 has True Random Number Generator (TRNG) block which is very
> similar to Exynos5250 for which the driver already exists
> (exynos-trng.c). There are two differences though:
>   1. Additional SSS PCLK clock has to be enabled to make TRNG registers
>      accessible.
>   2. All SSS registers (including TRNG area) are protected with
>      TrustZone and can only be accessed from EL3 monitor. So the
>      corresponding SMC calls have to be used instead to interact with
>      TRNG block.
> 
> This patch series enables TRNG support on Exynos850 SoC. It was tested
> on the E850-96 board running Debian rootfs like this:
> 
>     8<-------------------------------------------------------------->8
>     # cat /sys/devices/virtual/misc/hw_random/rng_current
>     12081400.rng
> 
>     # dd if=/dev/hwrng bs=100000 count=1 > /dev/null
>     ...
>     122KB/s
> 
>     # apt install rng-tools5
>     # rngtest -c 1000 < /dev/hwrng
>     ...
>     rngtest: starting FIPS tests...
>     rngtest: bits received from input: 20000032
>     rngtest: FIPS 140-2 successes: 1000
>     rngtest: FIPS 140-2 failures: 0
>     rngtest: FIPS 140-2(2001-10-10) Monobit: 0
>     rngtest: FIPS 140-2(2001-10-10) Poker: 0
>     rngtest: FIPS 140-2(2001-10-10) Runs: 0
>     rngtest: FIPS 140-2(2001-10-10) Long run: 0
>     rngtest: FIPS 140-2(2001-10-10) Continuous run: 0
>     rngtest: input channel speed: (min=941.855; avg=965.515;
>              max=968.236)Kibits/s
>     rngtest: FIPS tests speed: (min=49.542; avg=52.886;
>              max=53.577)Mibits/s
>     rngtest: Program run time: 20590194 microseconds
>     8<-------------------------------------------------------------->8
> 
> SMC commands added in this series require LDFW (Loadable Firmware) to be
> loaded by the bootloader. In case of E850-96 board, at the moment only
> the LittleKernel based bootloader [1] is able to load LDFW. It is
> expected to be added into U-Boot port soon as well. See [2] for more
> details.
> 
> [1] https://gitlab.com/Linaro/96boards/e850-96/lk
> [2] https://docs.u-boot.org/en/latest/board/samsung/e850-96.html
> 
> Changes in v3:
>   - Rebased on top of the most recent linux-next
>   - Removed dts patch (7/7) from the series, as suggested by Krzysztof
>   - Addressed all review comments for v2 series
> 
> Changes in v2:
>   - Removed the patch for renaming the dt-bindings doc file
>   - Added the patch for using devm_clk_get_enabled() to get the clock
>   - Addressed all review comments for v1 series
> 
> Sam Protsenko (6):
>   dt-bindings: rng: Add Exynos850 support to exynos-trng
>   hwrng: exynos: Improve coding style
>   hwrng: exynos: Use devm_clk_get_enabled() to get the clock
>   hwrng: exynos: Implement bus clock control
>   hwrng: exynos: Add SMC based TRNG operation
>   hwrng: exynos: Enable Exynos850 support
> 
>  .../bindings/rng/samsung,exynos5250-trng.yaml |  40 +++-
>  drivers/char/hw_random/exynos-trng.c          | 225 +++++++++++++-----
>  2 files changed, 206 insertions(+), 59 deletions(-)
> 
> -- 
> 2.39.2

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt