Re: [PATCH 1/2] ASoC: uda134x: replace a macro with a value in platform struct.

[Liam Girdwood <lrg@slimlogic.co.uk>]

On Thu, 2010-06-24 at 20:21 +0400, Vladimir Zapolskiy wrote:
> On Thu, Jun 24, 2010 at 8:05 PM, Liam Girdwood <lrg@slimlogic.co.uk> wrote:
> > On Thu, 2010-06-24 at 19:54 +0400, Vladimir Zapolskiy wrote:
> >> Hi,
> >>
> >> On Thu, Jun 24, 2010 at 5:49 PM, Liam Girdwood <lrg@slimlogic.co.uk> wrote:
> >> > On Thu, 2010-06-24 at 17:38 +0400, Vladimir Zapolskiy wrote:
> >
> >> >> @@ -559,9 +546,11 @@ static int uda134x_soc_probe(struct platform_device *pdev)
> >> >>       codec->num_dai = 1;
> >> >>       codec->read = uda134x_read_reg_cache;
> >> >>       codec->write = uda134x_write;
> >> >> -#ifdef POWER_OFF_ON_STANDBY
> >> >> -     codec->set_bias_level = uda134x_set_bias_level;
> >> >> -#endif
> >> >> +
> >> >> +     if (!pd->is_powered_on_standby) {
> >> >> +             codec->set_bias_level = uda134x_set_bias_level;
> >> >
> >> > The codec ops pointers have moved in multi-component. Can you put this
> >> > check in your uda134x_set_bias_level() instead.
> >> >
> >> Not sure I understand the problem, and I do not see a simple solution
> >> for actual bugfix in patch 2/2, if this check is placed in
> >> uda134x_set_bias_level(). Please could you provide me with a hint or
> >> link to some codec source file?
> >
> > I was meaning something like :-
> >
> > uda134x_set_bias_level()
> > {
> >        if (pd->is_powered_on_standby)
> >                return
> >
> >        /* rest of func */
> >
> > }
> >
> Thank you, good that I meant the same. But that makes quite impossible
> to set a mandatory bias level on initialization without additional code
> complication.
> 

Sorry, I'm just trying to make the multi-component merge simpler here.

The multi-component branch is here :-

http://git.kernel.org/?p=linux/kernel/git/lrg/asoc-2.6.git;a=shortlog;h=refs/heads/topic/multi-component

I'll merge this myself later.

> > The multi-component branch makes codec->set_bias_level a driver level
> > operation (that can be shared amongst other uda134x codec devices so we
> > cant change this at runtime)
> >
> Looks like I miss something, because in codec drivers I observe
> codec->set_bias_level is set in probe function, like it is in uda134x.
> 
> Would it be better just to drop a condition of pd->is_powered_on_standby?
> What do you think?

Both looks fine.

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk