[PATCH v2 0/5] input: misc: bma150: Add support for device tree

[PATCH v2 0/5] input: misc: bma150: Add support for device tree

[Paweł Chmiel]

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

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

Changes from v1:
  - Correct devm input unregistering
  - Correct IRQ type in DT documentation
  - Add DT properties for bma150_cfg and document
  - Add patch removing pdata
  - Fix race condition in input device registering

Jonathan Bakker (5):
  dt-bindings: input: Add binding for bma150 sensor
  input: misc: bma150: Use managed resources helpers
  input: misc: bma150: Add support for device tree
  input: misc: bma150: Drop platform data
  input: misc: bma150: Register input device after setting private data

 .../bindings/input/bosch,bma150.txt           |  38 ++++++
 drivers/input/misc/bma150.c                   | 128 ++++++++++--------
 include/dt-bindings/input/bma150.h            |  22 +++
 include/linux/bma150.h                        |  38 ++----
 4 files changed, 144 insertions(+), 82 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/bosch,bma150.txt
 create mode 100644 include/dt-bindings/input/bma150.h

-- 
2.17.1

[PATCH v2 1/5] dt-bindings: input: Add binding for bma150 sensor

[Paweł Chmiel]

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

Add device tree bindings for Bosch BMA150 Accelerometer Sensor

Changes from v1:
 - Add properties for all of bma150_cfg
 - Correct IRQ type in example

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 .../bindings/input/bosch,bma150.txt           | 38 +++++++++++++++++++
 include/dt-bindings/input/bma150.h            | 22 +++++++++++
 include/linux/bma150.h                        | 13 +------
 3 files changed, 62 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/bosch,bma150.txt
 create mode 100644 include/dt-bindings/input/bma150.h

diff --git a/Documentation/devicetree/bindings/input/bosch,bma150.txt b/Documentation/devicetree/bindings/input/bosch,bma150.txt
new file mode 100644
index 000000000000..f644d132f79c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/bosch,bma150.txt
@@ -0,0 +1,38 @@
+* 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
+- any-motion-int : bool for if the any motion interrupt should be enabled
+- hg-int : bool for if the high-G interrupt should be enabled
+- lg-int : bool for if the low-G interrupt should be enabled
+- any-motion-cfg : array of integers for any motion duration and threshold
+- hg-cfg : array of integers for high-G hysterisis, duration, and threshold
+- lg-cfg : array of integers for low-G hysterisis, duration, and threshold
+- range : configuration of range, one of BMA150_RANGE_* as defined in [1]
+- bandwidth : refresh rate of device, one of BMA150_BW_* as defined in [1]
+
+Example:
+
+bma150@38 {
+	compatible = "bosch,bma150";
+	reg = <0x38>;
+	interrupt-parent = <&gph0>;
+	interrupts = <1 IRQ_TYPE_EDGE_RISING>;
+	any-motion-int;
+	hg-int;
+	lg-int;
+	any-motion-cfg = <0 0>;
+	hg-cfg = <0 150 160>;
+	lg-cfg = <0 150 20>;
+	range = <BMA150_RANGE_2G>;
+	bandwidth = <BMA150_BW_50HZ>;
+};
+
+[1] include/dt-bindings/input/bma150.h
diff --git a/include/dt-bindings/input/bma150.h b/include/dt-bindings/input/bma150.h
new file mode 100644
index 000000000000..fb38ca787f0f
--- /dev/null
+++ b/include/dt-bindings/input/bma150.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides bindings for the BMA150 accelerometer
+ */
+#ifndef _DT_BINDINGS_INPUT_BMA150_H
+#define _DT_BINDINGS_INPUT_BMA150_H
+
+/* Range */
+#define BMA150_RANGE_2G		0
+#define BMA150_RANGE_4G		1
+#define BMA150_RANGE_8G		2
+
+/* Refresh rate */
+#define BMA150_BW_25HZ		0
+#define BMA150_BW_50HZ		1
+#define BMA150_BW_100HZ		2
+#define BMA150_BW_190HZ		3
+#define BMA150_BW_375HZ		4
+#define BMA150_BW_750HZ		5
+#define BMA150_BW_1500HZ	6
+
+#endif /* _DT_BINDINGS_INPUT_BMA150_H */
diff --git a/include/linux/bma150.h b/include/linux/bma150.h
index 97ade7cdc870..b85266a9c35c 100644
--- a/include/linux/bma150.h
+++ b/include/linux/bma150.h
@@ -20,19 +20,10 @@
 #ifndef _BMA150_H_
 #define _BMA150_H_
 
-#define BMA150_DRIVER		"bma150"
+#include <dt-bindings/input/bma150.h>
 
-#define BMA150_RANGE_2G		0
-#define BMA150_RANGE_4G		1
-#define BMA150_RANGE_8G		2
+#define BMA150_DRIVER		"bma150"
 
-#define BMA150_BW_25HZ		0
-#define BMA150_BW_50HZ		1
-#define BMA150_BW_100HZ		2
-#define BMA150_BW_190HZ		3
-#define BMA150_BW_375HZ		4
-#define BMA150_BW_750HZ		5
-#define BMA150_BW_1500HZ	6
 
 struct bma150_cfg {
 	bool any_motion_int;		/* Set to enable any-motion interrupt */
-- 
2.17.1

[PATCH v2 2/5] input: misc: bma150: Use managed resources helpers

[Paweł Chmiel]

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

The driver can be cleaned up by using managed resource helpers

Changes from v1:
 - Correct devm input unregistering

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

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index 1efcfdf9f8a8..79acaaf86b7e 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);
@@ -581,13 +578,12 @@ static int bma150_probe(struct i2c_client *client,
 			dev_err(&client->dev,
 				"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,28 +591,12 @@ 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)
 {
-	struct bma150_data *bma150 = i2c_get_clientdata(client);
-
 	pm_runtime_disable(&client->dev);
 
-	if (client->irq > 0) {
-		free_irq(client->irq, bma150);
-		input_unregister_device(bma150->input);
-	} else {
-		input_unregister_polled_device(bma150->input_polled);
-		input_free_polled_device(bma150->input_polled);
-	}
-
-	kfree(bma150);
-
 	return 0;
 }
 
-- 
2.17.1

[PATCH v2 3/5] input: misc: bma150: Add support for device tree

[Paweł Chmiel]

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

Add of_match table to enable bma150 to be probed via DT

Changes from v1:
 - Add properties for all of bma150_cfg

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 drivers/input/misc/bma150.c | 64 ++++++++++++++++++++++++++++++++++++-
 include/linux/bma150.h      | 20 ++++++------
 2 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index 79acaaf86b7e..e86df79490ad 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>
@@ -146,7 +147,7 @@ struct bma150_data {
  * are stated and verified by Bosch Sensortec where they are configured
  * to provide a generic sensitivity performance.
  */
-static const struct bma150_cfg default_cfg = {
+static struct bma150_cfg default_cfg = {
 	.any_motion_int = 1,
 	.hg_int = 1,
 	.lg_int = 1,
@@ -518,6 +519,51 @@ static int bma150_register_polled_device(struct bma150_data *bma150)
 	return 0;
 }
 
+int bma150_cfg_from_of(struct device_node *np)
+{
+	int error;
+
+	default_cfg.any_motion_int =
+			of_property_read_bool(np, "any-motion-int");
+	default_cfg.hg_int =
+			of_property_read_bool(np, "hg-int");
+	default_cfg.lg_int =
+			of_property_read_bool(np, "lg-int");
+
+	error = of_property_read_u32_array(np, "any-motion-cfg",
+			&default_cfg.any_motion_dur, 2);
+	if (error < 0 && error != -EINVAL)
+		return error;
+
+	error = of_property_read_u32_array(np, "hg-cfg",
+			&default_cfg.hg_hyst, 3);
+	if (error < 0 && error != -EINVAL)
+		return error;
+
+	error = of_property_read_u32_array(np, "lg-cfg",
+			&default_cfg.lg_hyst, 3);
+	if (error < 0 && error != -EINVAL)
+		return error;
+
+	error = of_property_read_u32(np, "range",
+			&default_cfg.range);
+	if (error < 0 && error != -EINVAL)
+		return error;
+	else if (default_cfg.range < BMA150_RANGE_2G ||
+			default_cfg.range > BMA150_RANGE_8G)
+		return -EINVAL;
+
+	error = of_property_read_u32(np, "bandwidth",
+			&default_cfg.bandwidth);
+	if (error < 0 && error != -EINVAL)
+		return error;
+	else if (default_cfg.bandwidth < BMA150_BW_25HZ ||
+			default_cfg.bandwidth > BMA150_BW_1500HZ)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int bma150_probe(struct i2c_client *client,
 				  const struct i2c_device_id *id)
 {
@@ -557,6 +603,13 @@ static int bma150_probe(struct i2c_client *client,
 			}
 		}
 		cfg = &pdata->cfg;
+	} else if (client->dev.of_node) {
+		error = bma150_cfg_from_of(client->dev.of_node);
+		if (error) {
+			dev_err(&client->dev, "Failed to parse of data\n");
+			return error;
+		}
+		cfg = &default_cfg;
 	} else {
 		cfg = &default_cfg;
 	}
@@ -620,6 +673,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 },
@@ -632,6 +693,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,
diff --git a/include/linux/bma150.h b/include/linux/bma150.h
index b85266a9c35c..ad19dc7a30d7 100644
--- a/include/linux/bma150.h
+++ b/include/linux/bma150.h
@@ -29,16 +29,16 @@ struct bma150_cfg {
 	bool any_motion_int;		/* Set to enable any-motion interrupt */
 	bool hg_int;			/* Set to enable high-G interrupt */
 	bool lg_int;			/* Set to enable low-G interrupt */
-	unsigned char any_motion_dur;	/* Any-motion duration */
-	unsigned char any_motion_thres;	/* Any-motion threshold */
-	unsigned char hg_hyst;		/* High-G hysterisis */
-	unsigned char hg_dur;		/* High-G duration */
-	unsigned char hg_thres;		/* High-G threshold */
-	unsigned char lg_hyst;		/* Low-G hysterisis */
-	unsigned char lg_dur;		/* Low-G duration */
-	unsigned char lg_thres;		/* Low-G threshold */
-	unsigned char range;		/* one of BMA0150_RANGE_xxx */
-	unsigned char bandwidth;	/* one of BMA0150_BW_xxx */
+	u32 any_motion_dur;		/* Any-motion duration */
+	u32 any_motion_thres;		/* Any-motion threshold */
+	u32 hg_hyst;			/* High-G hysterisis */
+	u32 hg_dur;			/* High-G duration */
+	u32 hg_thres;			/* High-G threshold */
+	u32 lg_hyst;			/* Low-G hysterisis */
+	u32 lg_dur;			/* Low-G duration */
+	u32 lg_thres;			/* Low-G threshold */
+	u32 range;			/* one of BMA0150_RANGE_xxx */
+	u32 bandwidth;			/* one of BMA0150_BW_xxx */
 };
 
 struct bma150_platform_data {
-- 
2.17.1

[PATCH v2 4/5] input: misc: bma150: Drop platform data

[Paweł Chmiel]

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

bma150 supports DT now and as there are no in-kernel users of
the platform data, remove it.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
 drivers/input/misc/bma150.c | 27 +++++----------------------
 include/linux/bma150.h      |  5 -----
 2 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index e86df79490ad..1cdc8ce97968 100644
--- a/drivers/input/misc/bma150.c
+++ b/drivers/input/misc/bma150.c
@@ -567,8 +567,6 @@ int bma150_cfg_from_of(struct device_node *np)
 static int bma150_probe(struct i2c_client *client,
 				  const struct i2c_device_id *id)
 {
-	const struct bma150_platform_data *pdata =
-			dev_get_platdata(&client->dev);
 	const struct bma150_cfg *cfg;
 	struct bma150_data *bma150;
 	int chip_id;
@@ -592,27 +590,12 @@ static int bma150_probe(struct i2c_client *client,
 
 	bma150->client = client;
 
-	if (pdata) {
-		if (pdata->irq_gpio_cfg) {
-			error = pdata->irq_gpio_cfg();
-			if (error) {
-				dev_err(&client->dev,
-					"IRQ GPIO conf. error %d, error %d\n",
-					client->irq, error);
-				return error;
-			}
-		}
-		cfg = &pdata->cfg;
-	} else if (client->dev.of_node) {
-		error = bma150_cfg_from_of(client->dev.of_node);
-		if (error) {
-			dev_err(&client->dev, "Failed to parse of data\n");
-			return error;
-		}
-		cfg = &default_cfg;
-	} else {
-		cfg = &default_cfg;
+	error = bma150_cfg_from_of(client->dev.of_node);
+	if (error) {
+		dev_err(&client->dev, "Failed to parse of data\n");
+		return error;
 	}
+	cfg = &default_cfg;
 
 	error = bma150_initialize(bma150, cfg);
 	if (error)
diff --git a/include/linux/bma150.h b/include/linux/bma150.h
index ad19dc7a30d7..650ffe9fa4cf 100644
--- a/include/linux/bma150.h
+++ b/include/linux/bma150.h
@@ -41,9 +41,4 @@ struct bma150_cfg {
 	u32 bandwidth;			/* one of BMA0150_BW_xxx */
 };
 
-struct bma150_platform_data {
-	struct bma150_cfg cfg;
-	int (*irq_gpio_cfg)(void);
-};
-
 #endif /* _BMA150_H_ */
-- 
2.17.1

[PATCH v2 5/5] input: misc: bma150: Register input device after setting private data

[Paweł Chmiel]

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

Otherwise we introduce a race condition where userspace can request input
before we're ready leading to null pointer dereference such as

input: bma150 as /devices/platform/i2c-gpio-2/i2c-5/5-0038/input/input3
Unable to handle kernel NULL pointer dereference at virtual address 00000018
pgd = (ptrval)
[00000018] *pgd=55dac831, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1] PREEMPT ARM
Modules linked in: bma150 input_polldev [last unloaded: bma150]
CPU: 0 PID: 2870 Comm: accelerometer Not tainted 5.0.0-rc3-dirty #46
Hardware name: Samsung S5PC110/S5PV210-based board
PC is at input_event+0x8/0x60
LR is at bma150_report_xyz+0x9c/0xe0 [bma150]
pc : [<80450f70>]    lr : [<7f0a614c>]    psr: 800d0013
sp : a4c1fd78  ip : 00000081  fp : 00020000
r10: 00000000  r9 : a5e2944c  r8 : a7455000
r7 : 00000016  r6 : 00000101  r5 : a7617940  r4 : 80909048
r3 : fffffff2  r2 : 00000000  r1 : 00000003  r0 : 00000000
Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 54e34019  DAC: 00000051
Process accelerometer (pid: 2870, stack limit = 0x(ptrval))
Stackck: (0xa4c1fd78 to 0xa4c20000)
fd60:                                                       fffffff3 fc813f6c
fd80: 40410581 d7530ce3 a5e2817c a7617f00 a5e29404 a5e2817c 00000000 7f008324
fda0: a5e28000 8044f59c a5fdd9d0 a5e2945c a46a4a00 a5e29668 a7455000 80454f10
fdc0: 80909048 a5e29668 a5fdd9d0 a46a4a00 806316d0 00000000 a46a4a00 801df5f0
fde0: 00000000 d7530ce3 a4c1fec0 a46a4a00 00000000 a5fdd9d0 a46a4a08 801df53c
fe00: 00000000 801d74bc a4c1fec0 00000000 a4c1ff70 00000000 a7038da8 00000000
fe20: a46a4a00 801e91fc a411bbe0 801f2e88 00000004 00000000 80909048 00000041
fe40: 00000000 00020000 00000000 dead4ead a6a88da0 00000000 ffffe000 806fcae8
fe60: a4c1fec8 00000000 80909048 00000002 a5fdd9d0 a7660110 a411bab0 00000001
fe80: dead4ead ffffffff ffffffff a4c1fe8c a4c1fe8c d7530ce3 20000013 80909048
fea0: 80909048 a4c1ff70 00000001 fffff000 a4c1e000 00000005 00026038 801eabd8
fec0: a7660110 a411bab0 b9394901 00000006 a696201b 76fb3000 00000000 a7039720
fee0: a5fdd9d0 00000101 00000002 00000096 00000000 00000000 00000000 a4c1ff00
ff00: a6b310f4 805cb174 a6b310f4 00000010 00000fe0 00000010 a4c1e000 d7530ce3
ff20: 00000003 a5f41400 a5f41424 00000000 a6962000 00000000 00000003 00000002
ff40: ffffff9c 000a0000 80909048 d7530ce3 a6962000 00000003 80909048 ffffff9c
ff60: a6962000 801d890c 00000000 00000000 00020000 a7590000 00000004 00000100
ff80: 00000001 d7530ce3 000288b8 00026320 000288b8 00000005 80101204 a4c1e000
ffa0: 00000005 80101000 000288b8 00026320 000288b8 000a0000 00000000 00000000
ffc0: 000288b8 00026320 000288b8 00000005 7eef3bac 000264e8 00028ad8 00026038
ffe0: 00000005 7eef3300 76f76e91 76f78546 800d0030 000288b8 00000000 00000000
[<80450f70>] (input_event) from [<a5e2817c>] (0xa5e2817c)
Code: e1a08148 eaffffa8 e351001f 812fff1e (e590c018)
---[ end trace 1c691ee85f2ff243 ]---

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

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index 1cdc8ce97968..64caf43e5bca 100644
--- a/drivers/input/misc/bma150.c
+++ b/drivers/input/misc/bma150.c
@@ -470,7 +470,6 @@ static void bma150_init_input_device(struct bma150_data *bma150,
 static int bma150_register_input_device(struct bma150_data *bma150)
 {
 	struct input_dev *idev;
-	int error;
 
 	idev = devm_input_allocate_device(&bma150->client->dev);
 	if (!idev)
@@ -482,18 +481,14 @@ static int bma150_register_input_device(struct bma150_data *bma150)
 	idev->close = bma150_irq_close;
 	input_set_drvdata(idev, bma150);
 
-	error = input_register_device(idev);
-	if (error)
-		return error;
-
 	bma150->input = idev;
-	return 0;
+
+	return input_register_device(idev);
 }
 
 static int bma150_register_polled_device(struct bma150_data *bma150)
 {
 	struct input_polled_dev *ipoll_dev;
-	int error;
 
 	ipoll_dev = devm_input_allocate_polled_device(&bma150->client->dev);
 	if (!ipoll_dev)
@@ -509,14 +504,10 @@ 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)
-		return error;
-
 	bma150->input_polled = ipoll_dev;
 	bma150->input = ipoll_dev->input;
 
-	return 0;
+	return input_register_polled_device(ipoll_dev);
 }
 
 int bma150_cfg_from_of(struct device_node *np)
-- 
2.17.1

Re: [PATCH v2 2/5] input: misc: bma150: Use managed resources helpers

[Dmitry Torokhov]

On Sat, Feb 02, 2019 at 04:18:03PM +0100, Paweł Chmiel wrote:
> From: Jonathan Bakker <xc-racer2@live.ca>
> 
> The driver can be cleaned up by using managed resource helpers
> 
> Changes from v1:
>  - Correct devm input unregistering
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>

Applied, thank you.

> ---
>  drivers/input/misc/bma150.c | 44 ++++++++++---------------------------
>  1 file changed, 12 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
> index 1efcfdf9f8a8..79acaaf86b7e 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);
> @@ -581,13 +578,12 @@ static int bma150_probe(struct i2c_client *client,
>  			dev_err(&client->dev,
>  				"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,28 +591,12 @@ 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)
>  {
> -	struct bma150_data *bma150 = i2c_get_clientdata(client);
> -
>  	pm_runtime_disable(&client->dev);
>  
> -	if (client->irq > 0) {
> -		free_irq(client->irq, bma150);
> -		input_unregister_device(bma150->input);
> -	} else {
> -		input_unregister_polled_device(bma150->input_polled);
> -		input_free_polled_device(bma150->input_polled);
> -	}
> -
> -	kfree(bma150);
> -
>  	return 0;
>  }
>  
> -- 
> 2.17.1
> 

-- 
Dmitry

Re: [PATCH v2 5/5] input: misc: bma150: Register input device after setting private data

[Dmitry Torokhov]

On Sat, Feb 02, 2019 at 04:18:06PM +0100, Paweł Chmiel wrote:
> From: Jonathan Bakker <xc-racer2@live.ca>
> 
> Otherwise we introduce a race condition where userspace can request input
> before we're ready leading to null pointer dereference such as
> 
> input: bma150 as /devices/platform/i2c-gpio-2/i2c-5/5-0038/input/input3
> Unable to handle kernel NULL pointer dereference at virtual address 00000018
> pgd = (ptrval)
> [00000018] *pgd=55dac831, *pte=00000000, *ppte=00000000
> Internal error: Oops: 17 [#1] PREEMPT ARM
> Modules linked in: bma150 input_polldev [last unloaded: bma150]
> CPU: 0 PID: 2870 Comm: accelerometer Not tainted 5.0.0-rc3-dirty #46
> Hardware name: Samsung S5PC110/S5PV210-based board
> PC is at input_event+0x8/0x60
> LR is at bma150_report_xyz+0x9c/0xe0 [bma150]
> pc : [<80450f70>]    lr : [<7f0a614c>]    psr: 800d0013
> sp : a4c1fd78  ip : 00000081  fp : 00020000
> r10: 00000000  r9 : a5e2944c  r8 : a7455000
> r7 : 00000016  r6 : 00000101  r5 : a7617940  r4 : 80909048
> r3 : fffffff2  r2 : 00000000  r1 : 00000003  r0 : 00000000
> Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: 54e34019  DAC: 00000051
> Process accelerometer (pid: 2870, stack limit = 0x(ptrval))
> Stackck: (0xa4c1fd78 to 0xa4c20000)
> fd60:                                                       fffffff3 fc813f6c
> fd80: 40410581 d7530ce3 a5e2817c a7617f00 a5e29404 a5e2817c 00000000 7f008324
> fda0: a5e28000 8044f59c a5fdd9d0 a5e2945c a46a4a00 a5e29668 a7455000 80454f10
> fdc0: 80909048 a5e29668 a5fdd9d0 a46a4a00 806316d0 00000000 a46a4a00 801df5f0
> fde0: 00000000 d7530ce3 a4c1fec0 a46a4a00 00000000 a5fdd9d0 a46a4a08 801df53c
> fe00: 00000000 801d74bc a4c1fec0 00000000 a4c1ff70 00000000 a7038da8 00000000
> fe20: a46a4a00 801e91fc a411bbe0 801f2e88 00000004 00000000 80909048 00000041
> fe40: 00000000 00020000 00000000 dead4ead a6a88da0 00000000 ffffe000 806fcae8
> fe60: a4c1fec8 00000000 80909048 00000002 a5fdd9d0 a7660110 a411bab0 00000001
> fe80: dead4ead ffffffff ffffffff a4c1fe8c a4c1fe8c d7530ce3 20000013 80909048
> fea0: 80909048 a4c1ff70 00000001 fffff000 a4c1e000 00000005 00026038 801eabd8
> fec0: a7660110 a411bab0 b9394901 00000006 a696201b 76fb3000 00000000 a7039720
> fee0: a5fdd9d0 00000101 00000002 00000096 00000000 00000000 00000000 a4c1ff00
> ff00: a6b310f4 805cb174 a6b310f4 00000010 00000fe0 00000010 a4c1e000 d7530ce3
> ff20: 00000003 a5f41400 a5f41424 00000000 a6962000 00000000 00000003 00000002
> ff40: ffffff9c 000a0000 80909048 d7530ce3 a6962000 00000003 80909048 ffffff9c
> ff60: a6962000 801d890c 00000000 00000000 00020000 a7590000 00000004 00000100
> ff80: 00000001 d7530ce3 000288b8 00026320 000288b8 00000005 80101204 a4c1e000
> ffa0: 00000005 80101000 000288b8 00026320 000288b8 000a0000 00000000 00000000
> ffc0: 000288b8 00026320 000288b8 00000005 7eef3bac 000264e8 00028ad8 00026038
> ffe0: 00000005 7eef3300 76f76e91 76f78546 800d0030 000288b8 00000000 00000000
> [<80450f70>] (input_event) from [<a5e2817c>] (0xa5e2817c)
> Code: e1a08148 eaffffa8 e351001f 812fff1e (e590c018)
> ---[ end trace 1c691ee85f2ff243 ]---
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>

Applied, thank you.

> ---
>  drivers/input/misc/bma150.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
> index 1cdc8ce97968..64caf43e5bca 100644
> --- a/drivers/input/misc/bma150.c
> +++ b/drivers/input/misc/bma150.c
> @@ -470,7 +470,6 @@ static void bma150_init_input_device(struct bma150_data *bma150,
>  static int bma150_register_input_device(struct bma150_data *bma150)
>  {
>  	struct input_dev *idev;
> -	int error;
>  
>  	idev = devm_input_allocate_device(&bma150->client->dev);
>  	if (!idev)
> @@ -482,18 +481,14 @@ static int bma150_register_input_device(struct bma150_data *bma150)
>  	idev->close = bma150_irq_close;
>  	input_set_drvdata(idev, bma150);
>  
> -	error = input_register_device(idev);
> -	if (error)
> -		return error;
> -
>  	bma150->input = idev;
> -	return 0;
> +
> +	return input_register_device(idev);
>  }
>  
>  static int bma150_register_polled_device(struct bma150_data *bma150)
>  {
>  	struct input_polled_dev *ipoll_dev;
> -	int error;
>  
>  	ipoll_dev = devm_input_allocate_polled_device(&bma150->client->dev);
>  	if (!ipoll_dev)
> @@ -509,14 +504,10 @@ 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)
> -		return error;
> -
>  	bma150->input_polled = ipoll_dev;
>  	bma150->input = ipoll_dev->input;
>  
> -	return 0;
> +	return input_register_polled_device(ipoll_dev);
>  }
>  
>  int bma150_cfg_from_of(struct device_node *np)
> -- 
> 2.17.1
> 

-- 
Dmitry

Re: [PATCH v2 5/5] input: misc: bma150: Register input device after setting private data

[Dmitry Torokhov]

On Wed, Feb 06, 2019 at 10:53:07AM -0800, Dmitry Torokhov wrote:
> On Sat, Feb 02, 2019 at 04:18:06PM +0100, Paweł Chmiel wrote:
> > From: Jonathan Bakker <xc-racer2@live.ca>
> > 
> > Otherwise we introduce a race condition where userspace can request input
> > before we're ready leading to null pointer dereference such as
> > 
> > input: bma150 as /devices/platform/i2c-gpio-2/i2c-5/5-0038/input/input3
> > Unable to handle kernel NULL pointer dereference at virtual address 00000018
> > pgd = (ptrval)
> > [00000018] *pgd=55dac831, *pte=00000000, *ppte=00000000
> > Internal error: Oops: 17 [#1] PREEMPT ARM
> > Modules linked in: bma150 input_polldev [last unloaded: bma150]
> > CPU: 0 PID: 2870 Comm: accelerometer Not tainted 5.0.0-rc3-dirty #46
> > Hardware name: Samsung S5PC110/S5PV210-based board
> > PC is at input_event+0x8/0x60
> > LR is at bma150_report_xyz+0x9c/0xe0 [bma150]
> > pc : [<80450f70>]    lr : [<7f0a614c>]    psr: 800d0013
> > sp : a4c1fd78  ip : 00000081  fp : 00020000
> > r10: 00000000  r9 : a5e2944c  r8 : a7455000
> > r7 : 00000016  r6 : 00000101  r5 : a7617940  r4 : 80909048
> > r3 : fffffff2  r2 : 00000000  r1 : 00000003  r0 : 00000000
> > Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> > Control: 10c5387d  Table: 54e34019  DAC: 00000051
> > Process accelerometer (pid: 2870, stack limit = 0x(ptrval))
> > Stackck: (0xa4c1fd78 to 0xa4c20000)
> > fd60:                                                       fffffff3 fc813f6c
> > fd80: 40410581 d7530ce3 a5e2817c a7617f00 a5e29404 a5e2817c 00000000 7f008324
> > fda0: a5e28000 8044f59c a5fdd9d0 a5e2945c a46a4a00 a5e29668 a7455000 80454f10
> > fdc0: 80909048 a5e29668 a5fdd9d0 a46a4a00 806316d0 00000000 a46a4a00 801df5f0
> > fde0: 00000000 d7530ce3 a4c1fec0 a46a4a00 00000000 a5fdd9d0 a46a4a08 801df53c
> > fe00: 00000000 801d74bc a4c1fec0 00000000 a4c1ff70 00000000 a7038da8 00000000
> > fe20: a46a4a00 801e91fc a411bbe0 801f2e88 00000004 00000000 80909048 00000041
> > fe40: 00000000 00020000 00000000 dead4ead a6a88da0 00000000 ffffe000 806fcae8
> > fe60: a4c1fec8 00000000 80909048 00000002 a5fdd9d0 a7660110 a411bab0 00000001
> > fe80: dead4ead ffffffff ffffffff a4c1fe8c a4c1fe8c d7530ce3 20000013 80909048
> > fea0: 80909048 a4c1ff70 00000001 fffff000 a4c1e000 00000005 00026038 801eabd8
> > fec0: a7660110 a411bab0 b9394901 00000006 a696201b 76fb3000 00000000 a7039720
> > fee0: a5fdd9d0 00000101 00000002 00000096 00000000 00000000 00000000 a4c1ff00
> > ff00: a6b310f4 805cb174 a6b310f4 00000010 00000fe0 00000010 a4c1e000 d7530ce3
> > ff20: 00000003 a5f41400 a5f41424 00000000 a6962000 00000000 00000003 00000002
> > ff40: ffffff9c 000a0000 80909048 d7530ce3 a6962000 00000003 80909048 ffffff9c
> > ff60: a6962000 801d890c 00000000 00000000 00020000 a7590000 00000004 00000100
> > ff80: 00000001 d7530ce3 000288b8 00026320 000288b8 00000005 80101204 a4c1e000
> > ffa0: 00000005 80101000 000288b8 00026320 000288b8 000a0000 00000000 00000000
> > ffc0: 000288b8 00026320 000288b8 00000005 7eef3bac 000264e8 00028ad8 00026038
> > ffe0: 00000005 7eef3300 76f76e91 76f78546 800d0030 000288b8 00000000 00000000
> > [<80450f70>] (input_event) from [<a5e2817c>] (0xa5e2817c)
> > Code: e1a08148 eaffffa8 e351001f 812fff1e (e590c018)
> > ---[ end trace 1c691ee85f2ff243 ]---
> > 
> > Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> 
> Applied, thank you.

Actually I'll move it to the current release and mark for sable.

> 
> > ---
> >  drivers/input/misc/bma150.c | 15 +++------------
> >  1 file changed, 3 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
> > index 1cdc8ce97968..64caf43e5bca 100644
> > --- a/drivers/input/misc/bma150.c
> > +++ b/drivers/input/misc/bma150.c
> > @@ -470,7 +470,6 @@ static void bma150_init_input_device(struct bma150_data *bma150,
> >  static int bma150_register_input_device(struct bma150_data *bma150)
> >  {
> >  	struct input_dev *idev;
> > -	int error;
> >  
> >  	idev = devm_input_allocate_device(&bma150->client->dev);
> >  	if (!idev)
> > @@ -482,18 +481,14 @@ static int bma150_register_input_device(struct bma150_data *bma150)
> >  	idev->close = bma150_irq_close;
> >  	input_set_drvdata(idev, bma150);
> >  
> > -	error = input_register_device(idev);
> > -	if (error)
> > -		return error;
> > -
> >  	bma150->input = idev;
> > -	return 0;
> > +
> > +	return input_register_device(idev);
> >  }
> >  
> >  static int bma150_register_polled_device(struct bma150_data *bma150)
> >  {
> >  	struct input_polled_dev *ipoll_dev;
> > -	int error;
> >  
> >  	ipoll_dev = devm_input_allocate_polled_device(&bma150->client->dev);
> >  	if (!ipoll_dev)
> > @@ -509,14 +504,10 @@ 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)
> > -		return error;
> > -
> >  	bma150->input_polled = ipoll_dev;
> >  	bma150->input = ipoll_dev->input;
> >  
> > -	return 0;
> > +	return input_register_polled_device(ipoll_dev);
> >  }
> >  
> >  int bma150_cfg_from_of(struct device_node *np)
> > -- 
> > 2.17.1
> > 
> 
> -- 
> Dmitry

-- 
Dmitry

Re: [PATCH v2 1/5] dt-bindings: input: Add binding for bma150 sensor

[Rob Herring]

On Sat, Feb 02, 2019 at 04:18:02PM +0100, Paweł Chmiel wrote:
> From: Jonathan Bakker <xc-racer2@live.ca>
> 
> Add device tree bindings for Bosch BMA150 Accelerometer Sensor
> 
> Changes from v1:
>  - Add properties for all of bma150_cfg
>  - Correct IRQ type in example
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
>  .../bindings/input/bosch,bma150.txt           | 38 +++++++++++++++++++
>  include/dt-bindings/input/bma150.h            | 22 +++++++++++
>  include/linux/bma150.h                        | 13 +------
>  3 files changed, 62 insertions(+), 11 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/bosch,bma150.txt
>  create mode 100644 include/dt-bindings/input/bma150.h
> 
> diff --git a/Documentation/devicetree/bindings/input/bosch,bma150.txt b/Documentation/devicetree/bindings/input/bosch,bma150.txt
> new file mode 100644
> index 000000000000..f644d132f79c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/bosch,bma150.txt
> @@ -0,0 +1,38 @@
> +* 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

This is implied and can be dropped.

> +- interrupts : Interrupt mapping for IRQ.  If not present device will be polled

> +- any-motion-int : bool for if the any motion interrupt should be enabled
> +- hg-int : bool for if the high-G interrupt should be enabled
> +- lg-int : bool for if the low-G interrupt should be enabled
> +- any-motion-cfg : array of integers for any motion duration and threshold
> +- hg-cfg : array of integers for high-G hysterisis, duration, and threshold
> +- lg-cfg : array of integers for low-G hysterisis, duration, and threshold
> +- range : configuration of range, one of BMA150_RANGE_* as defined in [1]
> +- bandwidth : refresh rate of device, one of BMA150_BW_* as defined in [1]

These all need vendor prefixes if they stay.

What determines all this configuration? It seems like a user may want to 
change at run-time in which case sysfs would be more appropriate.

I don't recall seeing other accelerometers with these, but seems these 
could apply to other accelerometers. In which case, they should be 
common.

> +
> +Example:
> +
> +bma150@38 {

accelerometer@38

> +	compatible = "bosch,bma150";
> +	reg = <0x38>;
> +	interrupt-parent = <&gph0>;
> +	interrupts = <1 IRQ_TYPE_EDGE_RISING>;
> +	any-motion-int;
> +	hg-int;
> +	lg-int;
> +	any-motion-cfg = <0 0>;
> +	hg-cfg = <0 150 160>;
> +	lg-cfg = <0 150 20>;
> +	range = <BMA150_RANGE_2G>;
> +	bandwidth = <BMA150_BW_50HZ>;
> +};
> +
> +[1] include/dt-bindings/input/bma150.h
> diff --git a/include/dt-bindings/input/bma150.h b/include/dt-bindings/input/bma150.h
> new file mode 100644
> index 000000000000..fb38ca787f0f
> --- /dev/null
> +++ b/include/dt-bindings/input/bma150.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This header provides bindings for the BMA150 accelerometer
> + */
> +#ifndef _DT_BINDINGS_INPUT_BMA150_H
> +#define _DT_BINDINGS_INPUT_BMA150_H
> +
> +/* Range */
> +#define BMA150_RANGE_2G		0
> +#define BMA150_RANGE_4G		1
> +#define BMA150_RANGE_8G		2
> +
> +/* Refresh rate */
> +#define BMA150_BW_25HZ		0
> +#define BMA150_BW_50HZ		1
> +#define BMA150_BW_100HZ		2
> +#define BMA150_BW_190HZ		3
> +#define BMA150_BW_375HZ		4
> +#define BMA150_BW_750HZ		5
> +#define BMA150_BW_1500HZ	6
> +
> +#endif /* _DT_BINDINGS_INPUT_BMA150_H */
> diff --git a/include/linux/bma150.h b/include/linux/bma150.h
> index 97ade7cdc870..b85266a9c35c 100644
> --- a/include/linux/bma150.h
> +++ b/include/linux/bma150.h
> @@ -20,19 +20,10 @@
>  #ifndef _BMA150_H_
>  #define _BMA150_H_
>  
> -#define BMA150_DRIVER		"bma150"
> +#include <dt-bindings/input/bma150.h>
>  
> -#define BMA150_RANGE_2G		0
> -#define BMA150_RANGE_4G		1
> -#define BMA150_RANGE_8G		2
> +#define BMA150_DRIVER		"bma150"
>  
> -#define BMA150_BW_25HZ		0
> -#define BMA150_BW_50HZ		1
> -#define BMA150_BW_100HZ		2
> -#define BMA150_BW_190HZ		3
> -#define BMA150_BW_375HZ		4
> -#define BMA150_BW_750HZ		5
> -#define BMA150_BW_1500HZ	6
>  
>  struct bma150_cfg {
>  	bool any_motion_int;		/* Set to enable any-motion interrupt */
> -- 
> 2.17.1
> 

Re: [PATCH v2 1/5] dt-bindings: input: Add binding for bma150 sensor

[Jonathan Bakker]

On 2019-02-18 11:18 a.m., Rob Herring wrote:
> On Sat, Feb 02, 2019 at 04:18:02PM +0100, Paweł Chmiel wrote:
>> From: Jonathan Bakker <xc-racer2@live.ca>
>>
>> Add device tree bindings for Bosch BMA150 Accelerometer Sensor
>>
>> Changes from v1:
>>  - Add properties for all of bma150_cfg
>>  - Correct IRQ type in example
>>
>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
>> ---
>>  .../bindings/input/bosch,bma150.txt           | 38 +++++++++++++++++++
>>  include/dt-bindings/input/bma150.h            | 22 +++++++++++
>>  include/linux/bma150.h                        | 13 +------
>>  3 files changed, 62 insertions(+), 11 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/input/bosch,bma150.txt
>>  create mode 100644 include/dt-bindings/input/bma150.h
>>
>> diff --git a/Documentation/devicetree/bindings/input/bosch,bma150.txt b/Documentation/devicetree/bindings/input/bosch,bma150.txt
>> new file mode 100644
>> index 000000000000..f644d132f79c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/bosch,bma150.txt
>> @@ -0,0 +1,38 @@
>> +* 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
> 
> This is implied and can be dropped.
> 
Ok, sounds good.
>> +- interrupts : Interrupt mapping for IRQ.  If not present device will be polled
> 
>> +- any-motion-int : bool for if the any motion interrupt should be enabled
>> +- hg-int : bool for if the high-G interrupt should be enabled
>> +- lg-int : bool for if the low-G interrupt should be enabled
>> +- any-motion-cfg : array of integers for any motion duration and threshold
>> +- hg-cfg : array of integers for high-G hysterisis, duration, and threshold
>> +- lg-cfg : array of integers for low-G hysterisis, duration, and threshold
>> +- range : configuration of range, one of BMA150_RANGE_* as defined in [1]
>> +- bandwidth : refresh rate of device, one of BMA150_BW_* as defined in [1]
> 
> These all need vendor prefixes if they stay.
> 
> What determines all this configuration? It seems like a user may want to 
> change at run-time in which case sysfs would be more appropriate.
> 
> I don't recall seeing other accelerometers with these, but seems these 
> could apply to other accelerometers. In which case, they should be 
> common.
> 
From my understanding of the pre-existing non-DT driver and the datasheet (available at https://media.digikey.com/pdf/Data%20Sheets/Bosch/BMA150.pdf), the *-int and *-cfg are for configuring of what will cause the chip to send an interrupt.  A sysfs property would probably work for them as well although I don't know if they can be adjusted on the fly or not.  My device doesn't have the interrupt line hooked up and instead uses the polling method, so the only tunables I can test are the range and bandwith which are used as a type of digital smoothing.

As I personally use the defaults and there are no current in-tree users, I am fine with dropping all of this configuration and going with the defaults that are said to provide "generic sensitivity performance".  If someone wants to add it in the future, they can add the sysfs and/or DT properties.  The similar bma180 IIO driver hardcodes everything.

However, looking through all this again, instead of making this driver DT-aware, it might make more sense to expand the bma180 IIO driver to add support for bma150.  Most of the accelerometer drivers are already in iio plus it would add support for the temperature part of the sensor.  Would it then make sense to remove this driver entirely or can the two co-exist?
>> +
>> +Example:
>> +
>> +bma150@38 {
> 
> accelerometer@38
Got it.
> 
>> +	compatible = "bosch,bma150";
>> +	reg = <0x38>;
>> +	interrupt-parent = <&gph0>;
>> +	interrupts = <1 IRQ_TYPE_EDGE_RISING>;
>> +	any-motion-int;
>> +	hg-int;
>> +	lg-int;
>> +	any-motion-cfg = <0 0>;
>> +	hg-cfg = <0 150 160>;
>> +	lg-cfg = <0 150 20>;
>> +	range = <BMA150_RANGE_2G>;
>> +	bandwidth = <BMA150_BW_50HZ>;
>> +};
>> +
>> +[1] include/dt-bindings/input/bma150.h
>> diff --git a/include/dt-bindings/input/bma150.h b/include/dt-bindings/input/bma150.h
>> new file mode 100644
>> index 000000000000..fb38ca787f0f
>> --- /dev/null
>> +++ b/include/dt-bindings/input/bma150.h
>> @@ -0,0 +1,22 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * This header provides bindings for the BMA150 accelerometer
>> + */
>> +#ifndef _DT_BINDINGS_INPUT_BMA150_H
>> +#define _DT_BINDINGS_INPUT_BMA150_H
>> +
>> +/* Range */
>> +#define BMA150_RANGE_2G		0
>> +#define BMA150_RANGE_4G		1
>> +#define BMA150_RANGE_8G		2
>> +
>> +/* Refresh rate */
>> +#define BMA150_BW_25HZ		0
>> +#define BMA150_BW_50HZ		1
>> +#define BMA150_BW_100HZ		2
>> +#define BMA150_BW_190HZ		3
>> +#define BMA150_BW_375HZ		4
>> +#define BMA150_BW_750HZ		5
>> +#define BMA150_BW_1500HZ	6
>> +
>> +#endif /* _DT_BINDINGS_INPUT_BMA150_H */
>> diff --git a/include/linux/bma150.h b/include/linux/bma150.h
>> index 97ade7cdc870..b85266a9c35c 100644
>> --- a/include/linux/bma150.h
>> +++ b/include/linux/bma150.h
>> @@ -20,19 +20,10 @@
>>  #ifndef _BMA150_H_
>>  #define _BMA150_H_
>>  
>> -#define BMA150_DRIVER		"bma150"
>> +#include <dt-bindings/input/bma150.h>
>>  
>> -#define BMA150_RANGE_2G		0
>> -#define BMA150_RANGE_4G		1
>> -#define BMA150_RANGE_8G		2
>> +#define BMA150_DRIVER		"bma150"
>>  
>> -#define BMA150_BW_25HZ		0
>> -#define BMA150_BW_50HZ		1
>> -#define BMA150_BW_100HZ		2
>> -#define BMA150_BW_190HZ		3
>> -#define BMA150_BW_375HZ		4
>> -#define BMA150_BW_750HZ		5
>> -#define BMA150_BW_1500HZ	6
>>  
>>  struct bma150_cfg {
>>  	bool any_motion_int;		/* Set to enable any-motion interrupt */
>> -- 
>> 2.17.1
>>
Thanks,
Jonathan