Re: [PATCH v3 1/2] iio:light: Add Dyna-Image AP3223 ambient light and proximity driver

2015-10-03 Thread Jonathan Cameron
On 30/09/15 02:36, Suresh Rajashekara wrote:
> Minimal implementation with support for raw light and proximity reading.
> 
> This is based on the driver provided by the vendor
> (which was an input driver). Authors of the
> driver from the vendor included
> * John Huang 
> * Templeton Tsai 
> * Vic Lee 
> 
> v3
> * Resubmitting due to typo in accompanying patch
> 
> v2
> * Using regmap framework
> * Error handling
> * Code cleanups following comments
> 
> Signed-off-by: Suresh Rajashekara 
Fundamentally fine and heading in the right direction.
A lot of cases of eating error codes that should be returned and
a few other easy cleanups that will make it more readable and
easier to review.

Thanks,

Jonathan
> ---
>  drivers/iio/light/Kconfig  |  11 +
>  drivers/iio/light/Makefile |   1 +
>  drivers/iio/light/ap3223.c | 706 
> +
>  3 files changed, 718 insertions(+)
>  create mode 100644 drivers/iio/light/ap3223.c
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index ae68c64..5c93ef0 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -27,6 +27,17 @@ config AL3320A
>To compile this driver as a module, choose M here: the
>module will be called al3320a.
>  
> +config AP3223
> + tristate "AP3223 ambient light and proximity sensor"
> + depends on I2C
> + select REGMAP_I2C
> + help
> +  Say Y here if you want to build a driver for the Dyna Image AP3223
> +  ambient light and proximity sensor.
> +
> +  To compile this driver as a module, choose M here: the
> +  module will be called ap3223.
> +
>  config APDS9300
>   tristate "APDS9300 ambient light sensor"
>   depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index b12a516..13a74f9 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -5,6 +5,7 @@
>  # When adding new entries keep the list in alphabetical order
>  obj-$(CONFIG_ADJD_S311)  += adjd_s311.o
>  obj-$(CONFIG_AL3320A)+= al3320a.o
> +obj-$(CONFIG_AP3223) += ap3223.o
>  obj-$(CONFIG_APDS9300)   += apds9300.o
>  obj-$(CONFIG_CM32181)+= cm32181.o
>  obj-$(CONFIG_CM3232) += cm3232.o
> diff --git a/drivers/iio/light/ap3223.c b/drivers/iio/light/ap3223.c
> new file mode 100644
> index 000..6b457c1
> --- /dev/null
> +++ b/drivers/iio/light/ap3223.c
> @@ -0,0 +1,706 @@
> +/*
> + * Copyright (C) 2015 Google, Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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.
> + *
> + * Note about the original authors:
> + *
> + * The driver for AP3223 was originally distributed by dyna image in a
> + * different framework (as an input driver). This driver uses code from
> + * that driver and converts it to iio framework. The non-iio driver from
> + * dyna image is not available online anywhere, so there is no reference
> + * for it here. However, that driver is also GPLv2.
> + * The following is part of the header found in that file
> + * (The GPL notice from the original header is removed)
> + *
> + * >> This file is part of the AP3223, AP3212C and AP3216C sensor driver.
> + * >> AP3426 is combined proximity and ambient light sensor.
> + * >> AP3216C is combined proximity, ambient light sensor and IRLED.
> + * >>
> + * >> Contact: John Huang 
> + * >>   Templeton Tsai 
> + *
> + * Another author initials mentioned in that file was just YC (and no name).
> + *
> + * Not sure for what kernel version the driver from dyna image was written 
> for.
> + * Vic Lee  made modifications to it to run on 3.14.
> + *
> + * Datasheet:
> + * 
> http://www.dyna-image.com/english/product/optical-sensor-detail.php?cpid=2&dpid=8#doc
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define AP3223_DRV_NAME "ap3223"
> +
> +/* ap3223 control registers */
> +
> +#define  AP3223_REG_SYS_CTRL 0x00
> +#define AP3223_REG_SYS_CTRL_SHIFT(0)
> +#define AP3223_REG_SYS_CTRL_MASK 0x07
> +
> +/* System Mode (AP3223_REG_SYS_CTRL) */
> +
> +#define AP3223_SYS_DEV_DOWN  0x00
> +#define AP3223_SYS_ALS_ENABLE0x01
> +#define AP3223_SYS_PS_ENABLE 0x02
> +#define AP3223_SYS_ALS_PS_ENABLE 0x03
> +#define AP3223_SYS_DEV_RESET 0x04
> +
> +#define AP3223_REG_SYS_INTSTATUS 0x01
> +#define AP3223_REG_SYS_INT_SHIFT (0)
> +#define AP3223_REG_SYS_INT_

[PATCH v3 1/2] iio:light: Add Dyna-Image AP3223 ambient light and proximity driver

2015-09-29 Thread Suresh Rajashekara
Minimal implementation with support for raw light and proximity reading.

This is based on the driver provided by the vendor
(which was an input driver). Authors of the
driver from the vendor included
* John Huang 
* Templeton Tsai 
* Vic Lee 

v3
* Resubmitting due to typo in accompanying patch

v2
* Using regmap framework
* Error handling
* Code cleanups following comments

Signed-off-by: Suresh Rajashekara 
---
 drivers/iio/light/Kconfig  |  11 +
 drivers/iio/light/Makefile |   1 +
 drivers/iio/light/ap3223.c | 706 +
 3 files changed, 718 insertions(+)
 create mode 100644 drivers/iio/light/ap3223.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index ae68c64..5c93ef0 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -27,6 +27,17 @@ config AL3320A
 To compile this driver as a module, choose M here: the
 module will be called al3320a.
 
+config AP3223
+   tristate "AP3223 ambient light and proximity sensor"
+   depends on I2C
+   select REGMAP_I2C
+   help
+Say Y here if you want to build a driver for the Dyna Image AP3223
+ambient light and proximity sensor.
+
+To compile this driver as a module, choose M here: the
+module will be called ap3223.
+
 config APDS9300
tristate "APDS9300 ambient light sensor"
depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index b12a516..13a74f9 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -5,6 +5,7 @@
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_ADJD_S311)+= adjd_s311.o
 obj-$(CONFIG_AL3320A)  += al3320a.o
+obj-$(CONFIG_AP3223)   += ap3223.o
 obj-$(CONFIG_APDS9300) += apds9300.o
 obj-$(CONFIG_CM32181)  += cm32181.o
 obj-$(CONFIG_CM3232)   += cm3232.o
diff --git a/drivers/iio/light/ap3223.c b/drivers/iio/light/ap3223.c
new file mode 100644
index 000..6b457c1
--- /dev/null
+++ b/drivers/iio/light/ap3223.c
@@ -0,0 +1,706 @@
+/*
+ * Copyright (C) 2015 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ * Note about the original authors:
+ *
+ * The driver for AP3223 was originally distributed by dyna image in a
+ * different framework (as an input driver). This driver uses code from
+ * that driver and converts it to iio framework. The non-iio driver from
+ * dyna image is not available online anywhere, so there is no reference
+ * for it here. However, that driver is also GPLv2.
+ * The following is part of the header found in that file
+ * (The GPL notice from the original header is removed)
+ *
+ * >> This file is part of the AP3223, AP3212C and AP3216C sensor driver.
+ * >> AP3426 is combined proximity and ambient light sensor.
+ * >> AP3216C is combined proximity, ambient light sensor and IRLED.
+ * >>
+ * >> Contact: John Huang 
+ * >> Templeton Tsai 
+ *
+ * Another author initials mentioned in that file was just YC (and no name).
+ *
+ * Not sure for what kernel version the driver from dyna image was written for.
+ * Vic Lee  made modifications to it to run on 3.14.
+ *
+ * Datasheet:
+ * 
http://www.dyna-image.com/english/product/optical-sensor-detail.php?cpid=2&dpid=8#doc
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define AP3223_DRV_NAME "ap3223"
+
+/* ap3223 control registers */
+
+#defineAP3223_REG_SYS_CTRL 0x00
+#define AP3223_REG_SYS_CTRL_SHIFT  (0)
+#define AP3223_REG_SYS_CTRL_MASK   0x07
+
+/* System Mode (AP3223_REG_SYS_CTRL) */
+
+#define AP3223_SYS_DEV_DOWN0x00
+#define AP3223_SYS_ALS_ENABLE  0x01
+#define AP3223_SYS_PS_ENABLE   0x02
+#define AP3223_SYS_ALS_PS_ENABLE   0x03
+#define AP3223_SYS_DEV_RESET   0x04
+
+#define AP3223_REG_SYS_INTSTATUS   0x01
+#define AP3223_REG_SYS_INT_SHIFT   (0)
+#define AP3223_REG_SYS_INT_ALS_SHIFT   (0)
+#define AP3223_REG_SYS_INT_PS_SHIFT(1)
+#define AP3223_REG_SYS_INT_OBJ_SHIFT   (4)
+#define AP3223_REG_SYS_INT_IR_OV_SHIFT (5)
+
+/* INT FLAG BIT MASK */
+
+#define AP3223_REG_SYS_INT_MASK0x03
+#define AP3223_REG_SYS_INT_AMASK   0x01
+#define AP3223_REG_SYS_INT_PMASK   0x02
+#define AP3223_REG_SYS_INT_OBJ_MASK0x10
+#define AP3223_REG_SYS_INT_IR_OV_MASK  0x20
+
+#define AP3223_REG_SYS_INTCTRL 0x02
+
+/* INT Clear Manner */
+
+#defineAP3223_SYS_INT_CLEAR_AU