Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935093AbcKWKZe (ORCPT ); Wed, 23 Nov 2016 05:25:34 -0500 Received: from mga07.intel.com ([134.134.136.100]:27698 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934197AbcKWKYw (ORCPT ); Wed, 23 Nov 2016 05:24:52 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,537,1473145200"; d="scan'208";a="8365915" Date: Wed, 23 Nov 2016 12:22:36 +0200 From: Mika Westerberg To: Jacek Anaszewski Cc: tnhuynh@apm.com, Richard Purdie , linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, Loc Ho , Thang Nguyen , Phong Vo , patches@apm.com, "Rafael J. Wysocki" Subject: Re: [PATCH V1] leds: pca955x: Add ACPI support for pca955x Message-ID: <20161123102236.GO1446@lahna.fi.intel.com> References: <1479890546-22644-1-git-send-email-tnhuynh@apm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2702 Lines: 98 On Wed, Nov 23, 2016 at 11:19:21AM +0100, Jacek Anaszewski wrote: > Hi Tin, > > Thanks for the patch. While adding ACPI support please > always add ACPI maintainers on CC. > > Adding Rafael and Mika. > > Thanks, > Jacek Anaszewski > > On 11/23/2016 09:42 AM, tnhuynh@apm.com wrote: > > From: Tin Huynh > > > > This patch enables ACPI support for leds-pca955x driver. > > > > Signed-off-by: Tin Huynh > > --- > > drivers/leds/leds-pca955x.c | 26 +++++++++++++++++++++++++- > > 1 files changed, 25 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c > > index 840401a..8770090 100644 > > --- a/drivers/leds/leds-pca955x.c > > +++ b/drivers/leds/leds-pca955x.c > > @@ -40,6 +40,7 @@ > > * bits the chip supports. > > */ > > > > +#include > > #include > > #include > > #include > > @@ -100,6 +101,17 @@ struct pca955x_chipdef { > > }; > > MODULE_DEVICE_TABLE(i2c, pca955x_id); > > > > +#ifdef CONFIG_ACPI > > +static const struct acpi_device_id pca955x_acpi_ids[] = { > > + { .id = "PCA9550", .driver_data = pca9550 }, > > + { .id = "PCA9551", .driver_data = pca9551 }, > > + { .id = "PCA9552", .driver_data = pca9552 }, > > + { .id = "PCA9553", .driver_data = pca9553 }, Some like to write this { "PCA9553", pca9553 }, instead but both work. > > + { } > > +}; > > +MODULE_DEVICE_TABLE(acpi, pca955x_acpi_ids); > > +#endif > > + > > struct pca955x { > > struct mutex lock; > > struct pca955x_led *leds; > > @@ -250,7 +262,18 @@ static int pca955x_probe(struct i2c_client *client, > > struct led_platform_data *pdata; > > int i, err; > > > > - chip = &pca955x_chipdefs[id->driver_data]; > > + if (id) > > + chip = &pca955x_chipdefs[id->driver_data]; > > +#ifdef CONFIG_ACPI You should not need these #ifdefs as acpi_match_device() should be stubbed out when !CONFIG_ACPI. Otherwise look good to me. > > + else { > > + const struct acpi_device_id *acpi_id; > > + > > + acpi_id = acpi_match_device(pca955x_acpi_ids, &client->dev); > > + if (!acpi_id) > > + return -ENODEV; > > + chip = &pca955x_chipdefs[acpi_id->driver_data]; > > + } > > +#endif > > adapter = to_i2c_adapter(client->dev.parent); > > pdata = dev_get_platdata(&client->dev); > > > > @@ -358,6 +381,7 @@ static int pca955x_remove(struct i2c_client *client) > > static struct i2c_driver pca955x_driver = { > > .driver = { > > .name = "leds-pca955x", > > + .acpi_match_table = ACPI_PTR(pca955x_acpi_ids), > > }, > > .probe = pca955x_probe, > > .remove = pca955x_remove, > > > > > -- > Best regards, > Jacek Anaszewski