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=-6.7 required=3.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 CAD67C43381 for ; Tue, 12 Mar 2019 17:57:04 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 97BAF2175B for ; Tue, 12 Mar 2019 17:57:04 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="eRhxE3xP"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="lggNsMNw" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 97BAF2175B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=x6pCWzuZzgak6/8DVinGkRDanl9z1X/y/VLelUMI4OE=; b=eRhxE3xPVkEmNu B061ZiIYYqOftJv0+NkAWUgtjMmHvr4pRXg1kHlAPLOLYygMY4e/8hpmUv3kjcQRI4PnSqD/R76rI rr+/99aoSwB0kL5T+gU5EoXM68qncM7YM+CepaeTISO5fIWdKkvTJUg48WfB7+oFqjfP+GZ7vFaSL qjIaBexCwpL5O4ELEAbpw6FeKNDtX7NHn53aZLhdgUBTFagnq46QPxvaGlPNhX7JLSS5f73oCOZwW nhvfDGG4D/sYD/TW3psr2Xk5Wu59PScJQeDfINtNbzPshB6ytOV2EC3qdqJmUN57aEJozfZ3ZNitc f6dvGKfJkNxv7k5EczNg==; 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 1h3le5-0005lX-C7; Tue, 12 Mar 2019 17:56:57 +0000 Received: from mail-pg1-x544.google.com ([2607:f8b0:4864:20::544]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h3le1-0005kO-Lx; Tue, 12 Mar 2019 17:56:55 +0000 Received: by mail-pg1-x544.google.com with SMTP id m2so2396883pgl.5; Tue, 12 Mar 2019 10:56:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:openpgp:autocrypt:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=POtoNijm/I88rTq3ReCjdm11bcq7B3Y6dDelxolkZ3s=; b=lggNsMNwc3IS6t/fhvHuG8k32dNq8VGQl2WVa8j/OsHLd5aFWPisDHwDu2ji+/jYIZ Myfh4y30p9c8ssFb9UTwtRrGeDqN3SXImh3sY33lnDrgFxJZM1ViuvHkVNY9V3rO+mcj /4weIAVDhCp1ymjB6FGn1RYpVVyITu5etZsXxBinSAzMRIRg2w3yNMuMon88FeTC6+zh 9z08b7jrdzGP6xFVb/LECdqIGFNQ3SUSvRai1OC65F0a4UoE1t/5f2bEivknLTKaeInk MTmlKkDr6xwBI1Bh10OQRNymRPHts4z75JMX2/T5UzNgMgxzDPTfMLBGg1I3uLxCUcm5 iBLA== 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:openpgp:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=POtoNijm/I88rTq3ReCjdm11bcq7B3Y6dDelxolkZ3s=; b=TBjSiSWrXftEELpadH8t3Cs52LsfA3TkiM0SnMUGqqNOdjXA8CKjxLVXMRiGKjH658 oqkgYNTBsHWwGO2l3/XaD/g9MJ5Inb1IWAEXvoBIE/zXWZuNwHWbcoQEyVckER7dQQa7 3+qSyXIWVBH62cKEOyxNFMOIXHy/qqucXancnnhXbPwH9V5pbVvt3n6CB4PWnmoW2c4h 7SN8dNIOehavYE1WKWHExMp0403Jl2sEpy3dXaGybbp4vImeOWOxFZSnFuDQ5rkcDsdC yPpODMhroydRh94HX/1wIIREElcm6gVDZDQ/LqHR/1iQWWkfsfXmYatPMhVhiI2Qa2oF F4Tw== X-Gm-Message-State: APjAAAXrnOnWPhv+M87rP6oj1hFF+tjab9B2fVgofLk5QR5ZUTTnhmQy Tr6ZxZPFJy8PSyMWXPIM5MuJZbqc X-Google-Smtp-Source: APXvYqxPmJy3wMnHMDIPNPRkdYC/b6FOaEruPK9OlkXSMpB7rr8FIQRbdPhllmsfTRxEj7xZavdZwA== X-Received: by 2002:a62:2ad1:: with SMTP id q200mr39095354pfq.34.1552413412766; Tue, 12 Mar 2019 10:56:52 -0700 (PDT) Received: from [10.67.50.0] ([192.19.223.250]) by smtp.googlemail.com with ESMTPSA id t185sm14366981pfd.165.2019.03.12.10.56.51 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 Mar 2019 10:56:52 -0700 (PDT) Subject: Re: [PATCH net-next] net: phy: improve handling link_change_notify callback To: Heiner Kallweit , Andrew Lunn , David Miller , Heiko Stuebner References: <411b1c33-a245-0706-8afd-c4bea1b90f68@gmail.com> From: Florian Fainelli Openpgp: preference=signencrypt Autocrypt: addr=f.fainelli@gmail.com; prefer-encrypt=mutual; keydata= mQGiBEjPuBIRBACW9MxSJU9fvEOCTnRNqG/13rAGsj+vJqontvoDSNxRgmafP8d3nesnqPyR xGlkaOSDuu09rxuW+69Y2f1TzjFuGpBk4ysWOR85O2Nx8AJ6fYGCoeTbovrNlGT1M9obSFGQ X3IzRnWoqlfudjTO5TKoqkbOgpYqIo5n1QbEjCCwCwCg3DOH/4ug2AUUlcIT9/l3pGvoRJ0E AICDzi3l7pmC5IWn2n1mvP5247urtHFs/uusE827DDj3K8Upn2vYiOFMBhGsxAk6YKV6IP0d ZdWX6fqkJJlu9cSDvWtO1hXeHIfQIE/xcqvlRH783KrihLcsmnBqOiS6rJDO2x1eAgC8meAX SAgsrBhcgGl2Rl5gh/jkeA5ykwbxA/9u1eEuL70Qzt5APJmqVXR+kWvrqdBVPoUNy/tQ8mYc nzJJ63ng3tHhnwHXZOu8hL4nqwlYHRa9eeglXYhBqja4ZvIvCEqSmEukfivk+DlIgVoOAJbh qIWgvr3SIEuR6ayY3f5j0f2ejUMYlYYnKdiHXFlF9uXm1ELrb0YX4GMHz7QnRmxvcmlhbiBG YWluZWxsaSA8Zi5mYWluZWxsaUBnbWFpbC5jb20+iGYEExECACYCGyMGCwkIBwMCBBUCCAME FgIDAQIeAQIXgAUCVF/S8QUJHlwd3wAKCRBhV5kVtWN2DvCVAJ4u4/bPF4P3jxb4qEY8I2gS 6hG0gACffNWlqJ2T4wSSn+3o7CCZNd7SLSC5BA0ESM+4EhAQAL/o09boR9D3Vk1Tt7+gpYr3 WQ6hgYVON905q2ndEoA2J0dQxJNRw3snabHDDzQBAcqOvdi7YidfBVdKi0wxHhSuRBfuOppu pdXkb7zxuPQuSveCLqqZWRQ+Cc2QgF7SBqgznbe6Ngout5qXY5Dcagk9LqFNGhJQzUGHAsIs hap1f0B1PoUyUNeEInV98D8Xd/edM3mhO9nRpUXRK9Bvt4iEZUXGuVtZLT52nK6Wv2EZ1TiT OiqZlf1P+vxYLBx9eKmabPdm3yjalhY8yr1S1vL0gSA/C6W1o/TowdieF1rWN/MYHlkpyj9c Rpc281gAO0AP3V1G00YzBEdYyi0gaJbCEQnq8Vz1vDXFxHzyhgGz7umBsVKmYwZgA8DrrB0M oaP35wuGR3RJcaG30AnJpEDkBYHznI2apxdcuTPOHZyEilIRrBGzDwGtAhldzlBoBwE3Z3MY 31TOpACu1ZpNOMysZ6xiE35pWkwc0KYm4hJA5GFfmWSN6DniimW3pmdDIiw4Ifcx8b3mFrRO BbDIW13E51j9RjbO/nAaK9ndZ5LRO1B/8Fwat7bLzmsCiEXOJY7NNpIEpkoNoEUfCcZwmLrU +eOTPzaF6drw6ayewEi5yzPg3TAT6FV3oBsNg3xlwU0gPK3v6gYPX5w9+ovPZ1/qqNfOrbsE FRuiSVsZQ5s3AAMFD/9XjlnnVDh9GX/r/6hjmr4U9tEsM+VQXaVXqZuHKaSmojOLUCP/YVQo 7IiYaNssCS4FCPe4yrL4FJJfJAsbeyDykMN7wAnBcOkbZ9BPJPNCbqU6dowLOiy8AuTYQ48m vIyQ4Ijnb6GTrtxIUDQeOBNuQC/gyyx3nbL/lVlHbxr4tb6YkhkO6shjXhQh7nQb33FjGO4P WU11Nr9i/qoV8QCo12MQEo244RRA6VMud06y/E449rWZFSTwGqb0FS0seTcYNvxt8PB2izX+ HZA8SL54j479ubxhfuoTu5nXdtFYFj5Lj5x34LKPx7MpgAmj0H7SDhpFWF2FzcC1bjiW9mjW HaKaX23Awt97AqQZXegbfkJwX2Y53ufq8Np3e1542lh3/mpiGSilCsaTahEGrHK+lIusl6mz Joil+u3k01ofvJMK0ZdzGUZ/aPMZ16LofjFA+MNxWrZFrkYmiGdv+LG45zSlZyIvzSiG2lKy kuVag+IijCIom78P9jRtB1q1Q5lwZp2TLAJlz92DmFwBg1hyFzwDADjZ2nrDxKUiybXIgZp9 aU2d++ptEGCVJOfEW4qpWCCLPbOT7XBr+g/4H3qWbs3j/cDDq7LuVYIe+wchy/iXEJaQVeTC y5arMQorqTFWlEOgRA8OP47L9knl9i4xuR0euV6DChDrguup2aJVU4hPBBgRAgAPAhsMBQJU X9LxBQkeXB3fAAoJEGFXmRW1Y3YOj4UAn3nrFLPZekMeqX5aD/aq/dsbXSfyAKC45Go0YyxV HGuUuzv+GKZ6nsysJ7kCDQRXG8fwARAA6q/pqBi5PjHcOAUgk2/2LR5LjjesK50bCaD4JuNc YDhFR7Vs108diBtsho3w8WRd9viOqDrhLJTroVckkk74OY8r+3t1E0Dd4wHWHQZsAeUvOwDM PQMqTUBFuMi6ydzTZpFA2wBR9x6ofl8Ax+zaGBcFrRlQnhsuXLnM1uuvS39+pmzIjasZBP2H UPk5ifigXcpelKmj6iskP3c8QN6x6GjUSmYx+xUfs/GNVSU1XOZn61wgPDbgINJd/THGdqiO iJxCLuTMqlSsmh1+E1dSdfYkCb93R/0ZHvMKWlAx7MnaFgBfsG8FqNtZu3PCLfizyVYYjXbV WO1A23riZKqwrSJAATo5iTS65BuYxrFsFNPrf7TitM8E76BEBZk0OZBvZxMuOs6Z1qI8YKVK UrHVGFq3NbuPWCdRul9SX3VfOunr9Gv0GABnJ0ET+K7nspax0xqq7zgnM71QEaiaH17IFYGS sG34V7Wo3vyQzsk7qLf9Ajno0DhJ+VX43g8+AjxOMNVrGCt9RNXSBVpyv2AMTlWCdJ5KI6V4 KEzWM4HJm7QlNKE6RPoBxJVbSQLPd9St3h7mxLcne4l7NK9eNgNnneT7QZL8fL//s9K8Ns1W t60uQNYvbhKDG7+/yLcmJgjF74XkGvxCmTA1rW2bsUriM533nG9gAOUFQjURkwI8jvMAEQEA AYkCaAQYEQIACQUCVxvH8AIbAgIpCRBhV5kVtWN2DsFdIAQZAQIABgUCVxvH8AAKCRCH0Jac RAcHBIkHD/9nmfog7X2ZXMzL9ktT++7x+W/QBrSTCTmq8PK+69+INN1ZDOrY8uz6htfTLV9+ e2W6G8/7zIvODuHk7r+yQ585XbplgP0V5Xc8iBHdBgXbqnY5zBrcH+Q/oQ2STalEvaGHqNoD UGyLQ/fiKoLZTPMur57Fy1c9rTuKiSdMgnT0FPfWVDfpR2Ds0gpqWePlRuRGOoCln5GnREA/ 2MW2rWf+CO9kbIR+66j8b4RUJqIK3dWn9xbENh/aqxfonGTCZQ2zC4sLd25DQA4w1itPo+f5 V/SQxuhnlQkTOCdJ7b/mby/pNRz1lsLkjnXueLILj7gNjwTabZXYtL16z24qkDTI1x3g98R/ xunb3/fQwR8FY5/zRvXJq5us/nLvIvOmVwZFkwXc+AF+LSIajqQz9XbXeIP/BDjlBNXRZNdo dVuSU51ENcMcilPr2EUnqEAqeczsCGpnvRCLfVQeSZr2L9N4svNhhfPOEscYhhpHTh0VPyxI pPBNKq+byuYPMyk3nj814NKhImK0O4gTyCK9b+gZAVvQcYAXvSouCnTZeJRrNHJFTgTgu6E0 caxTGgc5zzQHeX67eMzrGomG3ZnIxmd1sAbgvJUDaD2GrYlulfwGWwWyTNbWRvMighVdPkSF 6XFgQaosWxkV0OELLy2N485YrTr2Uq64VKyxpncLh50e2RnyAJ9Za0Dx0yyp44iD1OvHtkEI M5kY0ACeNhCZJvZ5g4C2Lc9fcTHu8jxmEkI= Message-ID: Date: Tue, 12 Mar 2019 10:56:45 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <411b1c33-a245-0706-8afd-c4bea1b90f68@gmail.com> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190312_105653_719897_9F070D03 X-CRM114-Status: GOOD ( 29.03 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 3/3/19 10:58 AM, Heiner Kallweit wrote: > 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. 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 neglectable and > I think justified by the much simpler code. > > Changes are compile-tested only. > > Signed-off-by: Heiner Kallweit While you are it, we should probably audit all drivers making use of PHYLIB and see whether the logic to compare the previous link with the current link parameters is still necessary, and if it is, we should consider moving that to the core PHYLIB. > --- > 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); > -- Florian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel