On Mon, Jun 10, 2019 at 12:21:47PM +0200, Ulf Hansson wrote: > On Fri, 7 Jun 2019 at 17:31, Sudeep Holla <sudeep.holla@xxxxxxx> wrote: > > > > On Mon, May 13, 2019 at 09:22:57PM +0200, Ulf Hansson wrote: > > > When the hierarchical CPU topology is used and when a CPU has been put > > > offline (hotplug), that same CPU prevents its PM domain and thus also > > > potential master PM domains, from being powered off. This is because genpd > > > observes the CPU's attached device as being active from a runtime PM point > > > of view. > > > > > > To deal with this, let's decrease the runtime PM usage count by calling > > > pm_runtime_put_sync_suspend() of the attached struct device when putting > > > the CPU offline. Consequentially, we must then increase the runtime PM > > > usage count, while putting the CPU online again. > > > > > > > Why is this firmware/driver specific ? Why can't this be dealt in core > > pm-domain ? I am concerned that if any other architectures or firmware > > method decides to use this feature, this need to be duplicated there. > > What is the core pm-domain? Do you refer to the generic PM domain (genpd), no? > Sorry for my bad choice of names. I just wrote names as I understand rather than looking for exact match. But yes, I meant generic place where such ref-counting is done currently for other things. > In such case, this is not the job of genpd, but rather the opposite > (to *monitor* the reference count). > OK, I need to understand that then. > > > > The way I see this is pure reference counting and is hardware/firmware/ > > driver agnostic and can be made generic. > > As stated in the another reply, I would rather start with having more > things driver specific rather than generic. Later on we can always > consider to move/split things, when there are more users. > > In this particular case, the runtime PM reference counting is done on > the struct device*, that genpd returned via > dev_pm_domain_attach_by_name(). And because > dev_pm_domain_attach_by_name() is called from PSCI code, I decided to > keep this struct device* internal to PSCI. Sure, I understand your intent. I have just mentioned my thoughts/comments as I reviewed. -- Regards, Sudeep
