Device core now has facilities to create driver-specific device attributes
as part of driver probing, use them.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/keyboard/lm8323.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
index 3964f6e0f6af..d5195415533a 100644
--- a/drivers/input/keyboard/lm8323.c
+++ b/drivers/input/keyboard/lm8323.c
@@ -615,6 +615,12 @@ static ssize_t lm8323_set_disable(struct device *dev,
}
static DEVICE_ATTR(disable_kp, 0644, lm8323_show_disable, lm8323_set_disable);
+static struct attribute *lm8323_attrs[] = {
+ &dev_attr_disable_kp.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(lm8323);
+
static int lm8323_probe(struct i2c_client *client)
{
struct lm8323_platform_data *pdata = dev_get_platdata(&client->dev);
@@ -696,9 +702,6 @@ static int lm8323_probe(struct i2c_client *client)
}
lm->kp_enabled = true;
- err = device_create_file(&client->dev, &dev_attr_disable_kp);
- if (err < 0)
- goto fail2;
idev->name = pdata->name ? : "LM8323 keypad";
snprintf(lm->phys, sizeof(lm->phys),
@@ -719,14 +722,14 @@ static int lm8323_probe(struct i2c_client *client)
err = input_register_device(idev);
if (err) {
dev_dbg(&client->dev, "error registering input device\n");
- goto fail3;
+ goto fail2;
}
err = request_threaded_irq(client->irq, NULL, lm8323_irq,
IRQF_TRIGGER_LOW|IRQF_ONESHOT, "lm8323", lm);
if (err) {
dev_err(&client->dev, "could not get IRQ %d\n", client->irq);
- goto fail4;
+ goto fail3;
}
i2c_set_clientdata(client, lm);
@@ -736,11 +739,9 @@ static int lm8323_probe(struct i2c_client *client)
return 0;
-fail4:
+fail3:
input_unregister_device(idev);
idev = NULL;
-fail3:
- device_remove_file(&client->dev, &dev_attr_disable_kp);
fail2:
while (--pwm >= 0)
if (lm->pwm[pwm].enabled)
@@ -761,8 +762,6 @@ static void lm8323_remove(struct i2c_client *client)
input_unregister_device(lm->idev);
- device_remove_file(&lm->client->dev, &dev_attr_disable_kp);
-
for (i = 0; i < 3; i++)
if (lm->pwm[i].enabled)
led_classdev_unregister(&lm->pwm[i].cdev);
@@ -823,8 +822,9 @@ static const struct i2c_device_id lm8323_id[] = {
static struct i2c_driver lm8323_i2c_driver = {
.driver = {
- .name = "lm8323",
- .pm = pm_sleep_ptr(&lm8323_pm_ops),
+ .name = "lm8323",
+ .pm = pm_sleep_ptr(&lm8323_pm_ops),
+ .dev_groups = lm8323_groups,
},
.probe = lm8323_probe,
.remove = lm8323_remove,
--
2.41.0.487.g6d72f3e995-goog
From: Yangtao Li <[email protected]>
Use devm_* api to simplify code, this makes it unnecessary to explicitly
release resources.
Signed-off-by: Yangtao Li <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/keyboard/lm8323.c | 77 +++++++++++----------------------
1 file changed, 26 insertions(+), 51 deletions(-)
diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
index d5195415533a..7bee93e9b0f5 100644
--- a/drivers/input/keyboard/lm8323.c
+++ b/drivers/input/keyboard/lm8323.c
@@ -556,6 +556,7 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
const char *name)
{
struct lm8323_pwm *pwm;
+ int err;
BUG_ON(id > 3);
@@ -575,9 +576,11 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
pwm->cdev.name = name;
pwm->cdev.brightness_set = lm8323_pwm_set_brightness;
pwm->cdev.groups = lm8323_pwm_groups;
- if (led_classdev_register(dev, &pwm->cdev) < 0) {
- dev_err(dev, "couldn't register PWM %d\n", id);
- return -1;
+
+ err = devm_led_classdev_register(dev, &pwm->cdev);
+ if (err) {
+ dev_err(dev, "couldn't register PWM %d: %d\n", id, err);
+ return err;
}
pwm->enabled = true;
}
@@ -585,8 +588,6 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
return 0;
}
-static struct i2c_driver lm8323_i2c_driver;
-
static ssize_t lm8323_show_disable(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -648,12 +649,13 @@ static int lm8323_probe(struct i2c_client *client)
return -EINVAL;
}
- lm = kzalloc(sizeof *lm, GFP_KERNEL);
- idev = input_allocate_device();
- if (!lm || !idev) {
- err = -ENOMEM;
- goto fail1;
- }
+ lm = devm_kzalloc(&client->dev, sizeof(*lm), GFP_KERNEL);
+ if (!lm)
+ return -ENOMEM;
+
+ idev = devm_input_allocate_device(&client->dev);
+ if (!idev)
+ return -ENOMEM;
lm->client = client;
lm->idev = idev;
@@ -669,8 +671,10 @@ static int lm8323_probe(struct i2c_client *client)
lm8323_reset(lm);
- /* Nothing's set up to service the IRQ yet, so just spin for max.
- * 100ms until we can configure. */
+ /*
+ * Nothing's set up to service the IRQ yet, so just spin for max.
+ * 100ms until we can configure.
+ */
tmo = jiffies + msecs_to_jiffies(100);
while (lm8323_read(lm, LM8323_CMD_READ_INT, data, 1) == 1) {
if (data[0] & INT_NOINIT)
@@ -690,15 +694,14 @@ static int lm8323_probe(struct i2c_client *client)
/* If a true probe check the device */
if (lm8323_read_id(lm, data) != 0) {
dev_err(&client->dev, "device not found\n");
- err = -ENODEV;
- goto fail1;
+ return -ENODEV;
}
for (pwm = 0; pwm < LM8323_NUM_PWMS; pwm++) {
err = init_pwm(lm, pwm + 1, &client->dev,
pdata->pwm_names[pwm]);
- if (err < 0)
- goto fail2;
+ if (err)
+ return err;
}
lm->kp_enabled = true;
@@ -722,14 +725,16 @@ static int lm8323_probe(struct i2c_client *client)
err = input_register_device(idev);
if (err) {
dev_dbg(&client->dev, "error registering input device\n");
- goto fail2;
+ return err;
}
- err = request_threaded_irq(client->irq, NULL, lm8323_irq,
- IRQF_TRIGGER_LOW|IRQF_ONESHOT, "lm8323", lm);
+ err = devm_request_threaded_irq(&client->dev, client->irq,
+ NULL, lm8323_irq,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ "lm8323", lm);
if (err) {
dev_err(&client->dev, "could not get IRQ %d\n", client->irq);
- goto fail3;
+ return err;
}
i2c_set_clientdata(client, lm);
@@ -738,35 +743,6 @@ static int lm8323_probe(struct i2c_client *client)
enable_irq_wake(client->irq);
return 0;
-
-fail3:
- input_unregister_device(idev);
- idev = NULL;
-fail2:
- while (--pwm >= 0)
- if (lm->pwm[pwm].enabled)
- led_classdev_unregister(&lm->pwm[pwm].cdev);
-fail1:
- input_free_device(idev);
- kfree(lm);
- return err;
-}
-
-static void lm8323_remove(struct i2c_client *client)
-{
- struct lm8323_chip *lm = i2c_get_clientdata(client);
- int i;
-
- disable_irq_wake(client->irq);
- free_irq(client->irq, lm);
-
- input_unregister_device(lm->idev);
-
- for (i = 0; i < 3; i++)
- if (lm->pwm[i].enabled)
- led_classdev_unregister(&lm->pwm[i].cdev);
-
- kfree(lm);
}
/*
@@ -827,7 +803,6 @@ static struct i2c_driver lm8323_i2c_driver = {
.dev_groups = lm8323_groups,
},
.probe = lm8323_probe,
- .remove = lm8323_remove,
.id_table = lm8323_id,
};
MODULE_DEVICE_TABLE(i2c, lm8323_id);
--
2.41.0.487.g6d72f3e995-goog
Le 24/07/2023 à 07:29, Dmitry Torokhov a écrit :
> From: Yangtao Li <[email protected]>
>
> Use devm_* api to simplify code, this makes it unnecessary to explicitly
> release resources.
>
> Signed-off-by: Yangtao Li <[email protected]>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> drivers/input/keyboard/lm8323.c | 77 +++++++++++----------------------
> 1 file changed, 26 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
> index d5195415533a..7bee93e9b0f5 100644
> --- a/drivers/input/keyboard/lm8323.c
> +++ b/drivers/input/keyboard/lm8323.c
> @@ -556,6 +556,7 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
> const char *name)
> {
> struct lm8323_pwm *pwm;
> + int err;
>
> BUG_ON(id > 3);
>
> @@ -575,9 +576,11 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
> pwm->cdev.name = name;
> pwm->cdev.brightness_set = lm8323_pwm_set_brightness;
> pwm->cdev.groups = lm8323_pwm_groups;
> - if (led_classdev_register(dev, &pwm->cdev) < 0) {
> - dev_err(dev, "couldn't register PWM %d\n", id);
> - return -1;
> +
> + err = devm_led_classdev_register(dev, &pwm->cdev);
> + if (err) {
> + dev_err(dev, "couldn't register PWM %d: %d\n", id, err);
> + return err;
> }
> pwm->enabled = true;
> }
> @@ -585,8 +588,6 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
> return 0;
> }
>
> -static struct i2c_driver lm8323_i2c_driver;
> -
> static ssize_t lm8323_show_disable(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -648,12 +649,13 @@ static int lm8323_probe(struct i2c_client *client)
> return -EINVAL;
> }
>
> - lm = kzalloc(sizeof *lm, GFP_KERNEL);
> - idev = input_allocate_device();
> - if (!lm || !idev) {
> - err = -ENOMEM;
> - goto fail1;
> - }
> + lm = devm_kzalloc(&client->dev, sizeof(*lm), GFP_KERNEL);
> + if (!lm)
> + return -ENOMEM;
> +
> + idev = devm_input_allocate_device(&client->dev);
> + if (!idev)
> + return -ENOMEM;
>
> lm->client = client;
> lm->idev = idev;
> @@ -669,8 +671,10 @@ static int lm8323_probe(struct i2c_client *client)
>
> lm8323_reset(lm);
>
> - /* Nothing's set up to service the IRQ yet, so just spin for max.
> - * 100ms until we can configure. */
> + /*
> + * Nothing's set up to service the IRQ yet, so just spin for max.
> + * 100ms until we can configure.
> + */
> tmo = jiffies + msecs_to_jiffies(100);
> while (lm8323_read(lm, LM8323_CMD_READ_INT, data, 1) == 1) {
> if (data[0] & INT_NOINIT)
> @@ -690,15 +694,14 @@ static int lm8323_probe(struct i2c_client *client)
> /* If a true probe check the device */
> if (lm8323_read_id(lm, data) != 0) {
> dev_err(&client->dev, "device not found\n");
> - err = -ENODEV;
> - goto fail1;
> + return -ENODEV;
> }
>
> for (pwm = 0; pwm < LM8323_NUM_PWMS; pwm++) {
> err = init_pwm(lm, pwm + 1, &client->dev,
> pdata->pwm_names[pwm]);
> - if (err < 0)
> - goto fail2;
> + if (err)
> + return err;
> }
>
> lm->kp_enabled = true;
> @@ -722,14 +725,16 @@ static int lm8323_probe(struct i2c_client *client)
> err = input_register_device(idev);
> if (err) {
> dev_dbg(&client->dev, "error registering input device\n");
> - goto fail2;
> + return err;
> }
>
> - err = request_threaded_irq(client->irq, NULL, lm8323_irq,
> - IRQF_TRIGGER_LOW|IRQF_ONESHOT, "lm8323", lm);
> + err = devm_request_threaded_irq(&client->dev, client->irq,
> + NULL, lm8323_irq,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + "lm8323", lm);
> if (err) {
> dev_err(&client->dev, "could not get IRQ %d\n", client->irq);
> - goto fail3;
> + return err;
> }
>
> i2c_set_clientdata(client, lm);
> @@ -738,35 +743,6 @@ static int lm8323_probe(struct i2c_client *client)
> enable_irq_wake(client->irq);
>
> return 0;
> -
> -fail3:
> - input_unregister_device(idev);
> - idev = NULL;
> -fail2:
> - while (--pwm >= 0)
> - if (lm->pwm[pwm].enabled)
> - led_classdev_unregister(&lm->pwm[pwm].cdev);
This and...
> -fail1:
> - input_free_device(idev);
> - kfree(lm);
> - return err;
> -}
> -
> -static void lm8323_remove(struct i2c_client *client)
> -{
> - struct lm8323_chip *lm = i2c_get_clientdata(client);
> - int i;
> -
> - disable_irq_wake(client->irq);
> - free_irq(client->irq, lm);
> -
> - input_unregister_device(lm->idev);
> -
> - for (i = 0; i < 3; i++)
> - if (lm->pwm[i].enabled)
> - led_classdev_unregister(&lm->pwm[i].cdev);
this look wrong.
What you left for lm8323 looks correct.
CJ
> -
> - kfree(lm);
> }
>
> /*
> @@ -827,7 +803,6 @@ static struct i2c_driver lm8323_i2c_driver = {
> .dev_groups = lm8323_groups,
> },
> .probe = lm8323_probe,
> - .remove = lm8323_remove,
> .id_table = lm8323_id,
> };
> MODULE_DEVICE_TABLE(i2c, lm8323_id);
On Mon, Jul 24, 2023 at 08:53:11PM +0200, Christophe JAILLET wrote:
> Le 24/07/2023 ? 07:29, Dmitry Torokhov a ?crit?:
> > From: Yangtao Li <[email protected]>
> > +
> > + err = devm_led_classdev_register(dev, &pwm->cdev);
^^^^^^^^^^^^^^
...
> > -
> > - for (i = 0; i < 3; i++)
> > - if (lm->pwm[i].enabled)
> > - led_classdev_unregister(&lm->pwm[i].cdev);
>
> this look wrong.
Why? It will be cleared up by devm.
Thanks.
--
Dmitry
Le 24/07/2023 à 21:01, Dmitry Torokhov a écrit :
> On Mon, Jul 24, 2023 at 08:53:11PM +0200, Christophe JAILLET wrote:
>> Le 24/07/2023 à 07:29, Dmitry Torokhov a écrit :
>>> From: Yangtao Li <[email protected]>
>>> +
>>> + err = devm_led_classdev_register(dev, &pwm->cdev);
>
> ^^^^^^^^^^^^^^
Oops,
For some reason, I missed this hunk :(.
>
>>> -
>>> - for (i = 0; i < 3; i++)
>>> - if (lm->pwm[i].enabled)
>>> - led_classdev_unregister(&lm->pwm[i].cdev);
>>
>> this look wrong.
>
> Why? It will be cleared up by devm.
>
> Thanks.
>
Sorry for the noise.
I've been puzzled by [1].
CJ
[1]: https://lore.kernel.org/all/[email protected]/