Am Mon, 15 Mar 2021 12:19:15 +0100
schrieb Pavel Machek [off-list ref]:

quoted

quoted

+       struct led_classdev cdev;
+};
+
+static struct simatic_ipc_led simatic_ipc_leds_io[] = {
+       {1 << 15, "simatic-ipc:green:" LED_FUNCTION_STATUS "-1" },
+       {1 << 7,  "simatic-ipc:yellow:" LED_FUNCTION_STATUS "-1"
},
+       {1 << 14, "simatic-ipc:red:" LED_FUNCTION_STATUS "-2" },
+       {1 << 6,  "simatic-ipc:yellow:" LED_FUNCTION_STATUS "-2"
},
+       {1 << 13, "simatic-ipc:red:" LED_FUNCTION_STATUS "-3" },
+       {1 << 5,  "simatic-ipc:yellow:" LED_FUNCTION_STATUS "-3"
},  

Can you use BIT() macro here? And can it be sorted by the bit
order?  

There's nothing wrong with << and this order is fine.

But I still don't like the naming. simantic-ipc: prefix is
useless. Having 6 status leds is not good, either.

You asked about a picture before, so here is one example

https://support.industry.siemens.com/cs/document/67235073/simatic-ipc427d?dti=0&pnid=16756&lc=en-WW

page 19 shows how the box looks like
page 135 it what the implementation is based on

Externally human visible are 3 "lights", which can be off, red, green,
yellow. Internally every single "light" has two leds and yellow is a
mix when red and green are on.
Unfortunately hw does not allow all 4 states for all 3 lights. Some
boxes implement "yellow" mixing in hw.

That is why the same name is used with two colors.

maybe those LEDs qualify for multi-color? 

"status" was the best name i found in the dt-bindings header

i guess i can be creative with the names and take ie "status", "fault",
and "indicator"

i will also drop that prefix "simatic-ipc" that was inspired by
"tpacpi", or should it be "platform:<color>:<name>"?

regards,
Henning

quoted

quoted

+       struct simatic_ipc_led *led =
+               container_of(led_cd, struct simatic_ipc_led,
cdev);  

One line?  

80 columns. It is fine as it is.

Best regards,

								Pavel