Skip to contentSkip to conversation

[2.6.22] pasemi: hardware rng driver

PATCH In review Olof Johansson 3 participants 9 messages Apr 25, 2007 – Apr 26, 2007

Subsystems (2)
PowerPC Hardware random number generator core
lore
Share & tools
lore

Olof Johansson

author
#0 lore raw

PATCH 1/1 [2.6.22] pasemi: hardware rng driver

Driver for the on-chip hardware random number generator on PA Semi PA6T-1682M.

Signed-off-by Egor Martovetsky <[email protected]>
Signed-off-by Olof Johansson <[email protected]>
drivers/char/hw_random/Kconfig +14 -0
Index: powerpc/drivers/char/hw_random/Kconfig
===================================================================
@@ -91,3 +91,17 @@ config HW_RANDOM_OMAP
 	  module will be called omap-rng.
 
  	  If unsure, say Y.
+
+config HW_RANDOM_PASEMI
+	tristate "PA Semi HW Random Number Generator support"
+	depends on HW_RANDOM && PPC_PASEMI
+	default HW_RANDOM
+	---help---
+	  This driver provides kernel-side support for the Random Number
+	  Generator hardware found on PA6T-1682M processor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called pasemi-rng.
+
+	  If unsure, say Y.
+
drivers/char/hw_random/Makefile +1 -0
Index: powerpc/drivers/char/hw_random/Makefile
===================================================================
@@ -10,3 +10,4 @@ obj-$(CONFIG_HW_RANDOM_GEODE) += geode-r
 obj-$(CONFIG_HW_RANDOM_VIA) += via-rng.o
 obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o
 obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o
+obj-$(CONFIG_HW_RANDOM_PASEMI) += pasemi-rng.o
drivers/char/hw_random/pasemi-rng.c new file +152 -0

152 lines not on this page · raw message

#1 lore raw

drivers/char/hw_random/pasemi-rng.c line 50

Quoted 7 lines · Olof Johansson
drivers/char/hw_random/pasemi-rng.c new file +7 -0
+static void __iomem *rng_regs;
+
+static int pasemi_rng_data_present(struct hwrng *rng)
+{
+	return (in_le32(rng_regs + SDCRNG_CTL_REG)
+		& SDCRNG_CTL_FVLD_M) ? 1 : 0;
+}

It would be nicer to get rid of the global rng_regs variable by sticking it into rng->priv.

drivers/char/hw_random/pasemi-rng.c line 95

Quoted 10 lines
drivers/char/hw_random/pasemi-rng.c new file +10 -0
+static int __devinit rng_probe(struct of_device *ofdev,
+				     const struct of_device_id *match)
+{
+	struct device_node *rng_np;
+	struct resource res;
+	int err = 0;
+
+	rng_np = of_find_compatible_node(NULL, "rng", "1682m-rng");
+	if (!rng_np)
+		return -ENODEV;

I would guess that the call to of_find_compatible_node is entirely bogus here, because the device is already passed in as ofdev in to the probe function.

drivers/char/hw_random/pasemi-rng.c line 148

Quoted 11 lines
drivers/char/hw_random/pasemi-rng.c new file +11 -0
+int rng_init(void)
+{
+       return of_register_platform_driver(&rng_driver);
+}
+
+void rng_exit(void)
+{
+       of_unregister_platform_driver(&rng_driver);
+}
+
+device_initcall(rng_init);

rng_init and rng_exit should be static
rng_init should be __init
rng_exit should be __exit
Since the driver is tristate in Kconfig, it would be more conventional to use module_init() instead of device_initcall().
rng_exit needs to be marked as module_exit() to allow unloading the driver.

	Arnd <><
author
#2 lore raw
Quoted 12 lines · Arnd Bergmann

drivers/char/hw_random/pasemi-rng.c line 50

Quoted 7 lines · Olof Johansson
drivers/char/hw_random/pasemi-rng.c new file +7 -0
+static void __iomem *rng_regs;
+
+static int pasemi_rng_data_present(struct hwrng *rng)
+{
+	return (in_le32(rng_regs + SDCRNG_CTL_REG)
+		& SDCRNG_CTL_FVLD_M) ? 1 : 0;
+}

It would be nicer to get rid of the global rng_regs variable by sticking it into rng->priv.

Good point.

Quoted 6 lines
Quoted 6 lines
drivers/char/hw_random/pasemi-rng.c new file +6 -0
+static int __devinit rng_probe(struct of_device *ofdev,
+				     const struct of_device_id *match)
+{
+	struct device_node *rng_np;
+	struct resource res;
+	int err = 0;
Quoted 8 lines

drivers/char/hw_random/pasemi-rng.c line 95

Quoted 4 lines
drivers/char/hw_random/pasemi-rng.c new file +4 -0
+
+	rng_np = of_find_compatible_node(NULL, "rng", "1682m-rng");
+	if (!rng_np)
+		return -ENODEV;

I would guess that the call to of_find_compatible_node is entirely bogus here, because the device is already passed in as ofdev in to the probe function.

Yup. Leftover from pre-of_platform

Quoted 10 lines
Quoted 10 lines
drivers/char/hw_random/pasemi-rng.c new file +10 -0
+int rng_init(void)
+{
+       return of_register_platform_driver(&rng_driver);
+}
+
+void rng_exit(void)
+{
+       of_unregister_platform_driver(&rng_driver);
+}
+
Quoted 8 lines
Quoted 1 line

+device_initcall(rng_init);

rng_init and rng_exit should be static
rng_init should be __init
rng_exit should be __exit
Since the driver is tristate in Kconfig, it would be more conventional to use module_init() instead of device_initcall().
rng_exit needs to be marked as module_exit() to allow unloading the driver.

Yup, will fix.

Thanks.

-Olof

#3 lore raw
In reply to Arnd Bergmann
Quoted 12 lines · On Thursday 26 April 2007 01:38:31 Arnd Bergmann wrote:

drivers/char/hw_random/pasemi-rng.c line 50

Quoted 7 lines · Olof Johansson
drivers/char/hw_random/pasemi-rng.c new file +7 -0
+static void __iomem *rng_regs;
+
+static int pasemi_rng_data_present(struct hwrng *rng)
+{
+	return (in_le32(rng_regs + SDCRNG_CTL_REG)
+		& SDCRNG_CTL_FVLD_M) ? 1 : 0;
+}

It would be nicer to get rid of the global rng_regs variable by sticking it into rng->priv.

Yeah, I think that would be better. Saves one global variable.

Signature · 1 line
Greetings Michael.
author
#4 lore raw

PATCH v2 1/1 [2.6.22] pasemi: hardware rng driver

In reply to Olof Johansson

Driver for the on-chip hardware random number generator on PA Semi PA6T-1682M.

Signed-off-by Egor Martovetsky <[email protected]>
Signed-off-by Olof Johansson <[email protected]>

---

Changes since last version:
* Moved register pointer to hwrng->priv
* Cleanups (__init/__exit, module_init/exit)

drivers/char/hw_random/Kconfig +14 -0
Index: powerpc/drivers/char/hw_random/Kconfig
===================================================================
@@ -91,3 +91,17 @@ config HW_RANDOM_OMAP
 	  module will be called omap-rng.
 
  	  If unsure, say Y.
+
+config HW_RANDOM_PASEMI
+	tristate "PA Semi HW Random Number Generator support"
+	depends on HW_RANDOM && PPC_PASEMI
+	default HW_RANDOM
+	---help---
+	  This driver provides kernel-side support for the Random Number
+	  Generator hardware found on PA6T-1682M processor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called pasemi-rng.
+
+	  If unsure, say Y.
+
drivers/char/hw_random/Makefile +1 -0
Index: powerpc/drivers/char/hw_random/Makefile
===================================================================
@@ -10,3 +10,4 @@ obj-$(CONFIG_HW_RANDOM_GEODE) += geode-r
 obj-$(CONFIG_HW_RANDOM_VIA) += via-rng.o
 obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o
 obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o
+obj-$(CONFIG_HW_RANDOM_PASEMI) += pasemi-rng.o
drivers/char/hw_random/pasemi-rng.c new file +150 -0

150 lines not on this page · raw message

author
#6 lore raw

PATCH v3 1/1 [2.6.22] pasemi: hardware rng driver

In reply to Olof Johansson

Driver for the on-chip hardware random number generator on PA Semi PA6T-1682M.

Signed-off-by Egor Martovetsky <[email protected]>
Signed-off-by Olof Johansson <[email protected]>

---

v3:
* error return codes
* iounmap on register fail
* fix iounmap race on remove

v2:
* Moved register pointer to hwrng->priv
* Cleanups (__init/__exit, module_init/exit)

drivers/char/hw_random/Kconfig +14 -0
Index: powerpc/drivers/char/hw_random/Kconfig
===================================================================
@@ -91,3 +91,17 @@ config HW_RANDOM_OMAP
 	  module will be called omap-rng.
 
  	  If unsure, say Y.
+
+config HW_RANDOM_PASEMI
+	tristate "PA Semi HW Random Number Generator support"
+	depends on HW_RANDOM && PPC_PASEMI
+	default HW_RANDOM
+	---help---
+	  This driver provides kernel-side support for the Random Number
+	  Generator hardware found on PA6T-1682M processor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called pasemi-rng.
+
+	  If unsure, say Y.
+
drivers/char/hw_random/Makefile +1 -0
Index: powerpc/drivers/char/hw_random/Makefile
===================================================================
@@ -10,3 +10,4 @@ obj-$(CONFIG_HW_RANDOM_GEODE) += geode-r
 obj-$(CONFIG_HW_RANDOM_VIA) += via-rng.o
 obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o
 obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o
+obj-$(CONFIG_HW_RANDOM_PASEMI) += pasemi-rng.o
drivers/char/hw_random/pasemi-rng.c new file +158 -0

158 lines not on this page · raw message

#7 lore raw
In reply to Olof Johansson
Quoted 6 lines · On Wednesday 25 April 2007 22:45:12 Olof Johansson wrote:
drivers/char/hw_random/pasemi-rng.c new file +6 -0
+static int __devinit rng_probe(struct of_device *ofdev,
+				     const struct of_device_id *match)
+{
+	struct device_node *rng_np;
+	struct resource res;
+	int err = 0;

drivers/char/hw_random/pasemi-rng.c line 101

Quoted 10 lines
drivers/char/hw_random/pasemi-rng.c new file +10 -0
+
+	rng_np = of_find_compatible_node(NULL, "rng", "1682m-rng");
+	if (!rng_np)
+		return -ENODEV;
+
+	err = of_address_to_resource(rng_np, 0, &res);
+	of_node_put(rng_np);
+
+	if (err)
+		return -EINVAL;

I think EINVAL is not the correct error code. I'd suggest ENODEV.

drivers/char/hw_random/pasemi-rng.c line 107

Quoted 5 lines
drivers/char/hw_random/pasemi-rng.c new file +5 -0
+	if (!rng_regs)
+		rng_regs = ioremap(res.start, 0x100);
+
+	if (!rng_regs)
+		return -EPERM;

I think EPERM is not the correct error code. I'd suggest ENOMEM.

drivers/char/hw_random/pasemi-rng.c line 111

Quoted 3 lines
drivers/char/hw_random/pasemi-rng.c new file +3 -0
+	printk(KERN_INFO "Registering PA Semi RNG\n");
+
+	return hwrng_register(&pasemi_rng);

Resource leak.
Please do something like

	err = hwrng_register(&pasemi_rng);
	if (err)
		iounmap(rng_regs);
	return err;

drivers/char/hw_random/pasemi-rng.c line 117

Quoted 6 lines
drivers/char/hw_random/pasemi-rng.c new file +6 -0
+}
+
+static int rng_remove(struct of_device *dev)
+{
+	iounmap(rng_regs);
+	hwrng_unregister(&pasemi_rng);

Swap these to prevent race conditions.

Quoted 3 lines
drivers/char/hw_random/pasemi-rng.c new file +3 -0
+
+	return 0;
+}
Signature · 1 line
Greetings Michael.
author
#8 lore raw

Hi Michael,

All good points, fixed in v3 (posted shortly).

Thanks,

-Olof

Quoted 57 lines · Michael Buesch
Quoted 6 lines · On Wednesday 25 April 2007 22:45:12 Olof Johansson wrote:
drivers/char/hw_random/pasemi-rng.c new file +6 -0
+static int __devinit rng_probe(struct of_device *ofdev,
+				     const struct of_device_id *match)
+{
+	struct device_node *rng_np;
+	struct resource res;
+	int err = 0;

drivers/char/hw_random/pasemi-rng.c line 101

Quoted 10 lines
drivers/char/hw_random/pasemi-rng.c new file +10 -0
+
+	rng_np = of_find_compatible_node(NULL, "rng", "1682m-rng");
+	if (!rng_np)
+		return -ENODEV;
+
+	err = of_address_to_resource(rng_np, 0, &res);
+	of_node_put(rng_np);
+
+	if (err)
+		return -EINVAL;

I think EINVAL is not the correct error code. I'd suggest ENODEV.

drivers/char/hw_random/pasemi-rng.c line 107

Quoted 5 lines
drivers/char/hw_random/pasemi-rng.c new file +5 -0
+	if (!rng_regs)
+		rng_regs = ioremap(res.start, 0x100);
+
+	if (!rng_regs)
+		return -EPERM;

I think EPERM is not the correct error code. I'd suggest ENOMEM.

drivers/char/hw_random/pasemi-rng.c line 111

Quoted 3 lines
drivers/char/hw_random/pasemi-rng.c new file +3 -0
+	printk(KERN_INFO "Registering PA Semi RNG\n");
+
+	return hwrng_register(&pasemi_rng);

Resource leak.
Please do something like

	err = hwrng_register(&pasemi_rng);
	if (err)
		iounmap(rng_regs);
	return err;

drivers/char/hw_random/pasemi-rng.c line 117

Quoted 6 lines
drivers/char/hw_random/pasemi-rng.c new file +6 -0
+}
+
+static int rng_remove(struct of_device *dev)
+{
+	iounmap(rng_regs);
+	hwrng_unregister(&pasemi_rng);

Swap these to prevent race conditions.

Quoted 3 lines
drivers/char/hw_random/pasemi-rng.c new file +3 -0
+
+	return 0;
+}
Signature · 1 line
Greetings Michael.