From patchwork Tue Aug 2 16:13:11 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 1029682 Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p72GDS2S009514 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 2 Aug 2011 16:13:49 GMT Received: from canuck.infradead.org ([2001:4978:20e::1]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QoHar-0001ct-JD; Tue, 02 Aug 2011 16:13:21 +0000 Received: from localhost ([127.0.0.1] helo=canuck.infradead.org) by canuck.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QoHar-0006Y6-3E; Tue, 02 Aug 2011 16:13:21 +0000 Received: from cantor2.suse.de ([195.135.220.15] helo=mx2.suse.de) by canuck.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QoHan-0006XW-5t for linux-arm-kernel@lists.infradead.org; Tue, 02 Aug 2011 16:13:18 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 0A2CC8C5DF; Tue, 2 Aug 2011 18:13:13 +0200 (CEST) Date: Tue, 02 Aug 2011 18:13:11 +0200 Message-ID: From: Takashi Iwai To: Mark Brown Subject: Re: [alsa-devel] [PATCH 1/1] ASoC: core: cache index fix In-Reply-To: <20110802154815.GE25884@opensource.wolfsonmicro.com> References: <65EE16ACC360FA4D99C96DC085B3F7721F410B@039-SN1MPN1-002.039d.mgd.msft.net> <20110802083850.GB9553@opensource.wolfsonmicro.com> <65EE16ACC360FA4D99C96DC085B3F7721F427D@039-SN1MPN1-002.039d.mgd.msft.net> <65EE16ACC360FA4D99C96DC085B3F7721F43AA@039-SN1MPN1-002.039d.mgd.msft.net> <65EE16ACC360FA4D99C96DC085B3F7721F443D@039-SN1MPN1-002.039d.mgd.msft.net> <65EE16ACC360FA4D99C96DC085B3F7721F45A8@039-SN1MPN1-002.039d.mgd.msft.net> <20110802154815.GE25884@opensource.wolfsonmicro.com> User-Agent: Wanderlust/2.15.6 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.7 Emacs/23.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20110802_121317_529000_30438B25 X-CRM114-Status: GOOD ( 25.68 ) X-Spam-Score: -3.1 (---) X-Spam-Report: SpamAssassin version 3.3.1 on canuck.infradead.org summary: Content analysis details: (-3.1 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [195.135.220.15 listed in list.dnswl.org] -0.8 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain Cc: "alsa-devel@alsa-project.org" , "s.hauer@pengutronix.de" , "w.sang@pengutronix.de" , Dong Aisheng-B29396 , "lrg@ti.com" , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Tue, 02 Aug 2011 16:13:49 +0000 (UTC) At Wed, 3 Aug 2011 00:48:16 +0900, Mark Brown wrote: > > On Tue, Aug 02, 2011 at 02:52:29PM +0200, Takashi Iwai wrote: > > > Mark, Liam, could you guys take a deeper look and clean these messes > > up? > > Like I've indicated several times now we should just get rid of the code > or hide it from the rest of the subsystem, it's being too cute for > vanishingly little value. The register maps for these devices are > usually at most 255 registers so the memory savings are really not > meaningful. I'm hoping the guys working with this device will find time > to look at fixing things, but if not I'd imagine we'll get to it at some > point in the release cycle. Well, there aren't so many drivers suffering from this bug, so a temporary fix would be easy like below (totally untested). Takashi diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c index 923b364..6aea8e2 100644 --- a/sound/soc/codecs/ad1980.c +++ b/sound/soc/codecs/ad1980.c @@ -244,7 +244,7 @@ static int ad1980_soc_remove(struct snd_soc_codec *codec) static struct snd_soc_codec_driver soc_codec_dev_ad1980 = { .probe = ad1980_soc_probe, .remove = ad1980_soc_remove, - .reg_cache_size = ARRAY_SIZE(ad1980_reg), + .reg_cache_size = ARRAY_SIZE(ad1980_reg) << 1, .reg_word_size = sizeof(u16), .reg_cache_default = ad1980_reg, .reg_cache_step = 2, diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c index 76258f2..52e5ba3 100644 --- a/sound/soc/codecs/sgtl5000.c +++ b/sound/soc/codecs/sgtl5000.c @@ -1437,7 +1437,7 @@ static struct snd_soc_codec_driver sgtl5000_driver = { .suspend = sgtl5000_suspend, .resume = sgtl5000_resume, .set_bias_level = sgtl5000_set_bias_level, - .reg_cache_size = ARRAY_SIZE(sgtl5000_regs), + .reg_cache_size = SGTL5000_MAX_REG_OFFSET, .reg_word_size = sizeof(u16), .reg_cache_step = 2, .reg_cache_default = sgtl5000_regs, diff --git a/sound/soc/codecs/wm9705.c b/sound/soc/codecs/wm9705.c index 646b58d..7088a89 100644 --- a/sound/soc/codecs/wm9705.c +++ b/sound/soc/codecs/wm9705.c @@ -374,7 +374,7 @@ static struct snd_soc_codec_driver soc_codec_dev_wm9705 = { .resume = wm9705_soc_resume, .read = ac97_read, .write = ac97_write, - .reg_cache_size = ARRAY_SIZE(wm9705_reg), + .reg_cache_size = ARRAY_SIZE(wm9705_reg) << 1, .reg_word_size = sizeof(u16), .reg_cache_step = 2, .reg_cache_default = wm9705_reg, diff --git a/sound/soc/codecs/wm9712.c b/sound/soc/codecs/wm9712.c index 90117f8..6149f02 100644 --- a/sound/soc/codecs/wm9712.c +++ b/sound/soc/codecs/wm9712.c @@ -662,7 +662,7 @@ static struct snd_soc_codec_driver soc_codec_dev_wm9712 = { .read = ac97_read, .write = ac97_write, .set_bias_level = wm9712_set_bias_level, - .reg_cache_size = ARRAY_SIZE(wm9712_reg), + .reg_cache_size = ARRAY_SIZE(wm9712_reg) << 1, .reg_word_size = sizeof(u16), .reg_cache_step = 2, .reg_cache_default = wm9712_reg, diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c index 7167cb6..1995671 100644 --- a/sound/soc/codecs/wm9713.c +++ b/sound/soc/codecs/wm9713.c @@ -1245,7 +1245,7 @@ static struct snd_soc_codec_driver soc_codec_dev_wm9713 = { .read = ac97_read, .write = ac97_write, .set_bias_level = wm9713_set_bias_level, - .reg_cache_size = ARRAY_SIZE(wm9713_reg), + .reg_cache_size = ARRAY_SIZE(wm9713_reg) << 1, .reg_word_size = sizeof(u16), .reg_cache_step = 2, .reg_cache_default = wm9713_reg, diff --git a/sound/soc/soc-cache.c b/sound/soc/soc-cache.c index d9f8ade..d999bc8 100644 --- a/sound/soc/soc-cache.c +++ b/sound/soc/soc-cache.c @@ -408,6 +408,7 @@ static int snd_soc_rbtree_cache_init(struct snd_soc_codec *codec) unsigned int val; int i; int ret; + int step = 1; codec->reg_cache = kmalloc(sizeof *rbtree_ctx, GFP_KERNEL); if (!codec->reg_cache) @@ -421,7 +422,9 @@ static int snd_soc_rbtree_cache_init(struct snd_soc_codec *codec) return 0; word_size = codec->driver->reg_word_size; - for (i = 0; i < codec->driver->reg_cache_size; ++i) { + if (codec->driver->reg_cache_step) + step = codec->driver->reg_cache_step; + for (i = 0; i < codec->driver->reg_cache_size; i += step) { val = snd_soc_get_cache_val(codec->reg_def_copy, i, word_size); if (!val) @@ -820,9 +823,12 @@ static int snd_soc_flat_cache_sync(struct snd_soc_codec *codec) int ret; const struct snd_soc_codec_driver *codec_drv; unsigned int val; + int step = 1; codec_drv = codec->driver; - for (i = 0; i < codec_drv->reg_cache_size; ++i) { + if (codec_drv->reg_cache_step) + step = codec_drv->reg_cache_step; + for (i = 0; i < codec_drv->reg_cache_size; i += step) { WARN_ON(codec->writable_register && codec->writable_register(codec, i)); ret = snd_soc_cache_read(codec, i, &val); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 83ad8ca..596773c 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3269,12 +3269,25 @@ int snd_soc_register_codec(struct device *dev, * the cache. */ if (codec_drv->reg_cache_default) { - codec->reg_def_copy = kmemdup(codec_drv->reg_cache_default, - reg_size, GFP_KERNEL); + codec->reg_def_copy = kzalloc(reg_size, GFP_KERNEL); if (!codec->reg_def_copy) { ret = -ENOMEM; goto fail; } + if (codec_drv->reg_cache_step > 1) { + /* FIXME: reg_cache_default with step > 1 is + * supposed to be a packed array, so here we + * expand into the flat cache array + */ + int step = codec_drv->reg_cache_step; + int wsize = codec_dev->reg_word_size; + for (i = 0; i < reg_size / step; i += wsize) + memcpy(codec->reg_def_copy + i * step, + codec->reg_cache_default + i, + wsize); + } else + memcpy(codec->reg_def_copy, + codec_drv->reg_cache_default, reg_size); } }