| Message ID | 1393533032-1619-4-git-send-email-florian.vaussard@epfl.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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
+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
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
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; + }; }; };
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(+)