From: Daniel Thompson Subject: Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver Date: Wed, 17 Aug 2016 17:01:15 +0100 Message-ID: <0c7ae690-31f5-5d10-3168-2a54d4974399@linaro.org> References: <1471448151-20850-1-git-send-email-prasannatsmkumar@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit To: PrasannaKumar Muralidharan , mpm@selenic.com, herbert@gondor.apana.org.au, robh+dt@kernel.org, mark.rutland@arm.com, ralf@linux-mips.org, davem@davemloft.net, geert@linux-m68k.org, akpm@linux-foundation.org, gregkh@linuxfoundation.org, mchehab@kernel.org, linux@roeck-us.net, boris.brezillon@free-electrons.com, harvey.hunt@imgtec.com, alex.smith@imgtec.com, lee.jones@linaro.org, f.fainelli@gmail.com, kieran@ksquared.org.uk, krzk@kernel.org, joshua.henderson@microchip.com, yendapally.reddy@broadcom.com, narmstrong@baylibre.com, wangkefeng.wang@huawei.com, chunkeey@googlemail.com, noltari@gmail.com, linus.walleij@linaro.org, pankaj.dev@st.com, mathieu.poirier@linaro.org, linux-crypto@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Return-path: In-Reply-To: <1471448151-20850-1-git-send-email-prasannatsmkumar@gmail.com> Sender: linux-mips-bounce@linux-mips.org Errors-to: linux-mips-bounce@linux-mips.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-subscribe: List-owner: List-post: List-archive: List-Id: linux-crypto.vger.kernel.org On 17/08/16 16:35, PrasannaKumar Muralidharan wrote: > This patch adds support for hardware random number generator present in > JZ4780 SoC. > > Signed-off-by: PrasannaKumar Muralidharan > --- > .../devicetree/bindings/rng/ingenic,jz4780-rng.txt | 12 +++ > MAINTAINERS | 5 + > arch/mips/boot/dts/ingenic/jz4780.dtsi | 7 +- > drivers/char/hw_random/Kconfig | 14 +++ > drivers/char/hw_random/Makefile | 1 + > drivers/char/hw_random/jz4780-rng.c | 105 +++++++++++++++++++++ > 6 files changed, 143 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt > create mode 100644 drivers/char/hw_random/jz4780-rng.c > > diff --git a/Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt b/Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt > new file mode 100644 > index 0000000..03abf56 > --- /dev/null > +++ b/Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt > @@ -0,0 +1,12 @@ > +Ingenic jz4780 RNG driver > + > +Required properties: > +- compatible : Should be "ingenic,jz4780-rng" > +- reg : Specifies base physical address and size of the registers. > + > +Example: > + > +rng: rng@100000D8 { > + compatible = "ingenic,jz4780-rng"; > + reg = <0x100000D8 0x8>; > +}; Device tree bindings should be a separate patch. See Documentation/devicetree/bindings/submitting-patches.txt . > diff --git a/MAINTAINERS b/MAINTAINERS > index 08e9efe..c0c66eb 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6002,6 +6002,11 @@ M: Zubair Lutfullah Kakakhel > S: Maintained > F: drivers/dma/dma-jz4780.c > > +INGENIC JZ4780 HW RNG Driver > +M: PrasannaKumar Muralidharan > +S: Maintained > +F: drivers/char/hw_random/jz4780-rng.c > + > INTEGRITY MEASUREMENT ARCHITECTURE (IMA) > M: Mimi Zohar > M: Dmitry Kasatkin > diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi > index b868b42..f11d139 100644 > --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi > +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi > @@ -36,7 +36,7 @@ > > cgu: jz4780-cgu@10000000 { > compatible = "ingenic,jz4780-cgu"; > - reg = <0x10000000 0x100>; > + reg = <0x10000000 0xD8>; > clocks = <&ext>, <&rtc>; > clock-names = "ext", "rtc"; > @@ -44,6 +44,11 @@ > #clock-cells = <1>; > }; > > + rng: jz4780-rng@100000D8 { > + compatible = "ingenic,jz4780-rng"; > + reg = <0x100000D8 0x8>; > + }; > + > uart0: serial@10030000 { > compatible = "ingenic,jz4780-uart"; > reg = <0x10030000 0x100>; Updates to device tree files are also normally sent as a separate patch later in the series than the driver itself (at least they are in arm land). > diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig > index 56ad5a59..c336fe8 100644 > --- a/drivers/char/hw_random/Kconfig > +++ b/drivers/char/hw_random/Kconfig > @@ -294,6 +294,20 @@ config HW_RANDOM_POWERNV > > If unsure, say Y. > > +config HW_RANDOM_JZ4780 > + tristate "JZ4780 HW random number generator support" > + depends on MACH_INGENIC > + depends on HAS_IOMEM > + default HW_RANDOM > + ---help--- > + This driver provides kernel-side support for the Random Number > + Generator hardware found on JZ4780 SOCs. > + > + To compile this driver as a module, choose M here: the > + module will be called jz4780-rng. > + > + If unsure, say Y. > + Shouldn't this be inserted either in alphabetic order (which not applicable for this file) or at the end of the file? > config HW_RANDOM_EXYNOS > tristate "EXYNOS HW random number generator support" > depends on ARCH_EXYNOS || COMPILE_TEST > diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile > index 04bb0b0..a155066 100644 > --- a/drivers/char/hw_random/Makefile > +++ b/drivers/char/hw_random/Makefile > @@ -26,6 +26,7 @@ obj-$(CONFIG_HW_RANDOM_PSERIES) += pseries-rng.o > obj-$(CONFIG_HW_RANDOM_POWERNV) += powernv-rng.o > obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-rng.o > obj-$(CONFIG_HW_RANDOM_HISI) += hisi-rng.o > +obj-$(CONFIG_HW_RANDOM_JZ4780) += jz4780-rng.o This looks like it inserts into the makefile in a gratuitously different order than the one in the KConfig file. > obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o > obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o > obj-$(CONFIG_HW_RANDOM_IPROC_RNG200) += iproc-rng200.o > 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 > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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; > +} > + > +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); > + 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); If platform_get_resource() returns NULL I think this code will crash. > + size = resource_size(res); > + > + jz4780_rng->mem = devm_ioremap(&pdev->dev, res->start, 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; > +} > + > +static int jz4780_rng_remove(struct platform_device *pdev) > +{ > + struct jz4780_rng *jz4780_rng = platform_get_drvdata(pdev); > + > + jz4780_rng_writel(jz4780_rng, 0, REG_RNG_CTRL); > + hwrng_unregister(&jz4780_rng->rng); These two are in same order as probe() function. I would expect them to be reversed (so hardware cannot be shutdown until we have unregistered it). > + > + return 0; > +} > + > +static const struct of_device_id jz4780_rng_dt_match[] = { > + { .compatible = "ingenic,jz4780-rng", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, jz4780_rng_dt_match); > + > +static struct platform_driver jz4780_rng_driver = { > + .driver = { > + .name = "jz4780-rng", > + .of_match_table = jz4780_rng_dt_match, > + }, > + .probe = jz4780_rng_probe, > + .remove = jz4780_rng_remove, > +}; > +module_platform_driver(jz4780_rng_driver); > + > +MODULE_DESCRIPTION("Ingenic JZ4780 H/W Random Number Generator driver"); > +MODULE_AUTHOR("PrasannaKumar Muralidharan "); > +MODULE_LICENSE("GPL"); >