From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Hilman Subject: Re: [PATCH 4/6] ARM: OMAP4: PMU: Add runtime PM support Date: Thu, 31 May 2012 15:36:36 -0700 Message-ID: <87d35kyo3v.fsf@ti.com> References: <1336599355-10983-1-git-send-email-jon-hunter@ti.com> <87wr3uelgp.fsf@ti.com> <4FC548A3.2040906@ti.com> <4FC54D3B.10301@ti.com> <87pq9l7306.fsf@ti.com> <20120531012923.GB8506@mudshark.cambridge.arm.com> <4FC7B465.60703@ti.com> <87r4u0ytd9.fsf@ti.com> <4FC7E161.1080400@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from na3sys009aog112.obsmtp.com ([74.125.149.207]:52136 "EHLO na3sys009aog112.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758476Ab2EaWgh (ORCPT ); Thu, 31 May 2012 18:36:37 -0400 Received: by pbcwy7 with SMTP id wy7so1911288pbc.3 for ; Thu, 31 May 2012 15:36:35 -0700 (PDT) In-Reply-To: <4FC7E161.1080400@ti.com> (Jon Hunter's message of "Thu, 31 May 2012 16:23:45 -0500") Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: Jon Hunter Cc: Will Deacon , linux-omap , Ming Lei , Benoit Cousson , Paul Walmsley Jon Hunter writes: > Hi Kevin, > > On 05/31/2012 03:42 PM, Kevin Hilman wrote: >> Jon Hunter writes: >> >>> Hi Kevin, Will, >>> >>> On 05/30/2012 08:29 PM, Will Deacon wrote: >>>> Hi Kevin, >>>> >>>> On Wed, May 30, 2012 at 10:50:01PM +0100, Kevin Hilman wrote: >>>>> Basically, I don't like the result when we have to hack around missing >>>>> runtime PM support for a driver, so IMO, the driver should be updated. >>>>> >>>>> IOW, it looks to me like the armpmu driver should grow runtime PM >>>>> support. The current armpmu_release|reserve should probably be replaced >>>>> with runtime PM get/put, and the functionality in those functions would >>>>> be the runtime PM callbacks instead. >>>>> >>>>> Will, any objections to armpmu growing runtime PM support? >>>> >>>> My plan for the armpmu reservation is to kill the global reservation scheme >>>> that we currently have and push those function pointers into the arm_pmu, >>>> so that fits with what you'd like. >>>> >>>> The only concern I have is that we need the mutual exclusion even when we >>>> don't have support for runtime PM. If we can solve that then I'm fine with >>>> the approach. >>> >>> To add a bit more food for thought, I had implemented a quick patch to >>> add runtime PM support for PMU. You will notice that I have been >>> conservative on where I have placed the pm_runtime_get/put calls, >>> because I am not too familiar with the PMU driver to know exactly >>> where we need to maintain the PMU context. So right now these are just >>> around the reserve_hardware/release_hardware calls. This works on OMAP >>> for some quick testing. However, I would need to make sure this does >>> not break compilation without runtime PM enabled. >>> >>> Let me know your thoughts. >> >> That looks good, but I'm curious what would be done in the new >> plat->runtime_* hooks. Maybe the irq enable/disable stuff in the pmu >> driver needs to be moved into the runtime PM hooks? > > For omap4, the plat->runtime_* hooks look like ... > > +static int omap4_pmu_runtime_resume(struct device *dev) > +{ > + /* configure CTI0 for PMU IRQ routing */ > + cti_unlock(&omap4_cti[0]); > + cti_map_trigger(&omap4_cti[0], 1, 6, 2); > + cti_enable(&omap4_cti[0]); > + > + /* configure CTI1 for PMU IRQ routing */ > + cti_unlock(&omap4_cti[1]); > + cti_map_trigger(&omap4_cti[1], 1, 6, 3); > + cti_enable(&omap4_cti[1]); > + > + return 0; > +} > + > +static int omap4_pmu_runtime_suspend(struct device *dev) > +{ > + cti_disable(&omap4_cti[0]); > + cti_disable(&omap4_cti[1]); > + > + return 0; > +} > > This is what I have implemented so far and currently testing. So really > just using the hooks to configure the cross triggering interface. > > Is this what you were thinking? > Basically, yes. But since I haven't studied the PMU driver closely, I have some dumb questions. My concern is that these look bsically like the plat->irq_[enable|disable] hooks, so I guess the root of my question is do we need both the irq enable/disable and runtime suspend/resume hooks in plat? or can we get by with one set. Kevin