| Message ID | 1441711176-4258-2-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomi, Thanks for the update. On 09/08/2015 01:19 PM, Tomi Valkeinen wrote: > This patch adds basic support for a kernel driver to get a LED device. > This will be used by the led-backlight driver. > > Only OF version is implemented for now, and the behavior is similar to > PWM's of_pwm_get() and pwm_put(). > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/leds/Makefile | 6 +++- > drivers/leds/led-class.c | 13 +++++++- > drivers/leds/led-of.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++ > drivers/leds/leds.h | 1 + > include/linux/leds-of.h | 26 +++++++++++++++ According to existing naming convention this should be "of_leds.h". > include/linux/leds.h | 2 ++ > 6 files changed, 128 insertions(+), 2 deletions(-) > create mode 100644 drivers/leds/led-of.c > create mode 100644 include/linux/leds-of.h > > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 8d6a24a2f513..6fd22e411810 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -1,7 +1,11 @@ > > # LED Core > obj-$(CONFIG_NEW_LEDS) += led-core.o > -obj-$(CONFIG_LEDS_CLASS) += led-class.o > + > +obj-$(CONFIG_LEDS_CLASS) += led-class-objs.o > +led-class-objs-y := led-class.o > +led-class-objs-$(CONFIG_OF) += led-of.o > + > obj-$(CONFIG_LEDS_CLASS_FLASH) += led-class-flash.o > obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o > > diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c > index beabfbc6f7cd..1234f9dc3537 100644 > --- a/drivers/leds/led-class.c > +++ b/drivers/leds/led-class.c > @@ -22,7 +22,7 @@ > #include <linux/timer.h> > #include "leds.h" > > -static struct class *leds_class; > +struct class *leds_class; > > static ssize_t brightness_show(struct device *dev, > struct device_attribute *attr, char *buf) > @@ -216,6 +216,17 @@ static int led_resume(struct device *dev) > > static SIMPLE_DEV_PM_OPS(leds_class_dev_pm_ops, led_suspend, led_resume); > > +/** > + * led_put() - release a LED device, reserved with led_get() > + * @led_cdev: LED device > + */ > +void led_put(struct led_classdev *led_cdev) > +{ > + put_device(led_cdev->dev); > + module_put(led_cdev->dev->parent->driver->owner); > +} > +EXPORT_SYMBOL_GPL(led_put); > + > static int match_name(struct device *dev, const void *data) > { > if (!dev_name(dev)) > diff --git a/drivers/leds/led-of.c b/drivers/leds/led-of.c > new file mode 100644 > index 000000000000..32631682be07 > --- /dev/null > +++ b/drivers/leds/led-of.c > @@ -0,0 +1,82 @@ > +/* > + * LED Class Core OF support > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/leds.h> > +#include <linux/of.h> > +#include <linux/leds-of.h> Please keep alphabetical order. > +#include <linux/module.h> > + > +#include "leds.h" > + > +/* find OF node for the given led_cdev */ > +static struct device_node *find_led_of_node(struct led_classdev *led_cdev) > +{ > + struct device *led_dev = led_cdev->dev; > + struct device_node *child; > + > + for_each_child_of_node(led_dev->parent->of_node, child) { > + if (of_property_match_string(child, "label", led_cdev->name) == 0) Line over 80 characters. > + return child; > + } > + > + return NULL; > +} > + > +static int led_match_led_node(struct device *led_dev, const void *data) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(led_dev); > + const struct device_node *target_node = data; > + struct device_node *led_node; > + > + led_node = find_led_of_node(led_cdev); > + if (!led_node) > + return 0; > + > + of_node_put(led_node); > + > + return led_node == target_node ? 1 : 0; return led_node == target_node; > +} > + > +/** > + * of_led_get() - request a LED device via the LED framework > + * @np: device node to get the LED device from > + * > + * Returns the LED device parsed from the phandle specified in the "leds" > + * property of a device tree node or a negative error-code on failure. > + * > + * The caller must use led_put() to release the device after use. > + */ > +struct led_classdev *of_led_get(struct device_node *np) > +{ > + struct device *led_dev; > + struct led_classdev *led_cdev; > + struct device_node *led_node; > + > + led_node = of_parse_phandle(np, "leds", 0); > + if (!led_node) > + return ERR_PTR(-ENODEV); > + > + led_dev = class_find_device(leds_class, NULL, led_node, > + led_match_led_node); > + > + of_node_put(led_node); > + > + if (!led_dev) { > + pr_err("failed to find led device for node %s, deferring probe\n", > + of_node_full_name(led_node)); > + return ERR_PTR(-EPROBE_DEFER); > + } > + > + led_cdev = dev_get_drvdata(led_dev); > + > + if (!try_module_get(led_cdev->dev->parent->driver->owner)) > + return ERR_PTR(-ENODEV); > + > + return led_cdev; > +} > +EXPORT_SYMBOL_GPL(of_led_get); > diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h > index bc89d7ace2c4..ccc3abb417d4 100644 > --- a/drivers/leds/leds.h > +++ b/drivers/leds/leds.h > @@ -46,6 +46,7 @@ static inline int led_get_brightness(struct led_classdev *led_cdev) > > void led_stop_software_blink(struct led_classdev *led_cdev); > > +extern struct class *leds_class; > extern struct rw_semaphore leds_list_lock; > extern struct list_head leds_list; > > diff --git a/include/linux/leds-of.h b/include/linux/leds-of.h > new file mode 100644 > index 000000000000..7e8e64bd9811 > --- /dev/null > +++ b/include/linux/leds-of.h > @@ -0,0 +1,26 @@ > +/* > + * OF support for leds > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + */ > + > +#ifndef __LINUX_LEDS_OF_H_INCLUDED > +#define __LINUX_LEDS_OF_H_INCLUDED > + > +#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_LEDS_CLASS) > + > +extern struct led_classdev *of_led_get(struct device_node *np); > + > +#else > + > +static inline struct led_classdev *of_led_get(struct device_node *np) > +{ > + return -ENODEV; > +} > + > +#endif > + > +#endif /* __LINUX_LEDS_OF_H_INCLUDED */ > diff --git a/include/linux/leds.h b/include/linux/leds.h > index b122eeafb5dc..0fce71a06d68 100644 > --- a/include/linux/leds.h > +++ b/include/linux/leds.h > @@ -113,6 +113,8 @@ extern void devm_led_classdev_unregister(struct device *parent, > extern void led_classdev_suspend(struct led_classdev *led_cdev); > extern void led_classdev_resume(struct led_classdev *led_cdev); > > +extern void led_put(struct led_classdev *led_cdev); > + > /** > * led_blink_set - set blinking with software fallback > * @led_cdev: the LED to start blinking >
On 09/08/2015 01:19 PM, Tomi Valkeinen wrote: >> This patch adds basic support for a kernel driver to get a LED device. >> This will be used by the led-backlight driver. >> >> Only OF version is implemented for now, and the behavior is similar to >> PWM's of_pwm_get() and pwm_put(). >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> drivers/leds/Makefile | 6 +++- >> drivers/leds/led-class.c | 13 +++++++- >> drivers/leds/led-of.c | 82 >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> drivers/leds/leds.h | 1 + >> include/linux/leds-of.h | 26 +++++++++++++++ [...] >> diff --git a/include/linux/leds-of.h b/include/linux/leds-of.h >> new file mode 100644 >> index 000000000000..7e8e64bd9811 >> --- /dev/null >> +++ b/include/linux/leds-of.h >> @@ -0,0 +1,26 @@ >> +/* >> + * OF support for leds >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + */ >> + >> +#ifndef __LINUX_LEDS_OF_H_INCLUDED >> +#define __LINUX_LEDS_OF_H_INCLUDED >> + >> +#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_LEDS_CLASS) >> + >> +extern struct led_classdev *of_led_get(struct device_node *np); >> + >> +#else >> + >> +static inline struct led_classdev *of_led_get(struct device_node *np) >> +{ >> + return -ENODEV; >> +} >> + >> +#endif >> + >> +#endif /* __LINUX_LEDS_OF_H_INCLUDED */ >> diff --git a/include/linux/leds.h b/include/linux/leds.h >> index b122eeafb5dc..0fce71a06d68 100644 >> --- a/include/linux/leds.h >> +++ b/include/linux/leds.h >> @@ -113,6 +113,8 @@ extern void devm_led_classdev_unregister(struct >> device *parent, >> extern void led_classdev_suspend(struct led_classdev *led_cdev); >> extern void led_classdev_resume(struct led_classdev *led_cdev); >> >> +extern void led_put(struct led_classdev *led_cdev); >> + This also needs no-op version. >> /** >> * led_blink_set - set blinking with software fallback >> * @led_cdev: the LED to start blinking >> > >
On 08/09/15 16:20, Jacek Anaszewski wrote: > Hi Tomi, > > Thanks for the update. > > On 09/08/2015 01:19 PM, Tomi Valkeinen wrote: >> This patch adds basic support for a kernel driver to get a LED device. >> This will be used by the led-backlight driver. >> >> Only OF version is implemented for now, and the behavior is similar to >> PWM's of_pwm_get() and pwm_put(). >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> drivers/leds/Makefile | 6 +++- >> drivers/leds/led-class.c | 13 +++++++- >> drivers/leds/led-of.c | 82 >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> drivers/leds/leds.h | 1 + >> include/linux/leds-of.h | 26 +++++++++++++++ > > According to existing naming convention this should be "of_leds.h". Right. I was thinking it's "leds" first, and "of" second, but I see of_*.h is the convention. >> +#include <linux/leds.h> >> +#include <linux/of.h> >> +#include <linux/leds-of.h> > > Please keep alphabetical order. Yep. >> +#include <linux/module.h> >> + >> +#include "leds.h" >> + >> +/* find OF node for the given led_cdev */ >> +static struct device_node *find_led_of_node(struct led_classdev >> *led_cdev) >> +{ >> + struct device *led_dev = led_cdev->dev; >> + struct device_node *child; >> + >> + for_each_child_of_node(led_dev->parent->of_node, child) { >> + if (of_property_match_string(child, "label", led_cdev->name) >> == 0) > > Line over 80 characters. I don't like to split lines to exact 80 chars, when it makes the code more difficult to read. In this case it's 3 chars over 80, and splitting the function call above to two lines doesn't look nice to me. I'll do the func call separately, then it stays under 80 chars. >> + return child; >> + } >> + >> + return NULL; >> +} >> + >> +static int led_match_led_node(struct device *led_dev, const void *data) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(led_dev); >> + const struct device_node *target_node = data; >> + struct device_node *led_node; >> + >> + led_node = find_led_of_node(led_cdev); >> + if (!led_node) >> + return 0; >> + >> + of_node_put(led_node); >> + >> + return led_node == target_node ? 1 : 0; > > return led_node == target_node; Again a matter of taste, but to me, == returns a bool, whereas the match function here returns an int. But I'm fine with plain == here. Tomi
On 08/09/15 17:04, Jacek Anaszewski wrote: >>> +#endif /* __LINUX_LEDS_OF_H_INCLUDED */ >>> diff --git a/include/linux/leds.h b/include/linux/leds.h >>> index b122eeafb5dc..0fce71a06d68 100644 >>> --- a/include/linux/leds.h >>> +++ b/include/linux/leds.h >>> @@ -113,6 +113,8 @@ extern void devm_led_classdev_unregister(struct >>> device *parent, >>> extern void led_classdev_suspend(struct led_classdev *led_cdev); >>> extern void led_classdev_resume(struct led_classdev *led_cdev); >>> >>> +extern void led_put(struct led_classdev *led_cdev); >>> + > > This also needs no-op version. Ok, but... I think other already existing functions need no-ops also. If there's a driver that uses of_led_get and led_put, it's sure to use some other led_* functions also. So if we want that driver to be compilable when LED support is disabled in the kernel, we need to provide no-ops for all those functions. Probably: led_set_brightness led_blink_set_oneshot led_blink_set Tomi
On 09/09/2015 02:16 PM, Tomi Valkeinen wrote: > > > On 08/09/15 17:04, Jacek Anaszewski wrote: > >>>> +#endif /* __LINUX_LEDS_OF_H_INCLUDED */ >>>> diff --git a/include/linux/leds.h b/include/linux/leds.h >>>> index b122eeafb5dc..0fce71a06d68 100644 >>>> --- a/include/linux/leds.h >>>> +++ b/include/linux/leds.h >>>> @@ -113,6 +113,8 @@ extern void devm_led_classdev_unregister(struct >>>> device *parent, >>>> extern void led_classdev_suspend(struct led_classdev *led_cdev); >>>> extern void led_classdev_resume(struct led_classdev *led_cdev); >>>> >>>> +extern void led_put(struct led_classdev *led_cdev); >>>> + >> >> This also needs no-op version. > > Ok, but... I think other already existing functions need no-ops also. If > there's a driver that uses of_led_get and led_put, it's sure to use some > other led_* functions also. > > So if we want that driver to be compilable when LED support is disabled > in the kernel, we need to provide no-ops for all those functions. > > Probably: > > led_set_brightness > led_blink_set_oneshot > led_blink_set That's right. It needs to be addressed soon too. Potentially this could show up by breaking randconfig build.
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 8d6a24a2f513..6fd22e411810 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -1,7 +1,11 @@ # LED Core obj-$(CONFIG_NEW_LEDS) += led-core.o -obj-$(CONFIG_LEDS_CLASS) += led-class.o + +obj-$(CONFIG_LEDS_CLASS) += led-class-objs.o +led-class-objs-y := led-class.o +led-class-objs-$(CONFIG_OF) += led-of.o + obj-$(CONFIG_LEDS_CLASS_FLASH) += led-class-flash.o obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index beabfbc6f7cd..1234f9dc3537 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -22,7 +22,7 @@ #include <linux/timer.h> #include "leds.h" -static struct class *leds_class; +struct class *leds_class; static ssize_t brightness_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -216,6 +216,17 @@ static int led_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(leds_class_dev_pm_ops, led_suspend, led_resume); +/** + * led_put() - release a LED device, reserved with led_get() + * @led_cdev: LED device + */ +void led_put(struct led_classdev *led_cdev) +{ + put_device(led_cdev->dev); + module_put(led_cdev->dev->parent->driver->owner); +} +EXPORT_SYMBOL_GPL(led_put); + static int match_name(struct device *dev, const void *data) { if (!dev_name(dev)) diff --git a/drivers/leds/led-of.c b/drivers/leds/led-of.c new file mode 100644 index 000000000000..32631682be07 --- /dev/null +++ b/drivers/leds/led-of.c @@ -0,0 +1,82 @@ +/* + * LED Class Core OF support + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/leds.h> +#include <linux/of.h> +#include <linux/leds-of.h> +#include <linux/module.h> + +#include "leds.h" + +/* find OF node for the given led_cdev */ +static struct device_node *find_led_of_node(struct led_classdev *led_cdev) +{ + struct device *led_dev = led_cdev->dev; + struct device_node *child; + + for_each_child_of_node(led_dev->parent->of_node, child) { + if (of_property_match_string(child, "label", led_cdev->name) == 0) + return child; + } + + return NULL; +} + +static int led_match_led_node(struct device *led_dev, const void *data) +{ + struct led_classdev *led_cdev = dev_get_drvdata(led_dev); + const struct device_node *target_node = data; + struct device_node *led_node; + + led_node = find_led_of_node(led_cdev); + if (!led_node) + return 0; + + of_node_put(led_node); + + return led_node == target_node ? 1 : 0; +} + +/** + * of_led_get() - request a LED device via the LED framework + * @np: device node to get the LED device from + * + * Returns the LED device parsed from the phandle specified in the "leds" + * property of a device tree node or a negative error-code on failure. + * + * The caller must use led_put() to release the device after use. + */ +struct led_classdev *of_led_get(struct device_node *np) +{ + struct device *led_dev; + struct led_classdev *led_cdev; + struct device_node *led_node; + + led_node = of_parse_phandle(np, "leds", 0); + if (!led_node) + return ERR_PTR(-ENODEV); + + led_dev = class_find_device(leds_class, NULL, led_node, + led_match_led_node); + + of_node_put(led_node); + + if (!led_dev) { + pr_err("failed to find led device for node %s, deferring probe\n", + of_node_full_name(led_node)); + return ERR_PTR(-EPROBE_DEFER); + } + + led_cdev = dev_get_drvdata(led_dev); + + if (!try_module_get(led_cdev->dev->parent->driver->owner)) + return ERR_PTR(-ENODEV); + + return led_cdev; +} +EXPORT_SYMBOL_GPL(of_led_get); diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h index bc89d7ace2c4..ccc3abb417d4 100644 --- a/drivers/leds/leds.h +++ b/drivers/leds/leds.h @@ -46,6 +46,7 @@ static inline int led_get_brightness(struct led_classdev *led_cdev) void led_stop_software_blink(struct led_classdev *led_cdev); +extern struct class *leds_class; extern struct rw_semaphore leds_list_lock; extern struct list_head leds_list; diff --git a/include/linux/leds-of.h b/include/linux/leds-of.h new file mode 100644 index 000000000000..7e8e64bd9811 --- /dev/null +++ b/include/linux/leds-of.h @@ -0,0 +1,26 @@ +/* + * OF support for leds + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#ifndef __LINUX_LEDS_OF_H_INCLUDED +#define __LINUX_LEDS_OF_H_INCLUDED + +#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_LEDS_CLASS) + +extern struct led_classdev *of_led_get(struct device_node *np); + +#else + +static inline struct led_classdev *of_led_get(struct device_node *np) +{ + return -ENODEV; +} + +#endif + +#endif /* __LINUX_LEDS_OF_H_INCLUDED */ diff --git a/include/linux/leds.h b/include/linux/leds.h index b122eeafb5dc..0fce71a06d68 100644 --- a/include/linux/leds.h +++ b/include/linux/leds.h @@ -113,6 +113,8 @@ extern void devm_led_classdev_unregister(struct device *parent, extern void led_classdev_suspend(struct led_classdev *led_cdev); extern void led_classdev_resume(struct led_classdev *led_cdev); +extern void led_put(struct led_classdev *led_cdev); + /** * led_blink_set - set blinking with software fallback * @led_cdev: the LED to start blinking
This patch adds basic support for a kernel driver to get a LED device. This will be used by the led-backlight driver. Only OF version is implemented for now, and the behavior is similar to PWM's of_pwm_get() and pwm_put(). Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/leds/Makefile | 6 +++- drivers/leds/led-class.c | 13 +++++++- drivers/leds/led-of.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/leds/leds.h | 1 + include/linux/leds-of.h | 26 +++++++++++++++ include/linux/leds.h | 2 ++ 6 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 drivers/leds/led-of.c create mode 100644 include/linux/leds-of.h