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=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 EC40FC6379F for ; Fri, 20 Nov 2020 01:49:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AF2522225B for ; Fri, 20 Nov 2020 01:49:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727301AbgKTBtZ (ORCPT ); Thu, 19 Nov 2020 20:49:25 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:40302 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726504AbgKTBtY (ORCPT ); Thu, 19 Nov 2020 20:49:24 -0500 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1kfvY7-0082oq-Go; Fri, 20 Nov 2020 02:49:19 +0100 Date: Fri, 20 Nov 2020 02:49:19 +0100 From: Andrew Lunn To: Dan Murphy Cc: davem@davemloft.net, f.fainelli@gmail.com, hkallweit1@gmail.com, robh@kernel.org, ciorneiioana@gmail.com, devicetree@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net-next v4 4/4] net: phy: dp83td510: Add support for the DP83TD510 Ethernet PHY Message-ID: <20201120014919.GB1804098@lunn.ch> References: <20201117201555.26723-1-dmurphy@ti.com> <20201117201555.26723-5-dmurphy@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201117201555.26723-5-dmurphy@ti.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > +static int dp83td510_config_init(struct phy_device *phydev) > +{ > + struct dp83td510_private *dp83td510 = phydev->priv; > + int ret = 0; > + > + if (phy_interface_is_rgmii(phydev)) { > + if (dp83td510->rgmii_delay) { > + ret = phy_set_bits_mmd(phydev, DP83TD510_DEVADDR, > + DP83TD510_MAC_CFG_1, > + dp83td510->rgmii_delay); Just to be safe, you should always write rgmii_delay, even if it is zero. We have had too many bugs with RGMII delays which cause bad backwards compatibility problems, so i would prefer to do a write which might be unneeded, that find a bug here in a few years time. > + if (ret) > + return ret; > + } > + } > + > + if (phydev->interface == PHY_INTERFACE_MODE_RMII) { > + ret = phy_modify(phydev, DP83TD510_GEN_CFG, > + DP83TD510_FIFO_DEPTH_MASK, > + dp83td510->tx_fifo_depth); So there is no need to set the FIFO depth for the other three RGMII modes? Or should this also be phy_interface_is_rgmii(phydev)? > +#if IS_ENABLED(CONFIG_OF_MDIO) > +static int dp83td510_of_init(struct phy_device *phydev) > +{ > + struct dp83td510_private *dp83td510 = phydev->priv; > + struct device *dev = &phydev->mdio.dev; > + struct device_node *of_node = dev->of_node; You need to move this assignment to later in order to keep with reverse christmas tree. > +#else > +static int dp83869_of_init(struct phy_device *phydev) > +{ > + dp83td510->hi_diff_output = DP83TD510_2_4V_P2P > + dp83td510->tx_fifo_depth = DP83TD510_FIFO_DEPTH_5_B_NIB You don't have DT, so there is no fine control, but you still need to do the basic 2ns delay as indicated by the phydev->interface value. So i think you still need to set dp83td510->rgmii_delay depending on which RGMII mode is requested. Andrew