Marek
On 10/6/20 9:41 AM, Marek Behun wrote:
Adding Rob to Cc, Rob, could we have your opinion on this? Mine is below.
Dan, I looked at the datasheet, I understand this.
Nonetheless, device tree should describe how devices are connected to
each other. The chip has 3 pins for 3 LED strings.
If this controller should be able to support 3 LED strings via 3
outputs, the device tree binding nodes should, in my opinion, describe
each pin connected string. The nodes should maybe even be called
'led-string@N' where N is from [0, 1, 2].
The fact that the device is bank controlled and there are only two
banks (and it is configurable by which bank each LED string is
controlled) is more relevant to the driver, not as much to device tree
binding.
But yes, I do realize that if we had 3 child nodes, and the driver
created 3 LEDs, then changing brithrness on one of the 3 LEDs would
change brightness on at least another one, which we do not want.
Maybe this driver could parse these 3 `led-string` nodes, each having
defined bank via `led-sources`, and then register LED classdevs for
each bank that is mentioned. This way the device tree would be more
correct, IMO, and the driver would not have the problem mentioned in
the paragraph above.
Unfortunately we cannot and should not change the ABI now.
Using the led-sources as the bank indicator does not conform to the
definition of the description of the led-sources in the yaml.
The preference was to use the led-sources to define the LED out to the bank.
Here is the conversation on how the driver got to where it is.
https://lore.kernel.org/patchwork/patch/972337/
Dan
