This patch series adds kernel support for StarFive hardware random
number generator. First 2 patches add bindings documentation and driver
for this module. Patch 3 adds devicetree entry for VisionFive v2 SoC.
Patch 3 needs to be applied on top of:
https://patchwork.kernel.org/project/linux-riscv/patch/[email protected]/
Changes since v1:
- updated of_match_ptr and added pm_sleep_ptr in Patch 2. (by Krzysztof)
- drop "status" in dts as module is always on in Patch 3. (by Krzysztof)
Jia Jie Ho (3):
dt-bindings: rng: Add StarFive TRNG module
hwrng: starfive - Add TRNG driver for StarFive SoC
riscv: dts: starfive: Add TRNG node for VisionFive 2
.../bindings/rng/starfive,jh7110-trng.yaml | 55 +++
MAINTAINERS | 6 +
arch/riscv/boot/dts/starfive/jh7110.dtsi | 10 +
drivers/char/hw_random/Kconfig | 11 +
drivers/char/hw_random/Makefile | 1 +
drivers/char/hw_random/starfive-trng.c | 388 ++++++++++++++++++
6 files changed, 471 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml
create mode 100644 drivers/char/hw_random/starfive-trng.c
--
2.25.1
Add documentation to describe Starfive true random number generator
module.
Co-developed-by: Jenny Zhang <[email protected]>
Signed-off-by: Jenny Zhang <[email protected]>
Signed-off-by: Jia Jie Ho <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/rng/starfive,jh7110-trng.yaml | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml
diff --git a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml
new file mode 100644
index 000000000000..2b76ce25acc4
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rng/starfive,jh7110-trng.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive SoC TRNG Module
+
+maintainers:
+ - Jia Jie Ho <[email protected]>
+
+properties:
+ compatible:
+ const: starfive,jh7110-trng
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Hardware reference clock
+ - description: AHB reference clock
+
+ clock-names:
+ items:
+ - const: hclk
+ - const: ahb
+
+ resets:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - resets
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ rng: rng@1600C000 {
+ compatible = "starfive,jh7110-trng";
+ reg = <0x1600C000 0x4000>;
+ clocks = <&clk 15>, <&clk 16>;
+ clock-names = "hclk", "ahb";
+ resets = <&reset 3>;
+ interrupts = <30>;
+ };
+...
--
2.25.1
Adding StarFive TRNG controller node to VisionFive 2 SoC.
Co-developed-by: Jenny Zhang <[email protected]>
Signed-off-by: Jenny Zhang <[email protected]>
Signed-off-by: Jia Jie Ho <[email protected]>
---
arch/riscv/boot/dts/starfive/jh7110.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 4ac159d79d66..3c29e0bc6246 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -455,5 +455,15 @@ uart5: serial@12020000 {
reg-shift = <2>;
status = "disabled";
};
+
+ rng: rng@1600c000 {
+ compatible = "starfive,jh7110-trng";
+ reg = <0x0 0x1600C000 0x0 0x4000>;
+ clocks = <&stgcrg JH7110_STGCLK_SEC_HCLK>,
+ <&stgcrg JH7110_STGCLK_SEC_MISCAHB>;
+ clock-names = "hclk", "ahb";
+ resets = <&stgcrg JH7110_STGRST_SEC_TOP_HRESETN>;
+ interrupts = <30>;
+ };
};
};
--
2.25.1
This adds driver support for the hardware random number generator in
Starfive SoCs and adds StarFive TRNG entry to MAINTAINERS.
Co-developed-by: Jenny Zhang <[email protected]>
Signed-off-by: Jenny Zhang <[email protected]>
Signed-off-by: Jia Jie Ho <[email protected]>
---
MAINTAINERS | 6 +
drivers/char/hw_random/Kconfig | 11 +
drivers/char/hw_random/Makefile | 1 +
drivers/char/hw_random/starfive-trng.c | 388 +++++++++++++++++++++++++
4 files changed, 406 insertions(+)
create mode 100644 drivers/char/hw_random/starfive-trng.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 65140500d9f8..b91e3fc11fc6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19626,6 +19626,12 @@ F: drivers/reset/reset-starfive.c
F: include/linux/reset/starfive.h
F: include/dt-bindings/reset/starfive*
+STARFIVE TRNG DRIVER
+M: Jia Jie Ho <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/rng/starfive*
+F: drivers/char/hw_random/starfive-trng.c
+
STATIC BRANCH/CALL
M: Peter Zijlstra <[email protected]>
M: Josh Poimboeuf <[email protected]>
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 3da8e85f8aae..ddd7ada7f877 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -549,6 +549,17 @@ config HW_RANDOM_CN10K
To compile this driver as a module, choose M here.
The module will be called cn10k_rng. If unsure, say Y.
+config HW_RANDOM_STARFIVE
+ tristate "StarFive HW Random Number Generator support"
+ depends on SOC_STARFIVE
+ depends on HW_RANDOM
+ help
+ This driver provides support for the Hardware Random Number
+ Generator in StarFive SoCs.
+
+ To compile this driver as a module, choose M here.
+ The module will be called starfive-trng.
+
endif # HW_RANDOM
config UML_RANDOM
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 3e948cf04476..f68ac370847f 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -47,3 +47,4 @@ obj-$(CONFIG_HW_RANDOM_XIPHERA) += xiphera-trng.o
obj-$(CONFIG_HW_RANDOM_ARM_SMCCC_TRNG) += arm_smccc_trng.o
obj-$(CONFIG_HW_RANDOM_CN10K) += cn10k-rng.o
obj-$(CONFIG_HW_RANDOM_POLARFIRE_SOC) += mpfs-rng.o
+obj-$(CONFIG_HW_RANDOM_STARFIVE) += starfive-trng.o
diff --git a/drivers/char/hw_random/starfive-trng.c b/drivers/char/hw_random/starfive-trng.c
new file mode 100644
index 000000000000..a6fb369de814
--- /dev/null
+++ b/drivers/char/hw_random/starfive-trng.c
@@ -0,0 +1,388 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TRNG driver for the StarFive SoC
+ *
+ * Copyright (C) 2022 StarFive Technology Co.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/hw_random.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/random.h>
+#include <linux/reset.h>
+
+/* trng register offset */
+#define STARFIVE_CTRL 0x00
+#define STARFIVE_STAT 0x04
+#define STARFIVE_MODE 0x08
+#define STARFIVE_SMODE 0x0C
+#define STARFIVE_IE 0x10
+#define STARFIVE_ISTAT 0x14
+#define STARFIVE_RAND0 0x20
+#define STARFIVE_RAND1 0x24
+#define STARFIVE_RAND2 0x28
+#define STARFIVE_RAND3 0x2C
+#define STARFIVE_RAND4 0x30
+#define STARFIVE_RAND5 0x34
+#define STARFIVE_RAND6 0x38
+#define STARFIVE_RAND7 0x3C
+#define STARFIVE_AUTO_RQSTS 0x60
+#define STARFIVE_AUTO_AGE 0x64
+
+/* CTRL CMD */
+#define STARFIVE_CTRL_EXEC_NOP 0x0
+#define STARFIVE_CTRL_GENE_RANDNUM 0x1
+#define STARFIVE_CTRL_EXEC_RANDRESEED 0x2
+
+/* STAT */
+#define STARFIVE_STAT_NONCE_MODE BIT(2)
+#define STARFIVE_STAT_R256 BIT(3)
+#define STARFIVE_STAT_MISSION_MODE BIT(8)
+#define STARFIVE_STAT_SEEDED BIT(9)
+#define STARFIVE_STAT_LAST_RESEED(x) ((x) << 16)
+#define STARFIVE_STAT_SRVC_RQST BIT(27)
+#define STARFIVE_STAT_RAND_GENERATING BIT(30)
+#define STARFIVE_STAT_RAND_SEEDING BIT(31)
+
+/* MODE */
+#define STARFIVE_MODE_R256 BIT(3)
+
+/* SMODE */
+#define STARFIVE_SMODE_NONCE_MODE BIT(2)
+#define STARFIVE_SMODE_MISSION_MODE BIT(8)
+#define STARFIVE_SMODE_MAX_REJECTS(x) ((x) << 16)
+
+/* IE */
+#define STARFIVE_IE_RAND_RDY_EN BIT(0)
+#define STARFIVE_IE_SEED_DONE_EN BIT(1)
+#define STARFIVE_IE_LFSR_LOCKUP_EN BIT(4)
+#define STARFIVE_IE_GLBL_EN BIT(31)
+
+#define STARFIVE_IE_ALL (STARFIVE_IE_GLBL_EN | \
+ STARFIVE_IE_RAND_RDY_EN | \
+ STARFIVE_IE_SEED_DONE_EN | \
+ STARFIVE_IE_LFSR_LOCKUP_EN)
+
+/* ISTAT */
+#define STARFIVE_ISTAT_RAND_RDY BIT(0)
+#define STARFIVE_ISTAT_SEED_DONE BIT(1)
+#define STARFIVE_ISTAT_LFSR_LOCKUP BIT(4)
+
+#define STARFIVE_RAND_LEN sizeof(u32)
+
+#define to_trng(p) container_of(p, struct starfive_trng, rng)
+
+enum reseed {
+ RANDOM_RESEED,
+ NONCE_RESEED,
+};
+
+enum mode {
+ PRNG_128BIT,
+ PRNG_256BIT,
+};
+
+struct starfive_trng {
+ struct device *dev;
+ void __iomem *base;
+ struct clk *hclk;
+ struct clk *ahb;
+ struct reset_control *rst;
+ struct hwrng rng;
+ struct completion random_done;
+ struct completion reseed_done;
+ u32 mode;
+ u32 mission;
+ u32 reseed;
+};
+
+static u16 autoreq;
+module_param(autoreq, ushort, 0);
+MODULE_PARM_DESC(autoreq, "Auto-reseeding after random number requests by host reaches specified counter:\n"
+ " 0 - disable counter\n"
+ " other - reload value for internal counter");
+
+static u16 autoage;
+module_param(autoage, ushort, 0);
+MODULE_PARM_DESC(autoage, "Auto-reseeding after specified timer countdowns to 0:\n"
+ " 0 - disable timer\n"
+ " other - reload value for internal timer");
+
+static inline int starfive_trng_wait_idle(struct starfive_trng *trng)
+{
+ u32 stat;
+
+ return readl_relaxed_poll_timeout(trng->base + STARFIVE_STAT, stat,
+ !(stat & (STARFIVE_STAT_RAND_GENERATING |
+ STARFIVE_STAT_RAND_SEEDING)),
+ 10, 100000);
+}
+
+static inline void starfive_trng_irq_mask_clear(struct starfive_trng *trng)
+{
+ /* clear register: ISTAT */
+ u32 data = readl(trng->base + STARFIVE_ISTAT);
+
+ writel(data, trng->base + STARFIVE_ISTAT);
+}
+
+static int starfive_trng_cmd(struct starfive_trng *trng, u32 cmd)
+{
+ int ret;
+
+ ret = starfive_trng_wait_idle(trng);
+ if (ret)
+ return -ETIMEDOUT;
+
+ switch (cmd) {
+ case STARFIVE_CTRL_EXEC_NOP:
+ writel(cmd, trng->base + STARFIVE_CTRL);
+ break;
+ case STARFIVE_CTRL_GENE_RANDNUM:
+ reinit_completion(&trng->random_done);
+ writel(cmd, trng->base + STARFIVE_CTRL);
+ ret = wait_for_completion_timeout(&trng->random_done, 3000);
+ if (!ret)
+ return -ETIMEDOUT;
+ break;
+ case STARFIVE_CTRL_EXEC_RANDRESEED:
+ reinit_completion(&trng->reseed_done);
+ writel(cmd, trng->base + STARFIVE_CTRL);
+ ret = wait_for_completion_timeout(&trng->reseed_done, 3000);
+ if (!ret)
+ return -ETIMEDOUT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int starfive_trng_init(struct hwrng *rng)
+{
+ struct starfive_trng *trng = to_trng(rng);
+ u32 mode, intr = 0;
+
+ /* disable Auto Request/Age register */
+ writel(autoage, trng->base + STARFIVE_AUTO_AGE);
+ writel(autoreq, trng->base + STARFIVE_AUTO_RQSTS);
+
+ /* clear register: ISTAT */
+ starfive_trng_irq_mask_clear(trng);
+
+ /* set smode/mode */
+ mode = readl(trng->base + STARFIVE_MODE);
+
+ switch (trng->mode) {
+ case PRNG_128BIT:
+ mode &= ~STARFIVE_MODE_R256;
+ break;
+ case PRNG_256BIT:
+ mode |= STARFIVE_MODE_R256;
+ break;
+ default:
+ mode |= STARFIVE_MODE_R256;
+ break;
+ }
+
+ intr |= STARFIVE_IE_ALL;
+ writel(intr, trng->base + STARFIVE_IE);
+ writel(mode, trng->base + STARFIVE_MODE);
+
+ starfive_trng_cmd(trng, STARFIVE_CTRL_EXEC_NOP);
+
+ return starfive_trng_cmd(trng, STARFIVE_CTRL_EXEC_RANDRESEED);
+}
+
+static irqreturn_t starfive_trng_irq(int irq, void *priv)
+{
+ u32 status;
+ struct starfive_trng *trng = (struct starfive_trng *)priv;
+
+ status = readl(trng->base + STARFIVE_ISTAT);
+ if (status & STARFIVE_ISTAT_RAND_RDY) {
+ writel(STARFIVE_ISTAT_RAND_RDY, trng->base + STARFIVE_ISTAT);
+ complete(&trng->random_done);
+ }
+
+ if (status & STARFIVE_ISTAT_SEED_DONE) {
+ writel(STARFIVE_ISTAT_SEED_DONE, trng->base + STARFIVE_ISTAT);
+ complete(&trng->reseed_done);
+ }
+
+ if (status & STARFIVE_ISTAT_LFSR_LOCKUP) {
+ writel(STARFIVE_ISTAT_LFSR_LOCKUP, trng->base + STARFIVE_ISTAT);
+ starfive_trng_cmd(trng, STARFIVE_CTRL_EXEC_RANDRESEED);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void starfive_trng_cleanup(struct hwrng *rng)
+{
+ struct starfive_trng *trng = to_trng(rng);
+
+ writel(0, trng->base + STARFIVE_CTRL);
+
+ reset_control_assert(trng->rst);
+ clk_disable_unprepare(trng->hclk);
+ clk_disable_unprepare(trng->ahb);
+}
+
+static int starfive_trng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
+{
+ struct starfive_trng *trng = to_trng(rng);
+ int ret;
+
+ pm_runtime_get_sync(trng->dev);
+
+ if (trng->mode == PRNG_256BIT)
+ max = min_t(size_t, max, (STARFIVE_RAND_LEN * 8));
+ else
+ max = min_t(size_t, max, (STARFIVE_RAND_LEN * 4));
+
+ ret = starfive_trng_cmd(trng, STARFIVE_CTRL_GENE_RANDNUM);
+ if (ret)
+ return ret;
+
+ memcpy_fromio(buf, trng->base + STARFIVE_RAND0, max);
+
+ pm_runtime_put_sync(trng->dev);
+
+ return max;
+}
+
+static int starfive_trng_probe(struct platform_device *pdev)
+{
+ int ret;
+ int irq;
+ struct starfive_trng *trng;
+
+ trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
+ if (!trng)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, trng);
+ trng->dev = &pdev->dev;
+
+ trng->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(trng->base))
+ return dev_err_probe(&pdev->dev, PTR_ERR(trng->base),
+ "Error remapping memory for platform device.\n");
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_irq(&pdev->dev, irq, starfive_trng_irq, 0, pdev->name,
+ (void *)trng);
+ if (ret)
+ return dev_err_probe(&pdev->dev, irq,
+ "Failed to register interrupt handler\n");
+
+ trng->hclk = devm_clk_get(&pdev->dev, "hclk");
+ if (IS_ERR(trng->hclk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(trng->hclk),
+ "Error getting hardware reference clock\n");
+
+ trng->ahb = devm_clk_get(&pdev->dev, "ahb");
+ if (IS_ERR(trng->ahb))
+ return dev_err_probe(&pdev->dev, PTR_ERR(trng->ahb),
+ "Error getting ahb reference clock\n");
+
+ trng->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
+ if (IS_ERR(trng->rst))
+ return dev_err_probe(&pdev->dev, PTR_ERR(trng->rst),
+ "Error getting hardware reset line\n");
+
+ clk_prepare_enable(trng->hclk);
+ clk_prepare_enable(trng->ahb);
+ reset_control_deassert(trng->rst);
+
+ init_completion(&trng->random_done);
+ init_completion(&trng->reseed_done);
+
+ trng->rng.name = dev_driver_string(&pdev->dev);
+ trng->rng.init = starfive_trng_init;
+ trng->rng.cleanup = starfive_trng_cleanup;
+ trng->rng.read = starfive_trng_read;
+
+ trng->mode = PRNG_256BIT;
+ trng->mission = 1;
+ trng->reseed = RANDOM_RESEED;
+
+ ret = devm_hwrng_register(&pdev->dev, &trng->rng);
+ if (ret) {
+ dev_err_probe(&pdev->dev, ret, "Failed to register hwrng\n");
+ goto err_fail_register;
+ }
+
+ pm_runtime_use_autosuspend(&pdev->dev);
+ pm_runtime_set_autosuspend_delay(&pdev->dev, 100);
+ pm_runtime_enable(&pdev->dev);
+
+ return 0;
+
+err_fail_register:
+ pm_runtime_disable(&pdev->dev);
+
+ reset_control_assert(trng->rst);
+ clk_disable_unprepare(trng->ahb);
+ clk_disable_unprepare(trng->hclk);
+
+ return ret;
+}
+
+static int __maybe_unused starfive_trng_suspend(struct device *dev)
+{
+ struct starfive_trng *trng = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(trng->hclk);
+ clk_disable_unprepare(trng->ahb);
+
+ return 0;
+}
+
+static int __maybe_unused starfive_trng_resume(struct device *dev)
+{
+ struct starfive_trng *trng = dev_get_drvdata(dev);
+
+ clk_prepare_enable(trng->hclk);
+ clk_prepare_enable(trng->ahb);
+
+ return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(starfive_trng_pm_ops, starfive_trng_suspend,
+ starfive_trng_resume);
+
+static const struct of_device_id trng_dt_ids[] __maybe_unused = {
+ { .compatible = "starfive,jh7110-trng" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, trng_dt_ids);
+
+static struct platform_driver starfive_trng_driver = {
+ .probe = starfive_trng_probe,
+ .driver = {
+ .name = "starfive-trng",
+ .pm = &starfive_trng_pm_ops,
+ .of_match_table = of_match_ptr(trng_dt_ids),
+ },
+};
+
+module_platform_driver(starfive_trng_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("StarFive True Random Number Generator");
--
2.25.1
On Wed, Dec 28, 2022 at 03:11:02PM +0800, Jia Jie Ho wrote:
>
> +static int starfive_trng_cmd(struct starfive_trng *trng, u32 cmd)
> +{
> + int ret;
> +
> + ret = starfive_trng_wait_idle(trng);
> + if (ret)
> + return -ETIMEDOUT;
> +
> + switch (cmd) {
> + case STARFIVE_CTRL_EXEC_NOP:
> + writel(cmd, trng->base + STARFIVE_CTRL);
> + break;
> + case STARFIVE_CTRL_GENE_RANDNUM:
> + reinit_completion(&trng->random_done);
> + writel(cmd, trng->base + STARFIVE_CTRL);
> + ret = wait_for_completion_timeout(&trng->random_done, 3000);
Please don't use a constant jiffies value, because it may vary
in length. Instead use a constant millisecond value and convert
it to jiffies.
> +static irqreturn_t starfive_trng_irq(int irq, void *priv)
> +{
> + u32 status;
> + struct starfive_trng *trng = (struct starfive_trng *)priv;
> +
> + status = readl(trng->base + STARFIVE_ISTAT);
> + if (status & STARFIVE_ISTAT_RAND_RDY) {
> + writel(STARFIVE_ISTAT_RAND_RDY, trng->base + STARFIVE_ISTAT);
> + complete(&trng->random_done);
> + }
> +
> + if (status & STARFIVE_ISTAT_SEED_DONE) {
> + writel(STARFIVE_ISTAT_SEED_DONE, trng->base + STARFIVE_ISTAT);
> + complete(&trng->reseed_done);
> + }
> +
> + if (status & STARFIVE_ISTAT_LFSR_LOCKUP) {
> + writel(STARFIVE_ISTAT_LFSR_LOCKUP, trng->base + STARFIVE_ISTAT);
> + starfive_trng_cmd(trng, STARFIVE_CTRL_EXEC_RANDRESEED);
You should not sleep in an IRQ handler.
> +static int starfive_trng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
You should respect the wait argument and not do polling/sleeping
if it is false.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> -----Original Message-----
> From: Herbert Xu <[email protected]>
> Sent: 6 January, 2023 4:39 PM
> To: JiaJie Ho <[email protected]>
> Cc: Olivia Mackall <[email protected]>; Rob Herring <[email protected]>;
> Krzysztof Kozlowski <[email protected]>; Emil Renner
> Berthing <[email protected]>; Conor Dooley <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 2/3] hwrng: starfive - Add TRNG driver for StarFive
> SoC
>
> On Wed, Dec 28, 2022 at 03:11:02PM +0800, Jia Jie Ho wrote:
> >
> > +static int starfive_trng_cmd(struct starfive_trng *trng, u32 cmd) {
> > + int ret;
> > +
> > + ret = starfive_trng_wait_idle(trng);
> > + if (ret)
> > + return -ETIMEDOUT;
> > +
> > + switch (cmd) {
> > + case STARFIVE_CTRL_EXEC_NOP:
> > + writel(cmd, trng->base + STARFIVE_CTRL);
> > + break;
> > + case STARFIVE_CTRL_GENE_RANDNUM:
> > + reinit_completion(&trng->random_done);
> > + writel(cmd, trng->base + STARFIVE_CTRL);
> > + ret = wait_for_completion_timeout(&trng->random_done,
> 3000);
>
> Please don't use a constant jiffies value, because it may vary in length.
> Instead use a constant millisecond value and convert it to jiffies.
>
I'll fix this in next version.
> > +static irqreturn_t starfive_trng_irq(int irq, void *priv) {
> > + u32 status;
> > + struct starfive_trng *trng = (struct starfive_trng *)priv;
> > +
> > + status = readl(trng->base + STARFIVE_ISTAT);
> > + if (status & STARFIVE_ISTAT_RAND_RDY) {
> > + writel(STARFIVE_ISTAT_RAND_RDY, trng->base +
> STARFIVE_ISTAT);
> > + complete(&trng->random_done);
> > + }
> > +
> > + if (status & STARFIVE_ISTAT_SEED_DONE) {
> > + writel(STARFIVE_ISTAT_SEED_DONE, trng->base +
> STARFIVE_ISTAT);
> > + complete(&trng->reseed_done);
> > + }
> > +
> > + if (status & STARFIVE_ISTAT_LFSR_LOCKUP) {
> > + writel(STARFIVE_ISTAT_LFSR_LOCKUP, trng->base +
> STARFIVE_ISTAT);
> > + starfive_trng_cmd(trng,
> STARFIVE_CTRL_EXEC_RANDRESEED);
>
> You should not sleep in an IRQ handler.
>
Will fix this too.
> > +static int starfive_trng_read(struct hwrng *rng, void *buf, size_t
> > +max, bool wait)
>
> You should respect the wait argument and not do polling/sleeping if it is false.
I'll add this in the next version.
Thanks for reviewing this patch.
Best regards,
Jia Jie
> -----Original Message-----
> From: Herbert Xu <[email protected]>
> Sent: 6 January, 2023 4:39 PM
> To: JiaJie Ho <[email protected]>
> Cc: Olivia Mackall <[email protected]>; Rob Herring <[email protected]>;
> Krzysztof Kozlowski <[email protected]>; Emil Renner
> Berthing <[email protected]>; Conor Dooley <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 2/3] hwrng: starfive - Add TRNG driver for StarFive
> SoC
>
> On Wed, Dec 28, 2022 at 03:11:02PM +0800, Jia Jie Ho wrote:
> > +static int starfive_trng_read(struct hwrng *rng, void *buf, size_t
> > +max, bool wait)
>
> You should respect the wait argument and not do polling/sleeping if it is false.
>
Hi Herbert,
My trng device requires sending a generate new number cmd before each read.
It then only populates the data registers with new random number and raise an interrupt when ready.
If user choose to not wait, they will always get stale bits.
Is it okay to always return error if wait=false ?
Thanks
Jia Jie
On Mon, Jan 09, 2023 at 02:58:14AM +0000, JiaJie Ho wrote:
>
> My trng device requires sending a generate new number cmd before each read.
> It then only populates the data registers with new random number and raise an interrupt when ready.
> If user choose to not wait, they will always get stale bits.
> Is it okay to always return error if wait=false ?
What is the length of the wait time? Is there an upper bound?
What is the average wait time?
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> -----Original Message-----
> From: Herbert Xu <[email protected]>
> Sent: 9 January, 2023 11:04 AM
> To: JiaJie Ho <[email protected]>
> Cc: Olivia Mackall <[email protected]>; Rob Herring <[email protected]>;
> Krzysztof Kozlowski <[email protected]>; Emil Renner
> Berthing <[email protected]>; Conor Dooley <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 2/3] hwrng: starfive - Add TRNG driver for StarFive
> SoC
>
> On Mon, Jan 09, 2023 at 02:58:14AM +0000, JiaJie Ho wrote:
> >
> > My trng device requires sending a generate new number cmd before each
> read.
> > It then only populates the data registers with new random number and
> raise an interrupt when ready.
> > If user choose to not wait, they will always get stale bits.
> > Is it okay to always return error if wait=false ?
>
> What is the length of the wait time? Is there an upper bound?
> What is the average wait time?
>
The average wait time is around 20 microseconds.
I measured from writel cmd to wait_for_completion done.
Thanks
Jia Jie
On Mon, Jan 09, 2023 at 03:41:14AM +0000, JiaJie Ho wrote:
>
> The average wait time is around 20 microseconds.
> I measured from writel cmd to wait_for_completion done.
Do you know of an upper bound? E.g., if we limit it to 40us how
many requests would fail on average?
Having a maximum delay of 40us would be OK with wait == 0. So
you could implement it in a way such that if the wait time exceeded
40us then you return if wait == 0, otherwise you can wait longer.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> -----Original Message-----
> From: Herbert Xu <[email protected]>
> Sent: 9 January, 2023 12:00 PM
> To: JiaJie Ho <[email protected]>
> Cc: Olivia Mackall <[email protected]>; Rob Herring <[email protected]>;
> Krzysztof Kozlowski <[email protected]>; Emil Renner
> Berthing <[email protected]>; Conor Dooley <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 2/3] hwrng: starfive - Add TRNG driver for StarFive
> SoC
>
> On Mon, Jan 09, 2023 at 03:41:14AM +0000, JiaJie Ho wrote:
> >
> > The average wait time is around 20 microseconds.
> > I measured from writel cmd to wait_for_completion done.
>
> Do you know of an upper bound? E.g., if we limit it to 40us how many
> requests would fail on average?
>
I don't have the upper bound.
I ran the rngtest with 1000 blocks, none of the wait time exceeded 30us.
> Having a maximum delay of 40us would be OK with wait == 0. So you could
> implement it in a way such that if the wait time exceeded 40us then you
> return if wait == 0, otherwise you can wait longer.
>
I'll do this then.
Thanks
Jia Jie