Re: [alsa-devel] [PATCH] ASoC: sun4i-i2s: Show detailed error when DAI configuration callbacks fail

On 14 December 2017 at 08:29, Chen-Yu Tsai wens@csie.org wrote:

When any of the DAI hardware configuration callbacks (.hw_param, .set_fmt, .set_sysclk) fails, there is no explanation about why it failed. This is particularly confusing for .hw_param, which covers many parameters of the DAI. Telling the users what parameter isn't supported, and what the requested value was goes a long way for developers trying to combine sun4i-i2s with external codecs.

This patch adds dev_err calls explaining what isn't supported or failed, and what the value was. sun4i_i2s_set_clk_rate()'s first parameter was changed to a struct snd_soc_dai *dai, so we can get the underlying device.

Signed-off-by: Chen-Yu Tsai wens@csie.org

Acked-by: Marcus Cooper codekipper@gmail.com

Thanks...I can get rid of shit loads of my debugging now.

---

sound/soc/sunxi/sun4i-i2s.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index 13d7ecabe1b6..dca1143c1150 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -269,10 +269,11 @@ static bool sun4i_i2s_oversample_is_valid(unsigned int oversample) return false; }

-static int sun4i_i2s_set_clk_rate(struct sun4i_i2s *i2s, +static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai, unsigned int rate, unsigned int word_size) {

•   struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
    unsigned int oversample_rate, clk_rate;
    int bclk_div, mclk_div;
    int ret;
  

@@ -300,6 +301,7 @@ static int sun4i_i2s_set_clk_rate(struct sun4i_i2s *i2s, break;

    default:

•           dev_err(dai->dev, "Unsupported sample rate: %u\n", rate);
            return -EINVAL;
    }
  
  

@@ -308,18 +310,25 @@ static int sun4i_i2s_set_clk_rate(struct sun4i_i2s *i2s, return ret;

    oversample_rate = i2s->mclk_freq / rate;

•   if (!sun4i_i2s_oversample_is_valid(oversample_rate))
  

•   if (!sun4i_i2s_oversample_is_valid(oversample_rate)) {
  

•           dev_err(dai->dev, "Unsupported oversample rate: %d\n",
  

•                   oversample_rate);
            return -EINVAL;
  

•   }
  
    bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
                                      word_size);
  

•   if (bclk_div < 0)
  

•   if (bclk_div < 0) {
  

•           dev_err(dai->dev, "Unsupported BCLK divider: %d\n", bclk_div);
            return -EINVAL;
  

•   }
  
    mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
                                      clk_rate, rate);
  

•   if (mclk_div < 0)
  

•   if (mclk_div < 0) {
  

•           dev_err(dai->dev, "Unsupported MCLK divider: %d\n", mclk_div);
            return -EINVAL;
  

•   }
  
    /* Adjust the clock division values if needed */
    bclk_div += i2s->variant->bclk_offset;
  

@@ -349,8 +358,11 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream, u32 width;

    channels = params_channels(params);

•   if (channels != 2)
  

•   if (channels != 2) {
  

•           dev_err(dai->dev, "Unsupported number of channels: %d\n",
  

•                   channels);
            return -EINVAL;
  

•   }
  
    if (i2s->variant->has_chcfg) {
            regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
  

@@ -382,6 +394,8 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream, width = DMA_SLAVE_BUSWIDTH_2_BYTES; break; default:

•           dev_err(dai->dev, "Unsupported physical sample width: %d\n",
  

•                   params_physical_width(params));
            return -EINVAL;
    }
    i2s->playback_dma_data.addr_width = width;
  

@@ -393,6 +407,8 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream, break;

    default:

•           dev_err(dai->dev, "Unsupported sample width: %d\n",
  

•                   params_width(params));
            return -EINVAL;
    }
  
  

@@ -401,7 +417,7 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream, regmap_field_write(i2s->field_fmt_sr, sr + i2s->variant->fmt_offset);

•   return sun4i_i2s_set_clk_rate(i2s, params_rate(params),
  

•   return sun4i_i2s_set_clk_rate(dai, params_rate(params),
                                  params_width(params));
  

}

@@ -426,6 +442,8 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) val = SUN4I_I2S_FMT0_FMT_RIGHT_J; break; default:

•           dev_err(dai->dev, "Unsupported format: %d\n",
  

•                   fmt & SND_SOC_DAIFMT_FORMAT_MASK);
            return -EINVAL;
    }
  
  

@@ -464,6 +482,8 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) case SND_SOC_DAIFMT_NB_NF: break; default:

•           dev_err(dai->dev, "Unsupported clock polarity: %d\n",
  

•                   fmt & SND_SOC_DAIFMT_INV_MASK);
            return -EINVAL;
    }
  
  

@@ -482,6 +502,8 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) val = SUN4I_I2S_CTRL_MODE_SLAVE; break; default:

•                   dev_err(dai->dev, "Unsupported slave setting: %d\n",
  

•                           fmt & SND_SOC_DAIFMT_MASTER_MASK);
                    return -EINVAL;
            }
            regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
  

@@ -504,6 +526,8 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) val = 0; break; default:

•                   dev_err(dai->dev, "Unsupported slave setting: %d\n",
  

•                           fmt & SND_SOC_DAIFMT_MASTER_MASK);
                    return -EINVAL;
            }
            regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
  

-- 2.15.0