public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Paul Walmsley <paul@pwsan.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Ricardo Neri <ricardo.neri@ti.com>,
	b-cousson@ti.com, linux-omap@vger.kernel.org, mythripk@ti.com,
	s-guiriec@ti.com, lrg@ti.com, peter.ujfalusi@ti.com
Subject: Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio
Date: Fri, 16 Dec 2011 12:59:09 -0800	[thread overview]
Message-ID: <20111216205908.GO32251@atomide.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1112161320040.12660@utopia.booyaka.com>

* Paul Walmsley <paul@pwsan.com> [111216 12:20]:
> Hi
> 
> On Fri, 16 Dec 2011, Tony Lindgren wrote:
> 
> > * Paul Walmsley <paul@pwsan.com> [111216 00:41]:
> > > On Fri, 16 Dec 2011, Tomi Valkeinen wrote:
> > > 
> > > > On Fri, 2011-12-16 at 01:27 -0700, Paul Walmsley wrote:
> > > > > On Fri, 16 Dec 2011, Tomi Valkeinen wrote:
> > > > > 
> > > > > > But with DT we can't use func pointers in platform_data either, right?
> > > > > 
> > > > > In the future, if someone wants to run a platform_data-less kernel, 
> > > > > they'll have to come up with a replacement mechanism for these.  Several 
> > > > > replacements have been proposed internally, such as having an 
> > > > > omap_bus/omap_device for devices with OMAP-specific integration, but right 
> > > > > now there are more pressing crises to deal with...
> > > > 
> > > > Ok. Benoit was telling me not to use pdata, so I thought it's a hard
> > > > rule for DT. He didn't give me a clear alternative, though =).
> > > 
> > > As far as I know, we've got no other choice.  And if we don't add these, 
> > > then not only will current code be broken, but when the time comes to 
> > > convert away from using platform_data function pointers, then no one will 
> > > know what functions should be exposed from the integration code for the 
> > > driver to call.
> > 
> > There really should not be any need for platform_data with device tree.
> 
> Sure, if there's some replacement way for drivers to call integration 
> functions.  Right now there's no substitute, AFAIK.  So far no one who has 
> written that platform_data function pointers should not be used has been 
> able to come up with a concrete alternative.
> 
> > Idling a device should be done with pm_runtime calls. Then the
> > bus code should idle the right device, in this case using the hwmod
> > calls.
> 
> There seems to be some misunderstanding here...
> 
> This patch falls into one of two cases, as discussed further in 
> 
>    http://marc.info/?l=linux-omap&m=132402667320943&w=2
> 
> If the device never needs to run in smart-idle wakeup mode, then what you 
> wrote is right -- there's no need for a new device->integration function 
> call here.  The HWMOD_SWSUP_SIDLE flag should work fine, and no 
> new platform_data function pointers are needed.
> 
> However, if the device does occasionally need to run at different levels 
> of "idle", as several of our devices do, then some new device->integration 
> function calls are indeed needed.  PM runtime doesn't provide an interface 
> for varying gradations of "idle".  This was discussed with Rafael and 
> Magnus a few months ago at LinuxCon Japan, and Rafael indicated at the 
> time that he felt those should be implemented by some other mechanism, 
> outside PM runtime.  This makes some sense since it's not clear right now 
> (to me, anyway) how to implement something like this in a completely 
> generic way.
> 
> ...
> 
> Generally speaking, there are several integration functions that we either 
> call now via platform_data function pointers, or will need to call in the 
> future, from drivers.  Even after the device tree conversion.  Adjusting 
> the idle state that an IP block can enter when it enters PM runtime idle 
> is one set.  Implementing integration-level device reset while the device 
> is enabled is another -- PM runtime (rightly in my view) doesn't have 
> anything to do with that.
> 
> > Most of the platform_data function pointers can be replaced with
> > Linux generic calls for regulator framework etc. If some frameworks
> > are missing, then that's obviously a problem that should be addressed.
> 
> As written here:
> 
>    http://marc.info/?l=linux-omap&m=132402415520208&w=2
> 
> there seem to be a few different options for long-term approaches.  But 
> implementing these will take quite some time and discussion.  Depending on 
> what's done, the solution might touch a lot of code.  In the meantime we 
> have bugs that should be fixed.  Dealing with them now with platform_data 
> function pointers has the nice side benefit that it provides visibility to 
> the remaining operations that do need to cross the device->integration 
> boundary.
> 
> So in case it was unclear, I'm not advocating platform_data function 
> pointers are the long-term answer.  But they seem to be an important step 
> in the transition to whatever mechanism appears next, since no alternative 
> seems to exist.

OK, sounds like there's no "official" way to do a bus level reset of
a device right now from the device itself. So let's keep the platform_data
around for that until we have some generic way of doing the reset from
driver.
 
> > Different devices can be handled with a combination of compatible
> > flag + data. For example, take a look at drivers/tty/serial/of_serial.c
> > and note how the of_platform_serial_table has a .data pointer for
> > each different device.
> 
> Hmm, I don't quite understand this part.  You're referring to 
> non-executable data here?

Right, the .data can contain knowledge how to reset device aaaa compared
to similar device bbbb, but sounds like that won't help here as we don't
have a way of doing it cleanly from the driver.

Regards,

Tony

      reply	other threads:[~2011-12-16 20:59 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-16  7:03 [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio Ricardo Neri
2011-12-16  8:14 ` Paul Walmsley
2011-12-16  8:18   ` Tomi Valkeinen
2011-12-16  8:27     ` Paul Walmsley
2011-12-16  8:47       ` Tomi Valkeinen
2011-12-16  9:09         ` Paul Walmsley
2011-12-16  9:11         ` Cousson, Benoit
2011-12-16  9:13         ` Paul Walmsley
2011-12-16 17:19           ` Tony Lindgren
2011-12-16 20:52             ` Paul Walmsley
2011-12-16 20:59               ` Tony Lindgren [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111216205908.GO32251@atomide.com \
    --to=tony@atomide.com \
    --cc=b-cousson@ti.com \
    --cc=linux-omap@vger.kernel.org \
    --cc=lrg@ti.com \
    --cc=mythripk@ti.com \
    --cc=paul@pwsan.com \
    --cc=peter.ujfalusi@ti.com \
    --cc=ricardo.neri@ti.com \
    --cc=s-guiriec@ti.com \
    --cc=tomi.valkeinen@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox