From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.0 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5CBF0ECE58C for ; Mon, 14 Oct 2019 22:12:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0C548217D9 for ; Mon, 14 Oct 2019 22:12:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="M7x+OyYc" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731008AbfJNWMZ (ORCPT ); Mon, 14 Oct 2019 18:12:25 -0400 Received: from pandora.armlinux.org.uk ([78.32.30.218]:58152 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730369AbfJNWMZ (ORCPT ); Mon, 14 Oct 2019 18:12:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=bdiOmpZ5RXcC/f/ihPKAhcTaPuRDCg1OJyatxTXbycA=; b=M7x+OyYci8CeJM4I9gxZ/lDOZ 7JCz/6Qn66QpeEL89nFUfLj/GDC+A4/4zdT0YgwHy2GCKXl7GbT7Qa44ILvPEzFZstTfROg0ff0Pw qxUxovho9gxZIB3aK4eE3evWWHHMJUWio2XGDWhWY9pYJZxtw96kDnDRGsEdK1ci7oewwITn+zhpK ERGqqNqeZKGF150G62cTrd6grs92JGlQvEJkUXced9mlaaTMUI9Q1QlUwDNdNpKYE5S5FsC3FMCHo HmPef99Ty6+75vJ3RTTbL7i/iGMV3pEcdVJ7b2uYehudwPOx+yVSUHyTnDqpM1tppmEC+jA+5IQm+ nNczPBotA==; Received: from shell.armlinux.org.uk ([2001:4d48:ad52:3201:5054:ff:fe00:4ec]:43726) by pandora.armlinux.org.uk with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1iK8Zb-0000Ht-Hz; Mon, 14 Oct 2019 23:12:16 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.92) (envelope-from ) id 1iK8ZX-0004mu-Rt; Mon, 14 Oct 2019 23:12:11 +0100 Date: Mon, 14 Oct 2019 23:12:11 +0100 From: Russell King - ARM Linux admin To: Heiner Kallweit Cc: Stefan Wahren , Andrew Lunn , Florian Fainelli , Daniel Wagner , bcm-kernel-feedback-list@broadcom.com, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org Subject: Re: lan78xx and phy_state_machine Message-ID: <20191014221211.GR25745@shell.armlinux.org.uk> References: <20191014140604.iddhmg5ckqhzlbkw@beryllium.lan> <20191014163004.GP25745@shell.armlinux.org.uk> <20191014192529.z7c5x6hzixxeplvw@beryllium.lan> <25cfc92d-f72b-d195-71b1-f5f238c7988d@gmx.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, Oct 14, 2019 at 10:20:15PM +0200, Heiner Kallweit wrote: > On 14.10.2019 21:51, Stefan Wahren wrote: > > [add more recipients] > > > > Am 14.10.19 um 21:25 schrieb Daniel Wagner: > >> Moving the phy_prepare_link() up in phy_connect_direct() ensures that > >> phydev->adjust_link is set when the phy_check_link_status() is called. > >> > >> diff --git a/drivers/net/phy/phy_device.c > >> b/drivers/net/phy/phy_device.c index 9d2bbb13293e..2a61812bcb0d 100644 > >> --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c > >> @@ -951,11 +951,12 @@ int phy_connect_direct(struct net_device *dev, > >> struct phy_device *phydev, if (!dev) return -EINVAL; > >> > >> + phy_prepare_link(phydev, handler); > >> + > >> rc = phy_attach_direct(dev, phydev, phydev->dev_flags, interface); > >> if (rc) > > If phy_attach_direct() fails we may have to reset phydev->adjust_link to NULL, > as we do in phy_disconnect(). Apart from that change looks good to me. Sorry, but it doesn't look good to me. I think there's a deeper question here - why is the phy state machine trying to call the link change function during attach? At this point, the PHY hasn't been "started" so it shouldn't be doing that. Note the documentation, specifically phy.rst's "Keeping Close Tabs on the PAL" section. Drivers are at liberty to use phy_prepare_link() _after_ phy_attach(), which means there is a window for phydev->adjust_link to be NULL. It should _not_ be called at this point. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up