On 09/08/2015 12:41 PM, Tomi Valkeinen wrote:
On 08/09/15 12:21, Jacek Anaszewski wrote:
The "static struct class *leds_class" from led-class.c, in one way or
another. of_led_get() needs to go through the led devices from the class.
For now I just removed the "static" from it, so that I can use it from
of.c.
I think we can go in this direction. I've skimmed through existing
class drivers and found similar examples (e.g. tty_class, rtc_class).
Yep.
Sorry, I didn't get that one. How does the backlight driver's
depend/select affect this?
OK, I confused something here. Backlight driver should depend on
LEDS_CLASS by defining "depends on LEDS_CLASS" in backlight Kconfig.
It should also depend on OF. In case of this driver the no-ops would be
only for the purpose of making the kernel image smaller, as the driver
probe will fail without them anyway. Nevertheless, there might be added
other drivers in the future, using of_led_get{put} API which would like
to make some decisions basing on whether LEDS_CLASS or/and OF are
turned on. No-ops would be of use then.
I agree.
What do you mean with "waste"?
I used 'waste' because we would be wasting time here for the call which
can be avoided at no cost. Of course if compiler will decide to inline
it.
In this case there's no need to get more performance by inlining
Why so?
Reserving and freeing resources are rarely hot paths. The functions in
question are usually called in a driver's probe and remove. Saving a few
CPU cycles there doesn't really matter, so I think readability is the
important part here.
OK, I agree.
and inlining forces the users of led_put to include module.h to compile.
We could include module.h from leds.h.
Yes we can. And I can do that in my patch. I don't agree with the
solution but neither do I really have a problem with it =).
Please go ahead as you originally planned, i.e. without inlining.
--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
