diff mbox

[v2,3/8] ARM: dts: omap3-overo: Use complete poweroff

Message ID 1393533032-1619-4-git-send-email-florian.vaussard@epfl.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Florian Vaussard Feb. 27, 2014, 8:30 p.m. UTC
Currently, the TWL4030 PMIC does not completely poweroff the processor.
Commit b0fc1da4d0359d3cce8f12e0f014aed0704ae202 introduced the necessary
binding to do this, so use it.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 arch/arm/boot/dts/omap3-overo.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Nishanth Menon Feb. 27, 2014, 8:38 p.m. UTC | #1
On 02/27/2014 02:30 PM, Florian Vaussard wrote:
> Currently, the TWL4030 PMIC does not completely poweroff the processor.
> Commit b0fc1da4d0359d3cce8f12e0f014aed0704ae202 introduced the necessary
> binding to do this, so use it.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> ---
>  arch/arm/boot/dts/omap3-overo.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
> index aea64c0..018e1e0 100644
> --- a/arch/arm/boot/dts/omap3-overo.dtsi
> +++ b/arch/arm/boot/dts/omap3-overo.dtsi
> @@ -73,6 +73,11 @@
>  			codec {
>  			};
>  		};
> +
> +		twl_power: power {
> +			compatible = "ti,twl4030-power";
> +			ti,use_poweroff;
> +		};
>  	};
>  };
>  
> 
Urrgh.. this slipped past.. :(

ti,system-power-controller is traditionally used for other PMICs from
TI to indicate that poweroff functionality will be provided by the
PMIC driver. similar approach is taken by Maxim as well.. I know the
commit introducing the binding has been around for long, but
considering that we do not have a single dts using this yet, should we
consider adding "ti,system-power-controller"(as against removing
ti,use_poweroff - so that older down stream dtbs still work) and using
it in the new code?

just proposing here.
Florian Vaussard Feb. 27, 2014, 8:48 p.m. UTC | #2
On 02/27/2014 09:38 PM, Nishanth Menon wrote:
> On 02/27/2014 02:30 PM, Florian Vaussard wrote:
>> Currently, the TWL4030 PMIC does not completely poweroff the processor.
>> Commit b0fc1da4d0359d3cce8f12e0f014aed0704ae202 introduced the necessary
>> binding to do this, so use it.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>> ---
>>  arch/arm/boot/dts/omap3-overo.dtsi | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
>> index aea64c0..018e1e0 100644
>> --- a/arch/arm/boot/dts/omap3-overo.dtsi
>> +++ b/arch/arm/boot/dts/omap3-overo.dtsi
>> @@ -73,6 +73,11 @@
>>  			codec {
>>  			};
>>  		};
>> +
>> +		twl_power: power {
>> +			compatible = "ti,twl4030-power";
>> +			ti,use_poweroff;
>> +		};
>>  	};
>>  };
>>  
>>
> Urrgh.. this slipped past.. :(
> 
> ti,system-power-controller is traditionally used for other PMICs from
> TI to indicate that poweroff functionality will be provided by the
> PMIC driver. similar approach is taken by Maxim as well.. I know the
> commit introducing the binding has been around for long, but
> considering that we do not have a single dts using this yet, should we
> consider adding "ti,system-power-controller"(as against removing
> ti,use_poweroff - so that older down stream dtbs still work) and using
> it in the new code?
> 

It does make sense, so I am not against it. My only concern is that I
find the name to be slightly less easy to understand, but I can live
with it :-)

I do not remember if DT maintainers came up with a clear policy to
deprecate a binding.

Regards,
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nishanth Menon Feb. 27, 2014, 9:07 p.m. UTC | #3
+devicetree list.

On 02/27/2014 02:48 PM, Florian Vaussard wrote:
> On 02/27/2014 09:38 PM, Nishanth Menon wrote:
>> On 02/27/2014 02:30 PM, Florian Vaussard wrote:
>>> Currently, the TWL4030 PMIC does not completely poweroff the processor.
>>> Commit b0fc1da4d0359d3cce8f12e0f014aed0704ae202 introduced the necessary
>>> binding to do this, so use it.
>>>
>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>>> ---
>>>  arch/arm/boot/dts/omap3-overo.dtsi | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
>>> index aea64c0..018e1e0 100644
>>> --- a/arch/arm/boot/dts/omap3-overo.dtsi
>>> +++ b/arch/arm/boot/dts/omap3-overo.dtsi
>>> @@ -73,6 +73,11 @@
>>>  			codec {
>>>  			};
>>>  		};
>>> +
>>> +		twl_power: power {
>>> +			compatible = "ti,twl4030-power";
>>> +			ti,use_poweroff;
>>> +		};
>>>  	};
>>>  };
>>>  
>>>
>> Urrgh.. this slipped past.. :(
>>
>> ti,system-power-controller is traditionally used for other PMICs from
>> TI to indicate that poweroff functionality will be provided by the
>> PMIC driver. similar approach is taken by Maxim as well.. I know the
>> commit introducing the binding has been around for long, but
>> considering that we do not have a single dts using this yet, should we
>> consider adding "ti,system-power-controller"(as against removing
>> ti,use_poweroff - so that older down stream dtbs still work) and using
>> it in the new code?
>>
> 
> It does make sense, so I am not against it. My only concern is that I
> find the name to be slightly less easy to understand, but I can live
> with it :-)
:)

> 
> I do not remember if DT maintainers came up with a clear policy to
> deprecate a binding.
I dont think we can depreciate a binding [1] - as you mentioned -
renaming the property is probably what is appropriate, but introducing
a new one which has the same behavior as the old one does'nt seem
covered either.. considering potential downstream kernel usage, I'd
suggest additional property inline with today's convention.


[1]
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2a9330010bea5982a5c6593824bc036bf62d67b7
Florian Vaussard March 3, 2014, 8:55 a.m. UTC | #4
On 02/27/2014 10:07 PM, Nishanth Menon wrote:
> +devicetree list.
> 
> On 02/27/2014 02:48 PM, Florian Vaussard wrote:
>> On 02/27/2014 09:38 PM, Nishanth Menon wrote:
>>> On 02/27/2014 02:30 PM, Florian Vaussard wrote:
>>>> Currently, the TWL4030 PMIC does not completely poweroff the processor.
>>>> Commit b0fc1da4d0359d3cce8f12e0f014aed0704ae202 introduced the necessary
>>>> binding to do this, so use it.
>>>>
>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>>>> ---
>>>>  arch/arm/boot/dts/omap3-overo.dtsi | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
>>>> index aea64c0..018e1e0 100644
>>>> --- a/arch/arm/boot/dts/omap3-overo.dtsi
>>>> +++ b/arch/arm/boot/dts/omap3-overo.dtsi
>>>> @@ -73,6 +73,11 @@
>>>>  			codec {
>>>>  			};
>>>>  		};
>>>> +
>>>> +		twl_power: power {
>>>> +			compatible = "ti,twl4030-power";
>>>> +			ti,use_poweroff;
>>>> +		};
>>>>  	};
>>>>  };
>>>>  
>>>>
>>> Urrgh.. this slipped past.. :(
>>>
>>> ti,system-power-controller is traditionally used for other PMICs from
>>> TI to indicate that poweroff functionality will be provided by the
>>> PMIC driver. similar approach is taken by Maxim as well.. I know the
>>> commit introducing the binding has been around for long, but
>>> considering that we do not have a single dts using this yet, should we
>>> consider adding "ti,system-power-controller"(as against removing
>>> ti,use_poweroff - so that older down stream dtbs still work) and using
>>> it in the new code?
>>>
>>
>> It does make sense, so I am not against it. My only concern is that I
>> find the name to be slightly less easy to understand, but I can live
>> with it :-)
> :)
> 
>>
>> I do not remember if DT maintainers came up with a clear policy to
>> deprecate a binding.
> I dont think we can depreciate a binding [1] - as you mentioned -
> renaming the property is probably what is appropriate, but introducing
> a new one which has the same behavior as the old one does'nt seem
> covered either.. considering potential downstream kernel usage, I'd
> suggest additional property inline with today's convention.
> 

Ok, so I will drop this patch from the series, so that the other patches
can hopefully go into 3.15. I will address this issue separately. Thank
you for pointing out this binding issue.

Regards,
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
index aea64c0..018e1e0 100644
--- a/arch/arm/boot/dts/omap3-overo.dtsi
+++ b/arch/arm/boot/dts/omap3-overo.dtsi
@@ -73,6 +73,11 @@ 
 			codec {
 			};
 		};
+
+		twl_power: power {
+			compatible = "ti,twl4030-power";
+			ti,use_poweroff;
+		};
 	};
 };