From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 70A48C43381 for ; Sat, 23 Feb 2019 23:42:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 36753206A3 for ; Sat, 23 Feb 2019 23:42:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b="SnLfLqW1" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727990AbfBWXml (ORCPT ); Sat, 23 Feb 2019 18:42:41 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:36458 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726773AbfBWXmk (ORCPT ); Sat, 23 Feb 2019 18:42:40 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding: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=JJE3kN4OhLim4zStEyW4eHlYjIdoS/F7Dh71OKQDYxo=; b=SnLfLqW17ou4NJruXI5xddOjHf +2WydfY30V1SWqFaaa58Z+SZsWMekp+8Q8mUG7lVAtsrgIkZ+MpR/025QG5ED5acqpYlNmuaGTVsp qtZRCPex0wm5zMvBa1yK5v6I1ffKgpFd50jwObApUsPKJE8ixS7Ax+RIxacxtd7c8+Zo=; Received: from andrew by vps0.lunn.ch with local (Exim 4.89) (envelope-from ) id 1gxgwF-0007Fn-OC; Sun, 24 Feb 2019 00:42:35 +0100 Date: Sun, 24 Feb 2019 00:42:35 +0100 From: Andrew Lunn To: Heiner Kallweit Cc: Russell King - ARM Linux admin , Florian Fainelli , "netdev@vger.kernel.org" Subject: Re: No traffic with Marvell switch and latest linux-next Message-ID: <20190223234235.GA26626@lunn.ch> References: <0907f213-df83-4bb8-8fad-21b1dedaeca5@gmail.com> <20190217154026.tykxexcndx7l5urk@shell.armlinux.org.uk> <291e7622-4402-e58f-503c-ffc7c6b2f055@gmail.com> <20190217165730.GF5968@lunn.ch> <20190217171027.GH5968@lunn.ch> <33f9ef8d-df62-e154-f880-f886abf54e0a@gmail.com> <20190218182136.GE14879@lunn.ch> <188fcef7-81fe-cffc-af71-1f37725b8611@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <188fcef7-81fe-cffc-af71-1f37725b8611@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org > it took me quite some time to debug this issue .. > > At first a bisect pointed to one of my commits: > 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status") > > Further digging lead me to some suspicious dsa code: > In dsa_port_fixed_link_register_of() there's a call to genphy_read_status(). > At the time of the call phydev->advertising is empty, therefore the fixed phy > settings are overwritten with defaults (10/half) what breaks the system. > > Worth to be mentioned is that for the PHY these two flags are set: > - is_pseudo_fixed_link (that's ok) > - autoneg -> I'm not sure this is correct. > > It seems that you once added the code in question: > 39b0c705195e ("net: dsa: Allow configuration of CPU & DSA port speeds/duplex") Hi Heiner For Ethernet switches, you can have device trees like this: switch0: switch@0 { compatible = "marvell,mv88e6085"; pinctrl-0 = <&pinctrl_gpio_switch0>; pinctrl-names = "default"; reg = <0>; dsa,member = <0 0>; interrupt-parent = <&gpio0>; interrupts = <27 IRQ_TYPE_LEVEL_LOW>; interrupt-controller; #interrupt-cells = <2>; eeprom-length = <512>; ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; label = "lan0"; phy-handle = <&switch0phy0>; }; ... switch0port5: port@5 { reg = <5>; label = "dsa"; phy-mode = "rgmii-txid"; link = <&switch1port6 &switch2port9>; fixed-link { speed = <1000>; full-duplex; }; }; This is a DSA port. It is used to connect two Ethernet switches together. In this case, the MACs are connected back to back. There are no PHYs involved. These ports don't have a netdev associated with them, since they are just pipes between switches, not user interfaces. If i remember correctly, the code you are looking at was to deal with the "rgmii-txid". Without the TXID, i could not get frames to pass between the ports. There is a second use case as well. The Vybrid FEC ethernet controller is a Fast Ethernet port. The switch ports are 1G. DSA drivers set the port connecting to the SoC to its highest speed. Again, there is no netdev associated to the switch port, and generally the MAC ports are connected back to back. This 100 vs 1000 does not work for the Vybrid. So we add a fixed PHY to the CPU port. port@6 { reg = <6>; label = "cpu"; ethernet = <&fec1>; fixed-link { speed = <100>; full-duplex; }; }; And there is a third use case: port@4 { reg = <4>; label = "optical4"; fixed-link { speed = <1000>; full-duplex; link-gpios = <&gpio6 3 GPIO_ACTIVE_HIGH>; }; }; We have a GPIO which tells us if the link it up, because there is not a PHY here, but an optical module. The GPIO tells us about loss of signal. Unfortunately we cannot use the SFP driver here, because of a hardware issue. In all cases end up in the same code. We want to tell the MAC to configure RGMII delays, or to configure the port speed, or to have the correct initial link state. The adjust_link() call can do this, if passed a phydev. And we have a phydev for the fixed-link PHY. However, since it has never been attached to a netdev, phy_start() called, etc, the state information is maybe not correct. So we call config_init() and read_status(), so phydev should reflect the state of the fixed-link PHY. And a fixed-link PHY is always c22, and it can be driven by genphy. Take a look at drivers/net/phy/swphy.c which is the core of the simulation, and fixed_phy.c > I did what I like to do most and removed some code. > W/o the calls to genphy_config_init() and genphy_read_status() it works again. > Do these calls have some purpose here with a fixed link? Maybe with all the core changes, these calls are no longer needed? We just need to ensure the state in phydev reflects the state of the fixed link, i.e. in this case 100 Full. Looking forward, at some point we are going to have to make fixed-link support higher speeds. That probably means we need a swphy-c45 which emulates the standard registers for 2.5G, 5G and 10G. At that point genphy will not work... Andrew