diff mbox

[5/5] mfd: rn5t618: register restart handler

Message ID 20160605001749.4062-7-stefan@agner.ch (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Stefan Agner June 5, 2016, 12:17 a.m. UTC
Use the PMIC's repower capability for reboots. Register a restart
handler with use a default priority of 128.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/mfd/rn5t618.c | 42 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

Comments

kernel test robot June 5, 2016, 1:24 a.m. UTC | #1
Hi,

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.7-rc1 next-20160603]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Stefan-Agner/regulator-add-Ricoh-RN5T567-PMIC-support/20160605-082219
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/mfd/rn5t618.c:24:29: fatal error: asm/system_misc.h: No such file or directory
    #include <asm/system_misc.h>
                                ^
   compilation terminated.

vim +24 drivers/mfd/rn5t618.c

    18	#include <linux/mfd/rn5t618.h>
    19	#include <linux/module.h>
    20	#include <linux/of_device.h>
    21	#include <linux/reboot.h>
    22	#include <linux/regmap.h>
    23	
  > 24	#include <asm/system_misc.h>
    25	
    26	static const struct mfd_cell rn5t618_cells[] = {
    27		{ .name = "rn5t618-regulator" },

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Stefan Agner June 5, 2016, 2:52 a.m. UTC | #2
On 2016-06-04 18:24, kbuild test robot wrote:
> Hi,
> 
> [auto build test ERROR on ljones-mfd/for-mfd-next]
> [also build test ERROR on v4.7-rc1 next-20160603]
> [if your patch is applied to the wrong git tree, please drop us a note
> to help improve the system]
> 
> url:   
> https://github.com/0day-ci/linux/commits/Stefan-Agner/regulator-add-Ricoh-RN5T567-PMIC-support/20160605-082219
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>>> drivers/mfd/rn5t618.c:24:29: fatal error: asm/system_misc.h: No such file or directory
>     #include <asm/system_misc.h>
>                                 ^
>    compilation terminated.
> 
> vim +24 drivers/mfd/rn5t618.c
> 
>     18	#include <linux/mfd/rn5t618.h>
>     19	#include <linux/module.h>
>     20	#include <linux/of_device.h>
>     21	#include <linux/reboot.h>
>     22	#include <linux/regmap.h>
>     23
>   > 24	#include <asm/system_misc.h>

Hm, that is an artifact of the older kernel version for which I wrote
the kernel first where this was required. Will remove this in v2.

--
Stefan

>     25
>     26	static const struct mfd_cell rn5t618_cells[] = {
>     27		{ .name = "rn5t618-regulator" },
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Marcel Ziswiler June 6, 2016, 1:25 p.m. UTC | #3
On June 5, 2016 2:17:49 AM GMT+02:00, Stefan Agner <stefan@agner.ch> wrote:
>Use the PMIC's repower capability for reboots. Register a restart
>handler with use a default priority of 128.

Apart from that last sentence above which is not clear to me the whole series looks fine to me.

Reviewed-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

>Signed-off-by: Stefan Agner <stefan@agner.ch>
>---
> drivers/mfd/rn5t618.c | 42 ++++++++++++++++++++++++++++++++++++------
> 1 file changed, 36 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
>index d9b4d40..d9bd61c 100644
>--- a/drivers/mfd/rn5t618.c
>+++ b/drivers/mfd/rn5t618.c
>@@ -12,13 +12,17 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
>  */
> 
>+#include <linux/delay.h>
> #include <linux/i2c.h>
> #include <linux/mfd/core.h>
> #include <linux/mfd/rn5t618.h>
> #include <linux/module.h>
> #include <linux/of_device.h>
>+#include <linux/reboot.h>
> #include <linux/regmap.h>
> 
>+#include <asm/system_misc.h>
>+
> static const struct mfd_cell rn5t618_cells[] = {
> 	{ .name = "rn5t618-regulator" },
> 	{ .name = "rn5t618-wdt" },
>@@ -50,17 +54,34 @@ static const struct regmap_config
>rn5t618_regmap_config = {
> };
> 
> static struct rn5t618 *rn5t618_pm_power_off;
>+static struct notifier_block rn5t618_restart_handler;
> 
>-static void rn5t618_power_off(void)
>+static void rn5t618_trigger_poweroff_sequence(bool repower)
> {
> 	/* disable automatic repower-on */
> 	regmap_update_bits(rn5t618_pm_power_off->regmap, RN5T618_REPCNT,
>-			   RN5T618_REPCNT_REPWRON, 0);
>+			   RN5T618_REPCNT_REPWRON, repower);
> 	/* start power-off sequence */
> 	regmap_update_bits(rn5t618_pm_power_off->regmap, RN5T618_SLPCNT,
> 			   RN5T618_SLPCNT_SWPWROFF, RN5T618_SLPCNT_SWPWROFF);
> }
> 
>+static void rn5t618_power_off(void)
>+{
>+	rn5t618_trigger_poweroff_sequence(false);
>+}
>+
>+static int rn5t618_restart(struct notifier_block *this,
>+			    unsigned long mode, void *cmd)
>+{
>+	rn5t618_trigger_poweroff_sequence(true);
>+
>+	mdelay(10);
>+
>+	return NOTIFY_DONE;
>+}
>+
>+
> static const struct of_device_id rn5t618_of_match[] = {
> 	{ .compatible = "ricoh,rn5t567", .data = (void *)RN5T567 },
> 	{ .compatible = "ricoh,rn5t618", .data = (void *)RN5T618 },
>@@ -103,13 +124,22 @@ static int rn5t618_i2c_probe(struct i2c_client
>*i2c,
> 		return ret;
> 	}
> 
>+	rn5t618_pm_power_off = priv;
> 	if (of_device_is_system_power_controller(i2c->dev.of_node)) {
>-		if (!pm_power_off) {
>-			rn5t618_pm_power_off = priv;
>+		if (!pm_power_off)
> 			pm_power_off = rn5t618_power_off;
>-		} else {
>+		else
>			dev_err(&i2c->dev, "Failed to set poweroff capability, already
>defined\n");
>-		}
>+	}
>+
>+	rn5t618_restart_handler.notifier_call = rn5t618_restart;
>+	rn5t618_restart_handler.priority = 128;
>+
>+	ret = register_restart_handler(&rn5t618_restart_handler);
>+	if (ret) {
>+		dev_err(&i2c->dev, "%s: cannot register restart handler, %d\n",
>+				__func__, ret);
>+		return ret;
> 	}
> 
> 	return 0;
Stefan Agner June 6, 2016, 5:19 p.m. UTC | #4
On 2016-06-06 06:25, Marcel Ziswiler wrote:
> On June 5, 2016 2:17:49 AM GMT+02:00, Stefan Agner <stefan@agner.ch> wrote:
>>Use the PMIC's repower capability for reboots. Register a restart
>>handler with use a default priority of 128.
> 
> Apart from that last sentence above which is not clear to me the whole
> series looks fine to me.

Check the kernel docs of the register_restart_handler:
http://lxr.free-electrons.com/source/kernel/reboot.c#L113

I am actually not sure anymore whether 128 is really a good choice. With
that, in the Colibri case, the restart handler would have the same
priority as the i.MX watchdog has (imx2_wdt.c), but the PMIC's restart
capabilities are clearly superior (as it resets all peripherals on the
module). In fact, using the watchdog to restart the system seems to hang
for some reason...

Maybe 192?

--
Stefan

> 
> Reviewed-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> 
>>Signed-off-by: Stefan Agner <stefan@agner.ch>
>>---
>> drivers/mfd/rn5t618.c | 42 ++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 36 insertions(+), 6 deletions(-)
>>
>>diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
>>index d9b4d40..d9bd61c 100644
>>--- a/drivers/mfd/rn5t618.c
>>+++ b/drivers/mfd/rn5t618.c
>>@@ -12,13 +12,17 @@
>> * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>  */
>>
>>+#include <linux/delay.h>
>> #include <linux/i2c.h>
>> #include <linux/mfd/core.h>
>> #include <linux/mfd/rn5t618.h>
>> #include <linux/module.h>
>> #include <linux/of_device.h>
>>+#include <linux/reboot.h>
>> #include <linux/regmap.h>
>>
>>+#include <asm/system_misc.h>
>>+
>> static const struct mfd_cell rn5t618_cells[] = {
>> 	{ .name = "rn5t618-regulator" },
>> 	{ .name = "rn5t618-wdt" },
>>@@ -50,17 +54,34 @@ static const struct regmap_config
>>rn5t618_regmap_config = {
>> };
>>
>> static struct rn5t618 *rn5t618_pm_power_off;
>>+static struct notifier_block rn5t618_restart_handler;
>>
>>-static void rn5t618_power_off(void)
>>+static void rn5t618_trigger_poweroff_sequence(bool repower)
>> {
>> 	/* disable automatic repower-on */
>> 	regmap_update_bits(rn5t618_pm_power_off->regmap, RN5T618_REPCNT,
>>-			   RN5T618_REPCNT_REPWRON, 0);
>>+			   RN5T618_REPCNT_REPWRON, repower);
>> 	/* start power-off sequence */
>> 	regmap_update_bits(rn5t618_pm_power_off->regmap, RN5T618_SLPCNT,
>> 			   RN5T618_SLPCNT_SWPWROFF, RN5T618_SLPCNT_SWPWROFF);
>> }
>>
>>+static void rn5t618_power_off(void)
>>+{
>>+	rn5t618_trigger_poweroff_sequence(false);
>>+}
>>+
>>+static int rn5t618_restart(struct notifier_block *this,
>>+			    unsigned long mode, void *cmd)
>>+{
>>+	rn5t618_trigger_poweroff_sequence(true);
>>+
>>+	mdelay(10);
>>+
>>+	return NOTIFY_DONE;
>>+}
>>+
>>+
>> static const struct of_device_id rn5t618_of_match[] = {
>> 	{ .compatible = "ricoh,rn5t567", .data = (void *)RN5T567 },
>> 	{ .compatible = "ricoh,rn5t618", .data = (void *)RN5T618 },
>>@@ -103,13 +124,22 @@ static int rn5t618_i2c_probe(struct i2c_client
>>*i2c,
>> 		return ret;
>> 	}
>>
>>+	rn5t618_pm_power_off = priv;
>> 	if (of_device_is_system_power_controller(i2c->dev.of_node)) {
>>-		if (!pm_power_off) {
>>-			rn5t618_pm_power_off = priv;
>>+		if (!pm_power_off)
>> 			pm_power_off = rn5t618_power_off;
>>-		} else {
>>+		else
>>			dev_err(&i2c->dev, "Failed to set poweroff capability, already
>>defined\n");
>>-		}
>>+	}
>>+
>>+	rn5t618_restart_handler.notifier_call = rn5t618_restart;
>>+	rn5t618_restart_handler.priority = 128;
>>+
>>+	ret = register_restart_handler(&rn5t618_restart_handler);
>>+	if (ret) {
>>+		dev_err(&i2c->dev, "%s: cannot register restart handler, %d\n",
>>+				__func__, ret);
>>+		return ret;
>> 	}
>>
>> 	return 0;
diff mbox

Patch

diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
index d9b4d40..d9bd61c 100644
--- a/drivers/mfd/rn5t618.c
+++ b/drivers/mfd/rn5t618.c
@@ -12,13 +12,17 @@ 
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/rn5t618.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/reboot.h>
 #include <linux/regmap.h>
 
+#include <asm/system_misc.h>
+
 static const struct mfd_cell rn5t618_cells[] = {
 	{ .name = "rn5t618-regulator" },
 	{ .name = "rn5t618-wdt" },
@@ -50,17 +54,34 @@  static const struct regmap_config rn5t618_regmap_config = {
 };
 
 static struct rn5t618 *rn5t618_pm_power_off;
+static struct notifier_block rn5t618_restart_handler;
 
-static void rn5t618_power_off(void)
+static void rn5t618_trigger_poweroff_sequence(bool repower)
 {
 	/* disable automatic repower-on */
 	regmap_update_bits(rn5t618_pm_power_off->regmap, RN5T618_REPCNT,
-			   RN5T618_REPCNT_REPWRON, 0);
+			   RN5T618_REPCNT_REPWRON, repower);
 	/* start power-off sequence */
 	regmap_update_bits(rn5t618_pm_power_off->regmap, RN5T618_SLPCNT,
 			   RN5T618_SLPCNT_SWPWROFF, RN5T618_SLPCNT_SWPWROFF);
 }
 
+static void rn5t618_power_off(void)
+{
+	rn5t618_trigger_poweroff_sequence(false);
+}
+
+static int rn5t618_restart(struct notifier_block *this,
+			    unsigned long mode, void *cmd)
+{
+	rn5t618_trigger_poweroff_sequence(true);
+
+	mdelay(10);
+
+	return NOTIFY_DONE;
+}
+
+
 static const struct of_device_id rn5t618_of_match[] = {
 	{ .compatible = "ricoh,rn5t567", .data = (void *)RN5T567 },
 	{ .compatible = "ricoh,rn5t618", .data = (void *)RN5T618 },
@@ -103,13 +124,22 @@  static int rn5t618_i2c_probe(struct i2c_client *i2c,
 		return ret;
 	}
 
+	rn5t618_pm_power_off = priv;
 	if (of_device_is_system_power_controller(i2c->dev.of_node)) {
-		if (!pm_power_off) {
-			rn5t618_pm_power_off = priv;
+		if (!pm_power_off)
 			pm_power_off = rn5t618_power_off;
-		} else {
+		else
 			dev_err(&i2c->dev, "Failed to set poweroff capability, already defined\n");
-		}
+	}
+
+	rn5t618_restart_handler.notifier_call = rn5t618_restart;
+	rn5t618_restart_handler.priority = 128;
+
+	ret = register_restart_handler(&rn5t618_restart_handler);
+	if (ret) {
+		dev_err(&i2c->dev, "%s: cannot register restart handler, %d\n",
+				__func__, ret);
+		return ret;
 	}
 
 	return 0;