Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and
 LRCLK in LPAIF invalid state
To:     Srinivasa Rao Mandadapu <srivasam@codeaurora.org>,
        agross@kernel.org, bjorn.andersson@linaro.org, lgirdwood@gmail.com,
        broonie@kernel.org, robh+dt@kernel.org, plai@codeaurora.org,
        bgoswami@codeaurora.org, perex@perex.cz, tiwai@suse.com,
        rohitkr@codeaurora.org, linux-arm-msm@vger.kernel.org,
        alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     V Sujith Kumar Reddy <vsujithk@codeaurora.org>
References: <1606539559-4277-1-git-send-email-srivasam@codeaurora.org>
 <1606539559-4277-2-git-send-email-srivasam@codeaurora.org>
From:   Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Message-ID: <4b680d9a-23eb-9746-e11e-1506d35e72c4@linaro.org>
Date:   Mon, 30 Nov 2020 11:09:52 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <1606539559-4277-2-git-send-email-srivasam@codeaurora.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 28/11/2020 04:59, Srinivasa Rao Mandadapu wrote:
> This reverts part of commit b1824968221c
> ("ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state")

This should probably go to Fixes tag!

> 

> To identify LPAIF invalid state after device suspend and resume,
> made I2S and DMA control registers not volatile, which is not necessary.
This comment is bit confusing!

Basically it should be something like
"DMA control registers are not volatile, so remove these from volatile 
registers list"

--srini


> Instead invalid reg state can be handled with regcache APIs.
> The BCLK ref count is necessary to enable clock only it's in disable state.
> 
> Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> ---
>   sound/soc/qcom/lpass-cpu.c      | 20 ++------------------
>   sound/soc/qcom/lpass-platform.c | 11 -----------
>   2 files changed, 2 insertions(+), 29 deletions(-)
> 
> diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
> index af684fd..c5e99c2 100644
> --- a/sound/soc/qcom/lpass-cpu.c
> +++ b/sound/soc/qcom/lpass-cpu.c
> @@ -270,18 +270,6 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
>   	struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
>   	unsigned int id = dai->driver->id;
>   	int ret = -EINVAL;
> -	unsigned int val = 0;
> -
> -	ret = regmap_read(drvdata->lpaif_map,
> -				LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id), &val);
> -	if (ret) {
> -		dev_err(dai->dev, "error reading from i2sctl reg: %d\n", ret);
> -		return ret;
> -	}
> -	if (val == LPAIF_I2SCTL_RESET_STATE) {
> -		dev_err(dai->dev, "error in i2sctl register state\n");
> -		return -ENOTRECOVERABLE;
> -	}
>   
>   	switch (cmd) {
>   	case SNDRV_PCM_TRIGGER_START:
> @@ -454,20 +442,16 @@ static bool lpass_cpu_regmap_volatile(struct device *dev, unsigned int reg)
>   	struct lpass_variant *v = drvdata->variant;
>   	int i;
>   
> -	for (i = 0; i < v->i2s_ports; ++i)
> -		if (reg == LPAIF_I2SCTL_REG(v, i))
> -			return true;
>   	for (i = 0; i < v->irq_ports; ++i)
>   		if (reg == LPAIF_IRQSTAT_REG(v, i))
>   			return true;
>   
>   	for (i = 0; i < v->rdma_channels; ++i)
> -		if (reg == LPAIF_RDMACURR_REG(v, i) || reg == LPAIF_RDMACTL_REG(v, i))
> +		if (reg == LPAIF_RDMACURR_REG(v, i))
>   			return true;
>   
>   	for (i = 0; i < v->wrdma_channels; ++i)
> -		if (reg == LPAIF_WRDMACURR_REG(v, i + v->wrdma_channel_start) ||
> -			reg == LPAIF_WRDMACTL_REG(v, i + v->wrdma_channel_start))
> +		if (reg == LPAIF_WRDMACURR_REG(v, i + v->wrdma_channel_start))
>   			return true;
>   
>   	return false;
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index 80b09de..0e71899 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -452,7 +452,6 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
>   	unsigned int reg_irqclr = 0, val_irqclr = 0;
>   	unsigned int  reg_irqen = 0, val_irqen = 0, val_mask = 0;
>   	unsigned int dai_id = cpu_dai->driver->id;
> -	unsigned int dma_ctrl_reg = 0;
>   
>   	ch = pcm_data->dma_ch;
>   	if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
> @@ -469,17 +468,7 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
>   		id = pcm_data->dma_ch - v->wrdma_channel_start;
>   		map = drvdata->lpaif_map;
>   	}
> -	ret = regmap_read(map, LPAIF_DMACTL_REG(v, ch, dir, dai_id), &dma_ctrl_reg);
> -	if (ret) {
> -		dev_err(soc_runtime->dev, "error reading from rdmactl reg: %d\n", ret);
> -		return ret;
> -	}
>   
> -	if (dma_ctrl_reg == LPAIF_DMACTL_RESET_STATE ||
> -		dma_ctrl_reg == LPAIF_DMACTL_RESET_STATE + 1) {
> -		dev_err(soc_runtime->dev, "error in rdmactl register state\n");
> -		return -ENOTRECOVERABLE;
> -	}
>   	switch (cmd) {
>   	case SNDRV_PCM_TRIGGER_START:
>   	case SNDRV_PCM_TRIGGER_RESUME:
>