From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754188AbdL1SrH (ORCPT ); Thu, 28 Dec 2017 13:47:07 -0500 Received: from pandora.armlinux.org.uk ([78.32.30.218]:35524 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750780AbdL1SrG (ORCPT ); Thu, 28 Dec 2017 13:47:06 -0500 Date: Thu, 28 Dec 2017 18:46:42 +0000 From: Russell King - ARM Linux To: Antoine Tenart Cc: Florian Fainelli , Andrew Lunn , thomas.petazzoni@free-electrons.com, ymarkman@marvell.com, jason@lakedaemon.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kishon@ti.com, nadavh@marvell.com, miquel.raynal@free-electrons.com, gregory.clement@free-electrons.com, stefanc@marvell.com, mw@semihalf.com, davem@davemloft.net, linux-arm-kernel@lists.infradead.org, sebastian.hesselbarth@gmail.com Subject: Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface Message-ID: <20171228184642.GV10595@n2100.armlinux.org.uk> References: <20171227221446.18459-1-antoine.tenart@free-electrons.com> <20171227221446.18459-6-antoine.tenart@free-electrons.com> <20171227222401.GT10595@n2100.armlinux.org.uk> <20171228074623.GA28444@lunn.ch> <20171228100519.GE2626@kwain> <462da70b-ba7d-6299-3e21-b619d3c4c7e6@gmail.com> <20171228182739.GH2626@kwain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171228182739.GH2626@kwain> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Dec 28, 2017 at 07:27:39PM +0100, Antoine Tenart wrote: > Hi Florian, > > On Thu, Dec 28, 2017 at 07:02:09AM -0800, Florian Fainelli wrote: > > On 12/28/2017 02:05 AM, Antoine Tenart wrote: > > > On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote: > > >> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote: > > >>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote: > > >>>> > > >>>> +&cps_eth2 { > > >>>> + /* CPS Lane 5 */ > > >>>> + status = "okay"; > > >>>> + phy-mode = "2500base-x"; > > >>>> + /* Generic PHY, providing serdes lanes */ > > >>>> + phys = <&cps_comphy5 2>; > > >>>> +}; > > >>>> + > > >>> > > >>> This is wrong. This lane is connected to a SFP cage which can support > > >>> more than just 2500base-X. Tying it in this way to 2500base-X means > > >>> that this port does not support conenctions at 1000base-X, despite > > >>> that's one of the most popular and more standardised speeds. > > >>> > > >> > > >> I agree with Russell here. SFP modules are hot pluggable, and support > > >> a range of interface modes. You need to query what the SFP module is > > >> in order to know how to configure the SERDES interface. The phylink > > >> infrastructure does that for you. > > > > > > Sure, I understand. We'll be able to support such interfaces only when > > > the phylink PPv2 support lands in. > > > > Should we expect PHYLINK support to make it as the first patch in your > > v2 of this patch series, or is someone else doing that? > > No, the phylink patch conflicts with Marcin's ACPI series and we agreed > to let him get his series merged first. And I will probably work on a > few other topics before having the chance to work on it. So it'll > probably be me doing that, but not right now. ACPI is going to be a problem with phylink for a while. There's patches queued in net-next which convert phylink and SFP mostly to the fwnode and property based systems, but phylib and i2c do not seem to have the necessary bits to be able to deal with those. Specifically, in DT we have "of_find_i2c_adapter_by_node()" but afaics there is no equivalent in ACPI - which means in an ACPI based system we have no way to determine the I2C bus associated with a SFP socket, which is a rather fundamental issue for SFP modules. For phylib side, there's "of_phy_attach()" but again there is no equivalent in ACPI. This should not be that much of a problem, because network drivers using the DT phylib calls (eg, "of_phy_connect()") are already restricted by this. That may have been solved by Marcin's series, but I've not seen it to know. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up From mboxrd@z Thu Jan 1 00:00:00 1970 From: linux@armlinux.org.uk (Russell King - ARM Linux) Date: Thu, 28 Dec 2017 18:46:42 +0000 Subject: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface In-Reply-To: <20171228182739.GH2626@kwain> References: <20171227221446.18459-1-antoine.tenart@free-electrons.com> <20171227221446.18459-6-antoine.tenart@free-electrons.com> <20171227222401.GT10595@n2100.armlinux.org.uk> <20171228074623.GA28444@lunn.ch> <20171228100519.GE2626@kwain> <462da70b-ba7d-6299-3e21-b619d3c4c7e6@gmail.com> <20171228182739.GH2626@kwain> Message-ID: <20171228184642.GV10595@n2100.armlinux.org.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Dec 28, 2017 at 07:27:39PM +0100, Antoine Tenart wrote: > Hi Florian, > > On Thu, Dec 28, 2017 at 07:02:09AM -0800, Florian Fainelli wrote: > > On 12/28/2017 02:05 AM, Antoine Tenart wrote: > > > On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote: > > >> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote: > > >>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote: > > >>>> > > >>>> +&cps_eth2 { > > >>>> + /* CPS Lane 5 */ > > >>>> + status = "okay"; > > >>>> + phy-mode = "2500base-x"; > > >>>> + /* Generic PHY, providing serdes lanes */ > > >>>> + phys = <&cps_comphy5 2>; > > >>>> +}; > > >>>> + > > >>> > > >>> This is wrong. This lane is connected to a SFP cage which can support > > >>> more than just 2500base-X. Tying it in this way to 2500base-X means > > >>> that this port does not support conenctions at 1000base-X, despite > > >>> that's one of the most popular and more standardised speeds. > > >>> > > >> > > >> I agree with Russell here. SFP modules are hot pluggable, and support > > >> a range of interface modes. You need to query what the SFP module is > > >> in order to know how to configure the SERDES interface. The phylink > > >> infrastructure does that for you. > > > > > > Sure, I understand. We'll be able to support such interfaces only when > > > the phylink PPv2 support lands in. > > > > Should we expect PHYLINK support to make it as the first patch in your > > v2 of this patch series, or is someone else doing that? > > No, the phylink patch conflicts with Marcin's ACPI series and we agreed > to let him get his series merged first. And I will probably work on a > few other topics before having the chance to work on it. So it'll > probably be me doing that, but not right now. ACPI is going to be a problem with phylink for a while. There's patches queued in net-next which convert phylink and SFP mostly to the fwnode and property based systems, but phylib and i2c do not seem to have the necessary bits to be able to deal with those. Specifically, in DT we have "of_find_i2c_adapter_by_node()" but afaics there is no equivalent in ACPI - which means in an ACPI based system we have no way to determine the I2C bus associated with a SFP socket, which is a rather fundamental issue for SFP modules. For phylib side, there's "of_phy_attach()" but again there is no equivalent in ACPI. This should not be that much of a problem, because network drivers using the DT phylib calls (eg, "of_phy_connect()") are already restricted by this. That may have been solved by Marcin's series, but I've not seen it to know. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up