Hello.
And thank you for all the comments.
Comments to comments:
>> +#undef DEBUG
>
> This shouldn't be here.
Removed.
>> + wl1273->mode = ucontrol->value.integer.value[0];
>
> You're not doing any validation of the supplied value here.
Validation added.
> Don't put your enums in arrays, it just makes things harder to maintain
> since you have to reference an index into an array rather than a named
> value.
Yes using one-element arrays is kind of meaningless, so arrays removed.
> My major comment about this driver is that it'd probably make sense to
> redo it on top of Liam's multi-component branch, though it shouldn't be
> a pressing concern for merge.
I didn't do this yet. But could you give me the git URL and then I'll resend?
> Wouldn't it be nicer to do this stuff with the ALSA constraint APIs
> rather than futzing with the constant data?
I principle I of course agree. But it seems - also discussed with the local
ALSA specialist - that using static constraints does not work here.
HALF_DUPLEX is not one of the SNDRV_PCM_HW_PARAM_'s... or something like that.
As a ALSA non-specialist I'd be willing to this leave as it is...
>> +static int wl1273_set_dai_fmt(struct snd_soc_dai *codec_dai,
>> + unsigned int fmt)
>> +{
>> + return 0;
>> +}
>
>Remove unused functions.
I tried leaving this out but then the codec stopped working. I guess I should
mention that my test environment is 2.6.32. I'm only able to test that
the codec compiles under 2.6.35. Anyway I left the function in.
>> +enum wl1273_mode wl1273_get_codec_mode(struct snd_soc_codec *codec)
>> +{
>> + struct wl1273_priv *wl1273 = snd_soc_codec_get_drvdata(codec);
>> + return wl1273->mode;
>> +}
>> +EXPORT_SYMBOL_GPL(wl1273_get_codec_mode);
>
> Why is this being exported?
This is because the soc_card driver needs to know the codec mode and accessing
the internals of the codec struct is ugly.
>> +static int wl1273_soc_suspend(struct platform_device *pdev, pm_message_t state)
>> +{
>> + return 0;
>> +}
>> +
>> +static int wl1273_soc_resume(struct platform_device *pdev)
>> +{
>> + return 0;
>> +}
>
> Remove unused functions.
Dropping these caused no problems, so removed...
Cheers,
Matti A.
Matti J. Aaltonen (1):
ASoC: TI WL1273 FM Radio Codec.
sound/soc/codecs/wl1273.c | 586 +++++++++++++++++++++++++++++++++++++++++++++
sound/soc/codecs/wl1273.h | 42 ++++
2 files changed, 628 insertions(+), 0 deletions(-)
create mode 100644 sound/soc/codecs/wl1273.c
create mode 100644 sound/soc/codecs/wl1273.h
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
