On Mon, May 18, 2015 at 02:12:54PM +0200, Arnaud Pouliquen wrote:
- if (data->dev_data->chipid == CHIPID_STIH407)
max_rf = STIH407_DAC_MAX_RF;- else
max_rf = STIH416_DAC_MAX_RF;
switch statements for selecting chip versions here and elsewhere please, that way new variants are easier to handle.
- dac->field = devm_kzalloc(data->dev,
sizeof(struct regmap_field *) * max_rf,GFP_KERNEL);
devm_kcalloc().
+static int stih407_sas_dac_supply(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol, int event) {- struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
- struct sti_sas_data *drvdata = dev_get_drvdata(codec->dev);
- struct sti_dac_audio *dac = &drvdata->dac;
- int ret = 0;
- switch (event) {
- case SND_SOC_DAPM_PRE_PMU:
/* Enable analog */ret = regmap_field_write(dac->field[STIH407_DAC_STANDBY_ANA],0);/* Disable standby */if (!ret)ret = regmap_field_write(dac->field[STIH407_DAC_STANDBY], 0);break;
These event functions all look very similar and I can't help but think that they look awfully like the sort of register write sequences that DAPM normally generates anyway. Is there any great reason for not doing these by registering multiple widgets with routes between them rather than with custom code?
+static int sti_sas_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,struct snd_soc_dai *dai)+{
- struct snd_soc_codec *codec = dai->codec;
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- int div;
- div = sti_sas_dai_clk_div[dai->id];
- if (cpu_dai->driver->ops->set_clkdiv)
return cpu_dai->driver->ops->set_clkdiv(cpu_dai,SND_SOC_CLOCK_OUT, div);- dev_warn(codec->dev, "WARN: CPU DAI not support sysclk div");
This is worrying, we shouldn't be peering inside the CPU DAI like this. I'd expect this to either be done autonomously by the CPU DAI or handled in a machine driver.
+config SND_SOC_STI_SAS
- tristate "codec Audio support for STI SAS codec"
- depends on SND_SOC_STI
- help
Say Y if you want to include STI SAS audio codec support
This appears to duplicate the CODEC Kconfig in the CODECs directory - I'd expect it to be there rather than in the STI directory given that that is where the code is.