Hi Rob, On 15/10/15 16:46, Rob Herring wrote: >> At some point I got feedback that the DT maintainers don't have time to >> look at each individual driver binding, but rely on the subsystem >> maintainers to handle them. Maybe I misunderstood that. > > True, but that doesn't mean to not copy us. If we didn't want to be > copied, we would update MAINTAINERS. Ok. >>> Why do we need 2 levels of LED nodes? >> >> Sorry, didn't get that. What do you mean with 2 levels? > > You have the node the "leds" phandle points to which is the actual LED > device and then this node which is just backlight properties. And then > presumably another phandle in the panel device to point to the > backlight device. Ok, I see what you mean. Well, I have to say this is far from perfect. I initially pushed for a PWM driver for the LED chip we use (tlc591xx), which would have allowed us to use pwm-backlight driver. But Andrew was using the same chip for more LED-ish use cases, for which a LED driver was more suitable. But what I think we really should have is a more generic way to represent output pins, so that GPIOs (well, GPOs really), PWMs and current controlled outputs would all be done the same way. It was rather difficult to use the LED driver and LED bindings for this, as (afaics) they were really never designed to be used for anything else than for simple LEDs (i.e. a LED connected directly to the LED chip). The flash support was added later, but that's almost as simple as the first case. >> These are for the backlight, not for the LED chip. So "LED" here is a >> chip that produces (most likely) a PWM signal, and "backlight" is the >> collection of components that use the PWM to produce the backlight >> itself, and use the power-supply and gpios. > > Okay, it wasn't clear that leds points to the LED controller node. The No, it doesn't point to the main LED node (the one having 'compatible'). It points to a child node. > example made it seem as it was the device. We already have a way to > describe LEDs and that is as child nodes of the LED controller node. True, but those child nodes are very limited. As I see it, those child nodes really describe the outputs of the LED chip, not what's on the other end of the lines. If on the other end of the lines is a more complex device, we need a proper device driver for it, with a proper DT node with compatible property etc. Now, one could argue that a "backlight" that gets the LED signal from a LED chip is really just a simple LED. But there are complications: - Our board needs a GPIO to enable the backlight. I can't say what exactly the GPIO does as my HW skills don't go far enough, but all this is after the LED chip. I also see the circuitry using powers, which in our case happen to be always on so we don't need to enable them explicitly. - We need a backlight device/driver (because of the Linux SW stack). So, maybe it would be possible to construct all that in a LED child node, and the LED driver would create a child device for the nodes which have 'compatible' property. But then, that would be very different from pwm-backlight, and the parent-child relationships are usually used to indicate a control relationship, right? The led-backlight in these patches is very much similar to pwm-backlight. > Please follow what was done for flash LEDs (leds/common.txt). The flash support is quite simple. I'm not sure how I could do the same for the backlight, as I described above. > What's wrong with the existing pwm-backlight binding in the PWM case? Nothing, if there's a PWM driver. But if the LED chip is modelled as a LED driver, pwm-backlight is out. I think there are two kinds of LED chips, PWM ones and current-controlling ones. And then there are the PWM devices which are clearly PWM ones. >>> Describe the h/w, not what you want for a driver. >> >> I think this describes the HW quite well. The LED chip works fine >> without any of the properties here, and these are specific to the >> backlight part of the board. > > A more complete example would be helpful here. Of our HW? I can't give the schematics but I hope I described it enough above. Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
