From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760543AbcAKPwg (ORCPT ); Mon, 11 Jan 2016 10:52:36 -0500 Received: from vi37-28-154-113.vibiznes.pl ([37.28.154.113]:40418 "EHLO vps-vb.mhejs.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760379AbcAKPwe (ORCPT ); Mon, 11 Jan 2016 10:52:34 -0500 Message-ID: <5693CFB2.3090301@maciej.szmigiero.name> Date: Mon, 11 Jan 2016 16:52:18 +0100 From: "Maciej S. Szmigiero" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0 MIME-Version: 1.0 To: Timur Tabi , "alsa-devel@alsa-project.org" CC: Nicolin Chen , Xiubo Li , Liam Girdwood , Mark Brown , "linuxppc-dev@lists.ozlabs.org" , linux-kernel , Fabio Estevam , Zidan Wang Subject: Re: [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile References: <56770FE1.4060202@maciej.szmigiero.name> <5692CE43.20708@tabi.org> In-Reply-To: <5692CE43.20708@tabi.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Timur, Thanks for review. On 10.01.2016 22:33, Timur Tabi wrote: > Maciej S. Szmigiero wrote: >> + regmap_write(regs, CCSR_SSI_SACNT, >> + ssi_private->regcache_sacnt); > > So I'm not familiar with all of the regcache features, but I understand this patch. > I was wondering if it makes sense to write the same exact value that was read previously. > Isn't it possible for the WR or RD bits to change between fsl_ssi_suspend() and fsl_ssi_resume()? These bits are only set in fsl_ssi_ac97_{read,write} which then wait 100usecs before returning. This should be enough for SSI core to finish the relevant operation and clear the bits again, so theoretically they shouldn't be set outside these functions. However, if AC'97 register access is done concurrently with suspend or resume the read / written reg data might be corrupted. It looks to me this is indeed possible since SSI PM callbacks are set in its platform driver struct but ASoC core only calls PM callbacks in snd_soc_dai_driver (which SSI driver don't set). If I am correct with this reasoning then these callbacks need to be added to snd_soc_dai_driver but platform driver ones should still be provided in case the driver is loaded but the sound card is not yet registered. I've CCed Zidan since he originally added PM support to this driver. > That is, should we be doing this instead? > > u32 temp; > regmap_read(regs, CCSR_SSI_SACNT, &temp); > temp &= 0x18; // preserve WR and RD > regmap_write(regs, CCSR_SSI_SACNT, (ssi_private->regcache_sacnt & ~0x18) | temp); > Maciej