Re: [PATCH V5] iio: acpi: Add ACPI0008 ALS driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/21/2013 01:58 AM, Marek Vasut wrote:
From: Martin Liska <marxin.liska@xxxxxxxxx>

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.

Signed-off-by: Martin Liska <marxin.liska@xxxxxxxxx>
Signed-off-by: Marek Vasut <marex@xxxxxxx>
Cc: Jonathan Cameron <jic23@xxxxxxxxxx>
Cc: Zhang Rui <rui.zhang@xxxxxxxxx>
---
  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

  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()

diff --git a/drivers/staging/iio/light/Kconfig b/drivers/staging/iio/light/Kconfig
index ca8d6e6..0238a7f 100644
--- a/drivers/staging/iio/light/Kconfig
+++ b/drivers/staging/iio/light/Kconfig
@@ -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
diff --git a/drivers/staging/iio/light/Makefile b/drivers/staging/iio/light/Makefile
index 9960fdf..a3f68bc 100644
--- a/drivers/staging/iio/light/Makefile
+++ b/drivers/staging/iio/light/Makefile
@@ -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
diff --git a/drivers/staging/iio/light/acpi-als.c b/drivers/staging/iio/light/acpi-als.c
new file mode 100644
index 0000000..f63427c
--- /dev/null
+++ b/drivers/staging/iio/light/acpi-als.c
@@ -0,0 +1,326 @@
[...]
+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.

+
+	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;
+
+	/* 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.

+
+	*val = als_read_value(als, ACPI_ALS_ILLUMINANCE);
+	return IIO_VAL_INT;
+}
[..]
+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);
+
+	evt_buffer = devm_kzalloc(dev, evt_buffer_size, GFP_KERNEL);
+	if (!evt_buffer)
+		return -ENOMEM;
+
+	iio = iio_device_alloc(sizeof(*als));

devm_...

[...]
--
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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux