On Tue, Feb 13, 2018 at 04:58:26PM +0000, srinivas.kandagatla@linaro.org wrote:
+static int q6hdmi_format_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct q6afe_dai_data *dai_data = kcontrol->private_data;
- int value = ucontrol->value.integer.value[0];
- dai_data->port_config[AFE_PORT_HDMI_RX].hdmi.datatype = value;
- return 0;
+}
No validation, and do we not need to tell a currently running stream if the format changed on it (or block such changes if they're not going to work, which seems more likely)?
+static int q6hdmi_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,struct snd_soc_dai *dai)+{
- struct q6afe_dai_data *dai_data = q6afe_get_dai_data(dai->dev);
- int channels = params_channels(params);
- struct q6afe_hdmi_cfg *hdmi = &dai_data->port_config[dai->id].hdmi;
- hdmi->sample_rate = params_rate(params);
- switch (params_format(params)) {
- case SNDRV_PCM_FORMAT_S16_LE:
hdmi->bit_width = 16;break;- case SNDRV_PCM_FORMAT_S24_LE:
hdmi->bit_width = 24;break;- }
This silently accepts invalid values.
- /*refer to HDMI spec CEA-861-E: Table 28 Audio InfoFrame Data Byte 4*/
Coding style, spaces around the /* */.
+static int q6afe_dai_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)+{
- struct q6afe_dai_data *dai_data = q6afe_get_dai_data(dai->dev);
- dai_data->is_port_started[dai->id] = false;
- return 0;
+}
If this is needed it makes me a bit worried that we've got some kind of bug with not shutting things down properly somewhere - what's going on here?
+static void q6afe_dai_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)+{
- struct q6afe_dai_data *dai_data = q6afe_get_dai_data(dai->dev);
- int rc;
- rc = q6afe_port_stop(dai_data->port[dai->id]);
- if (rc < 0)
dev_err(dai->dev, "fail to close AFE port\n");
Better to print the error code so users have more information to debug the problem.
.stream_name = "HDMI Playback",.rates = SNDRV_PCM_RATE_48000 |SNDRV_PCM_RATE_96000 |SNDRV_PCM_RATE_192000,
Indentation again.
+static int q6afe_of_xlate_dai_name(struct snd_soc_component *component,
struct of_phandle_args *args,const char **dai_name)+{
- int id = args->args[0];
- int i, ret = -EINVAL;
Coding style, don't mix initialization in with other variable declarations on the same line like this.
+int q6afe_dai_dev_remove(struct device *dev) +{
- return 0;
+}
Remove empty functions, if they can't be removed it's probably not OK for them to be empty either.