iio: acpi: Add ACPI0008 ALS driver
Subsystems (3)
PATCH v4 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 | 320 ++++++++++++++++++++++++++++++++++ 3 files changed, 331 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
drivers/staging/iio/light/Kconfig +10 -0
@@ -50,4 +50,14 @@ config TSL2x7x tmd2672, tsl2772, tmd2772 devices. Provides iio_events and direct access via sysfs. +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. + endmenu
drivers/staging/iio/light/Makefile +1 -0
@@ -7,3 +7,4 @@ obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o obj-$(CONFIG_TSL2583) += tsl2583.o obj-$(CONFIG_TSL2x7x) += tsl2x7x_core.o +obj-$(CONFIG_ACPI_ALS) += acpi-als.o
drivers/staging/iio/light/acpi-als.c new file +320 -0
320 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Quoted 10 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.
Signed-off-by Martin Liska <[email protected]>Signed-off-by Marek Vasut <[email protected]>Cc Jonathan Cameron <[email protected]>Cc Zhang Rui <[email protected]>
Hi,
Looks good, except for the trigger/buffer support. Which for one can't be enabled since you didn't set the INDIO_BUFFER_TRIGGERED mode flag. But also the implementation itself seems to have a few rough edges.
Quoted 20 lines
--- drivers/staging/iio/light/Kconfig | 10 ++ drivers/staging/iio/light/Makefile | 1 + drivers/staging/iio/light/acpi-als.c | 320 ++++++++++++++++++++++++++++++++++ 3 files changed, 331 insertions(+) create mode 100644 drivers/staging/iio/light/acpi-als.cV2: 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: SelectIIO_TRIGGERED_BUFFERas we need it heredrivers/staging/iio/light/Kconfig +0
-0@@ -50,4 +50,14 @@ config TSL2x7x tmd2672, tsl2772, tmd2772 devices. Provides iio_events and direct access via sysfs.
drivers/staging/iio/light/Kconfig config TSL2x7x · line 62
Quoted 10 lines
drivers/staging/iio/light/Kconfig +10
-0+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. +
Keep the entries in alphabetical order.
drivers/staging/iio/light/Makefile obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o · line 10
Quoted 10 lines
drivers/staging/iio/light/Kconfig +0
-0endmenudrivers/staging/iio/light/Makefile +1
-0@@ -7,3 +7,4 @@ obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o obj-$(CONFIG_TSL2583) += tsl2583.o obj-$(CONFIG_TSL2x7x) += tsl2x7x_core.o +obj-$(CONFIG_ACPI_ALS) += acpi-als.o
Same here.
Quoted 6 lines
drivers/staging/iio/light/acpi-als.c new file +0
-0@@ -0,0 +1,320 @@
[...]
Quoted 19 lines
drivers/staging/iio/light/acpi-als.c new file +19
-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); +uint16_t *buffer = als->evt_buffer; +s64 time_ns =iio_get_time_ns (); + +mutex_lock (&als->lock); + +memset (buffer,0 , als->evt_buffer_len); + +switch (event) { +case ACPI_ALS_NOTIFY_ILLUMINANCE : + *buffer++ =als_read_value (als,ACPI_ALS_ILLUMINANCE ); +break ; +default : +/* Unhandled event */ + dev_dbg(&device->dev, "Unhandled ACPI ALS event (%08x)!\n",
drivers/staging/iio/light/acpi-als.c line 116
Quoted 10 lines
drivers/staging/iio/light/acpi-als.c new file +10
-0+ event); +return ; + } + +if (iio->scan_timestamp) + *(s64 *)ALIGN ((uintptr_t )buffer,sizeof (s64)) = time_ns; + +if (iio_buffer_enabled (iio)) +iio_push_to_buffer (iio->buffer, (uint8_t *)als->evt_buffer); +
Normally you'd call iio_trigger_poll here and have the buffer handling in acpi_als_trigger_handler. This allows you for example to use other triggers, e.g. a timer based trigger.
Quoted 2 lines
drivers/staging/iio/light/acpi-als.c new file +2
-0+mutex_unlock (&als->lock); +}
[...]
Quoted 11 lines
drivers/staging/iio/light/acpi-als.c new file +11
-0+ +static int acpi_als_trigger_init (struct iio_dev *iio) +{ +struct iio_trigger *trig; +int ret; + + trig =iio_trigger_alloc ("%s-dev%i" , iio->name, iio->id); +if (!trig) +return -ENOMEM ; + + trig->dev.parent = iio->dev.parent;
drivers/staging/iio/light/acpi-als.c line 197
Quoted 10 lines
drivers/staging/iio/light/acpi-als.c new file +10
-0+ trig->private_data = iio; + trig->ops = &acpi_als_trigger_ops; + + ret =iio_trigger_register (trig); +if (ret) { +iio_trigger_free (trig); +return ret; + } + + iio->trig = trig;
als->trig = trig;
drivers/staging/iio/light/acpi-als.c line 205
Quoted 8 lines
drivers/staging/iio/light/acpi-als.c new file +8
-0+ +return 0 ; +} + +static void acpi_als_trigger_remove (struct iio_dev *iio) +{ +iio_trigger_unregister (iio->trig); +iio_trigger_free (iio->trig);
iio->trig, is the trigger that is currently assigned to the device. You should really use als->trig here.
Quoted 30 lines
drivers/staging/iio/light/acpi-als.c new file +30
-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); + + evt_buffer =devm_kzalloc (dev, evt_buffer_size,GFP_KERNEL ); +if (!evt_buffer) +return -ENOMEM ; + + iio =iio_device_alloc (sizeof (*als)); +if (!iio) { +dev_err (dev,"Failed to allocate IIO device\n" ); +return -ENOMEM ; + } + + als =iio_priv (iio);
drivers/staging/iio/light/acpi-als.c line 245
Quoted 10 lines
drivers/staging/iio/light/acpi-als.c new file +10
-0+ + device->driver_data = iio; + als->device = device; + als->evt_buffer = evt_buffer; +mutex_init (&als->lock); + + iio->name =ACPI_ALS_DEVICE_NAME ; + iio->dev.parent = dev; + iio->info = &acpi_als_info; + iio->modes =INDIO_DIRECT_MODE ;
INDIO_BUFFER_TRIGGERED
Quoted 27 lines
drivers/staging/iio/light/acpi-als.c new file +27
-0+ iio->channels = acpi_als_channels; + iio->num_channels =ARRAY_SIZE (acpi_als_channels); + + ret =iio_triggered_buffer_setup (iio, &iio_pollfunc_store_time, + &acpi_als_trigger_handler,NULL ); +if (ret) + goto err_iio; + + ret =acpi_als_trigger_init (iio); +if (ret) + goto err_trig; + + ret =iio_device_register (iio); +if (ret <0 ) + goto err_dev; + +return 0 ; + +err_dev: +acpi_als_trigger_remove (iio); +err_trig: +iio_triggered_buffer_cleanup (iio); +err_iio: +iio_device_free (iio); +return ret; +} +
[...]
Quoted 14 lines
drivers/staging/iio/light/acpi-als.c new file +14
-0+ +static struct acpi_driver acpi_als_driver = { + .name ="acpi_als" , + .class =ACPI_ALS_CLASS , + .ids = acpi_als_device_ids, + .ops = { + .add = acpi_als_add, + .remove = acpi_als_remove, + .notify = acpi_als_notify, + }, +}; + +static int acpi_als_init (void ) +{
drivers/staging/iio/light/acpi-als.c line 314
Quoted 10 lines
drivers/staging/iio/light/acpi-als.c new file +10
-0+return acpi_bus_register_driver (&acpi_als_driver); +} + +static void acpi_als_exit (void ) +{ +acpi_bus_unregister_driver (&acpi_als_driver); +} + +module_init (acpi_als_init); +module_exit (acpi_als_exit);
module_acpi_driver(acpi_als_driver);
Dear Lars-Peter Clausen,
Quoted 7 lines · Marek Vasut
Quoted 6 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 8 lines
Quoted 4 lines
Signed-off-by Martin Liska <[email protected]>Signed-off-by Marek Vasut <[email protected]>Cc Jonathan Cameron <[email protected]>Cc Zhang Rui <[email protected]>Hi,
Looks good, except for the trigger/buffer support.
I'm not quite firm on that one either.
Quoted 3 lines
Which for one can't be
enabled since you didn't set theINDIO_BUFFER_TRIGGEREDmode flag. But also the implementation itself seems to have a few rough edges.
Good catch.
Quoted 24 lines
Quoted 24 lines
---
drivers/staging/iio/light/Kconfig | 10 ++ drivers/staging/iio/light/Makefile | 1 + drivers/staging/iio/light/acpi-als.c | 320 ++++++++++++++++++++++++++++++++++ 3 files changed, 331 insertions(+) create mode 100644 drivers/staging/iio/light/acpi-als.cV2: 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_BUFFERas we need it here(quoted diff) +0
-0b/drivers/staging/iio/light/Kconfig index 4bed30e..9c3d146 100644
drivers/staging/iio/light/Kconfig +0
-0@@ -50,4 +50,14 @@ config TSL2x7x tmd2672, tsl2772, tmd2772 devices. Provides iio_events and direct access via sysfs.
Quoted 12 lines
drivers/staging/iio/light/Kconfig config TSL2x7x · line 62
Quoted 10 lines
drivers/staging/iio/light/Kconfig +10
-0+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. +Keep the entries in alphabetical order.
Fixed in my tree
Quoted 8 lines
Quoted 8 lines
endmenu
(quoted diff) +0
-0b/drivers/staging/iio/light/Makefile index 141af1e..13090e6 100644
drivers/staging/iio/light/Makefile +0
-0@@ -7,3 +7,4 @@ obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o
Quoted 7 lines
drivers/staging/iio/light/Makefile obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o · line 10
Quoted 5 lines
drivers/staging/iio/light/Makefile +1
-0obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o obj-$(CONFIG_TSL2583) += tsl2583.o obj-$(CONFIG_TSL2x7x) += tsl2x7x_core.o +obj-$(CONFIG_ACPI_ALS) += acpi-als.oSame here.
Fixed.
Quoted 34 lines
Quoted 6 lines
(quoted diff) +0
-0
b/drivers/staging/iio/light/acpi-als.cnew file mode 100644 index 0000000..6140613drivers/staging/iio/light/acpi-als.c new file +0
-0@@ -0,0 +1,320 @@[...]
Quoted 25 lines
drivers/staging/iio/light/acpi-als.c new file +25
-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); +uint16_t *buffer = als->evt_buffer; +s64 time_ns =iio_get_time_ns (); + +mutex_lock (&als->lock); + +memset (buffer,0 , als->evt_buffer_len); + +switch (event) { +case ACPI_ALS_NOTIFY_ILLUMINANCE : + *buffer++ =als_read_value (als,ACPI_ALS_ILLUMINANCE ); +break ; +default : +/* Unhandled event */ +dev_dbg (&device->dev,"Unhandled ACPI ALS event (%08x)!\n" , + event); +return ; + } + +if (iio->scan_timestamp) + *(s64 *)ALIGN ((uintptr_t )buffer,sizeof (s64)) = time_ns;
Quoted 8 lines
drivers/staging/iio/light/acpi-als.c line 116
Quoted 4 lines
drivers/staging/iio/light/acpi-als.c new file +4
-0+ +if (iio_buffer_enabled (iio)) +iio_push_to_buffer (iio->buffer, (uint8_t *)als->evt_buffer); +Normally you'd call
iio_trigger_pollhere and have the buffer handling inacpi_als_trigger_handler. This allows you for example to use other triggers, e.g. a timer based trigger.
Good, but this is not called from interrupt context. I recall there was a discussion about this and IRQ context issues.
Quoted 20 lines
Quoted 2 lines
drivers/staging/iio/light/acpi-als.c new file +2
-0+mutex_unlock (&als->lock); +}[...]
Quoted 15 lines
drivers/staging/iio/light/acpi-als.c new file +15
-0+ +static int acpi_als_trigger_init (struct iio_dev *iio) +{ +struct iio_trigger *trig; +int ret; + + trig =iio_trigger_alloc ("%s-dev%i" , iio->name, iio->id); +if (!trig) +return -ENOMEM ; + + trig->dev.parent = iio->dev.parent; + trig->private_data = iio; + trig->ops = &acpi_als_trigger_ops; + + ret =iio_trigger_register (trig);
Quoted 8 lines
drivers/staging/iio/light/acpi-als.c line 197
Quoted 6 lines
drivers/staging/iio/light/acpi-als.c new file +6
-0+if (ret) { +iio_trigger_free (trig); +return ret; + } + + iio->trig = trig;
als->trig= trig;
Fixed.
Quoted 11 lines
drivers/staging/iio/light/acpi-als.c line 205
Quoted 8 lines
drivers/staging/iio/light/acpi-als.c new file +8
-0+ +return 0 ; +} + +static void acpi_als_trigger_remove (struct iio_dev *iio) +{ +iio_trigger_unregister (iio->trig); +iio_trigger_free (iio->trig);
iio->trig, is the trigger that is currently assigned to the device. You should really useals->trighere.
Fixed.
Quoted 34 lines
Quoted 34 lines
drivers/staging/iio/light/acpi-als.c new file +34
-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); + + evt_buffer =devm_kzalloc (dev, evt_buffer_size,GFP_KERNEL ); +if (!evt_buffer) +return -ENOMEM ; + + iio =iio_device_alloc (sizeof (*als)); +if (!iio) { +dev_err (dev,"Failed to allocate IIO device\n" ); +return -ENOMEM ; + } + + als =iio_priv (iio); + + device->driver_data = iio; + als->device = device; + als->evt_buffer = evt_buffer;
Quoted 8 lines
drivers/staging/iio/light/acpi-als.c line 245
Quoted 6 lines
drivers/staging/iio/light/acpi-als.c new file +6
-0+mutex_init (&als->lock); + + iio->name =ACPI_ALS_DEVICE_NAME ; + iio->dev.parent = dev; + iio->info = &acpi_als_info; + iio->modes =INDIO_DIRECT_MODE ;
INDIO_BUFFER_TRIGGERED
Will this not mess up the RAW mode? Or do you mean:
iio->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
[...]
Thanks for the review. Now it seems we need to iron out the tougher parts -- like this stuff above and the iio_push_to_buffer(). I'd be glad if you could provide me with some assistance on that. I'll roll out V5 once we're clear on those.
Thanks!
Quoted 126 lines · Marek Vasut
Dear Lars-Peter Clausen,
Quoted 7 lines · Marek Vasut
Quoted 6 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 8 lines
Quoted 4 lines
Signed-off-by Martin Liska <[email protected]>Signed-off-by Marek Vasut <[email protected]>Cc Jonathan Cameron <[email protected]>Cc Zhang Rui <[email protected]>Hi,
Looks good, except for the trigger/buffer support.
I'm not quite firm on that one either.
Quoted 3 lines
Which for one can't be
enabled since you didn't set theINDIO_BUFFER_TRIGGEREDmode flag. But also the implementation itself seems to have a few rough edges.Good catch.
Quoted 24 lines
Quoted 24 lines
---
drivers/staging/iio/light/Kconfig | 10 ++ drivers/staging/iio/light/Makefile | 1 + drivers/staging/iio/light/acpi-als.c | 320 ++++++++++++++++++++++++++++++++++ 3 files changed, 331 insertions(+) create mode 100644 drivers/staging/iio/light/acpi-als.cV2: 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_BUFFERas we need it here(quoted diff) +0
-0b/drivers/staging/iio/light/Kconfig index 4bed30e..9c3d146 100644
drivers/staging/iio/light/Kconfig +0
-0@@ -50,4 +50,14 @@ config TSL2x7x tmd2672, tsl2772, tmd2772 devices. Provides iio_events and direct access via sysfs.Quoted 12 lines
drivers/staging/iio/light/Kconfig config TSL2x7x · line 62
Quoted 10 lines
drivers/staging/iio/light/Kconfig +10
-0+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. +Keep the entries in alphabetical order.
Fixed in my tree
Quoted 8 lines
Quoted 8 lines
endmenu
(quoted diff) +0
-0b/drivers/staging/iio/light/Makefile index 141af1e..13090e6 100644
drivers/staging/iio/light/Makefile +0
-0@@ -7,3 +7,4 @@ obj-$(CONFIG_SENSORS_ISL29018) += isl29018.oQuoted 7 lines
drivers/staging/iio/light/Makefile obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o · line 10
Quoted 5 lines
drivers/staging/iio/light/Makefile +1
-0obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o obj-$(CONFIG_TSL2583) += tsl2583.o obj-$(CONFIG_TSL2x7x) += tsl2x7x_core.o +obj-$(CONFIG_ACPI_ALS) += acpi-als.oSame here.
Fixed.
Quoted 34 lines
Quoted 6 lines
(quoted diff) +0
-0
b/drivers/staging/iio/light/acpi-als.cnew file mode 100644 index 0000000..6140613drivers/staging/iio/light/acpi-als.c new file +0
-0@@ -0,0 +1,320 @@[...]
Quoted 25 lines
drivers/staging/iio/light/acpi-als.c new file +25
-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); +uint16_t *buffer = als->evt_buffer; +s64 time_ns =iio_get_time_ns (); + +mutex_lock (&als->lock); + +memset (buffer,0 , als->evt_buffer_len); + +switch (event) { +case ACPI_ALS_NOTIFY_ILLUMINANCE : + *buffer++ =als_read_value (als,ACPI_ALS_ILLUMINANCE ); +break ; +default : +/* Unhandled event */ +dev_dbg (&device->dev,"Unhandled ACPI ALS event (%08x)!\n" , + event); +return ; + } + +if (iio->scan_timestamp) + *(s64 *)ALIGN ((uintptr_t )buffer,sizeof (s64)) = time_ns;Quoted 8 lines
drivers/staging/iio/light/acpi-als.c line 116
Quoted 4 lines
drivers/staging/iio/light/acpi-als.c new file +4
-0+ +if (iio_buffer_enabled (iio)) +iio_push_to_buffer (iio->buffer, (uint8_t *)als->evt_buffer); +Normally you'd call
iio_trigger_pollhere and have the buffer handling inacpi_als_trigger_handler. This allows you for example to use other triggers, e.g. a timer based trigger.
Quoted 2 lines
Good, but this is not called from interrupt context. I recall there was a discussion about this and IRQ context issues.
Hm, yes. This is indeed a problem and I guess we really need to fix this at somepoint in the IIO core. In the sysfs trigger we use a irq_work to call iio_trigger_poll from IRQ context. Which is a bit hackish, but it works fine for the moment, but on the long run we really need to find a better solution. But I guess you could use it for now.
[...]
Quoted 10 lines
Quoted 6 lines
drivers/staging/iio/light/acpi-als.c line 245
Quoted 4 lines
drivers/staging/iio/light/acpi-als.c new file +4
-0+ iio->name =ACPI_ALS_DEVICE_NAME ; + iio->dev.parent = dev; + iio->info = &acpi_als_info; + iio->modes =INDIO_DIRECT_MODE ;
INDIO_BUFFER_TRIGGEREDWill this not mess up the RAW mode? Or do you mean:
iio->modes=INDIO_DIRECT_MODE|INDIO_BUFFER_TRIGGERED;
Yes, that's what I meant.
- Lars