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=-8.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT 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 9F7EBC282C2 for ; Wed, 13 Feb 2019 17:40:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 67A9B2175B for ; Wed, 13 Feb 2019 17:40:57 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="BAGbHh7J" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2393338AbfBMRk4 (ORCPT ); Wed, 13 Feb 2019 12:40:56 -0500 Received: from mail-lj1-f195.google.com ([209.85.208.195]:33012 "EHLO mail-lj1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2393296AbfBMRkl (ORCPT ); Wed, 13 Feb 2019 12:40:41 -0500 Received: by mail-lj1-f195.google.com with SMTP id f24-v6so2795765ljk.0 for ; Wed, 13 Feb 2019 09:40:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=rai5rkmmhWrAWE7ZveUtGJYYbAbvdhsapt5KGXx1egI=; b=BAGbHh7Je1y0Im0u+gCfrig4eOPotRd2l/qWwHK8N62SD2mb0ARurt8bn22hA+OLAo VO7BiYcb1z9cVUrHIY8UQzMtoy7LAoWkHENvY147nPVZH73w3Ft7gsZs4UUxVmv6H2Ao dVGPPIaB37466LS5xJAriUBq6Ta7iuMlmVlKdiFnts/PSiSGZgBakxdBhesG1hWeST5I xT8kK2bmHdK6iaMp7MHqbWDnFMZKQPVP1L5iNg9CmGe56R7An8WEggkVwewWlmTlxmCK BF4w6SDtgwZDPQmbqXXr89dFXkiG27mH9P+NLglNxgHw1NcwUFrV9Q34S+Cfer0GaIBj pJ+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=rai5rkmmhWrAWE7ZveUtGJYYbAbvdhsapt5KGXx1egI=; b=P28VzDTCyIiLN6nCxdGjegN3+J8DDD/3taHEryBIppBdEplUgpuqH7deSU29a9Jdjd V9NJtdUJ38H12xxCYxCFqSYhcen9lvNw0W1HwUNJlZVfaT87JeC6ZHfjp3fqn3NvuiXU mpRQrv0jaVP7zEv4A4aLobbA22DTp5oDllzh0QaMGj5UooroU0Yg+9bojEpOySge3ouu DJ1mtNoW39Oky9bPM0Vi2f1U060XCP0pGoXwn0sqWAyzW1u/u7s/AWAScsGJDNCRgHGe iKUH3qpZzkD5w29d13FbaMNe3D/gEjx2iHiSRuEskavPTPjTiQ7+VPAP3saU1gLX+JGt 3Taw== X-Gm-Message-State: AHQUAuaAvIlIcFievkeiCz8fKSBoYnm091S/3jwLQXXFWxqmFrXVO9pO fMjnSZyakJpaHlaMMffgcT2qVw== X-Google-Smtp-Source: AHgI3IbVVJNTA0HT3un5xUaT/q0YpP+rJWC6QHkvUl4BY/wTD5yt03N9utkEqUVGWHqgKZOQrFMbtw== X-Received: by 2002:a2e:7615:: with SMTP id r21-v6mr39197ljc.131.1550079638152; Wed, 13 Feb 2019 09:40:38 -0800 (PST) Received: from centauri.lan (h-229-118.A785.priv.bahnhof.se. [5.150.229.118]) by smtp.gmail.com with ESMTPSA id f9-v6sm2946043ljj.73.2019.02.13.09.40.35 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Wed, 13 Feb 2019 09:40:36 -0800 (PST) Date: Wed, 13 Feb 2019 18:40:34 +0100 From: Niklas Cassel To: Marc Gonzalez Cc: Andrew Lunn , Florian Fainelli , Vinod Koul , David S Miller , linux-arm-msm@vger.kernel.org, Bjorn Andersson , netdev@vger.kernel.org, "Nori, Sekhar" , Peter Ujfalusi Subject: Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode Message-ID: <20190213174034.GA6954@centauri.lan> References: <20190212141922.12849-1-vkoul@kernel.org> <20190213131206.GA460@centauri.lan> <20190213132900.GA24589@lunn.ch> <1ab5edac-a36c-9dc5-52e5-dbd3b70e7728@free.fr> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="uAKRQypu60I7Lcqm" Content-Disposition: inline In-Reply-To: <1ab5edac-a36c-9dc5-52e5-dbd3b70e7728@free.fr> 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 --uAKRQypu60I7Lcqm Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Feb 13, 2019 at 02:40:18PM +0100, Marc Gonzalez wrote: > On 13/02/2019 14:29, Andrew Lunn wrote: > > >> So we have these modes: > >> > >> PHY_INTERFACE_MODE_RGMII: TX and RX delays disabled > >> PHY_INTERFACE_MODE_RGMII_ID: TX and RX delays enabled > >> PHY_INTERFACE_MODE_RGMII_RXID: RX delay enabled, TX delay disabled > >> PHY_INTERFACE_MODE_RGMII_TXID: TX delay enabled, RX delay disabled > >> > >> What I don't like with this patch, is that if we specify phy-mode > >> PHY_INTERFACE_MODE_RGMII_TXID, this patch will enable TX delay, > >> but RX delay will not be explicitly set. > > > > That is not the behaviour we want. It is best to assume the device is > > in a random state, and correctly enable/disable all delays as > > requested. Only leave the hardware alone if PHY_INTERFACE_MODE_NA is > > used. > > That's what my patch did: > https://www.spinics.net/lists/netdev/msg445053.html > > But see Florian's remarks: > https://www.spinics.net/lists/netdev/msg445133.html Hello Marc, I saw that comment from Florian. However that was way back in 2017. Maybe the phy-modes were not as well defined back then? Andrew recently suggested to fix the driver so that it conforms with the phy-modes, and fix any SoC that specified an incorrect phy-mode in DT and thus relied upon the broken behavior of the PHY driver: https://www.spinics.net/lists/netdev/msg445133.html So, I've rebased your old patch, see attachment. I suggest that Peter test it on am335x-evm. am335x-evm appears to rely on the current broken behavior of the PHY driver, so we will probably need to fix the am335x-evm according to this: https://www.spinics.net/lists/netdev/msg445117.html and merge that as well. Andrew, Florian, do you both agree? Kind regards, Niklas --uAKRQypu60I7Lcqm Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0001-net-phy-at803x-Fix-RGMII-RX-and-TX-clock-delays-setu.patch" >From 7f19f32d3074c9990e46349b76ae13dc9c1133d6 Mon Sep 17 00:00:00 2001 From: Marc Gonzalez Date: Wed, 13 Feb 2019 18:29:02 +0100 Subject: [PATCH] net: phy: at803x: Fix RGMII RX and TX clock delays setup The current code supports enabling RGMII RX and TX clock delays. The unstated assumption is that these settings are disabled by default at reset, which is not the case. RX clock delay is enabled at reset. And TX clock delay "survives" across SW resets. Thus, if the bootloader enables TX clock delay, it will remain enabled at reset in Linux. Provide disable functions to configure the RGMII clock delays exactly as specified in the fwspec. Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode") Reported-by: Peter Ujfalusi Signed-off-by: Marc Gonzalez Signed-off-by: Niklas Cassel --- drivers/net/phy/at803x.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c index 90dc62c15fc5..700c1e4d34ad 100644 --- a/drivers/net/phy/at803x.c +++ b/drivers/net/phy/at803x.c @@ -103,12 +103,24 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg, return phy_write(phydev, AT803X_DEBUG_DATA, val); } +static inline int at803x_enable_rx_delay(struct phy_device *phydev) +{ + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0, + AT803X_DEBUG_RX_CLK_DLY_EN); +} + static inline int at803x_disable_rx_delay(struct phy_device *phydev) { return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, AT803X_DEBUG_RX_CLK_DLY_EN, 0); } +static inline int at803x_enable_tx_delay(struct phy_device *phydev) +{ + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0, + AT803X_DEBUG_TX_CLK_DLY_EN); +} + static inline int at803x_disable_tx_delay(struct phy_device *phydev) { return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, @@ -242,20 +254,22 @@ static int at803x_config_init(struct phy_device *phydev) return ret; if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID || - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || - phydev->interface == PHY_INTERFACE_MODE_RGMII) { + phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) + ret = at803x_enable_rx_delay(phydev); + else ret = at803x_disable_rx_delay(phydev); - if (ret < 0) - return ret; - } + + if (ret < 0) + return ret; if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID || - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || - phydev->interface == PHY_INTERFACE_MODE_RGMII) { + phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) + ret = at803x_enable_tx_delay(phydev); + else ret = at803x_disable_tx_delay(phydev); - if (ret < 0) - return ret; - } + + if (ret < 0) + return ret; return 0; } -- 2.20.1 --uAKRQypu60I7Lcqm--