From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lee Jones Subject: Re: [PATCH 02/11] ASoC: ab8500: Revert back to find_next_bit() Date: Thu, 19 Dec 2013 16:48:24 +0000 Message-ID: <20131219164824.GA16145@lee--X1> References: <1387468508-12286-1-git-send-email-lee.jones@linaro.org> <1387468508-12286-3-git-send-email-lee.jones@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wi0-f171.google.com (mail-wi0-f171.google.com [209.85.212.171]) by alsa0.perex.cz (Postfix) with ESMTP id ECF532654D0 for ; Thu, 19 Dec 2013 17:48:32 +0100 (CET) Received: by mail-wi0-f171.google.com with SMTP id bz8so7151051wib.16 for ; Thu, 19 Dec 2013 08:48:32 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Iwai Cc: alsa-devel@alsa-project.org, linus.walleij@linaro.org, broonie@kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-Id: alsa-devel@alsa-project.org PiA+IENvbW1pdCAnMTY2YTM0ZCBBU29DOiBhYjg1MDA6IEZpeCBpbnZhbGlkIGNhc3QgdG8gbG9u ZyBwb2ludGVyJwo+ID4gcmF0aGVyIGNhcmVsZXNzbHkgY29udmVydHMgZmluZF9uZXh0X2JpdCgp IHRvIGZscygpIChmaW5kIGxhc3QKPiA+IGJpdCBzZXQpLCB3aGljaCBhcmUgbm90IHRoZSBzYW1l Lgo+IAo+IERvZXMgaXQgYnJlYWsgb24gdGhlIHJlYWwgbWFjaGluZXM/CgpJdCBkb2VzLCB0aGF0 J3MgaG93IEkgZm91bmQgdGhlIGJ1Zy4KCj4gZmxzKCkgYmVoYXZlcyBkaWZmZXJlbnRseSBmcm9t IGZpbmRfbmV4dF9iaXQoKSwgb2YgY291cnNlLCBidXQgaW4gdGhpcwo+IGNhc2UsIGl0IHNob3Vs ZCB3b3JrIHNhbWUgaW4gdGhlIGVuZCwgc2luY2UgdGhlcmUgYXJlIGF0IG1vc3QgdHdvCj4gYml0 cy4KClVuZm9ydHVuYXRlbHkgaXQgZG9lc24ndCB3b3JrIGF0IGFsbC4gVGhpcyBwYXRjaCBicmlu Z3MgYXVkaW8gYmFjayB0bwphIHdvcmtpbmcgc3RhdGUuIEl0IHRvb2sgbWUgdGhlIGJlc3QgcGFy dCBvZiBhIGRheSB0byB0cmFjayBkb3duIHRoZQppc3N1ZS4gOigKCj4gPiBBbHRob3VnaCB0aGlz IGltcGxlbWVudGF0aW9uIG1heSBub3QgYmUgcG9ydGFibGUsIHRoZSBhbHRlcm5hdGl2ZQo+ID4g YnJlYWtzIGF1ZGlvIG9uIGFueXRoaW5nIHVzaW5nIHRoZSBBQjg1MDAgQ09ERUMsIHdoaWNoIGlz IGEgbXVjaAo+ID4gZ3JlYXRlciBldmlsLiBCZXNpZGVzLCB0aGlzIGRyaXZlciB3aWxsIG5ldmVy IHJ1biBvbiBhbnl0aGluZwo+ID4gZWxzZS4KPiA+IAo+ID4gQ2M6IFRha2FzaGkgSXdhaSA8dGl3 YWlAc3VzZS5kZT4KPiA+IFNpZ25lZC1vZmYtYnk6IExlZSBKb25lcyA8bGVlLmpvbmVzQGxpbmFy by5vcmc+Cj4gPiAtLS0KPiA+ICBzb3VuZC9zb2MvY29kZWNzL2FiODUwMC1jb2RlYy5jIHwgNCAr Ky0tCj4gPiAgMSBmaWxlIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkK PiA+IAo+ID4gZGlmZiAtLWdpdCBhL3NvdW5kL3NvYy9jb2RlY3MvYWI4NTAwLWNvZGVjLmMgYi9z b3VuZC9zb2MvY29kZWNzL2FiODUwMC1jb2RlYy5jCj4gPiBpbmRleCAxYWQ5MmNiLi41MWFlM2U5 IDEwMDY0NAo+ID4gLS0tIGEvc291bmQvc29jL2NvZGVjcy9hYjg1MDAtY29kZWMuYwo+ID4gKysr IGIvc291bmQvc29jL2NvZGVjcy9hYjg1MDAtY29kZWMuYwo+ID4gQEAgLTIzMjIsNyArMjMyMiw3 IEBAIHN0YXRpYyBpbnQgYWI4NTAwX2NvZGVjX3NldF9kYWlfdGRtX3Nsb3Qoc3RydWN0IHNuZF9z b2NfZGFpICpkYWksCj4gPiAgCQlzbG90ID0gZmZzKHR4X21hc2spOwo+ID4gIAkJc25kX3NvY191 cGRhdGVfYml0cyhjb2RlYywgQUI4NTAwX0RBU0xPVENPTkYxLCBtYXNrLCBzbG90KTsKPiA+ICAJ CXNuZF9zb2NfdXBkYXRlX2JpdHMoY29kZWMsIEFCODUwMF9EQVNMT1RDT05GMywgbWFzaywgc2xv dCk7Cj4gPiAtCQlzbG90ID0gZmxzKHR4X21hc2spOwo+ID4gKwkJc2xvdCA9IGZpbmRfbmV4dF9i aXQoKHVuc2lnbmVkIGxvbmcgKikmdHhfbWFzaywgMzIsIHNsb3QgKyAxKTsKPiA+ICAJCXNuZF9z b2NfdXBkYXRlX2JpdHMoY29kZWMsIEFCODUwMF9EQVNMT1RDT05GMiwgbWFzaywgc2xvdCk7Cj4g PiAgCQlzbmRfc29jX3VwZGF0ZV9iaXRzKGNvZGVjLCBBQjg1MDBfREFTTE9UQ09ORjQsIG1hc2ss IHNsb3QpOwo+ID4gIAkJYnJlYWs7Cj4gPiBAQCAtMjM2NCw3ICsyMzY0LDcgQEAgc3RhdGljIGlu dCBhYjg1MDBfY29kZWNfc2V0X2RhaV90ZG1fc2xvdChzdHJ1Y3Qgc25kX3NvY19kYWkgKmRhaSwK PiA+ICAJCQkJQUI4NTAwX0FEU0xPVFNFTChzbG90KSwKPiA+ICAJCQkJQUI4NTAwX01BU0tfU0xP VChzbG90KSwKPiA+ICAJCQkJQUI4NTAwX0FEU0xPVFNFTFhfQURfT1VUX1RPX1NMT1QoQUI4NTAw X0FEX09VVDMsIHNsb3QpKTsKPiA+IC0JCXNsb3QgPSBmbHMocnhfbWFzayk7Cj4gPiArCQlzbG90 ID0gZmluZF9uZXh0X2JpdCgodW5zaWduZWQgbG9uZyAqKSZyeF9tYXNrLCAzMiwgc2xvdCArIDEp Owo+ID4gIAkJc25kX3NvY191cGRhdGVfYml0cyhjb2RlYywKPiA+ICAJCQkJQUI4NTAwX0FEU0xP VFNFTChzbG90KSwKPiA+ICAJCQkJQUI4NTAwX01BU0tfU0xPVChzbG90KSwKCi0tIApMZWUgSm9u ZXMKTGluYXJvIFNUTWljcm9lbGVjdHJvbmljcyBMYW5kaW5nIFRlYW0gTGVhZApMaW5hcm8ub3Jn IOKUgiBPcGVuIHNvdXJjZSBzb2Z0d2FyZSBmb3IgQVJNIFNvQ3MKRm9sbG93IExpbmFybzogRmFj ZWJvb2sgfCBUd2l0dGVyIHwgQmxvZwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpBbHNhLWRldmVsIG1haWxpbmcgbGlzdApBbHNhLWRldmVsQGFsc2EtcHJv amVjdC5vcmcKaHR0cDovL21haWxtYW4uYWxzYS1wcm9qZWN0Lm9yZy9tYWlsbWFuL2xpc3RpbmZv L2Fsc2EtZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: lee.jones@linaro.org (Lee Jones) Date: Thu, 19 Dec 2013 16:48:24 +0000 Subject: [PATCH 02/11] ASoC: ab8500: Revert back to find_next_bit() In-Reply-To: References: <1387468508-12286-1-git-send-email-lee.jones@linaro.org> <1387468508-12286-3-git-send-email-lee.jones@linaro.org> Message-ID: <20131219164824.GA16145@lee--X1> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org > > Commit '166a34d ASoC: ab8500: Fix invalid cast to long pointer' > > rather carelessly converts find_next_bit() to fls() (find last > > bit set), which are not the same. > > Does it break on the real machines? It does, that's how I found the bug. > fls() behaves differently from find_next_bit(), of course, but in this > case, it should work same in the end, since there are at most two > bits. Unfortunately it doesn't work at all. This patch brings audio back to a working state. It took me the best part of a day to track down the issue. :( > > Although this implementation may not be portable, the alternative > > breaks audio on anything using the AB8500 CODEC, which is a much > > greater evil. Besides, this driver will never run on anything > > else. > > > > Cc: Takashi Iwai > > Signed-off-by: Lee Jones > > --- > > sound/soc/codecs/ab8500-codec.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c > > index 1ad92cb..51ae3e9 100644 > > --- a/sound/soc/codecs/ab8500-codec.c > > +++ b/sound/soc/codecs/ab8500-codec.c > > @@ -2322,7 +2322,7 @@ static int ab8500_codec_set_dai_tdm_slot(struct snd_soc_dai *dai, > > slot = ffs(tx_mask); > > snd_soc_update_bits(codec, AB8500_DASLOTCONF1, mask, slot); > > snd_soc_update_bits(codec, AB8500_DASLOTCONF3, mask, slot); > > - slot = fls(tx_mask); > > + slot = find_next_bit((unsigned long *)&tx_mask, 32, slot + 1); > > snd_soc_update_bits(codec, AB8500_DASLOTCONF2, mask, slot); > > snd_soc_update_bits(codec, AB8500_DASLOTCONF4, mask, slot); > > break; > > @@ -2364,7 +2364,7 @@ static int ab8500_codec_set_dai_tdm_slot(struct snd_soc_dai *dai, > > AB8500_ADSLOTSEL(slot), > > AB8500_MASK_SLOT(slot), > > AB8500_ADSLOTSELX_AD_OUT_TO_SLOT(AB8500_AD_OUT3, slot)); > > - slot = fls(rx_mask); > > + slot = find_next_bit((unsigned long *)&rx_mask, 32, slot + 1); > > snd_soc_update_bits(codec, > > AB8500_ADSLOTSEL(slot), > > AB8500_MASK_SLOT(slot), -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755906Ab3LSQsg (ORCPT ); Thu, 19 Dec 2013 11:48:36 -0500 Received: from mail-wg0-f50.google.com ([74.125.82.50]:59434 "EHLO mail-wg0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755146Ab3LSQsd (ORCPT ); Thu, 19 Dec 2013 11:48:33 -0500 Date: Thu, 19 Dec 2013 16:48:24 +0000 From: Lee Jones To: Takashi Iwai Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linus.walleij@linaro.org, broonie@kernel.org, alsa-devel@alsa-project.org Subject: Re: [PATCH 02/11] ASoC: ab8500: Revert back to find_next_bit() Message-ID: <20131219164824.GA16145@lee--X1> References: <1387468508-12286-1-git-send-email-lee.jones@linaro.org> <1387468508-12286-3-git-send-email-lee.jones@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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 > > Commit '166a34d ASoC: ab8500: Fix invalid cast to long pointer' > > rather carelessly converts find_next_bit() to fls() (find last > > bit set), which are not the same. > > Does it break on the real machines? It does, that's how I found the bug. > fls() behaves differently from find_next_bit(), of course, but in this > case, it should work same in the end, since there are at most two > bits. Unfortunately it doesn't work at all. This patch brings audio back to a working state. It took me the best part of a day to track down the issue. :( > > Although this implementation may not be portable, the alternative > > breaks audio on anything using the AB8500 CODEC, which is a much > > greater evil. Besides, this driver will never run on anything > > else. > > > > Cc: Takashi Iwai > > Signed-off-by: Lee Jones > > --- > > sound/soc/codecs/ab8500-codec.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c > > index 1ad92cb..51ae3e9 100644 > > --- a/sound/soc/codecs/ab8500-codec.c > > +++ b/sound/soc/codecs/ab8500-codec.c > > @@ -2322,7 +2322,7 @@ static int ab8500_codec_set_dai_tdm_slot(struct snd_soc_dai *dai, > > slot = ffs(tx_mask); > > snd_soc_update_bits(codec, AB8500_DASLOTCONF1, mask, slot); > > snd_soc_update_bits(codec, AB8500_DASLOTCONF3, mask, slot); > > - slot = fls(tx_mask); > > + slot = find_next_bit((unsigned long *)&tx_mask, 32, slot + 1); > > snd_soc_update_bits(codec, AB8500_DASLOTCONF2, mask, slot); > > snd_soc_update_bits(codec, AB8500_DASLOTCONF4, mask, slot); > > break; > > @@ -2364,7 +2364,7 @@ static int ab8500_codec_set_dai_tdm_slot(struct snd_soc_dai *dai, > > AB8500_ADSLOTSEL(slot), > > AB8500_MASK_SLOT(slot), > > AB8500_ADSLOTSELX_AD_OUT_TO_SLOT(AB8500_AD_OUT3, slot)); > > - slot = fls(rx_mask); > > + slot = find_next_bit((unsigned long *)&rx_mask, 32, slot + 1); > > snd_soc_update_bits(codec, > > AB8500_ADSLOTSEL(slot), > > AB8500_MASK_SLOT(slot), -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog