Re: [alsa-devel] [PATCH 6/7] ASoC: max9867: Fix BSEL value in master mode.

On Tue, Feb 27, 2018 at 06:23:09PM +0100, Alexandre Belloni wrote:

This also needs a proper commit message.

Meanwhile more patches for this driver accumulated, so I'll send simple ones first and those I'm uncertain of later.

Now few questions: - should hw_params emit any error message when asked to set for example unsupported sample rate? Many drivers do so, but it seems strange. - table used in this driver is overkill, frequencies can be calculated directly (patch ready), but that brings question about SNDRV_PCM_RATE_CONTINUOUS: what does it exactly mean? What if codec is able to set "any" rate, but there are rounding errors?

On 30/01/2018 at 12:10:33 +0100, Ladislav Michl wrote:

...

Signed-off-by: Ladislav Michl ladis@linux-mips.org

Note: It should be reconsidered where BSEL should be set.

sound/soc/codecs/max9867.c | 43 +++---------------------------------------- 1 file changed, 3 insertions(+), 40 deletions(-)

diff --git a/sound/soc/codecs/max9867.c b/sound/soc/codecs/max9867.c index 026b7cf94910..6272cf5df4a9 100644 --- a/sound/soc/codecs/max9867.c +++ b/sound/soc/codecs/max9867.c @@ -148,46 +148,6 @@ static int max9867_dai_hw_params(struct snd_pcm_substream *substream, MAX9867_RAPID_LOCK, MAX9867_RAPID_LOCK); regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKHIGH, MAX9867_PLL, MAX9867_PLL);

• } else {

• unsigned long int bclk_rate, pclk_bclk_ratio;
  

• int bclk_value;
  

• bclk_rate = params_rate(params) * 2 * params_width(params);
  

• pclk_bclk_ratio = max9867->pclk/bclk_rate;
  

• switch (params_width(params)) {
  

• case 8:
  

• case 16:
  

• 	switch (pclk_bclk_ratio) {
  

• 	case 2:
  

• 		bclk_value = MAX9867_IFC1B_PCLK_2;
  

• 		break;
  

• 	case 4:
  

• 		bclk_value = MAX9867_IFC1B_PCLK_4;
  

• 		break;
  

• 	case 8:
  

• 		bclk_value = MAX9867_IFC1B_PCLK_8;
  

• 		break;
  

• 	case 16:
  

• 		bclk_value = MAX9867_IFC1B_PCLK_16;
  

• 		break;
  

• 	default:
  

• 		dev_err(codec->dev,
  

• 			"unsupported sampling rate\n");
  

• 		return -EINVAL;
  

• 	}
  

• 	break;
  

• case 24:
  

• 	bclk_value = MAX9867_IFC1B_24BIT;
  

• 	break;
  

• case 32:
  

• 	bclk_value = MAX9867_IFC1B_32BIT;
  

• 	break;
  

• default:
  

• 	dev_err(codec->dev, "unsupported sampling rate\n");
  

• 	return -EINVAL;
  

• }
  

• regmap_update_bits(max9867->regmap, MAX9867_IFC1B,
  

• 	MAX9867_IFC1B_BCLK_MASK, bclk_value);
  

  } return 0;

} @@ -244,6 +204,9 @@ static int max9867_set_dai_sysclk(struct snd_soc_dai *codec_dai, value &= ~MAX9867_FREQ_MASK; regmap_update_bits(max9867->regmap, MAX9867_SYSCLK, MAX9867_PSCLK_MASK, value);

• regmap_update_bits(max9867->regmap, MAX9867_IFC1B,

• 	MAX9867_IFC1B_BCLK_MASK, 0x010);
  

This magic value has to be defined somewhere.

...

• return 0;

}

-- 2.15.1

-- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com