Re: [PATCH v2 2/2] hwrng: add support for Airoha EN7581 TRNG
From: Martin Kaiser
Date: Fri Oct 18 2024 - 04:55:39 EST
Hi,
Thus wrote Christian Marangi (ansuelsmth@xxxxxxxxx):
> Add support for Airoha TRNG. The Airoha SoC provide a True RNG module
> that can output 4 bytes of raw data at times.
> The module makes use of various noise source to provide True Random
> Number Generation.
> On probe the module is reset to operate Health Test and verify correct
> execution of it.
> The module can also provide DRBG function but the execution mode is
> mutually exclusive, running as TRNG doesn't permit to also run it as
> DRBG.
> Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>
> ---
> Changes v2:
> - Sort include header
> - Add missing bitfield.h
> drivers/char/hw_random/Kconfig | 13 ++
> drivers/char/hw_random/Makefile | 1 +
> drivers/char/hw_random/airoha-trng.c | 243 +++++++++++++++++++++++++++
> 3 files changed, 257 insertions(+)
> create mode 100644 drivers/char/hw_random/airoha-trng.c
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 5912c2dd6398..bda283f290bc 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -62,6 +62,19 @@ config HW_RANDOM_AMD
> If unsure, say Y.
> +config HW_RANDOM_AIROHA
> + tristate "Airoha True HW Random Number Generator support"
> + depends on ARCH_AIROHA || COMPILE_TEST
> + default HW_RANDOM
> + help
> + This driver provides kernel-side support for the True Random Number
> + Generator hardware found on Airoha SoC.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called airoha-rng.
> +
> + If unsure, say Y.
> +
> config HW_RANDOM_ATMEL
> tristate "Atmel Random Number Generator support"
> depends on (ARCH_AT91 || COMPILE_TEST)
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index 01f012eab440..dfb717b12f0b 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -8,6 +8,7 @@ rng-core-y := core.o
> obj-$(CONFIG_HW_RANDOM_TIMERIOMEM) += timeriomem-rng.o
> obj-$(CONFIG_HW_RANDOM_INTEL) += intel-rng.o
> obj-$(CONFIG_HW_RANDOM_AMD) += amd-rng.o
> +obj-$(CONFIG_HW_RANDOM_AIROHA) += airoha-trng.o
> obj-$(CONFIG_HW_RANDOM_ATMEL) += atmel-rng.o
> obj-$(CONFIG_HW_RANDOM_BA431) += ba431-rng.o
> obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
> diff --git a/drivers/char/hw_random/airoha-trng.c b/drivers/char/hw_random/airoha-trng.c
> new file mode 100644
> index 000000000000..1dbfa9505c21
> --- /dev/null
> +++ b/drivers/char/hw_random/airoha-trng.c
> @@ -0,0 +1,243 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2024 Christian Marangi */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/hw_random.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/platform_device.h>
> +
> +#define TRNG_IP_RDY 0x800
> +#define CNT_TRANS GENMASK(15, 8)
> +#define SAMPLE_RDY BIT(0)
> +#define TRNG_NS_SEK_AND_DAT_EN 0x804
> +#define RNG_EN BIT(31) /* referenced as ring_en */
> +#define RAW_DATA_EN BIT(16)
> +#define TRNG_HEALTH_TEST_SW_RST 0x808
> +#define SW_RST BIT(0) /* Active High */
> +#define TRNG_INTR_EN 0x818
> +#define INTR_MASK BIT(16)
> +#define CONTINUOUS_HEALTH_INITR_EN BIT(2)
> +#define SW_STARTUP_INITR_EN BIT(1)
> +#define RST_STARTUP_INITR_EN BIT(0)
> +/* Notice that Health Test are done only out of Reset and with RNG_EN */
> +#define TRNG_HEALTH_TEST_STATUS 0x824
> +#define CONTINUOUS_HEALTH_AP_TEST_FAIL BIT(23)
> +#define CONTINUOUS_HEALTH_RC_TEST_FAIL BIT(22)
> +#define SW_STARTUP_TEST_DONE BIT(21)
> +#define SW_STARTUP_AP_TEST_FAIL BIT(20)
> +#define SW_STARTUP_RC_TEST_FAIL BIT(19)
> +#define RST_STARTUP_TEST_DONE BIT(18)
> +#define RST_STARTUP_AP_TEST_FAIL BIT(17)
> +#define RST_STARTUP_RC_TEST_FAIL BIT(16)
> +#define RAW_DATA_VALID BIT(7)
> +
> +#define TRNG_RAW_DATA_OUT 0x828
> +
> +#define TRNG_CNT_TRANS_VALID 0x80
> +#define BUSY_LOOP_SLEEP 10
> +#define BUSY_LOOP_TIMEOUT (BUSY_LOOP_SLEEP * 10000)
> +
> +struct airoha_trng {
> + void __iomem *base;
> + struct hwrng rng;
> + struct device *dev;
> +
> + struct completion rng_op_done;
> +};
> +
> +static int airoha_trng_irq_mask(struct airoha_trng *trng)
> +{
> + u32 val;
> +
> + val = readl(trng->base + TRNG_INTR_EN);
> + val |= INTR_MASK;
> + writel(val, trng->base + TRNG_INTR_EN);
> +
> + return 0;
> +}
> +
> +static int airoha_trng_irq_unmask(struct airoha_trng *trng)
> +{
> + u32 val;
> +
> + val = readl(trng->base + TRNG_INTR_EN);
> + val &= ~INTR_MASK;
> + writel(val, trng->base + TRNG_INTR_EN);
> +
> + return 0;
> +}
> +
> +static int airoha_trng_init(struct hwrng *rng)
> +{
> + struct airoha_trng *trng = container_of(rng, struct airoha_trng, rng);
> + int ret;
> + u32 val;
> +
> + val = readl(trng->base + TRNG_NS_SEK_AND_DAT_EN);
> + val |= RNG_EN;
> + writel(val, trng->base + TRNG_NS_SEK_AND_DAT_EN);
> +
> + /* Set out of SW Reset */
> + airoha_trng_irq_unmask(trng);
> + writel(0, trng->base + TRNG_HEALTH_TEST_SW_RST);
> +
> + ret = wait_for_completion_timeout(&trng->rng_op_done, BUSY_LOOP_TIMEOUT);
> + if (ret <= 0) {
> + dev_err(trng->dev, "Timeout waiting for Health Check\n");
> + airoha_trng_irq_mask(trng);
> + return -ENODEV;
> + }
> +
> + /* Check if Health Test Failed */
> + val = readl(trng->base + TRNG_HEALTH_TEST_STATUS);
> + if (val & (RST_STARTUP_AP_TEST_FAIL | RST_STARTUP_RC_TEST_FAIL)) {
> + dev_err(trng->dev, "Health Check fail: %s test fail\n",
> + val & RST_STARTUP_AP_TEST_FAIL ? "AP" : "RC");
> + return -ENODEV;
> + }
> +
> + /* Check if IP is ready */
> + ret = readl_poll_timeout(trng->base + TRNG_IP_RDY, val,
> + val & SAMPLE_RDY, 10, 1000);
> + if (ret < 0) {
> + dev_err(trng->dev, "Timeout waiting for IP ready");
> + return -ENODEV;
> + }
You could use dev_err_probe instead of dev_err + return here and in other
similar places.
Apart from this small remark, the driver looks good to me.
Reviewed-by: Martin Kaiser <martin@xxxxxxxxx>