Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752217AbbDXAsT (ORCPT ); Thu, 23 Apr 2015 20:48:19 -0400 Received: from mail-la0-f47.google.com ([209.85.215.47]:35486 "EHLO mail-la0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751319AbbDXAsO (ORCPT ); Thu, 23 Apr 2015 20:48:14 -0400 MIME-Version: 1.0 In-Reply-To: References: <1429134141-17924-1-git-send-email-cernekee@chromium.org> <1429134141-17924-2-git-send-email-cernekee@chromium.org> <20150418113632.GE26185@sirena.org.uk> From: Kevin Cernekee Date: Thu, 23 Apr 2015 17:47:49 -0700 Message-ID: Subject: Re: [PATCH 2/3] ASoC: tas571x: New driver for TI TAS571x power amplifiers To: Mark Brown Cc: Liam Girdwood , dgreid@chromium.org, Andrew Bresticker , Olof Johansson , alsa-devel@alsa-project.org, devicetree@vger.kernel.org, "linux-kernel@vger.kernel.org" , Lars-Peter Clausen Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1917 Lines: 48 On Sat, Apr 18, 2015 at 9:16 AM, Kevin Cernekee wrote: >>> + case SND_SOC_BIAS_OFF: >>> + /* Note that this kills I2C accesses. */ >>> + assert_pdn = 1; >> >> No, the GPIO set associated with it kills I2C access. I'd also expect >> to see the regmap being marked cache only before we do this and a resync >> of the register map when we power back up (assuming that is actually a >> power down). > > Hmm, not sure if this actually resets the registers back to power-on > defaults, but I'll check. Hi Mark, I have reworked the driver to do the following: - set appropriate regmap default values on probe - enable idle_bias_off - use regcache_cache_only() to prevent accesses to I2C when in SND_SOC_BIAS_OFF state (pdn asserted) - use regcache_sync() when transitioning from SND_SOC_BIAS_OFF -> SND_SOC_BIAS_STANDBY This is mostly working OK, but regcache_sync() assumes that the hardware registers have been reset back to the default values. The "pdn" GPIO doesn't actually reset the state of the tas571x; it just makes I2C inaccessible and inhibits audio output. So if the factory default for mute is 0, corner cases like this fail: - enter SND_SOC_BIAS_ON (e.g. play a wav file) - set mute to 1 - enter SND_SOC_BIAS_OFF (e.g. playback ends) - set mute to 0 - re-enter SND_SOC_BIAS_ON - regcache_sync() incorrectly assumes that the hardware register is already 0, but in fact it needs to be refreshed from the cache Aside from unnecessarily pulsing the reset GPIO when transitioning back from SND_SOC_BIAS_OFF or overriding regcache_default_sync(), can you think of a way to work around this? Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/