[PATCH 0/5] BH1770GLC, SFH7770 and APDS990x als / proximity sensor drivers

[PATCH 0/5] BH1770GLC, SFH7770 and APDS990x als / proximity sensor drivers

[Samu Onkalo]

Patch set contains two drivers. One for BH1770GLC / SFH7770 chips and
one for APDS990X chip.

Both drivers have similar sysfs based interface. Both supports
pm_runtime and regulator frame work. There is short documentation
for both drivers.

Unfortunately I can't promise data sheets to public access.

Samu Onkalo (5):
  misc: Driver for bh1770glc / sfh7770 ALS and proximity sensor
  misc: update bhsfh driver to Kconfig and Makefile
  misc: Driver for APDS990X ALS and proximity sensors
  misc: update apds990x driver to Kconfig and Makefile
  Documentation: Short descriptions for bhsfh and apds990x drivers

 Documentation/misc-devices/apds990x.txt |  126 +++
 Documentation/misc-devices/bhsfh.txt    |  125 +++
 drivers/misc/Kconfig                    |   21 +
 drivers/misc/Makefile                   |    2 +
 drivers/misc/apds990x.c                 | 1216 ++++++++++++++++++++++++++++
 drivers/misc/bhsfh.c                    | 1324 +++++++++++++++++++++++++++++++
 include/linux/i2c/apds990x.h            |   57 ++
 include/linux/i2c/bhsfh.h               |   42 +
 8 files changed, 2913 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/misc-devices/apds990x.txt
 create mode 100644 Documentation/misc-devices/bhsfh.txt
 create mode 100644 drivers/misc/apds990x.c
 create mode 100644 drivers/misc/bhsfh.c
 create mode 100644 include/linux/i2c/apds990x.h
 create mode 100644 include/linux/i2c/bhsfh.h

[PATCH 1/5] misc: Driver for bh1770glc / sfh7770 ALS and proximity sensor

[Samu Onkalo]

This is a driver for ROHM BH1770GLC and OSRAM SFH7770 combined
ALS and proximity sensor.

Interface is sysfs based. The driver uses interrupts to provide new data.
The driver supports pm_runtime and regulator frameworks.

See Documentation/misc-devices/bhsfh.txt for details

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
---
 drivers/misc/bhsfh.c      | 1324 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/bhsfh.h |   42 ++
 2 files changed, 1366 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/bhsfh.c
 create mode 100644 include/linux/i2c/bhsfh.h

diff --git a/drivers/misc/bhsfh.c b/drivers/misc/bhsfh.c
new file mode 100644
index 0000000..af56724
--- /dev/null
+++ b/drivers/misc/bhsfh.c
@@ -0,0 +1,1324 @@
+/*
+ * This file is part of the ROHM BH1770GLC / OSRAM SFH7770 sensor driver.
+ * Chip is combined proximity and ambient light sensor.
+ *
+ * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * Contact: Samu Onkalo <samu.p.onkalo@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+#include <linux/i2c/bhsfh.h>
+#include <linux/regulator/consumer.h>
+#include <linux/pm_runtime.h>
+#include <linux/workqueue.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+
+#define BHSFH_ALS_CONTROL	0x80 /* ALS operation mode control */
+#define BHSFH_PS_CONTROL	0x81 /* PS operation mode control */
+#define BHSFH_I_LED		0x82 /* active LED and LED1, LED2 current */
+#define BHSFH_I_LED3		0x83 /* LED3 current setting */
+#define BHSFH_ALS_PS_MEAS	0x84 /* Forced mode trigger */
+#define BHSFH_PS_MEAS_RATE	0x85 /* PS meas. rate at stand alone mode */
+#define BHSFH_ALS_MEAS_RATE	0x86 /* ALS meas. rate at stand alone mode */
+#define BHSFH_PART_ID		0x8a /* Part number and revision ID */
+#define BHSFH_MANUFACT_ID	0x8b /* Manufacturerer ID */
+#define BHSFH_ALS_DATA_0	0x8c /* ALS DATA low byte */
+#define BHSFH_ALS_DATA_1	0x8d /* ALS DATA high byte */
+#define BHSFH_ALS_PS_STATUS	0x8e /* Measurement data and int status */
+#define BHSFH_PS_DATA_LED1	0x8f /* PS data from LED1 */
+#define BHSFH_PS_DATA_LED2	0x90 /* PS data from LED2 */
+#define BHSFH_PS_DATA_LED3	0x91 /* PS data from LED3 */
+#define BHSFH_INTERRUPT		0x92 /* Interrupt setting */
+#define BHSFH_PS_TH_LED1	0x93 /* PS interrupt threshold for LED1 */
+#define BHSFH_PS_TH_LED2	0x94 /* PS interrupt threshold for LED2 */
+#define BHSFH_PS_TH_LED3	0x95 /* PS interrupt threshold for LED3 */
+#define BHSFH_ALS_TH_UP_0	0x96 /* ALS upper threshold low byte */
+#define BHSFH_ALS_TH_UP_1	0x97 /* ALS upper threshold high byte */
+#define BHSFH_ALS_TH_LOW_0	0x98 /* ALS lower threshold low byte */
+#define BHSFH_ALS_TH_LOW_1	0x99 /* ALS lower threshold high byte */
+
+/* MANUFACT_ID */
+#define BHSFH_MANUFACT_ROHM	0x01
+#define BHSFH_MANUFACT_OSRAM	0x03
+
+/* PART_ID */
+#define BHSFH_PART		0x90
+#define BHSFH_PART_MASK	0xf0
+#define BHSFH_REV_MASK		0x0f
+#define BHSFH_REV_SHIFT	0
+#define BHSFH_REV_0		0x00
+#define BHSFH_REV_1		0x01
+
+/* Operating modes for both */
+#define BHSFH_STANDBY		0x00
+#define BHSFH_FORCED		0x02
+#define BHSFH_STANDALONE	0x03
+#define BHSFH_SWRESET		(0x01 << 2)
+
+#define BHSFH_PS_TRIG_MEAS	(1 << 0)
+#define BHSFH_ALS_TRIG_MEAS	(1 << 1)
+
+/* Interrupt control */
+#define BHSFH_INT_OUTPUT_MODE	(1 << 3) /* 0 = latched */
+#define BHSFH_INT_POLARITY	(1 << 2) /* 1 = active high */
+#define BHSFH_INT_ALS_ENA	(1 << 1)
+#define BHSFH_INT_PS_ENA	(1 << 0)
+
+/* Interrupt status */
+#define BHSFH_INT_LED1_DATA	(1 << 0)
+#define BHSFH_INT_LED1_INT	(1 << 1)
+#define BHSFH_INT_LED2_DATA	(1 << 2)
+#define BHSFH_INT_LED2_INT	(1 << 3)
+#define BHSFH_INT_LED3_DATA	(1 << 4)
+#define BHSFH_INT_LED3_INT	(1 << 5)
+#define BHSFH_INT_LEDS_INT	((1 << 1) | (1 << 3) | (1 << 5))
+#define BHSFH_INT_ALS_DATA	(1 << 6)
+#define BHSFH_INT_ALS_INT	(1 << 7)
+
+/* Led channels */
+#define BHSFH_LED1		0x00
+
+#define BHSFH_DISABLE		0
+#define BHSFH_ENABLE		1
+#define BHSFH_PROX_CHANNELS	1
+
+/* Following are milliseconds */
+#define BHSFH_LUX_DEFAULT_RATE	200
+#define BHSFH_PROX_DEFAULT_RATE	20
+#define BHSFH_PROX_DEF_RATE_THRESH 200
+#define BHSFH_STARTUP_DELAY	50
+#define BHSFH_RESET_TIME	10
+
+#define BHSFH_LUX_RANGE		65535
+#define BHSFH_PROX_RANGE	255
+#define BHSFH_COEF_SCALER	1024
+#define BHSFH_CALIB_SCALER	8192
+#define BHSFH_LUX_NEUTRAL_CALIB_VALUE (1 * BHSFH_CALIB_SCALER)
+#define BHSFH_LUX_DEF_THRES	1000
+#define BHSFH_PROX_DEF_THRES	70
+#define BHSFH_PROX_DEF_ABS_THRES   100
+#define BHSFH_DEFAULT_PERSISTENCE  10
+#define BHSFH_PROX_MAX_PERSISTENCE 50
+#define BHSFH_LUX_GA_SCALE	16384
+#define BHSFH_LUX_CF_SCALE	2048 /* CF ChipFactor */
+#define BHSFH_NEUTRAL_CF	BHSFH_LUX_CF_SCALE
+#define BHSFH_LUX_CORR_SCALE	4096
+
+#define PROX_ABOVE_THRESHOLD	1
+#define PROX_BELOW_THRESHOLD	0
+
+#define PROX_IGNORE_LUX_LIMIT	500
+
+struct bhsfh_chip {
+	struct bhsfh_platform_data	*pdata;
+	char				chipname[10];
+	u8				revision;
+	struct i2c_client		*client;
+	struct regulator_bulk_data	regs[2];
+
+	struct mutex		mutex; /* avoid parallel access */
+
+	bool			int_mode_prox;
+	bool			int_mode_lux;
+	struct delayed_work	prox_work;
+
+	u32	lux_cf; /* Chip specific factor */
+	u32	lux_ga;
+	u32	lux_calib;
+	int	lux_rate;
+	u32	lux_corr;
+	u16	lux_data_raw;
+	u16	lux_threshold_hi;
+	u16	lux_threshold_lo;
+	u16	lux_thres_hi_onchip;
+	u16	lux_thres_lo_onchip;
+	bool	lux_wait_result;
+
+	int	prox_enable_count;
+	u16	prox_coef;
+	u16	prox_const;
+	int	prox_rate;
+	int	prox_rate_threshold;
+	u8	prox_persistence;
+	u8	prox_persistence_counter;
+	u8	prox_data;
+	u8	prox_threshold;
+	u8	prox_threshold_hw;
+	bool	prox_force_update;
+	u8	prox_abs_thres;
+	u8	prox_led;
+	u8	prox_channels; /* nbr of leds */
+};
+
+static const char reg_vcc[] = "Vcc";
+static const char reg_vleds[] = "Vleds";
+
+/* Supported stand alone rates in ms from chip data sheet */
+static s16 prox_rates[] = {10, 20, 30, 40, 70, 100, 200, 500, 1000, 2000};
+
+/* Supported IR-led currents in mA */
+static const u8 prox_curr_ma[] = {5, 10, 20, 50, 100, 150, 200};
+
+/* Supported stand alone rates in ms from chip data sheet */
+static s16 lux_rates[] = {100, 200, 500, 1000, 2000};
+
+static inline int bhsfh_interrupt_control(struct bhsfh_chip *chip,
+				int lux, int ps)
+{
+	/* Set interrupt modes, interrupt active low, latched */
+	return i2c_smbus_write_byte_data(chip->client,
+					BHSFH_INTERRUPT,
+					(lux << 1) | (ps << 0));
+}
+
+static inline int bhsfh_lux_interrupt_control(struct bhsfh_chip *chip,
+					int lux)
+{
+	chip->int_mode_lux = lux;
+	return bhsfh_interrupt_control(chip, lux, chip->int_mode_prox);
+}
+
+static inline int bhsfh_prox_interrupt_control(struct bhsfh_chip *chip,
+					int ps)
+{
+	chip->int_mode_prox = ps;
+	return bhsfh_interrupt_control(chip, chip->int_mode_lux, ps);
+}
+
+static int bhsfh_lux_rate(struct bhsfh_chip *chip, int rate)
+{
+	int i;
+
+	/* Check that requested rate is supported one */
+	for (i = 0; i < ARRAY_SIZE(lux_rates); i++)
+		if (lux_rates[i] == rate) {
+			chip->lux_rate = rate;
+			break;
+		}
+	if (i == ARRAY_SIZE(lux_rates))
+		return -EINVAL;
+
+	/* sysfs may call this when the chip is powered off */
+	if (pm_runtime_suspended(&chip->client->dev))
+		return 0;
+
+	/* Proper proximity response needs fastest lux rate (100ms) */
+	if (chip->prox_enable_count)
+		i = 0;
+
+	return i2c_smbus_write_byte_data(chip->client,
+					BHSFH_ALS_MEAS_RATE,
+					i);
+}
+
+static int bhsfh_prox_rate(struct bhsfh_chip *chip, int mode)
+{
+	int rate;
+
+	rate = (mode == PROX_ABOVE_THRESHOLD) ?
+		chip->prox_rate_threshold : chip->prox_rate;
+
+	return i2c_smbus_write_byte_data(chip->client,
+					BHSFH_PS_MEAS_RATE,
+					rate);
+}
+
+/* InfraredLED is controlled by the chip during proximity scanning */
+static inline int bhsfh_led_cfg(struct bhsfh_chip *chip)
+{
+	/* LED cfg, current for leds 1 and 2 */
+	return i2c_smbus_write_byte_data(chip->client,
+					BHSFH_I_LED,
+					(BHSFH_LED1 << 6) |
+					(BHSFH_LED_5mA << 3) |
+					chip->prox_led);
+}
+
+/*
+ * Following two functions converts raw ps values from HW to normalized
+ * values. Purpose is to compensate differences between different sensor
+ * versions and variants so that result means about the same between
+ * versions.
+ */
+static inline u8 bhsfh_psraw_to_adjusted(struct bhsfh_chip *chip, u8 psraw)
+{
+	u16 adjusted;
+	adjusted = (u16)(((u32)(psraw + chip->prox_const) * chip->prox_coef) /
+		BHSFH_COEF_SCALER);
+	if (adjusted > BHSFH_PROX_RANGE)
+		adjusted = BHSFH_PROX_RANGE;
+	return adjusted;
+}
+
+static inline u8 bhsfh_psadjusted_to_raw(struct bhsfh_chip *chip, u8 ps)
+{
+	u16 raw;
+
+	raw = (((u32)ps * BHSFH_COEF_SCALER) / chip->prox_coef);
+	if (raw > chip->prox_const)
+		raw = raw - chip->prox_const;
+	else
+		raw = 0;
+	return raw;
+}
+
+/*
+ * Following two functions converts raw lux values from HW to normalized
+ * values. Purpose is to compensate differences between different sensor
+ * versions and variants so that result means about the same between
+ * versions.
+ */
+static int bhsfh_prox_set_threshold(struct bhsfh_chip *chip)
+{
+	u8 tmp = 0;
+
+	/* sysfs may call this when the chip is powered off */
+	if (pm_runtime_suspended(&chip->client->dev))
+		return 0;
+
+	tmp = bhsfh_psadjusted_to_raw(chip, chip->prox_threshold);
+	chip->prox_threshold_hw = tmp;
+
+	return	i2c_smbus_write_byte_data(chip->client, BHSFH_PS_TH_LED1,
+					tmp);
+}
+
+static inline u16 bhsfh_lux_raw_to_adjusted(struct bhsfh_chip *chip, u16 raw)
+{
+	u32 lux;
+	lux = ((u32)raw * chip->lux_corr) / BHSFH_LUX_CORR_SCALE;
+	return min(lux, (u32)BHSFH_LUX_RANGE);
+}
+
+static inline u16 bhsfh_lux_adjusted_to_raw(struct bhsfh_chip *chip,
+					u16 adjusted)
+{
+	return (u32)adjusted * BHSFH_LUX_CORR_SCALE / chip->lux_corr;
+}
+
+static int bhsfh_lux_update_thresholds(struct bhsfh_chip *chip,
+					u16 threshold_hi, u16 threshold_lo)
+{
+	u8 data[4];
+	int ret;
+
+	/* sysfs may call this when the chip is powered off */
+	if (pm_runtime_suspended(&chip->client->dev))
+		return 0;
+
+	/*
+	 * Compensate threshold values with the correction factors if not
+	 * set to minimum or maximum.
+	 * Min & max values disables interrupts.
+	 */
+	if ((threshold_hi != BHSFH_LUX_RANGE) && (threshold_hi != 0))
+		threshold_hi = bhsfh_lux_adjusted_to_raw(chip, threshold_hi);
+
+	if ((threshold_lo != BHSFH_LUX_RANGE) && (threshold_lo != 0))
+		threshold_lo = bhsfh_lux_adjusted_to_raw(chip, threshold_lo);
+
+	if ((chip->lux_thres_hi_onchip == threshold_hi) &&
+		(chip->lux_thres_lo_onchip == threshold_lo))
+		return 0;
+
+	chip->lux_thres_hi_onchip = threshold_hi;
+	chip->lux_thres_lo_onchip = threshold_lo;
+
+	data[0] = threshold_hi;
+	data[1] = threshold_hi >> 8;
+	data[2] = threshold_lo;
+	data[3] = threshold_lo >> 8;
+
+	ret = i2c_smbus_write_i2c_block_data(chip->client,
+					BHSFH_ALS_TH_UP_0,
+					ARRAY_SIZE(data),
+					data);
+	return ret;
+}
+
+static int bhsfh_lux_get_result(struct bhsfh_chip *chip)
+{
+	u16 data;
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(chip->client, BHSFH_ALS_DATA_0);
+	if (ret < 0)
+		return ret;
+
+	data = ret & 0xff;
+	ret = i2c_smbus_read_byte_data(chip->client, BHSFH_ALS_DATA_1);
+	if (ret < 0)
+		return ret;
+
+	chip->lux_data_raw = data | ((ret & 0xff) << 8);
+
+	return 0;
+}
+
+/* Calculate correction value which contains chip and device specific parts */
+static u32 bhsfh_get_corr_value(struct bhsfh_chip *chip)
+{
+	u32 tmp;
+	/* Impact of glass attenuation correction */
+	tmp = (BHSFH_LUX_CORR_SCALE * chip->lux_ga) / BHSFH_LUX_GA_SCALE;
+	/* Impact of chip factor correction */
+	tmp = (tmp * chip->lux_cf) / BHSFH_LUX_CF_SCALE;
+	/* Impact of Device specific calibration correction */
+	tmp = (tmp * chip->lux_calib) / BHSFH_CALIB_SCALER;
+	return tmp;
+}
+
+static int bhsfh_lux_read_result(struct bhsfh_chip *chip)
+{
+	bhsfh_lux_get_result(chip);
+	return bhsfh_lux_raw_to_adjusted(chip, chip->lux_data_raw);
+}
+
+static int bhsfh_chip_on(struct bhsfh_chip *chip)
+{
+	int ret = regulator_bulk_enable(ARRAY_SIZE(chip->regs),
+					chip->regs);
+	if (ret < 0)
+		return ret;
+
+	usleep_range(BHSFH_STARTUP_DELAY, BHSFH_STARTUP_DELAY * 2);
+
+	/* Reset the chip */
+	i2c_smbus_write_byte_data(chip->client, BHSFH_ALS_CONTROL,
+				BHSFH_SWRESET);
+	usleep_range(BHSFH_RESET_TIME, BHSFH_RESET_TIME * 2);
+
+	/*
+	 * ALS is started always since proximity needs als results
+	 * for realibility estimation.
+	 * Let's assume dark until the first ALS measurement is ready.
+	 */
+	chip->lux_data_raw = 0;
+	ret = i2c_smbus_write_byte_data(chip->client,
+					BHSFH_ALS_CONTROL, BHSFH_STANDALONE);
+
+	/* Assume reset defaults */
+	chip->lux_thres_hi_onchip = BHSFH_LUX_RANGE;
+	chip->lux_thres_lo_onchip = 0;
+
+	return ret;
+}
+
+static void bhsfh_chip_off(struct bhsfh_chip *chip)
+{
+	bhsfh_interrupt_control(chip, BHSFH_DISABLE, BHSFH_DISABLE);
+
+	i2c_smbus_write_byte_data(chip->client,
+				BHSFH_ALS_CONTROL, BHSFH_STANDBY);
+	i2c_smbus_write_byte_data(chip->client,
+				BHSFH_PS_CONTROL, BHSFH_STANDBY);
+	regulator_bulk_disable(ARRAY_SIZE(chip->regs), chip->regs);
+}
+
+static int bhsfh_prox_mode_control(struct bhsfh_chip *chip)
+{
+	if (chip->prox_enable_count) {
+		chip->prox_force_update = true; /* Force immediate update */
+
+		bhsfh_lux_rate(chip, chip->lux_rate);
+		bhsfh_prox_set_threshold(chip);
+		bhsfh_led_cfg(chip);
+		bhsfh_prox_rate(chip, PROX_BELOW_THRESHOLD);
+		bhsfh_prox_interrupt_control(chip, BHSFH_ENABLE);
+		i2c_smbus_write_byte_data(chip->client,
+					BHSFH_PS_CONTROL, BHSFH_STANDALONE);
+	} else {
+		chip->prox_data = 0;
+		bhsfh_lux_rate(chip, chip->lux_rate);
+		bhsfh_prox_interrupt_control(chip, BHSFH_DISABLE);
+		i2c_smbus_write_byte_data(chip->client,
+					BHSFH_PS_CONTROL, BHSFH_STANDBY);
+	}
+	return 0;
+}
+
+static int bhsfh_prox_rates(struct bhsfh_chip *chip, int rate,
+			int rate_threshold)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(prox_rates); i++)
+		if (prox_rates[i] == rate) {
+			chip->prox_rate = i;
+			break;
+		}
+
+	if (i == ARRAY_SIZE(prox_rates))
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(prox_rates); i++)
+		if (prox_rates[i] == rate_threshold) {
+			chip->prox_rate_threshold = i;
+			return 0;
+		}
+
+	return -EINVAL;
+}
+
+static int bhsfh_prox_read_result(struct bhsfh_chip *chip)
+{
+	int ret;
+	bool above;
+	u8 mode;
+
+	ret = i2c_smbus_read_byte_data(chip->client, BHSFH_PS_DATA_LED1);
+	if (ret < 0)
+		goto out;
+
+	if (ret > chip->prox_threshold_hw)
+		above = true;
+	else
+		above = false;
+
+	/*
+	 * when ALS levels goes above limit, proximity result may be
+	 * false proximity. Thus ignore the result. With real proximity
+	 * there is a shadow causing low als levels.
+	 */
+	if (chip->lux_data_raw > PROX_IGNORE_LUX_LIMIT)
+		ret = 0;
+
+	chip->prox_data = bhsfh_psraw_to_adjusted(chip, ret);
+
+	/* Strong proximity level or force mode requires immediate response */
+	if ((chip->prox_data >= chip->prox_abs_thres) ||
+	    (chip->prox_force_update))
+		chip->prox_persistence_counter = chip->prox_persistence;
+
+	chip->prox_force_update = false;
+
+	/* Persistence filttering to reduce false proximity events */
+	if (likely(above)) {
+		if (chip->prox_persistence_counter < chip->prox_persistence) {
+			chip->prox_persistence_counter++;
+			ret = -ENODATA;
+		} else {
+			mode = PROX_ABOVE_THRESHOLD;
+			ret = 0;
+		}
+	} else {
+		chip->prox_persistence_counter = 0;
+		mode = PROX_BELOW_THRESHOLD;
+		chip->prox_data = 0;
+		ret = 0;
+	}
+
+	/* Set proximity detection rate based on above or below value */
+	if (ret == 0) {
+		bhsfh_prox_rate(chip, mode);
+		sysfs_notify(&chip->client->dev.kobj, NULL, "prox0_input");
+	}
+out:
+	return ret;
+}
+
+static int bhsfh_detect(struct bhsfh_chip *chip)
+{
+	struct i2c_client *client = chip->client;
+	s32 ret;
+	u8 manu, part;
+
+	ret = i2c_smbus_read_byte_data(client, BHSFH_MANUFACT_ID);
+	if (ret < 0)
+		goto error;
+	manu = (u8)ret;
+
+	ret = i2c_smbus_read_byte_data(client, BHSFH_PART_ID);
+	if (ret < 0)
+		goto error;
+	part = (u8)ret;
+
+	chip->revision = (part & BHSFH_REV_MASK) >> BHSFH_REV_SHIFT;
+	chip->prox_coef = BHSFH_COEF_SCALER;
+	chip->prox_const = 0;
+	chip->lux_cf = BHSFH_NEUTRAL_CF;
+
+	if ((manu == BHSFH_MANUFACT_ROHM) &&
+	    ((part & BHSFH_PART_MASK)	== BHSFH_PART)) {
+		snprintf(chip->chipname, sizeof(chip->chipname), "BH1770GLC");
+		return 0;
+	}
+
+	if ((manu == BHSFH_MANUFACT_OSRAM) &&
+	    ((part & BHSFH_PART_MASK)	== BHSFH_PART)) {
+		snprintf(chip->chipname, sizeof(chip->chipname), "SFH7770");
+		/* Values selected by comparing different versions */
+		chip->prox_coef = 819; /* 0.8 * BHSFH_COEF_SCALER */
+		chip->prox_const = 40;
+		return 0;
+	}
+
+	ret = -ENODEV;
+error:
+	dev_dbg(&client->dev, "BHSFH or SFH7770 not found\n");
+
+	return ret;
+}
+
+/*
+ * This work is re-scheduled at every proximity interrupt.
+ * If this work is running, it means that there hasn't been any
+ * proximity interrupt in time. Situation is handled as no-proximity.
+ * It would be nice to have low-threshold interrupt or interrupt
+ * when measurement and hi-threshold are both 0. But neither of those exists.
+ * This is a workaroud for missing HW feature.
+ */
+
+static void bhsfh_prox_work(struct work_struct *work)
+{
+	struct bhsfh_chip *chip =
+		container_of(work, struct bhsfh_chip, prox_work.work);
+
+	mutex_lock(&chip->mutex);
+	bhsfh_prox_read_result(chip);
+	mutex_unlock(&chip->mutex);
+}
+
+/* This is threaded irq handler */
+static irqreturn_t bhsfh_irq(int irq, void *data)
+{
+	struct bhsfh_chip *chip = data;
+	int status;
+	int rate = 0;
+
+	mutex_lock(&chip->mutex);
+	status = i2c_smbus_read_byte_data(chip->client, BHSFH_ALS_PS_STATUS);
+
+	/* Acknowledge interrupt by reading this register */
+	i2c_smbus_read_byte_data(chip->client, BHSFH_INTERRUPT);
+
+	/*
+	 * Check if there is fresh data available for als.
+	 * If this is the very first data, update thresholds after that.
+	 */
+	if (status & BHSFH_INT_ALS_DATA) {
+		if (unlikely(chip->lux_wait_result)) {
+			chip->lux_wait_result = false;
+			bhsfh_lux_update_thresholds(chip,
+						chip->lux_threshold_hi,
+						chip->lux_threshold_lo);
+		}
+		bhsfh_lux_get_result(chip);
+	}
+
+	/* Disable interrupt logic to guarantee acknowledgement */
+	i2c_smbus_write_byte_data(chip->client, BHSFH_INTERRUPT,
+				  (0 << 1) | (0 << 0));
+
+	if ((status & BHSFH_INT_ALS_INT))
+		sysfs_notify(&chip->client->dev.kobj, NULL, "lux0_input");
+
+	if (chip->int_mode_prox)
+		if (status & BHSFH_INT_LEDS_INT) {
+			rate = prox_rates[chip->prox_rate_threshold];
+			bhsfh_prox_read_result(chip);
+		}
+
+	/* Re-enable interrupt logic */
+	i2c_smbus_write_byte_data(chip->client, BHSFH_INTERRUPT,
+				  (chip->int_mode_lux << 1) |
+				  (chip->int_mode_prox << 0));
+	mutex_unlock(&chip->mutex);
+
+	/*
+	 * Can't cancel work while keeping mutex since the work uses the
+	 * same mutex.
+	 */
+	if (rate) {
+		/*
+		 * Simulate missing no-proximity interrupt 50ms after the
+		 * next expected interrupt time.
+		 */
+		cancel_delayed_work_sync(&chip->prox_work);
+		schedule_delayed_work(&chip->prox_work,
+				msecs_to_jiffies(rate + 50));
+	}
+	return IRQ_HANDLED;
+}
+
+static ssize_t bhsfh_power_state_store(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	unsigned long value;
+	size_t ret;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+
+	mutex_lock(&chip->mutex);
+	if (value) {
+		pm_runtime_get_sync(dev);
+
+		ret = bhsfh_lux_rate(chip, chip->lux_rate);
+		ret |= bhsfh_lux_interrupt_control(chip, BHSFH_ENABLE);
+
+		if (ret < 0) {
+			pm_runtime_put(dev);
+			goto leave;
+		}
+
+		/* This causes interrupt after the next measurement cycle */
+		bhsfh_lux_update_thresholds(chip, BHSFH_LUX_DEF_THRES,
+					BHSFH_LUX_DEF_THRES);
+		/* Inform that we are waiting for a result from ALS */
+		chip->lux_wait_result = true;
+	} else {
+		pm_runtime_put(dev);
+	}
+	bhsfh_prox_mode_control(chip);
+	ret = count;
+leave:
+	mutex_unlock(&chip->mutex);
+	return ret;
+}
+
+static ssize_t bhsfh_power_state_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", !pm_runtime_suspended(dev));
+}
+
+static ssize_t bhsfh_lux_result_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	ssize_t ret;
+
+	mutex_lock(&chip->mutex);
+	if (pm_runtime_suspended(dev))
+		ret = -EIO; /* Chip is not enabled at all */
+	else if (chip->lux_wait_result)
+		ret = -EAGAIN; /* Waiting for result */
+	else
+		ret = sprintf(buf, "%d\n", bhsfh_lux_read_result(chip));
+	mutex_unlock(&chip->mutex);
+	return ret;
+}
+
+static ssize_t bhsfh_prox_enable_store(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	unsigned long value;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+
+	mutex_lock(&chip->mutex);
+	if (value)
+		chip->prox_enable_count++;
+	else if (chip->prox_enable_count > 0)
+		chip->prox_enable_count--;
+	else
+		goto leave;
+	/* Run control only when chip is power on */
+	if (!pm_runtime_suspended(dev))
+		bhsfh_prox_mode_control(chip);
+leave:
+	mutex_unlock(&chip->mutex);
+	return count;
+}
+
+static ssize_t bhsfh_prox_enable_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%d\n", chip->prox_enable_count);
+}
+
+static ssize_t bhsfh_prox_result_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	ssize_t ret;
+
+	mutex_lock(&chip->mutex);
+	if (chip->prox_enable_count && !pm_runtime_suspended(dev))
+		ret = sprintf(buf, "%d\n", chip->prox_data);
+	else
+		ret = -EIO;
+	mutex_unlock(&chip->mutex);
+	return ret;
+}
+
+static ssize_t bhsfh_get_prox_rate_avail(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	int i;
+	int pos = 0;
+	for (i = ARRAY_SIZE(prox_rates); i > 0; i--)
+		pos += sprintf(buf + pos, "%d ", prox_rates[i - 1]);
+	sprintf(buf + pos - 1, "\n");
+	return pos;
+}
+
+static ssize_t bhsfh_get_prox_rate(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%d %d\n", prox_rates[chip->prox_rate],
+		prox_rates[chip->prox_rate_threshold]);
+}
+
+static ssize_t bhsfh_set_prox_rate(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	int rate = 0, rate_threshold = 0;
+	int ret;
+
+	ret = sscanf(buf, "%d %d", &rate, &rate_threshold);
+	if (ret < 0)
+		return ret;
+
+	if (ret == 0)
+		return count;
+
+	/* Second value is optional */
+	if (ret == 1)
+		rate_threshold = prox_rates[chip->prox_rate_threshold];
+
+	mutex_lock(&chip->mutex);
+	ret = bhsfh_prox_rates(chip, rate, rate_threshold);
+	mutex_unlock(&chip->mutex);
+	if (ret < 0)
+		return ret;
+	return count;
+}
+
+static ssize_t bhsfh_get_prox_thres(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%d\n", chip->prox_threshold);
+}
+
+static ssize_t bhsfh_set_prox_thres(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	unsigned long value;
+	int ret;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+	if (value > BHSFH_PROX_RANGE)
+		return -EINVAL;
+
+	mutex_lock(&chip->mutex);
+	chip->prox_threshold = value;
+	ret = bhsfh_prox_set_threshold(chip);
+	mutex_unlock(&chip->mutex);
+	if (ret < 0)
+		return ret;
+	return count;
+}
+
+static ssize_t bhsfh_prox_persistence_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", chip->prox_persistence);
+}
+
+static ssize_t bhsfh_prox_persistence_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t len)
+{
+	struct bhsfh_chip *chip = dev_get_drvdata(dev);
+	unsigned long value;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+
+	if (value > BHSFH_PROX_MAX_PERSISTENCE)
+		return -EINVAL;
+
+	chip->prox_persistence = value;
+
+	return len;
+}
+
+static ssize_t bhsfh_prox_abs_thres_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip = dev_get_drvdata(dev);
+	return snprintf(buf, PAGE_SIZE, "%u\n", chip->prox_abs_thres);
+}
+
+static ssize_t bhsfh_prox_abs_thres_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t len)
+{
+	struct bhsfh_chip *chip = dev_get_drvdata(dev);
+	unsigned long value;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+
+	if (value > BHSFH_PROX_RANGE)
+		return -EINVAL;
+
+	chip->prox_abs_thres = value;
+
+	return len;
+}
+
+static ssize_t bhsfh_chip_id_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%s rev %d\n", chip->chipname, chip->revision);
+}
+
+static ssize_t bhsfh_lux_calib_format_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", BHSFH_CALIB_SCALER);
+}
+
+static ssize_t bhsfh_lux_calib_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip = dev_get_drvdata(dev);
+	return snprintf(buf, PAGE_SIZE, "%u\n", chip->lux_calib);
+}
+
+static ssize_t bhsfh_lux_calib_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct bhsfh_chip *chip = dev_get_drvdata(dev);
+	unsigned long value;
+	u32 old_calib;
+	u32 new_corr;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+
+	mutex_lock(&chip->mutex);
+	old_calib = chip->lux_calib;
+	chip->lux_calib = value;
+	new_corr = bhsfh_get_corr_value(chip);
+	if (new_corr == 0) {
+		chip->lux_calib = old_calib;
+		mutex_unlock(&chip->mutex);
+		return -EINVAL;
+	}
+	chip->lux_corr = new_corr;
+	mutex_unlock(&chip->mutex);
+
+	return len;
+}
+
+static ssize_t bhsfh_get_lux_rate_avail(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	int i;
+	int pos = 0;
+	for (i = ARRAY_SIZE(lux_rates); i > 0; i--)
+		pos += sprintf(buf + pos, "%d ", lux_rates[i - 1]);
+	sprintf(buf + pos - 1, "\n");
+	return pos;
+}
+
+static ssize_t bhsfh_get_lux_rate(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%d\n", chip->lux_rate);
+}
+
+static ssize_t bhsfh_set_lux_rate(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	unsigned long rate;
+	int ret;
+
+	if (strict_strtoul(buf, 0, &rate))
+		return -EINVAL;
+
+	mutex_lock(&chip->mutex);
+	ret = bhsfh_lux_rate(chip, rate);
+	mutex_unlock(&chip->mutex);
+
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static ssize_t bhsfh_get_lux_thres_range(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%d %d\n", chip->lux_threshold_lo,
+		chip->lux_threshold_hi);
+}
+
+static ssize_t bhsfh_set_lux_thres_range(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
+	int thres_lo;
+	int thres_hi;
+	int ret;
+
+	ret = sscanf(buf, "%d %d", &thres_lo, &thres_hi);
+	if (ret < 0)
+		return ret;
+
+	if (ret != 2)
+		return -EINVAL;
+
+	if (thres_lo > thres_hi)
+		return -EINVAL;
+
+	if (thres_hi > BHSFH_LUX_RANGE)
+		return -EINVAL;
+
+	if (thres_lo < 0)
+		return -EINVAL;
+
+	mutex_lock(&chip->mutex);
+	chip->lux_threshold_lo = thres_lo;
+	chip->lux_threshold_hi = thres_hi;
+	/*
+	 * Don't update values in HW if we are still waiting for
+	 * first interrupt to come after device handle open call.
+	 */
+	if (!chip->lux_wait_result)
+		ret = bhsfh_lux_update_thresholds(chip, thres_hi, thres_lo);
+	mutex_unlock(&chip->mutex);
+
+	if (ret < 0)
+		return ret;
+	return len;
+}
+
+static DEVICE_ATTR(prox0_enable, S_IRUGO | S_IWUSR, bhsfh_prox_enable_show,
+						 bhsfh_prox_enable_store);
+static DEVICE_ATTR(prox0_abs_thres, S_IRUGO | S_IWUSR,
+					      bhsfh_prox_abs_thres_show,
+					      bhsfh_prox_abs_thres_store);
+static DEVICE_ATTR(prox0_input, S_IRUGO, bhsfh_prox_result_show, NULL);
+static DEVICE_ATTR(prox0_persistence, S_IRUGO | S_IWUSR,
+					      bhsfh_prox_persistence_show,
+					      bhsfh_prox_persistence_store);
+static DEVICE_ATTR(prox0_rate, S_IRUGO | S_IWUSR, bhsfh_get_prox_rate,
+					       bhsfh_set_prox_rate);
+static DEVICE_ATTR(prox0_rate_avail, S_IRUGO, bhsfh_get_prox_rate_avail, NULL);
+static DEVICE_ATTR(prox0_threshold, S_IRUGO | S_IWUSR, bhsfh_get_prox_thres,
+						    bhsfh_set_prox_thres);
+
+static DEVICE_ATTR(lux0_calib, S_IRUGO | S_IWUSR, bhsfh_lux_calib_show,
+						 bhsfh_lux_calib_store);
+static DEVICE_ATTR(lux0_calib_format, S_IRUGO, bhsfh_lux_calib_format_show,
+					      NULL);
+static DEVICE_ATTR(lux0_input, S_IRUGO, bhsfh_lux_result_show, NULL);
+static DEVICE_ATTR(lux0_rate, S_IRUGO | S_IWUSR, bhsfh_get_lux_rate,
+						bhsfh_set_lux_rate);
+static DEVICE_ATTR(lux0_rate_avail, S_IRUGO, bhsfh_get_lux_rate_avail, NULL);
+static DEVICE_ATTR(lux0_threshold_range, S_IRUGO | S_IWUSR,
+					      bhsfh_get_lux_thres_range,
+					      bhsfh_set_lux_thres_range);
+
+static DEVICE_ATTR(chip_id, S_IRUGO, bhsfh_chip_id_show, NULL);
+static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR, bhsfh_power_state_show,
+						 bhsfh_power_state_store);
+
+
+static struct attribute *sysfs_attrs[] = {
+	&dev_attr_lux0_calib.attr,
+	&dev_attr_lux0_calib_format.attr,
+	&dev_attr_lux0_input.attr,
+	&dev_attr_lux0_rate.attr,
+	&dev_attr_lux0_rate_avail.attr,
+	&dev_attr_lux0_threshold_range.attr,
+	&dev_attr_prox0_abs_thres.attr,
+	&dev_attr_prox0_input.attr,
+	&dev_attr_prox0_enable.attr,
+	&dev_attr_prox0_persistence.attr,
+	&dev_attr_prox0_rate.attr,
+	&dev_attr_prox0_rate_avail.attr,
+	&dev_attr_prox0_threshold.attr,
+	&dev_attr_chip_id.attr,
+	&dev_attr_power_state.attr,
+	NULL
+};
+
+static struct attribute_group bhsfh_attribute_group = {
+	.attrs = sysfs_attrs
+};
+
+static int __devinit bhsfh_probe(struct i2c_client *client,
+				const struct i2c_device_id *id)
+{
+	struct bhsfh_chip *chip;
+	int err;
+
+	chip = kzalloc(sizeof *chip, GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, chip);
+	chip->client  = client;
+
+	mutex_init(&chip->mutex);
+	INIT_DELAYED_WORK(&chip->prox_work, bhsfh_prox_work);
+
+	if (client->dev.platform_data == NULL) {
+		dev_err(&client->dev, "platform data is mandatory\n");
+		err = -EINVAL;
+		goto fail1;
+	}
+
+	chip->pdata		= client->dev.platform_data;
+	chip->lux_calib		= BHSFH_LUX_NEUTRAL_CALIB_VALUE;
+	chip->lux_rate		= BHSFH_LUX_DEFAULT_RATE;
+	chip->lux_threshold_lo	= BHSFH_LUX_DEF_THRES;
+	chip->lux_threshold_hi	= BHSFH_LUX_DEF_THRES;
+
+	if (chip->pdata->glass_attenuation == 0)
+		chip->lux_ga = BHFSH_NEUTRAL_GA;
+	else
+		chip->lux_ga = chip->pdata->glass_attenuation;
+
+	chip->prox_threshold	= BHSFH_PROX_DEF_THRES;
+	chip->prox_led		= chip->pdata->led_def_curr;
+	chip->prox_abs_thres	= BHSFH_PROX_DEF_ABS_THRES;
+	chip->prox_persistence	= BHSFH_DEFAULT_PERSISTENCE;
+	chip->prox_data		= 0;
+
+	bhsfh_prox_rates(chip, BHSFH_PROX_DEFAULT_RATE,
+			BHSFH_PROX_DEF_RATE_THRESH);
+
+	chip->regs[0].supply = reg_vcc;
+	chip->regs[1].supply = reg_vleds;
+
+	err = regulator_bulk_get(&client->dev,
+				 ARRAY_SIZE(chip->regs), chip->regs);
+	if (err < 0) {
+		dev_err(&client->dev, "Cannot get regulators\n");
+		goto fail1;
+	}
+
+	err = regulator_bulk_enable(ARRAY_SIZE(chip->regs), chip->regs);
+	if (err < 0) {
+		dev_err(&client->dev, "Cannot enable regulators\n");
+		goto fail2;
+	}
+
+	usleep_range(BHSFH_STARTUP_DELAY, BHSFH_STARTUP_DELAY * 2);
+	err = bhsfh_detect(chip);
+	if (err < 0)
+		goto fail3;
+
+	/* Start chip */
+	bhsfh_chip_on(chip);
+	pm_runtime_set_active(&client->dev);
+	pm_runtime_enable(&client->dev);
+
+	chip->lux_corr = bhsfh_get_corr_value(chip);
+	if (chip->lux_corr == 0) {
+		dev_err(&client->dev, "Improper correction values\n");
+		err = -EINVAL;
+		goto fail3;
+	}
+
+	if (chip->pdata->setup_resources) {
+		err = chip->pdata->setup_resources();
+		if (err) {
+			err = -EINVAL;
+			goto fail3;
+		}
+	}
+
+	err = sysfs_create_group(&chip->client->dev.kobj,
+				&bhsfh_attribute_group);
+	if (err < 0) {
+		dev_err(&chip->client->dev, "Sysfs registration failed\n");
+		goto fail4;
+	}
+
+	/*
+	 * Chip needs level triggered interrupt to work. However,
+	 * level triggering doesn't work always correctly with power
+	 * management. Select both
+	 */
+	err = request_threaded_irq(client->irq, NULL,
+				bhsfh_irq,
+				IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
+				IRQF_TRIGGER_LOW,
+				"bhsfh", chip);
+	if (err) {
+		dev_err(&client->dev, "could not get IRQ %d\n",
+			client->irq);
+		goto fail5;
+	}
+	regulator_bulk_disable(ARRAY_SIZE(chip->regs), chip->regs);
+	return err;
+fail5:
+	sysfs_remove_group(&chip->client->dev.kobj,
+			&bhsfh_attribute_group);
+fail4:
+	if (chip->pdata->release_resources)
+		chip->pdata->release_resources();
+fail3:
+	regulator_bulk_disable(ARRAY_SIZE(chip->regs), chip->regs);
+fail2:
+	regulator_bulk_free(ARRAY_SIZE(chip->regs), chip->regs);
+fail1:
+	kfree(chip);
+	return err;
+}
+
+static int __devexit bhsfh_remove(struct i2c_client *client)
+{
+	struct bhsfh_chip *chip = i2c_get_clientdata(client);
+
+	free_irq(client->irq, chip);
+
+	sysfs_remove_group(&chip->client->dev.kobj,
+			&bhsfh_attribute_group);
+
+	if (chip->pdata->release_resources)
+		chip->pdata->release_resources();
+
+	cancel_delayed_work_sync(&chip->prox_work);
+
+	if (!pm_runtime_suspended(&client->dev))
+		bhsfh_chip_off(chip);
+
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+
+	regulator_bulk_free(ARRAY_SIZE(chip->regs), chip->regs);
+	kfree(chip);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int bhsfh_suspend(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct bhsfh_chip *chip = i2c_get_clientdata(client);
+
+	bhsfh_chip_off(chip);
+
+	return 0;
+}
+
+static int bhsfh_resume(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct bhsfh_chip *chip = i2c_get_clientdata(client);
+
+	bhsfh_chip_on(chip);
+
+	return 0;
+}
+
+#else
+#define bhsfh_suspend  NULL
+#define bhsfh_shutdown NULL
+#define bhsfh_resume   NULL
+#endif
+
+#ifdef CONFIG_PM_RUNTIME
+static int bhsfh_runtime_suspend(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct bhsfh_chip *chip = i2c_get_clientdata(client);
+
+	bhsfh_chip_off(chip);
+
+	return 0;
+}
+
+static int bhsfh_runtime_resume(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct bhsfh_chip *chip = i2c_get_clientdata(client);
+
+	bhsfh_chip_on(chip);
+
+	return 0;
+}
+#endif
+
+static const struct i2c_device_id bhsfh_id[] = {
+	{"bh1770glc", 0 },
+	{"sfh7770", 0 },
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, bhsfh_id);
+
+static const struct dev_pm_ops bhsfh_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(bhsfh_suspend, bhsfh_resume)
+	SET_RUNTIME_PM_OPS(bhsfh_runtime_suspend, bhsfh_runtime_resume, NULL)
+};
+
+static struct i2c_driver bhsfh_driver = {
+	.driver	 = {
+		.name	= "bhsfh",
+		.owner	= THIS_MODULE,
+		.pm	= &bhsfh_pm_ops,
+	},
+	.probe	  = bhsfh_probe,
+	.remove	  = __devexit_p(bhsfh_remove),
+	.id_table = bhsfh_id,
+};
+
+static int __init bhsfh_init(void)
+{
+	return i2c_add_driver(&bhsfh_driver);
+}
+
+static void __exit bhsfh_exit(void)
+{
+	i2c_del_driver(&bhsfh_driver);
+}
+
+MODULE_DESCRIPTION("BH1770GLC / SFH7770 combined ALS and proximity sensor");
+MODULE_AUTHOR("Samu Onkalo, Nokia Corporation");
+MODULE_LICENSE("GPL v2");
+
+module_init(bhsfh_init);
+module_exit(bhsfh_exit);
diff --git a/include/linux/i2c/bhsfh.h b/include/linux/i2c/bhsfh.h
new file mode 100644
index 0000000..3e0ee86
--- /dev/null
+++ b/include/linux/i2c/bhsfh.h
@@ -0,0 +1,42 @@
+/*
+ * This file is part of the ROHM BH1770GLC / OSRAM SFH7770 sensor driver.
+ * Chip is combined proximity and ambient light sensor.
+ *
+ * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * Contact: Samu Onkalo <samu.p.onkalo@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#ifndef __BHSFH_H__
+#define __BHSFH_H__
+
+struct bhsfh_platform_data {
+#define BHSFH_LED_5mA   0
+#define BHSFH_LED_10mA  1
+#define BHSFH_LED_20mA  2
+#define BHSFH_LED_50mA  3
+#define BHSFH_LED_100mA 4
+#define BHSFH_LED_150mA 5
+#define BHSFH_LED_200mA 6
+	__u8 led_def_curr;
+#define BHFSH_NEUTRAL_GA 16384 /* 16384 / 16384 = 1 */
+	__u32 glass_attenuation;
+	int (*setup_resources)(void);
+	int (*release_resources)(void);
+};
+#endif
-- 
1.6.0.4

Re: [PATCH 1/5] misc: Driver for bh1770glc / sfh7770 ALS and proximity sensor

[Alan Cox]

On Tue,  5 Oct 2010 12:42:55 +0300
Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org> wrote:

> This is a driver for ROHM BH1770GLC and OSRAM SFH7770 combined
> ALS and proximity sensor.

Same comment about regulators.


> +/* Supported stand alone rates in ms from chip data sheet */
> +static s16 prox_rates[] = {10, 20, 30, 40, 70, 100, 200, 500, 1000, 2000};
> +
> +/* Supported IR-led currents in mA */
> +static const u8 prox_curr_ma[] = {5, 10, 20, 50, 100, 150, 200};
> +
> +/* Supported stand alone rates in ms from chip data sheet */
> +static s16 lux_rates[] = {100, 200, 500, 1000, 2000};

Any reason only one of the three is const ?


> +static int bhsfh_prox_rates(struct bhsfh_chip *chip, int rate,
> +			int rate_threshold)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(prox_rates); i++)
> +		if (prox_rates[i] == rate) {
> +			chip->prox_rate = i;
> +			break;
> +		}
> +
> +	if (i == ARRAY_SIZE(prox_rates))
> +		return -EINVAL;
> +
> +	for (i = 0; i < ARRAY_SIZE(prox_rates); i++)
> +		if (prox_rates[i] == rate_threshold) {
> +			chip->prox_rate_threshold = i;
> +			return 0;
> +		}
> +
> +	return -EINVAL;

This makes it hard for generic code. Wouldn't picking the best (first at
least as good as required) be a bit more polite to user space ?


> +static ssize_t bhsfh_lux_result_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
> +	ssize_t ret;
> +
> +	mutex_lock(&chip->mutex);
> +	if (pm_runtime_suspended(dev))
> +		ret = -EIO; /* Chip is not enabled at all */
> +	else if (chip->lux_wait_result)
> +		ret = -EAGAIN; /* Waiting for result */

This makes no sense because you can't poll() a sysfs file



> +static ssize_t bhsfh_lux_calib_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct bhsfh_chip *chip = dev_get_drvdata(dev);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", chip->lux_calib);
> +}

This is short chip->mutex locks as you sometimes temporarily change the
value (error path below)

> +
> +static ssize_t bhsfh_lux_calib_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t len)
> +{
> +	struct bhsfh_chip *chip = dev_get_drvdata(dev);
> +	unsigned long value;
> +	u32 old_calib;
> +	u32 new_corr;
> +
> +	if (strict_strtoul(buf, 0, &value))
> +		return -EINVAL;
> +
> +	mutex_lock(&chip->mutex);
> +	old_calib = chip->lux_calib;
> +	chip->lux_calib = value;
> +	new_corr = bhsfh_get_corr_value(chip);
> +	if (new_corr == 0) {
> +		chip->lux_calib = old_calib;
> +		mutex_unlock(&chip->mutex);
> +		return -EINVAL;
> +	}
> +	chip->lux_corr = new_corr;
> +	mutex_unlock(&chip->mutex);
> +
> +	return len;

Re: [PATCH 1/5] misc: Driver for bh1770glc / sfh7770 ALS and proximity sensor

[Onkalo Samu]

Alan, thanks for comments

-Samu

On Tue, 2010-10-05 at 13:21 +0200, ext Alan Cox wrote:
> On Tue,  5 Oct 2010 12:42:55 +0300
> Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org> wrote:
> 
> > This is a driver for ROHM BH1770GLC and OSRAM SFH7770 combined
> > ALS and proximity sensor.
> 
> Same comment about regulators.

?

> 
> 
> > +/* Supported stand alone rates in ms from chip data sheet */
> > +static s16 prox_rates[] = {10, 20, 30, 40, 70, 100, 200, 500, 1000, 2000};
> > +
> > +/* Supported IR-led currents in mA */
> > +static const u8 prox_curr_ma[] = {5, 10, 20, 50, 100, 150, 200};
> > +
> > +/* Supported stand alone rates in ms from chip data sheet */
> > +static s16 lux_rates[] = {100, 200, 500, 1000, 2000};
> 
> Any reason only one of the three is const ?

No real reason.

> 
> 
> > +static int bhsfh_prox_rates(struct bhsfh_chip *chip, int rate,
> > +			int rate_threshold)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(prox_rates); i++)
> > +		if (prox_rates[i] == rate) {
> > +			chip->prox_rate = i;
> > +			break;
> > +		}
> > +
> > +	if (i == ARRAY_SIZE(prox_rates))
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(prox_rates); i++)
> > +		if (prox_rates[i] == rate_threshold) {
> > +			chip->prox_rate_threshold = i;
> > +			return 0;
> > +		}
> > +
> > +	return -EINVAL;
> 
> This makes it hard for generic code. Wouldn't picking the best (first at
> least as good as required) be a bit more polite to user space ?

Well, perhaps it is better to have hardcoded (or platform specific)
rates instead of control interface. I'll change that.

> 
> 
> > +static ssize_t bhsfh_lux_result_show(struct device *dev,
> > +				   struct device_attribute *attr, char *buf)
> > +{
> > +	struct bhsfh_chip *chip =  dev_get_drvdata(dev);
> > +	ssize_t ret;
> > +
> > +	mutex_lock(&chip->mutex);
> > +	if (pm_runtime_suspended(dev))
> > +		ret = -EIO; /* Chip is not enabled at all */
> > +	else if (chip->lux_wait_result)
> > +		ret = -EAGAIN; /* Waiting for result */
> 
> This makes no sense because you can't poll() a sysfs file

ok, what should be returned when there is no valid results available?

> 
> 
> 
> > +static ssize_t bhsfh_lux_calib_show(struct device *dev,
> > +				 struct device_attribute *attr, char *buf)
> > +{
> > +	struct bhsfh_chip *chip = dev_get_drvdata(dev);
> > +	return snprintf(buf, PAGE_SIZE, "%u\n", chip->lux_calib);
> > +}
> 
> This is short chip->mutex locks as you sometimes temporarily change the
> value (error path below)
> 

?

> > +
> > +static ssize_t bhsfh_lux_calib_store(struct device *dev,
> > +				  struct device_attribute *attr,
> > +				  const char *buf, size_t len)
> > +{
> > +	struct bhsfh_chip *chip = dev_get_drvdata(dev);
> > +	unsigned long value;
> > +	u32 old_calib;
> > +	u32 new_corr;
> > +
> > +	if (strict_strtoul(buf, 0, &value))
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&chip->mutex);
> > +	old_calib = chip->lux_calib;
> > +	chip->lux_calib = value;
> > +	new_corr = bhsfh_get_corr_value(chip);
> > +	if (new_corr == 0) {
> > +		chip->lux_calib = old_calib;
> > +		mutex_unlock(&chip->mutex);
> > +		return -EINVAL;
> > +	}
> > +	chip->lux_corr = new_corr;
> > +	mutex_unlock(&chip->mutex);
> > +
> > +	return len;

Re: [PATCH 1/5] misc: Driver for bh1770glc / sfh7770 ALS and proximity sensor

[Alan Cox]

On Tue, 05 Oct 2010 14:29:50 +0300
Onkalo Samu <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org> wrote:

> 
> Alan, thanks for comments
> 
> -Samu
> 
> On Tue, 2010-10-05 at 13:21 +0200, ext Alan Cox wrote:
> > On Tue,  5 Oct 2010 12:42:55 +0300
> > Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org> wrote:
> > 
> > > This is a driver for ROHM BH1770GLC and OSRAM SFH7770 combined
> > > ALS and proximity sensor.
> > 
> > Same comment about regulators.
> 
> ?

As the other review for the light sensor


> > > +	for (i = 0; i < ARRAY_SIZE(prox_rates); i++)
> > > +		if (prox_rates[i] == rate_threshold) {
> > > +			chip->prox_rate_threshold = i;
> > > +			return 0;
> > > +		}
> > > +
> > > +	return -EINVAL;
> > 
> > This makes it hard for generic code. Wouldn't picking the best (first at
> > least as good as required) be a bit more polite to user space ?
> 
> Well, perhaps it is better to have hardcoded (or platform specific)
> rates instead of control interface. I'll change that.

I don't see a problem with a rate setting where you ask for 'at least X'
and get the nearest ? Ie swap the == for a >= ?

> > This makes no sense because you can't poll() a sysfs file
> 
> ok, what should be returned when there is no valid results available?

I have the same problem with the isl29020 - I think you have to block,
there isn't any other sane response. A user given -EAGAIN simply doesn't
know what to do


> > > +static ssize_t bhsfh_lux_calib_show(struct device *dev,
> > > +				 struct device_attribute *attr, char *buf)
> > > +{
> > > +	struct bhsfh_chip *chip = dev_get_drvdata(dev);
> > > +	return snprintf(buf, PAGE_SIZE, "%u\n", chip->lux_calib);
> > > +}
> > 
> > This is short chip->mutex locks as you sometimes temporarily change the
> > value (error path below)
> > 
> 
> ?

If I am doing a calib store and a calib show in parallel then the show
can give a bogus answer because of this bit of code

> > > +	mutex_lock(&chip->mutex);
> > > +	old_calib = chip->lux_calib;
> > > +	chip->lux_calib = value;
> > > +	new_corr = bhsfh_get_corr_value(chip);
> > > +	if (new_corr == 0) {

[Show can occur here]

> > > +		chip->lux_calib = old_calib;
> > > +		mutex_unlock(&chip->mutex);
> > > +		return -EINVAL;
> > > +	}
> > > +	chip->lux_corr = new_corr;
> > > +	mutex_unlock(&chip->mutex);
> > > +
> > > +	return len;


-- 
--
	"Alan, I'm getting a bit worried about you."
				-- Linus Torvalds

Re: [PATCH 1/5] misc: Driver for bh1770glc / sfh7770 ALS and proximity sensor

[Onkalo Samu]

On Tue, 2010-10-05 at 14:01 +0200, ext Alan Cox wrote:
> On Tue, 05 Oct 2010 14:29:50 +0300

> > > > +	for (i = 0; i < ARRAY_SIZE(prox_rates); i++)
> > > > +		if (prox_rates[i] == rate_threshold) {
> > > > +			chip->prox_rate_threshold = i;
> > > > +			return 0;
> > > > +		}
> > > > +
> > > > +	return -EINVAL;
> > > 
> > > This makes it hard for generic code. Wouldn't picking the best (first at
> > > least as good as required) be a bit more polite to user space ?
> > 
> > Well, perhaps it is better to have hardcoded (or platform specific)
> > rates instead of control interface. I'll change that.
> 
> I don't see a problem with a rate setting where you ask for 'at least X'
> and get the nearest ? Ie swap the == for a >= ?

ok.

> 
> > > This makes no sense because you can't poll() a sysfs file
> > 
> > ok, what should be returned when there is no valid results available?
> 
> I have the same problem with the isl29020 - I think you have to block,
> there isn't any other sane response. A user given -EAGAIN simply doesn't
> know what to do
> 

-EIO then the chip is not running and otherwise block until data is
available.

> 
> > > > +static ssize_t bhsfh_lux_calib_show(struct device *dev,
> > > > +				 struct device_attribute *attr, char *buf)
> > > > +{
> > > > +	struct bhsfh_chip *chip = dev_get_drvdata(dev);
> > > > +	return snprintf(buf, PAGE_SIZE, "%u\n", chip->lux_calib);
> > > > +}
> > > 
> > > This is short chip->mutex locks as you sometimes temporarily change the
> > > value (error path below)
> > > 
> > 
> > ?

of course...

> If I am doing a calib store and a calib show in parallel then the show
> can give a bogus answer because of this bit of code
> 
> > > > +	mutex_lock(&chip->mutex);
> > > > +	old_calib = chip->lux_calib;
> > > > +	chip->lux_calib = value;
> > > > +	new_corr = bhsfh_get_corr_value(chip);
> > > > +	if (new_corr == 0) {
> 
> [Show can occur here]
> 
> > > > +		chip->lux_calib = old_calib;
> > > > +		mutex_unlock(&chip->mutex);
> > > > +		return -EINVAL;
> > > > +	}
> > > > +	chip->lux_corr = new_corr;
> > > > +	mutex_unlock(&chip->mutex);
> > > > +
> > > > +	return len;
> 
> 

[PATCH 2/5] misc: update bhsfh driver to Kconfig and Makefile

[Samu Onkalo]

Compilation support for bhsfh driver

Signed-off-by: Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
---
 drivers/misc/Kconfig  |   10 ++++++++++
 drivers/misc/Makefile |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index b743312..f2e8065 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -314,6 +314,16 @@ config SENSORS_BH1780
 	  This driver can also be built as a module.  If so, the module
 	  will be called bh1780gli.
 
+config SENSORS_BHSFH
+         tristate "BH1770GLC / SFH7770 combined ALS / Proximity sensor";
+         depends on I2C
+         ---help---
+           Say Y here if you want to build a driver for BH1770GLC / SFH7770
+           combined ambient light and proximity sensor chip
+
+           To compile this driver as a module, choose M here: the
+           module will be called bhsfh. If unsure, say N here.
+
 config HMC6352
 	tristate "Honeywell HMC6352 compass"
 	depends on I2C
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 42eab95..fd5a4b7 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_TIFM_CORE)       	+= tifm_core.o
 obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
 obj-$(CONFIG_PHANTOM)		+= phantom.o
 obj-$(CONFIG_SENSORS_BH1780)	+= bh1780gli.o
+obj-$(CONFIG_SENSORS_BHSFH)	+= bhsfh.o
 obj-$(CONFIG_SGI_IOC4)		+= ioc4.o
 obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
 obj-$(CONFIG_KGDB_TESTS)	+= kgdbts.o
-- 
1.6.0.4

[PATCH 4/5] misc: update apds990x driver to Kconfig and Makefile

[Samu Onkalo]

Compilation support for apds990x driver

Signed-off-by: Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
---
 drivers/misc/Kconfig  |   11 +++++++++++
 drivers/misc/Makefile |    1 +
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index f2e8065..cc7c363 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -324,6 +324,17 @@ config SENSORS_BHSFH
            To compile this driver as a module, choose M here: the
            module will be called bhsfh. If unsure, say N here.
 
+config SENSORS_APDS990X
+	 tristate "APDS990X combined als and proximity sensors"
+	 depends on I2C
+	 default n
+	 ---help---
+	   Say Y here if you want to build a driver for Avago APDS990x
+	   combined ambient light and proximity sensor chip
+
+	   To compile this driver as a module, choose M here: the
+	   module will be called apds990x. If unsure, say N here.
+
 config HMC6352
 	tristate "Honeywell HMC6352 compass"
 	depends on I2C
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index fd5a4b7..b90cea7 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
 obj-$(CONFIG_PHANTOM)		+= phantom.o
 obj-$(CONFIG_SENSORS_BH1780)	+= bh1780gli.o
 obj-$(CONFIG_SENSORS_BHSFH)	+= bhsfh.o
+obj-$(CONFIG_SENSORS_APDS990X)	+= apds990x.o
 obj-$(CONFIG_SGI_IOC4)		+= ioc4.o
 obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
 obj-$(CONFIG_KGDB_TESTS)	+= kgdbts.o
-- 
1.6.0.4

[PATCH 3/5] misc: Driver for APDS990X ALS and proximity sensors

[Samu Onkalo]

This is a driver for Avago APDS990X combined ALS and proximity sensor.

Interface is sysfs based. The driver uses interrupts to provide new data.
The driver supports pm_runtime and regulator frameworks.

See Documentation/misc-devices/apds990x.txt for details

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
---
 drivers/misc/apds990x.c      | 1216 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/apds990x.h |   57 ++
 2 files changed, 1273 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/apds990x.c
 create mode 100644 include/linux/i2c/apds990x.h

diff --git a/drivers/misc/apds990x.c b/drivers/misc/apds990x.c
new file mode 100644
index 0000000..5d86cda
--- /dev/null
+++ b/drivers/misc/apds990x.c
@@ -0,0 +1,1216 @@
+/*
+ * This file is part of the APDS990x sensor driver.
+ * Chip is combined proximity and ambient light sensor.
+ *
+ * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * Contact: Samu Onkalo <samu.p.onkalo@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+#include <linux/regulator/consumer.h>
+#include <linux/pm_runtime.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/i2c/apds990x.h>
+
+/* Register map */
+#define APDS990X_ENABLE	 0x00 /* Enable of states and interrupts */
+#define APDS990X_ATIME	 0x01 /* ALS ADC time  */
+#define APDS990X_PTIME	 0x02 /* Proximity ADC time  */
+#define APDS990X_WTIME	 0x03 /* Wait time  */
+#define APDS990X_AILTL	 0x04 /* ALS interrupt low threshold low byte */
+#define APDS990X_AILTH	 0x05 /* ALS interrupt low threshold hi byte */
+#define APDS990X_AIHTL	 0x06 /* ALS interrupt hi threshold low byte */
+#define APDS990X_AIHTH	 0x07 /* ALS interrupt hi threshold hi byte */
+#define APDS990X_PILTL	 0x08 /* Proximity interrupt low threshold low byte */
+#define APDS990X_PILTH	 0x09 /* Proximity interrupt low threshold hi byte */
+#define APDS990X_PIHTL	 0x0a /* Proximity interrupt hi threshold low byte */
+#define APDS990X_PIHTH	 0x0b /* Proximity interrupt hi threshold hi byte */
+#define APDS990X_PERS	 0x0c /* Interrupt persistence filters */
+#define APDS990X_CONFIG	 0x0d /* Configuration */
+#define APDS990X_PPCOUNT 0x0e /* Proximity pulse count */
+#define APDS990X_CONTROL 0x0f /* Gain control register */
+#define APDS990X_REV	 0x11 /* Revision Number */
+#define APDS990X_ID	 0x12 /* Device ID */
+#define APDS990X_STATUS	 0x13 /* Device status */
+#define APDS990X_CDATAL	 0x14 /* Clear ADC low data register */
+#define APDS990X_CDATAH	 0x15 /* Clear ADC high data register */
+#define APDS990X_IRDATAL 0x16 /* IR ADC low data register */
+#define APDS990X_IRDATAH 0x17 /* IR ADC high data register */
+#define APDS990X_PDATAL	 0x18 /* Proximity ADC low data register */
+#define APDS990X_PDATAH	 0x19 /* Proximity ADC high data register */
+
+/* Control */
+#define APDS990X_MAX_AGAIN	3
+
+/* Enable register */
+#define APDS990X_EN_PIEN	(0x1 << 5)
+#define APDS990X_EN_AIEN	(0x1 << 4)
+#define APDS990X_EN_WEN		(0x1 << 3)
+#define APDS990X_EN_PEN		(0x1 << 2)
+#define APDS990X_EN_AEN		(0x1 << 1)
+#define APDS990X_EN_PON		(0x1 << 0)
+#define APDS990X_EN_DISABLE_ALL 0
+
+/* Status register */
+#define APDS990X_ST_PINT	(0x1 << 5)
+#define APDS990X_ST_AINT	(0x1 << 4)
+
+/* I2C access types */
+#define APDS990x_CMD_TYPE_MASK	(0x03 << 5)
+#define APDS990x_CMD_TYPE_RB	(0x00 << 5) /* Repeated byte */
+#define APDS990x_CMD_TYPE_INC	(0x01 << 5) /* Auto increment */
+#define APDS990x_CMD_TYPE_SPE	(0x03 << 5) /* Special function */
+
+#define APDS990x_ADDR_SHIFT	0
+#define APDS990x_CMD		0x80
+
+/* Interrupt ack commands */
+#define APDS990X_INT_ACK_ALS	0x6
+#define APDS990X_INT_ACK_PS	0x5
+#define APDS990X_INT_ACK_BOTH	0x7
+
+/* ptime */
+#define APDS990X_PTIME_DEFAULT	0xff /* Recommended conversion time 2.7ms*/
+
+/* wtime */
+#define APDS990X_WTIME_DEFAULT	0xee /* ~50ms wait time */
+
+#define APDS990X_TIME_TO_ADC	1024 /* One timetick as ADC count value */
+
+/* ppcount */
+#define APDS990X_PPCOUNT_DEFAULT 5
+
+/* Persistence */
+#define APDS990X_APERS_SHIFT	0
+#define APDS990X_PPERS_SHIFT	4
+
+/* Supported ID:s */
+#define APDS990X_ID_0		0x0
+#define APDS990X_ID_4		0x4
+#define APDS990X_ID_29		0x29
+
+/* pgain and pdiode settings */
+#define APDS_PGAIN_1X	       0x0
+#define APDS_PDIODE_IR	       0x2
+
+#define APDS990X_LUX_OUTPUT_SCALE 10
+
+/* Reverse chip factors for threshold calculation */
+struct reverse_factors {
+	u32 afactor;
+	int cf1;
+	int irf1;
+	int cf2;
+	int irf2;
+};
+
+struct apds990x_chip {
+	struct apds990x_platform_data	*pdata;
+	struct i2c_client		*client;
+	struct mutex			mutex; /* avoid parallel access */
+	struct regulator_bulk_data	regs[2];
+
+	int	prox_en;
+	bool	prox_continuous_mode;
+	bool	lux_wait_fresh_res;
+
+	/* Chip parameters */
+	struct	apds990x_chip_factors	cf;
+	struct	reverse_factors		rcf;
+	u16	atime;		/* als integration time */
+	u16	atime_tot;	/* als reporting period */
+	u16	fsm_time;	/* state machine loop round time */
+	u16	a_max_result;	/* Max possible ADC value with current atime */
+	u8	again_meas;	/* Gain used in last measurement */
+	u8	again_next;	/* Next calculated gain */
+	u8	pgain;
+	u8	pdiode;
+	u8	pdrive;
+	u8	lux_persistence;
+	u8	prox_persistence;
+
+	u32	lux_raw;
+	u32	lux;
+	u16	lux_clear;
+	u16	lux_ir;
+	u16	lux_calib;
+	u32	lux_thres_hi;
+	u32	lux_thres_lo;
+
+	u32	prox_thres;
+	u16	prox_data;
+	u16	prox_calib;
+
+	char	chipname[10];
+	u8	revision;
+};
+
+#define APDS_CALIB_SCALER		8192
+#define APDS_LUX_NEUTRAL_CALIB_VALUE	(1 * APDS_CALIB_SCALER)
+#define APDS_PROX_NEUTRAL_CALIB_VALUE	(1 * APDS_CALIB_SCALER)
+
+#define APDS_PROX_DEF_THRES		600
+#define APDS_PROX_HYSTERESIS		50
+#define APDS_LUX_DEF_THRES_HI		101
+#define APDS_LUX_DEF_THRES_LO		100
+#define APDS_DEFAULT_PROX_PERS		1
+#define APDS_DEFAULT_LUX_PERS		3
+
+#define APDS_STARTUP_DELAY		10 /* ms */
+#define APDS_POWERUP_DELAY		100 /* ms */
+#define APDS_RANGE			65535 /* Same range for both */
+#define APDS_LUX_GAIN_LO_LIMIT		100
+#define APDS_LUX_GAIN_LO_LIMIT_STRICT	25
+
+#define TIMESTEP			87 /* 2.7ms is about 87 / 32 */
+#define TIME_STEP_SCALER		32
+
+#define APDS_LUX_AVERAGING_TIME 50 /* tolerates 50/60Hz ripple */
+
+static const u8 again[]	= {1, 8, 16, 120}; /* ALS gain steps */
+static const u8 ir_currents[]	= {100, 50, 25, 12}; /* IRled currents in mA */
+static const u16 arates[]	= {100, 200, 500, 1000};
+static const u8 avail_apersis[] = {0, 1, 2, 3, 5, 10, 15, 20};
+
+/* Regulators */
+static const char reg_vcc[] = "Vdd";
+static const char reg_vled[] = "Vled";
+
+static int apds990x_read_byte(struct apds990x_chip *chip, u8 reg, u8 *data)
+{
+	struct i2c_client *client = chip->client;
+	s32 ret;
+
+	reg &= ~APDS990x_CMD_TYPE_MASK;
+	reg |= APDS990x_CMD | APDS990x_CMD_TYPE_RB;
+
+	ret = i2c_smbus_read_byte_data(client, reg);
+	*data = ret;
+	return (int)ret;
+}
+
+static int apds990x_read_word(struct apds990x_chip *chip, u8 reg, u16 *data)
+{
+	struct i2c_client *client = chip->client;
+	s32 ret;
+
+	reg &= ~APDS990x_CMD_TYPE_MASK;
+	reg |= APDS990x_CMD | APDS990x_CMD_TYPE_INC;
+
+	ret = i2c_smbus_read_word_data(client, reg);
+	*data = ret;
+	return (int)ret;
+}
+
+static int apds990x_write_byte(struct apds990x_chip *chip, u8 reg, u8 data)
+{
+	struct i2c_client *client = chip->client;
+	s32 ret;
+
+	reg &= ~APDS990x_CMD_TYPE_MASK;
+	reg |= APDS990x_CMD | APDS990x_CMD_TYPE_RB;
+
+	ret = i2c_smbus_write_byte_data(client, reg, data);
+	return (int)ret;
+}
+
+static int apds990x_write_word(struct apds990x_chip *chip, u8 reg, u16 data)
+{
+	struct i2c_client *client = chip->client;
+	s32 ret;
+
+	reg &= ~APDS990x_CMD_TYPE_MASK;
+	reg |= APDS990x_CMD | APDS990x_CMD_TYPE_INC;
+
+	ret = i2c_smbus_write_word_data(client, reg, data);
+	return (int)ret;
+}
+
+static int apds990x_mode_on(struct apds990x_chip *chip)
+{
+	/* ALS is mandatory, proximity optional */
+	u8 reg = APDS990X_EN_AIEN | APDS990X_EN_PON | APDS990X_EN_AEN |
+		APDS990X_EN_WEN;
+
+	if (chip->prox_en)
+		reg |= APDS990X_EN_PIEN | APDS990X_EN_PEN;
+
+	return apds990x_write_byte(chip, APDS990X_ENABLE, reg);
+}
+
+static u16 apds990x_lux_to_threshold(struct apds990x_chip *chip, u32 lux)
+{
+	u32 thres;
+	u32 cpl;
+	u32 ir;
+
+	/*
+	 * Reported LUX value is a combination of the IR and CLEAR channel
+	 * values. However, interrupt threshold is only for clear channel.
+	 * This function approximates needed HW threshold value for a given
+	 * LUX value in the current lightning type.
+	 * IR level compared to visible light varies heavily depending on the
+	 * source of the light
+	 *
+	 * Calculate threshold value for the next measurement period.
+	 * Math: threshold = lux * cpl where
+	 * cpl = atime * again / (glass_attenuation * device_factor)
+	 * (count-per-lux)
+	 *
+	 * First remove calibration. Division by four is to avoid overflow
+	 */
+	lux = lux * (APDS_CALIB_SCALER / 4) / (chip->lux_calib / 4);
+
+	/* Multiplication by 64 is to increase accuracy */
+	cpl = ((u32)chip->atime * (u32)again[chip->again_next] *
+		APDS_PARAM_SCALE * 64) / (chip->cf.ga * chip->cf.df);
+
+	thres = lux * cpl / 64;
+	/*
+	 * Convert IR light from the latest result to match with
+	 * new gain step. This helps to adapt with the current
+	 * source of light.
+	 */
+	ir = (u32)chip->lux_ir * (u32)again[chip->again_next] /
+		(u32)again[chip->again_meas];
+
+	/*
+	 * Compensate count with IR light impact
+	 * IAC1 > IAC2 (see apds990x_get_lux for formulas)
+	 */
+	if (chip->lux_clear * APDS_PARAM_SCALE >=
+		chip->rcf.afactor * chip->lux_ir)
+		thres = (chip->rcf.cf1 * thres + chip->rcf.irf1 * ir) /
+			APDS_PARAM_SCALE;
+	else
+		thres = (chip->rcf.cf2 * thres + chip->rcf.irf2 * ir) /
+			APDS_PARAM_SCALE;
+
+	if (thres >= chip->a_max_result)
+		thres = chip->a_max_result - 1;
+	return thres;
+}
+
+static inline int apds990x_set_atime(struct apds990x_chip *chip, u32 time_ms)
+{
+	u8 reg_value;
+
+	chip->atime = time_ms;
+	/* Formula is specified in the data sheet */
+	reg_value = 256 - ((time_ms * TIME_STEP_SCALER) / TIMESTEP);
+	/* Calculate max ADC value for given integration time */
+	chip->a_max_result = (u16)(256 - reg_value) * APDS990X_TIME_TO_ADC;
+	return apds990x_write_byte(chip, APDS990X_ATIME, reg_value);
+}
+
+static int apds990x_refresh_pthres(struct apds990x_chip *chip, int data)
+{
+	int ret, lo, hi;
+
+	/* If the chip is not in use, don't try to access it */
+	if (pm_runtime_suspended(&chip->client->dev))
+		return 0;
+
+	if (data < chip->prox_thres) {
+		lo = 0;
+		hi = chip->prox_thres;
+	} else {
+		lo = chip->prox_thres - APDS_PROX_HYSTERESIS;
+		if (chip->prox_continuous_mode)
+			hi = chip->prox_thres;
+		else
+			hi = APDS_RANGE;
+	}
+
+	ret = apds990x_write_word(chip, APDS990X_PILTL, lo);
+	ret |= apds990x_write_word(chip, APDS990X_PIHTL, hi);
+	return ret;
+}
+
+static int apds990x_refresh_athres(struct apds990x_chip *chip)
+{
+	int ret;
+	/* If the chip is not in use, don't try to access it */
+	if (pm_runtime_suspended(&chip->client->dev))
+		return 0;
+
+	ret = apds990x_write_word(chip, APDS990X_AILTL,
+			apds990x_lux_to_threshold(chip, chip->lux_thres_lo));
+	ret |= apds990x_write_word(chip, APDS990X_AIHTL,
+			apds990x_lux_to_threshold(chip, chip->lux_thres_hi));
+
+	return ret;
+}
+
+static void apds990x_force_a_refresh(struct apds990x_chip *chip)
+{
+	/* This will force ALS interrupt after the next measurement. */
+	apds990x_write_word(chip, APDS990X_AILTL, APDS_LUX_DEF_THRES_LO);
+	apds990x_write_word(chip, APDS990X_AIHTL, APDS_LUX_DEF_THRES_HI);
+}
+
+static void apds990x_force_p_refresh(struct apds990x_chip *chip)
+{
+	/* This will force proximity interrupt after the next measurement. */
+	apds990x_write_word(chip, APDS990X_PILTL, APDS_PROX_DEF_THRES - 1);
+	apds990x_write_word(chip, APDS990X_PIHTL, APDS_PROX_DEF_THRES);
+}
+
+static int apds990x_calc_again(struct apds990x_chip *chip)
+{
+	int curr_again = chip->again_meas;
+	int next_again = chip->again_meas;
+	int ret = 0;
+
+	/* Calculate suitable als gain */
+	if (chip->lux_clear == chip->a_max_result)
+		next_again -= 2; /* ALS saturated. Decrease gain by 2 steps */
+	else if (chip->lux_clear > chip->a_max_result / 2)
+		next_again--;
+	else if (chip->lux_clear < APDS_LUX_GAIN_LO_LIMIT_STRICT)
+		next_again += 2; /* Too dark. Increase gain by 2 steps */
+	else if (chip->lux_clear < APDS_LUX_GAIN_LO_LIMIT)
+		next_again++;
+
+	/* Let's check can we trust the measured result */
+	if ((next_again < curr_again) &&
+		(chip->lux_clear == chip->a_max_result)) {
+		/*
+		 * Next gain is smaller and current measurement saturated.
+		 * Result can be totally garbage due to saturation
+		 */
+		ret = -ERANGE;
+	} else if ((next_again > curr_again) &&
+		(chip->lux_clear < APDS_LUX_GAIN_LO_LIMIT_STRICT)) {
+		/*
+		 * Next gain is larger and current measurement underflows.
+		 * Result can be totally garbage due to underflow
+		 */
+		ret = -ERANGE;
+	}
+
+	/* Limit gain to available range */
+	if (next_again < 0)
+		next_again = 0;
+	else if (next_again > APDS990X_MAX_AGAIN)
+		next_again = APDS990X_MAX_AGAIN;
+
+	chip->again_next = next_again;
+	apds990x_write_byte(chip, APDS990X_CONTROL,
+			(chip->pdrive << 6) |
+			(chip->pdiode << 4) |
+			(chip->pgain << 2) |
+			(chip->again_next << 0));
+
+	if (ret < 0)
+		apds990x_force_a_refresh(chip); /* Bad result -> re-measure */
+	else
+		apds990x_refresh_athres(chip);
+	return ret;
+}
+
+static int apds990x_get_lux(struct apds990x_chip *chip, int clear, int ir)
+{
+	int iac, iac1, iac2; /* IR adjusted counts */
+	u32 lpc; /* Lux per count */
+
+	/* Formulas:
+	 * iac1 = CF1 * CLEAR_CH - IRF1 * IR_CH
+	 * iac2 = CF2 * CLEAR_CH - IRF2 * IR_CH
+	 */
+	iac1 = (chip->cf.cf1 * clear - chip->cf.irf1 * ir) / APDS_PARAM_SCALE;
+	iac2 = (chip->cf.cf2 * clear - chip->cf.irf2 * ir) / APDS_PARAM_SCALE;
+
+	iac = max(iac1, iac2);
+	iac = max(iac, 0);
+
+	lpc = APDS990X_LUX_OUTPUT_SCALE * (chip->cf.df * chip->cf.ga) /
+		(u32)(again[chip->again_meas] * (u32)chip->atime);
+
+	return (iac * lpc) / APDS_PARAM_SCALE;
+}
+
+static int apds990x_ack_int(struct apds990x_chip *chip, u8 mode)
+{
+	struct i2c_client *client = chip->client;
+	s32 ret;
+	u8 reg = APDS990x_CMD | APDS990x_CMD_TYPE_SPE;
+
+	switch (mode & (APDS990X_ST_AINT | APDS990X_ST_PINT)) {
+	case APDS990X_ST_AINT:
+		reg |= APDS990X_INT_ACK_ALS;
+		break;
+	case APDS990X_ST_PINT:
+		reg |= APDS990X_INT_ACK_PS;
+		break;
+	default:
+		reg |= APDS990X_INT_ACK_BOTH;
+		break;
+	}
+
+	ret = i2c_smbus_read_byte_data(client, reg);
+	return (int)ret;
+}
+
+static irqreturn_t apds990x_irq(int irq, void *data)
+{
+	struct apds990x_chip *chip = data;
+	u8 status;
+
+	apds990x_read_byte(chip, APDS990X_STATUS, &status);
+	apds990x_ack_int(chip, status);
+
+	mutex_lock(&chip->mutex);
+	if (!pm_runtime_suspended(&chip->client->dev)) {
+		if (status & APDS990X_ST_AINT) {
+			apds990x_read_word(chip, APDS990X_CDATAL,
+					&chip->lux_clear);
+			apds990x_read_word(chip, APDS990X_IRDATAL,
+					&chip->lux_ir);
+			/* Store used gain for calculations */
+			chip->again_meas = chip->again_next;
+
+			chip->lux_raw = apds990x_get_lux(chip,
+							chip->lux_clear,
+							chip->lux_ir);
+
+			if (apds990x_calc_again(chip) == 0) {
+				/* Result is valid */
+				chip->lux = chip->lux_raw;
+				chip->lux_wait_fresh_res = false;
+				sysfs_notify(&chip->client->dev.kobj,
+					NULL, "lux0_input");
+			}
+		}
+
+		if (status & APDS990X_ST_PINT)
+			if (chip->prox_en) {
+				u16 clr_ch;
+
+				apds990x_read_word(chip, APDS990X_CDATAL,
+						&clr_ch);
+				/*
+				 * If ALS channel is saturated at min gain,
+				 * proximity gives false posivite values.
+				 * Just ignore them.
+				 */
+				if ((chip->again_meas == 0) &&
+					(clr_ch == chip->a_max_result)) {
+					chip->prox_data = 0;
+				} else {
+					apds990x_read_word(chip,
+							APDS990X_PDATAL,
+							&chip->prox_data);
+				}
+				apds990x_refresh_pthres(chip, chip->prox_data);
+				if (chip->prox_data < chip->prox_thres)
+					chip->prox_data = 0;
+				sysfs_notify(&chip->client->dev.kobj,
+					NULL, "prox0_input");
+			}
+	}
+	mutex_unlock(&chip->mutex);
+	return IRQ_HANDLED;
+}
+
+static int apds990x_configure(struct apds990x_chip *chip)
+{
+	/* It is recommended to use disabled mode during these operations */
+	apds990x_write_byte(chip, APDS990X_ENABLE, APDS990X_EN_DISABLE_ALL);
+
+	/* conversion and wait times for different state machince states */
+	apds990x_write_byte(chip, APDS990X_PTIME, APDS990X_PTIME_DEFAULT);
+	apds990x_write_byte(chip, APDS990X_WTIME, APDS990X_WTIME_DEFAULT);
+	apds990x_set_atime(chip, APDS_LUX_AVERAGING_TIME);
+
+	apds990x_write_byte(chip, APDS990X_CONFIG, 0);
+
+	/*
+	 * Total time for one FSM round without proximity.
+	 * One bit in wait is about 2.7 ms = 27 / 10
+	 */
+	chip->fsm_time = APDS_LUX_AVERAGING_TIME +
+		(256 - APDS990X_WTIME_DEFAULT) * 27 / 10;
+
+	/* Persistence levels */
+	apds990x_write_byte(chip, APDS990X_PERS,
+			(chip->lux_persistence << APDS990X_APERS_SHIFT) |
+			(chip->prox_persistence << APDS990X_PPERS_SHIFT));
+
+	apds990x_write_byte(chip, APDS990X_PPCOUNT, APDS990X_PPCOUNT_DEFAULT);
+
+	/* Start with relatively small gain */
+	chip->again_meas = 1;
+	chip->again_next = 1;
+	apds990x_write_byte(chip, APDS990X_CONTROL,
+			(chip->pdrive << 6) |
+			(chip->pdiode << 4) |
+			(chip->pgain << 2) |
+			(chip->again_next << 0));
+	return 0;
+}
+
+static int apds990x_detect(struct apds990x_chip *chip)
+{
+	struct i2c_client *client = chip->client;
+	int ret;
+	u8 id;
+
+	ret = apds990x_read_byte(chip, APDS990X_ID, &id);
+	if (ret < 0) {
+		dev_err(&client->dev, "ID read failed\n");
+		return ret;
+	}
+
+	ret = apds990x_read_byte(chip, APDS990X_REV, &chip->revision);
+	if (ret < 0) {
+		dev_err(&client->dev, "REV read failed\n");
+		return ret;
+	}
+
+	switch (id) {
+	case APDS990X_ID_0:
+	case APDS990X_ID_4:
+	case APDS990X_ID_29:
+		snprintf(chip->chipname, sizeof(chip->chipname), "APDS-990x");
+		break;
+	default:
+		ret = -ENODEV;
+		break;
+	}
+	return ret;
+}
+
+static int apds990x_chip_on(struct apds990x_chip *chip)
+{
+	int err	 = regulator_bulk_enable(ARRAY_SIZE(chip->regs),
+					chip->regs);
+	if (err < 0)
+		return err;
+
+	usleep_range(APDS_STARTUP_DELAY, 2 * APDS_STARTUP_DELAY);
+
+	/* Refresh all configs in case of regulators were off */
+	apds990x_configure(chip);
+	apds990x_mode_on(chip);
+	return 0;
+}
+
+static int apds990x_chip_off(struct apds990x_chip *chip)
+{
+	apds990x_write_byte(chip, APDS990X_ENABLE, APDS990X_EN_DISABLE_ALL);
+	regulator_bulk_disable(ARRAY_SIZE(chip->regs), chip->regs);
+	return 0;
+}
+
+static ssize_t apds990x_lux_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct apds990x_chip *chip = dev_get_drvdata(dev);
+	u32 result;
+
+	if (pm_runtime_suspended(dev))
+		return -EIO;
+	else if (chip->lux_wait_fresh_res)
+		return -EAGAIN;
+
+	result = (chip->lux * chip->lux_calib) / APDS_CALIB_SCALER;
+	result = result / APDS990X_LUX_OUTPUT_SCALE;
+	if (result > APDS_RANGE)
+		result = APDS_RANGE;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", result);
+}
+
+static DEVICE_ATTR(lux0_input, S_IRUGO, apds990x_lux_show, NULL);
+
+static ssize_t apds990x_lux10x_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct apds990x_chip *chip = dev_get_drvdata(dev);
+	u32 result;
+
+	if (pm_runtime_suspended(dev))
+		return -EIO;
+	else if (chip->lux_wait_fresh_res)
+		return -EAGAIN;
+
+	result = (chip->lux * chip->lux_calib) / APDS_CALIB_SCALER;
+	if (result > (APDS_RANGE * APDS990X_LUX_OUTPUT_SCALE))
+		result = APDS_RANGE * APDS990X_LUX_OUTPUT_SCALE;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", result);
+}
+
+static DEVICE_ATTR(lux0_input10x, S_IRUGO, apds990x_lux10x_show, NULL);
+
+static ssize_t apds990x_lux_calib_format_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", APDS_CALIB_SCALER);
+}
+
+static DEVICE_ATTR(lux0_calib_format, S_IRUGO, apds990x_lux_calib_format_show,
+					      NULL);
+
+static ssize_t apds990x_lux_calib_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct apds990x_chip *chip = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", chip->lux_calib);
+}
+
+static ssize_t apds990x_lux_calib_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct apds990x_chip *chip = dev_get_drvdata(dev);
+	unsigned long value;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+
+	if (chip->lux_calib > APDS_RANGE)
+		return -EINVAL;
+
+	chip->lux_calib = value;
+
+	return len;
+}
+
+static DEVICE_ATTR(lux0_calib, S_IRUGO | S_IWUSR, apds990x_lux_calib_show,
+		apds990x_lux_calib_store);
+
+static ssize_t apds990x_rate_avail(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	int i;
+	int pos = 0;
+	for (i = 0; i < ARRAY_SIZE(arates); i++)
+		pos += sprintf(buf + pos, "%d ", arates[i]);
+	sprintf(buf + pos - 1, "\n");
+	return pos;
+}
+
+static ssize_t apds990x_rate_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%d\n", chip->atime_tot);
+}
+
+static ssize_t apds990x_rate_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	unsigned long value;
+	int ret = len;
+	int i;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(arates); i++)
+		if (value == arates[i])
+			break;
+
+	if (i == ARRAY_SIZE(arates))
+		return -EINVAL;
+
+	mutex_lock(&chip->mutex);
+
+	chip->atime_tot = value;
+
+	/* Convert to available HW values */
+	value /= chip->fsm_time;
+	for (i = 0; i < sizeof(avail_apersis) - 1; i++)
+		if (value >= avail_apersis[i])
+			break;
+	chip->lux_persistence = value;
+
+	/* Persistence levels */
+	apds990x_write_byte(chip, APDS990X_PERS,
+			(chip->lux_persistence << APDS990X_APERS_SHIFT) |
+			(chip->prox_persistence << APDS990X_PPERS_SHIFT));
+
+	mutex_unlock(&chip->mutex);
+
+	return ret;
+}
+
+static DEVICE_ATTR(lux0_rate_avail, S_IRUGO, apds990x_rate_avail, NULL);
+
+static DEVICE_ATTR(lux0_rate, S_IRUGO | S_IWUSR, apds990x_rate_show,
+						 apds990x_rate_store);
+
+static ssize_t apds990x_prox_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	if (pm_runtime_suspended(dev) || !chip->prox_en)
+		return -EIO;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", chip->prox_data);
+}
+
+static DEVICE_ATTR(prox0_input, S_IRUGO, apds990x_prox_show, NULL);
+
+static ssize_t apds990x_prox_enable_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%d\n", chip->prox_en);
+}
+
+static ssize_t apds990x_prox_enable_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	unsigned long value;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+
+	mutex_lock(&chip->mutex);
+	if (value) {
+		chip->prox_en++;
+	} else {
+		if (chip->prox_en > 0)
+			chip->prox_en--;
+	}
+
+	if (!pm_runtime_suspended(dev))
+		apds990x_mode_on(chip);
+	mutex_unlock(&chip->mutex);
+	return len;
+}
+
+static DEVICE_ATTR(prox0_enable, S_IRUGO | S_IWUSR, apds990x_prox_enable_show,
+						   apds990x_prox_enable_store);
+
+static ssize_t apds990x_prox_continuous_mode_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%d\n", chip->prox_continuous_mode);
+}
+
+static ssize_t apds990x_prox_continuous_mode_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	unsigned long value;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+
+	chip->prox_continuous_mode = !!value;
+	return len;
+}
+
+static DEVICE_ATTR(prox0_continuous_mode, S_IRUGO | S_IWUSR,
+		apds990x_prox_continuous_mode_show,
+		apds990x_prox_continuous_mode_store);
+
+static ssize_t apds990x_lux_threshold_range_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%d %d\n", chip->lux_thres_lo, chip->lux_thres_hi);
+}
+
+static ssize_t apds990x_lux_threshold_range_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	int thres_lo;
+	int thres_hi;
+	int ret;
+
+	ret = sscanf(buf, "%d %d", &thres_lo, &thres_hi);
+	if (ret < 0)
+		return ret;
+
+	if (ret != 2)
+		return -EINVAL;
+
+	if (thres_lo > thres_hi)
+		return -EINVAL;
+
+	if (thres_hi > APDS_RANGE * APDS990X_LUX_OUTPUT_SCALE)
+		return -EINVAL;
+
+	if (thres_lo < 0)
+		return -EINVAL;
+
+	mutex_lock(&chip->mutex);
+	chip->lux_thres_lo = thres_lo;
+	chip->lux_thres_hi = thres_hi;
+	if (!chip->lux_wait_fresh_res)
+		apds990x_refresh_athres(chip);
+	mutex_unlock(&chip->mutex);
+
+	return len;
+}
+
+static DEVICE_ATTR(lux0_threshold_range, S_IRUGO | S_IWUSR,
+		apds990x_lux_threshold_range_show,
+		apds990x_lux_threshold_range_store);
+
+static ssize_t apds990x_prox_threshold_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%d\n", chip->prox_thres);
+}
+
+static ssize_t apds990x_prox_threshold_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	unsigned long value;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+
+	if ((value > APDS_RANGE) || (value == 0) ||
+		(value < APDS_PROX_HYSTERESIS))
+		return -EINVAL;
+
+	chip->prox_thres = value;
+
+	apds990x_force_p_refresh(chip);
+	return len;
+}
+
+static DEVICE_ATTR(prox0_threshold, S_IRUGO | S_IWUSR,
+		apds990x_prox_threshold_show,
+		apds990x_prox_threshold_store);
+
+static ssize_t apds990x_power_state_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", !pm_runtime_suspended(dev));
+	return 0;
+}
+
+static ssize_t apds990x_power_state_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	unsigned long value;
+
+	if (strict_strtoul(buf, 0, &value))
+		return -EINVAL;
+	if (value) {
+		chip->lux_wait_fresh_res = true;
+		pm_runtime_get_sync(dev);
+		apds990x_force_a_refresh(chip);
+		apds990x_force_p_refresh(chip);
+	} else {
+		pm_runtime_put(dev);
+	}
+	return len;
+}
+
+static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
+		apds990x_power_state_show,
+		apds990x_power_state_store);
+
+static ssize_t apds990x_chip_id_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct apds990x_chip *chip =  dev_get_drvdata(dev);
+	return sprintf(buf, "%s %d\n", chip->chipname, chip->revision);
+}
+
+static DEVICE_ATTR(chip_id, S_IRUGO, apds990x_chip_id_show, NULL);
+
+static struct attribute *sysfs_attrs_ctrl[] = {
+	&dev_attr_lux0_calib.attr,
+	&dev_attr_lux0_calib_format.attr,
+	&dev_attr_lux0_input.attr,
+	&dev_attr_lux0_input10x.attr,
+	&dev_attr_lux0_rate.attr,
+	&dev_attr_lux0_rate_avail.attr,
+	&dev_attr_lux0_threshold_range.attr,
+	&dev_attr_prox0_enable.attr,
+	&dev_attr_prox0_input.attr,
+	&dev_attr_prox0_continuous_mode.attr,
+	&dev_attr_prox0_threshold.attr,
+	&dev_attr_chip_id.attr,
+	&dev_attr_power_state.attr,
+	NULL
+};
+
+static struct attribute_group apds990x_attribute_group[] = {
+	{.attrs = sysfs_attrs_ctrl },
+};
+
+static int __devinit apds990x_probe(struct i2c_client *client,
+				const struct i2c_device_id *id)
+{
+	struct apds990x_chip *chip;
+	int err;
+
+	chip = kzalloc(sizeof *chip, GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, chip);
+	chip->client  = client;
+
+	mutex_init(&chip->mutex);
+	chip->pdata	= client->dev.platform_data;
+
+	if (chip->pdata == NULL) {
+		dev_err(&client->dev, "platform data is mandatory\n");
+		err = -EINVAL;
+		goto fail1;
+	}
+
+	if (chip->pdata->cf.ga == 0) {
+		/* set uncovered sensor default parameters */
+		chip->cf.ga = 1966; /* 0.48 * APDS_PARAM_SCALE */
+		chip->cf.cf1 = 4096; /* 1.00 * APDS_PARAM_SCALE */
+		chip->cf.irf1 = 9134; /* 2.23 * APDS_PARAM_SCALE */
+		chip->cf.cf2 = 2867; /* 0.70 * APDS_PARAM_SCALE */
+		chip->cf.irf2 = 5816; /* 1.42 * APDS_PARAM_SCALE */
+		chip->cf.df = 52;
+	} else {
+		chip->cf = chip->pdata->cf;
+	}
+
+	/* precalculate inverse chip factors for threshold control */
+	chip->rcf.afactor =
+		(chip->cf.irf1 - chip->cf.irf2) * APDS_PARAM_SCALE /
+		(chip->cf.cf1 - chip->cf.cf2);
+	chip->rcf.cf1 = APDS_PARAM_SCALE * APDS_PARAM_SCALE /
+		chip->cf.cf1;
+	chip->rcf.irf1 = chip->cf.irf1 * APDS_PARAM_SCALE /
+		chip->cf.cf1;
+	chip->rcf.cf2 = APDS_PARAM_SCALE * APDS_PARAM_SCALE /
+		chip->cf.cf2;
+	chip->rcf.irf2 = chip->cf.irf2 * APDS_PARAM_SCALE /
+		chip->cf.cf2;
+
+	/* Set something to start with */
+	chip->lux_thres_hi = APDS_LUX_DEF_THRES_HI;
+	chip->lux_thres_lo = APDS_LUX_DEF_THRES_LO;
+	chip->lux_calib = APDS_LUX_NEUTRAL_CALIB_VALUE;
+	chip->lux_persistence = APDS_DEFAULT_LUX_PERS;
+	chip->atime_tot = 200;
+
+	chip->prox_thres = APDS_PROX_DEF_THRES;
+	chip->pdrive = chip->pdata->pdrive;
+	chip->pdiode = APDS_PDIODE_IR;
+	chip->pgain = APDS_PGAIN_1X;
+	chip->prox_calib = APDS_PROX_NEUTRAL_CALIB_VALUE;
+	chip->prox_persistence = APDS_DEFAULT_PROX_PERS;
+	chip->prox_continuous_mode = false;
+
+	chip->regs[0].supply = reg_vcc;
+	chip->regs[1].supply = reg_vled;
+
+	err = regulator_bulk_get(&client->dev,
+				 ARRAY_SIZE(chip->regs), chip->regs);
+	if (err < 0) {
+		dev_err(&client->dev, "Cannot get regulators\n");
+		goto fail1;
+	}
+
+	err = regulator_bulk_enable(ARRAY_SIZE(chip->regs), chip->regs);
+	if (err < 0) {
+		dev_err(&client->dev, "Cannot enable regulators\n");
+		goto fail2;
+	}
+
+	msleep(APDS_POWERUP_DELAY);
+
+	err = apds990x_detect(chip);
+	if (err < 0) {
+		dev_err(&client->dev, "APDS990X not found\n");
+		goto fail3;
+	}
+
+	apds990x_configure(chip);
+	apds990x_mode_on(chip);
+
+	pm_runtime_set_active(&client->dev);
+	pm_runtime_enable(&client->dev);
+
+	if (chip->pdata->setup_resources) {
+		err = chip->pdata->setup_resources();
+		if (err) {
+			err = -EINVAL;
+			goto fail3;
+		}
+	}
+
+	err = sysfs_create_group(&chip->client->dev.kobj,
+				apds990x_attribute_group);
+	if (err < 0) {
+		dev_err(&chip->client->dev, "Sysfs registration failed\n");
+		goto fail4;
+	}
+
+	err = request_threaded_irq(client->irq, NULL,
+				apds990x_irq,
+				IRQF_TRIGGER_FALLING | IRQF_TRIGGER_LOW |
+				IRQF_ONESHOT,
+				"apds990x", chip);
+	if (err) {
+		dev_err(&client->dev, "could not get IRQ %d\n",
+			client->irq);
+		goto fail5;
+	}
+	return err;
+fail5:
+	sysfs_remove_group(&chip->client->dev.kobj,
+			&apds990x_attribute_group[0]);
+fail4:
+	if (chip->pdata && chip->pdata->release_resources)
+		chip->pdata->release_resources();
+fail3:
+	regulator_bulk_disable(ARRAY_SIZE(chip->regs), chip->regs);
+fail2:
+	regulator_bulk_free(ARRAY_SIZE(chip->regs), chip->regs);
+fail1:
+	kfree(chip);
+	return err;
+}
+
+static int __devexit apds990x_remove(struct i2c_client *client)
+{
+	struct apds990x_chip *chip = i2c_get_clientdata(client);
+
+	free_irq(client->irq, chip);
+	sysfs_remove_group(&chip->client->dev.kobj,
+			apds990x_attribute_group);
+
+	if (chip->pdata && chip->pdata->release_resources)
+		chip->pdata->release_resources();
+
+	if (!pm_runtime_suspended(&client->dev))
+		apds990x_chip_off(chip);
+
+	regulator_bulk_free(ARRAY_SIZE(chip->regs), chip->regs);
+
+	kfree(chip);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int apds990x_suspend(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct apds990x_chip *chip = i2c_get_clientdata(client);
+
+	apds990x_chip_off(chip);
+	return 0;
+}
+
+static int apds990x_resume(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct apds990x_chip *chip = i2c_get_clientdata(client);
+
+	apds990x_chip_on(chip);
+	return 0;
+}
+#else
+#define apds990x_suspend  NULL
+#define apds990x_resume	  NULL
+#define apds990x_shutdown NULL
+#endif
+
+#ifdef CONFIG_PM_RUNTIME
+static int apds990x_runtime_suspend(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct apds990x_chip *chip = i2c_get_clientdata(client);
+
+	apds990x_chip_off(chip);
+	return 0;
+}
+
+static int apds990x_runtime_resume(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct apds990x_chip *chip = i2c_get_clientdata(client);
+
+	apds990x_chip_on(chip);
+	return 0;
+}
+
+#endif
+
+static const struct i2c_device_id apds990x_id[] = {
+	{"apds990x", 0 },
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, apds990x_id);
+
+static const struct dev_pm_ops apds990x_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(apds990x_suspend, apds990x_resume)
+	SET_RUNTIME_PM_OPS(apds990x_runtime_suspend,
+			apds990x_runtime_resume,
+			NULL)
+};
+
+static struct i2c_driver apds990x_driver = {
+	.driver	 = {
+		.name	= "apds990x",
+		.owner	= THIS_MODULE,
+		.pm	= &apds990x_pm_ops,
+	},
+	.probe	  = apds990x_probe,
+	.remove	  = __devexit_p(apds990x_remove),
+	.id_table = apds990x_id,
+};
+
+static int __init apds990x_init(void)
+{
+	return i2c_add_driver(&apds990x_driver);
+}
+
+static void __exit apds990x_exit(void)
+{
+	i2c_del_driver(&apds990x_driver);
+}
+
+MODULE_DESCRIPTION("APDS990X combined ALS and proximity sensor");
+MODULE_AUTHOR("Samu Onkalo, Nokia Corporation");
+MODULE_LICENSE("GPL v2");
+
+module_init(apds990x_init);
+module_exit(apds990x_exit);
diff --git a/include/linux/i2c/apds990x.h b/include/linux/i2c/apds990x.h
new file mode 100644
index 0000000..f256962
--- /dev/null
+++ b/include/linux/i2c/apds990x.h
@@ -0,0 +1,57 @@
+/*
+ * This file is part of the APDS990x sensor driver.
+ * Chip is combined proximity and ambient light sensor.
+ *
+ * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * Contact: Samu Onkalo <samu.p.onkalo@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#ifndef __APDS990X_H__
+#define __APDS990X_H__
+
+
+#define APDS_IRLED_CURR_12mA	0x3
+#define APDS_IRLED_CURR_25mA	0x2
+#define APDS_IRLED_CURR_50mA	0x1
+#define APDS_IRLED_CURR_100mA	0x0
+
+/*
+ * Structure for tuning ALS calculation to match with environment.
+ * There depends on the material above the sensor and the sensor
+ * itself. If the GA is zero, driver will use uncovered sensor default values
+ * format: decimal value * APDS_PARAM_SCALE
+ */
+#define APDS_PARAM_SCALE 4096
+struct apds990x_chip_factors {
+	int ga;         /* Glass attenuation */
+	int cf1;        /* Clear channel factor 1 */
+	int irf1;       /* Ir channel factor 1 */
+	int cf2;        /* Clear channel factor 2 */
+	int irf2;       /* Ir channel factor 2 */
+	int df;         /* Device factor. Decimal number */
+};
+
+struct apds990x_platform_data {
+	struct apds990x_chip_factors cf;
+	u8     pdrive;
+	int    (*setup_resources)(void);
+	int    (*release_resources)(void);
+};
+
+#endif
-- 
1.6.0.4

Re: [PATCH 3/5] misc: Driver for APDS990X ALS and proximity sensors

[Alan Cox]

> +static int apds990x_refresh_athres(struct apds990x_chip *chip)
> +{
> +	int ret;
> +	/* If the chip is not in use, don't try to access it */
> +	if (pm_runtime_suspended(&chip->client->dev))
> +		return 0;
> +
> +	ret = apds990x_write_word(chip, APDS990X_AILTL,
> +			apds990x_lux_to_threshold(chip, chip->lux_thres_lo));
> +	ret |= apds990x_write_word(chip, APDS990X_AIHTL,
> +			apds990x_lux_to_threshold(chip, chip->lux_thres_hi));
> +

What is the locking theory in the driver for things like these multi word
writes. It's not clear from a first read how you guaranteed the refreshes
don't get tangled up or how you know there won't be an interrupt half way
through one of these pairs of accesses

> +static void apds990x_force_a_refresh(struct apds990x_chip *chip)
> +{
> +	/* This will force ALS interrupt after the next measurement. */
> +	apds990x_write_word(chip, APDS990X_AILTL, APDS_LUX_DEF_THRES_LO);
> +	apds990x_write_word(chip, APDS990X_AIHTL, APDS_LUX_DEF_THRES_HI);
> +}
> +
> +static void apds990x_force_p_refresh(struct apds990x_chip *chip)
> +{
> +	/* This will force proximity interrupt after the next measurement. */
> +	apds990x_write_word(chip, APDS990X_PILTL, APDS_PROX_DEF_THRES - 1);
> +	apds990x_write_word(chip, APDS990X_PIHTL, APDS_PROX_DEF_THRES);
> +}
> +

> +
> +	if (ret < 0)
> +		apds990x_force_a_refresh(chip); /* Bad result -> re-measure */

What stops this getting stuck forever - I admit it would probably need a
drastic case of strobe lighting but people do take phones to raves ..


> +static DEVICE_ATTR(lux0_input, S_IRUGO, apds990x_lux_show, NULL);

Are well all agreed on lux0_input ? I'm just putting some of the other
bits together to submit and using the same spec

> +static DEVICE_ATTR(lux0_input10x, S_IRUGO, apds990x_lux10x_show, NULL);

This seems daft. No doubt we'll want lux0_input100x next year

What's wrong with outputting %d.%d format to lux0_input - existing apps
will read the integer bit fine and we've not defined the interface yet
anyway.


> +	if ((value > APDS_RANGE) || (value == 0) ||
> +		(value < APDS_PROX_HYSTERESIS))
> +		return -EINVAL;

Excess brackets


> +	chip->regs[0].supply = reg_vcc;
> +	chip->regs[1].supply = reg_vled;

Shouldn't these come from the provided platform data ? and again if there
isn't a platform one supplied just skip the regulators. There are going
to be cases of boxes with regulator API stuff in use but who don't have
regulators for that device.



On the sysfs side

- not clear how power_state and runtime pm interact - do we in fact need
  power state ?
- lux0_input range being fixed seems inconvenient, the sensors I've got
  queued here use lux0_input as you do but also provide a read (and
  optionally writable) range limit in lux

static DEVICE_ATTR(lux0_sensor_range, S_IRUGO | S_IWUSR,
	als_sensing_range_show, als_sensing_range_store);

and in your case you could just return 65535


The sampling rate stuff is a bit different to what I have - in my case
writing to the sensing range alters the sampling rate requirements. I can
easily make either one adjust the other however so that fits nicely

Re: [PATCH 3/5] misc: Driver for APDS990X ALS and proximity sensors

[Onkalo Samu]

Thanks

-Samu

On Tue, 2010-10-05 at 13:23 +0200, ext Alan Cox wrote:
> > +static int apds990x_refresh_athres(struct apds990x_chip *chip)
> > +{
> > +	int ret;
> > +	/* If the chip is not in use, don't try to access it */
> > +	if (pm_runtime_suspended(&chip->client->dev))
> > +		return 0;
> > +
> > +	ret = apds990x_write_word(chip, APDS990X_AILTL,
> > +			apds990x_lux_to_threshold(chip, chip->lux_thres_lo));
> > +	ret |= apds990x_write_word(chip, APDS990X_AIHTL,
> > +			apds990x_lux_to_threshold(chip, chip->lux_thres_hi));
> > +
> 
> What is the locking theory in the driver for things like these multi word
> writes. It's not clear from a first read how you guaranteed the refreshes
> don't get tangled up or how you know there won't be an interrupt half way
> through one of these pairs of accesses
> 
I'll double check that and add comments or mutexes if needed.

> > +static void apds990x_force_a_refresh(struct apds990x_chip *chip)
> > +{
> > +	/* This will force ALS interrupt after the next measurement. */
> > +	apds990x_write_word(chip, APDS990X_AILTL, APDS_LUX_DEF_THRES_LO);
> > +	apds990x_write_word(chip, APDS990X_AIHTL, APDS_LUX_DEF_THRES_HI);
> > +}
> > +
> > +static void apds990x_force_p_refresh(struct apds990x_chip *chip)
> > +{
> > +	/* This will force proximity interrupt after the next measurement. */
> > +	apds990x_write_word(chip, APDS990X_PILTL, APDS_PROX_DEF_THRES - 1);
> > +	apds990x_write_word(chip, APDS990X_PIHTL, APDS_PROX_DEF_THRES);
> > +}
> > +
> 
> > +
> > +	if (ret < 0)
> > +		apds990x_force_a_refresh(chip); /* Bad result -> re-measure */
> 
> What stops this getting stuck forever - I admit it would probably need a
> drastic case of strobe lighting but people do take phones to raves ..

Sensor averages light over 50ms period. Strobe light can cause anything.
But problem is that if the measurement overflows bright light causes 0
lux as a result. Re-measurement takes also one measurement period so it
not causing i.e. interrupt flow, but it may cause situation that resuls
is never available.

> 
> 
> > +static DEVICE_ATTR(lux0_input, S_IRUGO, apds990x_lux_show, NULL);
> 
> Are well all agreed on lux0_input ? I'm just putting some of the other
> bits together to submit and using the same spec
> 
> > +static DEVICE_ATTR(lux0_input10x, S_IRUGO, apds990x_lux10x_show, NULL);
> 
> This seems daft. No doubt we'll want lux0_input100x next year
> 
> What's wrong with outputting %d.%d format to lux0_input - existing apps
> will read the integer bit fine and we've not defined the interface yet
> anyway.
> 

Makes sense.

> 
> > +	if ((value > APDS_RANGE) || (value == 0) ||
> > +		(value < APDS_PROX_HYSTERESIS))
> > +		return -EINVAL;
> 
> Excess brackets
> 
> 
> > +	chip->regs[0].supply = reg_vcc;
> > +	chip->regs[1].supply = reg_vled;
> 
> Shouldn't these come from the provided platform data ? and again if there
> isn't a platform one supplied just skip the regulators. There are going
> to be cases of boxes with regulator API stuff in use but who don't have
> regulators for that device.

I'll change that.

> 
> 
> 
> On the sysfs side
> 
> - not clear how power_state and runtime pm interact - do we in fact need
>   power state ?

power_state turns chip on or off. In off state runtime pm is used to
handle bookkeeping and also handling chip state transitions including
regulators. Other ideas than separate power_state for this kind of
control? 


> - lux0_input range being fixed seems inconvenient, the sensors I've got
>   queued here use lux0_input as you do but also provide a read (and
>   optionally writable) range limit in lux
> 
> static DEVICE_ATTR(lux0_sensor_range, S_IRUGO | S_IWUSR,
> 	als_sensing_range_show, als_sensing_range_store);
> 
> and in your case you could just return 65535
> 

I'll add that

> 
> The sampling rate stuff is a bit different to what I have - in my case
> writing to the sensing range alters the sampling rate requirements. I can
> easily make either one adjust the other however so that fits nicely
> 
> 

Re: [PATCH 3/5] misc: Driver for APDS990X ALS and proximity sensors

[Alan Cox]

> > - not clear how power_state and runtime pm interact - do we in fact need
> >   power state ?
> 
> power_state turns chip on or off. In off state runtime pm is used to
> handle bookkeeping and also handling chip state transitions including
> regulators. Other ideas than separate power_state for this kind of
> control? 

Ok that makes sense.

> 
> 
> > - lux0_input range being fixed seems inconvenient, the sensors I've got
> >   queued here use lux0_input as you do but also provide a read (and
> >   optionally writable) range limit in lux
> > 
> > static DEVICE_ATTR(lux0_sensor_range, S_IRUGO | S_IWUSR,
> > 	als_sensing_range_show, als_sensing_range_store);
> > 
> > and in your case you could just return 65535
> > 
> 
> I'll add that

Cool. I'll bash our sensor queue to fit this API

Re: [PATCH 3/5] misc: Driver for APDS990X ALS and proximity sensors

[Onkalo Samu]

On Tue, 2010-10-05 at 15:00 +0200, ext Alan Cox wrote:
> > > - not clear how power_state and runtime pm interact - do we in fact need
> > >   power state ?
> > 
> > power_state turns chip on or off. In off state runtime pm is used to
> > handle bookkeeping and also handling chip state transitions including
> > regulators. Other ideas than separate power_state for this kind of
> > control? 
> 
> Ok that makes sense.
> 
> > 
> > 
> > > - lux0_input range being fixed seems inconvenient, the sensors I've got
> > >   queued here use lux0_input as you do but also provide a read (and
> > >   optionally writable) range limit in lux
> > > 
> > > static DEVICE_ATTR(lux0_sensor_range, S_IRUGO | S_IWUSR,
> > > 	als_sensing_range_show, als_sensing_range_store);
> > > 
> > > and in your case you could just return 65535
> > > 
> > 
> > I'll add that
> 
> Cool. I'll bash our sensor queue to fit this API

There are quite many naming convetion changes proposed
by Jonathan Cameron. I think that those are valid.

-Samu

Re: [PATCH 3/5] misc: Driver for APDS990X ALS and proximity sensors

[Mark Brown]

On Tue, Oct 05, 2010 at 12:23:23PM +0100, Alan Cox wrote:

> > +	chip->regs[0].supply = reg_vcc;
> > +	chip->regs[1].supply = reg_vled;

> Shouldn't these come from the provided platform data ? and again if there
> isn't a platform one supplied just skip the regulators. There are going

No, don't do this.  The device should just request the supplies that
correspond to the physical supplies for the device and let the regulator
API worry about actually providing regulators to match it.  If
regulators are getting passed in as platform data something has gone
wrong.

> to be cases of boxes with regulator API stuff in use but who don't have
> regulators for that device.

If there genuinely is no supply present on the pin then the device needs
to know that; if there is a supply present but it's not software managed
then the regulator API can cope with that.

Trying to handle this in individual device drivers just results in reams
of cut'n'paste code in device drivers, frequently requring the addition
of platform data to devices that would not otherwise require any.  We've
tried it, it's painful.  Having the machine specific data provided as a
generic regulator API service provides a great degree of simplification.

[PATCH 5/5] Documentation: Short descriptions for bhsfh and apds990x drivers

[Samu Onkalo]

Add short documentation for two ALS / proximity chip drivers.

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
---
 Documentation/misc-devices/apds990x.txt |  126 +++++++++++++++++++++++++++++++
 Documentation/misc-devices/bhsfh.txt    |  125 ++++++++++++++++++++++++++++++
 2 files changed, 251 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/misc-devices/apds990x.txt
 create mode 100644 Documentation/misc-devices/bhsfh.txt

diff --git a/Documentation/misc-devices/apds990x.txt b/Documentation/misc-devices/apds990x.txt
new file mode 100644
index 0000000..edffbea
--- /dev/null
+++ b/Documentation/misc-devices/apds990x.txt
@@ -0,0 +1,126 @@
+Kernel driver apds990x
+======================
+
+Supported chips:
+Avago APDS990X
+
+Data sheet:
+Not freely available
+
+Author:
+Samu Onkalo <samu.p.onkalo@nokia.com>
+
+Description
+-----------
+
+APDS990x is a combined ambient light and proximity sensor. ALS and proximity
+functionality are highly connected. ALS measurement path must be running
+while the proximity functionality is enabled.
+
+ALS produces only raw measurement values. Further processing is needed
+to get sensible LUX values. Vice versa, HW threshold control has nothing
+to do with LUX values. Threshold detection triggs to raw conversion values.
+Driver makes necessary conversions to both directions so that user handles
+only LUX values. ALS contains 4 different gain steps. Driver automatically
+selects suitable gain step. After each measurement, reliability of the results
+is estimated and new measurement is trigged if necessary.
+
+Platform data can provide tuned values to the conversion formulas if
+values are known. Othervise plain sensor default values are used.
+
+Proximity side is little bit simpler. It produces directly usable values.
+
+Driver controls chip operational state using pm_runtime framework.
+Voltage regulators are controlled based on chip operational state.
+
+SYSFS
+-----
+chip_id
+	RO - shows detected chip type and version
+
+power_state
+	RW - enable / disable chip. Uses counting logic
+	     1 enables the chip
+	     0 disables the chip
+lux0_input
+	RO - measured LUX value
+	     range: 0 .. 65535
+	     sysfs_notify called when threshold interrupt occurs
+
+lux0_input10x
+	RO - Same as lux0_input but values are 10x larger so accuracy
+	     is 0.1 lux
+	     range: 0 .. 655350
+lux0_rate
+	RW - measurement period in milliseconds
+
+lux0_rate_avail
+	RO - supported measurement periods
+
+lux0_calib
+	RW - calibration value. Set to neutral value by default
+
+lux0_calib_format
+	RO - neutral calibration value
+
+lux0_threshold_range
+	RW - lo and hi threshold values like "100 1000"
+	     range: 0 .. 65535
+
+prox0_enable
+	RW - enable / disable proximity - counting logic
+	     1 enables the proximity
+	     0 disables the proximity
+
+prox0_input
+	RO - measured proximity value
+	     range 0 .. 1023
+	     sysfs_notify called when threshold interrupt occurs
+
+prox0_continuous_mode
+	RW - When set, results above threshold are reported periodically.
+	     In this mode rough distance estimation is possible.
+	     Values: 0 and 1
+
+prox0_threshold
+	RW - threshold level which trigs proximity events.
+
+Platform data
+-------------
+
+
+#define APDS_IRLED_CURR_12mA	0x3
+#define APDS_IRLED_CURR_25mA	0x2
+#define APDS_IRLED_CURR_50mA	0x1
+#define APDS_IRLED_CURR_100mA	0x0
+
+/*
+ * Structure for tuning ALS calculation to match with environment.
+ * There depends on the material above the sensor and the sensor
+ * itself. If the GA is zero, driver will use uncovered sensor default values
+ * format: decimal value * APDS_PARAM_SCALE
+ */
+#define APDS_PARAM_SCALE 4096
+struct apds990x_chip_factors {
+	int ga;		/* Glass attenuation */
+	int cf1;	/* Clear channel factor 1 */
+	int irf1;	/* Ir channel factor 1 */
+	int cf2;	/* Clear channel factor 2 */
+	int irf2;	/* Ir channel factor 2 */
+	int df;		/* Device factor. Decimal number */
+};
+
+struct apds990x_platform_data {
+	struct apds990x_chip_factors cf;
+	u8     pdrive;
+	int    (*setup_resources)(void);
+	int    (*release_resources)(void);
+};
+
+chip factors are specific to for example dark plastic window
+above the sensor. Driver uses plain (uncovered) sensor default values
+if the platform data contains zero ga factor.
+
+pdrive is the IR led driver current
+
+setup / release resources handles interrupt line configurations.
diff --git a/Documentation/misc-devices/bhsfh.txt b/Documentation/misc-devices/bhsfh.txt
new file mode 100644
index 0000000..7f72f84
--- /dev/null
+++ b/Documentation/misc-devices/bhsfh.txt
@@ -0,0 +1,125 @@
+Kernel driver bhsfh
+===================
+
+Supported chips:
+ROHM BH1770GLC
+OSRAM SFH7770
+
+Data sheet:
+Not freely available
+
+Author:
+Samu Onkalo <samu.p.onkalo@nokia.com>
+
+Description
+-----------
+BH1770GLC and SFH7770 (I'm calling them as bhsfh) are combined ambient
+light and proximity sensors. ALS and proximity parts operates on their own,
+but they shares common I2C interface and interrupt logic.
+
+ALS produces 16 bit LUX values. The chip contains interrupt logic to produce
+low and high threshold interrupts.
+
+Proximity part contains IR-led driver up to 3 IR leds. The chip measures
+amount of reflected IR light and produces proximity result. Resolution is
+8 bit. Driver supports only one channel. Driver uses ALS results to estimate
+realibility of the proximity results. Thus ALS is always running while
+proximity detection is needed.
+
+Driver uses threshold interrupts to avoid need for polling the values.
+Proximity low interrupt doesn't exists in the chip. This is simulated
+by using a delayed work.
+
+SYSFS
+-----
+
+chip_id
+	RO - shows detected chip type and version
+
+power_state
+	RW - enable / disable chip. Uses counting logic
+	     1 enables the chip
+	     0 disables the chip
+
+lux0_input
+	RO - measured LUX value
+	     range: 0 .. 65535
+	     sysfs_notify called when threshold interrupt occurs
+
+lux0_rate
+	RW - measurement period in milliseconds
+
+lux0_rate_avail
+	RO - supported measurement periods
+
+lux0_threshold_range
+	RW - lo and hi threshold values like "100 1000"
+	     range: 0 .. 65535
+
+lux0_calib
+	RW - calibration value. Set to neutral value by default
+
+lux0_calib_format
+	RO - neutral calibration value
+
+prox0_input
+	RO - measured proximity value
+	     range 0 .. 255
+	     sysfs_notify called when threshold interrupt occurs
+
+prox0_enable
+	RW - enable / disable proximity - uses counting logic
+	     1 enables the proximity
+	     0 disables the proximity
+
+prox0_persistence
+	RW - number of proximity interrupts needed before triggering the event
+
+prox0_rate
+	RW - supported measurement periods. There are two values in this entry:
+	     First one is used when waiting for proximity event and the second
+	     one is used when waiting proximity reason to go away.
+
+prox0_rate
+	RO - available measurement periods
+
+prox0_threshold
+	RW - threshold level which trigs proximity events.
+	     Filtered by persistence filter
+
+prox0_abs_thres
+	RW - threshold level which trigs event immediately
+
+
+Platform data:
+
+struct bhsfh_platform_data {
+#define BHSFH_LED_5mA	0
+#define BHSFH_LED_10mA	1
+#define BHSFH_LED_20mA	2
+#define BHSFH_LED_50mA	3
+#define BHSFH_LED_100mA 4
+#define BHSFH_LED_150mA 5
+#define BHSFH_LED_200mA 6
+	__u8 led_def_curr;
+#define BHFSH_NEUTRAL_GA 16384 /* 16384 / 16384 = 1 */
+	__u32 glass_attenuation;
+	int (*setup_resources)(void);
+	int (*release_resources)(void);
+};
+
+led_def_curr controls IR led driving current
+glass_attenuation tells darkness of the covering window
+setup / release resources are call back functions for controlling
+interrupt line.
+
+Example:
+static struct bhsfh_platform_data rm680_bhsfh_data = {
+	.led_def_curr	   = BHSFH_LED_50mA,
+	.glass_attenuation = (16384 * 385) / 100,
+	.setup_resources   = bhsfh_setup,
+	.release_resources = bhsfh_release,
+};
+
+glass_attenuation: 385 in above formula means 3.85 in decimal.
+light_above_sensors = light_above_cover_window / 3.85
-- 
1.6.0.4

Re: [PATCH 5/5] Documentation: Short descriptions for bhsfh and apds990x drivers

[Jonathan Cameron]

On 10/05/10 10:42, Samu Onkalo wrote:
> Add short documentation for two ALS / proximity chip drivers.

Hi Samu,

Good to see the documentation as it makes commenting on naming etc far simpler.

There are a lot of comments, but that's because you define a lot of new
interfaces and as ever I'm interested in working out how to keep them
clear and as relevant to as many parts as possible.

For reference, a very similar device driver (capability wise) was proposed on
linux-iio by Dongguen Kim. I don't personally care about the location of these
drivers, but I do care about the interfaces.

http://marc.info/?t=128453241000001&r=1&w=2

[PATCH V2] staging: iio: tmd2771x: Add tmd2771x proximity and ambient light sensor driver

> Signed-off-by: Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> ---
>  Documentation/misc-devices/apds990x.txt |  126 +++++++++++++++++++++++++++++++
>  Documentation/misc-devices/bhsfh.txt    |  125 ++++++++++++++++++++++++++++++
>  2 files changed, 251 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/misc-devices/apds990x.txt
>  create mode 100644 Documentation/misc-devices/bhsfh.txt
> 
> diff --git a/Documentation/misc-devices/apds990x.txt b/Documentation/misc-devices/apds990x.txt
> new file mode 100644
> index 0000000..edffbea
> --- /dev/null
> +++ b/Documentation/misc-devices/apds990x.txt
> @@ -0,0 +1,126 @@
> +Kernel driver apds990x
> +======================
> +
> +Supported chips:
> +Avago APDS990X
> +
> +Data sheet:
> +Not freely available
> +
> +Author:
> +Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> +
> +Description
> +-----------
> +
> +APDS990x is a combined ambient light and proximity sensor. ALS and proximity
> +functionality are highly connected. ALS measurement path must be running
> +while the proximity functionality is enabled.
> +
> +ALS produces only raw measurement values. Further processing is needed
> +to get sensible LUX values. Vice versa, HW threshold control has nothing
> +to do with LUX values. Threshold detection triggs to raw conversion values.
> +Driver makes necessary conversions to both directions so that user handles
> +only LUX values. ALS contains 4 different gain steps. Driver automatically
> +selects suitable gain step. After each measurement, reliability of the results
> +is estimated and new measurement is trigged if necessary.
Nice.
> +
> +Platform data can provide tuned values to the conversion formulas if
> +values are known. Othervise plain sensor default values are used.
Typo: Otherwise
> +
> +Proximity side is little bit simpler. It produces directly usable values.
In what sense?  What units are the values in?  If they are raw values,
I'd denote that in the attribute name (_raw rather than _input).
> +
> +Driver controls chip operational state using pm_runtime framework.
> +Voltage regulators are controlled based on chip operational state.
> +
> +SYSFS
> +-----
> +chip_id
> +	RO - shows detected chip type and version
> +
> +power_state
> +	RW - enable / disable chip. Uses counting logic
> +	     1 enables the chip
> +	     0 disables the chip
> +lux0_input
> +	RO - measured LUX value
> +	     range: 0 .. 65535
> +	     sysfs_notify called when threshold interrupt occurs
Personally I'm still in favour (for generality) of using illuminance0
rather than lux0 but I think the mass of drivers mean I'm losing that
argument. (argument for those who haven't seen it before is that lux is
the unit, the thing being measured is illuminance - units don't
generalize to other sensor types e.g. acceleration)

> +
> +lux0_input10x
> +	RO - Same as lux0_input but values are 10x larger so accuracy
> +	     is 0.1 lux
> +	     range: 0 .. 655350
Why do we care?  Just do a bit of fixed point arithmetic and output
it in lux0_input (yes hwmon is fussy about not having decimal places,
I think everyone else is more flexible in cases like this).
> +lux0_rate
> +	RW - measurement period in milliseconds
Then it's not a rate is it.  Please fix naming or what comes out.
Rate = sampling frequency, not sampling period.

> +
> +lux0_rate_avail
> +	RO - supported measurement periods
> +
> +lux0_calib
> +	RW - calibration value. Set to neutral value by default
That tells me nothing about what this parameter is.  Please provide
some info on this so we can work out a clearer naming convention.
> +
> +lux0_calib_format
> +	RO - neutral calibration value
> +
> +lux0_threshold_range
> +	RW - lo and hi threshold values like "100 1000"
> +	     range: 0 .. 65535
Are these conceptually one value... Maybe. I proposed an
RFC with a general naming convention the other day. I'm happy to reopen
that debate if you disagree, but do feel it is vital to pick a standard
and keep to it. I've move iio to the conventions proposed in that discussion
(on linux-iio for review at the moment).

Under that convention this would be (I think, given I don't have the data sheet)

lux0_input_thresh_rising_value
lux0_input_thresh_falling_value

For reference the naming convention discussion follows from

http://linux.derkeiler.com/Mailing-Lists/Kernel/2010-09/msg08622.html
(lkml.org seems to be down)
> +
> +prox0_enable
For proximity, I'd be more inclined to not abbreviate this.
> +	RW - enable / disable proximity - counting logic
> +	     1 enables the proximity
> +	     0 disables the proximity
So this is concerned with the thresholds below?  Again, proposed
naming would be:

proximity0_thresh_rising_en (might have naming wrong there, I'm not sure what it
	is).
> +
> +prox0_input
> +	RO - measured proximity value
> +	     range 0 .. 1023
> +	     sysfs_notify called when threshold interrupt occurs
So this only exists when a threshold interrupt has occurred?  If so can we
name it in a fashion to indicate this?  If the units are SI or at least
obvious, please call it _raw to indicate that.
> +
> +prox0_continuous_mode
> +	RW - When set, results above threshold are reported periodically.
> +	     In this mode rough distance estimation is possible.
> +	     Values: 0 and 1
This naming needs some thought.  It's far from obvious that _continuous_mode
would correspond to this behaviour.
> +
> +prox0_threshold
> +	RW - threshold level which trigs proximity events.
Name could be more informative.
> +
> +Platform data
> +-------------
> +
> +
> +#define APDS_IRLED_CURR_12mA	0x3
> +#define APDS_IRLED_CURR_25mA	0x2
> +#define APDS_IRLED_CURR_50mA	0x1
> +#define APDS_IRLED_CURR_100mA	0x0
> +
> +/*
> + * Structure for tuning ALS calculation to match with environment.
> + * There depends on the material above the sensor and the sensor
> + * itself. If the GA is zero, driver will use uncovered sensor default values
> + * format: decimal value * APDS_PARAM_SCALE
> + */
> +#define APDS_PARAM_SCALE 4096
> +struct apds990x_chip_factors {
> +	int ga;		/* Glass attenuation */
> +	int cf1;	/* Clear channel factor 1 */
> +	int irf1;	/* Ir channel factor 1 */
> +	int cf2;	/* Clear channel factor 2 */
> +	int irf2;	/* Ir channel factor 2 */
> +	int df;		/* Device factor. Decimal number */
> +};
> +
> +struct apds990x_platform_data {
> +	struct apds990x_chip_factors cf;
> +	u8     pdrive;
> +	int    (*setup_resources)(void);
> +	int    (*release_resources)(void);
> +};
> +
> +chip factors are specific to for example dark plastic window
> +above the sensor. Driver uses plain (uncovered) sensor default values
> +if the platform data contains zero ga factor.
> +
> +pdrive is the IR led driver current
> +
> +setup / release resources handles interrupt line configurations.
> diff --git a/Documentation/misc-devices/bhsfh.txt b/Documentation/misc-devices/bhsfh.txt
> new file mode 100644
> index 0000000..7f72f84
> --- /dev/null
> +++ b/Documentation/misc-devices/bhsfh.txt
> @@ -0,0 +1,125 @@
> +Kernel driver bhsfh
> +===================
> +
> +Supported chips:
> +ROHM BH1770GLC
> +OSRAM SFH7770
> +
> +Data sheet:
> +Not freely available
> +
> +Author:
> +Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> +
> +Description
> +-----------
> +BH1770GLC and SFH7770 (I'm calling them as bhsfh) are combined ambient
> +light and proximity sensors. ALS and proximity parts operates on their own,
> +but they shares common I2C interface and interrupt logic.
> +
> +ALS produces 16 bit LUX values. The chip contains interrupt logic to produce
> +low and high threshold interrupts.
> +
> +Proximity part contains IR-led driver up to 3 IR leds. The chip measures
> +amount of reflected IR light and produces proximity result. Resolution is
> +8 bit. Driver supports only one channel. Driver uses ALS results to estimate
> +realibility of the proximity results. Thus ALS is always running while
> +proximity detection is needed.
> +
> +Driver uses threshold interrupts to avoid need for polling the values.
> +Proximity low interrupt doesn't exists in the chip. This is simulated
> +by using a delayed work.
> +
> +SYSFS
> +-----
> +
> +chip_id
> +	RO - shows detected chip type and version
> +
> +power_state
> +	RW - enable / disable chip. Uses counting logic
> +	     1 enables the chip
> +	     0 disables the chip
> +
> +lux0_input
> +	RO - measured LUX value
> +	     range: 0 .. 65535
> +	     sysfs_notify called when threshold interrupt occurs
> +
> +lux0_rate
> +	RW - measurement period in milliseconds
> +
> +lux0_rate_avail
> +	RO - supported measurement periods
> +
> +lux0_threshold_range
> +	RW - lo and hi threshold values like "100 1000"
> +	     range: 0 .. 65535
> +
> +lux0_calib
> +	RW - calibration value. Set to neutral value by default
That is not sufficient documentation.  What is the calibration value used
for?  As a random user coming in who isn't going to read the data sheet this
comment tells me nothing.
> +
> +lux0_calib_format
> +	RO - neutral calibration value
This one tells me even less.
> +
> +prox0_input
> +	RO - measured proximity value
> +	     range 0 .. 255
> +	     sysfs_notify called when threshold interrupt occurs
> +
> +prox0_enable
> +	RW - enable / disable proximity - uses counting logic
> +	     1 enables the proximity
> +	     0 disables the proximity
So this is enabling the threshold?  Or is it turning on the ability to
read directly from the sensor?  Not clear in the naming (and it should be).
> +
> +prox0_persistence
> +	RW - number of proximity interrupts needed before triggering the event
naming RFC proposed calling this (roughly given I'm not clear on type of interrupt)

proximity0_thresh_rising_period
> +
> +prox0_rate
> +	RW - supported measurement periods. There are two values in this entry:
> +	     First one is used when waiting for proximity event and the second
> +	     one is used when waiting proximity reason to go away.
That's not conceptually one value, so break it in two.
> +
> +prox0_rate
> +	RO - available measurement periods
> +
> +prox0_threshold
> +	RW - threshold level which trigs proximity events.
> +	     Filtered by persistence filter
> +
> +prox0_abs_thres
> +	RW - threshold level which trigs event immediately
The naming of these last two is extremely confusing and inconsistent. 
Why abs?  Is it on the absolute value?  That seems an odd thing given
I doubt proximity is a signed value?  Or is this just a second alarm
that ignores the persistence filter?

> +
> +Platform data:
> +
> +struct bhsfh_platform_data {
> +#define BHSFH_LED_5mA	0
> +#define BHSFH_LED_10mA	1
> +#define BHSFH_LED_20mA	2
> +#define BHSFH_LED_50mA	3
> +#define BHSFH_LED_100mA 4
> +#define BHSFH_LED_150mA 5
> +#define BHSFH_LED_200mA 6
> +	__u8 led_def_curr;
> +#define BHFSH_NEUTRAL_GA 16384 /* 16384 / 16384 = 1 */
> +	__u32 glass_attenuation;
> +	int (*setup_resources)(void);
> +	int (*release_resources)(void);
> +};
> +
> +led_def_curr controls IR led driving current
> +glass_attenuation tells darkness of the covering window
> +setup / release resources are call back functions for controlling
> +interrupt line.
> +
> +Example:
> +static struct bhsfh_platform_data rm680_bhsfh_data = {
> +	.led_def_curr	   = BHSFH_LED_50mA,
> +	.glass_attenuation = (16384 * 385) / 100,
> +	.setup_resources   = bhsfh_setup,
> +	.release_resources = bhsfh_release,
> +};
> +
> +glass_attenuation: 385 in above formula means 3.85 in decimal.
> +light_above_sensors = light_above_cover_window / 3.85

Re: [PATCH 5/5] Documentation: Short descriptions for bhsfh and apds990x drivers

[Onkalo Samu]

Jonathan, thanks.

-Samu

On Tue, 2010-10-05 at 13:53 +0200, ext Jonathan Cameron wrote:
> On 10/05/10 10:42, Samu Onkalo wrote:
> > Add short documentation for two ALS / proximity chip drivers.
> 
> Hi Samu,
> 
> Good to see the documentation as it makes commenting on naming etc far simpler.
> 
> There are a lot of comments, but that's because you define a lot of new
> interfaces and as ever I'm interested in working out how to keep them
> clear and as relevant to as many parts as possible.
> 
> For reference, a very similar device driver (capability wise) was proposed on
> linux-iio by Dongguen Kim. I don't personally care about the location of these
> drivers, but I do care about the interfaces.
> 
> http://marc.info/?t=128453241000001&r=1&w=2
> 
> [PATCH V2] staging: iio: tmd2771x: Add tmd2771x proximity and ambient light sensor driver
> 

Looks quite similar indeed.

> > Signed-off-by: Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> > ---
> >  Documentation/misc-devices/apds990x.txt |  126 +++++++++++++++++++++++++++++++
> >  Documentation/misc-devices/bhsfh.txt    |  125 ++++++++++++++++++++++++++++++
> >  2 files changed, 251 insertions(+), 0 deletions(-)
> >  create mode 100644 Documentation/misc-devices/apds990x.txt
> >  create mode 100644 Documentation/misc-devices/bhsfh.txt
> >
> > diff --git a/Documentation/misc-devices/apds990x.txt b/Documentation/misc-devices/apds990x.txt
> > new file mode 100644
> > index 0000000..edffbea
> > --- /dev/null
> > +++ b/Documentation/misc-devices/apds990x.txt
> > @@ -0,0 +1,126 @@
> > +Kernel driver apds990x
> > +======================
> > +
> > +Supported chips:
> > +Avago APDS990X
> > +
> > +Data sheet:
> > +Not freely available
> > +
> > +Author:
> > +Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> > +
> > +Description
> > +-----------
> > +
> > +APDS990x is a combined ambient light and proximity sensor. ALS and proximity
> > +functionality are highly connected. ALS measurement path must be running
> > +while the proximity functionality is enabled.
> > +
> > +ALS produces only raw measurement values. Further processing is needed
> > +to get sensible LUX values. Vice versa, HW threshold control has nothing
> > +to do with LUX values. Threshold detection triggs to raw conversion values.
> > +Driver makes necessary conversions to both directions so that user handles
> > +only LUX values. ALS contains 4 different gain steps. Driver automatically
> > +selects suitable gain step. After each measurement, reliability of the results
> > +is estimated and new measurement is trigged if necessary.
> Nice.
> > +
> > +Platform data can provide tuned values to the conversion formulas if
> > +values are known. Othervise plain sensor default values are used.
> Typo: Otherwise
> > +
> > +Proximity side is little bit simpler. It produces directly usable values.
> In what sense?  What units are the values in?  If they are raw values,
> I'd denote that in the attribute name (_raw rather than _input).

I'll change it to _raw. I just meant here that proximity doesn't need
lot of conversions etc. to have usable values.

> > +
> > +Driver controls chip operational state using pm_runtime framework.
> > +Voltage regulators are controlled based on chip operational state.
> > +
> > +SYSFS
> > +-----
> > +chip_id
> > +     RO - shows detected chip type and version
> > +
> > +power_state
> > +     RW - enable / disable chip. Uses counting logic
> > +          1 enables the chip
> > +          0 disables the chip
> > +lux0_input
> > +     RO - measured LUX value
> > +          range: 0 .. 65535
> > +          sysfs_notify called when threshold interrupt occurs
> Personally I'm still in favour (for generality) of using illuminance0
> rather than lux0 but I think the mass of drivers mean I'm losing that
> argument. (argument for those who haven't seen it before is that lux is
> the unit, the thing being measured is illuminance - units don't
> generalize to other sensor types e.g. acceleration)
> 

I think Alan just got one driver queued with lux :)

> > +
> > +lux0_input10x
> > +     RO - Same as lux0_input but values are 10x larger so accuracy
> > +          is 0.1 lux
> > +          range: 0 .. 655350
> Why do we care?  Just do a bit of fixed point arithmetic and output
> it in lux0_input (yes hwmon is fussy about not having decimal places,
> I think everyone else is more flexible in cases like this).

%d.%d :)

> > +lux0_rate
> > +     RW - measurement period in milliseconds
> Then it's not a rate is it.  Please fix naming or what comes out.
> Rate = sampling frequency, not sampling period.
> 

rate -> period

> > +
> > +lux0_rate_avail
> > +     RO - supported measurement periods
> > +
> > +lux0_calib
> > +     RW - calibration value. Set to neutral value by default
> That tells me nothing about what this parameter is.  Please provide
> some info on this so we can work out a clearer naming convention.
> > +

Shortly - results are multiplied with calib value. And calib_format
tells what is the neutral value ( == 1.00 multiplier). This is used to
compensate differences between sensors. To get proper value, you need
calibrated source of light and check what is the output from the sensor.
Then just set calib value so that reported lux value is about the
correct one.

> > +lux0_calib_format
> > +     RO - neutral calibration value
> > +
> > +lux0_threshold_range
> > +     RW - lo and hi threshold values like "100 1000"
> > +          range: 0 .. 65535
> Are these conceptually one value... Maybe. I proposed an
> RFC with a general naming convention the other day. I'm happy to reopen
> that debate if you disagree, but do feel it is vital to pick a standard
> and keep to it. I've move iio to the conventions proposed in that discussion
> (on linux-iio for review at the moment).
> 
> Under that convention this would be (I think, given I don't have the data sheet)
> 
> lux0_input_thresh_rising_value
> lux0_input_thresh_falling_value
> 

Data sheets often uses HI and LO threshold naming. But yes, interrupt
comes when the level is above the hi-limit or below lo-limit.
Strictly, it is not crossing the limit.

Two separate entries makes sense but how about the naming...

lux0_input_thresh_above_value
lux0_input_thresh_below_value

would tell when interrupt is triggered.

> For reference the naming convention discussion follows from
> 
> http://linux.derkeiler.com/Mailing-Lists/Kernel/2010-09/msg08622.html
> (lkml.org seems to be down)
> > +
> > +prox0_enable
> For proximity, I'd be more inclined to not abbreviate this.
> > +     RW - enable / disable proximity - counting logic
> > +          1 enables the proximity
> > +          0 disables the proximity
> So this is concerned with the thresholds below?  Again, proposed
> naming would be:
> 
> proximity0_thresh_rising_en (might have naming wrong there, I'm not sure what it
>         is).

no, this is enabling the proximity measurement in general. Obviously it
is not clear :(

> > +
> > +prox0_input
> > +     RO - measured proximity value
> > +          range 0 .. 1023
> > +          sysfs_notify called when threshold interrupt occurs
> So this only exists when a threshold interrupt has occurred?  If so can we
> name it in a fashion to indicate this?  If the units are SI or at least
> obvious, please call it _raw to indicate that.

_input -> _raw

> > +
> > +prox0_continuous_mode
> > +     RW - When set, results above threshold are reported periodically.
> > +          In this mode rough distance estimation is possible.
> > +          Values: 0 and 1
> This naming needs some thought.  It's far from obvious that _continuous_mode
> would correspond to this behaviour.

any ideas about naming?


> > +
> > +prox0_threshold
> > +     RW - threshold level which trigs proximity events.
> Name could be more informative.

prox0_raw_thres_above_value?

> > +
> > +Platform data
> > +-------------
> > +
> > +
> > +#define APDS_IRLED_CURR_12mA 0x3
> > +#define APDS_IRLED_CURR_25mA 0x2
> > +#define APDS_IRLED_CURR_50mA 0x1
> > +#define APDS_IRLED_CURR_100mA        0x0
> > +
> > +/*
> > + * Structure for tuning ALS calculation to match with environment.
> > + * There depends on the material above the sensor and the sensor
> > + * itself. If the GA is zero, driver will use uncovered sensor default values
> > + * format: decimal value * APDS_PARAM_SCALE
> > + */
> > +#define APDS_PARAM_SCALE 4096
> > +struct apds990x_chip_factors {
> > +     int ga;         /* Glass attenuation */
> > +     int cf1;        /* Clear channel factor 1 */
> > +     int irf1;       /* Ir channel factor 1 */
> > +     int cf2;        /* Clear channel factor 2 */
> > +     int irf2;       /* Ir channel factor 2 */
> > +     int df;         /* Device factor. Decimal number */
> > +};
> > +
> > +struct apds990x_platform_data {
> > +     struct apds990x_chip_factors cf;
> > +     u8     pdrive;
> > +     int    (*setup_resources)(void);
> > +     int    (*release_resources)(void);
> > +};
> > +
> > +chip factors are specific to for example dark plastic window
> > +above the sensor. Driver uses plain (uncovered) sensor default values
> > +if the platform data contains zero ga factor.
> > +
> > +pdrive is the IR led driver current
> > +
> > +setup / release resources handles interrupt line configurations.
> > diff --git a/Documentation/misc-devices/bhsfh.txt b/Documentation/misc-devices/bhsfh.txt
> > new file mode 100644
> > index 0000000..7f72f84
> > --- /dev/null
> > +++ b/Documentation/misc-devices/bhsfh.txt
> > @@ -0,0 +1,125 @@
> > +Kernel driver bhsfh
> > +===================
> > +
> > +Supported chips:
> > +ROHM BH1770GLC
> > +OSRAM SFH7770
> > +
> > +Data sheet:
> > +Not freely available
> > +
> > +Author:
> > +Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> > +
> > +Description
> > +-----------
> > +BH1770GLC and SFH7770 (I'm calling them as bhsfh) are combined ambient
> > +light and proximity sensors. ALS and proximity parts operates on their own,
> > +but they shares common I2C interface and interrupt logic.
> > +
> > +ALS produces 16 bit LUX values. The chip contains interrupt logic to produce
> > +low and high threshold interrupts.
> > +
> > +Proximity part contains IR-led driver up to 3 IR leds. The chip measures
> > +amount of reflected IR light and produces proximity result. Resolution is
> > +8 bit. Driver supports only one channel. Driver uses ALS results to estimate
> > +realibility of the proximity results. Thus ALS is always running while
> > +proximity detection is needed.
> > +
> > +Driver uses threshold interrupts to avoid need for polling the values.
> > +Proximity low interrupt doesn't exists in the chip. This is simulated
> > +by using a delayed work.
> > +
> > +SYSFS
> > +-----
> > +
> > +chip_id
> > +     RO - shows detected chip type and version
> > +
> > +power_state
> > +     RW - enable / disable chip. Uses counting logic
> > +          1 enables the chip
> > +          0 disables the chip
> > +
> > +lux0_input
> > +     RO - measured LUX value
> > +          range: 0 .. 65535
> > +          sysfs_notify called when threshold interrupt occurs
> > +
> > +lux0_rate
> > +     RW - measurement period in milliseconds
> > +
> > +lux0_rate_avail
> > +     RO - supported measurement periods
> > +
> > +lux0_threshold_range
> > +     RW - lo and hi threshold values like "100 1000"
> > +          range: 0 .. 65535
> > +
> > +lux0_calib
> > +     RW - calibration value. Set to neutral value by default
> That is not sufficient documentation.  What is the calibration value used
> for?  As a random user coming in who isn't going to read the data sheet this
> comment tells me nothing.
> > +
> > +lux0_calib_format
> > +     RO - neutral calibration value
> This one tells me even less.
> > +
> > +prox0_input
> > +     RO - measured proximity value
> > +          range 0 .. 255
> > +          sysfs_notify called when threshold interrupt occurs
> > +
> > +prox0_enable
> > +     RW - enable / disable proximity - uses counting logic
> > +          1 enables the proximity
> > +          0 disables the proximity
> So this is enabling the threshold?  Or is it turning on the ability to
> read directly from the sensor?  Not clear in the naming (and it should be).

prox0_raw_enable?

> > +
> > +prox0_persistence
> > +     RW - number of proximity interrupts needed before triggering the event
> naming RFC proposed calling this (roughly given I'm not clear on type of interrupt)
> 
> proximity0_thresh_rising_period

prox0_thresh_above_count


> > +
> > +prox0_rate
> > +     RW - supported measurement periods. There are two values in this entry:
> > +          First one is used when waiting for proximity event and the second
> > +          one is used when waiting proximity reason to go away.
> That's not conceptually one value, so break it in two.

ok

> > +
> > +prox0_rate
> > +     RO - available measurement periods
> > +
> > +prox0_threshold
> > +     RW - threshold level which trigs proximity events.
> > +          Filtered by persistence filter
> > +

prox0_raw_thres_above_value?


> > +prox0_abs_thres
> > +     RW - threshold level which trigs event immediately
> The naming of these last two is extremely confusing and inconsistent.
> Why abs?  Is it on the absolute value?  That seems an odd thing given
> I doubt proximity is a signed value?  Or is this just a second alarm
> that ignores the persistence filter?
> 


Idea here is that "prox0_threshold" limit is something which must trig
proximit event, but there is no hurry. Since the sensor may be noisy
(false events every now and then), there must be several interrupts in a
row (persistence value) to get proximity event at that level. 
However, high enough value (sensor covered totally) must trig event
immediately (abs_thres).

prox0_raw_thres_above_value_strict?

> > +
> > +Platform data:
> > +
> > +struct bhsfh_platform_data {
> > +#define BHSFH_LED_5mA        0
> > +#define BHSFH_LED_10mA       1
> > +#define BHSFH_LED_20mA       2
> > +#define BHSFH_LED_50mA       3
> > +#define BHSFH_LED_100mA 4
> > +#define BHSFH_LED_150mA 5
> > +#define BHSFH_LED_200mA 6
> > +     __u8 led_def_curr;
> > +#define BHFSH_NEUTRAL_GA 16384 /* 16384 / 16384 = 1 */
> > +     __u32 glass_attenuation;
> > +     int (*setup_resources)(void);
> > +     int (*release_resources)(void);
> > +};
> > +
> > +led_def_curr controls IR led driving current
> > +glass_attenuation tells darkness of the covering window
> > +setup / release resources are call back functions for controlling
> > +interrupt line.
> > +
> > +Example:
> > +static struct bhsfh_platform_data rm680_bhsfh_data = {
> > +     .led_def_curr      = BHSFH_LED_50mA,
> > +     .glass_attenuation = (16384 * 385) / 100,
> > +     .setup_resources   = bhsfh_setup,
> > +     .release_resources = bhsfh_release,
> > +};
> > +
> > +glass_attenuation: 385 in above formula means 3.85 in decimal.
> > +light_above_sensors = light_above_cover_window / 3.85
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] Documentation: Short descriptions for bhsfh and apds990x drivers

[Jonathan Cameron]

On 10/05/10 13:22, Onkalo Samu wrote:
> 
> Jonathan, thanks.
> 
> -Samu
> 
> On Tue, 2010-10-05 at 13:53 +0200, ext Jonathan Cameron wrote:
>> On 10/05/10 10:42, Samu Onkalo wrote:
>>> Add short documentation for two ALS / proximity chip drivers.
>>
>> Hi Samu,
>>
>> Good to see the documentation as it makes commenting on naming etc far simpler.
>>
>> There are a lot of comments, but that's because you define a lot of new
>> interfaces and as ever I'm interested in working out how to keep them
>> clear and as relevant to as many parts as possible.
>>
>> For reference, a very similar device driver (capability wise) was proposed on
>> linux-iio by Dongguen Kim. I don't personally care about the location of these
>> drivers, but I do care about the interfaces.
>>
>> http://marc.info/?t=128453241000001&r=1&w=2
>>
>> [PATCH V2] staging: iio: tmd2771x: Add tmd2771x proximity and ambient light sensor driver
>>
> 
> Looks quite similar indeed.
> 
>>> Signed-off-by: Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  Documentation/misc-devices/apds990x.txt |  126 +++++++++++++++++++++++++++++++
>>>  Documentation/misc-devices/bhsfh.txt    |  125 ++++++++++++++++++++++++++++++
>>>  2 files changed, 251 insertions(+), 0 deletions(-)
>>>  create mode 100644 Documentation/misc-devices/apds990x.txt
>>>  create mode 100644 Documentation/misc-devices/bhsfh.txt
>>>
>>> diff --git a/Documentation/misc-devices/apds990x.txt b/Documentation/misc-devices/apds990x.txt
>>> new file mode 100644
>>> index 0000000..edffbea
>>> --- /dev/null
>>> +++ b/Documentation/misc-devices/apds990x.txt
>>> @@ -0,0 +1,126 @@
>>> +Kernel driver apds990x
>>> +======================
>>> +
>>> +Supported chips:
>>> +Avago APDS990X
>>> +
>>> +Data sheet:
>>> +Not freely available
>>> +
>>> +Author:
>>> +Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
>>> +
>>> +Description
>>> +-----------
>>> +
>>> +APDS990x is a combined ambient light and proximity sensor. ALS and proximity
>>> +functionality are highly connected. ALS measurement path must be running
>>> +while the proximity functionality is enabled.
>>> +
>>> +ALS produces only raw measurement values. Further processing is needed
>>> +to get sensible LUX values. Vice versa, HW threshold control has nothing
>>> +to do with LUX values. Threshold detection triggs to raw conversion values.
>>> +Driver makes necessary conversions to both directions so that user handles
>>> +only LUX values. ALS contains 4 different gain steps. Driver automatically
>>> +selects suitable gain step. After each measurement, reliability of the results
>>> +is estimated and new measurement is trigged if necessary.
>> Nice.
>>> +
>>> +Platform data can provide tuned values to the conversion formulas if
>>> +values are known. Othervise plain sensor default values are used.
>> Typo: Otherwise
>>> +
>>> +Proximity side is little bit simpler. It produces directly usable values.
>> In what sense?  What units are the values in?  If they are raw values,
>> I'd denote that in the attribute name (_raw rather than _input).
> 
> I'll change it to _raw. I just meant here that proximity doesn't need
> lot of conversions etc. to have usable values.
Cool. That is what I thought you meant, just wanted to be sure.
>>> +
>>> +Driver controls chip operational state using pm_runtime framework.
>>> +Voltage regulators are controlled based on chip operational state.
>>> +
>>> +SYSFS
>>> +-----
>>> +chip_id
>>> +     RO - shows detected chip type and version
>>> +
>>> +power_state
>>> +     RW - enable / disable chip. Uses counting logic
>>> +          1 enables the chip
>>> +          0 disables the chip
>>> +lux0_input
>>> +     RO - measured LUX value
>>> +          range: 0 .. 65535
>>> +          sysfs_notify called when threshold interrupt occurs
>> Personally I'm still in favour (for generality) of using illuminance0
>> rather than lux0 but I think the mass of drivers mean I'm losing that
>> argument. (argument for those who haven't seen it before is that lux is
>> the unit, the thing being measured is illuminance - units don't
>> generalize to other sensor types e.g. acceleration)
>>
> 
> I think Alan just got one driver queued with lux :)
Yup, that was largely why I was giving up on the argument. I won it
with als but as that never merged, the consensus is beating me
this time ;)  Its a small point anyway and I can always blame it
on Alan if anyone points out it is inconsistent in the future :)
> 
>>> +
>>> +lux0_input10x
>>> +     RO - Same as lux0_input but values are 10x larger so accuracy
>>> +          is 0.1 lux
>>> +          range: 0 .. 655350
>> Why do we care?  Just do a bit of fixed point arithmetic and output
>> it in lux0_input (yes hwmon is fussy about not having decimal places,
>> I think everyone else is more flexible in cases like this).
> 
> %d.%d :)
Exactly, might be hideous, but it does the job.
> 
>>> +lux0_rate
>>> +     RW - measurement period in milliseconds
>> Then it's not a rate is it.  Please fix naming or what comes out.
>> Rate = sampling frequency, not sampling period.
>>
> 
> rate -> period
Snag there is that I don't think anyone is using period for this
particular parameter... If it's easy I'd stick with rate or sampling
frequency and output 1/rate  If I'm wrong and for example Alan has used
this then feel free!
> 
>>> +
>>> +lux0_rate_avail
>>> +     RO - supported measurement periods
>>> +
>>> +lux0_calib
>>> +     RW - calibration value. Set to neutral value by default
>> That tells me nothing about what this parameter is.  Please provide
>> some info on this so we can work out a clearer naming convention.
>>> +
> 
> Shortly - results are multiplied with calib value. And calib_format
> tells what is the neutral value ( == 1.00 multiplier).
Then can we have calib_default or perhaps. calib_neutral
It clearly isn't what we would normally think of as a format.
> This is used to
> compensate differences between sensors. To get proper value, you need
> calibrated source of light and check what is the output from the sensor.
> Then just set calib value so that reported lux value is about the
> correct one.
In IIO we have gone with _calibscale for the equivalent parameter
(and _calibbias for devices where there is an offset as well).

> 
>>> +lux0_calib_format
>>> +     RO - neutral calibration value
>>> +
>>> +lux0_threshold_range
>>> +     RW - lo and hi threshold values like "100 1000"
>>> +          range: 0 .. 65535
>> Are these conceptually one value... Maybe. I proposed an
>> RFC with a general naming convention the other day. I'm happy to reopen
>> that debate if you disagree, but do feel it is vital to pick a standard
>> and keep to it. I've move iio to the conventions proposed in that discussion
>> (on linux-iio for review at the moment).
>>
>> Under that convention this would be (I think, given I don't have the data sheet)
>>
>> lux0_input_thresh_rising_value
>> lux0_input_thresh_falling_value
>>
> 
> Data sheets often uses HI and LO threshold naming. But yes, interrupt
> comes when the level is above the hi-limit or below lo-limit.
> Strictly, it is not crossing the limit.
Fair point, the rising falling was the most concise way of indicating
which side of the threshold causes the interrupt.  A lot of devices
act as 'edge' triggers. That is they won't retrigger until the
value has fallen below the threshold. (corresponds to an infinite
delay before resetting)
> 
> Two separate entries makes sense but how about the naming...
> 
> lux0_input_thresh_above_value
> lux0_input_thresh_below_value

Works for me.  
> 
> would tell when interrupt is triggered.
> 
>> For reference the naming convention discussion follows from
>>
>> http://linux.derkeiler.com/Mailing-Lists/Kernel/2010-09/msg08622.html
>> (lkml.org seems to be down)
>>> +
>>> +prox0_enable
>> For proximity, I'd be more inclined to not abbreviate this.
>>> +     RW - enable / disable proximity - counting logic
>>> +          1 enables the proximity
>>> +          0 disables the proximity
>> So this is concerned with the thresholds below?  Again, proposed
>> naming would be:
>>
>> proximity0_thresh_rising_en (might have naming wrong there, I'm not sure what it
>>         is).
> 
> no, this is enabling the proximity measurement in general. Obviously it
> is not clear :(
Then your original was fine!  I just got confused by the other attributes.
> 
>>> +
>>> +prox0_input
>>> +     RO - measured proximity value
>>> +          range 0 .. 1023
>>> +          sysfs_notify called when threshold interrupt occurs
>> So this only exists when a threshold interrupt has occurred?  If so can we
>> name it in a fashion to indicate this?  If the units are SI or at least
>> obvious, please call it _raw to indicate that.
> 
> _input -> _raw
> 
>>> +
>>> +prox0_continuous_mode
>>> +     RW - When set, results above threshold are reported periodically.
>>> +          In this mode rough distance estimation is possible.
>>> +          Values: 0 and 1
>> This naming needs some thought.  It's far from obvious that _continuous_mode
>> would correspond to this behaviour.
> 
> any ideas about naming?
I've seen this in a couple of devices but none of the drivers actually support
it yet.  Perhaps, persistenceperiod? (added to meanperiod and period that
we already have defined for IIO).
> 
> 
>>> +
>>> +prox0_threshold
>>> +     RW - threshold level which trigs proximity events.
>> Name could be more informative.
> 
> prox0_raw_thres_above_value?
If it doesn't matter to you can we have thresh rather than thres
(easy way of matching IIO!)
> 
>>> +
>>> +Platform data
>>> +-------------
>>> +
>>> +
>>> +#define APDS_IRLED_CURR_12mA 0x3
>>> +#define APDS_IRLED_CURR_25mA 0x2
>>> +#define APDS_IRLED_CURR_50mA 0x1
>>> +#define APDS_IRLED_CURR_100mA        0x0
>>> +
>>> +/*
>>> + * Structure for tuning ALS calculation to match with environment.
>>> + * There depends on the material above the sensor and the sensor
>>> + * itself. If the GA is zero, driver will use uncovered sensor default values
>>> + * format: decimal value * APDS_PARAM_SCALE
>>> + */
>>> +#define APDS_PARAM_SCALE 4096
>>> +struct apds990x_chip_factors {
>>> +     int ga;         /* Glass attenuation */
>>> +     int cf1;        /* Clear channel factor 1 */
>>> +     int irf1;       /* Ir channel factor 1 */
>>> +     int cf2;        /* Clear channel factor 2 */
>>> +     int irf2;       /* Ir channel factor 2 */
>>> +     int df;         /* Device factor. Decimal number */
>>> +};
>>> +
>>> +struct apds990x_platform_data {
>>> +     struct apds990x_chip_factors cf;
>>> +     u8     pdrive;
>>> +     int    (*setup_resources)(void);
>>> +     int    (*release_resources)(void);
>>> +};
>>> +
>>> +chip factors are specific to for example dark plastic window
>>> +above the sensor. Driver uses plain (uncovered) sensor default values
>>> +if the platform data contains zero ga factor.
>>> +
>>> +pdrive is the IR led driver current
>>> +
>>> +setup / release resources handles interrupt line configurations.
>>> diff --git a/Documentation/misc-devices/bhsfh.txt b/Documentation/misc-devices/bhsfh.txt
>>> new file mode 100644
>>> index 0000000..7f72f84
>>> --- /dev/null
>>> +++ b/Documentation/misc-devices/bhsfh.txt
>>> @@ -0,0 +1,125 @@
>>> +Kernel driver bhsfh
>>> +===================
>>> +
>>> +Supported chips:
>>> +ROHM BH1770GLC
>>> +OSRAM SFH7770
>>> +
>>> +Data sheet:
>>> +Not freely available
>>> +
>>> +Author:
>>> +Samu Onkalo <samu.p.onkalo-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
>>> +
>>> +Description
>>> +-----------
>>> +BH1770GLC and SFH7770 (I'm calling them as bhsfh) are combined ambient
>>> +light and proximity sensors. ALS and proximity parts operates on their own,
>>> +but they shares common I2C interface and interrupt logic.
>>> +
>>> +ALS produces 16 bit LUX values. The chip contains interrupt logic to produce
>>> +low and high threshold interrupts.
>>> +
>>> +Proximity part contains IR-led driver up to 3 IR leds. The chip measures
>>> +amount of reflected IR light and produces proximity result. Resolution is
>>> +8 bit. Driver supports only one channel. Driver uses ALS results to estimate
>>> +realibility of the proximity results. Thus ALS is always running while
>>> +proximity detection is needed.
>>> +
>>> +Driver uses threshold interrupts to avoid need for polling the values.
>>> +Proximity low interrupt doesn't exists in the chip. This is simulated
>>> +by using a delayed work.
>>> +
>>> +SYSFS
>>> +-----
>>> +
>>> +chip_id
>>> +     RO - shows detected chip type and version
>>> +
>>> +power_state
>>> +     RW - enable / disable chip. Uses counting logic
>>> +          1 enables the chip
>>> +          0 disables the chip
>>> +
>>> +lux0_input
>>> +     RO - measured LUX value
>>> +          range: 0 .. 65535
>>> +          sysfs_notify called when threshold interrupt occurs
>>> +
>>> +lux0_rate
>>> +     RW - measurement period in milliseconds
>>> +
>>> +lux0_rate_avail
>>> +     RO - supported measurement periods
>>> +
>>> +lux0_threshold_range
>>> +     RW - lo and hi threshold values like "100 1000"
>>> +          range: 0 .. 65535
>>> +
>>> +lux0_calib
>>> +     RW - calibration value. Set to neutral value by default
>> That is not sufficient documentation.  What is the calibration value used
>> for?  As a random user coming in who isn't going to read the data sheet this
>> comment tells me nothing.
>>> +
>>> +lux0_calib_format
>>> +     RO - neutral calibration value
>> This one tells me even less.
>>> +
>>> +prox0_input
>>> +     RO - measured proximity value
>>> +          range 0 .. 255
>>> +          sysfs_notify called when threshold interrupt occurs
>>> +
>>> +prox0_enable
>>> +     RW - enable / disable proximity - uses counting logic
>>> +          1 enables the proximity
>>> +          0 disables the proximity
>> So this is enabling the threshold?  Or is it turning on the ability to
>> read directly from the sensor?  Not clear in the naming (and it should be).
> 
> prox0_raw_enable?
prox0_raw_en (again if it doesn't cost you anything, it matches various iio devices).
> 
>>> +
>>> +prox0_persistence
>>> +     RW - number of proximity interrupts needed before triggering the event
>> naming RFC proposed calling this (roughly given I'm not clear on type of interrupt)
>>
>> proximity0_thresh_rising_period
> 
> prox0_thresh_above_count
period is defined as exactly this in IIO so I'd rather stay with that if possible.
Snag is that is in seconds so conversion code would be needed.
> 
> 
>>> +
>>> +prox0_rate
>>> +     RW - supported measurement periods. There are two values in this entry:
>>> +          First one is used when waiting for proximity event and the second
>>> +          one is used when waiting proximity reason to go away.
>> That's not conceptually one value, so break it in two.
> 
> ok
> 
>>> +
>>> +prox0_rate
>>> +     RO - available measurement periods
>>> +
>>> +prox0_threshold
>>> +     RW - threshold level which trigs proximity events.
>>> +          Filtered by persistence filter
>>> +
> 
> prox0_raw_thres_above_value?
thresh again if it doesn't matter. Otherwise yes.
> 
> 
>>> +prox0_abs_thres
>>> +     RW - threshold level which trigs event immediately
>> The naming of these last two is extremely confusing and inconsistent.
>> Why abs?  Is it on the absolute value?  That seems an odd thing given
>> I doubt proximity is a signed value?  Or is this just a second alarm
>> that ignores the persistence filter?
>>
> 
> 
> Idea here is that "prox0_threshold" limit is something which must trig
> proximit event, but there is no hurry. Since the sensor may be noisy
> (false events every now and then), there must be several interrupts in a
> row (persistence value) to get proximity event at that level. 
> However, high enough value (sensor covered totally) must trig event
> immediately (abs_thres).
> 
> prox0_raw_thres_above_value_strict?
Could we just add numbers to the two detectors one of which has a
period of 1 (or not specified) Easier to generalise than applying
another keyword to the name.

prox0_raw_thresh_above0_value
prox0_raw_thresh_above0_period

and this one
prox0_raw_thresh_above1_value

I think that would be the best place to apply the index?
Attributes that apply to both alarms would just not specify
it.  So if they were for example, enabled together.

prox0_raw_thresh_above_en

> 
>>> +
>>> +Platform data:
>>> +
>>> +struct bhsfh_platform_data {
>>> +#define BHSFH_LED_5mA        0
>>> +#define BHSFH_LED_10mA       1
>>> +#define BHSFH_LED_20mA       2
>>> +#define BHSFH_LED_50mA       3
>>> +#define BHSFH_LED_100mA 4
>>> +#define BHSFH_LED_150mA 5
>>> +#define BHSFH_LED_200mA 6
>>> +     __u8 led_def_curr;
>>> +#define BHFSH_NEUTRAL_GA 16384 /* 16384 / 16384 = 1 */
>>> +     __u32 glass_attenuation;
>>> +     int (*setup_resources)(void);
>>> +     int (*release_resources)(void);
>>> +};
>>> +
>>> +led_def_curr controls IR led driving current
>>> +glass_attenuation tells darkness of the covering window
>>> +setup / release resources are call back functions for controlling
>>> +interrupt line.
>>> +
>>> +Example:
>>> +static struct bhsfh_platform_data rm680_bhsfh_data = {
>>> +     .led_def_curr      = BHSFH_LED_50mA,
>>> +     .glass_attenuation = (16384 * 385) / 100,
>>> +     .setup_resources   = bhsfh_setup,
>>> +     .release_resources = bhsfh_release,
>>> +};
>>> +
>>> +glass_attenuation: 385 in above formula means 3.85 in decimal.
>>> +light_above_sensors = light_above_cover_window / 3.85
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/