[PATCH 0/4] input: misc: gp2a: Add device tree support

[PATCH 0/4] input: misc: gp2a: Add device tree support

[Paweł Chmiel]

The main goal of this patchset is to add device tree support to driver.

First patch is doing little cleanup by using managed resource helpers.

Second patch adds support for light sensor part, which is supported by hw,
but was not supported by driver.

The last two patches adds device tree support to driver,
with documentation.

It was tested on s5pv210-galaxys and s5pv210-fascinate4g.

Jonathan Bakker (4):
  input: misc: gp2a: Use managed resource helpers
  input: misc: gp2a: Add support for light sensor
  input: misc: gp2a: Enable device tree
  dt-bindings: input: Add documentation for gp2a sensor

 .../bindings/input/sharp,gp2ap002a00f.txt     |  29 ++++
 drivers/input/misc/Kconfig                    |   2 +
 drivers/input/misc/gp2ap002a00f.c             | 154 +++++++++++++++---
 include/linux/input/gp2ap002a00f.h            |   4 +
 4 files changed, 163 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt

-- 
2.17.1

[PATCH 1/4] input: misc: gp2a: Use managed resource helpers

[Paweł Chmiel]

From: Jonathan Bakker <xc-racer2@live.ca>

Simplify cleanup of failures by using managed resource helpers

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 drivers/input/misc/gp2ap002a00f.c | 37 ++++++++++---------------------
 1 file changed, 12 insertions(+), 25 deletions(-)

diff --git a/drivers/input/misc/gp2ap002a00f.c b/drivers/input/misc/gp2ap002a00f.c
index c6a29e57b5e4..79c8c4c56d1a 100644
--- a/drivers/input/misc/gp2ap002a00f.c
+++ b/drivers/input/misc/gp2ap002a00f.c
@@ -138,14 +138,15 @@ static int gp2a_probe(struct i2c_client *client,
 			return error;
 	}
 
-	error = gpio_request_one(pdata->vout_gpio, GPIOF_IN, GP2A_I2C_NAME);
+	error = devm_gpio_request_one(&client->dev, pdata->vout_gpio,
+				      GPIOF_IN, GP2A_I2C_NAME);
 	if (error)
 		goto err_hw_shutdown;
 
-	dt = kzalloc(sizeof(struct gp2a_data), GFP_KERNEL);
+	dt = devm_kzalloc(&client->dev, sizeof(struct gp2a_data), GFP_KERNEL);
 	if (!dt) {
 		error = -ENOMEM;
-		goto err_free_gpio;
+		goto err_hw_shutdown;
 	}
 
 	dt->pdata = pdata;
@@ -153,12 +154,12 @@ static int gp2a_probe(struct i2c_client *client,
 
 	error = gp2a_initialize(dt);
 	if (error < 0)
-		goto err_free_mem;
+		goto err_hw_shutdown;
 
-	dt->input = input_allocate_device();
+	dt->input = devm_input_allocate_device(&client->dev);
 	if (!dt->input) {
 		error = -ENOMEM;
-		goto err_free_mem;
+		goto err_hw_shutdown;
 	}
 
 	input_set_drvdata(dt->input, dt);
@@ -171,19 +172,18 @@ static int gp2a_probe(struct i2c_client *client,
 
 	input_set_capability(dt->input, EV_SW, SW_FRONT_PROXIMITY);
 
-	error = request_threaded_irq(client->irq, NULL, gp2a_irq,
-			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
-				IRQF_ONESHOT,
-			GP2A_I2C_NAME, dt);
+	error = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+			gp2a_irq, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
+			IRQF_ONESHOT, GP2A_I2C_NAME, dt);
 	if (error) {
 		dev_err(&client->dev, "irq request failed\n");
-		goto err_free_input_dev;
+		goto err_hw_shutdown;
 	}
 
 	error = input_register_device(dt->input);
 	if (error) {
 		dev_err(&client->dev, "device registration failed\n");
-		goto err_free_irq;
+		goto err_hw_shutdown;
 	}
 
 	device_init_wakeup(&client->dev, pdata->wakeup);
@@ -191,14 +191,6 @@ static int gp2a_probe(struct i2c_client *client,
 
 	return 0;
 
-err_free_irq:
-	free_irq(client->irq, dt);
-err_free_input_dev:
-	input_free_device(dt->input);
-err_free_mem:
-	kfree(dt);
-err_free_gpio:
-	gpio_free(pdata->vout_gpio);
 err_hw_shutdown:
 	if (pdata->hw_shutdown)
 		pdata->hw_shutdown(client);
@@ -210,12 +202,7 @@ static int gp2a_remove(struct i2c_client *client)
 	struct gp2a_data *dt = i2c_get_clientdata(client);
 	const struct gp2a_platform_data *pdata = dt->pdata;
 
-	free_irq(client->irq, dt);
-
 	input_unregister_device(dt->input);
-	kfree(dt);
-
-	gpio_free(pdata->vout_gpio);
 
 	if (pdata->hw_shutdown)
 		pdata->hw_shutdown(client);
-- 
2.17.1

[PATCH 2/4] input: misc: gp2a: Add support for light sensor

[Paweł Chmiel]

From: Jonathan Bakker <xc-racer2@live.ca>

The gp2a driver previously only supported the proximity part of the
sensor while the hardware supports both.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 drivers/input/misc/Kconfig         |  2 +
 drivers/input/misc/gp2ap002a00f.c  | 71 +++++++++++++++++++++++++++++-
 include/linux/input/gp2ap002a00f.h |  4 ++
 3 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index ca59a2be9bc5..a532efb4e6d8 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -252,6 +252,8 @@ config INPUT_GP2A
 	tristate "Sharp GP2AP002A00F I2C Proximity/Opto sensor driver"
 	depends on I2C
 	depends on GPIOLIB || COMPILE_TEST
+	depends on IIO
+	select INPUT_POLLDEV
 	help
 	  Say Y here if you have a Sharp GP2AP002A00F proximity/als combo-chip
 	  hooked to an I2C bus.
diff --git a/drivers/input/misc/gp2ap002a00f.c b/drivers/input/misc/gp2ap002a00f.c
index 79c8c4c56d1a..090c8c313295 100644
--- a/drivers/input/misc/gp2ap002a00f.c
+++ b/drivers/input/misc/gp2ap002a00f.c
@@ -10,9 +10,12 @@
  */
 
 #include <linux/i2c.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
 #include <linux/irq.h>
 #include <linux/slab.h>
 #include <linux/input.h>
+#include <linux/input-polldev.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/gpio.h>
@@ -20,7 +23,9 @@
 #include <linux/input/gp2ap002a00f.h>
 
 struct gp2a_data {
+	struct iio_channel *channel;
 	struct input_dev *input;
+	struct input_polled_dev *poll_dev;
 	const struct gp2a_platform_data *pdata;
 	struct i2c_client *i2c_client;
 };
@@ -58,6 +63,19 @@ static irqreturn_t gp2a_irq(int irq, void *handle)
 	return IRQ_HANDLED;
 }
 
+static void gp2a_poll(struct input_polled_dev *dev)
+{
+	struct gp2a_data *dt = dev->private;
+	int ret, value;
+
+	ret = iio_read_channel_processed(dt->channel, &value);
+	if (ret < 0)
+		dev_err(&dt->i2c_client->dev, "failed to read value!");
+
+	input_report_abs(dev->input, ABS_MISC, value);
+	input_sync(dev->input);
+}
+
 static int gp2a_enable(struct gp2a_data *dt)
 {
 	return i2c_smbus_write_byte_data(dt->i2c_client, GP2A_ADDR_OPMOD,
@@ -127,7 +145,7 @@ static int gp2a_probe(struct i2c_client *client,
 {
 	const struct gp2a_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct gp2a_data *dt;
-	int error;
+	int error, value;
 
 	if (!pdata)
 		return -EINVAL;
@@ -152,6 +170,49 @@ static int gp2a_probe(struct i2c_client *client,
 	dt->pdata = pdata;
 	dt->i2c_client = client;
 
+	dt->channel = devm_iio_channel_get(&client->dev, "light");
+	if (!IS_ERR(dt->channel)) {
+		if (!dt->channel->indio_dev) {
+			error = -ENXIO;
+			goto err_hw_shutdown;
+		}
+
+		if (dt->pdata->light_adc_max <= 0 ||
+			dt->pdata->light_adc_fuzz <= 0) {
+			error = -EINVAL;
+			goto err_hw_shutdown;
+		}
+
+		dt->poll_dev = devm_input_allocate_polled_device(&client->dev);
+		if (!dt->poll_dev) {
+			dev_err(&client->dev,
+				"failed to allocate polled input device");
+			error = -ENOMEM;
+			goto err_hw_shutdown;
+		}
+
+		if (!device_property_read_u32(&client->dev, "poll-interval",
+					      &value))
+			dt->poll_dev->poll_interval = value;
+
+		dt->poll_dev->poll = gp2a_poll;
+		dt->poll_dev->private = dt;
+
+		dt->poll_dev->input->name = GP2A_I2C_NAME;
+
+		input_set_capability(dt->poll_dev->input, EV_ABS, ABS_MISC);
+		input_set_abs_params(dt->poll_dev->input, ABS_MISC, 0,
+				     dt->pdata->light_adc_max,
+				     dt->pdata->light_adc_fuzz, 0);
+
+		error = input_register_polled_device(dt->poll_dev);
+		if (error)
+			goto err_hw_shutdown;
+	} else if (PTR_ERR(dt->channel) == -EPROBE_DEFER) {
+		error = -EPROBE_DEFER;
+		goto err_hw_shutdown;
+	}
+
 	error = gp2a_initialize(dt);
 	if (error < 0)
 		goto err_hw_shutdown;
@@ -203,6 +264,8 @@ static int gp2a_remove(struct i2c_client *client)
 	const struct gp2a_platform_data *pdata = dt->pdata;
 
 	input_unregister_device(dt->input);
+	if (dt->poll_dev)
+		input_unregister_polled_device(dt->poll_dev);
 
 	if (pdata->hw_shutdown)
 		pdata->hw_shutdown(client);
@@ -243,6 +306,12 @@ static int __maybe_unused gp2a_resume(struct device *dev)
 		mutex_unlock(&dt->input->mutex);
 	}
 
+	if (dt->poll_dev) {
+		/* Out of range value so real value goes through next */
+		input_abs_set_val(dt->poll_dev->input, ABS_MISC,
+				  -dt->pdata->light_adc_max);
+	}
+
 	return retval;
 }
 
diff --git a/include/linux/input/gp2ap002a00f.h b/include/linux/input/gp2ap002a00f.h
index 3614a13a8297..0212948897b4 100644
--- a/include/linux/input/gp2ap002a00f.h
+++ b/include/linux/input/gp2ap002a00f.h
@@ -12,12 +12,16 @@
  * @wakeup: Set to true if the proximity can wake the device from suspend
  * @hw_setup: Callback for setting up hardware such as gpios and vregs
  * @hw_shutdown: Callback for properly shutting down hardware
+ * @light_adc_max: Maximum adc value the light sensor will read
+ * @light_adc_fuzz: Fuzz value for light adc sensor
  */
 struct gp2a_platform_data {
 	int vout_gpio;
 	bool wakeup;
 	int (*hw_setup)(struct i2c_client *client);
 	int (*hw_shutdown)(struct i2c_client *client);
+	int light_adc_max;
+	int light_adc_fuzz;
 };
 
 #endif
-- 
2.17.1

[PATCH 3/4] input: misc: gp2a: Enable device tree

[Paweł Chmiel]

From: Jonathan Bakker <xc-racer2@live.ca>

Enable probing of gp2a via device tree via a simple of_match table

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 drivers/input/misc/gp2ap002a00f.c | 46 +++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/input/misc/gp2ap002a00f.c b/drivers/input/misc/gp2ap002a00f.c
index 090c8c313295..01a7d44ae921 100644
--- a/drivers/input/misc/gp2ap002a00f.c
+++ b/drivers/input/misc/gp2ap002a00f.c
@@ -21,6 +21,8 @@
 #include <linux/gpio.h>
 #include <linux/delay.h>
 #include <linux/input/gp2ap002a00f.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 
 struct gp2a_data {
 	struct iio_channel *channel;
@@ -140,6 +142,35 @@ static int gp2a_initialize(struct gp2a_data *dt)
 	return error;
 }
 
+static struct gp2a_platform_data *gp2a_parse_dt_pdata(struct device *dev)
+{
+	struct gp2a_platform_data *pdata;
+	int ret;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	pdata->wakeup = of_property_read_bool(dev->of_node, "wakeup");
+
+	pdata->vout_gpio = of_get_named_gpio(dev->of_node, "vout-gpio", 0);
+	if (pdata->vout_gpio < 0) {
+		dev_err(dev, "failed to find vout-gpio");
+		return ERR_PTR(-EINVAL);
+	}
+
+	ret = device_property_read_u32(dev, "light-adc-max",
+				       &pdata->light_adc_max);
+	if (ret)
+		pdata->light_adc_max = 4096;
+	ret = device_property_read_u32(dev, "light-adc-fuzz",
+				       &pdata->light_adc_fuzz);
+	if (ret)
+		pdata->light_adc_fuzz = 80;
+
+	return pdata;
+}
+
 static int gp2a_probe(struct i2c_client *client,
 				const struct i2c_device_id *id)
 {
@@ -147,6 +178,12 @@ static int gp2a_probe(struct i2c_client *client,
 	struct gp2a_data *dt;
 	int error, value;
 
+	if (IS_ENABLED(CONFIG_OF) && client->dev.of_node) {
+		pdata = gp2a_parse_dt_pdata(&client->dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
+
 	if (!pdata)
 		return -EINVAL;
 
@@ -317,6 +354,14 @@ static int __maybe_unused gp2a_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(gp2a_pm, gp2a_suspend, gp2a_resume);
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id gp2a_of_match[] = {
+	{ .compatible = "sharp,gp2ap002a00f" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, gp2a_of_match);
+#endif
+
 static const struct i2c_device_id gp2a_i2c_id[] = {
 	{ GP2A_I2C_NAME, 0 },
 	{ }
@@ -326,6 +371,7 @@ MODULE_DEVICE_TABLE(i2c, gp2a_i2c_id);
 static struct i2c_driver gp2a_i2c_driver = {
 	.driver = {
 		.name	= GP2A_I2C_NAME,
+		.of_match_table = of_match_ptr(gp2a_of_match),
 		.pm	= &gp2a_pm,
 	},
 	.probe		= gp2a_probe,
-- 
2.17.1

[PATCH 4/4] dt-bindings: input: Add documentation for gp2a sensor

[Paweł Chmiel]

From: Jonathan Bakker <xc-racer2@live.ca>

This commit adds documentation for Sharp GP2AP002A00F.
It's Proximity/Opto Sensor connected over i2c.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 .../bindings/input/sharp,gp2ap002a00f.txt     | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt

diff --git a/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt b/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
new file mode 100644
index 000000000000..c524eb7d3d60
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
@@ -0,0 +1,29 @@
+* Sharp GP2AP002A00F I2C Proximity/Opto Sensor
+
+Required properties:
+- compatible : Should be "sharp,gp2ap002a00f"
+- reg : The I2C address of the sensor
+- vout-gpio : The gpio connected to the vout pin
+- interrupt-parent : should be the phandle for the interrupt controller
+- interrupts : Interrupt mapping for GPIO IRQ, it should by configured with
+		flags IRQ_TYPE_EDGE_BOTH
+
+Optional properties:
+- wakeup : If the device is capable of waking up the system
+- io-channels : Phandle to an ADC channel connected to the light sensor
+- io-channel-names = "light";
+- poll-interval : Poll interval time in milliseconds, default 500ms
+- light-adc-max : Maximum light value reported, default 4096
+- light-adc-fuzz : Fuzz value for reported light value, default 80
+
+Example:
+
+gp2a@44 {
+	compatible = "sharp,gp2ap002a00f";
+	reg = <0x44>;
+	vout-gpio = <&gph0 2 GPIO_ACTIVE_HIGH>;
+	interrupt-parent = <&gph0>;
+	interrupts = <2 IRQ_TYPE_EDGE_BOTH>;
+	io-channels = <&adc 9>;
+	io-channel-names = "light";
+};
-- 
2.17.1

Re: [PATCH 1/4] input: misc: gp2a: Use managed resource helpers

[Dmitry Torokhov]

On Fri, Jan 25, 2019 at 06:50:42PM +0100, Paweł Chmiel wrote:
> From: Jonathan Bakker <xc-racer2@live.ca>
> 
> Simplify cleanup of failures by using managed resource helpers
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
>  drivers/input/misc/gp2ap002a00f.c | 37 ++++++++++---------------------
>  1 file changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/input/misc/gp2ap002a00f.c b/drivers/input/misc/gp2ap002a00f.c
> index c6a29e57b5e4..79c8c4c56d1a 100644
> --- a/drivers/input/misc/gp2ap002a00f.c
> +++ b/drivers/input/misc/gp2ap002a00f.c
> @@ -138,14 +138,15 @@ static int gp2a_probe(struct i2c_client *client,
>  			return error;
>  	}
>  
> -	error = gpio_request_one(pdata->vout_gpio, GPIOF_IN, GP2A_I2C_NAME);
> +	error = devm_gpio_request_one(&client->dev, pdata->vout_gpio,
> +				      GPIOF_IN, GP2A_I2C_NAME);
>  	if (error)
>  		goto err_hw_shutdown;
>  
> -	dt = kzalloc(sizeof(struct gp2a_data), GFP_KERNEL);
> +	dt = devm_kzalloc(&client->dev, sizeof(struct gp2a_data), GFP_KERNEL);
>  	if (!dt) {
>  		error = -ENOMEM;
> -		goto err_free_gpio;
> +		goto err_hw_shutdown;
>  	}
>  
>  	dt->pdata = pdata;
> @@ -153,12 +154,12 @@ static int gp2a_probe(struct i2c_client *client,
>  
>  	error = gp2a_initialize(dt);
>  	if (error < 0)
> -		goto err_free_mem;
> +		goto err_hw_shutdown;
>  
> -	dt->input = input_allocate_device();
> +	dt->input = devm_input_allocate_device(&client->dev);
>  	if (!dt->input) {
>  		error = -ENOMEM;
> -		goto err_free_mem;
> +		goto err_hw_shutdown;
>  	}
>  
>  	input_set_drvdata(dt->input, dt);
> @@ -171,19 +172,18 @@ static int gp2a_probe(struct i2c_client *client,
>  
>  	input_set_capability(dt->input, EV_SW, SW_FRONT_PROXIMITY);
>  
> -	error = request_threaded_irq(client->irq, NULL, gp2a_irq,
> -			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
> -				IRQF_ONESHOT,
> -			GP2A_I2C_NAME, dt);
> +	error = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> +			gp2a_irq, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
> +			IRQF_ONESHOT, GP2A_I2C_NAME, dt);
>  	if (error) {
>  		dev_err(&client->dev, "irq request failed\n");
> -		goto err_free_input_dev;
> +		goto err_hw_shutdown;
>  	}
>  
>  	error = input_register_device(dt->input);
>  	if (error) {
>  		dev_err(&client->dev, "device registration failed\n");
> -		goto err_free_irq;
> +		goto err_hw_shutdown;
>  	}
>  
>  	device_init_wakeup(&client->dev, pdata->wakeup);
> @@ -191,14 +191,6 @@ static int gp2a_probe(struct i2c_client *client,
>  
>  	return 0;
>  
> -err_free_irq:
> -	free_irq(client->irq, dt);
> -err_free_input_dev:
> -	input_free_device(dt->input);
> -err_free_mem:
> -	kfree(dt);
> -err_free_gpio:
> -	gpio_free(pdata->vout_gpio);
>  err_hw_shutdown:
>  	if (pdata->hw_shutdown)
>  		pdata->hw_shutdown(client);
> @@ -210,12 +202,7 @@ static int gp2a_remove(struct i2c_client *client)
>  	struct gp2a_data *dt = i2c_get_clientdata(client);
>  	const struct gp2a_platform_data *pdata = dt->pdata;
>  
> -	free_irq(client->irq, dt);
> -
>  	input_unregister_device(dt->input);

You do not need explicitly unregister input device if it is managed
(allocated with devm).

> -	kfree(dt);
> -
> -	gpio_free(pdata->vout_gpio);
>  
>  	if (pdata->hw_shutdown)
>  		pdata->hw_shutdown(client);

This is however is wrong, as you can't shutdown hardware before
disapling/freeing IRQ, etc. Given that there are no users of
gp2a_platform_data in kernel I'd recommend creating a preparatory patch
removing platform data support from the driver.

Thanks.

-- 
Dmitry

Re: [PATCH 2/4] input: misc: gp2a: Add support for light sensor

[Dmitry Torokhov]

On Fri, Jan 25, 2019 at 06:50:43PM +0100, Paweł Chmiel wrote:
> From: Jonathan Bakker <xc-racer2@live.ca>
> 
> The gp2a driver previously only supported the proximity part of the
> sensor while the hardware supports both.
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
>  drivers/input/misc/Kconfig         |  2 +
>  drivers/input/misc/gp2ap002a00f.c  | 71 +++++++++++++++++++++++++++++-
>  include/linux/input/gp2ap002a00f.h |  4 ++
>  3 files changed, 76 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index ca59a2be9bc5..a532efb4e6d8 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -252,6 +252,8 @@ config INPUT_GP2A
>  	tristate "Sharp GP2AP002A00F I2C Proximity/Opto sensor driver"
>  	depends on I2C
>  	depends on GPIOLIB || COMPILE_TEST
> +	depends on IIO
> +	select INPUT_POLLDEV
>  	help
>  	  Say Y here if you have a Sharp GP2AP002A00F proximity/als combo-chip
>  	  hooked to an I2C bus.
> diff --git a/drivers/input/misc/gp2ap002a00f.c b/drivers/input/misc/gp2ap002a00f.c
> index 79c8c4c56d1a..090c8c313295 100644
> --- a/drivers/input/misc/gp2ap002a00f.c
> +++ b/drivers/input/misc/gp2ap002a00f.c
> @@ -10,9 +10,12 @@
>   */
>  
>  #include <linux/i2c.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
>  #include <linux/irq.h>
>  #include <linux/slab.h>
>  #include <linux/input.h>
> +#include <linux/input-polldev.h>
>  #include <linux/module.h>
>  #include <linux/interrupt.h>
>  #include <linux/gpio.h>
> @@ -20,7 +23,9 @@
>  #include <linux/input/gp2ap002a00f.h>
>  
>  struct gp2a_data {
> +	struct iio_channel *channel;
>  	struct input_dev *input;
> +	struct input_polled_dev *poll_dev;
>  	const struct gp2a_platform_data *pdata;
>  	struct i2c_client *i2c_client;
>  };
> @@ -58,6 +63,19 @@ static irqreturn_t gp2a_irq(int irq, void *handle)
>  	return IRQ_HANDLED;
>  }
>  
> +static void gp2a_poll(struct input_polled_dev *dev)
> +{
> +	struct gp2a_data *dt = dev->private;
> +	int ret, value;
> +
> +	ret = iio_read_channel_processed(dt->channel, &value);
> +	if (ret < 0)
> +		dev_err(&dt->i2c_client->dev, "failed to read value!");
> +
> +	input_report_abs(dev->input, ABS_MISC, value);
> +	input_sync(dev->input);

No, light sensor is not an input device, keep it in IIO please.

-- 
Dmitry

Re: [PATCH 4/4] dt-bindings: input: Add documentation for gp2a sensor

[Dmitry Torokhov]

On Fri, Jan 25, 2019 at 06:50:45PM +0100, Paweł Chmiel wrote:
> From: Jonathan Bakker <xc-racer2@live.ca>
> 
> This commit adds documentation for Sharp GP2AP002A00F.
> It's Proximity/Opto Sensor connected over i2c.
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
>  .../bindings/input/sharp,gp2ap002a00f.txt     | 29 +++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt b/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
> new file mode 100644
> index 000000000000..c524eb7d3d60
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
> @@ -0,0 +1,29 @@
> +* Sharp GP2AP002A00F I2C Proximity/Opto Sensor
> +
> +Required properties:
> +- compatible : Should be "sharp,gp2ap002a00f"
> +- reg : The I2C address of the sensor
> +- vout-gpio : The gpio connected to the vout pin

Do you know what it is for?

> +- interrupt-parent : should be the phandle for the interrupt controller
> +- interrupts : Interrupt mapping for GPIO IRQ, it should by configured with
> +		flags IRQ_TYPE_EDGE_BOTH
> +
> +Optional properties:
> +- wakeup : If the device is capable of waking up the system
> +- io-channels : Phandle to an ADC channel connected to the light sensor
> +- io-channel-names = "light";
> +- poll-interval : Poll interval time in milliseconds, default 500ms
> +- light-adc-max : Maximum light value reported, default 4096
> +- light-adc-fuzz : Fuzz value for reported light value, default 80
> +
> +Example:
> +
> +gp2a@44 {
> +	compatible = "sharp,gp2ap002a00f";
> +	reg = <0x44>;
> +	vout-gpio = <&gph0 2 GPIO_ACTIVE_HIGH>;
> +	interrupt-parent = <&gph0>;
> +	interrupts = <2 IRQ_TYPE_EDGE_BOTH>;
> +	io-channels = <&adc 9>;
> +	io-channel-names = "light";
> +};
> -- 
> 2.17.1
> 

-- 
Dmitry

Re: [PATCH 4/4] dt-bindings: input: Add documentation for gp2a sensor

[Jonathan Bakker]

Hi Dmitry,

Thanks for your review of the patches.

Considering that the light sensor part should be in IIO, should the entire driver be rewritten as an IIO driver?  There's already the driver for gp2ap020a00f there which is presumably the gp2ap002a00f's successor and does the same functions.

On 2019-01-25 5:32 p.m., Dmitry Torokhov wrote:
> On Fri, Jan 25, 2019 at 06:50:45PM +0100, Paweł Chmiel wrote:
>> From: Jonathan Bakker <xc-racer2@live.ca>
>>
>> This commit adds documentation for Sharp GP2AP002A00F.
>> It's Proximity/Opto Sensor connected over i2c.
>>
>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
>> ---
>>  .../bindings/input/sharp,gp2ap002a00f.txt     | 29 +++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
>>
>> diff --git a/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt b/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
>> new file mode 100644
>> index 000000000000..c524eb7d3d60
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
>> @@ -0,0 +1,29 @@
>> +* Sharp GP2AP002A00F I2C Proximity/Opto Sensor
>> +
>> +Required properties:
>> +- compatible : Should be "sharp,gp2ap002a00f"
>> +- reg : The I2C address of the sensor
>> +- vout-gpio : The gpio connected to the vout pin
> 
> Do you know what it is for?
> 
It's the control of the main power supply to the chip.
>> +- interrupt-parent : should be the phandle for the interrupt controller
>> +- interrupts : Interrupt mapping for GPIO IRQ, it should by configured with
>> +		flags IRQ_TYPE_EDGE_BOTH
>> +
>> +Optional properties:
>> +- wakeup : If the device is capable of waking up the system
>> +- io-channels : Phandle to an ADC channel connected to the light sensor
>> +- io-channel-names = "light";
>> +- poll-interval : Poll interval time in milliseconds, default 500ms
>> +- light-adc-max : Maximum light value reported, default 4096
>> +- light-adc-fuzz : Fuzz value for reported light value, default 80
>> +
>> +Example:
>> +
>> +gp2a@44 {
>> +	compatible = "sharp,gp2ap002a00f";
>> +	reg = <0x44>;
>> +	vout-gpio = <&gph0 2 GPIO_ACTIVE_HIGH>;
>> +	interrupt-parent = <&gph0>;
>> +	interrupts = <2 IRQ_TYPE_EDGE_BOTH>;
>> +	io-channels = <&adc 9>;
>> +	io-channel-names = "light";
>> +};
>> -- 
>> 2.17.1
>>
> 
Thanks,
Jonathan

Re: [PATCH 4/4] dt-bindings: input: Add documentation for gp2a sensor

[Dmitry Torokhov]

On Sat, Jan 26, 2019 at 03:14:14AM +0000, Jonathan Bakker wrote:
> Hi Dmitry,
> 
> Thanks for your review of the patches.
> 
> Considering that the light sensor part should be in IIO, should the entire driver be rewritten as an IIO driver?  There's already the driver for gp2ap020a00f there which is presumably the gp2ap002a00f's successor and does the same functions.

I'd be fine with that.

> 
> On 2019-01-25 5:32 p.m., Dmitry Torokhov wrote:
> > On Fri, Jan 25, 2019 at 06:50:45PM +0100, Paweł Chmiel wrote:
> >> From: Jonathan Bakker <xc-racer2@live.ca>
> >>
> >> This commit adds documentation for Sharp GP2AP002A00F.
> >> It's Proximity/Opto Sensor connected over i2c.
> >>
> >> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> >> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> >> ---
> >>  .../bindings/input/sharp,gp2ap002a00f.txt     | 29 +++++++++++++++++++
> >>  1 file changed, 29 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt b/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
> >> new file mode 100644
> >> index 000000000000..c524eb7d3d60
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/input/sharp,gp2ap002a00f.txt
> >> @@ -0,0 +1,29 @@
> >> +* Sharp GP2AP002A00F I2C Proximity/Opto Sensor
> >> +
> >> +Required properties:
> >> +- compatible : Should be "sharp,gp2ap002a00f"
> >> +- reg : The I2C address of the sensor
> >> +- vout-gpio : The gpio connected to the vout pin
> > 
> > Do you know what it is for?
> > 
> It's the control of the main power supply to the chip.

In this case it should be a power supply (regulator), not gpio.

Thanks.

-- 
Dmitry