[PATCH] iio: light: ltr501: Added ltr303 driver support

[PATCH] iio: light: ltr501: Added ltr303 driver support

[Maslov Dmitry]

Previously ltr501 driver supported a number of light and,
proximity sensors including ltr501, ltr559 and ltr301.
This adds support for another light sensor ltr303
used in Seeed Studio reTerminal, a carrier board
for Raspberry Pi 4 CM.

Signed-off-by: Maslov Dmitry <maslovdmitry@seeed.cc>
---
 drivers/iio/light/ltr501.c | 46 +++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 7e51aaac0bf..733f9224bd1 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * ltr501.c - Support for Lite-On LTR501 ambient light and proximity sensor
+ * ltr.c - Support for Lite-On LTR501, LTR509, LTR301, LTR303 ambient light
+ * and proximity sensors (only LTR5xx)
  *
  * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
  *
@@ -16,6 +17,8 @@
 #include <linux/regmap.h>
 #include <linux/acpi.h>
 #include <linux/regulator/consumer.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/events.h>
@@ -65,11 +68,15 @@
 #define LTR501_ALS_DEF_PERIOD 500000
 #define LTR501_PS_DEF_PERIOD 100000
 
+#define LTR501_ALS_DEF_GAIN		(BIT(4) | BIT(3) | BIT(2))
+
 #define LTR501_REGMAP_NAME "ltr501_regmap"
 
 #define LTR501_LUX_CONV(vis_coeff, vis_data, ir_coeff, ir_data) \
 			((vis_coeff * vis_data) - (ir_coeff * ir_data))
 
+static const struct i2c_device_id ltr501_id[];
+
 static const int int_time_mapping[] = {100000, 50000, 200000, 400000};
 
 static const struct reg_field reg_field_it =
@@ -98,6 +105,9 @@ enum {
 	ltr501 = 0,
 	ltr559,
 	ltr301,
+	ltr303,
+
+	ltr_max
 };
 
 struct ltr501_gain {
@@ -1231,6 +1241,18 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
 		.channels = ltr301_channels,
 		.no_channels = ARRAY_SIZE(ltr301_channels),
 	},
+	[ltr303] = {
+		.partid = 0x0A,
+		.als_gain = ltr559_als_gain_tbl,
+		.als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl),
+		.als_mode_active = BIT(0),
+		.als_gain_mask = BIT(2) | BIT(3) | BIT(4),
+		.als_gain_shift = 2,
+		.info = &ltr301_info,
+		.info_no_irq = &ltr301_info_no_irq,
+		.channels = ltr301_channels,
+		.no_channels = ARRAY_SIZE(ltr301_channels),
+	},
 };
 
 static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)
@@ -1337,7 +1359,7 @@ static int ltr501_init(struct ltr501_data *data)
 	if (ret < 0)
 		return ret;
 
-	data->als_contr = status | data->chip_info->als_mode_active;
+	data->als_contr = status | data->chip_info->als_mode_active | LTR501_ALS_DEF_GAIN;
 
 	ret = regmap_read(data->regmap, LTR501_PS_CONTR, &status);
 	if (ret < 0)
@@ -1504,7 +1526,23 @@ static int ltr501_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
-	if (id) {
+	if (client->dev.of_node) {
+		int i = 0;
+
+		chip_idx = (int)of_device_get_match_data(&client->dev);
+		for (i = 0; i < ltr_max; i++) {
+			if (ltr501_id[i].name == NULL) {
+				break;
+			}
+			if (ltr501_id[i].driver_data == chip_idx) {
+				name = ltr501_id[i].name;
+				break;
+			}
+		}
+		if (i >= ltr_max) {
+			name = LTR501_DRV_NAME;
+		}
+	} else if (id) {
 		name = id->name;
 		chip_idx = id->driver_data;
 	} else  if (ACPI_HANDLE(&client->dev)) {
@@ -1597,6 +1635,7 @@ static const struct acpi_device_id ltr_acpi_match[] = {
 	{"LTER0501", ltr501},
 	{"LTER0559", ltr559},
 	{"LTER0301", ltr301},
+	{"LTER0303", ltr303},
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, ltr_acpi_match);
@@ -1605,6 +1644,7 @@ static const struct i2c_device_id ltr501_id[] = {
 	{ "ltr501", ltr501},
 	{ "ltr559", ltr559},
 	{ "ltr301", ltr301},
+	{ "ltr303", ltr303},
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ltr501_id);
-- 
2.25.1

Re: [PATCH] iio: light: ltr501: Added ltr303 driver support

[Lars-Peter Clausen]

On 10/20/21 9:35 AM, Maslov Dmitry wrote:
> Previously ltr501 driver supported a number of light and,
> proximity sensors including ltr501, ltr559 and ltr301.
> This adds support for another light sensor ltr303
> used in Seeed Studio reTerminal, a carrier board
> for Raspberry Pi 4 CM.

Hi,

Thanks for the patch. This looks good. But there are a couple of 
different things in this change that go beyond what is described in the 
commit message. It would be best to split these out into separate 
patches and provide a description of what they are doing.


>
> Signed-off-by: Maslov Dmitry <maslovdmitry@seeed.cc>
> ---
>   drivers/iio/light/ltr501.c | 46 +++++++++++++++++++++++++++++++++++---
>   1 file changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 7e51aaac0bf..733f9224bd1 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   /*
> - * ltr501.c - Support for Lite-On LTR501 ambient light and proximity sensor
> + * ltr.c - Support for Lite-On LTR501, LTR509, LTR301, LTR303 ambient light
The filename did not change. Me personally I'm a big fan of not 
referencing the filename in the file, because it is easily outdated when 
renamed. So maybe just remove that part of the comment.
> + * and proximity sensors (only LTR5xx)
>    *
>    * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
>    *
> [...]
> @@ -1231,6 +1241,18 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
>   		.channels = ltr301_channels,
>   		.no_channels = ARRAY_SIZE(ltr301_channels),
>   	},
> +	[ltr303] = {
> +		.partid = 0x0A,
> +		.als_gain = ltr559_als_gain_tbl,
> +		.als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl),
> +		.als_mode_active = BIT(0),
> +		.als_gain_mask = BIT(2) | BIT(3) | BIT(4),
> +		.als_gain_shift = 2,
> +		.info = &ltr301_info,
> +		.info_no_irq = &ltr301_info_no_irq,
> +		.channels = ltr301_channels,
> +		.no_channels = ARRAY_SIZE(ltr301_channels),
> +	},
>   };
>   
>   static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)
> @@ -1337,7 +1359,7 @@ static int ltr501_init(struct ltr501_data *data)
>   	if (ret < 0)
>   		return ret;
>   
> -	data->als_contr = status | data->chip_info->als_mode_active;
> +	data->als_contr = status | data->chip_info->als_mode_active | LTR501_ALS_DEF_GAIN;
This is not mentioned in the commit description. Why is this necessary 
now, but wasn't before?
>   
>   	ret = regmap_read(data->regmap, LTR501_PS_CONTR, &status);
>   	if (ret < 0)
> @@ -1504,7 +1526,23 @@ static int ltr501_probe(struct i2c_client *client,
>   	if (ret < 0)
>   		return ret;
>   
> -	if (id) {
> +	if (client->dev.of_node) {
> +		int i = 0;
> +
> +		chip_idx = (int)of_device_get_match_data(&client->dev);
> +		for (i = 0; i < ltr_max; i++) {
> +			if (ltr501_id[i].name == NULL) {
> +				break;
> +			}
> +			if (ltr501_id[i].driver_data == chip_idx) {
> +				name = ltr501_id[i].name;
> +				break;
> +			}
> +		}
> +		if (i >= ltr_max) {
> +			name = LTR501_DRV_NAME;
> +		}

Same here, this doesn't seem to be 303 specific and there is no mention 
of why this is needed in the patch description. There are no other 
drivers which seem to do something similar, why is it need for this driver?


> +	} else if (id) {
>   		name = id->name;
>   		chip_idx = id->driver_data;
>   	} else  if (ACPI_HANDLE(&client->dev)) {

Re: [PATCH] iio: light: ltr501: Added ltr303 driver support

[Jonathan Cameron]

On Mon, 25 Oct 2021 23:30:23 +0200
Maslov Dmitry <maslovdmitry@seeed.cc> wrote:

> Previously ltr501 driver supported a number of light and,
> proximity sensors including ltr501, ltr559 and ltr301.
> This adds support for another light sensor ltr303
> used in Seeed Studio reTerminal, a carrier board
> for Raspberry Pi 4 CM.
> 
> It is a fix of previous commit, that simplified ltr303 support
> and removed a lot of unnecessary code. Filename has also been removed
> from the file.
> 
> Signed-off-by: Maslov Dmitry <maslovdmitry@seeed.cc>

I missed that email should have had the subject

[PATCH v2] iio: light: ltr501: Add ltr303 support

> ---
>  drivers/iio/light/ltr501.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 7e51aaac0bf..d92d324d5e3 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * ltr501.c - Support for Lite-On LTR501 ambient light and proximity sensor
> + * Support for Lite-On LTR501, LTR509, LTR301, LTR303 ambient light
> + * and proximity sensors (only LTR5xx)
>   *
>   * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
>   *
> @@ -98,6 +99,9 @@ enum {
>  	ltr501 = 0,
>  	ltr559,
>  	ltr301,
> +	ltr303,
> +
> +	ltr_max
>  };
>  
>  struct ltr501_gain {
> @@ -1231,6 +1235,18 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
>  		.channels = ltr301_channels,
>  		.no_channels = ARRAY_SIZE(ltr301_channels),
>  	},
> +	[ltr303] = {
> +		.partid = 0x0A,
> +		.als_gain = ltr559_als_gain_tbl,
> +		.als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl),
> +		.als_mode_active = BIT(0),
> +		.als_gain_mask = BIT(2) | BIT(3) | BIT(4),
> +		.als_gain_shift = 2,
> +		.info = &ltr301_info,
> +		.info_no_irq = &ltr301_info_no_irq,
> +		.channels = ltr301_channels,
> +		.no_channels = ARRAY_SIZE(ltr301_channels),
> +	},
>  };
>  
>  static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)
> @@ -1597,6 +1613,7 @@ static const struct acpi_device_id ltr_acpi_match[] = {
>  	{"LTER0501", ltr501},
>  	{"LTER0559", ltr559},
>  	{"LTER0301", ltr301},
> +	{"LTER0303", ltr303},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(acpi, ltr_acpi_match);
> @@ -1605,6 +1622,7 @@ static const struct i2c_device_id ltr501_id[] = {
>  	{ "ltr501", ltr501},
>  	{ "ltr559", ltr559},
>  	{ "ltr301", ltr301},
> +	{ "ltr303", ltr303},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, ltr501_id);
> @@ -1613,6 +1631,7 @@ static const struct of_device_id ltr501_of_match[] = {
>  	{ .compatible = "liteon,ltr501", },
>  	{ .compatible = "liteon,ltr559", },
>  	{ .compatible = "liteon,ltr301", },
> +	{ .compatible = "liteon,ltr303", },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, ltr501_of_match);

[PATCH] iio: light: ltr501: Added ltr303 driver support

[Maslov Dmitry]

Previously ltr501 driver supported a number of light and,
proximity sensors including ltr501, ltr559 and ltr301.
This adds support for another light sensor ltr303
used in Seeed Studio reTerminal, a carrier board
for Raspberry Pi 4 CM.

It is a fix of previous commit, that simplified ltr303 support
and removed a lot of unnecessary code. Filename has also been removed
from the file.

Signed-off-by: Maslov Dmitry <maslovdmitry@seeed.cc>
---
 drivers/iio/light/ltr501.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 7e51aaac0bf..d92d324d5e3 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * ltr501.c - Support for Lite-On LTR501 ambient light and proximity sensor
+ * Support for Lite-On LTR501, LTR509, LTR301, LTR303 ambient light
+ * and proximity sensors (only LTR5xx)
  *
  * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
  *
@@ -98,6 +99,9 @@ enum {
 	ltr501 = 0,
 	ltr559,
 	ltr301,
+	ltr303,
+
+	ltr_max
 };
 
 struct ltr501_gain {
@@ -1231,6 +1235,18 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
 		.channels = ltr301_channels,
 		.no_channels = ARRAY_SIZE(ltr301_channels),
 	},
+	[ltr303] = {
+		.partid = 0x0A,
+		.als_gain = ltr559_als_gain_tbl,
+		.als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl),
+		.als_mode_active = BIT(0),
+		.als_gain_mask = BIT(2) | BIT(3) | BIT(4),
+		.als_gain_shift = 2,
+		.info = &ltr301_info,
+		.info_no_irq = &ltr301_info_no_irq,
+		.channels = ltr301_channels,
+		.no_channels = ARRAY_SIZE(ltr301_channels),
+	},
 };
 
 static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)
@@ -1597,6 +1613,7 @@ static const struct acpi_device_id ltr_acpi_match[] = {
 	{"LTER0501", ltr501},
 	{"LTER0559", ltr559},
 	{"LTER0301", ltr301},
+	{"LTER0303", ltr303},
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, ltr_acpi_match);
@@ -1605,6 +1622,7 @@ static const struct i2c_device_id ltr501_id[] = {
 	{ "ltr501", ltr501},
 	{ "ltr559", ltr559},
 	{ "ltr301", ltr301},
+	{ "ltr303", ltr303},
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ltr501_id);
@@ -1613,6 +1631,7 @@ static const struct of_device_id ltr501_of_match[] = {
 	{ .compatible = "liteon,ltr501", },
 	{ .compatible = "liteon,ltr559", },
 	{ .compatible = "liteon,ltr301", },
+	{ .compatible = "liteon,ltr303", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, ltr501_of_match);
-- 
2.25.1