Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936488AbcKDQtq convert rfc822-to-8bit (ORCPT ); Fri, 4 Nov 2016 12:49:46 -0400 Received: from unicorn.mansr.com ([81.2.72.234]:43800 "EHLO unicorn.mansr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932511AbcKDQtp (ORCPT ); Fri, 4 Nov 2016 12:49:45 -0400 From: =?iso-8859-1?Q?M=E5ns_Rullg=E5rd?= To: Florian Fainelli Cc: Sebastian Frias , "David S. Miller" , netdev , LKML , Mason , Andrew Lunn Subject: Re: [PATCH 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level References: <581CA300.1060609@laposte.net> <581CAB12.3030604@laposte.net> Date: Fri, 04 Nov 2016 16:49:39 +0000 In-Reply-To: (Florian Fainelli's message of "Fri, 4 Nov 2016 09:28:52 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2505 Lines: 61 Florian Fainelli writes: > On 11/04/2016 08:36 AM, Sebastian Frias wrote: >> Hi M?ns, >> >> On 11/04/2016 04:18 PM, M?ns Rullg?rd wrote: >>> Sebastian Frias writes: >>> >>>> The delay can be applied at PHY or MAC level, but since >>>> PHY drivers will apply the delay at PHY level when using >>>> one of the "internal delay" declinations of RGMII mode >>>> (like PHY_INTERFACE_MODE_RGMII_TXID), applying it again >>>> at MAC level causes issues. >>> >>> The Broadcom GENET driver does the same thing. >>> >> >> Well, I don't know who uses that driver, or why they did it that way. > > I do use this driver and it works for me (tm), although I tested mostly > with Broadcom PHYs and Ethernet switches, rarely with third party PHYs, > but had that too, but all of that is in tree though, > drivers/net/phy/broadcom.com, drivers/net/dsa/b53/ so feel free to > "audit" that part of the code too. > > The configuration of the GENET port multiplexer requires us to specify > how we want to align the clock and data, if we don't do that, and the > PHY is also not agreeing with how its own delays should be configured, > mayhem ensues, ranging from occasional transmit success, to high rates > of CRC/FCS errors in best cases. > > I did verify that the settings were correct using a scope FWIW. > >> >> However, with the current code and DT bindings, if one requires >> the delay, phy-connection-type="rgmii-txid" must be set. > > Yes, and we would set it correctly for our Broadcom reference boards > using this driver. > >> >> But when doing so, both the Atheros 8035 and the Aurora NB8800 drivers >> will apply the delay. >> >> I think a better way of dealing with this is that both, PHY and MAC >> drivers exchange information so that the delay is applied only once. > > Exchange what information? The PHY device interface (phydev->interface) > conveys the needed information for both entities. There doesn't seem to be any consensus among the drivers regarding where the delay should be applied. Since only a few drivers, MAC or PHY, act on this property, most combinations still work by chance. It is common for boards to set the delay at the PHY using external config pins so no software setup is required (although I have one Sigma based board that gets this wrong). I suspect if drivers/net/ethernet/broadcom/genet were used with one of the four PHY drivers that also set the delay based on this DT property, things would go wrong. -- M?ns Rullg?rd