On 7/13/20 5:06 PM, Rob Herring wrote:
On Mon, Jul 13, 2020 at 12:11:51AM +0200, Jacek Anaszewski wrote:
On 7/12/20 11:40 PM, Marek Behun wrote:
On Sun, 12 Jul 2020 23:27:07 +0200
Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx> wrote:
+ multi-led@0 {
+ reg = <0>;
+ color = <LED_COLOR_ID_MULTI>;
+ function = LED_FUNCTION_POWER;
Please provide child nodes for each color LED. Let's stick
to the bindings closely and not make any deviations from
the beginning.
Why? It would make sense if there were devices using this controller
having other configuration, but on Omnia, all LEDs are RGB.
Also, if I do this, should I also make the driver check in the probe
function whether the per-channel child nodes are correct? Eg. if they
are always three: one for red, one for green and one for blue? Or
should the driver ignore this and only the device tree binding specify
it?
Because the way the driver is written now, it only registers
multi-color RGB LEDs.
This is not RGB framework, but multicolor framework. It is not justified
to pretend that RGB is default. Unless you would state that clearly in
the comment in DT, but that should be agreed upon with Rob.
If the LEDs are fixed in h/w and never vary for this controller, then
they don't need to be in DT. However, is it really possible that a
channel only supports 1 color of LED? I don't think so.
Theoretically we could allow for registering two LEDs as LED multi
color device and the other one as monochrome LED class device
but I admit that this would be a bit convoluted and entail unnecessary
complexity in the driver.
So I'm OK with no child nodes for these bindings, but let's add some
comment justifying that.
--
Best regards,
Jacek Anaszewski
