On 08/31/2013 08:02 PM, Jonathan Cameron wrote:
On 08/28/13 16:00, Jacek Anaszewski wrote:
Add a new driver for the ambient light/proximity sensor
device. The driver exposes three channels: light_clear
light_ir and proximity. It also supports triggered buffer,
high and low ambient light threshold event and proximity
detection events.
Signed-off-by: Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx>
Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
---
This driver supports:
- reading channels in 'one shot' mode through read_raw callback,
- four events - rising and falling ambient light events and
rising and falling proximity roc events.
- triggers for all the three channels (triggers can't be enabled
simultaneosly with proximity detection event)
I'm a little confused by this statement. Can these 3 triggers fire at
different rates? The buffering etc assumes scans across all available
channels and that is indeed what you do in the trigger handler.
No, they fire at the same rate. The proximity detection event
can't be enabled in the same time as the triggers because there
are different interrupt modes dedicated to the proximity detection
event and the proximity threshold event which is exploited for the
triggers. In the proximity detection interrupt mode (PIN_PS_DETECT)
there is single interrupt generated upon crossing the threshold.
If the interrupts should be generated upon every measurement
cycle, the PIN_PS, PIN_ALS or PIN_ALS_OR_PS mode should be set.
In those modes the interrupts are being generated if the low or high
threshold is crossed. Therefore, the illuminance events can be enabled
simultaneously with triggers. Actually from the device point of view,
they are enabled all the time, when not in the PIN_PS_DETECT mode. The
interrupts are being generated when the output data falls below the low
threshold or rises above the high threshold. In specific case, when the
output value is 0 - no interrupt is being generated as it is impossible
to set the low threshold to the value less than 0.
When both triggers and illuminance events are enabled the
generic_buffers application displays new samples of data
only if the thresholds are crossed.
This is why the triggers and events are so tightly coupled
in the driver. I hope that I didn't confuse you even more
by this explanation :)
This is a follow-up of the previous series. The patch with
DT bindings documentation [1] for the driver has been already
acked [2]. This patch includes following improvements:
- split event_handler to two handlers, for als and proximity
events. It saves cpu resources as the proximity
event is signalized with both rising and falling edges,
whereas als events exploits only the falling one
That is truely hideous. Not that I can see a better way.
I was going to ask which combinations of events are possible then
noticed you lay it out clearly below! Even by normal standards
of odd hardware this is ugly.
I do have a few minor issues left in line. I very nearly
applied this as is, but given, unfortunately, it has taken me
too long to review it to get it in within the upcoming merge cycle
(which may open any time) we have plenty of time to clean these
little things up.
- the function gp2ap020a00f_adjust_lux_mode is now called
from one common place - gp2ap020a00f_als_event_handler
- brought back data_ready_queue, which cuts down the time
needed for obtaining measurement result when accessing
read_raw sysfs attribute
- made slight adjustements in the code formating (one line
exceeds 80 characters limit but I think it is acceptable
in that case)
Yup, not much that can be done sensibly about that one line so
it is fine.
[1] - http://www.spinics.net/lists/linux-iio/msg09768.html
[2] - http://www.spinics.net/lists/linux-iio/msg09772.html
Thanks,
Jacek Anaszewski
drivers/iio/light/Kconfig | 12 +
drivers/iio/light/Makefile | 1 +
drivers/iio/light/gp2ap020a00f.c | 1618 ++++++++++++++++++++++++++++++++++++++
3 files changed, 1631 insertions(+)
create mode 100644 drivers/iio/light/gp2ap020a00f.c
<snip>
+/*
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx>
+ *
+ * IIO features supported by the driver:
+ *
+ * Read-only raw channels:
+ * - illiminance_clear [lux]
+ * - illiminance_ir
+ * - proximity
+ *
+ * Triggered buffer:
+ * - illiminance_clear
+ * - illiminance_ir
+ * - proximity
+ *
+ * Events:
+ * - illuminance_clear (rising and falling)
+ * - proximity (rising and falling)
+ * - both falling and rising thresholds for the proximity events
+ * must be set to the values greater than 0.
+ *
+ * The driver supports triggered buffers for all the three
+ * channels as well as high and low threshold events for the
+ * illuminance_clear and proxmimity channels. Triggers
+ * can be enabled simultaneously with both illuminance_clear
+ * events. Proximity events cannot be enabled simultaneously
+ * with any triggers or illuminance events. Enabling/disabling
+ * one of the proximity events automatically enables/disables
+ * the other one.
Good description that clearly answered my main remaining question!
+ *
<snip>
+static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
+ enum gp2ap020a00f_cmd cmd)
+{
+ int err = 0;
+
+ switch (cmd) {
+ case GP2AP020A00F_CMD_READ_RAW_CLEAR:
+ if (data->cur_opmode != GP2AP020A00F_OPMODE_SHUTDOWN)
+ return -EBUSY;
+ err = gp2ap020a00f_set_operation_mode(data,
+ GP2AP020A00F_OPMODE_READ_RAW_CLEAR);
+ break;
+ case GP2AP020A00F_CMD_READ_RAW_IR:
+ if (data->cur_opmode != GP2AP020A00F_OPMODE_SHUTDOWN)
+ return -EBUSY;
+ err = gp2ap020a00f_set_operation_mode(data,
+ GP2AP020A00F_OPMODE_READ_RAW_IR);
+ break;
+ case GP2AP020A00F_CMD_READ_RAW_PROXIMITY:
+ if (data->cur_opmode != GP2AP020A00F_OPMODE_SHUTDOWN)
+ return -EBUSY;
+ err = gp2ap020a00f_set_operation_mode(data,
+ GP2AP020A00F_OPMODE_READ_RAW_PROXIMITY);
+ break;
+ case GP2AP020A00F_CMD_TRIGGER_CLEAR_EN:
+ if (data->cur_opmode == GP2AP020A00F_OPMODE_PROX_DETECT)
+ return -EBUSY;
+ if (!gp2ap020a00f_als_enabled(data))
+ err = gp2ap020a00f_alter_opmode(data,
+ GP2AP020A00F_OPMODE_ALS,
+ GP2AP020A00F_ADD_MODE);
+ set_bit(GP2AP020A00F_FLAG_ALS_CLEAR_TRIGGER, &data->flags);
+ break;
+ case GP2AP020A00F_CMD_TRIGGER_CLEAR_DIS:
+ clear_bit(GP2AP020A00F_FLAG_ALS_CLEAR_TRIGGER, &data->flags);
+ if (gp2ap020a00f_als_enabled(data))
+ break;
+ err = gp2ap020a00f_alter_opmode(data,
+ GP2AP020A00F_OPMODE_ALS,
+ GP2AP020A00F_SUBTRACT_MODE);
+ break;
+ case GP2AP020A00F_CMD_TRIGGER_IR_EN:
+ if (data->cur_opmode == GP2AP020A00F_OPMODE_PROX_DETECT)
+ return -EBUSY;
+ if (!gp2ap020a00f_als_enabled(data))
+ err = gp2ap020a00f_alter_opmode(data,
+ GP2AP020A00F_OPMODE_ALS,
+ GP2AP020A00F_ADD_MODE);
+ set_bit(GP2AP020A00F_FLAG_ALS_IR_TRIGGER, &data->flags);
+ break;
+ case GP2AP020A00F_CMD_TRIGGER_IR_DIS:
+ clear_bit(GP2AP020A00F_FLAG_ALS_IR_TRIGGER, &data->flags);
+ if (gp2ap020a00f_als_enabled(data))
+ break;
+ err = gp2ap020a00f_alter_opmode(data,
+ GP2AP020A00F_OPMODE_ALS,
+ GP2AP020A00F_SUBTRACT_MODE);
+ break;
+ case GP2AP020A00F_CMD_TRIGGER_PROX_EN:
+ if (data->cur_opmode == GP2AP020A00F_OPMODE_PROX_DETECT)
+ return -EBUSY;
+ err = gp2ap020a00f_alter_opmode(data,
+ GP2AP020A00F_OPMODE_PS,
+ GP2AP020A00F_ADD_MODE);
+ set_bit(GP2AP020A00F_FLAG_PROX_TRIGGER, &data->flags);
+ break;
+ case GP2AP020A00F_CMD_TRIGGER_PROX_DIS:
+ clear_bit(GP2AP020A00F_FLAG_PROX_TRIGGER, &data->flags);
+ err = gp2ap020a00f_alter_opmode(data,
+ GP2AP020A00F_OPMODE_PS,
+ GP2AP020A00F_SUBTRACT_MODE);
+ break;
+ case GP2AP020A00F_CMD_ALS_HIGH_EV_EN:
+ if (test_bit(GP2AP020A00F_FLAG_ALS_RISING_EVENT,
+ &data->flags) ||
+ data->cur_opmode == GP2AP020A00F_OPMODE_PROX_DETECT)
+ return -EBUSY;
EBUSY is fine for the case of proximity being enabled, but for the case
where the event is already on we'd normally just expect to return 0
rather than an error at all. Of course you wouldn't expect userspace to
do this normally but it still isn't really an error.
You do this quite a few times in this function.
OK, I will fix that.
+ if (!gp2ap020a00f_als_enabled(data)) {
+ err = gp2ap020a00f_alter_opmode(data,
+ GP2AP020A00F_OPMODE_ALS,
+ GP2AP020A00F_ADD_MODE);
+ if (err < 0)
+ return err;
+ }
+ set_bit(GP2AP020A00F_FLAG_ALS_RISING_EVENT, &data->flags);
+ err = gp2ap020a00f_write_event_threshold(data,
+ GP2AP020A00F_THRESH_TH, true);
+ break;
+ case GP2AP020A00F_CMD_ALS_HIGH_EV_DIS:
+ if (!test_bit(GP2AP020A00F_FLAG_ALS_RISING_EVENT, &data->flags))
+ return -EBUSY;
+ clear_bit(GP2AP020A00F_FLAG_ALS_RISING_EVENT, &data->flags);
+ if (!gp2ap020a00f_als_enabled(data)) {
+ err = gp2ap020a00f_alter_opmode(data,
+ GP2AP020A00F_OPMODE_ALS,
+ GP2AP020A00F_SUBTRACT_MODE);
+ if (err < 0)
+ return err;
+ }
+ err = gp2ap020a00f_write_event_threshold(data,
+ GP2AP020A00F_THRESH_TH, false);
+ break;
+ case GP2AP020A00F_CMD_ALS_LOW_EV_EN:
+ if (test_bit(GP2AP020A00F_FLAG_ALS_FALLING_EVENT,
+ &data->flags) ||
+ data->cur_opmode == GP2AP020A00F_OPMODE_PROX_DETECT)
+ return -EBUSY;
+ if (!gp2ap020a00f_als_enabled(data)) {
+ err = gp2ap020a00f_alter_opmode(data,
+ GP2AP020A00F_OPMODE_ALS,
+ GP2AP020A00F_ADD_MODE);
+ if (err < 0)
+ return err;
+ }
+ set_bit(GP2AP020A00F_FLAG_ALS_FALLING_EVENT, &data->flags);
+ err = gp2ap020a00f_write_event_threshold(data,
+ GP2AP020A00F_THRESH_TL, true);
+ break;
+ case GP2AP020A00F_CMD_ALS_LOW_EV_DIS:
+ if (!test_bit(GP2AP020A00F_FLAG_ALS_FALLING_EVENT,
+ &data->flags))
+ return -EBUSY;
+ clear_bit(GP2AP020A00F_FLAG_ALS_FALLING_EVENT, &data->flags);
+ if (!gp2ap020a00f_als_enabled(data)) {
+ err = gp2ap020a00f_alter_opmode(data,
+ GP2AP020A00F_OPMODE_ALS,
+ GP2AP020A00F_SUBTRACT_MODE);
+ if (err < 0)
+ return err;
+ }
+ err = gp2ap020a00f_write_event_threshold(data,
+ GP2AP020A00F_THRESH_TL, false);
+ break;
+ case GP2AP020A00F_CMD_PROX_HIGH_EV_EN:
+ if (test_bit(GP2AP020A00F_FLAG_PROX_RISING_EVENT,
+ &data->flags) ||
+ gp2ap020a00f_als_enabled(data) ||
+ data->cur_opmode == GP2AP020A00F_OPMODE_PS)
+ return -EBUSY;
+ if (!gp2ap020a00f_prox_detect_enabled(data)) {
+ err = gp2ap020a00f_set_operation_mode(data,
+ GP2AP020A00F_OPMODE_PROX_DETECT);
+ if (err < 0)
+ return err;
+ }
+ set_bit(GP2AP020A00F_FLAG_PROX_RISING_EVENT, &data->flags);
+ err = gp2ap020a00f_write_event_threshold(data,
+ GP2AP020A00F_THRESH_PH, true);
+ break;
+ case GP2AP020A00F_CMD_PROX_HIGH_EV_DIS:
+ if (!test_bit(GP2AP020A00F_FLAG_PROX_RISING_EVENT,
+ &data->flags))
+ return -EBUSY;
+ clear_bit(GP2AP020A00F_FLAG_PROX_RISING_EVENT, &data->flags);
+ err = gp2ap020a00f_set_operation_mode(data,
+ GP2AP020A00F_OPMODE_SHUTDOWN);
+ if (err < 0)
+ return err;
+ err = gp2ap020a00f_write_event_threshold(data,
+ GP2AP020A00F_THRESH_PH, false);
+ break;
+ case GP2AP020A00F_CMD_PROX_LOW_EV_EN:
+ if (test_bit(GP2AP020A00F_FLAG_PROX_FALLING_EVENT,
+ &data->flags) ||
+ gp2ap020a00f_als_enabled(data) ||
+ data->cur_opmode == GP2AP020A00F_OPMODE_PS)
+ return -EBUSY;
+ if (!gp2ap020a00f_prox_detect_enabled(data)) {
+ err = gp2ap020a00f_set_operation_mode(data,
+ GP2AP020A00F_OPMODE_PROX_DETECT);
+ if (err < 0)
+ return err;
+ }
+ set_bit(GP2AP020A00F_FLAG_PROX_FALLING_EVENT, &data->flags);
+ err = gp2ap020a00f_write_event_threshold(data,
+ GP2AP020A00F_THRESH_PL, true);
+ break;
+ case GP2AP020A00F_CMD_PROX_LOW_EV_DIS:
+ if (!test_bit(GP2AP020A00F_FLAG_PROX_FALLING_EVENT,
+ &data->flags))
+ return -EBUSY;
+ clear_bit(GP2AP020A00F_FLAG_PROX_FALLING_EVENT, &data->flags);
+ err = gp2ap020a00f_set_operation_mode(data,
+ GP2AP020A00F_OPMODE_SHUTDOWN);
+ if (err < 0)
+ return err;
+ err = gp2ap020a00f_write_event_threshold(data,
+ GP2AP020A00F_THRESH_PL, false);
+ break;
+ }
+
+ return err;
+}
+
+static irqreturn_t gp2ap020a00f_als_event_handler(int irq, void *data)
+{
+ struct iio_dev *indio_dev = data;
+ struct gp2ap020a00f_data *priv = iio_priv(indio_dev);
+ u8 op_reg_flags, d0_reg_buf[2];
+ unsigned int output_val, op_reg_val;
+ int thresh_val_id, ret;
+
+ mutex_lock(&priv->lock);
+
+ /* Read interrupt flags */
+ ret = regmap_read(priv->regmap, GP2AP020A00F_OP_REG,
+ &op_reg_val);
+ if (ret < 0)
+ goto unlock;
+
+ op_reg_flags = op_reg_val & (GP2AP020A00F_FLAG_A | GP2AP020A00F_FLAG_P
+ | GP2AP020A00F_PROX_DETECT);
+
+ op_reg_val &= (~GP2AP020A00F_FLAG_A & ~GP2AP020A00F_FLAG_P
+ & ~GP2AP020A00F_PROX_DETECT);
+
+ /* Clear interrupt flags (if not in INTTYPE_PULSE mode) */
+ if (priv->cur_opmode != GP2AP020A00F_OPMODE_PROX_DETECT) {
+ ret = regmap_write(priv->regmap, GP2AP020A00F_OP_REG,
+ op_reg_val);
+ if (ret < 0)
+ goto unlock;
+ }
+
+ if (op_reg_flags & GP2AP020A00F_FLAG_A) {
+ /* Check D0 register to assess if the lux mode
+ * transition is required.
+ */
+ ret = regmap_bulk_read(priv->regmap, GP2AP020A00F_D0_L_REG,
+ d0_reg_buf, 2);
+ if (ret < 0)
+ goto unlock;
+
+ output_val = le16_to_cpup((__le16 *)d0_reg_buf);
+
+ if (gp2ap020a00f_adjust_lux_mode(priv, output_val))
+ goto unlock;
+
+ gp2ap020a00f_output_to_lux(priv, &output_val);
+
+ /*
+ * We need to check output value to distinguish
+ * between high and low ambient light threshold event.
+ */
+ if (test_bit(GP2AP020A00F_FLAG_ALS_RISING_EVENT,
+ &priv->flags)) {
+ thresh_val_id =
+ GP2AP020A00F_THRESH_VAL_ID(GP2AP020A00F_TH_L_REG);
+ if (output_val > priv->thresh_val[thresh_val_id])
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(
+ IIO_LIGHT,
+ GP2AP020A00F_SCAN_MODE_LIGHT_CLEAR,
+ IIO_MOD_LIGHT_CLEAR,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_RISING),
+ iio_get_time_ns());
+ }
+
+ if (test_bit(GP2AP020A00F_FLAG_ALS_FALLING_EVENT,
+ &priv->flags)) {
+ thresh_val_id =
+ GP2AP020A00F_THRESH_VAL_ID(GP2AP020A00F_TL_L_REG);
+ if (output_val < priv->thresh_val[thresh_val_id])
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(
+ IIO_LIGHT,
+ GP2AP020A00F_SCAN_MODE_LIGHT_CLEAR,
+ IIO_MOD_LIGHT_CLEAR,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_FALLING),
+ iio_get_time_ns());
+ }
+ }
+
+ if (priv->cur_opmode == GP2AP020A00F_OPMODE_READ_RAW_CLEAR ||
+ priv->cur_opmode == GP2AP020A00F_OPMODE_READ_RAW_IR ||
+ priv->cur_opmode == GP2AP020A00F_OPMODE_READ_RAW_PROXIMITY) {
+ set_bit(GP2AP020A00F_FLAG_DATA_READY, &priv->flags);
+ wake_up(&priv->data_ready_queue);
+ goto unlock;
+ }
+
+ /* This fires off trigger handler. */
That comment is missleading. It fires off the trigger, which in turn
'may' call the trigger_handler.
You might want to only call this iff the trigger is enabled though...
+ irq_work_queue(&priv->work);
+unlock:
+ mutex_unlock(&priv->lock);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t gp2ap020a00f_trigger_handler(int irq, void *data)
+{
+ struct iio_poll_func *pf = data;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct gp2ap020a00f_data *lgt = iio_priv(indio_dev);
+ size_t d_size = 0;
+ __le32 light_lux;
+ int i, out_val, ret;
+
+ for_each_set_bit(i, indio_dev->active_scan_mask,
+ indio_dev->masklength) {
+ ret = regmap_bulk_read(lgt->regmap,
+ GP2AP020A00F_DATA_REG(i),
+ &lgt->buffer[d_size], 2);
+ if (ret < 0)
+ goto done;
+
+ if (i == GP2AP020A00F_SCAN_MODE_LIGHT_CLEAR ||
+ i == GP2AP020A00F_SCAN_MODE_LIGHT_IR) {
+ out_val = le16_to_cpup((__le16 *)&lgt->buffer[d_size]);
+ gp2ap020a00f_output_to_lux(lgt, &out_val);
+ light_lux = cpu_to_le32(out_val);
+ memcpy(&lgt->buffer[d_size], (u8 *)&light_lux, 4);
+ d_size += 4;
+ } else {
+ d_size += 2;
+ }
+ }
+
+ if (indio_dev->scan_timestamp) {
+ s64 *timestamp = (s64 *)((u8 *)lgt->buffer +
+ ALIGN(d_size, sizeof(s64)));
+ *timestamp = pf->timestamp;
+ }
+
+ iio_push_to_buffers(indio_dev, lgt->buffer);
+done:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
<snip>
+static int gp2ap020a00f_write_prox_event_config(struct iio_dev *indio_dev,
+ u64 event_code, int state)
+{
+ struct gp2ap020a00f_data *data = iio_priv(indio_dev);
+ enum gp2ap020a00f_cmd cmd_high_ev, cmd_low_ev;
+ int err;
+
+ cmd_high_ev = state ? GP2AP020A00F_CMD_PROX_HIGH_EV_EN :
+ GP2AP020A00F_CMD_PROX_HIGH_EV_DIS;
+ cmd_low_ev = state ? GP2AP020A00F_CMD_PROX_LOW_EV_EN :
+ GP2AP020A00F_CMD_PROX_LOW_EV_DIS;
+
+ /*
+ * In order to enable proximity detection feature in the device
+ * both high and low threshold registers have to be written
+ * with different values, greater than zero.
+ */
+ if (state) {
+ if (data->thresh_val[GP2AP020A00F_THRESH_PL] == 0)
+ return -EINVAL;
+
+ if (data->thresh_val[GP2AP020A00F_THRESH_PH] == 0)
+ return -EINVAL;
+ }
+
+ err = gp2ap020a00f_exec_cmd(data, cmd_high_ev);
+ if (err < 0)
+ return err;
+
+ err = gp2ap020a00f_exec_cmd(data, cmd_low_ev);
+ if (err < 0)
+ return err;
+
+ free_irq(data->client->irq, indio_dev);
+
Ouch, this is hideous. Not your fault, just uggly hardware.
+ if (state)
+ err = request_threaded_irq(data->client->irq, NULL,
+ &gp2ap020a00f_prox_event_handler,
+ IRQF_TRIGGER_RISING |
+ IRQF_TRIGGER_FALLING |
+ IRQF_ONESHOT,
+ "gp2ap020a00f_prox_event",
+ indio_dev);
+ else {
+ err = request_threaded_irq(data->client->irq, NULL,
+ &gp2ap020a00f_als_event_handler,
+ IRQF_TRIGGER_FALLING |
+ IRQF_ONESHOT,
+ "gp2ap020a00f_als_event",
+ indio_dev);
+ }
+
+ return err;
+}
+
<snip>
+static int gp2ap020a00f_read_channel(struct gp2ap020a00f_data *data,
+ struct iio_chan_spec const *chan, int *val)
+{
To supress a gcc warning I'm seeing set cmd to something pr add a default
with error return to the switch statement.
+ enum gp2ap020a00f_cmd cmd;
+ int err;
+
+ switch (chan->scan_index) {
+ case GP2AP020A00F_SCAN_MODE_LIGHT_CLEAR:
+ cmd = GP2AP020A00F_CMD_READ_RAW_CLEAR;
+ break;
+ case GP2AP020A00F_SCAN_MODE_LIGHT_IR:
+ cmd = GP2AP020A00F_CMD_READ_RAW_IR;
+ break;
+ case GP2AP020A00F_SCAN_MODE_PROXIMITY:
+ cmd = GP2AP020A00F_CMD_READ_RAW_PROXIMITY;
+ break;
+ }
+
+ err = gp2ap020a00f_exec_cmd(data, cmd);
+ if (err < 0) {
+ dev_err(&data->client->dev,
+ "gp2ap020a00f_exec_cmd failed\n");
+ goto error_ret;
+ }
+
+ err = gp2ap020a00f_read_output(data, chan->address, val);
+ if (err < 0)
+ dev_err(&data->client->dev,
+ "gp2ap020a00f_read_output failed\n");
+
+ data->cur_opmode = GP2AP020A00F_OPMODE_SHUTDOWN;
+
+ if (cmd == GP2AP020A00F_CMD_READ_RAW_CLEAR ||
+ cmd == GP2AP020A00F_CMD_READ_RAW_IR)
+ gp2ap020a00f_output_to_lux(data, val);
+
+error_ret:
+ return err;
+}
+
+static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct gp2ap020a00f_data *data = iio_priv(indio_dev);
+ int i, err = 0;
+
+ mutex_lock(&data->lock);
+
This confuses me a little. Do we actually have 3 different
sources of interrupts or is it just the case that any of these
being enabled results in a single interrupt once all three are
available?
Besides PIN_PS_DETECT interrupt mode that I described above there
are three more: PIN_ALS, PIN_PS and PIN_ALS_OR_PS. The first two
enable ALS and proximity interrupts respectively and the third one
enables both. The device has also corresponding operation modes -
OP_ALS, OP_PS and OP_ALS_AND_PS, which enable only ALS block,
only PS block or both blocks in the device respectively.
The output data for both illuminance channels is generated
at one run, by ALS block. If one of them isn't enabled in the
scan config it is simply ignored by the driver. The proximity
output data is produced only when the PS block is enabled.
The exec_cmd functions checks what operation mode is currently
enabled and sets the device in the OP_ALS_AND_PS mode if the
scan mode corresponding to the one of the modes is already enabled.
+ /* Enable triggers according to the scan_mask */
+ for_each_set_bit(i, indio_dev->active_scan_mask,
+ indio_dev->masklength) {
+ switch (i) {
+ case GP2AP020A00F_SCAN_MODE_LIGHT_CLEAR:
+ err = gp2ap020a00f_exec_cmd(data,
+ GP2AP020A00F_CMD_TRIGGER_CLEAR_EN);
+ break;
+ case GP2AP020A00F_SCAN_MODE_LIGHT_IR:
+ err = gp2ap020a00f_exec_cmd(data,
+ GP2AP020A00F_CMD_TRIGGER_IR_EN);
+ break;
+ case GP2AP020A00F_SCAN_MODE_PROXIMITY:
+ err = gp2ap020a00f_exec_cmd(data,
+ GP2AP020A00F_CMD_TRIGGER_PROX_EN);
+ break;
+ }
+ }
+
+ if (err < 0)
+ goto error_unlock;
+
+ data->buffer = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
+ if (!data->buffer) {
+ err = -ENOMEM;
+ goto error_unlock;
+ }
+
+ err = iio_triggered_buffer_postenable(indio_dev);
+
+error_unlock:
+ mutex_unlock(&data->lock);
+
+ return err;
+}
+
<snip>
+static int gp2ap020a00f_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct gp2ap020a00f_data *data;
+ struct iio_dev *indio_dev;
+ struct regmap *regmap;
+ int err;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+
+ data->vled_reg = devm_regulator_get(&client->dev, "vled");
+ if (IS_ERR(data->vled_reg))
+ return PTR_ERR(data->vled_reg);
+
+ err = regulator_enable(data->vled_reg);
+ if (err)
+ return err;
+
+ regmap = devm_regmap_init_i2c(client, &gp2ap020a00f_regmap_config);
+ if (IS_ERR(regmap)) {
+ dev_err(&client->dev, "Regmap initialization failed.\n");
+ err = PTR_ERR(regmap);
+ goto error_regulator_disable;
+ }
+
+ /* Initialize device registers */
+ err = regmap_bulk_write(regmap, GP2AP020A00F_OP_REG,
+ gp2ap020a00f_reg_init_tab,
+ ARRAY_SIZE(gp2ap020a00f_reg_init_tab));
+
+ if (err < 0) {
+ dev_err(&client->dev, "Device initialization failed.\n");
+ goto error_regulator_disable;
+ }
+
+ i2c_set_clientdata(client, indio_dev);
+
+ data->client = client;
+ data->cur_opmode = GP2AP020A00F_OPMODE_SHUTDOWN;
+ data->regmap = regmap;
+ init_waitqueue_head(&data->data_ready_queue);
+
+ mutex_init(&data->lock);
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->channels = gp2ap020a00f_channels;
+ indio_dev->num_channels = ARRAY_SIZE(gp2ap020a00f_channels);
+ indio_dev->info = &gp2ap020a00f_info;
+ indio_dev->name = id->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ /* Allocate buffer */
+ err = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time,
+ &gp2ap020a00f_trigger_handler, &gp2ap020a00f_buffer_setup_ops);
+ if (err < 0)
+ goto error_regulator_disable;
+
+ /* Allocate trigger */
+ data->trig = devm_iio_trigger_alloc(&client->dev, "%s-trigger",
+ indio_dev->name);
+ if (data->trig == NULL) {
+ err = -ENOMEM;
+ dev_err(&indio_dev->dev, "Failed to allocate iio trigger.\n");
+ goto error_uninit_buffer;
+ }
+
Perhaps a comment here to explain that this needs to be enabled for read_raw
calls to work? (took me a minute to figure out why you requested it here).
I will remove this in the next patch as the interrupt handler can't
execute simultaneously with read_output function because of mutex lock.
Polling read_raw attribute currently works only because of timeout on
the data_ready_queue. I recklessly brought back that solution.
I am going to implement polling an interrupt flag in the read_channel
function and bring back lux mode adjustment there.
+ err = request_threaded_irq(client->irq, NULL,
+ &gp2ap020a00f_als_event_handler,
+ IRQF_TRIGGER_FALLING |
+ IRQF_ONESHOT,
+ "gp2ap020a00f_als_event",
+ indio_dev);
+ if (err < 0) {
+ dev_err(&client->dev, "Irq request failed.\n");
+ goto error_uninit_buffer;
+ }
+
+ data->trig->ops = &gp2ap020a00f_trigger_ops;
+ data->trig->dev.parent = &data->client->dev;
+
+ init_irq_work(&data->work, gp2ap020a00f_iio_trigger_work);
+
+ err = iio_trigger_register(data->trig);
+ if (err < 0) {
+ dev_err(&client->dev, "Failed to register iio trigger.\n");
+ goto error_free_irq;
+ }
+
+ err = iio_device_register(indio_dev);
+ if (err < 0)
+ goto error_trigger_unregister;
+
+ return 0;
+
+error_trigger_unregister:
+ iio_trigger_unregister(data->trig);
+error_free_irq:
+ free_irq(client->irq, indio_dev);
+error_uninit_buffer:
+ iio_triggered_buffer_cleanup(indio_dev);
+error_regulator_disable:
+ regulator_disable(data->vled_reg);
+
+ return err;
+}
+
Thanks,
Jacek
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
