linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Introduce keystone reset driver
@ 2014-04-14 17:41 Ivan Khoronzhuk
  2014-04-14 17:41 ` [PATCH v2 1/5] Power: reset: keystone-reset: introduce " Ivan Khoronzhuk
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Ivan Khoronzhuk @ 2014-04-14 17:41 UTC (permalink / raw)
  To: dbaryshkov, dwmw2, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, santosh.shilimkar, grant.likely
  Cc: rdunlap, linux, ivan.khoronzhuk, grygorii.strashko, olof,
	w-kwok2, sboyd, devicetree, linux-doc, linux-kernel,
	linux-arm-kernel

These patches introduce keystone reset driver.

The keystone SoC can be rebooted in several ways. By external reset
pin, by soft and by watchdogs. This driver allows software reset or reset
by one of the watchdogs. Also added opportunity to set soft/hard reset type.

Based on v3.15-rc1

v1..v2
	- re basedon on v3.15-rc1 without changes

Ivan Khoronzhuk (5):
  Power: reset: keystone-reset: introduce keystone reset driver
  Power: reset: add bindings for keystone reset driver
  ARM: keystone: remove redundant reset stuff
  ARM: dts: keystone: update reset node to work with reset driver
  ARM: keystone: enable reset driver support

 .../bindings/power/reset/keystone-reset.txt        |  59 +++++++
 arch/arm/boot/dts/keystone.dtsi                    |   4 +-
 arch/arm/configs/keystone_defconfig                |   3 +
 arch/arm/mach-keystone/keystone.c                  |  35 -----
 drivers/power/reset/Kconfig                        |   7 +
 drivers/power/reset/Makefile                       |   1 +
 drivers/power/reset/keystone-reset.c               | 171 +++++++++++++++++++++
 7 files changed, 244 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/reset/keystone-reset.txt
 create mode 100644 drivers/power/reset/keystone-reset.c

-- 
1.8.3.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/5] Power: reset: keystone-reset: introduce keystone reset driver
  2014-04-14 17:41 [PATCH v2 0/5] Introduce keystone reset driver Ivan Khoronzhuk
@ 2014-04-14 17:41 ` Ivan Khoronzhuk
  2014-04-14 17:41 ` [PATCH v2 2/5] Power: reset: add bindings for " Ivan Khoronzhuk
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Ivan Khoronzhuk @ 2014-04-14 17:41 UTC (permalink / raw)
  To: dbaryshkov, dwmw2, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, santosh.shilimkar, grant.likely
  Cc: rdunlap, linux, ivan.khoronzhuk, grygorii.strashko, olof,
	w-kwok2, sboyd, devicetree, linux-doc, linux-kernel,
	linux-arm-kernel

The keystone SoC can be rebooted in several ways. By external reset
pin, by soft and by watchdogs. To allow keystone SoC reset if
watchdog is triggered we have to enable it in reset mux configuration
register regarding of watchdog configuration. Also we need to set
soft/hard reset we are going to use.

So add keystone reset driver to handle all this stuff.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 drivers/power/reset/Kconfig          |   7 ++
 drivers/power/reset/Makefile         |   1 +
 drivers/power/reset/keystone-reset.c | 171 +++++++++++++++++++++++++++++++++++
 3 files changed, 179 insertions(+)
 create mode 100644 drivers/power/reset/keystone-reset.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index fa0e4e0..4d2d3d8 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -57,3 +57,10 @@ config POWER_RESET_XGENE
 	depends on POWER_RESET
 	help
 	  Reboot support for the APM SoC X-Gene Eval boards.
+
+config POWER_RESET_KEYSTONE
+	bool "Keystone reset driver"
+	depends on ARCH_KEYSTONE
+	help
+	  Reboot support for the KEYSTONE SoCs.
+
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index a5b4a77..802a420 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
 obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
 obj-$(CONFIG_POWER_RESET_VEXPRESS) += vexpress-poweroff.o
 obj-$(CONFIG_POWER_RESET_XGENE) += xgene-reboot.o
+obj-$(CONFIG_POWER_RESET_KEYSTONE) += keystone-reset.o
diff --git a/drivers/power/reset/keystone-reset.c b/drivers/power/reset/keystone-reset.c
new file mode 100644
index 0000000..70fee79
--- /dev/null
+++ b/drivers/power/reset/keystone-reset.c
@@ -0,0 +1,171 @@
+/*
+ * TI keystone reboot driver
+ *
+ * Copyright (C) 2014 Texas Instruments Incorporated. http://www.ti.com/
+ *
+ * Author: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/io.h>
+#include <linux/reboot.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <asm/system_misc.h>
+
+#define RSCTRL_KEY_MASK			0xffff0000
+#define RSCTRL_KEY			0x5a69
+#define RSCTRL_RESET			BIT(16)
+
+#define RSCFG_RSTYPE_SOFT		0x300f
+#define RSCFG_RSTYPE_HARD		0x0
+
+#define RSTYPE_RG			0x0
+#define RSCTRL_RG			0x4
+#define RSCFG_RG			0x8
+#define RSISO_RG			0xc
+
+#define RSMUX_OMODE_MASK		0xe
+#define RSMUX_OMODE_RESET_SOC		0xa
+#define RSMUX_OMODE_RESET_OFF		0x0
+#define RSMUX_LOCK_MASK			0x1
+#define RSMUX_LOCK_SET			0x1
+
+#define WDT_MUX_NUMBER			0x4
+
+static void __iomem *rspll_base;
+
+/**
+ * rsctrl_enable_rspll_write - enable access to RSCTRL, RSCFG
+ * To be able to access to RSCTRL, RSCFG registers
+ * we has to write a key before
+ */
+static void rsctrl_enable_rspll_write(void)
+{
+	void __iomem *rstctrl_rg;
+	u32 val;
+
+	rstctrl_rg = rspll_base + RSCTRL_RG;
+	val = readl(rstctrl_rg);
+	val &= RSCTRL_KEY_MASK;
+	val |= RSCTRL_KEY;
+	writel(val, rstctrl_rg);
+}
+
+static void rsctrl_restart(enum reboot_mode mode, const char *cmd)
+{
+	u32 val;
+	void __iomem *rstctrl;
+
+	/* enable write access to RSTCTRL */
+	rsctrl_enable_rspll_write();
+
+	/* reset the SOC */
+	rstctrl = rspll_base + RSCTRL_RG;
+	val = readl(rstctrl);
+	val &= ~RSCTRL_RESET;
+	writel(val, rstctrl);
+}
+
+static struct of_device_id rsctrl_of_match[] = {
+	{.compatible = "ti,keystone-reset", },
+	{},
+};
+
+static int rsctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	void __iomem *rsmux_base;
+	void __iomem *rg;
+	struct resource *res;
+	u32 val;
+	int ret;
+	int i;
+
+	if (!np)
+		return -ENODEV;
+
+	i = of_property_match_string(np, "reg-names", "pllregs");
+	res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+	rspll_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(rspll_base))
+		return PTR_ERR(rspll_base);
+
+	i = of_property_match_string(np, "reg-names", "muxregs");
+	res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+	rsmux_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(rsmux_base))
+		return PTR_ERR(rsmux_base);
+
+	/* set soft/hard reset */
+	val = of_property_read_bool(np, "ti,soft-reset");
+	val = val ? RSCFG_RSTYPE_SOFT : RSCFG_RSTYPE_HARD;
+
+	rsctrl_enable_rspll_write();
+	writel(val, rspll_base + RSCFG_RG);
+
+	arm_pm_restart = rsctrl_restart;
+
+	/* disable reset isolation for all module clocks */
+	writel(0, rspll_base + RSISO_RG);
+
+	/* enable reset for watchdogs from list */
+	for (i = 0; i < WDT_MUX_NUMBER; i++) {
+		ret = of_property_read_u32_index(np, "ti,wdt_list", i, &val);
+		if (ret == -EOVERFLOW && !i) {
+			dev_err(dev, "ti,wdt_list property has to contain at"
+				"least one entry\n");
+			return -EINVAL;
+		} else if (ret) {
+			break;
+		}
+
+		if (val >= WDT_MUX_NUMBER) {
+			dev_err(dev, "ti,wdt_list property can contain"
+				"only numbers < 4\n");
+			return -EINVAL;
+		}
+
+		rg = rsmux_base + val*4;
+
+		val = readl(rg);
+		val &= ~RSMUX_OMODE_MASK;
+		val |= RSMUX_OMODE_RESET_SOC | RSMUX_LOCK_SET;
+		writel(val, rg);
+	}
+
+	/* disable reset for watchdogs from not list */
+	for (i = 0; i < WDT_MUX_NUMBER; i++) {
+		rg = rsmux_base + i*4;
+
+		val = readl(rg);
+		if (!(val & RSMUX_LOCK_MASK)) {
+			val &= ~RSMUX_OMODE_MASK;
+			val |= RSMUX_OMODE_RESET_OFF | RSMUX_LOCK_SET;
+			writel(val, rg);
+		}
+	}
+
+	devm_iounmap(dev, rsmux_base);
+	return 0;
+}
+
+static struct platform_driver rsctrl_driver = {
+	.probe = rsctrl_probe,
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = KBUILD_MODNAME,
+		.of_match_table = rsctrl_of_match,
+	},
+};
+module_platform_driver(rsctrl_driver);
+
+MODULE_AUTHOR("Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>");
+MODULE_DESCRIPTION("Texas Instruments keystone reset driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" KBUILD_MODNAME);
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/5] Power: reset: add bindings for keystone reset driver
  2014-04-14 17:41 [PATCH v2 0/5] Introduce keystone reset driver Ivan Khoronzhuk
  2014-04-14 17:41 ` [PATCH v2 1/5] Power: reset: keystone-reset: introduce " Ivan Khoronzhuk
@ 2014-04-14 17:41 ` Ivan Khoronzhuk
  2014-04-14 18:44   ` Arnd Bergmann
  2014-04-14 17:41 ` [PATCH v2 3/5] ARM: keystone: remove redundant reset stuff Ivan Khoronzhuk
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Ivan Khoronzhuk @ 2014-04-14 17:41 UTC (permalink / raw)
  To: dbaryshkov, dwmw2, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, santosh.shilimkar, grant.likely
  Cc: rdunlap, linux, ivan.khoronzhuk, grygorii.strashko, olof,
	w-kwok2, sboyd, devicetree, linux-doc, linux-kernel,
	linux-arm-kernel

This node is intended to allow SoC reset in case of software reset
or appropriate watchdogs.

The Keystone SoCs can contain up to 4 watchdog timers to reset
SoC. Each watchdog timer event input is connected to the Reset Mux
block. The Reset Mux block can be configured to cause reset or not.

Additionally soft or hard reset can be configured.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 .../bindings/power/reset/keystone-reset.txt        | 59 ++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/keystone-reset.txt

diff --git a/Documentation/devicetree/bindings/power/reset/keystone-reset.txt b/Documentation/devicetree/bindings/power/reset/keystone-reset.txt
new file mode 100644
index 0000000..5ad5883
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/keystone-reset.txt
@@ -0,0 +1,59 @@
+* Device tree bindings for Texas Instruments keystone reset
+
+This node is intended to allow SoC reset in case of software reset
+of selected watchdogs.
+
+The Keystone SoCs can contain up to 4 watchdog timers to reset
+SoC. Each watchdog timer event input is connected to the Reset Mux
+block. The Reset Mux block can be configured to cause reset or not.
+
+Additionally soft or hard reset can be configured.
+
+Required properties:
+
+- compatible:		ti,keystone-reset
+
+- reg:			Contains offset/length value for mux registers.
+
+			reg = <0x23100e4 0x10>,
+			      <0x2620328 0x10>;
+
+-reg-names:		Contains two ranges "pllregs" and "muxregs".
+			"pllregs" - PLL reset control regs: RSTYPE, RSCTRL,
+			RSCFG, RSISO.
+			"muxregs" - mux block registers for all watchdogs.
+
+Optional properties:
+
+- ti,soft-reset:	Boolean option indicating soft reset.
+			By default hard reset is used.
+
+- ti,wdt_list:		WDT list that can cause SoC reset.
+			The list in format: <0>, <2>;
+			Begins from 0 to 3, as keystone can contain up
+			to 4 SoC reset watchdogs.
+
+Example 1:
+Setup keystone reset so that in case software reset or
+WDT1 is triggered it issues hard reset for SoC.
+
+rstctrl: reset-controller {
+	compatible = "ti,keystone-reset";
+	reg = <0x23100e4 0x10>,
+	      <0x2620328 0x10>;
+	reg-names = "pllregs", "muxregs";
+	ti,wdt_list = <0>;
+};
+
+Example 2:
+Setup keystone reset so that in case of software reset or
+WDT1 or WDT3 is triggered it issues soft reset for SoC.
+
+rstctrl: reset-controller {
+	compatible = "ti,keystone-reset";
+	reg = <0x23100e4 0x10>,
+	      <0x2620328 0x10>;
+	reg-names = "pllregs", "muxregs";
+	ti,wdt_list = <0>, <2>;
+	ti,soft-reset;
+};
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/5] ARM: keystone: remove redundant reset stuff
  2014-04-14 17:41 [PATCH v2 0/5] Introduce keystone reset driver Ivan Khoronzhuk
  2014-04-14 17:41 ` [PATCH v2 1/5] Power: reset: keystone-reset: introduce " Ivan Khoronzhuk
  2014-04-14 17:41 ` [PATCH v2 2/5] Power: reset: add bindings for " Ivan Khoronzhuk
@ 2014-04-14 17:41 ` Ivan Khoronzhuk
  2014-04-14 17:41 ` [PATCH v2 4/5] ARM: dts: keystone: update reset node to work with reset driver Ivan Khoronzhuk
  2014-04-14 17:41 ` [PATCH v2 5/5] ARM: keystone: enable reset driver support Ivan Khoronzhuk
  4 siblings, 0 replies; 9+ messages in thread
From: Ivan Khoronzhuk @ 2014-04-14 17:41 UTC (permalink / raw)
  To: dbaryshkov, dwmw2, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, santosh.shilimkar, grant.likely
  Cc: rdunlap, linux, ivan.khoronzhuk, grygorii.strashko, olof,
	w-kwok2, sboyd, devicetree, linux-doc, linux-kernel,
	linux-arm-kernel

Remove reset stuff in flavour of using keystone reset driver:
driver/power/reset/keystone-reset.c

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 arch/arm/mach-keystone/keystone.c | 35 -----------------------------------
 1 file changed, 35 deletions(-)

diff --git a/arch/arm/mach-keystone/keystone.c b/arch/arm/mach-keystone/keystone.c
index e0b9e1b..c96e2a4 100644
--- a/arch/arm/mach-keystone/keystone.c
+++ b/arch/arm/mach-keystone/keystone.c
@@ -23,24 +23,8 @@
 
 #include "keystone.h"
 
-#define PLL_RESET_WRITE_KEY_MASK		0xffff0000
-#define PLL_RESET_WRITE_KEY			0x5a69
-#define PLL_RESET				BIT(16)
-
-static void __iomem *keystone_rstctrl;
-
 static void __init keystone_init(void)
 {
-	struct device_node *node;
-
-	node = of_find_compatible_node(NULL, NULL, "ti,keystone-reset");
-	if (WARN_ON(!node))
-		pr_warn("ti,keystone-reset node undefined\n");
-
-	keystone_rstctrl = of_iomap(node, 0);
-	if (WARN_ON(!keystone_rstctrl))
-		pr_warn("ti,keystone-reset iomap error\n");
-
 	keystone_pm_runtime_init();
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
@@ -50,24 +34,6 @@ static const char *keystone_match[] __initconst = {
 	NULL,
 };
 
-void keystone_restart(enum reboot_mode mode, const char *cmd)
-{
-	u32 val;
-
-	BUG_ON(!keystone_rstctrl);
-
-	/* Enable write access to RSTCTRL */
-	val = readl(keystone_rstctrl);
-	val &= PLL_RESET_WRITE_KEY_MASK;
-	val |= PLL_RESET_WRITE_KEY;
-	writel(val, keystone_rstctrl);
-
-	/* Reset the SOC */
-	val = readl(keystone_rstctrl);
-	val &= ~PLL_RESET;
-	writel(val, keystone_rstctrl);
-}
-
 DT_MACHINE_START(KEYSTONE, "Keystone")
 #if defined(CONFIG_ZONE_DMA) && defined(CONFIG_ARM_LPAE)
 	.dma_zone_size	= SZ_2G,
@@ -75,5 +41,4 @@ DT_MACHINE_START(KEYSTONE, "Keystone")
 	.smp		= smp_ops(keystone_smp_ops),
 	.init_machine	= keystone_init,
 	.dt_compat	= keystone_match,
-	.restart	= keystone_restart,
 MACHINE_END
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/5] ARM: dts: keystone: update reset node to work with reset driver
  2014-04-14 17:41 [PATCH v2 0/5] Introduce keystone reset driver Ivan Khoronzhuk
                   ` (2 preceding siblings ...)
  2014-04-14 17:41 ` [PATCH v2 3/5] ARM: keystone: remove redundant reset stuff Ivan Khoronzhuk
@ 2014-04-14 17:41 ` Ivan Khoronzhuk
  2014-04-14 17:41 ` [PATCH v2 5/5] ARM: keystone: enable reset driver support Ivan Khoronzhuk
  4 siblings, 0 replies; 9+ messages in thread
From: Ivan Khoronzhuk @ 2014-04-14 17:41 UTC (permalink / raw)
  To: dbaryshkov, dwmw2, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, santosh.shilimkar, grant.likely
  Cc: rdunlap, linux, ivan.khoronzhuk, grygorii.strashko, olof,
	w-kwok2, sboyd, devicetree, linux-doc, linux-kernel,
	linux-arm-kernel

The reset controller registers are part of the PLL Controller MMRs.
According to TRM there are the following registers:
RSTYPE, RSCTRL, RSCFG and RSISO. Currently declared only one of them,
but that is not enough to correctly setup reset properties, so add
whole range of pll registers - pllregs.

Also add range for reset multiplex registers for SoC on the device.
These registers are located in Bootcfg memory space and needed
to setup behaviour after appropriate watchdog is triggered.

Add "ti,wdt_list" option to declare what watchdog are used to reboot
the SoC.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 arch/arm/boot/dts/keystone.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index 90823eb..9d6b850 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -69,7 +69,9 @@
 
 		rstctrl: reset-controller {
 			compatible = "ti,keystone-reset";
-			reg = <0x023100e8 4>;	/* pll reset control reg */
+			reg = <0x23100e4 0x10>, <0x2620328 0x10>;
+			reg-names = "pllregs", "muxregs";
+			ti,wdt_list = <0>;
 		};
 
 		/include/ "keystone-clocks.dtsi"
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 5/5] ARM: keystone: enable reset driver support
  2014-04-14 17:41 [PATCH v2 0/5] Introduce keystone reset driver Ivan Khoronzhuk
                   ` (3 preceding siblings ...)
  2014-04-14 17:41 ` [PATCH v2 4/5] ARM: dts: keystone: update reset node to work with reset driver Ivan Khoronzhuk
@ 2014-04-14 17:41 ` Ivan Khoronzhuk
  4 siblings, 0 replies; 9+ messages in thread
From: Ivan Khoronzhuk @ 2014-04-14 17:41 UTC (permalink / raw)
  To: dbaryshkov, dwmw2, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, santosh.shilimkar, grant.likely
  Cc: rdunlap, linux, ivan.khoronzhuk, grygorii.strashko, olof,
	w-kwok2, sboyd, devicetree, linux-doc, linux-kernel,
	linux-arm-kernel

Enable reset driver support in order to have opportunity
to reboot SoC by watchdog and by software.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 arch/arm/configs/keystone_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/configs/keystone_defconfig b/arch/arm/configs/keystone_defconfig
index ec9a41d..86f02cb 100644
--- a/arch/arm/configs/keystone_defconfig
+++ b/arch/arm/configs/keystone_defconfig
@@ -131,6 +131,9 @@ CONFIG_SPI=y
 CONFIG_SPI_DAVINCI=y
 CONFIG_SPI_SPIDEV=y
 # CONFIG_HWMON is not set
+CONFIG_POWER_SUPPLY=y
+CONFIG_POWER_RESET=y
+CONFIG_POWER_RESET_KEYSTONE=y
 CONFIG_WATCHDOG=y
 CONFIG_WATCHDOG_CORE=y
 CONFIG_DAVINCI_WATCHDOG=y
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/5] Power: reset: add bindings for keystone reset driver
  2014-04-14 17:41 ` [PATCH v2 2/5] Power: reset: add bindings for " Ivan Khoronzhuk
@ 2014-04-14 18:44   ` Arnd Bergmann
  2014-04-15 11:25     ` Ivan Khoronzhuk
  0 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2014-04-14 18:44 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Ivan Khoronzhuk, dbaryshkov, dwmw2, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, santosh.shilimkar,
	grant.likely, devicetree, grygorii.strashko, linux, linux-doc,
	w-kwok2, rdunlap, sboyd, linux-kernel, olof

On Monday 14 April 2014 20:41:20 Ivan Khoronzhuk wrote:
> +Optional properties:
> +
> +- ti,soft-reset:       Boolean option indicating soft reset.
> +                       By default hard reset is used.
> +
> +- ti,wdt_list:         WDT list that can cause SoC reset.
> +                       The list in format: <0>, <2>;
> +                       Begins from 0 to 3, as keystone can contain up
> +                       to 4 SoC reset watchdogs.
> 

This looks like your binding just describes a subset of the
watchdog timer registers. If so, don't do a standalone reset
driver, but instead do a watchdog driver that can also be
used for reset, and have a binding that properly describes
the watchdog hardware.

It is bad to have overlapping register ranges between logical
devices, and it's also generally wrong to describe devices that
are not actually there: The hardware contains a watchdog, not
a system-reset device, so you should not make one up because
it seems easier given the Linux driver model.

	Arnd

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/5] Power: reset: add bindings for keystone reset driver
  2014-04-14 18:44   ` Arnd Bergmann
@ 2014-04-15 11:25     ` Ivan Khoronzhuk
  2014-05-05 18:53       ` Ivan Khoronzhuk
  0 siblings, 1 reply; 9+ messages in thread
From: Ivan Khoronzhuk @ 2014-04-15 11:25 UTC (permalink / raw)
  To: Arnd Bergmann, linux-arm-kernel
  Cc: dbaryshkov, dwmw2, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, santosh.shilimkar, grant.likely,
	devicetree, grygorii.strashko, linux, linux-doc, w-kwok2,
	rdunlap, sboyd, linux-kernel, olof


On 04/14/2014 09:44 PM, Arnd Bergmann wrote:
> On Monday 14 April 2014 20:41:20 Ivan Khoronzhuk wrote:
>> +Optional properties:
>> +
>> +- ti,soft-reset:       Boolean option indicating soft reset.
>> +                       By default hard reset is used.
>> +
>> +- ti,wdt_list:         WDT list that can cause SoC reset.
>> +                       The list in format: <0>, <2>;
>> +                       Begins from 0 to 3, as keystone can contain up
>> +                       to 4 SoC reset watchdogs.
>>
> This looks like your binding just describes a subset of the
> watchdog timer registers. If so, don't do a standalone reset

The registers are not a subset of watchdog hardware it's SoC specific future
controlled by SoC specific registers (bootregs and PLL regs).

For watchog IP setup, the Keystone uses the watchdog driver common with 
other
SoCs -- davinci_watchdog that is not depend on other SoC settings like 
this driver does.

The Keystone SoCs have separate registers to tune Keystone2 reset 
functionality
by configuring Reset multiplexer &  PLL. And it tunes not only watchdog 
usage.
The keystone SoC can be rebooted in several ways. By external reset pin, 
by soft and
by watchdogs. This driver allows software reset or reset by one of the 
watchdogs
(and other settings) independently on watchdog driver settings. This is 
job of reset driver.

> It's
> driver, but instead do a watchdog driver that can also be
> used for reset, and have a binding that properly describes
> the watchdog hardware.
>
> It is bad to have overlapping register ranges between logical

WDT doesn't overlap with this driver.

> devices, and it's also generally wrong to describe devices that
> are not actually there: The hardware contains a watchdog, not
> a system-reset device, so you should not make one up because
> it seems easier given the Linux driver model.
>
> 	Arnd



-- 
Regards,
Ivan Khoronzhuk


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/5] Power: reset: add bindings for keystone reset driver
  2014-04-15 11:25     ` Ivan Khoronzhuk
@ 2014-05-05 18:53       ` Ivan Khoronzhuk
  0 siblings, 0 replies; 9+ messages in thread
From: Ivan Khoronzhuk @ 2014-05-05 18:53 UTC (permalink / raw)
  To: Arnd Bergmann, linux-arm-kernel
  Cc: dbaryshkov, dwmw2, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, santosh.shilimkar, grant.likely,
	devicetree, grygorii.strashko, linux, linux-doc, w-kwok2,
	rdunlap, sboyd, linux-kernel, olof


On 04/15/2014 02:25 PM, Ivan Khoronzhuk wrote:
>
> On 04/14/2014 09:44 PM, Arnd Bergmann wrote:
>> On Monday 14 April 2014 20:41:20 Ivan Khoronzhuk wrote:
>>> +Optional properties:
>>> +
>>> +- ti,soft-reset:       Boolean option indicating soft reset.
>>> +                       By default hard reset is used.
>>> +
>>> +- ti,wdt_list:         WDT list that can cause SoC reset.
>>> +                       The list in format: <0>, <2>;
>>> +                       Begins from 0 to 3, as keystone can contain up
>>> +                       to 4 SoC reset watchdogs.
>>>
>> This looks like your binding just describes a subset of the
>> watchdog timer registers. If so, don't do a standalone reset
>
> The registers are not a subset of watchdog hardware it's SoC specific 
> future
> controlled by SoC specific registers (bootregs and PLL regs).
>
> For watchog IP setup, the Keystone uses the watchdog driver common 
> with other
> SoCs -- davinci_watchdog that is not depend on other SoC settings like 
> this driver does.
>
> The Keystone SoCs have separate registers to tune Keystone2 reset 
> functionality
> by configuring Reset multiplexer &  PLL. And it tunes not only 
> watchdog usage.
> The keystone SoC can be rebooted in several ways. By external reset 
> pin, by soft and
> by watchdogs. This driver allows software reset or reset by one of the 
> watchdogs
> (and other settings) independently on watchdog driver settings. This 
> is job of reset driver.
>
>> It's
>> driver, but instead do a watchdog driver that can also be
>> used for reset, and have a binding that properly describes
>> the watchdog hardware.
>>
>> It is bad to have overlapping register ranges between logical
>
> WDT doesn't overlap with this driver.
>
>> devices, and it's also generally wrong to describe devices that
>> are not actually there: The hardware contains a watchdog, not
>> a system-reset device, so you should not make one up because
>> it seems easier given the Linux driver model.
>>
>>     Arnd 

Hi Arnd,

Could I do smth additional to make bindings explanation more clear?
Or this is enough?

I can write like the following:
Optional properties:

- ti,soft-reset:       Boolean option indicating soft reset.
                        By default hard reset is used.

- ti,wdt_list:         WDT list that can cause SoC reset. This is not
                         related to WDT driver, it's just needed to enable
                         a SoC related reset that is triggered by one of 
watchdogs.
                        The list in format: <0>, <2>;
                        Begins from 0 to 3, as keystone can contain up
                        to 4 SoC reset watchdogs. Can be in random order.

That is OK?

-- 
Regards,
Ivan Khoronzhuk


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-05-05 18:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-14 17:41 [PATCH v2 0/5] Introduce keystone reset driver Ivan Khoronzhuk
2014-04-14 17:41 ` [PATCH v2 1/5] Power: reset: keystone-reset: introduce " Ivan Khoronzhuk
2014-04-14 17:41 ` [PATCH v2 2/5] Power: reset: add bindings for " Ivan Khoronzhuk
2014-04-14 18:44   ` Arnd Bergmann
2014-04-15 11:25     ` Ivan Khoronzhuk
2014-05-05 18:53       ` Ivan Khoronzhuk
2014-04-14 17:41 ` [PATCH v2 3/5] ARM: keystone: remove redundant reset stuff Ivan Khoronzhuk
2014-04-14 17:41 ` [PATCH v2 4/5] ARM: dts: keystone: update reset node to work with reset driver Ivan Khoronzhuk
2014-04-14 17:41 ` [PATCH v2 5/5] ARM: keystone: enable reset driver support Ivan Khoronzhuk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).