[PATCH RFC v2 09/16] arm: domain: Add platform callbacks for domain power on/off

[lina.iyer@linaro.org (Lina Iyer)]

On Mon, Jul 06 2015 at 09:18 -0600, Lina Iyer wrote:
>On Fri, Jul 03 2015 at 05:36 -0600, Geert Uytterhoeven wrote:
>>Hi Lina,
>>
>>On Thu, Jul 2, 2015 at 9:38 PM, Lina Iyer <lina.iyer@linaro.org> wrote:
>>>On Tue, Jun 30 2015 at 09:10 -0600, Geert Uytterhoeven wrote:
>>>>On Mon, Jun 29, 2015 at 6:32 PM, Lina Iyer <lina.iyer@linaro.org> wrote:
>>>>>On Mon, Jun 29 2015 at 07:36 -0600, Geert Uytterhoeven wrote:
>
>[...]
>
>>>With what you have suggested, the platform driver creates the genpd and
>>>would pass the CPU genpd to the common code for common operations. (This
>>>was what was done in [1]). The platform driver would set the power_on()
>>>and power_off() callbacks and that would have to be overriden in order
>>>handle common CPU domain suspend/resume activities. Overwriting members
>>>of an object allocated by the platform driver, is something we should
>>>avoid.
>>
>>Instead of letting the generic code override the .power_{on,off}() callbacks,
>>the platform code could call the generic CPU-related methods from its own
>>.power_{on,off}() callbacks?
>>
>We dont want to impose on platform drivers to call back into the CPU
>domain core code to do common domain power down activities. On a generic
>ARM CPU domain, we may not even need a platform specific domain
>callbacks. IMHO, this is not desirable.
>
Any thoughts on this?

>>struct rmobile_pm_domain already has .suspend() and .resume() methods.
>>The former is used to e.g. prevent the PM domains containing CPUs to be
>>powered down (in the absence of cpuidle integration). That requires scanning
>>the DT for CPUs, and it would indeed be good to have that scanning support
>>in generic code.
>>The latter became unused after the removal of sh7372 support, which did have
>>some cpuidle integration.
>>
>The CPUs may still be attached to domain, even if they are not powered
>off by CPUIdle. The code increases the reference count for every online
>CPU and any CPUs that may come online. The reference count, only goes
>down when the CPU powers down through CPUIdle or hotplug. So, if a CPU
>is not integrated with CPUIdle, it would remain in use (unless powered
>off by hotplug) and the domain would not be powered down.
>
>>>Or instead of allocating the memory in your platform driver for the CPU
>>>genpd, you could query and get the genpd and then add your additions on
>>>top. You could add other flags like GENPD_FLAG_PM_CLK, but *not*
>>>overwrite the power on/off callbacks in the genpd. They still have to be
>>>registered separately like in this patch. Again, not every elegant, IMO.
>>
>>Just wondering, can I set up the .attach_dev() and .detach_dev()?
>>
>I am presuming, you are concerned with the fore mentioned case of CPUs
>not participating in CPUIdle. With pm_runtime_put_sync() not happening
>for CPUs that do not power down, you should not have to worry about the
>domain being powered down.
>
>Please correct me if I misunderstood your point.
>
>>Actually, for R-Mobile hardware I don't have a need to set GENPD_FLAG_PM_CLK
>>or .attach_dev() and .detach_dev() for CPU PM domains, as they are only
>>needed for devices with MSTP clocks. CPU and L2 cache don't have these,
>>and there are no other devices in e.g. a3sm and a2sl.
>>
>>The GIC has an MSTP clock, but it's part of a different power domain.
>>
>Okay.
>
>[...]
>
>>>This the common code would get its own callbacks and when that genpd
>>>powers off, the platform genpd would power down. But no new code is
>>>needed in your platform driver. The benefit is that platform code and
>>>generic CPU domain code may exist and act in parallel and may only be
>>>related if specified in the DT and the problem with that approach is
>>>that this virtual PM domain is not a h/w domain, hence specifying in DT
>>>is questionable.
>>
>>Indeed, I don't like this option, as the DT would no longer describe HW,
>>but the Linux implementation.
>>
>>So let's continue with your approach, and see how it turns out. We can
>>always change and improvide code, while changing DT is more complicated.
>>
>Agreed.
>
>Thanks for your time, Geert.
>
>-- Lina