Hook up "pm_power_off" to palmas power off routine if there is DT
property "ti,system-power-controller" defined, so platform which is
powered by this regulator can be powered off properly.
Based on work by:
Mallikarjun Kasoju <[email protected]>
Signed-off-by: Bill Huang <[email protected]>
cc: Mallikarjun Kasoju <[email protected]>
---
.../devicetree/bindings/regulator/palmas-pmic.txt | 5 +++++
drivers/mfd/palmas.c | 23 ++++++++++++++++++--
include/linux/mfd/palmas.h | 1 +
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
index 30b0581..4adca2a 100644
--- a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
+++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
@@ -36,6 +36,9 @@ Optional nodes:
ti,smps-range - OTP has the wrong range set for the hardware so override
0 - low range, 1 - high range.
+- ti,system-power-controller: Telling whether or not this pmic is controlling
+ the system power.
+
Example:
#include <dt-bindings/interrupt-controller/irq.h>
@@ -48,6 +51,8 @@ pmic {
ti,ldo6-vibrator;
+ ti,system-power-controller;
+
regulators {
smps12_reg : smps12 {
regulator-name = "smps12";
diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index e4d1c70..0662b21 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -229,6 +229,22 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c,
PALMAS_POWER_CTRL_ENABLE2_MASK;
if (i2c->irq)
palmas_set_pdata_irq_flag(i2c, pdata);
+
+ pdata->pm_off = of_property_read_bool(node,
+ "ti,system-power-controller");
+}
+
+static struct palmas *palmas_dev;
+static void palmas_power_off(void)
+{
+ unsigned int addr;
+
+ if (!palmas_dev)
+ return;
+
+ addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL);
+
+ regmap_update_bits(palmas_dev->regmap[0], addr, 1, 0);
}
static unsigned int palmas_features = PALMAS_PMIC_FEATURE_SMPS10_BOOST;
@@ -423,10 +439,13 @@ no_irq:
*/
if (node) {
ret = of_platform_populate(node, NULL, NULL, &i2c->dev);
- if (ret < 0)
+ if (ret < 0) {
goto err_irq;
- else
+ } else if (pdata->pm_off && !pm_power_off) {
+ palmas_dev = palmas;
+ pm_power_off = palmas_power_off;
return ret;
+ }
}
return ret;
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 1a8dd7a..061cce0 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -258,6 +258,7 @@ struct palmas_platform_data {
*/
int mux_from_pdata;
u8 pad1, pad2;
+ bool pm_off;
struct palmas_pmic_platform_data *pmic_pdata;
struct palmas_gpadc_platform_data *gpadc_pdata;
--
1.7.9.5
On 07/30/2013 07:05 AM, Bill Huang wrote:
> Hook up "pm_power_off" to palmas power off routine if there is DT
> property "ti,system-power-controller" defined, so platform which is
> powered by this regulator can be powered off properly.
>
> Based on work by:
> Mallikarjun Kasoju <[email protected]>
>
> Signed-off-by: Bill Huang <[email protected]>
> cc: Mallikarjun Kasoju <[email protected]>
> ---
> .../devicetree/bindings/regulator/palmas-pmic.txt | 5 +++++
> drivers/mfd/palmas.c | 23 ++++++++++++++++++--
> include/linux/mfd/palmas.h | 1 +
any reason why it wont fit in drivers/power/reset/ is'nt it the right
place to add this?
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> index 30b0581..4adca2a 100644
> --- a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> +++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> @@ -36,6 +36,9 @@ Optional nodes:
> ti,smps-range - OTP has the wrong range set for the hardware so override
> 0 - low range, 1 - high range.
>
> +- ti,system-power-controller: Telling whether or not this pmic is controlling
controller or master?
> + the system power.
> +
> Example:
>
> #include <dt-bindings/interrupt-controller/irq.h>
> @@ -48,6 +51,8 @@ pmic {
>
> ti,ldo6-vibrator;
>
> + ti,system-power-controller;
> +
> regulators {
> smps12_reg : smps12 {
> regulator-name = "smps12";
> diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
> index e4d1c70..0662b21 100644
> --- a/drivers/mfd/palmas.c
> +++ b/drivers/mfd/palmas.c
> @@ -229,6 +229,22 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c,
> PALMAS_POWER_CTRL_ENABLE2_MASK;
> if (i2c->irq)
> palmas_set_pdata_irq_flag(i2c, pdata);
> +
> + pdata->pm_off = of_property_read_bool(node,
> + "ti,system-power-controller");
> +}
> +
> +static struct palmas *palmas_dev;
> +static void palmas_power_off(void)
> +{
> + unsigned int addr;
> +
> + if (!palmas_dev)
> + return;
> +
> + addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL);
> +
> + regmap_update_bits(palmas_dev->regmap[0], addr, 1, 0);
slave = PALMAS_BASE_TO_SLAVE(base);
addr = PALMAS_BASE_TO_REG(base, reg);
r = regmap_update_bits(palmas->regmap[slave], addr, mask, val);
instead?
just for reference, an old implementation I had done is available at [1]
[1]
http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=drivers/mfd/palmas-poweroff.c;hb=p-linux-omap-3.4
--
Regards,
Nishanth Menon
On Tue, 2013-07-30 at 21:17 +0800, Nishanth Menon wrote:
> On 07/30/2013 07:05 AM, Bill Huang wrote:
> > Hook up "pm_power_off" to palmas power off routine if there is DT
> > property "ti,system-power-controller" defined, so platform which is
> > powered by this regulator can be powered off properly.
> >
> > Based on work by:
> > Mallikarjun Kasoju <[email protected]>
> >
> > Signed-off-by: Bill Huang <[email protected]>
> > cc: Mallikarjun Kasoju <[email protected]>
> > ---
> > .../devicetree/bindings/regulator/palmas-pmic.txt | 5 +++++
> > drivers/mfd/palmas.c | 23 ++++++++++++++++++--
> > include/linux/mfd/palmas.h | 1 +
> any reason why it wont fit in drivers/power/reset/ is'nt it the right
> place to add this?
> > 3 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > index 30b0581..4adca2a 100644
> > --- a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > +++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > @@ -36,6 +36,9 @@ Optional nodes:
> > ti,smps-range - OTP has the wrong range set for the hardware so override
> > 0 - low range, 1 - high range.
> >
> > +- ti,system-power-controller: Telling whether or not this pmic is controlling
> controller or master?
We name it controller since it controls the system power of the
platform, I'm not sure will it make more sense to be master? Or does it
makes sense to other pmic which also control the system power (the same
property are already used in pmic TPS6586x and TPS65910).
> > + the system power.
> > +
> > Example:
> >
> > #include <dt-bindings/interrupt-controller/irq.h>
> > @@ -48,6 +51,8 @@ pmic {
> >
> > ti,ldo6-vibrator;
> >
> > + ti,system-power-controller;
> > +
> > regulators {
> > smps12_reg : smps12 {
> > regulator-name = "smps12";
> > diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
> > index e4d1c70..0662b21 100644
> > --- a/drivers/mfd/palmas.c
> > +++ b/drivers/mfd/palmas.c
> > @@ -229,6 +229,22 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c,
> > PALMAS_POWER_CTRL_ENABLE2_MASK;
> > if (i2c->irq)
> > palmas_set_pdata_irq_flag(i2c, pdata);
> > +
> > + pdata->pm_off = of_property_read_bool(node,
> > + "ti,system-power-controller");
> > +}
> > +
> > +static struct palmas *palmas_dev;
> > +static void palmas_power_off(void)
> > +{
> > + unsigned int addr;
> > +
> > + if (!palmas_dev)
> > + return;
> > +
> > + addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL);
> > +
> > + regmap_update_bits(palmas_dev->regmap[0], addr, 1, 0);
> slave = PALMAS_BASE_TO_SLAVE(base);
> addr = PALMAS_BASE_TO_REG(base, reg);
>
> r = regmap_update_bits(palmas->regmap[slave], addr, mask, val);
> instead?
>
> just for reference, an old implementation I had done is available at [1]
>
Thanks, I'll send out v2.
>
> [1]
> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=drivers/mfd/palmas-poweroff.c;hb=p-linux-omap-3.4
>
On Tue, 30 Jul 2013, Bill Huang wrote:
> Hook up "pm_power_off" to palmas power off routine if there is DT
> property "ti,system-power-controller" defined, so platform which is
> powered by this regulator can be powered off properly.
>
> Based on work by:
> Mallikarjun Kasoju <[email protected]>
>
> Signed-off-by: Bill Huang <[email protected]>
> cc: Mallikarjun Kasoju <[email protected]>
Please put the 'Cc:' (not 'cc:') above the SoBs, then drop the "Based
on work by:" and replace with:
Signed-off-by: Mallikarjun Kasoju <[email protected]>
Signed-off-by: Bill Huang <[email protected]>
This insinuates that the original patch was crated by Mallikarjun.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Wed, 2013-07-31 at 17:56 +0800, Lee Jones wrote:
> On Tue, 30 Jul 2013, Bill Huang wrote:
>
> > Hook up "pm_power_off" to palmas power off routine if there is DT
> > property "ti,system-power-controller" defined, so platform which is
> > powered by this regulator can be powered off properly.
> >
> > Based on work by:
> > Mallikarjun Kasoju <[email protected]>
> >
> > Signed-off-by: Bill Huang <[email protected]>
> > cc: Mallikarjun Kasoju <[email protected]>
>
> Please put the 'Cc:' (not 'cc:') above the SoBs, then drop the "Based
> on work by:" and replace with:
>
> Signed-off-by: Mallikarjun Kasoju <[email protected]>
> Signed-off-by: Bill Huang <[email protected]>
>
> This insinuates that the original patch was crated by Mallikarjun.
>
Thanks, will fix in next version.
On 07/31/2013 01:22 AM, Bill Huang wrote:
> On Tue, 2013-07-30 at 21:17 +0800, Nishanth Menon wrote:
>> On 07/30/2013 07:05 AM, Bill Huang wrote:
[...]
>>> diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
>>> index 30b0581..4adca2a 100644
>>> --- a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
>>> +++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
>>> @@ -36,6 +36,9 @@ Optional nodes:
>>> ti,smps-range - OTP has the wrong range set for the hardware so override
>>> 0 - low range, 1 - high range.
>>>
>>> +- ti,system-power-controller: Telling whether or not this pmic is controlling
>> controller or master?
>
> We name it controller since it controls the system power of the
> platform, I'm not sure will it make more sense to be master? Or does it
> makes sense to other pmic which also control the system power (the same
> property are already used in pmic TPS6586x and TPS65910).
No strong feelings either way, especially if there is precedence.
--
Regards,
Nishanth Menon
On 07/31/2013 12:22 AM, Bill Huang wrote:
> On Tue, 2013-07-30 at 21:17 +0800, Nishanth Menon wrote:
>> On 07/30/2013 07:05 AM, Bill Huang wrote:
>>> Hook up "pm_power_off" to palmas power off routine if there is DT
>>> property "ti,system-power-controller" defined, so platform which is
>>> powered by this regulator can be powered off properly.
>>>
>>> Based on work by:
>>> Mallikarjun Kasoju <[email protected]>
>>>
>>> Signed-off-by: Bill Huang <[email protected]>
>>> cc: Mallikarjun Kasoju <[email protected]>
>>> ---
>>> .../devicetree/bindings/regulator/palmas-pmic.txt | 5 +++++
>>> drivers/mfd/palmas.c | 23 ++++++++++++++++++--
>>> include/linux/mfd/palmas.h | 1 +
>> any reason why it wont fit in drivers/power/reset/ is'nt it the right
>> place to add this?
>>> 3 files changed, 27 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
>>> index 30b0581..4adca2a 100644
>>> --- a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
>>> +++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
>>> @@ -36,6 +36,9 @@ Optional nodes:
>>> ti,smps-range - OTP has the wrong range set for the hardware so override
>>> 0 - low range, 1 - high range.
>>>
>>> +- ti,system-power-controller: Telling whether or not this pmic is controlling
>> controller or master?
>
> We name it controller since it controls the system power of the
> platform, I'm not sure will it make more sense to be master? Or does it
> makes sense to other pmic which also control the system power (the same
> property are already used in pmic TPS6586x and TPS65910).
The property name quoted above is probably the best choice since it's
consistent with other properties that do the exact same thing in other
bindings.
On 07/31/2013 03:56 AM, Lee Jones wrote:
> On Tue, 30 Jul 2013, Bill Huang wrote:
>
>> Hook up "pm_power_off" to palmas power off routine if there is DT
>> property "ti,system-power-controller" defined, so platform which is
>> powered by this regulator can be powered off properly.
>>
>> Based on work by:
>> Mallikarjun Kasoju <[email protected]>
>>
>> Signed-off-by: Bill Huang <[email protected]>
>> cc: Mallikarjun Kasoju <[email protected]>
>
> Please put the 'Cc:' (not 'cc:') above the SoBs, then drop the "Based
> on work by:" and replace with:
>
> Signed-off-by: Mallikarjun Kasoju <[email protected]>
> Signed-off-by: Bill Huang <[email protected]>
>
> This insinuates that the original patch was crated by Mallikarjun.
That advice may not be correct. Did Mallikarjun actually create *this*
patch? More likely, this patch was based on an equivalent change to some
other PMIC, and Bill just applied the same technique to this other
driver. If Mallikarjun really did write this patch, then the git author
field should also be set to Mallikarjun not Bill.
On Wed, 31 Jul 2013, Stephen Warren wrote:
> On 07/31/2013 03:56 AM, Lee Jones wrote:
> > On Tue, 30 Jul 2013, Bill Huang wrote:
> >
> >> Hook up "pm_power_off" to palmas power off routine if there is DT
> >> property "ti,system-power-controller" defined, so platform which is
> >> powered by this regulator can be powered off properly.
> >>
> >> Based on work by:
> >> Mallikarjun Kasoju <[email protected]>
> >>
> >> Signed-off-by: Bill Huang <[email protected]>
> >> cc: Mallikarjun Kasoju <[email protected]>
> >
> > Please put the 'Cc:' (not 'cc:') above the SoBs, then drop the "Based
> > on work by:" and replace with:
> >
> > Signed-off-by: Mallikarjun Kasoju <[email protected]>
> > Signed-off-by: Bill Huang <[email protected]>
> >
> > This insinuates that the original patch was crated by Mallikarjun.
>
> That advice may not be correct. Did Mallikarjun actually create *this*
> patch? More likely, this patch was based on an equivalent change to some
> other PMIC, and Bill just applied the same technique to this other
> driver.
Yes, I agree with this, and I'm sure there is a place for "Based on
work by:" or "Originally authored by:" tags, but in general, I think
the SoBs can paint a pretty good picture.
For example, if this patch is simply using techniques which already
exist in other drivers, I would personally not mention it in the
commit message. A massive percentage of kernel code has been
influenced by already existing implementations. Not much truly new and
unique kernel code enters the kernel these days.
> If Mallikarjun really did write this patch, then the git author
> field should also be set to Mallikarjun not Bill.
That's not how I'm lead to believe it works. I am under the impression
that if you take an already existing patch and upstream it with little
changes, then you keep the original creator's authorship and apply
your SoB before sending. Whereas if you have make considerable (down
to perception) changes to the patch, then you may adopt authorship. To
credit the efforts of the original author in this case I would advise
to keep the first SoB. Providing they agree with the changes of course.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 08/01/2013 02:47 AM, Lee Jones wrote:
> On Wed, 31 Jul 2013, Stephen Warren wrote:
>
>> On 07/31/2013 03:56 AM, Lee Jones wrote:
>>> On Tue, 30 Jul 2013, Bill Huang wrote:
>>>
>>>> Hook up "pm_power_off" to palmas power off routine if there is DT
>>>> property "ti,system-power-controller" defined, so platform which is
>>>> powered by this regulator can be powered off properly.
>>>>
>>>> Based on work by:
>>>> Mallikarjun Kasoju <[email protected]>
>>>>
>>>> Signed-off-by: Bill Huang <[email protected]>
>>>> cc: Mallikarjun Kasoju <[email protected]>
>>>
>>> Please put the 'Cc:' (not 'cc:') above the SoBs, then drop the "Based
>>> on work by:" and replace with:
>>>
>>> Signed-off-by: Mallikarjun Kasoju <[email protected]>
>>> Signed-off-by: Bill Huang <[email protected]>
>>>
>>> This insinuates that the original patch was crated by Mallikarjun.
>>
>> That advice may not be correct. Did Mallikarjun actually create *this*
>> patch? More likely, this patch was based on an equivalent change to some
>> other PMIC, and Bill just applied the same technique to this other
>> driver.
>
> Yes, I agree with this, and I'm sure there is a place for "Based on
> work by:" or "Originally authored by:" tags, but in general, I think
> the SoBs can paint a pretty good picture.
>
> For example, if this patch is simply using techniques which already
> exist in other drivers, I would personally not mention it in the
> commit message. A massive percentage of kernel code has been
> influenced by already existing implementations. Not much truly new and
> unique kernel code enters the kernel these days.
>
>> If Mallikarjun really did write this patch, then the git author
>> field should also be set to Mallikarjun not Bill.
>
> That's not how I'm lead to believe it works. I am under the impression
> that if you take an already existing patch and upstream it with little
> changes, then you keep the original creator's authorship and apply
> your SoB before sending.
Yes.
> Whereas if you have make considerable (down
> to perception) changes to the patch, then you may adopt authorship. To
> credit the efforts of the original author in this case I would advise
> to keep the first SoB. Providing they agree with the changes of course.
I'm not sure about this. I've certainly made significant changes then
still been asked to maintain the original authorship. There is plenty of
opportunity to list the changes you made in the free-form commit
description, or for smaller changes, to inline the list into the final
tag paragraph.
Either way, this situation is quite unlikely to apply to this simple patch.
But anyway, Bill knows the exact history of the patch and can presumably
choose the correct approach.