Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver
From: Corentin LABBE <clabbe.montjoie@gmail.com>
Date: 2016-08-17 19:07:02
Also in:
linux-crypto, linux-mips, lkml
Hello I have just some minor comments below
quoted hunk ↗ jump to hunk
diff --git a/drivers/char/hw_random/jz4780-rng.c b/drivers/char/hw_random/jz4780-rng.c new file mode 100644 index 0000000..c9d2cde --- /dev/null +++ b/drivers/char/hw_random/jz4780-rng.c@@ -0,0 +1,105 @@ +/* + * jz4780-rng.c - Random Number Generator driver for J4780 + * + * Copyright 2016 (C) PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include <linux/hw_random.h> +#include <linux/device.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/io.h> +#include <linux/platform_device.h> +#include <linux/err.h>
You could sort them by alphabetical order
+
+#define REG_RNG_CTRL 0x0
+#define REG_RNG_DATA 0x4
+
+struct jz4780_rng {
+ struct device *dev;
+ struct hwrng rng;
+ void __iomem *mem;
+};
+
+static u32 jz4780_rng_readl(struct jz4780_rng *rng, u32 offset)
+{
+ return readl(rng->mem + offset);
+}
+
+static void jz4780_rng_writel(struct jz4780_rng *rng, u32 val, u32 offset)
+{
+ writel(val, rng->mem + offset);
+}
+
+static int jz4780_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
+{
+ struct jz4780_rng *jz4780_rng = container_of(rng, struct jz4780_rng,
+ rng);
+ u32 *data = buf;
+ *data = jz4780_rng_readl(jz4780_rng, REG_RNG_DATA);
+ return 4;
+}If max is less than 4, its bad
+
+static int jz4780_rng_probe(struct platform_device *pdev)
+{
+ struct jz4780_rng *jz4780_rng;
+ struct resource *res;
+ resource_size_t size;
+ int ret;
+
+ jz4780_rng = devm_kzalloc(&pdev->dev, sizeof(struct jz4780_rng),
+ GFP_KERNEL);You could write sizeof(*js480_rng)
+ if (!jz4780_rng) + return -ENOMEM; + + jz4780_rng->dev = &pdev->dev; + jz4780_rng->rng.name = "jz4780"; + jz4780_rng->rng.read = jz4780_rng_read; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + size = resource_size(res); + + jz4780_rng->mem = devm_ioremap(&pdev->dev, res->start, size);
You could save code by using devm_ioremap_resource (don't need size)
+ if (IS_ERR(jz4780_rng->mem)) + return PTR_ERR(jz4780_rng->mem); + + platform_set_drvdata(pdev, jz4780_rng); + jz4780_rng_writel(jz4780_rng, 1, REG_RNG_CTRL); + ret = hwrng_register(&jz4780_rng->rng); + + return ret; +}
You could write directly return hwrng_register(..) Regards LABBE Corentin