Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-19 Thread PrasannaKumar Muralidharan
> __ARM_FEATURE_UNALIGNED (cf.,
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0774f/chr1383660321827.html)
> . MIPSEL does not define such a macro.
>
> # MIPS ci20 creator with GCC 4.6
> $ gcc -march=native -dM -E -  #define __BIGGEST_ALIGNMENT__ 8
>
> If the MIPS CPU does not tolerate unaligned data access, then the
> following could SIGBUS:
>
>> +   u32 *data = buf;
>> +   *data = jz4780_rng_readl(jz4780_rng, REG_RNG_DATA);
>
> If GCC emits code that uses the MIPS unaligned load and store
> instructions, then there's probably going to be a performance penalty.
>
> Regardless of what the CPU tolerates, I believe unaligned data access
> is undefined behavior in C/C++. I believe you should memcpy the value
> into the buffer.

I am not sure whether this is required. 'buf' is part of a structure
so I guess it is properly aligned by padding. Not sure though, will
look about this.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-19 Thread PrasannaKumar Muralidharan
> Please forgive my ignorance Prasanna...
>
> For the JZ4780 I have, there are two registers in play. The first is
> the control register which enables/disables the RNG. The control
> register is named ERNG. The second register is the data register, and
> it produces the random stream. The data register is named RNG. ERNG is
> located at 0x10D8 and RNG is located at 0x10DC. This kind of
> confuses me because I don't see where 0x10D8 is ever added to
> those values (maybe its in the descriptor?):
>
> +#define REG_RNG_CTRL   0x0
> +#define REG_RNG_DATA   0x4

The base address 0x10D8 is defined in jz4780.dtsi file.
REG_RNG_CTRL and REG_RNG_DATA are offsets. In jz4780_rng_readl and
jz4780_rng_writel functions the ioremap'd base address is added with
offset.

> Also, testing with a userland PoC for the device, you have to throttle
> reads from RNG register. If reads occur with a 0 delay, then the
> random value appears fixed. If the delay is too small, then you can
> watch random values being shifted-in in a barrel like fashion.
> Unfortunately, the manual did not discuss how long to wait for a value
> to be ready. I found spinning in a loop for 5000 was too small and
> witnessed the shifting; while spinning in a loop for 1 avoided the
> shift observation. I don't what number of JIFFIES that translates to.

I can calculate the speed and make sure the delay is met in the
driver. Thanks a lot for providing this info.

> Finally, from looking at the native Ingenic driver (which was not very
> impressive), they enabled/disabled the RNG register on demand. There
> was also a [possible related] note in the manual about not applying
> VCC for over a second. I can only say "possibly related" because I was
> not sure if the register was part of the controller they were
> discussing. The userland PoC worked fine when enabling/disabling the
> RNG register. So I'm not sure about this (from jz4780_rng_probe):
>
> +   platform_set_drvdata(pdev, jz4780_rng);
> +   jz4780_rng_writel(jz4780_rng, 1, REG_RNG_CTRL);
> +   ret = hwrng_register(_rng->rng);
>
> And this (from jz4780_rng_remove):
>
> +   jz4780_rng_writel(jz4780_rng, 0, REG_RNG_CTRL);
> +   hwrng_unregister(_rng->rng);
>
> Anyway, I hope that helps you avoid some land mines (if they are present).

Looking at JZ4780 Programmers manual I could not find any info about
VCC. Can you point me to it?
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-19 Thread Jeffrey Walton
On Wed, Aug 17, 2016 at 11:35 AM, PrasannaKumar Muralidharan
 wrote:
> This patch adds support for hardware random number generator present in
> JZ4780 SoC.
>
> Signed-off-by: PrasannaKumar Muralidharan 
> ---
>  ...
> +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;
> +}

My bad, I should have spotted this earlier

i686, x86_64 and some ARM will sometimes define a macro indicating
unaligned data access is allowed. For example, see
__ARM_FEATURE_UNALIGNED (cf.,
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0774f/chr1383660321827.html)
. MIPSEL does not define such a macro.

# MIPS ci20 creator with GCC 4.6
$ gcc -march=native -dM -E -  +   u32 *data = buf;
> +   *data = jz4780_rng_readl(jz4780_rng, REG_RNG_DATA);

If GCC emits code that uses the MIPS unaligned load and store
instructions, then there's probably going to be a performance penalty.

Regardless of what the CPU tolerates, I believe unaligned data access
is undefined behavior in C/C++. I believe you should memcpy the value
into the buffer.

Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-19 Thread Jeffrey Walton
On Wed, Aug 17, 2016 at 11:35 AM, 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 000..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@10D8 {
> +   compatible = "ingenic,jz4780-rng";
> +   reg = <0x10D8 0x8>;
> +};
> 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@1000 {
> compatible = "ingenic,jz4780-cgu";
> -   reg = <0x1000 0x100>;
> +   reg = <0x1000 0xD8>;
>
> clocks = <>, <>;
> clock-names = "ext", "rtc";
> @@ -44,6 +44,11 @@
> #clock-cells = <1>;
> };
>
> +   rng: jz4780-rng@10D8 {
> +   compatible = "ingenic,jz4780-rng";
> +   reg = <0x10D8 0x8>;
> +   };
> +
> uart0: serial@1003 {
> compatible = "ingenic,jz4780-uart";
> reg = <0x1003 0x100>;
> 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.
> +
>  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
>  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 000..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.
> + 

Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-18 Thread Rob Herring
On Wed, Aug 17, 2016 at 09:05:51PM +0530, 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 +++

Acked-by: Rob Herring 

>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-18 Thread Daniel Thompson

On 18/08/16 12:53, LABBE Corentin wrote:

On Thu, Aug 18, 2016 at 10:44:18AM +0530, PrasannaKumar Muralidharan wrote:

+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


Data will be 4 bytes.



No, according to comment in include/linux/hw_random.h "drivers can fill up to max 
bytes of data"
So you cannot write more than max bytes without risking buffer overflow.

And if max > 4, hwrng client need to recall your read function.
The better example I found is tpm_get_random() in 
drivers/char/tpm/tpm-interface.c for handling both problem.


Right now the core code will never actually ask a RNG driver for <4 
bytes so perhaps it would be better to update the comment in 
include/linux/hw_random.h !


For devices with 32-bit RNG registers the extra code to handle a special 
case that doesn't actually exist is a waste.


There are 14 drivers in drivers/char/hw_random that support the ->read() 
interface but only three of these actually support max == 1 (existing 
accepted behavior varies between return 0, return 2, return 4 and return 
-EIO).



Daniel.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-18 Thread LABBE Corentin
On Thu, Aug 18, 2016 at 10:44:18AM +0530, PrasannaKumar Muralidharan wrote:
> >> +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
> 
> Data will be 4 bytes.
> 

No, according to comment in include/linux/hw_random.h "drivers can fill up to 
max bytes of data"
So you cannot write more than max bytes without risking buffer overflow.

And if max > 4, hwrng client need to recall your read function.
The better example I found is tpm_get_random() in 
drivers/char/tpm/tpm-interface.c for handling both problem.

Regards

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-17 Thread PrasannaKumar Muralidharan
> I have just some minor comments below

Appreciate your review.

>> diff --git a/drivers/char/hw_random/jz4780-rng.c 
>> b/drivers/char/hw_random/jz4780-rng.c
>> new file mode 100644
>> index 000..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 
>
> You could sort them by alphabetical order

Sure, will do.

>> +
>> +#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

Data will be 4 bytes.

>> +
>> +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(>dev, sizeof(struct jz4780_rng),
>> + GFP_KERNEL);
>
> You could write sizeof(*js480_rng)

Will do.

>> + if (!jz4780_rng)
>> + return -ENOMEM;
>> +
>> + jz4780_rng->dev = >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(>dev, res->start, size);
> You could save code by using devm_ioremap_resource (don't need size)

Will do.

>> + 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(_rng->rng);
>> +
>> + return ret;
>> +}
> You could write directly return hwrng_register(..)

Will do.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-17 Thread Corentin LABBE
Hello

I have just some minor comments below

> diff --git a/drivers/char/hw_random/jz4780-rng.c 
> b/drivers/char/hw_random/jz4780-rng.c
> new file mode 100644
> index 000..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 

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(>dev, sizeof(struct jz4780_rng),
> + GFP_KERNEL);

You could write sizeof(*js480_rng)

> + if (!jz4780_rng)
> + return -ENOMEM;
> +
> + jz4780_rng->dev = >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(>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(_rng->rng);
> +
> + return ret;
> +}
You could write directly return hwrng_register(..)

Regards

LABBE Corentin

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-17 Thread PrasannaKumar Muralidharan
On 17 August 2016 at 21:31, Daniel Thompson  wrote:
> 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 000..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@10D8 {
>> +   compatible = "ingenic,jz4780-rng";
>> +   reg = <0x10D8 0x8>;
>> +};
>
>
> Device tree bindings should be a separate patch. See
> Documentation/devicetree/bindings/submitting-patches.txt .

Sure, Will submit it as a separate patch.

>> 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@1000 {
>> compatible = "ingenic,jz4780-cgu";
>> -   reg = <0x1000 0x100>;
>> +   reg = <0x1000 0xD8>;
>> clocks = <>, <>;
>> clock-names = "ext", "rtc";
>> @@ -44,6 +44,11 @@
>> #clock-cells = <1>;
>> };
>>
>> +   rng: jz4780-rng@10D8 {
>> +   compatible = "ingenic,jz4780-rng";
>> +   reg = <0x10D8 0x8>;
>> +   };
>> +
>> uart0: serial@1003 {
>> compatible = "ingenic,jz4780-uart";
>> reg = <0x1003 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).

Will follow the convention.

>> 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?

I missed it. Will take into account while sending next revision.

>>  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 

Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-17 Thread Daniel Thompson

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 000..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@10D8 {
+   compatible = "ingenic,jz4780-rng";
+   reg = <0x10D8 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@1000 {
compatible = "ingenic,jz4780-cgu";
-   reg = <0x1000 0x100>;
+   reg = <0x1000 0xD8>;
clocks = <>, <>;
clock-names = "ext", "rtc";
@@ -44,6 +44,11 @@
#clock-cells = <1>;
};

+   rng: jz4780-rng@10D8 {
+   compatible = "ingenic,jz4780-rng";
+   reg = <0x10D8 0x8>;
+   };
+
uart0: serial@1003 {
compatible = "ingenic,jz4780-uart";
reg = <0x1003 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 000..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 

[PATCH] Add Ingenic JZ4780 hardware RNG driver

2016-08-17 Thread PrasannaKumar Muralidharan
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 000..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@10D8 {
+   compatible = "ingenic,jz4780-rng";
+   reg = <0x10D8 0x8>;
+};
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@1000 {
compatible = "ingenic,jz4780-cgu";
-   reg = <0x1000 0x100>;
+   reg = <0x1000 0xD8>;
 
clocks = <>, <>;
clock-names = "ext", "rtc";
@@ -44,6 +44,11 @@
#clock-cells = <1>;
};
 
+   rng: jz4780-rng@10D8 {
+   compatible = "ingenic,jz4780-rng";
+   reg = <0x10D8 0x8>;
+   };
+
uart0: serial@1003 {
compatible = "ingenic,jz4780-uart";
reg = <0x1003 0x100>;
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.
+
 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
 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 000..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