On Fri, Jun 01, 2012 at 12:50:34PM +0200, Ola Lilja wrote:
+/*
- Extended interface for codec-driver
- */
So, a few issues below. Could you please submit a version with this extended API removed or made driver internal as much as possible? The rest of the driver looks good so it'd be good to split this stuff out and review separately.
+int ab8500_audio_init_audioblock(struct snd_soc_codec *codec) +{
- int status;
- dev_dbg(codec->dev, "%s: Enter.\n", __func__);
- /* Reset audio-registers and disable 32kHz-clock output 2 */
- status = ab8500_sysctrl_write(AB8500_STW4500CTRL3,
AB8500_STW4500CTRL3_CLK32KOUT2DIS |AB8500_STW4500CTRL3_RESETAUDN,AB8500_STW4500CTRL3_RESETAUDN);- if (status < 0)
return status;- return 0;
+} +EXPORT_SYMBOL_GPL(ab8500_audio_init_audioblock);
This is really odd (I'm sure I've queried this on previous versions of the driver) - why is something else reinitialising the device? Shouldn't the CODEC driver be doing this by itself - if nothing else I'd expect it to cause confusion if the device is reinitialised underneath the driver?
I didn't register this last time as I'd assumed that the issue was a missing static rather than something else calling the function.
+int ab8500_audio_setup_mics(struct snd_soc_codec *codec,
struct amic_settings *amics)+{
Similar thing here, I'd expect this to just be platform data.
+int ab8500_audio_set_ear_cmv(struct snd_soc_codec *codec,
enum ear_cm_voltage ear_cmv)+{
- char *cmv_str;
- switch (ear_cmv) {
- case EAR_CMV_0_95V:
cmv_str = "0.95V";break;- case EAR_CMV_1_10V:
cmv_str = "1.10V";break;- case EAR_CMV_1_27V:
cmv_str = "1.27V";break;- case EAR_CMV_1_58V:
cmv_str = "1.58V";break;- default:
dev_err(codec->dev,"%s: Unknown earpiece CM-voltage (%d)!\n",__func__, (int)ear_cmv);return -EINVAL;- }
- dev_dbg(codec->dev, "%s: Earpiece CM-voltage: %s\n", __func__,
cmv_str);- snd_soc_update_bits(codec, AB8500_ANACONF1, AB8500_ANACONF1_EARSELCM,
ear_cmv);- return 0;
+} +EXPORT_SYMBOL_GPL(ab8500_audio_set_ear_cmv);
This is *possibly* OK, though again it does look platform dataish.