Message ID | 20160605001749.4062-7-stefan@agner.ch (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
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
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
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;
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 --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;
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(-)