On 25/2/24 01:43, Jonathan Cameron wrote:
On Sun, 18 Feb 2024 16:18:26 +1030
Subhajit Ghosh <subhajit.ghosh@xxxxxxxxxxxxxx> wrote:
Driver support for Avago (Broadcom) APDS9306 Ambient Light Sensor.
It has two channels - ALS and CLEAR. The ALS (Ambient Light Sensor)
channel approximates the response of the human-eye providing direct
read out where the output count is proportional to ambient light levels.
It is internally temperature compensated and rejects 50Hz and 60Hz flicker
caused by artificial light sources. Hardware interrupt configuration is
optional. It is a low power device with 20 bit resolution and has
configurable adaptive interrupt mode and interrupt persistence mode.
The device also features inbuilt hardware gain, multiple integration time
selection options and sampling frequency selection options.
This driver also uses the IIO GTS (Gain Time Scale) Helpers Namespace for
Scales, Gains and Integration time implementation.
Signed-off-by: Subhajit Ghosh <subhajit.ghosh@xxxxxxxxxxxxxx>
I applied this but then got some build warnings that made me look
more closely at the int_src handling.
This is confusing because of the less than helpful datasheet defintion
of a 2 bit register that takes values 0 and 1 only.
I thought about trying to fix this up whilst applying but the event code
issue is too significant to do without a means to test it.
Jonathan
+static int apds9306_read_data(struct apds9306_data *data, int *val, int reg)
+{
+ struct device *dev = data->dev;
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct apds9306_regfields *rf = &data->rf;
+ int ret, delay, intg_time, intg_time_idx, repeat_rate_idx, int_src;
+ int status = 0;
+ u8 buff[3];
+
+ ret = pm_runtime_resume_and_get(data->dev);
+ if (ret)
+ return ret;
+
+ ret = regmap_field_read(rf->intg_time, &intg_time_idx);
+ if (ret)
+ return ret;
+
+ ret = regmap_field_read(rf->repeat_rate, &repeat_rate_idx);
+ if (ret)
+ return ret;
+
+ ret = regmap_field_read(rf->int_src, &int_src);
+ if (ret)
+ return ret;
+
+ intg_time = iio_gts_find_int_time_by_sel(&data->gts, intg_time_idx);
+ if (intg_time < 0)
+ return intg_time;
+
+ /* Whichever is greater - integration time period or sampling period. */
+ delay = max(intg_time, apds9306_repeat_rate_period[repeat_rate_idx]);
+
+ /*
+ * Clear stale data flag that might have been set by the interrupt
+ * handler if it got data available flag set in the status reg.
+ */
+ data->read_data_available = 0;
+
+ /*
+ * If this function runs parallel with the interrupt handler, either
+ * this reads and clears the status registers or the interrupt handler
+ * does. The interrupt handler sets a flag for read data available
+ * in our private structure which we read here.
+ */
+ ret = regmap_read_poll_timeout(data->regmap, APDS9306_MAIN_STATUS_REG,
+ status, data->read_data_available ||
+ (status & (APDS9306_ALS_DATA_STAT_MASK |
+ APDS9306_ALS_INT_STAT_MASK)),
+ APDS9306_ALS_READ_DATA_DELAY_US, delay * 2);
+
+ if (ret)
+ return ret;
+
+ /* If we reach here before the interrupt handler we push an event */
+ if ((status & APDS9306_ALS_INT_STAT_MASK))
+ iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_LIGHT,
+ int_src, IIO_EV_TYPE_THRESH, IIO_EV_DIR_EITHER),
You are pushing an event on channel 0 or 1 (which is non obvious as that
int_src is a 2 bit value again). However you don't use indexed channels
so this is wrong.
It's also pushing IIO_LIGHT for both channels which makes no sense as you
only have one IIO_LIGHT channel.
Hi Jonathan,
For the above fix I am supplying the second parameter to IIO_UNMOD_EVENT_CODE()
as "0" which gives me the below output from userspace:
./iio_event_monitor /dev/iio:device0
Event: time: xx, type: illuminance, channel: 0, evtype: thresh, direction: either
Event: time: yy, type: intensity, channel: 0, evtype: thresh, direction: either
As I do not have indexed channels, I have used zero for both Light and Intensity
channel numbers. Should I make the intensity type as channel one for the output
to look like this?
Event: time: xx, type: illuminance, channel: 0, evtype: thresh, direction: either
Event: time: yy, type: intensity, channel: 1, evtype: thresh, direction: either
What do you think?
Regards,
Subhajit Ghosh
+ iio_get_time_ns(indio_dev));
+
+ ret = regmap_bulk_read(data->regmap, reg, buff, sizeof(buff));
+ if (ret) {
+ dev_err_ratelimited(dev, "read data failed\n");
+ return ret;
+ }
+
+ *val = get_unaligned_le24(&buff);
+
+ pm_runtime_mark_last_busy(data->dev);
+ pm_runtime_put_autosuspend(data->dev);
+
+ return 0;
+}
+
...
