From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jose Abreu Subject: Re: [PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main driver flow Date: Thu, 2 Aug 2018 15:15:19 +0100 Message-ID: <9c9d9d73-5566-a432-4dad-91cd8f5267a9@synopsys.com> References: <39b57ee542fc20a1c457c12e6011dc239dda8725.1533125016.git.joabreu@synopsys.com> <20180801152330.GE32125@lunn.ch> <6dae65da-699f-77c9-2f50-775c888b5a12@synopsys.com> <20180802140323.GB7462@lunn.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Cc: , "David S. Miller" , "Joao Pinto" , Giuseppe Cavallaro , Alexandre Torgue To: Andrew Lunn , Jose Abreu Return-path: Received: from smtprelay4.synopsys.com ([198.182.47.9]:39617 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732329AbeHBQGt (ORCPT ); Thu, 2 Aug 2018 12:06:49 -0400 In-Reply-To: <20180802140323.GB7462@lunn.ch> Sender: netdev-owner@vger.kernel.org List-ID: Hi Andrew, On 02-08-2018 15:03, Andrew Lunn wrote: > On Thu, Aug 02, 2018 at 09:26:28AM +0100, Jose Abreu wrote: >> Hi Andrew, >> >> Thanks for the review! >> >> On 01-08-2018 16:23, Andrew Lunn wrote: >>>> @@ -842,6 +863,12 @@ static void stmmac_adjust_link(struct net_device *dev) >>>> new_state = true; >>>> ctrl &= ~priv->hw->link.speed_mask; >>>> switch (phydev->speed) { >>>> + case SPEED_10000: >>>> + ctrl |= priv->hw->link.speed10000; >>>> + break; >>>> + case SPEED_2500: >>>> + ctrl |= priv->hw->link.speed2500; >>>> + break; >>>> case SPEED_1000: >>>> ctrl |= priv->hw->link.speed1000; >>>> break; >>> Hi Jose >>> >>> What PHY did you test this with? >> We had some shipping issues with the 10G phy so right now I'm >> using a 1G phy ... > Please add that to the commit message. It is useful for people to know > this is untested above 1G, and so probably broken.... Ok, will do. > >> I would expect that as MDIO is used in both >> phys then phylib would take care of everything as long as I >> specify in the DT the right interface (SGMII) ... Am I making a >> wrong assumption? >> >>> 10G phys change the interface mode when the speed change. In general, >>> 10/100/1000G copper uses SGMII. A 1G SFP optical module generally >>> wants 1000Base-X. 2.5G wants 2500Base-X, 10G copper wants 10GKR, etc. >>> >>> So your adjust link callback needs to look at phydev->interface and >>> reconfigure the MAC as requested. >> Sorry, I'm not a phy expert but as long as I use MDIO shouldn't >> this be transparent to MAC? I mean, there are no registers about >> the interface to use in XGMAC2, there is only this speed >> selection register that its implemented already in the >> stmmac_adjust_link. > MDIO is the control plane used to manage the PHY. But here we are > talking about the data plane. As i said, the link between the MAC and > PHY will need to change depending on what the PHY is doing. SGMII will > work for 10/100/1000, but nothing above that. Sorry, I made a mistake. Where it reads SGMII in my reply I was referring to XGMII. > It could be this speed > register also changes the SERDES configuration, but you really should > confirm this and find out exactly what it is doing. There can be > multiple ways of doing one speed, e.g. SGMII at 1G. So if the PHY > wants you to do 1000Base-X and the MAC can only do SGMII, you need to > be raising an error. phylink makes this simpler. It ask the MAC driver > for all the modes it supports. It will then not ask the MAC to swap to > something it does not support. Ok. XGMII support is optional in the MAC so I will need to add a check for that. Thanks and Best Regards, Jose Miguel Abreu > > I suggest you get the datasheet for the PHY you are expecting to get, > once shipping is fixed. See what it says about its MAC side interface. > You can also look at the Marvell 10G driver, e.g. > mv3310_update_interface(). > > Andrew