diff mbox

[V5] i2c: mux: pca954x: Add ACPI support for pca954x

Message ID 1480414363-16318-1-git-send-email-tnhuynh@apm.com
State Accepted
Headers show

Commit Message

tnhuynh@apm.com Nov. 29, 2016, 10:12 a.m. UTC
This patch enables ACPI support for mux-pca954x driver.

Signed-off-by: Tin Huynh <tnhuynh@apm.com>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c |   28 +++++++++++++++++++++++++++-
 1 files changed, 27 insertions(+), 1 deletions(-)

Change from V4:
  -Add ACPI_PTR.

Change from V3:
 -Readd CONFIG_ACPI.

Change from V2:
 -Remove CONFIG_ACPI.

Change from V1 :
 -Don't shadow id variable.
 -Include sorted header.
 -Redefine acpi_device_id.
 -Add CONFIG_ACPI.

Comments

Peter Rosin Nov. 29, 2016, 10:15 a.m. UTC | #1
On 2016-11-29 11:12, Tin Huynh wrote:
> This patch enables ACPI support for mux-pca954x driver.

Looks good, thanks!

> Signed-off-by: Tin Huynh <tnhuynh@apm.com>

Acked-by: Peter Rosin <peda@axentia.se>

Cheers,
Peter

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
tnhuynh@apm.com Nov. 29, 2016, 3:12 p.m. UTC | #2
On Tue, Nov 29, 2016 at 5:15 PM, Peter Rosin <peda@axentia.se> wrote:
> On 2016-11-29 11:12, Tin Huynh wrote:
>> This patch enables ACPI support for mux-pca954x driver.
>
> Looks good, thanks!
>
>> Signed-off-by: Tin Huynh <tnhuynh@apm.com>
>
> Acked-by: Peter Rosin <peda@axentia.se>
>
> Cheers,
> Peter
>
Added Mika
However , in http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpio-pca953x.c?id=f32517bf1ae0a2de72b3f27200233bd3ad65bfeb
 log , i didn't see the CONFIG_ACPI and CONFIG_PTR.
I have been acked with the same patch to support for i2c-leds (
without CONFIG_ACPI)
About logic , if we don't enable CONFIG_ACPI , acpi_match_table will
be NULL . So , the probe functions will be not called
and nothing happens.
I though we should use V3 (https://lkml.org/lkml/2016/11/28/893)
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Rosin Nov. 29, 2016, 3:50 p.m. UTC | #3
On 2016-11-29 16:12, Tin Huynh wrote:
> On Tue, Nov 29, 2016 at 5:15 PM, Peter Rosin <peda@axentia.se> wrote:
>> On 2016-11-29 11:12, Tin Huynh wrote:
>>> This patch enables ACPI support for mux-pca954x driver.
>>
>> Looks good, thanks!
>>
>>> Signed-off-by: Tin Huynh <tnhuynh@apm.com>
>>
>> Acked-by: Peter Rosin <peda@axentia.se>
>>
>> Cheers,
>> Peter
>>
> Added Mika
> However , in http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpio-pca953x.c?id=f32517bf1ae0a2de72b3f27200233bd3ad65bfeb
>  log , i didn't see the CONFIG_ACPI and CONFIG_PTR.
> I have been acked with the same patch to support for i2c-leds (
> without CONFIG_ACPI)
> About logic , if we don't enable CONFIG_ACPI , acpi_match_table will
> be NULL . So , the probe functions will be not called
> and nothing happens.
> I though we should use V3 (https://lkml.org/lkml/2016/11/28/893)
> 

V3 will work, but you waste resources (not much, but still) for the
ACPI table even though it is never going to be used, so it's best
to #ifdef it out. (At least I think so, but maybe GCC sees that the
variable isn't really used for anything useful and manages to kill
it off? Anyway, I'm assuming that there really are resources wasted
if you don't do the #ifdef...)

But the thing is that you do not want a bunch of #ifdefs *in* the
code. It's ok to #ifdef out a variable, but not *in* the code. That
way lies madness.

So the code is going to have a call to acpi_match_device(). If
CONFIG_ACPI isn't set, that function compiles to an inline function
simply returning NULL, but the compiler still evaluates its args
and complains if the above variable is not found, which it isn't
since it's #ifdef out. So, we need to "hide" the variable if it
doesn't exist, using the ACPI_PTR macro.

For gpio-pca953x, the same reasoning can be used, but the resource
waste is even less in that case.

Cheers,
Peter

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Nov. 29, 2016, 7:08 p.m. UTC | #4
On Tue, Nov 29, 2016 at 05:12:43PM +0700, Tin Huynh wrote:
> This patch enables ACPI support for mux-pca954x driver.
> 
> Signed-off-by: Tin Huynh <tnhuynh@apm.com>

Applied to for-next, thanks!
diff mbox

Patch

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 1091346..f81f8b5 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -35,6 +35,7 @@ 
  * warranty of any kind, whether express or implied.
  */
 
+#include <linux/acpi.h>
 #include <linux/device.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
@@ -120,6 +121,21 @@  struct pca954x {
 };
 MODULE_DEVICE_TABLE(i2c, pca954x_id);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id pca954x_acpi_ids[] = {
+	{ .id = "PCA9540", .driver_data = pca_9540 },
+	{ .id = "PCA9542", .driver_data = pca_9540 },
+	{ .id = "PCA9543", .driver_data = pca_9543 },
+	{ .id = "PCA9544", .driver_data = pca_9544 },
+	{ .id = "PCA9545", .driver_data = pca_9545 },
+	{ .id = "PCA9546", .driver_data = pca_9545 },
+	{ .id = "PCA9547", .driver_data = pca_9547 },
+	{ .id = "PCA9548", .driver_data = pca_9548 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, pca954x_acpi_ids);
+#endif
+
 #ifdef CONFIG_OF
 static const struct of_device_id pca954x_of_match[] = {
 	{ .compatible = "nxp,pca9540", .data = &chips[pca_9540] },
@@ -245,8 +261,17 @@  static int pca954x_probe(struct i2c_client *client,
 	match = of_match_device(of_match_ptr(pca954x_of_match), &client->dev);
 	if (match)
 		data->chip = of_device_get_match_data(&client->dev);
-	else
+	else if (id)
 		data->chip = &chips[id->driver_data];
+	else {
+		const struct acpi_device_id *acpi_id;
+
+		acpi_id = acpi_match_device(ACPI_PTR(pca954x_acpi_ids), 
+						&client->dev);
+		if (!acpi_id)
+			return -ENODEV;
+		data->chip = &chips[acpi_id->driver_data];
+	}
 
 	data->last_chan = 0;		   /* force the first selection */
 
@@ -321,6 +346,7 @@  static int pca954x_resume(struct device *dev)
 		.name	= "pca954x",
 		.pm	= &pca954x_pm,
 		.of_match_table = of_match_ptr(pca954x_of_match),
+		.acpi_match_table = ACPI_PTR(pca954x_acpi_ids),
 	},
 	.probe		= pca954x_probe,
 	.remove		= pca954x_remove,