From patchwork Wed Feb 28 10:23:13 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ladislav Michl X-Patchwork-Id: 10247127 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4488D60212 for ; Wed, 28 Feb 2018 10:30:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 35ACB28C9A for ; Wed, 28 Feb 2018 10:30:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2935728C9F; Wed, 28 Feb 2018 10:30:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0C6E028C9A for ; Wed, 28 Feb 2018 10:30:48 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id 9A3B0267222; Wed, 28 Feb 2018 11:23:23 +0100 (CET) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id C619A26722C; Wed, 28 Feb 2018 11:23:20 +0100 (CET) Received: from cvs.linux-mips.org (eddie.linux-mips.org [148.251.95.138]) by alsa0.perex.cz (Postfix) with ESMTP id 553D6267217 for ; Wed, 28 Feb 2018 11:23:17 +0100 (CET) Received: (from localhost user: 'ladis' uid#1021 fake: STDIN (ladis@eddie.linux-mips.org)) by eddie.linux-mips.org id S23990498AbeB1KXQeMABB (ORCPT ); Wed, 28 Feb 2018 11:23:16 +0100 Date: Wed, 28 Feb 2018 11:23:13 +0100 From: Ladislav Michl To: Alexandre Belloni Message-ID: <20180228102313.GA12699@lenoch> References: <20180130110604.GA18123@lenoch> <20180130111033.GG18123@lenoch> <20180227172309.GG1479@piout.net> <20180227190338.GB1961@lenoch> <20180228100054.GJ1479@piout.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20180228100054.GJ1479@piout.net> User-Agent: Mutt/1.9.3 (2018-01-21) Cc: alsa-devel@alsa-project.org, Charles Keepax , Nicolas Ferre , Kuninori Morimoto , anish kumar Subject: Re: [alsa-devel] [PATCH 6/7] ASoC: max9867: Fix BSEL value in master mode. X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Feb 28, 2018 at 11:00:54AM +0100, Alexandre Belloni wrote: > On 27/02/2018 at 20:03:38 +0100, Ladislav Michl wrote: > > 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. > > I would thin that is correct because then the eror goes up to userspace > and it knows it has to resample. Perhaps I didn't express it clearly enough. I mean all those dev_err(dev, "sunsupported rate\n"); Here only human sitting in realspace is able to react somehow, but I'd say he's only annoyed. I would expect driver is able to set any rate it is advertising, so returned -EINVAL is considered to be a programming error (which shouldn't clutter kernel log). > > - 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? > > That is a good question I can't answer. Thank you anyway, hopefully someone else would be able to answer. Consider patch bellow - with introduced change codec is able to set any rate in master mode (it is an example only, it needs more work), but not any rate will be matched exactly. Of course, for each master clock rate it is possible to compute exact supported rates and advertise them. Is that way forward or is there any trick with SNDRV_PCM_RATE_CONTINUOUS? Subject: ASoC: max9867: Use continuous sample rate Drop "Common NI Values Table" and calculate LRCLK divider. Signed-off-by: Ladislav Michl diff --git a/sound/soc/codecs/max9867.c b/sound/soc/codecs/max9867.c index 0a6bd9f4d00a..d9b81a95a200 100644 --- a/sound/soc/codecs/max9867.c +++ b/sound/soc/codecs/max9867.c @@ -126,75 +126,19 @@ static const struct snd_soc_dapm_route max9867_audio_map[] = { {"LINE_IN", NULL, "Right Line"}, }; -enum rates { - pcm_rate_8, pcm_rate_16, pcm_rate_24, - pcm_rate_32, pcm_rate_44, - pcm_rate_48, max_pcm_rate, -}; - -static const struct ni_div_rates { - u32 mclk; - u16 ni[max_pcm_rate]; -} ni_div[] = { - {11289600, {0x116A, 0x22D4, 0x343F, 0x45A9, 0x6000, 0x687D} }, - {12000000, {0x1062, 0x20C5, 0x3127, 0x4189, 0x5A51, 0x624E} }, - {12288000, {0x1000, 0x2000, 0x3000, 0x4000, 0x5833, 0x6000} }, - {13000000, {0x0F20, 0x1E3F, 0x2D5F, 0x3C7F, 0x535F, 0x5ABE} }, - {19200000, {0x0A3D, 0x147B, 0x1EB8, 0x28F6, 0x3873, 0x3D71} }, - {24000000, {0x1062, 0x20C5, 0x1893, 0x4189, 0x5A51, 0x624E} }, - {26000000, {0x0F20, 0x1E3F, 0x16AF, 0x3C7F, 0x535F, 0x5ABE} }, - {27000000, {0x0E90, 0x1D21, 0x15D8, 0x3A41, 0x5048, 0x5762} }, -}; - -static inline int get_ni_value(int mclk, int rate) -{ - int i, ret = 0; - - /* find the closest rate index*/ - for (i = 0; i < ARRAY_SIZE(ni_div); i++) { - if (ni_div[i].mclk >= mclk) - break; - } - if (i == ARRAY_SIZE(ni_div)) - return -EINVAL; - - switch (rate) { - case 8000: - return ni_div[i].ni[pcm_rate_8]; - case 16000: - return ni_div[i].ni[pcm_rate_16]; - case 32000: - return ni_div[i].ni[pcm_rate_32]; - case 44100: - return ni_div[i].ni[pcm_rate_44]; - case 48000: - return ni_div[i].ni[pcm_rate_48]; - default: - pr_err("%s wrong rate %d\n", __func__, rate); - ret = -EINVAL; - } - return ret; -} - static int max9867_dai_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) { struct snd_soc_component *component = dai->component; struct max9867_priv *max9867 = snd_soc_component_get_drvdata(component); - unsigned int ni_h, ni_l; - int value; + unsigned int ni = DIV_ROUND_CLOSEST_ULL(96ULL * 0x10000 * params_rate(params), + max9867->pclk); - value = get_ni_value(max9867->sysclk, params_rate(params)); - if (value < 0) - return value; - - ni_h = (0xFF00 & value) >> 8; - ni_l = 0x00FF & value; /* set up the ni value */ regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKHIGH, - MAX9867_NI_HIGH_MASK, ni_h); + MAX9867_NI_HIGH_MASK, (0xFF00 & ni) >> 8); regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKLOW, - MAX9867_NI_LOW_MASK, ni_l); + MAX9867_NI_LOW_MASK, 0x00FF & ni); if (!max9867->master) { /* * digital pll locks on to any externally supplied LRCLK signal @@ -241,13 +185,13 @@ static int max9867_set_dai_sysclk(struct snd_soc_dai *codec_dai, /* Set the prescaler based on the master clock frequency*/ if (freq >= 10000000 && freq <= 20000000) { value |= MAX9867_PSCLK_10_20; - max9867->pclk = freq; + max9867->pclk = freq; } else if (freq >= 20000000 && freq <= 40000000) { value |= MAX9867_PSCLK_20_40; - max9867->pclk = freq/2; + max9867->pclk = freq / 2; } else if (freq >= 40000000 && freq <= 60000000) { value |= MAX9867_PSCLK_40_60; - max9867->pclk = freq/4; + max9867->pclk = freq / 4; } else { dev_err(component->dev, "Invalid clock frequency %uHz (required 10-60MHz)\n", @@ -322,10 +266,6 @@ static const struct snd_soc_dai_ops max9867_dai_ops = { .hw_params = max9867_dai_hw_params, }; -#define MAX9867_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |\ - SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000) -#define MAX9867_FORMATS (SNDRV_PCM_FMTBIT_S16_LE) - static struct snd_soc_dai_driver max9867_dai[] = { { .name = "max9867-aif1", @@ -333,15 +273,19 @@ static struct snd_soc_dai_driver max9867_dai[] = { .stream_name = "HiFi Playback", .channels_min = 1, .channels_max = 2, - .rates = MAX9867_RATES, - .formats = MAX9867_FORMATS, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .rate_min = 8000, + .rate_max = 48000, + .formats = SNDRV_PCM_FMTBIT_S16_LE, }, .capture = { .stream_name = "HiFi Capture", .channels_min = 1, .channels_max = 2, - .rates = MAX9867_RATES, - .formats = MAX9867_FORMATS, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .rate_min = 8000, + .rate_max = 48000, + .formats = SNDRV_PCM_FMTBIT_S16_LE, }, .ops = &max9867_dai_ops, }