* [PATCH 1/2] Input: lm8323 - rely on device core to create kp_disable attribute @ 2023-07-24 5:28 Dmitry Torokhov 2023-07-24 5:29 ` [PATCH 2/2] Input: lm8323 - convert to use devm_* api Dmitry Torokhov 0 siblings, 1 reply; 5+ messages in thread From: Dmitry Torokhov @ 2023-07-24 5:28 UTC (permalink / raw) To: linux-input; +Cc: Yangtao Li, linux-kernel Device core now has facilities to create driver-specific device attributes as part of driver probing, use them. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] Input: lm8323 - convert to use devm_* api 2023-07-24 5:28 [PATCH 1/2] Input: lm8323 - rely on device core to create kp_disable attribute Dmitry Torokhov @ 2023-07-24 5:29 ` Dmitry Torokhov 2023-07-24 18:53 ` Christophe JAILLET 0 siblings, 1 reply; 5+ messages in thread From: Dmitry Torokhov @ 2023-07-24 5:29 UTC (permalink / raw) To: linux-input; +Cc: Yangtao Li, linux-kernel From: Yangtao Li <frank.li@vivo.com> Use devm_* api to simplify code, this makes it unnecessary to explicitly release resources. Signed-off-by: Yangtao Li <frank.li@vivo.com> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] Input: lm8323 - convert to use devm_* api 2023-07-24 5:29 ` [PATCH 2/2] Input: lm8323 - convert to use devm_* api Dmitry Torokhov @ 2023-07-24 18:53 ` Christophe JAILLET 2023-07-24 19:01 ` Dmitry Torokhov 0 siblings, 1 reply; 5+ messages in thread From: Christophe JAILLET @ 2023-07-24 18:53 UTC (permalink / raw) To: Dmitry Torokhov, linux-input; +Cc: Yangtao Li, linux-kernel Le 24/07/2023 à 07:29, Dmitry Torokhov a écrit : > From: Yangtao Li <frank.li@vivo.com> > > Use devm_* api to simplify code, this makes it unnecessary to explicitly > release resources. > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > --- > 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); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] Input: lm8323 - convert to use devm_* api 2023-07-24 18:53 ` Christophe JAILLET @ 2023-07-24 19:01 ` Dmitry Torokhov 2023-07-24 19:26 ` Christophe JAILLET 0 siblings, 1 reply; 5+ messages in thread From: Dmitry Torokhov @ 2023-07-24 19:01 UTC (permalink / raw) To: Christophe JAILLET; +Cc: linux-input, Yangtao Li, linux-kernel 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 <frank.li@vivo.com> > > + > > + 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] Input: lm8323 - convert to use devm_* api 2023-07-24 19:01 ` Dmitry Torokhov @ 2023-07-24 19:26 ` Christophe JAILLET 0 siblings, 0 replies; 5+ messages in thread From: Christophe JAILLET @ 2023-07-24 19:26 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, Yangtao Li, linux-kernel 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 <frank.li@vivo.com> >>> + >>> + 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/20230714080611.81302-8-frank.li@vivo.com/ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-24 19:26 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-07-24 5:28 [PATCH 1/2] Input: lm8323 - rely on device core to create kp_disable attribute Dmitry Torokhov 2023-07-24 5:29 ` [PATCH 2/2] Input: lm8323 - convert to use devm_* api Dmitry Torokhov 2023-07-24 18:53 ` Christophe JAILLET 2023-07-24 19:01 ` Dmitry Torokhov 2023-07-24 19:26 ` Christophe JAILLET
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).