On 02.05.2018 16:43, Pierre-Louis Bossart wrote:
+static int dmic_trigger(struct dai *dai, int cmd, int direction) +{ + struct dmic_pdata *dmic = dai_get_drvdata(dai);
+ trace_dmic("tri");
+ /* dai private is set in dmic_probe(), error if not set */ + if (!dmic) { + trace_dmic_error("trn"); + return -EINVAL; + }
+ if (direction != DAI_DIR_CAPTURE) { + trace_dmic_error("cap"); + return -EINVAL; + }
+ switch (cmd) { + case COMP_TRIGGER_START: + if (dmic->state == COMP_STATE_PREPARE || + dmic->state == COMP_STATE_PAUSED) { + dmic_start(dai, direction); + } else { + trace_dmic_error("cst"); + trace_value(dmic->state); + } + break; + case COMP_TRIGGER_RELEASE: + if (dmic->state == COMP_STATE_PREPARE) { + dmic_start(dai, direction); + } else { + trace_dmic_error("crl"); + trace_value(dmic->state); + } + break; + case COMP_TRIGGER_STOP: + case COMP_TRIGGER_PAUSE: + dmic->state = COMP_STATE_PAUSED; + dmic_stop(dai);
shouldn't the state assignment be protected by a spinlock (as done for the start case)? Also is this really PAUSED?
I followed in this part the ssp driver. I have no plans to implement anything like RAM buffer for recording pause so I don't know what should be done with it. Could COMP_TRIGGER_PAUSE be removed and return error if attempted?
Liam or Keyon, can you look at this? The spinlock usage is odd and I can't remember why we would use PAUSE even for SSP.
Your comments would be useful. This is an open that I didn't yet address in RFC v2 patch.
Thanks, Seppo