Re: [PATCH v2 1/2] ASoC: soc-compress: add a config item for soc-compress

[Vinod Koul <vinod.koul@intel.com>]

On Tue, Jun 16, 2015 at 06:36:22PM +0200, Takashi Iwai wrote:
> At Tue, 16 Jun 2015 21:55:07 +0530,
> Vinod Koul wrote:
> > 
> > On Tue, Jun 16, 2015 at 06:17:12PM +0200, Takashi Iwai wrote:
> > > At Tue, 16 Jun 2015 18:14:40 +0200,
> > > Takashi Iwai wrote:
> > > > 
> > > > At Tue, 16 Jun 2015 17:06:07 +0100,
> > > > Mark Brown wrote:
> > > > > 
> > > > > On Tue, Jun 16, 2015 at 12:36:16PM +0000, Jie, Yang wrote:
> > > > > 
> > > > > > > OK, that should have been in the commit message.
> > > > > 
> > > > > > OK, so let me add it to the commit message and resend?
> > > > > 
> > > > > Well, there's still the stubs to consider.  Should we really be
> > > > > returning an error or should we silently ignore the error and hide the
> > > > > DAI if the user deconfigured compressed audio?  Or rearrange things so
> > > > > we don't need stubs?  Given that the machine driver has to select
> > > > > compressed support it's not something that should ever really hit the
> > > > > stubs, it's not truly a runtime thing...
> > > > 
> > > > Yes, I guess that leaving without dummy function will give unresolved
> > > > symbol errors at module link time, so the user can catch before
> > > > actually running it.  Of course, this should be checked actually.
> > > 
> > > Oh, scratch this.  It won't work well in the current code.
> > > Possibly with weak linking, but...
> > Right as the soc_new_compress() is called from soc-core for compressed dai
> > links. Since ASoC drivers don't use any of compress APIs directly, it would
> > be difficult to add a compile time failure so we are stuck with silent
> > runtime failures :(
> 
> One feasible option would be to make the driver's dai creation a
> function pointer to the generic constructor.  That is,
> soc_probe_link_dais() will run like:
> 
> 	if (cpu_dai->driver->create)
> 		ret = cpu_dai->driver->create(rtd, num);
> 
> and each driver passes soc_new_compress (or whatever) there.  This is
> flexible and can be extended if any new stuff has to be handled.
> Even PCM creation or dai widget links can be passed in that form,
> too.
and the default would be snd_pcm_new so that *most* drivers dont need to add
this. Only Intel atom driver should add soc_new_compress() as create ops,
hence direct reference so no gaurds required

Looks good to me then, Mark you okay with this approach?

-- 
~Vinod