Skip to contentSkip to conversation

iio: acpi: Add ACPI0008 ALS driver

PATCH In review Marek Vasut 3 participants 7 messages Jul 20, 2013 – Sep 30, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v5 1/1 iio: acpi: Add ACPI0008 ALS driver

From: Martin Liska <[email protected]>

Add basic implementation of the ACPI0008 Ambient Light Sensor driver. This driver currently supports only the ALI property, yet is ready to be easily extended to handle ALC, ALT, ALP ones as well.

---
 drivers/staging/iio/light/Kconfig    |   10 ++
 drivers/staging/iio/light/Makefile   |    1 +
 drivers/staging/iio/light/acpi-als.c |  326 ++++++++++++++++++++++++++++++++++
 3 files changed, 337 insertions(+)
 create mode 100644 drivers/staging/iio/light/acpi-als.c

V2: Fix the channel mask, so it's really reading RAW data.
V3: Put scan timestamp into the buffer only when enabled,
Set the light sensor ID to 0 instead of 1
V4: Select IIO_TRIGGERED_BUFFER as we need it here
V5: Use irq_work to trigger the buffer
Use module_acpi_driver()

drivers/staging/iio/light/Kconfig +10 -0
index ca8d6e6..0238a7f 100644
@@ -3,6 +3,16 @@
 #
 menu "Light sensors"
 
+config ACPI_ALS
+	tristate "ACPI Ambient Light Sensor"
+	depends on ACPI
+	select IIO_TRIGGERED_BUFFER
+	help
+	 Support for the ACPI0008 Ambient Light Sensor.
+
+	 This driver can also be built as a module.  If so, the module
+	 will be called acpi-als.
+
 config SENSORS_ISL29018
 	tristate "ISL 29018 light and proximity sensor"
 	depends on I2C
drivers/staging/iio/light/Makefile +1 -0
index 9960fdf..a3f68bc 100644
@@ -2,6 +2,7 @@
 # Makefile for industrial I/O Light sensors
 #
 
+obj-$(CONFIG_ACPI_ALS)	+= acpi-als.o
 obj-$(CONFIG_SENSORS_ISL29018)	+= isl29018.o
 obj-$(CONFIG_SENSORS_ISL29028)	+= isl29028.o
 obj-$(CONFIG_TSL2583)	+= tsl2583.o
drivers/staging/iio/light/acpi-als.c new file +326 -0

326 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#1 lore raw

Dear Marek Vasut,

Quoted 11 lines

From: Martin Liska <[email protected]>

Add basic implementation of the ACPI0008 Ambient Light Sensor driver. This driver currently supports only the ALI property, yet is ready to be easily extended to handle ALC, ALT, ALP ones as well.

---

[...]

Bump?

Best regards,
Marek Vasut

#2 lore raw
In reply to Marek Vasut
Quoted 6 lines · Marek Vasut

From: Martin Liska <[email protected]>

Add basic implementation of the ACPI0008 Ambient Light Sensor driver. This driver currently supports only the ALI property, yet is ready to be easily extended to handle ALC, ALT, ALP ones as well.

Quoted 8 lines
---
  drivers/staging/iio/light/Kconfig    |   10 ++
  drivers/staging/iio/light/Makefile   |    1 +
  drivers/staging/iio/light/acpi-als.c |  326 ++++++++++++++++++++++++++++++++++

New IIO drivers should go to drivers/iio not drivers/staging/iio

Quoted 49 lines
  3 files changed, 337 insertions(+)

create mode 100644 drivers/staging/iio/light/acpi-als.c

V2: Fix the channel mask, so it's really reading RAW data.
V3: Put scan timestamp into the buffer only when enabled,
Set the light sensor ID to 0 instead of 1
V4: Select IIO_TRIGGERED_BUFFER as we need it here
V5: Use irq_work to trigger the buffer
Use module_acpi_driver()

drivers/staging/iio/light/Kconfig +10 -0
index ca8d6e6..0238a7f 100644
@@ -3,6 +3,16 @@
  #
  menu "Light sensors"
 
+config ACPI_ALS
+	tristate "ACPI Ambient Light Sensor"
+	depends on ACPI
+	select IIO_TRIGGERED_BUFFER
+	help
+	 Support for the ACPI0008 Ambient Light Sensor.
+
+	 This driver can also be built as a module.  If so, the module
+	 will be called acpi-als.
+
  config SENSORS_ISL29018
  	tristate "ISL 29018 light and proximity sensor"
  	depends on I2C
drivers/staging/iio/light/Makefile +1 -0
index 9960fdf..a3f68bc 100644
@@ -2,6 +2,7 @@
  # Makefile for industrial I/O Light sensors
  #
 
+obj-$(CONFIG_ACPI_ALS)	+= acpi-als.o
  obj-$(CONFIG_SENSORS_ISL29018)	+= isl29018.o
  obj-$(CONFIG_SENSORS_ISL29028)	+= isl29028.o
  obj-$(CONFIG_TSL2583)	+= tsl2583.o
drivers/staging/iio/light/acpi-als.c new file +0 -0
new file mode 100644
index 0000000..f63427c
@@ -0,0 +1,326 @@

[...]

drivers/staging/iio/light/acpi-als.c line 106

Quoted 7 lines
drivers/staging/iio/light/acpi-als.c new file +7 -0
+static void acpi_als_notify(struct acpi_device *device, u32 event)
+{
+	struct iio_dev *iio = acpi_driver_data(device);
+	struct acpi_als *als = iio_priv(iio);
+	s64 time_ns = iio_get_time_ns();
+
+	mutex_lock(&als->lock);

Hm, so you lock the mutex here and unlock the mutex acpi_als_trigger_handler. This really needs some explanation. You also need to implement validate_trigger and validate_device callbacks to make sure that this trigger is only used with this device and vice versa.

Quoted 10 lines
drivers/staging/iio/light/acpi-als.c new file +10 -0
+
+	if (iio_buffer_enabled(iio)) {
+		als->evt_time = time_ns;
+		als->evt_event = event;
+		irq_work_queue(&als->work);
+	}
+}
+
+static int acpi_als_read_raw(struct iio_dev *iio,
+			struct iio_chan_spec const *chan, int *val,

drivers/staging/iio/light/acpi-als.c line 126

Quoted 10 lines
drivers/staging/iio/light/acpi-als.c new file +10 -0
+			int *val2, long mask)
+{
+	struct acpi_als *als = iio_priv(iio);
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	/* we support only illumination (_ALI) so far. */
+	if (chan->type != IIO_LIGHT)
+		return -EINVAL;

Since you only registered a IIO_LIGHT channel with a RAW property this function will never be called with anything else, so strictly speaking the checks above are unnecessary.

Quoted 4 lines
drivers/staging/iio/light/acpi-als.c new file +4 -0
+
+	*val = als_read_value(als, ACPI_ALS_ILLUMINANCE);
+	return IIO_VAL_INT;
+}

[..]

Quoted 12 lines
drivers/staging/iio/light/acpi-als.c new file +12 -0
+static int acpi_als_add(struct acpi_device *device)
+{
+	struct acpi_als *als;
+	struct iio_dev *iio;
+	struct device *dev = &device->dev;
+	int ret;
+
+	/*
+	 * The event buffer contains timestamp and all the data from
+	 * the ACPI0008 block. There are multiple, but so far we only
+	 * support _ALI (illuminance). Yes, we're ready for more!
+	 */

drivers/staging/iio/light/acpi-als.c line 244

Quoted 10 lines
drivers/staging/iio/light/acpi-als.c new file +10 -0
+	uint16_t *evt_buffer;
+	const unsigned int evt_sources = 1;
+	const unsigned int evt_buffer_size = sizeof(int64_t) +
+				(sizeof(uint16_t) * evt_sources);
+
+	evt_buffer = devm_kzalloc(dev, evt_buffer_size, GFP_KERNEL);
+	if (!evt_buffer)
+		return -ENOMEM;
+
+	iio = iio_device_alloc(sizeof(*als));

devm_...

[...]

maintainer
#3 lore raw
Quoted 68 lines · Lars-Peter Clausen
Quoted 6 lines · Marek Vasut

From: Martin Liska <[email protected]>

Add basic implementation of the ACPI0008 Ambient Light Sensor driver. This driver currently supports only the ALI property, yet is ready to be easily extended to handle ALC, ALT, ALP ones as well.

Quoted 8 lines
---
  drivers/staging/iio/light/Kconfig    |   10 ++
  drivers/staging/iio/light/Makefile   |    1 +
  drivers/staging/iio/light/acpi-als.c |  326 ++++++++++++++++++++++++++++++++++

New IIO drivers should go to drivers/iio not drivers/staging/iio

Quoted 49 lines
  3 files changed, 337 insertions(+)

create mode 100644 drivers/staging/iio/light/acpi-als.c

V2: Fix the channel mask, so it's really reading RAW data.
V3: Put scan timestamp into the buffer only when enabled,
Set the light sensor ID to 0 instead of 1
V4: Select IIO_TRIGGERED_BUFFER as we need it here
V5: Use irq_work to trigger the buffer
Use module_acpi_driver()

drivers/staging/iio/light/Kconfig +10 -0
index ca8d6e6..0238a7f 100644
@@ -3,6 +3,16 @@
  #
  menu "Light sensors"
 
+config ACPI_ALS
+    tristate "ACPI Ambient Light Sensor"
+    depends on ACPI
+    select IIO_TRIGGERED_BUFFER
+    help
+     Support for the ACPI0008 Ambient Light Sensor.
+
+     This driver can also be built as a module.  If so, the module
+     will be called acpi-als.
+
  config SENSORS_ISL29018
      tristate "ISL 29018 light and proximity sensor"
      depends on I2C
drivers/staging/iio/light/Makefile +1 -0
index 9960fdf..a3f68bc 100644
@@ -2,6 +2,7 @@
  # Makefile for industrial I/O Light sensors
  #
 
+obj-$(CONFIG_ACPI_ALS)    += acpi-als.o
  obj-$(CONFIG_SENSORS_ISL29018)    += isl29018.o
  obj-$(CONFIG_SENSORS_ISL29028)    += isl29028.o
  obj-$(CONFIG_TSL2583)    += tsl2583.o
drivers/staging/iio/light/acpi-als.c new file +0 -0
new file mode 100644
index 0000000..f63427c
@@ -0,0 +1,326 @@

[...]

Quoted 11 lines

drivers/staging/iio/light/acpi-als.c line 106

Quoted 7 lines
drivers/staging/iio/light/acpi-als.c new file +7 -0
+static void acpi_als_notify(struct acpi_device *device, u32 event)
+{
+    struct iio_dev *iio = acpi_driver_data(device);
+    struct acpi_als *als = iio_priv(iio);
+    s64 time_ns = iio_get_time_ns();
+
+    mutex_lock(&als->lock);

Hm, so you lock the mutex here and unlock the mutex acpi_als_trigger_handler. This really needs some explanation. You also need to implement validate_trigger and validate_device callbacks to make sure that this trigger is only used with this device and vice versa.

It may need some annotation as well to avoid various checks picking this up.

Quoted 46 lines
Quoted 11 lines
drivers/staging/iio/light/acpi-als.c new file +10 -0
+
+    if (iio_buffer_enabled(iio)) {
+        als->evt_time = time_ns;
+        als->evt_event = event;
+        irq_work_queue(&als->work);
+    }
+}
+
+static int acpi_als_read_raw(struct iio_dev *iio,
+            struct iio_chan_spec const *chan, int *val,

drivers/staging/iio/light/acpi-als.c line 126

Quoted 10 lines
drivers/staging/iio/light/acpi-als.c new file +10 -0
+            int *val2, long mask)
+{
+    struct acpi_als *als = iio_priv(iio);
+
+    if (mask != IIO_CHAN_INFO_RAW)
+        return -EINVAL;
+
+    /* we support only illumination (_ALI) so far. */
+    if (chan->type != IIO_LIGHT)
+        return -EINVAL;

Since you only registered a IIO_LIGHT channel with a RAW property this function will never be called with anything else, so strictly speaking the checks above are unnecessary.

Quoted 4 lines
drivers/staging/iio/light/acpi-als.c new file +4 -0
+
+    *val = als_read_value(als, ACPI_ALS_ILLUMINANCE);
+    return IIO_VAL_INT;
+}

[..]

Quoted 16 lines
drivers/staging/iio/light/acpi-als.c new file +16 -0
+static int acpi_als_add(struct acpi_device *device)
+{
+    struct acpi_als *als;
+    struct iio_dev *iio;
+    struct device *dev = &device->dev;
+    int ret;
+
+    /*
+     * The event buffer contains timestamp and all the data from
+     * the ACPI0008 block. There are multiple, but so far we only
+     * support _ALI (illuminance). Yes, we're ready for more!
+     */
+    uint16_t *evt_buffer;
+    const unsigned int evt_sources = 1;
+    const unsigned int evt_buffer_size = sizeof(int64_t) +
+                (sizeof(uint16_t) * evt_sources);
Quoted 8 lines

drivers/staging/iio/light/acpi-als.c line 244

Quoted 6 lines
drivers/staging/iio/light/acpi-als.c new file +6 -0
+
+    evt_buffer = devm_kzalloc(dev, evt_buffer_size, GFP_KERNEL);
+    if (!evt_buffer)
+        return -ENOMEM;
+
+    iio = iio_device_alloc(sizeof(*als));

devm_...

Also for the trigger allocation.

Quoted 6 lines

[...]

Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#4 lore raw

Dear Jonathan Cameron,

[...]

Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
drivers/staging/iio/light/acpi-als.c new file +4 -0
+static void acpi_als_notify(struct acpi_device *device, u32 event)
+{
+    struct iio_dev *iio = acpi_driver_data(device);
+    struct acpi_als *als = iio_priv(iio);
Quoted 8 lines

drivers/staging/iio/light/acpi-als.c line 106

Quoted 3 lines
drivers/staging/iio/light/acpi-als.c new file +3 -0
+    s64 time_ns = iio_get_time_ns();
+
+    mutex_lock(&als->lock);

Hm, so you lock the mutex here and unlock the mutex acpi_als_trigger_handler. This really needs some explanation. You also need to implement validate_trigger and validate_device callbacks to make sure that this trigger is only used with this device and vice versa.

Quoted 2 lines

It may need some annotation as well to avoid various checks picking this up.

Do you have any particular one in mind?

[...]

Quoted 9 lines
Quoted 7 lines

drivers/staging/iio/light/acpi-als.c line 244

Quoted 5 lines
drivers/staging/iio/light/acpi-als.c new file +5 -0
+    evt_buffer = devm_kzalloc(dev, evt_buffer_size, GFP_KERNEL);
+    if (!evt_buffer)
+        return -ENOMEM;
+
+    iio = iio_device_alloc(sizeof(*als));

devm_...

Also for the trigger allocation.

I'm on 3.12.0-rc2 (next 20130927), don't see either of them existing.

Best regards,
Marek Vasut

#5 lore raw
In reply to Lars-Peter Clausen

Dear Lars-Peter Clausen,

Quoted 82 lines · Marek Vasut
Quoted 10 lines

From: Martin Liska <[email protected]>

Add basic implementation of the ACPI0008 Ambient Light Sensor driver. This driver currently supports only the ALI property, yet is ready to be easily extended to handle ALC, ALT, ALP ones as well.

Quoted 6 lines

---

  drivers/staging/iio/light/Kconfig    |   10 ++
  drivers/staging/iio/light/Makefile   |    1 +
  drivers/staging/iio/light/acpi-als.c |  326

++++++++++++++++++++++++++++++++++

New IIO drivers should go to drivers/iio not drivers/staging/iio

Quoted 59 lines
  3 files changed, 337 insertions(+)

create mode 100644 drivers/staging/iio/light/acpi-als.c

V2: Fix the channel mask, so it's really reading RAW data.
V3: Put scan timestamp into the buffer only when enabled,

Set the light sensor ID to 0 instead of 1

V4: Select IIO_TRIGGERED_BUFFER as we need it here
V5: Use irq_work to trigger the buffer

Use module_acpi_driver()

(quoted diff) +0 -0

b/drivers/staging/iio/light/Kconfig index ca8d6e6..0238a7f 100644

drivers/staging/iio/light/Kconfig +10 -0
@@ -3,6 +3,16 @@
 
  #
  menu "Light sensors"
 
+config ACPI_ALS
+	tristate "ACPI Ambient Light Sensor"
+	depends on ACPI
+	select IIO_TRIGGERED_BUFFER
+	help
+	 Support for the ACPI0008 Ambient Light Sensor.
+
+	 This driver can also be built as a module.  If so, the module
+	 will be called acpi-als.
+
 
  config SENSORS_ISL29018

tristate "ISL 29018 light and proximity sensor" depends on I2C

(quoted diff) +0 -0

b/drivers/staging/iio/light/Makefile index 9960fdf..a3f68bc 100644

drivers/staging/iio/light/Makefile +1 -0
@@ -2,6 +2,7 @@
 
  # Makefile for industrial I/O Light sensors
  #
 
+obj-$(CONFIG_ACPI_ALS)	+= acpi-als.o
 
  obj-$(CONFIG_SENSORS_ISL29018)	+= isl29018.o
  obj-$(CONFIG_SENSORS_ISL29028)	+= isl29028.o
  obj-$(CONFIG_TSL2583)	+= tsl2583.o
(quoted diff) +0 -0

b/drivers/staging/iio/light/acpi-als.c new file mode 100644 index 0000000..f63427c

drivers/staging/iio/light/acpi-als.c new file +0 -0
@@ -0,0 +1,326 @@

[...]

Quoted 12 lines

drivers/staging/iio/light/acpi-als.c line 106

Quoted 7 lines
drivers/staging/iio/light/acpi-als.c new file +7 -0
+static void acpi_als_notify(struct acpi_device *device, u32 event)
+{
+	struct iio_dev *iio = acpi_driver_data(device);
+	struct acpi_als *als = iio_priv(iio);
+	s64 time_ns = iio_get_time_ns();
+
+	mutex_lock(&als->lock);

Hm, so you lock the mutex here and unlock the mutex acpi_als_trigger_handler. This really needs some explanation. You also need to implement validate_trigger and validate_device callbacks to make sure that this trigger is only used with this device and vice versa.

This is true. I trigger the IRQ handler below here, which in turn calls iio_trigger_poll() . This starts the trigger handler and upon it's completion, the mutex is unlocked.

The problem I just noticed here is that if the iio buffer is disabled, this will deadlock. Nice :-/

Quoted 16 lines
Quoted 16 lines
drivers/staging/iio/light/acpi-als.c new file +16 -0
+
+	if (iio_buffer_enabled(iio)) {
+		als->evt_time = time_ns;
+		als->evt_event = event;
+		irq_work_queue(&als->work);
+	}
+}
+
+static int acpi_als_read_raw(struct iio_dev *iio,
+			struct iio_chan_spec const *chan, int *val,
+			int *val2, long mask)
+{
+	struct acpi_als *als = iio_priv(iio);
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
Quoted 8 lines

drivers/staging/iio/light/acpi-als.c line 126

Quoted 4 lines
drivers/staging/iio/light/acpi-als.c new file +4 -0
+
+	/* we support only illumination (_ALI) so far. */
+	if (chan->type != IIO_LIGHT)
+		return -EINVAL;

Since you only registered a IIO_LIGHT channel with a RAW property this function will never be called with anything else, so strictly speaking the checks above are unnecessary.

The plan is to implement all of the ACPI 0008 spec , so the check will hopefully later morph to switch() {} statement for all the channels.

Quoted 32 lines
Quoted 4 lines
drivers/staging/iio/light/acpi-als.c new file +4 -0
+
+	*val = als_read_value(als, ACPI_ALS_ILLUMINANCE);
+	return IIO_VAL_INT;
+}

[..]

Quoted 12 lines
drivers/staging/iio/light/acpi-als.c new file +12 -0
+static int acpi_als_add(struct acpi_device *device)
+{
+	struct acpi_als *als;
+	struct iio_dev *iio;
+	struct device *dev = &device->dev;
+	int ret;
+
+	/*
+	 * The event buffer contains timestamp and all the data from
+	 * the ACPI0008 block. There are multiple, but so far we only
+	 * support _ALI (illuminance). Yes, we're ready for more!
+	 */

drivers/staging/iio/light/acpi-als.c line 244

Quoted 10 lines
drivers/staging/iio/light/acpi-als.c new file +10 -0
+	uint16_t *evt_buffer;
+	const unsigned int evt_sources = 1;
+	const unsigned int evt_buffer_size = sizeof(int64_t) +
+				(sizeof(uint16_t) * evt_sources);
+
+	evt_buffer = devm_kzalloc(dev, evt_buffer_size, GFP_KERNEL);
+	if (!evt_buffer)
+		return -ENOMEM;
+
+	iio = iio_device_alloc(sizeof(*als));

devm_...

Quoted 1 line

[...]

devm_iio_device_alloc()? I don't see this stuff in next/master anywhere. Where is this supposed to be?

maintainer
#6 lore raw
Quoted 174 lines · Marek Vasut

Dear Lars-Peter Clausen,

Quoted 4 lines · Marek Vasut
Quoted 3 lines

From: Martin Liska <[email protected]>

Add basic implementation of the ACPI0008 Ambient Light Sensor

driver.

Quoted 1 line
Quoted 1 line

This driver currently supports only the ALI property, yet is ready

to

Quoted 77 lines
Quoted 6 lines

be easily extended to handle ALC, ALT, ALP ones as well.

Quoted 6 lines

---

  drivers/staging/iio/light/Kconfig    |   10 ++
  drivers/staging/iio/light/Makefile   |    1 +
  drivers/staging/iio/light/acpi-als.c |  326

++++++++++++++++++++++++++++++++++

New IIO drivers should go to drivers/iio not drivers/staging/iio

Quoted 59 lines
  3 files changed, 337 insertions(+)

create mode 100644 drivers/staging/iio/light/acpi-als.c

V2: Fix the channel mask, so it's really reading RAW data.
V3: Put scan timestamp into the buffer only when enabled,

Set the light sensor ID to 0 instead of 1

V4: Select IIO_TRIGGERED_BUFFER as we need it here
V5: Use irq_work to trigger the buffer

Use module_acpi_driver()

(quoted diff) +0 -0

b/drivers/staging/iio/light/Kconfig index ca8d6e6..0238a7f 100644

drivers/staging/iio/light/Kconfig +10 -0
@@ -3,6 +3,16 @@
 
  #
  menu "Light sensors"
 
+config ACPI_ALS
+	tristate "ACPI Ambient Light Sensor"
+	depends on ACPI
+	select IIO_TRIGGERED_BUFFER
+	help
+	 Support for the ACPI0008 Ambient Light Sensor.
+
+	 This driver can also be built as a module.  If so, the module
+	 will be called acpi-als.
+
 
  config SENSORS_ISL29018

tristate "ISL 29018 light and proximity sensor" depends on I2C

(quoted diff) +0 -0

b/drivers/staging/iio/light/Makefile index 9960fdf..a3f68bc 100644

drivers/staging/iio/light/Makefile +1 -0
@@ -2,6 +2,7 @@
 
  # Makefile for industrial I/O Light sensors
  #
 
+obj-$(CONFIG_ACPI_ALS)	+= acpi-als.o
 
  obj-$(CONFIG_SENSORS_ISL29018)	+= isl29018.o
  obj-$(CONFIG_SENSORS_ISL29028)	+= isl29028.o
  obj-$(CONFIG_TSL2583)	+= tsl2583.o
(quoted diff) +0 -0

b/drivers/staging/iio/light/acpi-als.c new file mode 100644 index 0000000..f63427c

drivers/staging/iio/light/acpi-als.c new file +0 -0
@@ -0,0 +1,326 @@

[...]

Quoted 10 lines

drivers/staging/iio/light/acpi-als.c line 106

Quoted 7 lines
drivers/staging/iio/light/acpi-als.c new file +7 -0
+static void acpi_als_notify(struct acpi_device *device, u32 event)
+{
+	struct iio_dev *iio = acpi_driver_data(device);
+	struct acpi_als *als = iio_priv(iio);
+	s64 time_ns = iio_get_time_ns();
+
+	mutex_lock(&als->lock);

Hm, so you lock the mutex here and unlock the mutex acpi_als_trigger_handler. This really needs some explanation. You

also

Quoted 1 line

need to implement validate_trigger and validate_device callbacks to

make

Quoted 1 line

sure that this trigger is only used with this device and vice versa.

This is true. I trigger the IRQ handler below here, which in turn calls

iio_trigger_poll() . This starts the trigger handler and upon it's completion,
the mutex is unlocked.

The problem I just noticed here is that if the iio buffer is disabled, this will
deadlock. Nice :-/

Quoted 14 lines
Quoted 14 lines
drivers/staging/iio/light/acpi-als.c new file +14 -0
+
+	if (iio_buffer_enabled(iio)) {
+		als->evt_time = time_ns;
+		als->evt_event = event;
+		irq_work_queue(&als->work);
+	}
+}
+
+static int acpi_als_read_raw(struct iio_dev *iio,
+			struct iio_chan_spec const *chan, int *val,
+			int *val2, long mask)
+{
+	struct acpi_als *als = iio_priv(iio);
+
Quoted 8 lines

drivers/staging/iio/light/acpi-als.c line 126

Quoted 6 lines
drivers/staging/iio/light/acpi-als.c new file +6 -0
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	/* we support only illumination (_ALI) so far. */
+	if (chan->type != IIO_LIGHT)
+		return -EINVAL;

Since you only registered a IIO_LIGHT channel with a RAW property

this

Quoted 1 line

function will never be called with anything else, so strictly

speaking the

Quoted 1 line

checks above are unnecessary.

The plan is to implement all of the ACPI 0008 spec , so the check will hopefully
later morph to switch() {} statement for all the channels.

Quoted 32 lines
Quoted 4 lines
drivers/staging/iio/light/acpi-als.c new file +4 -0
+
+	*val = als_read_value(als, ACPI_ALS_ILLUMINANCE);
+	return IIO_VAL_INT;
+}

[..]

Quoted 12 lines
drivers/staging/iio/light/acpi-als.c new file +12 -0
+static int acpi_als_add(struct acpi_device *device)
+{
+	struct acpi_als *als;
+	struct iio_dev *iio;
+	struct device *dev = &device->dev;
+	int ret;
+
+	/*
+	 * The event buffer contains timestamp and all the data from
+	 * the ACPI0008 block. There are multiple, but so far we only
+	 * support _ALI (illuminance). Yes, we're ready for more!
+	 */

drivers/staging/iio/light/acpi-als.c line 244

Quoted 10 lines
drivers/staging/iio/light/acpi-als.c new file +10 -0
+	uint16_t *evt_buffer;
+	const unsigned int evt_sources = 1;
+	const unsigned int evt_buffer_size = sizeof(int64_t) +
+				(sizeof(uint16_t) * evt_sources);
+
+	evt_buffer = devm_kzalloc(dev, evt_buffer_size, GFP_KERNEL);
+	if (!evt_buffer)
+		return -ENOMEM;
+
+	iio = iio_device_alloc(sizeof(*als));

devm_...

Quoted 5 lines
Quoted 1 line

[...]

devm_iio_device_alloc()? I don't see this stuff in next/master anywhere. Where
is this supposed to be?

Strange. It is Linus' tree and has been since the last merge window.

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line
Sent from my Android phone with K-9 Mail. Please excuse my brevity.