Re: [PATCH v5 08/11] firmware: imx: enable trng

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 23/08/2023 09:33, Pankaj Gupta wrote:
> Enabled trng on imx93 platform through enclave fw.
> 
> Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
> Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
> ---
>  drivers/firmware/imx/Kconfig              |  9 +++
>  drivers/firmware/imx/Makefile             |  3 +-
>  drivers/firmware/imx/ele_base_msg.c       | 88 +++++++++++++++++++++++
>  drivers/firmware/imx/ele_common.c         | 44 +++++++++++-
>  drivers/firmware/imx/ele_common.h         |  2 +
>  drivers/firmware/imx/ele_fw_api.c         | 54 ++++++++++++++
>  drivers/firmware/imx/ele_trng.c           | 48 +++++++++++++
>  drivers/firmware/imx/se_fw.c              | 23 +++++-
>  include/linux/firmware/imx/ele_base_msg.h | 11 +++
>  include/linux/firmware/imx/ele_fw_api.h   |  5 ++
>  10 files changed, 283 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/firmware/imx/ele_trng.c
> 

firmware is not a place for RNG driver. You did not even CC RNG folks...

> +
> +int ele_trng_init(struct device *dev)
> +{
> +	struct ele_trng *trng;
> +	int ret;
> +
> +	trng = devm_kzalloc(dev, sizeof(*trng), GFP_KERNEL);
> +	if (!trng)
> +		return -ENOMEM;
> +
> +	trng->dev         = dev;
> +	trng->rng.name    = "ele-trng";
> +	trng->rng.read    = ele_get_hwrng;
> +	trng->rng.priv    = (unsigned long)trng;
> +	trng->rng.quality = 1024;
> +
> +	dev_dbg(dev, "registering ele-trng\n");
> +
> +	ret = devm_hwrng_register(dev, &trng->rng);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(dev, "Successfully registered ele-trng\n");

Drop all simple trace replacements. Use tracing.

> +	return 0;
> +}
> +
> +int ele_get_hwrng(struct hwrng *rng,
> +		  void *data, size_t len, bool wait)
> +{
> +	struct ele_trng *trng = (struct ele_trng *)rng->priv;
> +
> +	return ele_get_random(trng->dev, data, len);
> +}
> diff --git a/drivers/firmware/imx/se_fw.c b/drivers/firmware/imx/se_fw.c
> index 88300c41d62b..6083c42dcda3 100644
> --- a/drivers/firmware/imx/se_fw.c
> +++ b/drivers/firmware/imx/se_fw.c
> @@ -21,10 +21,10 @@
>  #include <linux/of_reserved_mem.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> -#include <linux/delay.h>
>  #include <linux/sys_soc.h>
>  
>  #include "se_fw.h"
> +#include "ele_common.h"
>  
>  #define SOC_ID_OF_IMX8ULP		0x084D
>  #define SOC_ID_OF_IMX93			0x9300
> @@ -43,6 +43,9 @@ struct imx_info {
>  	uint8_t *pool_name;
>  	bool reserved_dma_ranges;
>  	bool init_fw;
> +	/* platform specific flag to enable/disable the ELE True RNG */
> +	bool start_rng;
> +	bool enable_ele_trng;
>  };
>  
>  static LIST_HEAD(priv_data_list);
> @@ -58,6 +61,8 @@ static const struct imx_info imx8ulp_info = {
>  	.pool_name = "sram-pool",
>  	.reserved_dma_ranges = true,
>  	.init_fw = false,
> +	.start_rng = true,
> +	.enable_ele_trng = false,
>  };
>  
>  static const struct imx_info imx93_info = {
> @@ -71,6 +76,8 @@ static const struct imx_info imx93_info = {
>  	.pool_name = NULL,
>  	.reserved_dma_ranges = true,
>  	.init_fw = true,
> +	.start_rng = true,
> +	.enable_ele_trng = true,
>  };
>  
>  static const struct of_device_id se_fw_match[] = {
> @@ -1140,9 +1147,23 @@ static int se_fw_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	/* start ele rng */
> +	if (info->start_rng) {
> +		ret = ele_do_start_rng(dev);
> +		if (ret)
> +			dev_err(dev, "Failed to start ele rng\n");
> +	}
> +
> +	if (!ret && info->enable_ele_trng) {
> +		ret = ele_trng_init(dev);
> +		if (ret)
> +			dev_err(dev, "Failed to init ele-trng\n");
> +	}
> +
>  	pr_info("i.MX secure-enclave: %s's mu#%d interface to firmware, configured.\n",

dev_info

>  		info->se_name,
>  		priv->ele_mu_id);
> +

Not related.

>  	return devm_of_platform_populate(dev);
>  



Best regards,
Krzysztof