On Sat, 14 Nov 2020 22:58:45 +0100 Andreas Färber <afaerber@xxxxxxx> wrote: Hi Andreas, > > /* STM32F0 command interface at address 0x2a */ > > /* leds device (in STM32F0) at address 0x2b */ > > Update and move this comment now that the node documents the address? Sounds reasonable. > > + * in most cases users have wifi cards in > > + * these slots > > Doesn't U-Boot detect mSATA and switches SerDes configuration? You could > then have it set LED_FUNCTION_DISK in case of mSATA detected. Yes, but this also needs to be changed by u-boot. And current version of MCU firmware on Omnia doesn't connect the mSATA/PCI3 LED when in HW controlled mode, so the LED has to be blinked in software anyway, for now. Another problem is that user can put non wireless/disk PCIe device into this slot. What should the LED function be then? ... > I recently didn't find any DT binding for the netdev LED trigger, but > you could set trigger-sources to associate the LEDS with PCIe nodes even > if unused. Same for the LAN LEDs and switch port nodes, if you give them > labels. I am also working with Pavel in LED subsystem. Trigger-sources does not work currently - you can put it in device tree, but the drivers ignore it. I am currently also working on transparent HW offloading of LED triggers: if you set netdev trigger for lan1 LED, the system will just enable HW controlled mode on Omnia, instead of blinking this LED in software. So please wait till this is done. BTW, another issue is the devicename and function part of LED: The `linux,default-trigger` property is deprecated in favor of `function`. So somehow the system should enable netdev trigger on the LED is trigger-source points to a network device and function is compatible with netdev trigger. What should this functions be? We have: LED_FUNCTION_ACTIVITY LED_FUNCTION_RX LED_FUNCTION_TX LED_FUNCTION_WAN LED_FUNCTION_LAN Jacek thinks that LED_FUNCTION_ACTIVITY should be used for system activity trigger LED_FUNCTION_RX/TX on uart LED_FUNCTION_LAN on a network device But I and Pavel think that if the LED_FUNCTION_ACTIVITY is used, the trigger should be selected depending on trigger-source: - if it points to a network device, "netdev" - if it points to a block device, a potential "blkdev" trigger which does not exist now - ... Also RX/TX should be IMO used this way: for the netdev trigger you can use whether it should blink only on rx, only on tx, or on both. Please look at: https://www.spinics.net/lists/linux-leds/msg16632.html > > > + * - there are 2 LEDs dedicated for user: A and > > + * B. Again there is no such function defined. > > + * For now we use LED_FUNCTION_DEBUG > > I'd suggest the more neutral LED_FUNCTION_INDICATOR. Hmm, that sounds reasonable. Thanks. Marek