linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] input: misc: bma150: Add support for device tree
@ 2019-01-25 18:43 Paweł Chmiel
  2019-01-25 18:43 ` [PATCH 1/3] input: misc: bma150: Use managed resources helpers Paweł Chmiel
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
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	[flat|nested] 9+ messages in thread

* [PATCH 1/3] input: misc: bma150: Use managed resources helpers
  2019-01-25 18:43 [PATCH 0/3] input: misc: bma150: Add support for device tree Paweł Chmiel
@ 2019-01-25 18:43 ` Paweł Chmiel
  2019-01-26  1:29   ` Dmitry Torokhov
  2019-01-25 18:43 ` [PATCH 2/3] input: misc: bma150: Add support for device tree Paweł Chmiel
  2019-01-25 18:44 ` [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor Paweł Chmiel
  2 siblings, 1 reply; 9+ messages in thread
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

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	[flat|nested] 9+ messages in thread

* [PATCH 2/3] input: misc: bma150: Add support for device tree
  2019-01-25 18:43 [PATCH 0/3] input: misc: bma150: Add support for device tree Paweł Chmiel
  2019-01-25 18:43 ` [PATCH 1/3] input: misc: bma150: Use managed resources helpers Paweł Chmiel
@ 2019-01-25 18:43 ` Paweł Chmiel
  2019-01-25 18:44 ` [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor Paweł Chmiel
  2 siblings, 0 replies; 9+ messages in thread
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

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	[flat|nested] 9+ messages in thread

* [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
  2019-01-25 18:43 [PATCH 0/3] input: misc: bma150: Add support for device tree Paweł Chmiel
  2019-01-25 18:43 ` [PATCH 1/3] input: misc: bma150: Use managed resources helpers Paweł Chmiel
  2019-01-25 18:43 ` [PATCH 2/3] input: misc: bma150: Add support for device tree Paweł Chmiel
@ 2019-01-25 18:44 ` Paweł Chmiel
  2019-01-26  1:28   ` Dmitry Torokhov
  2 siblings, 1 reply; 9+ messages in thread
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

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	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
  2019-01-25 18:44 ` [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor Paweł Chmiel
@ 2019-01-26  1:28   ` Dmitry Torokhov
  2019-01-26  3:39     ` Jonathan Bakker
  0 siblings, 1 reply; 9+ messages in thread
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

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	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] input: misc: bma150: Use managed resources helpers
  2019-01-25 18:43 ` [PATCH 1/3] input: misc: bma150: Use managed resources helpers Paweł Chmiel
@ 2019-01-26  1:29   ` Dmitry Torokhov
  2019-01-26  3:40     ` Jonathan Bakker
  0 siblings, 1 reply; 9+ messages in thread
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

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	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
  2019-01-26  1:28   ` Dmitry Torokhov
@ 2019-01-26  3:39     ` Jonathan Bakker
  2019-01-28 19:31       ` Dmitry Torokhov
  0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Bakker @ 2019-01-26  3:39 UTC (permalink / raw)
  To: Dmitry Torokhov, Paweł Chmiel
  Cc: robh+dt, mark.rutland, devicetree, linux-input, linux-kernel



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	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] input: misc: bma150: Use managed resources helpers
  2019-01-26  1:29   ` Dmitry Torokhov
@ 2019-01-26  3:40     ` Jonathan Bakker
  0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Bakker @ 2019-01-26  3:40 UTC (permalink / raw)
  To: Dmitry Torokhov, Paweł Chmiel
  Cc: robh+dt, mark.rutland, devicetree, linux-input, linux-kernel



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	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor
  2019-01-26  3:39     ` Jonathan Bakker
@ 2019-01-28 19:31       ` Dmitry Torokhov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2019-01-28 19:31 UTC (permalink / raw)
  To: Jonathan Bakker
  Cc: Paweł Chmiel, robh+dt, mark.rutland, devicetree,
	linux-input, linux-kernel

On Sat, Jan 26, 2019 at 03:39:17AM +0000, Jonathan Bakker wrote:
> 
> 
> 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.

OK, if you do not need to handle them then we can leave this task to the
next user ;)

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-01-28 19:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-25 18:43 [PATCH 0/3] input: misc: bma150: Add support for device tree Paweł Chmiel
2019-01-25 18:43 ` [PATCH 1/3] input: misc: bma150: Use managed resources helpers Paweł Chmiel
2019-01-26  1:29   ` Dmitry Torokhov
2019-01-26  3:40     ` Jonathan Bakker
2019-01-25 18:43 ` [PATCH 2/3] input: misc: bma150: Add support for device tree Paweł Chmiel
2019-01-25 18:44 ` [PATCH 3/3] input: dt-bindings: Add binding for bma150 sensor Paweł Chmiel
2019-01-26  1:28   ` Dmitry Torokhov
2019-01-26  3:39     ` Jonathan Bakker
2019-01-28 19:31       ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).