| Message ID | 20190424224900.8018-7-digetx@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | Add support for Maxim 77663 MFD | expand |
On Thu, Apr 25, 2019 at 01:49:00AM +0300, Dmitry Osipenko wrote: > Provide system power-off functionality that allows to turn off machine > gracefully. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/mfd/max77620.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/mfd/max77620.c b/drivers/mfd/max77620.c > index 9b0009c29610..e56223bde568 100644 > --- a/drivers/mfd/max77620.c > +++ b/drivers/mfd/max77620.c > @@ -37,6 +37,8 @@ > #include <linux/regmap.h> > #include <linux/slab.h> > > +static struct max77620_chip *max77620_scratch; > + > static const struct resource gpio_resources[] = { > DEFINE_RES_IRQ(MAX77620_IRQ_TOP_GPIO), > }; > @@ -481,6 +483,15 @@ static int max77620_read_es_version(struct max77620_chip *chip) > return ret; > } > > +static void max77620_pm_power_off(void) > +{ > + struct max77620_chip *chip = max77620_scratch; > + > + regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG1, > + MAX77620_ONOFFCNFG1_SFT_RST, > + MAX77620_ONOFFCNFG1_SFT_RST); > +} I think this is only partially correct. See here for a driver that I had proposed a while back: https://github.com/thierryreding/linux/commit/d0eaa77b402f62bd236d76e3edeb3ccf296cbe81 Note that that driver is part of a larger series to move away from all the pm_power_off hackery. There was a fair bit of discussion back when I proposed the original power off driver for max77620: https://lkml.org/lkml/2017/1/12/470 I think I may have a more up-to-date local branch of the system-power branch from my github repository if you're interested in looking at some of that code. There wasn't a whole lot of feedback on the patches, but the feedback I did get was generally positive. However, since it didn't gain any traction I eventually abandoned that effort. It might be worth picking it up again, since, as far as I can tell, the situation around power off and restart hasn't changed in the meantime. Thierry > + > static int max77620_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -488,6 +499,7 @@ static int max77620_probe(struct i2c_client *client, > struct max77620_chip *chip; > const struct mfd_cell *mfd_cells; > int n_mfd_cells; > + bool pm_off; > int ret; > > chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL); > @@ -554,6 +566,13 @@ static int max77620_probe(struct i2c_client *client, > return ret; > } > > + pm_off = of_property_read_bool(client->dev.of_node, > + "maxim,system-power-controller"); > + if (pm_off && !pm_power_off) { > + max77620_scratch = chip; > + pm_power_off = max77620_pm_power_off; > + } > + > return 0; > } > > -- > 2.21.0 >
25.04.2019 14:22, Thierry Reding пишет: > On Thu, Apr 25, 2019 at 01:49:00AM +0300, Dmitry Osipenko wrote: >> Provide system power-off functionality that allows to turn off machine >> gracefully. >> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/mfd/max77620.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/drivers/mfd/max77620.c b/drivers/mfd/max77620.c >> index 9b0009c29610..e56223bde568 100644 >> --- a/drivers/mfd/max77620.c >> +++ b/drivers/mfd/max77620.c >> @@ -37,6 +37,8 @@ >> #include <linux/regmap.h> >> #include <linux/slab.h> >> >> +static struct max77620_chip *max77620_scratch; >> + >> static const struct resource gpio_resources[] = { >> DEFINE_RES_IRQ(MAX77620_IRQ_TOP_GPIO), >> }; >> @@ -481,6 +483,15 @@ static int max77620_read_es_version(struct max77620_chip *chip) >> return ret; >> } >> >> +static void max77620_pm_power_off(void) >> +{ >> + struct max77620_chip *chip = max77620_scratch; >> + >> + regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG1, >> + MAX77620_ONOFFCNFG1_SFT_RST, >> + MAX77620_ONOFFCNFG1_SFT_RST); >> +} > > I think this is only partially correct. See here for a driver that I had > proposed a while back: > > https://github.com/thierryreding/linux/commit/d0eaa77b402f62bd236d76e3edeb3ccf296cbe81 > > Note that that driver is part of a larger series to move away from all > the pm_power_off hackery. There was a fair bit of discussion back when I > proposed the original power off driver for max77620: > > https://lkml.org/lkml/2017/1/12/470 > > I think I may have a more up-to-date local branch of the system-power > branch from my github repository if you're interested in looking at some > of that code. There wasn't a whole lot of feedback on the patches, but > the feedback I did get was generally positive. However, since it didn't > gain any traction I eventually abandoned that effort. It might be worth > picking it up again, since, as far as I can tell, the situation around > power off and restart hasn't changed in the meantime. Hello Thierry, Thank you very much for the feedback. IIUC, you're asking for a comprehensive solution that nobody managed to get upstreamed for years and thus I think it is absolutely fine to have at least a practical minimum implemented for the start. In yours system-power series you are saying that the restart handlers "lack any means of locking against concurrently registering handlers or formal definitions on what proper priorities are to order handlers", it looks to me that it will be much easier to just fix the missing locks and properly define the priorities. https://github.com/thierryreding/linux/commit/16e386d4692716c3f2423732a2181fb589421526 In the LKML discussion there is also pointer to the "poweroff handler call chain" series from 2014 which is similar to the restart handlers and actually looks nice, sadly it didn't got too far. https://lkml.org/lkml/2014/10/21/5 I may try to continue the effort of getting proper restart / poweroff handlers into upstream at some point in the future. Meanwhile there are much bigger and fun problems to solve in the kernel and user spaces, let's get everything step by step on by as-needed basis.
diff --git a/drivers/mfd/max77620.c b/drivers/mfd/max77620.c index 9b0009c29610..e56223bde568 100644 --- a/drivers/mfd/max77620.c +++ b/drivers/mfd/max77620.c @@ -37,6 +37,8 @@ #include <linux/regmap.h> #include <linux/slab.h> +static struct max77620_chip *max77620_scratch; + static const struct resource gpio_resources[] = { DEFINE_RES_IRQ(MAX77620_IRQ_TOP_GPIO), }; @@ -481,6 +483,15 @@ static int max77620_read_es_version(struct max77620_chip *chip) return ret; } +static void max77620_pm_power_off(void) +{ + struct max77620_chip *chip = max77620_scratch; + + regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG1, + MAX77620_ONOFFCNFG1_SFT_RST, + MAX77620_ONOFFCNFG1_SFT_RST); +} + static int max77620_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -488,6 +499,7 @@ static int max77620_probe(struct i2c_client *client, struct max77620_chip *chip; const struct mfd_cell *mfd_cells; int n_mfd_cells; + bool pm_off; int ret; chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL); @@ -554,6 +566,13 @@ static int max77620_probe(struct i2c_client *client, return ret; } + pm_off = of_property_read_bool(client->dev.of_node, + "maxim,system-power-controller"); + if (pm_off && !pm_power_off) { + max77620_scratch = chip; + pm_power_off = max77620_pm_power_off; + } + return 0; }
Provide system power-off functionality that allows to turn off machine gracefully. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/mfd/max77620.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)