On Wed, Dec 21, 2022 at 01:12:16AM +0100, Andrew Lunn wrote: > On Thu, Dec 15, 2022 at 05:35:47PM +0000, Russell King (Oracle) wrote: > > On Thu, Dec 15, 2022 at 12:54:36AM +0100, Christian Marangi wrote: > > > Add additional hardware only triggers commonly supported by switch LEDs. > > > > > > Additional modes: > > > link_10: LED on with link up AND speed 10mbps > > > link_100: LED on with link up AND speed 100mbps > > > link_1000: LED on with link up AND speed 1000mbps > > > half_duplex: LED on with link up AND half_duplex mode > > > full_duplex: LED on with link up AND full duplex mode > > > > Looking at Marvell 88e151x, I don't think this is usable there. > > We have the option of supporting link_1000 on one of the LEDs, > > link_100 on another, and link_10 on the other. It's rather rare > > for all three leds to be wired though. > > The 88e151x will need to enumerate what it actually supports from the > above list, per LED. I also think we can carefully expand the list > above, adding a few more modes. We just need to ensure what is added > is reasonably generic, modes we expect multiple PHY to support. What > we need to avoid is adding every single mode a PHY supports, but no > other PHY has. > > > This is also a PHY where "activity" mode is supported (illuminated > > or blinking if any traffic is transmitted or received) but may not > > support individual directional traffic in hardware. However, it > > does support forcing the LED on or off, so software mode can handle > > those until the user selects a combination of modes that are > > supported in the hardware. > > > > > Additional blink interval modes: > > > blink_2hz: LED blink on any even at 2Hz (250ms) > > > blink_4hz: LED blink on any even at 4Hz (125ms) > > > blink_8hz: LED blink on any even at 8Hz (62ms) > > > > This seems too restrictive. For example, Marvell 88e151x supports > > none of these, but does support 42, 84, 170, 340, 670ms. > > I would actually drop this whole idea of being able to configure the > blink period. It seems like it is going to cause problems. I expect > most PHYs actual share the period across multiple LEDs, which you > cannot easily model here. > > So i would have the driver hard coded to pick a frequency at thats' it. > Yes I think "for now" it's the only way and just drop blink configuration support. -- Ansuel
