2024-02-28 21:28:08

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/1] iio: light: isl29018: Drop ACPI specifics from the code

Convert the module to be property provider agnostic. The ACPI
APIs are not required and can be replaced by respective agnostic
ones.

Include mod_devicetable.h explicitly to replace the dropped of.h
which included mod_devicetable.h indirectly.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/iio/light/isl29018.c | 44 +++++++++++++-----------------------
1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
index 43484c18b101..fe7b9ad7ff35 100644
--- a/drivers/iio/light/isl29018.c
+++ b/drivers/iio/light/isl29018.c
@@ -8,17 +8,19 @@
* Copyright (c) 2010, NVIDIA Corporation.
*/

-#include <linux/module.h>
#include <linux/i2c.h>
#include <linux/err.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/delay.h>
+#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
+
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
-#include <linux/acpi.h>

#define ISL29018_CONV_TIME_MS 100

@@ -523,9 +525,9 @@ static const struct attribute_group isl29023_group = {
};

enum {
- isl29018,
- isl29023,
- isl29035,
+ isl29018 = 1,
+ isl29023 = 2,
+ isl29035 = 3,
};

static int isl29018_chip_init(struct isl29018_chip *chip)
@@ -687,20 +689,6 @@ static const struct isl29018_chip_info isl29018_chip_info_tbl[] = {
},
};

-static const char *isl29018_match_acpi_device(struct device *dev, int *data)
-{
- const struct acpi_device_id *id;
-
- id = acpi_match_device(dev->driver->acpi_match_table, dev);
-
- if (!id)
- return NULL;
-
- *data = (int)id->driver_data;
-
- return dev_name(dev);
-}
-
static void isl29018_disable_regulator_action(void *_data)
{
struct isl29018_chip *chip = _data;
@@ -718,6 +706,7 @@ static int isl29018_probe(struct i2c_client *client)
struct iio_dev *indio_dev;
int err;
const char *name = NULL;
+ const void *match;
int dev_id = 0;

indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
@@ -728,14 +717,15 @@ static int isl29018_probe(struct i2c_client *client)

i2c_set_clientdata(client, indio_dev);

- if (id) {
+ match = device_get_match_data(dev);
+ if (match) {
+ name = dev_name(dev);
+ dev_id = (int)(uintptr_t)match;
+ } else if (id) {
name = id->name;
dev_id = id->driver_data;
}

- if (ACPI_HANDLE(&client->dev))
- name = isl29018_match_acpi_device(&client->dev, &dev_id);
-
mutex_init(&chip->lock);

chip->type = dev_id;
@@ -832,15 +822,13 @@ static int isl29018_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend,
isl29018_resume);

-#ifdef CONFIG_ACPI
static const struct acpi_device_id isl29018_acpi_match[] = {
{"ISL29018", isl29018},
{"ISL29023", isl29023},
{"ISL29035", isl29035},
- {},
+ {}
};
MODULE_DEVICE_TABLE(acpi, isl29018_acpi_match);
-#endif

static const struct i2c_device_id isl29018_id[] = {
{"isl29018", isl29018},
@@ -854,14 +842,14 @@ static const struct of_device_id isl29018_of_match[] = {
{ .compatible = "isil,isl29018", },
{ .compatible = "isil,isl29023", },
{ .compatible = "isil,isl29035", },
- { },
+ {}
};
MODULE_DEVICE_TABLE(of, isl29018_of_match);

static struct i2c_driver isl29018_driver = {
.driver = {
.name = "isl29018",
- .acpi_match_table = ACPI_PTR(isl29018_acpi_match),
+ .acpi_match_table = isl29018_acpi_match,
.pm = pm_sleep_ptr(&isl29018_pm_ops),
.of_match_table = isl29018_of_match,
},
--
2.43.0.rc1.1.gbec44491f096



2024-03-03 13:58:19

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: light: isl29018: Drop ACPI specifics from the code

On Wed, 28 Feb 2024 23:27:46 +0200
Andy Shevchenko <[email protected]> wrote:

> Convert the module to be property provider agnostic. The ACPI
> APIs are not required and can be replaced by respective agnostic
> ones.
>
> Include mod_devicetable.h explicitly to replace the dropped of.h
> which included mod_devicetable.h indirectly.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/iio/light/isl29018.c | 44 +++++++++++++-----------------------
> 1 file changed, 16 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
> index 43484c18b101..fe7b9ad7ff35 100644
> --- a/drivers/iio/light/isl29018.c
> +++ b/drivers/iio/light/isl29018.c
> @@ -8,17 +8,19 @@
> * Copyright (c) 2010, NVIDIA Corporation.
> */
>
> -#include <linux/module.h>
> #include <linux/i2c.h>
> #include <linux/err.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/delay.h>
> +#include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> +
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> -#include <linux/acpi.h>
>
> #define ISL29018_CONV_TIME_MS 100
>
> @@ -523,9 +525,9 @@ static const struct attribute_group isl29023_group = {
> };
>
> enum {
> - isl29018,
> - isl29023,
> - isl29035,
> + isl29018 = 1,
> + isl29023 = 2,
> + isl29035 = 3,

I really don't like doing this non zero trick (I did once let a few instances of
this trick in and should get round to cleaning them up properly...)

I'd rather see a trivial chip_info structure with the names in it and pointers
in each of the fw match tables + use of i2c_get_match_data() to deal with
all the options.

+ get rid of the type / dev_id usage infavor of everything in a chip_info
structure. I think that is just the intergration times arrays.

Obviously bit mean to ask you to do this larger refactor, but I'm not going
to take this avoidance of enum == 0 in the meantime.

Jonathan



> };
>
> static int isl29018_chip_init(struct isl29018_chip *chip)
> @@ -687,20 +689,6 @@ static const struct isl29018_chip_info isl29018_chip_info_tbl[] = {
> },
> };
>
> -static const char *isl29018_match_acpi_device(struct device *dev, int *data)
> -{
> - const struct acpi_device_id *id;
> -
> - id = acpi_match_device(dev->driver->acpi_match_table, dev);
> -
> - if (!id)
> - return NULL;
> -
> - *data = (int)id->driver_data;
> -
> - return dev_name(dev);
> -}
> -
> static void isl29018_disable_regulator_action(void *_data)
> {
> struct isl29018_chip *chip = _data;
> @@ -718,6 +706,7 @@ static int isl29018_probe(struct i2c_client *client)
> struct iio_dev *indio_dev;
> int err;
> const char *name = NULL;
> + const void *match;
> int dev_id = 0;
>
> indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> @@ -728,14 +717,15 @@ static int isl29018_probe(struct i2c_client *client)
>
> i2c_set_clientdata(client, indio_dev);
>
> - if (id) {
> + match = device_get_match_data(dev);
> + if (match) {
> + name = dev_name(dev);
> + dev_id = (int)(uintptr_t)match;
> + } else if (id) {
> name = id->name;
> dev_id = id->driver_data;
> }
>
> - if (ACPI_HANDLE(&client->dev))
> - name = isl29018_match_acpi_device(&client->dev, &dev_id);
> -
> mutex_init(&chip->lock);
>
> chip->type = dev_id;
> @@ -832,15 +822,13 @@ static int isl29018_resume(struct device *dev)
> static DEFINE_SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend,
> isl29018_resume);
>
> -#ifdef CONFIG_ACPI
> static const struct acpi_device_id isl29018_acpi_match[] = {
> {"ISL29018", isl29018},
> {"ISL29023", isl29023},
> {"ISL29035", isl29035},
> - {},
> + {}
> };
> MODULE_DEVICE_TABLE(acpi, isl29018_acpi_match);
> -#endif
>
> static const struct i2c_device_id isl29018_id[] = {
> {"isl29018", isl29018},
> @@ -854,14 +842,14 @@ static const struct of_device_id isl29018_of_match[] = {
> { .compatible = "isil,isl29018", },
> { .compatible = "isil,isl29023", },
> { .compatible = "isil,isl29035", },
> - { },
> + {}
> };
> MODULE_DEVICE_TABLE(of, isl29018_of_match);
>
> static struct i2c_driver isl29018_driver = {
> .driver = {
> .name = "isl29018",
> - .acpi_match_table = ACPI_PTR(isl29018_acpi_match),
> + .acpi_match_table = isl29018_acpi_match,
> .pm = pm_sleep_ptr(&isl29018_pm_ops),
> .of_match_table = isl29018_of_match,
> },