| Message ID | 20221216113013.126881-3-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | leds: lookup-table support + int3472/media privacy LED support | expand |
On Fri, Dec 16, 2022 at 12:30:04PM +0100, Hans de Goede wrote: > Split out part of of_led_get() into a generic __led_get() helper function. > > This is a preparation patch for adding a generic (non devicetree specific) > led_get() function. ... > +static struct led_classdev *__led_get(struct device *led_dev) > +{ > + struct led_classdev *led_cdev; > + > + if (!led_dev) > + return ERR_PTR(-EPROBE_DEFER); > + > + led_cdev = dev_get_drvdata(led_dev); > + > + if (!try_module_get(led_cdev->dev->parent->driver->owner)) { > + put_device(led_cdev->dev); > + return ERR_PTR(-ENODEV); > + } > + > + return led_cdev; > +} If I'm not mistaken, the entire series leaves this function as is. At the end I found three _get functions: __led_get led_get __devm_led_get As far as I can see the above can be named more precisely, i.e. led_module_get(). Or alike if this sounds not good either.
Hi, On 12/16/22 14:54, Andy Shevchenko wrote: > On Fri, Dec 16, 2022 at 12:30:04PM +0100, Hans de Goede wrote: >> Split out part of of_led_get() into a generic __led_get() helper function. >> >> This is a preparation patch for adding a generic (non devicetree specific) >> led_get() function. > > ... > >> +static struct led_classdev *__led_get(struct device *led_dev) >> +{ >> + struct led_classdev *led_cdev; >> + >> + if (!led_dev) >> + return ERR_PTR(-EPROBE_DEFER); >> + >> + led_cdev = dev_get_drvdata(led_dev); >> + >> + if (!try_module_get(led_cdev->dev->parent->driver->owner)) { >> + put_device(led_cdev->dev); >> + return ERR_PTR(-ENODEV); >> + } >> + >> + return led_cdev; >> +} > > If I'm not mistaken, the entire series leaves this function as is. > At the end I found three _get functions: > __led_get > led_get > __devm_led_get > > > As far as I can see the above can be named more precisely, i.e. > led_module_get(). Or alike if this sounds not good either. led_module_get() sounds good, I will rename this for v4. Regards, Hans
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index 7391d2cf1370..5f40110bd260 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -215,6 +215,23 @@ static int led_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(leds_class_dev_pm_ops, led_suspend, led_resume); +static struct led_classdev *__led_get(struct device *led_dev) +{ + struct led_classdev *led_cdev; + + if (!led_dev) + return ERR_PTR(-EPROBE_DEFER); + + led_cdev = dev_get_drvdata(led_dev); + + if (!try_module_get(led_cdev->dev->parent->driver->owner)) { + put_device(led_cdev->dev); + return ERR_PTR(-ENODEV); + } + + return led_cdev; +} + /** * of_led_get() - request a LED device via the LED framework * @np: device node to get the LED device from @@ -226,7 +243,6 @@ static SIMPLE_DEV_PM_OPS(leds_class_dev_pm_ops, led_suspend, led_resume); struct led_classdev *of_led_get(struct device_node *np, int index) { struct device *led_dev; - struct led_classdev *led_cdev; struct device_node *led_node; led_node = of_parse_phandle(np, "leds", index); @@ -236,17 +252,7 @@ struct led_classdev *of_led_get(struct device_node *np, int index) led_dev = class_find_device_by_of_node(leds_class, led_node); of_node_put(led_node); - if (!led_dev) - return ERR_PTR(-EPROBE_DEFER); - - led_cdev = dev_get_drvdata(led_dev); - - if (!try_module_get(led_cdev->dev->parent->driver->owner)) { - put_device(led_cdev->dev); - return ERR_PTR(-ENODEV); - } - - return led_cdev; + return __led_get(led_dev); } EXPORT_SYMBOL_GPL(of_led_get);
Split out part of of_led_get() into a generic __led_get() helper function. This is a preparation patch for adding a generic (non devicetree specific) led_get() function. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/leds/led-class.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)