On Sun, Apr 19, 2015 at 02:27:31AM +0530, Vinod Koul wrote:
+static int is_hda_widget_type(struct snd_soc_dapm_widget *w) +{
- return ((w->id == snd_soc_dapm_dai_link) ||
(w->id == snd_soc_dapm_dai_in) ||(w->id == snd_soc_dapm_aif_in) ||(w->id == snd_soc_dapm_aif_out) ||(w->id == snd_soc_dapm_dai_out)) ? 1 : 0;
Please use switch statements, and again no need to convert logic values into logic values.
+static int hda_sst_dapm_pre_pmu_event(struct snd_soc_dapm_widget *w,
- int w_type, struct ssth_lib *ctx, struct hda_platform_info *pinfo)
+{
- int ret = 0;
- struct ssth_module_config *mconfig = w->priv;
- struct ssth_pipe *s_pipe = mconfig->pipe;
- dev_dbg(ctx->dev, "%s: widget =%s type=%d\n", __func__, w->name, w_type);
- /*check resource available */
- if (!hda_sst_is_pipe_mcps_available(pinfo, ctx, mconfig))
return -1;
Proper error code please.
- if (w_type == SSTH_WIDGET_VMIXER ||
w_type == SSTH_WIDGET_MIXER) {
switch.
if (!hda_sst_is_pipe_mem_available(pinfo, ctx, mconfig))return -ENOMEM;ret = ssth_create_pipeline(ctx, mconfig->pipe);if (ret < 0)return ret;if (list_empty(&s_pipe->w_list)) {ret = hda_sst_get_pipe_widget(ctx->dev, w, s_pipe);if (ret < 0)return ret;}ret = hda_init_pipe_modules(ctx, s_pipe, pinfo);if (ret < 0)return ret;
The error handling here appears to be a bit incomplete, we don't unwind anything we did.
- switch (event) {
- case SND_SOC_DAPM_PRE_PMU:
return hda_sst_dapm_pre_pmu_event(w, w_type, ctx, pinfo);- break;
Please follow the kernel coding style.
+static int hda_sst_vmixer_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *k, int event)+{
- struct snd_soc_dapm_context *dapm = w->dapm;
- dev_dbg(dapm->dev, "%s: widget = %s\n", __func__, w->name);
- return hda_sst_event_handler(w, event, SSTH_WIDGET_VMIXER);
+}
+static int hda_sst_mixer_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *k, int event)+{
- struct snd_soc_dapm_context *dapm = w->dapm;
- dev_dbg(dapm->dev, "%s: widget = %s\n", __func__, w->name);
- return hda_sst_event_handler(w, event, SSTH_WIDGET_MIXER);
+}
Lots of indirection and wrapping going on here which seems to make things more confusing. Can you try writing out the event handling directly and having it call common functions to perform the shared operations rather than having a single event handler with per type cases? Or alternatively use the DAPM widget type rather than your own ones.
- /* if FE - Playback, then parse sink list , Capture then source list
- if BE - Playback, then parse source list , Capture then sink list
- */
Coding style on comments too BTW.
- if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
w = dai->playback_widget;(is_fe) ? (dir = 1) : (dir = 0);
This looks like a very complicated way of writing dir = is_fe.