Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

[Deepthi Dharwar <deepthi@linux.vnet.ibm.com>]

On 08/23/2013 02:54 AM, Scott Wood wrote:
> On Thu, 2013-08-22 at 11:20 +0530, Deepthi Dharwar wrote:
>> On 08/22/2013 01:38 AM, Scott Wood wrote:
>>> On Wed, 2013-08-21 at 10:23 +0530, Deepthi Dharwar wrote:
>>>> On 08/19/2013 11:47 PM, Scott Wood wrote:
>>>>> What actual functionality is common to all powerpc but not common to
>>>>> other arches?
>>>
>>
>> The functionality here is idle states on powerpc  like the snooze loop
>> that is common.
>> Also, the basic registration of the driver, hotplug notifier etc for
>> powerpc.
> 
> The snooze loop uses things like SPRN_PURR, get_lppaca(), and CTRL which
> aren't common to all PPC (they might be common to all book3s64).  I also
> don't see any hook for the low power mode entry -- is "snooze" just a
> busy loop plus the de-emphasis stuff like HMT and CTRL[RUN]?  I'm not
> familiar with the term "snooze" in this context.  I don't think we'd use
> anything like that on our chips; we'd always at least "wait" or "doze"
> depending on the chip.
>

Duly noted. Lot of stuff are common across book3s64. So my later
versions of this patchset does just that. (V5 posted out yesterday).
The driver is common only to IBM-POWER platform. Other PPC variants
can have their own driver.


> It's not clear what is powerpc-specific about the notifier -- perhaps it
> should go in drivers/cpuidle/.

Currently all the arcs have their own hotplug notifier. Unifying this
across all archs is a challenge that needs to be taken going forward.

Thanks for the review.
Regards,
Deepthi


>>> The way forward is to give this file a more appropriate name based on
>>> the hardware that it actually targets -- and to refactor it so that the
>>> answer to that question is not complicated.
>>
>> Sure, thanks.
>> Our idea was to have POWER archs idle states merged at the first go.
>> Only that is what is enabled in the current version (V4 posted out)
>> ( Code is enabled for PSERIES and POWERNV only)
>> If needed, other POWERPC archs might benefit by extending the same
>> driver, that is why it is named cpuidle-powerpc.c
>>
>> But if having cpuidle backend-driver separately for other powerpc arcs
>> makes sense such that each one have their own state information etc
>> then it makes sense to name the files as cpuidle-power.c,
>> cpuilde-ppc32.c and so on.
> 
> Thanks.
> 
> -Scott
> 
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 
>