From patchwork Fri Jul 21 14:18:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Marc Gonzalez X-Patchwork-Id: 9857055 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4734460388 for ; Fri, 21 Jul 2017 14:18:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 22E0228736 for ; Fri, 21 Jul 2017 14:18:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 17646287BA; Fri, 21 Jul 2017 14:18:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 755D928736 for ; Fri, 21 Jul 2017 14:18:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=8U3I7HUYHsWCH4kN1VTxcg5/eI9D4NgCx1dwCC0B2Zk=; b=NIkhvbwiOFK7ey E9JeRwPEwS81og+NMXBIYAnkun97gUgYVOldSZ9bcHbRa0LhwGE9nwVZMpy9nC1mxSyC2i/kNSgRr Ts8QedC8vma4nz8OMhGYfUlkcIZwlbulF6eobej/GYFcM2uJR0+xKiMXPs0H/88DkbyPh8HvAkXrJ Zw8hQJwkoEBCf1O2W80nLwLW/bxATog3eTACngr90aU0hwqKKbrhP/JIGkuBvpuGd8Y8maBAE/pGt 9qwuLXyJNy9/BalGSWxc1LgiBjYlon2kOMo2hPmt/kb8DCbNNV/1hP6tYRlOKfXDqTHwtQyT98NE+ UxTrGtM0TQCyzVQmnjUg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dYYlS-0002Rh-Mk; Fri, 21 Jul 2017 14:18:46 +0000 Received: from merlin.infradead.org ([2001:8b0:10b:1231::1]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dYYlQ-0002RP-O9 for linux-arm-kernel@bombadil.infradead.org; Fri, 21 Jul 2017 14:18:45 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=merlin.20170209; h=Content-Transfer-Encoding:Content-Type: In-Reply-To:MIME-Version:Date:Message-ID:From:References:CC:To:Subject:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=5kqIyDpUn7DV31IHQlS1vnsGVo4HvCZoKj45k2CUDDk=; b=PTtWmW7poW44eMFTPhNRW0B4CM Vx7/c+DMmZkbX4YqduKZavsel7g/5KVgKd3Vik06lyPcIZ0jU4uriJCkdLF7t/onjojzSA9n3oO/g DYsOFpBoAJ1/OEvTyGso5QVzaU/nX/rB5wfqv0sTCuw9AC2pGd/mdZ9QS10zWKioBZaf5h9VVosnX wkLHDsxOiDTgWjZpHJNQyfWrI9z/2H83Rgr35j2JjrMqadPT6SGRtkv778QINkc4OlfNMsLYD0zyu s5gjNigZ6QhOtq3YAv0t2IQ69QEh+9RhAyicBWzSnlJ0V7EGSVptYRf90QYyvEWIh9LVNUk2Hr32L BK8uU3KQ==; Received: from us-smtp-delivery-107.mimecast.com ([216.205.24.107]) by merlin.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dYYlJ-0006yN-Gt for linux-arm-kernel@lists.infradead.org; Fri, 21 Jul 2017 14:18:42 +0000 Received: from CPH-EX1.SDESIGNS.COM (195-215-56-170-static.dk.customer.tdc.net [195.215.56.170]) (Using TLS) by us-smtp-1.mimecast.com with ESMTP id us-mta-79-1L5rELtOOSyNMzrnCOtamw-1; Fri, 21 Jul 2017 10:18:14 -0400 Received: from [172.27.0.114] (172.27.0.114) by CPH-EX1.sdesigns.com (192.168.10.36) with Microsoft SMTP Server (TLS) id 14.3.294.0; Fri, 21 Jul 2017 16:18:10 +0200 Subject: Re: [PATCH v2 3/4] net: ethernet: nb8800: Fix RGMII TX clock delay setup To: Mans Rullgard References: <2617d673-ef81-5e7f-4730-ed3aa3f1c2c6@sigmadesigns.com> <5a68bec8-0b1d-bcfe-65fb-b10399ec75be@sigmadesigns.com> <53fbf5e1-8d37-9734-be3f-77128ada9dfb@sigmadesigns.com> From: Marc Gonzalez Message-ID: <168bfa24-1dfe-e389-ab75-367e6bd2141a@sigmadesigns.com> Date: Fri, 21 Jul 2017 16:18:09 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 SeaMonkey/2.49.1 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [172.27.0.114] X-MC-Unique: 1L5rELtOOSyNMzrnCOtamw-1 X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrew Lunn , Grygorii Strashko , Mason , Thibaud Cornic , netdev , Timur Tabi , Daniel Mack , Florian Fainelli , Martin Blumenstingl , Fabio Estevam , "David S. Miller" , Linux ARM , Zefir Kurtisi Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On 21/07/2017 15:47, Måns Rullgård wrote: > Marc Gonzalez wrote: > >> On 21/07/2017 15:04, Måns Rullgård wrote: >> >>> Marc Gonzalez wrote: >>> >>>> According to commit e5f3a4a56ce2a707b2fb8ce37e4414dcac89c672 >>>> ("Documentation: devicetree: clarify usage of the RGMII phy-modes") >>>> there are 4 RGMII modes to handle: >>>> >>>> "rgmii" (RX and TX delays are added by the MAC when required) >>>> "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, >>>> the MAC should not add the RX or TX delays in this case) >>>> "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, >>>> the MAC should not add an RX delay in this case) >>>> "rgmii-txid" (RGMII with internal TX delay provided by the PHY, >>>> the MAC should not add an TX delay in this case) >>>> >>>> Add TX delay in the MAC only for rgmii and rgmii-rxid. >>>> >>>> Signed-off-by: Marc Gonzalez >>>> --- >>>> drivers/net/ethernet/aurora/nb8800.c | 6 ++++-- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c >>>> index ded041dbafe7..f3ed320eb4ad 100644 >>>> --- a/drivers/net/ethernet/aurora/nb8800.c >>>> +++ b/drivers/net/ethernet/aurora/nb8800.c >>>> @@ -1268,11 +1268,13 @@ static int nb8800_tangox_init(struct net_device *dev) >>>> break; >>>> >>>> case PHY_INTERFACE_MODE_RGMII: >>>> - pad_mode = PAD_MODE_RGMII; >>>> + case PHY_INTERFACE_MODE_RGMII_RXID: >>>> + pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; >>>> break; >>>> >>>> + case PHY_INTERFACE_MODE_RGMII_ID: >>>> case PHY_INTERFACE_MODE_RGMII_TXID: >>>> - pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; >>>> + pad_mode = PAD_MODE_RGMII; >>>> break; >>>> >>>> default: >>> >>> I still don't like this. Having both the MAC and PHY drivers react to >>> the phy-connection-type property is bound to cause trouble somewhere. >>> >>> The only way out of the current mess is to define new properties for >>> both MAC and PHY that override the existing ones if present. >> >> Do you mean defining 4 new bindings and their corresponding >> phy_interface_t enum values? For example: >> >> "rgmii-v2" >> "rgmii-id-v2" >> "rgmii-rxid-v2" >> "rgmii-txid-v2" >> >> PHY_INTERFACE_MODE_RGMII_V2, >> PHY_INTERFACE_MODE_RGMII_ID_V2, >> PHY_INTERFACE_MODE_RGMII_RXID_V2, >> PHY_INTERFACE_MODE_RGMII_TXID_V2, >> >> And then handling these new enums in the at803x and nb8800 drivers? > > It has already been suggested to add new properties specifying desired > delays in picoseconds. If present on the MAC node, the MAC driver > should attempt to provide the delay, and if present on the PHY node, the > PHY driver is responsible. Sorry, I had already forgotten about Florian's suggestion: > If you introduced PHY and/or MAC specific properties to configure the > delays in the appropriate unit of time (say ps), you could use a > non-compliant 'phy-mode' just to satisfy the driver/PHY library and > still override the delays you need. So we would need two properties (RX and TX). "rgmii_rx_skew_ps" and "rgmii_tx_skew_ps" but it's not clear to me how the MAC probe function communicates the arguments to the phy driver. Looks like we would need to add two fields to struct phy_device, and maybe define a new phy-mode to instruct the PHY driver to look for the two fields. I don't have time to work on that for now, but I do need to fix the nb8800 driver now. Can we apply the following patch in the interim? Regards. diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c index ded041dbafe7..e94159507847 100644 --- a/drivers/net/ethernet/aurora/nb8800.c +++ b/drivers/net/ethernet/aurora/nb8800.c @@ -1268,11 +1268,10 @@ static int nb8800_tangox_init(struct net_device *dev) break; case PHY_INTERFACE_MODE_RGMII: - pad_mode = PAD_MODE_RGMII; - break; - + case PHY_INTERFACE_MODE_RGMII_ID: + case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID: - pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; + pad_mode = PAD_MODE_RGMII; break; default: