* [PATCH v3 0/5] Unify simple reset drivers
@ 2017-08-16 9:46 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:46 UTC (permalink / raw)
To: linux-kernel
Cc: Alexandru Gagniuc, Andre Przywara, Maxime Coquelin,
Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen,
Andreas Färber, linux-arm-kernel, kernel, Philipp Zabel
Hi,
This series introduces common reset ops for simple reset controllers.
"Simple" in this context means that they allow to directly control reset
lines by setting and clearing bits in a single register or in a contiguous
register range that is exclusive to the reset controller.
Parts of the sunxi driver and the socfpga, stm32, and zx2967 drivers are
merged into a single reset-simple driver.
The sunxi driver is kept around to register the early reset controllers,
but it reuses the exported reset_simple_ops.
The first patch depends on [1] or commit 726cc791c9b9 ("reset: sunxi: fix
number of reset lines") in linux-next. On linux-next, the second patch will
trivially conflict with commit 7799167b7a14 ("regulator: Convert to using
%pOF instead of full_name").
Changes since v2 [2]:
- Use of_device_get_match_data instead of of_match_device,
add kerneldoc comment for struct reset_simple_devdata and struct
reset_simple_data, suggested by Chen-Yu.
- Add reg_offset and nr_reset override to struct reset_simple_devdata,
instead of special casing socfpga, always check "altr,modrst-offset"
device tree property if a default reg_offset is set, based on
Alexandru's advice.
- Rename "*inverted" properties to "*active_low".
[1] https://patchwork.kernel.org/patch/9895433/
[2] https://patchwork.kernel.org/patch/9895833/
https://patchwork.kernel.org/patch/9895819/
https://patchwork.kernel.org/patch/9895831/
https://patchwork.kernel.org/patch/9895815/
https://patchwork.kernel.org/patch/9895841/
regards
Philipp
Philipp Zabel (5):
reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
reset: socfpga: use the reset-simple driver
reset: stm32: use the reset-simple driver
reset: zx2967: use the reset-simple driver
reset: simple: read back to make sure changes are applied
MAINTAINERS | 1 -
drivers/reset/Kconfig | 24 ++----
drivers/reset/Makefile | 4 +-
drivers/reset/reset-simple.c | 188 ++++++++++++++++++++++++++++++++++++++++++
drivers/reset/reset-simple.h | 45 ++++++++++
drivers/reset/reset-socfpga.c | 154 ----------------------------------
drivers/reset/reset-stm32.c | 108 ------------------------
drivers/reset/reset-sunxi.c | 104 ++---------------------
drivers/reset/reset-zx2967.c | 99 ----------------------
9 files changed, 249 insertions(+), 478 deletions(-)
create mode 100644 drivers/reset/reset-simple.c
create mode 100644 drivers/reset/reset-simple.h
delete mode 100644 drivers/reset/reset-socfpga.c
delete mode 100644 drivers/reset/reset-stm32.c
delete mode 100644 drivers/reset/reset-zx2967.c
--
2.11.0
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 0/5] Unify simple reset drivers
@ 2017-08-16 9:46 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This series introduces common reset ops for simple reset controllers.
"Simple" in this context means that they allow to directly control reset
lines by setting and clearing bits in a single register or in a contiguous
register range that is exclusive to the reset controller.
Parts of the sunxi driver and the socfpga, stm32, and zx2967 drivers are
merged into a single reset-simple driver.
The sunxi driver is kept around to register the early reset controllers,
but it reuses the exported reset_simple_ops.
The first patch depends on [1] or commit 726cc791c9b9 ("reset: sunxi: fix
number of reset lines") in linux-next. On linux-next, the second patch will
trivially conflict with commit 7799167b7a14 ("regulator: Convert to using
%pOF instead of full_name").
Changes since v2 [2]:
- Use of_device_get_match_data instead of of_match_device,
add kerneldoc comment for struct reset_simple_devdata and struct
reset_simple_data, suggested by Chen-Yu.
- Add reg_offset and nr_reset override to struct reset_simple_devdata,
instead of special casing socfpga, always check "altr,modrst-offset"
device tree property if a default reg_offset is set, based on
Alexandru's advice.
- Rename "*inverted" properties to "*active_low".
[1] https://patchwork.kernel.org/patch/9895433/
[2] https://patchwork.kernel.org/patch/9895833/
https://patchwork.kernel.org/patch/9895819/
https://patchwork.kernel.org/patch/9895831/
https://patchwork.kernel.org/patch/9895815/
https://patchwork.kernel.org/patch/9895841/
regards
Philipp
Philipp Zabel (5):
reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
reset: socfpga: use the reset-simple driver
reset: stm32: use the reset-simple driver
reset: zx2967: use the reset-simple driver
reset: simple: read back to make sure changes are applied
MAINTAINERS | 1 -
drivers/reset/Kconfig | 24 ++----
drivers/reset/Makefile | 4 +-
drivers/reset/reset-simple.c | 188 ++++++++++++++++++++++++++++++++++++++++++
drivers/reset/reset-simple.h | 45 ++++++++++
drivers/reset/reset-socfpga.c | 154 ----------------------------------
drivers/reset/reset-stm32.c | 108 ------------------------
drivers/reset/reset-sunxi.c | 104 ++---------------------
drivers/reset/reset-zx2967.c | 99 ----------------------
9 files changed, 249 insertions(+), 478 deletions(-)
create mode 100644 drivers/reset/reset-simple.c
create mode 100644 drivers/reset/reset-simple.h
delete mode 100644 drivers/reset/reset-socfpga.c
delete mode 100644 drivers/reset/reset-stm32.c
delete mode 100644 drivers/reset/reset-zx2967.c
--
2.11.0
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-16 9:46 ` Philipp Zabel
-1 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:46 UTC (permalink / raw)
To: linux-kernel
Cc: Alexandru Gagniuc, Andre Przywara, Maxime Coquelin,
Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen,
Andreas Färber, linux-arm-kernel, kernel, Philipp Zabel
Split reusable parts out of the sunxi driver, to add a driver for simple
reset controllers with reset lines that can be controlled by toggling
bits in exclusive, contiguous register ranges using read-modify-write
cycles under a spinlock. The separate sunxi driver still remains to
register the early reset controllers, but it reuses the reset-simple
ops.
The following patches will replace compatible reset drivers with
reset-simple, extending it where necessary.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v2:
- Add kerneldoc comment for struct reset_simple_devdata and struct
reset_simple_data.
- Rename "inverted" properties to "active_low".
- Use of_device_get_match_data instead of of_match_device.
---
drivers/reset/Kconfig | 11 ++++
drivers/reset/Makefile | 1 +
drivers/reset/reset-simple.c | 134 +++++++++++++++++++++++++++++++++++++++++++
drivers/reset/reset-simple.h | 41 +++++++++++++
drivers/reset/reset-sunxi.c | 104 ++-------------------------------
5 files changed, 193 insertions(+), 98 deletions(-)
create mode 100644 drivers/reset/reset-simple.c
create mode 100644 drivers/reset/reset-simple.h
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 52d5251660b9b..f7ba01a71daee 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -68,6 +68,16 @@ config RESET_PISTACHIO
help
This enables the reset driver for ImgTec Pistachio SoCs.
+config RESET_SIMPLE
+ bool "Simple Reset Controller Driver" if COMPILE_TEST
+ default ARCH_SUNXI
+ help
+ This enables a simple reset controller driver for reset lines that
+ that can be asserted and deasserted by toggling bits in a contiguous,
+ exclusive register space.
+
+ Currently this driver supports Allwinner SoCs.
+
config RESET_SOCFPGA
bool "SoCFPGA Reset Driver" if COMPILE_TEST
default ARCH_SOCFPGA
@@ -83,6 +93,7 @@ config RESET_STM32
config RESET_SUNXI
bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
default ARCH_SUNXI
+ select RESET_SIMPLE
help
This enables the reset driver for Allwinner SoCs.
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index b62783f50fe5b..ab4af99c3dfdc 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
obj-$(CONFIG_RESET_MESON) += reset-meson.o
obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
+obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
obj-$(CONFIG_RESET_STM32) += reset-stm32.o
obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
new file mode 100644
index 0000000000000..70c57c0758c39
--- /dev/null
+++ b/drivers/reset/reset-simple.c
@@ -0,0 +1,134 @@
+/*
+ * Simple Reset Controller Driver
+ *
+ * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
+ *
+ * Based on Allwinner SoCs Reset Controller driver
+ *
+ * Copyright 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+
+#include "reset-simple.h"
+
+static inline struct reset_simple_data *
+to_reset_simple_data(struct reset_controller_dev *rcdev)
+{
+ return container_of(rcdev, struct reset_simple_data, rcdev);
+}
+
+static int reset_simple_set(struct reset_controller_dev *rcdev,
+ unsigned long id, bool assert)
+{
+ struct reset_simple_data *data = to_reset_simple_data(rcdev);
+ int reg_width = sizeof(u32);
+ int bank = id / (reg_width * BITS_PER_BYTE);
+ int offset = id % (reg_width * BITS_PER_BYTE);
+ unsigned long flags;
+ u32 reg;
+
+ spin_lock_irqsave(&data->lock, flags);
+
+ reg = readl(data->membase + (bank * reg_width));
+ if (assert ^ data->active_low)
+ reg |= BIT(offset);
+ else
+ reg &= ~BIT(offset);
+ writel(reg, data->membase + (bank * reg_width));
+
+ spin_unlock_irqrestore(&data->lock, flags);
+
+ return 0;
+}
+
+static int reset_simple_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ return reset_simple_set(rcdev, id, true);
+}
+
+static int reset_simple_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ return reset_simple_set(rcdev, id, false);
+}
+
+const struct reset_control_ops reset_simple_ops = {
+ .assert = reset_simple_assert,
+ .deassert = reset_simple_deassert,
+};
+
+/**
+ * struct reset_simple_devdata - simple reset controller properties
+ * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
+ * are set to assert the reset.
+ */
+struct reset_simple_devdata {
+ bool active_low;
+};
+
+static const struct reset_simple_devdata reset_simple_active_low = {
+ .active_low = true,
+};
+
+static const struct of_device_id reset_simple_dt_ids[] = {
+ { .compatible = "allwinner,sun6i-a31-clock-reset",
+ .data = &reset_simple_active_low },
+ { /* sentinel */ },
+};
+
+static int reset_simple_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct reset_simple_devdata *devdata;
+ struct reset_simple_data *data;
+ void __iomem *membase;
+ struct resource *res;
+
+ devdata = of_device_get_match_data(dev);
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ membase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(membase))
+ return PTR_ERR(membase);
+
+ spin_lock_init(&data->lock);
+ data->membase = membase;
+ data->rcdev.owner = THIS_MODULE;
+ data->rcdev.nr_resets = resource_size(res) * BITS_PER_BYTE;
+ data->rcdev.ops = &reset_simple_ops;
+ data->rcdev.of_node = dev->of_node;
+
+ if (devdata)
+ data->active_low = devdata->active_low;
+
+ return devm_reset_controller_register(dev, &data->rcdev);
+}
+
+static struct platform_driver reset_simple_driver = {
+ .probe = reset_simple_probe,
+ .driver = {
+ .name = "simple-reset",
+ .of_match_table = reset_simple_dt_ids,
+ },
+};
+builtin_platform_driver(reset_simple_driver);
diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
new file mode 100644
index 0000000000000..39af2014b5f12
--- /dev/null
+++ b/drivers/reset/reset-simple.h
@@ -0,0 +1,41 @@
+/*
+ * Simple Reset Controller ops
+ *
+ * Based on Allwinner SoCs Reset Controller driver
+ *
+ * Copyright 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __RESET_SIMPLE_H__
+#define __RESET_SIMPLE_H__
+
+#include <linux/io.h>
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+
+/**
+ * struct reset_simple_data - driver data for simple reset controllers
+ * @lock: spinlock to protect registers during read-modify-write cycles
+ * @membase: memory mapped I/O register range
+ * @rcdev: reset controller device base structure
+ * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
+ * are set to assert the reset. Note that this says nothing about
+ * the voltage level of the actual reset line.
+ */
+struct reset_simple_data {
+ spinlock_t lock;
+ void __iomem *membase;
+ struct reset_controller_dev rcdev;
+ bool active_low;
+};
+
+extern const struct reset_control_ops reset_simple_ops;
+
+#endif /* __RESET_SIMPLE_H__ */
diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
index 2c7dd1fd08df6..db9a1a75523f4 100644
--- a/drivers/reset/reset-sunxi.c
+++ b/drivers/reset/reset-sunxi.c
@@ -22,64 +22,11 @@
#include <linux/spinlock.h>
#include <linux/types.h>
-struct sunxi_reset_data {
- spinlock_t lock;
- void __iomem *membase;
- struct reset_controller_dev rcdev;
-};
-
-static int sunxi_reset_assert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct sunxi_reset_data *data = container_of(rcdev,
- struct sunxi_reset_data,
- rcdev);
- int reg_width = sizeof(u32);
- int bank = id / (reg_width * BITS_PER_BYTE);
- int offset = id % (reg_width * BITS_PER_BYTE);
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * reg_width));
- writel(reg & ~BIT(offset), data->membase + (bank * reg_width));
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static int sunxi_reset_deassert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct sunxi_reset_data *data = container_of(rcdev,
- struct sunxi_reset_data,
- rcdev);
- int reg_width = sizeof(u32);
- int bank = id / (reg_width * BITS_PER_BYTE);
- int offset = id % (reg_width * BITS_PER_BYTE);
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * reg_width));
- writel(reg | BIT(offset), data->membase + (bank * reg_width));
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static const struct reset_control_ops sunxi_reset_ops = {
- .assert = sunxi_reset_assert,
- .deassert = sunxi_reset_deassert,
-};
+#include "reset-simple.h"
static int sunxi_reset_init(struct device_node *np)
{
- struct sunxi_reset_data *data;
+ struct reset_simple_data *data;
struct resource res;
resource_size_t size;
int ret;
@@ -108,8 +55,9 @@ static int sunxi_reset_init(struct device_node *np)
data->rcdev.owner = THIS_MODULE;
data->rcdev.nr_resets = size * 8;
- data->rcdev.ops = &sunxi_reset_ops;
+ data->rcdev.ops = &reset_simple_ops;
data->rcdev.of_node = np;
+ data->active_low = true;
return reset_controller_register(&data->rcdev);
@@ -122,6 +70,8 @@ static int sunxi_reset_init(struct device_node *np)
* These are the reset controller we need to initialize early on in
* our system, before we can even think of using a regular device
* driver for it.
+ * The controllers that we can register through the regular device
+ * model are handled by the simple reset driver directly.
*/
static const struct of_device_id sunxi_early_reset_dt_ids[] __initconst = {
{ .compatible = "allwinner,sun6i-a31-ahb1-reset", },
@@ -135,45 +85,3 @@ void __init sun6i_reset_init(void)
for_each_matching_node(np, sunxi_early_reset_dt_ids)
sunxi_reset_init(np);
}
-
-/*
- * And these are the controllers we can register through the regular
- * device model.
- */
-static const struct of_device_id sunxi_reset_dt_ids[] = {
- { .compatible = "allwinner,sun6i-a31-clock-reset", },
- { /* sentinel */ },
-};
-
-static int sunxi_reset_probe(struct platform_device *pdev)
-{
- struct sunxi_reset_data *data;
- struct resource *res;
-
- data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- data->membase = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(data->membase))
- return PTR_ERR(data->membase);
-
- spin_lock_init(&data->lock);
-
- data->rcdev.owner = THIS_MODULE;
- data->rcdev.nr_resets = resource_size(res) * 8;
- data->rcdev.ops = &sunxi_reset_ops;
- data->rcdev.of_node = pdev->dev.of_node;
-
- return devm_reset_controller_register(&pdev->dev, &data->rcdev);
-}
-
-static struct platform_driver sunxi_reset_driver = {
- .probe = sunxi_reset_probe,
- .driver = {
- .name = "sunxi-reset",
- .of_match_table = sunxi_reset_dt_ids,
- },
-};
-builtin_platform_driver(sunxi_reset_driver);
--
2.11.0
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
@ 2017-08-16 9:46 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:46 UTC (permalink / raw)
To: linux-arm-kernel
Split reusable parts out of the sunxi driver, to add a driver for simple
reset controllers with reset lines that can be controlled by toggling
bits in exclusive, contiguous register ranges using read-modify-write
cycles under a spinlock. The separate sunxi driver still remains to
register the early reset controllers, but it reuses the reset-simple
ops.
The following patches will replace compatible reset drivers with
reset-simple, extending it where necessary.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v2:
- Add kerneldoc comment for struct reset_simple_devdata and struct
reset_simple_data.
- Rename "inverted" properties to "active_low".
- Use of_device_get_match_data instead of of_match_device.
---
drivers/reset/Kconfig | 11 ++++
drivers/reset/Makefile | 1 +
drivers/reset/reset-simple.c | 134 +++++++++++++++++++++++++++++++++++++++++++
drivers/reset/reset-simple.h | 41 +++++++++++++
drivers/reset/reset-sunxi.c | 104 ++-------------------------------
5 files changed, 193 insertions(+), 98 deletions(-)
create mode 100644 drivers/reset/reset-simple.c
create mode 100644 drivers/reset/reset-simple.h
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 52d5251660b9b..f7ba01a71daee 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -68,6 +68,16 @@ config RESET_PISTACHIO
help
This enables the reset driver for ImgTec Pistachio SoCs.
+config RESET_SIMPLE
+ bool "Simple Reset Controller Driver" if COMPILE_TEST
+ default ARCH_SUNXI
+ help
+ This enables a simple reset controller driver for reset lines that
+ that can be asserted and deasserted by toggling bits in a contiguous,
+ exclusive register space.
+
+ Currently this driver supports Allwinner SoCs.
+
config RESET_SOCFPGA
bool "SoCFPGA Reset Driver" if COMPILE_TEST
default ARCH_SOCFPGA
@@ -83,6 +93,7 @@ config RESET_STM32
config RESET_SUNXI
bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
default ARCH_SUNXI
+ select RESET_SIMPLE
help
This enables the reset driver for Allwinner SoCs.
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index b62783f50fe5b..ab4af99c3dfdc 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
obj-$(CONFIG_RESET_MESON) += reset-meson.o
obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
+obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
obj-$(CONFIG_RESET_STM32) += reset-stm32.o
obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
new file mode 100644
index 0000000000000..70c57c0758c39
--- /dev/null
+++ b/drivers/reset/reset-simple.c
@@ -0,0 +1,134 @@
+/*
+ * Simple Reset Controller Driver
+ *
+ * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
+ *
+ * Based on Allwinner SoCs Reset Controller driver
+ *
+ * Copyright 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+
+#include "reset-simple.h"
+
+static inline struct reset_simple_data *
+to_reset_simple_data(struct reset_controller_dev *rcdev)
+{
+ return container_of(rcdev, struct reset_simple_data, rcdev);
+}
+
+static int reset_simple_set(struct reset_controller_dev *rcdev,
+ unsigned long id, bool assert)
+{
+ struct reset_simple_data *data = to_reset_simple_data(rcdev);
+ int reg_width = sizeof(u32);
+ int bank = id / (reg_width * BITS_PER_BYTE);
+ int offset = id % (reg_width * BITS_PER_BYTE);
+ unsigned long flags;
+ u32 reg;
+
+ spin_lock_irqsave(&data->lock, flags);
+
+ reg = readl(data->membase + (bank * reg_width));
+ if (assert ^ data->active_low)
+ reg |= BIT(offset);
+ else
+ reg &= ~BIT(offset);
+ writel(reg, data->membase + (bank * reg_width));
+
+ spin_unlock_irqrestore(&data->lock, flags);
+
+ return 0;
+}
+
+static int reset_simple_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ return reset_simple_set(rcdev, id, true);
+}
+
+static int reset_simple_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ return reset_simple_set(rcdev, id, false);
+}
+
+const struct reset_control_ops reset_simple_ops = {
+ .assert = reset_simple_assert,
+ .deassert = reset_simple_deassert,
+};
+
+/**
+ * struct reset_simple_devdata - simple reset controller properties
+ * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
+ * are set to assert the reset.
+ */
+struct reset_simple_devdata {
+ bool active_low;
+};
+
+static const struct reset_simple_devdata reset_simple_active_low = {
+ .active_low = true,
+};
+
+static const struct of_device_id reset_simple_dt_ids[] = {
+ { .compatible = "allwinner,sun6i-a31-clock-reset",
+ .data = &reset_simple_active_low },
+ { /* sentinel */ },
+};
+
+static int reset_simple_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct reset_simple_devdata *devdata;
+ struct reset_simple_data *data;
+ void __iomem *membase;
+ struct resource *res;
+
+ devdata = of_device_get_match_data(dev);
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ membase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(membase))
+ return PTR_ERR(membase);
+
+ spin_lock_init(&data->lock);
+ data->membase = membase;
+ data->rcdev.owner = THIS_MODULE;
+ data->rcdev.nr_resets = resource_size(res) * BITS_PER_BYTE;
+ data->rcdev.ops = &reset_simple_ops;
+ data->rcdev.of_node = dev->of_node;
+
+ if (devdata)
+ data->active_low = devdata->active_low;
+
+ return devm_reset_controller_register(dev, &data->rcdev);
+}
+
+static struct platform_driver reset_simple_driver = {
+ .probe = reset_simple_probe,
+ .driver = {
+ .name = "simple-reset",
+ .of_match_table = reset_simple_dt_ids,
+ },
+};
+builtin_platform_driver(reset_simple_driver);
diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
new file mode 100644
index 0000000000000..39af2014b5f12
--- /dev/null
+++ b/drivers/reset/reset-simple.h
@@ -0,0 +1,41 @@
+/*
+ * Simple Reset Controller ops
+ *
+ * Based on Allwinner SoCs Reset Controller driver
+ *
+ * Copyright 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __RESET_SIMPLE_H__
+#define __RESET_SIMPLE_H__
+
+#include <linux/io.h>
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+
+/**
+ * struct reset_simple_data - driver data for simple reset controllers
+ * @lock: spinlock to protect registers during read-modify-write cycles
+ * @membase: memory mapped I/O register range
+ * @rcdev: reset controller device base structure
+ * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
+ * are set to assert the reset. Note that this says nothing about
+ * the voltage level of the actual reset line.
+ */
+struct reset_simple_data {
+ spinlock_t lock;
+ void __iomem *membase;
+ struct reset_controller_dev rcdev;
+ bool active_low;
+};
+
+extern const struct reset_control_ops reset_simple_ops;
+
+#endif /* __RESET_SIMPLE_H__ */
diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
index 2c7dd1fd08df6..db9a1a75523f4 100644
--- a/drivers/reset/reset-sunxi.c
+++ b/drivers/reset/reset-sunxi.c
@@ -22,64 +22,11 @@
#include <linux/spinlock.h>
#include <linux/types.h>
-struct sunxi_reset_data {
- spinlock_t lock;
- void __iomem *membase;
- struct reset_controller_dev rcdev;
-};
-
-static int sunxi_reset_assert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct sunxi_reset_data *data = container_of(rcdev,
- struct sunxi_reset_data,
- rcdev);
- int reg_width = sizeof(u32);
- int bank = id / (reg_width * BITS_PER_BYTE);
- int offset = id % (reg_width * BITS_PER_BYTE);
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * reg_width));
- writel(reg & ~BIT(offset), data->membase + (bank * reg_width));
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static int sunxi_reset_deassert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct sunxi_reset_data *data = container_of(rcdev,
- struct sunxi_reset_data,
- rcdev);
- int reg_width = sizeof(u32);
- int bank = id / (reg_width * BITS_PER_BYTE);
- int offset = id % (reg_width * BITS_PER_BYTE);
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * reg_width));
- writel(reg | BIT(offset), data->membase + (bank * reg_width));
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static const struct reset_control_ops sunxi_reset_ops = {
- .assert = sunxi_reset_assert,
- .deassert = sunxi_reset_deassert,
-};
+#include "reset-simple.h"
static int sunxi_reset_init(struct device_node *np)
{
- struct sunxi_reset_data *data;
+ struct reset_simple_data *data;
struct resource res;
resource_size_t size;
int ret;
@@ -108,8 +55,9 @@ static int sunxi_reset_init(struct device_node *np)
data->rcdev.owner = THIS_MODULE;
data->rcdev.nr_resets = size * 8;
- data->rcdev.ops = &sunxi_reset_ops;
+ data->rcdev.ops = &reset_simple_ops;
data->rcdev.of_node = np;
+ data->active_low = true;
return reset_controller_register(&data->rcdev);
@@ -122,6 +70,8 @@ static int sunxi_reset_init(struct device_node *np)
* These are the reset controller we need to initialize early on in
* our system, before we can even think of using a regular device
* driver for it.
+ * The controllers that we can register through the regular device
+ * model are handled by the simple reset driver directly.
*/
static const struct of_device_id sunxi_early_reset_dt_ids[] __initconst = {
{ .compatible = "allwinner,sun6i-a31-ahb1-reset", },
@@ -135,45 +85,3 @@ void __init sun6i_reset_init(void)
for_each_matching_node(np, sunxi_early_reset_dt_ids)
sunxi_reset_init(np);
}
-
-/*
- * And these are the controllers we can register through the regular
- * device model.
- */
-static const struct of_device_id sunxi_reset_dt_ids[] = {
- { .compatible = "allwinner,sun6i-a31-clock-reset", },
- { /* sentinel */ },
-};
-
-static int sunxi_reset_probe(struct platform_device *pdev)
-{
- struct sunxi_reset_data *data;
- struct resource *res;
-
- data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- data->membase = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(data->membase))
- return PTR_ERR(data->membase);
-
- spin_lock_init(&data->lock);
-
- data->rcdev.owner = THIS_MODULE;
- data->rcdev.nr_resets = resource_size(res) * 8;
- data->rcdev.ops = &sunxi_reset_ops;
- data->rcdev.of_node = pdev->dev.of_node;
-
- return devm_reset_controller_register(&pdev->dev, &data->rcdev);
-}
-
-static struct platform_driver sunxi_reset_driver = {
- .probe = sunxi_reset_probe,
- .driver = {
- .name = "sunxi-reset",
- .of_match_table = sunxi_reset_dt_ids,
- },
-};
-builtin_platform_driver(sunxi_reset_driver);
--
2.11.0
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v3 2/5] reset: socfpga: use the reset-simple driver
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-16 9:46 ` Philipp Zabel
-1 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:46 UTC (permalink / raw)
To: linux-kernel
Cc: Alexandru Gagniuc, Andre Przywara, Maxime Coquelin,
Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen,
Andreas Färber, linux-arm-kernel, kernel, Philipp Zabel
Add reset line status readback, inverted status support, and socfpga
device tree quirks to the simple reset driver, and use it to replace
the socfpga driver.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
- Rename "status_inverted" properties to "status_active_low".
- Add reg_offset and nr_reset override to struct reset_simple_devdata.
- Drop socfpga special case, instead check "altr,modrst-offset" device
tree property if a default reg_offset is set.
---
drivers/reset/Kconfig | 10 +--
drivers/reset/Makefile | 1 -
drivers/reset/reset-simple.c | 50 +++++++++++++-
drivers/reset/reset-simple.h | 4 ++
drivers/reset/reset-socfpga.c | 154 ------------------------------------------
5 files changed, 55 insertions(+), 164 deletions(-)
delete mode 100644 drivers/reset/reset-socfpga.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index f7ba01a71daee..78a8f6057985b 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -70,19 +70,13 @@ config RESET_PISTACHIO
config RESET_SIMPLE
bool "Simple Reset Controller Driver" if COMPILE_TEST
- default ARCH_SUNXI
+ default ARCH_SOCFPGA || ARCH_SUNXI
help
This enables a simple reset controller driver for reset lines that
that can be asserted and deasserted by toggling bits in a contiguous,
exclusive register space.
- Currently this driver supports Allwinner SoCs.
-
-config RESET_SOCFPGA
- bool "SoCFPGA Reset Driver" if COMPILE_TEST
- default ARCH_SOCFPGA
- help
- This enables the reset controller driver for Altera SoCFPGAs.
+ Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
config RESET_STM32
bool "STM32 Reset Driver" if COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index ab4af99c3dfdc..25f5f722dec01 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -12,7 +12,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
-obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
obj-$(CONFIG_RESET_STM32) += reset-stm32.o
obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index 70c57c0758c39..84b08ce4def01 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -68,25 +68,58 @@ static int reset_simple_deassert(struct reset_controller_dev *rcdev,
return reset_simple_set(rcdev, id, false);
}
+static int reset_simple_status(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct reset_simple_data *data = to_reset_simple_data(rcdev);
+ int reg_width = sizeof(u32);
+ int bank = id / (reg_width * BITS_PER_BYTE);
+ int offset = id % (reg_width * BITS_PER_BYTE);
+ u32 reg;
+
+ reg = readl(data->membase + (bank * reg_width));
+
+ return !(reg & BIT(offset)) ^ !data->status_active_low;
+}
+
const struct reset_control_ops reset_simple_ops = {
.assert = reset_simple_assert,
.deassert = reset_simple_deassert,
+ .status = reset_simple_status,
};
/**
* struct reset_simple_devdata - simple reset controller properties
+ * @reg_offset: offset between base address and first reset register.
+ * @nr_resets: number of resets. If not set, default to resource size in bits.
* @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
* are set to assert the reset.
+ * @status_active_low: if true, bits read back as cleared while the reset is
+ * asserted. Otherwise, bits read back as set while the
+ * reset is asserted.
*/
struct reset_simple_devdata {
+ u32 reg_offset;
+ u32 nr_resets;
bool active_low;
+ bool status_active_low;
+};
+
+#define SOCFPGA_NR_BANKS 8
+
+static const struct reset_simple_devdata reset_simple_socfpga = {
+ .reg_offset = 0x10,
+ .nr_resets = SOCFPGA_NR_BANKS * 32,
+ .status_active_low = true,
};
static const struct reset_simple_devdata reset_simple_active_low = {
.active_low = true,
+ .status_active_low = true,
};
static const struct of_device_id reset_simple_dt_ids[] = {
+ { .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
{ .compatible = "allwinner,sun6i-a31-clock-reset",
.data = &reset_simple_active_low },
{ /* sentinel */ },
@@ -118,8 +151,23 @@ static int reset_simple_probe(struct platform_device *pdev)
data->rcdev.ops = &reset_simple_ops;
data->rcdev.of_node = dev->of_node;
- if (devdata)
+ if (devdata) {
+ u32 reg_offset = devdata->reg_offset;
+
+ if (reg_offset &&
+ of_property_read_u32(dev->of_node, "altr,modrst-offset",
+ ®_offset)) {
+ dev_warn(dev,
+ "missing altr,modrst-offset property, assuming 0x%x!\n",
+ reg_offset);
+ }
+
+ data->membase += reg_offset;
+ if (devdata->nr_resets)
+ data->rcdev.nr_resets = devdata->nr_resets;
data->active_low = devdata->active_low;
+ data->status_active_low = devdata->status_active_low;
+ }
return devm_reset_controller_register(dev, &data->rcdev);
}
diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
index 39af2014b5f12..8a496022baeff 100644
--- a/drivers/reset/reset-simple.h
+++ b/drivers/reset/reset-simple.h
@@ -28,12 +28,16 @@
* @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
* are set to assert the reset. Note that this says nothing about
* the voltage level of the actual reset line.
+ * @status_active_low: if true, bits read back as cleared while the reset is
+ * asserted. Otherwise, bits read back as set while the
+ * reset is asserted.
*/
struct reset_simple_data {
spinlock_t lock;
void __iomem *membase;
struct reset_controller_dev rcdev;
bool active_low;
+ bool status_active_low;
};
extern const struct reset_control_ops reset_simple_ops;
diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
deleted file mode 100644
index 07224c0198920..0000000000000
--- a/drivers/reset/reset-socfpga.c
+++ /dev/null
@@ -1,154 +0,0 @@
-/*
- * Socfpga Reset Controller Driver
- *
- * Copyright 2014 Steffen Trumtrar <s.trumtrar@pengutronix.de>
- *
- * based on
- * Allwinner SoCs Reset Controller driver
- *
- * Copyright 2013 Maxime Ripard
- *
- * Maxime Ripard <maxime.ripard@free-electrons.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/init.h>
-#include <linux/of.h>
-#include <linux/platform_device.h>
-#include <linux/reset-controller.h>
-#include <linux/spinlock.h>
-#include <linux/types.h>
-
-#define BANK_INCREMENT 4
-#define NR_BANKS 8
-
-struct socfpga_reset_data {
- spinlock_t lock;
- void __iomem *membase;
- struct reset_controller_dev rcdev;
-};
-
-static int socfpga_reset_assert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct socfpga_reset_data *data = container_of(rcdev,
- struct socfpga_reset_data,
- rcdev);
- int bank = id / BITS_PER_LONG;
- int offset = id % BITS_PER_LONG;
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * BANK_INCREMENT));
- writel(reg | BIT(offset), data->membase + (bank * BANK_INCREMENT));
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static int socfpga_reset_deassert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct socfpga_reset_data *data = container_of(rcdev,
- struct socfpga_reset_data,
- rcdev);
-
- int bank = id / BITS_PER_LONG;
- int offset = id % BITS_PER_LONG;
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * BANK_INCREMENT));
- writel(reg & ~BIT(offset), data->membase + (bank * BANK_INCREMENT));
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static int socfpga_reset_status(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct socfpga_reset_data *data = container_of(rcdev,
- struct socfpga_reset_data, rcdev);
- int bank = id / BITS_PER_LONG;
- int offset = id % BITS_PER_LONG;
- u32 reg;
-
- reg = readl(data->membase + (bank * BANK_INCREMENT));
-
- return !(reg & BIT(offset));
-}
-
-static const struct reset_control_ops socfpga_reset_ops = {
- .assert = socfpga_reset_assert,
- .deassert = socfpga_reset_deassert,
- .status = socfpga_reset_status,
-};
-
-static int socfpga_reset_probe(struct platform_device *pdev)
-{
- struct socfpga_reset_data *data;
- struct resource *res;
- struct device *dev = &pdev->dev;
- struct device_node *np = dev->of_node;
- u32 modrst_offset;
-
- /*
- * The binding was mainlined without the required property.
- * Do not continue, when we encounter an old DT.
- */
- if (!of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
- dev_err(&pdev->dev, "%s missing #reset-cells property\n",
- pdev->dev.of_node->full_name);
- return -EINVAL;
- }
-
- data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- data->membase = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(data->membase))
- return PTR_ERR(data->membase);
-
- if (of_property_read_u32(np, "altr,modrst-offset", &modrst_offset)) {
- dev_warn(dev, "missing altr,modrst-offset property, assuming 0x10!\n");
- modrst_offset = 0x10;
- }
- data->membase += modrst_offset;
-
- spin_lock_init(&data->lock);
-
- data->rcdev.owner = THIS_MODULE;
- data->rcdev.nr_resets = NR_BANKS * BITS_PER_LONG;
- data->rcdev.ops = &socfpga_reset_ops;
- data->rcdev.of_node = pdev->dev.of_node;
-
- return devm_reset_controller_register(dev, &data->rcdev);
-}
-
-static const struct of_device_id socfpga_reset_dt_ids[] = {
- { .compatible = "altr,rst-mgr", },
- { /* sentinel */ },
-};
-
-static struct platform_driver socfpga_reset_driver = {
- .probe = socfpga_reset_probe,
- .driver = {
- .name = "socfpga-reset",
- .of_match_table = socfpga_reset_dt_ids,
- },
-};
-builtin_platform_driver(socfpga_reset_driver);
--
2.11.0
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v3 2/5] reset: socfpga: use the reset-simple driver
@ 2017-08-16 9:46 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:46 UTC (permalink / raw)
To: linux-arm-kernel
Add reset line status readback, inverted status support, and socfpga
device tree quirks to the simple reset driver, and use it to replace
the socfpga driver.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
- Rename "status_inverted" properties to "status_active_low".
- Add reg_offset and nr_reset override to struct reset_simple_devdata.
- Drop socfpga special case, instead check "altr,modrst-offset" device
tree property if a default reg_offset is set.
---
drivers/reset/Kconfig | 10 +--
drivers/reset/Makefile | 1 -
drivers/reset/reset-simple.c | 50 +++++++++++++-
drivers/reset/reset-simple.h | 4 ++
drivers/reset/reset-socfpga.c | 154 ------------------------------------------
5 files changed, 55 insertions(+), 164 deletions(-)
delete mode 100644 drivers/reset/reset-socfpga.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index f7ba01a71daee..78a8f6057985b 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -70,19 +70,13 @@ config RESET_PISTACHIO
config RESET_SIMPLE
bool "Simple Reset Controller Driver" if COMPILE_TEST
- default ARCH_SUNXI
+ default ARCH_SOCFPGA || ARCH_SUNXI
help
This enables a simple reset controller driver for reset lines that
that can be asserted and deasserted by toggling bits in a contiguous,
exclusive register space.
- Currently this driver supports Allwinner SoCs.
-
-config RESET_SOCFPGA
- bool "SoCFPGA Reset Driver" if COMPILE_TEST
- default ARCH_SOCFPGA
- help
- This enables the reset controller driver for Altera SoCFPGAs.
+ Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
config RESET_STM32
bool "STM32 Reset Driver" if COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index ab4af99c3dfdc..25f5f722dec01 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -12,7 +12,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
-obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
obj-$(CONFIG_RESET_STM32) += reset-stm32.o
obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index 70c57c0758c39..84b08ce4def01 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -68,25 +68,58 @@ static int reset_simple_deassert(struct reset_controller_dev *rcdev,
return reset_simple_set(rcdev, id, false);
}
+static int reset_simple_status(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct reset_simple_data *data = to_reset_simple_data(rcdev);
+ int reg_width = sizeof(u32);
+ int bank = id / (reg_width * BITS_PER_BYTE);
+ int offset = id % (reg_width * BITS_PER_BYTE);
+ u32 reg;
+
+ reg = readl(data->membase + (bank * reg_width));
+
+ return !(reg & BIT(offset)) ^ !data->status_active_low;
+}
+
const struct reset_control_ops reset_simple_ops = {
.assert = reset_simple_assert,
.deassert = reset_simple_deassert,
+ .status = reset_simple_status,
};
/**
* struct reset_simple_devdata - simple reset controller properties
+ * @reg_offset: offset between base address and first reset register.
+ * @nr_resets: number of resets. If not set, default to resource size in bits.
* @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
* are set to assert the reset.
+ * @status_active_low: if true, bits read back as cleared while the reset is
+ * asserted. Otherwise, bits read back as set while the
+ * reset is asserted.
*/
struct reset_simple_devdata {
+ u32 reg_offset;
+ u32 nr_resets;
bool active_low;
+ bool status_active_low;
+};
+
+#define SOCFPGA_NR_BANKS 8
+
+static const struct reset_simple_devdata reset_simple_socfpga = {
+ .reg_offset = 0x10,
+ .nr_resets = SOCFPGA_NR_BANKS * 32,
+ .status_active_low = true,
};
static const struct reset_simple_devdata reset_simple_active_low = {
.active_low = true,
+ .status_active_low = true,
};
static const struct of_device_id reset_simple_dt_ids[] = {
+ { .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
{ .compatible = "allwinner,sun6i-a31-clock-reset",
.data = &reset_simple_active_low },
{ /* sentinel */ },
@@ -118,8 +151,23 @@ static int reset_simple_probe(struct platform_device *pdev)
data->rcdev.ops = &reset_simple_ops;
data->rcdev.of_node = dev->of_node;
- if (devdata)
+ if (devdata) {
+ u32 reg_offset = devdata->reg_offset;
+
+ if (reg_offset &&
+ of_property_read_u32(dev->of_node, "altr,modrst-offset",
+ ®_offset)) {
+ dev_warn(dev,
+ "missing altr,modrst-offset property, assuming 0x%x!\n",
+ reg_offset);
+ }
+
+ data->membase += reg_offset;
+ if (devdata->nr_resets)
+ data->rcdev.nr_resets = devdata->nr_resets;
data->active_low = devdata->active_low;
+ data->status_active_low = devdata->status_active_low;
+ }
return devm_reset_controller_register(dev, &data->rcdev);
}
diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
index 39af2014b5f12..8a496022baeff 100644
--- a/drivers/reset/reset-simple.h
+++ b/drivers/reset/reset-simple.h
@@ -28,12 +28,16 @@
* @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
* are set to assert the reset. Note that this says nothing about
* the voltage level of the actual reset line.
+ * @status_active_low: if true, bits read back as cleared while the reset is
+ * asserted. Otherwise, bits read back as set while the
+ * reset is asserted.
*/
struct reset_simple_data {
spinlock_t lock;
void __iomem *membase;
struct reset_controller_dev rcdev;
bool active_low;
+ bool status_active_low;
};
extern const struct reset_control_ops reset_simple_ops;
diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
deleted file mode 100644
index 07224c0198920..0000000000000
--- a/drivers/reset/reset-socfpga.c
+++ /dev/null
@@ -1,154 +0,0 @@
-/*
- * Socfpga Reset Controller Driver
- *
- * Copyright 2014 Steffen Trumtrar <s.trumtrar@pengutronix.de>
- *
- * based on
- * Allwinner SoCs Reset Controller driver
- *
- * Copyright 2013 Maxime Ripard
- *
- * Maxime Ripard <maxime.ripard@free-electrons.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/init.h>
-#include <linux/of.h>
-#include <linux/platform_device.h>
-#include <linux/reset-controller.h>
-#include <linux/spinlock.h>
-#include <linux/types.h>
-
-#define BANK_INCREMENT 4
-#define NR_BANKS 8
-
-struct socfpga_reset_data {
- spinlock_t lock;
- void __iomem *membase;
- struct reset_controller_dev rcdev;
-};
-
-static int socfpga_reset_assert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct socfpga_reset_data *data = container_of(rcdev,
- struct socfpga_reset_data,
- rcdev);
- int bank = id / BITS_PER_LONG;
- int offset = id % BITS_PER_LONG;
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * BANK_INCREMENT));
- writel(reg | BIT(offset), data->membase + (bank * BANK_INCREMENT));
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static int socfpga_reset_deassert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct socfpga_reset_data *data = container_of(rcdev,
- struct socfpga_reset_data,
- rcdev);
-
- int bank = id / BITS_PER_LONG;
- int offset = id % BITS_PER_LONG;
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * BANK_INCREMENT));
- writel(reg & ~BIT(offset), data->membase + (bank * BANK_INCREMENT));
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static int socfpga_reset_status(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct socfpga_reset_data *data = container_of(rcdev,
- struct socfpga_reset_data, rcdev);
- int bank = id / BITS_PER_LONG;
- int offset = id % BITS_PER_LONG;
- u32 reg;
-
- reg = readl(data->membase + (bank * BANK_INCREMENT));
-
- return !(reg & BIT(offset));
-}
-
-static const struct reset_control_ops socfpga_reset_ops = {
- .assert = socfpga_reset_assert,
- .deassert = socfpga_reset_deassert,
- .status = socfpga_reset_status,
-};
-
-static int socfpga_reset_probe(struct platform_device *pdev)
-{
- struct socfpga_reset_data *data;
- struct resource *res;
- struct device *dev = &pdev->dev;
- struct device_node *np = dev->of_node;
- u32 modrst_offset;
-
- /*
- * The binding was mainlined without the required property.
- * Do not continue, when we encounter an old DT.
- */
- if (!of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
- dev_err(&pdev->dev, "%s missing #reset-cells property\n",
- pdev->dev.of_node->full_name);
- return -EINVAL;
- }
-
- data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- data->membase = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(data->membase))
- return PTR_ERR(data->membase);
-
- if (of_property_read_u32(np, "altr,modrst-offset", &modrst_offset)) {
- dev_warn(dev, "missing altr,modrst-offset property, assuming 0x10!\n");
- modrst_offset = 0x10;
- }
- data->membase += modrst_offset;
-
- spin_lock_init(&data->lock);
-
- data->rcdev.owner = THIS_MODULE;
- data->rcdev.nr_resets = NR_BANKS * BITS_PER_LONG;
- data->rcdev.ops = &socfpga_reset_ops;
- data->rcdev.of_node = pdev->dev.of_node;
-
- return devm_reset_controller_register(dev, &data->rcdev);
-}
-
-static const struct of_device_id socfpga_reset_dt_ids[] = {
- { .compatible = "altr,rst-mgr", },
- { /* sentinel */ },
-};
-
-static struct platform_driver socfpga_reset_driver = {
- .probe = socfpga_reset_probe,
- .driver = {
- .name = "socfpga-reset",
- .of_match_table = socfpga_reset_dt_ids,
- },
-};
-builtin_platform_driver(socfpga_reset_driver);
--
2.11.0
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v3 3/5] reset: stm32: use the reset-simple driver
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-16 9:46 ` Philipp Zabel
-1 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:46 UTC (permalink / raw)
To: linux-kernel
Cc: Alexandru Gagniuc, Andre Przywara, Maxime Coquelin,
Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen,
Andreas Färber, linux-arm-kernel, kernel, Philipp Zabel
The reset-simple driver can be used without changes.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/reset/Kconfig | 11 ++---
drivers/reset/Makefile | 1 -
drivers/reset/reset-simple.c | 1 +
drivers/reset/reset-stm32.c | 108 -------------------------------------------
4 files changed, 4 insertions(+), 117 deletions(-)
delete mode 100644 drivers/reset/reset-stm32.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 78a8f6057985b..29f4487c290fc 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -70,19 +70,14 @@ config RESET_PISTACHIO
config RESET_SIMPLE
bool "Simple Reset Controller Driver" if COMPILE_TEST
- default ARCH_SOCFPGA || ARCH_SUNXI
+ default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
help
This enables a simple reset controller driver for reset lines that
that can be asserted and deasserted by toggling bits in a contiguous,
exclusive register space.
- Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
-
-config RESET_STM32
- bool "STM32 Reset Driver" if COMPILE_TEST
- default ARCH_STM32
- help
- This enables the RCC reset controller driver for STM32 MCUs.
+ Currently this driver supports Altera SoCFPGAs, the RCC reset
+ controller in STM32 MCUs, and Allwinner SoCs.
config RESET_SUNXI
bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 25f5f722dec01..e8c3a032f4780 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -12,7 +12,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
-obj-$(CONFIG_RESET_STM32) += reset-stm32.o
obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index 84b08ce4def01..e54a29b8b532d 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -120,6 +120,7 @@ static const struct reset_simple_devdata reset_simple_active_low = {
static const struct of_device_id reset_simple_dt_ids[] = {
{ .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
+ { .compatible = "st,stm32-rcc", },
{ .compatible = "allwinner,sun6i-a31-clock-reset",
.data = &reset_simple_active_low },
{ /* sentinel */ },
diff --git a/drivers/reset/reset-stm32.c b/drivers/reset/reset-stm32.c
deleted file mode 100644
index 3a7c8527e66af..0000000000000
--- a/drivers/reset/reset-stm32.c
+++ /dev/null
@@ -1,108 +0,0 @@
-/*
- * Copyright (C) Maxime Coquelin 2015
- * Author: Maxime Coquelin <mcoquelin.stm32@gmail.com>
- * License terms: GNU General Public License (GPL), version 2
- *
- * Heavily based on sunxi driver from Maxime Ripard.
- */
-
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
-#include <linux/reset-controller.h>
-#include <linux/slab.h>
-#include <linux/spinlock.h>
-#include <linux/types.h>
-
-struct stm32_reset_data {
- spinlock_t lock;
- void __iomem *membase;
- struct reset_controller_dev rcdev;
-};
-
-static int stm32_reset_assert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct stm32_reset_data *data = container_of(rcdev,
- struct stm32_reset_data,
- rcdev);
- int bank = id / BITS_PER_LONG;
- int offset = id % BITS_PER_LONG;
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * 4));
- writel(reg | BIT(offset), data->membase + (bank * 4));
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct stm32_reset_data *data = container_of(rcdev,
- struct stm32_reset_data,
- rcdev);
- int bank = id / BITS_PER_LONG;
- int offset = id % BITS_PER_LONG;
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * 4));
- writel(reg & ~BIT(offset), data->membase + (bank * 4));
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static const struct reset_control_ops stm32_reset_ops = {
- .assert = stm32_reset_assert,
- .deassert = stm32_reset_deassert,
-};
-
-static const struct of_device_id stm32_reset_dt_ids[] = {
- { .compatible = "st,stm32-rcc", },
- { /* sentinel */ },
-};
-
-static int stm32_reset_probe(struct platform_device *pdev)
-{
- struct stm32_reset_data *data;
- struct resource *res;
-
- data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- data->membase = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(data->membase))
- return PTR_ERR(data->membase);
-
- spin_lock_init(&data->lock);
-
- data->rcdev.owner = THIS_MODULE;
- data->rcdev.nr_resets = resource_size(res) * 8;
- data->rcdev.ops = &stm32_reset_ops;
- data->rcdev.of_node = pdev->dev.of_node;
-
- return devm_reset_controller_register(&pdev->dev, &data->rcdev);
-}
-
-static struct platform_driver stm32_reset_driver = {
- .probe = stm32_reset_probe,
- .driver = {
- .name = "stm32-rcc-reset",
- .of_match_table = stm32_reset_dt_ids,
- },
-};
-builtin_platform_driver(stm32_reset_driver);
--
2.11.0
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v3 3/5] reset: stm32: use the reset-simple driver
@ 2017-08-16 9:46 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:46 UTC (permalink / raw)
To: linux-arm-kernel
The reset-simple driver can be used without changes.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/reset/Kconfig | 11 ++---
drivers/reset/Makefile | 1 -
drivers/reset/reset-simple.c | 1 +
drivers/reset/reset-stm32.c | 108 -------------------------------------------
4 files changed, 4 insertions(+), 117 deletions(-)
delete mode 100644 drivers/reset/reset-stm32.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 78a8f6057985b..29f4487c290fc 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -70,19 +70,14 @@ config RESET_PISTACHIO
config RESET_SIMPLE
bool "Simple Reset Controller Driver" if COMPILE_TEST
- default ARCH_SOCFPGA || ARCH_SUNXI
+ default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
help
This enables a simple reset controller driver for reset lines that
that can be asserted and deasserted by toggling bits in a contiguous,
exclusive register space.
- Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
-
-config RESET_STM32
- bool "STM32 Reset Driver" if COMPILE_TEST
- default ARCH_STM32
- help
- This enables the RCC reset controller driver for STM32 MCUs.
+ Currently this driver supports Altera SoCFPGAs, the RCC reset
+ controller in STM32 MCUs, and Allwinner SoCs.
config RESET_SUNXI
bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 25f5f722dec01..e8c3a032f4780 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -12,7 +12,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
-obj-$(CONFIG_RESET_STM32) += reset-stm32.o
obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index 84b08ce4def01..e54a29b8b532d 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -120,6 +120,7 @@ static const struct reset_simple_devdata reset_simple_active_low = {
static const struct of_device_id reset_simple_dt_ids[] = {
{ .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
+ { .compatible = "st,stm32-rcc", },
{ .compatible = "allwinner,sun6i-a31-clock-reset",
.data = &reset_simple_active_low },
{ /* sentinel */ },
diff --git a/drivers/reset/reset-stm32.c b/drivers/reset/reset-stm32.c
deleted file mode 100644
index 3a7c8527e66af..0000000000000
--- a/drivers/reset/reset-stm32.c
+++ /dev/null
@@ -1,108 +0,0 @@
-/*
- * Copyright (C) Maxime Coquelin 2015
- * Author: Maxime Coquelin <mcoquelin.stm32@gmail.com>
- * License terms: GNU General Public License (GPL), version 2
- *
- * Heavily based on sunxi driver from Maxime Ripard.
- */
-
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
-#include <linux/reset-controller.h>
-#include <linux/slab.h>
-#include <linux/spinlock.h>
-#include <linux/types.h>
-
-struct stm32_reset_data {
- spinlock_t lock;
- void __iomem *membase;
- struct reset_controller_dev rcdev;
-};
-
-static int stm32_reset_assert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct stm32_reset_data *data = container_of(rcdev,
- struct stm32_reset_data,
- rcdev);
- int bank = id / BITS_PER_LONG;
- int offset = id % BITS_PER_LONG;
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * 4));
- writel(reg | BIT(offset), data->membase + (bank * 4));
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct stm32_reset_data *data = container_of(rcdev,
- struct stm32_reset_data,
- rcdev);
- int bank = id / BITS_PER_LONG;
- int offset = id % BITS_PER_LONG;
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(data->membase + (bank * 4));
- writel(reg & ~BIT(offset), data->membase + (bank * 4));
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
-static const struct reset_control_ops stm32_reset_ops = {
- .assert = stm32_reset_assert,
- .deassert = stm32_reset_deassert,
-};
-
-static const struct of_device_id stm32_reset_dt_ids[] = {
- { .compatible = "st,stm32-rcc", },
- { /* sentinel */ },
-};
-
-static int stm32_reset_probe(struct platform_device *pdev)
-{
- struct stm32_reset_data *data;
- struct resource *res;
-
- data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- data->membase = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(data->membase))
- return PTR_ERR(data->membase);
-
- spin_lock_init(&data->lock);
-
- data->rcdev.owner = THIS_MODULE;
- data->rcdev.nr_resets = resource_size(res) * 8;
- data->rcdev.ops = &stm32_reset_ops;
- data->rcdev.of_node = pdev->dev.of_node;
-
- return devm_reset_controller_register(&pdev->dev, &data->rcdev);
-}
-
-static struct platform_driver stm32_reset_driver = {
- .probe = stm32_reset_probe,
- .driver = {
- .name = "stm32-rcc-reset",
- .of_match_table = stm32_reset_dt_ids,
- },
-};
-builtin_platform_driver(stm32_reset_driver);
--
2.11.0
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v3 4/5] reset: zx2967: use the reset-simple driver
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-16 9:47 ` Philipp Zabel
-1 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:47 UTC (permalink / raw)
To: linux-kernel
Cc: Alexandru Gagniuc, Andre Przywara, Maxime Coquelin,
Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen,
Andreas Färber, linux-arm-kernel, kernel, Philipp Zabel
The reset-simple driver can be used without changes.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
MAINTAINERS | 1 -
drivers/reset/Kconfig | 10 +----
drivers/reset/Makefile | 1 -
drivers/reset/reset-simple.c | 2 +
drivers/reset/reset-zx2967.c | 99 --------------------------------------------
5 files changed, 4 insertions(+), 109 deletions(-)
delete mode 100644 drivers/reset/reset-zx2967.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 57853844969bf..30af04516d8a0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2061,7 +2061,6 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
S: Maintained
F: arch/arm/mach-zx/
F: drivers/clk/zte/
-F: drivers/reset/reset-zx2967.c
F: drivers/soc/zte/
F: Documentation/devicetree/bindings/arm/zte.txt
F: Documentation/devicetree/bindings/clock/zx296702-clk.txt
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 29f4487c290fc..6dfea01023618 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -70,14 +70,14 @@ config RESET_PISTACHIO
config RESET_SIMPLE
bool "Simple Reset Controller Driver" if COMPILE_TEST
- default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
+ default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI || ARCH_ZX
help
This enables a simple reset controller driver for reset lines that
that can be asserted and deasserted by toggling bits in a contiguous,
exclusive register space.
Currently this driver supports Altera SoCFPGAs, the RCC reset
- controller in STM32 MCUs, and Allwinner SoCs.
+ controller in STM32 MCUs, Allwinner SoCs, and ZTE's zx2967 family.
config RESET_SUNXI
bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
@@ -114,12 +114,6 @@ config RESET_UNIPHIER
Say Y if you want to control reset signals provided by System Control
block, Media I/O block, Peripheral Block.
-config RESET_ZX2967
- bool "ZTE ZX2967 Reset Driver"
- depends on ARCH_ZX || COMPILE_TEST
- help
- This enables the reset controller driver for ZTE's zx2967 family.
-
config RESET_ZYNQ
bool "ZYNQ Reset Driver" if COMPILE_TEST
default ARCH_ZYNQ
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index e8c3a032f4780..3a70c254b5ea5 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -16,6 +16,5 @@ obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
-obj-$(CONFIG_RESET_ZX2967) += reset-zx2967.o
obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index e54a29b8b532d..13e7d5559acc9 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -123,6 +123,8 @@ static const struct of_device_id reset_simple_dt_ids[] = {
{ .compatible = "st,stm32-rcc", },
{ .compatible = "allwinner,sun6i-a31-clock-reset",
.data = &reset_simple_active_low },
+ { .compatible = "zte,zx296718-reset",
+ .data = &reset_simple_active_low },
{ /* sentinel */ },
};
diff --git a/drivers/reset/reset-zx2967.c b/drivers/reset/reset-zx2967.c
deleted file mode 100644
index 4f319f7753d4d..0000000000000
--- a/drivers/reset/reset-zx2967.c
+++ /dev/null
@@ -1,99 +0,0 @@
-/*
- * ZTE's zx2967 family reset controller driver
- *
- * Copyright (C) 2017 ZTE Ltd.
- *
- * Author: Baoyou Xie <baoyou.xie@linaro.org>
- *
- * License terms: GNU General Public License (GPL) version 2
- */
-
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
-#include <linux/reset-controller.h>
-
-struct zx2967_reset {
- void __iomem *reg_base;
- spinlock_t lock;
- struct reset_controller_dev rcdev;
-};
-
-static int zx2967_reset_act(struct reset_controller_dev *rcdev,
- unsigned long id, bool assert)
-{
- struct zx2967_reset *reset = NULL;
- int bank = id / 32;
- int offset = id % 32;
- u32 reg;
- unsigned long flags;
-
- reset = container_of(rcdev, struct zx2967_reset, rcdev);
-
- spin_lock_irqsave(&reset->lock, flags);
-
- reg = readl_relaxed(reset->reg_base + (bank * 4));
- if (assert)
- reg &= ~BIT(offset);
- else
- reg |= BIT(offset);
- writel_relaxed(reg, reset->reg_base + (bank * 4));
-
- spin_unlock_irqrestore(&reset->lock, flags);
-
- return 0;
-}
-
-static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- return zx2967_reset_act(rcdev, id, true);
-}
-
-static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- return zx2967_reset_act(rcdev, id, false);
-}
-
-static const struct reset_control_ops zx2967_reset_ops = {
- .assert = zx2967_reset_assert,
- .deassert = zx2967_reset_deassert,
-};
-
-static int zx2967_reset_probe(struct platform_device *pdev)
-{
- struct zx2967_reset *reset;
- struct resource *res;
-
- reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
- if (!reset)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- reset->reg_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(reset->reg_base))
- return PTR_ERR(reset->reg_base);
-
- spin_lock_init(&reset->lock);
-
- reset->rcdev.owner = THIS_MODULE;
- reset->rcdev.nr_resets = resource_size(res) * 8;
- reset->rcdev.ops = &zx2967_reset_ops;
- reset->rcdev.of_node = pdev->dev.of_node;
-
- return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
-}
-
-static const struct of_device_id zx2967_reset_dt_ids[] = {
- { .compatible = "zte,zx296718-reset", },
- {},
-};
-
-static struct platform_driver zx2967_reset_driver = {
- .probe = zx2967_reset_probe,
- .driver = {
- .name = "zx2967-reset",
- .of_match_table = zx2967_reset_dt_ids,
- },
-};
-builtin_platform_driver(zx2967_reset_driver);
--
2.11.0
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v3 4/5] reset: zx2967: use the reset-simple driver
@ 2017-08-16 9:47 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:47 UTC (permalink / raw)
To: linux-arm-kernel
The reset-simple driver can be used without changes.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
MAINTAINERS | 1 -
drivers/reset/Kconfig | 10 +----
drivers/reset/Makefile | 1 -
drivers/reset/reset-simple.c | 2 +
drivers/reset/reset-zx2967.c | 99 --------------------------------------------
5 files changed, 4 insertions(+), 109 deletions(-)
delete mode 100644 drivers/reset/reset-zx2967.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 57853844969bf..30af04516d8a0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2061,7 +2061,6 @@ L: linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
S: Maintained
F: arch/arm/mach-zx/
F: drivers/clk/zte/
-F: drivers/reset/reset-zx2967.c
F: drivers/soc/zte/
F: Documentation/devicetree/bindings/arm/zte.txt
F: Documentation/devicetree/bindings/clock/zx296702-clk.txt
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 29f4487c290fc..6dfea01023618 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -70,14 +70,14 @@ config RESET_PISTACHIO
config RESET_SIMPLE
bool "Simple Reset Controller Driver" if COMPILE_TEST
- default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
+ default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI || ARCH_ZX
help
This enables a simple reset controller driver for reset lines that
that can be asserted and deasserted by toggling bits in a contiguous,
exclusive register space.
Currently this driver supports Altera SoCFPGAs, the RCC reset
- controller in STM32 MCUs, and Allwinner SoCs.
+ controller in STM32 MCUs, Allwinner SoCs, and ZTE's zx2967 family.
config RESET_SUNXI
bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
@@ -114,12 +114,6 @@ config RESET_UNIPHIER
Say Y if you want to control reset signals provided by System Control
block, Media I/O block, Peripheral Block.
-config RESET_ZX2967
- bool "ZTE ZX2967 Reset Driver"
- depends on ARCH_ZX || COMPILE_TEST
- help
- This enables the reset controller driver for ZTE's zx2967 family.
-
config RESET_ZYNQ
bool "ZYNQ Reset Driver" if COMPILE_TEST
default ARCH_ZYNQ
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index e8c3a032f4780..3a70c254b5ea5 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -16,6 +16,5 @@ obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
-obj-$(CONFIG_RESET_ZX2967) += reset-zx2967.o
obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index e54a29b8b532d..13e7d5559acc9 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -123,6 +123,8 @@ static const struct of_device_id reset_simple_dt_ids[] = {
{ .compatible = "st,stm32-rcc", },
{ .compatible = "allwinner,sun6i-a31-clock-reset",
.data = &reset_simple_active_low },
+ { .compatible = "zte,zx296718-reset",
+ .data = &reset_simple_active_low },
{ /* sentinel */ },
};
diff --git a/drivers/reset/reset-zx2967.c b/drivers/reset/reset-zx2967.c
deleted file mode 100644
index 4f319f7753d4d..0000000000000
--- a/drivers/reset/reset-zx2967.c
+++ /dev/null
@@ -1,99 +0,0 @@
-/*
- * ZTE's zx2967 family reset controller driver
- *
- * Copyright (C) 2017 ZTE Ltd.
- *
- * Author: Baoyou Xie <baoyou.xie@linaro.org>
- *
- * License terms: GNU General Public License (GPL) version 2
- */
-
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
-#include <linux/reset-controller.h>
-
-struct zx2967_reset {
- void __iomem *reg_base;
- spinlock_t lock;
- struct reset_controller_dev rcdev;
-};
-
-static int zx2967_reset_act(struct reset_controller_dev *rcdev,
- unsigned long id, bool assert)
-{
- struct zx2967_reset *reset = NULL;
- int bank = id / 32;
- int offset = id % 32;
- u32 reg;
- unsigned long flags;
-
- reset = container_of(rcdev, struct zx2967_reset, rcdev);
-
- spin_lock_irqsave(&reset->lock, flags);
-
- reg = readl_relaxed(reset->reg_base + (bank * 4));
- if (assert)
- reg &= ~BIT(offset);
- else
- reg |= BIT(offset);
- writel_relaxed(reg, reset->reg_base + (bank * 4));
-
- spin_unlock_irqrestore(&reset->lock, flags);
-
- return 0;
-}
-
-static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- return zx2967_reset_act(rcdev, id, true);
-}
-
-static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- return zx2967_reset_act(rcdev, id, false);
-}
-
-static const struct reset_control_ops zx2967_reset_ops = {
- .assert = zx2967_reset_assert,
- .deassert = zx2967_reset_deassert,
-};
-
-static int zx2967_reset_probe(struct platform_device *pdev)
-{
- struct zx2967_reset *reset;
- struct resource *res;
-
- reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
- if (!reset)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- reset->reg_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(reset->reg_base))
- return PTR_ERR(reset->reg_base);
-
- spin_lock_init(&reset->lock);
-
- reset->rcdev.owner = THIS_MODULE;
- reset->rcdev.nr_resets = resource_size(res) * 8;
- reset->rcdev.ops = &zx2967_reset_ops;
- reset->rcdev.of_node = pdev->dev.of_node;
-
- return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
-}
-
-static const struct of_device_id zx2967_reset_dt_ids[] = {
- { .compatible = "zte,zx296718-reset", },
- {},
-};
-
-static struct platform_driver zx2967_reset_driver = {
- .probe = zx2967_reset_probe,
- .driver = {
- .name = "zx2967-reset",
- .of_match_table = zx2967_reset_dt_ids,
- },
-};
-builtin_platform_driver(zx2967_reset_driver);
--
2.11.0
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v3 5/5] reset: simple: read back to make sure changes are applied
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-16 9:47 ` Philipp Zabel
-1 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:47 UTC (permalink / raw)
To: linux-kernel
Cc: Alexandru Gagniuc, Andre Przywara, Maxime Coquelin,
Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen,
Andreas Färber, linux-arm-kernel, kernel, Philipp Zabel
Read back the register after setting or clearing a reset bit to make
sure that the changes are applied to the reset controller hardware.
Theoretically, this avoids the write to stay stuck in a store buffer
during the delay of an assert-delay-deassert sequence, and makes sure
that the reset really is asserted for the specified duration.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/reset/reset-simple.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index 13e7d5559acc9..d98a7e7d802d1 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -39,17 +39,20 @@ static int reset_simple_set(struct reset_controller_dev *rcdev,
int reg_width = sizeof(u32);
int bank = id / (reg_width * BITS_PER_BYTE);
int offset = id % (reg_width * BITS_PER_BYTE);
+ void __iomem *addr = data->membase + (bank * reg_width);
unsigned long flags;
u32 reg;
spin_lock_irqsave(&data->lock, flags);
- reg = readl(data->membase + (bank * reg_width));
+ reg = readl(addr);
if (assert ^ data->active_low)
reg |= BIT(offset);
else
reg &= ~BIT(offset);
- writel(reg, data->membase + (bank * reg_width));
+ writel(reg, addr);
+ /* Read back to make sure the write doesn't linger in a store buffer */
+ readl(addr);
spin_unlock_irqrestore(&data->lock, flags);
--
2.11.0
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v3 5/5] reset: simple: read back to make sure changes are applied
@ 2017-08-16 9:47 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 9:47 UTC (permalink / raw)
To: linux-arm-kernel
Read back the register after setting or clearing a reset bit to make
sure that the changes are applied to the reset controller hardware.
Theoretically, this avoids the write to stay stuck in a store buffer
during the delay of an assert-delay-deassert sequence, and makes sure
that the reset really is asserted for the specified duration.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/reset/reset-simple.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index 13e7d5559acc9..d98a7e7d802d1 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -39,17 +39,20 @@ static int reset_simple_set(struct reset_controller_dev *rcdev,
int reg_width = sizeof(u32);
int bank = id / (reg_width * BITS_PER_BYTE);
int offset = id % (reg_width * BITS_PER_BYTE);
+ void __iomem *addr = data->membase + (bank * reg_width);
unsigned long flags;
u32 reg;
spin_lock_irqsave(&data->lock, flags);
- reg = readl(data->membase + (bank * reg_width));
+ reg = readl(addr);
if (assert ^ data->active_low)
reg |= BIT(offset);
else
reg &= ~BIT(offset);
- writel(reg, data->membase + (bank * reg_width));
+ writel(reg, addr);
+ /* Read back to make sure the write doesn't linger in a store buffer */
+ readl(addr);
spin_unlock_irqrestore(&data->lock, flags);
--
2.11.0
^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-16 11:30 ` Andre Przywara
-1 siblings, 0 replies; 54+ messages in thread
From: Andre Przywara @ 2017-08-16 11:30 UTC (permalink / raw)
To: Philipp Zabel, linux-kernel
Cc: Alexandru Gagniuc, Maxime Coquelin, Alexandre Torgue,
Maxime Ripard, Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev,
Steffen Trumtrar, Dinh Nguyen, Andreas Färber,
linux-arm-kernel, kernel
Hi Philipp,
sorry for the delay, I was on holidays.
Thanks for putting together the series, this looks very good to me.
One comment below...
On 16/08/17 10:46, Philipp Zabel wrote:
> Split reusable parts out of the sunxi driver, to add a driver for simple
> reset controllers with reset lines that can be controlled by toggling
> bits in exclusive, contiguous register ranges using read-modify-write
> cycles under a spinlock. The separate sunxi driver still remains to
> register the early reset controllers, but it reuses the reset-simple
> ops.
>
> The following patches will replace compatible reset drivers with
> reset-simple, extending it where necessary.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Changes since v2:
> - Add kerneldoc comment for struct reset_simple_devdata and struct
> reset_simple_data.
> - Rename "inverted" properties to "active_low".
> - Use of_device_get_match_data instead of of_match_device.
> ---
> drivers/reset/Kconfig | 11 ++++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-simple.c | 134 +++++++++++++++++++++++++++++++++++++++++++
> drivers/reset/reset-simple.h | 41 +++++++++++++
> drivers/reset/reset-sunxi.c | 104 ++-------------------------------
> 5 files changed, 193 insertions(+), 98 deletions(-)
> create mode 100644 drivers/reset/reset-simple.c
> create mode 100644 drivers/reset/reset-simple.h
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 52d5251660b9b..f7ba01a71daee 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -68,6 +68,16 @@ config RESET_PISTACHIO
> help
> This enables the reset driver for ImgTec Pistachio SoCs.
>
> +config RESET_SIMPLE
> + bool "Simple Reset Controller Driver" if COMPILE_TEST
> + default ARCH_SUNXI
> + help
> + This enables a simple reset controller driver for reset lines that
> + that can be asserted and deasserted by toggling bits in a contiguous,
> + exclusive register space.
> +
> + Currently this driver supports Allwinner SoCs.
> +
> config RESET_SOCFPGA
> bool "SoCFPGA Reset Driver" if COMPILE_TEST
> default ARCH_SOCFPGA
> @@ -83,6 +93,7 @@ config RESET_STM32
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> default ARCH_SUNXI
> + select RESET_SIMPLE
> help
> This enables the reset driver for Allwinner SoCs.
>
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index b62783f50fe5b..ab4af99c3dfdc 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
> obj-$(CONFIG_RESET_MESON) += reset-meson.o
> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> +obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
> obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> new file mode 100644
> index 0000000000000..70c57c0758c39
> --- /dev/null
> +++ b/drivers/reset/reset-simple.c
> @@ -0,0 +1,134 @@
> +/*
> + * Simple Reset Controller Driver
> + *
> + * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
> + *
> + * Based on Allwinner SoCs Reset Controller driver
> + *
> + * Copyright 2013 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +
> +#include "reset-simple.h"
> +
> +static inline struct reset_simple_data *
> +to_reset_simple_data(struct reset_controller_dev *rcdev)
> +{
> + return container_of(rcdev, struct reset_simple_data, rcdev);
> +}
> +
> +static int reset_simple_set(struct reset_controller_dev *rcdev,
> + unsigned long id, bool assert)
> +{
> + struct reset_simple_data *data = to_reset_simple_data(rcdev);
> + int reg_width = sizeof(u32);
> + int bank = id / (reg_width * BITS_PER_BYTE);
> + int offset = id % (reg_width * BITS_PER_BYTE);
> + unsigned long flags;
> + u32 reg;
> +
> + spin_lock_irqsave(&data->lock, flags);
> +
> + reg = readl(data->membase + (bank * reg_width));
> + if (assert ^ data->active_low)
> + reg |= BIT(offset);
> + else
> + reg &= ~BIT(offset);
> + writel(reg, data->membase + (bank * reg_width));
> +
> + spin_unlock_irqrestore(&data->lock, flags);
> +
> + return 0;
> +}
> +
> +static int reset_simple_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return reset_simple_set(rcdev, id, true);
> +}
> +
> +static int reset_simple_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return reset_simple_set(rcdev, id, false);
> +}
> +
> +const struct reset_control_ops reset_simple_ops = {
> + .assert = reset_simple_assert,
> + .deassert = reset_simple_deassert,
> +};
> +
> +/**
> + * struct reset_simple_devdata - simple reset controller properties
> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> + * are set to assert the reset.
> + */
> +struct reset_simple_devdata {
> + bool active_low;
> +};
> +
> +static const struct reset_simple_devdata reset_simple_active_low = {
> + .active_low = true,
> +};
> +
> +static const struct of_device_id reset_simple_dt_ids[] = {
> + { .compatible = "allwinner,sun6i-a31-clock-reset",
> + .data = &reset_simple_active_low },
Can we have a additional generic compatible string here? New users of
this driver then wouldn't need to explicitly enter their name into the
driver, but could just use the generic name as a fallback. This would
enable the driver without any Linux code change just by adding a DT node.
compatible = "nexell,s5p6818-reset", "simple-reset";
Whenever we need a quirk (now or in the future), we can add the specific
name into this structure along with the required workarounds.
Cheers,
Andre.
> + { /* sentinel */ },
> +};
> +
> +static int reset_simple_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + const struct reset_simple_devdata *devdata;
> + struct reset_simple_data *data;
> + void __iomem *membase;
> + struct resource *res;
> +
> + devdata = of_device_get_match_data(dev);
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + membase = devm_ioremap_resource(dev, res);
> + if (IS_ERR(membase))
> + return PTR_ERR(membase);
> +
> + spin_lock_init(&data->lock);
> + data->membase = membase;
> + data->rcdev.owner = THIS_MODULE;
> + data->rcdev.nr_resets = resource_size(res) * BITS_PER_BYTE;
> + data->rcdev.ops = &reset_simple_ops;
> + data->rcdev.of_node = dev->of_node;
> +
> + if (devdata)
> + data->active_low = devdata->active_low;
> +
> + return devm_reset_controller_register(dev, &data->rcdev);
> +}
> +
> +static struct platform_driver reset_simple_driver = {
> + .probe = reset_simple_probe,
> + .driver = {
> + .name = "simple-reset",
> + .of_match_table = reset_simple_dt_ids,
> + },
> +};
> +builtin_platform_driver(reset_simple_driver);
> diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
> new file mode 100644
> index 0000000000000..39af2014b5f12
> --- /dev/null
> +++ b/drivers/reset/reset-simple.h
> @@ -0,0 +1,41 @@
> +/*
> + * Simple Reset Controller ops
> + *
> + * Based on Allwinner SoCs Reset Controller driver
> + *
> + * Copyright 2013 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __RESET_SIMPLE_H__
> +#define __RESET_SIMPLE_H__
> +
> +#include <linux/io.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +
> +/**
> + * struct reset_simple_data - driver data for simple reset controllers
> + * @lock: spinlock to protect registers during read-modify-write cycles
> + * @membase: memory mapped I/O register range
> + * @rcdev: reset controller device base structure
> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> + * are set to assert the reset. Note that this says nothing about
> + * the voltage level of the actual reset line.
> + */
> +struct reset_simple_data {
> + spinlock_t lock;
> + void __iomem *membase;
> + struct reset_controller_dev rcdev;
> + bool active_low;
> +};
> +
> +extern const struct reset_control_ops reset_simple_ops;
> +
> +#endif /* __RESET_SIMPLE_H__ */
> diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
> index 2c7dd1fd08df6..db9a1a75523f4 100644
> --- a/drivers/reset/reset-sunxi.c
> +++ b/drivers/reset/reset-sunxi.c
> @@ -22,64 +22,11 @@
> #include <linux/spinlock.h>
> #include <linux/types.h>
>
> -struct sunxi_reset_data {
> - spinlock_t lock;
> - void __iomem *membase;
> - struct reset_controller_dev rcdev;
> -};
> -
> -static int sunxi_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct sunxi_reset_data *data = container_of(rcdev,
> - struct sunxi_reset_data,
> - rcdev);
> - int reg_width = sizeof(u32);
> - int bank = id / (reg_width * BITS_PER_BYTE);
> - int offset = id % (reg_width * BITS_PER_BYTE);
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * reg_width));
> - writel(reg & ~BIT(offset), data->membase + (bank * reg_width));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static int sunxi_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct sunxi_reset_data *data = container_of(rcdev,
> - struct sunxi_reset_data,
> - rcdev);
> - int reg_width = sizeof(u32);
> - int bank = id / (reg_width * BITS_PER_BYTE);
> - int offset = id % (reg_width * BITS_PER_BYTE);
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * reg_width));
> - writel(reg | BIT(offset), data->membase + (bank * reg_width));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static const struct reset_control_ops sunxi_reset_ops = {
> - .assert = sunxi_reset_assert,
> - .deassert = sunxi_reset_deassert,
> -};
> +#include "reset-simple.h"
>
> static int sunxi_reset_init(struct device_node *np)
> {
> - struct sunxi_reset_data *data;
> + struct reset_simple_data *data;
> struct resource res;
> resource_size_t size;
> int ret;
> @@ -108,8 +55,9 @@ static int sunxi_reset_init(struct device_node *np)
>
> data->rcdev.owner = THIS_MODULE;
> data->rcdev.nr_resets = size * 8;
> - data->rcdev.ops = &sunxi_reset_ops;
> + data->rcdev.ops = &reset_simple_ops;
> data->rcdev.of_node = np;
> + data->active_low = true;
>
> return reset_controller_register(&data->rcdev);
>
> @@ -122,6 +70,8 @@ static int sunxi_reset_init(struct device_node *np)
> * These are the reset controller we need to initialize early on in
> * our system, before we can even think of using a regular device
> * driver for it.
> + * The controllers that we can register through the regular device
> + * model are handled by the simple reset driver directly.
> */
> static const struct of_device_id sunxi_early_reset_dt_ids[] __initconst = {
> { .compatible = "allwinner,sun6i-a31-ahb1-reset", },
> @@ -135,45 +85,3 @@ void __init sun6i_reset_init(void)
> for_each_matching_node(np, sunxi_early_reset_dt_ids)
> sunxi_reset_init(np);
> }
> -
> -/*
> - * And these are the controllers we can register through the regular
> - * device model.
> - */
> -static const struct of_device_id sunxi_reset_dt_ids[] = {
> - { .compatible = "allwinner,sun6i-a31-clock-reset", },
> - { /* sentinel */ },
> -};
> -
> -static int sunxi_reset_probe(struct platform_device *pdev)
> -{
> - struct sunxi_reset_data *data;
> - struct resource *res;
> -
> - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->membase = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(data->membase))
> - return PTR_ERR(data->membase);
> -
> - spin_lock_init(&data->lock);
> -
> - data->rcdev.owner = THIS_MODULE;
> - data->rcdev.nr_resets = resource_size(res) * 8;
> - data->rcdev.ops = &sunxi_reset_ops;
> - data->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> -}
> -
> -static struct platform_driver sunxi_reset_driver = {
> - .probe = sunxi_reset_probe,
> - .driver = {
> - .name = "sunxi-reset",
> - .of_match_table = sunxi_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(sunxi_reset_driver);
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
@ 2017-08-16 11:30 ` Andre Przywara
0 siblings, 0 replies; 54+ messages in thread
From: Andre Przywara @ 2017-08-16 11:30 UTC (permalink / raw)
To: linux-arm-kernel
Hi Philipp,
sorry for the delay, I was on holidays.
Thanks for putting together the series, this looks very good to me.
One comment below...
On 16/08/17 10:46, Philipp Zabel wrote:
> Split reusable parts out of the sunxi driver, to add a driver for simple
> reset controllers with reset lines that can be controlled by toggling
> bits in exclusive, contiguous register ranges using read-modify-write
> cycles under a spinlock. The separate sunxi driver still remains to
> register the early reset controllers, but it reuses the reset-simple
> ops.
>
> The following patches will replace compatible reset drivers with
> reset-simple, extending it where necessary.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Changes since v2:
> - Add kerneldoc comment for struct reset_simple_devdata and struct
> reset_simple_data.
> - Rename "inverted" properties to "active_low".
> - Use of_device_get_match_data instead of of_match_device.
> ---
> drivers/reset/Kconfig | 11 ++++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-simple.c | 134 +++++++++++++++++++++++++++++++++++++++++++
> drivers/reset/reset-simple.h | 41 +++++++++++++
> drivers/reset/reset-sunxi.c | 104 ++-------------------------------
> 5 files changed, 193 insertions(+), 98 deletions(-)
> create mode 100644 drivers/reset/reset-simple.c
> create mode 100644 drivers/reset/reset-simple.h
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 52d5251660b9b..f7ba01a71daee 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -68,6 +68,16 @@ config RESET_PISTACHIO
> help
> This enables the reset driver for ImgTec Pistachio SoCs.
>
> +config RESET_SIMPLE
> + bool "Simple Reset Controller Driver" if COMPILE_TEST
> + default ARCH_SUNXI
> + help
> + This enables a simple reset controller driver for reset lines that
> + that can be asserted and deasserted by toggling bits in a contiguous,
> + exclusive register space.
> +
> + Currently this driver supports Allwinner SoCs.
> +
> config RESET_SOCFPGA
> bool "SoCFPGA Reset Driver" if COMPILE_TEST
> default ARCH_SOCFPGA
> @@ -83,6 +93,7 @@ config RESET_STM32
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> default ARCH_SUNXI
> + select RESET_SIMPLE
> help
> This enables the reset driver for Allwinner SoCs.
>
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index b62783f50fe5b..ab4af99c3dfdc 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
> obj-$(CONFIG_RESET_MESON) += reset-meson.o
> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> +obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
> obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> new file mode 100644
> index 0000000000000..70c57c0758c39
> --- /dev/null
> +++ b/drivers/reset/reset-simple.c
> @@ -0,0 +1,134 @@
> +/*
> + * Simple Reset Controller Driver
> + *
> + * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
> + *
> + * Based on Allwinner SoCs Reset Controller driver
> + *
> + * Copyright 2013 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +
> +#include "reset-simple.h"
> +
> +static inline struct reset_simple_data *
> +to_reset_simple_data(struct reset_controller_dev *rcdev)
> +{
> + return container_of(rcdev, struct reset_simple_data, rcdev);
> +}
> +
> +static int reset_simple_set(struct reset_controller_dev *rcdev,
> + unsigned long id, bool assert)
> +{
> + struct reset_simple_data *data = to_reset_simple_data(rcdev);
> + int reg_width = sizeof(u32);
> + int bank = id / (reg_width * BITS_PER_BYTE);
> + int offset = id % (reg_width * BITS_PER_BYTE);
> + unsigned long flags;
> + u32 reg;
> +
> + spin_lock_irqsave(&data->lock, flags);
> +
> + reg = readl(data->membase + (bank * reg_width));
> + if (assert ^ data->active_low)
> + reg |= BIT(offset);
> + else
> + reg &= ~BIT(offset);
> + writel(reg, data->membase + (bank * reg_width));
> +
> + spin_unlock_irqrestore(&data->lock, flags);
> +
> + return 0;
> +}
> +
> +static int reset_simple_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return reset_simple_set(rcdev, id, true);
> +}
> +
> +static int reset_simple_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return reset_simple_set(rcdev, id, false);
> +}
> +
> +const struct reset_control_ops reset_simple_ops = {
> + .assert = reset_simple_assert,
> + .deassert = reset_simple_deassert,
> +};
> +
> +/**
> + * struct reset_simple_devdata - simple reset controller properties
> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> + * are set to assert the reset.
> + */
> +struct reset_simple_devdata {
> + bool active_low;
> +};
> +
> +static const struct reset_simple_devdata reset_simple_active_low = {
> + .active_low = true,
> +};
> +
> +static const struct of_device_id reset_simple_dt_ids[] = {
> + { .compatible = "allwinner,sun6i-a31-clock-reset",
> + .data = &reset_simple_active_low },
Can we have a additional generic compatible string here? New users of
this driver then wouldn't need to explicitly enter their name into the
driver, but could just use the generic name as a fallback. This would
enable the driver without any Linux code change just by adding a DT node.
compatible = "nexell,s5p6818-reset", "simple-reset";
Whenever we need a quirk (now or in the future), we can add the specific
name into this structure along with the required workarounds.
Cheers,
Andre.
> + { /* sentinel */ },
> +};
> +
> +static int reset_simple_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + const struct reset_simple_devdata *devdata;
> + struct reset_simple_data *data;
> + void __iomem *membase;
> + struct resource *res;
> +
> + devdata = of_device_get_match_data(dev);
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + membase = devm_ioremap_resource(dev, res);
> + if (IS_ERR(membase))
> + return PTR_ERR(membase);
> +
> + spin_lock_init(&data->lock);
> + data->membase = membase;
> + data->rcdev.owner = THIS_MODULE;
> + data->rcdev.nr_resets = resource_size(res) * BITS_PER_BYTE;
> + data->rcdev.ops = &reset_simple_ops;
> + data->rcdev.of_node = dev->of_node;
> +
> + if (devdata)
> + data->active_low = devdata->active_low;
> +
> + return devm_reset_controller_register(dev, &data->rcdev);
> +}
> +
> +static struct platform_driver reset_simple_driver = {
> + .probe = reset_simple_probe,
> + .driver = {
> + .name = "simple-reset",
> + .of_match_table = reset_simple_dt_ids,
> + },
> +};
> +builtin_platform_driver(reset_simple_driver);
> diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
> new file mode 100644
> index 0000000000000..39af2014b5f12
> --- /dev/null
> +++ b/drivers/reset/reset-simple.h
> @@ -0,0 +1,41 @@
> +/*
> + * Simple Reset Controller ops
> + *
> + * Based on Allwinner SoCs Reset Controller driver
> + *
> + * Copyright 2013 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __RESET_SIMPLE_H__
> +#define __RESET_SIMPLE_H__
> +
> +#include <linux/io.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +
> +/**
> + * struct reset_simple_data - driver data for simple reset controllers
> + * @lock: spinlock to protect registers during read-modify-write cycles
> + * @membase: memory mapped I/O register range
> + * @rcdev: reset controller device base structure
> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> + * are set to assert the reset. Note that this says nothing about
> + * the voltage level of the actual reset line.
> + */
> +struct reset_simple_data {
> + spinlock_t lock;
> + void __iomem *membase;
> + struct reset_controller_dev rcdev;
> + bool active_low;
> +};
> +
> +extern const struct reset_control_ops reset_simple_ops;
> +
> +#endif /* __RESET_SIMPLE_H__ */
> diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
> index 2c7dd1fd08df6..db9a1a75523f4 100644
> --- a/drivers/reset/reset-sunxi.c
> +++ b/drivers/reset/reset-sunxi.c
> @@ -22,64 +22,11 @@
> #include <linux/spinlock.h>
> #include <linux/types.h>
>
> -struct sunxi_reset_data {
> - spinlock_t lock;
> - void __iomem *membase;
> - struct reset_controller_dev rcdev;
> -};
> -
> -static int sunxi_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct sunxi_reset_data *data = container_of(rcdev,
> - struct sunxi_reset_data,
> - rcdev);
> - int reg_width = sizeof(u32);
> - int bank = id / (reg_width * BITS_PER_BYTE);
> - int offset = id % (reg_width * BITS_PER_BYTE);
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * reg_width));
> - writel(reg & ~BIT(offset), data->membase + (bank * reg_width));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static int sunxi_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct sunxi_reset_data *data = container_of(rcdev,
> - struct sunxi_reset_data,
> - rcdev);
> - int reg_width = sizeof(u32);
> - int bank = id / (reg_width * BITS_PER_BYTE);
> - int offset = id % (reg_width * BITS_PER_BYTE);
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * reg_width));
> - writel(reg | BIT(offset), data->membase + (bank * reg_width));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static const struct reset_control_ops sunxi_reset_ops = {
> - .assert = sunxi_reset_assert,
> - .deassert = sunxi_reset_deassert,
> -};
> +#include "reset-simple.h"
>
> static int sunxi_reset_init(struct device_node *np)
> {
> - struct sunxi_reset_data *data;
> + struct reset_simple_data *data;
> struct resource res;
> resource_size_t size;
> int ret;
> @@ -108,8 +55,9 @@ static int sunxi_reset_init(struct device_node *np)
>
> data->rcdev.owner = THIS_MODULE;
> data->rcdev.nr_resets = size * 8;
> - data->rcdev.ops = &sunxi_reset_ops;
> + data->rcdev.ops = &reset_simple_ops;
> data->rcdev.of_node = np;
> + data->active_low = true;
>
> return reset_controller_register(&data->rcdev);
>
> @@ -122,6 +70,8 @@ static int sunxi_reset_init(struct device_node *np)
> * These are the reset controller we need to initialize early on in
> * our system, before we can even think of using a regular device
> * driver for it.
> + * The controllers that we can register through the regular device
> + * model are handled by the simple reset driver directly.
> */
> static const struct of_device_id sunxi_early_reset_dt_ids[] __initconst = {
> { .compatible = "allwinner,sun6i-a31-ahb1-reset", },
> @@ -135,45 +85,3 @@ void __init sun6i_reset_init(void)
> for_each_matching_node(np, sunxi_early_reset_dt_ids)
> sunxi_reset_init(np);
> }
> -
> -/*
> - * And these are the controllers we can register through the regular
> - * device model.
> - */
> -static const struct of_device_id sunxi_reset_dt_ids[] = {
> - { .compatible = "allwinner,sun6i-a31-clock-reset", },
> - { /* sentinel */ },
> -};
> -
> -static int sunxi_reset_probe(struct platform_device *pdev)
> -{
> - struct sunxi_reset_data *data;
> - struct resource *res;
> -
> - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->membase = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(data->membase))
> - return PTR_ERR(data->membase);
> -
> - spin_lock_init(&data->lock);
> -
> - data->rcdev.owner = THIS_MODULE;
> - data->rcdev.nr_resets = resource_size(res) * 8;
> - data->rcdev.ops = &sunxi_reset_ops;
> - data->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> -}
> -
> -static struct platform_driver sunxi_reset_driver = {
> - .probe = sunxi_reset_probe,
> - .driver = {
> - .name = "sunxi-reset",
> - .of_match_table = sunxi_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(sunxi_reset_driver);
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
2017-08-16 11:30 ` Andre Przywara
@ 2017-08-16 12:12 ` Andreas Färber
-1 siblings, 0 replies; 54+ messages in thread
From: Andreas Färber @ 2017-08-16 12:12 UTC (permalink / raw)
To: Andre Przywara, Philipp Zabel, linux-kernel
Cc: Alexandru Gagniuc, Maxime Coquelin, Alexandre Torgue,
Maxime Ripard, Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev,
Steffen Trumtrar, Dinh Nguyen, linux-arm-kernel, kernel
Hi Andre,
Am 16.08.2017 um 13:30 schrieb Andre Przywara:
> On 16/08/17 10:46, Philipp Zabel wrote:
>> +/**
>> + * struct reset_simple_devdata - simple reset controller properties
>> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>> + * are set to assert the reset.
>> + */
>> +struct reset_simple_devdata {
>> + bool active_low;
>> +};
>> +
>> +static const struct reset_simple_devdata reset_simple_active_low = {
>> + .active_low = true,
>> +};
>> +
>> +static const struct of_device_id reset_simple_dt_ids[] = {
>> + { .compatible = "allwinner,sun6i-a31-clock-reset",
>> + .data = &reset_simple_active_low },
>
> Can we have a additional generic compatible string here? New users of
> this driver then wouldn't need to explicitly enter their name into the
> driver, but could just use the generic name as a fallback. This would
> enable the driver without any Linux code change just by adding a DT node.
>
> compatible = "nexell,s5p6818-reset", "simple-reset";
>
> Whenever we need a quirk (now or in the future), we can add the specific
> name into this structure along with the required workarounds.
Same question about binding here. However the way it is done today, we
would also need some optional active-low property then or two different
compatible strings, as this is currently controlled via the DT matches.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
@ 2017-08-16 12:12 ` Andreas Färber
0 siblings, 0 replies; 54+ messages in thread
From: Andreas Färber @ 2017-08-16 12:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi Andre,
Am 16.08.2017 um 13:30 schrieb Andre Przywara:
> On 16/08/17 10:46, Philipp Zabel wrote:
>> +/**
>> + * struct reset_simple_devdata - simple reset controller properties
>> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>> + * are set to assert the reset.
>> + */
>> +struct reset_simple_devdata {
>> + bool active_low;
>> +};
>> +
>> +static const struct reset_simple_devdata reset_simple_active_low = {
>> + .active_low = true,
>> +};
>> +
>> +static const struct of_device_id reset_simple_dt_ids[] = {
>> + { .compatible = "allwinner,sun6i-a31-clock-reset",
>> + .data = &reset_simple_active_low },
>
> Can we have a additional generic compatible string here? New users of
> this driver then wouldn't need to explicitly enter their name into the
> driver, but could just use the generic name as a fallback. This would
> enable the driver without any Linux code change just by adding a DT node.
>
> compatible = "nexell,s5p6818-reset", "simple-reset";
>
> Whenever we need a quirk (now or in the future), we can add the specific
> name into this structure along with the required workarounds.
Same question about binding here. However the way it is done today, we
would also need some optional active-low property then or two different
compatible strings, as this is currently controlled via the DT matches.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 3/5] reset: stm32: use the reset-simple driver
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-16 12:52 ` Eugeniy Paltsev
-1 siblings, 0 replies; 54+ messages in thread
From: Eugeniy Paltsev @ 2017-08-16 12:52 UTC (permalink / raw)
To: p.zabel
Cc: linux-kernel, alex.g, Eugeniy.Paltsev, andre.przywara,
s.trumtrar, wens, baoyou.xie, mcoquelin.stm32, dinguyen,
linux-arm-kernel, alexandre.torgue, afaerber, maxime.ripard,
kernel
Hi Philipp,
On Wed, 2017-08-16 at 11:46 +0200, Philipp Zabel wrote:
> The reset-simple driver can be used without changes.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> [snip]
>
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -120,6 +120,7 @@ static const struct reset_simple_devdata
> reset_simple_active_low = {
>
> static const struct of_device_id reset_simple_dt_ids[] = {
> { .compatible = "altr,rst-mgr", .data =
> &reset_simple_socfpga },
> + { .compatible = "st,stm32-rcc", },
> { .compatible = "allwinner,sun6i-a31-clock-reset",
> .data = &reset_simple_active_low },
> { /* sentinel */ },
What about adding generic compatible strings for future use?
I mean next:
--------------------->8---------------------
static const struct of_device_id reset_simple_dt_ids[] = {
{ .compatible = "reset-simple-active-low",
.data = &reset_simple_active_low },
{ .compatible = "reset-simple-active-high",
.data = &reset_simple_active_high },
/* ... */
{ /* sentinel */ },
};
static const struct reset_simple_devdata reset_simple_active_high = {
.active_low = false,
};
static const struct reset_simple_devdata reset_simple_active_low = {
.active_low = true,
};
--------------------->8---------------------
--
Eugeniy Paltsev
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 3/5] reset: stm32: use the reset-simple driver
@ 2017-08-16 12:52 ` Eugeniy Paltsev
0 siblings, 0 replies; 54+ messages in thread
From: Eugeniy Paltsev @ 2017-08-16 12:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi Philipp,
On Wed, 2017-08-16 at 11:46 +0200, Philipp Zabel wrote:
> The reset-simple driver can be used without changes.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> [snip]
>
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -120,6 +120,7 @@ static const struct reset_simple_devdata
> reset_simple_active_low = {
> ?
> ?static const struct of_device_id reset_simple_dt_ids[] = {
> ? { .compatible = "altr,rst-mgr", .data =
> &reset_simple_socfpga },
> + { .compatible = "st,stm32-rcc", },
> ? { .compatible = "allwinner,sun6i-a31-clock-reset",
> ? .data = &reset_simple_active_low },
> ? { /* sentinel */ },
What about adding generic compatible strings for future use?
I mean next:
--------------------->8---------------------
static const struct of_device_id reset_simple_dt_ids[] = {
{ .compatible = "reset-simple-active-low",
.data = &reset_simple_active_low },
{ .compatible = "reset-simple-active-high",
.data = &reset_simple_active_high },
/* ... */
{ /* sentinel */ },
};
static const struct reset_simple_devdata reset_simple_active_high = {
.active_low = false,
};
static const struct reset_simple_devdata reset_simple_active_low = {
.active_low = true,
};
--------------------->8---------------------
--
?Eugeniy Paltsev
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 3/5] reset: stm32: use the reset-simple driver
2017-08-16 12:52 ` Eugeniy Paltsev
@ 2017-08-16 12:55 ` Andreas Färber
-1 siblings, 0 replies; 54+ messages in thread
From: Andreas Färber @ 2017-08-16 12:55 UTC (permalink / raw)
To: Eugeniy Paltsev
Cc: p.zabel, linux-kernel, alex.g, andre.przywara, s.trumtrar, wens,
baoyou.xie, mcoquelin.stm32, dinguyen, linux-arm-kernel,
alexandre.torgue, maxime.ripard, kernel
Hi Eugeniy,
Am 16.08.2017 um 14:52 schrieb Eugeniy Paltsev:
> On Wed, 2017-08-16 at 11:46 +0200, Philipp Zabel wrote:
>> The reset-simple driver can be used without changes.
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> [snip]
>>
>> --- a/drivers/reset/reset-simple.c
>> +++ b/drivers/reset/reset-simple.c
>> @@ -120,6 +120,7 @@ static const struct reset_simple_devdata
>> reset_simple_active_low = {
>>
>> static const struct of_device_id reset_simple_dt_ids[] = {
>> { .compatible = "altr,rst-mgr", .data =
>> &reset_simple_socfpga },
>> + { .compatible = "st,stm32-rcc", },
>> { .compatible = "allwinner,sun6i-a31-clock-reset",
>> .data = &reset_simple_active_low },
>> { /* sentinel */ },
>
> What about adding generic compatible strings for future use?
Please see 1/5 for the same discussion, raised by Andre and me.
Thanks,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 3/5] reset: stm32: use the reset-simple driver
@ 2017-08-16 12:55 ` Andreas Färber
0 siblings, 0 replies; 54+ messages in thread
From: Andreas Färber @ 2017-08-16 12:55 UTC (permalink / raw)
To: linux-arm-kernel
Hi Eugeniy,
Am 16.08.2017 um 14:52 schrieb Eugeniy Paltsev:
> On Wed, 2017-08-16 at 11:46 +0200, Philipp Zabel wrote:
>> The reset-simple driver can be used without changes.
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> [snip]
>>
>> --- a/drivers/reset/reset-simple.c
>> +++ b/drivers/reset/reset-simple.c
>> @@ -120,6 +120,7 @@ static const struct reset_simple_devdata
>> reset_simple_active_low = {
>>
>> static const struct of_device_id reset_simple_dt_ids[] = {
>> { .compatible = "altr,rst-mgr", .data =
>> &reset_simple_socfpga },
>> + { .compatible = "st,stm32-rcc", },
>> { .compatible = "allwinner,sun6i-a31-clock-reset",
>> .data = &reset_simple_active_low },
>> { /* sentinel */ },
>
> What about adding generic compatible strings for future use?
Please see 1/5 for the same discussion, raised by Andre and me.
Thanks,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 3/5] reset: stm32: use the reset-simple driver
2017-08-16 12:52 ` Eugeniy Paltsev
@ 2017-08-16 13:03 ` Andre Przywara
-1 siblings, 0 replies; 54+ messages in thread
From: Andre Przywara @ 2017-08-16 13:03 UTC (permalink / raw)
To: Eugeniy Paltsev, p.zabel
Cc: linux-kernel, alex.g, s.trumtrar, wens, baoyou.xie,
mcoquelin.stm32, dinguyen, linux-arm-kernel, alexandre.torgue,
afaerber, maxime.ripard, kernel
Hi,
On 16/08/17 13:52, Eugeniy Paltsev wrote:
> Hi Philipp,
>
> On Wed, 2017-08-16 at 11:46 +0200, Philipp Zabel wrote:
>> The reset-simple driver can be used without changes.
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> [snip]
>>
>> --- a/drivers/reset/reset-simple.c
>> +++ b/drivers/reset/reset-simple.c
>> @@ -120,6 +120,7 @@ static const struct reset_simple_devdata
>> reset_simple_active_low = {
>>
>> static const struct of_device_id reset_simple_dt_ids[] = {
>> { .compatible = "altr,rst-mgr", .data =
>> &reset_simple_socfpga },
>> + { .compatible = "st,stm32-rcc", },
>> { .compatible = "allwinner,sun6i-a31-clock-reset",
>> .data = &reset_simple_active_low },
>> { /* sentinel */ },
>
> What about adding generic compatible strings for future use?
Exactly ;-) (see my previous email)
But we still need the pre-existing strings in here, of course.
> I mean next:
>
> --------------------->8---------------------
>
> static const struct of_device_id reset_simple_dt_ids[] = {
> { .compatible = "reset-simple-active-low",
> .data = &reset_simple_active_low },
> { .compatible = "reset-simple-active-high",
> .data = &reset_simple_active_high },
Either that or maybe even better define a "simple-reset" string and add
standard properties like "active-low".
Cheers,
Andre.
> /* ... */
> { /* sentinel */ },
> };
>
> static const struct reset_simple_devdata reset_simple_active_high = {
> .active_low = false,
> };
>
> static const struct reset_simple_devdata reset_simple_active_low = {
> .active_low = true,
> };
>
> --------------------->8---------------------
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 3/5] reset: stm32: use the reset-simple driver
@ 2017-08-16 13:03 ` Andre Przywara
0 siblings, 0 replies; 54+ messages in thread
From: Andre Przywara @ 2017-08-16 13:03 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 16/08/17 13:52, Eugeniy Paltsev wrote:
> Hi Philipp,
>
> On Wed, 2017-08-16 at 11:46 +0200, Philipp Zabel wrote:
>> The reset-simple driver can be used without changes.
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> [snip]
>>
>> --- a/drivers/reset/reset-simple.c
>> +++ b/drivers/reset/reset-simple.c
>> @@ -120,6 +120,7 @@ static const struct reset_simple_devdata
>> reset_simple_active_low = {
>>
>> static const struct of_device_id reset_simple_dt_ids[] = {
>> { .compatible = "altr,rst-mgr", .data =
>> &reset_simple_socfpga },
>> + { .compatible = "st,stm32-rcc", },
>> { .compatible = "allwinner,sun6i-a31-clock-reset",
>> .data = &reset_simple_active_low },
>> { /* sentinel */ },
>
> What about adding generic compatible strings for future use?
Exactly ;-) (see my previous email)
But we still need the pre-existing strings in here, of course.
> I mean next:
>
> --------------------->8---------------------
>
> static const struct of_device_id reset_simple_dt_ids[] = {
> { .compatible = "reset-simple-active-low",
> .data = &reset_simple_active_low },
> { .compatible = "reset-simple-active-high",
> .data = &reset_simple_active_high },
Either that or maybe even better define a "simple-reset" string and add
standard properties like "active-low".
Cheers,
Andre.
> /* ... */
> { /* sentinel */ },
> };
>
> static const struct reset_simple_devdata reset_simple_active_high = {
> .active_low = false,
> };
>
> static const struct reset_simple_devdata reset_simple_active_low = {
> .active_low = true,
> };
>
> --------------------->8---------------------
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
2017-08-16 12:12 ` Andreas Färber
@ 2017-08-16 15:11 ` Philipp Zabel
-1 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 15:11 UTC (permalink / raw)
To: Andreas Färber, Andre Przywara, linux-kernel
Cc: Alexandru Gagniuc, Maxime Coquelin, Alexandre Torgue,
Maxime Ripard, Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev,
Steffen Trumtrar, Dinh Nguyen, linux-arm-kernel, kernel
On Wed, 2017-08-16 at 14:12 +0200, Andreas Färber wrote:
> Hi Andre,
>
> Am 16.08.2017 um 13:30 schrieb Andre Przywara:
> > On 16/08/17 10:46, Philipp Zabel wrote:
> > > +/**
> > > + * struct reset_simple_devdata - simple reset controller properties
> > > + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> > > + * are set to assert the reset.
> > > + */
> > > +struct reset_simple_devdata {
> > > > > > + bool active_low;
> > > +};
> > > +
> > > +static const struct reset_simple_devdata reset_simple_active_low = {
> > > > > > + .active_low = true,
> > > +};
> > > +
> > > +static const struct of_device_id reset_simple_dt_ids[] = {
> > > > > > + { .compatible = "allwinner,sun6i-a31-clock-reset",
> > > + .data = &reset_simple_active_low },
> >
> > Can we have a additional generic compatible string here? New users of
> > this driver then wouldn't need to explicitly enter their name into the
> > driver, but could just use the generic name as a fallback. This would
> > enable the driver without any Linux code change just by adding a DT node.
> >
> > compatible = "nexell,s5p6818-reset", "simple-reset";
> >
> > Whenever we need a quirk (now or in the future), we can add the specific
> > name into this structure along with the required workarounds.
>
> Same question about binding here. However the way it is done today, we
> would also need some optional active-low property then or two different
> compatible strings, as this is currently controlled via the DT matches.
I'd like to decouple this from the issue at hand, which is de-
duplicating simple reset code without device tree changes.
I'll make a separate suggestion for a simple binding on top of this
series.
regards
Philipp
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
@ 2017-08-16 15:11 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-16 15:11 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2017-08-16 at 14:12 +0200, Andreas F?rber wrote:
> Hi Andre,
>
> Am 16.08.2017 um 13:30 schrieb Andre Przywara:
> > On 16/08/17 10:46, Philipp Zabel wrote:
> > > +/**
> > > + * struct reset_simple_devdata - simple reset controller properties
> > > + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> > > + *??????????????are set to assert the reset.
> > > + */
> > > +struct reset_simple_devdata {
> > > > > > + bool active_low;
> > > +};
> > > +
> > > +static const struct reset_simple_devdata reset_simple_active_low = {
> > > > > > + .active_low = true,
> > > +};
> > > +
> > > +static const struct of_device_id reset_simple_dt_ids[] = {
> > > > > > + { .compatible = "allwinner,sun6i-a31-clock-reset",
> > > + .data = &reset_simple_active_low },
> >
> > Can we have a additional generic compatible string here? New users of
> > this driver then wouldn't need to explicitly enter their name into the
> > driver, but could just use the generic name as a fallback. This would
> > enable the driver without any Linux code change just by adding a DT node.
> >
> > compatible = "nexell,s5p6818-reset", "simple-reset";
> >
> > Whenever we need a quirk (now or in the future), we can add the specific
> > name into this structure along with the required workarounds.
>
> Same question about binding here. However the way it is done today, we
> would also need some optional active-low property then or two different
> compatible strings, as this is currently controlled via the DT matches.
I'd like to decouple this from the issue at hand, which is de-
duplicating simple reset code without device tree changes.
I'll make a separate suggestion for a simple binding on top of this
series.
regards
Philipp
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
2017-08-16 15:11 ` Philipp Zabel
@ 2017-08-16 15:17 ` Andre Przywara
-1 siblings, 0 replies; 54+ messages in thread
From: Andre Przywara @ 2017-08-16 15:17 UTC (permalink / raw)
To: Philipp Zabel, Andreas Färber, linux-kernel
Cc: Alexandru Gagniuc, Maxime Coquelin, Alexandre Torgue,
Maxime Ripard, Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev,
Steffen Trumtrar, Dinh Nguyen, linux-arm-kernel, kernel
Hi,
On 16/08/17 16:11, Philipp Zabel wrote:
> On Wed, 2017-08-16 at 14:12 +0200, Andreas Färber wrote:
>> Hi Andre,
>>
>> Am 16.08.2017 um 13:30 schrieb Andre Przywara:
>>> On 16/08/17 10:46, Philipp Zabel wrote:
>>>> +/**
>>>> + * struct reset_simple_devdata - simple reset controller properties
>>>> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>>>> + * are set to assert the reset.
>>>> + */
>>>> +struct reset_simple_devdata {
>>>>>>> + bool active_low;
>>>> +};
>>>> +
>>>> +static const struct reset_simple_devdata reset_simple_active_low = {
>>>>>>> + .active_low = true,
>>>> +};
>>>> +
>>>> +static const struct of_device_id reset_simple_dt_ids[] = {
>>>>>>> + { .compatible = "allwinner,sun6i-a31-clock-reset",
>>>> + .data = &reset_simple_active_low },
>>>
>>> Can we have a additional generic compatible string here? New users of
>>> this driver then wouldn't need to explicitly enter their name into the
>>> driver, but could just use the generic name as a fallback. This would
>>> enable the driver without any Linux code change just by adding a DT node.
>>>
>>> compatible = "nexell,s5p6818-reset", "simple-reset";
>>>
>>> Whenever we need a quirk (now or in the future), we can add the specific
>>> name into this structure along with the required workarounds.
>>
>> Same question about binding here. However the way it is done today, we
>> would also need some optional active-low property then or two different
>> compatible strings, as this is currently controlled via the DT matches.
>
> I'd like to decouple this from the issue at hand, which is de-
> duplicating simple reset code without device tree changes.
Agreed, this is an orthogonal issue, actually being enabled by this series.
> I'll make a separate suggestion for a simple binding on top of this
> series.
Thanks! Happy to review it.
I actually have a user at hand already ...
Cheers,
Andre.
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
@ 2017-08-16 15:17 ` Andre Przywara
0 siblings, 0 replies; 54+ messages in thread
From: Andre Przywara @ 2017-08-16 15:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 16/08/17 16:11, Philipp Zabel wrote:
> On Wed, 2017-08-16 at 14:12 +0200, Andreas F?rber wrote:
>> Hi Andre,
>>
>> Am 16.08.2017 um 13:30 schrieb Andre Przywara:
>>> On 16/08/17 10:46, Philipp Zabel wrote:
>>>> +/**
>>>> + * struct reset_simple_devdata - simple reset controller properties
>>>> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>>>> + * are set to assert the reset.
>>>> + */
>>>> +struct reset_simple_devdata {
>>>>>>> + bool active_low;
>>>> +};
>>>> +
>>>> +static const struct reset_simple_devdata reset_simple_active_low = {
>>>>>>> + .active_low = true,
>>>> +};
>>>> +
>>>> +static const struct of_device_id reset_simple_dt_ids[] = {
>>>>>>> + { .compatible = "allwinner,sun6i-a31-clock-reset",
>>>> + .data = &reset_simple_active_low },
>>>
>>> Can we have a additional generic compatible string here? New users of
>>> this driver then wouldn't need to explicitly enter their name into the
>>> driver, but could just use the generic name as a fallback. This would
>>> enable the driver without any Linux code change just by adding a DT node.
>>>
>>> compatible = "nexell,s5p6818-reset", "simple-reset";
>>>
>>> Whenever we need a quirk (now or in the future), we can add the specific
>>> name into this structure along with the required workarounds.
>>
>> Same question about binding here. However the way it is done today, we
>> would also need some optional active-low property then or two different
>> compatible strings, as this is currently controlled via the DT matches.
>
> I'd like to decouple this from the issue at hand, which is de-
> duplicating simple reset code without device tree changes.
Agreed, this is an orthogonal issue, actually being enabled by this series.
> I'll make a separate suggestion for a simple binding on top of this
> series.
Thanks! Happy to review it.
I actually have a user at hand already ...
Cheers,
Andre.
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
2017-08-16 15:11 ` Philipp Zabel
@ 2017-08-16 16:41 ` Andreas Färber
-1 siblings, 0 replies; 54+ messages in thread
From: Andreas Färber @ 2017-08-16 16:41 UTC (permalink / raw)
To: Philipp Zabel
Cc: Andre Przywara, linux-kernel, Alexandru Gagniuc, Maxime Coquelin,
Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen, linux-arm-kernel,
kernel
Am 16.08.2017 um 17:11 schrieb Philipp Zabel:
> On Wed, 2017-08-16 at 14:12 +0200, Andreas Färber wrote:
>> Am 16.08.2017 um 13:30 schrieb Andre Przywara:
>>> On 16/08/17 10:46, Philipp Zabel wrote:
>>>> +/**
>>>> + * struct reset_simple_devdata - simple reset controller properties
>>>> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>>>> + * are set to assert the reset.
>>>> + */
>>>> +struct reset_simple_devdata {
>>>>>>> + bool active_low;
>>>> +};
>>>> +
>>>> +static const struct reset_simple_devdata reset_simple_active_low = {
>>>>>>> + .active_low = true,
>>>> +};
>>>> +
>>>> +static const struct of_device_id reset_simple_dt_ids[] = {
>>>>>>> + { .compatible = "allwinner,sun6i-a31-clock-reset",
>>>> + .data = &reset_simple_active_low },
>>>
>>> Can we have a additional generic compatible string here? New users of
>>> this driver then wouldn't need to explicitly enter their name into the
>>> driver, but could just use the generic name as a fallback. This would
>>> enable the driver without any Linux code change just by adding a DT node.
>>>
>>> compatible = "nexell,s5p6818-reset", "simple-reset";
>>>
>>> Whenever we need a quirk (now or in the future), we can add the specific
>>> name into this structure along with the required workarounds.
>>
>> Same question about binding here. However the way it is done today, we
>> would also need some optional active-low property then or two different
>> compatible strings, as this is currently controlled via the DT matches.
>
> I'd like to decouple this from the issue at hand, which is de-
> duplicating simple reset code without device tree changes.
>
> I'll make a separate suggestion for a simple binding on top of this
> series.
Okay, I'll continue using my own driver then, until it's clear how
exactly I'm supposed to piggy-back on this new driver.
Cheers,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
@ 2017-08-16 16:41 ` Andreas Färber
0 siblings, 0 replies; 54+ messages in thread
From: Andreas Färber @ 2017-08-16 16:41 UTC (permalink / raw)
To: linux-arm-kernel
Am 16.08.2017 um 17:11 schrieb Philipp Zabel:
> On Wed, 2017-08-16 at 14:12 +0200, Andreas F?rber wrote:
>> Am 16.08.2017 um 13:30 schrieb Andre Przywara:
>>> On 16/08/17 10:46, Philipp Zabel wrote:
>>>> +/**
>>>> + * struct reset_simple_devdata - simple reset controller properties
>>>> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>>>> + * are set to assert the reset.
>>>> + */
>>>> +struct reset_simple_devdata {
>>>>>>> + bool active_low;
>>>> +};
>>>> +
>>>> +static const struct reset_simple_devdata reset_simple_active_low = {
>>>>>>> + .active_low = true,
>>>> +};
>>>> +
>>>> +static const struct of_device_id reset_simple_dt_ids[] = {
>>>>>>> + { .compatible = "allwinner,sun6i-a31-clock-reset",
>>>> + .data = &reset_simple_active_low },
>>>
>>> Can we have a additional generic compatible string here? New users of
>>> this driver then wouldn't need to explicitly enter their name into the
>>> driver, but could just use the generic name as a fallback. This would
>>> enable the driver without any Linux code change just by adding a DT node.
>>>
>>> compatible = "nexell,s5p6818-reset", "simple-reset";
>>>
>>> Whenever we need a quirk (now or in the future), we can add the specific
>>> name into this structure along with the required workarounds.
>>
>> Same question about binding here. However the way it is done today, we
>> would also need some optional active-low property then or two different
>> compatible strings, as this is currently controlled via the DT matches.
>
> I'd like to decouple this from the issue at hand, which is de-
> duplicating simple reset code without device tree changes.
>
> I'll make a separate suggestion for a simple binding on top of this
> series.
Okay, I'll continue using my own driver then, until it's clear how
exactly I'm supposed to piggy-back on this new driver.
Cheers,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
2017-08-16 16:41 ` Andreas Färber
@ 2017-08-16 16:46 ` Andre Przywara
-1 siblings, 0 replies; 54+ messages in thread
From: Andre Przywara @ 2017-08-16 16:46 UTC (permalink / raw)
To: Andreas Färber, Philipp Zabel
Cc: linux-kernel, Alexandru Gagniuc, Maxime Coquelin,
Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen, linux-arm-kernel,
kernel
Hi,
On 16/08/17 17:41, Andreas Färber wrote:
> Am 16.08.2017 um 17:11 schrieb Philipp Zabel:
>> On Wed, 2017-08-16 at 14:12 +0200, Andreas Färber wrote:
>>> Am 16.08.2017 um 13:30 schrieb Andre Przywara:
>>>> On 16/08/17 10:46, Philipp Zabel wrote:
>>>>> +/**
>>>>> + * struct reset_simple_devdata - simple reset controller properties
>>>>> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>>>>> + * are set to assert the reset.
>>>>> + */
>>>>> +struct reset_simple_devdata {
>>>>>>>> + bool active_low;
>>>>> +};
>>>>> +
>>>>> +static const struct reset_simple_devdata reset_simple_active_low = {
>>>>>>>> + .active_low = true,
>>>>> +};
>>>>> +
>>>>> +static const struct of_device_id reset_simple_dt_ids[] = {
>>>>>>>> + { .compatible = "allwinner,sun6i-a31-clock-reset",
>>>>> + .data = &reset_simple_active_low },
>>>>
>>>> Can we have a additional generic compatible string here? New users of
>>>> this driver then wouldn't need to explicitly enter their name into the
>>>> driver, but could just use the generic name as a fallback. This would
>>>> enable the driver without any Linux code change just by adding a DT node.
>>>>
>>>> compatible = "nexell,s5p6818-reset", "simple-reset";
>>>>
>>>> Whenever we need a quirk (now or in the future), we can add the specific
>>>> name into this structure along with the required workarounds.
>>>
>>> Same question about binding here. However the way it is done today, we
>>> would also need some optional active-low property then or two different
>>> compatible strings, as this is currently controlled via the DT matches.
>>
>> I'd like to decouple this from the issue at hand, which is de-
>> duplicating simple reset code without device tree changes.
>>
>> I'll make a separate suggestion for a simple binding on top of this
>> series.
>
> Okay, I'll continue using my own driver then, until it's clear how
> exactly I'm supposed to piggy-back on this new driver.
It's not very nice, but if you can't wait, what about using:
compatible = "your_vendor,your-reset",
"allwinner,sun6i-a31-clock-reset";
for now, then later adding the new generic compatible string at the end,
once we agreed on the naming?
That should give you support for all ranges of kernels.
Cheers,
Andre.
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
@ 2017-08-16 16:46 ` Andre Przywara
0 siblings, 0 replies; 54+ messages in thread
From: Andre Przywara @ 2017-08-16 16:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 16/08/17 17:41, Andreas F?rber wrote:
> Am 16.08.2017 um 17:11 schrieb Philipp Zabel:
>> On Wed, 2017-08-16 at 14:12 +0200, Andreas F?rber wrote:
>>> Am 16.08.2017 um 13:30 schrieb Andre Przywara:
>>>> On 16/08/17 10:46, Philipp Zabel wrote:
>>>>> +/**
>>>>> + * struct reset_simple_devdata - simple reset controller properties
>>>>> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>>>>> + * are set to assert the reset.
>>>>> + */
>>>>> +struct reset_simple_devdata {
>>>>>>>> + bool active_low;
>>>>> +};
>>>>> +
>>>>> +static const struct reset_simple_devdata reset_simple_active_low = {
>>>>>>>> + .active_low = true,
>>>>> +};
>>>>> +
>>>>> +static const struct of_device_id reset_simple_dt_ids[] = {
>>>>>>>> + { .compatible = "allwinner,sun6i-a31-clock-reset",
>>>>> + .data = &reset_simple_active_low },
>>>>
>>>> Can we have a additional generic compatible string here? New users of
>>>> this driver then wouldn't need to explicitly enter their name into the
>>>> driver, but could just use the generic name as a fallback. This would
>>>> enable the driver without any Linux code change just by adding a DT node.
>>>>
>>>> compatible = "nexell,s5p6818-reset", "simple-reset";
>>>>
>>>> Whenever we need a quirk (now or in the future), we can add the specific
>>>> name into this structure along with the required workarounds.
>>>
>>> Same question about binding here. However the way it is done today, we
>>> would also need some optional active-low property then or two different
>>> compatible strings, as this is currently controlled via the DT matches.
>>
>> I'd like to decouple this from the issue at hand, which is de-
>> duplicating simple reset code without device tree changes.
>>
>> I'll make a separate suggestion for a simple binding on top of this
>> series.
>
> Okay, I'll continue using my own driver then, until it's clear how
> exactly I'm supposed to piggy-back on this new driver.
It's not very nice, but if you can't wait, what about using:
compatible = "your_vendor,your-reset",
"allwinner,sun6i-a31-clock-reset";
for now, then later adding the new generic compatible string at the end,
once we agreed on the naming?
That should give you support for all ranges of kernels.
Cheers,
Andre.
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 2/5] reset: socfpga: use the reset-simple driver
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-16 20:46 ` Alexandru Gagniuc
-1 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 20:46 UTC (permalink / raw)
To: Philipp Zabel, linux-kernel
Cc: Andre Przywara, Maxime Coquelin, Alexandre Torgue, Maxime Ripard,
Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev, Steffen Trumtrar,
Dinh Nguyen, Andreas Färber, linux-arm-kernel, kernel
Hi Phillip,
On 08/16/2017 02:46 AM, Philipp Zabel wrote:
[snip]
> @@ -118,8 +151,23 @@ static int reset_simple_probe(struct platform_device *pdev)
> data->rcdev.ops = &reset_simple_ops;
> data->rcdev.of_node = dev->of_node;
>
> - if (devdata)
> + if (devdata) {
> + u32 reg_offset = devdata->reg_offset;
> +
> + if (reg_offset &&
> + of_property_read_u32(dev->of_node, "altr,modrst-offset",
> + ®_offset)) {
> + dev_warn(dev,
> + "missing altr,modrst-offset property, assuming 0x%x!\n",
> + reg_offset);
> + }
I would not make reading dt properties dependent on the presence of
devdata. That breaks being able to configure the reset controller from
dt. I would either just read the "altr,modrst-offset" property, or read
it when
of_device_is_compatible(dev->of_node, "altr,rst-mgr"));
Ideally, we could have a more generic "reg-offset" binding for new reset
controllers, but that is beyond the scope of this patch.
Alex
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 2/5] reset: socfpga: use the reset-simple driver
@ 2017-08-16 20:46 ` Alexandru Gagniuc
0 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 20:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi Phillip,
On 08/16/2017 02:46 AM, Philipp Zabel wrote:
[snip]
> @@ -118,8 +151,23 @@ static int reset_simple_probe(struct platform_device *pdev)
> data->rcdev.ops = &reset_simple_ops;
> data->rcdev.of_node = dev->of_node;
>
> - if (devdata)
> + if (devdata) {
> + u32 reg_offset = devdata->reg_offset;
> +
> + if (reg_offset &&
> + of_property_read_u32(dev->of_node, "altr,modrst-offset",
> + ®_offset)) {
> + dev_warn(dev,
> + "missing altr,modrst-offset property, assuming 0x%x!\n",
> + reg_offset);
> + }
I would not make reading dt properties dependent on the presence of
devdata. That breaks being able to configure the reset controller from
dt. I would either just read the "altr,modrst-offset" property, or read
it when
of_device_is_compatible(dev->of_node, "altr,rst-mgr"));
Ideally, we could have a more generic "reg-offset" binding for new reset
controllers, but that is beyond the scope of this patch.
Alex
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-16 20:46 ` Alexandru Gagniuc
-1 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 20:46 UTC (permalink / raw)
To: Philipp Zabel, linux-kernel
Cc: Andre Przywara, Maxime Coquelin, Alexandre Torgue, Maxime Ripard,
Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev, Steffen Trumtrar,
Dinh Nguyen, Andreas Färber, linux-arm-kernel, kernel
On 08/16/2017 02:46 AM, Philipp Zabel wrote:
> Split reusable parts out of the sunxi driver, to add a driver for simple
> reset controllers with reset lines that can be controlled by toggling
> bits in exclusive, contiguous register ranges using read-modify-write
> cycles under a spinlock. The separate sunxi driver still remains to
> register the early reset controllers, but it reuses the reset-simple
> ops.
>
> The following patches will replace compatible reset drivers with
> reset-simple, extending it where necessary.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> ---
> Changes since v2:
> - Add kerneldoc comment for struct reset_simple_devdata and struct
> reset_simple_data.
> - Rename "inverted" properties to "active_low".
> - Use of_device_get_match_data instead of of_match_device.
> ---
> drivers/reset/Kconfig | 11 ++++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-simple.c | 134 +++++++++++++++++++++++++++++++++++++++++++
> drivers/reset/reset-simple.h | 41 +++++++++++++
> drivers/reset/reset-sunxi.c | 104 ++-------------------------------
> 5 files changed, 193 insertions(+), 98 deletions(-)
> create mode 100644 drivers/reset/reset-simple.c
> create mode 100644 drivers/reset/reset-simple.h
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 52d5251660b9b..f7ba01a71daee 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -68,6 +68,16 @@ config RESET_PISTACHIO
> help
> This enables the reset driver for ImgTec Pistachio SoCs.
>
> +config RESET_SIMPLE
> + bool "Simple Reset Controller Driver" if COMPILE_TEST
> + default ARCH_SUNXI
> + help
> + This enables a simple reset controller driver for reset lines that
> + that can be asserted and deasserted by toggling bits in a contiguous,
> + exclusive register space.
> +
> + Currently this driver supports Allwinner SoCs.
> +
> config RESET_SOCFPGA
> bool "SoCFPGA Reset Driver" if COMPILE_TEST
> default ARCH_SOCFPGA
> @@ -83,6 +93,7 @@ config RESET_STM32
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> default ARCH_SUNXI
> + select RESET_SIMPLE
> help
> This enables the reset driver for Allwinner SoCs.
>
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index b62783f50fe5b..ab4af99c3dfdc 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
> obj-$(CONFIG_RESET_MESON) += reset-meson.o
> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> +obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
> obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> new file mode 100644
> index 0000000000000..70c57c0758c39
> --- /dev/null
> +++ b/drivers/reset/reset-simple.c
> @@ -0,0 +1,134 @@
> +/*
> + * Simple Reset Controller Driver
> + *
> + * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
> + *
> + * Based on Allwinner SoCs Reset Controller driver
> + *
> + * Copyright 2013 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +
> +#include "reset-simple.h"
> +
> +static inline struct reset_simple_data *
> +to_reset_simple_data(struct reset_controller_dev *rcdev)
> +{
> + return container_of(rcdev, struct reset_simple_data, rcdev);
> +}
> +
> +static int reset_simple_set(struct reset_controller_dev *rcdev,
> + unsigned long id, bool assert)
> +{
> + struct reset_simple_data *data = to_reset_simple_data(rcdev);
> + int reg_width = sizeof(u32);
> + int bank = id / (reg_width * BITS_PER_BYTE);
> + int offset = id % (reg_width * BITS_PER_BYTE);
> + unsigned long flags;
> + u32 reg;
> +
> + spin_lock_irqsave(&data->lock, flags);
> +
> + reg = readl(data->membase + (bank * reg_width));
> + if (assert ^ data->active_low)
> + reg |= BIT(offset);
> + else
> + reg &= ~BIT(offset);
> + writel(reg, data->membase + (bank * reg_width));
> +
> + spin_unlock_irqrestore(&data->lock, flags);
> +
> + return 0;
> +}
> +
> +static int reset_simple_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return reset_simple_set(rcdev, id, true);
> +}
> +
> +static int reset_simple_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return reset_simple_set(rcdev, id, false);
> +}
> +
> +const struct reset_control_ops reset_simple_ops = {
> + .assert = reset_simple_assert,
> + .deassert = reset_simple_deassert,
> +};
> +
> +/**
> + * struct reset_simple_devdata - simple reset controller properties
> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> + * are set to assert the reset.
> + */
> +struct reset_simple_devdata {
> + bool active_low;
> +};
> +
> +static const struct reset_simple_devdata reset_simple_active_low = {
> + .active_low = true,
> +};
> +
> +static const struct of_device_id reset_simple_dt_ids[] = {
> + { .compatible = "allwinner,sun6i-a31-clock-reset",
> + .data = &reset_simple_active_low },
> + { /* sentinel */ },
> +};
> +
> +static int reset_simple_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + const struct reset_simple_devdata *devdata;
> + struct reset_simple_data *data;
> + void __iomem *membase;
> + struct resource *res;
> +
> + devdata = of_device_get_match_data(dev);
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + membase = devm_ioremap_resource(dev, res);
> + if (IS_ERR(membase))
> + return PTR_ERR(membase);
> +
> + spin_lock_init(&data->lock);
> + data->membase = membase;
> + data->rcdev.owner = THIS_MODULE;
> + data->rcdev.nr_resets = resource_size(res) * BITS_PER_BYTE;
> + data->rcdev.ops = &reset_simple_ops;
> + data->rcdev.of_node = dev->of_node;
> +
> + if (devdata)
> + data->active_low = devdata->active_low;
> +
> + return devm_reset_controller_register(dev, &data->rcdev);
> +}
> +
> +static struct platform_driver reset_simple_driver = {
> + .probe = reset_simple_probe,
> + .driver = {
> + .name = "simple-reset",
> + .of_match_table = reset_simple_dt_ids,
> + },
> +};
> +builtin_platform_driver(reset_simple_driver);
> diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
> new file mode 100644
> index 0000000000000..39af2014b5f12
> --- /dev/null
> +++ b/drivers/reset/reset-simple.h
> @@ -0,0 +1,41 @@
> +/*
> + * Simple Reset Controller ops
> + *
> + * Based on Allwinner SoCs Reset Controller driver
> + *
> + * Copyright 2013 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __RESET_SIMPLE_H__
> +#define __RESET_SIMPLE_H__
> +
> +#include <linux/io.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +
> +/**
> + * struct reset_simple_data - driver data for simple reset controllers
> + * @lock: spinlock to protect registers during read-modify-write cycles
> + * @membase: memory mapped I/O register range
> + * @rcdev: reset controller device base structure
> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> + * are set to assert the reset. Note that this says nothing about
> + * the voltage level of the actual reset line.
> + */
> +struct reset_simple_data {
> + spinlock_t lock;
> + void __iomem *membase;
> + struct reset_controller_dev rcdev;
> + bool active_low;
> +};
> +
> +extern const struct reset_control_ops reset_simple_ops;
> +
> +#endif /* __RESET_SIMPLE_H__ */
> diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
> index 2c7dd1fd08df6..db9a1a75523f4 100644
> --- a/drivers/reset/reset-sunxi.c
> +++ b/drivers/reset/reset-sunxi.c
> @@ -22,64 +22,11 @@
> #include <linux/spinlock.h>
> #include <linux/types.h>
>
> -struct sunxi_reset_data {
> - spinlock_t lock;
> - void __iomem *membase;
> - struct reset_controller_dev rcdev;
> -};
> -
> -static int sunxi_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct sunxi_reset_data *data = container_of(rcdev,
> - struct sunxi_reset_data,
> - rcdev);
> - int reg_width = sizeof(u32);
> - int bank = id / (reg_width * BITS_PER_BYTE);
> - int offset = id % (reg_width * BITS_PER_BYTE);
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * reg_width));
> - writel(reg & ~BIT(offset), data->membase + (bank * reg_width));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static int sunxi_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct sunxi_reset_data *data = container_of(rcdev,
> - struct sunxi_reset_data,
> - rcdev);
> - int reg_width = sizeof(u32);
> - int bank = id / (reg_width * BITS_PER_BYTE);
> - int offset = id % (reg_width * BITS_PER_BYTE);
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * reg_width));
> - writel(reg | BIT(offset), data->membase + (bank * reg_width));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static const struct reset_control_ops sunxi_reset_ops = {
> - .assert = sunxi_reset_assert,
> - .deassert = sunxi_reset_deassert,
> -};
> +#include "reset-simple.h"
>
> static int sunxi_reset_init(struct device_node *np)
> {
> - struct sunxi_reset_data *data;
> + struct reset_simple_data *data;
> struct resource res;
> resource_size_t size;
> int ret;
> @@ -108,8 +55,9 @@ static int sunxi_reset_init(struct device_node *np)
>
> data->rcdev.owner = THIS_MODULE;
> data->rcdev.nr_resets = size * 8;
> - data->rcdev.ops = &sunxi_reset_ops;
> + data->rcdev.ops = &reset_simple_ops;
> data->rcdev.of_node = np;
> + data->active_low = true;
>
> return reset_controller_register(&data->rcdev);
>
> @@ -122,6 +70,8 @@ static int sunxi_reset_init(struct device_node *np)
> * These are the reset controller we need to initialize early on in
> * our system, before we can even think of using a regular device
> * driver for it.
> + * The controllers that we can register through the regular device
> + * model are handled by the simple reset driver directly.
> */
> static const struct of_device_id sunxi_early_reset_dt_ids[] __initconst = {
> { .compatible = "allwinner,sun6i-a31-ahb1-reset", },
> @@ -135,45 +85,3 @@ void __init sun6i_reset_init(void)
> for_each_matching_node(np, sunxi_early_reset_dt_ids)
> sunxi_reset_init(np);
> }
> -
> -/*
> - * And these are the controllers we can register through the regular
> - * device model.
> - */
> -static const struct of_device_id sunxi_reset_dt_ids[] = {
> - { .compatible = "allwinner,sun6i-a31-clock-reset", },
> - { /* sentinel */ },
> -};
> -
> -static int sunxi_reset_probe(struct platform_device *pdev)
> -{
> - struct sunxi_reset_data *data;
> - struct resource *res;
> -
> - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->membase = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(data->membase))
> - return PTR_ERR(data->membase);
> -
> - spin_lock_init(&data->lock);
> -
> - data->rcdev.owner = THIS_MODULE;
> - data->rcdev.nr_resets = resource_size(res) * 8;
> - data->rcdev.ops = &sunxi_reset_ops;
> - data->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> -}
> -
> -static struct platform_driver sunxi_reset_driver = {
> - .probe = sunxi_reset_probe,
> - .driver = {
> - .name = "sunxi-reset",
> - .of_match_table = sunxi_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(sunxi_reset_driver);
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
@ 2017-08-16 20:46 ` Alexandru Gagniuc
0 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 20:46 UTC (permalink / raw)
To: linux-arm-kernel
On 08/16/2017 02:46 AM, Philipp Zabel wrote:
> Split reusable parts out of the sunxi driver, to add a driver for simple
> reset controllers with reset lines that can be controlled by toggling
> bits in exclusive, contiguous register ranges using read-modify-write
> cycles under a spinlock. The separate sunxi driver still remains to
> register the early reset controllers, but it reuses the reset-simple
> ops.
>
> The following patches will replace compatible reset drivers with
> reset-simple, extending it where necessary.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> ---
> Changes since v2:
> - Add kerneldoc comment for struct reset_simple_devdata and struct
> reset_simple_data.
> - Rename "inverted" properties to "active_low".
> - Use of_device_get_match_data instead of of_match_device.
> ---
> drivers/reset/Kconfig | 11 ++++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-simple.c | 134 +++++++++++++++++++++++++++++++++++++++++++
> drivers/reset/reset-simple.h | 41 +++++++++++++
> drivers/reset/reset-sunxi.c | 104 ++-------------------------------
> 5 files changed, 193 insertions(+), 98 deletions(-)
> create mode 100644 drivers/reset/reset-simple.c
> create mode 100644 drivers/reset/reset-simple.h
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 52d5251660b9b..f7ba01a71daee 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -68,6 +68,16 @@ config RESET_PISTACHIO
> help
> This enables the reset driver for ImgTec Pistachio SoCs.
>
> +config RESET_SIMPLE
> + bool "Simple Reset Controller Driver" if COMPILE_TEST
> + default ARCH_SUNXI
> + help
> + This enables a simple reset controller driver for reset lines that
> + that can be asserted and deasserted by toggling bits in a contiguous,
> + exclusive register space.
> +
> + Currently this driver supports Allwinner SoCs.
> +
> config RESET_SOCFPGA
> bool "SoCFPGA Reset Driver" if COMPILE_TEST
> default ARCH_SOCFPGA
> @@ -83,6 +93,7 @@ config RESET_STM32
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> default ARCH_SUNXI
> + select RESET_SIMPLE
> help
> This enables the reset driver for Allwinner SoCs.
>
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index b62783f50fe5b..ab4af99c3dfdc 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
> obj-$(CONFIG_RESET_MESON) += reset-meson.o
> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> +obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
> obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> new file mode 100644
> index 0000000000000..70c57c0758c39
> --- /dev/null
> +++ b/drivers/reset/reset-simple.c
> @@ -0,0 +1,134 @@
> +/*
> + * Simple Reset Controller Driver
> + *
> + * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
> + *
> + * Based on Allwinner SoCs Reset Controller driver
> + *
> + * Copyright 2013 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +
> +#include "reset-simple.h"
> +
> +static inline struct reset_simple_data *
> +to_reset_simple_data(struct reset_controller_dev *rcdev)
> +{
> + return container_of(rcdev, struct reset_simple_data, rcdev);
> +}
> +
> +static int reset_simple_set(struct reset_controller_dev *rcdev,
> + unsigned long id, bool assert)
> +{
> + struct reset_simple_data *data = to_reset_simple_data(rcdev);
> + int reg_width = sizeof(u32);
> + int bank = id / (reg_width * BITS_PER_BYTE);
> + int offset = id % (reg_width * BITS_PER_BYTE);
> + unsigned long flags;
> + u32 reg;
> +
> + spin_lock_irqsave(&data->lock, flags);
> +
> + reg = readl(data->membase + (bank * reg_width));
> + if (assert ^ data->active_low)
> + reg |= BIT(offset);
> + else
> + reg &= ~BIT(offset);
> + writel(reg, data->membase + (bank * reg_width));
> +
> + spin_unlock_irqrestore(&data->lock, flags);
> +
> + return 0;
> +}
> +
> +static int reset_simple_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return reset_simple_set(rcdev, id, true);
> +}
> +
> +static int reset_simple_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return reset_simple_set(rcdev, id, false);
> +}
> +
> +const struct reset_control_ops reset_simple_ops = {
> + .assert = reset_simple_assert,
> + .deassert = reset_simple_deassert,
> +};
> +
> +/**
> + * struct reset_simple_devdata - simple reset controller properties
> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> + * are set to assert the reset.
> + */
> +struct reset_simple_devdata {
> + bool active_low;
> +};
> +
> +static const struct reset_simple_devdata reset_simple_active_low = {
> + .active_low = true,
> +};
> +
> +static const struct of_device_id reset_simple_dt_ids[] = {
> + { .compatible = "allwinner,sun6i-a31-clock-reset",
> + .data = &reset_simple_active_low },
> + { /* sentinel */ },
> +};
> +
> +static int reset_simple_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + const struct reset_simple_devdata *devdata;
> + struct reset_simple_data *data;
> + void __iomem *membase;
> + struct resource *res;
> +
> + devdata = of_device_get_match_data(dev);
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + membase = devm_ioremap_resource(dev, res);
> + if (IS_ERR(membase))
> + return PTR_ERR(membase);
> +
> + spin_lock_init(&data->lock);
> + data->membase = membase;
> + data->rcdev.owner = THIS_MODULE;
> + data->rcdev.nr_resets = resource_size(res) * BITS_PER_BYTE;
> + data->rcdev.ops = &reset_simple_ops;
> + data->rcdev.of_node = dev->of_node;
> +
> + if (devdata)
> + data->active_low = devdata->active_low;
> +
> + return devm_reset_controller_register(dev, &data->rcdev);
> +}
> +
> +static struct platform_driver reset_simple_driver = {
> + .probe = reset_simple_probe,
> + .driver = {
> + .name = "simple-reset",
> + .of_match_table = reset_simple_dt_ids,
> + },
> +};
> +builtin_platform_driver(reset_simple_driver);
> diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
> new file mode 100644
> index 0000000000000..39af2014b5f12
> --- /dev/null
> +++ b/drivers/reset/reset-simple.h
> @@ -0,0 +1,41 @@
> +/*
> + * Simple Reset Controller ops
> + *
> + * Based on Allwinner SoCs Reset Controller driver
> + *
> + * Copyright 2013 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __RESET_SIMPLE_H__
> +#define __RESET_SIMPLE_H__
> +
> +#include <linux/io.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +
> +/**
> + * struct reset_simple_data - driver data for simple reset controllers
> + * @lock: spinlock to protect registers during read-modify-write cycles
> + * @membase: memory mapped I/O register range
> + * @rcdev: reset controller device base structure
> + * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> + * are set to assert the reset. Note that this says nothing about
> + * the voltage level of the actual reset line.
> + */
> +struct reset_simple_data {
> + spinlock_t lock;
> + void __iomem *membase;
> + struct reset_controller_dev rcdev;
> + bool active_low;
> +};
> +
> +extern const struct reset_control_ops reset_simple_ops;
> +
> +#endif /* __RESET_SIMPLE_H__ */
> diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
> index 2c7dd1fd08df6..db9a1a75523f4 100644
> --- a/drivers/reset/reset-sunxi.c
> +++ b/drivers/reset/reset-sunxi.c
> @@ -22,64 +22,11 @@
> #include <linux/spinlock.h>
> #include <linux/types.h>
>
> -struct sunxi_reset_data {
> - spinlock_t lock;
> - void __iomem *membase;
> - struct reset_controller_dev rcdev;
> -};
> -
> -static int sunxi_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct sunxi_reset_data *data = container_of(rcdev,
> - struct sunxi_reset_data,
> - rcdev);
> - int reg_width = sizeof(u32);
> - int bank = id / (reg_width * BITS_PER_BYTE);
> - int offset = id % (reg_width * BITS_PER_BYTE);
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * reg_width));
> - writel(reg & ~BIT(offset), data->membase + (bank * reg_width));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static int sunxi_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct sunxi_reset_data *data = container_of(rcdev,
> - struct sunxi_reset_data,
> - rcdev);
> - int reg_width = sizeof(u32);
> - int bank = id / (reg_width * BITS_PER_BYTE);
> - int offset = id % (reg_width * BITS_PER_BYTE);
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * reg_width));
> - writel(reg | BIT(offset), data->membase + (bank * reg_width));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static const struct reset_control_ops sunxi_reset_ops = {
> - .assert = sunxi_reset_assert,
> - .deassert = sunxi_reset_deassert,
> -};
> +#include "reset-simple.h"
>
> static int sunxi_reset_init(struct device_node *np)
> {
> - struct sunxi_reset_data *data;
> + struct reset_simple_data *data;
> struct resource res;
> resource_size_t size;
> int ret;
> @@ -108,8 +55,9 @@ static int sunxi_reset_init(struct device_node *np)
>
> data->rcdev.owner = THIS_MODULE;
> data->rcdev.nr_resets = size * 8;
> - data->rcdev.ops = &sunxi_reset_ops;
> + data->rcdev.ops = &reset_simple_ops;
> data->rcdev.of_node = np;
> + data->active_low = true;
>
> return reset_controller_register(&data->rcdev);
>
> @@ -122,6 +70,8 @@ static int sunxi_reset_init(struct device_node *np)
> * These are the reset controller we need to initialize early on in
> * our system, before we can even think of using a regular device
> * driver for it.
> + * The controllers that we can register through the regular device
> + * model are handled by the simple reset driver directly.
> */
> static const struct of_device_id sunxi_early_reset_dt_ids[] __initconst = {
> { .compatible = "allwinner,sun6i-a31-ahb1-reset", },
> @@ -135,45 +85,3 @@ void __init sun6i_reset_init(void)
> for_each_matching_node(np, sunxi_early_reset_dt_ids)
> sunxi_reset_init(np);
> }
> -
> -/*
> - * And these are the controllers we can register through the regular
> - * device model.
> - */
> -static const struct of_device_id sunxi_reset_dt_ids[] = {
> - { .compatible = "allwinner,sun6i-a31-clock-reset", },
> - { /* sentinel */ },
> -};
> -
> -static int sunxi_reset_probe(struct platform_device *pdev)
> -{
> - struct sunxi_reset_data *data;
> - struct resource *res;
> -
> - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->membase = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(data->membase))
> - return PTR_ERR(data->membase);
> -
> - spin_lock_init(&data->lock);
> -
> - data->rcdev.owner = THIS_MODULE;
> - data->rcdev.nr_resets = resource_size(res) * 8;
> - data->rcdev.ops = &sunxi_reset_ops;
> - data->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> -}
> -
> -static struct platform_driver sunxi_reset_driver = {
> - .probe = sunxi_reset_probe,
> - .driver = {
> - .name = "sunxi-reset",
> - .of_match_table = sunxi_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(sunxi_reset_driver);
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 3/5] reset: stm32: use the reset-simple driver
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-16 20:50 ` Alexandru Gagniuc
-1 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 20:50 UTC (permalink / raw)
To: Philipp Zabel, linux-kernel
Cc: Andre Przywara, Maxime Coquelin, Alexandre Torgue, Maxime Ripard,
Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev, Steffen Trumtrar,
Dinh Nguyen, Andreas Färber, linux-arm-kernel, kernel
On 08/16/2017 02:46 AM, Philipp Zabel wrote:
> The reset-simple driver can be used without changes.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> ---
> drivers/reset/Kconfig | 11 ++---
> drivers/reset/Makefile | 1 -
> drivers/reset/reset-simple.c | 1 +
> drivers/reset/reset-stm32.c | 108 -------------------------------------------
> 4 files changed, 4 insertions(+), 117 deletions(-)
> delete mode 100644 drivers/reset/reset-stm32.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 78a8f6057985b..29f4487c290fc 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -70,19 +70,14 @@ config RESET_PISTACHIO
>
> config RESET_SIMPLE
> bool "Simple Reset Controller Driver" if COMPILE_TEST
> - default ARCH_SOCFPGA || ARCH_SUNXI
> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
If this series gets respun, could you please look into removing the
ARCH_ dependency here?
Alex
> help
> This enables a simple reset controller driver for reset lines that
> that can be asserted and deasserted by toggling bits in a contiguous,
> exclusive register space.
>
> - Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
> -
> -config RESET_STM32
> - bool "STM32 Reset Driver" if COMPILE_TEST
> - default ARCH_STM32
> - help
> - This enables the RCC reset controller driver for STM32 MCUs.
> + Currently this driver supports Altera SoCFPGAs, the RCC reset
> + controller in STM32 MCUs, and Allwinner SoCs.
>
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 25f5f722dec01..e8c3a032f4780 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -12,7 +12,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> -obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index 84b08ce4def01..e54a29b8b532d 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -120,6 +120,7 @@ static const struct reset_simple_devdata reset_simple_active_low = {
>
> static const struct of_device_id reset_simple_dt_ids[] = {
> { .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
> + { .compatible = "st,stm32-rcc", },
> { .compatible = "allwinner,sun6i-a31-clock-reset",
> .data = &reset_simple_active_low },
> { /* sentinel */ },
> diff --git a/drivers/reset/reset-stm32.c b/drivers/reset/reset-stm32.c
> deleted file mode 100644
> index 3a7c8527e66af..0000000000000
> --- a/drivers/reset/reset-stm32.c
> +++ /dev/null
> @@ -1,108 +0,0 @@
> -/*
> - * Copyright (C) Maxime Coquelin 2015
> - * Author: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> - * License terms: GNU General Public License (GPL), version 2
> - *
> - * Heavily based on sunxi driver from Maxime Ripard.
> - */
> -
> -#include <linux/err.h>
> -#include <linux/io.h>
> -#include <linux/of.h>
> -#include <linux/of_address.h>
> -#include <linux/platform_device.h>
> -#include <linux/reset-controller.h>
> -#include <linux/slab.h>
> -#include <linux/spinlock.h>
> -#include <linux/types.h>
> -
> -struct stm32_reset_data {
> - spinlock_t lock;
> - void __iomem *membase;
> - struct reset_controller_dev rcdev;
> -};
> -
> -static int stm32_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct stm32_reset_data *data = container_of(rcdev,
> - struct stm32_reset_data,
> - rcdev);
> - int bank = id / BITS_PER_LONG;
> - int offset = id % BITS_PER_LONG;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * 4));
> - writel(reg | BIT(offset), data->membase + (bank * 4));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct stm32_reset_data *data = container_of(rcdev,
> - struct stm32_reset_data,
> - rcdev);
> - int bank = id / BITS_PER_LONG;
> - int offset = id % BITS_PER_LONG;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * 4));
> - writel(reg & ~BIT(offset), data->membase + (bank * 4));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static const struct reset_control_ops stm32_reset_ops = {
> - .assert = stm32_reset_assert,
> - .deassert = stm32_reset_deassert,
> -};
> -
> -static const struct of_device_id stm32_reset_dt_ids[] = {
> - { .compatible = "st,stm32-rcc", },
> - { /* sentinel */ },
> -};
> -
> -static int stm32_reset_probe(struct platform_device *pdev)
> -{
> - struct stm32_reset_data *data;
> - struct resource *res;
> -
> - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->membase = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(data->membase))
> - return PTR_ERR(data->membase);
> -
> - spin_lock_init(&data->lock);
> -
> - data->rcdev.owner = THIS_MODULE;
> - data->rcdev.nr_resets = resource_size(res) * 8;
> - data->rcdev.ops = &stm32_reset_ops;
> - data->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> -}
> -
> -static struct platform_driver stm32_reset_driver = {
> - .probe = stm32_reset_probe,
> - .driver = {
> - .name = "stm32-rcc-reset",
> - .of_match_table = stm32_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(stm32_reset_driver);
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 3/5] reset: stm32: use the reset-simple driver
@ 2017-08-16 20:50 ` Alexandru Gagniuc
0 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 20:50 UTC (permalink / raw)
To: linux-arm-kernel
On 08/16/2017 02:46 AM, Philipp Zabel wrote:
> The reset-simple driver can be used without changes.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> ---
> drivers/reset/Kconfig | 11 ++---
> drivers/reset/Makefile | 1 -
> drivers/reset/reset-simple.c | 1 +
> drivers/reset/reset-stm32.c | 108 -------------------------------------------
> 4 files changed, 4 insertions(+), 117 deletions(-)
> delete mode 100644 drivers/reset/reset-stm32.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 78a8f6057985b..29f4487c290fc 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -70,19 +70,14 @@ config RESET_PISTACHIO
>
> config RESET_SIMPLE
> bool "Simple Reset Controller Driver" if COMPILE_TEST
> - default ARCH_SOCFPGA || ARCH_SUNXI
> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
If this series gets respun, could you please look into removing the
ARCH_ dependency here?
Alex
> help
> This enables a simple reset controller driver for reset lines that
> that can be asserted and deasserted by toggling bits in a contiguous,
> exclusive register space.
>
> - Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
> -
> -config RESET_STM32
> - bool "STM32 Reset Driver" if COMPILE_TEST
> - default ARCH_STM32
> - help
> - This enables the RCC reset controller driver for STM32 MCUs.
> + Currently this driver supports Altera SoCFPGAs, the RCC reset
> + controller in STM32 MCUs, and Allwinner SoCs.
>
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 25f5f722dec01..e8c3a032f4780 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -12,7 +12,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> -obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index 84b08ce4def01..e54a29b8b532d 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -120,6 +120,7 @@ static const struct reset_simple_devdata reset_simple_active_low = {
>
> static const struct of_device_id reset_simple_dt_ids[] = {
> { .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
> + { .compatible = "st,stm32-rcc", },
> { .compatible = "allwinner,sun6i-a31-clock-reset",
> .data = &reset_simple_active_low },
> { /* sentinel */ },
> diff --git a/drivers/reset/reset-stm32.c b/drivers/reset/reset-stm32.c
> deleted file mode 100644
> index 3a7c8527e66af..0000000000000
> --- a/drivers/reset/reset-stm32.c
> +++ /dev/null
> @@ -1,108 +0,0 @@
> -/*
> - * Copyright (C) Maxime Coquelin 2015
> - * Author: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> - * License terms: GNU General Public License (GPL), version 2
> - *
> - * Heavily based on sunxi driver from Maxime Ripard.
> - */
> -
> -#include <linux/err.h>
> -#include <linux/io.h>
> -#include <linux/of.h>
> -#include <linux/of_address.h>
> -#include <linux/platform_device.h>
> -#include <linux/reset-controller.h>
> -#include <linux/slab.h>
> -#include <linux/spinlock.h>
> -#include <linux/types.h>
> -
> -struct stm32_reset_data {
> - spinlock_t lock;
> - void __iomem *membase;
> - struct reset_controller_dev rcdev;
> -};
> -
> -static int stm32_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct stm32_reset_data *data = container_of(rcdev,
> - struct stm32_reset_data,
> - rcdev);
> - int bank = id / BITS_PER_LONG;
> - int offset = id % BITS_PER_LONG;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * 4));
> - writel(reg | BIT(offset), data->membase + (bank * 4));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct stm32_reset_data *data = container_of(rcdev,
> - struct stm32_reset_data,
> - rcdev);
> - int bank = id / BITS_PER_LONG;
> - int offset = id % BITS_PER_LONG;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * 4));
> - writel(reg & ~BIT(offset), data->membase + (bank * 4));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static const struct reset_control_ops stm32_reset_ops = {
> - .assert = stm32_reset_assert,
> - .deassert = stm32_reset_deassert,
> -};
> -
> -static const struct of_device_id stm32_reset_dt_ids[] = {
> - { .compatible = "st,stm32-rcc", },
> - { /* sentinel */ },
> -};
> -
> -static int stm32_reset_probe(struct platform_device *pdev)
> -{
> - struct stm32_reset_data *data;
> - struct resource *res;
> -
> - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->membase = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(data->membase))
> - return PTR_ERR(data->membase);
> -
> - spin_lock_init(&data->lock);
> -
> - data->rcdev.owner = THIS_MODULE;
> - data->rcdev.nr_resets = resource_size(res) * 8;
> - data->rcdev.ops = &stm32_reset_ops;
> - data->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> -}
> -
> -static struct platform_driver stm32_reset_driver = {
> - .probe = stm32_reset_probe,
> - .driver = {
> - .name = "stm32-rcc-reset",
> - .of_match_table = stm32_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(stm32_reset_driver);
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 4/5] reset: zx2967: use the reset-simple driver
2017-08-16 9:47 ` Philipp Zabel
@ 2017-08-16 20:50 ` Alexandru Gagniuc
-1 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 20:50 UTC (permalink / raw)
To: Philipp Zabel, linux-kernel
Cc: Andre Przywara, Maxime Coquelin, Alexandre Torgue, Maxime Ripard,
Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev, Steffen Trumtrar,
Dinh Nguyen, Andreas Färber, linux-arm-kernel, kernel
On 08/16/2017 02:47 AM, Philipp Zabel wrote:
> The reset-simple driver can be used without changes.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> ---
> MAINTAINERS | 1 -
> drivers/reset/Kconfig | 10 +----
> drivers/reset/Makefile | 1 -
> drivers/reset/reset-simple.c | 2 +
> drivers/reset/reset-zx2967.c | 99 --------------------------------------------
> 5 files changed, 4 insertions(+), 109 deletions(-)
> delete mode 100644 drivers/reset/reset-zx2967.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 57853844969bf..30af04516d8a0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2061,7 +2061,6 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> S: Maintained
> F: arch/arm/mach-zx/
> F: drivers/clk/zte/
> -F: drivers/reset/reset-zx2967.c
> F: drivers/soc/zte/
> F: Documentation/devicetree/bindings/arm/zte.txt
> F: Documentation/devicetree/bindings/clock/zx296702-clk.txt
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 29f4487c290fc..6dfea01023618 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -70,14 +70,14 @@ config RESET_PISTACHIO
>
> config RESET_SIMPLE
> bool "Simple Reset Controller Driver" if COMPILE_TEST
> - default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI || ARCH_ZX
> help
> This enables a simple reset controller driver for reset lines that
> that can be asserted and deasserted by toggling bits in a contiguous,
> exclusive register space.
>
> Currently this driver supports Altera SoCFPGAs, the RCC reset
> - controller in STM32 MCUs, and Allwinner SoCs.
> + controller in STM32 MCUs, Allwinner SoCs, and ZTE's zx2967 family.
>
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> @@ -114,12 +114,6 @@ config RESET_UNIPHIER
> Say Y if you want to control reset signals provided by System Control
> block, Media I/O block, Peripheral Block.
>
> -config RESET_ZX2967
> - bool "ZTE ZX2967 Reset Driver"
> - depends on ARCH_ZX || COMPILE_TEST
> - help
> - This enables the reset controller driver for ZTE's zx2967 family.
> -
> config RESET_ZYNQ
> bool "ZYNQ Reset Driver" if COMPILE_TEST
> default ARCH_ZYNQ
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index e8c3a032f4780..3a70c254b5ea5 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -16,6 +16,5 @@ obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
> obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> -obj-$(CONFIG_RESET_ZX2967) += reset-zx2967.o
> obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
>
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index e54a29b8b532d..13e7d5559acc9 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -123,6 +123,8 @@ static const struct of_device_id reset_simple_dt_ids[] = {
> { .compatible = "st,stm32-rcc", },
> { .compatible = "allwinner,sun6i-a31-clock-reset",
> .data = &reset_simple_active_low },
> + { .compatible = "zte,zx296718-reset",
> + .data = &reset_simple_active_low },
> { /* sentinel */ },
> };
>
> diff --git a/drivers/reset/reset-zx2967.c b/drivers/reset/reset-zx2967.c
> deleted file mode 100644
> index 4f319f7753d4d..0000000000000
> --- a/drivers/reset/reset-zx2967.c
> +++ /dev/null
> @@ -1,99 +0,0 @@
> -/*
> - * ZTE's zx2967 family reset controller driver
> - *
> - * Copyright (C) 2017 ZTE Ltd.
> - *
> - * Author: Baoyou Xie <baoyou.xie@linaro.org>
> - *
> - * License terms: GNU General Public License (GPL) version 2
> - */
> -
> -#include <linux/of_address.h>
> -#include <linux/platform_device.h>
> -#include <linux/reset-controller.h>
> -
> -struct zx2967_reset {
> - void __iomem *reg_base;
> - spinlock_t lock;
> - struct reset_controller_dev rcdev;
> -};
> -
> -static int zx2967_reset_act(struct reset_controller_dev *rcdev,
> - unsigned long id, bool assert)
> -{
> - struct zx2967_reset *reset = NULL;
> - int bank = id / 32;
> - int offset = id % 32;
> - u32 reg;
> - unsigned long flags;
> -
> - reset = container_of(rcdev, struct zx2967_reset, rcdev);
> -
> - spin_lock_irqsave(&reset->lock, flags);
> -
> - reg = readl_relaxed(reset->reg_base + (bank * 4));
> - if (assert)
> - reg &= ~BIT(offset);
> - else
> - reg |= BIT(offset);
> - writel_relaxed(reg, reset->reg_base + (bank * 4));
> -
> - spin_unlock_irqrestore(&reset->lock, flags);
> -
> - return 0;
> -}
> -
> -static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - return zx2967_reset_act(rcdev, id, true);
> -}
> -
> -static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - return zx2967_reset_act(rcdev, id, false);
> -}
> -
> -static const struct reset_control_ops zx2967_reset_ops = {
> - .assert = zx2967_reset_assert,
> - .deassert = zx2967_reset_deassert,
> -};
> -
> -static int zx2967_reset_probe(struct platform_device *pdev)
> -{
> - struct zx2967_reset *reset;
> - struct resource *res;
> -
> - reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
> - if (!reset)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - reset->reg_base = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(reset->reg_base))
> - return PTR_ERR(reset->reg_base);
> -
> - spin_lock_init(&reset->lock);
> -
> - reset->rcdev.owner = THIS_MODULE;
> - reset->rcdev.nr_resets = resource_size(res) * 8;
> - reset->rcdev.ops = &zx2967_reset_ops;
> - reset->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
> -}
> -
> -static const struct of_device_id zx2967_reset_dt_ids[] = {
> - { .compatible = "zte,zx296718-reset", },
> - {},
> -};
> -
> -static struct platform_driver zx2967_reset_driver = {
> - .probe = zx2967_reset_probe,
> - .driver = {
> - .name = "zx2967-reset",
> - .of_match_table = zx2967_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(zx2967_reset_driver);
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 4/5] reset: zx2967: use the reset-simple driver
@ 2017-08-16 20:50 ` Alexandru Gagniuc
0 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 20:50 UTC (permalink / raw)
To: linux-arm-kernel
On 08/16/2017 02:47 AM, Philipp Zabel wrote:
> The reset-simple driver can be used without changes.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> ---
> MAINTAINERS | 1 -
> drivers/reset/Kconfig | 10 +----
> drivers/reset/Makefile | 1 -
> drivers/reset/reset-simple.c | 2 +
> drivers/reset/reset-zx2967.c | 99 --------------------------------------------
> 5 files changed, 4 insertions(+), 109 deletions(-)
> delete mode 100644 drivers/reset/reset-zx2967.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 57853844969bf..30af04516d8a0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2061,7 +2061,6 @@ L: linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
> S: Maintained
> F: arch/arm/mach-zx/
> F: drivers/clk/zte/
> -F: drivers/reset/reset-zx2967.c
> F: drivers/soc/zte/
> F: Documentation/devicetree/bindings/arm/zte.txt
> F: Documentation/devicetree/bindings/clock/zx296702-clk.txt
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 29f4487c290fc..6dfea01023618 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -70,14 +70,14 @@ config RESET_PISTACHIO
>
> config RESET_SIMPLE
> bool "Simple Reset Controller Driver" if COMPILE_TEST
> - default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI || ARCH_ZX
> help
> This enables a simple reset controller driver for reset lines that
> that can be asserted and deasserted by toggling bits in a contiguous,
> exclusive register space.
>
> Currently this driver supports Altera SoCFPGAs, the RCC reset
> - controller in STM32 MCUs, and Allwinner SoCs.
> + controller in STM32 MCUs, Allwinner SoCs, and ZTE's zx2967 family.
>
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> @@ -114,12 +114,6 @@ config RESET_UNIPHIER
> Say Y if you want to control reset signals provided by System Control
> block, Media I/O block, Peripheral Block.
>
> -config RESET_ZX2967
> - bool "ZTE ZX2967 Reset Driver"
> - depends on ARCH_ZX || COMPILE_TEST
> - help
> - This enables the reset controller driver for ZTE's zx2967 family.
> -
> config RESET_ZYNQ
> bool "ZYNQ Reset Driver" if COMPILE_TEST
> default ARCH_ZYNQ
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index e8c3a032f4780..3a70c254b5ea5 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -16,6 +16,5 @@ obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
> obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> -obj-$(CONFIG_RESET_ZX2967) += reset-zx2967.o
> obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
>
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index e54a29b8b532d..13e7d5559acc9 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -123,6 +123,8 @@ static const struct of_device_id reset_simple_dt_ids[] = {
> { .compatible = "st,stm32-rcc", },
> { .compatible = "allwinner,sun6i-a31-clock-reset",
> .data = &reset_simple_active_low },
> + { .compatible = "zte,zx296718-reset",
> + .data = &reset_simple_active_low },
> { /* sentinel */ },
> };
>
> diff --git a/drivers/reset/reset-zx2967.c b/drivers/reset/reset-zx2967.c
> deleted file mode 100644
> index 4f319f7753d4d..0000000000000
> --- a/drivers/reset/reset-zx2967.c
> +++ /dev/null
> @@ -1,99 +0,0 @@
> -/*
> - * ZTE's zx2967 family reset controller driver
> - *
> - * Copyright (C) 2017 ZTE Ltd.
> - *
> - * Author: Baoyou Xie <baoyou.xie@linaro.org>
> - *
> - * License terms: GNU General Public License (GPL) version 2
> - */
> -
> -#include <linux/of_address.h>
> -#include <linux/platform_device.h>
> -#include <linux/reset-controller.h>
> -
> -struct zx2967_reset {
> - void __iomem *reg_base;
> - spinlock_t lock;
> - struct reset_controller_dev rcdev;
> -};
> -
> -static int zx2967_reset_act(struct reset_controller_dev *rcdev,
> - unsigned long id, bool assert)
> -{
> - struct zx2967_reset *reset = NULL;
> - int bank = id / 32;
> - int offset = id % 32;
> - u32 reg;
> - unsigned long flags;
> -
> - reset = container_of(rcdev, struct zx2967_reset, rcdev);
> -
> - spin_lock_irqsave(&reset->lock, flags);
> -
> - reg = readl_relaxed(reset->reg_base + (bank * 4));
> - if (assert)
> - reg &= ~BIT(offset);
> - else
> - reg |= BIT(offset);
> - writel_relaxed(reg, reset->reg_base + (bank * 4));
> -
> - spin_unlock_irqrestore(&reset->lock, flags);
> -
> - return 0;
> -}
> -
> -static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - return zx2967_reset_act(rcdev, id, true);
> -}
> -
> -static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - return zx2967_reset_act(rcdev, id, false);
> -}
> -
> -static const struct reset_control_ops zx2967_reset_ops = {
> - .assert = zx2967_reset_assert,
> - .deassert = zx2967_reset_deassert,
> -};
> -
> -static int zx2967_reset_probe(struct platform_device *pdev)
> -{
> - struct zx2967_reset *reset;
> - struct resource *res;
> -
> - reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
> - if (!reset)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - reset->reg_base = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(reset->reg_base))
> - return PTR_ERR(reset->reg_base);
> -
> - spin_lock_init(&reset->lock);
> -
> - reset->rcdev.owner = THIS_MODULE;
> - reset->rcdev.nr_resets = resource_size(res) * 8;
> - reset->rcdev.ops = &zx2967_reset_ops;
> - reset->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
> -}
> -
> -static const struct of_device_id zx2967_reset_dt_ids[] = {
> - { .compatible = "zte,zx296718-reset", },
> - {},
> -};
> -
> -static struct platform_driver zx2967_reset_driver = {
> - .probe = zx2967_reset_probe,
> - .driver = {
> - .name = "zx2967-reset",
> - .of_match_table = zx2967_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(zx2967_reset_driver);
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 3/5] reset: stm32: use the reset-simple driver
2017-08-16 20:50 ` Alexandru Gagniuc
@ 2017-08-16 20:52 ` Andreas Färber
-1 siblings, 0 replies; 54+ messages in thread
From: Andreas Färber @ 2017-08-16 20:52 UTC (permalink / raw)
To: Alexandru Gagniuc
Cc: Philipp Zabel, linux-kernel, Andre Przywara, Maxime Coquelin,
Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen, linux-arm-kernel,
kernel
Am 16.08.2017 um 22:50 schrieb Alexandru Gagniuc:
> On 08/16/2017 02:46 AM, Philipp Zabel wrote:
>> The reset-simple driver can be used without changes.
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>
> Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
>
>> ---
>> drivers/reset/Kconfig | 11 ++---
>> drivers/reset/Makefile | 1 -
>> drivers/reset/reset-simple.c | 1 +
>> drivers/reset/reset-stm32.c | 108
>> -------------------------------------------
>> 4 files changed, 4 insertions(+), 117 deletions(-)
>> delete mode 100644 drivers/reset/reset-stm32.c
>>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 78a8f6057985b..29f4487c290fc 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -70,19 +70,14 @@ config RESET_PISTACHIO
>>
>> config RESET_SIMPLE
>> bool "Simple Reset Controller Driver" if COMPILE_TEST
>> - default ARCH_SOCFPGA || ARCH_SUNXI
>> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
>
> If this series gets respun, could you please look into removing the
> ARCH_ dependency here?
Why? It's not a dependency, just the default. Doing it here decouples it
from having to do Kconfig changes in the arm-soc tree as well.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 3/5] reset: stm32: use the reset-simple driver
@ 2017-08-16 20:52 ` Andreas Färber
0 siblings, 0 replies; 54+ messages in thread
From: Andreas Färber @ 2017-08-16 20:52 UTC (permalink / raw)
To: linux-arm-kernel
Am 16.08.2017 um 22:50 schrieb Alexandru Gagniuc:
> On 08/16/2017 02:46 AM, Philipp Zabel wrote:
>> The reset-simple driver can be used without changes.
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>
> Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
>
>> ---
>> drivers/reset/Kconfig | 11 ++---
>> drivers/reset/Makefile | 1 -
>> drivers/reset/reset-simple.c | 1 +
>> drivers/reset/reset-stm32.c | 108
>> -------------------------------------------
>> 4 files changed, 4 insertions(+), 117 deletions(-)
>> delete mode 100644 drivers/reset/reset-stm32.c
>>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 78a8f6057985b..29f4487c290fc 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -70,19 +70,14 @@ config RESET_PISTACHIO
>>
>> config RESET_SIMPLE
>> bool "Simple Reset Controller Driver" if COMPILE_TEST
>> - default ARCH_SOCFPGA || ARCH_SUNXI
>> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
>
> If this series gets respun, could you please look into removing the
> ARCH_ dependency here?
Why? It's not a dependency, just the default. Doing it here decouples it
from having to do Kconfig changes in the arm-soc tree as well.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 3/5] reset: stm32: use the reset-simple driver
2017-08-16 20:52 ` Andreas Färber
@ 2017-08-16 20:55 ` Alexandru Gagniuc
-1 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 20:55 UTC (permalink / raw)
To: Andreas Färber
Cc: Philipp Zabel, linux-kernel, Andre Przywara, Maxime Coquelin,
Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen, linux-arm-kernel,
kernel
On 08/16/2017 01:52 PM, Andreas Färber wrote:
> Am 16.08.2017 um 22:50 schrieb Alexandru Gagniuc:
>> On 08/16/2017 02:46 AM, Philipp Zabel wrote:
>>> The reset-simple driver can be used without changes.
>>>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>
>> Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
>>
>>> ---
>>> drivers/reset/Kconfig | 11 ++---
>>> drivers/reset/Makefile | 1 -
>>> drivers/reset/reset-simple.c | 1 +
>>> drivers/reset/reset-stm32.c | 108
>>> -------------------------------------------
>>> 4 files changed, 4 insertions(+), 117 deletions(-)
>>> delete mode 100644 drivers/reset/reset-stm32.c
>>>
>>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>>> index 78a8f6057985b..29f4487c290fc 100644
>>> --- a/drivers/reset/Kconfig
>>> +++ b/drivers/reset/Kconfig
>>> @@ -70,19 +70,14 @@ config RESET_PISTACHIO
>>>
>>> config RESET_SIMPLE
>>> bool "Simple Reset Controller Driver" if COMPILE_TEST
>>> - default ARCH_SOCFPGA || ARCH_SUNXI
>>> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
>>
>> If this series gets respun, could you please look into removing the
>> ARCH_ dependency here?
>
> Why?
Because the driver has to keep track of all its users, which is a
layering violation.
> It's not a dependency, just the default. Doing it here decouples it
> from having to do Kconfig changes in the arm-soc tree as well.
Doing it here allows this list to grow without bounds.
> Regards,
> Andreas
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 3/5] reset: stm32: use the reset-simple driver
@ 2017-08-16 20:55 ` Alexandru Gagniuc
0 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 20:55 UTC (permalink / raw)
To: linux-arm-kernel
On 08/16/2017 01:52 PM, Andreas F?rber wrote:
> Am 16.08.2017 um 22:50 schrieb Alexandru Gagniuc:
>> On 08/16/2017 02:46 AM, Philipp Zabel wrote:
>>> The reset-simple driver can be used without changes.
>>>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>
>> Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
>>
>>> ---
>>> drivers/reset/Kconfig | 11 ++---
>>> drivers/reset/Makefile | 1 -
>>> drivers/reset/reset-simple.c | 1 +
>>> drivers/reset/reset-stm32.c | 108
>>> -------------------------------------------
>>> 4 files changed, 4 insertions(+), 117 deletions(-)
>>> delete mode 100644 drivers/reset/reset-stm32.c
>>>
>>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>>> index 78a8f6057985b..29f4487c290fc 100644
>>> --- a/drivers/reset/Kconfig
>>> +++ b/drivers/reset/Kconfig
>>> @@ -70,19 +70,14 @@ config RESET_PISTACHIO
>>>
>>> config RESET_SIMPLE
>>> bool "Simple Reset Controller Driver" if COMPILE_TEST
>>> - default ARCH_SOCFPGA || ARCH_SUNXI
>>> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
>>
>> If this series gets respun, could you please look into removing the
>> ARCH_ dependency here?
>
> Why?
Because the driver has to keep track of all its users, which is a
layering violation.
> It's not a dependency, just the default. Doing it here decouples it
> from having to do Kconfig changes in the arm-soc tree as well.
Doing it here allows this list to grow without bounds.
> Regards,
> Andreas
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 5/5] reset: simple: read back to make sure changes are applied
2017-08-16 9:47 ` Philipp Zabel
@ 2017-08-16 21:00 ` Alexandru Gagniuc
-1 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 21:00 UTC (permalink / raw)
To: Philipp Zabel, linux-kernel
Cc: Andre Przywara, Maxime Coquelin, Alexandre Torgue, Maxime Ripard,
Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev, Steffen Trumtrar,
Dinh Nguyen, Andreas Färber, linux-arm-kernel, kernel
On 08/16/2017 02:47 AM, Philipp Zabel wrote:
> Read back the register after setting or clearing a reset bit to make
> sure that the changes are applied to the reset controller hardware.
> Theoretically, this avoids the write to stay stuck in a store buffer
Is there hardware where this has been observed to happen, or is this
purely theoretical? It would be nice to have a "this is needed on
hardware XYZ because ABC, and doesn't affect other hardware" comment in
the source.
> during the delay of an assert-delay-deassert sequence, and makes sure
> that the reset really is asserted for the specified duration.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> drivers/reset/reset-simple.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index 13e7d5559acc9..d98a7e7d802d1 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -39,17 +39,20 @@ static int reset_simple_set(struct reset_controller_dev *rcdev,
> int reg_width = sizeof(u32);
> int bank = id / (reg_width * BITS_PER_BYTE);
> int offset = id % (reg_width * BITS_PER_BYTE);
> + void __iomem *addr = data->membase + (bank * reg_width);
> unsigned long flags;
> u32 reg;
>
> spin_lock_irqsave(&data->lock, flags);
>
> - reg = readl(data->membase + (bank * reg_width));
> + reg = readl(addr);
> if (assert ^ data->active_low)
> reg |= BIT(offset);
> else
> reg &= ~BIT(offset);
> - writel(reg, data->membase + (bank * reg_width));
> + writel(reg, addr);
> + /* Read back to make sure the write doesn't linger in a store buffer */
> + readl(addr);
You're not using the returned value to check that the reset was actually
set. This seems a very arbitrary readback workaround, which gives no
indication if it actually succeeded or not.
Also the set() is now asymmetrical to clear(). In cases when releasing
reset on a HW block that is about to have IO performed on it, one would
want to make sure the reset is actually deasserted before doing any IO.
Alex
>
> spin_unlock_irqrestore(&data->lock, flags);
>
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 5/5] reset: simple: read back to make sure changes are applied
@ 2017-08-16 21:00 ` Alexandru Gagniuc
0 siblings, 0 replies; 54+ messages in thread
From: Alexandru Gagniuc @ 2017-08-16 21:00 UTC (permalink / raw)
To: linux-arm-kernel
On 08/16/2017 02:47 AM, Philipp Zabel wrote:
> Read back the register after setting or clearing a reset bit to make
> sure that the changes are applied to the reset controller hardware.
> Theoretically, this avoids the write to stay stuck in a store buffer
Is there hardware where this has been observed to happen, or is this
purely theoretical? It would be nice to have a "this is needed on
hardware XYZ because ABC, and doesn't affect other hardware" comment in
the source.
> during the delay of an assert-delay-deassert sequence, and makes sure
> that the reset really is asserted for the specified duration.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> drivers/reset/reset-simple.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index 13e7d5559acc9..d98a7e7d802d1 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -39,17 +39,20 @@ static int reset_simple_set(struct reset_controller_dev *rcdev,
> int reg_width = sizeof(u32);
> int bank = id / (reg_width * BITS_PER_BYTE);
> int offset = id % (reg_width * BITS_PER_BYTE);
> + void __iomem *addr = data->membase + (bank * reg_width);
> unsigned long flags;
> u32 reg;
>
> spin_lock_irqsave(&data->lock, flags);
>
> - reg = readl(data->membase + (bank * reg_width));
> + reg = readl(addr);
> if (assert ^ data->active_low)
> reg |= BIT(offset);
> else
> reg &= ~BIT(offset);
> - writel(reg, data->membase + (bank * reg_width));
> + writel(reg, addr);
> + /* Read back to make sure the write doesn't linger in a store buffer */
> + readl(addr);
You're not using the returned value to check that the reset was actually
set. This seems a very arbitrary readback workaround, which gives no
indication if it actually succeeded or not.
Also the set() is now asymmetrical to clear(). In cases when releasing
reset on a HW block that is about to have IO performed on it, one would
want to make sure the reset is actually deasserted before doing any IO.
Alex
>
> spin_unlock_irqrestore(&data->lock, flags);
>
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 3/5] reset: stm32: use the reset-simple driver
2017-08-16 20:55 ` Alexandru Gagniuc
@ 2017-08-17 9:19 ` Andre Przywara
-1 siblings, 0 replies; 54+ messages in thread
From: Andre Przywara @ 2017-08-17 9:19 UTC (permalink / raw)
To: Alexandru Gagniuc, Andreas Färber
Cc: Philipp Zabel, linux-kernel, Maxime Coquelin, Alexandre Torgue,
Maxime Ripard, Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev,
Steffen Trumtrar, Dinh Nguyen, linux-arm-kernel, kernel
Hi,
On 16/08/17 21:55, Alexandru Gagniuc wrote:
>
>
> On 08/16/2017 01:52 PM, Andreas Färber wrote:
>> Am 16.08.2017 um 22:50 schrieb Alexandru Gagniuc:
>>> On 08/16/2017 02:46 AM, Philipp Zabel wrote:
>>>> The reset-simple driver can be used without changes.
>>>>
>>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>>
>>> Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
>>>
>>>> ---
>>>> drivers/reset/Kconfig | 11 ++---
>>>> drivers/reset/Makefile | 1 -
>>>> drivers/reset/reset-simple.c | 1 +
>>>> drivers/reset/reset-stm32.c | 108
>>>> -------------------------------------------
>>>> 4 files changed, 4 insertions(+), 117 deletions(-)
>>>> delete mode 100644 drivers/reset/reset-stm32.c
>>>>
>>>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>>>> index 78a8f6057985b..29f4487c290fc 100644
>>>> --- a/drivers/reset/Kconfig
>>>> +++ b/drivers/reset/Kconfig
>>>> @@ -70,19 +70,14 @@ config RESET_PISTACHIO
>>>>
>>>> config RESET_SIMPLE
>>>> bool "Simple Reset Controller Driver" if COMPILE_TEST
>>>> - default ARCH_SOCFPGA || ARCH_SUNXI
>>>> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
>>>
>>> If this series gets respun, could you please look into removing the
>>> ARCH_ dependency here?
>>
>> Why?
>
> Because the driver has to keep track of all its users, which is a
> layering violation.
"keep track of all its users": not necessarily, this is mostly for
convenience reasons to get automatic coverage for those SoCs without
changing various defconfigs - or distribution .configs, for that matter.
Especially the latter may prove to be nasty and introduce regressions.
But I see that this list may grow big and this is a bit beyond the scope
of the otherwise neat "default ARCH_xxx" trick.
So given the generic nature of this driver, can't we make this simply:
"default ARM || ARM64"
That would enable it everywhere automatically, but people can still
disable this if they know what they do.
Alternatively for ARM64 this is actually a candidate for the (one and
only) defconfig.
Cheers,
Andre.
>> It's not a dependency, just the default. Doing it here decouples it
>> from having to do Kconfig changes in the arm-soc tree as well.
>
> Doing it here allows this list to grow without bounds.
>
>> Regards,
>> Andreas
>>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 3/5] reset: stm32: use the reset-simple driver
@ 2017-08-17 9:19 ` Andre Przywara
0 siblings, 0 replies; 54+ messages in thread
From: Andre Przywara @ 2017-08-17 9:19 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 16/08/17 21:55, Alexandru Gagniuc wrote:
>
>
> On 08/16/2017 01:52 PM, Andreas F?rber wrote:
>> Am 16.08.2017 um 22:50 schrieb Alexandru Gagniuc:
>>> On 08/16/2017 02:46 AM, Philipp Zabel wrote:
>>>> The reset-simple driver can be used without changes.
>>>>
>>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>>
>>> Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
>>>
>>>> ---
>>>> drivers/reset/Kconfig | 11 ++---
>>>> drivers/reset/Makefile | 1 -
>>>> drivers/reset/reset-simple.c | 1 +
>>>> drivers/reset/reset-stm32.c | 108
>>>> -------------------------------------------
>>>> 4 files changed, 4 insertions(+), 117 deletions(-)
>>>> delete mode 100644 drivers/reset/reset-stm32.c
>>>>
>>>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>>>> index 78a8f6057985b..29f4487c290fc 100644
>>>> --- a/drivers/reset/Kconfig
>>>> +++ b/drivers/reset/Kconfig
>>>> @@ -70,19 +70,14 @@ config RESET_PISTACHIO
>>>>
>>>> config RESET_SIMPLE
>>>> bool "Simple Reset Controller Driver" if COMPILE_TEST
>>>> - default ARCH_SOCFPGA || ARCH_SUNXI
>>>> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
>>>
>>> If this series gets respun, could you please look into removing the
>>> ARCH_ dependency here?
>>
>> Why?
>
> Because the driver has to keep track of all its users, which is a
> layering violation.
"keep track of all its users": not necessarily, this is mostly for
convenience reasons to get automatic coverage for those SoCs without
changing various defconfigs - or distribution .configs, for that matter.
Especially the latter may prove to be nasty and introduce regressions.
But I see that this list may grow big and this is a bit beyond the scope
of the otherwise neat "default ARCH_xxx" trick.
So given the generic nature of this driver, can't we make this simply:
"default ARM || ARM64"
That would enable it everywhere automatically, but people can still
disable this if they know what they do.
Alternatively for ARM64 this is actually a candidate for the (one and
only) defconfig.
Cheers,
Andre.
>> It's not a dependency, just the default. Doing it here decouples it
>> from having to do Kconfig changes in the arm-soc tree as well.
>
> Doing it here allows this list to grow without bounds.
>
>> Regards,
>> Andreas
>>
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 5/5] reset: simple: read back to make sure changes are applied
2017-08-16 21:00 ` Alexandru Gagniuc
@ 2017-08-21 8:21 ` Philipp Zabel
-1 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-21 8:21 UTC (permalink / raw)
To: Alexandru Gagniuc, linux-kernel
Cc: Andre Przywara, Maxime Coquelin, Alexandre Torgue, Maxime Ripard,
Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev, Steffen Trumtrar,
Dinh Nguyen, Andreas Färber, linux-arm-kernel, kernel
Hi Alexandru,
On Wed, 2017-08-16 at 14:00 -0700, Alexandru Gagniuc wrote:
>
> On 08/16/2017 02:47 AM, Philipp Zabel wrote:
> > Read back the register after setting or clearing a reset bit to make
> > sure that the changes are applied to the reset controller hardware.
> > Theoretically, this avoids the write to stay stuck in a store buffer
>
> Is there hardware where this has been observed to happen, or is this
> purely theoretical? It would be nice to have a "this is needed on
> hardware XYZ because ABC, and doesn't affect other hardware" comment in
> the source.
This is purely theoretical, at least in the context of reset
controllers. I'm happy to drop this patch for now.
> > during the delay of an assert-delay-deassert sequence, and makes sure
> > that the reset really is asserted for the specified duration.
> >
> > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> > drivers/reset/reset-simple.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> > index 13e7d5559acc9..d98a7e7d802d1 100644
> > --- a/drivers/reset/reset-simple.c
> > +++ b/drivers/reset/reset-simple.c
> > @@ -39,17 +39,20 @@ static int reset_simple_set(struct reset_controller_dev *rcdev,
> > > > int reg_width = sizeof(u32);
> > > > int bank = id / (reg_width * BITS_PER_BYTE);
> > > > int offset = id % (reg_width * BITS_PER_BYTE);
> > > > + void __iomem *addr = data->membase + (bank * reg_width);
> > > > unsigned long flags;
> > > > u32 reg;
> >
> > > > spin_lock_irqsave(&data->lock, flags);
> >
> > > > - reg = readl(data->membase + (bank * reg_width));
> > > > + reg = readl(addr);
> > > > if (assert ^ data->active_low)
> > > > reg |= BIT(offset);
> > > > else
> > > > reg &= ~BIT(offset);
> > > > - writel(reg, data->membase + (bank * reg_width));
> > > > + writel(reg, addr);
> > > > + /* Read back to make sure the write doesn't linger in a store buffer */
> > > > + readl(addr);
>
> You're not using the returned value to check that the reset was actually
> set. This seems a very arbitrary readback workaround, which gives no
> indication if it actually succeeded or not.
True. For those reset controllers that support status readback, this
would be a better check.
> Also the set() is now asymmetrical to clear(). In cases when releasing
> reset on a HW block that is about to have IO performed on it, one would
> want to make sure the reset is actually deasserted before doing any IO.
Thanks for this observation, the reset_simple_set function name is
ambiguous. I'll rename it to reset_simple_update.
The function is called both for assert and deassert operations, and
whether the bit is set or cleared depends on both the assert parameter
and the active_low flag.
regards
Philipp
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 5/5] reset: simple: read back to make sure changes are applied
@ 2017-08-21 8:21 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-21 8:21 UTC (permalink / raw)
To: linux-arm-kernel
Hi Alexandru,
On Wed, 2017-08-16 at 14:00 -0700, Alexandru Gagniuc wrote:
>
> On 08/16/2017 02:47 AM, Philipp Zabel wrote:
> > Read back the register after setting or clearing a reset bit to make
> > sure that the changes are applied to the reset controller hardware.
> > Theoretically, this avoids the write to stay stuck in a store buffer
>
> Is there hardware where this has been observed to happen, or is this?
> purely theoretical? It would be nice to have a "this is needed on?
> hardware XYZ because ABC, and doesn't affect other hardware" comment in?
> the source.
This is purely theoretical, at least in the context of reset
controllers. I'm happy to drop this patch for now.
> > during the delay of an assert-delay-deassert sequence, and makes sure
> > that the reset really is asserted for the specified duration.
> >
> > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> > ?drivers/reset/reset-simple.c | 7 +++++--
> > ?1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> > index 13e7d5559acc9..d98a7e7d802d1 100644
> > --- a/drivers/reset/reset-simple.c
> > +++ b/drivers/reset/reset-simple.c
> > @@ -39,17 +39,20 @@ static int reset_simple_set(struct reset_controller_dev *rcdev,
> > > > ? int reg_width = sizeof(u32);
> > > > ? int bank = id / (reg_width * BITS_PER_BYTE);
> > > > ? int offset = id % (reg_width * BITS_PER_BYTE);
> > > > + void __iomem *addr = data->membase + (bank * reg_width);
> > > > ? unsigned long flags;
> > > > ? u32 reg;
> >
> > > > ? spin_lock_irqsave(&data->lock, flags);
> >
> > > > - reg = readl(data->membase + (bank * reg_width));
> > > > + reg = readl(addr);
> > > > ? if (assert ^ data->active_low)
> > > > ? reg |= BIT(offset);
> > > > ? else
> > > > ? reg &= ~BIT(offset);
> > > > - writel(reg, data->membase + (bank * reg_width));
> > > > + writel(reg, addr);
> > > > + /* Read back to make sure the write doesn't linger in a store buffer */
> > > > + readl(addr);
>
> You're not using the returned value to check that the reset was actually?
> set. This seems a very arbitrary readback workaround, which gives no?
> indication if it actually succeeded or not.
True. For those reset controllers that support status readback, this
would be a better check.
> Also the set() is now asymmetrical to clear(). In cases when releasing?
> reset on a HW block that is about to have IO performed on it, one would?
> want to make sure the reset is actually deasserted before doing any IO.
Thanks for this observation, the reset_simple_set function name is
ambiguous. I'll rename it to reset_simple_update.
The function is called both for assert and deassert operations, and
whether the bit is set or cleared depends on both the assert parameter
and the active_low flag.
regards
Philipp
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 3/5] reset: stm32: use the reset-simple driver
2017-08-17 9:19 ` Andre Przywara
@ 2017-08-21 8:38 ` Philipp Zabel
-1 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-21 8:38 UTC (permalink / raw)
To: Andre Przywara, Alexandru Gagniuc, Andreas Färber
Cc: linux-kernel, Maxime Coquelin, Alexandre Torgue, Maxime Ripard,
Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev, Steffen Trumtrar,
Dinh Nguyen, linux-arm-kernel, kernel
On Thu, 2017-08-17 at 10:19 +0100, Andre Przywara wrote:
> Hi,
>
> On 16/08/17 21:55, Alexandru Gagniuc wrote:
> >
> >
> > On 08/16/2017 01:52 PM, Andreas Färber wrote:
> > > Am 16.08.2017 um 22:50 schrieb Alexandru Gagniuc:
> > > > On 08/16/2017 02:46 AM, Philipp Zabel wrote:
> > > > > The reset-simple driver can be used without changes.
> > > > >
> > > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > >
> > > > > > > > Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> > > >
> > > > > ---
> > > > > drivers/reset/Kconfig | 11 ++---
> > > > > drivers/reset/Makefile | 1 -
> > > > > drivers/reset/reset-simple.c | 1 +
> > > > > drivers/reset/reset-stm32.c | 108
> > > > > -------------------------------------------
> > > > > 4 files changed, 4 insertions(+), 117 deletions(-)
> > > > > delete mode 100644 drivers/reset/reset-stm32.c
> > > > >
> > > > > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > > > > index 78a8f6057985b..29f4487c290fc 100644
> > > > > --- a/drivers/reset/Kconfig
> > > > > +++ b/drivers/reset/Kconfig
> > > > > @@ -70,19 +70,14 @@ config RESET_PISTACHIO
> > > > >
> > > > > config RESET_SIMPLE
> > > > > bool "Simple Reset Controller Driver" if COMPILE_TEST
> > > > > - default ARCH_SOCFPGA || ARCH_SUNXI
> > > > > + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
> > > >
> > > > If this series gets respun, could you please look into removing the
> > > > ARCH_ dependency here?
> > >
> > > Why?
> >
> > Because the driver has to keep track of all its users, which is a
> > layering violation.
>
> "keep track of all its users": not necessarily, this is mostly for
> convenience reasons to get automatic coverage for those SoCs without
> changing various defconfigs - or distribution .configs, for that matter.
> Especially the latter may prove to be nasty and introduce regressions.
>
> But I see that this list may grow big and this is a bit beyond the scope
> of the otherwise neat "default ARCH_xxx" trick.
>
> So given the generic nature of this driver, can't we make this simply:
> "default ARM || ARM64"
> That would enable it everywhere automatically, but people can still
> disable this if they know what they do.
>
> Alternatively for ARM64 this is actually a candidate for the (one and
> only) defconfig.
I would like to make this driver selectable. but for that to happen,
ARCH_HAS_RESET_CONTROLLER and RESET_CONTROLLER have to be merged into a
single selectable symbol first.
Until then, I'd be fine with making this driver:
bool "Simple Reset Controller Driver"
default ARM || ARM64
but in my opinion it is equally ok to have the default ARCH_* list for
a while.
regards
Philipp
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 3/5] reset: stm32: use the reset-simple driver
@ 2017-08-21 8:38 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-21 8:38 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2017-08-17 at 10:19 +0100, Andre Przywara wrote:
> Hi,
>
> On 16/08/17 21:55, Alexandru Gagniuc wrote:
> >
> >
> > On 08/16/2017 01:52 PM, Andreas F?rber wrote:
> > > Am 16.08.2017 um 22:50 schrieb Alexandru Gagniuc:
> > > > On 08/16/2017 02:46 AM, Philipp Zabel wrote:
> > > > > The reset-simple driver can be used without changes.
> > > > >
> > > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > >
> > > > > > > > Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> > > >
> > > > > ---
> > > > > ?drivers/reset/Kconfig????????|??11 ++---
> > > > > ?drivers/reset/Makefile???????|???1 -
> > > > > ?drivers/reset/reset-simple.c |???1 +
> > > > > ?drivers/reset/reset-stm32.c??| 108
> > > > > -------------------------------------------
> > > > > ?4 files changed, 4 insertions(+), 117 deletions(-)
> > > > > ?delete mode 100644 drivers/reset/reset-stm32.c
> > > > >
> > > > > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > > > > index 78a8f6057985b..29f4487c290fc 100644
> > > > > --- a/drivers/reset/Kconfig
> > > > > +++ b/drivers/reset/Kconfig
> > > > > @@ -70,19 +70,14 @@ config RESET_PISTACHIO
> > > > >
> > > > > ?config RESET_SIMPLE
> > > > > ?????bool "Simple Reset Controller Driver" if COMPILE_TEST
> > > > > -????default ARCH_SOCFPGA || ARCH_SUNXI
> > > > > +????default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
> > > >
> > > > If this series gets respun, could you please look into removing the
> > > > ARCH_ dependency here?
> > >
> > > Why?
> >
> > Because the driver has to keep track of all its users, which is a
> > layering violation.
>
> "keep track of all its users": not necessarily, this is mostly for
> convenience reasons to get automatic coverage for those SoCs without
> changing various defconfigs - or distribution .configs, for that matter.
> Especially the latter may prove to be nasty and introduce regressions.
>
> But I see that this list may grow big and this is a bit beyond the scope
> of the otherwise neat "default ARCH_xxx" trick.
>
> So given the generic nature of this driver, can't we make this simply:
> "default ARM || ARM64"
> That would enable it everywhere automatically, but people can still
> disable this if they know what they do.
>
> Alternatively for ARM64 this is actually a candidate for the (one and
> only) defconfig.
I would like to make this driver selectable. but for that to happen,
ARCH_HAS_RESET_CONTROLLER and RESET_CONTROLLER have to be merged into a
single selectable symbol first.
Until then, I'd be fine with making this driver:
bool "Simple Reset Controller Driver"
default ARM || ARM64
but in my opinion it is equally ok to have the default ARCH_* list for
a while.
regards
Philipp
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 2/5] reset: socfpga: use the reset-simple driver
2017-08-16 20:46 ` Alexandru Gagniuc
@ 2017-08-21 8:45 ` Philipp Zabel
-1 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-21 8:45 UTC (permalink / raw)
To: Alexandru Gagniuc, linux-kernel
Cc: Andre Przywara, Maxime Coquelin, Alexandre Torgue, Maxime Ripard,
Chen-Yu Tsai, Baoyou Xie, Eugeniy Paltsev, Steffen Trumtrar,
Dinh Nguyen, Andreas Färber, linux-arm-kernel, kernel
On Wed, 2017-08-16 at 13:46 -0700, Alexandru Gagniuc wrote:
> Hi Phillip,
>
> On 08/16/2017 02:46 AM, Philipp Zabel wrote:
> [snip]
> > @@ -118,8 +151,23 @@ static int reset_simple_probe(struct
> > platform_device *pdev)
> > data->rcdev.ops = &reset_simple_ops;
> > data->rcdev.of_node = dev->of_node;
> >
> > - if (devdata)
> > + if (devdata) {
> > + u32 reg_offset = devdata->reg_offset;
> > +
> > + if (reg_offset &&
> > + of_property_read_u32(dev->of_node,
> > "altr,modrst-offset",
> > + ®_offset)) {
> > + dev_warn(dev,
> > + "missing altr,modrst-offset
> > property, assuming 0x%x!\n",
> > + reg_offset);
> > + }
>
> I would not make reading dt properties dependent on the presence of
> devdata. That breaks being able to configure the reset controller from
> dt. I would either just read the "altr,modrst-offset" property, or read
> it when
>
> of_device_is_compatible(dev->of_node, "altr,rst-mgr"));
Checking the compatible again is unnecessary, we already have obtained
devdata depending on the compatible value. I had effectively the same
check in v2:
if (devdata == &reset_simple_socfpga)
I can change back to that, if you prefer. Note that the
if (devdata)
reg_offset = devdata->reg_offset;
part has to stay now that reg_offset is stored in devdata.
> Ideally, we could have a more generic "reg-offset" binding for new reset
> controllers, but that is beyond the scope of this patch.
Agreed.
regards
Philipp
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 2/5] reset: socfpga: use the reset-simple driver
@ 2017-08-21 8:45 ` Philipp Zabel
0 siblings, 0 replies; 54+ messages in thread
From: Philipp Zabel @ 2017-08-21 8:45 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2017-08-16 at 13:46 -0700, Alexandru Gagniuc wrote:
> Hi Phillip,
>
> On 08/16/2017 02:46 AM, Philipp Zabel wrote:
> [snip]
> > @@ -118,8 +151,23 @@ static int reset_simple_probe(struct
> > platform_device *pdev)
> > ? data->rcdev.ops = &reset_simple_ops;
> > ? data->rcdev.of_node = dev->of_node;
> >
> > - if (devdata)
> > + if (devdata) {
> > + u32 reg_offset = devdata->reg_offset;
> > +
> > + if (reg_offset &&
> > + ????of_property_read_u32(dev->of_node,
> > "altr,modrst-offset",
> > + ?®_offset)) {
> > + dev_warn(dev,
> > + ?"missing altr,modrst-offset
> > property, assuming 0x%x!\n",
> > + ?reg_offset);
> > + }
>
> I would not make reading dt properties dependent on the presence of?
> devdata. That breaks being able to configure the reset controller from?
> dt. I would either just read the "altr,modrst-offset" property, or read?
> it when
>
> of_device_is_compatible(dev->of_node, "altr,rst-mgr"));
Checking the compatible again is unnecessary, we already have obtained
devdata depending on the compatible value. I had effectively the same
check in v2:
if (devdata == &reset_simple_socfpga)
I can change back to that, if you prefer. Note that the
if (devdata)
reg_offset = devdata->reg_offset;
part has to stay now that reg_offset is stored in devdata.
> Ideally, we could have a more generic "reg-offset" binding for new reset?
> controllers, but that is beyond the scope of this patch.
Agreed.
regards
Philipp
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [v3,3/5] reset: stm32: use the reset-simple driver
2017-08-16 9:46 ` Philipp Zabel
@ 2017-08-29 8:39 ` Gabriel FERNANDEZ
-1 siblings, 0 replies; 54+ messages in thread
From: Gabriel FERNANDEZ @ 2017-08-29 8:39 UTC (permalink / raw)
To: Philipp Zabel, linux-kernel
Cc: Alexandru Gagniuc, Andre Przywara, Maxime Coquelin,
Alexandre TORGUE, Maxime Ripard, Chen-Yu Tsai, Baoyou Xie,
Eugeniy Paltsev, Steffen Trumtrar, Dinh Nguyen,
Andreas Färber, linux-arm-kernel, kernel
On 08/16/2017 11:46 AM, Philipp Zabel wrote:
> The reset-simple driver can be used without changes.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> ---
> drivers/reset/Kconfig | 11 ++---
> drivers/reset/Makefile | 1 -
> drivers/reset/reset-simple.c | 1 +
> drivers/reset/reset-stm32.c | 108 -------------------------------------------
> 4 files changed, 4 insertions(+), 117 deletions(-)
> delete mode 100644 drivers/reset/reset-stm32.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 78a8f6057985b..29f4487c290fc 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -70,19 +70,14 @@ config RESET_PISTACHIO
>
> config RESET_SIMPLE
> bool "Simple Reset Controller Driver" if COMPILE_TEST
> - default ARCH_SOCFPGA || ARCH_SUNXI
> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
> help
> This enables a simple reset controller driver for reset lines that
> that can be asserted and deasserted by toggling bits in a contiguous,
> exclusive register space.
>
> - Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
> -
> -config RESET_STM32
> - bool "STM32 Reset Driver" if COMPILE_TEST
> - default ARCH_STM32
> - help
> - This enables the RCC reset controller driver for STM32 MCUs.
> + Currently this driver supports Altera SoCFPGAs, the RCC reset
> + controller in STM32 MCUs, and Allwinner SoCs.
>
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 25f5f722dec01..e8c3a032f4780 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -12,7 +12,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> -obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index 84b08ce4def01..e54a29b8b532d 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -120,6 +120,7 @@ static const struct reset_simple_devdata reset_simple_active_low = {
>
> static const struct of_device_id reset_simple_dt_ids[] = {
> { .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
> + { .compatible = "st,stm32-rcc", },
> { .compatible = "allwinner,sun6i-a31-clock-reset",
> .data = &reset_simple_active_low },
> { /* sentinel */ },
> diff --git a/drivers/reset/reset-stm32.c b/drivers/reset/reset-stm32.c
> deleted file mode 100644
> index 3a7c8527e66af..0000000000000
> --- a/drivers/reset/reset-stm32.c
> +++ /dev/null
> @@ -1,108 +0,0 @@
> -/*
> - * Copyright (C) Maxime Coquelin 2015
> - * Author: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> - * License terms: GNU General Public License (GPL), version 2
> - *
> - * Heavily based on sunxi driver from Maxime Ripard.
> - */
> -
> -#include <linux/err.h>
> -#include <linux/io.h>
> -#include <linux/of.h>
> -#include <linux/of_address.h>
> -#include <linux/platform_device.h>
> -#include <linux/reset-controller.h>
> -#include <linux/slab.h>
> -#include <linux/spinlock.h>
> -#include <linux/types.h>
> -
> -struct stm32_reset_data {
> - spinlock_t lock;
> - void __iomem *membase;
> - struct reset_controller_dev rcdev;
> -};
> -
> -static int stm32_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct stm32_reset_data *data = container_of(rcdev,
> - struct stm32_reset_data,
> - rcdev);
> - int bank = id / BITS_PER_LONG;
> - int offset = id % BITS_PER_LONG;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * 4));
> - writel(reg | BIT(offset), data->membase + (bank * 4));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct stm32_reset_data *data = container_of(rcdev,
> - struct stm32_reset_data,
> - rcdev);
> - int bank = id / BITS_PER_LONG;
> - int offset = id % BITS_PER_LONG;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * 4));
> - writel(reg & ~BIT(offset), data->membase + (bank * 4));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static const struct reset_control_ops stm32_reset_ops = {
> - .assert = stm32_reset_assert,
> - .deassert = stm32_reset_deassert,
> -};
> -
> -static const struct of_device_id stm32_reset_dt_ids[] = {
> - { .compatible = "st,stm32-rcc", },
> - { /* sentinel */ },
> -};
> -
> -static int stm32_reset_probe(struct platform_device *pdev)
> -{
> - struct stm32_reset_data *data;
> - struct resource *res;
> -
> - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->membase = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(data->membase))
> - return PTR_ERR(data->membase);
> -
> - spin_lock_init(&data->lock);
> -
> - data->rcdev.owner = THIS_MODULE;
> - data->rcdev.nr_resets = resource_size(res) * 8;
> - data->rcdev.ops = &stm32_reset_ops;
> - data->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> -}
> -
> -static struct platform_driver stm32_reset_driver = {
> - .probe = stm32_reset_probe,
> - .driver = {
> - .name = "stm32-rcc-reset",
> - .of_match_table = stm32_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(stm32_reset_driver);
Acked-by: Gabriel Fernandez <gabriel.fernandez@st.com>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [v3,3/5] reset: stm32: use the reset-simple driver
@ 2017-08-29 8:39 ` Gabriel FERNANDEZ
0 siblings, 0 replies; 54+ messages in thread
From: Gabriel FERNANDEZ @ 2017-08-29 8:39 UTC (permalink / raw)
To: linux-arm-kernel
On 08/16/2017 11:46 AM, Philipp Zabel wrote:
> The reset-simple driver can be used without changes.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> ---
> drivers/reset/Kconfig | 11 ++---
> drivers/reset/Makefile | 1 -
> drivers/reset/reset-simple.c | 1 +
> drivers/reset/reset-stm32.c | 108 -------------------------------------------
> 4 files changed, 4 insertions(+), 117 deletions(-)
> delete mode 100644 drivers/reset/reset-stm32.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 78a8f6057985b..29f4487c290fc 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -70,19 +70,14 @@ config RESET_PISTACHIO
>
> config RESET_SIMPLE
> bool "Simple Reset Controller Driver" if COMPILE_TEST
> - default ARCH_SOCFPGA || ARCH_SUNXI
> + default ARCH_SOCFPGA || ARCH_STM32 || ARCH_SUNXI
> help
> This enables a simple reset controller driver for reset lines that
> that can be asserted and deasserted by toggling bits in a contiguous,
> exclusive register space.
>
> - Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
> -
> -config RESET_STM32
> - bool "STM32 Reset Driver" if COMPILE_TEST
> - default ARCH_STM32
> - help
> - This enables the RCC reset controller driver for STM32 MCUs.
> + Currently this driver supports Altera SoCFPGAs, the RCC reset
> + controller in STM32 MCUs, and Allwinner SoCs.
>
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 25f5f722dec01..e8c3a032f4780 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -12,7 +12,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> -obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index 84b08ce4def01..e54a29b8b532d 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -120,6 +120,7 @@ static const struct reset_simple_devdata reset_simple_active_low = {
>
> static const struct of_device_id reset_simple_dt_ids[] = {
> { .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
> + { .compatible = "st,stm32-rcc", },
> { .compatible = "allwinner,sun6i-a31-clock-reset",
> .data = &reset_simple_active_low },
> { /* sentinel */ },
> diff --git a/drivers/reset/reset-stm32.c b/drivers/reset/reset-stm32.c
> deleted file mode 100644
> index 3a7c8527e66af..0000000000000
> --- a/drivers/reset/reset-stm32.c
> +++ /dev/null
> @@ -1,108 +0,0 @@
> -/*
> - * Copyright (C) Maxime Coquelin 2015
> - * Author: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> - * License terms: GNU General Public License (GPL), version 2
> - *
> - * Heavily based on sunxi driver from Maxime Ripard.
> - */
> -
> -#include <linux/err.h>
> -#include <linux/io.h>
> -#include <linux/of.h>
> -#include <linux/of_address.h>
> -#include <linux/platform_device.h>
> -#include <linux/reset-controller.h>
> -#include <linux/slab.h>
> -#include <linux/spinlock.h>
> -#include <linux/types.h>
> -
> -struct stm32_reset_data {
> - spinlock_t lock;
> - void __iomem *membase;
> - struct reset_controller_dev rcdev;
> -};
> -
> -static int stm32_reset_assert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct stm32_reset_data *data = container_of(rcdev,
> - struct stm32_reset_data,
> - rcdev);
> - int bank = id / BITS_PER_LONG;
> - int offset = id % BITS_PER_LONG;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * 4));
> - writel(reg | BIT(offset), data->membase + (bank * 4));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct stm32_reset_data *data = container_of(rcdev,
> - struct stm32_reset_data,
> - rcdev);
> - int bank = id / BITS_PER_LONG;
> - int offset = id % BITS_PER_LONG;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&data->lock, flags);
> -
> - reg = readl(data->membase + (bank * 4));
> - writel(reg & ~BIT(offset), data->membase + (bank * 4));
> -
> - spin_unlock_irqrestore(&data->lock, flags);
> -
> - return 0;
> -}
> -
> -static const struct reset_control_ops stm32_reset_ops = {
> - .assert = stm32_reset_assert,
> - .deassert = stm32_reset_deassert,
> -};
> -
> -static const struct of_device_id stm32_reset_dt_ids[] = {
> - { .compatible = "st,stm32-rcc", },
> - { /* sentinel */ },
> -};
> -
> -static int stm32_reset_probe(struct platform_device *pdev)
> -{
> - struct stm32_reset_data *data;
> - struct resource *res;
> -
> - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->membase = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(data->membase))
> - return PTR_ERR(data->membase);
> -
> - spin_lock_init(&data->lock);
> -
> - data->rcdev.owner = THIS_MODULE;
> - data->rcdev.nr_resets = resource_size(res) * 8;
> - data->rcdev.ops = &stm32_reset_ops;
> - data->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> -}
> -
> -static struct platform_driver stm32_reset_driver = {
> - .probe = stm32_reset_probe,
> - .driver = {
> - .name = "stm32-rcc-reset",
> - .of_match_table = stm32_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(stm32_reset_driver);
Acked-by: Gabriel Fernandez <gabriel.fernandez@st.com>
^ permalink raw reply [flat|nested] 54+ messages in thread
end of thread, other threads:[~2017-08-29 8:40 UTC | newest]
Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-16 9:46 [PATCH v3 0/5] Unify simple reset drivers Philipp Zabel
2017-08-16 9:46 ` Philipp Zabel
2017-08-16 9:46 ` [PATCH v3 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967 Philipp Zabel
2017-08-16 9:46 ` Philipp Zabel
2017-08-16 11:30 ` Andre Przywara
2017-08-16 11:30 ` Andre Przywara
2017-08-16 12:12 ` Andreas Färber
2017-08-16 12:12 ` Andreas Färber
2017-08-16 15:11 ` Philipp Zabel
2017-08-16 15:11 ` Philipp Zabel
2017-08-16 15:17 ` Andre Przywara
2017-08-16 15:17 ` Andre Przywara
2017-08-16 16:41 ` Andreas Färber
2017-08-16 16:41 ` Andreas Färber
2017-08-16 16:46 ` Andre Przywara
2017-08-16 16:46 ` Andre Przywara
2017-08-16 20:46 ` Alexandru Gagniuc
2017-08-16 20:46 ` Alexandru Gagniuc
2017-08-16 9:46 ` [PATCH v3 2/5] reset: socfpga: use the reset-simple driver Philipp Zabel
2017-08-16 9:46 ` Philipp Zabel
2017-08-16 20:46 ` Alexandru Gagniuc
2017-08-16 20:46 ` Alexandru Gagniuc
2017-08-21 8:45 ` Philipp Zabel
2017-08-21 8:45 ` Philipp Zabel
2017-08-16 9:46 ` [PATCH v3 3/5] reset: stm32: " Philipp Zabel
2017-08-16 9:46 ` Philipp Zabel
2017-08-16 12:52 ` Eugeniy Paltsev
2017-08-16 12:52 ` Eugeniy Paltsev
2017-08-16 12:55 ` Andreas Färber
2017-08-16 12:55 ` Andreas Färber
2017-08-16 13:03 ` Andre Przywara
2017-08-16 13:03 ` Andre Przywara
2017-08-16 20:50 ` Alexandru Gagniuc
2017-08-16 20:50 ` Alexandru Gagniuc
2017-08-16 20:52 ` Andreas Färber
2017-08-16 20:52 ` Andreas Färber
2017-08-16 20:55 ` Alexandru Gagniuc
2017-08-16 20:55 ` Alexandru Gagniuc
2017-08-17 9:19 ` Andre Przywara
2017-08-17 9:19 ` Andre Przywara
2017-08-21 8:38 ` Philipp Zabel
2017-08-21 8:38 ` Philipp Zabel
2017-08-29 8:39 ` [v3,3/5] " Gabriel FERNANDEZ
2017-08-29 8:39 ` Gabriel FERNANDEZ
2017-08-16 9:47 ` [PATCH v3 4/5] reset: zx2967: " Philipp Zabel
2017-08-16 9:47 ` Philipp Zabel
2017-08-16 20:50 ` Alexandru Gagniuc
2017-08-16 20:50 ` Alexandru Gagniuc
2017-08-16 9:47 ` [PATCH v3 5/5] reset: simple: read back to make sure changes are applied Philipp Zabel
2017-08-16 9:47 ` Philipp Zabel
2017-08-16 21:00 ` Alexandru Gagniuc
2017-08-16 21:00 ` Alexandru Gagniuc
2017-08-21 8:21 ` Philipp Zabel
2017-08-21 8:21 ` Philipp Zabel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.