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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sat, 29 Jul 2023 19:19:15 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Daniel Golle <daniel@...rotopia.org>
Cc: netdev <netdev@...r.kernel.org>, Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <rmk+kernel@...linux.org.uk>,
	Simon Horman <simon.horman@...igine.com>,
	Christian Marangi <ansuelsmth@...il.com>,
	SkyLake Huang <SkyLake.Huang@...iatek.com>
Subject: Re: [PATCH v2 net-next 0/3] Support offload LED blinking to PHY.

On Sat, Jul 29, 2023 at 01:38:13PM +0100, Daniel Golle wrote:
> 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?

Hi Daniel

I've not had chance to look at this yet. I really need to stop being
lazy and get my Marvell patches merged, the DT binding for defaults,
and then look at this ordering issue.

I do however agree, it is going to be messy, since as you say, the PHY
does not know what MAC it is bound to until quite late. Maybe we can
use the netdev_trig_notify() to update the value from
supports_hw_control()?

	Andrew