From patchwork Fri Aug 25 16:42:51 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andy Shevchenko X-Patchwork-Id: 9922483 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 768EB602BD for ; Fri, 25 Aug 2017 16:43:07 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 673C6280CF for ; Fri, 25 Aug 2017 16:43:07 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5BC79283CF; Fri, 25 Aug 2017 16:43:07 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 985B9280CF for ; Fri, 25 Aug 2017 16:43:03 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id 8D808267871; Fri, 25 Aug 2017 18:43:00 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 8420F267871; Fri, 25 Aug 2017 18:42:59 +0200 (CEST) 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) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Aug 2017 09:42:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,426,1498546800"; d="scan'208"; a="1188159129" Received: from smile.fi.intel.com (HELO smile) ([10.237.72.86]) by fmsmga001.fm.intel.com with ESMTP; 25 Aug 2017 09:42:51 -0700 Message-ID: <1503679371.25945.110.camel@linux.intel.com> From: Andy Shevchenko To: John Keeping 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 X-Mailer: Evolution 3.22.6-1 Mime-Version: 1.0 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 Subject: Re: [alsa-devel] [PATCH] ASoC: rt5677: Reintroduce I2C device IDs X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" 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 X-Virus-Scanned: ClamAV using ClamSMTP 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 +   /* 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! 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);