| Message ID | 20200919180304.2885-7-marek.behun@nic.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers) | expand |
Hi! > Now that the potential use-after-free issue is resolved we can use > devres for LED registration in this driver. > > By using devres version of LED registering function we can remove the > .remove method from this driver. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > Cc: Dan Murphy <dmurphy@ti.com> AFAICT this one is buggy, I sent explanation before. Why are you resubmitting it? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Sun, 20 Sep 2020 23:45:32 +0200 Pavel Machek <pavel@ucw.cz> wrote: > Hi! > > > Now that the potential use-after-free issue is resolved we can use > > devres for LED registration in this driver. > > > > By using devres version of LED registering function we can remove the > > .remove method from this driver. > > > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > > Cc: Dan Murphy <dmurphy@ti.com> > > AFAICT this one is buggy, I sent explanation before. Why are you > resubmitting it? > > Pavel The previous patch in this series (v3 5/9) should solve this issue and th commit message explains how. Marek
On Sun 2020-09-20 23:54:36, Marek Behun wrote: > On Sun, 20 Sep 2020 23:45:32 +0200 > Pavel Machek <pavel@ucw.cz> wrote: > > > Hi! > > > > > Now that the potential use-after-free issue is resolved we can use > > > devres for LED registration in this driver. > > > > > > By using devres version of LED registering function we can remove the > > > .remove method from this driver. > > > > > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > > > Cc: Dan Murphy <dmurphy@ti.com> > > > > AFAICT this one is buggy, I sent explanation before. Why are you > > resubmitting it? > > The previous patch in this series (v3 5/9) should solve this issue and > th commit message explains how. Aha, let me see. Will 5/9 have some side-effects, like device appearing at different place in sysfs? First few patches look ok, but it would be really nice someone tested complete sereies. Best regards, Pavel
On Tue, 22 Sep 2020 18:38:42 +0200 Pavel Machek <pavel@ucw.cz> wrote: > On Sun 2020-09-20 23:54:36, Marek Behun wrote: > > On Sun, 20 Sep 2020 23:45:32 +0200 > > Pavel Machek <pavel@ucw.cz> wrote: > > > > > Hi! > > > > > > > Now that the potential use-after-free issue is resolved we can use > > > > devres for LED registration in this driver. > > > > > > > > By using devres version of LED registering function we can remove the > > > > .remove method from this driver. > > > > > > > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > > > > Cc: Dan Murphy <dmurphy@ti.com> > > > > > > AFAICT this one is buggy, I sent explanation before. Why are you > > > resubmitting it? > > > > The previous patch in this series (v3 5/9) should solve this issue and > > th commit message explains how. > > Aha, let me see. > > Will 5/9 have some side-effects, like device appearing at different > place in sysfs? Yes, unfortunately. Before this path the led should be in /sys/devices/.../i2c-client/leds/led or somthing like that, and after /sys/devices/..c/i2c-client/mfd/leds/led But it should have been this way from beginning, I think. The other driver, regulator, registers its device under the mfd device. The question is whether this will break something for someone. I don't think so, but... > > First few patches look ok, but it would be really nice someone tested > complete sereies. > > Best regards, > Pavel
Pavel On 9/22/20 11:58 AM, Marek Behun wrote: > On Tue, 22 Sep 2020 18:38:42 +0200 > Pavel Machek <pavel@ucw.cz> wrote: > >> On Sun 2020-09-20 23:54:36, Marek Behun wrote: >>> On Sun, 20 Sep 2020 23:45:32 +0200 >>> Pavel Machek <pavel@ucw.cz> wrote: >>> >>>> Hi! >>>> >>>>> Now that the potential use-after-free issue is resolved we can use >>>>> devres for LED registration in this driver. >>>>> >>>>> By using devres version of LED registering function we can remove the >>>>> .remove method from this driver. >>>>> >>>>> Signed-off-by: Marek Behún <marek.behun@nic.cz> >>>>> Cc: Dan Murphy <dmurphy@ti.com> >>>> AFAICT this one is buggy, I sent explanation before. Why are you >>>> resubmitting it? >>> The previous patch in this series (v3 5/9) should solve this issue and >>> th commit message explains how. >> Aha, let me see. >> >> Will 5/9 have some side-effects, like device appearing at different >> place in sysfs? > Yes, unfortunately. Before this path the led should be in > /sys/devices/.../i2c-client/leds/led > or somthing like that, and after > /sys/devices/..c/i2c-client/mfd/leds/led > > But it should have been this way from beginning, I think. The other > driver, regulator, registers its device under the mfd device. > > The question is whether this will break something for someone. I don't > think so, but... > >> First few patches look ok, but it would be really nice someone tested >> complete sereies. For the LM36274 patches Tested-by: Dan Murphy <dmurphy@ti.com> Also found some other legacy issues I sent patches for. Dan
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c index 74c236d1a60c8..10a63b7f2ecce 100644 --- a/drivers/leds/leds-lm36274.c +++ b/drivers/leds/leds-lm36274.c @@ -142,7 +142,8 @@ static int lm36274_probe(struct platform_device *pdev) chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT; chip->led_dev.brightness_set_blocking = lm36274_brightness_set; - ret = led_classdev_register_ext(chip->dev, &chip->led_dev, &init_data); + ret = devm_led_classdev_register_ext(chip->dev, &chip->led_dev, + &init_data); if (ret) dev_err(chip->dev, "Failed to register LED for node %pfw\n", init_data.fwnode); @@ -152,15 +153,6 @@ static int lm36274_probe(struct platform_device *pdev) return ret; } -static int lm36274_remove(struct platform_device *pdev) -{ - struct lm36274 *chip = platform_get_drvdata(pdev); - - led_classdev_unregister(&chip->led_dev); - - return 0; -} - static const struct of_device_id of_lm36274_leds_match[] = { { .compatible = "ti,lm36274-backlight", }, {}, @@ -169,7 +161,6 @@ MODULE_DEVICE_TABLE(of, of_lm36274_leds_match); static struct platform_driver lm36274_driver = { .probe = lm36274_probe, - .remove = lm36274_remove, .driver = { .name = "lm36274-leds", },
Now that the potential use-after-free issue is resolved we can use devres for LED registration in this driver. By using devres version of LED registering function we can remove the .remove method from this driver. Signed-off-by: Marek Behún <marek.behun@nic.cz> Cc: Dan Murphy <dmurphy@ti.com> --- drivers/leds/leds-lm36274.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)