Re: [PATCH] hw_random: Add Broadcom BCM2835 RNG Driver

2013-03-28 Thread Lubomir Rintel
On Fri, 2013-03-22 at 20:44 -0600, Stephen Warren wrote:
> On 03/22/2013 06:55 AM, Lubomir Rintel wrote:
> > Signed-off-by: Lubomir Rintel 
> 
> A commit description would be useful.
> 
> >  arch/arm/boot/dts/bcm2835.dtsi   |5 +
> >  arch/arm/configs/bcm2835_defconfig   |3 +-
> >  drivers/char/hw_random/Kconfig   |   12 +++
> >  drivers/char/hw_random/Makefile  |1 +
> >  drivers/char/hw_random/bcm2835-rng.c |  137 
> > ++
> 
> This should be split into 3 separate patches: (1) The driver itself, (2)
> the change to bcm2835.dtsi, and (3) the change to bcm2835_defconfig.
> 
> Since you're adding a new device to device tree for the first time, you
> should write a binding document for it; most likely
> Documentation/devicetree/bindings/rng/brcm,bcm2835.txt (or perhaps
> /random/ rather than /rng/?)
> 
> Is this driver based on the downstream Raspberry Pi kernel's driver? If
> so, some mention of that fact would be appropriate in the commit
> description, or even the git author field. I note that in the downstream
> kernel, the commit which adds the RNG driver isn't signed off at all.
> This probably means you need to get Dom to add his signed-off-by line
> for that commit before basing your work on it. See
> Documentation/SubmittingPatches for more details.

Got this message, will follow up with updated patch:

On Wed, 2013-03-27 at 18:22 +, popcornmix wrote:
On Wed, Mar 27, 2013 at 4:55 PM, Lubomir Rintel  wrote:
> > Hi!
> >
> > I'm currently in the process of mainlining the drivers from
Raspberry Pi
> > tree. I was asked for a signoff for the random number generator. It
> > seems to have been commit by you in the downstream tree, I'm
wondering
> > if you could help me, and respond with a Signed-off-by tag if the
code
> > comes from you, or point me to someone else?
> >
> > commit e95a8204d7f8fc4f38900c99080103254c3cef11
> > Author: popcornmix 
> > Date:   Wed Jan 30 11:44:26 2013 +
> >
> > Add hwrng (hardware random number generator) driver
> >
> > Thank you!
> > --
> > Lubomir Rintel 
> >
> 
> Sorry, I'm not up to speed with exactly what constitutes a sign off.
> I did write the original hw-rng driver, and give my permission for it
> to be upstreamed.
> Is this sufficient?
>  Signed-off-by: Dom Cobley 
>  Signed-off-by: Dom Cobley 
>  Signed-off-by: popcornmix 
> 
> (not sure which is more suitable - they are all me).

-- 
Lubomir Rintel (ext. #7715)
GoodData Code Feng Shui Critic

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


Re: [PATCH] hw_random: Add Broadcom BCM2835 RNG Driver

2013-03-28 Thread Lubomir Rintel
On Fri, 2013-03-22 at 20:44 -0600, Stephen Warren wrote:
 On 03/22/2013 06:55 AM, Lubomir Rintel wrote:
  Signed-off-by: Lubomir Rintel lkund...@v3.sk
 
 A commit description would be useful.
 
   arch/arm/boot/dts/bcm2835.dtsi   |5 +
   arch/arm/configs/bcm2835_defconfig   |3 +-
   drivers/char/hw_random/Kconfig   |   12 +++
   drivers/char/hw_random/Makefile  |1 +
   drivers/char/hw_random/bcm2835-rng.c |  137 
  ++
 
 This should be split into 3 separate patches: (1) The driver itself, (2)
 the change to bcm2835.dtsi, and (3) the change to bcm2835_defconfig.
 
 Since you're adding a new device to device tree for the first time, you
 should write a binding document for it; most likely
 Documentation/devicetree/bindings/rng/brcm,bcm2835.txt (or perhaps
 /random/ rather than /rng/?)
 
 Is this driver based on the downstream Raspberry Pi kernel's driver? If
 so, some mention of that fact would be appropriate in the commit
 description, or even the git author field. I note that in the downstream
 kernel, the commit which adds the RNG driver isn't signed off at all.
 This probably means you need to get Dom to add his signed-off-by line
 for that commit before basing your work on it. See
 Documentation/SubmittingPatches for more details.

Got this message, will follow up with updated patch:

On Wed, 2013-03-27 at 18:22 +, popcornmix wrote:
On Wed, Mar 27, 2013 at 4:55 PM, Lubomir Rintel lkund...@v3.sk wrote:
  Hi!
 
  I'm currently in the process of mainlining the drivers from
Raspberry Pi
  tree. I was asked for a signoff for the random number generator. It
  seems to have been commit by you in the downstream tree, I'm
wondering
  if you could help me, and respond with a Signed-off-by tag if the
code
  comes from you, or point me to someone else?
 
  commit e95a8204d7f8fc4f38900c99080103254c3cef11
  Author: popcornmix popcorn...@gmail.com
  Date:   Wed Jan 30 11:44:26 2013 +
 
  Add hwrng (hardware random number generator) driver
 
  Thank you!
  --
  Lubomir Rintel lkund...@v3.sk
 
 
 Sorry, I'm not up to speed with exactly what constitutes a sign off.
 I did write the original hw-rng driver, and give my permission for it
 to be upstreamed.
 Is this sufficient?
  Signed-off-by: Dom Cobley d...@broadcom.com
  Signed-off-by: Dom Cobley popcorn...@gmail.com
  Signed-off-by: popcornmix popcorn...@gmail.com
 
 (not sure which is more suitable - they are all me).

-- 
Lubomir Rintel (ext. #7715)
GoodData Code Feng Shui Critic

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] hw_random: Add Broadcom BCM2835 RNG Driver

2013-03-26 Thread Stephen Warren
On 03/24/2013 08:37 AM, Lubomir Rintel wrote:
> On Fri, 2013-03-22 at 20:44 -0600, Stephen Warren wrote:
> 
> Thank you for your response!
> 
>> On 03/22/2013 06:55 AM, Lubomir Rintel wrote:
>>> Signed-off-by: Lubomir Rintel 
>>
>> A commit description would be useful.
>>
>>>  arch/arm/boot/dts/bcm2835.dtsi   |5 +
>>>  arch/arm/configs/bcm2835_defconfig   |3 +-
>>>  drivers/char/hw_random/Kconfig   |   12 +++
>>>  drivers/char/hw_random/Makefile  |1 +
>>>  drivers/char/hw_random/bcm2835-rng.c |  137 
>>> ++
>>
>> This should be split into 3 separate patches: (1) The driver itself, (2)
>> the change to bcm2835.dtsi, and (3) the change to bcm2835_defconfig.
>>
>> Since you're adding a new device to device tree for the first time, you
>> should write a binding document for it; most likely
>> Documentation/devicetree/bindings/rng/brcm,bcm2835.txt (or perhaps
>> /random/ rather than /rng/?)
> 
> Okay. I'm tempted to stick to "rng" instead of "random" as it seems more 
> consistent to me, but I don't have a strong reason to back it with. What 
> would 
> be a good reason for using "random"?

I figured that "random" might be more meaningful/understandable for some
people. It's not a big deal either way though.

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


Re: [PATCH] hw_random: Add Broadcom BCM2835 RNG Driver

2013-03-26 Thread Stephen Warren
On 03/24/2013 08:37 AM, Lubomir Rintel wrote:
 On Fri, 2013-03-22 at 20:44 -0600, Stephen Warren wrote:
 
 Thank you for your response!
 
 On 03/22/2013 06:55 AM, Lubomir Rintel wrote:
 Signed-off-by: Lubomir Rintel lkundrak at v3.sk

 A commit description would be useful.

  arch/arm/boot/dts/bcm2835.dtsi   |5 +
  arch/arm/configs/bcm2835_defconfig   |3 +-
  drivers/char/hw_random/Kconfig   |   12 +++
  drivers/char/hw_random/Makefile  |1 +
  drivers/char/hw_random/bcm2835-rng.c |  137 
 ++

 This should be split into 3 separate patches: (1) The driver itself, (2)
 the change to bcm2835.dtsi, and (3) the change to bcm2835_defconfig.

 Since you're adding a new device to device tree for the first time, you
 should write a binding document for it; most likely
 Documentation/devicetree/bindings/rng/brcm,bcm2835.txt (or perhaps
 /random/ rather than /rng/?)
 
 Okay. I'm tempted to stick to rng instead of random as it seems more 
 consistent to me, but I don't have a strong reason to back it with. What 
 would 
 be a good reason for using random?

I figured that random might be more meaningful/understandable for some
people. It's not a big deal either way though.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] hw_random: Add Broadcom BCM2835 RNG Driver

2013-03-24 Thread Lubomir Rintel
On Fri, 2013-03-22 at 20:44 -0600, Stephen Warren wrote:

Thank you for your response!

> On 03/22/2013 06:55 AM, Lubomir Rintel wrote:
> > Signed-off-by: Lubomir Rintel 
> 
> A commit description would be useful.
> 
> >  arch/arm/boot/dts/bcm2835.dtsi   |5 +
> >  arch/arm/configs/bcm2835_defconfig   |3 +-
> >  drivers/char/hw_random/Kconfig   |   12 +++
> >  drivers/char/hw_random/Makefile  |1 +
> >  drivers/char/hw_random/bcm2835-rng.c |  137 
> > ++
> 
> This should be split into 3 separate patches: (1) The driver itself, (2)
> the change to bcm2835.dtsi, and (3) the change to bcm2835_defconfig.
> 
> Since you're adding a new device to device tree for the first time, you
> should write a binding document for it; most likely
> Documentation/devicetree/bindings/rng/brcm,bcm2835.txt (or perhaps
> /random/ rather than /rng/?)

Okay. I'm tempted to stick to "rng" instead of "random" as it seems more 
consistent to me, but I don't have a strong reason to back it with. What would 
be a good reason for using "random"?

> Is this driver based on the downstream Raspberry Pi kernel's driver? If
> so, some mention of that fact would be appropriate in the commit
> description, or even the git author field. I note that in the downstream
> kernel, the commit which adds the RNG driver isn't signed off at all.
> This probably means you need to get Dom to add his signed-off-by line
> for that commit before basing your work on it. See
> Documentation/SubmittingPatches for more details.

Ok, will try to.

I'll still follow up with an updated patch, so that it can be reviewed.

> > diff --git a/drivers/char/hw_random/bcm2835-rng.c 
> > b/drivers/char/hw_random/bcm2835-rng.c
> 
> > + * Redistribution and use in source and binary forms, with or without
> ...
> > + * ALTERNATIVELY, this software may be distributed under the terms of the
> > + * GNU General Public License ("GPL") version 2, as published by the Free
> > + * Software Foundation.
> 
> I am not a lawyer, but I'd be tempted to exercise that right, and
> distribute this patch solely under the terms of the GPLv2, and hence
> remove the other license. It's not a big deal, but it'd simplify the
> licensing in the upstream kernel.

Will do.

> > +static int bcm2835_rng_read(struct hwrng *rng, void *buf, size_t max,
> > +  bool wait)
> 
> > +   while ((__raw_readl(rng_base + RNG_STATUS)>>24) == 0) {
> 
> You'd usually put spaces around the >>.

Will be fixed.

> > +static int bcm2835_rng_probe(struct platform_device *op)
> 
> "pdev" is a more typical name than "op".

Will be adjusted.

> > +{
> > +   struct device *dev = >dev;
> > +   struct device_node *np = dev->of_node;
> > +   void __iomem *rng_base;
> > +   int err;
> > +
> > +   /* map peripheral */
> > +   rng_base = of_iomap(np, 0);
> > +   if (WARN(!rng_base, "failed to remap rng regs"))
> > +   return -ENODEV;
> 
> The WARN() doesn't seem necessary. dev_err() inside the if{} would be
> more appropriate,

Makes sense, will be adjusted.

> > +static struct platform_driver bcm2835_rng_driver = {
> ...
> > +};
> > +
> > +module_platform_driver(bcm2835_rng_driver);
> 
> Typically, no blank line there.

Ok

> > +MODULE_AUTHOR("Lubomir Rintel ");
> > +MODULE_DESCRIPTION("BCM2835 Random Number Generator (RNG) driver");
> > +MODULE_LICENSE("GPL and additional rights");
> 
> I think that should be "GPLv2 and ...".

Ok

-- 
Lubomir Rintel 

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


Re: [PATCH] hw_random: Add Broadcom BCM2835 RNG Driver

2013-03-24 Thread Lubomir Rintel
On Fri, 2013-03-22 at 20:44 -0600, Stephen Warren wrote:

Thank you for your response!

 On 03/22/2013 06:55 AM, Lubomir Rintel wrote:
  Signed-off-by: Lubomir Rintel lkundrak at v3.sk
 
 A commit description would be useful.
 
   arch/arm/boot/dts/bcm2835.dtsi   |5 +
   arch/arm/configs/bcm2835_defconfig   |3 +-
   drivers/char/hw_random/Kconfig   |   12 +++
   drivers/char/hw_random/Makefile  |1 +
   drivers/char/hw_random/bcm2835-rng.c |  137 
  ++
 
 This should be split into 3 separate patches: (1) The driver itself, (2)
 the change to bcm2835.dtsi, and (3) the change to bcm2835_defconfig.
 
 Since you're adding a new device to device tree for the first time, you
 should write a binding document for it; most likely
 Documentation/devicetree/bindings/rng/brcm,bcm2835.txt (or perhaps
 /random/ rather than /rng/?)

Okay. I'm tempted to stick to rng instead of random as it seems more 
consistent to me, but I don't have a strong reason to back it with. What would 
be a good reason for using random?

 Is this driver based on the downstream Raspberry Pi kernel's driver? If
 so, some mention of that fact would be appropriate in the commit
 description, or even the git author field. I note that in the downstream
 kernel, the commit which adds the RNG driver isn't signed off at all.
 This probably means you need to get Dom to add his signed-off-by line
 for that commit before basing your work on it. See
 Documentation/SubmittingPatches for more details.

Ok, will try to.

I'll still follow up with an updated patch, so that it can be reviewed.

  diff --git a/drivers/char/hw_random/bcm2835-rng.c 
  b/drivers/char/hw_random/bcm2835-rng.c
 
  + * Redistribution and use in source and binary forms, with or without
 ...
  + * ALTERNATIVELY, this software may be distributed under the terms of the
  + * GNU General Public License (GPL) version 2, as published by the Free
  + * Software Foundation.
 
 I am not a lawyer, but I'd be tempted to exercise that right, and
 distribute this patch solely under the terms of the GPLv2, and hence
 remove the other license. It's not a big deal, but it'd simplify the
 licensing in the upstream kernel.

Will do.

  +static int bcm2835_rng_read(struct hwrng *rng, void *buf, size_t max,
  +  bool wait)
 
  +   while ((__raw_readl(rng_base + RNG_STATUS)24) == 0) {
 
 You'd usually put spaces around the .

Will be fixed.

  +static int bcm2835_rng_probe(struct platform_device *op)
 
 pdev is a more typical name than op.

Will be adjusted.

  +{
  +   struct device *dev = op-dev;
  +   struct device_node *np = dev-of_node;
  +   void __iomem *rng_base;
  +   int err;
  +
  +   /* map peripheral */
  +   rng_base = of_iomap(np, 0);
  +   if (WARN(!rng_base, failed to remap rng regs))
  +   return -ENODEV;
 
 The WARN() doesn't seem necessary. dev_err() inside the if{} would be
 more appropriate,

Makes sense, will be adjusted.

  +static struct platform_driver bcm2835_rng_driver = {
 ...
  +};
  +
  +module_platform_driver(bcm2835_rng_driver);
 
 Typically, no blank line there.

Ok

  +MODULE_AUTHOR(Lubomir Rintel lkundrak at v3.sk);
  +MODULE_DESCRIPTION(BCM2835 Random Number Generator (RNG) driver);
  +MODULE_LICENSE(GPL and additional rights);
 
 I think that should be GPLv2 and 

Ok

-- 
Lubomir Rintel lkund...@v3.sk

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] hw_random: Add Broadcom BCM2835 RNG Driver

2013-03-22 Thread Stephen Warren
On 03/22/2013 06:55 AM, Lubomir Rintel wrote:
> Signed-off-by: Lubomir Rintel 

A commit description would be useful.

>  arch/arm/boot/dts/bcm2835.dtsi   |5 +
>  arch/arm/configs/bcm2835_defconfig   |3 +-
>  drivers/char/hw_random/Kconfig   |   12 +++
>  drivers/char/hw_random/Makefile  |1 +
>  drivers/char/hw_random/bcm2835-rng.c |  137 
> ++

This should be split into 3 separate patches: (1) The driver itself, (2)
the change to bcm2835.dtsi, and (3) the change to bcm2835_defconfig.

Since you're adding a new device to device tree for the first time, you
should write a binding document for it; most likely
Documentation/devicetree/bindings/rng/brcm,bcm2835.txt (or perhaps
/random/ rather than /rng/?)

Is this driver based on the downstream Raspberry Pi kernel's driver? If
so, some mention of that fact would be appropriate in the commit
description, or even the git author field. I note that in the downstream
kernel, the commit which adds the RNG driver isn't signed off at all.
This probably means you need to get Dom to add his signed-off-by line
for that commit before basing your work on it. See
Documentation/SubmittingPatches for more details.

> diff --git a/drivers/char/hw_random/bcm2835-rng.c 
> b/drivers/char/hw_random/bcm2835-rng.c

> + * Redistribution and use in source and binary forms, with or without
...
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") version 2, as published by the Free
> + * Software Foundation.

I am not a lawyer, but I'd be tempted to exercise that right, and
distribute this patch solely under the terms of the GPLv2, and hence
remove the other license. It's not a big deal, but it'd simplify the
licensing in the upstream kernel.

> +static int bcm2835_rng_read(struct hwrng *rng, void *buf, size_t max,
> +bool wait)

> + while ((__raw_readl(rng_base + RNG_STATUS)>>24) == 0) {

You'd usually put spaces around the >>.

> +static int bcm2835_rng_probe(struct platform_device *op)

"pdev" is a more typical name than "op".

> +{
> + struct device *dev = >dev;
> + struct device_node *np = dev->of_node;
> + void __iomem *rng_base;
> + int err;
> +
> + /* map peripheral */
> + rng_base = of_iomap(np, 0);
> + if (WARN(!rng_base, "failed to remap rng regs"))
> + return -ENODEV;

The WARN() doesn't seem necessary. dev_err() inside the if{} would be
more appropriate,

> +static struct platform_driver bcm2835_rng_driver = {
...
> +};
> +
> +module_platform_driver(bcm2835_rng_driver);

Typically, no blank line there.

> +MODULE_AUTHOR("Lubomir Rintel ");
> +MODULE_DESCRIPTION("BCM2835 Random Number Generator (RNG) driver");
> +MODULE_LICENSE("GPL and additional rights");

I think that should be "GPLv2 and ...".
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] hw_random: Add Broadcom BCM2835 RNG Driver

2013-03-22 Thread Lubomir Rintel
Signed-off-by: Lubomir Rintel 
Cc: Stephen Warren 
Cc: Matt Mackall 
Cc: linux-rpi-ker...@lists.infradead.org
---
 arch/arm/boot/dts/bcm2835.dtsi   |5 +
 arch/arm/configs/bcm2835_defconfig   |3 +-
 drivers/char/hw_random/Kconfig   |   12 +++
 drivers/char/hw_random/Makefile  |1 +
 drivers/char/hw_random/bcm2835-rng.c |  137 ++
 5 files changed, 157 insertions(+), 1 deletions(-)
 create mode 100644 drivers/char/hw_random/bcm2835-rng.c

diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 7e0481e..dc22336 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -34,6 +34,11 @@
reg = <0x7e10 0x28>;
};
 
+   rng {
+   compatible = "brcm,bcm2835-rng";
+   reg = <0x7e104000 0x10>;
+   };
+
uart@20201000 {
compatible = "brcm,bcm2835-pl011", "arm,pl011", 
"arm,primecell";
reg = <0x7e201000 0x1000>;
diff --git a/arch/arm/configs/bcm2835_defconfig 
b/arch/arm/configs/bcm2835_defconfig
index af472e4..611bda2 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -59,7 +59,8 @@ CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_SERIAL_AMBA_PL011=y
 CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
 CONFIG_TTY_PRINTK=y
-# CONFIG_HW_RANDOM is not set
+CONFIG_HW_RANDOM=y
+CONFIG_HW_RANDOM_BCM2835=y
 CONFIG_I2C=y
 CONFIG_I2C_CHARDEV=y
 CONFIG_I2C_BCM2835=y
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index c5a0262..2f9dbf7 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -86,6 +86,18 @@ config HW_RANDOM_BCM63XX
 
  If unusure, say Y.
 
+config HW_RANDOM_BCM2835
+   tristate "Broadcom BCM2835 Random Number Generator support"
+   depends on HW_RANDOM && ARCH_BCM2835
+   default HW_RANDOM
+   ---help---
+ This driver provides kernel-side support for the Random Number
+ Generator hardware found on the Broadcom BCM2835 SoCs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bcm2835-rng
+
+ If unsure, say Y.
 
 config HW_RANDOM_GEODE
tristate "AMD Geode HW Random Number Generator support"
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 1fd7eec..bed467c 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -26,3 +26,4 @@ obj-$(CONFIG_HW_RANDOM_PPC4XX) += ppc4xx-rng.o
 obj-$(CONFIG_HW_RANDOM_PSERIES) += pseries-rng.o
 obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-rng.o
 obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
+obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o
diff --git a/drivers/char/hw_random/bcm2835-rng.c 
b/drivers/char/hw_random/bcm2835-rng.c
new file mode 100644
index 000..66adc7f
--- /dev/null
+++ b/drivers/char/hw_random/bcm2835-rng.c
@@ -0,0 +1,137 @@
+/**
+ * Copyright (c) 2010-2012 Broadcom. All rights reserved.
+ * Copyright (c) 2013 Lubomir Rintel
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions, and the following disclaimer,
+ *without modification.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. The names of the above-listed copyright holders may not be used
+ *to endorse or promote products derived from this software without
+ *specific prior written permission.
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2, as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
+ * IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define RNG_CTRL   0x0
+#define RNG_STATUS 0x4

[PATCH] hw_random: Add Broadcom BCM2835 RNG Driver

2013-03-22 Thread Lubomir Rintel
Signed-off-by: Lubomir Rintel lkund...@v3.sk
Cc: Stephen Warren swar...@wwwdotorg.org
Cc: Matt Mackall m...@selenic.com
Cc: linux-rpi-ker...@lists.infradead.org
---
 arch/arm/boot/dts/bcm2835.dtsi   |5 +
 arch/arm/configs/bcm2835_defconfig   |3 +-
 drivers/char/hw_random/Kconfig   |   12 +++
 drivers/char/hw_random/Makefile  |1 +
 drivers/char/hw_random/bcm2835-rng.c |  137 ++
 5 files changed, 157 insertions(+), 1 deletions(-)
 create mode 100644 drivers/char/hw_random/bcm2835-rng.c

diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 7e0481e..dc22336 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -34,6 +34,11 @@
reg = 0x7e10 0x28;
};
 
+   rng {
+   compatible = brcm,bcm2835-rng;
+   reg = 0x7e104000 0x10;
+   };
+
uart@20201000 {
compatible = brcm,bcm2835-pl011, arm,pl011, 
arm,primecell;
reg = 0x7e201000 0x1000;
diff --git a/arch/arm/configs/bcm2835_defconfig 
b/arch/arm/configs/bcm2835_defconfig
index af472e4..611bda2 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -59,7 +59,8 @@ CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_SERIAL_AMBA_PL011=y
 CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
 CONFIG_TTY_PRINTK=y
-# CONFIG_HW_RANDOM is not set
+CONFIG_HW_RANDOM=y
+CONFIG_HW_RANDOM_BCM2835=y
 CONFIG_I2C=y
 CONFIG_I2C_CHARDEV=y
 CONFIG_I2C_BCM2835=y
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index c5a0262..2f9dbf7 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -86,6 +86,18 @@ config HW_RANDOM_BCM63XX
 
  If unusure, say Y.
 
+config HW_RANDOM_BCM2835
+   tristate Broadcom BCM2835 Random Number Generator support
+   depends on HW_RANDOM  ARCH_BCM2835
+   default HW_RANDOM
+   ---help---
+ This driver provides kernel-side support for the Random Number
+ Generator hardware found on the Broadcom BCM2835 SoCs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bcm2835-rng
+
+ If unsure, say Y.
 
 config HW_RANDOM_GEODE
tristate AMD Geode HW Random Number Generator support
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 1fd7eec..bed467c 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -26,3 +26,4 @@ obj-$(CONFIG_HW_RANDOM_PPC4XX) += ppc4xx-rng.o
 obj-$(CONFIG_HW_RANDOM_PSERIES) += pseries-rng.o
 obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-rng.o
 obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
+obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o
diff --git a/drivers/char/hw_random/bcm2835-rng.c 
b/drivers/char/hw_random/bcm2835-rng.c
new file mode 100644
index 000..66adc7f
--- /dev/null
+++ b/drivers/char/hw_random/bcm2835-rng.c
@@ -0,0 +1,137 @@
+/**
+ * Copyright (c) 2010-2012 Broadcom. All rights reserved.
+ * Copyright (c) 2013 Lubomir Rintel
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions, and the following disclaimer,
+ *without modification.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. The names of the above-listed copyright holders may not be used
+ *to endorse or promote products derived from this software without
+ *specific prior written permission.
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License (GPL) version 2, as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS AS
+ * IS AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include linux/hw_random.h
+#include linux/init.h
+#include linux/io.h
+#include linux/kernel.h
+#include linux/module.h
+#include 

Re: [PATCH] hw_random: Add Broadcom BCM2835 RNG Driver

2013-03-22 Thread Stephen Warren
On 03/22/2013 06:55 AM, Lubomir Rintel wrote:
 Signed-off-by: Lubomir Rintel lkund...@v3.sk

A commit description would be useful.

  arch/arm/boot/dts/bcm2835.dtsi   |5 +
  arch/arm/configs/bcm2835_defconfig   |3 +-
  drivers/char/hw_random/Kconfig   |   12 +++
  drivers/char/hw_random/Makefile  |1 +
  drivers/char/hw_random/bcm2835-rng.c |  137 
 ++

This should be split into 3 separate patches: (1) The driver itself, (2)
the change to bcm2835.dtsi, and (3) the change to bcm2835_defconfig.

Since you're adding a new device to device tree for the first time, you
should write a binding document for it; most likely
Documentation/devicetree/bindings/rng/brcm,bcm2835.txt (or perhaps
/random/ rather than /rng/?)

Is this driver based on the downstream Raspberry Pi kernel's driver? If
so, some mention of that fact would be appropriate in the commit
description, or even the git author field. I note that in the downstream
kernel, the commit which adds the RNG driver isn't signed off at all.
This probably means you need to get Dom to add his signed-off-by line
for that commit before basing your work on it. See
Documentation/SubmittingPatches for more details.

 diff --git a/drivers/char/hw_random/bcm2835-rng.c 
 b/drivers/char/hw_random/bcm2835-rng.c

 + * Redistribution and use in source and binary forms, with or without
...
 + * ALTERNATIVELY, this software may be distributed under the terms of the
 + * GNU General Public License (GPL) version 2, as published by the Free
 + * Software Foundation.

I am not a lawyer, but I'd be tempted to exercise that right, and
distribute this patch solely under the terms of the GPLv2, and hence
remove the other license. It's not a big deal, but it'd simplify the
licensing in the upstream kernel.

 +static int bcm2835_rng_read(struct hwrng *rng, void *buf, size_t max,
 +bool wait)

 + while ((__raw_readl(rng_base + RNG_STATUS)24) == 0) {

You'd usually put spaces around the .

 +static int bcm2835_rng_probe(struct platform_device *op)

pdev is a more typical name than op.

 +{
 + struct device *dev = op-dev;
 + struct device_node *np = dev-of_node;
 + void __iomem *rng_base;
 + int err;
 +
 + /* map peripheral */
 + rng_base = of_iomap(np, 0);
 + if (WARN(!rng_base, failed to remap rng regs))
 + return -ENODEV;

The WARN() doesn't seem necessary. dev_err() inside the if{} would be
more appropriate,

 +static struct platform_driver bcm2835_rng_driver = {
...
 +};
 +
 +module_platform_driver(bcm2835_rng_driver);

Typically, no blank line there.

 +MODULE_AUTHOR(Lubomir Rintel lkund...@v3.sk);
 +MODULE_DESCRIPTION(BCM2835 Random Number Generator (RNG) driver);
 +MODULE_LICENSE(GPL and additional rights);

I think that should be GPLv2 and 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/