| Message ID | 1411834906-9533-1-git-send-email-romain.perier@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Sep 27, 2014 at 04:21:44PM +0000, Romain Perier wrote: > When the property "active-semi,system-power-controller" is found in the > devicetree, the function pm_power_off is defined. This function sends the > rights bit fields to the global off control register. shutdown/poweroff > commands are now supported for hardware components which use these PMU. We really need to come up with a standard property for this and document it rather than continuing to add individual device specific properties all doing the same thing, and probably also some helper code and/or a standard operation for this - there's a lot of drivers implementing the same pattern here. > + if (dev->of_node && > + of_property_read_bool(dev->of_node, > + "active-semi,system-power-controller")) { > + act8865_i2c_client = client; Indentation seems messed up here - tabs vs spaces?
Hi Mark, 2014-09-28 12:33 GMT+02:00 Mark Brown <broonie@kernel.org>: > On Sat, Sep 27, 2014 at 04:21:44PM +0000, Romain Perier wrote: >> When the property "active-semi,system-power-controller" is found in the >> devicetree, the function pm_power_off is defined. This function sends the >> rights bit fields to the global off control register. shutdown/poweroff >> commands are now supported for hardware components which use these PMU. > > We really need to come up with a standard property for this and document > it rather than continuing to add individual device specific properties > all doing the same thing, and probably also some helper code and/or a > standard operation for this - there's a lot of drivers implementing the > same pattern here. I completly agree about adding a unified and generic property for that purpose. I already proposed something for that on devicetree ML, see the thread "Proposal: generic property for system-power-controller". Unfortunately I did not get replies :) . > >> + if (dev->of_node && >> + of_property_read_bool(dev->of_node, >> + "active-semi,system-power-controller")) { >> + act8865_i2c_client = client; > > Indentation seems messed up here - tabs vs spaces? Yes, I really don't understand where the problem is. I use good emacs settings to be compatible with kernel coding style and git diff does not show me indent/spaces problems :/ . Sorry because this is a very boring issue when reviewing patches... I will investigate ^^ Romain
On Sun, Sep 28, 2014 at 02:00:54PM +0200, PERIER Romain wrote: > 2014-09-28 12:33 GMT+02:00 Mark Brown <broonie@kernel.org>: > > We really need to come up with a standard property for this and document > > it rather than continuing to add individual device specific properties > > all doing the same thing, and probably also some helper code and/or a > > standard operation for this - there's a lot of drivers implementing the > > same pattern here. > I completly agree about adding a unified and generic property for that purpose. > I already proposed something for that on devicetree ML, see the thread > "Proposal: generic property for system-power-controller". > Unfortunately I did not get replies :) . Did you CC relevant maintainers and send a patch?
2014-09-28 14:25 GMT+02:00 Mark Brown <broonie@kernel.org>: > On Sun, Sep 28, 2014 at 02:00:54PM +0200, PERIER Romain wrote: >> 2014-09-28 12:33 GMT+02:00 Mark Brown <broonie@kernel.org>: > >> > We really need to come up with a standard property for this and document >> > it rather than continuing to add individual device specific properties >> > all doing the same thing, and probably also some helper code and/or a >> > standard operation for this - there's a lot of drivers implementing the >> > same pattern here. > >> I completly agree about adding a unified and generic property for that purpose. >> I already proposed something for that on devicetree ML, see the thread >> "Proposal: generic property for system-power-controller". >> Unfortunately I did not get replies :) . > > Did you CC relevant maintainers and send a patch? No I did not propose patches as I was an open discussion. Perhaps I might propose something as part of my contribution for act8865... (at least for the property)
Well, I will think about it and I will propose a separated patch with a generic property and helper functions. 2014-09-28 15:25 GMT+02:00 PERIER Romain <romain.perier@gmail.com>: > 2014-09-28 14:25 GMT+02:00 Mark Brown <broonie@kernel.org>: >> On Sun, Sep 28, 2014 at 02:00:54PM +0200, PERIER Romain wrote: >>> 2014-09-28 12:33 GMT+02:00 Mark Brown <broonie@kernel.org>: >> >>> > We really need to come up with a standard property for this and document >>> > it rather than continuing to add individual device specific properties >>> > all doing the same thing, and probably also some helper code and/or a >>> > standard operation for this - there's a lot of drivers implementing the >>> > same pattern here. >> >>> I completly agree about adding a unified and generic property for that purpose. >>> I already proposed something for that on devicetree ML, see the thread >>> "Proposal: generic property for system-power-controller". >>> Unfortunately I did not get replies :) . >> >> Did you CC relevant maintainers and send a patch? > > No I did not propose patches as I was an open discussion. Perhaps I > might propose something as part of my contribution for act8865... (at > least for the property)
diff --git a/drivers/regulator/act8865-regulator.c b/drivers/regulator/act8865-regulator.c index afd06f9..6cf202d 100644 --- a/drivers/regulator/act8865-regulator.c +++ b/drivers/regulator/act8865-regulator.c @@ -61,6 +61,8 @@ #define ACT8846_REG12_VSET 0xa0 #define ACT8846_REG12_CTRL 0xa1 #define ACT8846_REG13_CTRL 0xb1 +#define ACT8846_GLB_OFF_CTRL 0xc3 +#define ACT8846_OFF_SYSMASK 0x18 /* * ACT8865 Global Register Map. @@ -84,6 +86,7 @@ #define ACT8865_LDO3_CTRL 0x61 #define ACT8865_LDO4_VSET 0x64 #define ACT8865_LDO4_CTRL 0x65 +#define ACT8865_MSTROFF 0x20 /* * Field Definitions. @@ -98,6 +101,8 @@ struct act8865 { struct regmap *regmap; + int off_reg; + int off_mask; }; static const struct regmap_config act8865_regmap_config = { @@ -275,6 +280,16 @@ static struct regulator_init_data return NULL; } +static struct i2c_client *act8865_i2c_client; +static void act8865_power_off(void) +{ + struct act8865 *act8865; + + act8865 = i2c_get_clientdata(act8865_i2c_client); + regmap_write(act8865->regmap, act8865->off_reg, act8865->off_mask); + while (1); +} + static int act8865_pmic_probe(struct i2c_client *client, const struct i2c_device_id *i2c_id) { @@ -285,6 +300,7 @@ static int act8865_pmic_probe(struct i2c_client *client, int i, ret, num_regulators; struct act8865 *act8865; unsigned long type; + int off_reg, off_mask; pdata = dev_get_platdata(dev); @@ -304,10 +320,14 @@ static int act8865_pmic_probe(struct i2c_client *client, case ACT8846: regulators = act8846_regulators; num_regulators = ARRAY_SIZE(act8846_regulators); + off_reg = ACT8846_GLB_OFF_CTRL; + off_mask = ACT8846_OFF_SYSMASK; break; case ACT8865: regulators = act8865_regulators; num_regulators = ARRAY_SIZE(act8865_regulators); + off_reg = ACT8865_SYS_CTRL; + off_mask = ACT8865_MSTROFF; break; default: dev_err(dev, "invalid device id %lu\n", type); @@ -345,6 +365,15 @@ static int act8865_pmic_probe(struct i2c_client *client, return ret; } + if (dev->of_node && + of_property_read_bool(dev->of_node, + "active-semi,system-power-controller")) { + act8865_i2c_client = client; + act8865->off_reg = off_reg; + act8865->off_mask = off_mask; + pm_power_off = act8865_power_off; + } + /* Finally register devices */ for (i = 0; i < num_regulators; i++) { const struct regulator_desc *desc = ®ulators[i];
When the property "active-semi,system-power-controller" is found in the devicetree, the function pm_power_off is defined. This function sends the rights bit fields to the global off control register. shutdown/poweroff commands are now supported for hardware components which use these PMU. Signed-off-by: Romain Perier <romain.perier@gmail.com> --- drivers/regulator/act8865-regulator.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)