[PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Ricardo Neri]

When playing audio through HDMI, the module should be put in
no-idle mode. This is to prevent the DSS_L3_ICLK to be shut down.
Once audio playback is stopped, the module is set back to smart-idle
wakeup-capable.

Also, a omap_hwmod structure is added to hdmi_ip_data to have access
to idle-mode configuration.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 drivers/video/omap2/dss/hdmi.c    |   13 +++++++++++++
 drivers/video/omap2/dss/ti_hdmi.h |    4 ++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 5c93041..e177338 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -35,6 +35,7 @@
 #include <video/omapdss.h>
 #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
 	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
+#include <plat/omap_hwmod.h>
 #include <sound/soc.h>
 #include <sound/pcm_params.h>
 #include "ti_hdmi_4xxx_ip.h"
@@ -572,12 +573,16 @@ int hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		omap_hwmod_set_slave_idlemode(ip_data->oh,
+			HWMOD_IDLEMODE_NO);
 		ip_data->ops->audio_enable(ip_data, true);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		ip_data->ops->audio_enable(ip_data, false);
+		omap_hwmod_set_slave_idlemode(ip_data->oh,
+			HWMOD_IDLEMODE_SMART_WKUP);
 		break;
 	default:
 		err = -EINVAL;
@@ -733,8 +738,16 @@ static int hdmi_audio_startup(struct snd_pcm_substream *substream,
 
 static int hdmi_audio_codec_probe(struct snd_soc_codec *codec)
 {
+	struct platform_device *pdev = to_platform_device(codec->dev);
 	struct hdmi_ip_data *priv = &hdmi.ip_data;
 
+	priv->oh = omap_hwmod_lookup("dss_hdmi");
+
+	if (!priv->oh) {
+		dev_err(&pdev->dev, "Cannot find omap_hwmod for hdmi\n");
+		return -ENODEV;
+	}
+
 	snd_soc_codec_set_drvdata(codec, priv);
 	return 0;
 }
diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
index 7503f7f..5ef3cb1 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -126,6 +126,10 @@ struct hdmi_ip_data {
 	const struct ti_hdmi_ip_ops *ops;
 	struct hdmi_config cfg;
 	struct hdmi_pll_info pll_data;
+#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
+	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
+	struct omap_hwmod *oh;
+#endif
 };
 int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
 void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
-- 
1.7.0.4

Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Paul Walmsley]

Hi

some comments

On Fri, 16 Dec 2011, Ricardo Neri wrote:

> When playing audio through HDMI, the module should be put in
> no-idle mode. This is to prevent the DSS_L3_ICLK to be shut down.
> Once audio playback is stopped, the module is set back to smart-idle
> wakeup-capable.

This is due to a hardware bug/erratum?  If so, the erratum ID should be 
mentioned.  Also, please mention what happens if the module runs in 
smart-idle - some type of audio glitch?

> Also, a omap_hwmod structure is added to hdmi_ip_data to have access
> to idle-mode configuration.
> 
> Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
> ---
>  drivers/video/omap2/dss/hdmi.c    |   13 +++++++++++++
>  drivers/video/omap2/dss/ti_hdmi.h |    4 ++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 5c93041..e177338 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -35,6 +35,7 @@
>  #include <video/omapdss.h>
>  #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
>  	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
> +#include <plat/omap_hwmod.h>
>  #include <sound/soc.h>
>  #include <sound/pcm_params.h>
>  #include "ti_hdmi_4xxx_ip.h"
> @@ -572,12 +573,16 @@ int hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
>  	case SNDRV_PCM_TRIGGER_START:
>  	case SNDRV_PCM_TRIGGER_RESUME:
>  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		omap_hwmod_set_slave_idlemode(ip_data->oh,
> +			HWMOD_IDLEMODE_NO);

omap_hwmod functions should not be called from device driver code.  Device 
drivers should be completely independent of architecture and 
subarchitecture code.  If you need to do something like this, the way to 
do it is to call an omap_device function through a platform_data function 
pointer.  You may need to create omap_device and/or omap_hwmod functions 
to do what you want to do.

>  		ip_data->ops->audio_enable(ip_data, true);
>  		break;
>  	case SNDRV_PCM_TRIGGER_STOP:
>  	case SNDRV_PCM_TRIGGER_SUSPEND:
>  	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>  		ip_data->ops->audio_enable(ip_data, false);
> +		omap_hwmod_set_slave_idlemode(ip_data->oh,
> +			HWMOD_IDLEMODE_SMART_WKUP);

Same comment as above.

>  		break;
>  	default:
>  		err = -EINVAL;


And then the rest of this patch shouldn't be needed.

> @@ -733,8 +738,16 @@ static int hdmi_audio_startup(struct snd_pcm_substream *substream,
>  
>  static int hdmi_audio_codec_probe(struct snd_soc_codec *codec)
>  {
> +	struct platform_device *pdev = to_platform_device(codec->dev);
>  	struct hdmi_ip_data *priv = &hdmi.ip_data;
>  
> +	priv->oh = omap_hwmod_lookup("dss_hdmi");
> +
> +	if (!priv->oh) {
> +		dev_err(&pdev->dev, "Cannot find omap_hwmod for hdmi\n");
> +		return -ENODEV;
> +	}
> +
>  	snd_soc_codec_set_drvdata(codec, priv);
>  	return 0;
>  }
> diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
> index 7503f7f..5ef3cb1 100644
> --- a/drivers/video/omap2/dss/ti_hdmi.h
> +++ b/drivers/video/omap2/dss/ti_hdmi.h
> @@ -126,6 +126,10 @@ struct hdmi_ip_data {
>  	const struct ti_hdmi_ip_ops *ops;
>  	struct hdmi_config cfg;
>  	struct hdmi_pll_info pll_data;
> +#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
> +	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
> +	struct omap_hwmod *oh;
> +#endif
>  };
>  int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
>  void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


- Paul

Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 904 bytes --]

On Fri, 2011-12-16 at 01:14 -0700, Paul Walmsley wrote:
> > @@ -572,12 +573,16 @@ int hdmi_audio_trigger(struct
> snd_pcm_substream *substream, int cmd,
> >       case SNDRV_PCM_TRIGGER_START:
> >       case SNDRV_PCM_TRIGGER_RESUME:
> >       case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> > +             omap_hwmod_set_slave_idlemode(ip_data->oh,
> > +                     HWMOD_IDLEMODE_NO);
> 
> omap_hwmod functions should not be called from device driver code.
> Device 
> drivers should be completely independent of architecture and 
> subarchitecture code.  If you need to do something like this, the way
> to 
> do it is to call an omap_device function through a platform_data
> function 
> pointer.  You may need to create omap_device and/or omap_hwmod
> functions 
> to do what you want to do. 

But with DT we can't use func pointers in platform_data either, right?

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Paul Walmsley]

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...


- Paul

Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]

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 =).

Ricardo, struct omap_dss_board_info already contains a few function
pointers, for example get_context_loss_count. I think
omap_hwmod_set_slave_idlemode can be handled the same way.

Although I'm not sure should the function pointer be just
"set_slave_idlemode", and thus just a direct way to call the hwmod func,
or should it be something more use case specific, like, say
"hdmi_audio_start/stop".

I'm thinking it should be the latter. As far as I understand, the bug is
not in the HDMI IP, but somewhere in the L3 side? And if so, it's not
the HDMI driver's job to know how to fix the bug, but just to offer
hooks so that the platform code can fix it.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2436 bytes --]

On Fri, 16 Dec 2011, Tomi Valkeinen wrote:

> Ricardo, struct omap_dss_board_info already contains a few function
> pointers, for example get_context_loss_count. I think
> omap_hwmod_set_slave_idlemode can be handled the same way.
> 
> Although I'm not sure should the function pointer be just
> "set_slave_idlemode", and thus just a direct way to call the hwmod func,
> or should it be something more use case specific, like, say
> "hdmi_audio_start/stop".
> 
> I'm thinking it should be the latter. As far as I understand, the bug is
> not in the HDMI IP, but somewhere in the L3 side? And if so, it's not
> the HDMI driver's job to know how to fix the bug, but just to offer
> hooks so that the platform code can fix it.

Should the HDMI module ever be allowed to run in smart-idle wakeup mode 
while it's active?  If not, then the easy way to fix this bug would be to 
add the HWMOD_SWSUP_SIDLE flag to the HDMI omap_hwmod.flags field.  No 
omap_device/omap_hwmod code/pdata hacking needed.

On the other hand, if there are times when the HDMI module should run in 
smart-idle wakeup while it's active, then some code would be needed.  
It's ironic that the bug is related to L3 interface clock autoidle, but 
that we no longer have the ability to control that on OMAP4...  My 
recollection is that the McBSP + sidetone blocks have exactly the same bug 
on OMAP3, but that case is even worse because they can't use the module 
slave idle bits to fix the problem due to some power management 
regression.

So anyway, off the top of my head, I'd naïvely suggest something like 
omap_hwmod_block_slave_autoidle() and omap_hwmod_allow_slave_autoidle().  
I don't think we'd have a situation where we'd need to use-count these, 
unless you think otherwise.  So I'd suggest having those functions do 
nothing if HWMOD_SWSUP_SIDLE is set in omap_hwmod.flags.  Otherwise, I'd 
suggest that block() should set a new omap_hwmod._int_flags bit, 
_HWMOD_SWSUP_SIDLE_FORCE or something, and call _set_slave_idlemode() 
appropriately.  allow() should clear that flag and call 
_set_slave_idlemode() appropriately.  It will also be necessary to change 
every place in the hwmod code that tests omap_hwmod.flags against 
HWMOD_SWSUP_SIDLE to also test omap_hwmod._int_flags against 
_HWMOD_SWSUP_SIDLE_FORCE, e.g., in _{enable,disable}_wakeup(), 
_{enable,idle}_sysc(), etc.

Thoughts welcome.

- Paul

Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Cousson, Benoit]

On 12/16/2011 9:47 AM, 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 =).

Well, it is a hard requirement for DT...
So we'd better find alternative now, than introducing code we will have 
to remove later for my point of view.
But I guess it is up to you to decide the best strategy here...

I'm sorry if the DT community did not provide any better guidelines to 
replace that :-(

There are a bunch of arch that does not have any pdev/pdata, so some 
alternative should exist... I hope :-)

Regards,
Benoit

Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Paul Walmsley]

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.


- Paul

Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Tony Lindgren]

* 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.

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.

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.

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.

Regards,

Tony

Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Paul Walmsley]

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.

> 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?


- Paul

Re: [PATCH] ASoC: OMAP: HDMI: Prevent DSS module from going idle when playing audio

[Tony Lindgren]

* 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