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=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,URIBL_BLOCKED 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 7DE31C43387 for ; Mon, 17 Dec 2018 18:41:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3DD052086C for ; Mon, 17 Dec 2018 18:41:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="P0xASlQw" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389090AbeLQSlr (ORCPT ); Mon, 17 Dec 2018 13:41:47 -0500 Received: from mail-wr1-f66.google.com ([209.85.221.66]:36834 "EHLO mail-wr1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728155AbeLQSlq (ORCPT ); Mon, 17 Dec 2018 13:41:46 -0500 Received: by mail-wr1-f66.google.com with SMTP id u4so12397743wrp.3; Mon, 17 Dec 2018 10:41:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=hoZqGmqHjh/RabrPalse1qzP0SaXa+/ulIH/GqmaHa8=; b=P0xASlQwZ/eEi2GxW9FpndgA2I5NcaF/nH+fsqnCrjVtwFCqcv811X9bekIuawFivF Q/21fw0j+/vWfBI0utM/dTQ5kMd1r5ZeRXrwWeEPu6yJ3b3DpWIgRvQEdaClMf9mKp6N 8UTt1hSIy9itCZ+ODSozN4QWP0MNQUw3eX+YDIv9i2kZygsx7bM446LhVsHHGaMznoXb MEWD1+oAYO55pPUSHqykDi1WzYeqVLTxc/0ITsizNvkRdgUkp1a2a27Tg2kGFQKaOeaQ iWEtxr0rFui1TG2VtpwejScUK4eJh5rY9u+N4i2+j6T8S/GWiBCfdKyOBDyXHgizZ1pv AbKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=hoZqGmqHjh/RabrPalse1qzP0SaXa+/ulIH/GqmaHa8=; b=TjVPkXOBC1OGULzN/vZsTp+0XFD/TPQ2DX5aMD7yOKXPUKXP3jRFyjLnwFRu0U4XKv Gmk3VVjQBo7kNDsZbPTdzZJVs3Y5IeuWxcEFUPE7FoowIiZD7kh8dG4KNe3nHbDfemCh 8LOPZBke4TsUMf7XsOFlXusV6brhigWE6IeQ1StlEg66X6aIwGZw4WLj99SWfkd8nRpT w/P4DFi0ynzftmKxY2C1kPa9jgDnBa7XFuWosLOusuEg7+S7uZAVbchtG7ZLR//s2d4e /NIFQOstjRRrkNQa+RD07gX7cRWrlUn+KNsD6hy9TbGJSs3NFCz4F4z8fDZiokaAYQgM 1Qkg== X-Gm-Message-State: AA+aEWbpe0jFobrCpZJk07QbgXegu8QsQX5AVQcXwOa6RZ4VNc8a5q+T ZeHwpsJNXqIDg5ba1zT95fwX7xuR X-Google-Smtp-Source: AFSGD/Ve2+bLDCv1NAILMv++OLjB45DVaWkI+F6DC0I7JMx6VD1E4hLNuqe2kkbO7NgNkEZaHCs5Vw== X-Received: by 2002:adf:c7cc:: with SMTP id y12mr12215926wrg.52.1545072102837; Mon, 17 Dec 2018 10:41:42 -0800 (PST) Received: from ?IPv6:2003:ea:8bcf:e300:9098:392c:b017:10bc? (p200300EA8BCFE3009098392CB01710BC.dip0.t-ipconnect.de. [2003:ea:8bcf:e300:9098:392c:b017:10bc]) by smtp.googlemail.com with ESMTPSA id o5sm16700wmg.25.2018.12.17.10.41.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 17 Dec 2018 10:41:42 -0800 (PST) Subject: Re: [RFC PATCH net v3] net: phy: Fix the issue that netif always links up after resuming To: Kunihiko Hayashi , Andrew Lunn , Florian Fainelli , "David S. Miller" Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <1543825349-423-1-git-send-email-hayashi.kunihiko@socionext.com> <20181217154102.34D9.4A936039@socionext.com> From: Heiner Kallweit Message-ID: Date: Mon, 17 Dec 2018 19:41:34 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.3.3 MIME-Version: 1.0 In-Reply-To: <20181217154102.34D9.4A936039@socionext.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 17.12.2018 07:41, Kunihiko Hayashi wrote: > Hi, > > Gentle ping... > Are there any comments about changes since v2? > > v2: https://www.spinics.net/lists/netdev/msg536926.html > > Thank you, > > On Mon, 3 Dec 2018 17:22:29 +0900 wrote: > >> Even though the link is down before entering hibernation, >> there is an issue that the network interface always links up after resuming >> from hibernation. >> >> The phydev->state is PHY_READY before enabling the network interface, so >> the link is down. After resuming from hibernation, the phydev->state is >> forcibly set to PHY_UP in mdio_bus_phy_restore(), and the link becomes up. >> >> This patch adds a new convenient function to check whether the PHY is in >> a started state, and expects to solve the issue by changing phydev->state >> to PHY_UP and calling phy_start_machine() only when the PHY is started. >> This convenience function and the related change to phy_stop() are part of the following already and don't need to be part of your patch. https://patchwork.ozlabs.org/patch/1014171/ >> Suggested-by: Heiner Kallweit >> Signed-off-by: Kunihiko Hayashi >> --- >> >> Changes since v2: >> - add mutex lock/unlock for changing phydev->state >> - check whether the mutex is locked in phy_is_started() >> >> Changes since v1: >> - introduce a new helper function phy_is_started() and use it instead of >> checking link status >> - replace checking phydev->state with phy_is_started() in >> phy_stop_machine() >> >> drivers/net/phy/phy.c | 2 +- >> drivers/net/phy/phy_device.c | 12 +++++++++--- >> include/linux/phy.h | 13 +++++++++++++ >> 3 files changed, 23 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c >> index 1d73ac3..f484d03 100644 >> --- a/drivers/net/phy/phy.c >> +++ b/drivers/net/phy/phy.c >> @@ -670,7 +670,7 @@ void phy_stop_machine(struct phy_device *phydev) >> cancel_delayed_work_sync(&phydev->state_queue); >> >> mutex_lock(&phydev->lock); >> - if (phydev->state > PHY_UP && phydev->state != PHY_HALTED) >> + if (phy_is_started(phydev)) >> phydev->state = PHY_UP; I'm wondering whether we need to do this. If the PHY is attached, then mdio_bus_phy_suspend() calls phy_stop_machine() which does exactly the same. If the PHY is not attached, then we don't have to do anything. Therefore I think we just have to do the same as in mdio_bus_phy_resume(): if (phydev->attached_dev && phydev->adjust_link) phy_start_machine(phydev); Can you test this? >> mutex_unlock(&phydev->lock); >> } >> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c >> index ab33d17..4897d24 100644 >> --- a/drivers/net/phy/phy_device.c >> +++ b/drivers/net/phy/phy_device.c >> @@ -309,10 +309,16 @@ static int mdio_bus_phy_restore(struct device *dev) >> return ret; >> >> /* The PHY needs to renegotiate. */ >> - phydev->link = 0; >> - phydev->state = PHY_UP; >> + mutex_lock(&phydev->lock); >> + if (phy_is_started(phydev)) { >> + phydev->state = PHY_UP; >> + mutex_unlock(&phydev->lock); >> + phydev->link = 0; >> + phy_start_machine(phydev); >> + } else { >> + mutex_unlock(&phydev->lock); >> + } >> >> - phy_start_machine(phydev); >> >> return 0; >> } >> diff --git a/include/linux/phy.h b/include/linux/phy.h >> index 3ea87f7..dd21537 100644 >> --- a/include/linux/phy.h >> +++ b/include/linux/phy.h >> @@ -898,6 +898,19 @@ static inline bool phy_is_pseudo_fixed_link(struct phy_device *phydev) >> } >> >> /** >> + * phy_is_started - Convenience function for testing whether a PHY is in >> + * a started state >> + * @phydev: the phy_device struct >> + * >> + * The caller must have taken the phy_device mutex lock. >> + */ >> +static inline bool phy_is_started(struct phy_device *phydev) >> +{ >> + WARN_ON(!mutex_is_locked(&phydev->lock)); >> + return phydev->state >= PHY_UP && phydev->state != PHY_HALTED; >> +} >> + >> +/** >> * phy_write_mmd - Convenience function for writing a register >> * on an MMD on a given PHY. >> * @phydev: The phy_device struct >> -- >> 2.7.4 > > --- > Best Regards, > Kunihiko Hayashi > > >