[alsa-devel] [PATCH 1/1] ASoC: codecs: max98088: Combined DAI operations for DAI1 and DAI2.

[Peter Hsiang]

On Thu, Jun 09, 2011, Jin Park wrote:

> Combined DAI operations for DAI1 and DAI2.
> For this, added id into DAI runtime data to identify each DAI.
> 
> Signed-off-by: Jin Park <jinyoungp at nvidia.com>

> @@ -1262,87 +1262,43 @@ static int max98088_dai1_hw_params(struct snd_pcm_substream *substream,
>
> -       cdata = &max98088->dai[1];
> +       cdata = &max98088->dai[dai->id - 1];

Please add BUG_ON protection in case later someone changes the id usage.

Mark, what's your thought on the use of id for this purpose here?
Would it be better to have 2 wrapper DAI functions passing in the DAI number?


> +static int max98088_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>  {
> +       unsigned short fmt_reg;
> +       unsigned short clk_reg;
> +       unsigned short clkcfg_hi_reg;
> +       unsigned short clkcfg_lo_reg;
> +       u8 clk_val;
> +       u8 fmt_val = 0;
> +
> +       switch (dai->id) {
> +       case 1:
> +               fmt_reg = M98088_REG_14_DAI1_FORMAT;
> +               clk_reg = M98088_REG_15_DAI1_CLOCK;
> +               clkcfg_hi_reg = M98088_REG_12_DAI1_CLKCFG_HI;
> +               clkcfg_lo_reg = M98088_REG_13_DAI1_CLKCFG_LO;
> +               break;
> +       case 2:
> +               fmt_reg = M98088_REG_1C_DAI2_FORMAT;
> +               clk_reg = M98088_REG_1D_DAI2_CLOCK;
> +               clkcfg_hi_reg = M98088_REG_1A_DAI2_CLKCFG_HI;
> +               clkcfg_lo_reg = M98088_REG_1B_DAI2_CLKCFG_LO;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }

These added local variables and their initialization repeats.
It may be better to put them in struct max98088_cdata[].


> @@ -1470,127 +1439,54 @@ static int max98088_dai1_set_fmt(struct snd_soc_dai *codec_dai,
> -               reg15val = M98088_DAI_BSEL64;
> +               clk_val = M98088_DAI_BSEL64;
>                if (max98088->digmic)
> -                       reg15val |= M98088_DAI_OSR64;
> -               snd_soc_write(codec, M98088_REG_15_DAI1_CLOCK, reg15val);
> +                       clk_val |= M98088_DAI_OSR64;
> +               snd_soc_write(codec, clk_reg, clk_val);

DAI2 does not have an OSR bit in the _CLOCK register.
In the combined function, this should be conditionally for DAI1 only.

Peter