From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751840AbdKUAAq (ORCPT ); Mon, 20 Nov 2017 19:00:46 -0500 Received: from mail-pg0-f68.google.com ([74.125.83.68]:33524 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751519AbdKUAAp (ORCPT ); Mon, 20 Nov 2017 19:00:45 -0500 X-Google-Smtp-Source: AGs4zMYXUQE1dTY1bD4FYyEYIoee2YBGjZ/23XSkAxiMh7FHBMeWFUdnA+0xUVYJwK8pWQluqtmsgA== Date: Mon, 20 Nov 2017 16:00:43 -0800 From: Nicolin Chen To: "Maciej S. Szmigiero" Cc: Timur Tabi , Xiubo Li , alsa-devel@alsa-project.org, linux-kernel , Takashi Iwai , Liam Girdwood , Mark Brown , Fabio Estevam , linuxppc-dev@lists.ozlabs.org Subject: Re: [alsa-devel] [PATCH] ASoC: fsl_ssi: call _fsl_ssi_set_dai_fmt() just once in AC'97 mode Message-ID: <20171121000042.GB14136@Asurada-Nvidia> References: <81ad2e7e-e070-c38e-b318-9607b4558ee7@maciej.szmigiero.name> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <81ad2e7e-e070-c38e-b318-9607b4558ee7@maciej.szmigiero.name> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 20, 2017 at 11:13:45PM +0100, Maciej S. Szmigiero wrote: > In AC'97 mode we configure and start SSI RX / TX on probe path via > a call to _fsl_ssi_set_dai_fmt() function. > We don't need to call this function again later and in fact don't want to > do it since this function temporarily sets STCR, SRCR and SCR to some > intermediate values. > > We need to make sure, however, that only proper channel slots are enabled > at playback start time since some AC'97 CODECs (like VT1613) were observed > requesting via SLOTREQ (and so enabling at SSI) spurious ones just after > an AC'97 link is started but before the CODEC is configured by its driver. I don't really understand this part. Why do we need to *make sure* and set SACCDIS and SACCEN again since they're initialized already? Could you please elaborate a bit more? > Theoretically, this should be necessary only for the very first playback > but let's play safe here and make sure that no extra slots are enabled > every time a playback is started. > > Signed-off-by: Maciej S. Szmigiero > --- > sound/soc/fsl/fsl_ssi.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c > index 48bb850a34d9..dad80b4b0cfc 100644 > --- a/sound/soc/fsl/fsl_ssi.c > +++ b/sound/soc/fsl/fsl_ssi.c > @@ -630,12 +630,6 @@ static void fsl_ssi_setup_ac97(struct fsl_ssi_private *ssi_private) > regmap_write(regs, CCSR_SSI_SACNT, > CCSR_SSI_SACNT_AC97EN | CCSR_SSI_SACNT_FV); > > - /* no SACC{ST,EN,DIS} regs on imx21-class SSI */ > - if (!ssi_private->soc->imx21regs) { > - regmap_write(regs, CCSR_SSI_SACCDIS, 0xff); > - regmap_write(regs, CCSR_SSI_SACCEN, 0x300); > - } > @@ -1149,9 +1146,16 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd, > case SNDRV_PCM_TRIGGER_START: > case SNDRV_PCM_TRIGGER_RESUME: > case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > + /* no SACC{ST,EN,DIS} regs on imx21-class SSI */ > + if (fsl_ssi_is_ac97(ssi_private) && > + !ssi_private->soc->imx21regs) { > + regmap_write(regs, CCSR_SSI_SACCDIS, 0xff); > + regmap_write(regs, CCSR_SSI_SACCEN, 0x300); > + } And second, why could we ignore them for STREAM_CAPTURE here? Well, at least this part could be moved into fsl_ssi_tx_config() since we have an abstraction layer of register configurations. Thanks Nic > + > fsl_ssi_tx_config(ssi_private, true); > - else > + } else > fsl_ssi_rx_config(ssi_private, true); > break; > > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel