From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Subject: Re: [PATCH] ASoC: rt5677: Reintroduce I2C device IDs Date: Fri, 25 Aug 2017 19:42:51 +0300 Message-ID: <1503679371.25945.110.camel@linux.intel.com> References: <1503453106-5564-1-git-send-email-trini@konsulko.com> <1503669407.25945.102.camel@linux.intel.com> <20170825142426.GN2827@bill-the-cat> <20170825170504.7d547aa4.john@metanate.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by alsa0.perex.cz (Postfix) with ESMTP id CCF34267870 for ; Fri, 25 Aug 2017 18:42:56 +0200 (CEST) In-Reply-To: <20170825170504.7d547aa4.john@metanate.com> 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: John Keeping Cc: Oder Chiou , Tom Rini , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, Takashi Iwai , Liam Girdwood , Mark Brown , Bard Liao , Linus Torvalds List-Id: alsa-devel@alsa-project.org T24gRnJpLCAyMDE3LTA4LTI1IGF0IDE3OjA1ICswMTAwLCBKb2huIEtlZXBpbmcgd3JvdGU6Cj4g T24gRnJpLCAyNSBBdWcgMjAxNyAxMDoyNDoyNiAtMDQwMCwgVG9tIFJpbmkgd3JvdGU6Cj4gPiBP biBGcmksIEF1ZyAyNSwgMjAxNyBhdCAwNDo1Njo0N1BNICswMzAwLCBBbmR5IFNoZXZjaGVua28g d3JvdGU6Cgo+ID4gPiBBcHBhcmVudGx5IHlvdSBhcmUgdGhlIG9uZSB3aG8gdGVzdGVkIHRoZSBj b21taXQKPiA+ID4gCTg5MTI4NTM0ZjkyNSAoIkFTb0M6IHJ0NTY3NzogQWRkIEFDUEkgc3VwcG9y dCIpCj4gPiA+IHllYXIgYWdvLsKgwqAKPiA+IAo+ID4gWWVzLgo+ID4gCj4gPiA+IFRoZSBjb21t aXQgc3RhdGVzIHRoYXQgQUNQSSBwcm9wZXJ0aWVzIHRoYXQgYXJlIHVzZWQgaW4gQ2hyb21lYm9v awo+ID4gPiBQaXhlbAo+ID4gPiAyMDE1IGlzIG5vbi1zdGFuZGFyZCAobm90IHRoZSBzYW1lIGFz IGZvciBEVCkuCj4gPiA+IAo+ID4gPiBIb3dldmVyLCBEU0RUIHNob3dzIHRoZSBvcHBvc2l0ZSHC oMKgCj4gPiAKPiA+IEludGVyZXN0aW5nLsKgwqBJJ20gbm90IGFuIEFDUEkgcGVyc29uLCBJIGp1 c3QgdGVzdGVkIHdoYXQgSm9obiBjYW1lCj4gPiB1cAo+ID4gd2l0aC4KPiAKPiBBbmQgdGhlIHBh dGNoIGFkZGluZyB0aGlzIHdhcyB0aGUgZmlyc3QgKGFuZCBzdGlsbCBvbmx5KSB0aW1lIEkndmUK PiByZWFsbHkgbG9va2VkIGF0IEFDUEksIHNvIGl0J3MgcXVpdGUgcG9zc2libGUgdGhhdCBJIG1p c3VuZGVyc3Rvb2QKPiBzb21ldGhpbmcgYXQgdGhlIHRpbWUuCgpNYXliZS4KCj4gRnJvbSBtZW1v cnksIEkgdGhpbmsgdGhlIHBhcnRpY3VsYXIgcHJvYmxlbSBJIHdhcyByZWZlcnJpbmcgdG8gaW4g dGhlCj4gY29tbWl0IG1lc3NhZ2Ugd2FzIHRoYXQgY2VydGFpbiBHUElPcyB3ZXJlIG9ubHkgZGVm aW5lZCBieSBpbmRleCBhbmQKPiBub3QKPiBieSBwcm9wZXJ0eSBuYW1lIChzcGVjaWZpY2FsbHkg InBsdWctZGV0LWdwaW9zIiwgIm1pYy1wcmVzZW50LWdwaW9zIgo+IGFuZAo+ICJoZWFkcGhvbmUt ZW5hYmxlLWdwaW9zIiksIGFuZCBoYXZpbmcgZHVtcGVkIERTRFQganVzdCBub3cgSSBkbyBub3QK PiBzZWUKPiB0aG9zZSBzdHJpbmdzIGFwcGVhcmluZyBhbnl3aGVyZS4KCkV4YWN0bHksIGFuZCB0 aGlzIHBhcnQgb2YgdGhlIHBhdGNoIEknbSBfbm90XyB0YWxraW5nIGFib3V0IChpdCdzIHByZXR0 eQptdWNoIGdvb2QgYW5kIHdvcmtpbmcpLgoKV2hhdCBJJ20gdGFsa2luZyBhYm91dCBpcyBhIHNw ZWNpZmljIGZ1bmN0aW9uIGNhbGxlZAoKcnQ1Njc3X3JlYWRfYWNwaV9wcm9wZXJ0aWVzKCkKCmlu IHRoZSBydDU2NzcuYyBjb2RlYyBkcml2ZXIuCgpUaGUgcXVlc3Rpb24gaXMgZG8gd2UgaGF2ZSBf cmVhbCBwdWJsaWNseSBhdmFpbGFibGVfIGhhcmR3YXJlIHdpdGggdGhhdApraW5kIG9mIHByb3Bl cnRpZXM/CgpCZWNhdXNlIG5vdyBpdCdzIGEgbWVzcyAod3J0IHRvIHJlYWwgRFNEVCBhdHRhY2hl ZCB0byB0aGUgdGhyZWFkKS4KClRoZSBwcm9wb3NlZCBjaGFuZ2UgdG8gZml4IHRoaXMgaXMgbGlr ZQoKZGlmZiAtLWdpdCBhL3NvdW5kL3NvYy9jb2RlY3MvcnQ1Njc3LmMgYi9zb3VuZC9zb2MvY29k ZWNzL3J0NTY3Ny5jCmluZGV4IDY0NDhiN2EwMDIwMy4uMjhiZGU1ZjUwZWQ5IDEwMDY0NAotLS0g YS9zb3VuZC9zb2MvY29kZWNzL3J0NTY3Ny5jCisrKyBiL3NvdW5kL3NvYy9jb2RlY3MvcnQ1Njc3 LmMKQEAgLTUxNDUsMjAgKzUxNDUsMTggQEAgc3RhdGljIGludCBydDU2NzdfaTJjX3Byb2JlKHN0 cnVjdCBpMmNfY2xpZW50CippMmMpCsKgCQltYXRjaF9pZCA9IG9mX21hdGNoX2RldmljZShydDU2 Nzdfb2ZfbWF0Y2gsICZpMmMtPmRldik7CsKgCQlpZiAobWF0Y2hfaWQpCsKgCQkJcnQ1Njc3LT50 eXBlID0gKGVudW0gcnQ1Njc3X3R5cGUpbWF0Y2hfaWQtCj5kYXRhOwotCi0JCXJ0NTY3N19yZWFk X2RldmljZV9wcm9wZXJ0aWVzKHJ0NTY3NywgJmkyYy0+ZGV2KTsKwqAJfSBlbHNlIGlmIChBQ1BJ X0hBTkRMRSgmaTJjLT5kZXYpKSB7CsKgCQljb25zdCBzdHJ1Y3QgYWNwaV9kZXZpY2VfaWQgKmFj cGlfaWQ7CsKgCsKgCQlhY3BpX2lkID0gYWNwaV9tYXRjaF9kZXZpY2UocnQ1Njc3X2FjcGlfbWF0 Y2gsICZpMmMtCj5kZXYpOwrCoAkJaWYgKGFjcGlfaWQpCsKgCQkJcnQ1Njc3LT50eXBlID0gKGVu dW0gcnQ1Njc3X3R5cGUpYWNwaV9pZC0KPmRyaXZlcl9kYXRhOwotCi0JCXJ0NTY3N19yZWFkX2Fj cGlfcHJvcGVydGllcyhydDU2NzcsICZpMmMtPmRldik7CsKgCX0gZWxzZSB7CsKgCQlyZXR1cm4g LUVJTlZBTDsKwqAJfQrCoAorCXJ0NTY3N19yZWFkX2RldmljZV9wcm9wZXJ0aWVzKHJ0NTY3Nywg JmkyYy0+ZGV2KTsKKwrCoAkvKiBwb3ctbGRvMiBhbmQgcmVzZXQgYXJlIG9wdGlvbmFsLiBUaGUg Y29kZWMgcGlucyBtYXkgYmUKc3RhdGljYWxseQrCoAnCoCogY29ubmVjdGVkIG9uIHRoZSBib2Fy ZCB3aXRob3V0IGdwaW9zLiBJZiB0aGUgZ3BpbyBkZXZpY2UKcHJvcGVydHkKwqAJwqAqIGlzbid0 IHNwZWNpZmllZCwgZGV2bV9ncGlvZF9nZXRfb3B0aW9uYWwgcmV0dXJucyBOVUxMLgoKKyByZW1v dmluZyBydDU2NzdfcmVhZF9hY3BpX3Byb3BlcnRpZXMoKSBjb21wbGV0ZWx5LgoKVG9tLCBpZiB5 b3UgY2FuIHRlc3QgaXQgKGJhc2ljIHRlc3QgKyBtaWdodCBiZSBxdWFsaXR5IG9mIHNvdW5kKSBv biB5b3VyCkNocm9tZWJvb2ssIGl0IHdvdWxkIGJlIG5pY2UhCgoKLS0gCkFuZHkgU2hldmNoZW5r byA8YW5kcml5LnNoZXZjaGVua29AbGludXguaW50ZWwuY29tPgpJbnRlbCBGaW5sYW5kIE95Cl9f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkFsc2EtZGV2ZWwg bWFpbGluZyBsaXN0CkFsc2EtZGV2ZWxAYWxzYS1wcm9qZWN0Lm9yZwpodHRwOi8vbWFpbG1hbi5h bHNhLXByb2plY3Qub3JnL21haWxtYW4vbGlzdGluZm8vYWxzYS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934636AbdHYQm4 (ORCPT ); Fri, 25 Aug 2017 12:42:56 -0400 Received: from mga11.intel.com ([192.55.52.93]:26366 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934474AbdHYQmz (ORCPT ); Fri, 25 Aug 2017 12:42:55 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,426,1498546800"; d="scan'208";a="1188159129" Message-ID: <1503679371.25945.110.camel@linux.intel.com> Subject: Re: [PATCH] ASoC: rt5677: Reintroduce I2C device IDs From: Andy Shevchenko To: John Keeping Cc: Tom Rini , linux-kernel@vger.kernel.org, Bard Liao , Oder Chiou , Liam Girdwood , Jaroslav Kysela , Takashi Iwai , alsa-devel@alsa-project.org, Mark Brown , Linus Torvalds Date: Fri, 25 Aug 2017 19:42:51 +0300 In-Reply-To: <20170825170504.7d547aa4.john@metanate.com> References: <1503453106-5564-1-git-send-email-trini@konsulko.com> <1503669407.25945.102.camel@linux.intel.com> <20170825142426.GN2827@bill-the-cat> <20170825170504.7d547aa4.john@metanate.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.6-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2017-08-25 at 17:05 +0100, John Keeping wrote: > On Fri, 25 Aug 2017 10:24:26 -0400, Tom Rini wrote: > > On Fri, Aug 25, 2017 at 04:56:47PM +0300, Andy Shevchenko wrote: > > > Apparently you are the one who tested the commit > > > 89128534f925 ("ASoC: rt5677: Add ACPI support") > > > year ago.   > > > > Yes. > > > > > The commit states that ACPI properties that are used in Chromebook > > > Pixel > > > 2015 is non-standard (not the same as for DT). > > > > > > However, DSDT shows the opposite!   > > > > Interesting.  I'm not an ACPI person, I just tested what John came > > up > > with. > > And the patch adding this was the first (and still only) time I've > really looked at ACPI, so it's quite possible that I misunderstood > something at the time. Maybe. > From memory, I think the particular problem I was referring to in the > commit message was that certain GPIOs were only defined by index and > not > by property name (specifically "plug-det-gpios", "mic-present-gpios" > and > "headphone-enable-gpios"), and having dumped DSDT just now I do not > see > those strings appearing anywhere. Exactly, and this part of the patch I'm _not_ talking about (it's pretty much good and working). What I'm talking about is a specific function called rt5677_read_acpi_properties() in the rt5677.c codec driver. The question is do we have _real publicly available_ hardware with that kind of properties? Because now it's a mess (wrt to real DSDT attached to the thread). The proposed change to fix this is like diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c index 6448b7a00203..28bde5f50ed9 100644 --- a/sound/soc/codecs/rt5677.c +++ b/sound/soc/codecs/rt5677.c @@ -5145,20 +5145,18 @@ static int rt5677_i2c_probe(struct i2c_client *i2c)   match_id = of_match_device(rt5677_of_match, &i2c->dev);   if (match_id)   rt5677->type = (enum rt5677_type)match_id- >data; - - rt5677_read_device_properties(rt5677, &i2c->dev);   } else if (ACPI_HANDLE(&i2c->dev)) {   const struct acpi_device_id *acpi_id;     acpi_id = acpi_match_device(rt5677_acpi_match, &i2c- >dev);   if (acpi_id)   rt5677->type = (enum rt5677_type)acpi_id- >driver_data; - - rt5677_read_acpi_properties(rt5677, &i2c->dev);   } else {   return -EINVAL;   }   + rt5677_read_device_properties(rt5677, &i2c->dev); +   /* pow-ldo2 and reset are optional. The codec pins may be statically    * connected on the board without gpios. If the gpio device property    * isn't specified, devm_gpiod_get_optional returns NULL. + removing rt5677_read_acpi_properties() completely. Tom, if you can test it (basic test + might be quality of sound) on your Chromebook, it would be nice! -- Andy Shevchenko Intel Finland Oy