On 12/9/24 8:35 AM, Sverdlin, Alexander wrote:
Hi Andrew,
thanks for the prompt review!
On Mon, 2024-12-09 at 08:29 -0600, Andrew Davis wrote:
+ reg:
+ maxItems: 1
+ description: I2C slave address
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ enable-gpios:
+ maxItems: 1
+ description: GPIO pin to enable (active high) / disable the device
+
+ vled-supply:
+ description: LED supply
+
+patternProperties:
+ "^led@[0]$":
+ type: object
+ $ref: common.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ description:
+ Index of the LED.
+ const: 0
+
+ function: true
+ color: true
+ label: true
+ linux,default-trigger: true
+
+ required:
+ - reg
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/leds/common.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led-controller@2d {
+ compatible = "ti,lp8860";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x2d>;
+ enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+ vled-supply = <&vbatt>;
+
+ led@0 {
So same comment I made in the pre-public review, lets see what the DT
folks think:
I don't think we want to have the "@0" node naming. It forces us to
add the "reg =" below, and that then forces us to add the #*-cells above.
All this to work around not just calling the node "led-0". The driver
doesn't care either way, and there are no in-tree users of the old way,
so now should be a safe time to fix this while converting the binding.
If I understood you correctly here:
And one channel controlling the others is only in this "Display Mode",
but the currents to the others can be independently controlled in a
different mode (seems these modes have less features which is probably
why the driver doesn't make use of that today).
then some mapping between subnode and HW channel would be required.
We probably don't want to parse a node name in this case and carve out "-0"
part of it, in such case a well-defined property, such as "reg" would be
required.
We are not the only driver to do the "-#" method. Although yes just
using reg is easier for the Linux driver.
So preserving the status quo looks more future-proof IMO.
There will only ever be up to 4 LEDs, but you give fair points. No
strong preference from me then, and everything else looks good to me,
Acked-by: Andrew Davis <afd@xxxxxx>
