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=-1.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS 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 ADA89C282C4 for ; Tue, 12 Feb 2019 06:51:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 728B92184A for ; Tue, 12 Feb 2019 06:51:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="H9Y8Zyc/" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727600AbfBLGvR (ORCPT ); Tue, 12 Feb 2019 01:51:17 -0500 Received: from mail-wr1-f65.google.com ([209.85.221.65]:42407 "EHLO mail-wr1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726059AbfBLGvQ (ORCPT ); Tue, 12 Feb 2019 01:51:16 -0500 Received: by mail-wr1-f65.google.com with SMTP id q18so1354137wrx.9 for ; Mon, 11 Feb 2019 22:51:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=jpFTB0IlAeYuJ2uFb96dPG8+smIRZAJixk2ICsgUIkU=; b=H9Y8Zyc/2FXMFn4mb9JsAQQZFg0QrjvmQ2RhohnZ5wb1mkI7XiXE6MZg6+fmk+u96h ZeiqmSulVp5+lykYlMYo+/xw6PzW19DO7ZheS4Ch9aNq0jgoUz94yF7ir9K37ChCmeOr IATlz+KCfl6M5zx5V0T8s/UvFVUxUjZIZrz5zYwZwr9rhmIVZQKh+kbDcBLcyGV/pmJi 70I0kL9H4r87ka0HWs2ERakCTa399DVhhIdCcKRqEVH+kwAMIxtANP0dO5w5A9WW1ufe Yp9AQy/T7YWnKmaUcpSR+463cGqr3daR/Y9JCbI1ob8Ew7RBa4AUj9/3wh9XKqOn3i7L zPoQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=jpFTB0IlAeYuJ2uFb96dPG8+smIRZAJixk2ICsgUIkU=; b=hR/H2yCYMwVvHry/6V2ZE7r4psh0i9oBGUuxCgOPDMo7WG19GhLjthv+2uuwFRojmH 8RX6ZJjMka/+DULF1xYi757EnEOVdaYjnQjDlUQ+YvYTvm4rwJs0qOWySgsM4UfRWyNk ytz2YzvhmNkHKblsSo5uM/frPN37yU3toanqQEg1gqJbw2E3KfJ9Y0mEK0KAKHJ+cZfC h1Zh5u5Hi5f4kOQW3WJn7REPi0LrD8wVPy+0Pw7QdjOeBdZ6vd3tUiUNdzLOtTKQUt/Y ie9ISmJpZgCgJ2WpwgHitnaTIeBvMDqIjGOcjm2FdIKsuyrZCU1L7nVgb2lpZLqfGaqj fydw== X-Gm-Message-State: AHQUAubasGcfG9VWURLS1Yyobmp+t9LwC6EQgNwzh25VikR01vsaGD13 RSYS8Jt3MOQQ872GlBahy5FmxjzB X-Google-Smtp-Source: AHgI3IbKzIfFlr5jOS7ulCrST+GX3nafEX0k2hTMh1roLncOQrGYIly/CJXs1dZNOSzZHllwsPLrgw== X-Received: by 2002:adf:ee82:: with SMTP id b2mr1666459wro.185.1549954274174; Mon, 11 Feb 2019 22:51:14 -0800 (PST) Received: from ?IPv6:2003:ea:8bf1:e200:cc19:f40b:8bd9:1334? (p200300EA8BF1E200CC19F40B8BD91334.dip0.t-ipconnect.de. [2003:ea:8bf1:e200:cc19:f40b:8bd9:1334]) by smtp.googlemail.com with ESMTPSA id y24sm4202011wma.0.2019.02.11.22.51.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 11 Feb 2019 22:51:13 -0800 (PST) Subject: Re: [PATCH net] dsa: mv88e6xxx: Ensure all pending interrupts are handled prior to exit To: Andrew Lunn , John David Anglin Cc: Russell King , Vivien Didelot , Florian Fainelli , netdev@vger.kernel.org References: <20190130172818.GJ21904@lunn.ch> <2ea9fd81-f92d-9505-dd0b-bdd0f67d8ce7@bell.net> <20190130223846.GB30115@lunn.ch> <9415d82e-965b-7777-0ad0-f23d6c9f177e@bell.net> <53b49df8-53ed-704f-9197-230b18d83090@bell.net> <824d011b-3692-69c3-5e2c-58e950a80abf@bell.net> <6a1ebc61-3505-beb8-21cb-ea42ad9fe67e@bell.net> <20190211233327.GB8591@lunn.ch> <2b6bbb4c-1346-461b-ff7a-cb96b4142f7a@bell.net> <20190212035806.GE19023@lunn.ch> From: Heiner Kallweit Message-ID: <13c1e6d5-c287-0091-3b24-1978f9a18e7e@gmail.com> Date: Tue, 12 Feb 2019 07:51:05 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 MIME-Version: 1.0 In-Reply-To: <20190212035806.GE19023@lunn.ch> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 12.02.2019 04:58, Andrew Lunn wrote: >>> Hi David >>> >>> I just tested this on one of my boards. It loops endlessly: >>> >>> [ 47.173396] mv88e6xxx_g1_irq_thread_work: c881 a8 80 >>> [ 47.182108] mv88e6xxx_g1_irq_thread_work: c881 a8 80 >>> [ 47.190820] mv88e6xxx_g1_irq_thread_work: c881 a8 80 >>> [ 47.199535] mv88e6xxx_g1_irq_thread_work: c881 a8 80 >>> [ 47.208254] mv88e6xxx_g1_irq_thread_work: c881 a8 80 >>> >>> These are reg, ctl1, reg & ctl1. >>> >>> So there is an unhandled device interrupt. > > Hi Heiner > > Your patch Fixes: 2b3e88ea6528 ("net: phy: improve phy state > checking") is causing me problems with interrupts for the Marvell > switches. > Hi Andrew, what kernel version is it? And the PHY driver in use is "Marvell 88E6390" ? > That change means we don't check the PHY device if it caused an > interrupt when its state is less than UP. > > What i'm seeing is that the PHY is interrupting pretty early on after > a reboot when the previous boot had the interface up. > So this means that when going down for reboot the interrupts are not properly masked / disabled? Because (at least for net-next) we enable interrupts in phy_start() only. > [ 10.125702] Marvell 88E6390 mv88e6xxx-0:02: phy_start_interrupts > [ 10.162798] Marvell 88E6390 mv88e6xxx-0:02: phy_enable_interrupts > [ 10.168931] Marvell 88E6390 mv88e6xxx-0:02: marvell_ack_interrupt > [ 10.180164] Marvell 88E6390 mv88e6xxx-0:02: marvell_config_intr 1 > > a little later it interrupts: > > [ 12.999717] mv88e6xxx_g1_irq_thread_fn > [ 13.007253] mv88e6xxx_g2_irq_thread_fn: 4 811c 4 > [ 13.012015] libphy: __phy_is_started: phydev->state 1 PHY_UP 3 > [ 13.017941] Marvell 88E6390 mv88e6xxx-0:02: phy_interrupt: phy_is_started(phydev) 0 > > The current code just causes it to be ignored. So the interrupts fires > again, and again... > I would have more expected the opposite. If the interrupt is ignored (IRQ_NONE returned), then it doesn't get acked. And if it's not acked new interrupts should be blocked. Or is it different with this chip? > If i change to code to call into the PHY driver and let it handle the > interrupts, things keep running. A little bit later the interface is > configured up: > > [ 15.921326] mv88e6085 gpio-0:00 red: configuring for phy/gmii link mode > [ 15.928693] libphy: __phy_is_started: phydev->state 3 PHY_UP 3 > [ 15.929442] IPv6: ADDRCONF(NETDEV_UP): red: link is not ready > [ 15.935596] Marvell 88E6390 mv88e6xxx-0:02: m88e6390_config_aneg > [ 15.935608] Marvell 88E6390 mv88e6xxx-0:02: m88e6390_errata > > [ 16.071364] Marvell 88E6390 mv88e6xxx-0:02: m88e1510_config_aneg > [ 16.112362] Marvell 88E6390 mv88e6xxx-0:02: m88e1318_config_aneg > [ 16.151245] Marvell 88E6390 mv88e6xxx-0:02: m88e1121_config_aneg > [ 16.368206] Marvell 88E6390 mv88e6xxx-0:02: PHY state change UP -> NOLINK > > and after another interrupt the link goes up. > > [ 19.519840] mv88e6xxx_g1_irq_thread_fn > [ 19.528546] mv88e6xxx_g2_irq_thread_fn: 4 811c 4 > [ 19.534152] libphy: __phy_is_started: phydev->state 5 PHY_UP 3 > [ 19.540030] Marvell 88E6390 mv88e6xxx-0:02: phy_interrupt: phy_is_started(phydev) 1 > [ 19.547721] Marvell 88E6390 mv88e6xxx-0:02: m88e1121_did_interrupt > [ 19.559829] Marvell 88E6390 mv88e6xxx-0:02: marvell_ack_interrupt > [ 19.590753] Marvell 88E6390 mv88e6xxx-0:02: marvell_read_status > [ 19.596712] Marvell 88E6390 mv88e6xxx-0:02: marvell_update_link > [ 19.628387] Marvell 88E6390 mv88e6xxx-0:02: PHY state change NOLINK -> RUNNING > [ 19.628453] mv88e6085 gpio-0:00 red: Link is Up - 1Gbps/Full - flow control off > [ 19.635920] IPv6: ADDRCONF(NETDEV_CHANGE): red: link becomes ready > > I don't yet know why the first interrupt happens, before we configure > auto-neg, etc. But it is not too unreasonable. We have configured > interrupts, so it could be reporting link down etc. > > So i think we might need to revert part of this change, call into the > driver so long as the PHY is not in state PHY_HALTED. > > What do you think? > I will take a closer look later. > Andrew > Heiner