[PATCH 0/3] staging: iio: light: isl29018: add support for other chips

[PATCH 0/3] staging: iio: light: isl29018: add support for other chips

[Laurentiu Palcu]

Hi,

This patchset adds support for 2 similar ALS chips: ISL29023 and ISL29035.

The chip I performed tests on was a ISL29035, since I didn't have eval boards, for the
other 2 chips, handy.

The registers tables for each chip can be found here:

ISL29018: [1] - page 10
ISL29023: [2] - page 8
ISL29035: [3] - page 10

[1] http://www.intersil.com/content/dam/Intersil/documents/isl2/isl29018.pdf
[2] http://www.intersil.com/content/dam/Intersil/documents/isl2/isl29023.pdf
[3] http://www.intersil.com/content/dam/Intersil/documents/isl2/isl29035.pdf

thanks,
laurentiu

Laurentiu Palcu (3):
  staging: iio: light: isl29018: fix typo
  staging: iio: light: isl29018: add support for isl29023 and isl29035
  staging: iio: light: isl29018: add ACPI support

 drivers/staging/iio/light/isl29018.c | 205 ++++++++++++++++++++++++++++++-----
 1 file changed, 179 insertions(+), 26 deletions(-)

-- 
1.9.1

[PATCH 0/3] staging: iio: light: isl29018: add support for other chips

[Laurentiu Palcu]

Hi,

This patchset adds support for 2 similar ALS chips: ISL29023 and ISL29035.

The chip I performed tests on was a ISL29035, since I didn't have eval boards, for the
other 2 chips, handy.

The registers tables for each chip can be found here:

ISL29018: [1] - page 10
ISL29023: [2] - page 8
ISL29035: [3] - page 10

[1] http://www.intersil.com/content/dam/Intersil/documents/isl2/isl29018.pdf
[2] http://www.intersil.com/content/dam/Intersil/documents/isl2/isl29023.pdf
[3] http://www.intersil.com/content/dam/Intersil/documents/isl2/isl29035.pdf

thanks,
laurentiu

Laurentiu Palcu (3):
  staging: iio: light: isl29018: fix typo
  staging: iio: light: isl29018: add support for isl29023 and isl29035
  staging: iio: light: isl29018: add ACPI support

 drivers/staging/iio/light/isl29018.c | 205 ++++++++++++++++++++++++++++++-----
 1 file changed, 179 insertions(+), 26 deletions(-)

-- 
1.9.1

[PATCH 1/3] staging: iio: light: isl29018: fix typo

[Laurentiu Palcu]

isl29108 was used, instead of isl29018.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 drivers/staging/iio/light/isl29018.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
index 86cc8f9..63c70be 100644
--- a/drivers/staging/iio/light/isl29018.c
+++ b/drivers/staging/iio/light/isl29018.c
@@ -447,7 +447,7 @@ static struct attribute *isl29018_attributes[] = {
 	NULL
 };
 
-static const struct attribute_group isl29108_group = {
+static const struct attribute_group isl29018_group = {
 	.attrs = isl29018_attributes,
 };
 
@@ -510,8 +510,8 @@ static int isl29018_chip_init(struct isl29018_chip *chip)
 	return 0;
 }
 
-static const struct iio_info isl29108_info = {
-	.attrs = &isl29108_group,
+static const struct iio_info isl29018_info = {
+	.attrs = &isl29018_group,
 	.driver_module = THIS_MODULE,
 	.read_raw = &isl29018_read_raw,
 	.write_raw = &isl29018_write_raw,
@@ -579,7 +579,7 @@ static int isl29018_probe(struct i2c_client *client,
 	if (err)
 		return err;
 
-	indio_dev->info = &isl29108_info;
+	indio_dev->info = &isl29018_info;
 	indio_dev->channels = isl29018_channels;
 	indio_dev->num_channels = ARRAY_SIZE(isl29018_channels);
 	indio_dev->name = id->name;
-- 
1.9.1

[PATCH 1/3] staging: iio: light: isl29018: fix typo

[Laurentiu Palcu]

isl29108 was used, instead of isl29018.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 drivers/staging/iio/light/isl29018.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
index 86cc8f9..63c70be 100644
--- a/drivers/staging/iio/light/isl29018.c
+++ b/drivers/staging/iio/light/isl29018.c
@@ -447,7 +447,7 @@ static struct attribute *isl29018_attributes[] = {
 	NULL
 };
 
-static const struct attribute_group isl29108_group = {
+static const struct attribute_group isl29018_group = {
 	.attrs = isl29018_attributes,
 };
 
@@ -510,8 +510,8 @@ static int isl29018_chip_init(struct isl29018_chip *chip)
 	return 0;
 }
 
-static const struct iio_info isl29108_info = {
-	.attrs = &isl29108_group,
+static const struct iio_info isl29018_info = {
+	.attrs = &isl29018_group,
 	.driver_module = THIS_MODULE,
 	.read_raw = &isl29018_read_raw,
 	.write_raw = &isl29018_write_raw,
@@ -579,7 +579,7 @@ static int isl29018_probe(struct i2c_client *client,
 	if (err)
 		return err;
 
-	indio_dev->info = &isl29108_info;
+	indio_dev->info = &isl29018_info;
 	indio_dev->channels = isl29018_channels;
 	indio_dev->num_channels = ARRAY_SIZE(isl29018_channels);
 	indio_dev->name = id->name;
-- 
1.9.1

Re: [PATCH 1/3] staging: iio: light: isl29018: fix typo

[Jonathan Cameron]

On 29/08/14 15:26, Laurentiu Palcu wrote:
> isl29108 was used, instead of isl29018.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
Applied to the togreg branch of iio.git - initially pushed out as testing for the
autobuilders to play.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/isl29018.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index 86cc8f9..63c70be 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -447,7 +447,7 @@ static struct attribute *isl29018_attributes[] = {
>  	NULL
>  };
>  
> -static const struct attribute_group isl29108_group = {
> +static const struct attribute_group isl29018_group = {
>  	.attrs = isl29018_attributes,
>  };
>  
> @@ -510,8 +510,8 @@ static int isl29018_chip_init(struct isl29018_chip *chip)
>  	return 0;
>  }
>  
> -static const struct iio_info isl29108_info = {
> -	.attrs = &isl29108_group,
> +static const struct iio_info isl29018_info = {
> +	.attrs = &isl29018_group,
>  	.driver_module = THIS_MODULE,
>  	.read_raw = &isl29018_read_raw,
>  	.write_raw = &isl29018_write_raw,
> @@ -579,7 +579,7 @@ static int isl29018_probe(struct i2c_client *client,
>  	if (err)
>  		return err;
>  
> -	indio_dev->info = &isl29108_info;
> +	indio_dev->info = &isl29018_info;
>  	indio_dev->channels = isl29018_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(isl29018_channels);
>  	indio_dev->name = id->name;
> 

Re: [PATCH 1/3] staging: iio: light: isl29018: fix typo

[Jonathan Cameron]

On 29/08/14 15:26, Laurentiu Palcu wrote:
> isl29108 was used, instead of isl29018.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Applied to the togreg branch of iio.git - initially pushed out as testing for the
autobuilders to play.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/isl29018.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index 86cc8f9..63c70be 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -447,7 +447,7 @@ static struct attribute *isl29018_attributes[] = {
>  	NULL
>  };
>  
> -static const struct attribute_group isl29108_group = {
> +static const struct attribute_group isl29018_group = {
>  	.attrs = isl29018_attributes,
>  };
>  
> @@ -510,8 +510,8 @@ static int isl29018_chip_init(struct isl29018_chip *chip)
>  	return 0;
>  }
>  
> -static const struct iio_info isl29108_info = {
> -	.attrs = &isl29108_group,
> +static const struct iio_info isl29018_info = {
> +	.attrs = &isl29018_group,
>  	.driver_module = THIS_MODULE,
>  	.read_raw = &isl29018_read_raw,
>  	.write_raw = &isl29018_write_raw,
> @@ -579,7 +579,7 @@ static int isl29018_probe(struct i2c_client *client,
>  	if (err)
>  		return err;
>  
> -	indio_dev->info = &isl29108_info;
> +	indio_dev->info = &isl29018_info;
>  	indio_dev->channels = isl29018_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(isl29018_channels);
>  	indio_dev->name = id->name;
> 

[PATCH 2/3] staging: iio: light: isl29018: add support for isl29023 and isl29035

[Laurentiu Palcu]

Intersil chips ISL29018, ISL29023 and ISL29035 are very similar. They're
all ambience light sensors. The ISL29018, however, is also a proximity
sensor. The registers are similar too:

-------------+----------+----------
AVAILABLE IN | ADDR REG | NAME
   290xx     |          |
-------------+----------+----------
     18/23/35|       00h| COMMANDI
     18/23/35|       01h| COMMANDII (B4-7 are used only in 29018 for proximity)
     18/23/35|       02h| DATALSB
     18/23/35|       03h| DATAMSB
     18/23/35|       04h| INT_LT_LSB
     18/23/35|       05h| INT_LT_MSB
     18/23/35|       06h| INT_HT_LSB
     18/23/35|       07h| INT_HT_MSB
        18/23|       08h| TEST
           35|       0Fh| ID
-------------+----------+-----------

So, this patch will add support for ISL29023 and ISL29035 to the
existing isl29018 driver. Since these 2 chips don't have proximity
detection, the proximity sysfs attribute is not needed.

Also, for ISL29035, since it has an ID register, make use of it in order
to properly detect the chip and clear the brownout bit.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 drivers/staging/iio/light/isl29018.c | 163 ++++++++++++++++++++++++++++++-----
 1 file changed, 141 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
index 63c70be..b50f126 100644
--- a/drivers/staging/iio/light/isl29018.c
+++ b/drivers/staging/iio/light/isl29018.c
@@ -1,5 +1,5 @@
 /*
- * A iio driver for the light sensor ISL 29018.
+ * A iio driver for the light sensor ISL 29018/29023/29035.
  *
  * IIO driver for monitoring ambient light intensity in luxi, proximity
  * sensing and infrared sensing.
@@ -58,10 +58,18 @@
 #define ISL29018_TEST_SHIFT		0
 #define ISL29018_TEST_MASK		(0xFF << ISL29018_TEST_SHIFT)
 
+#define ISL29035_REG_DEVICE_ID		0x0F
+#define ISL29035_DEVICE_ID_SHIFT	0x03
+#define ISL29035_DEVICE_ID_MASK		(0x7 << ISL29035_DEVICE_ID_SHIFT)
+#define ISL29035_DEVICE_ID		0x5
+#define ISL29035_BOUT_SHIFT		0x07
+#define ISL29035_BOUT_MASK		(0x01 << ISL29035_BOUT_SHIFT)
+
 struct isl29018_chip {
 	struct device		*dev;
 	struct regmap		*regmap;
 	struct mutex		lock;
+	int			type;
 	unsigned int		lux_scale;
 	unsigned int		lux_uscale;
 	unsigned int		range;
@@ -407,23 +415,35 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
 	return ret;
 }
 
+#define ISL29018_LIGHT_CHANNEL {					\
+	.type = IIO_LIGHT,						\
+	.indexed = 1,							\
+	.channel = 0,							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |		\
+	BIT(IIO_CHAN_INFO_CALIBSCALE),					\
+}
+
+#define ISL29018_IR_CHANNEL {						\
+	.type = IIO_INTENSITY,						\
+	.modified = 1,							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.channel2 = IIO_MOD_LIGHT_IR,					\
+}
+
+#define ISL29018_PROXIMITY_CHANNEL {					\
+	.type = IIO_PROXIMITY,						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+}
+
 static const struct iio_chan_spec isl29018_channels[] = {
-	{
-		.type = IIO_LIGHT,
-		.indexed = 1,
-		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
-		BIT(IIO_CHAN_INFO_CALIBSCALE),
-	}, {
-		.type = IIO_INTENSITY,
-		.modified = 1,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.channel2 = IIO_MOD_LIGHT_IR,
-	}, {
-		/* Unindexed in current ABI.  But perhaps it should be. */
-		.type = IIO_PROXIMITY,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-	}
+	ISL29018_LIGHT_CHANNEL,
+	ISL29018_IR_CHANNEL,
+	ISL29018_PROXIMITY_CHANNEL,
+};
+
+static const struct iio_chan_spec isl29023_channels[] = {
+	ISL29018_LIGHT_CHANNEL,
+	ISL29018_IR_CHANNEL,
 };
 
 static IIO_DEVICE_ATTR(range, S_IRUGO | S_IWUSR, show_range, store_range, 0);
@@ -447,16 +467,63 @@ static struct attribute *isl29018_attributes[] = {
 	NULL
 };
 
+static struct attribute *isl29023_attributes[] = {
+	ISL29018_DEV_ATTR(range),
+	ISL29018_CONST_ATTR(range_available),
+	ISL29018_DEV_ATTR(adc_resolution),
+	ISL29018_CONST_ATTR(adc_resolution_available),
+	NULL
+};
+
 static const struct attribute_group isl29018_group = {
 	.attrs = isl29018_attributes,
 };
 
+static const struct attribute_group isl29023_group = {
+	.attrs = isl29023_attributes,
+};
+
+static int isl29035_detect(struct isl29018_chip *chip)
+{
+	int status;
+	unsigned int id;
+
+	status = regmap_read(chip->regmap, ISL29035_REG_DEVICE_ID, &id);
+	if (status < 0) {
+		dev_err(chip->dev,
+			"Error reading ID register with error %d\n",
+			status);
+		return status;
+	}
+
+	id = (id & ISL29035_DEVICE_ID_MASK) >> ISL29035_DEVICE_ID_SHIFT;
+
+	if (id != ISL29035_DEVICE_ID)
+		return -ENODEV;
+
+	/* clear out brownout bit */
+	return regmap_update_bits(chip->regmap, ISL29035_REG_DEVICE_ID,
+				  ISL29035_BOUT_MASK, 0);
+}
+
+enum {
+	isl29018,
+	isl29023,
+	isl29035,
+};
+
 static int isl29018_chip_init(struct isl29018_chip *chip)
 {
 	int status;
 	unsigned int new_adc_bit;
 	unsigned int new_range;
 
+	if (chip->type == isl29035) {
+		status = isl29035_detect(chip);
+		if (status < 0)
+			return status;
+	}
+
 	/* Code added per Intersil Application Note 1534:
 	 *     When VDD sinks to approximately 1.8V or below, some of
 	 * the part's registers may change their state. When VDD
@@ -517,6 +584,13 @@ static const struct iio_info isl29018_info = {
 	.write_raw = &isl29018_write_raw,
 };
 
+static const struct iio_info isl29023_info = {
+	.attrs = &isl29023_group,
+	.driver_module = THIS_MODULE,
+	.read_raw = &isl29018_read_raw,
+	.write_raw = &isl29018_write_raw,
+};
+
 static bool is_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
@@ -524,6 +598,7 @@ static bool is_volatile_reg(struct device *dev, unsigned int reg)
 	case ISL29018_REG_ADD_DATA_MSB:
 	case ISL29018_REG_ADD_COMMAND1:
 	case ISL29018_REG_TEST:
+	case ISL29035_REG_DEVICE_ID:
 		return true;
 	default:
 		return false;
@@ -543,6 +618,44 @@ static const struct regmap_config isl29018_regmap_config = {
 	.cache_type = REGCACHE_RBTREE,
 };
 
+/* isl29035_regmap_config: regmap configuration for ISL29035 */
+static const struct regmap_config isl29035_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.volatile_reg = is_volatile_reg,
+	.max_register = ISL29035_REG_DEVICE_ID,
+	.num_reg_defaults_raw = ISL29035_REG_DEVICE_ID + 1,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+struct chip_info {
+	const struct iio_chan_spec *channels;
+	int num_channels;
+	const struct iio_info *indio_info;
+	const struct regmap_config *regmap_cfg;
+};
+
+static const struct chip_info chip_info_tbl[] = {
+	[isl29018] = {
+		.channels = isl29018_channels,
+		.num_channels = ARRAY_SIZE(isl29018_channels),
+		.indio_info = &isl29018_info,
+		.regmap_cfg = &isl29018_regmap_config,
+	},
+	[isl29023] = {
+		.channels = isl29023_channels,
+		.num_channels = ARRAY_SIZE(isl29023_channels),
+		.indio_info = &isl29023_info,
+		.regmap_cfg = &isl29018_regmap_config,
+	},
+	[isl29035] = {
+		.channels = isl29023_channels,
+		.num_channels = ARRAY_SIZE(isl29023_channels),
+		.indio_info = &isl29023_info,
+		.regmap_cfg = &isl29035_regmap_config,
+	},
+};
+
 static int isl29018_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -562,13 +675,15 @@ static int isl29018_probe(struct i2c_client *client,
 
 	mutex_init(&chip->lock);
 
+	chip->type = id->driver_data;
 	chip->lux_scale = 1;
 	chip->lux_uscale = 0;
 	chip->range = 1000;
 	chip->adc_bit = 16;
 	chip->suspended = false;
 
-	chip->regmap = devm_regmap_init_i2c(client, &isl29018_regmap_config);
+	chip->regmap = devm_regmap_init_i2c(client,
+				chip_info_tbl[id->driver_data].regmap_cfg);
 	if (IS_ERR(chip->regmap)) {
 		err = PTR_ERR(chip->regmap);
 		dev_err(chip->dev, "regmap initialization failed: %d\n", err);
@@ -579,9 +694,9 @@ static int isl29018_probe(struct i2c_client *client,
 	if (err)
 		return err;
 
-	indio_dev->info = &isl29018_info;
-	indio_dev->channels = isl29018_channels;
-	indio_dev->num_channels = ARRAY_SIZE(isl29018_channels);
+	indio_dev->info = chip_info_tbl[id->driver_data].indio_info;
+	indio_dev->channels = chip_info_tbl[id->driver_data].channels;
+	indio_dev->num_channels = chip_info_tbl[id->driver_data].num_channels;
 	indio_dev->name = id->name;
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -633,7 +748,9 @@ static SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend, isl29018_resume);
 #endif
 
 static const struct i2c_device_id isl29018_id[] = {
-	{"isl29018", 0},
+	{"isl29018", isl29018},
+	{"isl29023", isl29023},
+	{"isl29035", isl29035},
 	{}
 };
 
@@ -641,6 +758,8 @@ MODULE_DEVICE_TABLE(i2c, isl29018_id);
 
 static const struct of_device_id isl29018_of_match[] = {
 	{ .compatible = "isil,isl29018", },
+	{ .compatible = "isil,isl29023", },
+	{ .compatible = "isil,isl29035", },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, isl29018_of_match);
-- 
1.9.1

[PATCH 2/3] staging: iio: light: isl29018: add support for isl29023 and isl29035

[Laurentiu Palcu]

Intersil chips ISL29018, ISL29023 and ISL29035 are very similar. They're
all ambience light sensors. The ISL29018, however, is also a proximity
sensor. The registers are similar too:

-------------+----------+----------
AVAILABLE IN | ADDR REG | NAME
   290xx     |          |
-------------+----------+----------
     18/23/35|       00h| COMMANDI
     18/23/35|       01h| COMMANDII (B4-7 are used only in 29018 for proximity)
     18/23/35|       02h| DATALSB
     18/23/35|       03h| DATAMSB
     18/23/35|       04h| INT_LT_LSB
     18/23/35|       05h| INT_LT_MSB
     18/23/35|       06h| INT_HT_LSB
     18/23/35|       07h| INT_HT_MSB
        18/23|       08h| TEST
           35|       0Fh| ID
-------------+----------+-----------

So, this patch will add support for ISL29023 and ISL29035 to the
existing isl29018 driver. Since these 2 chips don't have proximity
detection, the proximity sysfs attribute is not needed.

Also, for ISL29035, since it has an ID register, make use of it in order
to properly detect the chip and clear the brownout bit.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 drivers/staging/iio/light/isl29018.c | 163 ++++++++++++++++++++++++++++++-----
 1 file changed, 141 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
index 63c70be..b50f126 100644
--- a/drivers/staging/iio/light/isl29018.c
+++ b/drivers/staging/iio/light/isl29018.c
@@ -1,5 +1,5 @@
 /*
- * A iio driver for the light sensor ISL 29018.
+ * A iio driver for the light sensor ISL 29018/29023/29035.
  *
  * IIO driver for monitoring ambient light intensity in luxi, proximity
  * sensing and infrared sensing.
@@ -58,10 +58,18 @@
 #define ISL29018_TEST_SHIFT		0
 #define ISL29018_TEST_MASK		(0xFF << ISL29018_TEST_SHIFT)
 
+#define ISL29035_REG_DEVICE_ID		0x0F
+#define ISL29035_DEVICE_ID_SHIFT	0x03
+#define ISL29035_DEVICE_ID_MASK		(0x7 << ISL29035_DEVICE_ID_SHIFT)
+#define ISL29035_DEVICE_ID		0x5
+#define ISL29035_BOUT_SHIFT		0x07
+#define ISL29035_BOUT_MASK		(0x01 << ISL29035_BOUT_SHIFT)
+
 struct isl29018_chip {
 	struct device		*dev;
 	struct regmap		*regmap;
 	struct mutex		lock;
+	int			type;
 	unsigned int		lux_scale;
 	unsigned int		lux_uscale;
 	unsigned int		range;
@@ -407,23 +415,35 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
 	return ret;
 }
 
+#define ISL29018_LIGHT_CHANNEL {					\
+	.type = IIO_LIGHT,						\
+	.indexed = 1,							\
+	.channel = 0,							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |		\
+	BIT(IIO_CHAN_INFO_CALIBSCALE),					\
+}
+
+#define ISL29018_IR_CHANNEL {						\
+	.type = IIO_INTENSITY,						\
+	.modified = 1,							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.channel2 = IIO_MOD_LIGHT_IR,					\
+}
+
+#define ISL29018_PROXIMITY_CHANNEL {					\
+	.type = IIO_PROXIMITY,						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+}
+
 static const struct iio_chan_spec isl29018_channels[] = {
-	{
-		.type = IIO_LIGHT,
-		.indexed = 1,
-		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
-		BIT(IIO_CHAN_INFO_CALIBSCALE),
-	}, {
-		.type = IIO_INTENSITY,
-		.modified = 1,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.channel2 = IIO_MOD_LIGHT_IR,
-	}, {
-		/* Unindexed in current ABI.  But perhaps it should be. */
-		.type = IIO_PROXIMITY,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-	}
+	ISL29018_LIGHT_CHANNEL,
+	ISL29018_IR_CHANNEL,
+	ISL29018_PROXIMITY_CHANNEL,
+};
+
+static const struct iio_chan_spec isl29023_channels[] = {
+	ISL29018_LIGHT_CHANNEL,
+	ISL29018_IR_CHANNEL,
 };
 
 static IIO_DEVICE_ATTR(range, S_IRUGO | S_IWUSR, show_range, store_range, 0);
@@ -447,16 +467,63 @@ static struct attribute *isl29018_attributes[] = {
 	NULL
 };
 
+static struct attribute *isl29023_attributes[] = {
+	ISL29018_DEV_ATTR(range),
+	ISL29018_CONST_ATTR(range_available),
+	ISL29018_DEV_ATTR(adc_resolution),
+	ISL29018_CONST_ATTR(adc_resolution_available),
+	NULL
+};
+
 static const struct attribute_group isl29018_group = {
 	.attrs = isl29018_attributes,
 };
 
+static const struct attribute_group isl29023_group = {
+	.attrs = isl29023_attributes,
+};
+
+static int isl29035_detect(struct isl29018_chip *chip)
+{
+	int status;
+	unsigned int id;
+
+	status = regmap_read(chip->regmap, ISL29035_REG_DEVICE_ID, &id);
+	if (status < 0) {
+		dev_err(chip->dev,
+			"Error reading ID register with error %d\n",
+			status);
+		return status;
+	}
+
+	id = (id & ISL29035_DEVICE_ID_MASK) >> ISL29035_DEVICE_ID_SHIFT;
+
+	if (id != ISL29035_DEVICE_ID)
+		return -ENODEV;
+
+	/* clear out brownout bit */
+	return regmap_update_bits(chip->regmap, ISL29035_REG_DEVICE_ID,
+				  ISL29035_BOUT_MASK, 0);
+}
+
+enum {
+	isl29018,
+	isl29023,
+	isl29035,
+};
+
 static int isl29018_chip_init(struct isl29018_chip *chip)
 {
 	int status;
 	unsigned int new_adc_bit;
 	unsigned int new_range;
 
+	if (chip->type == isl29035) {
+		status = isl29035_detect(chip);
+		if (status < 0)
+			return status;
+	}
+
 	/* Code added per Intersil Application Note 1534:
 	 *     When VDD sinks to approximately 1.8V or below, some of
 	 * the part's registers may change their state. When VDD
@@ -517,6 +584,13 @@ static const struct iio_info isl29018_info = {
 	.write_raw = &isl29018_write_raw,
 };
 
+static const struct iio_info isl29023_info = {
+	.attrs = &isl29023_group,
+	.driver_module = THIS_MODULE,
+	.read_raw = &isl29018_read_raw,
+	.write_raw = &isl29018_write_raw,
+};
+
 static bool is_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
@@ -524,6 +598,7 @@ static bool is_volatile_reg(struct device *dev, unsigned int reg)
 	case ISL29018_REG_ADD_DATA_MSB:
 	case ISL29018_REG_ADD_COMMAND1:
 	case ISL29018_REG_TEST:
+	case ISL29035_REG_DEVICE_ID:
 		return true;
 	default:
 		return false;
@@ -543,6 +618,44 @@ static const struct regmap_config isl29018_regmap_config = {
 	.cache_type = REGCACHE_RBTREE,
 };
 
+/* isl29035_regmap_config: regmap configuration for ISL29035 */
+static const struct regmap_config isl29035_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.volatile_reg = is_volatile_reg,
+	.max_register = ISL29035_REG_DEVICE_ID,
+	.num_reg_defaults_raw = ISL29035_REG_DEVICE_ID + 1,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+struct chip_info {
+	const struct iio_chan_spec *channels;
+	int num_channels;
+	const struct iio_info *indio_info;
+	const struct regmap_config *regmap_cfg;
+};
+
+static const struct chip_info chip_info_tbl[] = {
+	[isl29018] = {
+		.channels = isl29018_channels,
+		.num_channels = ARRAY_SIZE(isl29018_channels),
+		.indio_info = &isl29018_info,
+		.regmap_cfg = &isl29018_regmap_config,
+	},
+	[isl29023] = {
+		.channels = isl29023_channels,
+		.num_channels = ARRAY_SIZE(isl29023_channels),
+		.indio_info = &isl29023_info,
+		.regmap_cfg = &isl29018_regmap_config,
+	},
+	[isl29035] = {
+		.channels = isl29023_channels,
+		.num_channels = ARRAY_SIZE(isl29023_channels),
+		.indio_info = &isl29023_info,
+		.regmap_cfg = &isl29035_regmap_config,
+	},
+};
+
 static int isl29018_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -562,13 +675,15 @@ static int isl29018_probe(struct i2c_client *client,
 
 	mutex_init(&chip->lock);
 
+	chip->type = id->driver_data;
 	chip->lux_scale = 1;
 	chip->lux_uscale = 0;
 	chip->range = 1000;
 	chip->adc_bit = 16;
 	chip->suspended = false;
 
-	chip->regmap = devm_regmap_init_i2c(client, &isl29018_regmap_config);
+	chip->regmap = devm_regmap_init_i2c(client,
+				chip_info_tbl[id->driver_data].regmap_cfg);
 	if (IS_ERR(chip->regmap)) {
 		err = PTR_ERR(chip->regmap);
 		dev_err(chip->dev, "regmap initialization failed: %d\n", err);
@@ -579,9 +694,9 @@ static int isl29018_probe(struct i2c_client *client,
 	if (err)
 		return err;
 
-	indio_dev->info = &isl29018_info;
-	indio_dev->channels = isl29018_channels;
-	indio_dev->num_channels = ARRAY_SIZE(isl29018_channels);
+	indio_dev->info = chip_info_tbl[id->driver_data].indio_info;
+	indio_dev->channels = chip_info_tbl[id->driver_data].channels;
+	indio_dev->num_channels = chip_info_tbl[id->driver_data].num_channels;
 	indio_dev->name = id->name;
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -633,7 +748,9 @@ static SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend, isl29018_resume);
 #endif
 
 static const struct i2c_device_id isl29018_id[] = {
-	{"isl29018", 0},
+	{"isl29018", isl29018},
+	{"isl29023", isl29023},
+	{"isl29035", isl29035},
 	{}
 };
 
@@ -641,6 +758,8 @@ MODULE_DEVICE_TABLE(i2c, isl29018_id);
 
 static const struct of_device_id isl29018_of_match[] = {
 	{ .compatible = "isil,isl29018", },
+	{ .compatible = "isil,isl29023", },
+	{ .compatible = "isil,isl29035", },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, isl29018_of_match);
-- 
1.9.1

Re: [PATCH 2/3] staging: iio: light: isl29018: add support for isl29023 and isl29035

[Jonathan Cameron]

On 29/08/14 15:26, Laurentiu Palcu wrote:
> Intersil chips ISL29018, ISL29023 and ISL29035 are very similar. They're
> all ambience light sensors. The ISL29018, however, is also a proximity
> sensor. The registers are similar too:
>
> -------------+----------+----------
> AVAILABLE IN | ADDR REG | NAME
>    290xx     |          |
> -------------+----------+----------
>      18/23/35|       00h| COMMANDI
>      18/23/35|       01h| COMMANDII (B4-7 are used only in 29018 for proximity)
>      18/23/35|       02h| DATALSB
>      18/23/35|       03h| DATAMSB
>      18/23/35|       04h| INT_LT_LSB
>      18/23/35|       05h| INT_LT_MSB
>      18/23/35|       06h| INT_HT_LSB
>      18/23/35|       07h| INT_HT_MSB
>         18/23|       08h| TEST
>            35|       0Fh| ID
> -------------+----------+-----------
>
> So, this patch will add support for ISL29023 and ISL29035 to the
> existing isl29018 driver. Since these 2 chips don't have proximity
> detection, the proximity sysfs attribute is not needed.
>
> Also, for ISL29035, since it has an ID register, make use of it in order
> to properly detect the chip and clear the brownout bit.
>
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
Hmm.  Your changes look good.  This driver still has quite a lot of non
standard ABI, but fair enough that you haven't dealt with that in this
patch.

Don't suppose you are interested in cleaning this up and getting the
driver out of staging?

Applied to the togreg branch of iio.git

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/isl29018.c | 163 ++++++++++++++++++++++++++++++-----
>  1 file changed, 141 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index 63c70be..b50f126 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -1,5 +1,5 @@
>  /*
> - * A iio driver for the light sensor ISL 29018.
> + * A iio driver for the light sensor ISL 29018/29023/29035.
>   *
>   * IIO driver for monitoring ambient light intensity in luxi, proximity
>   * sensing and infrared sensing.
> @@ -58,10 +58,18 @@
>  #define ISL29018_TEST_SHIFT		0
>  #define ISL29018_TEST_MASK		(0xFF << ISL29018_TEST_SHIFT)
>
> +#define ISL29035_REG_DEVICE_ID		0x0F
> +#define ISL29035_DEVICE_ID_SHIFT	0x03
> +#define ISL29035_DEVICE_ID_MASK		(0x7 << ISL29035_DEVICE_ID_SHIFT)
> +#define ISL29035_DEVICE_ID		0x5
> +#define ISL29035_BOUT_SHIFT		0x07
> +#define ISL29035_BOUT_MASK		(0x01 << ISL29035_BOUT_SHIFT)
> +
>  struct isl29018_chip {
>  	struct device		*dev;
>  	struct regmap		*regmap;
>  	struct mutex		lock;
> +	int			type;
>  	unsigned int		lux_scale;
>  	unsigned int		lux_uscale;
>  	unsigned int		range;
> @@ -407,23 +415,35 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
>  	return ret;
>  }
>
> +#define ISL29018_LIGHT_CHANNEL {					\
> +	.type = IIO_LIGHT,						\
> +	.indexed = 1,							\
> +	.channel = 0,							\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |		\
> +	BIT(IIO_CHAN_INFO_CALIBSCALE),					\
> +}
> +
> +#define ISL29018_IR_CHANNEL {						\
> +	.type = IIO_INTENSITY,						\
> +	.modified = 1,							\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +	.channel2 = IIO_MOD_LIGHT_IR,					\
> +}
> +
> +#define ISL29018_PROXIMITY_CHANNEL {					\
> +	.type = IIO_PROXIMITY,						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +}
> +
>  static const struct iio_chan_spec isl29018_channels[] = {
> -	{
> -		.type = IIO_LIGHT,
> -		.indexed = 1,
> -		.channel = 0,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> -		BIT(IIO_CHAN_INFO_CALIBSCALE),
> -	}, {
> -		.type = IIO_INTENSITY,
> -		.modified = 1,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> -		.channel2 = IIO_MOD_LIGHT_IR,
> -	}, {
> -		/* Unindexed in current ABI.  But perhaps it should be. */
> -		.type = IIO_PROXIMITY,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> -	}
> +	ISL29018_LIGHT_CHANNEL,
> +	ISL29018_IR_CHANNEL,
> +	ISL29018_PROXIMITY_CHANNEL,
> +};
> +
> +static const struct iio_chan_spec isl29023_channels[] = {
> +	ISL29018_LIGHT_CHANNEL,
> +	ISL29018_IR_CHANNEL,
>  };
>
>  static IIO_DEVICE_ATTR(range, S_IRUGO | S_IWUSR, show_range, store_range, 0);
> @@ -447,16 +467,63 @@ static struct attribute *isl29018_attributes[] = {
>  	NULL
>  };
>
> +static struct attribute *isl29023_attributes[] = {
> +	ISL29018_DEV_ATTR(range),
> +	ISL29018_CONST_ATTR(range_available),
> +	ISL29018_DEV_ATTR(adc_resolution),
> +	ISL29018_CONST_ATTR(adc_resolution_available),
These last two are non standard ABI, but they are in the existing driver
(one of the reasons it is still in staging).

> +	NULL
> +};
> +
>  static const struct attribute_group isl29018_group = {
>  	.attrs = isl29018_attributes,
>  };
>
> +static const struct attribute_group isl29023_group = {
> +	.attrs = isl29023_attributes,
> +};
> +
> +static int isl29035_detect(struct isl29018_chip *chip)
> +{
> +	int status;
> +	unsigned int id;
> +
> +	status = regmap_read(chip->regmap, ISL29035_REG_DEVICE_ID, &id);
> +	if (status < 0) {
> +		dev_err(chip->dev,
> +			"Error reading ID register with error %d\n",
> +			status);
> +		return status;
> +	}
> +
> +	id = (id & ISL29035_DEVICE_ID_MASK) >> ISL29035_DEVICE_ID_SHIFT;
> +
> +	if (id != ISL29035_DEVICE_ID)
> +		return -ENODEV;
> +
> +	/* clear out brownout bit */
> +	return regmap_update_bits(chip->regmap, ISL29035_REG_DEVICE_ID,
> +				  ISL29035_BOUT_MASK, 0);
> +}
> +
> +enum {
> +	isl29018,
> +	isl29023,
> +	isl29035,
> +};
> +
>  static int isl29018_chip_init(struct isl29018_chip *chip)
>  {
>  	int status;
>  	unsigned int new_adc_bit;
>  	unsigned int new_range;
>
> +	if (chip->type == isl29035) {
> +		status = isl29035_detect(chip);
> +		if (status < 0)
> +			return status;
> +	}
> +
>  	/* Code added per Intersil Application Note 1534:
>  	 *     When VDD sinks to approximately 1.8V or below, some of
>  	 * the part's registers may change their state. When VDD
> @@ -517,6 +584,13 @@ static const struct iio_info isl29018_info = {
>  	.write_raw = &isl29018_write_raw,
>  };
>
> +static const struct iio_info isl29023_info = {
> +	.attrs = &isl29023_group,
> +	.driver_module = THIS_MODULE,
> +	.read_raw = &isl29018_read_raw,
> +	.write_raw = &isl29018_write_raw,
> +};
> +
>  static bool is_volatile_reg(struct device *dev, unsigned int reg)
>  {
>  	switch (reg) {
> @@ -524,6 +598,7 @@ static bool is_volatile_reg(struct device *dev, unsigned int reg)
>  	case ISL29018_REG_ADD_DATA_MSB:
>  	case ISL29018_REG_ADD_COMMAND1:
>  	case ISL29018_REG_TEST:
> +	case ISL29035_REG_DEVICE_ID:
>  		return true;
>  	default:
>  		return false;
> @@ -543,6 +618,44 @@ static const struct regmap_config isl29018_regmap_config = {
>  	.cache_type = REGCACHE_RBTREE,
>  };
>
> +/* isl29035_regmap_config: regmap configuration for ISL29035 */
> +static const struct regmap_config isl29035_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.volatile_reg = is_volatile_reg,
> +	.max_register = ISL29035_REG_DEVICE_ID,
> +	.num_reg_defaults_raw = ISL29035_REG_DEVICE_ID + 1,
> +	.cache_type = REGCACHE_RBTREE,
> +};
> +
> +struct chip_info {
> +	const struct iio_chan_spec *channels;
> +	int num_channels;
> +	const struct iio_info *indio_info;
> +	const struct regmap_config *regmap_cfg;
> +};
> +
> +static const struct chip_info chip_info_tbl[] = {
> +	[isl29018] = {
> +		.channels = isl29018_channels,
> +		.num_channels = ARRAY_SIZE(isl29018_channels),
> +		.indio_info = &isl29018_info,
> +		.regmap_cfg = &isl29018_regmap_config,
> +	},
> +	[isl29023] = {
> +		.channels = isl29023_channels,
> +		.num_channels = ARRAY_SIZE(isl29023_channels),
> +		.indio_info = &isl29023_info,
> +		.regmap_cfg = &isl29018_regmap_config,
> +	},
> +	[isl29035] = {
> +		.channels = isl29023_channels,
> +		.num_channels = ARRAY_SIZE(isl29023_channels),
> +		.indio_info = &isl29023_info,
> +		.regmap_cfg = &isl29035_regmap_config,
> +	},
> +};
> +
>  static int isl29018_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> @@ -562,13 +675,15 @@ static int isl29018_probe(struct i2c_client *client,
>
>  	mutex_init(&chip->lock);
>
> +	chip->type = id->driver_data;
>  	chip->lux_scale = 1;
>  	chip->lux_uscale = 0;
>  	chip->range = 1000;
>  	chip->adc_bit = 16;
>  	chip->suspended = false;
>
> -	chip->regmap = devm_regmap_init_i2c(client, &isl29018_regmap_config);
> +	chip->regmap = devm_regmap_init_i2c(client,
> +				chip_info_tbl[id->driver_data].regmap_cfg);
>  	if (IS_ERR(chip->regmap)) {
>  		err = PTR_ERR(chip->regmap);
>  		dev_err(chip->dev, "regmap initialization failed: %d\n", err);
> @@ -579,9 +694,9 @@ static int isl29018_probe(struct i2c_client *client,
>  	if (err)
>  		return err;
>
> -	indio_dev->info = &isl29018_info;
> -	indio_dev->channels = isl29018_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(isl29018_channels);
> +	indio_dev->info = chip_info_tbl[id->driver_data].indio_info;
> +	indio_dev->channels = chip_info_tbl[id->driver_data].channels;
> +	indio_dev->num_channels = chip_info_tbl[id->driver_data].num_channels;
>  	indio_dev->name = id->name;
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> @@ -633,7 +748,9 @@ static SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend, isl29018_resume);
>  #endif
>
>  static const struct i2c_device_id isl29018_id[] = {
> -	{"isl29018", 0},
> +	{"isl29018", isl29018},
> +	{"isl29023", isl29023},
> +	{"isl29035", isl29035},
>  	{}
>  };
>
> @@ -641,6 +758,8 @@ MODULE_DEVICE_TABLE(i2c, isl29018_id);
>
>  static const struct of_device_id isl29018_of_match[] = {
>  	{ .compatible = "isil,isl29018", },
> +	{ .compatible = "isil,isl29023", },
> +	{ .compatible = "isil,isl29035", },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, isl29018_of_match);
>

Re: [PATCH 2/3] staging: iio: light: isl29018: add support for isl29023 and isl29035

[Jonathan Cameron]

On 29/08/14 15:26, Laurentiu Palcu wrote:
> Intersil chips ISL29018, ISL29023 and ISL29035 are very similar. They're
> all ambience light sensors. The ISL29018, however, is also a proximity
> sensor. The registers are similar too:
>
> -------------+----------+----------
> AVAILABLE IN | ADDR REG | NAME
>    290xx     |          |
> -------------+----------+----------
>      18/23/35|       00h| COMMANDI
>      18/23/35|       01h| COMMANDII (B4-7 are used only in 29018 for proximity)
>      18/23/35|       02h| DATALSB
>      18/23/35|       03h| DATAMSB
>      18/23/35|       04h| INT_LT_LSB
>      18/23/35|       05h| INT_LT_MSB
>      18/23/35|       06h| INT_HT_LSB
>      18/23/35|       07h| INT_HT_MSB
>         18/23|       08h| TEST
>            35|       0Fh| ID
> -------------+----------+-----------
>
> So, this patch will add support for ISL29023 and ISL29035 to the
> existing isl29018 driver. Since these 2 chips don't have proximity
> detection, the proximity sysfs attribute is not needed.
>
> Also, for ISL29035, since it has an ID register, make use of it in order
> to properly detect the chip and clear the brownout bit.
>
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
Hmm.  Your changes look good.  This driver still has quite a lot of non
standard ABI, but fair enough that you haven't dealt with that in this
patch.

Don't suppose you are interested in cleaning this up and getting the
driver out of staging?

Applied to the togreg branch of iio.git

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/isl29018.c | 163 ++++++++++++++++++++++++++++++-----
>  1 file changed, 141 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index 63c70be..b50f126 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -1,5 +1,5 @@
>  /*
> - * A iio driver for the light sensor ISL 29018.
> + * A iio driver for the light sensor ISL 29018/29023/29035.
>   *
>   * IIO driver for monitoring ambient light intensity in luxi, proximity
>   * sensing and infrared sensing.
> @@ -58,10 +58,18 @@
>  #define ISL29018_TEST_SHIFT		0
>  #define ISL29018_TEST_MASK		(0xFF << ISL29018_TEST_SHIFT)
>
> +#define ISL29035_REG_DEVICE_ID		0x0F
> +#define ISL29035_DEVICE_ID_SHIFT	0x03
> +#define ISL29035_DEVICE_ID_MASK		(0x7 << ISL29035_DEVICE_ID_SHIFT)
> +#define ISL29035_DEVICE_ID		0x5
> +#define ISL29035_BOUT_SHIFT		0x07
> +#define ISL29035_BOUT_MASK		(0x01 << ISL29035_BOUT_SHIFT)
> +
>  struct isl29018_chip {
>  	struct device		*dev;
>  	struct regmap		*regmap;
>  	struct mutex		lock;
> +	int			type;
>  	unsigned int		lux_scale;
>  	unsigned int		lux_uscale;
>  	unsigned int		range;
> @@ -407,23 +415,35 @@ static int isl29018_read_raw(struct iio_dev *indio_dev,
>  	return ret;
>  }
>
> +#define ISL29018_LIGHT_CHANNEL {					\
> +	.type = IIO_LIGHT,						\
> +	.indexed = 1,							\
> +	.channel = 0,							\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |		\
> +	BIT(IIO_CHAN_INFO_CALIBSCALE),					\
> +}
> +
> +#define ISL29018_IR_CHANNEL {						\
> +	.type = IIO_INTENSITY,						\
> +	.modified = 1,							\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +	.channel2 = IIO_MOD_LIGHT_IR,					\
> +}
> +
> +#define ISL29018_PROXIMITY_CHANNEL {					\
> +	.type = IIO_PROXIMITY,						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +}
> +
>  static const struct iio_chan_spec isl29018_channels[] = {
> -	{
> -		.type = IIO_LIGHT,
> -		.indexed = 1,
> -		.channel = 0,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> -		BIT(IIO_CHAN_INFO_CALIBSCALE),
> -	}, {
> -		.type = IIO_INTENSITY,
> -		.modified = 1,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> -		.channel2 = IIO_MOD_LIGHT_IR,
> -	}, {
> -		/* Unindexed in current ABI.  But perhaps it should be. */
> -		.type = IIO_PROXIMITY,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> -	}
> +	ISL29018_LIGHT_CHANNEL,
> +	ISL29018_IR_CHANNEL,
> +	ISL29018_PROXIMITY_CHANNEL,
> +};
> +
> +static const struct iio_chan_spec isl29023_channels[] = {
> +	ISL29018_LIGHT_CHANNEL,
> +	ISL29018_IR_CHANNEL,
>  };
>
>  static IIO_DEVICE_ATTR(range, S_IRUGO | S_IWUSR, show_range, store_range, 0);
> @@ -447,16 +467,63 @@ static struct attribute *isl29018_attributes[] = {
>  	NULL
>  };
>
> +static struct attribute *isl29023_attributes[] = {
> +	ISL29018_DEV_ATTR(range),
> +	ISL29018_CONST_ATTR(range_available),
> +	ISL29018_DEV_ATTR(adc_resolution),
> +	ISL29018_CONST_ATTR(adc_resolution_available),
These last two are non standard ABI, but they are in the existing driver
(one of the reasons it is still in staging).

> +	NULL
> +};
> +
>  static const struct attribute_group isl29018_group = {
>  	.attrs = isl29018_attributes,
>  };
>
> +static const struct attribute_group isl29023_group = {
> +	.attrs = isl29023_attributes,
> +};
> +
> +static int isl29035_detect(struct isl29018_chip *chip)
> +{
> +	int status;
> +	unsigned int id;
> +
> +	status = regmap_read(chip->regmap, ISL29035_REG_DEVICE_ID, &id);
> +	if (status < 0) {
> +		dev_err(chip->dev,
> +			"Error reading ID register with error %d\n",
> +			status);
> +		return status;
> +	}
> +
> +	id = (id & ISL29035_DEVICE_ID_MASK) >> ISL29035_DEVICE_ID_SHIFT;
> +
> +	if (id != ISL29035_DEVICE_ID)
> +		return -ENODEV;
> +
> +	/* clear out brownout bit */
> +	return regmap_update_bits(chip->regmap, ISL29035_REG_DEVICE_ID,
> +				  ISL29035_BOUT_MASK, 0);
> +}
> +
> +enum {
> +	isl29018,
> +	isl29023,
> +	isl29035,
> +};
> +
>  static int isl29018_chip_init(struct isl29018_chip *chip)
>  {
>  	int status;
>  	unsigned int new_adc_bit;
>  	unsigned int new_range;
>
> +	if (chip->type == isl29035) {
> +		status = isl29035_detect(chip);
> +		if (status < 0)
> +			return status;
> +	}
> +
>  	/* Code added per Intersil Application Note 1534:
>  	 *     When VDD sinks to approximately 1.8V or below, some of
>  	 * the part's registers may change their state. When VDD
> @@ -517,6 +584,13 @@ static const struct iio_info isl29018_info = {
>  	.write_raw = &isl29018_write_raw,
>  };
>
> +static const struct iio_info isl29023_info = {
> +	.attrs = &isl29023_group,
> +	.driver_module = THIS_MODULE,
> +	.read_raw = &isl29018_read_raw,
> +	.write_raw = &isl29018_write_raw,
> +};
> +
>  static bool is_volatile_reg(struct device *dev, unsigned int reg)
>  {
>  	switch (reg) {
> @@ -524,6 +598,7 @@ static bool is_volatile_reg(struct device *dev, unsigned int reg)
>  	case ISL29018_REG_ADD_DATA_MSB:
>  	case ISL29018_REG_ADD_COMMAND1:
>  	case ISL29018_REG_TEST:
> +	case ISL29035_REG_DEVICE_ID:
>  		return true;
>  	default:
>  		return false;
> @@ -543,6 +618,44 @@ static const struct regmap_config isl29018_regmap_config = {
>  	.cache_type = REGCACHE_RBTREE,
>  };
>
> +/* isl29035_regmap_config: regmap configuration for ISL29035 */
> +static const struct regmap_config isl29035_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.volatile_reg = is_volatile_reg,
> +	.max_register = ISL29035_REG_DEVICE_ID,
> +	.num_reg_defaults_raw = ISL29035_REG_DEVICE_ID + 1,
> +	.cache_type = REGCACHE_RBTREE,
> +};
> +
> +struct chip_info {
> +	const struct iio_chan_spec *channels;
> +	int num_channels;
> +	const struct iio_info *indio_info;
> +	const struct regmap_config *regmap_cfg;
> +};
> +
> +static const struct chip_info chip_info_tbl[] = {
> +	[isl29018] = {
> +		.channels = isl29018_channels,
> +		.num_channels = ARRAY_SIZE(isl29018_channels),
> +		.indio_info = &isl29018_info,
> +		.regmap_cfg = &isl29018_regmap_config,
> +	},
> +	[isl29023] = {
> +		.channels = isl29023_channels,
> +		.num_channels = ARRAY_SIZE(isl29023_channels),
> +		.indio_info = &isl29023_info,
> +		.regmap_cfg = &isl29018_regmap_config,
> +	},
> +	[isl29035] = {
> +		.channels = isl29023_channels,
> +		.num_channels = ARRAY_SIZE(isl29023_channels),
> +		.indio_info = &isl29023_info,
> +		.regmap_cfg = &isl29035_regmap_config,
> +	},
> +};
> +
>  static int isl29018_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> @@ -562,13 +675,15 @@ static int isl29018_probe(struct i2c_client *client,
>
>  	mutex_init(&chip->lock);
>
> +	chip->type = id->driver_data;
>  	chip->lux_scale = 1;
>  	chip->lux_uscale = 0;
>  	chip->range = 1000;
>  	chip->adc_bit = 16;
>  	chip->suspended = false;
>
> -	chip->regmap = devm_regmap_init_i2c(client, &isl29018_regmap_config);
> +	chip->regmap = devm_regmap_init_i2c(client,
> +				chip_info_tbl[id->driver_data].regmap_cfg);
>  	if (IS_ERR(chip->regmap)) {
>  		err = PTR_ERR(chip->regmap);
>  		dev_err(chip->dev, "regmap initialization failed: %d\n", err);
> @@ -579,9 +694,9 @@ static int isl29018_probe(struct i2c_client *client,
>  	if (err)
>  		return err;
>
> -	indio_dev->info = &isl29018_info;
> -	indio_dev->channels = isl29018_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(isl29018_channels);
> +	indio_dev->info = chip_info_tbl[id->driver_data].indio_info;
> +	indio_dev->channels = chip_info_tbl[id->driver_data].channels;
> +	indio_dev->num_channels = chip_info_tbl[id->driver_data].num_channels;
>  	indio_dev->name = id->name;
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> @@ -633,7 +748,9 @@ static SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend, isl29018_resume);
>  #endif
>
>  static const struct i2c_device_id isl29018_id[] = {
> -	{"isl29018", 0},
> +	{"isl29018", isl29018},
> +	{"isl29023", isl29023},
> +	{"isl29035", isl29035},
>  	{}
>  };
>
> @@ -641,6 +758,8 @@ MODULE_DEVICE_TABLE(i2c, isl29018_id);
>
>  static const struct of_device_id isl29018_of_match[] = {
>  	{ .compatible = "isil,isl29018", },
> +	{ .compatible = "isil,isl29023", },
> +	{ .compatible = "isil,isl29035", },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, isl29018_of_match);
>

[PATCH 3/3] staging: iio: light: isl29018: add ACPI support

[Laurentiu Palcu]

Add support for enumerating the device through ACPI.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 drivers/staging/iio/light/isl29018.c | 46 +++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
index b50f126..c54c6ae 100644
--- a/drivers/staging/iio/light/isl29018.c
+++ b/drivers/staging/iio/light/isl29018.c
@@ -30,6 +30,7 @@
 #include <linux/slab.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/acpi.h>
 
 #define CONVERSION_TIME_MS		100
 
@@ -656,12 +657,28 @@ static const struct chip_info chip_info_tbl[] = {
 	},
 };
 
+static char *isl29018_match_acpi_device(struct device *dev, int *data)
+{
+	const struct acpi_device_id *id;
+
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
+
+	if (!id)
+		return NULL;
+
+	*data = (int) id->driver_data;
+
+	return (char *) dev_name(dev);
+}
+
 static int isl29018_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct isl29018_chip *chip;
 	struct iio_dev *indio_dev;
 	int err;
+	char *name = NULL;
+	int dev_id = 0;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
 	if (indio_dev == NULL) {
@@ -673,9 +690,17 @@ static int isl29018_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, indio_dev);
 	chip->dev = &client->dev;
 
+	if (id) {
+		name = (char *) id->name;
+		dev_id = id->driver_data;
+	}
+
+	if (ACPI_HANDLE(&client->dev))
+		name = isl29018_match_acpi_device(&client->dev, &dev_id);
+
 	mutex_init(&chip->lock);
 
-	chip->type = id->driver_data;
+	chip->type = dev_id;
 	chip->lux_scale = 1;
 	chip->lux_uscale = 0;
 	chip->range = 1000;
@@ -683,7 +708,7 @@ static int isl29018_probe(struct i2c_client *client,
 	chip->suspended = false;
 
 	chip->regmap = devm_regmap_init_i2c(client,
-				chip_info_tbl[id->driver_data].regmap_cfg);
+				chip_info_tbl[dev_id].regmap_cfg);
 	if (IS_ERR(chip->regmap)) {
 		err = PTR_ERR(chip->regmap);
 		dev_err(chip->dev, "regmap initialization failed: %d\n", err);
@@ -694,10 +719,10 @@ static int isl29018_probe(struct i2c_client *client,
 	if (err)
 		return err;
 
-	indio_dev->info = chip_info_tbl[id->driver_data].indio_info;
-	indio_dev->channels = chip_info_tbl[id->driver_data].channels;
-	indio_dev->num_channels = chip_info_tbl[id->driver_data].num_channels;
-	indio_dev->name = id->name;
+	indio_dev->info = chip_info_tbl[dev_id].indio_info;
+	indio_dev->channels = chip_info_tbl[dev_id].channels;
+	indio_dev->num_channels = chip_info_tbl[dev_id].num_channels;
+	indio_dev->name = name;
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	err = devm_iio_device_register(&client->dev, indio_dev);
@@ -747,6 +772,14 @@ static SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend, isl29018_resume);
 #define ISL29018_PM_OPS NULL
 #endif
 
+static const struct acpi_device_id isl29018_acpi_match[] = {
+	{"ISL29018", isl29018},
+	{"ISL29023", isl29023},
+	{"ISL29035", isl29035},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, isl29018_acpi_match);
+
 static const struct i2c_device_id isl29018_id[] = {
 	{"isl29018", isl29018},
 	{"isl29023", isl29023},
@@ -768,6 +801,7 @@ static struct i2c_driver isl29018_driver = {
 	.class	= I2C_CLASS_HWMON,
 	.driver	 = {
 			.name = "isl29018",
+			.acpi_match_table = ACPI_PTR(isl29018_acpi_match),
 			.pm = ISL29018_PM_OPS,
 			.owner = THIS_MODULE,
 			.of_match_table = isl29018_of_match,
-- 
1.9.1

[PATCH 3/3] staging: iio: light: isl29018: add ACPI support

[Laurentiu Palcu]

Add support for enumerating the device through ACPI.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/staging/iio/light/isl29018.c | 46 +++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
index b50f126..c54c6ae 100644
--- a/drivers/staging/iio/light/isl29018.c
+++ b/drivers/staging/iio/light/isl29018.c
@@ -30,6 +30,7 @@
 #include <linux/slab.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/acpi.h>
 
 #define CONVERSION_TIME_MS		100
 
@@ -656,12 +657,28 @@ static const struct chip_info chip_info_tbl[] = {
 	},
 };
 
+static char *isl29018_match_acpi_device(struct device *dev, int *data)
+{
+	const struct acpi_device_id *id;
+
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
+
+	if (!id)
+		return NULL;
+
+	*data = (int) id->driver_data;
+
+	return (char *) dev_name(dev);
+}
+
 static int isl29018_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct isl29018_chip *chip;
 	struct iio_dev *indio_dev;
 	int err;
+	char *name = NULL;
+	int dev_id = 0;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
 	if (indio_dev == NULL) {
@@ -673,9 +690,17 @@ static int isl29018_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, indio_dev);
 	chip->dev = &client->dev;
 
+	if (id) {
+		name = (char *) id->name;
+		dev_id = id->driver_data;
+	}
+
+	if (ACPI_HANDLE(&client->dev))
+		name = isl29018_match_acpi_device(&client->dev, &dev_id);
+
 	mutex_init(&chip->lock);
 
-	chip->type = id->driver_data;
+	chip->type = dev_id;
 	chip->lux_scale = 1;
 	chip->lux_uscale = 0;
 	chip->range = 1000;
@@ -683,7 +708,7 @@ static int isl29018_probe(struct i2c_client *client,
 	chip->suspended = false;
 
 	chip->regmap = devm_regmap_init_i2c(client,
-				chip_info_tbl[id->driver_data].regmap_cfg);
+				chip_info_tbl[dev_id].regmap_cfg);
 	if (IS_ERR(chip->regmap)) {
 		err = PTR_ERR(chip->regmap);
 		dev_err(chip->dev, "regmap initialization failed: %d\n", err);
@@ -694,10 +719,10 @@ static int isl29018_probe(struct i2c_client *client,
 	if (err)
 		return err;
 
-	indio_dev->info = chip_info_tbl[id->driver_data].indio_info;
-	indio_dev->channels = chip_info_tbl[id->driver_data].channels;
-	indio_dev->num_channels = chip_info_tbl[id->driver_data].num_channels;
-	indio_dev->name = id->name;
+	indio_dev->info = chip_info_tbl[dev_id].indio_info;
+	indio_dev->channels = chip_info_tbl[dev_id].channels;
+	indio_dev->num_channels = chip_info_tbl[dev_id].num_channels;
+	indio_dev->name = name;
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	err = devm_iio_device_register(&client->dev, indio_dev);
@@ -747,6 +772,14 @@ static SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend, isl29018_resume);
 #define ISL29018_PM_OPS NULL
 #endif
 
+static const struct acpi_device_id isl29018_acpi_match[] = {
+	{"ISL29018", isl29018},
+	{"ISL29023", isl29023},
+	{"ISL29035", isl29035},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, isl29018_acpi_match);
+
 static const struct i2c_device_id isl29018_id[] = {
 	{"isl29018", isl29018},
 	{"isl29023", isl29023},
@@ -768,6 +801,7 @@ static struct i2c_driver isl29018_driver = {
 	.class	= I2C_CLASS_HWMON,
 	.driver	 = {
 			.name = "isl29018",
+			.acpi_match_table = ACPI_PTR(isl29018_acpi_match),
 			.pm = ISL29018_PM_OPS,
 			.owner = THIS_MODULE,
 			.of_match_table = isl29018_of_match,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] staging: iio: light: isl29018: add ACPI support

[Jonathan Cameron]

On 29/08/14 15:26, Laurentiu Palcu wrote:
> Add support for enumerating the device through ACPI.
>
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
Mostly fine, but I'm unclear on why we have a cast to a char * for name.

Maybe I just haven't had enough coffee this morning :)

Jonathan
> ---
>  drivers/staging/iio/light/isl29018.c | 46 +++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index b50f126..c54c6ae 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -30,6 +30,7 @@
>  #include <linux/slab.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> +#include <linux/acpi.h>
>
>  #define CONVERSION_TIME_MS		100
>
> @@ -656,12 +657,28 @@ static const struct chip_info chip_info_tbl[] = {
>  	},
>  };
>
> +static char *isl29018_match_acpi_device(struct device *dev, int *data)
> +{
> +	const struct acpi_device_id *id;
> +
> +	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> +
> +	if (!id)
> +		return NULL;
> +
> +	*data = (int) id->driver_data;
> +
> +	return (char *) dev_name(dev);
Silly question, but what is wrong with const char * as the return type?
Where this name ultimately ends up is a const char * anyway so you cast
the const away and then back again.
> +}
> +
>  static int isl29018_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
>  	struct isl29018_chip *chip;
>  	struct iio_dev *indio_dev;
>  	int err;
> +	char *name = NULL;
const char *

> +	int dev_id = 0;
>
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
>  	if (indio_dev == NULL) {
> @@ -673,9 +690,17 @@ static int isl29018_probe(struct i2c_client *client,
>  	i2c_set_clientdata(client, indio_dev);
>  	chip->dev = &client->dev;
>
> +	if (id) {
> +		name = (char *) id->name;
> +		dev_id = id->driver_data;
> +	}
> +
> +	if (ACPI_HANDLE(&client->dev))
> +		name = isl29018_match_acpi_device(&client->dev, &dev_id);
> +
>  	mutex_init(&chip->lock);
>
> -	chip->type = id->driver_data;
> +	chip->type = dev_id;
>  	chip->lux_scale = 1;
>  	chip->lux_uscale = 0;
>  	chip->range = 1000;
> @@ -683,7 +708,7 @@ static int isl29018_probe(struct i2c_client *client,
>  	chip->suspended = false;
>
>  	chip->regmap = devm_regmap_init_i2c(client,
> -				chip_info_tbl[id->driver_data].regmap_cfg);
> +				chip_info_tbl[dev_id].regmap_cfg);
>  	if (IS_ERR(chip->regmap)) {
>  		err = PTR_ERR(chip->regmap);
>  		dev_err(chip->dev, "regmap initialization failed: %d\n", err);
> @@ -694,10 +719,10 @@ static int isl29018_probe(struct i2c_client *client,
>  	if (err)
>  		return err;
>
> -	indio_dev->info = chip_info_tbl[id->driver_data].indio_info;
> -	indio_dev->channels = chip_info_tbl[id->driver_data].channels;
> -	indio_dev->num_channels = chip_info_tbl[id->driver_data].num_channels;
> -	indio_dev->name = id->name;
> +	indio_dev->info = chip_info_tbl[dev_id].indio_info;
> +	indio_dev->channels = chip_info_tbl[dev_id].channels;
> +	indio_dev->num_channels = chip_info_tbl[dev_id].num_channels;
> +	indio_dev->name = name;
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	err = devm_iio_device_register(&client->dev, indio_dev);
> @@ -747,6 +772,14 @@ static SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend, isl29018_resume);
>  #define ISL29018_PM_OPS NULL
>  #endif
>
> +static const struct acpi_device_id isl29018_acpi_match[] = {
> +	{"ISL29018", isl29018},
> +	{"ISL29023", isl29023},
> +	{"ISL29035", isl29035},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(acpi, isl29018_acpi_match);
> +
>  static const struct i2c_device_id isl29018_id[] = {
>  	{"isl29018", isl29018},
>  	{"isl29023", isl29023},
> @@ -768,6 +801,7 @@ static struct i2c_driver isl29018_driver = {
>  	.class	= I2C_CLASS_HWMON,
>  	.driver	 = {
>  			.name = "isl29018",
> +			.acpi_match_table = ACPI_PTR(isl29018_acpi_match),
>  			.pm = ISL29018_PM_OPS,
>  			.owner = THIS_MODULE,
>  			.of_match_table = isl29018_of_match,
>

Re: [PATCH 3/3] staging: iio: light: isl29018: add ACPI support

[Jonathan Cameron]

On 29/08/14 15:26, Laurentiu Palcu wrote:
> Add support for enumerating the device through ACPI.
>
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Mostly fine, but I'm unclear on why we have a cast to a char * for name.

Maybe I just haven't had enough coffee this morning :)

Jonathan
> ---
>  drivers/staging/iio/light/isl29018.c | 46 +++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index b50f126..c54c6ae 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -30,6 +30,7 @@
>  #include <linux/slab.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> +#include <linux/acpi.h>
>
>  #define CONVERSION_TIME_MS		100
>
> @@ -656,12 +657,28 @@ static const struct chip_info chip_info_tbl[] = {
>  	},
>  };
>
> +static char *isl29018_match_acpi_device(struct device *dev, int *data)
> +{
> +	const struct acpi_device_id *id;
> +
> +	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> +
> +	if (!id)
> +		return NULL;
> +
> +	*data = (int) id->driver_data;
> +
> +	return (char *) dev_name(dev);
Silly question, but what is wrong with const char * as the return type?
Where this name ultimately ends up is a const char * anyway so you cast
the const away and then back again.
> +}
> +
>  static int isl29018_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
>  	struct isl29018_chip *chip;
>  	struct iio_dev *indio_dev;
>  	int err;
> +	char *name = NULL;
const char *

> +	int dev_id = 0;
>
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
>  	if (indio_dev == NULL) {
> @@ -673,9 +690,17 @@ static int isl29018_probe(struct i2c_client *client,
>  	i2c_set_clientdata(client, indio_dev);
>  	chip->dev = &client->dev;
>
> +	if (id) {
> +		name = (char *) id->name;
> +		dev_id = id->driver_data;
> +	}
> +
> +	if (ACPI_HANDLE(&client->dev))
> +		name = isl29018_match_acpi_device(&client->dev, &dev_id);
> +
>  	mutex_init(&chip->lock);
>
> -	chip->type = id->driver_data;
> +	chip->type = dev_id;
>  	chip->lux_scale = 1;
>  	chip->lux_uscale = 0;
>  	chip->range = 1000;
> @@ -683,7 +708,7 @@ static int isl29018_probe(struct i2c_client *client,
>  	chip->suspended = false;
>
>  	chip->regmap = devm_regmap_init_i2c(client,
> -				chip_info_tbl[id->driver_data].regmap_cfg);
> +				chip_info_tbl[dev_id].regmap_cfg);
>  	if (IS_ERR(chip->regmap)) {
>  		err = PTR_ERR(chip->regmap);
>  		dev_err(chip->dev, "regmap initialization failed: %d\n", err);
> @@ -694,10 +719,10 @@ static int isl29018_probe(struct i2c_client *client,
>  	if (err)
>  		return err;
>
> -	indio_dev->info = chip_info_tbl[id->driver_data].indio_info;
> -	indio_dev->channels = chip_info_tbl[id->driver_data].channels;
> -	indio_dev->num_channels = chip_info_tbl[id->driver_data].num_channels;
> -	indio_dev->name = id->name;
> +	indio_dev->info = chip_info_tbl[dev_id].indio_info;
> +	indio_dev->channels = chip_info_tbl[dev_id].channels;
> +	indio_dev->num_channels = chip_info_tbl[dev_id].num_channels;
> +	indio_dev->name = name;
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	err = devm_iio_device_register(&client->dev, indio_dev);
> @@ -747,6 +772,14 @@ static SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend, isl29018_resume);
>  #define ISL29018_PM_OPS NULL
>  #endif
>
> +static const struct acpi_device_id isl29018_acpi_match[] = {
> +	{"ISL29018", isl29018},
> +	{"ISL29023", isl29023},
> +	{"ISL29035", isl29035},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(acpi, isl29018_acpi_match);
> +
>  static const struct i2c_device_id isl29018_id[] = {
>  	{"isl29018", isl29018},
>  	{"isl29023", isl29023},
> @@ -768,6 +801,7 @@ static struct i2c_driver isl29018_driver = {
>  	.class	= I2C_CLASS_HWMON,
>  	.driver	 = {
>  			.name = "isl29018",
> +			.acpi_match_table = ACPI_PTR(isl29018_acpi_match),
>  			.pm = ISL29018_PM_OPS,
>  			.owner = THIS_MODULE,
>  			.of_match_table = isl29018_of_match,
>

[PATCH v2 3/3] staging: iio: light: isl29018: add ACPI support

[Laurentiu Palcu]

Add support for enumerating the device through ACPI.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---

Changes in v2:
 * fix the unnecessary casting from const char * to char * and back;

Thank you Jonathan for the reviews,
laurentiu

 drivers/staging/iio/light/isl29018.c | 46 +++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
index b50f126..65a35da 100644
--- a/drivers/staging/iio/light/isl29018.c
+++ b/drivers/staging/iio/light/isl29018.c
@@ -30,6 +30,7 @@
 #include <linux/slab.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/acpi.h>
 
 #define CONVERSION_TIME_MS		100
 
@@ -656,12 +657,28 @@ static const struct chip_info chip_info_tbl[] = {
 	},
 };
 
+static const char *isl29018_match_acpi_device(struct device *dev, int *data)
+{
+	const struct acpi_device_id *id;
+
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
+
+	if (!id)
+		return NULL;
+
+	*data = (int) id->driver_data;
+
+	return dev_name(dev);
+}
+
 static int isl29018_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct isl29018_chip *chip;
 	struct iio_dev *indio_dev;
 	int err;
+	const char *name = NULL;
+	int dev_id = 0;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
 	if (indio_dev == NULL) {
@@ -673,9 +690,17 @@ static int isl29018_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, indio_dev);
 	chip->dev = &client->dev;
 
+	if (id) {
+		name = id->name;
+		dev_id = id->driver_data;
+	}
+
+	if (ACPI_HANDLE(&client->dev))
+		name = isl29018_match_acpi_device(&client->dev, &dev_id);
+
 	mutex_init(&chip->lock);
 
-	chip->type = id->driver_data;
+	chip->type = dev_id;
 	chip->lux_scale = 1;
 	chip->lux_uscale = 0;
 	chip->range = 1000;
@@ -683,7 +708,7 @@ static int isl29018_probe(struct i2c_client *client,
 	chip->suspended = false;
 
 	chip->regmap = devm_regmap_init_i2c(client,
-				chip_info_tbl[id->driver_data].regmap_cfg);
+				chip_info_tbl[dev_id].regmap_cfg);
 	if (IS_ERR(chip->regmap)) {
 		err = PTR_ERR(chip->regmap);
 		dev_err(chip->dev, "regmap initialization failed: %d\n", err);
@@ -694,10 +719,10 @@ static int isl29018_probe(struct i2c_client *client,
 	if (err)
 		return err;
 
-	indio_dev->info = chip_info_tbl[id->driver_data].indio_info;
-	indio_dev->channels = chip_info_tbl[id->driver_data].channels;
-	indio_dev->num_channels = chip_info_tbl[id->driver_data].num_channels;
-	indio_dev->name = id->name;
+	indio_dev->info = chip_info_tbl[dev_id].indio_info;
+	indio_dev->channels = chip_info_tbl[dev_id].channels;
+	indio_dev->num_channels = chip_info_tbl[dev_id].num_channels;
+	indio_dev->name = name;
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	err = devm_iio_device_register(&client->dev, indio_dev);
@@ -747,6 +772,14 @@ static SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend, isl29018_resume);
 #define ISL29018_PM_OPS NULL
 #endif
 
+static const struct acpi_device_id isl29018_acpi_match[] = {
+	{"ISL29018", isl29018},
+	{"ISL29023", isl29023},
+	{"ISL29035", isl29035},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, isl29018_acpi_match);
+
 static const struct i2c_device_id isl29018_id[] = {
 	{"isl29018", isl29018},
 	{"isl29023", isl29023},
@@ -768,6 +801,7 @@ static struct i2c_driver isl29018_driver = {
 	.class	= I2C_CLASS_HWMON,
 	.driver	 = {
 			.name = "isl29018",
+			.acpi_match_table = ACPI_PTR(isl29018_acpi_match),
 			.pm = ISL29018_PM_OPS,
 			.owner = THIS_MODULE,
 			.of_match_table = isl29018_of_match,
-- 
1.9.1

Re: [PATCH v2 3/3] staging: iio: light: isl29018: add ACPI support

[Jonathan Cameron]

On 01/09/14 10:20, Laurentiu Palcu wrote:
> Add support for enumerating the device through ACPI.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
Applied to the togreg branch of iio.git - initially pushed out as
testing for the autobuilders to play.

Thanks,

Jonathan
> ---
> 
> Changes in v2:
>  * fix the unnecessary casting from const char * to char * and back;
> 
> Thank you Jonathan for the reviews,
> laurentiu
> 
>  drivers/staging/iio/light/isl29018.c | 46 +++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index b50f126..65a35da 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -30,6 +30,7 @@
>  #include <linux/slab.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> +#include <linux/acpi.h>
>  
>  #define CONVERSION_TIME_MS		100
>  
> @@ -656,12 +657,28 @@ static const struct chip_info chip_info_tbl[] = {
>  	},
>  };
>  
> +static const char *isl29018_match_acpi_device(struct device *dev, int *data)
> +{
> +	const struct acpi_device_id *id;
> +
> +	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> +
> +	if (!id)
> +		return NULL;
> +
> +	*data = (int) id->driver_data;
> +
> +	return dev_name(dev);
> +}
> +
>  static int isl29018_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
>  	struct isl29018_chip *chip;
>  	struct iio_dev *indio_dev;
>  	int err;
> +	const char *name = NULL;
> +	int dev_id = 0;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
>  	if (indio_dev == NULL) {
> @@ -673,9 +690,17 @@ static int isl29018_probe(struct i2c_client *client,
>  	i2c_set_clientdata(client, indio_dev);
>  	chip->dev = &client->dev;
>  
> +	if (id) {
> +		name = id->name;
> +		dev_id = id->driver_data;
> +	}
> +
> +	if (ACPI_HANDLE(&client->dev))
> +		name = isl29018_match_acpi_device(&client->dev, &dev_id);
> +
>  	mutex_init(&chip->lock);
>  
> -	chip->type = id->driver_data;
> +	chip->type = dev_id;
>  	chip->lux_scale = 1;
>  	chip->lux_uscale = 0;
>  	chip->range = 1000;
> @@ -683,7 +708,7 @@ static int isl29018_probe(struct i2c_client *client,
>  	chip->suspended = false;
>  
>  	chip->regmap = devm_regmap_init_i2c(client,
> -				chip_info_tbl[id->driver_data].regmap_cfg);
> +				chip_info_tbl[dev_id].regmap_cfg);
>  	if (IS_ERR(chip->regmap)) {
>  		err = PTR_ERR(chip->regmap);
>  		dev_err(chip->dev, "regmap initialization failed: %d\n", err);
> @@ -694,10 +719,10 @@ static int isl29018_probe(struct i2c_client *client,
>  	if (err)
>  		return err;
>  
> -	indio_dev->info = chip_info_tbl[id->driver_data].indio_info;
> -	indio_dev->channels = chip_info_tbl[id->driver_data].channels;
> -	indio_dev->num_channels = chip_info_tbl[id->driver_data].num_channels;
> -	indio_dev->name = id->name;
> +	indio_dev->info = chip_info_tbl[dev_id].indio_info;
> +	indio_dev->channels = chip_info_tbl[dev_id].channels;
> +	indio_dev->num_channels = chip_info_tbl[dev_id].num_channels;
> +	indio_dev->name = name;
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	err = devm_iio_device_register(&client->dev, indio_dev);
> @@ -747,6 +772,14 @@ static SIMPLE_DEV_PM_OPS(isl29018_pm_ops, isl29018_suspend, isl29018_resume);
>  #define ISL29018_PM_OPS NULL
>  #endif
>  
> +static const struct acpi_device_id isl29018_acpi_match[] = {
> +	{"ISL29018", isl29018},
> +	{"ISL29023", isl29023},
> +	{"ISL29035", isl29035},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(acpi, isl29018_acpi_match);
> +
>  static const struct i2c_device_id isl29018_id[] = {
>  	{"isl29018", isl29018},
>  	{"isl29023", isl29023},
> @@ -768,6 +801,7 @@ static struct i2c_driver isl29018_driver = {
>  	.class	= I2C_CLASS_HWMON,
>  	.driver	 = {
>  			.name = "isl29018",
> +			.acpi_match_table = ACPI_PTR(isl29018_acpi_match),
>  			.pm = ISL29018_PM_OPS,
>  			.owner = THIS_MODULE,
>  			.of_match_table = isl29018_of_match,
>