From patchwork Wed Nov 9 13:02:36 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Sebastian Frias X-Patchwork-Id: 692758 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3tDRDk5hbmz9vDY for ; Thu, 10 Nov 2016 00:03:02 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=laposte.net header.i=@laposte.net header.b="GtwbyRwD"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932310AbcKINCn (ORCPT ); Wed, 9 Nov 2016 08:02:43 -0500 Received: from smtpoutz25.laposte.net ([194.117.213.100]:38057 "EHLO smtp.laposte.net" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752647AbcKINCk (ORCPT ); Wed, 9 Nov 2016 08:02:40 -0500 Received: from smtp.laposte.net (localhost [127.0.0.1]) by lpn-prd-vrout013 (Postfix) with ESMTP id 6B2F1104C02 for ; Wed, 9 Nov 2016 14:02:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=laposte.net; s=mail0; t=1478696558; bh=KUJkzunZan4lilC0dCxLTu05GKrYAE708qVVEX7wfC0=; h=Subject:To:References:Cc:From:Date:In-Reply-To; b=GtwbyRwDn18yvjSI84HmkKOaRjJdC672GrItdR9JIhsHC4fI/EH9W5RJfBW3GA2rq N5IqS2rYaL80oOZrvBpuNK8hJm8iVJAoq+DW5imjHE9lycYKYD+ddUMktFcKzR59x5 2erl2/oF1DN1omYnYaZirEHSCea5Y52WOBGaykcFuzBGPoIiVI1nu1yETLo53h/xQz aVTuv1YI3npJ05RBqkMwAL9rrj6Vo/fvpl02LYOTuYnO42vDgOs/Ps7c69/l8rsK8q 6i4paH3o6OlK5kM/fTvWPRgnpeKnjaG5blfhqNuomozyCcbCQVNLFG92GDttQSL6nh 3zOqEyk8+3VLA== Received: from smtp.laposte.net (localhost [127.0.0.1]) by lpn-prd-vrout013 (Postfix) with ESMTP id 5F32C104C13 for ; Wed, 9 Nov 2016 14:02:38 +0100 (CET) Received: from lpn-prd-vrin004 (lpn-prd-vrin004.laposte [10.128.63.5]) by lpn-prd-vrout013 (Postfix) with ESMTP id 5B77F104C0E for ; Wed, 9 Nov 2016 14:02:38 +0100 (CET) Received: from lpn-prd-vrin004 (localhost [127.0.0.1]) by lpn-prd-vrin004 (Postfix) with ESMTP id 4D5C671023E for ; Wed, 9 Nov 2016 14:02:38 +0100 (CET) Received: from [172.27.0.214] (APuteaux-554-1-100-170.w92-154.abo.wanadoo.fr [92.154.11.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by lpn-prd-vrin004 (Postfix) with ESMTPSA id 9FDAA710240; Wed, 9 Nov 2016 14:02:37 +0100 (CET) Subject: Re: [PATCH 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level To: =?UTF-8?B?TcOlbnMgUnVsbGfDpXJk?= , Florian Fainelli References: <581CA300.1060609@laposte.net> <581CAB12.3030604@laposte.net> Cc: "David S. Miller" , netdev , LKML , Mason , Andrew Lunn From: Sebastian Frias Message-ID: <58231E6C.2040603@laposte.net> Date: Wed, 9 Nov 2016 14:02:36 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: X-VR-SrcIP: 92.154.11.170 X-VR-FullState: 0 X-VR-Score: -100 X-VR-Cause-1: gggruggvucftvghtrhhoucdtuddrfeelfedrtddtgddujecutefuodetggdotefrodftvfcurfhrohhf X-VR-Cause-2: ihhlvgemucfntefrqffuvffgnecuuegrihhlohhuthemucehtddtnecusecvtfgvtghiphhivghnthhs X-VR-Cause-3: ucdlqddutddtmdenucfjughrpefuvfhfhffkffgfgggjtgfgsehtkegrtddtfeehnecuhfhrohhmpefu X-VR-Cause-4: vggsrghsthhirghnucfhrhhirghsuceoshhfkeegsehlrghpohhsthgvrdhnvghtqeenucfkphepledv X-VR-Cause-5: rdduheegrdduuddrudejtdenucfrrghrrghmpehmohguvgepshhmthhpohhuthdphhgvlhhopegludej X-VR-Cause-6: vddrvdejrddtrddvudegngdpihhnvghtpeelvddrudehgedruddurddujedtpdhmrghilhhfrhhomhep X-VR-Cause-7: shhfkeegsehlrghpohhsthgvrdhnvghtpdhrtghpthhtohepmhgrnhhssehmrghnshhrrdgtohhm X-VR-AvState: No X-VR-State: 0 X-VR-State: 0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 11/04/2016 05:49 PM, Måns Rullgård wrote: >>> 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. > Exactly, what about a patch like (I can make a formal submission, even merge it with the patch discussed in this thread, consider this a RFC): diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c index fba2699..4217ff4 100644 --- a/drivers/net/ethernet/aurora/nb8800.c +++ b/drivers/net/ethernet/aurora/nb8800.c @@ -1283,6 +1283,10 @@ static int nb8800_tangox_init(struct net_device *dev) case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID: pad_mode = PAD_MODE_RGMII; + + if ((dev->phydev->flags & PHY_SUPPORTS_TXID) == 0) + pad_mode |= PAD_MODE_GTX_CLK_DELAY; + break; default: diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c index 2e0c759..5eddb04 100644 --- a/drivers/net/phy/at803x.c +++ b/drivers/net/phy/at803x.c @@ -426,7 +426,9 @@ static int at803x_aneg_done(struct phy_device *phydev) .suspend = at803x_suspend, .resume = at803x_resume, .features = PHY_GBIT_FEATURES, - .flags = PHY_HAS_INTERRUPT, + .flags = PHY_HAS_INTERRUPT | + PHY_SUPPORTS_RXID | + PHY_SUPPORTS_TXID, .config_aneg = genphy_config_aneg, .read_status = genphy_read_status, .ack_interrupt = at803x_ack_interrupt, diff --git a/include/linux/phy.h b/include/linux/phy.h index e7e1fd3..0f0b17e 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -61,6 +61,8 @@ #define PHY_HAS_INTERRUPT 0x00000001 #define PHY_HAS_MAGICANEG 0x00000002 #define PHY_IS_INTERNAL 0x00000004 +#define PHY_SUPPORTS_RXID 0x00000008 +#define PHY_SUPPORTS_TXID 0x00000010 #define MDIO_DEVICE_IS_PHY 0x80000000 /* Interface Mode definitions */