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 A87BFC282C2 for ; Wed, 13 Feb 2019 13:12:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6C1DC222B1 for ; Wed, 13 Feb 2019 13:12:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="tVx4b+qw" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389429AbfBMNMM (ORCPT ); Wed, 13 Feb 2019 08:12:12 -0500 Received: from mail-lj1-f195.google.com ([209.85.208.195]:40051 "EHLO mail-lj1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726084AbfBMNMM (ORCPT ); Wed, 13 Feb 2019 08:12:12 -0500 Received: by mail-lj1-f195.google.com with SMTP id z25-v6so1941937ljk.7 for ; Wed, 13 Feb 2019 05:12:10 -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=c7b0ONA0YebL1729dD2yg7EjILZAAaIexfgZ7somhOA=; b=tVx4b+qwnvodFt9Ww1JXUcfDLivN8tQ22WWNXEwfSGE5S+biGEbsRJ+myPREAB6hJX k28zwNHDCuhndaBPhVLaSzp0uBl4riN3cMM6PybXW2wYtgrSFuHPimr7k/EYyhxd26ke tUz89ocWXjwG9GvMoblTX0s4qz8XzyRP2Qx+hViejSfW1exEWF+aJeRUQO+k39bzMfhB N+UYQ2reDO0lB+iyjZBmPZADUFAF665A/pIobT+3O3LU67xfmuIhPiHj6Nw5/pnv5YCu Tuw6kN8jblme2h0bb0721DnDHjSrIzcnYq7MQL3GYJn2ZDhiXoMsZpVqKDDYj9RYJPfE 8tKw== 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=c7b0ONA0YebL1729dD2yg7EjILZAAaIexfgZ7somhOA=; b=S8bA33/BEE9z9Rh44zVNWvbFFuVZGAeIxDLmj8f0XxihHV8oGA4jxES005ClgwO6T3 NuPwr4vAjmXvQTcJPbt/K2+oHPWxNbcOROmvUyth15r6sXFMGgFWT34ZoULxmXLNpWw2 I1oYfERdh34xtOmS3D22/WtBsNqzzOfa4EsBe2wOi3jCo3e5ud1k/Ax7uuglFhCAqLjw 4+rWRhoxAP4P4jd/x8Z4k54YW6FJkorUP88LTJ16mf4PcJ7ojDIPCHPLsF5r5VQSIFTM LeeZ7YxOBJgSFzJ9oxtmlOKTwYqNLMxvYs52dO3ZhRZp3X/xsXZQC8gDMClmcHOA+mJd RRJQ== X-Gm-Message-State: AHQUAuYBPrVavivw2euWBsUHzYmL8O+PsoqRo1CmckQ5bE8s8VUf1b0E PABl8ktRj03EXPa7w/iFR0bEJg== X-Google-Smtp-Source: AHgI3IZPJCo4NU5zu1cxUCvjM66lCw6r+EQkWZWrXtjSWlYIwBF4Nwbk2pcpQWkJeKu8sSGw8I/e6A== X-Received: by 2002:a2e:8645:: with SMTP id i5-v6mr294852ljj.166.1550063529603; Wed, 13 Feb 2019 05:12:09 -0800 (PST) Received: from centauri.lan (h-229-118.A785.priv.bahnhof.se. [5.150.229.118]) by smtp.gmail.com with ESMTPSA id t14sm477926lft.96.2019.02.13.05.12.07 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Wed, 13 Feb 2019 05:12:08 -0800 (PST) Date: Wed, 13 Feb 2019 14:12:06 +0100 From: Niklas Cassel To: Vinod Koul Cc: David S Miller , linux-arm-msm@vger.kernel.org, Bjorn Andersson , netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , "Nori, Sekhar" , Peter Ujfalusi , Marc Gonzalez Subject: Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode Message-ID: <20190213131206.GA460@centauri.lan> References: <20190212141922.12849-1-vkoul@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190212141922.12849-1-vkoul@kernel.org> 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 Tue, Feb 12, 2019 at 07:49:22PM +0530, Vinod Koul wrote: > Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode > should not have delay in PHY whereas RGMII_ID and RGMII_RXID/RGMII_TXID > can have delay in phy. > > So disable the delay only for RGMII mode and disable for other modes > > Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode") > Reported-by: Peter Ujfalusi > Signed-off-by: Vinod Koul > --- > drivers/net/phy/at803x.c | 33 ++++++++++++++++++++++++++------- > 1 file changed, 26 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c > index 8ff12938ab47..7b54b54e3316 100644 > --- a/drivers/net/phy/at803x.c > +++ b/drivers/net/phy/at803x.c > @@ -110,6 +110,18 @@ 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_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_rx_delay(struct phy_device *phydev) > { > return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, > @@ -255,18 +267,25 @@ static int at803x_config_init(struct phy_device *phydev) > if (ret < 0) > return ret; > > - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID || > - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || > - phydev->interface == PHY_INTERFACE_MODE_RGMII) { > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII) { > ret = at803x_disable_rx_delay(phydev); > if (ret < 0) > return ret; > + ret = at803x_disable_tx_delay(phydev); > + if (ret < 0) > + return ret; > + }; > + > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || > + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) { > + ret = at803x_enable_rx_delay(phydev); > + 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) { > - ret = at803x_disable_tx_delay(phydev); > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || > + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) { > + ret = at803x_enable_tx_delay(phydev); > if (ret < 0) > return ret; > } 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. Thus it will use the default value of the PHY, and for this PHY both TX and RX delays are enabled by default. This means that someone specifying PHY_INTERFACE_MODE_RGMII_TXID will actually be getting PHY_INTERFACE_MODE_RGMII_ID. Marc's patch: https://www.spinics.net/lists/netdev/msg445053.html does explicitly either enable or disable each delay (so it does not depend on default values). However, Marc's patch was never merged, because someone reported a regression on am335x-evm. On Vinod's V1 submission Andrew replied that if this change breaks some existing DT (because that DT specifies a phy-mode that does not match with reality), then we should fix so that DT specifies the correct phy-mode: https://www.spinics.net/lists/netdev/msg542638.html IMHO, this makes most sense, since if a DT specifies an incorrect phy-mode, then that is what the user should get. Kind regards, Niklas