On 2 March 2017 at 21:29, Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx> wrote: > On 03/02/2017 06:42 PM, Rafał Miłecki wrote: >> On 28 February 2017 at 22:20, Jacek Anaszewski >> <jacek.anaszewski@xxxxxxxxx> wrote: >>> Thanks for the patch. How about following what has been already >>> done for spi, i.e. adding a wrapper of_led_classdev_register() >>> (please refer to of_register_spi_device() in drivers/spi/spi.c)? >> >> I like this idea, but I'll need to split led_classdev_register into >> two functions: allocation & actual registering function. That way I >> can alloc, then read DT proprties and finally register LED (just like >> it's done in of_register_spi_device). >> >> Does it sound OK? > > Yes, after thinking a bit more about that I came to the same > conclusion. OK, I spent some extra time on this today, this seem a bit more complex. I was starring at of_register_spi_device for quite some time and it doesn't seem really related to this situation. In case of of_register_spi_device: 1) It's used internally in spi core code only 2) It handles DT controller child node totally on its own This patch I sent is about interaction between drivers registering LEDs and LED core code. There are few drivers (that register LEDs) parsing DT on their own. I'm afraid comparing this situation to of_register_spi_device is a bit confusing for me. I really don't see a place for of_led_classdev_register that would be called for every child of LED controller (?!). So I'd like to suggest not comparing this situation to of_register_spi_device and discussing this problem on our own. My current problem is that led_classdev_register: 1) Allocates struct device & registers LED 2) It means drivers can't touch struct device before having LED registered 3) It doesn't allow passing of_node 4) It is used in so many places it's almost impossible to modify all callers at once So maybe I could work on of_led_classdev_register that: 1) Would be almost identical to led_classdev_register 2) It would accept extra struct device_node argument Does it make sense? Or maybe it's even what you meant from the beginning? Two things I'll need to take in mind while working on this: 1) I'll need devm variant 2) I should share code between (of_)led_classdev_register functions -- Rafał
