On 06.10.2021 22:15, Dan Williams wrote:
+static struct led_state led_states[] = {
+ { .name = "ok", .bit = 2 },
+ { .name = "locate", .bit = 3 },
+ { .name = "failed", .bit = 4 },
+ { .name = "rebuild", .bit = 5 },
+ { .name = "pfa", .bit = 6 },
+ { .name = "hotspare", .bit = 7 },
+ { .name = "ica", .bit = 8 },
+ { .name = "ifa", .bit = 9 },
+ { .name = "invalid", .bit = 10 },
+ { .name = "disabled", .bit = 11 },
+};
include/linux/enclosure.h has common ABI definitions of industry
standard enclosure LED settings. The above looks to be open coding the
same?
The LED states in inluce/linux/enclosure.h aren't exactly the same...
there are states defined in NPEM/_DSM that aren't defined in
enclosure.h. In addition, while the enclosure driver allows "locate" to
be controlled independently, it looks like it will only allow a single
state (unsupported/ok/critical/etc) to be active at a time, while the
NPEM/_DSM allow all of the state bits to be independently set or
cleared. Maybe only one of those states would need to be set at a time,
I don't know, but that would impose a limitation on what NPEM/_DSM can
do. I'll take a closer look at this as an alternative to using
drivers/leds/led-class.c.
Have a look. Maybe Mariusz can weigh in here with his experience with
ledmon with what capabilities ledmon would need from this driver so we
can decide what if any extensions need to be made to the enclosure
infrastructure?
Hmm... In ledmon we are expecting one state to be set at the time. So,
I would expected from kernel to work the same.
Looking into ledmon code, all capabilities from this list could be
used[1].
>>>> + { .name = "ok", .bit = 2 },
>>>> + { .name = "locate", .bit = 3 },
>>>> + { .name = "failed", .bit = 4 },
>>>> + { .name = "rebuild", .bit = 5 },
>>>> + { .name = "pfa", .bit = 6 },
>>>> + { .name = "hotspare", .bit = 7 },
>>>> + { .name = "ica", .bit = 8 },
>>>> + { .name = "ifa", .bit = 9 },
[1]https://github.com/intel/ledmon/blob/master/src/ibpi.h#L60
Thanks,
Mariusz
