Re: [PATCH] net: phy: leds: Trigger leds only if PHY speed is known

On Fri, Jul 16, 2021 at 09:32:00PM +0300, Ivan T. Ivanov wrote:

Hi,

Quoting Andrew Lunn (2021-07-16 18:19:58)

quoted

On Fri, Jul 16, 2021 at 05:11:42PM +0300, Ivan T. Ivanov wrote:

quoted

This prevents "No phy led trigger registered for speed(-1)"
alert message which is coused by phy_led_trigger_chage_speed()
being called during attaching phy to net_device where phy device
speed could be still unknown.

Hi Ivan

It seems odd that when attaching the PHY we have link, but not the
speed. What PHY is this?

This is lan78xx on RPi3B+

quoted

quoted

-     if (phy->speed == 0)
+     if (phy->speed == 0 || phy->speed == SPEED_UNKNOWN)
              return;

This change makes sense. But i'm wondering if the original logic is
sound. We have link, but no speed information.

Well, probably my interpretation was not correct. The most probable
call to phy_led_trigger_change_speed() which couses this alert is
phy_attach_direct() -> phy_led_triggers_register(), I think. I am
not sure that we have link at this stage or not.

This does sound weird.

When a phy_device is allocated, it's explicitly initialised with:

	dev->speed = SPEED_UNKNOWN;
	dev->duplex = DUPLEX_UNKNOWN;
	dev->link = 0;
	dev->state = PHY_DOWN;

so, unless something is causing state to be read before we've attached
the phy to a network device, this is how this state should remain. I
wonder why you are seeing dev->link be non-zero.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!