2016-11-29 10:13:15

by Tin Huynh

[permalink] [raw]
Subject: [PATCH V5] i2c: mux: pca954x: Add ACPI support for pca954x

This patch enables ACPI support for mux-pca954x driver.

Signed-off-by: Tin Huynh <[email protected]>
---
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.


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,
--
1.7.1


2016-11-29 11:50:10

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH V5] i2c: mux: pca954x: Add ACPI support for pca954x

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 <[email protected]>

Acked-by: Peter Rosin <[email protected]>

Cheers,
Peter

2016-11-29 15:12:50

by Tin Huynh

[permalink] [raw]
Subject: Re: [PATCH V5] i2c: mux: pca954x: Add ACPI support for pca954x

On Tue, Nov 29, 2016 at 5:15 PM, Peter Rosin <[email protected]> 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 <[email protected]>
>
> Acked-by: Peter Rosin <[email protected]>
>
> 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)

2016-11-29 17:23:41

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH V5] i2c: mux: pca954x: Add ACPI support for pca954x

On 2016-11-29 16:12, Tin Huynh wrote:
> On Tue, Nov 29, 2016 at 5:15 PM, Peter Rosin <[email protected]> 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 <[email protected]>
>>
>> Acked-by: Peter Rosin <[email protected]>
>>
>> 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

2016-11-29 19:08:40

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH V5] i2c: mux: pca954x: Add ACPI support for pca954x

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 <[email protected]>

Applied to for-next, thanks!


Attachments:
(No filename) (196.00 B)
signature.asc (819.00 B)
Download all attachments