Re: [PATCH mvebu-dt v2 5/6] ARM: dts: turris-omnia: add LED controller node

["Marek Behún" <kabel@kernel.org>]

On Sat, 14 Nov 2020 22:58:45 +0100
Andreas Färber <afaerber@suse.de> 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