Re: [PATCH 6/9] ARM: tegra: Export tegra_powergate_power_on

[Stephen Warren <swarren@wwwdotorg.org>]

On 06/19/2014 02:02 AM, Peter De Schrijver wrote:
> On Wed, Jun 18, 2014 at 05:37:54PM +0200, Stephen Warren wrote:
>> On 06/18/2014 06:18 AM, Peter De Schrijver wrote:
>>> On Tue, Jun 17, 2014 at 11:51:20PM +0200, Thierry Reding wrote:
>>>> * PGP Signed by an unknown key
>>>>
>>>> On Tue, Jun 17, 2014 at 05:01:46PM +0300, Peter De Schrijver wrote:
>>>>> On Tue, Jun 17, 2014 at 02:13:15PM +0200, Thierry Reding wrote:
>>>>>>> Old Signed by an unknown key
>>>>>>
>>>>>> On Mon, Jun 16, 2014 at 04:01:02PM -0600, Stephen Warren wrote:
>>>>>>> On 06/04/2014 05:32 AM, Mikko Perttunen wrote:
>>>>>>>> This symbol needs to be exported to power on rails without using
>>>>>>>> tegra_powergate_sequence_power_up. tegra_powergate_sequence_power_up
>>>>>>>> cannot be used in situations where the driver wants to handle clocking
>>>>>>>> by itself.
>>>>>>>
>>>>>>> Thierry, are you OK with this change?
>>>>>>
>>>>>> I would've preferred tegra_powergate_sequence_power_up() to be used
>>>>>
>>>>> I don't think the current tegra_powergate_sequence_power_up() API is very well
>>>>> defined though. I don't think the clocks and resets required by the sequence
>>>>> should be provided by the driver. For one, there can be several clocks and
>>>>> resets that need to be controlled for a single domain.
>>>>
>>>> Do you have any suggestions for what the API should look like? Even if
>>>> we plan to move to some different API, I think there's some advantage in
>>>> using it consistently if for no other reason than to make it easier to
>>>> replace occurrences later on.
>>>>
>>>
>>> I think the API should only have the domain ID as input so:
>>>
>>> int tegra_powerdomain_on(int id) 
>>>
>>> /*
>>>  * Prerequisites: domain is off
>>>  * Result: domain is on, clocks of the modules in the domain are off, modules are in reset
>>>  */
>>>
>>> int tegra_powerdomain_off(int id)
>>>
>>> /*
>>>  * Prerequisites: all clocks of the modules in the domain are off
>>>  * result: domain is off
>>>  */
>>
>> That doesn't make sense; the PMC doesn't have access to the clock and
>> reset IDs - that's why the API requires them to be passed in.
>>
> 
> We should make driver look them up by name then. It doesn't make sense to
> move this knowledge to the drivers as there can be several modules in 1
> powerdomain. So every driver would then need to have access to all clock
> and reset IDs of the modules of the domain?

Having the drivers know the clocks, resets, and power domains that
affect their HW module seems perfectly reasonable.

Do we actually have any case where unrelated modules are in the same
power domain /and/ there's some need for those drivers to manipulate the
power domain?

BTW, any objections here should have been raised when the initial API
design for powergating was created. Now that we have the current API,
which in turn has driven the DT bindings for at least some HW modules,
we're stuck with it due to DT ABIness. Well, perhaps we can introduce
something new, but we'll always have to support the old way, so there's
little point.