search

aristanetworks / sonic

Open source drivers and initialization library for Arista platforms running SONiC
GNU General Public License v2.0
25 stars 30 forks source link

Individual fan status LED devices not symlinked to /sys/class/leds/ #6

Closed jleveque closed 7 years ago

jleveque commented 7 years ago

Not sure about the other platforms, but on the 7050-QX-32 and 7050-QX-32-S, the individual fan status LEDs are not symlinked to /sys/class/leds/.

The README states that the fan LEDs should also be symlinked to /sys/class/leds (presumably at sys/class/leds/fan1, sys/class/leds/fan2, etc.), however, there are no symlinks present. It makes sense to symlink all LED devices into this directory for consistency, as beacon, fan_status, psu1, psu2 and status LEDs are all currently symlinked there.

Staphylo commented 7 years ago

Our fan drivers currently don't create the leds through the led_classdev subsystem of the kernel. Only the status leds are exposed under /sys/class/leds so far. Our fan drivers just create a sysfs entry for each fan led in the hwmon folder next to the other fan related entries. This also apply to DCS-7260CX3-64. If you want the fan leds to be exposed under /sys/class/leds/ we will add this to our tasks. The README could use some update on that front.

jleveque commented 7 years ago

While not imperative, it would be convenient to have all LEDs exposed under /sys/class/leds in a consistent manner across platforms. However, the fan LEDs respond to different sets of values between the 7050-QX-32 and 7050-QX-32-S, so maybe it's better to reference them in their respective places in the hwmon folder to help make it clear they are different devices and thus function differently. I'll leave it up to you to make the best decision here.

I agree the README is out-of-date and could use some updating.

Staphylo commented 7 years ago

Bug should be fixed by commits a7ad5d9e968f0e272ca0245b378f5480a17a2385, 561f785021fc9b56d1b7f1873a66c3dba6774064, and 6e35bf5af108df6130fdd61218f04ec355939e35