[PATCH v3 00/24] iio: Clean up acpi_match_device() use cases

[PATCH v3 00/24] iio: Clean up acpi_match_device() use cases

[Andy Shevchenko]

There are current uses of acpi_match_device():
- as strange way of checking if the device was enumerated via ACPI
- as a way to get IIO device name as ACPI device instance name
- as above with accompanying driver data

Deduplicate its use by providing two new helper functions in IIO ACPI
library and update the rest accordingly.

This also includes a rework of previously sent ltr501 patch.

Besides that there ie a big clean up for the kxcjk-1013 driver, started
with the revert of the one patch discussed earlier today. Feel free to
route that one via fixes branch of your tree.

In v3:
- collected tags (Marius)
- added note to the documentation about usage of new API (Jonathan)
- added a handful patches for kxcjk-1013 driver

In v2:
- collected tags (Hans, Jean-Baptiste)
- updated SoB chain in patch 4

Andy Shevchenko (24):
  iio: magnetometer: bmc150: Drop dead code from the driver
  iio: adc: pac1934: Replace strange way of checking type of enumeration
  iio: imu: inv_mpu6050: Replace strange way of checking type of
    enumeration
  iio: acpi: Improve iio_read_acpi_mount_matrix()
  iio: acpi: Add iio_get_acpi_device_name_and_data() helper function
  iio: accel: kxcjk-1013: Remove redundant I²C ID
  iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"
  iio: accel: kxcjk-1013: Switch from CONFIG_PM guards to pm_ptr() etc
  iio: accel: kxcjk-1013: Use local variable for regs
  iio: accel: kxcjk-1013: Rename kxcjk1013_info
  iio: accel: kxcjk-1013: Start using chip_info variables instead of
    enum
  iio: accel: kxcjk-1013: Move odr_start_up_times up in the code
  iio: accel: kxcjk-1013: Convert ODR times array to variable in
    chip_info
  iio: accel: kxcjk-1013: Get rid of enum kx_chipset
  iio: accel: kxcjk-1013: Replace a variant of
    iio_get_acpi_device_name_and_data()
  iio: accel: kxcjk-1013: drop ACPI_PTR() and move ID out of CONFIG_ACPI
    guards
  iio: accel: mma9551: Replace custom implementation of
    iio_get_acpi_device_name()
  iio: accel: mma9553: Replace custom implementation of
    iio_get_acpi_device_name()
  iio: gyro: bmg160: Replace custom implementation of
    iio_get_acpi_device_name()
  iio: light: isl29018: Replace a variant of
    iio_get_acpi_device_name_and_data()
  iio: light: isl29018: drop ACPI_PTR() and CONFIG_ACPI guards
  iio: light: ltr501: Drop most likely fake ACPI IDs
  iio: light: ltr501: Add LTER0303 to the supported devices
  iio: light: ltr501: Replace a variant of
    iio_get_acpi_device_name_and_data()

 drivers/iio/accel/kxcjk-1013.c             | 449 +++++++++++----------
 drivers/iio/accel/mma9551.c                |  19 +-
 drivers/iio/accel/mma9553.c                |  19 +-
 drivers/iio/adc/pac1934.c                  |   2 +-
 drivers/iio/gyro/bmg160_core.c             |  15 -
 drivers/iio/gyro/bmg160_i2c.c              |   4 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c |   5 +-
 drivers/iio/industrialio-acpi.c            |  48 ++-
 drivers/iio/light/isl29018.c               |  38 +-
 drivers/iio/light/ltr501.c                 |  29 +-
 drivers/iio/magnetometer/bmc150_magn.c     |  15 -
 include/linux/iio/iio.h                    |  10 +
 12 files changed, 315 insertions(+), 338 deletions(-)

-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v3 01/24] iio: magnetometer: bmc150: Drop dead code from the driver

[Andy Shevchenko]

Since there is no ACPI IDs for this driver to be served for,
drop dead ACPI bits from it completely.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/magnetometer/bmc150_magn.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index 06d5a1ef1fbd..7de18c4a0ccb 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -14,7 +14,6 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
-#include <linux/acpi.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/iio/iio.h>
@@ -855,17 +854,6 @@ static const struct iio_buffer_setup_ops bmc150_magn_buffer_setup_ops = {
 	.postdisable = bmc150_magn_buffer_postdisable,
 };
 
-static const char *bmc150_magn_match_acpi_device(struct device *dev)
-{
-	const struct acpi_device_id *id;
-
-	id = acpi_match_device(dev->driver->acpi_match_table, dev);
-	if (!id)
-		return NULL;
-
-	return dev_name(dev);
-}
-
 int bmc150_magn_probe(struct device *dev, struct regmap *regmap,
 		      int irq, const char *name)
 {
@@ -894,9 +882,6 @@ int bmc150_magn_probe(struct device *dev, struct regmap *regmap,
 	if (ret)
 		return ret;
 
-	if (!name && ACPI_HANDLE(dev))
-		name = bmc150_magn_match_acpi_device(dev);
-
 	mutex_init(&data->mutex);
 
 	ret = bmc150_magn_init(data);
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 01/24] iio: magnetometer: bmc150: Drop dead code from the driver

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:04:50 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Since there is no ACPI IDs for this driver to be served for,
> drop dead ACPI bits from it completely.
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Given lots of somewhat independent stuff in this series, I'm going to review
an pick them up one by one.

Applied this one.

Thanks,

Jonathan

[PATCH v3 02/24] iio: adc: pac1934: Replace strange way of checking type of enumeration

[Andy Shevchenko]

When device is enumerated via ACPI the respective device node is of
ACPI device type. Use that to check for ACPI enumeration, rather than
calling for full match which is O(n) vs. O(1) for the regular check.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Marius Cristea <marius.cristea@microchip.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/adc/pac1934.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/pac1934.c b/drivers/iio/adc/pac1934.c
index 7ef249d83286..20802b7f49ea 100644
--- a/drivers/iio/adc/pac1934.c
+++ b/drivers/iio/adc/pac1934.c
@@ -1507,7 +1507,7 @@ static int pac1934_probe(struct i2c_client *client)
 		indio_dev->name = pac1934_chip_config[ret].name;
 	}
 
-	if (acpi_match_device(dev->driver->acpi_match_table, dev))
+	if (is_acpi_device_node(dev_fwnode(dev)))
 		ret = pac1934_acpi_parse_channel_config(client, info);
 	else
 		/*
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 02/24] iio: adc: pac1934: Replace strange way of checking type of enumeration

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:04:51 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> When device is enumerated via ACPI the respective device node is of
> ACPI device type. Use that to check for ACPI enumeration, rather than
> calling for full match which is O(n) vs. O(1) for the regular check.
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Marius Cristea <marius.cristea@microchip.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied
> ---
>  drivers/iio/adc/pac1934.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/pac1934.c b/drivers/iio/adc/pac1934.c
> index 7ef249d83286..20802b7f49ea 100644
> --- a/drivers/iio/adc/pac1934.c
> +++ b/drivers/iio/adc/pac1934.c
> @@ -1507,7 +1507,7 @@ static int pac1934_probe(struct i2c_client *client)
>  		indio_dev->name = pac1934_chip_config[ret].name;
>  	}
>  
> -	if (acpi_match_device(dev->driver->acpi_match_table, dev))
> +	if (is_acpi_device_node(dev_fwnode(dev)))
>  		ret = pac1934_acpi_parse_channel_config(client, info);
>  	else
>  		/*

[PATCH v3 03/24] iio: imu: inv_mpu6050: Replace strange way of checking type of enumeration

[Andy Shevchenko]

When device is enumerated via ACPI the respective device node is of
ACPI device type. Use that to check for ACPI enumeration, rather than
calling for full match which is O(n) vs. O(1) for the regular check.

Acked-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
index b15d8c94cc11..373e59f6d91a 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
@@ -104,14 +104,11 @@ static int inv_mpu_process_acpi_config(struct i2c_client *client,
 				       unsigned short *secondary_addr)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
-	const struct acpi_device_id *id;
 	u32 i2c_addr = 0;
 	LIST_HEAD(resources);
 	int ret;
 
-	id = acpi_match_device(client->dev.driver->acpi_match_table,
-			       &client->dev);
-	if (!id)
+	if (!is_acpi_device_node(dev_fwnode(&client->dev)))
 		return -ENODEV;
 
 	ret = acpi_dev_get_resources(adev, &resources,
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 03/24] iio: imu: inv_mpu6050: Replace strange way of checking type of enumeration

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:04:52 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> When device is enumerated via ACPI the respective device node is of
> ACPI device type. Use that to check for ACPI enumeration, rather than
> calling for full match which is O(n) vs. O(1) for the regular check.
> 
> Acked-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied.

[PATCH v3 04/24] iio: acpi: Improve iio_read_acpi_mount_matrix()

[Andy Shevchenko]

From: Andy Shevchenko <andy.shevchenko@gmail.com>

By using ACPI_HANDLE() the handler argument can be retrieved directly.
Replace ACPI_COMPANION() + dereference with ACPI_HANDLE().

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/industrialio-acpi.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/industrialio-acpi.c b/drivers/iio/industrialio-acpi.c
index 981b75d40780..1e46908f9534 100644
--- a/drivers/iio/industrialio-acpi.c
+++ b/drivers/iio/industrialio-acpi.c
@@ -28,17 +28,21 @@ bool iio_read_acpi_mount_matrix(struct device *dev,
 				char *acpi_method)
 {
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	struct acpi_device *adev = ACPI_COMPANION(dev);
 	char *str;
 	union acpi_object *obj, *elements;
+	acpi_handle handle;
 	acpi_status status;
 	int i, j, val[3];
 	bool ret = false;
 
-	if (!adev || !acpi_has_method(adev->handle, acpi_method))
+	handle = ACPI_HANDLE(dev);
+	if (!handle)
 		return false;
 
-	status = acpi_evaluate_object(adev->handle, acpi_method, NULL, &buffer);
+	if (!acpi_has_method(handle, acpi_method))
+		return false;
+
+	status = acpi_evaluate_object(handle, acpi_method, NULL, &buffer);
 	if (ACPI_FAILURE(status)) {
 		dev_err(dev, "Failed to get ACPI mount matrix: %d\n", status);
 		return false;
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 04/24] iio: acpi: Improve iio_read_acpi_mount_matrix()

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:04:53 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> By using ACPI_HANDLE() the handler argument can be retrieved directly.
> Replace ACPI_COMPANION() + dereference with ACPI_HANDLE().
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
There are two of you?  That explains so many things ;)

Anyhow, whilst a little unusual fair enough that you sent this with different
hats on.

Applied.


> ---
>  drivers/iio/industrialio-acpi.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-acpi.c b/drivers/iio/industrialio-acpi.c
> index 981b75d40780..1e46908f9534 100644
> --- a/drivers/iio/industrialio-acpi.c
> +++ b/drivers/iio/industrialio-acpi.c
> @@ -28,17 +28,21 @@ bool iio_read_acpi_mount_matrix(struct device *dev,
>  				char *acpi_method)
>  {
>  	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> -	struct acpi_device *adev = ACPI_COMPANION(dev);
>  	char *str;
>  	union acpi_object *obj, *elements;
> +	acpi_handle handle;
>  	acpi_status status;
>  	int i, j, val[3];
>  	bool ret = false;
>  
> -	if (!adev || !acpi_has_method(adev->handle, acpi_method))
> +	handle = ACPI_HANDLE(dev);
> +	if (!handle)
>  		return false;
>  
> -	status = acpi_evaluate_object(adev->handle, acpi_method, NULL, &buffer);
> +	if (!acpi_has_method(handle, acpi_method))
> +		return false;
> +
> +	status = acpi_evaluate_object(handle, acpi_method, NULL, &buffer);
>  	if (ACPI_FAILURE(status)) {
>  		dev_err(dev, "Failed to get ACPI mount matrix: %d\n", status);
>  		return false;

[PATCH v3 05/24] iio: acpi: Add iio_get_acpi_device_name_and_data() helper function

[Andy Shevchenko]

A few drivers duplicate the code to retrieve ACPI device instance name.
Some of them want an associated driver data as well.

In order of deduplication introduce the common helper functions.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/industrialio-acpi.c | 38 ++++++++++++++++++++++++++++++++-
 include/linux/iio/iio.h         | 10 +++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-acpi.c b/drivers/iio/industrialio-acpi.c
index 1e46908f9534..d67a43843799 100644
--- a/drivers/iio/industrialio-acpi.c
+++ b/drivers/iio/industrialio-acpi.c
@@ -2,7 +2,8 @@
 /* IIO ACPI helper functions */
 
 #include <linux/acpi.h>
-#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/export.h>
 #include <linux/iio/iio.h>
 #include <linux/sprintf.h>
 
@@ -87,3 +88,38 @@ bool iio_read_acpi_mount_matrix(struct device *dev,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(iio_read_acpi_mount_matrix);
+
+/**
+ * iio_get_acpi_device_name_and_data() - Return ACPI device instance name and driver data
+ * @dev:		Device structure
+ * @data:		Optional pointer to return driver data
+ *
+ * When device was enumerated by ACPI ID matching, the user might
+ * want to set description for the physical chip. In such cases
+ * the ACPI device instance name might be used. This call may be
+ * performed to retrieve this information.
+ *
+ * NOTE: This helper function exists only for backward compatibility,
+ * do not use in a new code!
+ *
+ * Returns: ACPI device instance name or %NULL.
+ */
+const char *iio_get_acpi_device_name_and_data(struct device *dev, const void **data)
+{
+	const struct acpi_device_id *id;
+	acpi_handle handle;
+
+	handle = ACPI_HANDLE(dev);
+	if (!handle)
+		return NULL;
+
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
+	if (!id)
+		return NULL;
+
+	if (data)
+		*data = (const void *)id->driver_data;
+
+	return dev_name(dev);
+}
+EXPORT_SYMBOL_GPL(iio_get_acpi_device_name_and_data);
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 3a9b57187a95..445d6666a291 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -831,6 +831,7 @@ int iio_device_resume_triggering(struct iio_dev *indio_dev);
 bool iio_read_acpi_mount_matrix(struct device *dev,
 				struct iio_mount_matrix *orientation,
 				char *acpi_method);
+const char *iio_get_acpi_device_name_and_data(struct device *dev, const void **data);
 #else
 static inline bool iio_read_acpi_mount_matrix(struct device *dev,
 					      struct iio_mount_matrix *orientation,
@@ -838,7 +839,16 @@ static inline bool iio_read_acpi_mount_matrix(struct device *dev,
 {
 	return false;
 }
+static inline const char *
+iio_get_acpi_device_name_and_data(struct device *dev, const void **data)
+{
+	return NULL;
+}
 #endif
+static inline const char *iio_get_acpi_device_name(struct device *dev)
+{
+	return iio_get_acpi_device_name_and_data(dev, NULL);
+}
 
 /**
  * iio_get_current_scan_type - Get the current scan type for a channel
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 05/24] iio: acpi: Add iio_get_acpi_device_name_and_data() helper function

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:04:54 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> A few drivers duplicate the code to retrieve ACPI device instance name.
> Some of them want an associated driver data as well.
> 
> In order of deduplication introduce the common helper functions.
It's a weird function, but clearly useful to reduce duplication of
weird code.  So fair enough.  Thanks for adding the do not use message.
Applied.

> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/iio/industrialio-acpi.c | 38 ++++++++++++++++++++++++++++++++-
>  include/linux/iio/iio.h         | 10 +++++++++
>  2 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-acpi.c b/drivers/iio/industrialio-acpi.c
> index 1e46908f9534..d67a43843799 100644
> --- a/drivers/iio/industrialio-acpi.c
> +++ b/drivers/iio/industrialio-acpi.c
> @@ -2,7 +2,8 @@
>  /* IIO ACPI helper functions */
>  
>  #include <linux/acpi.h>
> -#include <linux/dev_printk.h>
> +#include <linux/device.h>
> +#include <linux/export.h>
>  #include <linux/iio/iio.h>
>  #include <linux/sprintf.h>
>  
> @@ -87,3 +88,38 @@ bool iio_read_acpi_mount_matrix(struct device *dev,
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(iio_read_acpi_mount_matrix);
> +
> +/**
> + * iio_get_acpi_device_name_and_data() - Return ACPI device instance name and driver data
> + * @dev:		Device structure
> + * @data:		Optional pointer to return driver data
> + *
> + * When device was enumerated by ACPI ID matching, the user might
> + * want to set description for the physical chip. In such cases
> + * the ACPI device instance name might be used. This call may be
> + * performed to retrieve this information.
> + *
> + * NOTE: This helper function exists only for backward compatibility,
> + * do not use in a new code!
> + *
> + * Returns: ACPI device instance name or %NULL.
> + */
> +const char *iio_get_acpi_device_name_and_data(struct device *dev, const void **data)
> +{
> +	const struct acpi_device_id *id;
> +	acpi_handle handle;
> +
> +	handle = ACPI_HANDLE(dev);
> +	if (!handle)
> +		return NULL;
> +
> +	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> +	if (!id)
> +		return NULL;
> +
> +	if (data)
> +		*data = (const void *)id->driver_data;
> +
> +	return dev_name(dev);
> +}
> +EXPORT_SYMBOL_GPL(iio_get_acpi_device_name_and_data);
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 3a9b57187a95..445d6666a291 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -831,6 +831,7 @@ int iio_device_resume_triggering(struct iio_dev *indio_dev);
>  bool iio_read_acpi_mount_matrix(struct device *dev,
>  				struct iio_mount_matrix *orientation,
>  				char *acpi_method);
> +const char *iio_get_acpi_device_name_and_data(struct device *dev, const void **data);
>  #else
>  static inline bool iio_read_acpi_mount_matrix(struct device *dev,
>  					      struct iio_mount_matrix *orientation,
> @@ -838,7 +839,16 @@ static inline bool iio_read_acpi_mount_matrix(struct device *dev,
>  {
>  	return false;
>  }
> +static inline const char *
> +iio_get_acpi_device_name_and_data(struct device *dev, const void **data)
> +{
> +	return NULL;
> +}
>  #endif
> +static inline const char *iio_get_acpi_device_name(struct device *dev)
> +{
> +	return iio_get_acpi_device_name_and_data(dev, NULL);
> +}
>  
>  /**
>   * iio_get_current_scan_type - Get the current scan type for a channel

[PATCH v3 06/24] iio: accel: kxcjk-1013: Remove redundant I²C ID

[Andy Shevchenko]

The ACPI IDs are defined in the respective ID tables. Puting them
to the I²C ID legacy table has no meaning. Remove that ID.

Fixes: 3bfa74f86006 ("iio:kxcjk-1013: Add support for SMO8500 device")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index bbf65fc97b08..2eec95d8defb 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1717,7 +1717,6 @@ static const struct i2c_device_id kxcjk1013_id[] = {
 	{"kxtf9",     KXTF9},
 	{"kx022-1020", KX0221020},
 	{"kx023-1025", KX0231025},
-	{"SMO8500",   KXCJ91008},
 	{}
 };
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 06/24] iio: accel: kxcjk-1013: Remove redundant I²C ID

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:04:55 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> The ACPI IDs are defined in the respective ID tables. Puting them
> to the I²C ID legacy table has no meaning. Remove that ID.
> 
> Fixes: 3bfa74f86006 ("iio:kxcjk-1013: Add support for SMO8500 device")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied.
> ---
>  drivers/iio/accel/kxcjk-1013.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index bbf65fc97b08..2eec95d8defb 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -1717,7 +1717,6 @@ static const struct i2c_device_id kxcjk1013_id[] = {
>  	{"kxtf9",     KXTF9},
>  	{"kx022-1020", KX0221020},
>  	{"kx023-1025", KX0231025},
> -	{"SMO8500",   KXCJ91008},
>  	{}
>  };
>  

[PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"

[Andy Shevchenko]

The mentioned change effectively broke the ODR startup timeouts
settungs for KX023-1025 case. Let's revert it for now and see
how we can handle it with the better approach after switching
the driver to use data structure instead of enum.

This reverts commit d5cbe1502043124ff8af8136b80f93758c4a61e0.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 2eec95d8defb..208e701e1aed 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -174,7 +174,6 @@ enum kx_chipset {
 	KXCJ91008,
 	KXTJ21009,
 	KXTF9,
-	KX0221020,
 	KX0231025,
 	KX_MAX_CHIPS /* this must be last */
 };
@@ -582,8 +581,8 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 		return ret;
 	}
 
-	/* On KX023 and KX022, route all used interrupts to INT1 for now */
-	if ((data->chipset == KX0231025 || data->chipset == KX0221020) && data->client->irq > 0) {
+	/* On KX023, route all used interrupts to INT1 for now */
+	if (data->chipset == KX0231025 && data->client->irq > 0) {
 		ret = i2c_smbus_write_byte_data(data->client, KX023_REG_INC4,
 						KX023_REG_INC4_DRDY1 |
 						KX023_REG_INC4_WUFI1);
@@ -1509,7 +1508,6 @@ static int kxcjk1013_probe(struct i2c_client *client)
 	case KXTF9:
 		data->regs = &kxtf9_regs;
 		break;
-	case KX0221020:
 	case KX0231025:
 		data->regs = &kx0231025_regs;
 		break;
@@ -1715,7 +1713,6 @@ static const struct i2c_device_id kxcjk1013_id[] = {
 	{"kxcj91008", KXCJ91008},
 	{"kxtj21009", KXTJ21009},
 	{"kxtf9",     KXTF9},
-	{"kx022-1020", KX0221020},
 	{"kx023-1025", KX0231025},
 	{}
 };
@@ -1727,7 +1724,6 @@ static const struct of_device_id kxcjk1013_of_match[] = {
 	{ .compatible = "kionix,kxcj91008", },
 	{ .compatible = "kionix,kxtj21009", },
 	{ .compatible = "kionix,kxtf9", },
-	{ .compatible = "kionix,kx022-1020", },
 	{ .compatible = "kionix,kx023-1025", },
 	{ }
 };
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:04:56 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> The mentioned change effectively broke the ODR startup timeouts
> settungs for KX023-1025 case. Let's revert it for now and see
> how we can handle it with the better approach after switching
> the driver to use data structure instead of enum.
> 
> This reverts commit d5cbe1502043124ff8af8136b80f93758c4a61e0.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
I'll take this the slow way as I don't think there is time to chase the revert
through the various trees and still get the dependent patches in.
Hopefully we will fairly quickly get the missing table data and can
bring this back again.

For now, applied to the togreg branch of iio.git.  
I have tagged it as a fix though. and +CC Rayyan
(I'm guessing maybe that will bounce as you rarely miss people you should
CC!)

Jonathan

> ---
>  drivers/iio/accel/kxcjk-1013.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 2eec95d8defb..208e701e1aed 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -174,7 +174,6 @@ enum kx_chipset {
>  	KXCJ91008,
>  	KXTJ21009,
>  	KXTF9,
> -	KX0221020,
>  	KX0231025,
>  	KX_MAX_CHIPS /* this must be last */
>  };
> @@ -582,8 +581,8 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  		return ret;
>  	}
>  
> -	/* On KX023 and KX022, route all used interrupts to INT1 for now */
> -	if ((data->chipset == KX0231025 || data->chipset == KX0221020) && data->client->irq > 0) {
> +	/* On KX023, route all used interrupts to INT1 for now */
> +	if (data->chipset == KX0231025 && data->client->irq > 0) {
>  		ret = i2c_smbus_write_byte_data(data->client, KX023_REG_INC4,
>  						KX023_REG_INC4_DRDY1 |
>  						KX023_REG_INC4_WUFI1);
> @@ -1509,7 +1508,6 @@ static int kxcjk1013_probe(struct i2c_client *client)
>  	case KXTF9:
>  		data->regs = &kxtf9_regs;
>  		break;
> -	case KX0221020:
>  	case KX0231025:
>  		data->regs = &kx0231025_regs;
>  		break;
> @@ -1715,7 +1713,6 @@ static const struct i2c_device_id kxcjk1013_id[] = {
>  	{"kxcj91008", KXCJ91008},
>  	{"kxtj21009", KXTJ21009},
>  	{"kxtf9",     KXTF9},
> -	{"kx022-1020", KX0221020},
>  	{"kx023-1025", KX0231025},
>  	{}
>  };
> @@ -1727,7 +1724,6 @@ static const struct of_device_id kxcjk1013_of_match[] = {
>  	{ .compatible = "kionix,kxcj91008", },
>  	{ .compatible = "kionix,kxtj21009", },
>  	{ .compatible = "kionix,kxtf9", },
> -	{ .compatible = "kionix,kx022-1020", },
>  	{ .compatible = "kionix,kx023-1025", },
>  	{ }
>  };

Re: [PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"

[Rayyan Ansari]

On 26/10/2024 12:16, Jonathan Cameron wrote:
> On Thu, 24 Oct 2024 22:04:56 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
>> The mentioned change effectively broke the ODR startup timeouts
>> settungs for KX023-1025 case. Let's revert it for now and see
>> how we can handle it with the better approach after switching
>> the driver to use data structure instead of enum.
>>
>> This reverts commit d5cbe1502043124ff8af8136b80f93758c4a61e0.
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> I'll take this the slow way as I don't think there is time to chase the revert
> through the various trees and still get the dependent patches in.
> Hopefully we will fairly quickly get the missing table data and can
> bring this back again.
> 
> For now, applied to the togreg branch of iio.git.
> I have tagged it as a fix though. and +CC Rayyan
> (I'm guessing maybe that will bounce as you rarely miss people you should
> CC!)
Hi,
Sorry for not replying earlier, I've just caught up with the discussion.

I don't fully understand why this is breaking KX023-1025, but you know 
more than I do here.
Does this not mean that the use of KX022-1020 in the 3 devices (Lumia 
640, 640 XL, 735) using this from qcom-msm8226-microsoft-common.dtsi 
will now be broken?

Thanks,
Rayyan

Re: [PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"

[Jonathan Cameron]

On Sat, 26 Oct 2024 15:58:52 +0100
Rayyan Ansari <rayyan@ansari.sh> wrote:

> On 26/10/2024 12:16, Jonathan Cameron wrote:
> > On Thu, 24 Oct 2024 22:04:56 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> >> The mentioned change effectively broke the ODR startup timeouts
> >> settungs for KX023-1025 case. Let's revert it for now and see
> >> how we can handle it with the better approach after switching
> >> the driver to use data structure instead of enum.
> >>
> >> This reverts commit d5cbe1502043124ff8af8136b80f93758c4a61e0.
> >>
> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>  
> > I'll take this the slow way as I don't think there is time to chase the revert
> > through the various trees and still get the dependent patches in.
> > Hopefully we will fairly quickly get the missing table data and can
> > bring this back again.
> > 
> > For now, applied to the togreg branch of iio.git.
> > I have tagged it as a fix though. and +CC Rayyan
> > (I'm guessing maybe that will bounce as you rarely miss people you should
> > CC!)  
> Hi,
> Sorry for not replying earlier, I've just caught up with the discussion.
> 
> I don't fully understand why this is breaking KX023-1025, but you know 
> more than I do here.
> Does this not mean that the use of KX022-1020 in the 3 devices (Lumia 
> 640, 640 XL, 735) using this from qcom-msm8226-microsoft-common.dtsi 
> will now be broken?
> 
Yes.  The issues in the currently driver is here
https://elixir.bootlin.com/linux/v6.12-rc4/source/drivers/iio/accel/kxcjk-1013.c#L321

This array is indexed using the enum
https://elixir.bootlin.com/linux/v6.12-rc4/source/drivers/iio/accel/kxcjk-1013.c#L176
and the new entry for the KX022-1020 mean we are one short of those startup
time definitions.

Without that the values retrieved for the KX022-1025 are all 0.

It should be a relatively easy fix if we have those times.
One side effect of this series of Andy's is that it makes it much harder to have
similar bugs in future. 

Jonathan

> Thanks,
> Rayyan

Re: [PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"

[Andy Shevchenko]

On Sat, Oct 26, 2024 at 03:58:52PM +0100, Rayyan Ansari wrote:
> On 26/10/2024 12:16, Jonathan Cameron wrote:
> > On Thu, 24 Oct 2024 22:04:56 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > 
> > > The mentioned change effectively broke the ODR startup timeouts
> > > settungs for KX023-1025 case. Let's revert it for now and see
> > > how we can handle it with the better approach after switching
> > > the driver to use data structure instead of enum.
> > > 
> > > This reverts commit d5cbe1502043124ff8af8136b80f93758c4a61e0.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > I'll take this the slow way as I don't think there is time to chase the revert
> > through the various trees and still get the dependent patches in.
> > Hopefully we will fairly quickly get the missing table data and can
> > bring this back again.
> > 
> > For now, applied to the togreg branch of iio.git.
> > I have tagged it as a fix though. and +CC Rayyan
> > (I'm guessing maybe that will bounce as you rarely miss people you should
> > CC!)
> Hi,
> Sorry for not replying earlier, I've just caught up with the discussion.
> 
> I don't fully understand why this is breaking KX023-1025, but you know more
> than I do here.
> Does this not mean that the use of KX022-1020 in the 3 devices (Lumia 640,
> 640 XL, 735) using this from qcom-msm8226-microsoft-common.dtsi will now be
> broken?

Jonathan already answered to the question, so, please, rework the patch and
submit it again.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 08/24] iio: accel: kxcjk-1013: Switch from CONFIG_PM guards to pm_ptr() etc

[Andy Shevchenko]

Letting the compiler remove these functions when the kernel is built
without CONFIG_PM support is simpler and less error prone than the
use of #ifdef based config guards.

Removing instances of this approach from IIO also stops them being
copied into new drivers.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 208e701e1aed..0cc34e17a23f 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -315,7 +315,7 @@ static const char *const kxtf9_samp_freq_avail =
 	"25 50 100 200 400 800";
 
 /* Refer to section 4 of the specification */
-static __maybe_unused const struct {
+static const struct {
 	int odr_bits;
 	int usec;
 } odr_start_up_times[KX_MAX_CHIPS][12] = {
@@ -601,7 +601,6 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	return 0;
 }
 
-#ifdef CONFIG_PM
 static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
 {
 	int i;
@@ -614,7 +613,6 @@ static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
 
 	return KXCJK1013_MAX_STARTUP_TIME_US;
 }
-#endif
 
 static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
 {
@@ -1636,7 +1634,6 @@ static void kxcjk1013_remove(struct i2c_client *client)
 	mutex_unlock(&data->mutex);
 }
 
-#ifdef CONFIG_PM_SLEEP
 static int kxcjk1013_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
@@ -1664,9 +1661,7 @@ static int kxcjk1013_resume(struct device *dev)
 
 	return ret;
 }
-#endif
 
-#ifdef CONFIG_PM
 static int kxcjk1013_runtime_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
@@ -1700,12 +1695,10 @@ static int kxcjk1013_runtime_resume(struct device *dev)
 
 	return 0;
 }
-#endif
 
 static const struct dev_pm_ops kxcjk1013_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(kxcjk1013_suspend, kxcjk1013_resume)
-	SET_RUNTIME_PM_OPS(kxcjk1013_runtime_suspend,
-			   kxcjk1013_runtime_resume, NULL)
+	SYSTEM_SLEEP_PM_OPS(kxcjk1013_suspend, kxcjk1013_resume)
+	RUNTIME_PM_OPS(kxcjk1013_runtime_suspend, kxcjk1013_runtime_resume, NULL)
 };
 
 static const struct i2c_device_id kxcjk1013_id[] = {
@@ -1734,7 +1727,7 @@ static struct i2c_driver kxcjk1013_driver = {
 		.name	= KXCJK1013_DRV_NAME,
 		.acpi_match_table = ACPI_PTR(kx_acpi_match),
 		.of_match_table = kxcjk1013_of_match,
-		.pm	= &kxcjk1013_pm_ops,
+		.pm	= pm_ptr(&kxcjk1013_pm_ops),
 	},
 	.probe		= kxcjk1013_probe,
 	.remove		= kxcjk1013_remove,
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 08/24] iio: accel: kxcjk-1013: Switch from CONFIG_PM guards to pm_ptr() etc

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:04:57 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Letting the compiler remove these functions when the kernel is built
> without CONFIG_PM support is simpler and less error prone than the
> use of #ifdef based config guards.
> 
> Removing instances of this approach from IIO also stops them being
> copied into new drivers.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied. Indeed good to get rid of remaining cases of this.

Jonathan

> ---
>  drivers/iio/accel/kxcjk-1013.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 208e701e1aed..0cc34e17a23f 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -315,7 +315,7 @@ static const char *const kxtf9_samp_freq_avail =
>  	"25 50 100 200 400 800";
>  
>  /* Refer to section 4 of the specification */
> -static __maybe_unused const struct {
> +static const struct {
>  	int odr_bits;
>  	int usec;
>  } odr_start_up_times[KX_MAX_CHIPS][12] = {
> @@ -601,7 +601,6 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_PM
>  static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
>  {
>  	int i;
> @@ -614,7 +613,6 @@ static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
>  
>  	return KXCJK1013_MAX_STARTUP_TIME_US;
>  }
> -#endif
>  
>  static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
>  {
> @@ -1636,7 +1634,6 @@ static void kxcjk1013_remove(struct i2c_client *client)
>  	mutex_unlock(&data->mutex);
>  }
>  
> -#ifdef CONFIG_PM_SLEEP
>  static int kxcjk1013_suspend(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> @@ -1664,9 +1661,7 @@ static int kxcjk1013_resume(struct device *dev)
>  
>  	return ret;
>  }
> -#endif
>  
> -#ifdef CONFIG_PM
>  static int kxcjk1013_runtime_suspend(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> @@ -1700,12 +1695,10 @@ static int kxcjk1013_runtime_resume(struct device *dev)
>  
>  	return 0;
>  }
> -#endif
>  
>  static const struct dev_pm_ops kxcjk1013_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(kxcjk1013_suspend, kxcjk1013_resume)
> -	SET_RUNTIME_PM_OPS(kxcjk1013_runtime_suspend,
> -			   kxcjk1013_runtime_resume, NULL)
> +	SYSTEM_SLEEP_PM_OPS(kxcjk1013_suspend, kxcjk1013_resume)
> +	RUNTIME_PM_OPS(kxcjk1013_runtime_suspend, kxcjk1013_runtime_resume, NULL)
>  };
>  
>  static const struct i2c_device_id kxcjk1013_id[] = {
> @@ -1734,7 +1727,7 @@ static struct i2c_driver kxcjk1013_driver = {
>  		.name	= KXCJK1013_DRV_NAME,
>  		.acpi_match_table = ACPI_PTR(kx_acpi_match),
>  		.of_match_table = kxcjk1013_of_match,
> -		.pm	= &kxcjk1013_pm_ops,
> +		.pm	= pm_ptr(&kxcjk1013_pm_ops),
>  	},
>  	.probe		= kxcjk1013_probe,
>  	.remove		= kxcjk1013_remove,

[PATCH v3 09/24] iio: accel: kxcjk-1013: Use local variable for regs

[Andy Shevchenko]

Use local variable for regs in preparatory of further cleaning up changes.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 65 +++++++++++++++++++---------------
 1 file changed, 37 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 0cc34e17a23f..a5411d920025 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -445,9 +445,10 @@ MODULE_DEVICE_TABLE(acpi, kx_acpi_match);
 static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 			      enum kxcjk1013_mode mode)
 {
+	const struct kx_chipset_regs *regs = data->regs;
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
+	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -458,7 +459,7 @@ static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 	else
 		ret |= KXCJK1013_REG_CTRL1_BIT_PC1;
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->ctrl1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, regs->ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
 		return ret;
@@ -470,9 +471,10 @@ static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
 			      enum kxcjk1013_mode *mode)
 {
+	const struct kx_chipset_regs *regs = data->regs;
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
+	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -488,9 +490,10 @@ static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
 
 static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
 {
+	const struct kx_chipset_regs *regs = data->regs;
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
+	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -501,7 +504,7 @@ static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
 	ret |= (KXCJK1013_scale_table[range_index].gsel_0 << 3);
 	ret |= (KXCJK1013_scale_table[range_index].gsel_1 << 4);
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->ctrl1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, regs->ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
 		return ret;
@@ -514,6 +517,7 @@ static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
 
 static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 {
+	const struct kx_chipset_regs *regs = data->regs;
 	int ret;
 
 #ifdef CONFIG_ACPI
@@ -535,7 +539,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
+	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -544,7 +548,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	/* Set 12 bit mode */
 	ret |= KXCJK1013_REG_CTRL1_BIT_RES;
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->ctrl1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, regs->ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl\n");
 		return ret;
@@ -555,7 +559,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->data_ctrl);
+	ret = i2c_smbus_read_byte_data(data->client, regs->data_ctrl);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_data_ctrl\n");
 		return ret;
@@ -564,7 +568,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	data->odr_bits = ret;
 
 	/* Set up INT polarity */
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_ctrl1);
+	ret = i2c_smbus_read_byte_data(data->client, regs->int_ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
 		return ret;
@@ -575,7 +579,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	else
 		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEA;
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->int_ctrl1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, regs->int_ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
 		return ret;
@@ -637,18 +641,17 @@ static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
 
 static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 {
+	const struct kx_chipset_regs *regs = data->regs;
 	int ret;
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->wake_timer,
-					data->wake_dur);
+	ret = i2c_smbus_write_byte_data(data->client, regs->wake_timer, data->wake_dur);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
 			"Error writing reg_wake_timer\n");
 		return ret;
 	}
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->wake_thres,
-					data->wake_thres);
+	ret = i2c_smbus_write_byte_data(data->client, regs->wake_thres, data->wake_thres);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_wake_thres\n");
 		return ret;
@@ -660,6 +663,7 @@ static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 						bool status)
 {
+	const struct kx_chipset_regs *regs = data->regs;
 	int ret;
 	enum kxcjk1013_mode store_mode;
 
@@ -676,7 +680,7 @@ static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_ctrl1);
+	ret = i2c_smbus_read_byte_data(data->client, regs->int_ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
 		return ret;
@@ -687,13 +691,13 @@ static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 	else
 		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEN;
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->int_ctrl1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, regs->int_ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
 		return ret;
 	}
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
+	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -704,7 +708,7 @@ static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 	else
 		ret &= ~KXCJK1013_REG_CTRL1_BIT_WUFE;
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->ctrl1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, regs->ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
 		return ret;
@@ -722,6 +726,7 @@ static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 					      bool status)
 {
+	const struct kx_chipset_regs *regs = data->regs;
 	int ret;
 	enum kxcjk1013_mode store_mode;
 
@@ -734,7 +739,7 @@ static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_ctrl1);
+	ret = i2c_smbus_read_byte_data(data->client, regs->int_ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
 		return ret;
@@ -745,13 +750,13 @@ static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 	else
 		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEN;
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->int_ctrl1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, regs->int_ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
 		return ret;
 	}
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
+	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -762,7 +767,7 @@ static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 	else
 		ret &= ~KXCJK1013_REG_CTRL1_BIT_DRDY;
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->ctrl1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, regs->ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
 		return ret;
@@ -809,6 +814,7 @@ static int kxcjk1013_convert_odr_value(const struct kx_odr_map *map,
 
 static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 {
+	const struct kx_chipset_regs *regs = data->regs;
 	int ret;
 	enum kxcjk1013_mode store_mode;
 	const struct kx_odr_map *odr_setting;
@@ -834,7 +840,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->data_ctrl,
+	ret = i2c_smbus_write_byte_data(data->client, regs->data_ctrl,
 					odr_setting->odr_bits);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing data_ctrl\n");
@@ -843,7 +849,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 
 	data->odr_bits = odr_setting->odr_bits;
 
-	ret = i2c_smbus_write_byte_data(data->client, data->regs->wuf_ctrl,
+	ret = i2c_smbus_write_byte_data(data->client, regs->wuf_ctrl,
 					odr_setting->wuf_bits);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl2\n");
@@ -1245,9 +1251,10 @@ static void kxcjk1013_trig_reen(struct iio_trigger *trig)
 {
 	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
+	const struct kx_chipset_regs *regs = data->regs;
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_rel);
+	ret = i2c_smbus_read_byte_data(data->client, regs->int_rel);
 	if (ret < 0)
 		dev_err(&data->client->dev, "Error reading reg_int_rel\n");
 }
@@ -1299,8 +1306,9 @@ static const struct iio_trigger_ops kxcjk1013_trigger_ops = {
 static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
 {
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
+	const struct kx_chipset_regs *regs = data->regs;
 
-	int ret = i2c_smbus_read_byte_data(data->client, data->regs->int_src2);
+	int ret = i2c_smbus_read_byte_data(data->client, regs->int_src2);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_int_src2\n");
 		return;
@@ -1365,9 +1373,10 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
+	const struct kx_chipset_regs *regs = data->regs;
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_src1);
+	ret = i2c_smbus_read_byte_data(data->client, regs->int_src1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_int_src1\n");
 		goto ack_intr;
@@ -1390,7 +1399,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 	if (data->dready_trigger_on)
 		return IRQ_HANDLED;
 
-	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_rel);
+	ret = i2c_smbus_read_byte_data(data->client, regs->int_rel);
 	if (ret < 0)
 		dev_err(&data->client->dev, "Error reading reg_int_rel\n");
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 09/24] iio: accel: kxcjk-1013: Use local variable for regs

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:04:58 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Use local variable for regs in preparatory of further cleaning up changes.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied.   A little less obviously a good idea than some such cases
but not a bad idea either...

[PATCH v3 10/24] iio: accel: kxcjk-1013: Rename kxcjk1013_info

[Andy Shevchenko]

Rename kxcjk1013_info to kxcjk1013_iio_info in preparatory of
further cleaning up changes.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index a5411d920025..f97bdbbe71ed 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1211,7 +1211,7 @@ static const struct iio_buffer_setup_ops kxcjk1013_buffer_setup_ops = {
 	.postdisable		= kxcjk1013_buffer_postdisable,
 };
 
-static const struct iio_info kxcjk1013_info = {
+static const struct iio_info kxcjk1013_iio_info = {
 	.attrs			= &kxcjk1013_attrs_group,
 	.read_raw		= kxcjk1013_read_raw,
 	.write_raw		= kxcjk1013_write_raw,
@@ -1533,7 +1533,7 @@ static int kxcjk1013_probe(struct i2c_client *client)
 	indio_dev->available_scan_masks = kxcjk1013_scan_masks;
 	indio_dev->name = name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->info = &kxcjk1013_info;
+	indio_dev->info = &kxcjk1013_iio_info;
 
 	if (client->irq > 0 && data->acpi_type != ACPI_SMO8500) {
 		ret = devm_request_threaded_irq(&client->dev, client->irq,
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 10/24] iio: accel: kxcjk-1013: Rename kxcjk1013_info

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:04:59 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Rename kxcjk1013_info to kxcjk1013_iio_info in preparatory of
> further cleaning up changes.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied.
> ---
>  drivers/iio/accel/kxcjk-1013.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index a5411d920025..f97bdbbe71ed 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -1211,7 +1211,7 @@ static const struct iio_buffer_setup_ops kxcjk1013_buffer_setup_ops = {
>  	.postdisable		= kxcjk1013_buffer_postdisable,
>  };
>  
> -static const struct iio_info kxcjk1013_info = {
> +static const struct iio_info kxcjk1013_iio_info = {
>  	.attrs			= &kxcjk1013_attrs_group,
>  	.read_raw		= kxcjk1013_read_raw,
>  	.write_raw		= kxcjk1013_write_raw,
> @@ -1533,7 +1533,7 @@ static int kxcjk1013_probe(struct i2c_client *client)
>  	indio_dev->available_scan_masks = kxcjk1013_scan_masks;
>  	indio_dev->name = name;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->info = &kxcjk1013_info;
> +	indio_dev->info = &kxcjk1013_iio_info;
>  
>  	if (client->irq > 0 && data->acpi_type != ACPI_SMO8500) {
>  		ret = devm_request_threaded_irq(&client->dev, client->irq,

[PATCH v3 11/24] iio: accel: kxcjk-1013: Start using chip_info variables instead of enum

[Andy Shevchenko]

Instead of having a enum and keeping IDs as driver data pointers,
just have a chip_info struct per supported device.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 171 +++++++++++++++++++--------------
 1 file changed, 99 insertions(+), 72 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index f97bdbbe71ed..37c82fdf7c43 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -234,6 +234,55 @@ static const struct kx_chipset_regs kx0231025_regs = {
 	.wake_thres	= KX023_REG_ATH,
 };
 
+struct kx_chipset_info {
+	const struct kx_chipset_regs *regs;
+	enum kx_chipset chipset;
+	enum kx_acpi_type acpi_type;
+};
+
+static const struct kx_chipset_info kxcjk1013_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXCJK1013,
+};
+
+static const struct kx_chipset_info kxcj91008_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXCJ91008,
+};
+
+static const struct kx_chipset_info kxcj91008_kiox010a_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXCJ91008,
+	.acpi_type = ACPI_KIOX010A,
+};
+
+static const struct kx_chipset_info kxcj91008_kiox020a_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXCJ91008,
+	.acpi_type = ACPI_GENERIC,
+};
+
+static const struct kx_chipset_info kxcj91008_smo8500_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXCJ91008,
+	.acpi_type = ACPI_SMO8500,
+};
+
+static const struct kx_chipset_info kxtj21009_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXTJ21009,
+};
+
+static const struct kx_chipset_info kxtf9_info = {
+	.regs = &kxtf9_regs,
+	.chipset = KXTF9,
+};
+
+static const struct kx_chipset_info kx0231025_info = {
+	.regs = &kx0231025_regs,
+	.chipset = KX0231025,
+};
+
 enum kxcjk1013_axis {
 	AXIS_X,
 	AXIS_Y,
@@ -261,9 +310,7 @@ struct kxcjk1013_data {
 	int ev_enable_state;
 	bool motion_trigger_on;
 	int64_t timestamp;
-	enum kx_chipset chipset;
-	enum kx_acpi_type acpi_type;
-	const struct kx_chipset_regs *regs;
+	const struct kx_chipset_info *info;
 };
 
 enum kxcjk1013_mode {
@@ -425,27 +472,28 @@ static int kiox010a_dsm(struct device *dev, int fn_index)
 }
 
 static const struct acpi_device_id kx_acpi_match[] = {
-	{"KXCJ1013", KXCJK1013},
-	{"KXCJ1008", KXCJ91008},
-	{"KXCJ9000", KXCJ91008},
-	{"KIOX0008", KXCJ91008},
-	{"KIOX0009", KXTJ21009},
-	{"KIOX000A", KXCJ91008},
-	{"KIOX010A", KXCJ91008}, /* KXCJ91008 in the display of a yoga 2-in-1 */
-	{"KIOX020A", KXCJ91008}, /* KXCJ91008 in the base of a yoga 2-in-1 */
-	{"KXTJ1009", KXTJ21009},
-	{"KXJ2109",  KXTJ21009},
-	{"SMO8500",  KXCJ91008},
+	{"KIOX0008", (kernel_ulong_t)&kxcj91008_info },
+	{"KIOX0009", (kernel_ulong_t)&kxtj21009_info },
+	{"KIOX000A", (kernel_ulong_t)&kxcj91008_info },
+	/* KXCJ91008 in the display of a yoga 2-in-1 */
+	{"KIOX010A", (kernel_ulong_t)&kxcj91008_kiox010a_info },
+	/* KXCJ91008 in the base of a yoga 2-in-1 */
+	{"KIOX020A", (kernel_ulong_t)&kxcj91008_kiox020a_info },
+	{"KXCJ1008", (kernel_ulong_t)&kxcj91008_info },
+	{"KXCJ1013", (kernel_ulong_t)&kxcjk1013_info },
+	{"KXCJ9000", (kernel_ulong_t)&kxcj91008_info },
+	{"KXJ2109",  (kernel_ulong_t)&kxtj21009_info },
+	{"KXTJ1009", (kernel_ulong_t)&kxtj21009_info },
+	{"SMO8500",  (kernel_ulong_t)&kxcj91008_smo8500_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, kx_acpi_match);
-
 #endif
 
 static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 			      enum kxcjk1013_mode mode)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
@@ -471,7 +519,7 @@ static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
 			      enum kxcjk1013_mode *mode)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
@@ -490,7 +538,7 @@ static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
 
 static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
@@ -517,11 +565,11 @@ static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
 
 static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 #ifdef CONFIG_ACPI
-	if (data->acpi_type == ACPI_KIOX010A) {
+	if (data->info->acpi_type == ACPI_KIOX010A) {
 		/* Make sure the kbd and touchpad on 2-in-1s using 2 KXCJ91008-s work */
 		kiox010a_dsm(&data->client->dev, KIOX010A_SET_LAPTOP_MODE);
 	}
@@ -586,7 +634,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	}
 
 	/* On KX023, route all used interrupts to INT1 for now */
-	if (data->chipset == KX0231025 && data->client->irq > 0) {
+	if (data->info->chipset == KX0231025 && data->client->irq > 0) {
 		ret = i2c_smbus_write_byte_data(data->client, KX023_REG_INC4,
 						KX023_REG_INC4_DRDY1 |
 						KX023_REG_INC4_WUFI1);
@@ -607,8 +655,8 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 
 static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
 {
+	int idx = data->info->chipset;
 	int i;
-	int idx = data->chipset;
 
 	for (i = 0; i < ARRAY_SIZE(odr_start_up_times[idx]); ++i) {
 		if (odr_start_up_times[idx][i].odr_bits == data->odr_bits)
@@ -641,7 +689,7 @@ static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
 
 static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_write_byte_data(data->client, regs->wake_timer, data->wake_dur);
@@ -663,7 +711,7 @@ static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 						bool status)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 	enum kxcjk1013_mode store_mode;
 
@@ -726,7 +774,7 @@ static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 					      bool status)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 	enum kxcjk1013_mode store_mode;
 
@@ -814,7 +862,7 @@ static int kxcjk1013_convert_odr_value(const struct kx_odr_map *map,
 
 static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 	enum kxcjk1013_mode store_mode;
 	const struct kx_odr_map *odr_setting;
@@ -823,7 +871,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 	if (ret < 0)
 		return ret;
 
-	if (data->chipset == KXTF9)
+	if (data->info->chipset == KXTF9)
 		odr_setting = kxcjk1013_find_odr_value(kxtf9_samp_freq_table,
 						       ARRAY_SIZE(kxtf9_samp_freq_table),
 						       val, val2);
@@ -867,7 +915,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 
 static int kxcjk1013_get_odr(struct kxcjk1013_data *data, int *val, int *val2)
 {
-	if (data->chipset == KXTF9)
+	if (data->info->chipset == KXTF9)
 		return kxcjk1013_convert_odr_value(kxtf9_samp_freq_table,
 						   ARRAY_SIZE(kxtf9_samp_freq_table),
 						   data->odr_bits, val, val2);
@@ -1134,7 +1182,7 @@ static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
 	const char *str;
 
-	if (data->chipset == KXTF9)
+	if (data->info->chipset == KXTF9)
 		str = kxtf9_samp_freq_avail;
 	else
 		str = kxcjk1013_samp_freq_avail;
@@ -1251,7 +1299,7 @@ static void kxcjk1013_trig_reen(struct iio_trigger *trig)
 {
 	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, regs->int_rel);
@@ -1306,7 +1354,7 @@ static const struct iio_trigger_ops kxcjk1013_trigger_ops = {
 static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
 {
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 
 	int ret = i2c_smbus_read_byte_data(data->client, regs->int_src2);
 	if (ret < 0) {
@@ -1373,7 +1421,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, regs->int_src1);
@@ -1383,7 +1431,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 	}
 
 	if (ret & KXCJK1013_REG_INT_SRC1_BIT_WUFS) {
-		if (data->chipset == KXTF9)
+		if (data->info->chipset == KXTF9)
 			iio_push_event(indio_dev,
 				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 				       0,
@@ -1425,8 +1473,7 @@ static irqreturn_t kxcjk1013_data_rdy_trig_poll(int irq, void *private)
 }
 
 static const char *kxcjk1013_match_acpi_device(struct device *dev,
-					       enum kx_chipset *chipset,
-					       enum kx_acpi_type *acpi_type,
+					       const struct kx_chipset_info **info,
 					       const char **label)
 {
 	const struct acpi_device_id *id;
@@ -1435,16 +1482,12 @@ static const char *kxcjk1013_match_acpi_device(struct device *dev,
 	if (!id)
 		return NULL;
 
-	if (strcmp(id->id, "SMO8500") == 0) {
-		*acpi_type = ACPI_SMO8500;
-	} else if (strcmp(id->id, "KIOX010A") == 0) {
-		*acpi_type = ACPI_KIOX010A;
+	if (strcmp(id->id, "KIOX010A") == 0)
 		*label = "accel-display";
-	} else if (strcmp(id->id, "KIOX020A") == 0) {
+	else if (strcmp(id->id, "KIOX020A") == 0)
 		*label = "accel-base";
-	}
 
-	*chipset = (enum kx_chipset)id->driver_data;
+	*info = (const struct kx_chipset_info *)id->driver_data;
 
 	return dev_name(dev);
 }
@@ -1496,31 +1539,16 @@ static int kxcjk1013_probe(struct i2c_client *client)
 	msleep(20);
 
 	if (id) {
-		data->chipset = (enum kx_chipset)(id->driver_data);
 		name = id->name;
+		data->info = (const struct kx_chipset_info *)(id->driver_data);
 	} else if (ACPI_HANDLE(&client->dev)) {
-		name = kxcjk1013_match_acpi_device(&client->dev,
-						   &data->chipset,
-						   &data->acpi_type,
+		name = kxcjk1013_match_acpi_device(&client->dev, &data->info,
 						   &indio_dev->label);
 	} else
 		return -ENODEV;
 
-	switch (data->chipset) {
-	case KXCJK1013:
-	case KXCJ91008:
-	case KXTJ21009:
-		data->regs = &kxcjk1013_regs;
-		break;
-	case KXTF9:
-		data->regs = &kxtf9_regs;
-		break;
-	case KX0231025:
-		data->regs = &kx0231025_regs;
-		break;
-	default:
+	if (!data->info)
 		return -EINVAL;
-	}
 
 	ret = kxcjk1013_chip_init(data);
 	if (ret < 0)
@@ -1535,7 +1563,7 @@ static int kxcjk1013_probe(struct i2c_client *client)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &kxcjk1013_iio_info;
 
-	if (client->irq > 0 && data->acpi_type != ACPI_SMO8500) {
+	if (client->irq > 0 && data->info->acpi_type != ACPI_SMO8500) {
 		ret = devm_request_threaded_irq(&client->dev, client->irq,
 						kxcjk1013_data_rdy_trig_poll,
 						kxcjk1013_event_handler,
@@ -1711,22 +1739,21 @@ static const struct dev_pm_ops kxcjk1013_pm_ops = {
 };
 
 static const struct i2c_device_id kxcjk1013_id[] = {
-	{"kxcjk1013", KXCJK1013},
-	{"kxcj91008", KXCJ91008},
-	{"kxtj21009", KXTJ21009},
-	{"kxtf9",     KXTF9},
-	{"kx023-1025", KX0231025},
+	{"kxcjk1013",  (kernel_ulong_t)&kxcjk1013_info },
+	{"kxcj91008",  (kernel_ulong_t)&kxcj91008_info },
+	{"kxtj21009",  (kernel_ulong_t)&kxtj21009_info },
+	{"kxtf9", (kernel_ulong_t)&kxtf9_info },
+	{"kx023-1025", (kernel_ulong_t)&kx0231025_info },
 	{}
 };
-
 MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
 
 static const struct of_device_id kxcjk1013_of_match[] = {
-	{ .compatible = "kionix,kxcjk1013", },
-	{ .compatible = "kionix,kxcj91008", },
-	{ .compatible = "kionix,kxtj21009", },
-	{ .compatible = "kionix,kxtf9", },
-	{ .compatible = "kionix,kx023-1025", },
+	{ .compatible = "kionix,kxcjk1013", &kxcjk1013_info },
+	{ .compatible = "kionix,kxcj91008", &kxcj91008_info },
+	{ .compatible = "kionix,kxtj21009", &kxtj21009_info },
+	{ .compatible = "kionix,kxtf9", &kxtf9_info },
+	{ .compatible = "kionix,kx023-1025", &kx0231025_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, kxcjk1013_of_match);
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 11/24] iio: accel: kxcjk-1013: Start using chip_info variables instead of enum

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:00 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Instead of having a enum and keeping IDs as driver data pointers,
> just have a chip_info struct per supported device.
I'm not keen longer term on acpi_type, as the various bits dependent on that
should probably be done via optional callbacks in the kx_chipset_info
structure, but this is a sensible intermediate step.

I see the chipset one goes away later hence no comment on that.

I did a bit of white space massaging whilst applying this.
Hopefully that won't make me mess up applying the following patches.

>  enum kxcjk1013_mode {
> @@ -425,27 +472,28 @@ static int kiox010a_dsm(struct device *dev, int fn_index)
>  }
>  
>  static const struct acpi_device_id kx_acpi_match[] = {
> -	{"KXCJ1013", KXCJK1013},
> -	{"KXCJ1008", KXCJ91008},
> -	{"KXCJ9000", KXCJ91008},
> -	{"KIOX0008", KXCJ91008},
> -	{"KIOX0009", KXTJ21009},
> -	{"KIOX000A", KXCJ91008},
> -	{"KIOX010A", KXCJ91008}, /* KXCJ91008 in the display of a yoga 2-in-1 */
> -	{"KIOX020A", KXCJ91008}, /* KXCJ91008 in the base of a yoga 2-in-1 */
> -	{"KXTJ1009", KXTJ21009},
> -	{"KXJ2109",  KXTJ21009},
> -	{"SMO8500",  KXCJ91008},
> +	{"KIOX0008", (kernel_ulong_t)&kxcj91008_info },
> +	{"KIOX0009", (kernel_ulong_t)&kxtj21009_info },
> +	{"KIOX000A", (kernel_ulong_t)&kxcj91008_info },
> +	/* KXCJ91008 in the display of a yoga 2-in-1 */
> +	{"KIOX010A", (kernel_ulong_t)&kxcj91008_kiox010a_info },
> +	/* KXCJ91008 in the base of a yoga 2-in-1 */
> +	{"KIOX020A", (kernel_ulong_t)&kxcj91008_kiox020a_info },
> +	{"KXCJ1008", (kernel_ulong_t)&kxcj91008_info },
> +	{"KXCJ1013", (kernel_ulong_t)&kxcjk1013_info },
> +	{"KXCJ9000", (kernel_ulong_t)&kxcj91008_info },
> +	{"KXJ2109",  (kernel_ulong_t)&kxtj21009_info },
> +	{"KXTJ1009", (kernel_ulong_t)&kxtj21009_info },
> +	{"SMO8500",  (kernel_ulong_t)&kxcj91008_smo8500_info },
I'll tweak the spacing on this as well whilst here.
You did one end effectively, might as well do the other.

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(acpi, kx_acpi_match);

> @@ -1711,22 +1739,21 @@ static const struct dev_pm_ops kxcjk1013_pm_ops = {
>  };
>  
>  static const struct i2c_device_id kxcjk1013_id[] = {
> -	{"kxcjk1013", KXCJK1013},
> -	{"kxcj91008", KXCJ91008},
> -	{"kxtj21009", KXTJ21009},
> -	{"kxtf9",     KXTF9},
> -	{"kx023-1025", KX0231025},
> +	{"kxcjk1013",  (kernel_ulong_t)&kxcjk1013_info },
> +	{"kxcj91008",  (kernel_ulong_t)&kxcj91008_info },
> +	{"kxtj21009",  (kernel_ulong_t)&kxtj21009_info },
> +	{"kxtf9", (kernel_ulong_t)&kxtf9_info },
> +	{"kx023-1025", (kernel_ulong_t)&kx0231025_info },
>  	{}
I'm going to tweak the spacing of that whilst we are touching it.

Re: [PATCH v3 11/24] iio: accel: kxcjk-1013: Start using chip_info variables instead of enum

[Andy Shevchenko]

On Sat, Oct 26, 2024 at 12:26:13PM +0100, Jonathan Cameron wrote:
> On Thu, 24 Oct 2024 22:05:00 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Instead of having a enum and keeping IDs as driver data pointers,
> > just have a chip_info struct per supported device.
> I'm not keen longer term on acpi_type, as the various bits dependent on that
> should probably be done via optional callbacks in the kx_chipset_info
> structure, but this is a sensible intermediate step.

Yeah, I decided to postpone that because the main point of this series is to
clean up acpi_match_device(). And indeed I was thinking about callbacks and
other things (flags, string literals, etc) to be moved to chip_info.

> I see the chipset one goes away later hence no comment on that.
> 
> I did a bit of white space massaging whilst applying this.
> Hopefully that won't make me mess up applying the following patches.

Thanks!

(OTOH I'm not sure what spacing you meant because in the result it still seems
 slightly inconsistent.)

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 12/24] iio: accel: kxcjk-1013: Move odr_start_up_times up in the code

[Andy Shevchenko]

Move odr_start_up_times up in the code in a preparation of
the further cleaning up changes.

While at it, make it clear what values from enum are being used for
the respective array entries.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 154 ++++++++++++++++-----------------
 1 file changed, 77 insertions(+), 77 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 37c82fdf7c43..f384969a922d 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -178,6 +178,83 @@ enum kx_chipset {
 	KX_MAX_CHIPS /* this must be last */
 };
 
+/* Refer to section 4 of the specification */
+static const struct {
+	int odr_bits;
+	int usec;
+} odr_start_up_times[KX_MAX_CHIPS][12] = {
+	/* KXCJK-1013 */
+	[KXCJK1013] = {
+		{0x08, 100000},
+		{0x09, 100000},
+		{0x0A, 100000},
+		{0x0B, 100000},
+		{0, 80000},
+		{0x01, 41000},
+		{0x02, 21000},
+		{0x03, 11000},
+		{0x04, 6400},
+		{0x05, 3900},
+		{0x06, 2700},
+		{0x07, 2100},
+	},
+	/* KXCJ9-1008 */
+	[KXCJ91008] = {
+		{0x08, 100000},
+		{0x09, 100000},
+		{0x0A, 100000},
+		{0x0B, 100000},
+		{0, 80000},
+		{0x01, 41000},
+		{0x02, 21000},
+		{0x03, 11000},
+		{0x04, 6400},
+		{0x05, 3900},
+		{0x06, 2700},
+		{0x07, 2100},
+	},
+	/* KXCTJ2-1009 */
+	[KXTJ21009] = {
+		{0x08, 1240000},
+		{0x09, 621000},
+		{0x0A, 309000},
+		{0x0B, 151000},
+		{0, 80000},
+		{0x01, 41000},
+		{0x02, 21000},
+		{0x03, 11000},
+		{0x04, 6000},
+		{0x05, 4000},
+		{0x06, 3000},
+		{0x07, 2000},
+	},
+	/* KXTF9 */
+	[KXTF9] = {
+		{0x01, 81000},
+		{0x02, 41000},
+		{0x03, 21000},
+		{0x04, 11000},
+		{0x05, 5100},
+		{0x06, 2700},
+	},
+	/* KX023-1025 */
+	[KX0231025] = {
+		/* First 4 are not in datasheet, taken from KXCTJ2-1009 */
+		{0x08, 1240000},
+		{0x09, 621000},
+		{0x0A, 309000},
+		{0x0B, 151000},
+		{0, 81000},
+		{0x01, 40000},
+		{0x02, 22000},
+		{0x03, 12000},
+		{0x04, 7000},
+		{0x05, 4400},
+		{0x06, 3000},
+		{0x07, 3000},
+	},
+};
+
 enum kx_acpi_type {
 	ACPI_GENERIC,
 	ACPI_SMO8500,
@@ -361,83 +438,6 @@ static const struct kx_odr_map kxtf9_samp_freq_table[] = {
 static const char *const kxtf9_samp_freq_avail =
 	"25 50 100 200 400 800";
 
-/* Refer to section 4 of the specification */
-static const struct {
-	int odr_bits;
-	int usec;
-} odr_start_up_times[KX_MAX_CHIPS][12] = {
-	/* KXCJK-1013 */
-	{
-		{0x08, 100000},
-		{0x09, 100000},
-		{0x0A, 100000},
-		{0x0B, 100000},
-		{0, 80000},
-		{0x01, 41000},
-		{0x02, 21000},
-		{0x03, 11000},
-		{0x04, 6400},
-		{0x05, 3900},
-		{0x06, 2700},
-		{0x07, 2100},
-	},
-	/* KXCJ9-1008 */
-	{
-		{0x08, 100000},
-		{0x09, 100000},
-		{0x0A, 100000},
-		{0x0B, 100000},
-		{0, 80000},
-		{0x01, 41000},
-		{0x02, 21000},
-		{0x03, 11000},
-		{0x04, 6400},
-		{0x05, 3900},
-		{0x06, 2700},
-		{0x07, 2100},
-	},
-	/* KXCTJ2-1009 */
-	{
-		{0x08, 1240000},
-		{0x09, 621000},
-		{0x0A, 309000},
-		{0x0B, 151000},
-		{0, 80000},
-		{0x01, 41000},
-		{0x02, 21000},
-		{0x03, 11000},
-		{0x04, 6000},
-		{0x05, 4000},
-		{0x06, 3000},
-		{0x07, 2000},
-	},
-	/* KXTF9 */
-	{
-		{0x01, 81000},
-		{0x02, 41000},
-		{0x03, 21000},
-		{0x04, 11000},
-		{0x05, 5100},
-		{0x06, 2700},
-	},
-	/* KX023-1025 */
-	{
-		/* First 4 are not in datasheet, taken from KXCTJ2-1009 */
-		{0x08, 1240000},
-		{0x09, 621000},
-		{0x0A, 309000},
-		{0x0B, 151000},
-		{0, 81000},
-		{0x01, 40000},
-		{0x02, 22000},
-		{0x03, 12000},
-		{0x04, 7000},
-		{0x05, 4400},
-		{0x06, 3000},
-		{0x07, 3000},
-	},
-};
-
 static const struct {
 	u16 scale;
 	u8 gsel_0;
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 12/24] iio: accel: kxcjk-1013: Move odr_start_up_times up in the code

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:01 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Move odr_start_up_times up in the code in a preparation of
> the further cleaning up changes.
> 
> While at it, make it clear what values from enum are being used for
> the respective array entries.
I did some cheeky white space additions in this one as well.
Bit naughty in a code move patch, but I really want to get this
stuff cleaned up across IIO and it seems silly to chase your series
with another one changing the same code.

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/iio/accel/kxcjk-1013.c | 154 ++++++++++++++++-----------------
>  1 file changed, 77 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 37c82fdf7c43..f384969a922d 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -178,6 +178,83 @@ enum kx_chipset {
>  	KX_MAX_CHIPS /* this must be last */
>  };
>  
> +/* Refer to section 4 of the specification */
> +static const struct {
> +	int odr_bits;
> +	int usec;
> +} odr_start_up_times[KX_MAX_CHIPS][12] = {
> +	/* KXCJK-1013 */
> +	[KXCJK1013] = {
> +		{0x08, 100000},
> +		{0x09, 100000},
> +		{0x0A, 100000},
> +		{0x0B, 100000},
> +		{0, 80000},
I changed this to
		{ 0x00, 80000 }
as I think it helps readability to have them
all aligned

Spaces added to all entries.

Jonathan

> +		{0x01, 41000},
> +		{0x02, 21000},
> +		{0x03, 11000},
> +		{0x04, 6400},
> +		{0x05, 3900},
> +		{0x06, 2700},
> +		{0x07, 2100},
> +	},
> +	/* KXCJ9-1008 */
> +	[KXCJ91008] = {
> +		{0x08, 100000},
> +		{0x09, 100000},
> +		{0x0A, 100000},
> +		{0x0B, 100000},
> +		{0, 80000},
> +		{0x01, 41000},
> +		{0x02, 21000},
> +		{0x03, 11000},
> +		{0x04, 6400},
> +		{0x05, 3900},
> +		{0x06, 2700},
> +		{0x07, 2100},
> +	},
> +	/* KXCTJ2-1009 */
> +	[KXTJ21009] = {
> +		{0x08, 1240000},
> +		{0x09, 621000},
> +		{0x0A, 309000},
> +		{0x0B, 151000},
> +		{0, 80000},
> +		{0x01, 41000},
> +		{0x02, 21000},
> +		{0x03, 11000},
> +		{0x04, 6000},
> +		{0x05, 4000},
> +		{0x06, 3000},
> +		{0x07, 2000},
> +	},
> +	/* KXTF9 */
> +	[KXTF9] = {
> +		{0x01, 81000},
> +		{0x02, 41000},
> +		{0x03, 21000},
> +		{0x04, 11000},
> +		{0x05, 5100},
> +		{0x06, 2700},
> +	},
> +	/* KX023-1025 */
> +	[KX0231025] = {
> +		/* First 4 are not in datasheet, taken from KXCTJ2-1009 */
> +		{0x08, 1240000},
> +		{0x09, 621000},
> +		{0x0A, 309000},
> +		{0x0B, 151000},
> +		{0, 81000},
> +		{0x01, 40000},
> +		{0x02, 22000},
> +		{0x03, 12000},
> +		{0x04, 7000},
> +		{0x05, 4400},
> +		{0x06, 3000},
> +		{0x07, 3000},
> +	},
> +};
> +
>  enum kx_acpi_type {
>  	ACPI_GENERIC,
>  	ACPI_SMO8500,
> @@ -361,83 +438,6 @@ static const struct kx_odr_map kxtf9_samp_freq_table[] = {
>  static const char *const kxtf9_samp_freq_avail =
>  	"25 50 100 200 400 800";
>  
> -/* Refer to section 4 of the specification */
> -static const struct {
> -	int odr_bits;
> -	int usec;
> -} odr_start_up_times[KX_MAX_CHIPS][12] = {
> -	/* KXCJK-1013 */
> -	{
> -		{0x08, 100000},
> -		{0x09, 100000},
> -		{0x0A, 100000},
> -		{0x0B, 100000},
> -		{0, 80000},
> -		{0x01, 41000},
> -		{0x02, 21000},
> -		{0x03, 11000},
> -		{0x04, 6400},
> -		{0x05, 3900},
> -		{0x06, 2700},
> -		{0x07, 2100},
> -	},
> -	/* KXCJ9-1008 */
> -	{
> -		{0x08, 100000},
> -		{0x09, 100000},
> -		{0x0A, 100000},
> -		{0x0B, 100000},
> -		{0, 80000},
> -		{0x01, 41000},
> -		{0x02, 21000},
> -		{0x03, 11000},
> -		{0x04, 6400},
> -		{0x05, 3900},
> -		{0x06, 2700},
> -		{0x07, 2100},
> -	},
> -	/* KXCTJ2-1009 */
> -	{
> -		{0x08, 1240000},
> -		{0x09, 621000},
> -		{0x0A, 309000},
> -		{0x0B, 151000},
> -		{0, 80000},
> -		{0x01, 41000},
> -		{0x02, 21000},
> -		{0x03, 11000},
> -		{0x04, 6000},
> -		{0x05, 4000},
> -		{0x06, 3000},
> -		{0x07, 2000},
> -	},
> -	/* KXTF9 */
> -	{
> -		{0x01, 81000},
> -		{0x02, 41000},
> -		{0x03, 21000},
> -		{0x04, 11000},
> -		{0x05, 5100},
> -		{0x06, 2700},
> -	},
> -	/* KX023-1025 */
> -	{
> -		/* First 4 are not in datasheet, taken from KXCTJ2-1009 */
> -		{0x08, 1240000},
> -		{0x09, 621000},
> -		{0x0A, 309000},
> -		{0x0B, 151000},
> -		{0, 81000},
> -		{0x01, 40000},
> -		{0x02, 22000},
> -		{0x03, 12000},
> -		{0x04, 7000},
> -		{0x05, 4400},
> -		{0x06, 3000},
> -		{0x07, 3000},
> -	},
> -};
> -
>  static const struct {
>  	u16 scale;
>  	u8 gsel_0;

Re: [PATCH v3 12/24] iio: accel: kxcjk-1013: Move odr_start_up_times up in the code

[Andy Shevchenko]

On Sat, Oct 26, 2024 at 12:30:12PM +0100, Jonathan Cameron wrote:
> On Thu, 24 Oct 2024 22:05:01 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

...

> > While at it, make it clear what values from enum are being used for
> > the respective array entries.
> I did some cheeky white space additions in this one as well.
> Bit naughty in a code move patch, but I really want to get this
> stuff cleaned up across IIO and it seems silly to chase your series
> with another one changing the same code.

I like you change, thanks!

...

> > +		{0, 80000},
> I changed this to
> 		{ 0x00, 80000 }
> as I think it helps readability to have them
> all aligned
> 
> Spaces added to all entries.

Got it, please also add/fold the patch I sent today.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 13/24] iio: accel: kxcjk-1013: Convert ODR times array to variable in chip_info

[Andy Shevchenko]

Convert odr_start_up_times array to the variable in chip_info.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 166 ++++++++++++++++++---------------
 1 file changed, 89 insertions(+), 77 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index f384969a922d..91f890275a18 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -179,80 +179,84 @@ enum kx_chipset {
 };
 
 /* Refer to section 4 of the specification */
-static const struct {
+struct kx_odr_start_up_time {
 	int odr_bits;
 	int usec;
-} odr_start_up_times[KX_MAX_CHIPS][12] = {
-	/* KXCJK-1013 */
-	[KXCJK1013] = {
-		{0x08, 100000},
-		{0x09, 100000},
-		{0x0A, 100000},
-		{0x0B, 100000},
-		{0, 80000},
-		{0x01, 41000},
-		{0x02, 21000},
-		{0x03, 11000},
-		{0x04, 6400},
-		{0x05, 3900},
-		{0x06, 2700},
-		{0x07, 2100},
-	},
-	/* KXCJ9-1008 */
-	[KXCJ91008] = {
-		{0x08, 100000},
-		{0x09, 100000},
-		{0x0A, 100000},
-		{0x0B, 100000},
-		{0, 80000},
-		{0x01, 41000},
-		{0x02, 21000},
-		{0x03, 11000},
-		{0x04, 6400},
-		{0x05, 3900},
-		{0x06, 2700},
-		{0x07, 2100},
-	},
-	/* KXCTJ2-1009 */
-	[KXTJ21009] = {
-		{0x08, 1240000},
-		{0x09, 621000},
-		{0x0A, 309000},
-		{0x0B, 151000},
-		{0, 80000},
-		{0x01, 41000},
-		{0x02, 21000},
-		{0x03, 11000},
-		{0x04, 6000},
-		{0x05, 4000},
-		{0x06, 3000},
-		{0x07, 2000},
-	},
-	/* KXTF9 */
-	[KXTF9] = {
-		{0x01, 81000},
-		{0x02, 41000},
-		{0x03, 21000},
-		{0x04, 11000},
-		{0x05, 5100},
-		{0x06, 2700},
-	},
-	/* KX023-1025 */
-	[KX0231025] = {
-		/* First 4 are not in datasheet, taken from KXCTJ2-1009 */
-		{0x08, 1240000},
-		{0x09, 621000},
-		{0x0A, 309000},
-		{0x0B, 151000},
-		{0, 81000},
-		{0x01, 40000},
-		{0x02, 22000},
-		{0x03, 12000},
-		{0x04, 7000},
-		{0x05, 4400},
-		{0x06, 3000},
-		{0x07, 3000},
-	},
+};
+
+/* KXCJK-1013 */
+static const struct kx_odr_start_up_time kxcjk1013_odr_start_up_times[] = {
+	{0x08, 100000},
+	{0x09, 100000},
+	{0x0A, 100000},
+	{0x0B, 100000},
+	{0, 80000},
+	{0x01, 41000},
+	{0x02, 21000},
+	{0x03, 11000},
+	{0x04, 6400},
+	{0x05, 3900},
+	{0x06, 2700},
+	{0x07, 2100},
+};
+
+/* KXCJ9-1008 */
+static const struct kx_odr_start_up_time kxcj91008_odr_start_up_times[] = {
+	{0x08, 100000},
+	{0x09, 100000},
+	{0x0A, 100000},
+	{0x0B, 100000},
+	{0, 80000},
+	{0x01, 41000},
+	{0x02, 21000},
+	{0x03, 11000},
+	{0x04, 6400},
+	{0x05, 3900},
+	{0x06, 2700},
+	{0x07, 2100},
+};
+
+/* KXCTJ2-1009 */
+static const struct kx_odr_start_up_time kxtj21009_odr_start_up_times[] = {
+	{0x08, 1240000},
+	{0x09, 621000},
+	{0x0A, 309000},
+	{0x0B, 151000},
+	{0, 80000},
+	{0x01, 41000},
+	{0x02, 21000},
+	{0x03, 11000},
+	{0x04, 6000},
+	{0x05, 4000},
+	{0x06, 3000},
+	{0x07, 2000},
+};
+
+/* KXTF9 */
+static const struct kx_odr_start_up_time kxtf9_odr_start_up_times[] = {
+	{0x01, 81000},
+	{0x02, 41000},
+	{0x03, 21000},
+	{0x04, 11000},
+	{0x05, 5100},
+	{0x06, 2700},
+};
+
+/* KX023-1025 */
+static const struct kx_odr_start_up_time kx0231025_odr_start_up_times[] = {
+	/* First 4 are not in datasheet, taken from KXCTJ2-1009 */
+	{0x08, 1240000},
+	{0x09, 621000},
+	{0x0A, 309000},
+	{0x0B, 151000},
+	{0, 81000},
+	{0x01, 40000},
+	{0x02, 22000},
+	{0x03, 12000},
+	{0x04, 7000},
+	{0x05, 4400},
+	{0x06, 3000},
+	{0x07, 3000},
 };
 
 enum kx_acpi_type {
@@ -313,50 +317,59 @@ static const struct kx_chipset_regs kx0231025_regs = {
 
 struct kx_chipset_info {
 	const struct kx_chipset_regs *regs;
+	const struct kx_odr_start_up_time *times;
 	enum kx_chipset chipset;
 	enum kx_acpi_type acpi_type;
 };
 
 static const struct kx_chipset_info kxcjk1013_info = {
 	.regs = &kxcjk1013_regs,
+	.times = pm_ptr(kxcjk1013_odr_start_up_times),
 	.chipset = KXCJK1013,
 };
 
 static const struct kx_chipset_info kxcj91008_info = {
 	.regs = &kxcjk1013_regs,
+	.times = pm_ptr(kxcj91008_odr_start_up_times),
 	.chipset = KXCJ91008,
 };
 
 static const struct kx_chipset_info kxcj91008_kiox010a_info = {
 	.regs = &kxcjk1013_regs,
+	.times = pm_ptr(kxcj91008_odr_start_up_times),
 	.chipset = KXCJ91008,
 	.acpi_type = ACPI_KIOX010A,
 };
 
 static const struct kx_chipset_info kxcj91008_kiox020a_info = {
 	.regs = &kxcjk1013_regs,
+	.times = pm_ptr(kxcj91008_odr_start_up_times),
 	.chipset = KXCJ91008,
 	.acpi_type = ACPI_GENERIC,
 };
 
 static const struct kx_chipset_info kxcj91008_smo8500_info = {
 	.regs = &kxcjk1013_regs,
+	.times = pm_ptr(kxcj91008_odr_start_up_times),
 	.chipset = KXCJ91008,
 	.acpi_type = ACPI_SMO8500,
 };
 
 static const struct kx_chipset_info kxtj21009_info = {
 	.regs = &kxcjk1013_regs,
+	.times = pm_ptr(kxtj21009_odr_start_up_times),
 	.chipset = KXTJ21009,
 };
 
 static const struct kx_chipset_info kxtf9_info = {
 	.regs = &kxtf9_regs,
+	.times = pm_ptr(kxtf9_odr_start_up_times),
 	.chipset = KXTF9,
 };
 
 static const struct kx_chipset_info kx0231025_info = {
 	.regs = &kx0231025_regs,
+	.times = pm_ptr(kx0231025_odr_start_up_times),
 	.chipset = KX0231025,
 };
 
@@ -655,12 +668,11 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 
 static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
 {
-	int idx = data->info->chipset;
-	int i;
+	const struct kx_odr_start_up_time *times;
 
-	for (i = 0; i < ARRAY_SIZE(odr_start_up_times[idx]); ++i) {
-		if (odr_start_up_times[idx][i].odr_bits == data->odr_bits)
-			return odr_start_up_times[idx][i].usec;
+	for (times = data->info->times; times->usec; times++) {
+		if (times->odr_bits == data->odr_bits)
+			return times->usec;
 	}
 
 	return KXCJK1013_MAX_STARTUP_TIME_US;
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 13/24] iio: accel: kxcjk-1013: Convert ODR times array to variable in chip_info

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:02 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Convert odr_start_up_times array to the variable in chip_info.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Actually scratch the white space changes in previous. I'll do them here
instead and avoid the merge fun the earlier change would make.

Applied with 0x00 instead of 0 and spaces after { and before }.

Jonathan

> ---
>  drivers/iio/accel/kxcjk-1013.c | 166 ++++++++++++++++++---------------
>  1 file changed, 89 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index f384969a922d..91f890275a18 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -179,80 +179,84 @@ enum kx_chipset {
>  };
>  
>  /* Refer to section 4 of the specification */
> -static const struct {
> +struct kx_odr_start_up_time {
>  	int odr_bits;
>  	int usec;
> -} odr_start_up_times[KX_MAX_CHIPS][12] = {
> -	/* KXCJK-1013 */
> -	[KXCJK1013] = {
> -		{0x08, 100000},
> -		{0x09, 100000},
> -		{0x0A, 100000},
> -		{0x0B, 100000},
> -		{0, 80000},
> -		{0x01, 41000},
> -		{0x02, 21000},
> -		{0x03, 11000},
> -		{0x04, 6400},
> -		{0x05, 3900},
> -		{0x06, 2700},
> -		{0x07, 2100},
> -	},
> -	/* KXCJ9-1008 */
> -	[KXCJ91008] = {
> -		{0x08, 100000},
> -		{0x09, 100000},
> -		{0x0A, 100000},
> -		{0x0B, 100000},
> -		{0, 80000},
> -		{0x01, 41000},
> -		{0x02, 21000},
> -		{0x03, 11000},
> -		{0x04, 6400},
> -		{0x05, 3900},
> -		{0x06, 2700},
> -		{0x07, 2100},
> -	},
> -	/* KXCTJ2-1009 */
> -	[KXTJ21009] = {
> -		{0x08, 1240000},
> -		{0x09, 621000},
> -		{0x0A, 309000},
> -		{0x0B, 151000},
> -		{0, 80000},
> -		{0x01, 41000},
> -		{0x02, 21000},
> -		{0x03, 11000},
> -		{0x04, 6000},
> -		{0x05, 4000},
> -		{0x06, 3000},
> -		{0x07, 2000},
> -	},
> -	/* KXTF9 */
> -	[KXTF9] = {
> -		{0x01, 81000},
> -		{0x02, 41000},
> -		{0x03, 21000},
> -		{0x04, 11000},
> -		{0x05, 5100},
> -		{0x06, 2700},
> -	},
> -	/* KX023-1025 */
> -	[KX0231025] = {
> -		/* First 4 are not in datasheet, taken from KXCTJ2-1009 */
> -		{0x08, 1240000},
> -		{0x09, 621000},
> -		{0x0A, 309000},
> -		{0x0B, 151000},
> -		{0, 81000},
> -		{0x01, 40000},
> -		{0x02, 22000},
> -		{0x03, 12000},
> -		{0x04, 7000},
> -		{0x05, 4400},
> -		{0x06, 3000},
> -		{0x07, 3000},
> -	},
> +};
> +
> +/* KXCJK-1013 */
> +static const struct kx_odr_start_up_time kxcjk1013_odr_start_up_times[] = {
> +	{0x08, 100000},
> +	{0x09, 100000},
> +	{0x0A, 100000},
> +	{0x0B, 100000},
> +	{0, 80000},
> +	{0x01, 41000},
> +	{0x02, 21000},
> +	{0x03, 11000},
> +	{0x04, 6400},
> +	{0x05, 3900},
> +	{0x06, 2700},
> +	{0x07, 2100},
> +};
> +
> +/* KXCJ9-1008 */
> +static const struct kx_odr_start_up_time kxcj91008_odr_start_up_times[] = {
> +	{0x08, 100000},
> +	{0x09, 100000},
> +	{0x0A, 100000},
> +	{0x0B, 100000},
> +	{0, 80000},
> +	{0x01, 41000},
> +	{0x02, 21000},
> +	{0x03, 11000},
> +	{0x04, 6400},
> +	{0x05, 3900},
> +	{0x06, 2700},
> +	{0x07, 2100},
> +};
> +
> +/* KXCTJ2-1009 */
> +static const struct kx_odr_start_up_time kxtj21009_odr_start_up_times[] = {
> +	{0x08, 1240000},
> +	{0x09, 621000},
> +	{0x0A, 309000},
> +	{0x0B, 151000},
> +	{0, 80000},
> +	{0x01, 41000},
> +	{0x02, 21000},
> +	{0x03, 11000},
> +	{0x04, 6000},
> +	{0x05, 4000},
> +	{0x06, 3000},
> +	{0x07, 2000},
> +};
> +
> +/* KXTF9 */
> +static const struct kx_odr_start_up_time kxtf9_odr_start_up_times[] = {
> +	{0x01, 81000},
> +	{0x02, 41000},
> +	{0x03, 21000},
> +	{0x04, 11000},
> +	{0x05, 5100},
> +	{0x06, 2700},
> +};
> +
> +/* KX023-1025 */
> +static const struct kx_odr_start_up_time kx0231025_odr_start_up_times[] = {
> +	/* First 4 are not in datasheet, taken from KXCTJ2-1009 */
> +	{0x08, 1240000},
> +	{0x09, 621000},
> +	{0x0A, 309000},
> +	{0x0B, 151000},
> +	{0, 81000},
> +	{0x01, 40000},
> +	{0x02, 22000},
> +	{0x03, 12000},
> +	{0x04, 7000},
> +	{0x05, 4400},
> +	{0x06, 3000},
> +	{0x07, 3000},
>  };
>  
>  enum kx_acpi_type {
> @@ -313,50 +317,59 @@ static const struct kx_chipset_regs kx0231025_regs = {
>  
>  struct kx_chipset_info {
>  	const struct kx_chipset_regs *regs;
> +	const struct kx_odr_start_up_time *times;
>  	enum kx_chipset chipset;
>  	enum kx_acpi_type acpi_type;
>  };
>  
>  static const struct kx_chipset_info kxcjk1013_info = {
>  	.regs = &kxcjk1013_regs,
> +	.times = pm_ptr(kxcjk1013_odr_start_up_times),
>  	.chipset = KXCJK1013,
>  };
>  
>  static const struct kx_chipset_info kxcj91008_info = {
>  	.regs = &kxcjk1013_regs,
> +	.times = pm_ptr(kxcj91008_odr_start_up_times),
>  	.chipset = KXCJ91008,
>  };
>  
>  static const struct kx_chipset_info kxcj91008_kiox010a_info = {
>  	.regs = &kxcjk1013_regs,
> +	.times = pm_ptr(kxcj91008_odr_start_up_times),
>  	.chipset = KXCJ91008,
>  	.acpi_type = ACPI_KIOX010A,
>  };
>  
>  static const struct kx_chipset_info kxcj91008_kiox020a_info = {
>  	.regs = &kxcjk1013_regs,
> +	.times = pm_ptr(kxcj91008_odr_start_up_times),
>  	.chipset = KXCJ91008,
>  	.acpi_type = ACPI_GENERIC,
>  };
>  
>  static const struct kx_chipset_info kxcj91008_smo8500_info = {
>  	.regs = &kxcjk1013_regs,
> +	.times = pm_ptr(kxcj91008_odr_start_up_times),
>  	.chipset = KXCJ91008,
>  	.acpi_type = ACPI_SMO8500,
>  };
>  
>  static const struct kx_chipset_info kxtj21009_info = {
>  	.regs = &kxcjk1013_regs,
> +	.times = pm_ptr(kxtj21009_odr_start_up_times),
>  	.chipset = KXTJ21009,
>  };
>  
>  static const struct kx_chipset_info kxtf9_info = {
>  	.regs = &kxtf9_regs,
> +	.times = pm_ptr(kxtf9_odr_start_up_times),
>  	.chipset = KXTF9,
>  };
>  
>  static const struct kx_chipset_info kx0231025_info = {
>  	.regs = &kx0231025_regs,
> +	.times = pm_ptr(kx0231025_odr_start_up_times),
>  	.chipset = KX0231025,
>  };
>  
> @@ -655,12 +668,11 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  
>  static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
>  {
> -	int idx = data->info->chipset;
> -	int i;
> +	const struct kx_odr_start_up_time *times;
>  
> -	for (i = 0; i < ARRAY_SIZE(odr_start_up_times[idx]); ++i) {
> -		if (odr_start_up_times[idx][i].odr_bits == data->odr_bits)
> -			return odr_start_up_times[idx][i].usec;
> +	for (times = data->info->times; times->usec; times++) {
> +		if (times->odr_bits == data->odr_bits)
> +			return times->usec;
>  	}
>  
>  	return KXCJK1013_MAX_STARTUP_TIME_US;

[PATCH v3 14/24] iio: accel: kxcjk-1013: Get rid of enum kx_chipset

[Andy Shevchenko]

Instead of using enum, out of which only a couple of values
are being actully used, make a comparisons against pointer
to the respective chip_info structures.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 28 +++++-----------------------
 1 file changed, 5 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 91f890275a18..a8c73e401f95 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -169,15 +169,6 @@
 
 #define KXCJK1013_DEFAULT_WAKE_THRES	1
 
-enum kx_chipset {
-	KXCJK1013,
-	KXCJ91008,
-	KXTJ21009,
-	KXTF9,
-	KX0231025,
-	KX_MAX_CHIPS /* this must be last */
-};
-
 /* Refer to section 4 of the specification */
 struct kx_odr_start_up_time {
 	int odr_bits;
@@ -318,59 +309,50 @@ static const struct kx_chipset_regs kx0231025_regs = {
 struct kx_chipset_info {
 	const struct kx_chipset_regs *regs;
 	const struct kx_odr_start_up_time *times;
-	enum kx_chipset chipset;
 	enum kx_acpi_type acpi_type;
 };
 
 static const struct kx_chipset_info kxcjk1013_info = {
 	.regs = &kxcjk1013_regs,
 	.times = pm_ptr(kxcjk1013_odr_start_up_times),
-	.chipset = KXCJK1013,
 };
 
 static const struct kx_chipset_info kxcj91008_info = {
 	.regs = &kxcjk1013_regs,
 	.times = pm_ptr(kxcj91008_odr_start_up_times),
-	.chipset = KXCJ91008,
 };
 
 static const struct kx_chipset_info kxcj91008_kiox010a_info = {
 	.regs = &kxcjk1013_regs,
 	.times = pm_ptr(kxcj91008_odr_start_up_times),
-	.chipset = KXCJ91008,
 	.acpi_type = ACPI_KIOX010A,
 };
 
 static const struct kx_chipset_info kxcj91008_kiox020a_info = {
 	.regs = &kxcjk1013_regs,
 	.times = pm_ptr(kxcj91008_odr_start_up_times),
-	.chipset = KXCJ91008,
 	.acpi_type = ACPI_GENERIC,
 };
 
 static const struct kx_chipset_info kxcj91008_smo8500_info = {
 	.regs = &kxcjk1013_regs,
 	.times = pm_ptr(kxcj91008_odr_start_up_times),
-	.chipset = KXCJ91008,
 	.acpi_type = ACPI_SMO8500,
 };
 
 static const struct kx_chipset_info kxtj21009_info = {
 	.regs = &kxcjk1013_regs,
 	.times = pm_ptr(kxtj21009_odr_start_up_times),
-	.chipset = KXTJ21009,
 };
 
 static const struct kx_chipset_info kxtf9_info = {
 	.regs = &kxtf9_regs,
 	.times = pm_ptr(kxtf9_odr_start_up_times),
-	.chipset = KXTF9,
 };
 
 static const struct kx_chipset_info kx0231025_info = {
 	.regs = &kx0231025_regs,
 	.times = pm_ptr(kx0231025_odr_start_up_times),
-	.chipset = KX0231025,
 };
 
 enum kxcjk1013_axis {
@@ -647,7 +629,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	}
 
 	/* On KX023, route all used interrupts to INT1 for now */
-	if (data->info->chipset == KX0231025 && data->client->irq > 0) {
+	if (data->info == &kx0231025_info && data->client->irq > 0) {
 		ret = i2c_smbus_write_byte_data(data->client, KX023_REG_INC4,
 						KX023_REG_INC4_DRDY1 |
 						KX023_REG_INC4_WUFI1);
@@ -883,7 +865,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 	if (ret < 0)
 		return ret;
 
-	if (data->info->chipset == KXTF9)
+	if (data->info == &kxtf9_info)
 		odr_setting = kxcjk1013_find_odr_value(kxtf9_samp_freq_table,
 						       ARRAY_SIZE(kxtf9_samp_freq_table),
 						       val, val2);
@@ -927,7 +909,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 
 static int kxcjk1013_get_odr(struct kxcjk1013_data *data, int *val, int *val2)
 {
-	if (data->info->chipset == KXTF9)
+	if (data->info == &kxtf9_info)
 		return kxcjk1013_convert_odr_value(kxtf9_samp_freq_table,
 						   ARRAY_SIZE(kxtf9_samp_freq_table),
 						   data->odr_bits, val, val2);
@@ -1194,7 +1176,7 @@ static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
 	const char *str;
 
-	if (data->info->chipset == KXTF9)
+	if (data->info == &kxtf9_info)
 		str = kxtf9_samp_freq_avail;
 	else
 		str = kxcjk1013_samp_freq_avail;
@@ -1443,7 +1425,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 	}
 
 	if (ret & KXCJK1013_REG_INT_SRC1_BIT_WUFS) {
-		if (data->info->chipset == KXTF9)
+		if (data->info == &kxtf9_info)
 			iio_push_event(indio_dev,
 				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 				       0,
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 14/24] iio: accel: kxcjk-1013: Get rid of enum kx_chipset

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:03 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Instead of using enum, out of which only a couple of values
> are being actully used, make a comparisons against pointer
> to the respective chip_info structures.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Hmm. Maybe worth a revisit longer term to use callbacks for the
decisions instead of matching at all as this stuff tends not to end
up being specific to one version of a chip as more parts are added.

Anyhow, this is still an improvement so applied.

Jonathan

> ---
>  drivers/iio/accel/kxcjk-1013.c | 28 +++++-----------------------
>  1 file changed, 5 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 91f890275a18..a8c73e401f95 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -169,15 +169,6 @@
>  
>  #define KXCJK1013_DEFAULT_WAKE_THRES	1
>  
> -enum kx_chipset {
> -	KXCJK1013,
> -	KXCJ91008,
> -	KXTJ21009,
> -	KXTF9,
> -	KX0231025,
> -	KX_MAX_CHIPS /* this must be last */
> -};
> -
>  /* Refer to section 4 of the specification */
>  struct kx_odr_start_up_time {
>  	int odr_bits;
> @@ -318,59 +309,50 @@ static const struct kx_chipset_regs kx0231025_regs = {
>  struct kx_chipset_info {
>  	const struct kx_chipset_regs *regs;
>  	const struct kx_odr_start_up_time *times;
> -	enum kx_chipset chipset;
>  	enum kx_acpi_type acpi_type;
>  };
>  
>  static const struct kx_chipset_info kxcjk1013_info = {
>  	.regs = &kxcjk1013_regs,
>  	.times = pm_ptr(kxcjk1013_odr_start_up_times),
> -	.chipset = KXCJK1013,
>  };
>  
>  static const struct kx_chipset_info kxcj91008_info = {
>  	.regs = &kxcjk1013_regs,
>  	.times = pm_ptr(kxcj91008_odr_start_up_times),
> -	.chipset = KXCJ91008,
>  };
>  
>  static const struct kx_chipset_info kxcj91008_kiox010a_info = {
>  	.regs = &kxcjk1013_regs,
>  	.times = pm_ptr(kxcj91008_odr_start_up_times),
> -	.chipset = KXCJ91008,
>  	.acpi_type = ACPI_KIOX010A,
>  };
>  
>  static const struct kx_chipset_info kxcj91008_kiox020a_info = {
>  	.regs = &kxcjk1013_regs,
>  	.times = pm_ptr(kxcj91008_odr_start_up_times),
> -	.chipset = KXCJ91008,
>  	.acpi_type = ACPI_GENERIC,
>  };
>  
>  static const struct kx_chipset_info kxcj91008_smo8500_info = {
>  	.regs = &kxcjk1013_regs,
>  	.times = pm_ptr(kxcj91008_odr_start_up_times),
> -	.chipset = KXCJ91008,
>  	.acpi_type = ACPI_SMO8500,
>  };
>  
>  static const struct kx_chipset_info kxtj21009_info = {
>  	.regs = &kxcjk1013_regs,
>  	.times = pm_ptr(kxtj21009_odr_start_up_times),
> -	.chipset = KXTJ21009,
>  };
>  
>  static const struct kx_chipset_info kxtf9_info = {
>  	.regs = &kxtf9_regs,
>  	.times = pm_ptr(kxtf9_odr_start_up_times),
> -	.chipset = KXTF9,
>  };
>  
>  static const struct kx_chipset_info kx0231025_info = {
>  	.regs = &kx0231025_regs,
>  	.times = pm_ptr(kx0231025_odr_start_up_times),
> -	.chipset = KX0231025,
>  };
>  
>  enum kxcjk1013_axis {
> @@ -647,7 +629,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  	}
>  
>  	/* On KX023, route all used interrupts to INT1 for now */
> -	if (data->info->chipset == KX0231025 && data->client->irq > 0) {
> +	if (data->info == &kx0231025_info && data->client->irq > 0) {
>  		ret = i2c_smbus_write_byte_data(data->client, KX023_REG_INC4,
>  						KX023_REG_INC4_DRDY1 |
>  						KX023_REG_INC4_WUFI1);
> @@ -883,7 +865,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
>  	if (ret < 0)
>  		return ret;
>  
> -	if (data->info->chipset == KXTF9)
> +	if (data->info == &kxtf9_info)
>  		odr_setting = kxcjk1013_find_odr_value(kxtf9_samp_freq_table,
>  						       ARRAY_SIZE(kxtf9_samp_freq_table),
>  						       val, val2);
> @@ -927,7 +909,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
>  
>  static int kxcjk1013_get_odr(struct kxcjk1013_data *data, int *val, int *val2)
>  {
> -	if (data->info->chipset == KXTF9)
> +	if (data->info == &kxtf9_info)
>  		return kxcjk1013_convert_odr_value(kxtf9_samp_freq_table,
>  						   ARRAY_SIZE(kxtf9_samp_freq_table),
>  						   data->odr_bits, val, val2);
> @@ -1194,7 +1176,7 @@ static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
>  	struct kxcjk1013_data *data = iio_priv(indio_dev);
>  	const char *str;
>  
> -	if (data->info->chipset == KXTF9)
> +	if (data->info == &kxtf9_info)
>  		str = kxtf9_samp_freq_avail;
>  	else
>  		str = kxcjk1013_samp_freq_avail;
> @@ -1443,7 +1425,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
>  	}
>  
>  	if (ret & KXCJK1013_REG_INT_SRC1_BIT_WUFS) {
> -		if (data->info->chipset == KXTF9)
> +		if (data->info == &kxtf9_info)
>  			iio_push_event(indio_dev,
>  				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  				       0,

Re: [PATCH v3 14/24] iio: accel: kxcjk-1013: Get rid of enum kx_chipset

[Andy Shevchenko]

On Sat, Oct 26, 2024 at 12:37:36PM +0100, Jonathan Cameron wrote:
> On Thu, 24 Oct 2024 22:05:03 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Instead of using enum, out of which only a couple of values
> > are being actully used, make a comparisons against pointer
> > to the respective chip_info structures.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Hmm. Maybe worth a revisit longer term to use callbacks for the
> decisions instead of matching at all as this stuff tends not to end
> up being specific to one version of a chip as more parts are added.

As already mentioned in another reply, yes, the callbacks is the future.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 15/24] iio: accel: kxcjk-1013: Replace a variant of iio_get_acpi_device_name_and_data()

[Andy Shevchenko]

IIO core (ACPI part) provides a generic helper that may be used in
the driver. Replace a variant of iio_get_acpi_device_name_and_data().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 38 ++++++++++------------------------
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index a8c73e401f95..e8074e0c59a4 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1466,26 +1466,6 @@ static irqreturn_t kxcjk1013_data_rdy_trig_poll(int irq, void *private)
 		return IRQ_HANDLED;
 }
 
-static const char *kxcjk1013_match_acpi_device(struct device *dev,
-					       const struct kx_chipset_info **info,
-					       const char **label)
-{
-	const struct acpi_device_id *id;
-
-	id = acpi_match_device(dev->driver->acpi_match_table, dev);
-	if (!id)
-		return NULL;
-
-	if (strcmp(id->id, "KIOX010A") == 0)
-		*label = "accel-display";
-	else if (strcmp(id->id, "KIOX020A") == 0)
-		*label = "accel-base";
-
-	*info = (const struct kx_chipset_info *)id->driver_data;
-
-	return dev_name(dev);
-}
-
 static int kxcjk1013_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
@@ -1493,6 +1473,7 @@ static int kxcjk1013_probe(struct i2c_client *client)
 	struct kxcjk1013_data *data;
 	struct iio_dev *indio_dev;
 	struct kxcjk_1013_platform_data *pdata;
+	const void *ddata;
 	const char *name;
 	int ret;
 
@@ -1535,15 +1516,18 @@ static int kxcjk1013_probe(struct i2c_client *client)
 	if (id) {
 		name = id->name;
 		data->info = (const struct kx_chipset_info *)(id->driver_data);
-	} else if (ACPI_HANDLE(&client->dev)) {
-		name = kxcjk1013_match_acpi_device(&client->dev, &data->info,
-						   &indio_dev->label);
-	} else
+	} else {
+		name = iio_get_acpi_device_name_and_data(&client->dev, &ddata);
+		if (name)
+			data->info = ddata;
+		if (data->info == &kxcj91008_kiox010a_info)
+			indio_dev->label = "accel-display";
+		else if (data->info == &kxcj91008_kiox020a_info)
+			indio_dev->label = "accel-base";
+	}
+	if (!name)
 		return -ENODEV;
 
-	if (!data->info)
-		return -EINVAL;
-
 	ret = kxcjk1013_chip_init(data);
 	if (ret < 0)
 		return ret;
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 15/24] iio: accel: kxcjk-1013: Replace a variant of iio_get_acpi_device_name_and_data()

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:04 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> IIO core (ACPI part) provides a generic helper that may be used in
> the driver. Replace a variant of iio_get_acpi_device_name_and_data().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied.

[PATCH v3 16/24] iio: accel: kxcjk-1013: drop ACPI_PTR() and move ID out of CONFIG_ACPI guards

[Andy Shevchenko]

The complexity of config guards needed for ACPI_PTR() is not worthwhile
for the small amount of saved data. This example was doing it correctly
but I am proposing dropping this so as to reduce chance of cut and paste
where it is done wrong. Also added linux/mod_devicetable.h for
struct acpi_device_id definition.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 41 +++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index e8074e0c59a4..baa2674b93f0 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -4,11 +4,12 @@
  * Copyright (c) 2014, Intel Corporation.
  */
 
-#include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/bitops.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/types.h>
@@ -465,24 +466,6 @@ static int kiox010a_dsm(struct device *dev, int fn_index)
 	ACPI_FREE(obj);
 	return 0;
 }
-
-static const struct acpi_device_id kx_acpi_match[] = {
-	{"KIOX0008", (kernel_ulong_t)&kxcj91008_info },
-	{"KIOX0009", (kernel_ulong_t)&kxtj21009_info },
-	{"KIOX000A", (kernel_ulong_t)&kxcj91008_info },
-	/* KXCJ91008 in the display of a yoga 2-in-1 */
-	{"KIOX010A", (kernel_ulong_t)&kxcj91008_kiox010a_info },
-	/* KXCJ91008 in the base of a yoga 2-in-1 */
-	{"KIOX020A", (kernel_ulong_t)&kxcj91008_kiox020a_info },
-	{"KXCJ1008", (kernel_ulong_t)&kxcj91008_info },
-	{"KXCJ1013", (kernel_ulong_t)&kxcjk1013_info },
-	{"KXCJ9000", (kernel_ulong_t)&kxcj91008_info },
-	{"KXJ2109",  (kernel_ulong_t)&kxtj21009_info },
-	{"KXTJ1009", (kernel_ulong_t)&kxtj21009_info },
-	{"SMO8500",  (kernel_ulong_t)&kxcj91008_smo8500_info },
-	{ }
-};
-MODULE_DEVICE_TABLE(acpi, kx_acpi_match);
 #endif
 
 static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
@@ -1736,10 +1719,28 @@ static const struct of_device_id kxcjk1013_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, kxcjk1013_of_match);
 
+static const struct acpi_device_id kx_acpi_match[] = {
+	{"KIOX0008", (kernel_ulong_t)&kxcj91008_info },
+	{"KIOX0009", (kernel_ulong_t)&kxtj21009_info },
+	{"KIOX000A", (kernel_ulong_t)&kxcj91008_info },
+	/* KXCJ91008 in the display of a yoga 2-in-1 */
+	{"KIOX010A", (kernel_ulong_t)&kxcj91008_kiox010a_info },
+	/* KXCJ91008 in the base of a yoga 2-in-1 */
+	{"KIOX020A", (kernel_ulong_t)&kxcj91008_kiox020a_info },
+	{"KXCJ1008", (kernel_ulong_t)&kxcj91008_info },
+	{"KXCJ1013", (kernel_ulong_t)&kxcjk1013_info },
+	{"KXCJ9000", (kernel_ulong_t)&kxcj91008_info },
+	{"KXJ2109",  (kernel_ulong_t)&kxtj21009_info },
+	{"KXTJ1009", (kernel_ulong_t)&kxtj21009_info },
+	{"SMO8500",  (kernel_ulong_t)&kxcj91008_smo8500_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, kx_acpi_match);
+
 static struct i2c_driver kxcjk1013_driver = {
 	.driver = {
 		.name	= KXCJK1013_DRV_NAME,
-		.acpi_match_table = ACPI_PTR(kx_acpi_match),
+		.acpi_match_table = kx_acpi_match,
 		.of_match_table = kxcjk1013_of_match,
 		.pm	= pm_ptr(&kxcjk1013_pm_ops),
 	},
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 16/24] iio: accel: kxcjk-1013: drop ACPI_PTR() and move ID out of CONFIG_ACPI guards

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:05 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> The complexity of config guards needed for ACPI_PTR() is not worthwhile
> for the small amount of saved data. This example was doing it correctly
> but I am proposing dropping this so as to reduce chance of cut and paste
> where it is done wrong. Also added linux/mod_devicetable.h for
> struct acpi_device_id definition.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Ah. so my white space messing around earlier made this slightly harder to apply.
I carried forward the spaces after {

Applied,

Thanks,

Jonathan

[PATCH v3 17/24] iio: accel: mma9551: Replace custom implementation of iio_get_acpi_device_name()

[Andy Shevchenko]

IIO core (ACPI part) provides a generic helper that may be used in
the driver. Replace custom implementation of iio_get_acpi_device_name().

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/mma9551.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/accel/mma9551.c b/drivers/iio/accel/mma9551.c
index fa1799b0b0df..a5d20d8d08b8 100644
--- a/drivers/iio/accel/mma9551.c
+++ b/drivers/iio/accel/mma9551.c
@@ -4,11 +4,11 @@
  * Copyright (c) 2014, Intel Corporation.
  */
 
-#include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
 #include <linux/slab.h>
-#include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/iio/iio.h>
@@ -435,17 +435,6 @@ static int mma9551_gpio_probe(struct iio_dev *indio_dev)
 	return 0;
 }
 
-static const char *mma9551_match_acpi_device(struct device *dev)
-{
-	const struct acpi_device_id *id;
-
-	id = acpi_match_device(dev->driver->acpi_match_table, dev);
-	if (!id)
-		return NULL;
-
-	return dev_name(dev);
-}
-
 static int mma9551_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
@@ -464,8 +453,8 @@ static int mma9551_probe(struct i2c_client *client)
 
 	if (id)
 		name = id->name;
-	else if (ACPI_HANDLE(&client->dev))
-		name = mma9551_match_acpi_device(&client->dev);
+	else
+		name = iio_get_acpi_device_name(&client->dev);
 
 	ret = mma9551_init(data);
 	if (ret < 0)
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 17/24] iio: accel: mma9551: Replace custom implementation of iio_get_acpi_device_name()

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:06 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> IIO core (ACPI part) provides a generic helper that may be used in
> the driver. Replace custom implementation of iio_get_acpi_device_name().
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied.

> ---
>  drivers/iio/accel/mma9551.c | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9551.c b/drivers/iio/accel/mma9551.c
> index fa1799b0b0df..a5d20d8d08b8 100644
> --- a/drivers/iio/accel/mma9551.c
> +++ b/drivers/iio/accel/mma9551.c
> @@ -4,11 +4,11 @@
>   * Copyright (c) 2014, Intel Corporation.
>   */
>  
> -#include <linux/module.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
>  #include <linux/slab.h>
> -#include <linux/acpi.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/iio/iio.h>
> @@ -435,17 +435,6 @@ static int mma9551_gpio_probe(struct iio_dev *indio_dev)
>  	return 0;
>  }
>  
> -static const char *mma9551_match_acpi_device(struct device *dev)
> -{
> -	const struct acpi_device_id *id;
> -
> -	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> -	if (!id)
> -		return NULL;
> -
> -	return dev_name(dev);
> -}
> -
>  static int mma9551_probe(struct i2c_client *client)
>  {
>  	const struct i2c_device_id *id = i2c_client_get_device_id(client);
> @@ -464,8 +453,8 @@ static int mma9551_probe(struct i2c_client *client)
>  
>  	if (id)
>  		name = id->name;
> -	else if (ACPI_HANDLE(&client->dev))
> -		name = mma9551_match_acpi_device(&client->dev);
> +	else
> +		name = iio_get_acpi_device_name(&client->dev);
>  
>  	ret = mma9551_init(data);
>  	if (ret < 0)

[PATCH v3 18/24] iio: accel: mma9553: Replace custom implementation of iio_get_acpi_device_name()

[Andy Shevchenko]

IIO core (ACPI part) provides a generic helper that may be used in
the driver. Replace custom implementation of iio_get_acpi_device_name().

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/mma9553.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 86543f34ef17..1ea6aa007412 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -4,11 +4,11 @@
  * Copyright (c) 2014, Intel Corporation.
  */
 
-#include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
 #include <linux/slab.h>
-#include <linux/acpi.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/events.h>
@@ -1062,17 +1062,6 @@ static irqreturn_t mma9553_event_handler(int irq, void *private)
 	return IRQ_HANDLED;
 }
 
-static const char *mma9553_match_acpi_device(struct device *dev)
-{
-	const struct acpi_device_id *id;
-
-	id = acpi_match_device(dev->driver->acpi_match_table, dev);
-	if (!id)
-		return NULL;
-
-	return dev_name(dev);
-}
-
 static int mma9553_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
@@ -1091,9 +1080,9 @@ static int mma9553_probe(struct i2c_client *client)
 
 	if (id)
 		name = id->name;
-	else if (ACPI_HANDLE(&client->dev))
-		name = mma9553_match_acpi_device(&client->dev);
 	else
+		name = iio_get_acpi_device_name(&client->dev);
+	if (!name)
 		return -ENOSYS;
 
 	mutex_init(&data->mutex);
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 18/24] iio: accel: mma9553: Replace custom implementation of iio_get_acpi_device_name()

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:07 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> IIO core (ACPI part) provides a generic helper that may be used in
> the driver. Replace custom implementation of iio_get_acpi_device_name().
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied.

[PATCH v3 19/24] iio: gyro: bmg160: Replace custom implementation of iio_get_acpi_device_name()

[Andy Shevchenko]

IIO core (ACPI part) provides a generic helper that may be used in
the driver. Replace custom implementation of iio_get_acpi_device_name().

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/gyro/bmg160_core.c | 15 ---------------
 drivers/iio/gyro/bmg160_i2c.c  |  4 +++-
 2 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
index 10728d5ccae3..499078537fa4 100644
--- a/drivers/iio/gyro/bmg160_core.c
+++ b/drivers/iio/gyro/bmg160_core.c
@@ -8,7 +8,6 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
-#include <linux/acpi.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/iio/iio.h>
@@ -1055,17 +1054,6 @@ static const struct iio_buffer_setup_ops bmg160_buffer_setup_ops = {
 	.postdisable = bmg160_buffer_postdisable,
 };
 
-static const char *bmg160_match_acpi_device(struct device *dev)
-{
-	const struct acpi_device_id *id;
-
-	id = acpi_match_device(dev->driver->acpi_match_table, dev);
-	if (!id)
-		return NULL;
-
-	return dev_name(dev);
-}
-
 int bmg160_core_probe(struct device *dev, struct regmap *regmap, int irq,
 		      const char *name)
 {
@@ -1098,9 +1086,6 @@ int bmg160_core_probe(struct device *dev, struct regmap *regmap, int irq,
 
 	mutex_init(&data->mutex);
 
-	if (ACPI_HANDLE(dev))
-		name = bmg160_match_acpi_device(dev);
-
 	indio_dev->channels = bmg160_channels;
 	indio_dev->num_channels = ARRAY_SIZE(bmg160_channels);
 	indio_dev->name = name;
diff --git a/drivers/iio/gyro/bmg160_i2c.c b/drivers/iio/gyro/bmg160_i2c.c
index a81814df5205..9c5d7e8ee99c 100644
--- a/drivers/iio/gyro/bmg160_i2c.c
+++ b/drivers/iio/gyro/bmg160_i2c.c
@@ -17,7 +17,7 @@ static int bmg160_i2c_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
 	struct regmap *regmap;
-	const char *name = NULL;
+	const char *name;
 
 	regmap = devm_regmap_init_i2c(client, &bmg160_regmap_i2c_conf);
 	if (IS_ERR(regmap)) {
@@ -28,6 +28,8 @@ static int bmg160_i2c_probe(struct i2c_client *client)
 
 	if (id)
 		name = id->name;
+	else
+		name = iio_get_acpi_device_name(&client->dev);
 
 	return bmg160_core_probe(&client->dev, regmap, client->irq, name);
 }
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 19/24] iio: gyro: bmg160: Replace custom implementation of iio_get_acpi_device_name()

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:08 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> IIO core (ACPI part) provides a generic helper that may be used in
> the driver. Replace custom implementation of iio_get_acpi_device_name().
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied.

[PATCH v3 20/24] iio: light: isl29018: Replace a variant of iio_get_acpi_device_name_and_data()

[Andy Shevchenko]

IIO core (ACPI part) provides a generic helper that may be used in
the driver. Replace a variant of iio_get_acpi_device_name_and_data().

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/light/isl29018.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
index 8dfc750e68c0..526ee5619d26 100644
--- a/drivers/iio/light/isl29018.c
+++ b/drivers/iio/light/isl29018.c
@@ -687,20 +687,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;
@@ -716,9 +702,10 @@ static int isl29018_probe(struct i2c_client *client)
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
 	struct isl29018_chip *chip;
 	struct iio_dev *indio_dev;
+	const void *ddata;
+	const char *name;
+	int dev_id;
 	int err;
-	const char *name = NULL;
-	int dev_id = 0;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
 	if (!indio_dev)
@@ -731,11 +718,11 @@ static int isl29018_probe(struct i2c_client *client)
 	if (id) {
 		name = id->name;
 		dev_id = id->driver_data;
+	} else {
+		name = iio_get_acpi_device_name_and_data(&client->dev, &ddata);
+		dev_id = (intptr_t)ddata;
 	}
 
-	if (ACPI_HANDLE(&client->dev))
-		name = isl29018_match_acpi_device(&client->dev, &dev_id);
-
 	mutex_init(&chip->lock);
 
 	chip->type = dev_id;
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 20/24] iio: light: isl29018: Replace a variant of iio_get_acpi_device_name_and_data()

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:09 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> IIO core (ACPI part) provides a generic helper that may be used in
> the driver. Replace a variant of iio_get_acpi_device_name_and_data().
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
This one briefly had me confused but is indeed fine because we never
get an id match for a device coming from an ACPI binding.
There is no way to make such a match (unlike DT where the naming
is enough).

So the else is sufficient.

applied.
> ---
>  drivers/iio/light/isl29018.c | 25 ++++++-------------------
>  1 file changed, 6 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
> index 8dfc750e68c0..526ee5619d26 100644
> --- a/drivers/iio/light/isl29018.c
> +++ b/drivers/iio/light/isl29018.c
> @@ -687,20 +687,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;
> @@ -716,9 +702,10 @@ static int isl29018_probe(struct i2c_client *client)
>  	const struct i2c_device_id *id = i2c_client_get_device_id(client);
>  	struct isl29018_chip *chip;
>  	struct iio_dev *indio_dev;
> +	const void *ddata;
> +	const char *name;
> +	int dev_id;
>  	int err;
> -	const char *name = NULL;
> -	int dev_id = 0;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
>  	if (!indio_dev)
> @@ -731,11 +718,11 @@ static int isl29018_probe(struct i2c_client *client)
>  	if (id) {
>  		name = id->name;
>  		dev_id = id->driver_data;
> +	} else {
> +		name = iio_get_acpi_device_name_and_data(&client->dev, &ddata);
> +		dev_id = (intptr_t)ddata;
>  	}
>  
> -	if (ACPI_HANDLE(&client->dev))
> -		name = isl29018_match_acpi_device(&client->dev, &dev_id);
> -
>  	mutex_init(&chip->lock);
>  
>  	chip->type = dev_id;

[PATCH v3 21/24] iio: light: isl29018: drop ACPI_PTR() and CONFIG_ACPI guards

[Andy Shevchenko]

The complexity of config guards needed for ACPI_PTR() is not worthwhile
for the small amount of saved data. This example was doing it correctly
but I am proposing dropping this so as to reduce chance of cut and paste
where it is done wrong.  Also drop now unneeded linux/acpi.h include and
added linux/mod_devicetable.h for struct acpi_device_id definition.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/light/isl29018.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
index 526ee5619d26..56e1c915af64 100644
--- a/drivers/iio/light/isl29018.c
+++ b/drivers/iio/light/isl29018.c
@@ -8,17 +8,18 @@
  * 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/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
 
@@ -819,15 +820,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},
@@ -841,14 +840,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.1336.g36b5255a03ac

Re: [PATCH v3 21/24] iio: light: isl29018: drop ACPI_PTR() and CONFIG_ACPI guards

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:10 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> The complexity of config guards needed for ACPI_PTR() is not worthwhile
> for the small amount of saved data. This example was doing it correctly
> but I am proposing dropping this so as to reduce chance of cut and paste
> where it is done wrong.  Also drop now unneeded linux/acpi.h include and
> added linux/mod_devicetable.h for struct acpi_device_id definition.
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
I made one trivial tweak.

> ---
>  drivers/iio/light/isl29018.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
> index 526ee5619d26..56e1c915af64 100644
> --- a/drivers/iio/light/isl29018.c
> +++ b/drivers/iio/light/isl29018.c
> @@ -8,17 +8,18 @@
>   * 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/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
>  
> @@ -819,15 +820,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},
> @@ -841,14 +840,14 @@ static const struct of_device_id isl29018_of_match[] = {
>  	{ .compatible = "isil,isl29018", },
>  	{ .compatible = "isil,isl29023", },
>  	{ .compatible = "isil,isl29035", },
> -	{ },
> +	{}
	{ }

is my preference where possible. I'm not going to modify the others as would
involve fixing up spacing in lots of code not otherwise touched, but I put the
space back here to simplify a future 'tidy up' patch a tiny bit.

Jonathan

>  };
>  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,
>  		    },

[PATCH v3 22/24] iio: light: ltr501: Drop most likely fake ACPI IDs

[Andy Shevchenko]

The commits in question do not proove that ACPI IDs exist.
Quite likely it was a cargo cult addition while doing that
for DT-based enumeration. Drop most likely fake ACPI IDs.

The to be removed IDs has been checked against the following resources:
1) DuckDuckGo
2) Google
3) MS catalog: https://www.catalog.update.microsoft.com/Search.aspx
This gives no useful results in regard to DSDT, moreover, the official
vendor ID in the registry for Lite-On is LCI.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/light/ltr501.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 8c516ede9116..3fff5d58ba3c 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -1610,8 +1610,6 @@ static int ltr501_resume(struct device *dev)
 static DEFINE_SIMPLE_DEV_PM_OPS(ltr501_pm_ops, ltr501_suspend, ltr501_resume);
 
 static const struct acpi_device_id ltr_acpi_match[] = {
-	{ "LTER0501", ltr501 },
-	{ "LTER0559", ltr559 },
 	{ "LTER0301", ltr301 },
 	{ },
 };
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 22/24] iio: light: ltr501: Drop most likely fake ACPI IDs

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:11 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> The commits in question do not proove that ACPI IDs exist.
> Quite likely it was a cargo cult addition while doing that
> for DT-based enumeration. Drop most likely fake ACPI IDs.
> 
> The to be removed IDs has been checked against the following resources:
> 1) DuckDuckGo
> 2) Google
> 3) MS catalog: https://www.catalog.update.microsoft.com/Search.aspx
> This gives no useful results in regard to DSDT, moreover, the official
> vendor ID in the registry for Lite-On is LCI.
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied.
> ---
>  drivers/iio/light/ltr501.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 8c516ede9116..3fff5d58ba3c 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -1610,8 +1610,6 @@ static int ltr501_resume(struct device *dev)
>  static DEFINE_SIMPLE_DEV_PM_OPS(ltr501_pm_ops, ltr501_suspend, ltr501_resume);
>  
>  static const struct acpi_device_id ltr_acpi_match[] = {
> -	{ "LTER0501", ltr501 },
> -	{ "LTER0559", ltr559 },
>  	{ "LTER0301", ltr301 },
>  	{ },
>  };

[PATCH v3 23/24] iio: light: ltr501: Add LTER0303 to the supported devices

[Andy Shevchenko]

It has been found that the (non-vendor issued) ACPI ID for Lite-On
LTR303 is present in Microsoft catalog. Add it to the list of the
supported devices.

Link: https://www.catalog.update.microsoft.com/Search.aspx?q=lter0303
Closes: https://lore.kernel.org/r/9cdda3e0-d56e-466f-911f-96ffd6f602c8@redhat.com
Reported-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/light/ltr501.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 3fff5d58ba3c..4051d0d9e799 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -1611,6 +1611,8 @@ static DEFINE_SIMPLE_DEV_PM_OPS(ltr501_pm_ops, ltr501_suspend, ltr501_resume);
 
 static const struct acpi_device_id ltr_acpi_match[] = {
 	{ "LTER0301", ltr301 },
+	/* https://www.catalog.update.microsoft.com/Search.aspx?q=lter0303 */
+	{ "LTER0303", ltr303 },
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, ltr_acpi_match);
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 23/24] iio: light: ltr501: Add LTER0303 to the supported devices

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:12 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> It has been found that the (non-vendor issued) ACPI ID for Lite-On
> LTR303 is present in Microsoft catalog. Add it to the list of the
> supported devices.
> 
> Link: https://www.catalog.update.microsoft.com/Search.aspx?q=lter0303
> Closes: https://lore.kernel.org/r/9cdda3e0-d56e-466f-911f-96ffd6f602c8@redhat.com
> Reported-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

I'm not totally happy that the MS catalog is enough to justify inclusion.
Would prefer to have an actual device. 

I'll take it anyway as better to have a policy on this than make it up
each time. So for now I want:
1) A device name or
2) Entry in the MS catalog.

Not
3) Entry in a driver on random vendor website that we have no evidence
   is in their products. (the Bosch one the other day).

Jonathan

> ---
>  drivers/iio/light/ltr501.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 3fff5d58ba3c..4051d0d9e799 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -1611,6 +1611,8 @@ static DEFINE_SIMPLE_DEV_PM_OPS(ltr501_pm_ops, ltr501_suspend, ltr501_resume);
>  
>  static const struct acpi_device_id ltr_acpi_match[] = {
>  	{ "LTER0301", ltr301 },
> +	/* https://www.catalog.update.microsoft.com/Search.aspx?q=lter0303 */
> +	{ "LTER0303", ltr303 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(acpi, ltr_acpi_match);

Re: [PATCH v3 23/24] iio: light: ltr501: Add LTER0303 to the supported devices

[Andy Shevchenko]

On Sat, Oct 26, 2024 at 12:56:43PM +0100, Jonathan Cameron wrote:
> On Thu, 24 Oct 2024 22:05:12 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > It has been found that the (non-vendor issued) ACPI ID for Lite-On
> > LTR303 is present in Microsoft catalog. Add it to the list of the
> > supported devices.

> I'm not totally happy that the MS catalog is enough to justify inclusion.
> Would prefer to have an actual device.

Me to, but I trust Hans' intuition in this case.

> I'll take it anyway as better to have a policy on this than make it up
> each time. So for now I want:
> 1) A device name or
> 2) Entry in the MS catalog.
> 
> Not
> 3) Entry in a driver on random vendor website that we have no evidence
>    is in their products. (the Bosch one the other day).

Agree.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 24/24] iio: light: ltr501: Replace a variant of iio_get_acpi_device_name_and_data()

[Andy Shevchenko]

IIO core (ACPI part) provides a generic helper that may be used in
the driver. Replace a variant of iio_get_acpi_device_name_and_data().

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/light/ltr501.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 4051d0d9e799..c389be6a106d 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -15,7 +15,6 @@
 #include <linux/err.h>
 #include <linux/delay.h>
 #include <linux/regmap.h>
-#include <linux/acpi.h>
 #include <linux/regulator/consumer.h>
 
 #include <linux/iio/iio.h>
@@ -1422,17 +1421,6 @@ static int ltr501_powerdown(struct ltr501_data *data)
 				  data->ps_contr & ~LTR501_CONTR_ACTIVE);
 }
 
-static const char *ltr501_match_acpi_device(struct device *dev, int *chip_idx)
-{
-	const struct acpi_device_id *id;
-
-	id = acpi_match_device(dev->driver->acpi_match_table, dev);
-	if (!id)
-		return NULL;
-	*chip_idx = id->driver_data;
-	return dev_name(dev);
-}
-
 static int ltr501_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
@@ -1440,8 +1428,10 @@ static int ltr501_probe(struct i2c_client *client)
 	struct ltr501_data *data;
 	struct iio_dev *indio_dev;
 	struct regmap *regmap;
-	int ret, partid, chip_idx = 0;
-	const char *name = NULL;
+	int partid, chip_idx;
+	const void *ddata;
+	const char *name;
+	int ret;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (!indio_dev)
@@ -1523,11 +1513,12 @@ static int ltr501_probe(struct i2c_client *client)
 	if (id) {
 		name = id->name;
 		chip_idx = id->driver_data;
-	} else  if (ACPI_HANDLE(&client->dev)) {
-		name = ltr501_match_acpi_device(&client->dev, &chip_idx);
 	} else {
-		return -ENODEV;
+		name = iio_get_acpi_device_name_and_data(&client->dev, &ddata);
+		chip_idx = (intptr_t)ddata;
 	}
+	if (!name)
+		return -ENODEV;
 
 	data->chip_info = &ltr501_chip_info_tbl[chip_idx];
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v3 24/24] iio: light: ltr501: Replace a variant of iio_get_acpi_device_name_and_data()

[Jonathan Cameron]

On Thu, 24 Oct 2024 22:05:13 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> IIO core (ACPI part) provides a generic helper that may be used in
> the driver. Replace a variant of iio_get_acpi_device_name_and_data().
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied.

So I ended up picking up all 24, with just white space tweaks.

Thanks for you hard work cleaning this stuff up.

Jonathan

Re: [PATCH v3 00/24] iio: Clean up acpi_match_device() use cases

[Matti Vaittinen]

On 24/10/2024 22:04, Andy Shevchenko wrote:
> There are current uses of acpi_match_device():
> - as strange way of checking if the device was enumerated via ACPI
> - as a way to get IIO device name as ACPI device instance name
> - as above with accompanying driver data
> 
> Deduplicate its use by providing two new helper functions in IIO ACPI
> library and update the rest accordingly.
> 
> This also includes a rework of previously sent ltr501 patch.
> 
> Besides that there ie a big clean up for the kxcjk-1013 driver, started
> with the revert of the one patch discussed earlier today. Feel free to
> route that one via fixes branch of your tree.
> 
> In v3:
> - collected tags (Marius)
> - added note to the documentation about usage of new API (Jonathan)
> - added a handful patches for kxcjk-1013 driver
> 
> In v2:
> - collected tags (Hans, Jean-Baptiste)
> - updated SoB chain in patch 4
> 
> Andy Shevchenko (24):
>    iio: magnetometer: bmc150: Drop dead code from the driver
>    iio: adc: pac1934: Replace strange way of checking type of enumeration
>    iio: imu: inv_mpu6050: Replace strange way of checking type of
>      enumeration
>    iio: acpi: Improve iio_read_acpi_mount_matrix()
>    iio: acpi: Add iio_get_acpi_device_name_and_data() helper function
>    iio: accel: kxcjk-1013: Remove redundant I²C ID
>    iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"
>    iio: accel: kxcjk-1013: Switch from CONFIG_PM guards to pm_ptr() etc
>    iio: accel: kxcjk-1013: Use local variable for regs
>    iio: accel: kxcjk-1013: Rename kxcjk1013_info
>    iio: accel: kxcjk-1013: Start using chip_info variables instead of
>      enum
>    iio: accel: kxcjk-1013: Move odr_start_up_times up in the code
>    iio: accel: kxcjk-1013: Convert ODR times array to variable in
>      chip_info
>    iio: accel: kxcjk-1013: Get rid of enum kx_chipset
>    iio: accel: kxcjk-1013: Replace a variant of
>      iio_get_acpi_device_name_and_data()
>    iio: accel: kxcjk-1013: drop ACPI_PTR() and move ID out of CONFIG_ACPI
>      guards

I missed reviewing these kxcjk changes. Not sure I loved all of them but 
I must admit the resulting code is looks better in general.

Thanks for the clean-up Andy!

Yours,
	-- Matti

Re: [PATCH v3 00/24] iio: Clean up acpi_match_device() use cases

[Andy Shevchenko]

On Tue, Oct 29, 2024 at 08:34:48AM +0200, Matti Vaittinen wrote:
> On 24/10/2024 22:04, Andy Shevchenko wrote:
> > There are current uses of acpi_match_device():
> > - as strange way of checking if the device was enumerated via ACPI
> > - as a way to get IIO device name as ACPI device instance name
> > - as above with accompanying driver data
> > 
> > Deduplicate its use by providing two new helper functions in IIO ACPI
> > library and update the rest accordingly.
> > 
> > This also includes a rework of previously sent ltr501 patch.
> > 
> > Besides that there ie a big clean up for the kxcjk-1013 driver, started
> > with the revert of the one patch discussed earlier today. Feel free to
> > route that one via fixes branch of your tree.
> > 
> > In v3:
> > - collected tags (Marius)
> > - added note to the documentation about usage of new API (Jonathan)
> > - added a handful patches for kxcjk-1013 driver
> > 
> > In v2:
> > - collected tags (Hans, Jean-Baptiste)
> > - updated SoB chain in patch 4
> > 
> > Andy Shevchenko (24):
> >    iio: magnetometer: bmc150: Drop dead code from the driver
> >    iio: adc: pac1934: Replace strange way of checking type of enumeration
> >    iio: imu: inv_mpu6050: Replace strange way of checking type of
> >      enumeration
> >    iio: acpi: Improve iio_read_acpi_mount_matrix()
> >    iio: acpi: Add iio_get_acpi_device_name_and_data() helper function
> >    iio: accel: kxcjk-1013: Remove redundant I²C ID
> >    iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"
> >    iio: accel: kxcjk-1013: Switch from CONFIG_PM guards to pm_ptr() etc
> >    iio: accel: kxcjk-1013: Use local variable for regs
> >    iio: accel: kxcjk-1013: Rename kxcjk1013_info
> >    iio: accel: kxcjk-1013: Start using chip_info variables instead of
> >      enum
> >    iio: accel: kxcjk-1013: Move odr_start_up_times up in the code
> >    iio: accel: kxcjk-1013: Convert ODR times array to variable in
> >      chip_info
> >    iio: accel: kxcjk-1013: Get rid of enum kx_chipset
> >    iio: accel: kxcjk-1013: Replace a variant of
> >      iio_get_acpi_device_name_and_data()
> >    iio: accel: kxcjk-1013: drop ACPI_PTR() and move ID out of CONFIG_ACPI
> >      guards
> 
> I missed reviewing these kxcjk changes. Not sure I loved all of them

Patches are welcome! :-)

> but I must admit the resulting code is looks better in general.
> 
> Thanks for the clean-up Andy!

Thank you for looking at this!

-- 
With Best Regards,
Andy Shevchenko