Re: [PATCH v2 net-next 0/3] Support offload LED blinking to PHY.

[Daniel Golle <daniel@makrotopia.org>]

Hi Andrew,

On Sat, Jun 24, 2023 at 10:56:26PM +0200, Andrew Lunn wrote:
> 
> Allow offloading of the LED trigger netdev to PHY drivers and
> implement it for the Marvell PHY driver. Additionally, correct the
> handling of when the initial state of the LED cannot be represented by
> the trigger, and so an error is returned.

I've used this series in my tree and implemented support for all LED-
related features in the mediatek-ge-soc PHY driver:

https://github.com/dangowrt/linux/commit/3b9b30a9a6699d290964fc76c56cfcd1dadf5651

I've noticed there is a problem when setting up the netdev trigger using
the 'linux,default-trigger' property in device tree. In this case
led_classdev_register_ext is called *before* register_netdevice has
completed.
Hence supports_hw_control returns false when the 'netdev' trigger is
initially setup as default trigger.

To resolve this, I've tried wrapping led_classdev_register_ext() and
introducing led_classdev_register_ext_nodefault() which doesn't call
out to led_trigger_set_default, so that can be done later by the
caller. However, there isn't any good existing call from netdev to phy
informing the phy that the netdev has been registered, so the phy_leds
would have to register a netdevice_notifier and wait for the
NETDEV_REGISTER events, matching against the netdev's PHY... And that
seems a bit overkill, just to support netdev trigger offloading to work
when used as a default trigger...

Any better ideas anyone?




> 
> Since v1:
> 
> Add true kerneldoc for the new entries in struct phy_driver
> Add received Reviewed-by: tags
> 
> Since v0:
> 
> Make comments in struct phy_driver look more like kerneldoc
> Add cover letter
> 
> Andrew Lunn (3):
>   led: trig: netdev: Fix requesting offload device
>   net: phy: phy_device: Call into the PHY driver to set LED offload
>   net: phy: marvell: Add support for offloading LED blinking
> 
>  drivers/leds/trigger/ledtrig-netdev.c |   8 +-
>  drivers/net/phy/marvell.c             | 243 ++++++++++++++++++++++++++
>  drivers/net/phy/phy_device.c          |  68 +++++++
>  include/linux/phy.h                   |  33 ++++
>  4 files changed, 349 insertions(+), 3 deletions(-)
> 
> -- 
> 2.40.1
> 
>