Re: [PATCH 6/8] iio: light: Add support for Azoteq IQS621 ambient light sensor
Hi Jonathan, Thank you for your prompt review. On Tue, Oct 22, 2019 at 12:23:49PM +0100, Jonathan Cameron wrote: > On Sun, 20 Oct 2019 23:11:21 -0500 > Jeff LaBundy wrote: > > > This patch adds support for the Azoteq IQS621 ambient light sensor, > > capable of reporting intensity directly in units of lux. > > If they are in lux, should be using IIO_CHAN_INFO_PROCESSED to indicate > that. I was wondering why we had no scale then noticed this comment. > Sure thing; will do. > Other than that, this looks good to me. So with that tidied up in V2. > Acked-by: Jonathan Cameron > > One trivial comment to perhaps drop an error print as overly verbose inline. > > > > > Signed-off-by: Jeff LaBundy > > --- > > drivers/iio/light/Kconfig | 10 ++ > > drivers/iio/light/Makefile | 1 + > > drivers/iio/light/iqs621-als.c | 361 > > + > > 3 files changed, 372 insertions(+) > > create mode 100644 drivers/iio/light/iqs621-als.c > > > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > > index 4a1a883..aad26dc 100644 > > --- a/drivers/iio/light/Kconfig > > +++ b/drivers/iio/light/Kconfig > > @@ -162,6 +162,16 @@ config GP2AP020A00F > > To compile this driver as a module, choose M here: the > > module will be called gp2ap020a00f. > > > > +config IQS621_ALS > > + tristate "Azoteq IQS621 ambient light sensor" > > + depends on MFD_IQS62X > > + help > > + Say Y here if you want to build support for the Azoteq IQS621 > > + ambient light sensor. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called iqs621-als. > > + > > config SENSORS_ISL29018 > > tristate "Intersil 29018 light and proximity sensor" > > depends on I2C > > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > > index 00d1f9b..aa34358 100644 > > --- a/drivers/iio/light/Makefile > > +++ b/drivers/iio/light/Makefile > > @@ -20,6 +20,7 @@ obj-$(CONFIG_IIO_CROS_EC_LIGHT_PROX) += > > cros_ec_light_prox.o > > obj-$(CONFIG_GP2AP020A00F) += gp2ap020a00f.o > > obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o > > obj-$(CONFIG_HID_SENSOR_PROX) += hid-sensor-prox.o > > +obj-$(CONFIG_IQS621_ALS) += iqs621-als.o > > obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o > > obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o > > obj-$(CONFIG_ISL29125) += isl29125.o > > diff --git a/drivers/iio/light/iqs621-als.c b/drivers/iio/light/iqs621-als.c > > new file mode 100644 > > index 000..92a6173 > > --- /dev/null > > +++ b/drivers/iio/light/iqs621-als.c > > @@ -0,0 +1,361 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Azoteq IQS621 Ambient Light Sensor > > + * > > + * Copyright (C) 2019 > > + * Author: Jeff LaBundy > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define IQS621_ALS_FLAGS 0x16 > > +#define IQS621_ALS_FLAGS_LIGHT BIT(7) > > + > > +#define IQS621_ALS_UI_OUT 0x17 > > + > > +#define IQS621_ALS_THRESH_DARK 0x80 > > +#define IQS621_ALS_THRESH_DARK_MAX 1020 > > +#define IQS621_ALS_THRESH_DARK_SHIFT 2 > > + > > +#define IQS621_ALS_THRESH_LIGHT0x81 > > +#define IQS621_ALS_THRESH_LIGHT_MAX4080 > > +#define IQS621_ALS_THRESH_LIGHT_SHIFT 4 > > + > > +struct iqs621_als_private { > > + struct iqs62x_core *iqs62x; > > + struct notifier_block notifier; > > + struct mutex lock; > > + bool event_en; > > + u8 thresh_light; > > + u8 thresh_dark; > > + u8 flags; > > +}; > > + > > +static int iqs621_als_init(struct iqs621_als_private *iqs621_als) > > +{ > > + struct iqs62x_core *iqs62x = iqs621_als->iqs62x; > > + unsigned int val; > > + int error; > > + > > + mutex_lock(_als->lock); > > + > > + error = regmap_write(iqs62x->map, IQS621_ALS_THRESH_LIGHT, > > +iqs621_als->thresh_light); > > + if (error) > > + goto err_mutex; > > + > > + error = regmap_write(iqs62x->map, IQS621_ALS_THRESH_DARK, > > +iqs621_als->thresh_dark); > > + if (error) > > + goto err_mutex; > > + > > + error = regmap_read(iqs62x->map, IQS621_ALS_FLAGS, ); > > + if (error) > > + goto err_mutex; > > + iqs621_als->flags = val; > > + > > + error = regmap_update_bits(iqs62x->map, IQS620_GLBL_EVENT_MASK, > > + iqs62x->dev_desc->als_mask, > > + iqs621_als->event_en ? 0 : 0xFF); > > + > > +err_mutex: > > + mutex_unlock(_als->lock); > > + > > + return error; > > +} > > + > > +static int iqs621_als_notifier(struct notifier_block *notifier, > > + unsigned long event_flags, void *context) > > +{ > > + struct iqs62x_event_data
Re: [PATCH 6/8] iio: light: Add support for Azoteq IQS621 ambient light sensor
On Sun, 20 Oct 2019 23:11:21 -0500 Jeff LaBundy wrote: > This patch adds support for the Azoteq IQS621 ambient light sensor, > capable of reporting intensity directly in units of lux. If they are in lux, should be using IIO_CHAN_INFO_PROCESSED to indicate that. I was wondering why we had no scale then noticed this comment. Other than that, this looks good to me. So with that tidied up in V2. Acked-by: Jonathan Cameron One trivial comment to perhaps drop an error print as overly verbose inline. > > Signed-off-by: Jeff LaBundy > --- > drivers/iio/light/Kconfig | 10 ++ > drivers/iio/light/Makefile | 1 + > drivers/iio/light/iqs621-als.c | 361 > + > 3 files changed, 372 insertions(+) > create mode 100644 drivers/iio/light/iqs621-als.c > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index 4a1a883..aad26dc 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -162,6 +162,16 @@ config GP2AP020A00F > To compile this driver as a module, choose M here: the > module will be called gp2ap020a00f. > > +config IQS621_ALS > + tristate "Azoteq IQS621 ambient light sensor" > + depends on MFD_IQS62X > + help > + Say Y here if you want to build support for the Azoteq IQS621 > + ambient light sensor. > + > + To compile this driver as a module, choose M here: the module > + will be called iqs621-als. > + > config SENSORS_ISL29018 > tristate "Intersil 29018 light and proximity sensor" > depends on I2C > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > index 00d1f9b..aa34358 100644 > --- a/drivers/iio/light/Makefile > +++ b/drivers/iio/light/Makefile > @@ -20,6 +20,7 @@ obj-$(CONFIG_IIO_CROS_EC_LIGHT_PROX) += cros_ec_light_prox.o > obj-$(CONFIG_GP2AP020A00F) += gp2ap020a00f.o > obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o > obj-$(CONFIG_HID_SENSOR_PROX)+= hid-sensor-prox.o > +obj-$(CONFIG_IQS621_ALS) += iqs621-als.o > obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o > obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o > obj-$(CONFIG_ISL29125) += isl29125.o > diff --git a/drivers/iio/light/iqs621-als.c b/drivers/iio/light/iqs621-als.c > new file mode 100644 > index 000..92a6173 > --- /dev/null > +++ b/drivers/iio/light/iqs621-als.c > @@ -0,0 +1,361 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Azoteq IQS621 Ambient Light Sensor > + * > + * Copyright (C) 2019 > + * Author: Jeff LaBundy > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define IQS621_ALS_FLAGS 0x16 > +#define IQS621_ALS_FLAGS_LIGHT BIT(7) > + > +#define IQS621_ALS_UI_OUT0x17 > + > +#define IQS621_ALS_THRESH_DARK 0x80 > +#define IQS621_ALS_THRESH_DARK_MAX 1020 > +#define IQS621_ALS_THRESH_DARK_SHIFT 2 > + > +#define IQS621_ALS_THRESH_LIGHT 0x81 > +#define IQS621_ALS_THRESH_LIGHT_MAX 4080 > +#define IQS621_ALS_THRESH_LIGHT_SHIFT4 > + > +struct iqs621_als_private { > + struct iqs62x_core *iqs62x; > + struct notifier_block notifier; > + struct mutex lock; > + bool event_en; > + u8 thresh_light; > + u8 thresh_dark; > + u8 flags; > +}; > + > +static int iqs621_als_init(struct iqs621_als_private *iqs621_als) > +{ > + struct iqs62x_core *iqs62x = iqs621_als->iqs62x; > + unsigned int val; > + int error; > + > + mutex_lock(_als->lock); > + > + error = regmap_write(iqs62x->map, IQS621_ALS_THRESH_LIGHT, > + iqs621_als->thresh_light); > + if (error) > + goto err_mutex; > + > + error = regmap_write(iqs62x->map, IQS621_ALS_THRESH_DARK, > + iqs621_als->thresh_dark); > + if (error) > + goto err_mutex; > + > + error = regmap_read(iqs62x->map, IQS621_ALS_FLAGS, ); > + if (error) > + goto err_mutex; > + iqs621_als->flags = val; > + > + error = regmap_update_bits(iqs62x->map, IQS620_GLBL_EVENT_MASK, > +iqs62x->dev_desc->als_mask, > +iqs621_als->event_en ? 0 : 0xFF); > + > +err_mutex: > + mutex_unlock(_als->lock); > + > + return error; > +} > + > +static int iqs621_als_notifier(struct notifier_block *notifier, > +unsigned long event_flags, void *context) > +{ > + struct iqs62x_event_data *event_data = context; > + struct iqs621_als_private *iqs621_als; > + struct iio_dev *indio_dev; > + enum iio_event_direction dir; > + int error; > + > + iqs621_als = container_of(notifier, struct iqs621_als_private, > + notifier); > + indio_dev = iio_priv_to_dev(iqs621_als); > + > + if
[PATCH 6/8] iio: light: Add support for Azoteq IQS621 ambient light sensor
This patch adds support for the Azoteq IQS621 ambient light sensor, capable of reporting intensity directly in units of lux. Signed-off-by: Jeff LaBundy --- drivers/iio/light/Kconfig | 10 ++ drivers/iio/light/Makefile | 1 + drivers/iio/light/iqs621-als.c | 361 + 3 files changed, 372 insertions(+) create mode 100644 drivers/iio/light/iqs621-als.c diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig index 4a1a883..aad26dc 100644 --- a/drivers/iio/light/Kconfig +++ b/drivers/iio/light/Kconfig @@ -162,6 +162,16 @@ config GP2AP020A00F To compile this driver as a module, choose M here: the module will be called gp2ap020a00f. +config IQS621_ALS + tristate "Azoteq IQS621 ambient light sensor" + depends on MFD_IQS62X + help + Say Y here if you want to build support for the Azoteq IQS621 + ambient light sensor. + + To compile this driver as a module, choose M here: the module + will be called iqs621-als. + config SENSORS_ISL29018 tristate "Intersil 29018 light and proximity sensor" depends on I2C diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile index 00d1f9b..aa34358 100644 --- a/drivers/iio/light/Makefile +++ b/drivers/iio/light/Makefile @@ -20,6 +20,7 @@ obj-$(CONFIG_IIO_CROS_EC_LIGHT_PROX) += cros_ec_light_prox.o obj-$(CONFIG_GP2AP020A00F) += gp2ap020a00f.o obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o obj-$(CONFIG_HID_SENSOR_PROX) += hid-sensor-prox.o +obj-$(CONFIG_IQS621_ALS) += iqs621-als.o obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o obj-$(CONFIG_ISL29125) += isl29125.o diff --git a/drivers/iio/light/iqs621-als.c b/drivers/iio/light/iqs621-als.c new file mode 100644 index 000..92a6173 --- /dev/null +++ b/drivers/iio/light/iqs621-als.c @@ -0,0 +1,361 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Azoteq IQS621 Ambient Light Sensor + * + * Copyright (C) 2019 + * Author: Jeff LaBundy + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define IQS621_ALS_FLAGS 0x16 +#define IQS621_ALS_FLAGS_LIGHT BIT(7) + +#define IQS621_ALS_UI_OUT 0x17 + +#define IQS621_ALS_THRESH_DARK 0x80 +#define IQS621_ALS_THRESH_DARK_MAX 1020 +#define IQS621_ALS_THRESH_DARK_SHIFT 2 + +#define IQS621_ALS_THRESH_LIGHT0x81 +#define IQS621_ALS_THRESH_LIGHT_MAX4080 +#define IQS621_ALS_THRESH_LIGHT_SHIFT 4 + +struct iqs621_als_private { + struct iqs62x_core *iqs62x; + struct notifier_block notifier; + struct mutex lock; + bool event_en; + u8 thresh_light; + u8 thresh_dark; + u8 flags; +}; + +static int iqs621_als_init(struct iqs621_als_private *iqs621_als) +{ + struct iqs62x_core *iqs62x = iqs621_als->iqs62x; + unsigned int val; + int error; + + mutex_lock(_als->lock); + + error = regmap_write(iqs62x->map, IQS621_ALS_THRESH_LIGHT, +iqs621_als->thresh_light); + if (error) + goto err_mutex; + + error = regmap_write(iqs62x->map, IQS621_ALS_THRESH_DARK, +iqs621_als->thresh_dark); + if (error) + goto err_mutex; + + error = regmap_read(iqs62x->map, IQS621_ALS_FLAGS, ); + if (error) + goto err_mutex; + iqs621_als->flags = val; + + error = regmap_update_bits(iqs62x->map, IQS620_GLBL_EVENT_MASK, + iqs62x->dev_desc->als_mask, + iqs621_als->event_en ? 0 : 0xFF); + +err_mutex: + mutex_unlock(_als->lock); + + return error; +} + +static int iqs621_als_notifier(struct notifier_block *notifier, + unsigned long event_flags, void *context) +{ + struct iqs62x_event_data *event_data = context; + struct iqs621_als_private *iqs621_als; + struct iio_dev *indio_dev; + enum iio_event_direction dir; + int error; + + iqs621_als = container_of(notifier, struct iqs621_als_private, + notifier); + indio_dev = iio_priv_to_dev(iqs621_als); + + if (event_flags & BIT(IQS62X_EVENT_SYS_RESET)) { + error = iqs621_als_init(iqs621_als); + if (error) { + dev_err(indio_dev->dev.parent, + "Failed to re-initialize device: %d\n", error); + return NOTIFY_BAD; + } + + return NOTIFY_OK; + } + + if (!((event_data->als_flags ^ iqs621_als->flags) & + IQS621_ALS_FLAGS_LIGHT)) + return NOTIFY_DONE; + + iqs621_als->flags = event_data->als_flags; + + if