From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932505AbbEHSc1 (ORCPT ); Fri, 8 May 2015 14:32:27 -0400 Received: from saturn.retrosnub.co.uk ([178.18.118.26]:52554 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932164AbbEHSaZ (ORCPT ); Fri, 8 May 2015 14:30:25 -0400 Message-ID: <554CCF15.1040701@kernel.org> Date: Fri, 08 May 2015 10:58:29 -0400 From: Jonathan Cameron User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Gabriele Mazzotta CC: knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net, marxin.liska@gmail.com, marex@denx.de, rui.zhang@intel.com, linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org Subject: Re: [PATCH v9] iio: acpi: Add support for ACPI0008 Ambient Light Sensor References: <1430569857-31386-1-git-send-email-gabriele.mzt@gmail.com> In-Reply-To: <1430569857-31386-1-git-send-email-gabriele.mzt@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/05/15 08:30, Gabriele Mazzotta wrote: > This driver adds the initial support for the ACPI Ambient Light Sensor > as defined in Section 9.2 of the ACPI specification (Revision 5.0) [1]. > > Sensors complying with the standard are exposed as ACPI devices with > ACPI0008 as hardware ID and provide standard methods by which the OS > can query properties of the ambient light environment the system is > currently operating in. > > This driver currently allows only to get the current ambient light > illuminance reading through the _ALI method, but is ready to be > extended extended to handle _ALC, _ALT and _ALP as well. > > [1] http://www.acpi.info/DOWNLOADS/ACPIspec50.pdf > > Signed-off-by: Martin Liska > Signed-off-by: Marek Vasut > Signed-off-by: Gabriele Mazzotta > Cc: Zhang Rui Sorry, one last point inline that I missed before! (just noticed it when taking a last glance before applying the patch). Jonathan > --- > Changes since v8: > - Set realbits to 32 > - Fix license mismatch (using GPL v2 or later) > - Drop iio_device_unregister() in favor of devm_iio_device_register() > > drivers/iio/light/Kconfig | 12 +++ > drivers/iio/light/Makefile | 1 + > drivers/iio/light/acpi-als.c | 232 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 245 insertions(+) > create mode 100644 drivers/iio/light/acpi-als.c > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index 01a1a16..898b2b5 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -5,6 +5,18 @@ > > menu "Light sensors" > > +config ACPI_ALS > + tristate "ACPI Ambient Light Sensor" > + depends on ACPI > + select IIO_TRIGGERED_BUFFER > + select IIO_KFIFO_BUF > + help > + Say Y here if you want to build a driver for the ACPI0008 > + Ambient Light Sensor. > + > + To compile this driver as a module, choose M here: the module will > + be called acpi-als. > + > config ADJD_S311 > tristate "ADJD-S311-CR999 digital color sensor" > select IIO_BUFFER > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > index ad7c30f..d9aad52a 100644 > --- a/drivers/iio/light/Makefile > +++ b/drivers/iio/light/Makefile > @@ -3,6 +3,7 @@ > # > > # When adding new entries keep the list in alphabetical order > +obj-$(CONFIG_ACPI_ALS) += acpi-als.o > obj-$(CONFIG_ADJD_S311) += adjd_s311.o > obj-$(CONFIG_AL3320A) += al3320a.o > obj-$(CONFIG_APDS9300) += apds9300.o > diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c > new file mode 100644 > index 0000000..9839c9a > --- /dev/null > +++ b/drivers/iio/light/acpi-als.c > @@ -0,0 +1,232 @@ > +/* > + * ACPI Ambient Light Sensor Driver > + * > + * Based on ALS driver: > + * Copyright (C) 2009 Zhang Rui > + * > + * Rework for IIO subsystem: > + * Copyright (C) 2012-2013 Martin Liska > + * > + * Final cleanup and debugging: > + * Copyright (C) 2013-2014 Marek Vasut > + * Copyright (C) 2015 Gabriele Mazzotta > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, write to the Free Software Foundation, Inc., > + * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +#define ACPI_ALS_CLASS "als" > +#define ACPI_ALS_DEVICE_NAME "acpi-als" > +#define ACPI_ALS_NOTIFY_ILLUMINANCE 0x80 > + > +ACPI_MODULE_NAME("acpi-als"); > + > +/* > + * So far, there's only one channel in here, but the specification for > + * ACPI0008 says there can be more to what the block can report. Like > + * chromaticity and such. We are ready for incoming additions! > + */ > +static const struct iio_chan_spec acpi_als_channels[] = { > + { > + .type = IIO_LIGHT, > + .scan_type = { > + .sign = 's', > + .realbits = 32, > + .storagebits = 32, > + }, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + }, > +}; > + > +/* > + * The event buffer contains timestamp and all the data from > + * the ACPI0008 block. There are multiple, but so far we only > + * support _ALI (illuminance). Once someone adds new channels > + * to acpi_als_channels[], the evt_buffer below will grow > + * automatically. > + */ > +#define EVT_NR_SOURCES ARRAY_SIZE(acpi_als_channels) > +#define EVT_BUFFER_SIZE \ > + (sizeof(s64) + (EVT_NR_SOURCES * sizeof(s32))) > + > +struct acpi_als { > + struct acpi_device *device; > + struct mutex lock; > + > + s32 evt_buffer[EVT_BUFFER_SIZE]; > +}; > + > +/* > + * All types of properties the ACPI0008 block can report. The ALI, ALC, ALT > + * and ALP can all be handled by als_read_value() below, while the ALR is > + * special. > + * > + * The _ALR property returns tables that can be used to fine-tune the values > + * reported by the other props based on the particular hardware type and it's > + * location (it contains tables for "rainy", "bright inhouse lighting" etc.). > + * > + * So far, we support only ALI (illuminance). > + */ > +#define ACPI_ALS_ILLUMINANCE "_ALI" > +#define ACPI_ALS_CHROMATICITY "_ALC" > +#define ACPI_ALS_COLOR_TEMP "_ALT" > +#define ACPI_ALS_POLLING "_ALP" > +#define ACPI_ALS_TABLES "_ALR" > + > +static int als_read_value(struct acpi_als *als, char *prop, s32 *val) > +{ > + unsigned long long temp_val; > + acpi_status status; > + > + status = acpi_evaluate_integer(als->device->handle, prop, NULL, > + &temp_val); > + > + if (ACPI_FAILURE(status)) { > + ACPI_EXCEPTION((AE_INFO, status, "Error reading ALS %s", prop)); > + return -EIO; > + } > + > + *val = temp_val; > + > + return 0; > +} > + > +static void acpi_als_notify(struct acpi_device *device, u32 event) > +{ > + struct iio_dev *indio_dev = acpi_driver_data(device); > + struct acpi_als *als = iio_priv(indio_dev); > + s32 *buffer = als->evt_buffer; > + s64 time_ns = iio_get_time_ns(); > + s32 val; > + int ret; > + > + mutex_lock(&als->lock); > + > + memset(buffer, 0, EVT_BUFFER_SIZE); > + > + switch (event) { > + case ACPI_ALS_NOTIFY_ILLUMINANCE: > + ret = als_read_value(als, ACPI_ALS_ILLUMINANCE, &val); > + if (ret < 0) > + goto out; > + *buffer++ = val; > + break; > + default: > + /* Unhandled event */ > + dev_dbg(&device->dev, "Unhandled ACPI ALS event (%08x)!\n", > + event); > + goto out; > + } > + > + iio_push_to_buffers_with_timestamp(indio_dev, > + (u8 *)als->evt_buffer, time_ns); Why the u8* cast, the function takes a void * so there is no need to cast it. > + > +out: > + mutex_unlock(&als->lock); > +} > + > +static int acpi_als_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + struct acpi_als *als = iio_priv(indio_dev); > + s32 temp_val; > + int ret; > + > + if (mask != IIO_CHAN_INFO_RAW) > + return -EINVAL; > + > + /* we support only illumination (_ALI) so far. */ > + if (chan->type != IIO_LIGHT) > + return -EINVAL; > + > + ret = als_read_value(als, ACPI_ALS_ILLUMINANCE, &temp_val); > + if (ret < 0) > + return ret; > + > + *val = temp_val; > + > + return IIO_VAL_INT; > +} > + > +static const struct iio_info acpi_als_info = { > + .driver_module = THIS_MODULE, > + .read_raw = acpi_als_read_raw, > +}; > + > +static int acpi_als_add(struct acpi_device *device) > +{ > + struct acpi_als *als; > + struct iio_dev *indio_dev; > + struct iio_buffer *buffer; > + > + indio_dev = devm_iio_device_alloc(&device->dev, sizeof(*als)); > + if (!indio_dev) > + return -ENOMEM; > + > + als = iio_priv(indio_dev); > + > + device->driver_data = indio_dev; > + als->device = device; > + mutex_init(&als->lock); > + > + indio_dev->name = ACPI_ALS_DEVICE_NAME; > + indio_dev->dev.parent = &device->dev; > + indio_dev->info = &acpi_als_info; > + indio_dev->modes = INDIO_BUFFER_SOFTWARE; > + indio_dev->channels = acpi_als_channels; > + indio_dev->num_channels = ARRAY_SIZE(acpi_als_channels); > + > + buffer = devm_iio_kfifo_allocate(&device->dev); > + if (!buffer) > + return -ENOMEM; > + > + iio_device_attach_buffer(indio_dev, buffer); > + > + return devm_iio_device_register(&device->dev, indio_dev); > +} > + > +static const struct acpi_device_id acpi_als_device_ids[] = { > + {"ACPI0008", 0}, > + {}, > +}; > + > +MODULE_DEVICE_TABLE(acpi, acpi_als_device_ids); > + > +static struct acpi_driver acpi_als_driver = { > + .name = "acpi_als", > + .class = ACPI_ALS_CLASS, > + .ids = acpi_als_device_ids, > + .ops = { > + .add = acpi_als_add, > + .notify = acpi_als_notify, > + }, > +}; > + > +module_acpi_driver(acpi_als_driver); > + > +MODULE_AUTHOR("Zhang Rui "); > +MODULE_AUTHOR("Martin Liska "); > +MODULE_AUTHOR("Marek Vasut "); > +MODULE_DESCRIPTION("ACPI Ambient Light Sensor Driver"); > +MODULE_LICENSE("GPL"); >