On Fri, Oct 24, 2014 at 07:14:07PM +0800, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
Add machine driver for two Intel Cherryview-based platforms, Cherrytrail and Braswell, with RT5672 codec.
This doesn't seem to have any ACPI stuff - how does the driver get loaded?
+static int cht_aif1_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *codec_dai = rtd->codec_dai;
- int ret;
- unsigned int fmt;
- if (strncmp(codec_dai->name, "rt5670-aif1", 11))
return 0;- /* TDM 4 slots 24 bit, set Rx & Tx bitmask to 4 active slots */
- ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xF, 0xF, 4,
SNDRV_PCM_FORMAT_GSM);- if (ret < 0) {
dev_err(rtd->dev, "can't set codec TDM slot %d\n", ret);return ret;- }
This doesn't depend on the parameters so should be set on init and your use of SNDRV_PCM_FORMAT_GSM doesn't seem to make much sense here - what's going on?
- /* TDM slave Mode */
- fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF
| SND_SOC_DAIFMT_CBS_CFS;- ret = snd_soc_dai_set_fmt(codec_dai, fmt);
- if (ret < 0) {
dev_err(rtd->dev, "can't set codec DAI fmt %d\n", ret);return ret;- }
Likewise, set on init (in the dai_link)
+static int cht_set_bias_level(struct snd_soc_card *card,
struct snd_soc_dapm_context *dapm,enum snd_soc_bias_level level)+{
- switch (level) {
- case SND_SOC_BIAS_ON:
- case SND_SOC_BIAS_PREPARE:
- case SND_SOC_BIAS_STANDBY:
- case SND_SOC_BIAS_OFF:
break;- default:
dev_err(card->dev, "Invalid bias level=%d\n", level);return -EINVAL;- }
- card->dapm.bias_level = level;
- return 0;
+}
This does nothing and can be removed.
+static int cht_init(struct snd_soc_pcm_runtime *runtime) +{
- int ret;
- struct snd_soc_card *card = runtime->card;
- /* Set card bias level */
- cht_set_bias_level(card, &card->dapm, SND_SOC_BIAS_OFF);
- card->dapm.idle_bias_off = true;
- ret = snd_soc_dapm_sync(&card->dapm);
- if (ret) {
dev_err(card->dev, "unable to sync dapm\n");return ret;- }
- return ret;
+}
This also does nothing and can be removed.
+static struct snd_pcm_hw_constraint_list constraints_48000 = {
- .count = ARRAY_SIZE(rates_48000),
- .list = rates_48000,
+};
+static int cht_aif1_startup(struct snd_pcm_substream *substream) +{
- return snd_pcm_hw_constraint_list(substream->runtime, 0,
SNDRV_PCM_HW_PARAM_RATE,&constraints_48000);+}
If the rates are restricted to 48kHz then is there any need for rate dependent code in hw_params() at all?
+static int snd_cht_mc_remove(struct platform_device *pdev) +{
- struct snd_soc_card *soc_card = platform_get_drvdata(pdev);
- snd_soc_card_set_drvdata(soc_card, NULL);
- snd_soc_unregister_card(soc_card);
- platform_set_drvdata(pdev, NULL);
- return 0;
+}
No need to set the driver data to NULL, the driver core will do it anyway and it's buggy to look at the driver data for unbound devices.