[Sound-open-firmware] [PATCH 1/4] [RFC]DMIC: Add PDM microphones (DMIC) support to DAI

Seppo Ingalsuo seppo.ingalsuo at linux.intel.com
Wed May 2 13:52:37 CEST 2018


Thanks for reading this! I have questions below:


On 30.04.2018 23:22, Pierre-Louis Bossart wrote:
> On 4/30/18 11:04 AM, Seppo Ingalsuo wrote:
>> This patch adds the DMIC audio capture driver for SOF DAI component use.
>> The DMIC feature allows to directly attach one to (typically) four PDM
>> digital microphones into Intel SoC without a separate codec IC. This is
>> supported by APL and most successor platforms.
>>
>> Corresponding patches are needed for kernel driver and topology to 
>> enable
>> this feature.
>>
>> Tested in
>> APL UP squared board without connected microphones
>> SOF git master 3ad69eb715a09de9a0b91c56c9cca8a79ead00a9
>>
>> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo at linux.intel.com>
>> ---
>>   configure.ac            |    6 +
>>   src/audio/dai.c         |    5 +
>>   src/drivers/Makefile.am |    6 +-
>>   src/drivers/dmic.c      | 1331 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>
> this is a big one...
>
>>   src/include/sof/dmic.h  |  318 +++++++++++
>>   src/include/sof/io.h    |    8 +
>>   src/include/sof/trace.h |    3 +-
>>   src/include/uapi/ipc.h  |   61 ++-
>
> and all those files could really be in separate patches since they 
> prepare the work for dmic enablement.

OK, will do!

>
>>   8 files changed, 1734 insertions(+), 4 deletions(-)
>>   create mode 100644 src/drivers/dmic.c
>>   create mode 100644 src/include/sof/dmic.h
>>
>
> [...]
>
>> +/* This function returns a raw list of potential microphone clock 
>> and decimation
>> + * modes for achieving requested sample rates. The search is 
>> constrained by
>> + * decimation HW capabililies and setup parameters. The parameters 
>> such as
>> + * microphone clock min/max and duty cycle requirements need be 
>> checked from
>> + * used microphone component datasheet.
>> + */
>> +static void find_modes(struct decim_modes *modes,
>> +    struct sof_ipc_dai_dmic_params *prm, uint32_t fs)
>> +{
>> +    int clkdiv_min;
>> +    int clkdiv_max;
>> +    int clkdiv;
>> +    int c1;
>> +    int c2;
>> +    int du_min;
>> +    int du_max;
>> +    int pdmclk;
>> +    int osr;
>> +    int mfir;
>> +    int mcic;
>> +    int ioclk_test;
>> +    int osr_min = DMIC_MIN_OSR;
>> +    int i = 0;
>> +
>> +    /* Defaults, empty result */
>> +    modes->num_of_modes = 0;
>> +
>> +    /* The FIFO is not requested if sample rate is set to zero. Just
>> +     * return in such case with num_of_modes as zero.
>> +     */
>> +    if (fs == 0)
>> +        return;
>> +
>> +    /* Override DMIC_MIN_OSR for very high sample rates, use as minimum
>> +     * the nominal clock for the high rates.
>> +     */
>> +    if (fs >= DMIC_HIGH_RATE_MIN_FS)
>> +        osr_min = DMIC_HIGH_RATE_OSR_MIN;
>> +
>> +    /* Check for sane pdm clock, min 100 kHz, max ioclk/2 */
>> +    if (prm->pdmclk_max < DMIC_HW_PDM_CLK_MIN ||
>> +        prm->pdmclk_max > DMIC_HW_IOCLK / 2) {
>> +        trace_dmic_error("pmx");
>> +        return;
>> +    }
>> +    if (prm->pdmclk_min < DMIC_HW_PDM_CLK_MIN ||
>> +        prm->pdmclk_min > prm->pdmclk_max) {
>> +        trace_dmic_error("pmn");
>> +        return;
>> +    }
>> +
>> +    /* Check for sane duty cycle */
>> +    if (prm->duty_min > prm->duty_max) {
>> +        trace_dmic_error("pdu");
>> +        return;
>> +    }
>> +    if (prm->duty_min < DMIC_HW_DUTY_MIN ||
>> +        prm->duty_min > DMIC_HW_DUTY_MAX) {
>> +        trace_dmic_error("pdn");
>> +        return;
>> +    }
>> +    if (prm->duty_max < DMIC_HW_DUTY_MIN ||
>> +        prm->duty_max > DMIC_HW_DUTY_MAX) {
>> +        trace_dmic_error("pdx");
>> +        return;
>> +    }
>> +
>> +    /* Min and max clock dividers */
>> +    clkdiv_min = ceil_divide(DMIC_HW_IOCLK, prm->pdmclk_max);
>> +    clkdiv_min = MAX(clkdiv_min, DMIC_HW_CIC_DECIM_MIN);
>> +    clkdiv_max = DMIC_HW_IOCLK / prm->pdmclk_min;
>> +
>> +    /* Loop possible clock dividers and check based on resulting
>> +     * oversampling ratio that CIC and FIR decimation ratios are
>> +     * feasible. The ratios need to be integers. Also the mic clock
>> +     * duty cycle need to be within limits.
>> +     */
>> +    for (clkdiv = clkdiv_min; clkdiv <= clkdiv_max; clkdiv++) {
>> +        c1 = clkdiv >> 1;
>> +        c2 = clkdiv - c1;
>> +        du_min = 100 * c1 / clkdiv;
>
> du_min = 100 * (clkdiv/2)/clkdiv  -> 50?
>
>> +        du_max = 100 * c2 / clkdiv;
>
> c2 = clkdiv - c1 = clkdiv/2, so du_min == du_max ?
> what am I missing?

The duty cycle becomes non-50% when the divider is odd, e.g. clkdiv = 5 
case:

c1 = clkdiv >> 1 = 2
c2 = clkdiv - c1 = 5 - 2 = 3
du_min = 100*c1/clkdiv = 100*2/5 = 200/5 = 40
du_max = 100*c2/clkdif = 100*3/5 = 300/5 = 60

The latter equation can be though simplified to "c2 = 100 - c1;" that I 
didn't realize earlier.

>
>> +        pdmclk = DMIC_HW_IOCLK / clkdiv;
>> +        osr = pdmclk / fs;
>> +
>> +        /* Check that OSR constraints is met and clock duty cycle does
>> +         * not exceed microphone specification. If exceed proceed to
>> +         * next clkdiv.
>> +         */
>> +        if (osr < osr_min || du_min < prm->duty_min ||
>> +            du_max > prm->duty_max)
>> +            continue;
>> +
>> +        /* Loop FIR decimation factors candidates. If the
>> +         * integer divided decimation factors and clock dividers
>> +         * as multiplied with sample rate match the IO clock
>> +         * rate the division was exact and such decimation mode
>> +         * is possible. Then check that CIC decimation constraints
>> +         * are met. The passed decimation modes are added to array.
>> +         */
>> +        for (mfir = DMIC_HW_FIR_DECIM_MIN;
>> +            mfir <= DMIC_HW_FIR_DECIM_MAX; mfir++) {
>> +            mcic = osr / mfir;
>> +            ioclk_test = fs * mfir * mcic * clkdiv;
>> +
>> +            if (ioclk_test == DMIC_HW_IOCLK &&
>> +                mcic >= DMIC_HW_CIC_DECIM_MIN &&
>> +                mcic <= DMIC_HW_CIC_DECIM_MAX &&
>> +                i < DMIC_MAX_MODES) {
>> +                modes->clkdiv[i] = clkdiv;
>> +                modes->mcic[i] = mcic;
>> +                modes->mfir[i] = mfir;
>> +                i++;
>> +                modes->num_of_modes = i;
>> +            }
>> +        }
>> +    }
>> +}
>> +
>> +/* The previous raw modes list contains sane configuration 
>> possibilities. When
>> + * there is request for both FIFOs A and B operation this function 
>> returns
>> + * list of compatible settings.
>> + */
>> +static void match_modes(struct matched_modes *c, struct decim_modes *a,
>> +    struct decim_modes *b)
>> +{
>> +    int idx[DMIC_MAX_MODES];
>> +    int idx_length;
>> +    int i;
>> +    int n;
>> +    int m;
>> +
>> +    /* Check if previous search got results. */
>> +    c->num_of_modes = 0;
>> +    if (a->num_of_modes == 0 && b->num_of_modes == 0) {
>> +        /* Nothing to do */
>> +        return;
>> +    }
>> +
>> +    /* Check for request only for FIFO A or B. In such case pass 
>> list for
>> +     * A or B as such.
>> +     */
>> +    if (b->num_of_modes == 0) {
>> +        c->num_of_modes = a->num_of_modes;
>> +        for (i = 0; i < a->num_of_modes; i++) {
>> +            c->clkdiv[i] = a->clkdiv[i];
>> +            c->mcic[i] = a->mcic[i];
>> +            c->mfir_a[i] = a->mfir[i];
>> +            c->mfir_b[i] = 0;
>
> is this initialization to zero needed?

Yes, It indicates the FIFO B path won't be used.

>
>> +        }
>> +        return;
>> +    }
>> +
>> +    if (a->num_of_modes == 0) {
>> +        c->num_of_modes = b->num_of_modes;
>> +        for (i = 0; i < b->num_of_modes; i++) {
>> +            c->clkdiv[i] = b->clkdiv[i];
>> +            c->mcic[i] = b->mcic[i];
>> +            c->mfir_b[i] = b->mfir[i];
>> +            c->mfir_a[i] = 0;
>
> same here, is this initialization needed?

And vice versa no FIFO A path used.

>
>> +        }
>> +        return;
>> +    }
>> +
>> +    /* Merge a list of compatible modes */
>> +    i = 0;
>> +    for (n = 0; n < a->num_of_modes; n++) {
>> +        /* Find all indices of values a->clkdiv[n] in b->clkdiv[] */
>> +        idx_length = find_equal(idx, b->clkdiv, a->clkdiv[n],
>> +            b->num_of_modes, 0);
>> +        for (m = 0; m < idx_length; m++) {
>> +            if (b->mcic[idx[m]] == a->mcic[n]) {
>> +                c->clkdiv[i] = a->clkdiv[n];
>> +                c->mcic[i] = a->mcic[n];
>> +                c->mfir_a[i] = a->mfir[n];
>> +                c->mfir_b[i] = b->mfir[idx[m]];
>> +                i++;
>> +            }
>> +        }
>> +        c->num_of_modes = i;
>> +    }
>> +}
>> +
>> +/* Finds a suitable FIR decimation filter from the included set */
>> +static struct pdm_decim *get_fir(struct dmic_configuration *cfg, int 
>> mfir)
>> +{
>> +    int i;
>> +    int fs;
>> +    int cic_fs;
>> +    int fir_max_length;
>> +    struct pdm_decim *fir = NULL;
>> +
>> +    if (mfir <= 0)
>> +        return fir;
>> +
>> +    cic_fs = DMIC_HW_IOCLK / cfg->clkdiv / cfg->mcic;
>> +    fs = cic_fs / mfir;
>> +    /* FIR max. length depends on available cycles and coef RAM
>> +     * length. Exceeding this length sets HW overrun status and
>> +     * overwrite of other register.
>> +     */
>> +    fir_max_length = MIN(DMIC_HW_FIR_LENGTH_MAX,
>> +        DMIC_HW_IOCLK / fs / 2 - DMIC_FIR_PIPELINE_OVERHEAD);
>> +
>> +    for (i = 0; i < DMIC_FIR_LIST_LENGTH; i++) {
>> +        if (fir_list[i]->decim_factor == mfir &&
>> +            fir_list[i]->length <= fir_max_length) {
>> +            /* Store pointer, break from loop to avoid a
>> +             * Possible other mode with lower FIR length.
>> +             */
>> +            fir = fir_list[i];
>> +            break;
>> +        }
>> +    }
>> +
>> +    return fir;
>> +}
>> +
>> +/* Calculate scale and shift to use for FIR coefficients. Scale is 
>> applied
>> + * before write to HW coef RAM. Shift will be programmed to HW 
>> register.
>> + */
>> +static int fir_coef_scale(int32_t *fir_scale, int *fir_shift, int 
>> add_shift,
>> +    const int32_t coef[], int coef_length, int32_t gain)
>> +{
>> +    int32_t amax;
>> +    int32_t new_amax;
>> +    int32_t new_scale;
>> +    int32_t fir_gain;
>> +    int shift;
>> +    const int32_t coef_max_val = Q_CONVERT_FLOAT(0.9999, 20); /* 
>> Q1.20 */
>> +
>> +    /* Multiply gain passed from CIC with output full scale, result 
>> Q4.20 */
>> +    fir_gain = Q_MULTSR_32X32((int64_t)gain, DMIC_HW_SENS_Q23, 20, 
>> 23, 20);
>> +
>> +    /* Find the largest FIR coefficient value */
>> +    amax = find_max_abs_int32((int32_t *)coef, coef_length);
>> +
>> +    /* Scale with FIR gain */
>> +    new_amax = Q_MULTSR_32X32((int64_t)amax, fir_gain, 31, 20, 20);
>> +    if (new_amax <= 0)
>> +        return -EINVAL;
>> +
>> +    /* Needed scale for FIR taps, target is 0.9999 max */
>> +    new_scale = (int32_t)((((int64_t)coef_max_val << 20)) / new_amax);
>> +
>> +    /* Find shift value */
>> +    if (new_scale == 0)
>> +        return -EINVAL;
>> +
>> +    shift = 0;
>> +    while ((new_scale << shift) < (1 << 20))
>> +        shift++;
>
> don't we have some sort of macro for this (find number of leading 
> zeroes)?

Not yet I think but such would be useful. I'll add it to math/numbers. 
Since DSPs have an instruction for that it would be then easier to 
optimize into single location.

>
>> +
>> +    /* Add to shift in storate Q31 format and store to configuration */
>> +    *fir_shift = shift + add_shift;
>> +
>> +    /* Compensate shift value to scale and store to configuration
>> +     * as Q1.31. Also apply raw 32 bit to actual HW FIR coef
>> +     * precision.
>> +     */
>> +    *fir_scale = (fir_gain >> shift);
>> +    return 0;
>> +}
>> +
>> +/* This function selects with a simple criteria one mode to set up the
>> + * decimator. For the settings chosen for FIFOs A and B output a lookup
>> + * is done for FIR coefficients from the included coefficients tables.
>> + * For some decimation factors there may be several length 
>> coefficient sets.
>> + * It is due to possible restruction of decimation engine cycles per 
>> given
>> + * sample rate. If the coefficients length is exceeded the lookup 
>> continues.
>> + * Therefore the list of coefficient set must present the filters for a
>> + * decimation factor in decreasing length order.
>> + *
>> + * Note: If there is no filter available an error is returned. The 
>> parameters
>> + * should be reviewed for such case. If still a filter is missing it 
>> should be
>> + * added into the included set. FIR decimation with a high factor 
>> usually
>> + * needs compromizes into specifications and is not desirable.
>> + */
>> +static int select_mode(struct dmic_configuration *cfg,
>> +    struct matched_modes *modes)
>> +{
>> +    int32_t g_cic;
>> +    int32_t g_tmp;
>> +    int32_t fir_in_max;
>> +    int32_t cic_out_max;
>> +    int32_t gain_to_fir;
>> +    int idx[DMIC_MAX_MODES];
>> +    int n = 1;
>> +    int mmin;
>> +    int count;
>> +    int *mfir;
>> +    int mcic;
>> +    int bits_cic;
>> +    int ret;
>> +
>> +    /* If there are more than one possibilities select a mode with 
>> lowest
>> +     * FIR decimation factor. If there are several select mode with 
>> highest
>> +     * ioclk divider to minimize microphone power consumption. The 
>> highest
>> +     * clock divisors are in the end of list so select the last of 
>> list.
>> +     * The minimum OSR criteria used in previous ensures that 
>> quality in
>> +     * the candidates should be sufficient.
>> +     */
>> +    if (modes->num_of_modes == 0) {
>> +        trace_dmic_error("nom");
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (modes->mfir_a[0] > 0)
>> +        mfir = modes->mfir_a;
>> +    else
>> +        mfir = modes->mfir_b;
>
> does look like the initialization done above has a side effect here...
> worth a comment maybe?

Yes, it's checked for here. Will add.

>> +
>> +    mmin = find_min(mfir, modes->num_of_modes);
>> +    count = find_equal(idx, mfir, mmin, modes->num_of_modes, 0);
>> +    n = idx[count - 1];
>> +
>> +    /* Get microphone clock and decimation parameters for used mode 
>> from
>> +     * the list.
>> +     */
>> +    cfg->clkdiv = modes->clkdiv[n];
>> +    cfg->mfir_a = modes->mfir_a[n];
>> +    cfg->mfir_b = modes->mfir_b[n];
>> +    cfg->mcic = modes->mcic[n];
>> +    cfg->fir_a = NULL;
>> +    cfg->fir_b = NULL;
>> +
>> +    /* Find raw FIR coefficients to match the decimation foctors of FIR
>
> factors
>
>> +     * A and B.
>> +     */
>> +    if (cfg->mfir_a > 0) {
>> +        cfg->fir_a = get_fir(cfg, cfg->mfir_a);
>> +        if (!cfg->fir_a) {
>> +            trace_dmic_error("fam");
>> +            trace_value(cfg->mfir_a);
>> +            return -EINVAL;
>> +        }
>> +    }
>> +
>> +    if (cfg->mfir_b > 0) {
>> +        cfg->fir_b = get_fir(cfg, cfg->mfir_b);
>> +        if (!cfg->fir_b) {
>> +            trace_dmic_error("fbm");
>> +            trace_value(cfg->mfir_b);
>> +            return -EINVAL;
>> +        }
>> +    }
>> +
>> +    /* Calculate CIC shift from the decimation factor specific gain.
>> +     */
>> +    mcic = cfg->mcic;
>> +    g_cic = mcic * mcic * mcic * mcic * mcic;
>
> yikes, what is this mcic^5 ?

Yep, it's the gain for +1 / -1 PDM bits in CIC decimator part. It's 
needed to set the scaling HW shifter. It also gives some idea of 
oversampling ratios to use for a microphone to get some # of bits of 
resolution as PCM code.

>
>
>> +    g_tmp = g_cic;
>> +    bits_cic = 1;
>> +    if (g_tmp < 0) {
>> +        /* Erroneus decimation factor and CIC gain */
>
> erroneous?
>
>> +        trace_dmic_error("gci");
>> +        return -EINVAL;
>> +    }
>> +    while (g_tmp > 0) {
>> +        g_tmp = g_tmp >> 1;
>> +        bits_cic++;
>> +    }
>
> this again looks like finding the first non-zero bit

yep, to be replaced with function call

>
>> +    cfg->cic_shift = bits_cic - DMIC_HW_BITS_FIR_INPUT;
>> +
>> +    /* Calculate remaining gain to FIR as Q4.20. */
>> +    fir_in_max = (1 << (DMIC_HW_BITS_FIR_INPUT - 1));
>> +    if (cfg->cic_shift >= 0)
>> +        cic_out_max = g_cic >> cfg->cic_shift;
>> +    else
>> +        cic_out_max = g_cic << -cfg->cic_shift;
>> +
>> +    gain_to_fir = (int32_t)((((int64_t) fir_in_max) << 20) / 
>> cic_out_max);
>> +
>> +    /* Calculate FIR scale and shift */
>> +    if (cfg->mfir_a > 0) {
>> +        cfg->fir_a_length = cfg->fir_a->length;
>> +        ret = fir_coef_scale(&cfg->fir_a_scale, &cfg->fir_a_shift,
>> +            cfg->fir_a->shift, cfg->fir_a->coef, cfg->fir_a->length,
>> +            gain_to_fir);
>> +        if (ret < 0) {
>> +            /* Invalid coefficient set found, should not happen. */
>> +            trace_dmic_error("ina");
>> +            return -EINVAL;
>> +        }
>> +    } else {
>> +        cfg->fir_a_scale = 0;
>> +        cfg->fir_a_shift = 0;
>> +        cfg->fir_a_length = 0;
>> +    }
>> +
>> +    if (cfg->mfir_b > 0) {
>> +        cfg->fir_b_length = cfg->fir_b->length;
>> +        ret = fir_coef_scale(&cfg->fir_b_scale, &cfg->fir_b_shift,
>> +            cfg->fir_b->shift, cfg->fir_b->coef, cfg->fir_b->length,
>> +            gain_to_fir);
>> +        if (ret < 0) {
>> +            /* Invalid coefficient set found, should not happen. */
>> +            trace_dmic_error("inb");
>> +            return -EINVAL;
>> +        }
>> +    } else {
>> +        cfg->fir_b_scale = 0;
>> +        cfg->fir_b_shift = 0;
>> +        cfg->fir_b_length = 0;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +/* The FIFO input packer mode (IPM) settings are somewhat different in
>> + * HW versions. This helper function returns a suitable IPM bit field
>> + * value to use.
>> + */
>> +#if DMIC_HW_VERSION == 1
>> +
>> +static inline void ipm_helper(int *ipm, int stereo[], int swap[],
>> +    struct sof_ipc_dai_dmic_params *dmic)
>> +{
>> +    int pdm[DMIC_HW_CONTROLLERS];
>> +    int i;
>> +
>> +    /* Loop number of PDM controllers in the configuration. If mic A
>> +     * or B is enabled then a pdm controller is marked as active. 
>> Also it
>> +     * is checked whether the controller should operate as stereo or 
>> mono
>> +     * left (A) or mono right (B) mode. Mono right mode is setup as 
>> channel
>> +     * swapped mono left.
>> +     */
>> +    for (i = 0; i < dmic->number_of_pdm_controllers; i++) {
>> +        if (dmic->pdm[i].enable_mic_a > 0 ||
>> +            dmic->pdm[i].enable_mic_b > 0)
>> +            pdm[i] = 1;
>> +        else
>> +            pdm[i] = 0;
>> +
>> +        if (dmic->pdm[i].enable_mic_a > 0 &&
>> +            dmic->pdm[i].enable_mic_b > 0) {
>> +            stereo[i] = 1;
>> +            swap[i] = 0;
>> +        } else {
>> +            stereo[i] = 0;
>> +            if (dmic->pdm[i].enable_mic_a == 0)
>> +                swap[i] = 1;
>> +            else
>> +                swap[i] = 0;
>> +        }
>
> the logic is weird here.
>
> if (a || b)
>     ...
> else
>     ...
>
> if (a && b)
>     ...
> else
>     // this could be a || b or !a && !b??

I'm not sure this if spaghetti can be simplified. If the PDM bits 
processing is not stereo the mono-left mic (a) HW mode is used. If the 
the right mic (b) needs to be used as mono input then need to swap the 
PDM bit inputs to process it as mono-left.

The FIFO packer packer has a mono mode so the use or left channel 
processing for right microphone is only a HW internal thing.

>
>> +    }
>> +
>> +    /* IPM indicates active pdm controllers. */
>> +    *ipm = 0;
>> +
>> +    if (pdm[0] == 0 && pdm[1] > 0)
>> +        *ipm = 1;
>> +
>> +    if (pdm[0] > 0 && pdm[1] > 0)
>> +        *ipm = 2;
>> +}
>> +#endif
>> +
>> +#if DMIC_HW_VERSION == 2
>> +
>> +static inline void source_ipm_helper(int source[], int *ipm, int 
>> stereo[],
>> +    int swap[], struct sof_ipc_dai_dmic_params *dmic)
>> +{
>> +    int pdm[DMIC_HW_CONTROLLERS];
>> +    int i;
>> +    int n = 0;
>> +
>> +    /* Loop number of PDM controllers in the configuration. If mic A
>> +     * or B is enabled then a pdm controller is marked as active. 
>> Also it
>> +     * is checked whether the controller should operate as stereo or 
>> mono
>> +     * left (A) or mono right (B) mode. Mono right mode is setup as 
>> channel
>> +     * swapped mono left. The function returns also in array 
>> source[] the
>> +     * indice of enabled pdm controllers to be used for IPM 
>> configuration.
>> +     */
>> +    for (i = 0; i < dmic->number_of_pdm_controllers; i++) {
>> +        if (dmic->pdm[i].enable_mic_a > 0 ||
>> +            dmic->pdm[i].enable_mic_b > 0) {
>> +            pdm[i] = 1;
>> +            source[n] = i;
>> +            n++;
>> +        } else {
>> +            pdm[i] = 0;
>> +            swap[i] = 0;
>> +        }
>> +
>> +        if (dmic->pdm[i].enable_mic_a > 0 &&
>> +            dmic->pdm[i].enable_mic_b > 0) {
>> +            stereo[i] = 1;
>> +            swap[i] = 0;
>> +        } else {
>> +            stereo[i] = 0;
>> +            if (dmic->pdm[i].enable_mic_a == 0)
>> +                swap[i] = 1;
>> +            else
>> +                swap[i] = 0;
>> +        }
>> +    }
>> +
>> +    /* IPM bit field is set to count of active pdm controllers. */
>> +    *ipm = pdm[0];
>> +    for (i = 0; i < dmic->number_of_pdm_controllers; i++)
>> +        *ipm += pdm[i];
>> +}
>> +#endif
>> +
>> +static int configure_registers(struct dai *dai, struct 
>> dmic_configuration *cfg,
>> +    struct sof_ipc_dai_dmic_params *dmic)
>> +{
>> +    int stereo[DMIC_HW_CONTROLLERS];
>> +    int swap[DMIC_HW_CONTROLLERS];
>> +    uint32_t val;
>> +    int32_t ci;
>> +    uint32_t cu;
>> +    int ipm;
>> +    int of0;
>> +    int of1;
>> +    int fir_decim;
>> +    int fir_length;
>> +    int length;
>> +    int edge;
>> +    int dccomp;
>> +    int cic_start_a;
>> +    int cic_start_b;
>> +    int fir_start_a;
>> +    int fir_start_b;
>> +    int soft_reset;
>> +    int i;
>> +    int j;
>> +
>> +    struct dmic_pdata *pdata = dai_get_drvdata(dai);
>> +    int array_a = 0;
>> +    int array_b = 0;
>> +    int cic_mute = 0;
>> +    int fir_mute = 0;
>> +    int bfth = 1; /* Should be 3 for 8 entries, 1 is 2 entries */
>> +    int th = 0; /* Used with TIE=1 */
>> +
>> +    /* Normal start sequence */
>> +    dccomp = 1;
>> +    soft_reset = 1;
>> +    cic_start_a = 0;
>> +    cic_start_b = 0;
>> +    fir_start_a = 0;
>> +    fir_start_b = 0;
>> +
>> +#if DMIC_HW_VERSION == 2
>> +    int source[4] = {0, 0, 0, 0};
>> +#endif
>> +
>> +    /* pdata is set by dmic_probe(), error if it has not been set */
>> +    if (!pdata) {
>> +        trace_dmic_error("cfr");
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Sanity checks */
>> +    if (dmic->number_of_pdm_controllers > DMIC_HW_CONTROLLERS) {
>> +        trace_dmic_error("num");
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* OUTCONTROL0 and OUTCONTROL1 */
>> +    trace_dmic("reg");
>> +    of0 = (dmic->fifo_bits_a == 32) ? 2 : 0;
>> +    of1 = (dmic->fifo_bits_b == 32) ? 2 : 0;
>> +
>> +#if DMIC_HW_VERSION == 1
>> +    ipm_helper(&ipm, stereo, swap, dmic);
>> +    val = OUTCONTROL0_TIE(0) |
>> +        OUTCONTROL0_SIP(0) |
>> +        OUTCONTROL0_FINIT(1) |
>> +        OUTCONTROL0_FCI(0) |
>> +        OUTCONTROL0_BFTH(bfth) |
>> +        OUTCONTROL0_OF(of0) |
>> +        OUTCONTROL0_IPM(ipm) |
>> +        OUTCONTROL0_TH(th);
>> +    dmic_write(dai, OUTCONTROL0, val);
>> +    trace_value(val);
>> +
>> +    val = OUTCONTROL1_TIE(0) |
>> +        OUTCONTROL1_SIP(0) |
>> +        OUTCONTROL1_FINIT(1) |
>> +        OUTCONTROL1_FCI(0) |
>> +        OUTCONTROL1_BFTH(bfth) |
>> +        OUTCONTROL1_OF(of1) |
>> +        OUTCONTROL1_IPM(ipm) |
>> +        OUTCONTROL1_TH(th);
>> +    dmic_write(dai, OUTCONTROL1, val);
>> +    trace_value(val);
>> +#endif
>> +
>> +#if DMIC_HW_VERSION == 2
>> +    source_ipm_helper(source, &ipm, stereo, swap, dmic);
>> +    val = OUTCONTROL0_TIE(0) |
>> +        OUTCONTROL0_SIP(0) |
>> +        OUTCONTROL0_FINIT(1) |
>> +        OUTCONTROL0_FCI(0) |
>> +        OUTCONTROL0_BFTH(3) |
>> +        OUTCONTROL0_OF(of0) |
>> +        OUTCONTROL0_NUMBER_OF_DECIMATORS(ipm) |
>> +        OUTCONTROL0_IPM_SOURCE_1(source[0]) |
>> +        OUTCONTROL0_IPM_SOURCE_2(source[1]) |
>> +        OUTCONTROL0_IPM_SOURCE_3(source[2]) |
>> +        OUTCONTROL0_IPM_SOURCE_4(source[3]) |
>> +        OUTCONTROL0_TH(3);
>> +    dmic_write(dai, OUTCONTROL0, val);
>> +    trace_value(val);
>> +
>> +    val = OUTCONTROL1_TIE(0) |
>> +        OUTCONTROL1_SIP(0) |
>> +        OUTCONTROL1_FINIT(1) |
>> +        OUTCONTROL1_FCI(0) |
>> +        OUTCONTROL1_BFTH(3) |
>> +        OUTCONTROL1_OF(of1) |
>> +        OUTCONTROL1_NUMBER_OF_DECIMATORS(ipm) |
>> +        OUTCONTROL1_IPM_SOURCE_1(source[0]) |
>> +        OUTCONTROL1_IPM_SOURCE_2(source[1]) |
>> +        OUTCONTROL1_IPM_SOURCE_3(source[2]) |
>> +        OUTCONTROL1_IPM_SOURCE_4(source[3]) |
>> +        OUTCONTROL1_TH(3);
>> +    dmic_write(dai, OUTCONTROL1, val);
>> +    trace_value(val);
>> +#endif
>> +
>> +    /* Mark enabled microphones into private data to be later used
>> +     * for starting correct parts of the HW.
>> +     */
>> +    for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
>> +        pdata->enable[i] = (dmic->pdm[i].enable_mic_b << 1) |
>> +            dmic->pdm[i].enable_mic_a;
>> +    }
>> +
>> +    for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
>> +        /* CIC */
>> +        val = CIC_CONTROL_SOFT_RESET(soft_reset) |
>> +            CIC_CONTROL_CIC_START_B(cic_start_b) |
>> +            CIC_CONTROL_CIC_START_A(cic_start_a) |
>> + CIC_CONTROL_MIC_B_POLARITY(dmic->pdm[i].polarity_mic_a) |
>> + CIC_CONTROL_MIC_A_POLARITY(dmic->pdm[i].polarity_mic_b) |
>> +            CIC_CONTROL_MIC_MUTE(cic_mute) |
>> +            CIC_CONTROL_STEREO_MODE(stereo[i]);
>> +        dmic_write(dai, base[i] + CIC_CONTROL, val);
>> +        trace_value(val);
>> +
>> +        val = CIC_CONFIG_CIC_SHIFT(cfg->cic_shift + 8) |
>> +            CIC_CONFIG_COMB_COUNT(cfg->mcic - 1);
>> +        dmic_write(dai, base[i] + CIC_CONFIG, val);
>> +        trace_value(val);
>> +
>> +        /* Mono right channel mic usage requires swap of PDM channels
>> +         * since the mono decimation is done with only left channel
>> +         * processing active.
>> +         */
>> +        edge = dmic->pdm[i].clk_edge;
>> +        if (swap[i])
>> +            edge = edge ^ 1;
>
> ! edge?

If C standard has it defined that !0 gives 1 and !1 gives 0 it's OK to 
change. I used bitwise XOR to make it sure.

>
>> +
>> +        val = MIC_CONTROL_PDM_CLKDIV(cfg->clkdiv - 2) |
>> +            MIC_CONTROL_PDM_SKEW(dmic->pdm[i].skew) |
>> +            MIC_CONTROL_CLK_EDGE(edge) |
>> +            MIC_CONTROL_PDM_EN_B(cic_start_b) |
>> +            MIC_CONTROL_PDM_EN_A(cic_start_a);
>> +        dmic_write(dai, base[i] + MIC_CONTROL, val);
>> +        trace_value(val);
>> +
>> +        /* FIR A */
>> +        fir_decim = MAX(cfg->mfir_a - 1, 0);
>> +        fir_length = MAX(cfg->fir_a_length - 1, 0);
>> +        val = FIR_CONTROL_A_START(fir_start_a) |
>> +            FIR_CONTROL_A_ARRAY_START_EN(array_a) |
>> +            FIR_CONTROL_A_DCCOMP(dccomp) |
>> +            FIR_CONTROL_A_MUTE(fir_mute) |
>> +            FIR_CONTROL_A_STEREO(stereo[i]);
>> +        dmic_write(dai, base[i] + FIR_CONTROL_A, val);
>> +        trace_value(val);
>> +
>> +        val = FIR_CONFIG_A_FIR_DECIMATION(fir_decim) |
>> +            FIR_CONFIG_A_FIR_SHIFT(cfg->fir_a_shift) |
>> +            FIR_CONFIG_A_FIR_LENGTH(fir_length);
>> +        dmic_write(dai, base[i] + FIR_CONFIG_A, val);
>> +        trace_value(val);
>> +
>> +        val = DC_OFFSET_LEFT_A_DC_OFFS(DCCOMP_TC0);
>> +        dmic_write(dai, base[i] + DC_OFFSET_LEFT_A, val);
>> +        trace_value(val);
>> +
>> +        val = DC_OFFSET_RIGHT_A_DC_OFFS(DCCOMP_TC0);
>> +        dmic_write(dai, base[i] + DC_OFFSET_RIGHT_A, val);
>> +        trace_value(val);
>> +
>> +        val = OUT_GAIN_LEFT_A_GAIN(0);
>> +        dmic_write(dai, base[i] + OUT_GAIN_LEFT_A, val);
>> +        trace_value(val);
>> +
>> +        val = OUT_GAIN_RIGHT_A_GAIN(0);
>> +        dmic_write(dai, base[i] + OUT_GAIN_RIGHT_A, val);
>> +        trace_value(val);
>> +
>> +        /* FIR B */
>> +        fir_decim = MAX(cfg->mfir_b - 1, 0);
>> +        fir_length = MAX(cfg->fir_b_length - 1, 0);
>> +        val = FIR_CONTROL_B_START(fir_start_b) |
>> +            FIR_CONTROL_B_ARRAY_START_EN(array_b) |
>> +            FIR_CONTROL_B_DCCOMP(dccomp) |
>> +            FIR_CONTROL_B_MUTE(fir_mute) |
>> +            FIR_CONTROL_B_STEREO(stereo[i]);
>> +        dmic_write(dai, base[i] + FIR_CONTROL_B, val);
>> +        trace_value(val);
>> +
>> +        val = FIR_CONFIG_B_FIR_DECIMATION(fir_decim) |
>> +            FIR_CONFIG_B_FIR_SHIFT(cfg->fir_b_shift) |
>> +            FIR_CONFIG_B_FIR_LENGTH(fir_length);
>> +        dmic_write(dai, base[i] + FIR_CONFIG_B, val);
>> +        trace_value(val);
>> +
>> +        val = DC_OFFSET_LEFT_B_DC_OFFS(DCCOMP_TC0);
>> +        dmic_write(dai, base[i] + DC_OFFSET_LEFT_B, val);
>> +        trace_value(val);
>> +        trace_value(val);
>> +
>> +        val = DC_OFFSET_RIGHT_B_DC_OFFS(DCCOMP_TC0);
>> +        dmic_write(dai, base[i] + DC_OFFSET_RIGHT_B, val);
>> +        trace_value(val);
>> +
>> +        val = OUT_GAIN_LEFT_B_GAIN(0);
>> +        dmic_write(dai, base[i] + OUT_GAIN_LEFT_B, val);
>> +        trace_value(val);
>> +
>> +        val = OUT_GAIN_RIGHT_B_GAIN(0);
>> +        dmic_write(dai, base[i] + OUT_GAIN_RIGHT_B, val);
>> +        trace_value(val);
>> +
>> +        /* Write coef RAM A with scaled coefficient in reverse order */
>> +        length = cfg->fir_a_length;
>> +        for (j = 0; j < length; j++) {
>> +            ci = (int32_t)Q_MULTSR_32X32(
>> +                (int64_t)cfg->fir_a->coef[j],
>> +                cfg->fir_a_scale, 31, 20, DMIC_HW_FIR_COEF_Q);
>> +            cu = FIR_COEF_A(ci);
>> +            dmic_write(dai, coef_base_a[i]
>> +                + ((length - j - 1) << 2), cu);
>> +        }
>> +
>> +        /* Write coef RAM B with scaled coefficient in reverse order */
>> +        length = cfg->fir_b_length;
>> +        for (j = 0; j < length; j++) {
>> +            ci = (int32_t)Q_MULTSR_32X32(
>> +                (int64_t)cfg->fir_b->coef[j],
>> +                cfg->fir_b_scale, 31, 20, DMIC_HW_FIR_COEF_Q);
>> +            cu = FIR_COEF_B(ci);
>> +            dmic_write(dai, coef_base_b[i]
>> +                + ((length - j - 1) << 2), cu);
>> +        }
>> +    }
>> +
>> +    /* Function dmic_start() uses these to start the used FIFOs */
>> +    if (cfg->mfir_a > 0)
>> +        pdata->fifo_a = 1;
>> +    else
>> +        pdata->fifo_a = 0;
>> +
>> +    if (cfg->mfir_b > 0)
>> +        pdata->fifo_b = 1;
>> +    else
>> +        pdata->fifo_b = 0;
>> +
>> +    return 0;
>> +}
>> +
>> +static int dmic_set_config(struct dai *dai, struct 
>> sof_ipc_dai_config *config)
>> +{
>> +    struct decim_modes modes_a;
>> +    struct decim_modes modes_b;
>> +    struct matched_modes modes_ab;
>> +    struct dmic_configuration cfg;
>> +    int ret;
>> +    struct sof_ipc_dai_dmic_params *prm = &config->dmic;
>> +    struct dmic_pdata *dmic = dai_get_drvdata(dai);
>> +
>> +    trace_dmic("dsc");
>> +
>> +#if defined DMIC_FORCE_CONFIG
>> +    /* This is a temporary workaound to set parameters while
>> +     * there is no driver and topology scripts support to
>> +     * set these. Also the PCM sample format(s) and sample rate(s)
>> +     * setting would be better to be common with other DAI types.
>> +     */
>> +    prm->driver_ipc_version = 1;
>> +    prm->pdmclk_min = 768000; /* Min 768 kHz */
>> +    prm->pdmclk_max = 4800000; /* Max 4.80 MHz */
>> +    prm->fifo_fs_a = 48000;
>> +    prm->fifo_fs_b = 0;
>> +    prm->fifo_bits_a = 32;
>> +    prm->fifo_bits_b = 0;
>> +    prm->duty_min = 40; /* Min. 40% */
>> +    prm->duty_max = 60; /* Max. 60% */
>> +    prm->number_of_pdm_controllers = 2;
>> +    prm->pdm[0].clk_edge = 0;
>> +    prm->pdm[0].enable_mic_a = 1; /* Left */
>> +    prm->pdm[0].enable_mic_b = 1; /* Right */
>> +    prm->pdm[0].polarity_mic_a = 0;
>> +    prm->pdm[0].polarity_mic_b = 0;
>> +    prm->pdm[0].skew = 0;
>> +    prm->pdm[1].clk_edge = 0;
>> +    prm->pdm[1].enable_mic_a = 0; /* Left */
>> +    prm->pdm[1].enable_mic_b = 0; /* Right */
>> +    prm->pdm[1].polarity_mic_a = 0;
>> +    prm->pdm[1].polarity_mic_b = 0;
>> +    prm->pdm[1].skew = 0;
>> +#endif
>> +
>> +    trace_value(prm->driver_ipc_version);
>> +    trace_value(prm->pdmclk_min);
>> +    trace_value(prm->pdmclk_max);
>> +    trace_value(prm->fifo_fs_a);
>> +    trace_value(prm->fifo_fs_b);
>> +    trace_value(prm->fifo_bits_a);
>> +    trace_value(prm->fifo_bits_b);
>> +    trace_value(prm->duty_min);
>> +    trace_value(prm->duty_max);
>> +    trace_value(prm->number_of_pdm_controllers);
>> +
>> +    if (prm->driver_ipc_version != DMIC_IPC_VERSION) {
>> +        trace_dmic_error("ver");
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Match and select optimal decimators configuration for FIFOs A 
>> and B
>> +     * paths. This setup phase is still abstract. Successful completion
>> +     * points struct cfg to FIR coefficients and contains the scale 
>> value
>> +     * to use for FIR coefficient RAM write as well as the CIC and FIR
>> +     * shift values.
>> +     */
>> +    find_modes(&modes_a, prm, prm->fifo_fs_a);
>> +    if (modes_a.num_of_modes == 0 && prm->fifo_fs_a > 0) {
>> +        trace_dmic_error("amo");
>> +        return -EINVAL;
>> +    }
>> +
>> +    find_modes(&modes_b, prm, prm->fifo_fs_b);
>> +    if (modes_b.num_of_modes == 0 && prm->fifo_fs_b > 0) {
>> +        trace_dmic_error("bmo");
>> +        return -EINVAL;
>> +    }
>> +
>> +    match_modes(&modes_ab, &modes_a, &modes_b);
>> +    ret = select_mode(&cfg, &modes_ab);
>> +    if (ret < 0) {
>> +        trace_dmic_error("smo");
>> +        return -EINVAL;
>> +    }
>> +
>> +    trace_dmic("cfg");
>> +    trace_value(cfg.clkdiv);
>> +    trace_value(cfg.mcic);
>> +    trace_value(cfg.mfir_a);
>> +    trace_value(cfg.mfir_b);
>> +    trace_value(cfg.fir_a_length);
>> +    trace_value(cfg.fir_b_length);
>> +    trace_value(cfg.cic_shift);
>> +    trace_value(cfg.fir_a_shift);
>> +    trace_value(cfg.fir_b_shift);
>> +
>> +    /* Struct reg contains a mirror of actual HW registers. Determine
>> +     * register bits configuration from decimator configuration and the
>> +     * requested parameters.
>> +     */
>> +    ret = configure_registers(dai, &cfg, prm);
>> +    if (ret < 0) {
>> +        trace_dmic_error("cor");
>> +        return -EINVAL;
>> +    }
>> +
>> +    dmic->state = COMP_STATE_PREPARE;
>> +
>> +    return 0;
>> +}
>> +
>> +/* start the DMIC for capture */
>> +static void dmic_start(struct dai *dai, int direction)
>> +{
>> +    struct dmic_pdata *dmic = dai_get_drvdata(dai);
>> +    int i;
>> +    int mic_a;
>> +    int mic_b;
>> +    int fir_a;
>> +    int fir_b;
>> +
>> +    if (direction != DAI_DIR_CAPTURE)
>> +        return;
>> +
>> +    /* enable port */
>> +    spin_lock(&dmic->lock);
>> +    trace_dmic("sta");
>> +    dmic->state = COMP_STATE_ACTIVE;
>> +
>> +    if (dmic->fifo_a) {
>> +        trace_dmic("ffa");
>> +        /*  Clear FIFO A initialize, Enable interrupts to DSP,
>> +         *  Start FIFO A packer.
>> +         */
>> +        dmic_update_bits(dai, OUTCONTROL0,
>> +            OUTCONTROL0_FINIT_BIT | OUTCONTROL0_SIP_BIT,
>> +            OUTCONTROL0_SIP_BIT);
>> +    }
>> +    if (dmic->fifo_b) {
>> +        trace_dmic("ffb");
>> +        /*  Clear FIFO B initialize, Enable interrupts to DSP,
>> +         *  Start FIFO B packer.
>> +         */
>> +        dmic_update_bits(dai, OUTCONTROL1,
>> +            OUTCONTROL1_FINIT_BIT | OUTCONTROL1_SIP_BIT,
>> +            OUTCONTROL1_SIP_BIT);
>> +    }
>> +
>> +    for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
>> +        mic_a = dmic->enable[i] & 1;
>> +        mic_b = (dmic->enable[i] & 2) >> 1;
>> +        if (dmic->fifo_a)
>> +            fir_a = (dmic->enable[i] > 0) ? 1 : 0;
>> +        else
>> +            fir_a = 0;
>> +        if (dmic->fifo_b)
>> +            fir_b = (dmic->enable[i] > 0) ? 1 : 0;
>> +        else
>> +            fir_b = 0;
>> +
>> +        trace_dmic("mfn");
>> +        trace_value(mic_a);
>> +        trace_value(mic_b);
>> +        trace_value(fir_a);
>> +        trace_value(fir_b);
>> +
>> +        dmic_update_bits(dai, base[i] + CIC_CONTROL,
>> +            CIC_CONTROL_CIC_START_A_BIT |
>> +            CIC_CONTROL_CIC_START_B_BIT,
>> +            CIC_CONTROL_CIC_START_A(mic_a) |
>> +            CIC_CONTROL_CIC_START_B(mic_b));
>> +        dmic_update_bits(dai, base[i] + MIC_CONTROL,
>> +            MIC_CONTROL_PDM_EN_A_BIT |
>> +            MIC_CONTROL_PDM_EN_B_BIT,
>> +            MIC_CONTROL_PDM_EN_A(mic_a) |
>> +            MIC_CONTROL_PDM_EN_B(mic_b));
>> +
>> +        dmic_update_bits(dai, base[i] + FIR_CONTROL_A,
>> +            FIR_CONTROL_A_START_BIT, FIR_CONTROL_A_START(fir_a));
>> +        dmic_update_bits(dai, base[i] + FIR_CONTROL_B,
>> +            FIR_CONTROL_B_START_BIT, FIR_CONTROL_B_START(fir_b));
>> +    }
>> +
>> +    /* Clear soft reset for all/used PDM controllers. This should
>> +     * start capture in sync.
>> +     */
>> +    trace_dmic("unr");
>> +    for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
>> +        dmic_update_bits(dai, base[i] + CIC_CONTROL,
>> +            CIC_CONTROL_SOFT_RESET_BIT, 0);
>> +    }
>> +
>> +    spin_unlock(&dmic->lock);
>> +
>> +    /* Currently there's no DMIC HW internal mutings and wait times
>> +     * applied into this start sequence. It can be implemented here if
>> +     * start of audio capture would contain clicks and/or noise and it
>> +     * is not suppressed by gain ramp somewhere in the capture pipe.
>> +     */
>> +    trace_dmic("run");
>> +}
>> +
>> +/* stop the DMIC for capture */
>> +static void dmic_stop(struct dai *dai)
>> +{
>> +    struct dmic_pdata *dmic = dai_get_drvdata(dai);
>> +    int i;
>> +
>> +    trace_dmic("sto")
>> +
>> +    spin_lock(&dmic->lock);
>> +
>> +    /* stop Rx if we are not capturing */
>> +    if (dmic->state != COMP_STATE_ACTIVE) {
>> +        /* Set every PDM controller to soft reset */
>> +        trace_dmic("sre");
>> +        for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
>> +            dmic_update_bits(dai, base[i] + CIC_CONTROL,
>> +                CIC_CONTROL_SOFT_RESET_BIT, 0);
>> +        }
>> +
>> +        /* Stop FIFO packers and set FIFO initialize bits */
>> +        dmic_update_bits(dai, OUTCONTROL0,
>> +            OUTCONTROL0_SIP_BIT | OUTCONTROL0_FINIT_BIT,
>> +            OUTCONTROL0_FINIT_BIT);
>> +        dmic_update_bits(dai, OUTCONTROL1,
>> +            OUTCONTROL1_SIP_BIT | OUTCONTROL1_FINIT_BIT,
>> +            OUTCONTROL1_FINIT_BIT);
>> +
>> +        dmic->state = COMP_STATE_PREPARE;
>> +    }
>> +
>> +    /* Set soft reset for all PDM controllers.
>> +     */
>> +    trace_dmic("sre");
>> +    for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
>> +        dmic_update_bits(dai, base[i] + CIC_CONTROL,
>> +            CIC_CONTROL_SOFT_RESET_BIT, CIC_CONTROL_SOFT_RESET_BIT);
>> +    }
>> +
>> +    spin_unlock(&dmic->lock);
>> +}
>> +
>> +/* save DMIC context prior to entering D3 */
>> +static int dmic_context_store(struct dai *dai)
>> +{
>> +    /* TODO: Nothing stored at the moment. Could read the registers 
>> into
>> +     * persisten memory if needed but the large coef RAM is not 
>> desirable
>> +     * to copy. It would be better to store selected mode parametesr 
>> from
>> +     * previous configuration request and re-program registers from
>> +     * scratch.
>> +     */
>> +    return 0;
>> +}
>> +
>> +/* restore DMIC context after leaving D3 */
>> +static int dmic_context_restore(struct dai *dai)
>> +{
>> +    /* Nothing restored at the moment. */
>> +    return 0;
>> +}
>> +
>> +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?

>
>> +        break;
>> +    case COMP_TRIGGER_RESUME:
>> +        dmic_context_restore(dai);
>> +        break;
>> +    case COMP_TRIGGER_SUSPEND:
>> +        dmic_context_store(dai);
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +/* TODO: No idea what should be done here. Currently only trace the
>> + * status register that contains a few status and error bit fields.
>> + */
>> +static void dmic_irq_handler(void *data)
>> +{
>> +    struct dai *dai = data;
>> +    uint32_t val;
>> +
>> +    /* Trace OUTSTAT0 register */
>> +    val = dmic_read(dai, OUTSTAT0);
>> +    trace_dmic("irq");
>> +
>> +    if (val & OUTSTAT0_ROR_BIT)
>> +        trace_dmic_error("eor"); /* Full fifo or PDM overrrun */
>> +
>> +    trace_value(dmic_read(dai, OUTSTAT0));
>> +
>> +    /* clear IRQ */
>> +    platform_interrupt_clear(dmic_irq(dai), 1);
>> +}
>> +
>> +static int dmic_probe(struct dai *dai)
>> +{
>> +    struct dmic_pdata *dmic;
>> +
>> +    trace_dmic("pro");
>> +
>> +    /* allocate private data */
>> +    dmic = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, sizeof(*dmic));
>> +    dai_set_drvdata(dai, dmic);
>> +
>> +    spinlock_init(&dmic->lock);
>> +
>> +    /* Set state, note there is no playback direction support */
>> +    dmic->state = COMP_STATE_READY;
>> +
>> +    /* register our IRQ handler */
>> +    interrupt_register(dmic_irq(dai), dmic_irq_handler, dai);
>> +
>> +    platform_interrupt_unmask(dmic_irq(dai), 1);
>> +    interrupt_enable(dmic_irq(dai));
>> +
>> +    return 0;
>> +}
>> +
>> +/* DMIC has no loopback support */
>> +static inline int dmic_set_loopback_mode(struct dai *dai, uint32_t lbm)
>> +{
>> +    return -EINVAL;
>> +}
>> +
>> +const struct dai_ops dmic_ops = {
>> +    .trigger = dmic_trigger,
>> +    .set_config = dmic_set_config,
>> +    .pm_context_store = dmic_context_store,
>> +    .pm_context_restore = dmic_context_restore,
>> +    .probe = dmic_probe,
>> +    .set_loopback_mode = dmic_set_loopback_mode,
>> +};
>
> [cut here]
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>



More information about the Sound-open-firmware mailing list