* [PATCH 0/5] use managed functions for ad7897 driver
@ 2017-11-02 15:01 Andi Shyti
2017-11-02 15:01 ` [PATCH 1/5] Input: ad7897 - use managed devm_kzalloc Andi Shyti
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Andi Shyti @ 2017-11-02 15:01 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Hennerich, linux-input, linux-kernel, Andi Shyti, Andi Shyti
Hi Dmitry,
after our discussion[*], I decided to do it a more properly by
replacing in the touchscreen drivers the initialization functions
with their related managed functions.
I will slowly send patches for other drivers.
The last patch is trivial, but somehow it bothers me, please feel
free to reject it.
Thanks,
Andi
[*] https://marc.info/?l=linux-input&m=150671805312148&w=2
Andi Shyti (5):
Input: ad7897 - use managed devm_kzalloc
Input: ad7897 - use managed devm_input_allocate_device
Input: ad7897 - use managed devm_request_threaded_irq
Input: ad7897 - use managed devm_device_add_group
Input: ad7897 - use separate error handling for different allocators
drivers/input/touchscreen/ad7877.c | 45 +++++++++++---------------------------
1 file changed, 13 insertions(+), 32 deletions(-)
--
2.15.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/5] Input: ad7897 - use managed devm_kzalloc
2017-11-02 15:01 [PATCH 0/5] use managed functions for ad7897 driver Andi Shyti
@ 2017-11-02 15:01 ` Andi Shyti
2017-11-02 15:01 ` [PATCH 2/5] Input: ad7897 - use managed devm_input_allocate_device Andi Shyti
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Andi Shyti @ 2017-11-02 15:01 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Hennerich, linux-input, linux-kernel, Andi Shyti, Andi Shyti
Allocate the driver's structure by using devm_kzalloc instead of
simple kzalloc and remove all the related kfree calls.
CC: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Andi Shyti <andi@etezian.org>
---
drivers/input/touchscreen/ad7877.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index 0381c7809d1b..68941b72ed8a 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -707,9 +707,9 @@ static int ad7877_probe(struct spi_device *spi)
return err;
}
- ts = kzalloc(sizeof(struct ad7877), GFP_KERNEL);
+ ts = devm_kzalloc(&spi->dev, sizeof(struct ad7877), GFP_KERNEL);
input_dev = input_allocate_device();
- if (!ts || !input_dev) {
+ if (!input_dev) {
err = -ENOMEM;
goto err_free_mem;
}
@@ -799,7 +799,6 @@ static int ad7877_probe(struct spi_device *spi)
free_irq(spi->irq, ts);
err_free_mem:
input_free_device(input_dev);
- kfree(ts);
return err;
}
@@ -813,7 +812,6 @@ static int ad7877_remove(struct spi_device *spi)
free_irq(ts->spi->irq, ts);
input_unregister_device(ts->input);
- kfree(ts);
dev_dbg(&spi->dev, "unregistered touchscreen\n");
--
2.15.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/5] Input: ad7897 - use managed devm_input_allocate_device
2017-11-02 15:01 [PATCH 0/5] use managed functions for ad7897 driver Andi Shyti
2017-11-02 15:01 ` [PATCH 1/5] Input: ad7897 - use managed devm_kzalloc Andi Shyti
@ 2017-11-02 15:01 ` Andi Shyti
2017-11-02 15:01 ` [PATCH 3/5] Input: ad7897 - use managed devm_request_threaded_irq Andi Shyti
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Andi Shyti @ 2017-11-02 15:01 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Hennerich, linux-input, linux-kernel, Andi Shyti, Andi Shyti
Allocate the input device by using the devm_input_allocate_device
function and remove all the related cleanup function and adjust
the goto error handlings accordingly.
CC: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Andi Shyti <andi@etezian.org>
---
drivers/input/touchscreen/ad7877.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index 68941b72ed8a..ec88e3e0020b 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -708,11 +708,9 @@ static int ad7877_probe(struct spi_device *spi)
}
ts = devm_kzalloc(&spi->dev, sizeof(struct ad7877), GFP_KERNEL);
- input_dev = input_allocate_device();
- if (!input_dev) {
- err = -ENOMEM;
- goto err_free_mem;
- }
+ input_dev = devm_input_allocate_device(&spi->dev);
+ if (!input_dev)
+ return -ENOMEM;
spi_set_drvdata(spi, ts);
ts->spi = spi;
@@ -764,8 +762,7 @@ static int ad7877_probe(struct spi_device *spi)
if (verify != AD7877_MM_SEQUENCE){
dev_err(&spi->dev, "%s: Failed to probe %s\n",
dev_name(&spi->dev), input_dev->name);
- err = -ENODEV;
- goto err_free_mem;
+ return -ENODEV;
}
if (gpio3)
@@ -780,7 +777,7 @@ static int ad7877_probe(struct spi_device *spi)
spi->dev.driver->name, ts);
if (err) {
dev_dbg(&spi->dev, "irq %d busy?\n", spi->irq);
- goto err_free_mem;
+ return err;
}
err = sysfs_create_group(&spi->dev.kobj, &ad7877_attr_group);
@@ -797,8 +794,7 @@ static int ad7877_probe(struct spi_device *spi)
sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
err_free_irq:
free_irq(spi->irq, ts);
-err_free_mem:
- input_free_device(input_dev);
+
return err;
}
@@ -811,8 +807,6 @@ static int ad7877_remove(struct spi_device *spi)
ad7877_disable(ts);
free_irq(ts->spi->irq, ts);
- input_unregister_device(ts->input);
-
dev_dbg(&spi->dev, "unregistered touchscreen\n");
return 0;
--
2.15.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/5] Input: ad7897 - use managed devm_request_threaded_irq
2017-11-02 15:01 [PATCH 0/5] use managed functions for ad7897 driver Andi Shyti
2017-11-02 15:01 ` [PATCH 1/5] Input: ad7897 - use managed devm_kzalloc Andi Shyti
2017-11-02 15:01 ` [PATCH 2/5] Input: ad7897 - use managed devm_input_allocate_device Andi Shyti
@ 2017-11-02 15:01 ` Andi Shyti
2017-11-02 15:01 ` [PATCH 4/5] Input: ad7897 - use managed devm_device_add_group Andi Shyti
2017-11-02 15:01 ` [PATCH 5/5] Input: ad7897 - use separate error handling for different allocators Andi Shyti
4 siblings, 0 replies; 7+ messages in thread
From: Andi Shyti @ 2017-11-02 15:01 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Hennerich, linux-input, linux-kernel, Andi Shyti, Andi Shyti
Request the interrupt by using its related managed
devm_request_threaded_irq function, remove the cleanup function
calls and adjust the goto error handlings accordingly.
CC: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Andi Shyti <andi@etezian.org>
---
drivers/input/touchscreen/ad7877.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index ec88e3e0020b..3a4230219b23 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -772,7 +772,7 @@ static int ad7877_probe(struct spi_device *spi)
/* Request AD7877 /DAV GPIO interrupt */
- err = request_threaded_irq(spi->irq, NULL, ad7877_irq,
+ err = devm_request_threaded_irq(&spi->dev, spi->irq, NULL, ad7877_irq,
IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
spi->dev.driver->name, ts);
if (err) {
@@ -782,7 +782,7 @@ static int ad7877_probe(struct spi_device *spi)
err = sysfs_create_group(&spi->dev.kobj, &ad7877_attr_group);
if (err)
- goto err_free_irq;
+ return err;
err = input_register_device(input_dev);
if (err)
@@ -792,8 +792,6 @@ static int ad7877_probe(struct spi_device *spi)
err_remove_attr_group:
sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
-err_free_irq:
- free_irq(spi->irq, ts);
return err;
}
@@ -805,7 +803,6 @@ static int ad7877_remove(struct spi_device *spi)
sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
ad7877_disable(ts);
- free_irq(ts->spi->irq, ts);
dev_dbg(&spi->dev, "unregistered touchscreen\n");
--
2.15.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/5] Input: ad7897 - use managed devm_device_add_group
2017-11-02 15:01 [PATCH 0/5] use managed functions for ad7897 driver Andi Shyti
` (2 preceding siblings ...)
2017-11-02 15:01 ` [PATCH 3/5] Input: ad7897 - use managed devm_request_threaded_irq Andi Shyti
@ 2017-11-02 15:01 ` Andi Shyti
2017-11-03 0:02 ` Dmitry Torokhov
2017-11-02 15:01 ` [PATCH 5/5] Input: ad7897 - use separate error handling for different allocators Andi Shyti
4 siblings, 1 reply; 7+ messages in thread
From: Andi Shyti @ 2017-11-02 15:01 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Hennerich, linux-input, linux-kernel, Andi Shyti, Andi Shyti
Commit 57b8ff070f98 ("driver core: add devm_device_add_group()
and friends") has added the the managed version for creating
sysfs group files.
Use devm_device_add_group instead of sysfs_create_group and
remove the relative sysfs_remove_group and goto label.
CC: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Andi Shyti <andi@etezian.org>
---
drivers/input/touchscreen/ad7877.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index 3a4230219b23..c8a143db8681 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -780,28 +780,17 @@ static int ad7877_probe(struct spi_device *spi)
return err;
}
- err = sysfs_create_group(&spi->dev.kobj, &ad7877_attr_group);
+ err = devm_device_add_group(&spi->dev, &ad7877_attr_group);
if (err)
return err;
- err = input_register_device(input_dev);
- if (err)
- goto err_remove_attr_group;
-
- return 0;
-
-err_remove_attr_group:
- sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
-
- return err;
+ return input_register_device(input_dev);
}
static int ad7877_remove(struct spi_device *spi)
{
struct ad7877 *ts = spi_get_drvdata(spi);
- sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
-
ad7877_disable(ts);
dev_dbg(&spi->dev, "unregistered touchscreen\n");
--
2.15.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/5] Input: ad7897 - use separate error handling for different allocators
2017-11-02 15:01 [PATCH 0/5] use managed functions for ad7897 driver Andi Shyti
` (3 preceding siblings ...)
2017-11-02 15:01 ` [PATCH 4/5] Input: ad7897 - use managed devm_device_add_group Andi Shyti
@ 2017-11-02 15:01 ` Andi Shyti
4 siblings, 0 replies; 7+ messages in thread
From: Andi Shyti @ 2017-11-02 15:01 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Hennerich, linux-input, linux-kernel, Andi Shyti, Andi Shyti
Split the error between devm_kzalloc and
devm_input_allocate_device, there is no need to call the second
allocator if the first has failed. Besides this doesn't provide
practical advantages.
CC: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Andi Shyti <andi@etezian.org>
---
drivers/input/touchscreen/ad7877.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index c8a143db8681..dc2cb99dc43d 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -708,6 +708,9 @@ static int ad7877_probe(struct spi_device *spi)
}
ts = devm_kzalloc(&spi->dev, sizeof(struct ad7877), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+
input_dev = devm_input_allocate_device(&spi->dev);
if (!input_dev)
return -ENOMEM;
--
2.15.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 4/5] Input: ad7897 - use managed devm_device_add_group
2017-11-02 15:01 ` [PATCH 4/5] Input: ad7897 - use managed devm_device_add_group Andi Shyti
@ 2017-11-03 0:02 ` Dmitry Torokhov
0 siblings, 0 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2017-11-03 0:02 UTC (permalink / raw)
To: Andi Shyti; +Cc: Michael Hennerich, linux-input, linux-kernel, Andi Shyti
Hi Andi,
On Thu, Nov 02, 2017 at 05:01:35PM +0200, Andi Shyti wrote:
> Commit 57b8ff070f98 ("driver core: add devm_device_add_group()
> and friends") has added the the managed version for creating
> sysfs group files.
>
> Use devm_device_add_group instead of sysfs_create_group and
> remove the relative sysfs_remove_group and goto label.
>
> CC: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Andi Shyti <andi@etezian.org>
> ---
> drivers/input/touchscreen/ad7877.c | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
> index 3a4230219b23..c8a143db8681 100644
> --- a/drivers/input/touchscreen/ad7877.c
> +++ b/drivers/input/touchscreen/ad7877.c
> @@ -780,28 +780,17 @@ static int ad7877_probe(struct spi_device *spi)
> return err;
> }
>
> - err = sysfs_create_group(&spi->dev.kobj, &ad7877_attr_group);
> + err = devm_device_add_group(&spi->dev, &ad7877_attr_group);
> if (err)
> return err;
>
> - err = input_register_device(input_dev);
> - if (err)
> - goto err_remove_attr_group;
> -
> - return 0;
> -
> -err_remove_attr_group:
> - sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
> -
> - return err;
> + return input_register_device(input_dev);
> }
>
> static int ad7877_remove(struct spi_device *spi)
> {
> struct ad7877 *ts = spi_get_drvdata(spi);
>
> - sysfs_remove_group(&spi->dev.kobj, &ad7877_attr_group);
> -
> ad7877_disable(ts);
Can we have ad7877_disable(ts) invoked via devm_add_action_or_reset()
and get rid of ad7877_remove() completely.
I also do not see a benefit of splitting conversion of each individual
resource to a managed variant into a separate patch. Logically it is all
one operation, going from unmanaged to managed resources, and it would
be easier to review at once.
Thanks!
>
> dev_dbg(&spi->dev, "unregistered touchscreen\n");
> --
> 2.15.0
>
--
Dmitry
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-11-03 0:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-02 15:01 [PATCH 0/5] use managed functions for ad7897 driver Andi Shyti
2017-11-02 15:01 ` [PATCH 1/5] Input: ad7897 - use managed devm_kzalloc Andi Shyti
2017-11-02 15:01 ` [PATCH 2/5] Input: ad7897 - use managed devm_input_allocate_device Andi Shyti
2017-11-02 15:01 ` [PATCH 3/5] Input: ad7897 - use managed devm_request_threaded_irq Andi Shyti
2017-11-02 15:01 ` [PATCH 4/5] Input: ad7897 - use managed devm_device_add_group Andi Shyti
2017-11-03 0:02 ` Dmitry Torokhov
2017-11-02 15:01 ` [PATCH 5/5] Input: ad7897 - use separate error handling for different allocators Andi Shyti
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).