[PATCH 1/8] iio: light: cm32181: Add some extra register defines

[PATCH 1/8] iio: light: cm32181: Add some extra register defines

[Hans de Goede]

These come from a newer version of cm32181.c, which is floating around
the net, with a copyright of:

 * Copyright (C) 2014 Capella Microsystems Inc.
 * Author: Kevin Tsai <ktsai@capellamicro.com>
 *
 * This program is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License version 2, as published
 * by the Free Software Foundation.

Note that this removes the bogus CM32181_CMD_ALS_ENABLE define, there
is no enable bit, only a disable bit and enabled is the absence of
being disabled.

This is a preparation patch for adding support for the older
CM3218 model of the light sensor.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/light/cm32181.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index 5f4fb5674fa0..ee386afe811e 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -18,6 +18,9 @@
 
 /* Registers Address */
 #define CM32181_REG_ADDR_CMD		0x00
+#define CM32181_REG_ADDR_WH		0x01
+#define CM32181_REG_ADDR_WL		0x02
+#define CM32181_REG_ADDR_TEST		0x03
 #define CM32181_REG_ADDR_ALS		0x04
 #define CM32181_REG_ADDR_STATUS		0x06
 #define CM32181_REG_ADDR_ID		0x07
@@ -26,9 +29,13 @@
 #define CM32181_CONF_REG_NUM		0x01
 
 /* CMD register */
-#define CM32181_CMD_ALS_ENABLE		0x00
-#define CM32181_CMD_ALS_DISABLE		0x01
-#define CM32181_CMD_ALS_INT_EN		0x02
+#define CM32181_CMD_ALS_DISABLE		BIT(0)
+#define CM32181_CMD_ALS_INT_EN		BIT(1)
+#define CM32181_CMD_ALS_THRES_WINDOW	BIT(2)
+
+#define CM32181_CMD_ALS_PERS_SHIFT	4
+#define CM32181_CMD_ALS_PERS_MASK	(0x03 << CM32181_CMD_ALS_PERS_SHIFT)
+#define CM32181_CMD_ALS_PERS_DEFAULT	(0x01 << CM32181_CMD_ALS_PERS_SHIFT)
 
 #define CM32181_CMD_ALS_IT_SHIFT	6
 #define CM32181_CMD_ALS_IT_MASK		(0x0F << CM32181_CMD_ALS_IT_SHIFT)
@@ -82,7 +89,7 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
 		return -ENODEV;
 
 	/* Default Values */
-	cm32181->conf_regs[CM32181_REG_ADDR_CMD] = CM32181_CMD_ALS_ENABLE |
+	cm32181->conf_regs[CM32181_REG_ADDR_CMD] =
 			CM32181_CMD_ALS_IT_DEFAULT | CM32181_CMD_ALS_SM_DEFAULT;
 	cm32181->calibscale = CM32181_CALIBSCALE_DEFAULT;
 
-- 
2.26.0

[PATCH 2/8] iio: light: cm32181: Add support for the CM3218

[Hans de Goede]

Add support for the CM3218 which is an older version of the
CM32181.

This is based on a newer version of cm32181.c, with a copyright of:

 * Copyright (C) 2014 Capella Microsystems Inc.
 * Author: Kevin Tsai <ktsai@capellamicro.com>
 *
 * This program is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License version 2, as published
 * by the Free Software Foundation.

Which is floating around on the net in various places, but the changes
from this newer version never made it upstream.

This was tested on an Asus T100TA and an Asus T100CHI, which both come
with the CM3218 variant of the light sensor.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/light/cm32181.c | 51 ++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index ee386afe811e..fd371b36c7b3 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -55,15 +55,24 @@ static const u8 cm32181_reg[CM32181_CONF_REG_NUM] = {
 	CM32181_REG_ADDR_CMD,
 };
 
-static const int als_it_bits[] = {12, 8, 0, 1, 2, 3};
-static const int als_it_value[] = {25000, 50000, 100000, 200000, 400000,
-	800000};
+/* CM3218 Family */
+static const int cm3218_als_it_bits[] = { 0, 1, 2, 3 };
+static const int cm3218_als_it_values[] = { 100000, 200000, 400000, 800000 };
+
+/* CM32181 Family */
+static const int cm32181_als_it_bits[] = { 12, 8, 0, 1, 2, 3 };
+static const int cm32181_als_it_values[] = {
+	25000, 50000, 100000, 200000, 400000, 800000
+};
 
 struct cm32181_chip {
 	struct i2c_client *client;
 	struct mutex lock;
 	u16 conf_regs[CM32181_CONF_REG_NUM];
 	int calibscale;
+	int num_als_it;
+	const int *als_it_bits;
+	const int *als_it_values;
 };
 
 /**
@@ -85,8 +94,23 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
 		return ret;
 
 	/* check device ID */
-	if ((ret & 0xFF) != 0x81)
+	switch (ret & 0xFF) {
+	case 0x18: /* CM3218 */
+		dev_info(&client->dev, "Detected CM3218\n");
+		cm32181->num_als_it = ARRAY_SIZE(cm3218_als_it_bits);
+		cm32181->als_it_bits = cm3218_als_it_bits;
+		cm32181->als_it_values = cm3218_als_it_values;
+		break;
+	case 0x81: /* CM32181 */
+	case 0x82: /* CM32182, fully compat. with CM32181 */
+		dev_info(&client->dev, "Detected CM32181\n");
+		cm32181->num_als_it = ARRAY_SIZE(cm32181_als_it_bits);
+		cm32181->als_it_bits = cm32181_als_it_bits;
+		cm32181->als_it_values = cm32181_als_it_values;
+		break;
+	default:
 		return -ENODEV;
+	}
 
 	/* Default Values */
 	cm32181->conf_regs[CM32181_REG_ADDR_CMD] =
@@ -121,9 +145,9 @@ static int cm32181_read_als_it(struct cm32181_chip *cm32181, int *val2)
 	als_it = cm32181->conf_regs[CM32181_REG_ADDR_CMD];
 	als_it &= CM32181_CMD_ALS_IT_MASK;
 	als_it >>= CM32181_CMD_ALS_IT_SHIFT;
-	for (i = 0; i < ARRAY_SIZE(als_it_bits); i++) {
-		if (als_it == als_it_bits[i]) {
-			*val2 = als_it_value[i];
+	for (i = 0; i < cm32181->num_als_it; i++) {
+		if (als_it == cm32181->als_it_bits[i]) {
+			*val2 = cm32181->als_it_values[i];
 			return IIO_VAL_INT_PLUS_MICRO;
 		}
 	}
@@ -146,14 +170,14 @@ static int cm32181_write_als_it(struct cm32181_chip *cm32181, int val)
 	u16 als_it;
 	int ret, i, n;
 
-	n = ARRAY_SIZE(als_it_value);
+	n = cm32181->num_als_it;
 	for (i = 0; i < n; i++)
-		if (val <= als_it_value[i])
+		if (val <= cm32181->als_it_values[i])
 			break;
 	if (i >= n)
 		i = n - 1;
 
-	als_it = als_it_bits[i];
+	als_it = cm32181->als_it_bits[i];
 	als_it <<= CM32181_CMD_ALS_IT_SHIFT;
 
 	mutex_lock(&cm32181->lock);
@@ -265,11 +289,12 @@ static int cm32181_write_raw(struct iio_dev *indio_dev,
 static ssize_t cm32181_get_it_available(struct device *dev,
 			struct device_attribute *attr, char *buf)
 {
+	struct cm32181_chip *cm32181 = iio_priv(dev_to_iio_dev(dev));
 	int i, n, len;
 
-	n = ARRAY_SIZE(als_it_value);
+	n = cm32181->num_als_it;
 	for (i = 0, len = 0; i < n; i++)
-		len += sprintf(buf + len, "0.%06u ", als_it_value[i]);
+		len += sprintf(buf + len, "0.%06u ", cm32181->als_it_values[i]);
 	return len + sprintf(buf + len, "\n");
 }
 
@@ -346,6 +371,7 @@ static int cm32181_probe(struct i2c_client *client,
 }
 
 static const struct i2c_device_id cm32181_id[] = {
+	{ "cm3218", 0 },
 	{ "cm32181", 0 },
 	{ }
 };
@@ -353,6 +379,7 @@ static const struct i2c_device_id cm32181_id[] = {
 MODULE_DEVICE_TABLE(i2c, cm32181_id);
 
 static const struct of_device_id cm32181_of_match[] = {
+	{ .compatible = "capella,cm3218" },
 	{ .compatible = "capella,cm32181" },
 	{ }
 };
-- 
2.26.0

[PATCH 3/8] iio: light: cm32181: Handle ACPI instantiating a cm32181 client on the SMBus ARA

[Hans de Goede]

Some ACPI systems list 2 I2C resources for the CM3218 sensor. On these
systems the first I2cSerialBus ACPI-resource points to the SMBus Alert
Response Address (ARA, 0x0c) and the second I2cSerialBus ACPI-resource
points to the actual CM3218 sensor address.

From the ACPI/x86 side devices with more then 1 I2cSerialBus ACPI-resource
are handled by the drivers/platform/x86/i2c-multi-instantiate.c code.
This code will instantiate "cm32181" i2c_client-s for both resources.

Add a check to cm32181_probe() for the client's address being the ARA
address, and in that case fail the probe with -ENODEV.

On these ACPI systems the sensor may have a SMBus Alert asserted at boot,
if this is the case the sensor will not respond to any i2c_transfers on
its actual address until we read from the ARA register to clear the Alert.

Therefor we must (try to) read a byte from the client with the ARA
register, before returning -ENODEV, so that we clear the Alert and when
we get called again for the client instantiated for the second
I2cSerialBus ACPI-resource the sensor will respond to our i2c-transfers.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/light/cm32181.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index fd371b36c7b3..e8be20d3902c 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -51,6 +51,8 @@
 #define CM32181_CALIBSCALE_RESOLUTION	1000
 #define MLUX_PER_LUX			1000
 
+#define SMBUS_ALERT_RESPONSE_ADDRESS	0x0c
+
 static const u8 cm32181_reg[CM32181_CONF_REG_NUM] = {
 	CM32181_REG_ADDR_CMD,
 };
@@ -333,6 +335,20 @@ static int cm32181_probe(struct i2c_client *client,
 	struct iio_dev *indio_dev;
 	int ret;
 
+	/*
+	 * Some ACPI systems list 2 I2C resources for the CM3218 sensor, the
+	 * SMBus Alert Response Address (ARA, 0x0c) and the actual I2C address.
+	 * drivers/platform/x86/i2c-multi-instantiate.c instantiates "cm32181"
+	 * i2c_client-s for both resources, ignore the ARA client.
+	 * On these systems the sensor may have a SMBus Alert asserted at boot,
+	 * in that case the ARA must be read to clear the Alert otherwise the
+	 * sensor will not respond on its actual I2C address.
+	 */
+	if (client->addr == SMBUS_ALERT_RESPONSE_ADDRESS) {
+		i2c_smbus_read_byte(client);
+		return -ENODEV;
+	}
+
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*cm32181));
 	if (!indio_dev) {
 		dev_err(&client->dev, "devm_iio_device_alloc failed\n");
-- 
2.26.0

[PATCH 4/8] iio: light: cm32181: Change reg_init to use a bitmap of which registers to init

[Hans de Goede]

This is a preparation patch for reading some ACPI tables which give
init values for multiple registers.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/light/cm32181.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index e8be20d3902c..e31c1005b03d 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -26,7 +26,7 @@
 #define CM32181_REG_ADDR_ID		0x07
 
 /* Number of Configurable Registers */
-#define CM32181_CONF_REG_NUM		0x01
+#define CM32181_CONF_REG_NUM		4
 
 /* CMD register */
 #define CM32181_CMD_ALS_DISABLE		BIT(0)
@@ -53,10 +53,6 @@
 
 #define SMBUS_ALERT_RESPONSE_ADDRESS	0x0c
 
-static const u8 cm32181_reg[CM32181_CONF_REG_NUM] = {
-	CM32181_REG_ADDR_CMD,
-};
-
 /* CM3218 Family */
 static const int cm3218_als_it_bits[] = { 0, 1, 2, 3 };
 static const int cm3218_als_it_values[] = { 100000, 200000, 400000, 800000 };
@@ -71,6 +67,7 @@ struct cm32181_chip {
 	struct i2c_client *client;
 	struct mutex lock;
 	u16 conf_regs[CM32181_CONF_REG_NUM];
+	int init_regs_bitmap;
 	int calibscale;
 	int num_als_it;
 	const int *als_it_bits;
@@ -117,14 +114,17 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
 	/* Default Values */
 	cm32181->conf_regs[CM32181_REG_ADDR_CMD] =
 			CM32181_CMD_ALS_IT_DEFAULT | CM32181_CMD_ALS_SM_DEFAULT;
+	cm32181->init_regs_bitmap = BIT(CM32181_REG_ADDR_CMD);
 	cm32181->calibscale = CM32181_CALIBSCALE_DEFAULT;
 
 	/* Initialize registers*/
 	for (i = 0; i < CM32181_CONF_REG_NUM; i++) {
-		ret = i2c_smbus_write_word_data(client, cm32181_reg[i],
-			cm32181->conf_regs[i]);
-		if (ret < 0)
-			return ret;
+		if (cm32181->init_regs_bitmap & BIT(i)) {
+			ret = i2c_smbus_write_word_data(client, i,
+							cm32181->conf_regs[i]);
+			if (ret < 0)
+				return ret;
+		}
 	}
 
 	return 0;
-- 
2.26.0

[PATCH 5/8] iio: light: cm32181: Use units of 1/100000th for calibscale and lux_per_bit

[Hans de Goede]

Use units of 1/100000th for calibscale and lux_per_bit. The similar
cm3232 driver already uses 1/100000th as unit for calibscale.

This allows for higher-accuracy and makes it easier to add support
for getting device-specific calibscale and lux_per_bit values from
a device's ACPI tables, as the values in the ACPI tables also use
1/100000th units.

This units change means that our intermediate values in cm32181_get_lux()
may get quite big, change the type of the lux variable to a u64 to
deal with this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/light/cm32181.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index e31c1005b03d..4fdf2f4a23ab 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -45,11 +45,11 @@
 #define CM32181_CMD_ALS_SM_MASK		(0x03 << CM32181_CMD_ALS_SM_SHIFT)
 #define CM32181_CMD_ALS_SM_DEFAULT	(0x01 << CM32181_CMD_ALS_SM_SHIFT)
 
-#define CM32181_MLUX_PER_BIT		5	/* ALS_SM=01 IT=800ms */
-#define CM32181_MLUX_PER_BIT_BASE_IT	800000	/* Based on IT=800ms */
-#define	CM32181_CALIBSCALE_DEFAULT	1000
-#define CM32181_CALIBSCALE_RESOLUTION	1000
-#define MLUX_PER_LUX			1000
+#define CM32181_LUX_PER_BIT		500	/* ALS_SM=01 IT=800ms */
+#define CM32181_LUX_PER_BIT_RESOLUTION	100000
+#define CM32181_LUX_PER_BIT_BASE_IT	800000	/* Based on IT=800ms */
+#define CM32181_CALIBSCALE_DEFAULT	100000
+#define CM32181_CALIBSCALE_RESOLUTION	100000
 
 #define SMBUS_ALERT_RESPONSE_ADDRESS	0x0c
 
@@ -208,15 +208,15 @@ static int cm32181_get_lux(struct cm32181_chip *cm32181)
 	struct i2c_client *client = cm32181->client;
 	int ret;
 	int als_it;
-	unsigned long lux;
+	u64 lux;
 
 	ret = cm32181_read_als_it(cm32181, &als_it);
 	if (ret < 0)
 		return -EINVAL;
 
-	lux = CM32181_MLUX_PER_BIT;
-	lux *= CM32181_MLUX_PER_BIT_BASE_IT;
-	lux /= als_it;
+	lux = CM32181_LUX_PER_BIT;
+	lux *= CM32181_LUX_PER_BIT_BASE_IT;
+	lux = div_u64(lux, als_it);
 
 	ret = i2c_smbus_read_word_data(client, CM32181_REG_ADDR_ALS);
 	if (ret < 0)
@@ -224,8 +224,8 @@ static int cm32181_get_lux(struct cm32181_chip *cm32181)
 
 	lux *= ret;
 	lux *= cm32181->calibscale;
-	lux /= CM32181_CALIBSCALE_RESOLUTION;
-	lux /= MLUX_PER_LUX;
+	lux = div_u64(lux, CM32181_CALIBSCALE_RESOLUTION);
+	lux = div_u64(lux, CM32181_LUX_PER_BIT_RESOLUTION);
 
 	if (lux > 0xFFFF)
 		lux = 0xFFFF;
-- 
2.26.0

[PATCH 6/8] iio: light: cm32181: Make lux_per_bit and lux_per_bit_base_it runtime settings

[Hans de Goede]

Make lux_per_bit and lux_per_bit_base_it settings stored in struct
cm32181_chip instead of a hardcoded (defined) values.

This is a preparation patch for reading some ACPI tables which specify
a device specific lux_per_bit value.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/light/cm32181.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index 4fdf2f4a23ab..e5674d4a8143 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -69,6 +69,8 @@ struct cm32181_chip {
 	u16 conf_regs[CM32181_CONF_REG_NUM];
 	int init_regs_bitmap;
 	int calibscale;
+	int lux_per_bit;
+	int lux_per_bit_base_it;
 	int num_als_it;
 	const int *als_it_bits;
 	const int *als_it_values;
@@ -116,6 +118,8 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
 			CM32181_CMD_ALS_IT_DEFAULT | CM32181_CMD_ALS_SM_DEFAULT;
 	cm32181->init_regs_bitmap = BIT(CM32181_REG_ADDR_CMD);
 	cm32181->calibscale = CM32181_CALIBSCALE_DEFAULT;
+	cm32181->lux_per_bit = CM32181_LUX_PER_BIT;
+	cm32181->lux_per_bit_base_it = CM32181_LUX_PER_BIT_BASE_IT;
 
 	/* Initialize registers*/
 	for (i = 0; i < CM32181_CONF_REG_NUM; i++) {
@@ -214,8 +218,8 @@ static int cm32181_get_lux(struct cm32181_chip *cm32181)
 	if (ret < 0)
 		return -EINVAL;
 
-	lux = CM32181_LUX_PER_BIT;
-	lux *= CM32181_LUX_PER_BIT_BASE_IT;
+	lux = cm32181->lux_per_bit;
+	lux *= cm32181->lux_per_bit_base_it;
 	lux = div_u64(lux, als_it);
 
 	ret = i2c_smbus_read_word_data(client, CM32181_REG_ADDR_ALS);
-- 
2.26.0

[PATCH 7/8] iio: light: cm32181: Add support for parsing CPM0 and CPM1 ACPI tables

[Hans de Goede]

On ACPI based systems the CPLM3218 ACPI device node describing the
CM3218[1] sensor typically will have some extra tables with register
init values for initializing the sensor and calibration info.

This is based on a newer version of cm32181.c, with a copyright of:

 * Copyright (C) 2014 Capella Microsystems Inc.
 * Author: Kevin Tsai <ktsai@capellamicro.com>
 *
 * This program is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License version 2, as published
 * by the Free Software Foundation.

Which is floating around on the net in various places, but the changes
from this newer version never made it upstream.

This was tested on the following models: Acer Switch 10 SW5-012 (CM32181)
Asus T100TA (CM3218), Asus T100CHI (CM3218) and HP X2 10-n000nd (CM32181).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/light/cm32181.c | 98 +++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index e5674d4a8143..878fc13632d8 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -4,6 +4,7 @@
  * Author: Kevin Tsai <ktsai@capellamicro.com>
  */
 
+#include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
@@ -53,6 +54,15 @@
 
 #define SMBUS_ALERT_RESPONSE_ADDRESS	0x0c
 
+/* CPM0 Index 0: device-id (3218 or 32181), 1: Unknown, 2: init_regs_bitmap */
+#define CPM0_REGS_BITMAP		2
+#define CPM0_HEADER_SIZE		3
+
+/* CPM1 Index 0: lux_per_bit, 1: calibscale, 2: resolution (100000) */
+#define CPM1_LUX_PER_BIT		0
+#define CPM1_CALIBSCALE			1
+#define CPM1_SIZE			3
+
 /* CM3218 Family */
 static const int cm3218_als_it_bits[] = { 0, 1, 2, 3 };
 static const int cm3218_als_it_values[] = { 100000, 200000, 400000, 800000 };
@@ -76,6 +86,56 @@ struct cm32181_chip {
 	const int *als_it_values;
 };
 
+static int cm32181_read_als_it(struct cm32181_chip *cm32181, int *val2);
+
+#ifdef CONFIG_ACPI
+/**
+ * cm32181_acpi_get_cpm() - Get CPM object from ACPI
+ * @client	pointer of struct i2c_client.
+ * @obj_name	pointer of ACPI object name.
+ * @count	maximum size of return array.
+ * @vals	pointer of array for return elements.
+ *
+ * Convert ACPI CPM table to array.
+ *
+ * Return: -ENODEV for fail.  Otherwise is number of elements.
+ */
+static int cm32181_acpi_get_cpm(struct i2c_client *client, char *obj_name,
+				u64 *values, int count)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *cpm, *elem;
+	acpi_handle handle;
+	acpi_status status;
+	int i;
+
+	handle = ACPI_HANDLE(&client->dev);
+	if (!handle)
+		return -ENODEV;
+
+	status = acpi_evaluate_object(handle, obj_name, NULL, &buffer);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&client->dev, "object %s not found\n", obj_name);
+		return -ENODEV;
+	}
+
+	cpm = buffer.pointer;
+	if (cpm->package.count > count)
+		dev_warn(&client->dev, "%s table contains %d values, only using first %d values\n",
+			 obj_name, cpm->package.count, count);
+
+	count = min_t(int, cpm->package.count, count);
+	for (i = 0; i < count; i++) {
+		elem = &(cpm->package.elements[i]);
+		values[i] = elem->integer.value;
+	}
+
+	kfree(buffer.pointer);
+
+	return count;
+}
+#endif /* CONFIG_ACPI */
+
 /**
  * cm32181_reg_init() - Initialize CM32181 registers
  * @cm32181:	pointer of struct cm32181.
@@ -121,6 +181,44 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
 	cm32181->lux_per_bit = CM32181_LUX_PER_BIT;
 	cm32181->lux_per_bit_base_it = CM32181_LUX_PER_BIT_BASE_IT;
 
+#ifdef CONFIG_ACPI
+	if (ACPI_HANDLE(&client->dev)) {
+		u64 values[CPM0_HEADER_SIZE + CM32181_CONF_REG_NUM];
+		int count;
+
+		count = cm32181_acpi_get_cpm(client, "CPM0",
+					     values, ARRAY_SIZE(values));
+		if (count <= CPM0_HEADER_SIZE)
+			goto cpm_parsing_done;
+
+		count -= CPM0_HEADER_SIZE;
+
+		cm32181->init_regs_bitmap = values[CPM0_REGS_BITMAP];
+		cm32181->init_regs_bitmap &= GENMASK(count - 1, 0);
+		for (i = 0; i < count; i++) {
+			if (cm32181->init_regs_bitmap & BIT(i))
+				cm32181->conf_regs[i] =
+					values[CPM0_HEADER_SIZE + i];
+		}
+
+		count = cm32181_acpi_get_cpm(client, "CPM1",
+					     values, ARRAY_SIZE(values));
+		if (count != CPM1_SIZE)
+			goto cpm_parsing_done;
+
+		cm32181->lux_per_bit = values[CPM1_LUX_PER_BIT];
+
+		/* Check for uncalibrated devices */
+		if (values[CPM1_CALIBSCALE] == CM32181_CALIBSCALE_DEFAULT)
+			goto cpm_parsing_done;
+
+		cm32181->calibscale  = values[CPM1_CALIBSCALE];
+		/* CPM1 lux_per_bit is for the current it value */
+		cm32181_read_als_it(cm32181, &cm32181->lux_per_bit_base_it);
+	}
+cpm_parsing_done:
+#endif /* CONFIG_ACPI */
+
 	/* Initialize registers*/
 	for (i = 0; i < CM32181_CONF_REG_NUM; i++) {
 		if (cm32181->init_regs_bitmap & BIT(i)) {
-- 
2.26.0

[PATCH 8/8] iio: light: cm32181: Fix integartion time typo

[Hans de Goede]

Fix integartion time typo and while at it improve the comment with
the typo a bit in general.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/light/cm32181.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index 878fc13632d8..50cfb7d189df 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -237,7 +237,7 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
  *  @cm32181:	pointer of struct cm32181
  *  @val2:	pointer of int to load the als_it value.
  *
- *  Report the current integartion time by millisecond.
+ *  Report the current integration time in milliseconds.
  *
  *  Return: IIO_VAL_INT_PLUS_MICRO for success, otherwise -EINVAL.
  */
-- 
2.26.0

Re: [PATCH 3/8] iio: light: cm32181: Handle ACPI instantiating a cm32181 client on the SMBus ARA

[Andy Shevchenko]

On Sun, Apr 26, 2020 at 2:03 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Some ACPI systems list 2 I2C resources for the CM3218 sensor. On these
> systems the first I2cSerialBus ACPI-resource points to the SMBus Alert
> Response Address (ARA, 0x0c) and the second I2cSerialBus ACPI-resource
> points to the actual CM3218 sensor address.
>
> From the ACPI/x86 side devices with more then 1 I2cSerialBus ACPI-resource
> are handled by the drivers/platform/x86/i2c-multi-instantiate.c code.
> This code will instantiate "cm32181" i2c_client-s for both resources.
>
> Add a check to cm32181_probe() for the client's address being the ARA
> address, and in that case fail the probe with -ENODEV.
>
> On these ACPI systems the sensor may have a SMBus Alert asserted at boot,
> if this is the case the sensor will not respond to any i2c_transfers on
> its actual address until we read from the ARA register to clear the Alert.
>
> Therefor we must (try to) read a byte from the client with the ARA
> register, before returning -ENODEV, so that we clear the Alert and when
> we get called again for the client instantiated for the second
> I2cSerialBus ACPI-resource the sensor will respond to our i2c-transfers.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/iio/light/cm32181.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
> index fd371b36c7b3..e8be20d3902c 100644
> --- a/drivers/iio/light/cm32181.c
> +++ b/drivers/iio/light/cm32181.c
> @@ -51,6 +51,8 @@
>  #define CM32181_CALIBSCALE_RESOLUTION  1000
>  #define MLUX_PER_LUX                   1000
>
> +#define SMBUS_ALERT_RESPONSE_ADDRESS   0x0c
> +
>  static const u8 cm32181_reg[CM32181_CONF_REG_NUM] = {
>         CM32181_REG_ADDR_CMD,
>  };
> @@ -333,6 +335,20 @@ static int cm32181_probe(struct i2c_client *client,
>         struct iio_dev *indio_dev;
>         int ret;
>
> +       /*
> +        * Some ACPI systems list 2 I2C resources for the CM3218 sensor, the
> +        * SMBus Alert Response Address (ARA, 0x0c) and the actual I2C address.
> +        * drivers/platform/x86/i2c-multi-instantiate.c instantiates "cm32181"

> +        * i2c_client-s for both resources, ignore the ARA client.

A nit: 'I2C clients'

> +        * On these systems the sensor may have a SMBus Alert asserted at boot,
> +        * in that case the ARA must be read to clear the Alert otherwise the
> +        * sensor will not respond on its actual I2C address.
> +        */
> +       if (client->addr == SMBUS_ALERT_RESPONSE_ADDRESS) {
> +               i2c_smbus_read_byte(client);
> +               return -ENODEV;
> +       }
> +
>         indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*cm32181));
>         if (!indio_dev) {
>                 dev_err(&client->dev, "devm_iio_device_alloc failed\n");
> --
> 2.26.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/8] iio: light: cm32181: Change reg_init to use a bitmap of which registers to init

[Andy Shevchenko]

On Sun, Apr 26, 2020 at 2:03 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> This is a preparation patch for reading some ACPI tables which give
> init values for multiple registers.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/iio/light/cm32181.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
> index e8be20d3902c..e31c1005b03d 100644
> --- a/drivers/iio/light/cm32181.c
> +++ b/drivers/iio/light/cm32181.c
> @@ -26,7 +26,7 @@
>  #define CM32181_REG_ADDR_ID            0x07
>
>  /* Number of Configurable Registers */
> -#define CM32181_CONF_REG_NUM           0x01
> +#define CM32181_CONF_REG_NUM           4
>
>  /* CMD register */
>  #define CM32181_CMD_ALS_DISABLE                BIT(0)
> @@ -53,10 +53,6 @@
>
>  #define SMBUS_ALERT_RESPONSE_ADDRESS   0x0c
>
> -static const u8 cm32181_reg[CM32181_CONF_REG_NUM] = {
> -       CM32181_REG_ADDR_CMD,
> -};
> -
>  /* CM3218 Family */
>  static const int cm3218_als_it_bits[] = { 0, 1, 2, 3 };
>  static const int cm3218_als_it_values[] = { 100000, 200000, 400000, 800000 };
> @@ -71,6 +67,7 @@ struct cm32181_chip {
>         struct i2c_client *client;
>         struct mutex lock;
>         u16 conf_regs[CM32181_CONF_REG_NUM];

> +       int init_regs_bitmap;

unsigned long

>         int calibscale;
>         int num_als_it;
>         const int *als_it_bits;
> @@ -117,14 +114,17 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
>         /* Default Values */
>         cm32181->conf_regs[CM32181_REG_ADDR_CMD] =
>                         CM32181_CMD_ALS_IT_DEFAULT | CM32181_CMD_ALS_SM_DEFAULT;
> +       cm32181->init_regs_bitmap = BIT(CM32181_REG_ADDR_CMD);
>         cm32181->calibscale = CM32181_CALIBSCALE_DEFAULT;
>
>         /* Initialize registers*/
>         for (i = 0; i < CM32181_CONF_REG_NUM; i++) {
> -               ret = i2c_smbus_write_word_data(client, cm32181_reg[i],
> -                       cm32181->conf_regs[i]);
> -               if (ret < 0)
> -                       return ret;
> +               if (cm32181->init_regs_bitmap & BIT(i)) {

for_each_set_bit()

> +                       ret = i2c_smbus_write_word_data(client, i,
> +                                                       cm32181->conf_regs[i]);
> +                       if (ret < 0)
> +                               return ret;
> +               }
>         }
>
>         return 0;
> --
> 2.26.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 7/8] iio: light: cm32181: Add support for parsing CPM0 and CPM1 ACPI tables

[Andy Shevchenko]

On Sun, Apr 26, 2020 at 2:03 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> On ACPI based systems the CPLM3218 ACPI device node describing the
> CM3218[1] sensor typically will have some extra tables with register
> init values for initializing the sensor and calibration info.
>
> This is based on a newer version of cm32181.c, with a copyright of:
>
>  * Copyright (C) 2014 Capella Microsystems Inc.
>  * Author: Kevin Tsai <ktsai@capellamicro.com>
>  *
>  * This program is free software; you can redistribute it and/or modify it
>  * under the terms of the GNU General Public License version 2, as published
>  * by the Free Software Foundation.
>
> Which is floating around on the net in various places, but the changes
> from this newer version never made it upstream.
>
> This was tested on the following models: Acer Switch 10 SW5-012 (CM32181)
> Asus T100TA (CM3218), Asus T100CHI (CM3218) and HP X2 10-n000nd (CM32181).
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/iio/light/cm32181.c | 98 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
>
> diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
> index e5674d4a8143..878fc13632d8 100644
> --- a/drivers/iio/light/cm32181.c
> +++ b/drivers/iio/light/cm32181.c
> @@ -4,6 +4,7 @@
>   * Author: Kevin Tsai <ktsai@capellamicro.com>
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/i2c.h>
> @@ -53,6 +54,15 @@
>
>  #define SMBUS_ALERT_RESPONSE_ADDRESS   0x0c
>
> +/* CPM0 Index 0: device-id (3218 or 32181), 1: Unknown, 2: init_regs_bitmap */
> +#define CPM0_REGS_BITMAP               2
> +#define CPM0_HEADER_SIZE               3
> +
> +/* CPM1 Index 0: lux_per_bit, 1: calibscale, 2: resolution (100000) */
> +#define CPM1_LUX_PER_BIT               0
> +#define CPM1_CALIBSCALE                        1
> +#define CPM1_SIZE                      3
> +
>  /* CM3218 Family */
>  static const int cm3218_als_it_bits[] = { 0, 1, 2, 3 };
>  static const int cm3218_als_it_values[] = { 100000, 200000, 400000, 800000 };
> @@ -76,6 +86,56 @@ struct cm32181_chip {
>         const int *als_it_values;
>  };
>
> +static int cm32181_read_als_it(struct cm32181_chip *cm32181, int *val2);
> +
> +#ifdef CONFIG_ACPI
> +/**
> + * cm32181_acpi_get_cpm() - Get CPM object from ACPI
> + * @client     pointer of struct i2c_client.
> + * @obj_name   pointer of ACPI object name.
> + * @count      maximum size of return array.
> + * @vals       pointer of array for return elements.
> + *
> + * Convert ACPI CPM table to array.
> + *
> + * Return: -ENODEV for fail.  Otherwise is number of elements.
> + */
> +static int cm32181_acpi_get_cpm(struct i2c_client *client, char *obj_name,
> +                               u64 *values, int count)
> +{
> +       struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +       union acpi_object *cpm, *elem;
> +       acpi_handle handle;
> +       acpi_status status;
> +       int i;
> +
> +       handle = ACPI_HANDLE(&client->dev);
> +       if (!handle)
> +               return -ENODEV;
> +
> +       status = acpi_evaluate_object(handle, obj_name, NULL, &buffer);
> +       if (ACPI_FAILURE(status)) {
> +               dev_err(&client->dev, "object %s not found\n", obj_name);
> +               return -ENODEV;
> +       }
> +
> +       cpm = buffer.pointer;
> +       if (cpm->package.count > count)
> +               dev_warn(&client->dev, "%s table contains %d values, only using first %d values\n",
> +                        obj_name, cpm->package.count, count);
> +
> +       count = min_t(int, cpm->package.count, count);
> +       for (i = 0; i < count; i++) {
> +               elem = &(cpm->package.elements[i]);
> +               values[i] = elem->integer.value;
> +       }
> +
> +       kfree(buffer.pointer);
> +
> +       return count;
> +}
> +#endif /* CONFIG_ACPI */
> +
>  /**
>   * cm32181_reg_init() - Initialize CM32181 registers
>   * @cm32181:   pointer of struct cm32181.
> @@ -121,6 +181,44 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
>         cm32181->lux_per_bit = CM32181_LUX_PER_BIT;
>         cm32181->lux_per_bit_base_it = CM32181_LUX_PER_BIT_BASE_IT;
>
> +#ifdef CONFIG_ACPI
> +       if (ACPI_HANDLE(&client->dev)) {
> +               u64 values[CPM0_HEADER_SIZE + CM32181_CONF_REG_NUM];
> +               int count;
> +
> +               count = cm32181_acpi_get_cpm(client, "CPM0",
> +                                            values, ARRAY_SIZE(values));
> +               if (count <= CPM0_HEADER_SIZE)
> +                       goto cpm_parsing_done;
> +
> +               count -= CPM0_HEADER_SIZE;
> +
> +               cm32181->init_regs_bitmap = values[CPM0_REGS_BITMAP];
> +               cm32181->init_regs_bitmap &= GENMASK(count - 1, 0);
> +               for (i = 0; i < count; i++) {
> +                       if (cm32181->init_regs_bitmap & BIT(i))
> +                               cm32181->conf_regs[i] =
> +                                       values[CPM0_HEADER_SIZE + i];
> +               }
> +
> +               count = cm32181_acpi_get_cpm(client, "CPM1",
> +                                            values, ARRAY_SIZE(values));
> +               if (count != CPM1_SIZE)
> +                       goto cpm_parsing_done;
> +
> +               cm32181->lux_per_bit = values[CPM1_LUX_PER_BIT];
> +
> +               /* Check for uncalibrated devices */
> +               if (values[CPM1_CALIBSCALE] == CM32181_CALIBSCALE_DEFAULT)
> +                       goto cpm_parsing_done;
> +
> +               cm32181->calibscale  = values[CPM1_CALIBSCALE];
> +               /* CPM1 lux_per_bit is for the current it value */
> +               cm32181_read_als_it(cm32181, &cm32181->lux_per_bit_base_it);
> +       }
> +cpm_parsing_done:

Perhaps factor out to a helper, will
a) allow to get rid of a label;
b) drop indentation level.

> +#endif /* CONFIG_ACPI */
> +
>         /* Initialize registers*/
>         for (i = 0; i < CM32181_CONF_REG_NUM; i++) {
>                 if (cm32181->init_regs_bitmap & BIT(i)) {
> --
> 2.26.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 8/8] iio: light: cm32181: Fix integartion time typo

[Andy Shevchenko]

On Sun, Apr 26, 2020 at 2:03 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Fix integartion time typo and while at it improve the comment with
> the typo a bit in general.
>

FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

(Applicable to all patches, after addressing corresponding comments)

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/iio/light/cm32181.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
> index 878fc13632d8..50cfb7d189df 100644
> --- a/drivers/iio/light/cm32181.c
> +++ b/drivers/iio/light/cm32181.c
> @@ -237,7 +237,7 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
>   *  @cm32181:  pointer of struct cm32181
>   *  @val2:     pointer of int to load the als_it value.
>   *
> - *  Report the current integartion time by millisecond.
> + *  Report the current integration time in milliseconds.
>   *
>   *  Return: IIO_VAL_INT_PLUS_MICRO for success, otherwise -EINVAL.
>   */
> --
> 2.26.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 7/8] iio: light: cm32181: Add support for parsing CPM0 and CPM1 ACPI tables

[Hans de Goede]

Hi,

On 4/26/20 7:50 PM, Andy Shevchenko wrote:
> On Sun, Apr 26, 2020 at 2:03 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> On ACPI based systems the CPLM3218 ACPI device node describing the
>> CM3218[1] sensor typically will have some extra tables with register
>> init values for initializing the sensor and calibration info.
>>
>> This is based on a newer version of cm32181.c, with a copyright of:
>>
>>   * Copyright (C) 2014 Capella Microsystems Inc.
>>   * Author: Kevin Tsai <ktsai@capellamicro.com>
>>   *
>>   * This program is free software; you can redistribute it and/or modify it
>>   * under the terms of the GNU General Public License version 2, as published
>>   * by the Free Software Foundation.
>>
>> Which is floating around on the net in various places, but the changes
>> from this newer version never made it upstream.
>>
>> This was tested on the following models: Acer Switch 10 SW5-012 (CM32181)
>> Asus T100TA (CM3218), Asus T100CHI (CM3218) and HP X2 10-n000nd (CM32181).
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/iio/light/cm32181.c | 98 +++++++++++++++++++++++++++++++++++++
>>   1 file changed, 98 insertions(+)
>>
>> diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
>> index e5674d4a8143..878fc13632d8 100644
>> --- a/drivers/iio/light/cm32181.c
>> +++ b/drivers/iio/light/cm32181.c
>> @@ -4,6 +4,7 @@
>>    * Author: Kevin Tsai <ktsai@capellamicro.com>
>>    */
>>
>> +#include <linux/acpi.h>
>>   #include <linux/delay.h>
>>   #include <linux/err.h>
>>   #include <linux/i2c.h>
>> @@ -53,6 +54,15 @@
>>
>>   #define SMBUS_ALERT_RESPONSE_ADDRESS   0x0c
>>
>> +/* CPM0 Index 0: device-id (3218 or 32181), 1: Unknown, 2: init_regs_bitmap */
>> +#define CPM0_REGS_BITMAP               2
>> +#define CPM0_HEADER_SIZE               3
>> +
>> +/* CPM1 Index 0: lux_per_bit, 1: calibscale, 2: resolution (100000) */
>> +#define CPM1_LUX_PER_BIT               0
>> +#define CPM1_CALIBSCALE                        1
>> +#define CPM1_SIZE                      3
>> +
>>   /* CM3218 Family */
>>   static const int cm3218_als_it_bits[] = { 0, 1, 2, 3 };
>>   static const int cm3218_als_it_values[] = { 100000, 200000, 400000, 800000 };
>> @@ -76,6 +86,56 @@ struct cm32181_chip {
>>          const int *als_it_values;
>>   };
>>
>> +static int cm32181_read_als_it(struct cm32181_chip *cm32181, int *val2);
>> +
>> +#ifdef CONFIG_ACPI
>> +/**
>> + * cm32181_acpi_get_cpm() - Get CPM object from ACPI
>> + * @client     pointer of struct i2c_client.
>> + * @obj_name   pointer of ACPI object name.
>> + * @count      maximum size of return array.
>> + * @vals       pointer of array for return elements.
>> + *
>> + * Convert ACPI CPM table to array.
>> + *
>> + * Return: -ENODEV for fail.  Otherwise is number of elements.
>> + */
>> +static int cm32181_acpi_get_cpm(struct i2c_client *client, char *obj_name,
>> +                               u64 *values, int count)
>> +{
>> +       struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>> +       union acpi_object *cpm, *elem;
>> +       acpi_handle handle;
>> +       acpi_status status;
>> +       int i;
>> +
>> +       handle = ACPI_HANDLE(&client->dev);
>> +       if (!handle)
>> +               return -ENODEV;
>> +
>> +       status = acpi_evaluate_object(handle, obj_name, NULL, &buffer);
>> +       if (ACPI_FAILURE(status)) {
>> +               dev_err(&client->dev, "object %s not found\n", obj_name);
>> +               return -ENODEV;
>> +       }
>> +
>> +       cpm = buffer.pointer;
>> +       if (cpm->package.count > count)
>> +               dev_warn(&client->dev, "%s table contains %d values, only using first %d values\n",
>> +                        obj_name, cpm->package.count, count);
>> +
>> +       count = min_t(int, cpm->package.count, count);
>> +       for (i = 0; i < count; i++) {
>> +               elem = &(cpm->package.elements[i]);
>> +               values[i] = elem->integer.value;
>> +       }
>> +
>> +       kfree(buffer.pointer);
>> +
>> +       return count;
>> +}
>> +#endif /* CONFIG_ACPI */
>> +
>>   /**
>>    * cm32181_reg_init() - Initialize CM32181 registers
>>    * @cm32181:   pointer of struct cm32181.
>> @@ -121,6 +181,44 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
>>          cm32181->lux_per_bit = CM32181_LUX_PER_BIT;
>>          cm32181->lux_per_bit_base_it = CM32181_LUX_PER_BIT_BASE_IT;
>>
>> +#ifdef CONFIG_ACPI
>> +       if (ACPI_HANDLE(&client->dev)) {
>> +               u64 values[CPM0_HEADER_SIZE + CM32181_CONF_REG_NUM];
>> +               int count;
>> +
>> +               count = cm32181_acpi_get_cpm(client, "CPM0",
>> +                                            values, ARRAY_SIZE(values));
>> +               if (count <= CPM0_HEADER_SIZE)
>> +                       goto cpm_parsing_done;
>> +
>> +               count -= CPM0_HEADER_SIZE;
>> +
>> +               cm32181->init_regs_bitmap = values[CPM0_REGS_BITMAP];
>> +               cm32181->init_regs_bitmap &= GENMASK(count - 1, 0);
>> +               for (i = 0; i < count; i++) {
>> +                       if (cm32181->init_regs_bitmap & BIT(i))
>> +                               cm32181->conf_regs[i] =
>> +                                       values[CPM0_HEADER_SIZE + i];
>> +               }
>> +
>> +               count = cm32181_acpi_get_cpm(client, "CPM1",
>> +                                            values, ARRAY_SIZE(values));
>> +               if (count != CPM1_SIZE)
>> +                       goto cpm_parsing_done;
>> +
>> +               cm32181->lux_per_bit = values[CPM1_LUX_PER_BIT];
>> +
>> +               /* Check for uncalibrated devices */
>> +               if (values[CPM1_CALIBSCALE] == CM32181_CALIBSCALE_DEFAULT)
>> +                       goto cpm_parsing_done;
>> +
>> +               cm32181->calibscale  = values[CPM1_CALIBSCALE];
>> +               /* CPM1 lux_per_bit is for the current it value */
>> +               cm32181_read_als_it(cm32181, &cm32181->lux_per_bit_base_it);
>> +       }
>> +cpm_parsing_done:
> 
> Perhaps factor out to a helper, will
> a) allow to get rid of a label;
> b) drop indentation level.

Thank you for the reviews.

Factoring this into a helper is a good idea, that will also allow
getting rid of 1 of the CONFIG_ACPI #ifdef-s.

I also agree with your comments on the other patches.

I'll prepare and test a v2 with these changes and then submit
it with your Reviewed-by added.

Regards,

Hans