PROBLEM: bindings for drivers/mfd/twl4030-power.c

PROBLEM: bindings for drivers/mfd/twl4030-power.c

[Dr. H. Nikolaus Schaller]

I am trying to make ti,use_poweroff work on 3.17-rc1 for the GTA04 board.
Poweroff was broken for a while and I found that the driver isn't loaded at all.

It appears to me that commit e7cd1d1eb16fcdf53001b926187a82f1f3e1a7e6
did rename the compatible entry from "ti,twl4030-power" to "ti,twl4030-power-reset"
but this was not documented in the bindings and of course our DT does not
match.

Even your commit message talks about "ti,twl4030-power" although I can't find it
in the code.

Are "ti,twl4030-power" and "ti,twl4030-power-reset" doing the same?

BR,
Nikolaus

Re: PROBLEM: bindings for drivers/mfd/twl4030-power.c

[Tony Lindgren]

* Dr. H. Nikolaus Schaller <hns@goldelico.com> [140817 08:46]:
> I am trying to make ti,use_poweroff work on 3.17-rc1 for the GTA04 board.
> Poweroff was broken for a while and I found that the driver isn't loaded at all.
> 
> It appears to me that commit e7cd1d1eb16fcdf53001b926187a82f1f3e1a7e6
> did rename the compatible entry from "ti,twl4030-power" to "ti,twl4030-power-reset"
> but this was not documented in the bindings and of course our DT does not
> match.
> 
> Even your commit message talks about "ti,twl4030-power" although I can't find it
> in the code.

Hmm sorry did I accidentally remove ti,twl4030-power? If so, that should
be added back for sure. Do you have a patch for that already?
 
> Are "ti,twl4030-power" and "ti,twl4030-power-reset" doing the same?

No, they are separate where ti,twl4030-power does not configure the
twl4030 in any way where ti,twl4030-power-reset configures the warm
reset sequence.

For gta04, what you really want is to use ti,twl4030-power-idle or
even ti,twl4030-power-idle-osc-off if the board is wired to support
cutting off the oscillator.

And you probably also want to configure some wake-up interrupts at least
for the the UARTs in the board specific .dts file, see for example the
UART3 in the existing board files that have:

interrupts-extended = <&intc 74 &omap3_pmx_core OMAP3_UART3_RX>;

Regards,

Tony

Re: PROBLEM: bindings for drivers/mfd/twl4030-power.c

[Dr. H. Nikolaus Schaller]

Hi,

Am 25.08.2014 um 23:26 schrieb Tony Lindgren:

> * Dr. H. Nikolaus Schaller <hns@goldelico.com> [140817 08:46]:
>> I am trying to make ti,use_poweroff work on 3.17-rc1 for the GTA04 board.
>> Poweroff was broken for a while and I found that the driver isn't loaded at all.
>> 
>> It appears to me that commit e7cd1d1eb16fcdf53001b926187a82f1f3e1a7e6
>> did rename the compatible entry from "ti,twl4030-power" to "ti,twl4030-power-reset"
>> but this was not documented in the bindings and of course our DT does not
>> match.
>> 
>> Even your commit message talks about "ti,twl4030-power" although I can't find it
>> in the code.
> 
> Hmm sorry did I accidentally remove ti,twl4030-power? If so, that should
> be added back for sure. Do you have a patch for that already?

No, I have only updated our device tree because I don't know if it really should
be added back or not.

As you say the "ti,twl4030-power" does not configure anything. So what
is it good for?

> 
>> Are "ti,twl4030-power" and "ti,twl4030-power-reset" doing the same?
> 
> No, they are separate where ti,twl4030-power does not configure the
> twl4030 in any way where ti,twl4030-power-reset configures the warm
> reset sequence.

Yes, that is what I deduced because our old setting of "ti,twl4030-power" did
no longer configure the power-off and not even load the driver.

> 
> For gta04, what you really want is to use ti,twl4030-power-idle or
> even ti,twl4030-power-idle-osc-off if the board is wired to support
> cutting off the oscillator.

Ok, I see (but must admit that I don't understand the details even after
reading the bindings.txt).

Currently we develop without taking care of suspend (the DT migration was
much more troublesome work than anticipated) but that should be changed soon.

> 
> And you probably also want to configure some wake-up interrupts at least
> for the the UARTs in the board specific .dts file, see for example the
> UART3 in the existing board files that have:
> 
> interrupts-extended = <&intc 74 &omap3_pmx_core OMAP3_UART3_RX>;

thanks for the hint!

BR,
Nikolaus

Re: PROBLEM: bindings for drivers/mfd/twl4030-power.c

[Tony Lindgren]

* Dr. H. Nikolaus Schaller <hns@goldelico.com> [140901 09:54]:
> Hi,
> 
> Am 25.08.2014 um 23:26 schrieb Tony Lindgren:
> 
> > * Dr. H. Nikolaus Schaller <hns@goldelico.com> [140817 08:46]:
> >> I am trying to make ti,use_poweroff work on 3.17-rc1 for the GTA04 board.
> >> Poweroff was broken for a while and I found that the driver isn't loaded at all.
> >> 
> >> It appears to me that commit e7cd1d1eb16fcdf53001b926187a82f1f3e1a7e6
> >> did rename the compatible entry from "ti,twl4030-power" to "ti,twl4030-power-reset"
> >> but this was not documented in the bindings and of course our DT does not
> >> match.
> >> 
> >> Even your commit message talks about "ti,twl4030-power" although I can't find it
> >> in the code.
> > 
> > Hmm sorry did I accidentally remove ti,twl4030-power? If so, that should
> > be added back for sure. Do you have a patch for that already?
> 
> No, I have only updated our device tree because I don't know if it really should
> be added back or not.
> 
> As you say the "ti,twl4030-power" does not configure anything. So what
> is it good for?

Only for the poweroff if "ti,use_poweroff" is set. Care to do a patch
as you clearly have a use case to test it with?

> >> Are "ti,twl4030-power" and "ti,twl4030-power-reset" doing the same?
> > 
> > No, they are separate where ti,twl4030-power does not configure the
> > twl4030 in any way where ti,twl4030-power-reset configures the warm
> > reset sequence.
> 
> Yes, that is what I deduced because our old setting of "ti,twl4030-power" did
> no longer configure the power-off and not even load the driver.
> 
> > 
> > For gta04, what you really want is to use ti,twl4030-power-idle or
> > even ti,twl4030-power-idle-osc-off if the board is wired to support
> > cutting off the oscillator.
> 
> Ok, I see (but must admit that I don't understand the details even after
> reading the bindings.txt).

Well the twl4030 has resources such as GPIO pins and regulators that can
be configured to automatically change state for retention and off-idle.
That's how we can cut off the core voltage for idle.
 
> Currently we develop without taking care of suspend (the DT migration was
> much more troublesome work than anticipated) but that should be changed soon.

OK, yeah the PM features should be finally working now with mainline
and DT :)
 
> > And you probably also want to configure some wake-up interrupts at least
> > for the the UARTs in the board specific .dts file, see for example the
> > UART3 in the existing board files that have:
> > 
> > interrupts-extended = <&intc 74 &omap3_pmx_core OMAP3_UART3_RX>;
> 
> thanks for the hint!

No problem. And FYI, the reason why we can't add that automatically is
because there are alternate pins for UART3 pins depending on the
packaging and which pin is actually wired up.

Regards,

Tony

Re: PROBLEM: bindings for drivers/mfd/twl4030-power.c

[Nishanth Menon]

On 09/03/2014 01:45 PM, Tony Lindgren wrote:
> * Dr. H. Nikolaus Schaller <hns@goldelico.com> [140901 09:54]:
>> Hi,
>>
>> Am 25.08.2014 um 23:26 schrieb Tony Lindgren:
>>
>>> * Dr. H. Nikolaus Schaller <hns@goldelico.com> [140817 08:46]:
>>>> I am trying to make ti,use_poweroff work on 3.17-rc1 for the GTA04 board.
>>>> Poweroff was broken for a while and I found that the driver isn't loaded at all.
>>>>
>>>> It appears to me that commit e7cd1d1eb16fcdf53001b926187a82f1f3e1a7e6
>>>> did rename the compatible entry from "ti,twl4030-power" to "ti,twl4030-power-reset"
>>>> but this was not documented in the bindings and of course our DT does not
>>>> match.
>>>>
>>>> Even your commit message talks about "ti,twl4030-power" although I can't find it
>>>> in the code.
>>>
>>> Hmm sorry did I accidentally remove ti,twl4030-power? If so, that should
>>> be added back for sure. Do you have a patch for that already?
>>
>> No, I have only updated our device tree because I don't know if it really should
>> be added back or not.
>>
>> As you say the "ti,twl4030-power" does not configure anything. So what
>> is it good for?
> 
> Only for the poweroff if "ti,use_poweroff" is set. Care to do a patch
> as you clearly have a use case to test it with?

Tony, we were talking about supporting ti,system-power-controller as
the standard way of stating poweroff control is by the PMIC. this
seems to be standard in various SoCs. use_poweroff seems to predate
that standardization. Should'nt we start using
ti,system-power-controller instead?

-- 
Regards,
Nishanth Menon

Re: PROBLEM: bindings for drivers/mfd/twl4030-power.c

[Tony Lindgren]

* Nishanth Menon <nm@ti.com> [140903 11:51]:
> On 09/03/2014 01:45 PM, Tony Lindgren wrote:
> > * Dr. H. Nikolaus Schaller <hns@goldelico.com> [140901 09:54]:
> >> Hi,
> >>
> >> Am 25.08.2014 um 23:26 schrieb Tony Lindgren:
> >>
> >>> * Dr. H. Nikolaus Schaller <hns@goldelico.com> [140817 08:46]:
> >>>> I am trying to make ti,use_poweroff work on 3.17-rc1 for the GTA04 board.
> >>>> Poweroff was broken for a while and I found that the driver isn't loaded at all.
> >>>>
> >>>> It appears to me that commit e7cd1d1eb16fcdf53001b926187a82f1f3e1a7e6
> >>>> did rename the compatible entry from "ti,twl4030-power" to "ti,twl4030-power-reset"
> >>>> but this was not documented in the bindings and of course our DT does not
> >>>> match.
> >>>>
> >>>> Even your commit message talks about "ti,twl4030-power" although I can't find it
> >>>> in the code.
> >>>
> >>> Hmm sorry did I accidentally remove ti,twl4030-power? If so, that should
> >>> be added back for sure. Do you have a patch for that already?
> >>
> >> No, I have only updated our device tree because I don't know if it really should
> >> be added back or not.
> >>
> >> As you say the "ti,twl4030-power" does not configure anything. So what
> >> is it good for?
> > 
> > Only for the poweroff if "ti,use_poweroff" is set. Care to do a patch
> > as you clearly have a use case to test it with?
> 
> Tony, we were talking about supporting ti,system-power-controller as
> the standard way of stating poweroff control is by the PMIC. this
> seems to be standard in various SoCs. use_poweroff seems to predate
> that standardization. Should'nt we start using
> ti,system-power-controller instead?

Sure we can add that. But need to keep also parsing "ti,use_poweroff"
as it's already in use.

Regards,

Tony

Re: PROBLEM: bindings for drivers/mfd/twl4030-power.c

[Nishanth Menon]

On Wed, Sep 3, 2014 at 2:02 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Nishanth Menon <nm@ti.com> [140903 11:51]:
>> On 09/03/2014 01:45 PM, Tony Lindgren wrote:
>> > * Dr. H. Nikolaus Schaller <hns@goldelico.com> [140901 09:54]:
>> >> Hi,
>> >>
>> >> Am 25.08.2014 um 23:26 schrieb Tony Lindgren:
>> >>
>> >>> * Dr. H. Nikolaus Schaller <hns@goldelico.com> [140817 08:46]:
>> >>>> I am trying to make ti,use_poweroff work on 3.17-rc1 for the GTA04 board.
>> >>>> Poweroff was broken for a while and I found that the driver isn't loaded at all.
>> >>>>
>> >>>> It appears to me that commit e7cd1d1eb16fcdf53001b926187a82f1f3e1a7e6
>> >>>> did rename the compatible entry from "ti,twl4030-power" to "ti,twl4030-power-reset"
>> >>>> but this was not documented in the bindings and of course our DT does not
>> >>>> match.
>> >>>>
>> >>>> Even your commit message talks about "ti,twl4030-power" although I can't find it
>> >>>> in the code.
>> >>>
>> >>> Hmm sorry did I accidentally remove ti,twl4030-power? If so, that should
>> >>> be added back for sure. Do you have a patch for that already?
>> >>
>> >> No, I have only updated our device tree because I don't know if it really should
>> >> be added back or not.
>> >>
>> >> As you say the "ti,twl4030-power" does not configure anything. So what
>> >> is it good for?
>> >
>> > Only for the poweroff if "ti,use_poweroff" is set. Care to do a patch
>> > as you clearly have a use case to test it with?
>>
>> Tony, we were talking about supporting ti,system-power-controller as
>> the standard way of stating poweroff control is by the PMIC. this
>> seems to be standard in various SoCs. use_poweroff seems to predate
>> that standardization. Should'nt we start using
>> ti,system-power-controller instead?
>
> Sure we can add that. But need to keep also parsing "ti,use_poweroff"
> as it's already in use.
>

Yep.
https://patchwork.kernel.org/patch/4836381/
https://patchwork.kernel.org/patch/4836371/

Split documentation out (based on discussion in
https://patchwork.kernel.org/patch/4743321/).

-- 
---
Regards,
Nishanth Menon