| Message ID | 20230808210436.838995-1-andrew@lunn.ch (mailing list archive) |
|---|---|
| Headers | show |
| Series | Support offload LED blinking to PHY. | expand |
Hi Andrew, On Tue, Aug 08, 2023 at 11:04:32PM +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. As with ledtrig-timer, > disable offload when the trigger is deactivate, or replaced by another > trigger. I've tested the series and changed my driver accordingly, now deactivation of the offloaded tasks works fine when changing the trigger. Overall I believe this is good to go, however, what remains unresolved is the chicken-egg when assigning the 'netdev' trigger using linux,default-trigger in device tree: In this case selection of the netdev trigger happens on creation of the PHY instance which is *before* the creation of the netdev the PHY is going to be attached to. Hence 'netdev' gets activated *without* any hardware offloading. And while reading the current hardware state (as left behind by IC defaults or by the bootloader) works fine, by default the LEDs would show trigger 'none' in Linux right after boot (despite e.g. link and traffic indications are active by default -- which is would I tried to express by using linux,default-trigger...) Tested-by: Daniel Golle <daniel@makrotopia.org> > > Since v2: > Add support for link speeds, not just link > Add missing checks for return values > Add patch disabling offload when driver is deactivated > > 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 (4): > 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 > leds: trig-netdev: Disable offload on deactivation of trigger > > drivers/leds/trigger/ledtrig-netdev.c | 10 +- > drivers/net/phy/marvell.c | 281 ++++++++++++++++++++++++++ > drivers/net/phy/phy_device.c | 68 +++++++ > include/linux/phy.h | 33 +++ > 4 files changed, 389 insertions(+), 3 deletions(-) > > -- > 2.40.1 >
> Overall I believe this is good to go, however, what remains > unresolved is the chicken-egg when assigning the 'netdev' trigger > using linux,default-trigger in device tree: Agreed. This is pretty high up my TODO list to look at. Thanks for the tested-by. Andrew
Hello: This series was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 8 Aug 2023 23:04:32 +0200 you 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. As with ledtrig-timer, > disable offload when the trigger is deactivate, or replaced by another > trigger. > > [...] Here is the summary with links: - [net-next,v3,1/4] led: trig: netdev: Fix requesting offload device https://git.kernel.org/netdev/net-next/c/7df1f14c04cb - [net-next,v3,2/4] net: phy: phy_device: Call into the PHY driver to set LED offload https://git.kernel.org/netdev/net-next/c/1dcc03c9a7a8 - [net-next,v3,3/4] net: phy: marvell: Add support for offloading LED blinking https://git.kernel.org/netdev/net-next/c/460b0b648fab - [net-next,v3,4/4] leds: trig-netdev: Disable offload on deactivation of trigger https://git.kernel.org/netdev/net-next/c/e8fbcc47a8e9 You are awesome, thank you!