mbox series

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

Message ID 20190202151806.9064-1-pawel.mikolaj.chmiel@gmail.com
Headers show
Series input: misc: bma150: Add support for device tree | expand

Message

Paweł Chmiel Feb. 2, 2019, 3:18 p.m. UTC 
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

Comments

Dmitry Torokhov Feb. 6, 2019, 6:52 p.m. UTC | #1 
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 Torokhov Feb. 6, 2019, 6:53 p.m. UTC | #2 
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 Torokhov Feb. 6, 2019, 7:23 p.m. UTC | #3 
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