[PATCH 0/2] New hardware RNG for Octeon SOCs.

[PATCH 0/2] New hardware RNG for Octeon SOCs.

[David Daney]

Behold the Random Number Generator driver for Octeon!

The first patch adds some port definitions and the octeon_rng platform
device.  The second is the driver.

I am copying AKPM and Linus as there seems to be no hw_random maintainer.

Since Octeon is a mips port, we might want to merge both patches via
Ralf's tree.

Comments?

David Daney (2):
   MIPS: Octeon:  Add hardware RNG platform device.
   hw_random: Add hardware RNG for Octeon SOCs.

  arch/mips/cavium-octeon/setup.c              |   44 ++++++++
  arch/mips/include/asm/octeon/cvmx-rnm-defs.h |   86 +++++++++++++++
  drivers/char/hw_random/Kconfig               |   13 +++
  drivers/char/hw_random/Makefile              |    1 +
  drivers/char/hw_random/octeon-rng.c          |  146 
++++++++++++++++++++++++++
  5 files changed, 290 insertions(+), 0 deletions(-)
  create mode 100644 arch/mips/include/asm/octeon/cvmx-rnm-defs.h
  create mode 100644 drivers/char/hw_random/octeon-rng.c

[PATCH 1/2] MIPS: Octeon: Add hardware RNG platform device.

[David Daney]

Add a platform device for the Octeon Random Number Generator (RNG).

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
 arch/mips/cavium-octeon/setup.c              |   44 +++++++++++++
 arch/mips/include/asm/octeon/cvmx-rnm-defs.h |   86 ++++++++++++++++++++++++++
 2 files changed, 130 insertions(+), 0 deletions(-)
 create mode 100644 arch/mips/include/asm/octeon/cvmx-rnm-defs.h

diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c
index bc0c869..c67487a 100644
--- a/arch/mips/cavium-octeon/setup.c
+++ b/arch/mips/cavium-octeon/setup.c
@@ -33,6 +33,7 @@
 #include <asm/time.h>
 
 #include <asm/octeon/octeon.h>
+#include <asm/octeon/cvmx-rnm-defs.h>
 
 #ifdef CONFIG_CAVIUM_DECODE_RSL
 extern void cvmx_interrupt_rsl_decode(void);
@@ -931,3 +932,46 @@ out:
 	return ret;
 }
 device_initcall(octeon_cf_device_init);
+
+/* Octeon Random Number Generator.  */
+static int __init octeon_rng_device_init(void)
+{
+	struct platform_device *pd;
+	struct resource rng_resources[2];
+	unsigned int res_count;
+	int ret = 0;
+
+	memset(rng_resources, 0, sizeof(rng_resources));
+	res_count = 0;
+	rng_resources[res_count].flags = IORESOURCE_MEM;
+	rng_resources[res_count].start = XKPHYS_TO_PHYS(CVMX_RNM_CTL_STATUS);
+	rng_resources[res_count].end = rng_resources[res_count].start + 0xf;
+	res_count++;
+
+	rng_resources[res_count].flags = IORESOURCE_MEM;
+	rng_resources[res_count].start = cvmx_build_io_address(8, 0);
+	rng_resources[res_count].end = rng_resources[res_count].start + 0x7;
+	res_count++;
+
+	pd = platform_device_alloc("octeon_rng", -1);
+	if (!pd) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = platform_device_add_resources(pd, rng_resources, res_count);
+	if (ret)
+		goto fail;
+
+	ret = platform_device_add(pd);
+	if (ret)
+		goto fail;
+
+	return ret;
+fail:
+	platform_device_put(pd);
+
+out:
+	return ret;
+}
+device_initcall(octeon_rng_device_init);
diff --git a/arch/mips/include/asm/octeon/cvmx-rnm-defs.h b/arch/mips/include/asm/octeon/cvmx-rnm-defs.h
new file mode 100644
index 0000000..a25e2bc
--- /dev/null
+++ b/arch/mips/include/asm/octeon/cvmx-rnm-defs.h
@@ -0,0 +1,86 @@
+/***********************license start***************
+ * Author: Cavium Networks
+ *
+ * Contact: support@caviumnetworks.com
+ * This file is part of the OCTEON SDK
+ *
+ * Copyright (c) 2003-2008 Cavium Networks
+ *
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, Version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This file is distributed in the hope that it will be useful, but
+ * AS-IS and WITHOUT ANY WARRANTY; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE, TITLE, or
+ * NONINFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this file; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ * or visit http://www.gnu.org/licenses/.
+ *
+ * This file may also be available under a different license from Cavium.
+ * Contact Cavium Networks for more information
+ ***********************license end**************************************/
+
+#ifndef __CVMX_RNM_DEFS_H__
+#define __CVMX_RNM_DEFS_H__
+
+#define CVMX_RNM_BIST_STATUS \
+	 CVMX_ADD_IO_SEG(0x0001180040000008ull)
+#define CVMX_RNM_CTL_STATUS \
+	 CVMX_ADD_IO_SEG(0x0001180040000000ull)
+
+union cvmx_rnm_bist_status {
+	uint64_t u64;
+	struct cvmx_rnm_bist_status_s {
+		uint64_t reserved_2_63:62;
+		uint64_t rrc:1;
+		uint64_t mem:1;
+	} s;
+	struct cvmx_rnm_bist_status_s cn30xx;
+	struct cvmx_rnm_bist_status_s cn31xx;
+	struct cvmx_rnm_bist_status_s cn38xx;
+	struct cvmx_rnm_bist_status_s cn38xxp2;
+	struct cvmx_rnm_bist_status_s cn50xx;
+	struct cvmx_rnm_bist_status_s cn52xx;
+	struct cvmx_rnm_bist_status_s cn52xxp1;
+	struct cvmx_rnm_bist_status_s cn56xx;
+	struct cvmx_rnm_bist_status_s cn56xxp1;
+	struct cvmx_rnm_bist_status_s cn58xx;
+	struct cvmx_rnm_bist_status_s cn58xxp1;
+};
+
+union cvmx_rnm_ctl_status {
+	uint64_t u64;
+	struct cvmx_rnm_ctl_status_s {
+		uint64_t reserved_9_63:55;
+		uint64_t ent_sel:4;
+		uint64_t exp_ent:1;
+		uint64_t rng_rst:1;
+		uint64_t rnm_rst:1;
+		uint64_t rng_en:1;
+		uint64_t ent_en:1;
+	} s;
+	struct cvmx_rnm_ctl_status_cn30xx {
+		uint64_t reserved_4_63:60;
+		uint64_t rng_rst:1;
+		uint64_t rnm_rst:1;
+		uint64_t rng_en:1;
+		uint64_t ent_en:1;
+	} cn30xx;
+	struct cvmx_rnm_ctl_status_cn30xx cn31xx;
+	struct cvmx_rnm_ctl_status_cn30xx cn38xx;
+	struct cvmx_rnm_ctl_status_cn30xx cn38xxp2;
+	struct cvmx_rnm_ctl_status_s cn50xx;
+	struct cvmx_rnm_ctl_status_s cn52xx;
+	struct cvmx_rnm_ctl_status_s cn52xxp1;
+	struct cvmx_rnm_ctl_status_s cn56xx;
+	struct cvmx_rnm_ctl_status_s cn56xxp1;
+	struct cvmx_rnm_ctl_status_s cn58xx;
+	struct cvmx_rnm_ctl_status_s cn58xxp1;
+};
+
+#endif
-- 
1.6.0.6

Re: [PATCH 1/2] MIPS: Octeon: Add hardware RNG platform device.

[Andrew Morton]

On Mon, 10 Aug 2009 11:30:24 -0700
David Daney <ddaney@caviumnetworks.com> wrote:

> Add a platform device for the Octeon Random Number Generator (RNG).
> 
> ...
>
>  device_initcall(octeon_cf_device_init);
> +
> +/* Octeon Random Number Generator.  */
> +static int __init octeon_rng_device_init(void)
> +{
> +	struct platform_device *pd;
> +	struct resource rng_resources[2];
> +	unsigned int res_count;
> +	int ret = 0;
> +
> +	memset(rng_resources, 0, sizeof(rng_resources));
> +	res_count = 0;
> +	rng_resources[res_count].flags = IORESOURCE_MEM;
> +	rng_resources[res_count].start = XKPHYS_TO_PHYS(CVMX_RNM_CTL_STATUS);
> +	rng_resources[res_count].end = rng_resources[res_count].start + 0xf;
> +	res_count++;
> +
> +	rng_resources[res_count].flags = IORESOURCE_MEM;
> +	rng_resources[res_count].start = cvmx_build_io_address(8, 0);
> +	rng_resources[res_count].end = rng_resources[res_count].start + 0x7;
> +	res_count++;

You could do

	strut resource rng_resources[2] = {
		{
			.flags = IORESOURCE_MEM,
			.start = XKPHYS_TO_PHYS(CVMX_RNM_CTL_STATUS),
	{etc}

here. 

> +	pd = platform_device_alloc("octeon_rng", -1);
> +	if (!pd) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	ret = platform_device_add_resources(pd, rng_resources, res_count);

use ARRAY_SIZE() here.

> +	if (ret)
> +		goto fail;
> +
> +	ret = platform_device_add(pd);
> +	if (ret)
> +		goto fail;
> +
> +	return ret;
> +fail:
> +	platform_device_put(pd);
> +
> +out:
> +	return ret;
> +}

Or not bother ;)  It doesn't make any difference.

> --- /dev/null
> +++ b/arch/mips/include/asm/octeon/cvmx-rnm-defs.h
>
> ...
>
> +	uint64_t u64;
>
> ...
>

This file should include types.h (at least).

Re: [PATCH 1/2] MIPS: Octeon: Add hardware RNG platform device.

[Andrew Morton]

On Mon, 10 Aug 2009 11:30:24 -0700
David Daney <ddaney@caviumnetworks.com> wrote:

> Add a platform device for the Octeon Random Number Generator (RNG).
> 
> ...
>
>  device_initcall(octeon_cf_device_init);
> +
> +/* Octeon Random Number Generator.  */
> +static int __init octeon_rng_device_init(void)
> +{
> +	struct platform_device *pd;
> +	struct resource rng_resources[2];
> +	unsigned int res_count;
> +	int ret = 0;
> +
> +	memset(rng_resources, 0, sizeof(rng_resources));
> +	res_count = 0;
> +	rng_resources[res_count].flags = IORESOURCE_MEM;
> +	rng_resources[res_count].start = XKPHYS_TO_PHYS(CVMX_RNM_CTL_STATUS);
> +	rng_resources[res_count].end = rng_resources[res_count].start + 0xf;
> +	res_count++;
> +
> +	rng_resources[res_count].flags = IORESOURCE_MEM;
> +	rng_resources[res_count].start = cvmx_build_io_address(8, 0);
> +	rng_resources[res_count].end = rng_resources[res_count].start + 0x7;
> +	res_count++;

You could do

	strut resource rng_resources[2] = {
		{
			.flags = IORESOURCE_MEM,
			.start = XKPHYS_TO_PHYS(CVMX_RNM_CTL_STATUS),
	{etc}

here. 

> +	pd = platform_device_alloc("octeon_rng", -1);
> +	if (!pd) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	ret = platform_device_add_resources(pd, rng_resources, res_count);

use ARRAY_SIZE() here.

> +	if (ret)
> +		goto fail;
> +
> +	ret = platform_device_add(pd);
> +	if (ret)
> +		goto fail;
> +
> +	return ret;
> +fail:
> +	platform_device_put(pd);
> +
> +out:
> +	return ret;
> +}

Or not bother ;)  It doesn't make any difference.

> --- /dev/null
> +++ b/arch/mips/include/asm/octeon/cvmx-rnm-defs.h
>
> ...
>
> +	uint64_t u64;
>
> ...
>

This file should include types.h (at least).

[PATCH 2/2] hw_random: Add hardware RNG for Octeon SOCs.

[David Daney]

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
 drivers/char/hw_random/Kconfig      |   13 +++
 drivers/char/hw_random/Makefile     |    1 +
 drivers/char/hw_random/octeon-rng.c |  146 +++++++++++++++++++++++++++++++++++
 3 files changed, 160 insertions(+), 0 deletions(-)
 create mode 100644 drivers/char/hw_random/octeon-rng.c

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index ce66a70..121b782 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -126,6 +126,19 @@ config HW_RANDOM_OMAP
 
  	  If unsure, say Y.
 
+config HW_RANDOM_OCTEON
+	tristate "Octeon Random Number Generator support"
+	depends on HW_RANDOM && CPU_CAVIUM_OCTEON
+	default HW_RANDOM
+ 	---help---
+ 	  This driver provides kernel-side support for the Random Number
+	  Generator hardware found on Octeon processors.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called octeon-rng.
+
+ 	  If unsure, say Y.
+
 config HW_RANDOM_PASEMI
 	tristate "PA Semi HW Random Number Generator support"
 	depends on HW_RANDOM && PPC_PASEMI
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 676828b..5eeb130 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_HW_RANDOM_PASEMI) += pasemi-rng.o
 obj-$(CONFIG_HW_RANDOM_VIRTIO) += virtio-rng.o
 obj-$(CONFIG_HW_RANDOM_TX4939) += tx4939-rng.o
 obj-$(CONFIG_HW_RANDOM_MXC_RNGA) += mxc-rnga.o
+obj-$(CONFIG_HW_RANDOM_OCTEON) += octeon-rng.o
diff --git a/drivers/char/hw_random/octeon-rng.c b/drivers/char/hw_random/octeon-rng.c
new file mode 100644
index 0000000..84d33a7
--- /dev/null
+++ b/drivers/char/hw_random/octeon-rng.c
@@ -0,0 +1,146 @@
+/*
+ * Hardware Random Number Generator support for Cavium Networks
+ * Octeon processor family.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2009 Cavium Networks
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/hw_random.h>
+#include <linux/io.h>
+
+#include <asm/octeon/octeon.h>
+#include <asm/octeon/cvmx-rnm-defs.h>
+
+struct octeon_rng {
+	u64 control_status;
+	u64 result;
+};
+
+static int octeon_rng_init(struct hwrng *rng)
+{
+	struct octeon_rng *p = (struct octeon_rng *)rng->priv;
+	union cvmx_rnm_ctl_status ctl;
+
+	ctl.u64 = 0;
+	ctl.s.ent_en = 1; /* Enable the entropy source.  */
+	ctl.s.rng_en = 1; /* Enable the RNG hardware.  */
+	cvmx_write_csr(p->control_status, ctl.u64);
+	return 0;
+}
+
+static void octeon_rng_cleanup(struct hwrng *rng)
+{
+	struct octeon_rng *p = (struct octeon_rng *)rng->priv;
+	union cvmx_rnm_ctl_status ctl;
+
+	ctl.u64 = 0;
+	/* Disable everything.  */
+	cvmx_write_csr(p->control_status, ctl.u64);
+}
+
+static int octeon_rng_data_read(struct hwrng *rng, u32 *data)
+{
+	struct octeon_rng *p = (struct octeon_rng *)rng->priv;
+
+	*data = cvmx_read64_uint32(p->result);
+	return sizeof(u32);
+}
+
+static struct hwrng octeon_rng_ops = {
+	.name		= "octeon",
+	.init		= octeon_rng_init,
+	.cleanup	= octeon_rng_cleanup,
+	.data_read	= octeon_rng_data_read
+};
+
+static int __devinit octeon_rng_probe(struct platform_device *pdev)
+{
+	struct resource *res_ports;
+	struct resource *res_result;
+	struct octeon_rng *p;
+	int ret;
+
+	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	res_ports = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res_ports)
+		goto err_ports;
+
+	res_result = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res_result)
+		goto err_ports;
+
+
+	p->control_status = (u64)devm_ioremap_nocache(&pdev->dev,
+						      res_ports->start,
+						      sizeof(u64));
+	if (!p->control_status)
+		goto err_ports;
+
+	p->result = (u64)devm_ioremap_nocache(&pdev->dev,
+					      res_result->start,
+					      sizeof(u64));
+	if (!p->result)
+		goto err_r;
+	octeon_rng_ops.priv = (unsigned long)p;
+
+	dev_set_drvdata(&pdev->dev, &octeon_rng_ops);
+	ret = hwrng_register(&octeon_rng_ops);
+	if (ret)
+		goto err;
+
+	dev_info(&pdev->dev, "Octeon Random Number Generator\n");
+
+	return 0;
+err:
+	devm_iounmap(&pdev->dev, (void *)p->control_status);
+err_r:
+	devm_iounmap(&pdev->dev, (void *)p->result);
+err_ports:
+	devm_kfree(&pdev->dev, p);
+	return -ENOENT;
+}
+
+static int __exit octeon_rng_remove(struct platform_device *pdev)
+{
+	struct hwrng *rng = dev_get_drvdata(&pdev->dev);
+
+	hwrng_unregister(rng);
+
+	return 0;
+}
+
+static struct platform_driver octeon_rng_driver = {
+	.driver = {
+		.name		= "octeon_rng",
+		.owner		= THIS_MODULE,
+	},
+	.probe		= octeon_rng_probe,
+	.remove		= __exit_p(octeon_rng_remove),
+};
+
+static int __init octeon_rng_mod_init(void)
+{
+	return platform_driver_register(&octeon_rng_driver);
+}
+
+static void __exit octeon_rng_mod_exit(void)
+{
+	platform_driver_unregister(&octeon_rng_driver);
+}
+
+module_init(octeon_rng_mod_init);
+module_exit(octeon_rng_mod_exit);
+
+MODULE_AUTHOR("David Daney");
+MODULE_LICENSE("GPL");
-- 
1.6.0.6

Re: [PATCH 2/2] hw_random: Add hardware RNG for Octeon SOCs.

[Andrew Morton]

On Mon, 10 Aug 2009 11:30:25 -0700
David Daney <ddaney@caviumnetworks.com> wrote:
>

Now what's going on with all this typecasting?

> diff --git a/drivers/char/hw_random/octeon-rng.c b/drivers/char/hw_random/octeon-rng.c
> new file mode 100644
> index 0000000..84d33a7
> --- /dev/null
> +++ b/drivers/char/hw_random/octeon-rng.c
> @@ -0,0 +1,146 @@
> +/*
> + * Hardware Random Number Generator support for Cavium Networks
> + * Octeon processor family.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 2009 Cavium Networks
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/device.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +
> +#include <asm/octeon/octeon.h>
> +#include <asm/octeon/cvmx-rnm-defs.h>
> +
> +struct octeon_rng {
> +	u64 control_status;
> +	u64 result;
> +};
> +
> +static int octeon_rng_init(struct hwrng *rng)
> +{
> +	struct octeon_rng *p = (struct octeon_rng *)rng->priv;

Here it's unavoidable because some bad person went and made hwrng.priv
an `unsigned long'.  I haven't checked, but I bet it should have been a
void*.

> +	union cvmx_rnm_ctl_status ctl;
> +
> +	ctl.u64 = 0;
> +	ctl.s.ent_en = 1; /* Enable the entropy source.  */
> +	ctl.s.rng_en = 1; /* Enable the RNG hardware.  */
> +	cvmx_write_csr(p->control_status, ctl.u64);
> +	return 0;
> +}
> +
> +static void octeon_rng_cleanup(struct hwrng *rng)
> +{
> +	struct octeon_rng *p = (struct octeon_rng *)rng->priv;
> +	union cvmx_rnm_ctl_status ctl;
> +
> +	ctl.u64 = 0;
> +	/* Disable everything.  */
> +	cvmx_write_csr(p->control_status, ctl.u64);
> +}
> +
> +static int octeon_rng_data_read(struct hwrng *rng, u32 *data)
> +{
> +	struct octeon_rng *p = (struct octeon_rng *)rng->priv;
> +
> +	*data = cvmx_read64_uint32(p->result);
> +	return sizeof(u32);
> +}
> +
> +static struct hwrng octeon_rng_ops = {
> +	.name		= "octeon",
> +	.init		= octeon_rng_init,
> +	.cleanup	= octeon_rng_cleanup,
> +	.data_read	= octeon_rng_data_read
> +};
> +
> +static int __devinit octeon_rng_probe(struct platform_device *pdev)
> +{
> +	struct resource *res_ports;
> +	struct resource *res_result;
> +	struct octeon_rng *p;
> +	int ret;
> +
> +	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	res_ports = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res_ports)
> +		goto err_ports;
> +
> +	res_result = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (!res_result)
> +		goto err_ports;
> +
> +
> +	p->control_status = (u64)devm_ioremap_nocache(&pdev->dev,
> +						      res_ports->start,
> +						      sizeof(u64));

devm_ioremap_nocache() returns a `void __iomem *'.  Hence we should be
recording that value in a field of type `void __iomem *'.  Instead,
we're wedging it into a u64.

Something went wrong!

> +	if (!p->control_status)
> +		goto err_ports;
> +
> +	p->result = (u64)devm_ioremap_nocache(&pdev->dev,
> +					      res_result->start,
> +					      sizeof(u64));

Ditto.

> +	if (!p->result)
> +		goto err_r;
> +	octeon_rng_ops.priv = (unsigned long)p;

The hwrng.priv problem again.

> +	dev_set_drvdata(&pdev->dev, &octeon_rng_ops);
> +	ret = hwrng_register(&octeon_rng_ops);
> +	if (ret)
> +		goto err;
> +
> +	dev_info(&pdev->dev, "Octeon Random Number Generator\n");
> +
> +	return 0;
> +err:
> +	devm_iounmap(&pdev->dev, (void *)p->control_status);
> +err_r:
> +	devm_iounmap(&pdev->dev, (void *)p->result);
> +err_ports:
> +	devm_kfree(&pdev->dev, p);
> +	return -ENOENT;
> +}
> +
> +static int __exit octeon_rng_remove(struct platform_device *pdev)
> +{
> +	struct hwrng *rng = dev_get_drvdata(&pdev->dev);
> +
> +	hwrng_unregister(rng);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver octeon_rng_driver = {
> +	.driver = {
> +		.name		= "octeon_rng",
> +		.owner		= THIS_MODULE,
> +	},
> +	.probe		= octeon_rng_probe,
> +	.remove		= __exit_p(octeon_rng_remove),
> +};
> +
> +static int __init octeon_rng_mod_init(void)
> +{
> +	return platform_driver_register(&octeon_rng_driver);
> +}
> +
> +static void __exit octeon_rng_mod_exit(void)
> +{
> +	platform_driver_unregister(&octeon_rng_driver);
> +}
> +
> +module_init(octeon_rng_mod_init);
> +module_exit(octeon_rng_mod_exit);
> +
> +MODULE_AUTHOR("David Daney");
> +MODULE_LICENSE("GPL");

Please take another look and check that the selection of types was as
good as it could possibly be?

Also, let's all send rude emails to Herbert over the type of hwrng.priv ;)

Re: [PATCH 2/2] hw_random: Add hardware RNG for Octeon SOCs.

[Andrew Morton]

On Mon, 10 Aug 2009 11:30:25 -0700
David Daney <ddaney@caviumnetworks.com> wrote:
>

Now what's going on with all this typecasting?

> diff --git a/drivers/char/hw_random/octeon-rng.c b/drivers/char/hw_random/octeon-rng.c
> new file mode 100644
> index 0000000..84d33a7
> --- /dev/null
> +++ b/drivers/char/hw_random/octeon-rng.c
> @@ -0,0 +1,146 @@
> +/*
> + * Hardware Random Number Generator support for Cavium Networks
> + * Octeon processor family.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 2009 Cavium Networks
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/device.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +
> +#include <asm/octeon/octeon.h>
> +#include <asm/octeon/cvmx-rnm-defs.h>
> +
> +struct octeon_rng {
> +	u64 control_status;
> +	u64 result;
> +};
> +
> +static int octeon_rng_init(struct hwrng *rng)
> +{
> +	struct octeon_rng *p = (struct octeon_rng *)rng->priv;

Here it's unavoidable because some bad person went and made hwrng.priv
an `unsigned long'.  I haven't checked, but I bet it should have been a
void*.

> +	union cvmx_rnm_ctl_status ctl;
> +
> +	ctl.u64 = 0;
> +	ctl.s.ent_en = 1; /* Enable the entropy source.  */
> +	ctl.s.rng_en = 1; /* Enable the RNG hardware.  */
> +	cvmx_write_csr(p->control_status, ctl.u64);
> +	return 0;
> +}
> +
> +static void octeon_rng_cleanup(struct hwrng *rng)
> +{
> +	struct octeon_rng *p = (struct octeon_rng *)rng->priv;
> +	union cvmx_rnm_ctl_status ctl;
> +
> +	ctl.u64 = 0;
> +	/* Disable everything.  */
> +	cvmx_write_csr(p->control_status, ctl.u64);
> +}
> +
> +static int octeon_rng_data_read(struct hwrng *rng, u32 *data)
> +{
> +	struct octeon_rng *p = (struct octeon_rng *)rng->priv;
> +
> +	*data = cvmx_read64_uint32(p->result);
> +	return sizeof(u32);
> +}
> +
> +static struct hwrng octeon_rng_ops = {
> +	.name		= "octeon",
> +	.init		= octeon_rng_init,
> +	.cleanup	= octeon_rng_cleanup,
> +	.data_read	= octeon_rng_data_read
> +};
> +
> +static int __devinit octeon_rng_probe(struct platform_device *pdev)
> +{
> +	struct resource *res_ports;
> +	struct resource *res_result;
> +	struct octeon_rng *p;
> +	int ret;
> +
> +	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	res_ports = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res_ports)
> +		goto err_ports;
> +
> +	res_result = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (!res_result)
> +		goto err_ports;
> +
> +
> +	p->control_status = (u64)devm_ioremap_nocache(&pdev->dev,
> +						      res_ports->start,
> +						      sizeof(u64));

devm_ioremap_nocache() returns a `void __iomem *'.  Hence we should be
recording that value in a field of type `void __iomem *'.  Instead,
we're wedging it into a u64.

Something went wrong!

> +	if (!p->control_status)
> +		goto err_ports;
> +
> +	p->result = (u64)devm_ioremap_nocache(&pdev->dev,
> +					      res_result->start,
> +					      sizeof(u64));

Ditto.

> +	if (!p->result)
> +		goto err_r;
> +	octeon_rng_ops.priv = (unsigned long)p;

The hwrng.priv problem again.

> +	dev_set_drvdata(&pdev->dev, &octeon_rng_ops);
> +	ret = hwrng_register(&octeon_rng_ops);
> +	if (ret)
> +		goto err;
> +
> +	dev_info(&pdev->dev, "Octeon Random Number Generator\n");
> +
> +	return 0;
> +err:
> +	devm_iounmap(&pdev->dev, (void *)p->control_status);
> +err_r:
> +	devm_iounmap(&pdev->dev, (void *)p->result);
> +err_ports:
> +	devm_kfree(&pdev->dev, p);
> +	return -ENOENT;
> +}
> +
> +static int __exit octeon_rng_remove(struct platform_device *pdev)
> +{
> +	struct hwrng *rng = dev_get_drvdata(&pdev->dev);
> +
> +	hwrng_unregister(rng);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver octeon_rng_driver = {
> +	.driver = {
> +		.name		= "octeon_rng",
> +		.owner		= THIS_MODULE,
> +	},
> +	.probe		= octeon_rng_probe,
> +	.remove		= __exit_p(octeon_rng_remove),
> +};
> +
> +static int __init octeon_rng_mod_init(void)
> +{
> +	return platform_driver_register(&octeon_rng_driver);
> +}
> +
> +static void __exit octeon_rng_mod_exit(void)
> +{
> +	platform_driver_unregister(&octeon_rng_driver);
> +}
> +
> +module_init(octeon_rng_mod_init);
> +module_exit(octeon_rng_mod_exit);
> +
> +MODULE_AUTHOR("David Daney");
> +MODULE_LICENSE("GPL");

Please take another look and check that the selection of types was as
good as it could possibly be?

Also, let's all send rude emails to Herbert over the type of hwrng.priv ;)

Re: [PATCH 0/2] New hardware RNG for Octeon SOCs.

[Matt Mackall]

On Mon, 2009-08-10 at 11:21 -0700, David Daney wrote:
> Behold the Random Number Generator driver for Octeon!
> 
> The first patch adds some port definitions and the octeon_rng platform
> device.  The second is the driver.
> 
> I am copying AKPM and Linus as there seems to be no hw_random maintainer.

These now go through Herbert Xu and I.

-- 
http://selenic.com : development and support for Mercurial and Linux

Re: [PATCH 0/2] New hardware RNG for Octeon SOCs.

[David Daney]

Matt Mackall wrote:
> On Mon, 2009-08-10 at 11:21 -0700, David Daney wrote:
>> Behold the Random Number Generator driver for Octeon!
>>
>> The first patch adds some port definitions and the octeon_rng platform
>> device.  The second is the driver.
>>
>> I am copying AKPM and Linus as there seems to be no hw_random maintainer.
> 
> These now go through Herbert Xu and I.

I had been advised of this after I sent the messages.  Perhaps an entry 
in MAINTAINERS is in order.

David Daney

[PATCH] MAINTAINERS: Add Matt Mackall and Herbert Xu to HARDWARE RANDOM NUMBER GENERATOR

[Joe Perches]

On Mon, 2009-08-10 at 14:54 -0500, Matt Mackall wrote:
> On Mon, 2009-08-10 at 11:21 -0700, David Daney wrote:
> > I am copying AKPM and Linus as there seems to be no hw_random maintainer.
> These now go through Herbert Xu and I.

Perhaps this then:

Signed-off-by: Joe Perches <joe@perches.com>

diff --git a/MAINTAINERS b/MAINTAINERS
index b1114cf..9a27822 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2260,7 +2260,9 @@ S:	Orphan
 F:	drivers/hwmon/
 
 HARDWARE RANDOM NUMBER GENERATOR CORE
-S:	Orphan
+M:	Matt Mackall <mpm@selenic.com>
+M:	Herbert Xu <herbert@gondor.apana.org.au>
+S:	Odd fixes
 F:	Documentation/hw_random.txt
 F:	drivers/char/hw_random/
 F:	include/linux/hw_random.h

Re: [PATCH 0/2] New hardware RNG for Octeon SOCs.

[Andrew Morton]

On Mon, 10 Aug 2009 11:21:29 -0700
David Daney <ddaney@caviumnetworks.com> wrote:

> Behold the Random Number Generator driver for Octeon!

Hail!

> The first patch adds some port definitions and the octeon_rng platform
> device.  The second is the driver.
> 
> I am copying AKPM and Linus as there seems to be no hw_random maintainer.
> 
> Since Octeon is a mips port, we might want to merge both patches via
> Ralf's tree.
> 
> Comments?

Looks OK to me - I had a couple of minor comments.  Please send the
patches to Herbert and Ralf - I'd normally expect one of those two
gents to be the merge path for this work.