From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [RFC v5 1/9] drivers/mfd: Add lookup table for Panel Control as GPIO signal Date: Tue, 24 Mar 2015 09:51:03 +0100 Message-ID: <20150324085102.GB17183@ulmo.nvidia.com> References: <1426177893-17945-1-git-send-email-shobhit.kumar@intel.com> <1426177893-17945-2-git-send-email-shobhit.kumar@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0723398606==" Return-path: Received: from mail-pa0-f44.google.com (mail-pa0-f44.google.com [209.85.220.44]) by gabe.freedesktop.org (Postfix) with ESMTP id 789786E68E for ; Tue, 24 Mar 2015 01:51:28 -0700 (PDT) Received: by pacwe9 with SMTP id we9so219263307pac.1 for ; Tue, 24 Mar 2015 01:51:28 -0700 (PDT) In-Reply-To: <1426177893-17945-2-git-send-email-shobhit.kumar@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Shobhit Kumar Cc: Alexandre Courbot , Samuel Ortiz , Jani Nikula , intel-gfx , Daniel Vetter , Linus Walleij List-Id: intel-gfx@lists.freedesktop.org --===============0723398606== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="s/l3CgOIzMHHjg/5" Content-Disposition: inline --s/l3CgOIzMHHjg/5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Mar 12, 2015 at 10:01:25PM +0530, Shobhit Kumar wrote: > On some Intel SoC platforms, the panel enable/disable signals are > controlled by CRC PMIC. Add those control as a new GPIO in a lookup > table for gpio-crystalcove chip during CRC driver load >=20 > CC: Samuel Ortiz > Cc: Linus Walleij > Cc: Alexandre Courbot > Cc: Thierry Reding > Signed-off-by: Shobhit Kumar > --- > drivers/mfd/intel_soc_pmic_core.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) >=20 > diff --git a/drivers/mfd/intel_soc_pmic_core.c b/drivers/mfd/intel_soc_pm= ic_core.c > index 80cef04..365d5de 100644 > --- a/drivers/mfd/intel_soc_pmic_core.c > +++ b/drivers/mfd/intel_soc_pmic_core.c > @@ -24,8 +24,19 @@ > #include > #include > #include > +#include > #include "intel_soc_pmic_core.h" > =20 > +/* Lookup table for the Panel Enable/Disable line as GPIO signals */ > +struct gpiod_lookup_table panel_gpio_table =3D { Should this be static? > + /* Intel GFX is consumer */ > + .dev_id =3D "0000:00:02.0", > + .table =3D { > + /* Panel EN/DISABLE */ > + GPIO_LOOKUP("gpio_crystalcove", 94, "panel", GPIO_ACTIVE_HIGH), > + }, > +}; > + > /* > * On some boards the PMIC interrupt may come from a GPIO line. > * Try to lookup the ACPI table and see if such connection exists. If no= t, > @@ -85,6 +96,9 @@ static int intel_soc_pmic_i2c_probe(struct i2c_client *= i2c, > if (ret) > dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret); > =20 > + /* Add lookup table binding for Panel Control to the GPIO Chip */ > + gpiod_add_lookup_table(&panel_gpio_table); > + There's no corresponding gpiod_remove_lookup_table() call anywhere. That is understandable, given that that function doesn't exist. However, this driver can be unloaded (or at least unbound from the device), at which point the data effectively becomes stale. This shouldn't pose much of a problem because the driver can't be built as a module, so the data will stick around. However, what happens if you unload and then reload the driver? You'll be adding a second instance of the GPIO table. Given that the data is identical maybe that isn't even a problem. Then again, it's probably going to mess up the global list because you're adding the same entry twice. So I think that's something we need to fix. The same is true for the PWM lookup table in a later patch. Thierry --s/l3CgOIzMHHjg/5 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJVESV0AAoJEN0jrNd/PrOhHDIP/3LbBeFn6AEooPhMskrars1u Fl0tWBytMu5FxF+5D9ifkXv+QO9BmvUtJWgLSdUmhqwc/ZtP1xoLQBRD96h7na0i cUwOlpyOZwCvQ4ATXfjoOYzBjZuH1HCIdIPUIPL8spg4Ips5cYzI/2oxQsf2ND8n txz8rvOb9SWyAKC48MiOsFXTgwn+SSvdNDDB2Y5686xLJnGJFQQ+pE1TIsKLiQYh wqTYQh2x2c6A9LYaaUMUhruajUrnjp2fbXGNeA0/d2ekyfMjyOsBdxbWzf917sws llWFsJ0oAY3EqLlozgAhS13AAK5CwM/mWDYnJ0Rmt5qFGLts2NeWL+NMsQk0WXfa oHXGPHBN2MevrNiM19ThFmtCAZy/hQa5pDCfTkPYfFvY5xEp1Yq8F7gfx6rAfy7p iFTKYWMzG2HYUfOai0dyl5bnrkQNVgQ+NolhQHTPs+uI8r+d+PMJFXNZeR72A3x5 +fRpMO+l0uhfBW1pKeEGlVNaIcuEVOc2oFaYmNWw2tsA8/iTR1ObiUQehM2owVdY Yc6nSc2qY2qFz1Tfo/yuzTd2tKWww7xBhT1gJIx/zoxIP+dcOnITS8lok8C8dY/U NnDO/4cJTvc0AgPDTx+e7UFvrNL7KZUUPtecWRZYGpC6kyofhsr62RP3Nt9OYp7u bAsEuy57OegsGsUPwfnF =FSYr -----END PGP SIGNATURE----- --s/l3CgOIzMHHjg/5-- --===============0723398606== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============0723398606==--