quoted

quoted

I agree we need to make compromises. We cannot support every LED
feature of every PHY, they are simply too diverse. Hopefully we can
support some features of every PHY. In the worst case, a PHY simply
cannot be controlled via this method, which is the current state
today. So it is not worse off.

... and that compromise is that it's not going to be possible to enable
activity mode on 88e151x with how the code stands and with the
independent nature of "rx" and "tx" activity control currently in the
netdev trigger... making this whole approach somewhat useless for
Marvell PHYs.

Again we can consider adding an activity mode. It seems logical that
some switch may only support global traffic instead of independend tx or
rx... The feature are not mutually exclusive. One include the other 2.

Looking at the software trigger, adding NETDEV_LED_RXTX looks simple
to do. I also suspect it will be used by more than Marvell.

quoted

We really need to see a working implementation for this code for more
than just one PHY to prove that it is actually possible for it to
support other PHYs. If not, it isn't actually solving the problem,
and we're going to continue getting custom implementations to configure
the LED settings.

Agree that we need other user for this to catch some problem in the
implementation of this generic API.

We need a PHY driver implementation. The phylib core needs to be
involved, the cled code needs to call generic phylib functions which
take the phydev->lock before calling into the PHY driver. Probably the
phylib core can do all the memory allocation, and registration of the
LED to the LED core. If it is not too ugly, i would also do the DT
binding parsing in the core, so we don't end up with subtle
differences.

	Andrew