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@xxxxxxxxxx> 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@xxxxxxxxxx> > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html