boards: thingy52_nrf52832: Update gpio-leds binding

[MarekPieta]

Change updates gpio-leds DTS binding. The colors should be defined in RGB order.

[pdunaj]

I don't mind the change but some applications may actually depend on the old order.

[MarekPieta]

Yes, I am aware of this. Still I think that we should consistently use RGB order for all the boards (otherwise porting may cause troubles).

[anangl]

I'm not convinced about the change. We've got more boards in the tree with the LED order different than RGB, see for instance nRF52840 dongle. And as @pdunaj already signaled, this change might affect existing applications - some may blink in red now instead of blinking in green (led0 changes from green to red). I'm not sure this is worth it.

[MarekPieta]

@anangl, on the dongle the colors of a single LED are in order. There is only added a first channel of other monochromatic LED before them.
Maybe it would be good to make a single physical LED with all of the channels represented as a separate DTS node?

[pdunaj]

I'm not convinced about the change. We've got more boards in the tree with the LED order different than RGB, see for instance nRF52840 dongle. And as @pdunaj already signaled, this change might affect existing applications - some may blink in red now instead of blinking in green (led0 changes from green to red). I'm not sure this is worth it.

I think boards should not try to define leds order in the first place. This is something that zephyr often does to try to make its samples working out-of-the-box but which later has to be untangled at application level as often this is either done wrong (like in this case) or is not complete.
Order is clearly wrong. To fix it I would throw away gpio leds definitions along with all the led aliases out of board dtses - this way it could be defined correctly by application using it. If somebody thinks it is smart to have such things defined in a common place, you could create yet another dts overlay for such case that could be enabled on demand if needed.
But I know that we won't do that.

I am against this change as some people may depend on it. But if we fix the order now fewer applications may depend on it in the future. And this won't be the first case when application has to track and fix random change happening in Zephyr.

[pdunaj]

Maybe it would be good to make a single physical LED with all of the channels represented as a separate DTS node?

@MarekPieta Please don't do it. The whole thing is the effect of adding stuff that does not belong in board dts.
If order is not changed, leave it as is and fix it in application dts. We had already have to change pwm channels, pins or other stuff regarding leds. This one does not look like a big deal.

The main justification is what I mentioned in my first post - some people may depend on the CURRENT order. Although I think it is wrong, objectively speaking the order cannot be wrong. It is something that is purely subjective and people may set leds in ways that are more convenient for the application logic to handle them.

[anangl]

@anangl, on the dongle the colors of a single LED are in order. There is only added a first channel of other monochromatic LED before them.

Okay, this was not a best example. But there are others, like nrf52840_mdk or lpcxpresso11u68.

Why not use aliases for this, like it is for PWM LEDs?

[pdunaj]

@anangl, on the dongle the colors of a single LED are in order. There is only added a first channel of other monochromatic LED before them.

Okay, this was not a best example. But there are others, like nrf52840_mdk or lpcxpresso11u68.

Why not use aliases for this, like it is for PWM LEDs?

@anangl Adding aliases is not a fix. If somebody else comes in few months and says that the order is not matching app yet again will we add another set of aliases? This will only make dts a bigger mess.
If this change is not accepted I have asked Marek to fix that in app. I don't think there is any point on doing anything else.

[jfischer-no]

@anangl, on the dongle the colors of a single LED are in order. There is only added a first channel of other monochromatic LED before them.

Okay, this was not a best example. But there are others, like nrf52840_mdk or lpcxpresso11u68.
Why not use aliases for this, like it is for PWM LEDs?

@anangl Adding aliases is not a fix. If somebody else comes in few months and says that the order is not matching app yet again will we add another set of aliases? This will only make dts a bigger mess.
If this change is not accepted I have asked Marek to fix that in app. I don't think there is any point on doing anything else.

Is it the discussion about an unique LED ID? See also #21578 (comment)

[pdunaj]

@anangl, on the dongle the colors of a single LED are in order. There is only added a first channel of other monochromatic LED before them.

Okay, this was not a best example. But there are others, like nrf52840_mdk or lpcxpresso11u68.
Why not use aliases for this, like it is for PWM LEDs?

@anangl Adding aliases is not a fix. If somebody else comes in few months and says that the order is not matching app yet again will we add another set of aliases? This will only make dts a bigger mess.
If this change is not accepted I have asked Marek to fix that in app. I don't think there is any point on doing anything else.

Is it the discussion about an unique LED ID? See also #21578 (comment)

Hi @jfischer-no , it seems we simply have some time to spent on chatting :)

I think we have enough problems without id mentioned in the discussion. I don't like board dts being messed with things that are application dependent. But that's life - some configurations will simply not fit all cases.

In my opinion this should close within 5 minutes as discussion it any longer is waste of time. Zephyr should either keep the old order or take new one. Either choice will make some people unhappy and force them to change dts in application's overlay.

Hi @anangl , are you happy with this change or should we scrap it?

[anangl]

Hi @anangl , are you happy with this change or should we scrap it?

I'm still not convinced it's worth applying. That's why I haven't added my approval. But I won't insist on discarding it either. Since others accept it, that's fine with me.