[PATCH v3] iio : Add cm3218 smbus ara and acpi support

[PATCH v3] iio : Add cm3218 smbus ara and acpi support

[Marc CAPDEVILLE]

On asus T100, Capella cm3218 chip is implemented as ambiant light
sensor. This chip expose an smbus ARA protocol device on standard
address 0x0c. The chip is not functional before all alerts are
acknowledged.
On asus T100, this device is enumerated on ACPI bus and the description
give tow I2C connection. The first is the connection to the ARA device
and the second gives the real address of the device.

So, on device probe,If the i2c address is the ARA address and the
device is enumerated via acpi, we lookup for the real address in
the ACPI resource list and change it in the client structure.
if an interrupt resource is given, and only for cm3218 chip,
we declare an smbus_alert device.

Signed-off-by: Marc CAPDEVILLE <m.capdeville@no-log.org>
---

Thanks to all for your review.

It seems that capellamicro.com  mailhost timeout when sending to Kevin.

v3 :
   - rework acpi i2c adress lookup
   - comment style cleanup
   - add prefix cm32181_to constantes and macros

v2 :
   - cm3218 support always build
   - Cleanup of unneeded #if statement
   - Beter identifying chip in platform device, acpi and of_device

 drivers/iio/light/cm32181.c | 202 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 185 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index d6fd0dace74f..f1a7495e851b 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -18,8 +18,12 @@
 #include <linux/iio/sysfs.h>
 #include <linux/iio/events.h>
 #include <linux/init.h>
+#include <linux/i2c-smbus.h>
+#include <linux/acpi.h>
+#include <linux/of_device.h>
+#include <linux/resource_ext.h>
 
-/* Registers Address */
+/* Registers Addresses */
 #define CM32181_REG_ADDR_CMD		0x00
 #define CM32181_REG_ADDR_ALS		0x04
 #define CM32181_REG_ADDR_STATUS		0x06
@@ -45,18 +49,25 @@
 #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_MLUX_PER_LUX			1000
+
+#define CM32181_ID			0x81
+#define CM3218_ID			0x18
+
+#define CM3218_ARA_ADDR			0x0c
 
 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,
+static const int cm32181_als_it_bits[] = {12, 8, 0, 1, 2, 3};
+static const int cm32181_als_it_value[] = {25000, 50000, 100000, 200000, 400000,
 	800000};
 
 struct cm32181_chip {
 	struct i2c_client *client;
+	int chip_id;
+	struct i2c_client *ara;
 	struct mutex lock;
 	u16 conf_regs[CM32181_CONF_REG_NUM];
 	int calibscale;
@@ -81,7 +92,7 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
 		return ret;
 
 	/* check device ID */
-	if ((ret & 0xFF) != 0x81)
+	if ((ret & 0xFF) != cm32181->chip_id)
 		return -ENODEV;
 
 	/* Default Values */
@@ -103,7 +114,7 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
 /**
  *  cm32181_read_als_it() - Get sensor integration time (ms)
  *  @cm32181:	pointer of struct cm32181
- *  @val2:	pointer of int to load the als_it value.
+ *  @val2:	pointer of int to load the cm32181_als_it value.
  *
  *  Report the current integartion time by millisecond.
  *
@@ -117,9 +128,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 < ARRAY_SIZE(cm32181_als_it_bits); i++) {
+		if (als_it == cm32181_als_it_bits[i]) {
+			*val2 = cm32181_als_it_value[i];
 			return IIO_VAL_INT_PLUS_MICRO;
 		}
 	}
@@ -142,14 +153,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 = ARRAY_SIZE(cm32181_als_it_value);
 	for (i = 0; i < n; i++)
-		if (val <= als_it_value[i])
+		if (val <= cm32181_als_it_value[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);
@@ -195,7 +206,7 @@ static int cm32181_get_lux(struct cm32181_chip *cm32181)
 	lux *= ret;
 	lux *= cm32181->calibscale;
 	lux /= CM32181_CALIBSCALE_RESOLUTION;
-	lux /= MLUX_PER_LUX;
+	lux /= CM32181_MLUX_PER_LUX;
 
 	if (lux > 0xFFFF)
 		lux = 0xFFFF;
@@ -263,9 +274,9 @@ static ssize_t cm32181_get_it_available(struct device *dev,
 {
 	int i, n, len;
 
-	n = ARRAY_SIZE(als_it_value);
+	n = ARRAY_SIZE(cm32181_als_it_value);
 	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_value[i]);
 	return len + sprintf(buf + len, "\n");
 }
 
@@ -298,12 +309,89 @@ static const struct iio_info cm32181_info = {
 	.attrs			= &cm32181_attribute_group,
 };
 
+#if defined(CONFIG_ACPI)
+/* Filter acpi resources for a real i2c address on same adapter */
+static int cm3218_filter_i2c_address(struct acpi_resource *ares, void *data)
+{
+	struct i2c_client *client = data;
+	struct acpi_resource_i2c_serialbus *sb;
+	acpi_status status;
+	acpi_handle adapter_handle;
+
+	if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
+		return 1;
+
+	sb = &ares->data.i2c_serial_bus;
+	if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
+		return 1;
+
+	status = acpi_get_handle(ACPI_HANDLE(&client->dev),
+				 sb->resource_source.string_ptr,
+				 &adapter_handle);
+	if (ACPI_FAILURE(status))
+		return status;
+
+	if (adapter_handle != ACPI_HANDLE(&client->adapter->dev))
+		return 1;
+
+	if (sb->slave_address == CM3218_ARA_ADDR)
+		return 1;
+
+	return AE_OK;
+}
+
+/* Get the real i2c address from acpi resources */
+static int cm3218_acpi_get_address(struct i2c_client *client)
+{
+	int ret;
+	struct acpi_device *adev;
+	LIST_HEAD(res_list);
+	struct resource_entry *res_entry;
+	struct acpi_resource *ares;
+	struct acpi_resource_i2c_serialbus *sb;
+
+	adev = ACPI_COMPANION(&client->dev);
+	if (!adev)
+		return -ENODEV;
+
+	ret = acpi_dev_get_resources(adev,
+				     &res_list,
+				     cm3218_filter_i2c_address,
+				     client);
+	if (ret < 0)
+		return ret;
+
+	/* get first resource */
+	res_entry = list_entry(&res_list, struct resource_entry, node);
+	ares = (struct acpi_resource *)res_entry->res;
+	sb = &ares->data.i2c_serial_bus;
+
+	/* set i2c address */
+	client->addr = sb->slave_address;
+	client->flags &= ~I2C_CLIENT_TEN;
+	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
+		client->flags |= I2C_CLIENT_TEN;
+
+	acpi_dev_free_resource_list(&res_list);
+
+	return 0;
+}
+#else
+static inline int cm3218_acpi_get_address(struct i2c_client *client)
+{
+	return -ENODEV;
+}
+#endif
+
 static int cm32181_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
 	struct cm32181_chip *cm32181;
 	struct iio_dev *indio_dev;
 	int ret;
+	struct i2c_smbus_alert_setup ara_setup;
+	const struct of_device_id *of_id;
+	const struct acpi_device_id *acpi_id;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*cm32181));
 	if (!indio_dev) {
@@ -323,11 +411,65 @@ static int cm32181_probe(struct i2c_client *client,
 	indio_dev->name = id->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
+	/* Lookup for chip ID from platform, acpi or of device table */
+	if (id) {
+		cm32181->chip_id = id->driver_data;
+	} else if (ACPI_COMPANION(&client->dev)) {
+		acpi_id = acpi_match_device(client->dev.driver->acpi_match_table,
+					    &client->dev);
+		if (!acpi_id)
+			return -ENODEV;
+
+		cm32181->chip_id = (kernel_ulong_t)acpi_id->driver_data;
+	} else if (client->dev.of_node) {
+		of_id = of_match_device(clients->dev.driver->of_match_table,
+					&client->dev);
+		if (!of_id)
+			return -ENODEV;
+
+		cm32181->chip_id = (kernel_ulong_t)of_id->data;
+	} else {
+		return -ENODEV;
+	}
+
+	if (cm32181->chip_id == CM3218_ID) {
+		if (client->addr == CM3218_ARA_ADDR) {
+			/*
+			 * In case first address is the ARA device
+			 * lookup for a second one in acpi resources if
+			 * this client is enumerated on acpi
+			 */
+			ret = cm3218_acpi_get_address(client);
+			if (ret < 0)
+				return -ENODEV;
+		}
+
+#ifdef CONFIG_I2C_SMBUS
+		if (client->irq > 0) {
+			/* setup smb alert device */
+			ara_setup.irq = client->irq;
+			ara_setup.alert_edge_triggered = 0;
+			cm32181->ara = i2c_setup_smbus_alert(client->adapter,
+							     &ara_setup);
+			if (!cm32181->ara)
+				return -ENODEV;
+		} else {
+			return -ENODEV;
+		}
+#else
+		return -ENODEV;
+#endif
+	}
+
 	ret = cm32181_reg_init(cm32181);
 	if (ret) {
 		dev_err(&client->dev,
 			"%s: register init failed\n",
 			__func__);
+
+		if (cm32181->ara)
+			i2c_unregister_device(cm32181->ara);
+
 		return ret;
 	}
 
@@ -336,32 +478,58 @@ static int cm32181_probe(struct i2c_client *client,
 		dev_err(&client->dev,
 			"%s: regist device failed\n",
 			__func__);
+
+		if (cm32181->ara)
+			i2c_unregister_device(cm32181->ara);
+
 		return ret;
 	}
 
 	return 0;
 }
 
+static int cm32181_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct cm32181_chip *cm32181 = iio_priv(indio_dev);
+
+	if (cm32181->ara)
+		i2c_unregister_device(cm32181->ara);
+
+	return 0;
+};
+
 static const struct i2c_device_id cm32181_id[] = {
-	{ "cm32181", 0 },
+	{ "cm32181", CM32181_ID },
+	{ "cm3218", CM3218_ID },
 	{ }
 };
 
 MODULE_DEVICE_TABLE(i2c, cm32181_id);
 
 static const struct of_device_id cm32181_of_match[] = {
-	{ .compatible = "capella,cm32181" },
+	{ .compatible = "capella,cm32181", (void *)CM32181_ID },
+	{ .compatible = "capella,cm3218",  (void *)CM3218_ID },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, cm32181_of_match);
 
+static const struct acpi_device_id cm32181_acpi_match[] = {
+	{ "CPLM3218", CM3218_ID },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(acpi, cm32181_acpi_match);
+
 static struct i2c_driver cm32181_driver = {
 	.driver = {
 		.name	= "cm32181",
 		.of_match_table = of_match_ptr(cm32181_of_match),
+		.acpi_match_table = ACPI_PTR(cm32181_acpi_match),
 	},
 	.id_table       = cm32181_id,
 	.probe		= cm32181_probe,
+	.remove		= cm32181_remove,
 };
 
 module_i2c_driver(cm32181_driver);
-- 
2.11.0

Re: [PATCH v3] iio : Add cm3218 smbus ara and acpi support

[Lars-Peter Clausen]

On 10/27/2017 03:20 PM, Marc CAPDEVILLE wrote:
> On asus T100, Capella cm3218 chip is implemented as ambiant light
> sensor. This chip expose an smbus ARA protocol device on standard
> address 0x0c. The chip is not functional before all alerts are
> acknowledged.
> On asus T100, this device is enumerated on ACPI bus and the description
> give tow I2C connection. The first is the connection to the ARA device

two

> and the second gives the real address of the device.

Is there a document somewhere describing what this ARA stuff is and how it
works? Is this "Project ARA"?

> 
> So, on device probe,If the i2c address is the ARA address and the
> device is enumerated via acpi, we lookup for the real address in
> the ACPI resource list and change it in the client structure.
> if an interrupt resource is given, and only for cm3218 chip,
> we declare an smbus_alert device.
> 
> Signed-off-by: Marc CAPDEVILLE <m.capdeville@no-log.org>
[...]
> +/* Get the real i2c address from acpi resources */
> +static int cm3218_acpi_get_address(struct i2c_client *client)
> +{
> +	int ret;
> +	struct acpi_device *adev;
> +	LIST_HEAD(res_list);
> +	struct resource_entry *res_entry;
> +	struct acpi_resource *ares;
> +	struct acpi_resource_i2c_serialbus *sb;
> +
> +	adev = ACPI_COMPANION(&client->dev);
> +	if (!adev)
> +		return -ENODEV;
> +
> +	ret = acpi_dev_get_resources(adev,
> +				     &res_list,
> +				     cm3218_filter_i2c_address,
> +				     client);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* get first resource */
> +	res_entry = list_entry(&res_list, struct resource_entry, node);
> +	ares = (struct acpi_resource *)res_entry->res;
> +	sb = &ares->data.i2c_serial_bus;
> +
> +	/* set i2c address */
> +	client->addr = sb->slave_address;
> +	client->flags &= ~I2C_CLIENT_TEN;

I'm not convinced overwriting the address and flags is legal. Those are
owned by the I2C core and should be read-only in the client driver. Maybe
use i2c_new_dummy() to create a device for the new address?

> +	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
> +		client->flags |= I2C_CLIENT_TEN;
> +
> +	acpi_dev_free_resource_list(&res_list);
> +
> +	return 0;
> +}
[...]

Re: [PATCH v3] iio : Add cm3218 smbus ara and acpi support

[Lars-Peter Clausen]

On 10/30/2017 11:54 AM, Lars-Peter Clausen wrote:
> On 10/27/2017 03:20 PM, Marc CAPDEVILLE wrote:
>> On asus T100, Capella cm3218 chip is implemented as ambiant light
>> sensor. This chip expose an smbus ARA protocol device on standard
>> address 0x0c. The chip is not functional before all alerts are
>> acknowledged.
>> On asus T100, this device is enumerated on ACPI bus and the description
>> give tow I2C connection. The first is the connection to the ARA device
> 
> two
> 
>> and the second gives the real address of the device.
> 
> Is there a document somewhere describing what this ARA stuff is and how it
> works? Is this "Project ARA"?

Found it: "Address Response Alert". Given that the ARA address 0x0c is an
reserved address and should not be used for normal I2C devices I wonder if
the I2C core shouldn't ignore it and get the correct address on its own when
instantiating the I2C slave device? I'd assume that this issue is not
specific to this particular device.

> 
>>
>> So, on device probe,If the i2c address is the ARA address and the
>> device is enumerated via acpi, we lookup for the real address in
>> the ACPI resource list and change it in the client structure.
>> if an interrupt resource is given, and only for cm3218 chip,
>> we declare an smbus_alert device.
>>
>> Signed-off-by: Marc CAPDEVILLE <m.capdeville@no-log.org>
> [...]
>> +/* Get the real i2c address from acpi resources */
>> +static int cm3218_acpi_get_address(struct i2c_client *client)
>> +{
>> +	int ret;
>> +	struct acpi_device *adev;
>> +	LIST_HEAD(res_list);
>> +	struct resource_entry *res_entry;
>> +	struct acpi_resource *ares;
>> +	struct acpi_resource_i2c_serialbus *sb;
>> +
>> +	adev = ACPI_COMPANION(&client->dev);
>> +	if (!adev)
>> +		return -ENODEV;
>> +
>> +	ret = acpi_dev_get_resources(adev,
>> +				     &res_list,
>> +				     cm3218_filter_i2c_address,
>> +				     client);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* get first resource */
>> +	res_entry = list_entry(&res_list, struct resource_entry, node);
>> +	ares = (struct acpi_resource *)res_entry->res;
>> +	sb = &ares->data.i2c_serial_bus;
>> +
>> +	/* set i2c address */
>> +	client->addr = sb->slave_address;
>> +	client->flags &= ~I2C_CLIENT_TEN;
> 
> I'm not convinced overwriting the address and flags is legal. Those are
> owned by the I2C core and should be read-only in the client driver. Maybe
> use i2c_new_dummy() to create a device for the new address?
> 
>> +	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
>> +		client->flags |= I2C_CLIENT_TEN;
>> +
>> +	acpi_dev_free_resource_list(&res_list);
>> +
>> +	return 0;
>> +}
> [...]
> 

Re: [PATCH v3] iio : Add cm3218 smbus ara and acpi support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 231 bytes --]


> Is there a document somewhere describing what this ARA stuff is and how it
> works? Is this "Project ARA"?

Check the SMBus specs for "Alert Response Address".

Note: This is general info, I haven't checked this driver at all.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3] iio : Add cm3218 smbus ara and acpi support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3441 bytes --]

Hi Marc,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on v4.14-rc7 next-20171018]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Marc-CAPDEVILLE/iio-Add-cm3218-smbus-ara-and-acpi-support/20171030-195025
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: x86_64-randconfig-b0-10311428 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/iio/light/cm32181.c: In function 'cm32181_probe':
   drivers/iio/light/cm32181.c:424: error: 'clients' undeclared (first use in this function)
   drivers/iio/light/cm32181.c:424: error: (Each undeclared identifier is reported only once
   drivers/iio/light/cm32181.c:424: error: for each function it appears in.)
>> drivers/iio/light/cm32181.c:391: warning: unused variable 'ara_setup'

vim +/ara_setup +391 drivers/iio/light/cm32181.c

   384	
   385	static int cm32181_probe(struct i2c_client *client,
   386				const struct i2c_device_id *id)
   387	{
   388		struct cm32181_chip *cm32181;
   389		struct iio_dev *indio_dev;
   390		int ret;
 > 391		struct i2c_smbus_alert_setup ara_setup;
   392		const struct of_device_id *of_id;
   393		const struct acpi_device_id *acpi_id;
   394	
   395		indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*cm32181));
   396		if (!indio_dev) {
   397			dev_err(&client->dev, "devm_iio_device_alloc failed\n");
   398			return -ENOMEM;
   399		}
   400	
   401		cm32181 = iio_priv(indio_dev);
   402		i2c_set_clientdata(client, indio_dev);
   403		cm32181->client = client;
   404	
   405		mutex_init(&cm32181->lock);
   406		indio_dev->dev.parent = &client->dev;
   407		indio_dev->channels = cm32181_channels;
   408		indio_dev->num_channels = ARRAY_SIZE(cm32181_channels);
   409		indio_dev->info = &cm32181_info;
   410		indio_dev->name = id->name;
   411		indio_dev->modes = INDIO_DIRECT_MODE;
   412	
   413		/* Lookup for chip ID from platform, acpi or of device table */
   414		if (id) {
   415			cm32181->chip_id = id->driver_data;
   416		} else if (ACPI_COMPANION(&client->dev)) {
   417			acpi_id = acpi_match_device(client->dev.driver->acpi_match_table,
   418						    &client->dev);
   419			if (!acpi_id)
   420				return -ENODEV;
   421	
   422			cm32181->chip_id = (kernel_ulong_t)acpi_id->driver_data;
   423		} else if (client->dev.of_node) {
 > 424			of_id = of_match_device(clients->dev.driver->of_match_table,
   425						&client->dev);
   426			if (!of_id)
   427				return -ENODEV;
   428	
   429			cm32181->chip_id = (kernel_ulong_t)of_id->data;
   430		} else {
   431			return -ENODEV;
   432		}
   433	
   434		if (cm32181->chip_id == CM3218_ID) {
   435			if (client->addr == CM3218_ARA_ADDR) {
   436				/*
   437				 * In case first address is the ARA device
   438				 * lookup for a second one in acpi resources if
   439				 * this client is enumerated on acpi
   440				 */
   441				ret = cm3218_acpi_get_address(client);
   442				if (ret < 0)
   443					return -ENODEV;
   444			}
   445	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28748 bytes --]

[PATCH v3] iio : Add cm3218 smbus ara and acpi support

[Marc CAPDEVILLE]

I'm sorry, this patch is not functionnal.
I'll resubmit in v4 soon.

I apologize for the inconvenience.