From patchwork Tue Mar 19 18:56:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiner Kallweit X-Patchwork-Id: 10860265 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 57FFA1515 for ; Tue, 19 Mar 2019 18:57:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2E38229846 for ; Tue, 19 Mar 2019 18:57:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1F3AD2984A; Tue, 19 Mar 2019 18:57:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 7890429846 for ; Tue, 19 Mar 2019 18:57:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Date:Message-ID:To:Subject :From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=GF/eCaVm0eeV84tsNBKQuZnrhO07lq5sMtJSVNGOJnI=; b=O5ch0PVTLqToUN MKeDyvJr7teNUq5rRmf9DnSy4pI9VZ1DcGCbSEnUCcB/BRC1aKoStPJwjUsMfMjl3KMUDx36zu8KW 7f2+GK7jQ2r2wm4aYpRByHmq7BjQ3fNVddis14Nnwzez8kMH+sYtjYXlyCeqkvygkBm5/w14+4LqT 7ESG7UUr6OvqgP9C7lirRvrVXqzcfPIp84YXMrhxgXwvYp2NxZ6GJ2EqMG3oaiqy2dsUaVjaoSfSE OxS8fEoIU7ZrMrrEeWdNNsM+EmU14Frblfn/Sruo25uNlwrhRiAFmwlmo6FRl/l8Wzb4RLnx/Ig6S cMwleP3u3yrEoHUUwQdg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h6Jv5-00060p-Rd; Tue, 19 Mar 2019 18:57:03 +0000 Received: from mail-wr1-x444.google.com ([2a00:1450:4864:20::444]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h6Jv3-000607-6C; Tue, 19 Mar 2019 18:57:02 +0000 Received: by mail-wr1-x444.google.com with SMTP id p1so28221wrs.8; Tue, 19 Mar 2019 11:57:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:subject:to:cc:message-id:date:user-agent:mime-version :content-language:content-transfer-encoding; bh=GD6+ajMfoJ4G7ds9byUpsAHRyied3jGjZcWK+Y+GIh8=; b=YtPe8uscKpCZTA3OnXvWBSvqsHJCKN3ex6nrQ7ToRsklwZqoEgEAt+oWNOd/qtZ+yM KAu1KCkhimJEBKX+Y4y9yDUGxAreX9gLCkYGBoHsQcL60n7zDYN/sUJfosWM0Jj3qkDi vxBoMe3YjQU0T7Jgm9PWi/oUnQzgyzRBhS3i+YAMTjrP7fIpfFLQnUdvb4Dce4rK5DNL s3ntvzu45gvkDJHZJipq1bs0mLA2FZrOMBP/frhILq13kFravTjWJWziQ7nRrMD3vzQY IwT+2rWE8vknD5A8aoSv9aFsBA3KNonzf/ea2z8Ba9xa1AVZZD9B/mos8CDUX+L1EDJD kj9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:subject:to:cc:message-id:date:user-agent :mime-version:content-language:content-transfer-encoding; bh=GD6+ajMfoJ4G7ds9byUpsAHRyied3jGjZcWK+Y+GIh8=; b=uExDEWMnwOdTO0p71LqhQpBCZjCHW1oFnXAqCtbey1USbBVEt3B9SY6mVadPNLcxet ezxXGIKRNvg5BskiZBkFSgzTgRrGZ/KJaL03qVdm/Og0iLGNd6kYjELfWyFiDSkYJTox SUi8zDSpVHr/E/i6tHMm4tF82o+eg+A2KaO/Lsfp5d4vpF0tDE9CX+f9wCMsbkR/LVMQ BTXqpgBycrORzvgVMpt33jf+5VuKGeD+lyrhADDB6j2XHELY0CQc5lAwO6/nifT94vE6 52BD9jxdR5ZHlFnjGtlXNZm/Bd6N33PQUpLHSlM1VPQOigFrPNhsoQ9yJCmUPMLIlnSF jL2w== X-Gm-Message-State: APjAAAXAp7Ucjm8/4F0K7olpM0H0keTyB70RC0H0Hpm4tdh6eQ/aRhbF u/lI9+dR8AZc6yMKtqml2wHp/c/U X-Google-Smtp-Source: APXvYqysVDVANBr1z3NWHG69xBk7cdB5s37zbKg59tTBImOiPVydWg/SZYaXqewfZyyVOHonvP6KKQ== X-Received: by 2002:adf:fb82:: with SMTP id a2mr9257671wrr.214.1553021818668; Tue, 19 Mar 2019 11:56:58 -0700 (PDT) Received: from ?IPv6:2003:ea:8bc4:dc00:f0fd:910f:81c6:5261? (p200300EA8BC4DC00F0FD910F81C65261.dip0.t-ipconnect.de. [2003:ea:8bc4:dc00:f0fd:910f:81c6:5261]) by smtp.googlemail.com with ESMTPSA id x17sm39437651wrd.95.2019.03.19.11.56.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 19 Mar 2019 11:56:57 -0700 (PDT) From: Heiner Kallweit Subject: [PATCH net-next] net: phy: improve handling link_change_notify callback To: Andrew Lunn , Florian Fainelli , David Miller , Heiko Stuebner , David Wu Message-ID: <85f94f2f-8d1b-214f-a4af-afaf26d68ca0@gmail.com> Date: Tue, 19 Mar 2019 19:56:51 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.5.3 MIME-Version: 1.0 Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190319_115701_537885_3D70E83D X-CRM114-Status: GOOD ( 20.79 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "netdev@vger.kernel.org" , linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Currently the Phy driver's link_change_notify callback is called whenever the state machine is run (every second if polling), no matter whether the state changed or not. This isn't needed and may confuse users considering the name of the callback. Actually it contradicts its kernel-doc description. Therefore let's change the behavior and call this callback only in case of an actual state change. This requires changes to the at803x and rockchip drivers. at803x can be simplified so that it reacts on a state change to PHY_NOLINK only. The rockchip driver can also be much simplified. We simply re-init the AFE/DSP registers whenever we change to PHY_RUNNING and speed is 100Mbps. This causes very small overhead because we do this even if the speed was 100Mbps already. But this is negligible and I think justified by the much simpler code. Changes are compile-tested only. A little bit problematic seems to be to find somebody with the hardware to test the changes to the two PHY drivers. See also [0]. David may be able to test the Rockchip driver. [0] https://marc.info/?t=153782508800006&r=1&w=2 Signed-off-by: Heiner Kallweit --- drivers/net/phy/at803x.c | 26 +++++++++----------------- drivers/net/phy/phy.c | 8 ++++---- drivers/net/phy/rockchip.c | 31 ++----------------------------- 3 files changed, 15 insertions(+), 50 deletions(-) diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c index f3e96191e..f315ab468 100644 --- a/drivers/net/phy/at803x.c +++ b/drivers/net/phy/at803x.c @@ -324,8 +324,6 @@ static int at803x_config_intr(struct phy_device *phydev) static void at803x_link_change_notify(struct phy_device *phydev) { - struct at803x_priv *priv = phydev->priv; - /* * Conduct a hardware reset for AT8030 every time a link loss is * signalled. This is necessary to circumvent a hardware bug that @@ -333,25 +331,19 @@ static void at803x_link_change_notify(struct phy_device *phydev) * in the FIFO. In such cases, the FIFO enters an error mode it * cannot recover from by software. */ - if (phydev->state == PHY_NOLINK) { - if (phydev->mdio.reset && !priv->phy_reset) { - struct at803x_context context; + if (phydev->state == PHY_NOLINK && phydev->mdio.reset) { + struct at803x_context context; - at803x_context_save(phydev, &context); + at803x_context_save(phydev, &context); - phy_device_reset(phydev, 1); - msleep(1); - phy_device_reset(phydev, 0); - msleep(1); + phy_device_reset(phydev, 1); + msleep(1); + phy_device_reset(phydev, 0); + msleep(1); - at803x_context_restore(phydev, &context); + at803x_context_restore(phydev, &context); - phydev_dbg(phydev, "%s(): phy was reset\n", - __func__); - priv->phy_reset = true; - } - } else { - priv->phy_reset = false; + phydev_dbg(phydev, "%s(): phy was reset\n", __func__); } } diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 3745220c5..5938c5acf 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -891,9 +891,6 @@ void phy_state_machine(struct work_struct *work) old_state = phydev->state; - if (phydev->drv && phydev->drv->link_change_notify) - phydev->drv->link_change_notify(phydev); - switch (phydev->state) { case PHY_DOWN: case PHY_READY: @@ -940,10 +937,13 @@ void phy_state_machine(struct work_struct *work) if (err < 0) phy_error(phydev); - if (old_state != phydev->state) + if (old_state != phydev->state) { phydev_dbg(phydev, "PHY state change %s -> %s\n", phy_state_to_str(old_state), phy_state_to_str(phydev->state)); + if (phydev->drv && phydev->drv->link_change_notify) + phydev->drv->link_change_notify(phydev); + } /* Only re-schedule a PHY state machine change if we are polling the * PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving diff --git a/drivers/net/phy/rockchip.c b/drivers/net/phy/rockchip.c index 95abf7072..9053b1d01 100644 --- a/drivers/net/phy/rockchip.c +++ b/drivers/net/phy/rockchip.c @@ -104,41 +104,14 @@ static int rockchip_integrated_phy_config_init(struct phy_device *phydev) static void rockchip_link_change_notify(struct phy_device *phydev) { - int speed = SPEED_10; - - if (phydev->autoneg == AUTONEG_ENABLE) { - int reg = phy_read(phydev, MII_SPECIAL_CONTROL_STATUS); - - if (reg < 0) { - phydev_err(phydev, "phy_read err: %d.\n", reg); - return; - } - - if (reg & MII_SPEED_100) - speed = SPEED_100; - else if (reg & MII_SPEED_10) - speed = SPEED_10; - } else { - int bmcr = phy_read(phydev, MII_BMCR); - - if (bmcr < 0) { - phydev_err(phydev, "phy_read err: %d.\n", bmcr); - return; - } - - if (bmcr & BMCR_SPEED100) - speed = SPEED_100; - else - speed = SPEED_10; - } - /* * If mode switch happens from 10BT to 100BT, all DSP/AFE * registers are set to default values. So any AFE/DSP * registers have to be re-initialized in this case. */ - if ((phydev->speed == SPEED_10) && (speed == SPEED_100)) { + if (phydev->state == PHY_RUNNING && phydev->speed == SPEED_100) { int ret = rockchip_integrated_phy_analog_init(phydev); + if (ret) phydev_err(phydev, "rockchip_integrated_phy_analog_init err: %d.\n", ret);