Hi,
As it usually happens when there is a fair amount of HW IP block re-use,
competing implementations show up. In that case the BCM2835 HWRNG driver and
the BCM63xx RNG driver have exactly the same register offsets and this is
indeed the same piece of HW.
This patch series first prepares the bcm2835-rng to be more future proof and
support newer platforms, and the last part brings in what is necessary to
migrate the bcm63xx-rng over to bcm2835-rng. Finally we delete bcm63xx-rng
completely.
The reason why BCM2835 RNG was kept over BCM63xx RNG is because the former
deals correctly with a warm up count and the number of words available in the
FIFO size.
Thanks!
Florian Fainelli (12):
hwrng: bcm2835-rng: Obtain base register via resource
hwrng: bcm2835-rng: Define a driver private context
hwrng: bcm2835-rng: Move enabling to hwrng::init
hwrng: bcm2835-rng: Implementation cleanup callback
hwrng: bcm2835-rng: Use device managed helpers
hwrng: bcm2835-rng: Rework interrupt masking
hwrng: bcm2835-rng: Manage an optional clock
hwrng: bcm2835-rng: Abstract I/O accessors
hwrng: bcm2835-rng: Add Broadcom MIPS I/O accessors
dt-bindings: rng: Incorporate brcm,bcm6368.txt binding
hwrng: bcm2835-rng: Enable BCM2835 RNG to work on BCM63xx platforms
hwrng: bcm63xx-rng: Remove since bcm2835-rng takes over
.../devicetree/bindings/rng/brcm,bcm2835.txt | 22 ++-
.../devicetree/bindings/rng/brcm,bcm6368.txt | 17 ---
drivers/char/hw_random/Kconfig | 20 +--
drivers/char/hw_random/Makefile | 1 -
drivers/char/hw_random/bcm2835-rng.c | 166 ++++++++++++++-------
drivers/char/hw_random/bcm63xx-rng.c | 154 -------------------
6 files changed, 139 insertions(+), 241 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/rng/brcm,bcm6368.txt
delete mode 100644 drivers/char/hw_random/bcm63xx-rng.c
--
2.9.3
We should be disabling the RNG in a hwrng::cleanup callback if we are
not longer the system selected RNG, not wait until the device driver is
removed.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/char/hw_random/bcm2835-rng.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
index 82000a637504..4d0356110b1b 100644
--- a/drivers/char/hw_random/bcm2835-rng.c
+++ b/drivers/char/hw_random/bcm2835-rng.c
@@ -83,6 +83,14 @@ static int bcm2835_rng_init(struct hwrng *rng)
return 0;
}
+static void bcm2835_rng_cleanup(struct hwrng *rng)
+{
+ struct bcm2835_rng_priv *priv = to_rng_priv(rng);
+
+ /* disable rng hardware */
+ __raw_writel(0, priv->base + RNG_CTRL);
+}
+
static const struct of_device_id bcm2835_rng_of_match[] = {
{ .compatible = "brcm,bcm2835-rng"},
{ .compatible = "brcm,bcm-nsp-rng", .data = nsp_rng_init},
@@ -118,6 +126,7 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
priv->rng.name = "bcm2835-rng";
priv->rng.init = bcm2835_rng_init;
priv->rng.read = bcm2835_rng_read;
+ priv->rng.cleanup = bcm2835_rng_cleanup;
rng_id = of_match_node(bcm2835_rng_of_match, np);
if (!rng_id)
@@ -142,9 +151,6 @@ static int bcm2835_rng_remove(struct platform_device *pdev)
{
struct bcm2835_rng_priv *priv = platform_get_drvdata(pdev);
- /* disable rng hardware */
- __raw_writel(0, priv->base + RNG_CTRL);
-
/* unregister driver */
hwrng_unregister(&priv->rng);
--
2.9.3
bcm2835-rng is now capable of supporting the BCM63xx hardware, so remove
the driver which duplicates the same functionality.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/char/hw_random/Kconfig | 13 ---
drivers/char/hw_random/Makefile | 1 -
drivers/char/hw_random/bcm63xx-rng.c | 154 -----------------------------------
3 files changed, 168 deletions(-)
delete mode 100644 drivers/char/hw_random/bcm63xx-rng.c
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index d0689cc8c7fc..2d3775b9c0c7 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -73,19 +73,6 @@ config HW_RANDOM_ATMEL
If unsure, say Y.
-config HW_RANDOM_BCM63XX
- tristate "Broadcom BCM63xx Random Number Generator support"
- depends on BCM63XX || BMIPS_GENERIC
- default HW_RANDOM
- ---help---
- This driver provides kernel-side support for the Random Number
- Generator hardware found on the Broadcom BCM63xx SoCs.
-
- To compile this driver as a module, choose M here: the
- module will be called bcm63xx-rng
-
- If unusure, say Y.
-
config HW_RANDOM_BCM2835
tristate "Broadcom BCM2835/BCM63xx Random Number Generator support"
depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X || \
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 39a67defac67..470ea14ed6b7 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -8,7 +8,6 @@ obj-$(CONFIG_HW_RANDOM_TIMERIOMEM) += timeriomem-rng.o
obj-$(CONFIG_HW_RANDOM_INTEL) += intel-rng.o
obj-$(CONFIG_HW_RANDOM_AMD) += amd-rng.o
obj-$(CONFIG_HW_RANDOM_ATMEL) += atmel-rng.o
-obj-$(CONFIG_HW_RANDOM_BCM63XX) += bcm63xx-rng.o
obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
obj-$(CONFIG_HW_RANDOM_N2RNG) += n2-rng.o
n2-rng-y := n2-drv.o n2-asm.o
diff --git a/drivers/char/hw_random/bcm63xx-rng.c b/drivers/char/hw_random/bcm63xx-rng.c
deleted file mode 100644
index 5132c9cde50d..000000000000
--- a/drivers/char/hw_random/bcm63xx-rng.c
+++ /dev/null
@@ -1,154 +0,0 @@
-/*
- * Broadcom BCM63xx Random Number Generator support
- *
- * Copyright (C) 2011, Florian Fainelli <[email protected]>
- * Copyright (C) 2009, Broadcom Corporation
- *
- */
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/io.h>
-#include <linux/err.h>
-#include <linux/clk.h>
-#include <linux/platform_device.h>
-#include <linux/hw_random.h>
-#include <linux/of.h>
-
-#define RNG_CTRL 0x00
-#define RNG_EN (1 << 0)
-
-#define RNG_STAT 0x04
-#define RNG_AVAIL_MASK (0xff000000)
-
-#define RNG_DATA 0x08
-#define RNG_THRES 0x0c
-#define RNG_MASK 0x10
-
-struct bcm63xx_rng_priv {
- struct hwrng rng;
- struct clk *clk;
- void __iomem *regs;
-};
-
-#define to_rng_priv(rng) container_of(rng, struct bcm63xx_rng_priv, rng)
-
-static int bcm63xx_rng_init(struct hwrng *rng)
-{
- struct bcm63xx_rng_priv *priv = to_rng_priv(rng);
- u32 val;
- int error;
-
- error = clk_prepare_enable(priv->clk);
- if (error)
- return error;
-
- val = __raw_readl(priv->regs + RNG_CTRL);
- val |= RNG_EN;
- __raw_writel(val, priv->regs + RNG_CTRL);
-
- return 0;
-}
-
-static void bcm63xx_rng_cleanup(struct hwrng *rng)
-{
- struct bcm63xx_rng_priv *priv = to_rng_priv(rng);
- u32 val;
-
- val = __raw_readl(priv->regs + RNG_CTRL);
- val &= ~RNG_EN;
- __raw_writel(val, priv->regs + RNG_CTRL);
-
- clk_disable_unprepare(priv->clk);
-}
-
-static int bcm63xx_rng_data_present(struct hwrng *rng, int wait)
-{
- struct bcm63xx_rng_priv *priv = to_rng_priv(rng);
-
- return __raw_readl(priv->regs + RNG_STAT) & RNG_AVAIL_MASK;
-}
-
-static int bcm63xx_rng_data_read(struct hwrng *rng, u32 *data)
-{
- struct bcm63xx_rng_priv *priv = to_rng_priv(rng);
-
- *data = __raw_readl(priv->regs + RNG_DATA);
-
- return 4;
-}
-
-static int bcm63xx_rng_probe(struct platform_device *pdev)
-{
- struct resource *r;
- int ret;
- struct bcm63xx_rng_priv *priv;
-
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!r) {
- dev_err(&pdev->dev, "no iomem resource\n");
- return -ENXIO;
- }
-
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
- priv->rng.name = pdev->name;
- priv->rng.init = bcm63xx_rng_init;
- priv->rng.cleanup = bcm63xx_rng_cleanup;
- priv->rng.data_present = bcm63xx_rng_data_present;
- priv->rng.data_read = bcm63xx_rng_data_read;
-
- priv->clk = devm_clk_get(&pdev->dev, "ipsec");
- if (IS_ERR(priv->clk)) {
- ret = PTR_ERR(priv->clk);
- dev_err(&pdev->dev, "no clock for device: %d\n", ret);
- return ret;
- }
-
- if (!devm_request_mem_region(&pdev->dev, r->start,
- resource_size(r), pdev->name)) {
- dev_err(&pdev->dev, "request mem failed");
- return -EBUSY;
- }
-
- priv->regs = devm_ioremap_nocache(&pdev->dev, r->start,
- resource_size(r));
- if (!priv->regs) {
- dev_err(&pdev->dev, "ioremap failed");
- return -ENOMEM;
- }
-
- ret = devm_hwrng_register(&pdev->dev, &priv->rng);
- if (ret) {
- dev_err(&pdev->dev, "failed to register rng device: %d\n",
- ret);
- return ret;
- }
-
- dev_info(&pdev->dev, "registered RNG driver\n");
-
- return 0;
-}
-
-#ifdef CONFIG_OF
-static const struct of_device_id bcm63xx_rng_of_match[] = {
- { .compatible = "brcm,bcm6368-rng", },
- {},
-};
-MODULE_DEVICE_TABLE(of, bcm63xx_rng_of_match);
-#endif
-
-static struct platform_driver bcm63xx_rng_driver = {
- .probe = bcm63xx_rng_probe,
- .driver = {
- .name = "bcm63xx-rng",
- .of_match_table = of_match_ptr(bcm63xx_rng_of_match),
- },
-};
-
-module_platform_driver(bcm63xx_rng_driver);
-
-MODULE_AUTHOR("Florian Fainelli <[email protected]>");
-MODULE_DESCRIPTION("Broadcom BCM63xx RNG driver");
-MODULE_LICENSE("GPL");
--
2.9.3
Since the same block is used on BCM2835 and BCM6368, merge the bindings
and remove the brcm,bcm6368.txt binding document.
Signed-off-by: Florian Fainelli <[email protected]>
---
.../devicetree/bindings/rng/brcm,bcm2835.txt | 22 +++++++++++++++++++---
.../devicetree/bindings/rng/brcm,bcm6368.txt | 17 -----------------
2 files changed, 19 insertions(+), 20 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/rng/brcm,bcm6368.txt
diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm2835.txt b/Documentation/devicetree/bindings/rng/brcm,bcm2835.txt
index 26542690b578..627b29531a32 100644
--- a/Documentation/devicetree/bindings/rng/brcm,bcm2835.txt
+++ b/Documentation/devicetree/bindings/rng/brcm,bcm2835.txt
@@ -1,11 +1,19 @@
-BCM2835 Random number generator
+BCM2835/6368 Random number generator
Required properties:
-- compatible : should be "brcm,bcm2835-rng" or "brcm,bcm-nsp-rng" or
- "brcm,bcm5301x-rng"
+- compatible : should be one of
+ "brcm,bcm2835-rng"
+ "brcm,bcm-nsp-rng"
+ "brcm,bcm5301x-rng" or
+ "brcm,bcm6368-rng"
- reg : Specifies base physical address and size of the registers.
+Optional properties:
+
+- clocks : phandle to clock-controller plus clock-specifier pair
+- clock-names : "ipsec" as a clock name
+
Example:
rng {
@@ -17,3 +25,11 @@ rng@18033000 {
compatible = "brcm,bcm-nsp-rng";
reg = <0x18033000 0x14>;
};
+
+random: rng@10004180 {
+ compatible = "brcm,bcm6368-rng";
+ reg = <0x10004180 0x14>;
+
+ clocks = <&periph_clk 18>;
+ clock-names = "ipsec";
+};
diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm6368.txt b/Documentation/devicetree/bindings/rng/brcm,bcm6368.txt
deleted file mode 100644
index 4b5ac600bfbd..000000000000
--- a/Documentation/devicetree/bindings/rng/brcm,bcm6368.txt
+++ /dev/null
@@ -1,17 +0,0 @@
-BCM6368 Random number generator
-
-Required properties:
-
-- compatible : should be "brcm,bcm6368-rng"
-- reg : Specifies base physical address and size of the registers
-- clocks : phandle to clock-controller plus clock-specifier pair
-- clock-names : "ipsec" as a clock name
-
-Example:
- random: rng@10004180 {
- compatible = "brcm,bcm6368-rng";
- reg = <0x10004180 0x14>;
-
- clocks = <&periph_clk 18>;
- clock-names = "ipsec";
- };
--
2.9.3
We have now incorporated all necessary functionality for the BCM63xx
platforms to successfully migrate over bcm2835-rng, so add the final
bits: Kconfig selection and proper platform_device device type matching
to keep the same platform device name for registration to work.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/char/hw_random/Kconfig | 7 ++++---
drivers/char/hw_random/bcm2835-rng.c | 11 ++++++++++-
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 95a031e9eced..d0689cc8c7fc 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -87,12 +87,13 @@ config HW_RANDOM_BCM63XX
If unusure, say Y.
config HW_RANDOM_BCM2835
- tristate "Broadcom BCM2835 Random Number Generator support"
- depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X
+ tristate "Broadcom BCM2835/BCM63xx Random Number Generator support"
+ depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X || \
+ ARCH_BCM_63XX || BCM63XX || BMIPS_GENERIC
default HW_RANDOM
---help---
This driver provides kernel-side support for the Random Number
- Generator hardware found on the Broadcom BCM2835 SoCs.
+ Generator hardware found on the Broadcom BCM2835 and BCM63xx SoCs.
To compile this driver as a module, choose M here: the
module will be called bcm2835-rng
diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
index 650e0033c273..d9ffe14f312b 100644
--- a/drivers/char/hw_random/bcm2835-rng.c
+++ b/drivers/char/hw_random/bcm2835-rng.c
@@ -131,6 +131,7 @@ static const struct of_device_id bcm2835_rng_of_match[] = {
{ .compatible = "brcm,bcm2835-rng"},
{ .compatible = "brcm,bcm-nsp-rng", .data = &nsp_rng_of_data },
{ .compatible = "brcm,bcm5301x-rng", .data = &nsp_rng_of_data },
+ { .compatible = "brcm,bcm6368-rng"},
{},
};
@@ -164,7 +165,7 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
if (IS_ERR(priv->clk))
priv->clk = NULL;
- priv->rng.name = "bcm2835-rng";
+ priv->rng.name = pdev->id_entry->name;
priv->rng.init = bcm2835_rng_init;
priv->rng.read = bcm2835_rng_read;
priv->rng.cleanup = bcm2835_rng_cleanup;
@@ -190,12 +191,20 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
MODULE_DEVICE_TABLE(of, bcm2835_rng_of_match);
+static struct platform_device_id bcm2835_rng_devtype[] = {
+ { .name = "bcm2835-rng" },
+ { .name = "bcm63xx-rng" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, bcm2835_rng_devtype);
+
static struct platform_driver bcm2835_rng_driver = {
.driver = {
.name = "bcm2835-rng",
.of_match_table = bcm2835_rng_of_match,
},
.probe = bcm2835_rng_probe,
+ .id_table = bcm2835_rng_devtype,
};
module_platform_driver(bcm2835_rng_driver);
--
2.9.3
Broadcom MIPS HW is always strapped to match the system-wide endian such
that all I/O access to this RNG block is done with the native CPU
endian, account for that.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/char/hw_random/bcm2835-rng.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
index 500275d55044..650e0033c273 100644
--- a/drivers/char/hw_random/bcm2835-rng.c
+++ b/drivers/char/hw_random/bcm2835-rng.c
@@ -44,13 +44,22 @@ static inline struct bcm2835_rng_priv *to_rng_priv(struct hwrng *rng)
static inline u32 rng_readl(struct bcm2835_rng_priv *priv, u32 offset)
{
- return readl(priv->base + offset);
+ /* MIPS chips strapped for BE will automagically configure the
+ * peripheral registers for CPU-native byte order.
+ */
+ if (IS_ENABLED(CONFIG_MIPS) && IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+ return __raw_readl(priv->base + offset);
+ else
+ return readl(priv->base + offset);
}
static inline void rng_writel(struct bcm2835_rng_priv *priv, u32 val,
u32 offset)
{
- writel(val, priv->base + offset);
+ if (IS_ENABLED(CONFIG_MIPS) && IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+ __raw_writel(val, priv->base + offset);
+ else
+ writel(val, priv->base + offset);
}
static int bcm2835_rng_read(struct hwrng *rng, void *buf, size_t max,
--
2.9.3
Instead of making hwrng::priv host the base register address, define a
driver private context, make it per platform device instance and pass it
down the different functions.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/char/hw_random/bcm2835-rng.c | 55 ++++++++++++++++++++++--------------
1 file changed, 34 insertions(+), 21 deletions(-)
diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
index a818418a7e4c..0d72147ab45b 100644
--- a/drivers/char/hw_random/bcm2835-rng.c
+++ b/drivers/char/hw_random/bcm2835-rng.c
@@ -29,6 +29,11 @@
#define RNG_INT_OFF 0x1
+struct bcm2835_rng_priv {
+ struct hwrng rng;
+ void __iomem *base;
+};
+
static void __init nsp_rng_init(void __iomem *base)
{
u32 val;
@@ -39,34 +44,34 @@ static void __init nsp_rng_init(void __iomem *base)
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);
+}
+
static int bcm2835_rng_read(struct hwrng *rng, void *buf, size_t max,
bool wait)
{
- void __iomem *rng_base = (void __iomem *)rng->priv;
+ struct bcm2835_rng_priv *priv = to_rng_priv(rng);
u32 max_words = max / sizeof(u32);
u32 num_words, count;
- while ((__raw_readl(rng_base + RNG_STATUS) >> 24) == 0) {
+ while ((__raw_readl(priv->base + RNG_STATUS) >> 24) == 0) {
if (!wait)
return 0;
cpu_relax();
}
- num_words = readl(rng_base + RNG_STATUS) >> 24;
+ num_words = readl(priv->base + RNG_STATUS) >> 24;
if (num_words > max_words)
num_words = max_words;
for (count = 0; count < num_words; count++)
- ((u32 *)buf)[count] = readl(rng_base + RNG_DATA);
+ ((u32 *)buf)[count] = readl(priv->base + RNG_DATA);
return num_words * sizeof(u32);
}
-static struct hwrng bcm2835_rng_ops = {
- .name = "bcm2835",
- .read = bcm2835_rng_read,
-};
-
static const struct of_device_id bcm2835_rng_of_match[] = {
{ .compatible = "brcm,bcm2835-rng"},
{ .compatible = "brcm,bcm-nsp-rng", .data = nsp_rng_init},
@@ -80,19 +85,27 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
struct device_node *np = dev->of_node;
void (*rng_setup)(void __iomem *base);
const struct of_device_id *rng_id;
- void __iomem *rng_base;
+ struct bcm2835_rng_priv *priv;
struct resource *r;
int err;
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, priv);
+
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
/* map peripheral */
- rng_base = devm_ioremap_resource(dev, r);
- if (IS_ERR(rng_base)) {
+ priv->base = devm_ioremap_resource(dev, r);
+ if (IS_ERR(priv->base)) {
dev_err(dev, "failed to remap rng regs");
- return PTR_ERR(rng_base);
+ return PTR_ERR(priv->base);
}
- bcm2835_rng_ops.priv = (unsigned long)rng_base;
+
+ priv->rng.name = "bcm2835-rng";
+ priv->rng.read = bcm2835_rng_read;
rng_id = of_match_node(bcm2835_rng_of_match, np);
if (!rng_id)
@@ -101,14 +114,14 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
/* Check for rng init function, execute it */
rng_setup = rng_id->data;
if (rng_setup)
- rng_setup(rng_base);
+ rng_setup(priv->base);
/* set warm-up count & enable */
- __raw_writel(RNG_WARMUP_COUNT, rng_base + RNG_STATUS);
- __raw_writel(RNG_RBGEN, rng_base + RNG_CTRL);
+ __raw_writel(RNG_WARMUP_COUNT, priv->base + RNG_STATUS);
+ __raw_writel(RNG_RBGEN, priv->base + RNG_CTRL);
/* register driver */
- err = hwrng_register(&bcm2835_rng_ops);
+ err = hwrng_register(&priv->rng);
if (err)
dev_err(dev, "hwrng registration failed\n");
else
@@ -119,13 +132,13 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
static int bcm2835_rng_remove(struct platform_device *pdev)
{
- void __iomem *rng_base = (void __iomem *)bcm2835_rng_ops.priv;
+ struct bcm2835_rng_priv *priv = platform_get_drvdata(pdev);
/* disable rng hardware */
- __raw_writel(0, rng_base + RNG_CTRL);
+ __raw_writel(0, priv->base + RNG_CTRL);
/* unregister driver */
- hwrng_unregister(&bcm2835_rng_ops);
+ hwrng_unregister(&priv->rng);
return 0;
}
--
2.9.3
We should be moving the enabling of the HWRNG into a hwrng::init
callback since we can be disabled and enabled every time a different
hwrng is selected in the system.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/char/hw_random/bcm2835-rng.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
index 0d72147ab45b..82000a637504 100644
--- a/drivers/char/hw_random/bcm2835-rng.c
+++ b/drivers/char/hw_random/bcm2835-rng.c
@@ -72,6 +72,17 @@ static int bcm2835_rng_read(struct hwrng *rng, void *buf, size_t max,
return num_words * sizeof(u32);
}
+static int bcm2835_rng_init(struct hwrng *rng)
+{
+ struct bcm2835_rng_priv *priv = to_rng_priv(rng);
+
+ /* set warm-up count & enable */
+ __raw_writel(RNG_WARMUP_COUNT, priv->base + RNG_STATUS);
+ __raw_writel(RNG_RBGEN, priv->base + RNG_CTRL);
+
+ return 0;
+}
+
static const struct of_device_id bcm2835_rng_of_match[] = {
{ .compatible = "brcm,bcm2835-rng"},
{ .compatible = "brcm,bcm-nsp-rng", .data = nsp_rng_init},
@@ -105,6 +116,7 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
}
priv->rng.name = "bcm2835-rng";
+ priv->rng.init = bcm2835_rng_init;
priv->rng.read = bcm2835_rng_read;
rng_id = of_match_node(bcm2835_rng_of_match, np);
@@ -116,10 +128,6 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
if (rng_setup)
rng_setup(priv->base);
- /* set warm-up count & enable */
- __raw_writel(RNG_WARMUP_COUNT, priv->base + RNG_STATUS);
- __raw_writel(RNG_RBGEN, priv->base + RNG_CTRL);
-
/* register driver */
err = hwrng_register(&priv->rng);
if (err)
--
2.9.3
One of the last steps before bcm63xx-rng can be eliminated is to manage
a clock during hwrng::init and hwrng::cleanup, so fetch it in the probe
function, and manage it during these two steps when valid.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/char/hw_random/bcm2835-rng.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
index ed20e0b6b7ae..35928efb52e7 100644
--- a/drivers/char/hw_random/bcm2835-rng.c
+++ b/drivers/char/hw_random/bcm2835-rng.c
@@ -15,6 +15,7 @@
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/printk.h>
+#include <linux/clk.h>
#define RNG_CTRL 0x0
#define RNG_STATUS 0x4
@@ -33,6 +34,7 @@ struct bcm2835_rng_priv {
struct hwrng rng;
void __iomem *base;
bool mask_interrupts;
+ struct clk *clk;
};
static inline struct bcm2835_rng_priv *to_rng_priv(struct hwrng *rng)
@@ -67,6 +69,11 @@ static int bcm2835_rng_init(struct hwrng *rng)
{
struct bcm2835_rng_priv *priv = to_rng_priv(rng);
u32 val;
+ int ret;
+
+ ret = clk_prepare_enable(priv->clk);
+ if (ret)
+ return ret;
if (priv->mask_interrupts) {
/* mask the interrupt */
@@ -88,6 +95,8 @@ static void bcm2835_rng_cleanup(struct hwrng *rng)
/* disable rng hardware */
__raw_writel(0, priv->base + RNG_CTRL);
+
+ clk_disable_unprepare(priv->clk);
}
struct bcm2835_rng_of_data {
@@ -130,6 +139,11 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
return PTR_ERR(priv->base);
}
+ /* Clock is optional on most platforms */
+ priv->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(priv->clk))
+ priv->clk = NULL;
+
priv->rng.name = "bcm2835-rng";
priv->rng.init = bcm2835_rng_init;
priv->rng.read = bcm2835_rng_read;
--
2.9.3
Now that we have moved the RNG disabling into a hwrng::cleanup callback,
we can use the device managed registration operation and remove our
remove callback since it won't do anything necessary.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/char/hw_random/bcm2835-rng.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
index 4d0356110b1b..67b9bd3be28d 100644
--- a/drivers/char/hw_random/bcm2835-rng.c
+++ b/drivers/char/hw_random/bcm2835-rng.c
@@ -138,7 +138,7 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
rng_setup(priv->base);
/* register driver */
- err = hwrng_register(&priv->rng);
+ err = devm_hwrng_register(dev, &priv->rng);
if (err)
dev_err(dev, "hwrng registration failed\n");
else
@@ -147,16 +147,6 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
return err;
}
-static int bcm2835_rng_remove(struct platform_device *pdev)
-{
- struct bcm2835_rng_priv *priv = platform_get_drvdata(pdev);
-
- /* unregister driver */
- hwrng_unregister(&priv->rng);
-
- return 0;
-}
-
MODULE_DEVICE_TABLE(of, bcm2835_rng_of_match);
static struct platform_driver bcm2835_rng_driver = {
@@ -165,7 +155,6 @@ static struct platform_driver bcm2835_rng_driver = {
.of_match_table = bcm2835_rng_of_match,
},
.probe = bcm2835_rng_probe,
- .remove = bcm2835_rng_remove,
};
module_platform_driver(bcm2835_rng_driver);
--
2.9.3
Hi Florian,
> Florian Fainelli <[email protected]> hat am 2. November 2017 um 02:03 geschrieben:
>
>
> Hi,
>
> As it usually happens when there is a fair amount of HW IP block re-use,
> competing implementations show up. In that case the BCM2835 HWRNG driver and
> the BCM63xx RNG driver have exactly the same register offsets and this is
> indeed the same piece of HW.
>
> This patch series first prepares the bcm2835-rng to be more future proof and
> support newer platforms, and the last part brings in what is necessary to
> migrate the bcm63xx-rng over to bcm2835-rng. Finally we delete bcm63xx-rng
> completely.
>
> The reason why BCM2835 RNG was kept over BCM63xx RNG is because the former
> deals correctly with a warm up count and the number of words available in the
> FIFO size.
are these the same patches as in this branch [1]?
https://github.com/ffainelli/linux/commits/rng-consolidation
Regards
Stefan
>
> Thanks!
>
> Florian Fainelli (12):
> hwrng: bcm2835-rng: Obtain base register via resource
> hwrng: bcm2835-rng: Define a driver private context
> hwrng: bcm2835-rng: Move enabling to hwrng::init
> hwrng: bcm2835-rng: Implementation cleanup callback
> hwrng: bcm2835-rng: Use device managed helpers
> hwrng: bcm2835-rng: Rework interrupt masking
> hwrng: bcm2835-rng: Manage an optional clock
> hwrng: bcm2835-rng: Abstract I/O accessors
> hwrng: bcm2835-rng: Add Broadcom MIPS I/O accessors
> dt-bindings: rng: Incorporate brcm,bcm6368.txt binding
> hwrng: bcm2835-rng: Enable BCM2835 RNG to work on BCM63xx platforms
> hwrng: bcm63xx-rng: Remove since bcm2835-rng takes over
>
> .../devicetree/bindings/rng/brcm,bcm2835.txt | 22 ++-
> .../devicetree/bindings/rng/brcm,bcm6368.txt | 17 ---
> drivers/char/hw_random/Kconfig | 20 +--
> drivers/char/hw_random/Makefile | 1 -
> drivers/char/hw_random/bcm2835-rng.c | 166 ++++++++++++++-------
> drivers/char/hw_random/bcm63xx-rng.c | 154 -------------------
> 6 files changed, 139 insertions(+), 241 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/rng/brcm,bcm6368.txt
> delete mode 100644 drivers/char/hw_random/bcm63xx-rng.c
>
> --
> 2.9.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Florian,
> Florian Fainelli <[email protected]> hat am 2. November 2017 um 02:03 geschrieben:
>
>
> Hi,
>
> As it usually happens when there is a fair amount of HW IP block re-use,
> competing implementations show up. In that case the BCM2835 HWRNG driver and
> the BCM63xx RNG driver have exactly the same register offsets and this is
> indeed the same piece of HW.
>
> This patch series first prepares the bcm2835-rng to be more future proof and
> support newer platforms, and the last part brings in what is necessary to
> migrate the bcm63xx-rng over to bcm2835-rng. Finally we delete bcm63xx-rng
> completely.
>
> The reason why BCM2835 RNG was kept over BCM63xx RNG is because the former
> deals correctly with a warm up count and the number of words available in the
> FIFO size.
are these the same patches as in this branch [1]?
https://github.com/ffainelli/linux/commits/rng-consolidation
Regards
Stefan
>
> Thanks!
>
> Florian Fainelli (12):
> hwrng: bcm2835-rng: Obtain base register via resource
> hwrng: bcm2835-rng: Define a driver private context
> hwrng: bcm2835-rng: Move enabling to hwrng::init
> hwrng: bcm2835-rng: Implementation cleanup callback
> hwrng: bcm2835-rng: Use device managed helpers
> hwrng: bcm2835-rng: Rework interrupt masking
> hwrng: bcm2835-rng: Manage an optional clock
> hwrng: bcm2835-rng: Abstract I/O accessors
> hwrng: bcm2835-rng: Add Broadcom MIPS I/O accessors
> dt-bindings: rng: Incorporate brcm,bcm6368.txt binding
> hwrng: bcm2835-rng: Enable BCM2835 RNG to work on BCM63xx platforms
> hwrng: bcm63xx-rng: Remove since bcm2835-rng takes over
>
> .../devicetree/bindings/rng/brcm,bcm2835.txt | 22 ++-
> .../devicetree/bindings/rng/brcm,bcm6368.txt | 17 ---
> drivers/char/hw_random/Kconfig | 20 +--
> drivers/char/hw_random/Makefile | 1 -
> drivers/char/hw_random/bcm2835-rng.c | 166 ++++++++++++++-------
> drivers/char/hw_random/bcm63xx-rng.c | 154 -------------------
> 6 files changed, 139 insertions(+), 241 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/rng/brcm,bcm6368.txt
> delete mode 100644 drivers/char/hw_random/bcm63xx-rng.c
>
> --
> 2.9.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Stefan,
On 11/02/2017 12:01 PM, Stefan Wahren wrote:
> Hi Florian,
>
>> Florian Fainelli <[email protected]> hat am 2. November 2017 um 02:03 geschrieben:
>>
>>
>> Hi,
>>
>> As it usually happens when there is a fair amount of HW IP block re-use,
>> competing implementations show up. In that case the BCM2835 HWRNG driver and
>> the BCM63xx RNG driver have exactly the same register offsets and this is
>> indeed the same piece of HW.
>>
>> This patch series first prepares the bcm2835-rng to be more future proof and
>> support newer platforms, and the last part brings in what is necessary to
>> migrate the bcm63xx-rng over to bcm2835-rng. Finally we delete bcm63xx-rng
>> completely.
>>
>> The reason why BCM2835 RNG was kept over BCM63xx RNG is because the former
>> deals correctly with a warm up count and the number of words available in the
>> FIFO size.
>
> are these the same patches as in this branch [1]?
>
> https://github.com/ffainelli/linux/commits/rng-consolidation
Yes, this branch contains these 12 patches.
--
Florian
Florian Fainelli <[email protected]> writes:
> bcm2835-rng is now capable of supporting the BCM63xx hardware, so remove
> the driver which duplicates the same functionality.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/char/hw_random/Kconfig | 13 ---
> drivers/char/hw_random/Makefile | 1 -
> drivers/char/hw_random/bcm63xx-rng.c | 154 -----------------------------------
> 3 files changed, 168 deletions(-)
> delete mode 100644 drivers/char/hw_random/bcm63xx-rng.c
>
> diff --git a/drivers/char/hw_random/bcm63xx-rng.c b/drivers/char/hw_random/bcm63xx-rng.c
> deleted file mode 100644
> index 5132c9cde50d..000000000000
> --- a/drivers/char/hw_random/bcm63xx-rng.c
> +++ /dev/null
> -static int bcm63xx_rng_data_present(struct hwrng *rng, int wait)
> -{
> - struct bcm63xx_rng_priv *priv = to_rng_priv(rng);
> -
> - return __raw_readl(priv->regs + RNG_STAT) & RNG_AVAIL_MASK;
> -}
It looks like this method isn't in the 2835 implementation. Should it
get ported over?
On 11/03/2017 01:18 PM, Eric Anholt wrote:
> Florian Fainelli <[email protected]> writes:
>
>> bcm2835-rng is now capable of supporting the BCM63xx hardware, so remove
>> the driver which duplicates the same functionality.
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> drivers/char/hw_random/Kconfig | 13 ---
>> drivers/char/hw_random/Makefile | 1 -
>> drivers/char/hw_random/bcm63xx-rng.c | 154 -----------------------------------
>> 3 files changed, 168 deletions(-)
>> delete mode 100644 drivers/char/hw_random/bcm63xx-rng.c
>>
>
>> diff --git a/drivers/char/hw_random/bcm63xx-rng.c b/drivers/char/hw_random/bcm63xx-rng.c
>> deleted file mode 100644
>> index 5132c9cde50d..000000000000
>> --- a/drivers/char/hw_random/bcm63xx-rng.c
>> +++ /dev/null
>
>> -static int bcm63xx_rng_data_present(struct hwrng *rng, int wait)
>> -{
>> - struct bcm63xx_rng_priv *priv = to_rng_priv(rng);
>> -
>> - return __raw_readl(priv->regs + RNG_STAT) & RNG_AVAIL_MASK;
>> -}
>
> It looks like this method isn't in the 2835 implementation. Should it
> get ported over?
The read method already has something similar in that it busy loops
until it gets some random words available, which is why I did not port
it over.
--
Florian
Hi Florian,
> Florian Fainelli <[email protected]> hat am 2. November 2017 um 02:04 geschrieben:
>
>
> One of the last steps before bcm63xx-rng can be eliminated is to manage
> a clock during hwrng::init and hwrng::cleanup, so fetch it in the probe
> function, and manage it during these two steps when valid.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/char/hw_random/bcm2835-rng.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
> index ed20e0b6b7ae..35928efb52e7 100644
> --- a/drivers/char/hw_random/bcm2835-rng.c
> +++ b/drivers/char/hw_random/bcm2835-rng.c
> @@ -15,6 +15,7 @@
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/printk.h>
> +#include <linux/clk.h>
>
> #define RNG_CTRL 0x0
> #define RNG_STATUS 0x4
> @@ -33,6 +34,7 @@ struct bcm2835_rng_priv {
> struct hwrng rng;
> void __iomem *base;
> bool mask_interrupts;
> + struct clk *clk;
> };
>
> static inline struct bcm2835_rng_priv *to_rng_priv(struct hwrng *rng)
> @@ -67,6 +69,11 @@ static int bcm2835_rng_init(struct hwrng *rng)
> {
> struct bcm2835_rng_priv *priv = to_rng_priv(rng);
> u32 val;
> + int ret;
> +
> + ret = clk_prepare_enable(priv->clk);
> + if (ret)
> + return ret;
>
> if (priv->mask_interrupts) {
> /* mask the interrupt */
> @@ -88,6 +95,8 @@ static void bcm2835_rng_cleanup(struct hwrng *rng)
>
> /* disable rng hardware */
> __raw_writel(0, priv->base + RNG_CTRL);
> +
> + clk_disable_unprepare(priv->clk);
> }
>
> struct bcm2835_rng_of_data {
> @@ -130,6 +139,11 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
> return PTR_ERR(priv->base);
> }
>
> + /* Clock is optional on most platforms */
> + priv->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(priv->clk))
> + priv->clk = NULL;
at least in case of EPROBE_DEFERED this isn't the expected behavior. Maybe we should better trigger on non-existing clock?
> +
> priv->rng.name = "bcm2835-rng";
> priv->rng.init = bcm2835_rng_init;
> priv->rng.read = bcm2835_rng_read;
> --
> 2.9.3
>
Hi Stefan,
On 11/04/2017 06:50 AM, Stefan Wahren wrote:
> Hi Florian,
>
>> Florian Fainelli <[email protected]> hat am 2. November 2017 um 02:04 geschrieben:
>>
>>
>> One of the last steps before bcm63xx-rng can be eliminated is to manage
>> a clock during hwrng::init and hwrng::cleanup, so fetch it in the probe
>> function, and manage it during these two steps when valid.
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> drivers/char/hw_random/bcm2835-rng.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
>> index ed20e0b6b7ae..35928efb52e7 100644
>> --- a/drivers/char/hw_random/bcm2835-rng.c
>> +++ b/drivers/char/hw_random/bcm2835-rng.c
>> @@ -15,6 +15,7 @@
>> #include <linux/of_platform.h>
>> #include <linux/platform_device.h>
>> #include <linux/printk.h>
>> +#include <linux/clk.h>
>>
>> #define RNG_CTRL 0x0
>> #define RNG_STATUS 0x4
>> @@ -33,6 +34,7 @@ struct bcm2835_rng_priv {
>> struct hwrng rng;
>> void __iomem *base;
>> bool mask_interrupts;
>> + struct clk *clk;
>> };
>>
>> static inline struct bcm2835_rng_priv *to_rng_priv(struct hwrng *rng)
>> @@ -67,6 +69,11 @@ static int bcm2835_rng_init(struct hwrng *rng)
>> {
>> struct bcm2835_rng_priv *priv = to_rng_priv(rng);
>> u32 val;
>> + int ret;
>> +
>> + ret = clk_prepare_enable(priv->clk);
>> + if (ret)
>> + return ret;
>>
>> if (priv->mask_interrupts) {
>> /* mask the interrupt */
>> @@ -88,6 +95,8 @@ static void bcm2835_rng_cleanup(struct hwrng *rng)
>>
>> /* disable rng hardware */
>> __raw_writel(0, priv->base + RNG_CTRL);
>> +
>> + clk_disable_unprepare(priv->clk);
>> }
>>
>> struct bcm2835_rng_of_data {
>> @@ -130,6 +139,11 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
>> return PTR_ERR(priv->base);
>> }
>>
>> + /* Clock is optional on most platforms */
>> + priv->clk = devm_clk_get(dev, NULL);
>> + if (IS_ERR(priv->clk))
>> + priv->clk = NULL;
>
> at least in case of EPROBE_DEFERED this isn't the expected behavior. Maybe we should better trigger on non-existing clock?
Good point, so more like:
if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -ENODEV)?
--
Florian
> Florian Fainelli <[email protected]> hat am 4. November 2017 um 18:59 geschrieben:
>
>
> Hi Stefan,
>
> On 11/04/2017 06:50 AM, Stefan Wahren wrote:
> > Hi Florian,
> >
> >> Florian Fainelli <[email protected]> hat am 2. November 2017 um 02:04 geschrieben:
> >>
> >>
> >> One of the last steps before bcm63xx-rng can be eliminated is to manage
> >> a clock during hwrng::init and hwrng::cleanup, so fetch it in the probe
> >> function, and manage it during these two steps when valid.
> >>
> >> Signed-off-by: Florian Fainelli <[email protected]>
> >> ---
> >> drivers/char/hw_random/bcm2835-rng.c | 14 ++++++++++++++
> >> 1 file changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
> >> index ed20e0b6b7ae..35928efb52e7 100644
> >> --- a/drivers/char/hw_random/bcm2835-rng.c
> >> +++ b/drivers/char/hw_random/bcm2835-rng.c
> >> @@ -15,6 +15,7 @@
> >> #include <linux/of_platform.h>
> >> #include <linux/platform_device.h>
> >> #include <linux/printk.h>
> >> +#include <linux/clk.h>
> >>
> >> #define RNG_CTRL 0x0
> >> #define RNG_STATUS 0x4
> >> @@ -33,6 +34,7 @@ struct bcm2835_rng_priv {
> >> struct hwrng rng;
> >> void __iomem *base;
> >> bool mask_interrupts;
> >> + struct clk *clk;
> >> };
> >>
> >> static inline struct bcm2835_rng_priv *to_rng_priv(struct hwrng *rng)
> >> @@ -67,6 +69,11 @@ static int bcm2835_rng_init(struct hwrng *rng)
> >> {
> >> struct bcm2835_rng_priv *priv = to_rng_priv(rng);
> >> u32 val;
> >> + int ret;
> >> +
> >> + ret = clk_prepare_enable(priv->clk);
> >> + if (ret)
> >> + return ret;
> >>
> >> if (priv->mask_interrupts) {
> >> /* mask the interrupt */
> >> @@ -88,6 +95,8 @@ static void bcm2835_rng_cleanup(struct hwrng *rng)
> >>
> >> /* disable rng hardware */
> >> __raw_writel(0, priv->base + RNG_CTRL);
> >> +
> >> + clk_disable_unprepare(priv->clk);
> >> }
> >>
> >> struct bcm2835_rng_of_data {
> >> @@ -130,6 +139,11 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
> >> return PTR_ERR(priv->base);
> >> }
> >>
> >> + /* Clock is optional on most platforms */
> >> + priv->clk = devm_clk_get(dev, NULL);
> >> + if (IS_ERR(priv->clk))
> >> + priv->clk = NULL;
> >
> > at least in case of EPROBE_DEFERED this isn't the expected behavior. Maybe we should better trigger on non-existing clock?
>
> Good point, so more like:
>
> if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -ENODEV)?
Unfortunately we need to return the error in all other cases. Please take a look at devm_usb_get_phy in dwc2 [1]. AFAIK we don't need to take care of ENXIO in our case.
http://elixir.free-electrons.com/linux/latest/source/drivers/usb/dwc2/platform.c#L247
>
> --
> Florian
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Florian
> Florian Fainelli <[email protected]> hat am 2. November 2017 um 02:04 geschrieben:
>
>
> We have now incorporated all necessary functionality for the BCM63xx
> platforms to successfully migrate over bcm2835-rng, so add the final
> bits: Kconfig selection and proper platform_device device type matching
> to keep the same platform device name for registration to work.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/char/hw_random/Kconfig | 7 ++++---
> drivers/char/hw_random/bcm2835-rng.c | 11 ++++++++++-
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 95a031e9eced..d0689cc8c7fc 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -87,12 +87,13 @@ config HW_RANDOM_BCM63XX
> If unusure, say Y.
>
> config HW_RANDOM_BCM2835
> - tristate "Broadcom BCM2835 Random Number Generator support"
> - depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X
> + tristate "Broadcom BCM2835/BCM63xx Random Number Generator support"
> + depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X || \
> + ARCH_BCM_63XX || BCM63XX || BMIPS_GENERIC
> default HW_RANDOM
> ---help---
> This driver provides kernel-side support for the Random Number
> - Generator hardware found on the Broadcom BCM2835 SoCs.
> + Generator hardware found on the Broadcom BCM2835 and BCM63xx SoCs.
>
> To compile this driver as a module, choose M here: the
> module will be called bcm2835-rng
> diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
> index 650e0033c273..d9ffe14f312b 100644
> --- a/drivers/char/hw_random/bcm2835-rng.c
> +++ b/drivers/char/hw_random/bcm2835-rng.c
> @@ -131,6 +131,7 @@ static const struct of_device_id bcm2835_rng_of_match[] = {
> { .compatible = "brcm,bcm2835-rng"},
> { .compatible = "brcm,bcm-nsp-rng", .data = &nsp_rng_of_data },
> { .compatible = "brcm,bcm5301x-rng", .data = &nsp_rng_of_data },
> + { .compatible = "brcm,bcm6368-rng"},
> {},
> };
>
> @@ -164,7 +165,7 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
> if (IS_ERR(priv->clk))
> priv->clk = NULL;
>
> - priv->rng.name = "bcm2835-rng";
> + priv->rng.name = pdev->id_entry->name;
this change breaks registration on bcm2835, because the name is NULL.
Regards
Stefan
> priv->rng.init = bcm2835_rng_init;
> priv->rng.read = bcm2835_rng_read;
> priv->rng.cleanup = bcm2835_rng_cleanup;
> @@ -190,12 +191,20 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
>
> MODULE_DEVICE_TABLE(of, bcm2835_rng_of_match);
>
> +static struct platform_device_id bcm2835_rng_devtype[] = {
> + { .name = "bcm2835-rng" },
> + { .name = "bcm63xx-rng" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, bcm2835_rng_devtype);
> +
> static struct platform_driver bcm2835_rng_driver = {
> .driver = {
> .name = "bcm2835-rng",
> .of_match_table = bcm2835_rng_of_match,
> },
> .probe = bcm2835_rng_probe,
> + .id_table = bcm2835_rng_devtype,
> };
> module_platform_driver(bcm2835_rng_driver);
>
> --
> 2.9.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Florian
> > >> + /* Clock is optional on most platforms */
> > >> + priv->clk = devm_clk_get(dev, NULL);
> > >> + if (IS_ERR(priv->clk))
> > >> + priv->clk = NULL;
> > >
> > > at least in case of EPROBE_DEFERED this isn't the expected behavior. Maybe we should better trigger on non-existing clock?
> >
> > Good point, so more like:
> >
> > if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -ENODEV)?
>
> Unfortunately we need to return the error in all other cases. Please
> take a look at devm_usb_get_phy in dwc2 [1]. AFAIK we don't need to
> take care of ENXIO in our case.
A few subsystems have a get_optional() call, e.g.
devm_phy_optional_get(). It does not return an error when the phy is
not supposed to exist, but in all other cases, it does.
Maybe consider adding devm_clk_get_optional()?
Andrew
On Sat, Nov 04, 2017 at 08:37:31PM +0100, Andrew Lunn wrote:
> Hi Florian
>
> > > >> + /* Clock is optional on most platforms */
> > > >> + priv->clk = devm_clk_get(dev, NULL);
> > > >> + if (IS_ERR(priv->clk))
> > > >> + priv->clk = NULL;
> > > >
> > > > at least in case of EPROBE_DEFERED this isn't the expected behavior. Maybe we should better trigger on non-existing clock?
> > >
> > > Good point, so more like:
> > >
> > > if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -ENODEV)?
> >
>
> > Unfortunately we need to return the error in all other cases. Please
> > take a look at devm_usb_get_phy in dwc2 [1]. AFAIK we don't need to
> > take care of ENXIO in our case.
>
> A few subsystems have a get_optional() call, e.g.
> devm_phy_optional_get(). It does not return an error when the phy is
> not supposed to exist, but in all other cases, it does.
>
> Maybe consider adding devm_clk_get_optional()?
The clk API outside of DT doesn't have knowledge of when it's "complete"
to be able to determine whether the clock is not present or temporarily
missing. I've already NAK'd this suggestion.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
> The clk API outside of DT doesn't have knowledge of when it's "complete"
> to be able to determine whether the clock is not present or temporarily
> missing. I've already NAK'd this suggestion.
Hi Russell
O.K, yes, makes sense.
We do have of_clk_get_by_name() and of_clk_get(). Would optional
variants of these be acceptable?
Andrew
On 11/04/2017 11:27 AM, Stefan Wahren wrote:
> Hi Florian
>
>> Florian Fainelli <[email protected]> hat am 2. November 2017 um 02:04 geschrieben:
>>
>>
>> We have now incorporated all necessary functionality for the BCM63xx
>> platforms to successfully migrate over bcm2835-rng, so add the final
>> bits: Kconfig selection and proper platform_device device type matching
>> to keep the same platform device name for registration to work.
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> drivers/char/hw_random/Kconfig | 7 ++++---
>> drivers/char/hw_random/bcm2835-rng.c | 11 ++++++++++-
>> 2 files changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
>> index 95a031e9eced..d0689cc8c7fc 100644
>> --- a/drivers/char/hw_random/Kconfig
>> +++ b/drivers/char/hw_random/Kconfig
>> @@ -87,12 +87,13 @@ config HW_RANDOM_BCM63XX
>> If unusure, say Y.
>>
>> config HW_RANDOM_BCM2835
>> - tristate "Broadcom BCM2835 Random Number Generator support"
>> - depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X
>> + tristate "Broadcom BCM2835/BCM63xx Random Number Generator support"
>> + depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X || \
>> + ARCH_BCM_63XX || BCM63XX || BMIPS_GENERIC
>> default HW_RANDOM
>> ---help---
>> This driver provides kernel-side support for the Random Number
>> - Generator hardware found on the Broadcom BCM2835 SoCs.
>> + Generator hardware found on the Broadcom BCM2835 and BCM63xx SoCs.
>>
>> To compile this driver as a module, choose M here: the
>> module will be called bcm2835-rng
>> diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
>> index 650e0033c273..d9ffe14f312b 100644
>> --- a/drivers/char/hw_random/bcm2835-rng.c
>> +++ b/drivers/char/hw_random/bcm2835-rng.c
>> @@ -131,6 +131,7 @@ static const struct of_device_id bcm2835_rng_of_match[] = {
>> { .compatible = "brcm,bcm2835-rng"},
>> { .compatible = "brcm,bcm-nsp-rng", .data = &nsp_rng_of_data },
>> { .compatible = "brcm,bcm5301x-rng", .data = &nsp_rng_of_data },
>> + { .compatible = "brcm,bcm6368-rng"},
>> {},
>> };
>>
>> @@ -164,7 +165,7 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
>> if (IS_ERR(priv->clk))
>> priv->clk = NULL;
>>
>> - priv->rng.name = "bcm2835-rng";
>> + priv->rng.name = pdev->id_entry->name;
>
> this change breaks registration on bcm2835, because the name is NULL.
OH right, I will fix that in v2, I am assuming that if you did something
like this, things still work correctly for you on 2835:
if (!priv->id_entry->name)
priv->rng.name = "bcm2835-rng";
else
priv->rng.name = priv->id_entry->name;
?
>
> Regards
> Stefan
>
>> priv->rng.init = bcm2835_rng_init;
>> priv->rng.read = bcm2835_rng_read;
>> priv->rng.cleanup = bcm2835_rng_cleanup;
>> @@ -190,12 +191,20 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
>>
>> MODULE_DEVICE_TABLE(of, bcm2835_rng_of_match);
>>
>> +static struct platform_device_id bcm2835_rng_devtype[] = {
>> + { .name = "bcm2835-rng" },
>> + { .name = "bcm63xx-rng" },
>> + { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(platform, bcm2835_rng_devtype);
>> +
>> static struct platform_driver bcm2835_rng_driver = {
>> .driver = {
>> .name = "bcm2835-rng",
>> .of_match_table = bcm2835_rng_of_match,
>> },
>> .probe = bcm2835_rng_probe,
>> + .id_table = bcm2835_rng_devtype,
>> };
>> module_platform_driver(bcm2835_rng_driver);
>>
>> --
>> 2.9.3
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Florian
On Wed, Nov 01, 2017 at 06:04:06PM -0700, Florian Fainelli wrote:
> Since the same block is used on BCM2835 and BCM6368, merge the bindings
> and remove the brcm,bcm6368.txt binding document.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> .../devicetree/bindings/rng/brcm,bcm2835.txt | 22 +++++++++++++++++++---
> .../devicetree/bindings/rng/brcm,bcm6368.txt | 17 -----------------
> 2 files changed, 19 insertions(+), 20 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/rng/brcm,bcm6368.txt
Acked-by: Rob Herring <[email protected]>
> Florian Fainelli <[email protected]> hat am 6. November 2017 um 21:16 geschrieben:
>
>
> On 11/04/2017 11:27 AM, Stefan Wahren wrote:
> > Hi Florian
> >
> >> Florian Fainelli <[email protected]> hat am 2. November 2017 um 02:04 geschrieben:
> >>
> >>
> >> We have now incorporated all necessary functionality for the BCM63xx
> >> platforms to successfully migrate over bcm2835-rng, so add the final
> >> bits: Kconfig selection and proper platform_device device type matching
> >> to keep the same platform device name for registration to work.
> >>
> >> Signed-off-by: Florian Fainelli <[email protected]>
> >> ---
> >> drivers/char/hw_random/Kconfig | 7 ++++---
> >> drivers/char/hw_random/bcm2835-rng.c | 11 ++++++++++-
> >> 2 files changed, 14 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> >> index 95a031e9eced..d0689cc8c7fc 100644
> >> --- a/drivers/char/hw_random/Kconfig
> >> +++ b/drivers/char/hw_random/Kconfig
> >> @@ -87,12 +87,13 @@ config HW_RANDOM_BCM63XX
> >> If unusure, say Y.
> >>
> >> config HW_RANDOM_BCM2835
> >> - tristate "Broadcom BCM2835 Random Number Generator support"
> >> - depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X
> >> + tristate "Broadcom BCM2835/BCM63xx Random Number Generator support"
> >> + depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X || \
> >> + ARCH_BCM_63XX || BCM63XX || BMIPS_GENERIC
> >> default HW_RANDOM
> >> ---help---
> >> This driver provides kernel-side support for the Random Number
> >> - Generator hardware found on the Broadcom BCM2835 SoCs.
> >> + Generator hardware found on the Broadcom BCM2835 and BCM63xx SoCs.
> >>
> >> To compile this driver as a module, choose M here: the
> >> module will be called bcm2835-rng
> >> diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
> >> index 650e0033c273..d9ffe14f312b 100644
> >> --- a/drivers/char/hw_random/bcm2835-rng.c
> >> +++ b/drivers/char/hw_random/bcm2835-rng.c
> >> @@ -131,6 +131,7 @@ static const struct of_device_id bcm2835_rng_of_match[] = {
> >> { .compatible = "brcm,bcm2835-rng"},
> >> { .compatible = "brcm,bcm-nsp-rng", .data = &nsp_rng_of_data },
> >> { .compatible = "brcm,bcm5301x-rng", .data = &nsp_rng_of_data },
> >> + { .compatible = "brcm,bcm6368-rng"},
> >> {},
> >> };
> >>
> >> @@ -164,7 +165,7 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
> >> if (IS_ERR(priv->clk))
> >> priv->clk = NULL;
> >>
> >> - priv->rng.name = "bcm2835-rng";
> >> + priv->rng.name = pdev->id_entry->name;
> >
> > this change breaks registration on bcm2835, because the name is NULL.
>
> OH right, I will fix that in v2, I am assuming that if you did something
> like this, things still work correctly for you on 2835:
>
> if (!priv->id_entry->name)
> priv->rng.name = "bcm2835-rng";
> else
> priv->rng.name = priv->id_entry->name;
>
> ?
I think that should work, but i'm not really happy about it. I prefer a more general solution. Looking at the other rng drivers shows the following pattern:
priv->rng.name = pdev->name;
I need to verify this on bcm2835.
>
> >
> > Regards
> > Stefan
> >
> >> priv->rng.init = bcm2835_rng_init;
> >> priv->rng.read = bcm2835_rng_read;
> >> priv->rng.cleanup = bcm2835_rng_cleanup;
> >> @@ -190,12 +191,20 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
> >>
> >> MODULE_DEVICE_TABLE(of, bcm2835_rng_of_match);
> >>
> >> +static struct platform_device_id bcm2835_rng_devtype[] = {
> >> + { .name = "bcm2835-rng" },
> >> + { .name = "bcm63xx-rng" },
> >> + { /* sentinel */ }
> >> +};
> >> +MODULE_DEVICE_TABLE(platform, bcm2835_rng_devtype);
> >> +
> >> static struct platform_driver bcm2835_rng_driver = {
> >> .driver = {
> >> .name = "bcm2835-rng",
> >> .of_match_table = bcm2835_rng_of_match,
> >> },
> >> .probe = bcm2835_rng_probe,
> >> + .id_table = bcm2835_rng_devtype,
> >> };
> >> module_platform_driver(bcm2835_rng_driver);
> >>
> >> --
> >> 2.9.3
> >>
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> [email protected]
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
> --
> Florian
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> Stefan Wahren <[email protected]> hat am 7. November 2017 um 07:45 geschrieben:
>
>
> > Florian Fainelli <[email protected]> hat am 6. November 2017 um 21:16 geschrieben:
> >
> >
> > On 11/04/2017 11:27 AM, Stefan Wahren wrote:
> > > Hi Florian
> > >
> > >> Florian Fainelli <[email protected]> hat am 2. November 2017 um 02:04 geschrieben:
> > >>
> > >>
> > >> We have now incorporated all necessary functionality for the BCM63xx
> > >> platforms to successfully migrate over bcm2835-rng, so add the final
> > >> bits: Kconfig selection and proper platform_device device type matching
> > >> to keep the same platform device name for registration to work.
> > >>
> > >> Signed-off-by: Florian Fainelli <[email protected]>
> > >> ---
> > >> drivers/char/hw_random/Kconfig | 7 ++++---
> > >> drivers/char/hw_random/bcm2835-rng.c | 11 ++++++++++-
> > >> 2 files changed, 14 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> > >> index 95a031e9eced..d0689cc8c7fc 100644
> > >> --- a/drivers/char/hw_random/Kconfig
> > >> +++ b/drivers/char/hw_random/Kconfig
> > >> @@ -87,12 +87,13 @@ config HW_RANDOM_BCM63XX
> > >> If unusure, say Y.
> > >>
> > >> config HW_RANDOM_BCM2835
> > >> - tristate "Broadcom BCM2835 Random Number Generator support"
> > >> - depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X
> > >> + tristate "Broadcom BCM2835/BCM63xx Random Number Generator support"
> > >> + depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X || \
> > >> + ARCH_BCM_63XX || BCM63XX || BMIPS_GENERIC
> > >> default HW_RANDOM
> > >> ---help---
> > >> This driver provides kernel-side support for the Random Number
> > >> - Generator hardware found on the Broadcom BCM2835 SoCs.
> > >> + Generator hardware found on the Broadcom BCM2835 and BCM63xx SoCs.
> > >>
> > >> To compile this driver as a module, choose M here: the
> > >> module will be called bcm2835-rng
> > >> diff --git a/drivers/char/hw_random/bcm2835-rng.c b/drivers/char/hw_random/bcm2835-rng.c
> > >> index 650e0033c273..d9ffe14f312b 100644
> > >> --- a/drivers/char/hw_random/bcm2835-rng.c
> > >> +++ b/drivers/char/hw_random/bcm2835-rng.c
> > >> @@ -131,6 +131,7 @@ static const struct of_device_id bcm2835_rng_of_match[] = {
> > >> { .compatible = "brcm,bcm2835-rng"},
> > >> { .compatible = "brcm,bcm-nsp-rng", .data = &nsp_rng_of_data },
> > >> { .compatible = "brcm,bcm5301x-rng", .data = &nsp_rng_of_data },
> > >> + { .compatible = "brcm,bcm6368-rng"},
> > >> {},
> > >> };
> > >>
> > >> @@ -164,7 +165,7 @@ static int bcm2835_rng_probe(struct platform_device *pdev)
> > >> if (IS_ERR(priv->clk))
> > >> priv->clk = NULL;
> > >>
> > >> - priv->rng.name = "bcm2835-rng";
> > >> + priv->rng.name = pdev->id_entry->name;
> > >
> > > this change breaks registration on bcm2835, because the name is NULL.
> >
> > OH right, I will fix that in v2, I am assuming that if you did something
> > like this, things still work correctly for you on 2835:
> >
> > if (!priv->id_entry->name)
> > priv->rng.name = "bcm2835-rng";
> > else
> > priv->rng.name = priv->id_entry->name;
> >
> > ?
>
> I think that should work, but i'm not really happy about it. I prefer a more general solution. Looking at the other rng drivers shows the following pattern:
>
> priv->rng.name = pdev->name;
>
> I need to verify this on bcm2835.
Okay pdev->name works for bcm2835