Patchwork [v2,06/12] hwrng: bcm2835-rng: Rework interrupt masking

login
register
mail settings
Submitter Florian Fainelli
Date Nov. 8, 2017, 12:44 a.m.
Message ID <20171108004449.32730-7-f.fainelli@gmail.com>
Download mbox | patch
Permalink /patch/10047451/
State Accepted
Delegated to: Herbert Xu
Headers show

Comments

Florian Fainelli - Nov. 8, 2017, 12:44 a.m.
The interrupt masking done for Northstart Plus and Northstar (BCM5301X)
is moved from being a function pointer mapped to of_device_id::data into
a proper part of the hwrng::init callback. While at it, we also make the
of_data be a proper structure indicating the platform specifics, since
the day we need to add a second type of platform information, we would
have to do that anyway.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/char/hw_random/bcm2835-rng.c | 39 +++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 16 deletions(-)
Eric Anholt - Nov. 8, 2017, 6:26 p.m.
Florian Fainelli <f.fainelli@gmail.com> writes:

> The interrupt masking done for Northstart Plus and Northstar (BCM5301X)
> is moved from being a function pointer mapped to of_device_id::data into
> a proper part of the hwrng::init callback. While at it, we also make the
> of_data be a proper structure indicating the platform specifics, since
> the day we need to add a second type of platform information, we would
> have to do that anyway.

I still think we should just unconditionally mask off the interrupt
regardless of platform if we're not going to use it in the driver and
some platforms need it.  Looks like a fine refactor, though:

Reviewed-by: Eric Anholt <eric@anholt.net>
Stefan Wahren - Nov. 8, 2017, 6:46 p.m.
Hi Florian,

> Florian Fainelli <f.fainelli@gmail.com> hat am 8. November 2017 um 01:44 geschrieben:
> 
> 
> The interrupt masking done for Northstart Plus and Northstar (BCM5301X)
> is moved from being a function pointer mapped to of_device_id::data into
> a proper part of the hwrng::init callback. While at it, we also make the
> of_data be a proper structure indicating the platform specifics, since
> the day we need to add a second type of platform information, we would
> have to do that anyway.

in the lack of proper documentation for bcm2835 rng, is it possible that we should mask the interrupts for bcm2835 as well?
Eric Anholt - Nov. 8, 2017, 8:41 p.m.
Stefan Wahren <stefan.wahren@i2se.com> writes:

> Hi Florian,
>
>> Florian Fainelli <f.fainelli@gmail.com> hat am 8. November 2017 um 01:44 geschrieben:
>> 
>> 
>> The interrupt masking done for Northstart Plus and Northstar (BCM5301X)
>> is moved from being a function pointer mapped to of_device_id::data into
>> a proper part of the hwrng::init callback. While at it, we also make the
>> of_data be a proper structure indicating the platform specifics, since
>> the day we need to add a second type of platform information, we would
>> have to do that anyway.
>
> in the lack of proper documentation for bcm2835 rng, is it possible
> that we should mask the interrupts for bcm2835 as well?

I don't have the RTL nearby and the RNG block doesn't have the usual
#defines for power-on-reset values, so I'm not sure.  I don't see any
use of the RNG by the firmware that should impact Linux -- a driver
exists and it uses IRQs, but it shouldn't have been active, and even if
it was then its teardown process masks the interrupt off again.

However, masking it off ourselves should be harmless at worst.

Patch

diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
index 67b9bd3be28d..ed20e0b6b7ae 100644
--- a/drivers/char/hw_random/bcm2835-rng.c
+++ b/drivers/char/hw_random/bcm2835-rng.c
@@ -32,18 +32,9 @@ 
 struct bcm2835_rng_priv {
 	struct hwrng rng;
 	void __iomem *base;
+	bool mask_interrupts;
 };
 
-static void __init nsp_rng_init(void __iomem *base)
-{
-	u32 val;
-
-	/* mask the interrupt */
-	val = readl(base + RNG_INT_MASK);
-	val |= RNG_INT_OFF;
-	writel(val, base + RNG_INT_MASK);
-}
-
 static inline struct bcm2835_rng_priv *to_rng_priv(struct hwrng *rng)
 {
 	return container_of(rng, struct bcm2835_rng_priv, rng);
@@ -75,6 +66,14 @@  static int bcm2835_rng_read(struct hwrng *rng, void *buf, size_t max,
 static int bcm2835_rng_init(struct hwrng *rng)
 {
 	struct bcm2835_rng_priv *priv = to_rng_priv(rng);
+	u32 val;
+
+	if (priv->mask_interrupts) {
+		/* mask the interrupt */
+		val = readl(priv->base + RNG_INT_MASK);
+		val |= RNG_INT_OFF;
+		writel(val, priv->base + RNG_INT_MASK);
+	}
 
 	/* set warm-up count & enable */
 	__raw_writel(RNG_WARMUP_COUNT, priv->base + RNG_STATUS);
@@ -91,18 +90,26 @@  static void bcm2835_rng_cleanup(struct hwrng *rng)
 	__raw_writel(0, priv->base + RNG_CTRL);
 }
 
+struct bcm2835_rng_of_data {
+	bool mask_interrupts;
+};
+
+static const struct bcm2835_rng_of_data nsp_rng_of_data = {
+	.mask_interrupts = true,
+};
+
 static const struct of_device_id bcm2835_rng_of_match[] = {
 	{ .compatible = "brcm,bcm2835-rng"},
-	{ .compatible = "brcm,bcm-nsp-rng", .data = nsp_rng_init},
-	{ .compatible = "brcm,bcm5301x-rng", .data = nsp_rng_init},
+	{ .compatible = "brcm,bcm-nsp-rng", .data = &nsp_rng_of_data },
+	{ .compatible = "brcm,bcm5301x-rng", .data = &nsp_rng_of_data },
 	{},
 };
 
 static int bcm2835_rng_probe(struct platform_device *pdev)
 {
+	const struct bcm2835_rng_of_data *of_data;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
-	void (*rng_setup)(void __iomem *base);
 	const struct of_device_id *rng_id;
 	struct bcm2835_rng_priv *priv;
 	struct resource *r;
@@ -133,9 +140,9 @@  static int bcm2835_rng_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	/* Check for rng init function, execute it */
-	rng_setup = rng_id->data;
-	if (rng_setup)
-		rng_setup(priv->base);
+	of_data = rng_id->data;
+	if (of_data)
+		priv->mask_interrupts = of_data->mask_interrupts;
 
 	/* register driver */
 	err = devm_hwrng_register(dev, &priv->rng);