On Sat, Jul 13, 2024 at 10:13 AM Simon Glass sjg@chromium.org wrote:
Hi Sam,
Hi Simon,
Thank you for the review!
On Sat, 13 Jul 2024 at 00:44, Sam Protsenko semen.protsenko@linaro.org wrote:
+config RNG_EXYNOS
bool "Samsung Exynos True Random Number Generator support"depends on DM_RNGhelpEnable support for True Random Number Generator (TRNG)available in Exynos SoCs.Can you mention the needed firmware here?
Will do in v2.
+struct exynos_trng_variant {
please add comments
Will do in v2.
bool smc;int (*init)(struct udevice *dev);void (*exit)(struct udevice *dev);int (*read)(struct udevice *dev, void *data, size_t len);+};
+struct exynos_trng {
The convention is to add a _priv suffix
Will add in v2.
+static int exynos_trng_init_smc(struct udevice *dev) +{
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(dev, "SMC command for TRNG init failed (%d)\n",(int)res.a0);ret = -EIO;}if ((int)res.a0 == -1)dev_info(dev, "Make sure LDFW is loaded\n");return error here?
It's already done above: (res.a0 != HWRNG_RET_OK) condition includes -1 case. This line is basically only to help user figure out what might be wrong.
+static int exynos_trng_of_to_plat(struct udevice *dev) +{
struct exynos_trng *trng = dev_get_priv(dev);trng->data = (struct exynos_trng_variant *)dev_get_driver_data(dev);if (!trng->data->smc) {trng->base = dev_read_addr_ptr(dev);if (!trng->base)return -ENODEV;That has a special meaning in U-Boot (i.e. that there is no device). Devicetree problems should return -EINVAL
Will fix in v2.
}trng->clk = devm_clk_get(dev, "secss");if (IS_ERR(trng->clk))return -ENODEV;Should return the actual error
Will fix in v2.
trng->pclk = devm_clk_get_optional(dev, "pclk");if (IS_ERR(trng->pclk))return -ENODEV;same here
Will fix in v2.
return 0;+}
+static int exynos_trng_probe(struct udevice *dev) +{
struct exynos_trng *trng = dev_get_priv(dev);int err;s/err/ret/
Will fix in v2.
[snip]
Regards, Simon