drivers: sensor: APDS9306: Add lux conversion

- Add lux conversion to APDS-9306 driver
- Change settings of gain, resolution and
frequency to index-based settings
- Add Device Tree overlay sample for APDS-9306
- Fix wrong board name in light_polling README
- Add value checks for the attribute set API call
- Remove the reading of the sensor attributes
from the sensor and use buffered values instead
- Rename `frequency` property to `measurement period`

Closes #91104

Signed-off-by: Daniel Kampert <[email protected]>

Author: Kampi

drivers/sensor/apds9306/apds9306.c

@@ -1,5 +1,5 @@
 /* Copyright (c) 2024 Daniel Kampert
- * Author: Daniel Kampert <DanielKampert@kampis-Elektroecke.de>
+ * Author: Daniel Kampert <DanielKampert@kampis-elektroecke.de>
  */
 
 #include <zephyr/device.h>
@@ -45,42 +45,47 @@
 
 LOG_MODULE_REGISTER(avago_apds9306, CONFIG_SENSOR_LOG_LEVEL);
 
+/* Array length for the measurement period values. Aligned with avago,apds9306.yaml */
+static const uint8_t AVAGO_APDS_9306_MEASUREMENT_PERIOD_ARRAY_LENGTH = 7;
+
+/* Array length for the resolution values. Aligned with avago,apds9306.yaml */
+static const uint8_t AVAGO_APDS_9306_RESOLUTION_ARRAY_LENGTH = 6;
+
+/* See datasheet for the values. Aligned with avago,apds9306.yaml */
+static const uint8_t avago_apds9306_gain[] = {1, 3, 6, 9, 18};
+static const uint8_t AVAGO_APDS_9306_GAIN_ARRAY_LENGTH = ARRAY_SIZE(avago_apds9306_gain);
+
+/* See datasheet for the values. */
+/* Last value is rounded up to prevent floating point operations. */
+static const uint16_t avago_apds9306_integration_time[] = {400, 200, 100, 50, 25, 4};
+
+/* These values represent the gain based on the integration time. */
+/* A gain of 1 is used for a time of 3.125 ms (13 bits). */
+/* This results in a gain of 8 (2^3) for a time if 25 ms (16 bits), etc. */
+static const uint16_t avago_apds9306_integration_time_gain[] = {128, 64, 32, 16, 8, 1};
+
 struct apds9306_data {
 	uint32_t light;
+	uint8_t measurement_period_idx; /* This field holds the index of the current */
+					/* period measurement */
+	uint8_t gain_idx;       /* This field holds the index of the current sampling gain. */
+	uint8_t resolution_idx; /* This field holds the index of the current sampling */
+				/*  resolution.*/
+	uint8_t chip_id;
 };
 
 struct apds9306_config {
 	struct i2c_dt_spec i2c;
-	uint8_t resolution;
-	uint16_t frequency;
-	uint8_t gain;
+	uint8_t resolution_idx;
+	uint8_t measurement_period_idx;
+	uint8_t gain_idx;
 };
 
 struct apds9306_worker_item_t {
 	struct k_work_delayable dwork;
 	const struct device *dev;
 } apds9306_worker_item;
 
-static uint32_t apds9306_get_time_for_resolution(uint8_t value)
-{
-	switch (value) {
-	case 0:
-		return 400;
-	case 1:
-		return 200;
-	case 2:
-		return 100;
-	case 3:
-		return 50;
-	case 4:
-		return 25;
-	case 5:
-		return 4;
-	default:
-		return 100;
-	}
-}
-
 static int apds9306_enable(const struct device *dev)
 {
 	const struct apds9306_config *config = dev->config;
@@ -128,7 +133,41 @@ static void apds9306_worker(struct k_work *p_work)
 	}
 
 	data->light = sys_get_le24(buffer);
+	LOG_DBG("Last raw measurement: %u", data->light);
+
+	/* Based on the formula from the APDS-9309 datasheet, page 4:
+	 * https://docs.broadcom.com/doc/AV02-3689EN
+	 *
+	 *  Illuminance [Lux] = Data * (1 / (Gain * Integration Time)) * Factor [Lux]
+	 *
+	 * The factor is calculated with the given values from the
+	 * APDS-9306 datasheet, page 4.
+	 * 1. Convert the E value from uW/sqcm to Lux
+	 *   - 340.134 for the APDS-9306
+	 *   - 293.69 for the APDS-9306-065
+	 * 2. Use the formula from the APDS-9309 datasheet to get the factor by using
+	 *   - Gain = 3
+	 *   - Integration time = 100 ms
+	 * Caution: The unit is ms. We need a unit without a dimension to prevent wrong
+	 * units. So it must be converted into a value without dimension. This is done by converting
+	 * it into a bit value based on the resolution gain (=32).
+	 *   - ADC count = 2000
+	 * 3. Repeat it for both sensor types to get the factors (converted for integer operations)
+	 *   - APDS-9306: 16
+	 *   - APDS-9306-065: 14
+	 */
+	uint32_t gain = avago_apds9306_gain[data->gain_idx];
+	uint32_t integration_time = avago_apds9306_integration_time_gain[data->resolution_idx];
+	uint32_t factor = 16;
+
+	if (data->chip_id == APDS_9306_065_CHIP_ID) {
+		factor = 14;
+	}
 
+	data->light = (data->light * factor) / (gain * integration_time);
+
+	LOG_DBG("Gain: %u", gain);
+	LOG_DBG("Integration time: %u", integration_time);
 	LOG_DBG("Last measurement: %u", data->light);
 }
 
@@ -139,20 +178,30 @@ static int apds9306_attr_set(const struct device *dev, enum sensor_channel chann
 	uint8_t mask;
 	uint8_t temp;
 	const struct apds9306_config *config = dev->config;
+	struct apds9306_data *data = dev->data;
 
-	if (channel != SENSOR_CHAN_LIGHT) {
+	if ((channel != SENSOR_CHAN_ALL) && (channel != SENSOR_CHAN_LIGHT)) {
 		return -ENOTSUP;
 	}
 
 	if (attribute == SENSOR_ATTR_SAMPLING_FREQUENCY) {
+		if (value->val1 > (AVAGO_APDS_9306_MEASUREMENT_PERIOD_ARRAY_LENGTH - 1)) {
+			return -EINVAL;
+		}
 		reg = APDS9306_REGISTER_ALS_MEAS_RATE;
 		mask = GENMASK(2, 0);
 		temp = FIELD_PREP(0x07, value->val1);
 	} else if (attribute == SENSOR_ATTR_GAIN) {
+		if (value->val1 > (AVAGO_APDS_9306_GAIN_ARRAY_LENGTH - 1)) {
+			return -EINVAL;
+		}
 		reg = APDS9306_REGISTER_ALS_GAIN;
 		mask = GENMASK(2, 0);
 		temp = FIELD_PREP(0x07, value->val1);
 	} else if (attribute == SENSOR_ATTR_RESOLUTION) {
+		if (value->val1 > (AVAGO_APDS_9306_RESOLUTION_ARRAY_LENGTH - 1)) {
+			return -EINVAL;
+		}
 		reg = APDS9306_REGISTER_ALS_MEAS_RATE;
 		mask = GENMASK(7, 4);
 		temp = FIELD_PREP(0x07, value->val1) << 0x04;
@@ -165,51 +214,44 @@ static int apds9306_attr_set(const struct device *dev, enum sensor_channel chann
 		return -EFAULT;
 	}
 
+	/* We only save the new values when no error occurs to prevent invalid settings. */
+	if (attribute == SENSOR_ATTR_SAMPLING_FREQUENCY) {
+		data->measurement_period_idx = value->val1;
+	} else if (attribute == SENSOR_ATTR_GAIN) {
+		data->gain_idx = value->val1;
+	} else if (attribute == SENSOR_ATTR_RESOLUTION) {
+		data->resolution_idx = value->val1;
+	}
+
 	return 0;
 }
 
 static int apds9306_attr_get(const struct device *dev, enum sensor_channel channel,
 			     enum sensor_attribute attribute, struct sensor_value *value)
 {
-	uint8_t mask;
-	uint8_t temp;
-	uint8_t reg;
-	const struct apds9306_config *config = dev->config;
+	struct apds9306_data *data = dev->data;
 
-	if (channel != SENSOR_CHAN_LIGHT) {
+	if ((channel != SENSOR_CHAN_ALL) && (channel != SENSOR_CHAN_LIGHT)) {
 		return -ENOTSUP;
 	}
 
 	if (attribute == SENSOR_ATTR_SAMPLING_FREQUENCY) {
-		reg = APDS9306_REGISTER_ALS_MEAS_RATE;
-		mask = 0x00;
+		value->val1 = data->measurement_period_idx;
 	} else if (attribute == SENSOR_ATTR_GAIN) {
-		reg = APDS9306_REGISTER_ALS_GAIN;
-		mask = 0x00;
+		value->val1 = data->gain_idx;
 	} else if (attribute == SENSOR_ATTR_RESOLUTION) {
-		reg = APDS9306_REGISTER_ALS_MEAS_RATE;
-		mask = 0x04;
+		value->val1 = data->resolution_idx;
 	} else {
 		return -ENOTSUP;
 	}
 
-	if (i2c_reg_read_byte_dt(&config->i2c, reg, &temp)) {
-		LOG_ERR("Failed to read sensor attribute!");
-		return -EFAULT;
-	}
-
-	value->val1 = (temp >> mask) & 0x07;
-	value->val2 = 0;
-
 	return 0;
 }
 
 static int apds9306_sample_fetch(const struct device *dev, enum sensor_channel channel)
 {
-	uint8_t buffer;
-	uint8_t resolution;
 	uint16_t delay;
-	const struct apds9306_config *config = dev->config;
+	struct apds9306_data *data = dev->data;
 
 	if ((channel != SENSOR_CHAN_ALL) && (channel != SENSOR_CHAN_LIGHT)) {
 		return -ENOTSUP;
@@ -221,17 +263,10 @@ static int apds9306_sample_fetch(const struct device *dev, enum sensor_channel c
 		return -EFAULT;
 	}
 
-	/* Get the measurement resolution. */
-	if (i2c_reg_read_byte_dt(&config->i2c, APDS9306_REGISTER_ALS_MEAS_RATE, &buffer)) {
-		LOG_ERR("Failed reading resolution");
-		return -EFAULT;
-	}
-
 	/* Convert the resolution into a delay time and wait for the result. */
-	resolution = (buffer >> 4) & 0x07;
-	delay = apds9306_get_time_for_resolution(resolution);
-	LOG_DBG("Measurement resolution: %u", resolution);
-	LOG_DBG("Wait for %u ms", delay);
+	delay = avago_apds9306_integration_time[data->resolution_idx];
+	LOG_DBG("Measurement resolution index: %u", data->resolution_idx);
+	LOG_DBG("Wait for %d ms", delay);
 
 	/* We add a bit more delay to cover the startup time etc. */
 	if (!k_work_delayable_is_pending(&apds9306_worker_item.dwork)) {
@@ -252,13 +287,15 @@ static int apds9306_channel_get(const struct device *dev, enum sensor_channel ch
 {
 	struct apds9306_data *data = dev->data;
 
-	if (channel != SENSOR_CHAN_LIGHT) {
+	switch (channel) {
+	case SENSOR_CHAN_LIGHT:
+		value->val1 = data->light;
+		value->val2 = 0;
+		break;
+	default:
 		return -ENOTSUP;
 	}
 
-	value->val1 = data->light;
-	value->val2 = 0;
-
 	return 0;
 }
 
@@ -267,6 +304,7 @@ static int apds9306_sensor_setup(const struct device *dev)
 	uint32_t now;
 	uint8_t temp;
 	const struct apds9306_config *config = dev->config;
+	struct apds9306_data *data = dev->data;
 
 	/* Wait for the device to become ready after a possible power cycle. */
 	now = k_uptime_get_32();
@@ -285,19 +323,19 @@ static int apds9306_sensor_setup(const struct device *dev)
 		k_msleep(10);
 	} while (temp & APDS9306_BIT_POWER_ON_STATUS);
 
-	if (i2c_reg_read_byte_dt(&config->i2c, APDS9306_REGISTER_PART_ID, &temp)) {
+	if (i2c_reg_read_byte_dt(&config->i2c, APDS9306_REGISTER_PART_ID, &data->chip_id)) {
 		LOG_ERR("Failed reading chip id!");
 		return -EFAULT;
 	}
 
-	if ((temp != APDS_9306_CHIP_ID) && (temp != APDS_9306_065_CHIP_ID)) {
-		LOG_ERR("Invalid chip id! Found 0x%X!", temp);
+	if ((data->chip_id != APDS_9306_CHIP_ID) && (data->chip_id != APDS_9306_065_CHIP_ID)) {
+		LOG_ERR("Invalid chip id! Found 0x%X!", data->chip_id);
 		return -EFAULT;
 	}
 
-	if (temp == APDS_9306_CHIP_ID) {
+	if (data->chip_id == APDS_9306_CHIP_ID) {
 		LOG_DBG("APDS-9306 found!");
-	} else if (temp == APDS_9306_065_CHIP_ID) {
+	} else if (data->chip_id == APDS_9306_065_CHIP_ID) {
 		LOG_DBG("APDS-9306-065 found!");
 	}
 
@@ -316,6 +354,7 @@ static int apds9306_init(const struct device *dev)
 {
 	uint8_t value;
 	const struct apds9306_config *config = dev->config;
+	struct apds9306_data *data = dev->data;
 
 	LOG_DBG("Start to initialize APDS9306...");
 
@@ -329,16 +368,18 @@ static int apds9306_init(const struct device *dev)
 		return -EFAULT;
 	}
 
-	value = ((config->resolution & 0x07) << 4) | (config->frequency & 0x0F);
+	data->measurement_period_idx = config->measurement_period_idx;
+	data->resolution_idx = config->resolution_idx;
+	value = ((data->resolution_idx & 0x07) << 4) | (data->measurement_period_idx & 0x07);
 	LOG_DBG("Write configuration 0x%x to register 0x%x", value,
 		APDS9306_REGISTER_ALS_MEAS_RATE);
 	if (i2c_reg_write_byte_dt(&config->i2c, APDS9306_REGISTER_ALS_MEAS_RATE, value)) {
 		return -EFAULT;
 	}
 
-	value = config->gain;
+	data->gain_idx = config->gain_idx;
 	LOG_DBG("Write configuration 0x%x to register 0x%x", value, APDS9306_REGISTER_ALS_GAIN);
-	if (i2c_reg_write_byte_dt(&config->i2c, APDS9306_REGISTER_ALS_GAIN, value)) {
+	if (i2c_reg_write_byte_dt(&config->i2c, APDS9306_REGISTER_ALS_GAIN, data->gain_idx)) {
 		return -EFAULT;
 	}
 
@@ -358,9 +399,9 @@ static DEVICE_API(sensor, apds9306_driver_api) = {
 	static struct apds9306_data apds9306_data_##inst;                                          \
 	static const struct apds9306_config apds9306_config_##inst = {                             \
 		.i2c = I2C_DT_SPEC_INST_GET(inst),                                                 \
-		.resolution = DT_INST_PROP(inst, resolution),                                      \
-		.gain = DT_INST_PROP(inst, gain),                                                  \
-		.frequency = DT_INST_PROP(inst, frequency),                                        \
+		.resolution_idx = DT_INST_ENUM_IDX(inst, resolution),                              \
+		.gain_idx = DT_INST_ENUM_IDX(inst, gain),                                          \
+		.measurement_period_idx = DT_INST_ENUM_IDX(inst, measurement_period),              \
 	};                                                                                         \
                                                                                                    \
 	SENSOR_DEVICE_DT_INST_DEFINE(inst, apds9306_init, NULL, &apds9306_data_##inst,             \