Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 16 Jul 2021 17:19:58 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     "Ivan T. Ivanov" <iivanov@suse.de>
Cc:     Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "David S . Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: phy: leds: Trigger leds only if PHY speed is known
Message-ID: <YPGjnvB92ajEBKGJ@lunn.ch>
References: <20210716141142.12710-1-iivanov@suse.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210716141142.12710-1-iivanov@suse.de>
Precedence: bulk

On Fri, Jul 16, 2021 at 05:11:42PM +0300, Ivan T. Ivanov wrote:
> 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?

> -	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. So the LED trigger is
left in an indeterminate state. Rather than a plain return, maybe
phy_led_trigger_no_link(phy) should be called?

     Andrew