[PATCH 00/10] power/reset: Register drivers with restart handler

[PATCH 00/10] power/reset: Register drivers with restart handler

[Guenter Roeck]

Convert drivers to use the kernel restart handler instead of setting
arm_pm_restart directly.

This patch set depends on the kernel restart handler patchset submitted
earlier.

Patch 01/10 was tested with qemu. All other patches were compile tested only.

Some of the restart handlers loop forever after the reset instruction was
executed. It might makes sense to use mdelay() instead and return if resetting
the system failed. I did not implement that since I do not know what reasonable
delays would be.

The series does not include drivers to be introduced in the next commit window
(at91, versatile). Those can be converted later. I also did not convert the
sun6i driver, in the assumption that it will be removed since the sunxi
watchdog driver will register a restart handler for the architecture.

[PATCH 01/10] power/reset: vexpress: Register with kernel restart handler

[Guenter Roeck]

Use the kernel restart handler instead of setting arm_pm_restart directly.
This allows for more than one restart handler in the system.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/power/reset/vexpress-poweroff.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/power/reset/vexpress-poweroff.c b/drivers/power/reset/vexpress-poweroff.c
index 4dc102e2..03959ba 100644
--- a/drivers/power/reset/vexpress-poweroff.c
+++ b/drivers/power/reset/vexpress-poweroff.c
@@ -12,14 +12,14 @@
  */
 
 #include <linux/delay.h>
+#include <linux/notifier.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/reboot.h>
 #include <linux/stat.h>
 #include <linux/vexpress.h>
 
-#include <asm/system_misc.h>
-
 static void vexpress_reset_do(struct device *dev, const char *what)
 {
 	int err = -ENOENT;
@@ -43,11 +43,19 @@ static void vexpress_power_off(void)
 
 static struct device *vexpress_restart_device;
 
-static void vexpress_restart(enum reboot_mode reboot_mode, const char *cmd)
+static int vexpress_restart(struct notifier_block *this, unsigned long mode,
+			     void *cmd)
 {
 	vexpress_reset_do(vexpress_restart_device, "restart");
+
+	return NOTIFY_DONE;
 }
 
+static struct notifier_block vexpress_restart_nb = {
+	.notifier_call = vexpress_restart,
+	.priority = 128,
+};
+
 static ssize_t vexpress_reset_active_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
@@ -86,6 +94,17 @@ static struct of_device_id vexpress_reset_of_match[] = {
 	{}
 };
 
+static void _vexpress_register_restart_handler(struct device *dev)
+{
+	int err;
+
+	vexpress_restart_device = dev;
+	err = register_restart_handler(&vexpress_restart_nb);
+	if (err)
+		dev_err(dev, "cannot register restart handler (err=%d)\n", err);
+	device_create_file(dev, &dev_attr_active);
+}
+
 static int vexpress_reset_probe(struct platform_device *pdev)
 {
 	enum vexpress_reset_func func;
@@ -110,14 +129,10 @@ static int vexpress_reset_probe(struct platform_device *pdev)
 		break;
 	case FUNC_RESET:
 		if (!vexpress_restart_device)
-			vexpress_restart_device = &pdev->dev;
-		arm_pm_restart = vexpress_restart;
-		device_create_file(&pdev->dev, &dev_attr_active);
+			_vexpress_register_restart_handler(&pdev->dev);
 		break;
 	case FUNC_REBOOT:
-		vexpress_restart_device = &pdev->dev;
-		arm_pm_restart = vexpress_restart;
-		device_create_file(&pdev->dev, &dev_attr_active);
+		_vexpress_register_restart_handler(&pdev->dev);
 		break;
 	};
 
-- 
1.9.1

[PATCH 02/10] power/reset: xgene: Return -ENOMEM if out of memory

[Guenter Roeck]

It is customary to return an error code of -ENOMEM if the system
is out of memory. Also, in that case, the infrastructure will report
an error, so it is unnecessary to report it again.

Cc: Loc Ho <lho@apm.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/power/reset/xgene-reboot.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/power/reset/xgene-reboot.c b/drivers/power/reset/xgene-reboot.c
index ecd55f8..20cf7c3 100644
--- a/drivers/power/reset/xgene-reboot.c
+++ b/drivers/power/reset/xgene-reboot.c
@@ -61,10 +61,8 @@ static int xgene_reboot_probe(struct platform_device *pdev)
 	struct xgene_reboot_context *ctx;
 
 	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
-	if (!ctx) {
-		dev_err(&pdev->dev, "out of memory for context\n");
-		return -ENODEV;
-	}
+	if (!ctx)
+		return -ENOMEM;
 
 	ctx->csr = of_iomap(pdev->dev.of_node, 0);
 	if (!ctx->csr) {
-- 
1.9.1

[PATCH 03/10] power/reset: xgene: Drop devm_kfree

[Guenter Roeck]

Calling devm_kfree is unnecessary. Drop it.

Cc: Loc Ho <lho@apm.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/power/reset/xgene-reboot.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/power/reset/xgene-reboot.c b/drivers/power/reset/xgene-reboot.c
index 20cf7c3..aac0287 100644
--- a/drivers/power/reset/xgene-reboot.c
+++ b/drivers/power/reset/xgene-reboot.c
@@ -66,7 +66,6 @@ static int xgene_reboot_probe(struct platform_device *pdev)
 
 	ctx->csr = of_iomap(pdev->dev.of_node, 0);
 	if (!ctx->csr) {
-		devm_kfree(&pdev->dev, ctx);
 		dev_err(&pdev->dev, "can not map resource\n");
 		return -ENODEV;
 	}
-- 
1.9.1

[PATCH 04/10] power/reset: xgene: Use local variable dev instead of pdev->dev

[Guenter Roeck]

Using a local variable dev to point to the device is simpler then repeatedly
dereferencing pdev->dev.

Cc: Loc Ho <lho@apm.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/power/reset/xgene-reboot.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/power/reset/xgene-reboot.c b/drivers/power/reset/xgene-reboot.c
index aac0287..1bab003 100644
--- a/drivers/power/reset/xgene-reboot.c
+++ b/drivers/power/reset/xgene-reboot.c
@@ -33,7 +33,7 @@
 #include <asm/system_misc.h>
 
 struct xgene_reboot_context {
-	struct platform_device *pdev;
+	struct device *dev;
 	void *csr;
 	u32 mask;
 };
@@ -53,27 +53,28 @@ static void xgene_restart(char str, const char *cmd)
 	while (time_before(jiffies, timeout))
 		cpu_relax();
 
-	dev_emerg(&ctx->pdev->dev, "Unable to restart system\n");
+	dev_emerg(ctx->dev, "Unable to restart system\n");
 }
 
 static int xgene_reboot_probe(struct platform_device *pdev)
 {
 	struct xgene_reboot_context *ctx;
+	struct device *dev = &pdev->dev;
 
-	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
 
-	ctx->csr = of_iomap(pdev->dev.of_node, 0);
+	ctx->csr = of_iomap(dev->of_node, 0);
 	if (!ctx->csr) {
-		dev_err(&pdev->dev, "can not map resource\n");
+		dev_err(dev, "can not map resource\n");
 		return -ENODEV;
 	}
 
-	if (of_property_read_u32(pdev->dev.of_node, "mask", &ctx->mask))
+	if (of_property_read_u32(dev->of_node, "mask", &ctx->mask))
 		ctx->mask = 0xFFFFFFFF;
 
-	ctx->pdev = pdev;
+	ctx->dev = dev;
 	arm_pm_restart = xgene_restart;
 	xgene_restart_ctx = ctx;
 
-- 
1.9.1

[PATCH 05/10] power/reset: xgene: Use mdelay instead of jiffies based timeout

[Guenter Roeck]

jiffies are not running at this stage of system shutdown, meaning an
error in the reset function would never be reported. Replace with mdelay().

Cc: Loc Ho <lho@apm.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/power/reset/xgene-reboot.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/power/reset/xgene-reboot.c b/drivers/power/reset/xgene-reboot.c
index 1bab003..ae470b8 100644
--- a/drivers/power/reset/xgene-reboot.c
+++ b/drivers/power/reset/xgene-reboot.c
@@ -24,6 +24,7 @@
  * For system shutdown, this is board specify. If a board designer
  * implements GPIO shutdown, use the gpio-poweroff.c driver.
  */
+#include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/of_device.h>
 #include <linux/of_address.h>
@@ -43,15 +44,12 @@ static struct xgene_reboot_context *xgene_restart_ctx;
 static void xgene_restart(char str, const char *cmd)
 {
 	struct xgene_reboot_context *ctx = xgene_restart_ctx;
-	unsigned long timeout;
 
 	/* Issue the reboot */
 	if (ctx)
 		writel(ctx->mask, ctx->csr);
 
-	timeout = jiffies + HZ;
-	while (time_before(jiffies, timeout))
-		cpu_relax();
+	mdelay(1000);
 
 	dev_emerg(ctx->dev, "Unable to restart system\n");
 }
-- 
1.9.1

[PATCH 06/10] power/reset: xgene: Register with kernel restart handler

[Guenter Roeck]

Register with kernel restart handler instead of setting arm_pm_restart directly.

This patch also addresses the following compile warning.

drivers/power/reset/xgene-reboot.c: In function 'xgene_reboot_probe':
drivers/power/reset/xgene-reboot.c:77:17: warning:
	assignment from incompatible pointer type [enabled by default]

The warning was due to a mismatch between the type of arm_pm_restart
and the restart function.

Cc: Loc Ho <lho@apm.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/power/reset/xgene-reboot.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/power/reset/xgene-reboot.c b/drivers/power/reset/xgene-reboot.c
index ae470b8..1e7f5f5 100644
--- a/drivers/power/reset/xgene-reboot.c
+++ b/drivers/power/reset/xgene-reboot.c
@@ -26,32 +26,36 @@
  */
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/notifier.h>
 #include <linux/of_device.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
+#include <linux/reboot.h>
 #include <linux/stat.h>
 #include <linux/slab.h>
-#include <asm/system_misc.h>
 
 struct xgene_reboot_context {
 	struct device *dev;
 	void *csr;
 	u32 mask;
+	struct notifier_block restart_handler;
 };
 
-static struct xgene_reboot_context *xgene_restart_ctx;
-
-static void xgene_restart(char str, const char *cmd)
+static int xgene_restart_handler(struct notifier_block *this,
+				 unsigned long mode, void *cmd)
 {
-	struct xgene_reboot_context *ctx = xgene_restart_ctx;
+	struct xgene_reboot_context *ctx =
+		container_of(this, struct xgene_reboot_context,
+			     restart_handler);
 
 	/* Issue the reboot */
-	if (ctx)
-		writel(ctx->mask, ctx->csr);
+	writel(ctx->mask, ctx->csr);
 
 	mdelay(1000);
 
 	dev_emerg(ctx->dev, "Unable to restart system\n");
+
+	return NOTIFY_DONE;
 }
 
 static int xgene_reboot_probe(struct platform_device *pdev)
@@ -73,8 +77,11 @@ static int xgene_reboot_probe(struct platform_device *pdev)
 		ctx->mask = 0xFFFFFFFF;
 
 	ctx->dev = dev;
-	arm_pm_restart = xgene_restart;
-	xgene_restart_ctx = ctx;
+	ctx->restart_handler.notifier_call = xgene_restart_handler;
+	ctx->restart_handler.priority = 128;
+	err = register_restart_handler(&ctx->restart_handler);
+	if (err)
+		dev_err(dev, "cannot register restart handler (err=%d)\n", err);
 
 	return 0;
 }
-- 
1.9.1

[PATCH 07/10] power/reset: axxia: Register with kernel restart handler

[Guenter Roeck]

Register with kernel restart handler instead of setting arm_pm_restart
directly.

Cc: Anders Berg <anders.berg@lsi.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/power/reset/axxia-reset.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/power/reset/axxia-reset.c b/drivers/power/reset/axxia-reset.c
index 3b1f8d6..2a772d9 100644
--- a/drivers/power/reset/axxia-reset.c
+++ b/drivers/power/reset/axxia-reset.c
@@ -19,14 +19,12 @@
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/reboot.h>
 #include <linux/regmap.h>
 
-#include <asm/system_misc.h>
-
-
 #define SC_CRIT_WRITE_KEY	0x1000
 #define SC_LATCH_ON_RESET	0x1004
 #define SC_RESET_CONTROL	0x1008
@@ -39,7 +37,8 @@
 
 static struct regmap *syscon;
 
-static void do_axxia_restart(enum reboot_mode reboot_mode, const char *cmd)
+static int axxia_restart_handler(struct notifier_block *this,
+				 unsigned long mode, void *cmd)
 {
 	/* Access Key (0xab) */
 	regmap_write(syscon, SC_CRIT_WRITE_KEY, 0xab);
@@ -50,11 +49,19 @@ static void do_axxia_restart(enum reboot_mode reboot_mode, const char *cmd)
 	/* Assert chip reset */
 	regmap_update_bits(syscon, SC_RESET_CONTROL,
 			   RSTCTL_RST_CHIP, RSTCTL_RST_CHIP);
+
+	return NOTIFY_DONE;
 }
 
+static struct notifier_block axxia_restart_nb = {
+	.notifier_call = axxia_restart_handler,
+	.priority = 128,
+};
+
 static int axxia_reset_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	int err;
 
 	syscon = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
 	if (IS_ERR(syscon)) {
@@ -62,7 +69,9 @@ static int axxia_reset_probe(struct platform_device *pdev)
 		return PTR_ERR(syscon);
 	}
 
-	arm_pm_restart = do_axxia_restart;
+	err = register_restart_handler(&axxia_restart_nb);
+	if (err)
+		dev_err(dev, "cannot register restart handler (err=%d)\n", err);
 
 	return 0;
 }
-- 
1.9.1

[PATCH 08/10] power/reset: keystone: Register with kernel restart handler

[Guenter Roeck]

Register with kernel restart handler instead of setting arm_pm_restart directly.

Move notifier registration to the end of the probe function to avoid having to
implement error handling.

Cc: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/power/reset/keystone-reset.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/power/reset/keystone-reset.c b/drivers/power/reset/keystone-reset.c
index 408a18f..9d46586 100644
--- a/drivers/power/reset/keystone-reset.c
+++ b/drivers/power/reset/keystone-reset.c
@@ -12,9 +12,9 @@
 
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 #include <linux/reboot.h>
 #include <linux/regmap.h>
-#include <asm/system_misc.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_platform.h>
 
@@ -52,7 +52,8 @@ static inline int rsctrl_enable_rspll_write(void)
 				  RSCTRL_KEY_MASK, RSCTRL_KEY);
 }
 
-static void rsctrl_restart(enum reboot_mode mode, const char *cmd)
+static int rsctrl_restart_handler(struct notifier_block *this,
+				  unsigned long mode, void *cmd)
 {
 	/* enable write access to RSTCTRL */
 	rsctrl_enable_rspll_write();
@@ -60,8 +61,15 @@ static void rsctrl_restart(enum reboot_mode mode, const char *cmd)
 	/* reset the SOC */
 	regmap_update_bits(pllctrl_regs, rspll_offset + RSCTRL_RG,
 			   RSCTRL_RESET_MASK, 0);
+
+	return NOTIFY_DONE;
 }
 
+static struct notifier_block rsctrl_restart_nb = {
+	.notifier_call = rsctrl_restart_handler,
+	.priority = 128,
+};
+
 static struct of_device_id rsctrl_of_match[] = {
 	{.compatible = "ti,keystone-reset", },
 	{},
@@ -114,8 +122,6 @@ static int rsctrl_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	arm_pm_restart = rsctrl_restart;
-
 	/* disable a reset isolation for all module clocks */
 	ret = regmap_write(pllctrl_regs, rspll_offset + RSISO_RG, 0);
 	if (ret)
@@ -147,6 +153,10 @@ static int rsctrl_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	ret = register_restart_handler(&rsctrl_restart_nb);
+	if (ret)
+		dev_err(dev, "cannot register restart handler (err=%d)\n", ret);
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH 09/10] power/reset: hisi: Register with kernel restart handler

[Guenter Roeck]

Register with kernel restart handler instead of setting arm_pm_restart directly.

Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/power/reset/hisi-reboot.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/power/reset/hisi-reboot.c b/drivers/power/reset/hisi-reboot.c
index 0c91d02..e9774b0 100644
--- a/drivers/power/reset/hisi-reboot.c
+++ b/drivers/power/reset/hisi-reboot.c
@@ -14,27 +14,36 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/reboot.h>
 
 #include <asm/proc-fns.h>
-#include <asm/system_misc.h>
 
 static void __iomem *base;
 static u32 reboot_offset;
 
-static void hisi_restart(enum reboot_mode mode, const char *cmd)
+static int hisi_restart_handler(struct notifier_block *this,
+				unsigned long mode, void *cmd)
 {
 	writel_relaxed(0xdeadbeef, base + reboot_offset);
 
 	while (1)
 		cpu_do_idle();
+
+	return NOTIFY_DONE;
 }
 
+static struct notifier_block hisi_restart_nb = {
+	.notifier_call = hisi_restart_handler,
+	.priority = 128,
+};
+
 static int hisi_reboot_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	int err;
 
 	base = of_iomap(np, 0);
 	if (!base) {
@@ -47,7 +56,10 @@ static int hisi_reboot_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	arm_pm_restart = hisi_restart;
+	err = register_restart_handler(&hisi_restart_nb);
+	if (err)
+		dev_err(&pdev->dev, "cannot register restart handler (err=%d)\n",
+			err);
 
 	return 0;
 }
-- 
1.9.1

[PATCH 10/10] power/reset: brcmstb: Register with kernel restart handler

[Guenter Roeck]

Register with kernel restart handler instead of setting arm_pm_restart directly.

Cc: Marc Carino <marc.ceeeee@gmail.com>
Cc: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/power/reset/brcmstb-reboot.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
index 3f23692..3306241 100644
--- a/drivers/power/reset/brcmstb-reboot.c
+++ b/drivers/power/reset/brcmstb-reboot.c
@@ -16,6 +16,7 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/jiffies.h>
+#include <linux/notifier.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
@@ -26,8 +27,6 @@
 #include <linux/smp.h>
 #include <linux/mfd/syscon.h>
 
-#include <asm/system_misc.h>
-
 #define RESET_SOURCE_ENABLE_REG 1
 #define SW_MASTER_RESET_REG 2
 
@@ -35,7 +34,8 @@ static struct regmap *regmap;
 static u32 rst_src_en;
 static u32 sw_mstr_rst;
 
-static void brcmstb_reboot(enum reboot_mode mode, const char *cmd)
+static int brcmstb_restart_handler(struct notifier_block *this,
+				   unsigned long mode, void *cmd)
 {
 	int rc;
 	u32 tmp;
@@ -43,31 +43,38 @@ static void brcmstb_reboot(enum reboot_mode mode, const char *cmd)
 	rc = regmap_write(regmap, rst_src_en, 1);
 	if (rc) {
 		pr_err("failed to write rst_src_en (%d)\n", rc);
-		return;
+		return NOTIFY_DONE;
 	}
 
 	rc = regmap_read(regmap, rst_src_en, &tmp);
 	if (rc) {
 		pr_err("failed to read rst_src_en (%d)\n", rc);
-		return;
+		return NOTIFY_DONE;
 	}
 
 	rc = regmap_write(regmap, sw_mstr_rst, 1);
 	if (rc) {
 		pr_err("failed to write sw_mstr_rst (%d)\n", rc);
-		return;
+		return NOTIFY_DONE;
 	}
 
 	rc = regmap_read(regmap, sw_mstr_rst, &tmp);
 	if (rc) {
 		pr_err("failed to read sw_mstr_rst (%d)\n", rc);
-		return;
+		return NOTIFY_DONE;
 	}
 
 	while (1)
 		;
+
+	return NOTIFY_DONE;
 }
 
+static struct notifier_block brcmstb_restart_nb = {
+	.notifier_call = brcmstb_restart_handler,
+	.priority = 128,
+};
+
 static int brcmstb_reboot_probe(struct platform_device *pdev)
 {
 	int rc;
@@ -93,7 +100,10 @@ static int brcmstb_reboot_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	arm_pm_restart = brcmstb_reboot;
+	rc = register_restart_handler(&brcmstb_restart_nb);
+	if (rc)
+		dev_err(&pdev->dev,
+			"cannot register restart handler (err=%d)\n", rc);
 
 	return 0;
 }
-- 
1.9.1

Re: [PATCH 08/10] power/reset: keystone: Register with kernel restart handler

[Santosh Shilimkar]

On Friday 26 September 2014 08:57 PM, Guenter Roeck wrote:
> Register with kernel restart handler instead of setting arm_pm_restart directly.
> 
> Move notifier registration to the end of the probe function to avoid having to
> implement error handling.
> 
> Cc: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
Looks ok for me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Ivan,
Can you please just test it once ? Thanks !!

Regards,
Santosh

Re: [PATCH 07/10] power/reset: axxia: Register with kernel restart handler

[Anders Berg]

On Sat, Sep 27, 2014 at 2:57 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> Register with kernel restart handler instead of setting arm_pm_restart
> directly.
>
> Cc: Anders Berg <anders.berg@lsi.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/power/reset/axxia-reset.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>

Works fine.

Tested-by: Anders Berg <anders.berg@avagotech.com>

/Anders

> diff --git a/drivers/power/reset/axxia-reset.c b/drivers/power/reset/axxia-reset.c
> index 3b1f8d6..2a772d9 100644
> --- a/drivers/power/reset/axxia-reset.c
> +++ b/drivers/power/reset/axxia-reset.c
> @@ -19,14 +19,12 @@
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
> +#include <linux/notifier.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/reboot.h>
>  #include <linux/regmap.h>
>
> -#include <asm/system_misc.h>
> -
> -
>  #define SC_CRIT_WRITE_KEY      0x1000
>  #define SC_LATCH_ON_RESET      0x1004
>  #define SC_RESET_CONTROL       0x1008
> @@ -39,7 +37,8 @@
>
>  static struct regmap *syscon;
>
> -static void do_axxia_restart(enum reboot_mode reboot_mode, const char *cmd)
> +static int axxia_restart_handler(struct notifier_block *this,
> +                                unsigned long mode, void *cmd)
>  {
>         /* Access Key (0xab) */
>         regmap_write(syscon, SC_CRIT_WRITE_KEY, 0xab);
> @@ -50,11 +49,19 @@ static void do_axxia_restart(enum reboot_mode reboot_mode, const char *cmd)
>         /* Assert chip reset */
>         regmap_update_bits(syscon, SC_RESET_CONTROL,
>                            RSTCTL_RST_CHIP, RSTCTL_RST_CHIP);
> +
> +       return NOTIFY_DONE;
>  }
>
> +static struct notifier_block axxia_restart_nb = {
> +       .notifier_call = axxia_restart_handler,
> +       .priority = 128,
> +};
> +
>  static int axxia_reset_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
> +       int err;
>
>         syscon = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
>         if (IS_ERR(syscon)) {
> @@ -62,7 +69,9 @@ static int axxia_reset_probe(struct platform_device *pdev)
>                 return PTR_ERR(syscon);
>         }
>
> -       arm_pm_restart = do_axxia_restart;
> +       err = register_restart_handler(&axxia_restart_nb);
> +       if (err)
> +               dev_err(dev, "cannot register restart handler (err=%d)\n", err);
>
>         return 0;
>  }
> --
> 1.9.1
>

Re: [PATCH 08/10] power/reset: keystone: Register with kernel restart handler

[Ivan Khoronzhuk]

On 09/27/2014 03:57 AM, Guenter Roeck wrote:
> Register with kernel restart handler instead of setting arm_pm_restart directly.
>
> Move notifier registration to the end of the probe function to avoid having to
> implement error handling.
>
> Cc: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>   drivers/power/reset/keystone-reset.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/power/reset/keystone-reset.c b/drivers/power/reset/keystone-reset.c
> index 408a18f..9d46586 100644
> --- a/drivers/power/reset/keystone-reset.c
> +++ b/drivers/power/reset/keystone-reset.c
> @@ -12,9 +12,9 @@
>   
>   #include <linux/io.h>
>   #include <linux/module.h>
> +#include <linux/notifier.h>
>   #include <linux/reboot.h>
>   #include <linux/regmap.h>
> -#include <asm/system_misc.h>
>   #include <linux/mfd/syscon.h>
>   #include <linux/of_platform.h>
>   
> @@ -52,7 +52,8 @@ static inline int rsctrl_enable_rspll_write(void)
>   				  RSCTRL_KEY_MASK, RSCTRL_KEY);
>   }
>   
> -static void rsctrl_restart(enum reboot_mode mode, const char *cmd)
> +static int rsctrl_restart_handler(struct notifier_block *this,
> +				  unsigned long mode, void *cmd)
>   {
>   	/* enable write access to RSTCTRL */
>   	rsctrl_enable_rspll_write();
> @@ -60,8 +61,15 @@ static void rsctrl_restart(enum reboot_mode mode, const char *cmd)
>   	/* reset the SOC */
>   	regmap_update_bits(pllctrl_regs, rspll_offset + RSCTRL_RG,
>   			   RSCTRL_RESET_MASK, 0);
> +
> +	return NOTIFY_DONE;
>   }
>   
> +static struct notifier_block rsctrl_restart_nb = {
> +	.notifier_call = rsctrl_restart_handler,
> +	.priority = 128,
> +};
> +
>   static struct of_device_id rsctrl_of_match[] = {
>   	{.compatible = "ti,keystone-reset", },
>   	{},
> @@ -114,8 +122,6 @@ static int rsctrl_probe(struct platform_device *pdev)
>   	if (ret)
>   		return ret;
>   
> -	arm_pm_restart = rsctrl_restart;
> -
>   	/* disable a reset isolation for all module clocks */
>   	ret = regmap_write(pllctrl_regs, rspll_offset + RSISO_RG, 0);
>   	if (ret)
> @@ -147,6 +153,10 @@ static int rsctrl_probe(struct platform_device *pdev)
>   			return ret;
>   	}
>   
> +	ret = register_restart_handler(&rsctrl_restart_nb);
> +	if (ret)
> +		dev_err(dev, "cannot register restart handler (err=%d)\n", ret);

What about return ret? Even the register_restart_handler() always 
returns 0 currently.

> +
>   	return 0;
>   }
>   

Santosh,
I've tested it. That's fine.


-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH 08/10] power/reset: keystone: Register with kernel restart handler

[Ivan Khoronzhuk]

On 09/27/2014 03:57 AM, Guenter Roeck wrote:
> Register with kernel restart handler instead of setting arm_pm_restart directly.
>
> Move notifier registration to the end of the probe function to avoid having to
> implement error handling.
>
> Cc: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>   drivers/power/reset/keystone-reset.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/power/reset/keystone-reset.c b/drivers/power/reset/keystone-reset.c
> index 408a18f..9d46586 100644
> --- a/drivers/power/reset/keystone-reset.c
> +++ b/drivers/power/reset/keystone-reset.c
> @@ -12,9 +12,9 @@
>   
>   #include <linux/io.h>
>   #include <linux/module.h>
> +#include <linux/notifier.h>
>   #include <linux/reboot.h>
>   #include <linux/regmap.h>
> -#include <asm/system_misc.h>
>   #include <linux/mfd/syscon.h>
>   #include <linux/of_platform.h>
>   
> @@ -52,7 +52,8 @@ static inline int rsctrl_enable_rspll_write(void)
>   				  RSCTRL_KEY_MASK, RSCTRL_KEY);
>   }
>   
> -static void rsctrl_restart(enum reboot_mode mode, const char *cmd)
> +static int rsctrl_restart_handler(struct notifier_block *this,
> +				  unsigned long mode, void *cmd)
>   {
>   	/* enable write access to RSTCTRL */
>   	rsctrl_enable_rspll_write();
> @@ -60,8 +61,15 @@ static void rsctrl_restart(enum reboot_mode mode, const char *cmd)
>   	/* reset the SOC */
>   	regmap_update_bits(pllctrl_regs, rspll_offset + RSCTRL_RG,
>   			   RSCTRL_RESET_MASK, 0);
> +
> +	return NOTIFY_DONE;
>   }
>   
> +static struct notifier_block rsctrl_restart_nb = {
> +	.notifier_call = rsctrl_restart_handler,
> +	.priority = 128,
> +};
> +
>   static struct of_device_id rsctrl_of_match[] = {
>   	{.compatible = "ti,keystone-reset", },
>   	{},
> @@ -114,8 +122,6 @@ static int rsctrl_probe(struct platform_device *pdev)
>   	if (ret)
>   		return ret;
>   
> -	arm_pm_restart = rsctrl_restart;
> -
>   	/* disable a reset isolation for all module clocks */
>   	ret = regmap_write(pllctrl_regs, rspll_offset + RSISO_RG, 0);
>   	if (ret)
> @@ -147,6 +153,10 @@ static int rsctrl_probe(struct platform_device *pdev)
>   			return ret;
>   	}
>   
> +	ret = register_restart_handler(&rsctrl_restart_nb);
> +	if (ret)
> +		dev_err(dev, "cannot register restart handler (err=%d)\n", ret);

What about return ret? Even the register_restart_handler() always 
returns 0 currently.

> +
>   	return 0;
>   }
>   

Santosh,
I've tested it. That's fine.


-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH 08/10] power/reset: keystone: Register with kernel restart handler

[Guenter Roeck]

On Mon, Sep 29, 2014 at 04:53:14PM +0300, Ivan Khoronzhuk wrote:
> On 09/27/2014 03:57 AM, Guenter Roeck wrote:
> >Register with kernel restart handler instead of setting arm_pm_restart directly.
> >
> >Move notifier registration to the end of the probe function to avoid having to
> >implement error handling.
> >
> >Cc: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> >Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> >Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >---
> >  drivers/power/reset/keystone-reset.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/power/reset/keystone-reset.c b/drivers/power/reset/keystone-reset.c
> >index 408a18f..9d46586 100644
> >--- a/drivers/power/reset/keystone-reset.c
> >+++ b/drivers/power/reset/keystone-reset.c
> >@@ -12,9 +12,9 @@
> >  #include <linux/io.h>
> >  #include <linux/module.h>
> >+#include <linux/notifier.h>
> >  #include <linux/reboot.h>
> >  #include <linux/regmap.h>
> >-#include <asm/system_misc.h>
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/of_platform.h>
> >@@ -52,7 +52,8 @@ static inline int rsctrl_enable_rspll_write(void)
> >  				  RSCTRL_KEY_MASK, RSCTRL_KEY);
> >  }
> >-static void rsctrl_restart(enum reboot_mode mode, const char *cmd)
> >+static int rsctrl_restart_handler(struct notifier_block *this,
> >+				  unsigned long mode, void *cmd)
> >  {
> >  	/* enable write access to RSTCTRL */
> >  	rsctrl_enable_rspll_write();
> >@@ -60,8 +61,15 @@ static void rsctrl_restart(enum reboot_mode mode, const char *cmd)
> >  	/* reset the SOC */
> >  	regmap_update_bits(pllctrl_regs, rspll_offset + RSCTRL_RG,
> >  			   RSCTRL_RESET_MASK, 0);
> >+
> >+	return NOTIFY_DONE;
> >  }
> >+static struct notifier_block rsctrl_restart_nb = {
> >+	.notifier_call = rsctrl_restart_handler,
> >+	.priority = 128,
> >+};
> >+
> >  static struct of_device_id rsctrl_of_match[] = {
> >  	{.compatible = "ti,keystone-reset", },
> >  	{},
> >@@ -114,8 +122,6 @@ static int rsctrl_probe(struct platform_device *pdev)
> >  	if (ret)
> >  		return ret;
> >-	arm_pm_restart = rsctrl_restart;
> >-
> >  	/* disable a reset isolation for all module clocks */
> >  	ret = regmap_write(pllctrl_regs, rspll_offset + RSISO_RG, 0);
> >  	if (ret)
> >@@ -147,6 +153,10 @@ static int rsctrl_probe(struct platform_device *pdev)
> >  			return ret;
> >  	}
> >+	ret = register_restart_handler(&rsctrl_restart_nb);
> >+	if (ret)
> >+		dev_err(dev, "cannot register restart handler (err=%d)\n", ret);
> 
> What about return ret? Even the register_restart_handler() always
> returns 0 currently.
> 
Yes, you have a point. I'll redo the series and return an error in this case,
unless the driver has additional functionality (we don't want it to fail to
load if it also provides poweroff handling, for example).

Thanks,
Guenter