From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752041AbcHPSGT (ORCPT ); Tue, 16 Aug 2016 14:06:19 -0400 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:44888 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750803AbcHPSGS (ORCPT ); Tue, 16 Aug 2016 14:06:18 -0400 Date: Tue, 16 Aug 2016 18:20:06 +0100 From: Mark Brown To: John Keeping Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, Jie Yang , Bard Liao , Oder Chiou , Liam Girdwood , Jaroslav Kysela , Takashi Iwai , Ben Zhang , Dylan Reid Message-ID: <20160816172006.GW9347@sirena.org.uk> References: <20160814111823.1782-1-john@metanate.com> <20160814111823.1782-2-john@metanate.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="NUQP2JP/Zo+En7Or" Content-Disposition: inline In-Reply-To: <20160814111823.1782-2-john@metanate.com> X-Cookie: I can't drive 55. User-Agent: Mutt/1.6.0 (2016-04-01) X-SA-Exim-Connect-IP: 2a01:348:6:8808:fab::3 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [PATCH 1/2] ASoC: rt5677: Add ACPI support X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: No (on mezzanine.sirena.org.uk); Unknown failure Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --NUQP2JP/Zo+En7Or Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sun, Aug 14, 2016 at 12:18:22PM +0100, John Keeping wrote: > The Chromebook Pixel 2015 uses this codec with the ACPI ID RT5677CE, so > add an ACPI match table and support for reading properties from ACPI. This would be a lot easier to review with a concrete description of what "support for reading properties from ACPI" means and probably also split out a bit so that different things were being added separately. > +/* GPIO indexes defined by ACPI */ > +enum { > + RT5677_GPIO_PLUG_DET, > + RT5677_GPIO_MIC_PRESENT_L, > + RT5677_GPIO_HOTWORD_DET_L, > + RT5677_GPIO_DSP_INT, > + RT5677_GPIO_HP_AMP_SHDN_L, > +}; If these are an ABI you should explicitly assign the values so that they can't get remapped by future edits. If they're not an ABI I don't understand the comment. > + if (ACPI_HANDLE(dev)) { > + u32 val; > + > + if (!device_property_read_u32(dev, "DCLK", &val)) > + rt5677->pdata.dmic2_clk_pin = val; > + > + rt5677->pdata.in1_diff = device_property_read_bool(dev, "IN1"); > + rt5677->pdata.in2_diff = device_property_read_bool(dev, "IN2"); What happens if someone makes a machine which uses the DT<->ACPI mappings (especially given that this is currently undocumented)? That would not work which defeats the whole purpose of using the device property APIs. Shouldn't we be accepting either property? --NUQP2JP/Zo+En7Or Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJXs0tFAAoJECTWi3JdVIfQvXwH/1dddlrOZLW8tbq48xya5Hil 3bXIVCLTlG+Yp6HZPZXHjuvqBjrk3AKLT4ikn1VkePKT7Uf80Zu3X9mXxDVXKbCS 5uXXsLey6TfWOWqUeMdIP/1MLUS2X+Lf1vPEiS18HMhoDfoIA3Sd3T+d5BpFxiP7 uioC7bBDtYZTKT1Iho/QENI1JwjXQOERmSSymNKs74CGGBppVsgkqIQDTss/jTwr M9/KaeBNE4lC3Qi4yJiL4VxoLdKkEE8up0tUAQWttyoZbGnaTVUm7+TiQgqbrZDE vGUTqd+3AjxgQG3Jz/l2QTJ2XT1O4FR645ew7W0iSts5seEt8dSDqCniYeDNhPY= =FsiY -----END PGP SIGNATURE----- --NUQP2JP/Zo+En7Or--