* [PATCH v2 1/3] Input: ad7897 - use managed allocated resources
2017-11-08 14:04 [PATCH v2 0/3] use managed functions for ad7897 driver Andi Shyti
@ 2017-11-08 14:04 ` Andi Shyti
2017-11-08 15:31 ` Lars-Peter Clausen
2017-11-08 14:04 ` [PATCH v2 2/3] Input: ad7897 - use devm_add_action_or_reset to disable the device Andi Shyti
2017-11-08 14:04 ` [PATCH v2 3/3] Input: ad7897 - use separate error handling for different allocators Andi Shyti
2 siblings, 1 reply; 6+ messages in thread
From: Andi Shyti @ 2017-11-08 14:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Hennerich, linux-input, linux-kernel, Andi Shyti, Andi Shyti
Use managed allocated resources to simplify error handling during
probing failure and module exiting.
With this all the goto labels in the probe function together with
the cleanups in the remove function are unnecessary, therefore
removed.
CC: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Andi Shyti <andi@etezian.org>
---
drivers/input/touchscreen/ad7877.c | 42 +++++++++-----------------------------
1 file changed, 10 insertions(+), 32 deletions(-)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index 0381c7809d1b..c8a143db8681 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -707,12 +707,10 @@ static int ad7877_probe(struct spi_device *spi)
return err;
}
- ts = kzalloc(sizeof(struct ad7877), GFP_KERNEL);
- input_dev = input_allocate_device();
- if (!ts || !input_dev) {
- err = -ENOMEM;
- goto err_free_mem;
- }
+ ts = devm_kzalloc(&spi->dev, sizeof(struct ad7877), GFP_KERNEL);
+ 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)
@@ -775,45 +772,26 @@ 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) {
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);
- if (err)
- goto err_free_irq;
-
- err = input_register_device(input_dev);
+ err = devm_device_add_group(&spi->dev, &ad7877_attr_group);
if (err)
- goto err_remove_attr_group;
-
- return 0;
+ return err;
-err_remove_attr_group:
- 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);
- kfree(ts);
- 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);
- 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 [flat|nested] 6+ messages in thread* [PATCH v2 2/3] Input: ad7897 - use devm_add_action_or_reset to disable the device
2017-11-08 14:04 [PATCH v2 0/3] use managed functions for ad7897 driver Andi Shyti
2017-11-08 14:04 ` [PATCH v2 1/3] Input: ad7897 - use managed allocated resources Andi Shyti
@ 2017-11-08 14:04 ` Andi Shyti
2017-11-08 14:04 ` [PATCH v2 3/3] Input: ad7897 - use separate error handling for different allocators Andi Shyti
2 siblings, 0 replies; 6+ messages in thread
From: Andi Shyti @ 2017-11-08 14:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Hennerich, linux-input, linux-kernel, Andi Shyti, Andi Shyti
Use the ad7877_disable() as a custom action when the driver gets
removed instead of calling it from the remove function.
Because ad7877_remove() was just calling the disable function,
get rid of it.
CC: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Andi Shyti <andi@etezian.org>
---
drivers/input/touchscreen/ad7877.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index c8a143db8681..cf59e569d890 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -417,8 +417,10 @@ static irqreturn_t ad7877_irq(int irq, void *handle)
return IRQ_HANDLED;
}
-static void ad7877_disable(struct ad7877 *ts)
+static void ad7877_disable(void *data)
{
+ struct ad7877 *ts = data;
+
mutex_lock(&ts->mutex);
if (!ts->disabled) {
@@ -712,6 +714,10 @@ static int ad7877_probe(struct spi_device *spi)
if (!input_dev)
return -ENOMEM;
+ err = devm_add_action_or_reset(&spi->dev, ad7877_disable, ts);
+ if (err)
+ return err;
+
spi_set_drvdata(spi, ts);
ts->spi = spi;
ts->input = input_dev;
@@ -787,17 +793,6 @@ static int ad7877_probe(struct spi_device *spi)
return input_register_device(input_dev);
}
-static int ad7877_remove(struct spi_device *spi)
-{
- struct ad7877 *ts = spi_get_drvdata(spi);
-
- ad7877_disable(ts);
-
- dev_dbg(&spi->dev, "unregistered touchscreen\n");
-
- return 0;
-}
-
static int __maybe_unused ad7877_suspend(struct device *dev)
{
struct ad7877 *ts = dev_get_drvdata(dev);
@@ -824,7 +819,6 @@ static struct spi_driver ad7877_driver = {
.pm = &ad7877_pm,
},
.probe = ad7877_probe,
- .remove = ad7877_remove,
};
module_spi_driver(ad7877_driver);
--
2.15.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 3/3] Input: ad7897 - use separate error handling for different allocators
2017-11-08 14:04 [PATCH v2 0/3] use managed functions for ad7897 driver Andi Shyti
2017-11-08 14:04 ` [PATCH v2 1/3] Input: ad7897 - use managed allocated resources Andi Shyti
2017-11-08 14:04 ` [PATCH v2 2/3] Input: ad7897 - use devm_add_action_or_reset to disable the device Andi Shyti
@ 2017-11-08 14:04 ` Andi Shyti
2 siblings, 0 replies; 6+ messages in thread
From: Andi Shyti @ 2017-11-08 14:04 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 cf59e569d890..98deffde3fe2 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -710,6 +710,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 [flat|nested] 6+ messages in thread