Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH][RESEND] HID: elan: Make array buf static, shrinks object size
From: Colin King @ 2019-01-25 16:05 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Don't populate the array buf on the stack but instead make it
static. Makes the object code smaller by 43 bytes:

Before:
   text	   data	    bss	    dec	    hex	filename
   7769	   1520	      0	   9289	   2449	drivers/hid/hid-elan.o

After:
   text	   data	    bss	    dec	    hex	filename
   7662	   1584	      0	   9246	   241e	drivers/hid/hid-elan.o

(gcc version 8.2.0 x86_64)

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/hid/hid-elan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
index 0bfd6d1b44c1..1c62095cee99 100644
--- a/drivers/hid/hid-elan.c
+++ b/drivers/hid/hid-elan.c
@@ -393,7 +393,7 @@ static int elan_start_multitouch(struct hid_device *hdev)
 	 * This byte sequence will enable multitouch mode and disable
 	 * mouse emulation
 	 */
-	const unsigned char buf[] = { 0x0D, 0x00, 0x03, 0x21, 0x00 };
+	static const unsigned char buf[] = { 0x0D, 0x00, 0x03, 0x21, 0x00 };
 	unsigned char *dmabuf = kmemdup(buf, sizeof(buf), GFP_KERNEL);
 
 	if (!dmabuf)
-- 
2.19.1

^ permalink raw reply related

* [PATCH 0/4] input: misc: gp2a: Add device tree support
From: Paweł Chmiel @ 2019-01-25 17:50 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, mark.rutland, mchehab+samsung, colyli, ckeepax,
	andrew.smirnov, arnd, xiaotong.lu, xc-racer2,
	pawel.mikolaj.chmiel, linux-input, linux-kernel, devicetree

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

^ permalink raw reply

* [PATCH 1/4] input: misc: gp2a: Use managed resource helpers
From: Paweł Chmiel @ 2019-01-25 17:50 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, mark.rutland, mchehab+samsung, colyli, ckeepax,
	andrew.smirnov, arnd, xiaotong.lu, xc-racer2,
	pawel.mikolaj.chmiel, linux-input, linux-kernel, devicetree
In-Reply-To: <20190125175045.22576-1-pawel.mikolaj.chmiel@gmail.com>

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

^ permalink raw reply related

* [PATCH 2/4] input: misc: gp2a: Add support for light sensor
From: Paweł Chmiel @ 2019-01-25 17:50 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, mark.rutland, mchehab+samsung, colyli, ckeepax,
	andrew.smirnov, arnd, xiaotong.lu, xc-racer2,
	pawel.mikolaj.chmiel, linux-input, linux-kernel, devicetree
In-Reply-To: <20190125175045.22576-1-pawel.mikolaj.chmiel@gmail.com>

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

^ permalink raw reply related

* [PATCH 3/4] input: misc: gp2a: Enable device tree
From: Paweł Chmiel @ 2019-01-25 17:50 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, mark.rutland, mchehab+samsung, colyli, ckeepax,
	andrew.smirnov, arnd, xiaotong.lu, xc-racer2,
	pawel.mikolaj.chmiel, linux-input, linux-kernel, devicetree
In-Reply-To: <20190125175045.22576-1-pawel.mikolaj.chmiel@gmail.com>

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

^ permalink raw reply related

* [PATCH 4/4] dt-bindings: input: Add documentation for gp2a sensor
From: Paweł Chmiel @ 2019-01-25 17:50 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, mark.rutland, mchehab+samsung, colyli, ckeepax,
	andrew.smirnov, arnd, xiaotong.lu, xc-racer2,
	pawel.mikolaj.chmiel, linux-input, linux-kernel, devicetree
In-Reply-To: <20190125175045.22576-1-pawel.mikolaj.chmiel@gmail.com>

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

^ permalink raw reply related

* [PATCH 0/3] input: misc: bma150: Add support for device tree
From: Paweł Chmiel @ 2019-01-25 18:43 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, mark.rutland, pawel.mikolaj.chmiel, xc-racer2,
	devicetree, linux-input, linux-kernel

This small patchset adds support for device tree to Bosch BMA150 Accelerometer
Sensor driver.

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

Jonathan Bakker (3):
  input: misc: bma150: Use managed resources helpers
  input: misc: bma150: Add support for device tree
  input: dt-bindings: Add binding for bma150 sensor

 .../bindings/input/bosch,bma150.txt           | 20 ++++++++
 drivers/input/misc/bma150.c                   | 50 +++++++++----------
 2 files changed, 44 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/bosch,bma150.txt

-- 
2.17.1

^ permalink raw reply

* [PATCH 1/3] input: misc: bma150: Use managed resources helpers
From: Paweł Chmiel @ 2019-01-25 18:43 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, mark.rutland, pawel.mikolaj.chmiel, xc-racer2,
	devicetree, linux-input, linux-kernel
In-Reply-To: <20190125184400.17669-1-pawel.mikolaj.chmiel@gmail.com>

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

The driver can be cleaned up 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/bma150.c | 40 +++++++++++++------------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index 1efcfdf9f8a8..d101bb0a33d6 100644
--- a/drivers/input/misc/bma150.c
+++ b/drivers/input/misc/bma150.c
@@ -471,7 +471,7 @@ static int bma150_register_input_device(struct bma150_data *bma150)
 	struct input_dev *idev;
 	int error;
 
-	idev = input_allocate_device();
+	idev = devm_input_allocate_device(&bma150->client->dev);
 	if (!idev)
 		return -ENOMEM;
 
@@ -482,10 +482,8 @@ static int bma150_register_input_device(struct bma150_data *bma150)
 	input_set_drvdata(idev, bma150);
 
 	error = input_register_device(idev);
-	if (error) {
-		input_free_device(idev);
+	if (error)
 		return error;
-	}
 
 	bma150->input = idev;
 	return 0;
@@ -496,7 +494,7 @@ static int bma150_register_polled_device(struct bma150_data *bma150)
 	struct input_polled_dev *ipoll_dev;
 	int error;
 
-	ipoll_dev = input_allocate_polled_device();
+	ipoll_dev = devm_input_allocate_polled_device(&bma150->client->dev);
 	if (!ipoll_dev)
 		return -ENOMEM;
 
@@ -511,10 +509,8 @@ static int bma150_register_polled_device(struct bma150_data *bma150)
 	bma150_init_input_device(bma150, ipoll_dev->input);
 
 	error = input_register_polled_device(ipoll_dev);
-	if (error) {
-		input_free_polled_device(ipoll_dev);
+	if (error)
 		return error;
-	}
 
 	bma150->input_polled = ipoll_dev;
 	bma150->input = ipoll_dev->input;
@@ -543,7 +539,8 @@ static int bma150_probe(struct i2c_client *client,
 		return -EINVAL;
 	}
 
-	bma150 = kzalloc(sizeof(struct bma150_data), GFP_KERNEL);
+	bma150 = devm_kzalloc(&client->dev, sizeof(struct bma150_data),
+			      GFP_KERNEL);
 	if (!bma150)
 		return -ENOMEM;
 
@@ -556,7 +553,7 @@ static int bma150_probe(struct i2c_client *client,
 				dev_err(&client->dev,
 					"IRQ GPIO conf. error %d, error %d\n",
 					client->irq, error);
-				goto err_free_mem;
+				return error;
 			}
 		}
 		cfg = &pdata->cfg;
@@ -566,14 +563,14 @@ static int bma150_probe(struct i2c_client *client,
 
 	error = bma150_initialize(bma150, cfg);
 	if (error)
-		goto err_free_mem;
+		return error;
 
 	if (client->irq > 0) {
 		error = bma150_register_input_device(bma150);
 		if (error)
-			goto err_free_mem;
+			return error;
 
-		error = request_threaded_irq(client->irq,
+		error = devm_request_threaded_irq(&client->dev, client->irq,
 					NULL, bma150_irq_thread,
 					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
 					BMA150_DRIVER, bma150);
@@ -582,12 +579,12 @@ static int bma150_probe(struct i2c_client *client,
 				"irq request failed %d, error %d\n",
 				client->irq, error);
 			input_unregister_device(bma150->input);
-			goto err_free_mem;
+			return error;
 		}
 	} else {
 		error = bma150_register_polled_device(bma150);
 		if (error)
-			goto err_free_mem;
+			return error;
 	}
 
 	i2c_set_clientdata(client, bma150);
@@ -595,10 +592,6 @@ static int bma150_probe(struct i2c_client *client,
 	pm_runtime_enable(&client->dev);
 
 	return 0;
-
-err_free_mem:
-	kfree(bma150);
-	return error;
 }
 
 static int bma150_remove(struct i2c_client *client)
@@ -607,15 +600,10 @@ static int bma150_remove(struct i2c_client *client)
 
 	pm_runtime_disable(&client->dev);
 
-	if (client->irq > 0) {
-		free_irq(client->irq, bma150);
+	if (client->irq > 0)
 		input_unregister_device(bma150->input);
-	} else {
+	else
 		input_unregister_polled_device(bma150->input_polled);
-		input_free_polled_device(bma150->input_polled);
-	}
-
-	kfree(bma150);
 
 	return 0;
 }
-- 
2.17.1

^ permalink raw reply related

* [PATCH 2/3] input: misc: bma150: Add support for device tree
From: Paweł Chmiel @ 2019-01-25 18:43 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, mark.rutland, pawel.mikolaj.chmiel, xc-racer2,
	devicetree, linux-input, linux-kernel
In-Reply-To: <20190125184400.17669-1-pawel.mikolaj.chmiel@gmail.com>

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

Add basic of_match table to enable bma150 to be probed via DT

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

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index d101bb0a33d6..c26a118d89fa 100644
--- a/drivers/input/misc/bma150.c
+++ b/drivers/input/misc/bma150.c
@@ -31,6 +31,7 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/bma150.h>
@@ -628,6 +629,14 @@ static int bma150_resume(struct device *dev)
 
 static UNIVERSAL_DEV_PM_OPS(bma150_pm, bma150_suspend, bma150_resume, NULL);
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id bma150_of_match[] = {
+	{ .compatible = "bosch,bma150" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bma150_of_match);
+#endif
+
 static const struct i2c_device_id bma150_id[] = {
 	{ "bma150", 0 },
 	{ "smb380", 0 },
@@ -640,6 +649,7 @@ MODULE_DEVICE_TABLE(i2c, bma150_id);
 static struct i2c_driver bma150_driver = {
 	.driver = {
 		.name	= BMA150_DRIVER,
+		.of_match_table = of_match_ptr(bma150_of_match),
 		.pm	= &bma150_pm,
 	},
 	.class		= I2C_CLASS_HWMON,
-- 
2.17.1

^ permalink raw reply related

* [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
From: Paweł Chmiel @ 2019-01-25 18:44 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, mark.rutland, pawel.mikolaj.chmiel, xc-racer2,
	devicetree, linux-input, linux-kernel
In-Reply-To: <20190125184400.17669-1-pawel.mikolaj.chmiel@gmail.com>

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

Add device tree bindings for Bosch BMA150 Accelerometer Sensor

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

diff --git a/Documentation/devicetree/bindings/input/bosch,bma150.txt b/Documentation/devicetree/bindings/input/bosch,bma150.txt
new file mode 100644
index 000000000000..290c60e38c70
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/bosch,bma150.txt
@@ -0,0 +1,20 @@
+* Bosch BMA150 Accelerometer Sensor
+
+Also works for the SMB380 and BMA023 accelerometers
+
+Required properties:
+- compatible : Should be "bosch,bma150"
+- reg : The I2C address of the sensor
+
+Optional properties:
+- interrupt-parent : should be the phandle for the interrupt controller
+- interrupts : Interrupt mapping for IRQ.  If not present device will be polled
+
+Example:
+
+bma150@38 {
+	compatible = "bosch,bma150";
+	reg = <0x38>;
+	interrupt-parent = <&gph0>;
+	interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
+};
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH][RESEND] wistron_btns needs executable BIOS image
From: Eric Wong @ 2019-01-25 21:13 UTC (permalink / raw)
  To: Jakub Bogusz; +Cc: linux-kernel, mitr, Dmitry Torokhov, linux-input
In-Reply-To: <20190125191437.GA25902@stranger.qboosh.pl>

Jakub Bogusz <qboosh@pld-linux.org> wrote:
> Let's try once again... (take 3)
> 
> First time I sent this patch when I prepared it in 2013 for Linux 3.12,
> the second time after update for Linux 4.12 in 2017...
> And it still applies to Linux 4.20.

Adding Dmitry Torokhov and linux-input to Cc:
(that's what ./scripts/get_maintainer.pl says)

> Actual description in forwarded message.
> 
> 
> -- 
> Jakub Bogusz    http://qboosh.pl/

> From: Jakub Bogusz <qboosh@pld-linux.org>
> To: linux-kernel@vger.kernel.org, mitr@volny.cz
> Subject: [PATCH] wistron_btns needs executable BIOS image
> 
> Hello,
> 
> This patch (originally agains 3.1x, now I updated include to build
> against 4.12.x) fixes winstron_btns module issue with calling BIOS
> functions in non-executable memory.
> 
> Tested (on Linux 3.10.x and few later versions) on F-S Amilo 8210 laptop.
> 
> 
> -- 
> Jakub Bogusz    http://qboosh.pl/

> wistron_btns needs executable BIOS image.
> 
> Signed-off-by: Jakub Bogusz <qboosh@pld-linux.org>
> 
> --- linux-4.12/drivers/input/misc/wistron_btns.c.orig	2013-11-16 09:05:55.612742472 +0100
> +++ linux-4.12/drivers/input/misc/wistron_btns.c	2013-11-16 09:24:37.356028732 +0100
> @@ -33,6 +33,7 @@
>  #include <linux/types.h>
>  #include <linux/platform_device.h>
>  #include <linux/leds.h>
> +#include <asm/set_memory.h>
>  
>  /* How often we poll keys - msecs */
>  #define POLL_INTERVAL_DEFAULT	500 /* when idle */
> @@ -124,6 +125,7 @@
>  	if (entry_point >= 0xF0000) {
>  		bios_code_map_base = base;
>  		bios_entry_point = bios_code_map_base + (entry_point & 0xFFFF);
> +		set_memory_x((unsigned long)bios_code_map_base, 0x10000 >> PAGE_SHIFT);
>  	} else {
>  		iounmap(base);
>  		bios_code_map_base = ioremap(entry_point & ~0x3FFF, 0x4000);
> @@ -134,6 +136,7 @@
>  			goto err;
>  		}
>  		bios_entry_point = bios_code_map_base + (entry_point & 0x3FFF);
> +		set_memory_x((unsigned long)bios_code_map_base, 0x4000 >> PAGE_SHIFT);
>  	}
>  	/* The Windows driver maps 0x10000 bytes, we keep only one page... */
>  	bios_data_map_base = ioremap(0x400, 0xc00);

^ permalink raw reply

* Re: [PATCH][RESEND] wistron_btns needs executable BIOS image
From: Dmitry Torokhov @ 2019-01-25 21:41 UTC (permalink / raw)
  To: Eric Wong; +Cc: Jakub Bogusz, lkml, Miloslav Trmac, linux-input@vger.kernel.org
In-Reply-To: <20190125211349.nfbbcuaip5y6l5dr@dcvr>

On Fri, Jan 25, 2019 at 1:13 PM Eric Wong <e@80x24.org> wrote:
>
> Jakub Bogusz <qboosh@pld-linux.org> wrote:
> > Let's try once again... (take 3)
> >
> > First time I sent this patch when I prepared it in 2013 for Linux 3.12,
> > the second time after update for Linux 4.12 in 2017...
> > And it still applies to Linux 4.20.
>
> Adding Dmitry Torokhov and linux-input to Cc:
> (that's what ./scripts/get_maintainer.pl says)
>
> > Actual description in forwarded message.
> >
> >
> > --
> > Jakub Bogusz    http://qboosh.pl/
>
> > From: Jakub Bogusz <qboosh@pld-linux.org>
> > To: linux-kernel@vger.kernel.org, mitr@volny.cz
> > Subject: [PATCH] wistron_btns needs executable BIOS image
> >
> > Hello,
> >
> > This patch (originally agains 3.1x, now I updated include to build
> > against 4.12.x) fixes winstron_btns module issue with calling BIOS
> > functions in non-executable memory.
> >
> > Tested (on Linux 3.10.x and few later versions) on F-S Amilo 8210 laptop.
> >
> >
> > --
> > Jakub Bogusz    http://qboosh.pl/
>
> > wistron_btns needs executable BIOS image.
> >
> > Signed-off-by: Jakub Bogusz <qboosh@pld-linux.org>
> >
> > --- linux-4.12/drivers/input/misc/wistron_btns.c.orig 2013-11-16 09:05:55.612742472 +0100
> > +++ linux-4.12/drivers/input/misc/wistron_btns.c      2013-11-16 09:24:37.356028732 +0100
> > @@ -33,6 +33,7 @@
> >  #include <linux/types.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/leds.h>
> > +#include <asm/set_memory.h>
> >
> >  /* How often we poll keys - msecs */
> >  #define POLL_INTERVAL_DEFAULT        500 /* when idle */
> > @@ -124,6 +125,7 @@
> >       if (entry_point >= 0xF0000) {
> >               bios_code_map_base = base;
> >               bios_entry_point = bios_code_map_base + (entry_point & 0xFFFF);
> > +             set_memory_x((unsigned long)bios_code_map_base, 0x10000 >> PAGE_SHIFT);
> >       } else {
> >               iounmap(base);
> >               bios_code_map_base = ioremap(entry_point & ~0x3FFF, 0x4000);
> > @@ -134,6 +136,7 @@
> >                       goto err;
> >               }
> >               bios_entry_point = bios_code_map_base + (entry_point & 0x3FFF);
> > +             set_memory_x((unsigned long)bios_code_map_base, 0x4000 >> PAGE_SHIFT);
> >       }
> >       /* The Windows driver maps 0x10000 bytes, we keep only one page... */
> >       bios_data_map_base = ioremap(0x400, 0xc00);

It is really weird have __iomem region to have executable bit set. I
wonder if, along with the setting memory to X, we should not change
that ioremap() to memremap()?

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 2/2] Input: st1232 - add support for st1633
From: Dmitry Torokhov @ 2019-01-26  0:55 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: devicetree, linux-input, robh+dt, mark.rutland, chinyeow.sim.xt,
	linux-kernel, Martin Kepplinger
In-Reply-To: <20190124102125.25458-2-martink@posteo.de>

Hi Martin,

On Thu, Jan 24, 2019 at 11:21:25AM +0100, Martin Kepplinger wrote:
> From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> 
> Add support for the Sitronix ST1633 touchscreen controller to the st1232
> driver. A protocol spec can be found here:
> www.ampdisplay.com/documents/pdf/AM-320480B6TZQW-TC0H.pdf
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> ---
> 
> while this works, I'm not totally convinced by how the i2c path of probe
> looks. what do you say?

Not quite what I had in mind. See below...

>  static int st1232_ts_probe(struct i2c_client *client,
>  			   const struct i2c_device_id *id)
>  {
>  	struct st1232_ts_data *ts;
> +	struct st1232_ts_finger *finger;
>  	struct input_dev *input_dev;
>  	int error;
> +	const struct st_chip_info *match = NULL;
> +
> +	if (dev_fwnode(&client->dev))
> +		match = device_get_match_data(&client->dev);
> +	else if (id && id->driver_data == st1232)
> +		match = &st1232_chip_info;
> +	else if (id && id->driver_data == st1633)
> +		match = &st1633_chip_info;
> +	if (!match) {
> +		dev_err(&client->dev, "unknown device model\n");
> +		return -ENODEV;
> +	}

If you do:

static const struct i2c_device_id st1232_ts_id[] = {
	{ ST1232_TS_NAME, (unsigned long)&st1232_chip_info },
	{ ST1633_TS_NAME, (unsigned long)&st1633_chip_info },
	{ }
};


Then you can do:

	match = device_get_match_data(&client->dev);
	if (!match && id)
		match = (const void *)id->driver_data;
	if (!match) {
		dev_err(&client->dev, "unknown device model\n");
		return -ENODEV;
	}

Thanks.

-- 
Dmitry

^ permalink raw reply

* [git pull] Input updates for v5.0-rc3
From: Dmitry Torokhov @ 2019-01-26  1:11 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, linux-input

Hi Linus,

Please pull from:

	git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus

to receive updates for the input subsystem. You will get a fixup for
the input_event fix for y2038 Sparc64, and couple other minor fixes.

Changelog:
---------

Anders Roxell (1):
      Input: raspberrypi-ts - fix link error

Deepa Dinamani (2):
      Input: input_event - provide override for sparc64
      Input: input_event - fix the CONFIG_SPARC64 mixup

Dmitry Torokhov (1):
      Input: uinput - fix undefined behavior in uinput_validate_absinfo()

Lubomir Rintel (1):
      Input: olpc_apsp - assign priv->dev earlier

Tom Panfil (1):
      Input: xpad - add support for SteelSeries Stratus Duo

Diffstat:
--------

 drivers/input/joystick/xpad.c     | 3 +++
 drivers/input/misc/uinput.c       | 5 +++--
 drivers/input/serio/olpc_apsp.c   | 3 ++-
 drivers/input/touchscreen/Kconfig | 2 +-
 include/uapi/linux/input.h        | 6 +++++-
 5 files changed, 14 insertions(+), 5 deletions(-)

Thanks.


-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 1/4] input: misc: gp2a: Use managed resource helpers
From: Dmitry Torokhov @ 2019-01-26  1:17 UTC (permalink / raw)
  To: Paweł Chmiel
  Cc: robh+dt, mark.rutland, mchehab+samsung, colyli, ckeepax,
	andrew.smirnov, arnd, xiaotong.lu, xc-racer2, linux-input,
	linux-kernel, devicetree
In-Reply-To: <20190125175045.22576-2-pawel.mikolaj.chmiel@gmail.com>

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

^ permalink raw reply

* Re: [PATCH 2/4] input: misc: gp2a: Add support for light sensor
From: Dmitry Torokhov @ 2019-01-26  1:18 UTC (permalink / raw)
  To: Paweł Chmiel
  Cc: robh+dt, mark.rutland, mchehab+samsung, colyli, ckeepax,
	andrew.smirnov, arnd, xiaotong.lu, xc-racer2, linux-input,
	linux-kernel, devicetree
In-Reply-To: <20190125175045.22576-3-pawel.mikolaj.chmiel@gmail.com>

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

^ permalink raw reply

* Re: [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
From: Dmitry Torokhov @ 2019-01-26  1:28 UTC (permalink / raw)
  To: Paweł Chmiel
  Cc: robh+dt, mark.rutland, xc-racer2, devicetree, linux-input,
	linux-kernel
In-Reply-To: <20190125184400.17669-4-pawel.mikolaj.chmiel@gmail.com>

On Fri, Jan 25, 2019 at 07:44:00PM +0100, Paweł Chmiel wrote:
> From: Jonathan Bakker <xc-racer2@live.ca>
> 
> Add device tree bindings for Bosch BMA150 Accelerometer Sensor
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
>  .../bindings/input/bosch,bma150.txt           | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/bosch,bma150.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/bosch,bma150.txt b/Documentation/devicetree/bindings/input/bosch,bma150.txt
> new file mode 100644
> index 000000000000..290c60e38c70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/bosch,bma150.txt
> @@ -0,0 +1,20 @@
> +* Bosch BMA150 Accelerometer Sensor
> +
> +Also works for the SMB380 and BMA023 accelerometers
> +
> +Required properties:
> +- compatible : Should be "bosch,bma150"
> +- reg : The I2C address of the sensor
> +
> +Optional properties:
> +- interrupt-parent : should be the phandle for the interrupt controller
> +- interrupts : Interrupt mapping for IRQ.  If not present device will be polled
> +
> +Example:
> +
> +bma150@38 {
> +	compatible = "bosch,bma150";
> +	reg = <0x38>;
> +	interrupt-parent = <&gph0>;
> +	interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;

Hmm, here you say that IRQ_TYPE_LEVEL_HIGH, so it is level interrupts,
but the driver overrides to rising edge unconditionally. Since you are
the first to add DT support please make separate patch to driver to drop
the ORQ trigger from request_theraded_irq() leaving only IRQF_ONESHOT.

Also please create patch removing platform data support as noone is
using it upstream.

What about the rest of config parameters from bma150_cfg? They should be
handled as device properties too.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 1/3] input: misc: bma150: Use managed resources helpers
From: Dmitry Torokhov @ 2019-01-26  1:29 UTC (permalink / raw)
  To: Paweł Chmiel
  Cc: robh+dt, mark.rutland, xc-racer2, devicetree, linux-input,
	linux-kernel
In-Reply-To: <20190125184400.17669-2-pawel.mikolaj.chmiel@gmail.com>

On Fri, Jan 25, 2019 at 07:43:58PM +0100, Paweł Chmiel wrote:
> From: Jonathan Bakker <xc-racer2@live.ca>
> 
> The driver can be cleaned up 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/bma150.c | 40 +++++++++++++------------------------
>  1 file changed, 14 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
> index 1efcfdf9f8a8..d101bb0a33d6 100644
> --- a/drivers/input/misc/bma150.c
> +++ b/drivers/input/misc/bma150.c
> @@ -471,7 +471,7 @@ static int bma150_register_input_device(struct bma150_data *bma150)
>  	struct input_dev *idev;
>  	int error;
>  
> -	idev = input_allocate_device();
> +	idev = devm_input_allocate_device(&bma150->client->dev);
>  	if (!idev)
>  		return -ENOMEM;
>  
> @@ -482,10 +482,8 @@ static int bma150_register_input_device(struct bma150_data *bma150)
>  	input_set_drvdata(idev, bma150);
>  
>  	error = input_register_device(idev);
> -	if (error) {
> -		input_free_device(idev);
> +	if (error)
>  		return error;
> -	}
>  
>  	bma150->input = idev;
>  	return 0;
> @@ -496,7 +494,7 @@ static int bma150_register_polled_device(struct bma150_data *bma150)
>  	struct input_polled_dev *ipoll_dev;
>  	int error;
>  
> -	ipoll_dev = input_allocate_polled_device();
> +	ipoll_dev = devm_input_allocate_polled_device(&bma150->client->dev);
>  	if (!ipoll_dev)
>  		return -ENOMEM;
>  
> @@ -511,10 +509,8 @@ static int bma150_register_polled_device(struct bma150_data *bma150)
>  	bma150_init_input_device(bma150, ipoll_dev->input);
>  
>  	error = input_register_polled_device(ipoll_dev);
> -	if (error) {
> -		input_free_polled_device(ipoll_dev);
> +	if (error)
>  		return error;
> -	}
>  
>  	bma150->input_polled = ipoll_dev;
>  	bma150->input = ipoll_dev->input;
> @@ -543,7 +539,8 @@ static int bma150_probe(struct i2c_client *client,
>  		return -EINVAL;
>  	}
>  
> -	bma150 = kzalloc(sizeof(struct bma150_data), GFP_KERNEL);
> +	bma150 = devm_kzalloc(&client->dev, sizeof(struct bma150_data),
> +			      GFP_KERNEL);
>  	if (!bma150)
>  		return -ENOMEM;
>  
> @@ -556,7 +553,7 @@ static int bma150_probe(struct i2c_client *client,
>  				dev_err(&client->dev,
>  					"IRQ GPIO conf. error %d, error %d\n",
>  					client->irq, error);
> -				goto err_free_mem;
> +				return error;
>  			}
>  		}
>  		cfg = &pdata->cfg;
> @@ -566,14 +563,14 @@ static int bma150_probe(struct i2c_client *client,
>  
>  	error = bma150_initialize(bma150, cfg);
>  	if (error)
> -		goto err_free_mem;
> +		return error;
>  
>  	if (client->irq > 0) {
>  		error = bma150_register_input_device(bma150);
>  		if (error)
> -			goto err_free_mem;
> +			return error;
>  
> -		error = request_threaded_irq(client->irq,
> +		error = devm_request_threaded_irq(&client->dev, client->irq,
>  					NULL, bma150_irq_thread,
>  					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>  					BMA150_DRIVER, bma150);
> @@ -582,12 +579,12 @@ static int bma150_probe(struct i2c_client *client,
>  				"irq request failed %d, error %d\n",
>  				client->irq, error);
>  			input_unregister_device(bma150->input);

No need to unregister manually if you are using devm.

> -			goto err_free_mem;
> +			return error;
>  		}
>  	} else {
>  		error = bma150_register_polled_device(bma150);
>  		if (error)
> -			goto err_free_mem;
> +			return error;
>  	}
>  
>  	i2c_set_clientdata(client, bma150);
> @@ -595,10 +592,6 @@ static int bma150_probe(struct i2c_client *client,
>  	pm_runtime_enable(&client->dev);
>  
>  	return 0;
> -
> -err_free_mem:
> -	kfree(bma150);
> -	return error;
>  }
>  
>  static int bma150_remove(struct i2c_client *client)
> @@ -607,15 +600,10 @@ static int bma150_remove(struct i2c_client *client)
>  
>  	pm_runtime_disable(&client->dev);
>  
> -	if (client->irq > 0) {
> -		free_irq(client->irq, bma150);
> +	if (client->irq > 0)
>  		input_unregister_device(bma150->input);

Here as well.

> -	} else {
> +	else
>  		input_unregister_polled_device(bma150->input_polled);

And here.

> -		input_free_polled_device(bma150->input_polled);
> -	}
> -
> -	kfree(bma150);
>  
>  	return 0;
>  }
> -- 
> 2.17.1
> 

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 4/4] dt-bindings: input: Add documentation for gp2a sensor
From: Dmitry Torokhov @ 2019-01-26  1:32 UTC (permalink / raw)
  To: Paweł Chmiel
  Cc: robh+dt, mark.rutland, mchehab+samsung, colyli, ckeepax,
	andrew.smirnov, arnd, xiaotong.lu, xc-racer2, linux-input,
	linux-kernel, devicetree
In-Reply-To: <20190125175045.22576-5-pawel.mikolaj.chmiel@gmail.com>

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

^ permalink raw reply

* Re: [PATCH 4/4] dt-bindings: input: Add documentation for gp2a sensor
From: Jonathan Bakker @ 2019-01-26  3:14 UTC (permalink / raw)
  To: Dmitry Torokhov, Paweł Chmiel
  Cc: robh+dt@kernel.org, mark.rutland@arm.com,
	mchehab+samsung@kernel.org, colyli@suse.de,
	ckeepax@opensource.wolfsonmicro.com, andrew.smirnov@gmail.com,
	arnd@arndb.de, xiaotong.lu@spreadtrum.com,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <20190126013237.GF212026@dtor-ws>

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

^ permalink raw reply

* Re: [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
From: Jonathan Bakker @ 2019-01-26  3:39 UTC (permalink / raw)
  To: Dmitry Torokhov, Paweł Chmiel
  Cc: robh+dt@kernel.org, mark.rutland@arm.com,
	devicetree@vger.kernel.org, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190126012802.GD212026@dtor-ws>



On 2019-01-25 5:28 p.m., Dmitry Torokhov wrote:
> On Fri, Jan 25, 2019 at 07:44:00PM +0100, Paweł Chmiel wrote:
>> From: Jonathan Bakker <xc-racer2@live.ca>
>>
>> Add device tree bindings for Bosch BMA150 Accelerometer Sensor
>>
>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
>> ---
>>  .../bindings/input/bosch,bma150.txt           | 20 +++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/input/bosch,bma150.txt
>>
>> diff --git a/Documentation/devicetree/bindings/input/bosch,bma150.txt b/Documentation/devicetree/bindings/input/bosch,bma150.txt
>> new file mode 100644
>> index 000000000000..290c60e38c70
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/bosch,bma150.txt
>> @@ -0,0 +1,20 @@
>> +* Bosch BMA150 Accelerometer Sensor
>> +
>> +Also works for the SMB380 and BMA023 accelerometers
>> +
>> +Required properties:
>> +- compatible : Should be "bosch,bma150"
>> +- reg : The I2C address of the sensor
>> +
>> +Optional properties:
>> +- interrupt-parent : should be the phandle for the interrupt controller
>> +- interrupts : Interrupt mapping for IRQ.  If not present device will be polled
>> +
>> +Example:
>> +
>> +bma150@38 {
>> +	compatible = "bosch,bma150";
>> +	reg = <0x38>;
>> +	interrupt-parent = <&gph0>;
>> +	interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
> 
> Hmm, here you say that IRQ_TYPE_LEVEL_HIGH, so it is level interrupts,
> but the driver overrides to rising edge unconditionally. Since you are
> the first to add DT support please make separate patch to driver to drop
> the ORQ trigger from request_theraded_irq() leaving only IRQF_ONESHOT.
> 
This was simply an oversight on my part, my device was using the the polled method as opposed to an interrupt.  I'll correct this in v2.
> Also please create patch removing platform data support as noone is
> using it upstream.
> 
Will do.
> What about the rest of config parameters from bma150_cfg? They should be
> handled as device properties too.
> 
Ok, I can add them as well.  I didn't bother as the comment in the source code says that the default values are the ones recommended by Bosch.
> Thanks.
> 
Thanks,
Jonathan

^ permalink raw reply

* Re: [PATCH 1/3] input: misc: bma150: Use managed resources helpers
From: Jonathan Bakker @ 2019-01-26  3:40 UTC (permalink / raw)
  To: Dmitry Torokhov, Paweł Chmiel
  Cc: robh+dt@kernel.org, mark.rutland@arm.com,
	devicetree@vger.kernel.org, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190126012909.GE212026@dtor-ws>



On 2019-01-25 5:29 p.m., Dmitry Torokhov wrote:
> On Fri, Jan 25, 2019 at 07:43:58PM +0100, Paweł Chmiel wrote:
>> From: Jonathan Bakker <xc-racer2@live.ca>
>>
>> The driver can be cleaned up 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/bma150.c | 40 +++++++++++++------------------------
>>  1 file changed, 14 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
>> index 1efcfdf9f8a8..d101bb0a33d6 100644
>> --- a/drivers/input/misc/bma150.c
>> +++ b/drivers/input/misc/bma150.c
>> @@ -471,7 +471,7 @@ static int bma150_register_input_device(struct bma150_data *bma150)
>>  	struct input_dev *idev;
>>  	int error;
>>  
>> -	idev = input_allocate_device();
>> +	idev = devm_input_allocate_device(&bma150->client->dev);
>>  	if (!idev)
>>  		return -ENOMEM;
>>  
>> @@ -482,10 +482,8 @@ static int bma150_register_input_device(struct bma150_data *bma150)
>>  	input_set_drvdata(idev, bma150);
>>  
>>  	error = input_register_device(idev);
>> -	if (error) {
>> -		input_free_device(idev);
>> +	if (error)
>>  		return error;
>> -	}
>>  
>>  	bma150->input = idev;
>>  	return 0;
>> @@ -496,7 +494,7 @@ static int bma150_register_polled_device(struct bma150_data *bma150)
>>  	struct input_polled_dev *ipoll_dev;
>>  	int error;
>>  
>> -	ipoll_dev = input_allocate_polled_device();
>> +	ipoll_dev = devm_input_allocate_polled_device(&bma150->client->dev);
>>  	if (!ipoll_dev)
>>  		return -ENOMEM;
>>  
>> @@ -511,10 +509,8 @@ static int bma150_register_polled_device(struct bma150_data *bma150)
>>  	bma150_init_input_device(bma150, ipoll_dev->input);
>>  
>>  	error = input_register_polled_device(ipoll_dev);
>> -	if (error) {
>> -		input_free_polled_device(ipoll_dev);
>> +	if (error)
>>  		return error;
>> -	}
>>  
>>  	bma150->input_polled = ipoll_dev;
>>  	bma150->input = ipoll_dev->input;
>> @@ -543,7 +539,8 @@ static int bma150_probe(struct i2c_client *client,
>>  		return -EINVAL;
>>  	}
>>  
>> -	bma150 = kzalloc(sizeof(struct bma150_data), GFP_KERNEL);
>> +	bma150 = devm_kzalloc(&client->dev, sizeof(struct bma150_data),
>> +			      GFP_KERNEL);
>>  	if (!bma150)
>>  		return -ENOMEM;
>>  
>> @@ -556,7 +553,7 @@ static int bma150_probe(struct i2c_client *client,
>>  				dev_err(&client->dev,
>>  					"IRQ GPIO conf. error %d, error %d\n",
>>  					client->irq, error);
>> -				goto err_free_mem;
>> +				return error;
>>  			}
>>  		}
>>  		cfg = &pdata->cfg;
>> @@ -566,14 +563,14 @@ static int bma150_probe(struct i2c_client *client,
>>  
>>  	error = bma150_initialize(bma150, cfg);
>>  	if (error)
>> -		goto err_free_mem;
>> +		return error;
>>  
>>  	if (client->irq > 0) {
>>  		error = bma150_register_input_device(bma150);
>>  		if (error)
>> -			goto err_free_mem;
>> +			return error;
>>  
>> -		error = request_threaded_irq(client->irq,
>> +		error = devm_request_threaded_irq(&client->dev, client->irq,
>>  					NULL, bma150_irq_thread,
>>  					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>>  					BMA150_DRIVER, bma150);
>> @@ -582,12 +579,12 @@ static int bma150_probe(struct i2c_client *client,
>>  				"irq request failed %d, error %d\n",
>>  				client->irq, error);
>>  			input_unregister_device(bma150->input);
> 
> No need to unregister manually if you are using devm.
Ok, got it, will fix.  I seemed to do this a lot :)
> 
>> -			goto err_free_mem;
>> +			return error;
>>  		}
>>  	} else {
>>  		error = bma150_register_polled_device(bma150);
>>  		if (error)
>> -			goto err_free_mem;
>> +			return error;
>>  	}
>>  
>>  	i2c_set_clientdata(client, bma150);
>> @@ -595,10 +592,6 @@ static int bma150_probe(struct i2c_client *client,
>>  	pm_runtime_enable(&client->dev);
>>  
>>  	return 0;
>> -
>> -err_free_mem:
>> -	kfree(bma150);
>> -	return error;
>>  }
>>  
>>  static int bma150_remove(struct i2c_client *client)
>> @@ -607,15 +600,10 @@ static int bma150_remove(struct i2c_client *client)
>>  
>>  	pm_runtime_disable(&client->dev);
>>  
>> -	if (client->irq > 0) {
>> -		free_irq(client->irq, bma150);
>> +	if (client->irq > 0)
>>  		input_unregister_device(bma150->input);
> 
> Here as well.
> 
>> -	} else {
>> +	else
>>  		input_unregister_polled_device(bma150->input_polled);
> 
> And here.
> 
>> -		input_free_polled_device(bma150->input_polled);
>> -	}
>> -
>> -	kfree(bma150);
>>  
>>  	return 0;
>>  }
>> -- 
>> 2.17.1
>>
> 
> Thanks.
> 
Thanks,
Jonathan

^ permalink raw reply

* Re: [PATCH][RESEND] wistron_btns needs executable BIOS image
From: Jakub Bogusz @ 2019-01-26  8:23 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Eric Wong, lkml, Miloslav Trmac, linux-input@vger.kernel.org
In-Reply-To: <CAKdAkRSZ6cT=fO42YG320Po6cYtWaPgg2puAf5jMnC+hD+Em2g@mail.gmail.com>

On Fri, Jan 25, 2019 at 01:41:47PM -0800, Dmitry Torokhov wrote:
> On Fri, Jan 25, 2019 at 1:13 PM Eric Wong <e@80x24.org> wrote:
> >
> > Jakub Bogusz <qboosh@pld-linux.org> wrote:
> > > Let's try once again... (take 3)
> > >
> > > First time I sent this patch when I prepared it in 2013 for Linux 3.12,
> > > the second time after update for Linux 4.12 in 2017...
> > > And it still applies to Linux 4.20.
> >
> > Adding Dmitry Torokhov and linux-input to Cc:
> > (that's what ./scripts/get_maintainer.pl says)

Thanks. I was aware only of MAINTAINERS file.

[...]
> > > wistron_btns needs executable BIOS image.
> > >
> > > Signed-off-by: Jakub Bogusz <qboosh@pld-linux.org>
> > >
> > > --- linux-4.12/drivers/input/misc/wistron_btns.c.orig 2013-11-16 09:05:55.612742472 +0100
> > > +++ linux-4.12/drivers/input/misc/wistron_btns.c      2013-11-16 09:24:37.356028732 +0100
> > > @@ -33,6 +33,7 @@
> > >  #include <linux/types.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/leds.h>
> > > +#include <asm/set_memory.h>
> > >
> > >  /* How often we poll keys - msecs */
> > >  #define POLL_INTERVAL_DEFAULT        500 /* when idle */
> > > @@ -124,6 +125,7 @@
> > >       if (entry_point >= 0xF0000) {
> > >               bios_code_map_base = base;
> > >               bios_entry_point = bios_code_map_base + (entry_point & 0xFFFF);
> > > +             set_memory_x((unsigned long)bios_code_map_base, 0x10000 >> PAGE_SHIFT);
> > >       } else {
> > >               iounmap(base);
> > >               bios_code_map_base = ioremap(entry_point & ~0x3FFF, 0x4000);
> > > @@ -134,6 +136,7 @@
> > >                       goto err;
> > >               }
> > >               bios_entry_point = bios_code_map_base + (entry_point & 0x3FFF);
> > > +             set_memory_x((unsigned long)bios_code_map_base, 0x4000 >> PAGE_SHIFT);
> > >       }
> > >       /* The Windows driver maps 0x10000 bytes, we keep only one page... */
> > >       bios_data_map_base = ioremap(0x400, 0xc00);
> 
> It is really weird have __iomem region to have executable bit set. I
> wonder if, along with the setting memory to X, we should not change
> that ioremap() to memremap()?

Maybe... this is area of BIOS ROM, not I/O. But I'm not sure about
cacheability.

Looking at recent kernel code (4.19 to be exact) ioremap ends in
__ioremap_caller with _PAGE_CACHE_MODE_UC_MINUS, while memremap would end
in __ioremap_caller with e.g. (depending on flags) _PAGE_CACHE_MODE_WT.

Unfortunately now I have rare access to this hardware (FS Amilo) to
test.


-- 
Jakub Bogusz    http://qboosh.pl/

^ permalink raw reply

* [PATCH 0/3] ARM: dts: lpc32xx: fix lpc32xx-key device node
From: Vladimir Zapolskiy @ 2019-01-26 14:29 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring
  Cc: devicetree, linux-input, linux-arm-kernel, Sylvain Lemieux

The changeset adds missing but required 'clocks' property and fixes
'interrupts' property of a keypad controller on NXP LPC32xx powered boards.

The devicetree binding documentation is updated accordingly.

Vladimir Zapolskiy (3):
  Input: lpc32xx-key - add clocks property and fix DT binding example
  ARM: dts: lpc32xx: add required clocks property to keypad device node
  ARM: dts: lpc32xx: reparent keypad controller to SIC1

 Documentation/devicetree/bindings/input/lpc32xx-key.txt | 5 ++++-
 arch/arm/boot/dts/lpc32xx.dtsi                          | 4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.20.1

^ permalink raw reply

* [PATCH 1/3] Input: lpc32xx-key - add clocks property and fix DT binding example
From: Vladimir Zapolskiy @ 2019-01-26 14:29 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring
  Cc: devicetree, linux-input, linux-arm-kernel, Sylvain Lemieux
In-Reply-To: <20190126142921.16041-1-vz@mleia.com>

The keypad controller on NXP LPC32xx requires its clock gate to be open,
therefore add description of the requires 'clocks' property.

In addition adjust the example by adding description of required 'clocks'
property and by fixing 'interrupts' property.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 Documentation/devicetree/bindings/input/lpc32xx-key.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/lpc32xx-key.txt b/Documentation/devicetree/bindings/input/lpc32xx-key.txt
index bcf62f856358..2b075a080d30 100644
--- a/Documentation/devicetree/bindings/input/lpc32xx-key.txt
+++ b/Documentation/devicetree/bindings/input/lpc32xx-key.txt
@@ -8,6 +8,7 @@ Required Properties:
 - reg: Physical base address of the controller and length of memory mapped
   region.
 - interrupts: The interrupt number to the cpu.
+- clocks: phandle to clock controller plus clock-specifier pair
 - nxp,debounce-delay-ms: Debounce delay in ms
 - nxp,scan-delay-ms: Repeated scan period in ms
 - linux,keymap: the key-code to be reported when the key is pressed
@@ -22,7 +23,9 @@ Example:
 	key@40050000 {
 		compatible = "nxp,lpc3220-key";
 		reg = <0x40050000 0x1000>;
-		interrupts = <54 0>;
+		clocks = <&clk LPC32XX_CLK_KEY>;
+		interrupt-parent = <&sic1>;
+		interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;
 		keypad,num-rows = <1>;
 		keypad,num-columns = <1>;
 		nxp,debounce-delay-ms = <3>;
-- 
2.20.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox