[alsa-devel] [PATCH 2/2] ASoC: uda134x: fix bias level setup on initialization and runtime

[Vladimir Zapolskiy]

Hello,

On Thu, Jun 24, 2010 at 3:34 PM, Mark Brown
<broonie at opensource.wolfsonmicro.com> wrote:
> On 24 Jun 2010, at 12:17, Vladimir Zapolskiy wrote:
>
>>               /* ADC, DAC on */
>> -             reg = uda134x_read_reg_cache(codec, UDA134X_STATUS1);
>> -             uda134x_write(codec, UDA134X_STATUS1, reg | 0x03);
>> +             if (pd->model == UDA134X_UDA1341) {
>> +                     reg = uda134x_read_reg_cache(codec, UDA134X_STATUS1);
>> +                     uda134x_write(codec, UDA134X_STATUS1, reg | 0x03);
>> +             } else {
>> +                     reg = uda134x_read_reg_cache(codec, UDA134X_DATA011);
>> +                     uda134x_write(codec, UDA134X_DATA011, reg | 0x03);
>> +             }
>
> I'd be more comfortable if these used switch statements. That way any further
> device specifics will slot in more easily.
>
Agree with the comment.

> Of course, this should really be using DAPM...
>
>>
>> @@ -531,9 +541,7 @@ 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
>> +
>
> These changes for the bias level configuration look to be unrelated to the addition of
> the new CODEC and should be split into a separate patch. It'd also be much better to
> remove this ifdefery, it should be handled via platform data or just done unconditionally.
>
>
I confirm that's actually unrelated, so it should be done in a separate patch.

I  dislike the macro here too, I'll replace it with a platform data solution.

Best wishes,
Vladimir