Use devm_* api to simplify code, this makes it unnecessary to explicitly
release resources.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/input/keyboard/lm8333.c | 40 +++++++++------------------------
1 file changed, 11 insertions(+), 29 deletions(-)
diff --git a/drivers/input/keyboard/lm8333.c b/drivers/input/keyboard/lm8333.c
index c9f05764e36d..41d088933e01 100644
--- a/drivers/input/keyboard/lm8333.c
+++ b/drivers/input/keyboard/lm8333.c
@@ -129,6 +129,7 @@ static int lm8333_probe(struct i2c_client *client)
{
const struct lm8333_platform_data *pdata =
dev_get_platdata(&client->dev);
+ struct device *dev = &client->dev;
struct lm8333 *lm8333;
struct input_dev *input;
int err, active_time;
@@ -142,12 +143,10 @@ static int lm8333_probe(struct i2c_client *client)
return -EINVAL;
}
- lm8333 = kzalloc(sizeof(*lm8333), GFP_KERNEL);
- input = input_allocate_device();
- if (!lm8333 || !input) {
- err = -ENOMEM;
- goto free_mem;
- }
+ lm8333 = devm_kzalloc(dev, sizeof(*lm8333), GFP_KERNEL);
+ input = devm_input_allocate_device(dev);
+ if (!lm8333 || !input)
+ return -ENOMEM;
lm8333->client = client;
lm8333->input = input;
@@ -162,7 +161,7 @@ static int lm8333_probe(struct i2c_client *client)
LM8333_NUM_ROWS, LM8333_NUM_COLS,
lm8333->keycodes, input);
if (err)
- goto free_mem;
+ return err;
if (pdata->debounce_time) {
err = lm8333_write8(lm8333, LM8333_DEBOUNCE,
@@ -178,34 +177,18 @@ static int lm8333_probe(struct i2c_client *client)
dev_warn(&client->dev, "Unable to set active time\n");
}
- err = request_threaded_irq(client->irq, NULL, lm8333_irq_thread,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
- "lm8333", lm8333);
+ err = devm_request_threaded_irq(dev, client->irq, NULL, lm8333_irq_thread,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "lm8333", lm8333);
if (err)
- goto free_mem;
+ return err;
err = input_register_device(input);
if (err)
- goto free_irq;
+ return err;
i2c_set_clientdata(client, lm8333);
return 0;
-
- free_irq:
- free_irq(client->irq, lm8333);
- free_mem:
- input_free_device(input);
- kfree(lm8333);
- return err;
-}
-
-static void lm8333_remove(struct i2c_client *client)
-{
- struct lm8333 *lm8333 = i2c_get_clientdata(client);
-
- free_irq(client->irq, lm8333);
- input_unregister_device(lm8333->input);
- kfree(lm8333);
}
static const struct i2c_device_id lm8333_id[] = {
@@ -219,7 +202,6 @@ static struct i2c_driver lm8333_driver = {
.name = "lm8333",
},
.probe = lm8333_probe,
- .remove = lm8333_remove,
.id_table = lm8333_id,
};
module_i2c_driver(lm8333_driver);
--
2.39.0
Use devm_* api to simplify code, this makes it unnecessary to explicitly
release resources.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/input/keyboard/mcs_touchkey.c | 51 +++++++++++----------------
1 file changed, 21 insertions(+), 30 deletions(-)
diff --git a/drivers/input/keyboard/mcs_touchkey.c b/drivers/input/keyboard/mcs_touchkey.c
index de312d8eb974..a0d2e3b1bdc9 100644
--- a/drivers/input/keyboard/mcs_touchkey.c
+++ b/drivers/input/keyboard/mcs_touchkey.c
@@ -92,10 +92,18 @@ static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static void mcs_touchkey_poweroff(void *data)
+{
+ struct mcs_touchkey_data *touchkey = data;
+
+ touchkey->poweron(false);
+}
+
static int mcs_touchkey_probe(struct i2c_client *client)
{
const struct i2c_device_id *id = i2c_client_get_device_id(client);
const struct mcs_platform_data *pdata;
+ struct device *dev = &client->dev;
struct mcs_touchkey_data *data;
struct input_dev *input_dev;
unsigned int fw_reg;
@@ -109,13 +117,12 @@ static int mcs_touchkey_probe(struct i2c_client *client)
return -EINVAL;
}
- data = kzalloc(struct_size(data, keycodes, pdata->key_maxval + 1),
+ data = devm_kzalloc(dev, struct_size(data, keycodes, pdata->key_maxval + 1),
GFP_KERNEL);
- input_dev = input_allocate_device();
+ input_dev = devm_input_allocate_device(dev);
if (!data || !input_dev) {
dev_err(&client->dev, "Failed to allocate memory\n");
- error = -ENOMEM;
- goto err_free_mem;
+ return -ENOMEM;
}
data->client = client;
@@ -136,9 +143,8 @@ static int mcs_touchkey_probe(struct i2c_client *client)
fw_ver = i2c_smbus_read_byte_data(client, fw_reg);
if (fw_ver < 0) {
- error = fw_ver;
dev_err(&client->dev, "i2c read error[%d]\n", error);
- goto err_free_mem;
+ return fw_ver;
}
dev_info(&client->dev, "Firmware version: %d\n", fw_ver);
@@ -169,40 +175,26 @@ static int mcs_touchkey_probe(struct i2c_client *client)
if (pdata->poweron) {
data->poweron = pdata->poweron;
data->poweron(true);
+
+ error = devm_add_action_or_reset(dev, mcs_touchkey_poweroff, data);
+ if (error)
+ return error;
}
- error = request_threaded_irq(client->irq, NULL, mcs_touchkey_interrupt,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
- client->dev.driver->name, data);
+ error = devm_request_threaded_irq(dev, client->irq, NULL, mcs_touchkey_interrupt,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ client->dev.driver->name, data);
if (error) {
dev_err(&client->dev, "Failed to register interrupt\n");
- goto err_free_mem;
+ return error;
}
error = input_register_device(input_dev);
if (error)
- goto err_free_irq;
+ return error;
i2c_set_clientdata(client, data);
return 0;
-
-err_free_irq:
- free_irq(client->irq, data);
-err_free_mem:
- input_free_device(input_dev);
- kfree(data);
- return error;
-}
-
-static void mcs_touchkey_remove(struct i2c_client *client)
-{
- struct mcs_touchkey_data *data = i2c_get_clientdata(client);
-
- free_irq(client->irq, data);
- if (data->poweron)
- data->poweron(false);
- input_unregister_device(data->input_dev);
- kfree(data);
}
static void mcs_touchkey_shutdown(struct i2c_client *client)
@@ -259,7 +251,6 @@ static struct i2c_driver mcs_touchkey_driver = {
.pm = pm_sleep_ptr(&mcs_touchkey_pm_ops),
},
.probe = mcs_touchkey_probe,
- .remove = mcs_touchkey_remove,
.shutdown = mcs_touchkey_shutdown,
.id_table = mcs_touchkey_id,
};
--
2.39.0
Use devm_* api to simplify code, this makes it unnecessary to explicitly
release resources.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/input/keyboard/qt2160.c | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/drivers/input/keyboard/qt2160.c b/drivers/input/keyboard/qt2160.c
index 599ea85cfd30..218ef92b8c2b 100644
--- a/drivers/input/keyboard/qt2160.c
+++ b/drivers/input/keyboard/qt2160.c
@@ -340,6 +340,7 @@ static bool qt2160_identify(struct i2c_client *client)
static int qt2160_probe(struct i2c_client *client)
{
+ struct device *dev = &client->dev;
struct qt2160_data *qt2160;
struct input_dev *input;
int i;
@@ -358,12 +359,11 @@ static int qt2160_probe(struct i2c_client *client)
return -ENODEV;
/* Chip is valid and active. Allocate structure */
- qt2160 = kzalloc(sizeof(struct qt2160_data), GFP_KERNEL);
- input = input_allocate_device();
+ qt2160 = devm_kzalloc(dev, sizeof(struct qt2160_data), GFP_KERNEL);
+ input = devm_input_allocate_device(dev);
if (!qt2160 || !input) {
dev_err(&client->dev, "insufficient memory\n");
- error = -ENOMEM;
- goto err_free_mem;
+ return -ENOMEM;
}
qt2160->client = client;
@@ -389,23 +389,23 @@ static int qt2160_probe(struct i2c_client *client)
error = qt2160_write(client, QT2160_CMD_CALIBRATE, 1);
if (error) {
dev_err(&client->dev, "failed to calibrate device\n");
- goto err_free_mem;
+ return error;
}
if (client->irq) {
- error = request_irq(client->irq, qt2160_irq,
- IRQF_TRIGGER_FALLING, "qt2160", qt2160);
+ error = devm_request_irq(dev, client->irq, qt2160_irq,
+ IRQF_TRIGGER_FALLING, "qt2160", qt2160);
if (error) {
dev_err(&client->dev,
"failed to allocate irq %d\n", client->irq);
- goto err_free_mem;
+ return error;
}
}
error = qt2160_register_leds(qt2160);
if (error) {
dev_err(&client->dev, "Failed to register leds\n");
- goto err_free_irq;
+ return error;
}
error = input_register_device(qt2160->input);
@@ -422,29 +422,21 @@ static int qt2160_probe(struct i2c_client *client)
err_unregister_leds:
qt2160_unregister_leds(qt2160);
-err_free_irq:
- if (client->irq)
- free_irq(client->irq, qt2160);
-err_free_mem:
- input_free_device(input);
- kfree(qt2160);
return error;
}
static void qt2160_remove(struct i2c_client *client)
{
struct qt2160_data *qt2160 = i2c_get_clientdata(client);
+ struct device *dev = &client->dev;
qt2160_unregister_leds(qt2160);
/* Release IRQ so no queue will be scheduled */
if (client->irq)
- free_irq(client->irq, qt2160);
+ devm_free_irq(dev, client->irq, qt2160);
cancel_delayed_work_sync(&qt2160->dwork);
-
- input_unregister_device(qt2160->input);
- kfree(qt2160);
}
static const struct i2c_device_id qt2160_idtable[] = {
--
2.39.0
Hi Yangtao,
On Fri, Jul 14, 2023 at 04:06:10PM +0800, Yangtao Li wrote:
> Use devm_* api to simplify code, this makes it unnecessary to explicitly
> release resources.
>
> Signed-off-by: Yangtao Li <[email protected]>
> ---
> drivers/input/keyboard/qt2160.c | 30 +++++++++++-------------------
> 1 file changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/input/keyboard/qt2160.c b/drivers/input/keyboard/qt2160.c
> index 599ea85cfd30..218ef92b8c2b 100644
> --- a/drivers/input/keyboard/qt2160.c
> +++ b/drivers/input/keyboard/qt2160.c
> @@ -340,6 +340,7 @@ static bool qt2160_identify(struct i2c_client *client)
>
> static int qt2160_probe(struct i2c_client *client)
> {
> + struct device *dev = &client->dev;
You create a temporary, but half of the code does not use it. Please if
you introduce a temporary make sure everything is using it.
> struct qt2160_data *qt2160;
> struct input_dev *input;
> int i;
> @@ -358,12 +359,11 @@ static int qt2160_probe(struct i2c_client *client)
> return -ENODEV;
>
> /* Chip is valid and active. Allocate structure */
> - qt2160 = kzalloc(sizeof(struct qt2160_data), GFP_KERNEL);
> - input = input_allocate_device();
> + qt2160 = devm_kzalloc(dev, sizeof(struct qt2160_data), GFP_KERNEL);
> + input = devm_input_allocate_device(dev);
> if (!qt2160 || !input) {
This double check was a cheat when resources did not free automatically,
it makes no sense to carry with devm.
> dev_err(&client->dev, "insufficient memory\n");
> - error = -ENOMEM;
> - goto err_free_mem;
> + return -ENOMEM;
> }
>
> qt2160->client = client;
> @@ -389,23 +389,23 @@ static int qt2160_probe(struct i2c_client *client)
> error = qt2160_write(client, QT2160_CMD_CALIBRATE, 1);
> if (error) {
> dev_err(&client->dev, "failed to calibrate device\n");
> - goto err_free_mem;
> + return error;
> }
>
> if (client->irq) {
> - error = request_irq(client->irq, qt2160_irq,
> - IRQF_TRIGGER_FALLING, "qt2160", qt2160);
> + error = devm_request_irq(dev, client->irq, qt2160_irq,
> + IRQF_TRIGGER_FALLING, "qt2160", qt2160);
> if (error) {
> dev_err(&client->dev,
> "failed to allocate irq %d\n", client->irq);
> - goto err_free_mem;
> + return error;
> }
> }
>
> error = qt2160_register_leds(qt2160);
> if (error) {
> dev_err(&client->dev, "Failed to register leds\n");
> - goto err_free_irq;
> + return error;
> }
>
> error = input_register_device(qt2160->input);
> @@ -422,29 +422,21 @@ static int qt2160_probe(struct i2c_client *client)
>
> err_unregister_leds:
> qt2160_unregister_leds(qt2160);
LEDs can also be registered with devm.
> -err_free_irq:
> - if (client->irq)
> - free_irq(client->irq, qt2160);
> -err_free_mem:
> - input_free_device(input);
> - kfree(qt2160);
> return error;
> }
>
> static void qt2160_remove(struct i2c_client *client)
> {
> struct qt2160_data *qt2160 = i2c_get_clientdata(client);
> + struct device *dev = &client->dev;
>
> qt2160_unregister_leds(qt2160);
>
> /* Release IRQ so no queue will be scheduled */
> if (client->irq)
> - free_irq(client->irq, qt2160);
> + devm_free_irq(dev, client->irq, qt2160);
>
> cancel_delayed_work_sync(&qt2160->dwork);
It is not great that we are left with non-empty qt2160_remove(). The
driver should be converted away from using work item, and then entirety
of qt2160_remove() can be deleted.
I posted a series that cleans up the driver and incorporates an updated
version of your patch.
Thanks.
--
Dmitry
Hi Yangtao,
On Fri, Jul 14, 2023 at 04:06:04PM +0800, Yangtao Li wrote:
> Use devm_* api to simplify code, this makes it unnecessary to explicitly
> release resources.
>
> Signed-off-by: Yangtao Li <[email protected]>
> ---
> drivers/input/keyboard/lm8333.c | 40 +++++++++------------------------
> 1 file changed, 11 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/input/keyboard/lm8333.c b/drivers/input/keyboard/lm8333.c
> index c9f05764e36d..41d088933e01 100644
> --- a/drivers/input/keyboard/lm8333.c
> +++ b/drivers/input/keyboard/lm8333.c
> @@ -129,6 +129,7 @@ static int lm8333_probe(struct i2c_client *client)
> {
> const struct lm8333_platform_data *pdata =
> dev_get_platdata(&client->dev);
> + struct device *dev = &client->dev;
This temporary is used only in few places, while the rest are still
using &client->dev. I removed it, made a couple more changes and
applied, thank you.
--
Dmitry
On Fri, Jul 14, 2023 at 04:06:06PM +0800, Yangtao Li wrote:
> Use devm_* api to simplify code, this makes it unnecessary to explicitly
> release resources.
Applied with minor edits, thank you.
--
Dmitry