| Message ID | 20171027161426.m27e52jvnpgembsu@azrael (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> 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. I think the unrelated cleanups should have been put in a separate patch (I feel sorry for guiding you in the wrong direction and not pointing this out more clearly) more very minor comments below > Signed-off-by: Marc CAPDEVILLE <m.capdeville@no-log.org> > --- > v4 : > - rework acpi i2c adress lookup due to bad commit being sent. > > 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 */ Register 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) the unnecessary inline is still there... the compiler will figure out on its own even without it (and I think the function signatures should be the same with and without CONFIG_ACPI > +{ > + 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); >
On Fri, 27 Oct 2017 18:27:02 +0200 Marc CAPDEVILLE <m.capdeville@no-log.org> 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 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> Wolfram - this needs input from you on how to neatly handle an ACPI registered ARA. Thanks, Jonathan > --- > v4 : > - rework acpi i2c adress lookup due to bad commit being sent. > > 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; This bares so little resemblance to existing uses of the ARA infrastructure that I'm not going to take this until Wolfram has time to look at it. In all other instances the setup_smbus_alert stuff has been done from a bus driver, not a client driver. Unfortunately ACPI support is thin on the ground so it may be this is the only way we can do it. There was a relevant patch from Srinivas that google found for me: https://patchwork.ozlabs.org/patch/386410/ This ensures that we probe the device on the valid address not the ARA. Looks like Srinivas' patch never went anywhere. However I think it was correct in it's assumption that this sort of support should be in the core. > + } > + > +#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); -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2017-11-02 at 14:35 +0000, Jonathan Cameron wrote: > On Fri, 27 Oct 2017 18:27:02 +0200 > Marc CAPDEVILLE <m.capdeville@no-log.org> 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 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> > Wolfram - this needs input from you on how to neatly handle > an ACPI registered ARA. There was another RFC from Alan cox https://patchwork.ozlabs.org/patch/381773/ Thanks, Srinivas > > Thanks, > > Jonathan > > > > --- > > v4 : > > - rework acpi i2c adress lookup due to bad commit being sent. > > > > 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 100 > > 0 > > #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; > This bares so little resemblance to existing uses of the ARA > infrastructure that I'm not going to take this until Wolfram > has time to look at it. > > In all other instances the setup_smbus_alert stuff has been > done from a bus driver, not a client driver. > > Unfortunately ACPI support is thin on the ground so it may be > this is the only way we can do it. > > There was a relevant patch from Srinivas that google found > for me: https://patchwork.ozlabs.org/patch/386410/ > > This ensures that we probe the device on the valid address > not the ARA. > > Looks like Srinivas' patch never went anywhere. However > I think it was correct in it's assumption that this sort > of support should be in the core. > > > > > + } > > + > > +#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); -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 02, 2017 at 02:35:50PM +0000, Jonathan Cameron wrote: > On Fri, 27 Oct 2017 18:27:02 +0200 > Marc CAPDEVILLE <m.capdeville@no-log.org> 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 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> > > Wolfram - this needs input from you on how to neatly handle > an ACPI registered ARA. ACPI is really not my field. Try asking the I2C ACPI maintainers or Benjamin Tissoires <benjamin.tissoires@redhat.com> who did work on SMBus interrupts recently.
On 2/11/2017 22:49, Srinivas Pandruvada wrote: > On Thu, 2017-11-02 at 14:35 +0000, Jonathan Cameron wrote: >> On Fri, 27 Oct 2017 18:27:02 +0200 >> Marc CAPDEVILLE <m.capdeville@no-log.org> 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 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> >> Wolfram - this needs input from you on how to neatly handle >> an ACPI registered ARA. > There was another RFC from Alan cox > https://patchwork.ozlabs.org/patch/381773/ > Wolfram just merged this from me: [PATCH v11 00/10] Add sbs-manager with smbalert support https://www.spinics.net/lists/devicetree/msg191947.html Cleans up the smbus_alert driver a bit. note: alert_edge_triggered was removed. And for OF systems core creates the ara device.
On Thu, 2 Nov 2017 16:04:07 +0100 Wolfram Sang <wsa@the-dreams.de> wrote: > On Thu, Nov 02, 2017 at 02:35:50PM +0000, Jonathan Cameron wrote: > > On Fri, 27 Oct 2017 18:27:02 +0200 > > Marc CAPDEVILLE <m.capdeville@no-log.org> 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 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> > > > > Wolfram - this needs input from you on how to neatly handle > > an ACPI registered ARA. > > ACPI is really not my field. Try asking the I2C ACPI maintainers or > Benjamin Tissoires <benjamin.tissoires@redhat.com> who did work on SMBus > interrupts recently. > Hi Mika, Benjamin, So we've lost most of the context in this thread, but the basic question is how to handle smbus ARA support with ACPI. https://patchwork.kernel.org/patch/10030309/ Has the proposal made in this driver which is really not terribly nice (as it registers the ARA device by messing with the address and registering a second device). As I understood it the ARA device registration should be handled by the i2c master, but there are very few examples. Phil pointed out that equivalent OF support recently got taken from him.. https://www.spinics.net/lists/devicetree/msg191947.html https://www.spinics.net/lists/linux-i2c/msg31173.html Any thoughts on the right way to do this? Jonathan -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
+Jarkko On Sun, Nov 19, 2017 at 04:35:51PM +0000, Jonathan Cameron wrote: > On Thu, 2 Nov 2017 16:04:07 +0100 > Wolfram Sang <wsa@the-dreams.de> wrote: > > > On Thu, Nov 02, 2017 at 02:35:50PM +0000, Jonathan Cameron wrote: > > > On Fri, 27 Oct 2017 18:27:02 +0200 > > > Marc CAPDEVILLE <m.capdeville@no-log.org> 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 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> > > > > > > Wolfram - this needs input from you on how to neatly handle > > > an ACPI registered ARA. > > > > ACPI is really not my field. Try asking the I2C ACPI maintainers or > > Benjamin Tissoires <benjamin.tissoires@redhat.com> who did work on SMBus > > interrupts recently. > > > Hi Mika, Benjamin, > > So we've lost most of the context in this thread, but the basic question > is how to handle smbus ARA support with ACPI. > > https://patchwork.kernel.org/patch/10030309/ > > Has the proposal made in this driver which is really not terribly nice > (as it registers the ARA device by messing with the address and registering > a second device). > > As I understood it the ARA device registration should be handled by the > i2c master, but there are very few examples. > > Phil pointed out that equivalent OF support recently got taken from him.. > https://www.spinics.net/lists/devicetree/msg191947.html > https://www.spinics.net/lists/linux-i2c/msg31173.html > > Any thoughts on the right way to do this? There does not seem to be any way in ACPI to tell which "connection" is used to describe ARA so that part currently is something each driver needs to handle as they know the device the best. I don't think we have any means to handle it in generic way in I2C core except to provide some helpers that work on top of i2c_setup_smbus_alert() but understand ACPI resources. Say provide function like this: int acpi_i2c_setup_smbus_alert(struct i2c_adapter *adapter, int index); Which then extracts automatically I2cSerialBus connection from "index" and calls i2c_setup_smbus_alert() accordingly. In the long run we could introduce _DSD property that can be used to name the connection in the same way DT does; Name (_CRS, ResourceTemplate () { I2cSerialBus () { ... } // ARA I2cSerialBus () { ... } // normal device address }) Name (_DSD, Package () { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package () { Package () {"smbus_alert", 0} // Where 0 means the first I2cSerialBus ... } }) But it does not help the existing systems. -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20/11/2017 18:57, Mika Westerberg wrote: > +Jarkko > > On Sun, Nov 19, 2017 at 04:35:51PM +0000, Jonathan Cameron wrote: >> On Thu, 2 Nov 2017 16:04:07 +0100 >> Wolfram Sang <wsa@the-dreams.de> wrote: >> >>> On Thu, Nov 02, 2017 at 02:35:50PM +0000, Jonathan Cameron wrote: >>>> On Fri, 27 Oct 2017 18:27:02 +0200 >>>> Marc CAPDEVILLE <m.capdeville@no-log.org> 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 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> >>>> >>>> Wolfram - this needs input from you on how to neatly handle >>>> an ACPI registered ARA. >>> >>> ACPI is really not my field. Try asking the I2C ACPI maintainers or >>> Benjamin Tissoires <benjamin.tissoires@redhat.com> who did work on SMBus >>> interrupts recently. >>> >> Hi Mika, Benjamin, >> >> So we've lost most of the context in this thread, but the basic question >> is how to handle smbus ARA support with ACPI. >> >> https://patchwork.kernel.org/patch/10030309/ >> >> Has the proposal made in this driver which is really not terribly nice >> (as it registers the ARA device by messing with the address and registering >> a second device). >> >> As I understood it the ARA device registration should be handled by the >> i2c master, but there are very few examples. >> >> Phil pointed out that equivalent OF support recently got taken from him.. >> https://www.spinics.net/lists/devicetree/msg191947.html >> https://www.spinics.net/lists/linux-i2c/msg31173.html >> >> Any thoughts on the right way to do this? > > There does not seem to be any way in ACPI to tell which "connection" is > used to describe ARA so that part currently is something each driver > needs to handle as they know the device the best. I don't think we have > any means to handle it in generic way in I2C core except to provide some > helpers that work on top of i2c_setup_smbus_alert() but understand ACPI > resources. Say provide function like this: > > int acpi_i2c_setup_smbus_alert(struct i2c_adapter *adapter, int index); > > Which then extracts automatically I2cSerialBus connection from "index" > and calls i2c_setup_smbus_alert() accordingly. > > In the long run we could introduce _DSD property that can be used to > name the connection in the same way DT does; > > Name (_CRS, ResourceTemplate () { > I2cSerialBus () { ... } // ARA > I2cSerialBus () { ... } // normal device address > }) > > Name (_DSD, Package () { > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), > Package () { > Package () {"smbus_alert", 0} // Where 0 means the first I2cSerialBus > ... > } > }) > I wonder if it's worth involving the smbus_alert driver in this case. The cm3218 driver doesn't appear to be using the alert callback in strcut i2c_driver. So the smbus_alert driver is not going to notifiy the cm3218 driver. Are there more than one alert/ara capable devices on the bus? Perhaps a workaround in this case is if that acpi entry is defined the cm3218 driver handles that ara request directly to clear the interrupt.
On Tue, 21 Nov 2017 09:22:16 +0800 Phil Reid <preid@electromag.com.au> wrote: > On 20/11/2017 18:57, Mika Westerberg wrote: > > +Jarkko > > > > On Sun, Nov 19, 2017 at 04:35:51PM +0000, Jonathan Cameron wrote: > >> On Thu, 2 Nov 2017 16:04:07 +0100 > >> Wolfram Sang <wsa@the-dreams.de> wrote: > >> > >>> On Thu, Nov 02, 2017 at 02:35:50PM +0000, Jonathan Cameron wrote: > >>>> On Fri, 27 Oct 2017 18:27:02 +0200 > >>>> Marc CAPDEVILLE <m.capdeville@no-log.org> 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 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> > >>>> > >>>> Wolfram - this needs input from you on how to neatly handle > >>>> an ACPI registered ARA. > >>> > >>> ACPI is really not my field. Try asking the I2C ACPI maintainers or > >>> Benjamin Tissoires <benjamin.tissoires@redhat.com> who did work on SMBus > >>> interrupts recently. > >>> > >> Hi Mika, Benjamin, > >> > >> So we've lost most of the context in this thread, but the basic question > >> is how to handle smbus ARA support with ACPI. > >> > >> https://patchwork.kernel.org/patch/10030309/ > >> > >> Has the proposal made in this driver which is really not terribly nice > >> (as it registers the ARA device by messing with the address and registering > >> a second device). > >> > >> As I understood it the ARA device registration should be handled by the > >> i2c master, but there are very few examples. > >> > >> Phil pointed out that equivalent OF support recently got taken from him.. > >> https://www.spinics.net/lists/devicetree/msg191947.html > >> https://www.spinics.net/lists/linux-i2c/msg31173.html > >> > >> Any thoughts on the right way to do this? > > > > There does not seem to be any way in ACPI to tell which "connection" is > > used to describe ARA so that part currently is something each driver > > needs to handle as they know the device the best. I don't think we have > > any means to handle it in generic way in I2C core except to provide some > > helpers that work on top of i2c_setup_smbus_alert() but understand ACPI > > resources. Say provide function like this: > > > > int acpi_i2c_setup_smbus_alert(struct i2c_adapter *adapter, int index); > > > > Which then extracts automatically I2cSerialBus connection from "index" > > and calls i2c_setup_smbus_alert() accordingly. > > > > In the long run we could introduce _DSD property that can be used to > > name the connection in the same way DT does; > > > > Name (_CRS, ResourceTemplate () { > > I2cSerialBus () { ... } // ARA > > I2cSerialBus () { ... } // normal device address > > }) > > > > Name (_DSD, Package () { > > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), > > Package () { > > Package () {"smbus_alert", 0} // Where 0 means the first I2cSerialBus > > ... > > } > > }) > > > > I wonder if it's worth involving the smbus_alert driver in this case. > The cm3218 driver doesn't appear to be using the alert callback in strcut i2c_driver. True - though it really should be.. > So the smbus_alert driver is not going to notifiy the cm3218 driver. > Are there more than one alert/ara capable devices on the bus? I'm not keen on taking a work around for one board on the basis that it only has this one device on the bus - we will probably get a different one down the line where this isn't true - then we end up ripping up what has been done so far and starting again. I don't mind having some ACPI matching code in the driver but it needs to use the ARA infrastructure to actually handle things... If we then get nice generic ACPI code via some other means in the future we can move the driver over to that. Sometimes I wonder if some people just write ACPI tables with no though to generalization at all, or that the code running might be shared across different machines. (sometimes that assumption is valid, but not that often... oh well - usual case of if we all work together everyone wins but not worth one company trying to do things right...) > Perhaps a workaround in this case is if that acpi entry is defined the cm3218 driver > handles that ara request directly to clear the interrupt. > Jonathan -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 20 Nov 2017 12:57:56 +0200 Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > +Jarkko > > On Sun, Nov 19, 2017 at 04:35:51PM +0000, Jonathan Cameron wrote: > > On Thu, 2 Nov 2017 16:04:07 +0100 > > Wolfram Sang <wsa@the-dreams.de> wrote: > > > > > On Thu, Nov 02, 2017 at 02:35:50PM +0000, Jonathan Cameron wrote: > > > > On Fri, 27 Oct 2017 18:27:02 +0200 > > > > Marc CAPDEVILLE <m.capdeville@no-log.org> 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 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> > > > > > > > > Wolfram - this needs input from you on how to neatly handle > > > > an ACPI registered ARA. > > > > > > ACPI is really not my field. Try asking the I2C ACPI maintainers or > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> who did work on SMBus > > > interrupts recently. > > > > > Hi Mika, Benjamin, > > > > So we've lost most of the context in this thread, but the basic question > > is how to handle smbus ARA support with ACPI. > > > > https://patchwork.kernel.org/patch/10030309/ > > > > Has the proposal made in this driver which is really not terribly nice > > (as it registers the ARA device by messing with the address and registering > > a second device). > > > > As I understood it the ARA device registration should be handled by the > > i2c master, but there are very few examples. > > > > Phil pointed out that equivalent OF support recently got taken from him.. > > https://www.spinics.net/lists/devicetree/msg191947.html > > https://www.spinics.net/lists/linux-i2c/msg31173.html > > > > Any thoughts on the right way to do this? > > There does not seem to be any way in ACPI to tell which "connection" is > used to describe ARA so that part currently is something each driver > needs to handle as they know the device the best. I don't think we have > any means to handle it in generic way in I2C core except to provide some > helpers that work on top of i2c_setup_smbus_alert() but understand ACPI > resources. Say provide function like this: > > int acpi_i2c_setup_smbus_alert(struct i2c_adapter *adapter, int index); > > Which then extracts automatically I2cSerialBus connection from "index" > and calls i2c_setup_smbus_alert() accordingly. > > In the long run we could introduce _DSD property that can be used to > name the connection in the same way DT does; > > Name (_CRS, ResourceTemplate () { > I2cSerialBus () { ... } // ARA > I2cSerialBus () { ... } // normal device address > }) > > Name (_DSD, Package () { > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), > Package () { > Package () {"smbus_alert", 0} // Where 0 means the first I2cSerialBus > ... > } > }) > > But it does not help the existing systems. I'm curious - how would we go about promoting this piece of common sense? Jonathan > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 25/11/2017 21:57, Jonathan Cameron wrote: > On Tue, 21 Nov 2017 09:22:16 +0800 > Phil Reid <preid@electromag.com.au> wrote: > >> On 20/11/2017 18:57, Mika Westerberg wrote: >>> +Jarkko >>> >>> On Sun, Nov 19, 2017 at 04:35:51PM +0000, Jonathan Cameron wrote: >>>> On Thu, 2 Nov 2017 16:04:07 +0100 >>>> Wolfram Sang <wsa@the-dreams.de> wrote: >>>> >>>>> On Thu, Nov 02, 2017 at 02:35:50PM +0000, Jonathan Cameron wrote: >>>>>> On Fri, 27 Oct 2017 18:27:02 +0200 >>>>>> Marc CAPDEVILLE <m.capdeville@no-log.org> 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 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> >>>>>> >>>>>> Wolfram - this needs input from you on how to neatly handle >>>>>> an ACPI registered ARA. >>>>> >>>>> ACPI is really not my field. Try asking the I2C ACPI maintainers or >>>>> Benjamin Tissoires <benjamin.tissoires@redhat.com> who did work on SMBus >>>>> interrupts recently. >>>>> >>>> Hi Mika, Benjamin, >>>> >>>> So we've lost most of the context in this thread, but the basic question >>>> is how to handle smbus ARA support with ACPI. >>>> >>>> https://patchwork.kernel.org/patch/10030309/ >>>> >>>> Has the proposal made in this driver which is really not terribly nice >>>> (as it registers the ARA device by messing with the address and registering >>>> a second device). >>>> >>>> As I understood it the ARA device registration should be handled by the >>>> i2c master, but there are very few examples. >>>> >>>> Phil pointed out that equivalent OF support recently got taken from him.. >>>> https://www.spinics.net/lists/devicetree/msg191947.html >>>> https://www.spinics.net/lists/linux-i2c/msg31173.html >>>> >>>> Any thoughts on the right way to do this? >>> >>> There does not seem to be any way in ACPI to tell which "connection" is >>> used to describe ARA so that part currently is something each driver >>> needs to handle as they know the device the best. I don't think we have >>> any means to handle it in generic way in I2C core except to provide some >>> helpers that work on top of i2c_setup_smbus_alert() but understand ACPI >>> resources. Say provide function like this: >>> >>> int acpi_i2c_setup_smbus_alert(struct i2c_adapter *adapter, int index); >>> >>> Which then extracts automatically I2cSerialBus connection from "index" >>> and calls i2c_setup_smbus_alert() accordingly. >>> >>> In the long run we could introduce _DSD property that can be used to >>> name the connection in the same way DT does; >>> >>> Name (_CRS, ResourceTemplate () { >>> I2cSerialBus () { ... } // ARA >>> I2cSerialBus () { ... } // normal device address >>> }) >>> >>> Name (_DSD, Package () { >>> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), >>> Package () { >>> Package () {"smbus_alert", 0} // Where 0 means the first I2cSerialBus >>> ... >>> } >>> }) >>> >> >> I wonder if it's worth involving the smbus_alert driver in this case. >> The cm3218 driver doesn't appear to be using the alert callback in strcut i2c_driver. > > True - though it really should be.. > >> So the smbus_alert driver is not going to notifiy the cm3218 driver. >> Are there more than one alert/ara capable devices on the bus? > > I'm not keen on taking a work around for one board on the basis that > it only has this one device on the bus - we will probably get a different > one down the line where this isn't true - then we end up ripping up what > has been done so far and starting again. > > I don't mind having some ACPI matching code in the driver but it needs > to use the ARA infrastructure to actually handle things... I'd agree, I only suggested the approach as the driver didn't seem to be using the alert callback. Without using that callback there seems little point using it IMO. As you say the better approach is to have some generic ACPI code for the alert. > > If we then get nice generic ACPI code via some other means in the future > we can move the driver over to that. > > Sometimes I wonder if some people just write ACPI tables with no though > to generalization at all, or that the code running might be shared across > different machines. (sometimes that assumption is valid, but not that > often... oh well - usual case of if we all work together everyone wins > but not worth one company trying to do things right...) > >> Perhaps a workaround in this case is if that acpi entry is defined the cm3218 driver >> handles that ara request directly to clear the interrupt. >> > > Jonathan > >
On Sat, Nov 25, 2017 at 02:00:34PM +0000, Jonathan Cameron wrote: > > There does not seem to be any way in ACPI to tell which "connection" is > > used to describe ARA so that part currently is something each driver > > needs to handle as they know the device the best. I don't think we have > > any means to handle it in generic way in I2C core except to provide some > > helpers that work on top of i2c_setup_smbus_alert() but understand ACPI > > resources. Say provide function like this: > > > > int acpi_i2c_setup_smbus_alert(struct i2c_adapter *adapter, int index); > > > > Which then extracts automatically I2cSerialBus connection from "index" > > and calls i2c_setup_smbus_alert() accordingly. > > > > In the long run we could introduce _DSD property that can be used to > > name the connection in the same way DT does; > > > > Name (_CRS, ResourceTemplate () { > > I2cSerialBus () { ... } // ARA > > I2cSerialBus () { ... } // normal device address > > }) > > > > Name (_DSD, Package () { > > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), > > Package () { > > Package () {"smbus_alert", 0} // Where 0 means the first I2cSerialBus > > ... > > } > > }) > > > > But it does not help the existing systems. > > I'm curious - how would we go about promoting this piece of common sense? I guess a proper patch series including relevant mailing lists (like linux-acpi and linux-i2c) would be a good starting point :) -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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> --- v4 : - rework acpi i2c adress lookup due to bad commit being sent. 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(-)