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.
So preserving the status quo looks more future-proof IMO.
--
Alexander Sverdlin
Siemens AG
www.siemens.com
