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=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, 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 55D7CC282C4 for ; Tue, 12 Feb 2019 20:07:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 186C92083B for ; Tue, 12 Feb 2019 20:07:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ZNNU8MoJ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730378AbfBLUHF (ORCPT ); Tue, 12 Feb 2019 15:07:05 -0500 Received: from mail-wr1-f66.google.com ([209.85.221.66]:38588 "EHLO mail-wr1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727894AbfBLUHE (ORCPT ); Tue, 12 Feb 2019 15:07:04 -0500 Received: by mail-wr1-f66.google.com with SMTP id v13so4153100wrw.5 for ; Tue, 12 Feb 2019 12:07:02 -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=QS/NX/rmoF4PVTXdslyOTsIHHYyFzCubmrSUdWwb/N0=; b=ZNNU8MoJnzNtYYkBJrcaJke61o9cKT/0TBD9tAOhYJ8jZwml8eaKAV7O+0kWba9Y+B H5/xfVtxcYtvQFyVf4DtOZ4dgn5sMUSyTZ2Hu4YSmVKjoWozqLiGp8T3CE9He/BwIm2y vd2RpWkUJju4JJRbm0kHl7m1mri5Kuvsb/d0CUyQb6AJ/8INC8TC18IOSjgLcfz3vfZK KetLkr2DJxzXbCYJEwIonZhJZfPZg3jTdGWxS9AdU5NKGnpZeIPtEscx/ZSlsJX5T74I e4ufQuSHAqFJQmQKPTet4bDQXbRHmjjYdeLEYzSP3ONRYoJYx6EjlsDVJisCSQkJo257 V6jg== 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=QS/NX/rmoF4PVTXdslyOTsIHHYyFzCubmrSUdWwb/N0=; b=SctuWCIMPlbFX96Xq8BrxJam2TZn03uoK2n+TZldmiM8H1EQ05+/UAhphiPBb01zSG b4F7Nv/OcdkX2aGnu9MUg7wyG/xHukJ/tUcPpmRbQLSWvQaA+p2IofFpenmaU0CxbWVo Hie7Jjq8YnHXv4mzUlR+PiEEcstgsqb9d36l8PkAONxtejGci9NQGg8HYRhe7rQ+fWX6 bE1lagR0WeWNa8sof9b9tgUcZMjbrBwXrGzHVbgQmJ/hssNebuQjcqhcvLaNCTNOaS5C 7A8QKgKYDQdwJA+GG5CfnNoLyN/+P0Me3T8Nu+tTHxCs+sw1LfcKStzXfqPZdOa0B+su oziw== X-Gm-Message-State: AHQUAubzQphTi7eVC4sK/a3SHgmN3hYZ5pfNiZrQcYbJdnvzxy0YaDcc Min346k4U3XK9ELrInpO9EU= X-Google-Smtp-Source: AHgI3IYoQP0DcZpZufbq/KBQFiK/aayePlEEYWrTYBCQHxdXmsLFX5H8gM8qJtO/XFXkl4iAsepNOg== X-Received: by 2002:adf:f7c9:: with SMTP id a9mr4494350wrq.39.1550002022081; Tue, 12 Feb 2019 12:07:02 -0800 (PST) Received: from ?IPv6:2003:ea:8bf1:e200:788d:bad3:281d:6cd0? (p200300EA8BF1E200788DBAD3281D6CD0.dip0.t-ipconnect.de. [2003:ea:8bf1:e200:788d:bad3:281d:6cd0]) by smtp.googlemail.com with ESMTPSA id n3sm4174554wmf.46.2019.02.12.12.07.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 Feb 2019 12:07:01 -0800 (PST) Subject: Re: [PATCH net] net: phy: fix interrupt handling in non-started states To: Andrew Lunn Cc: Florian Fainelli , David Miller , "netdev@vger.kernel.org" , Russell King - ARM Linux References: <25e86edc-0b88-8c03-b692-776e971331f2@gmail.com> <20190212193716.GD7527@lunn.ch> From: Heiner Kallweit Message-ID: Date: Tue, 12 Feb 2019 21:06:53 +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: <20190212193716.GD7527@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 20:37, Andrew Lunn wrote: > On Tue, Feb 12, 2019 at 07:56:15PM +0100, Heiner Kallweit wrote: >> phylib enables interrupts before phy_start() has been called, and if >> we receive an interrupt in a non-started state, the interrupt handler >> returns IRQ_NONE. This causes problems with at least one Marvell chip >> as reported by Andrew. >> Fix this by handling interrupts the same as in phy_mac_interrupt(), >> basically always running the phylib state machine. It knows when it >> has to do something and when not. >> This change allows to handle interrupts gracefully even if they >> occur in a non-started state. >> >> Fixes: 2b3e88ea6528 ("net: phy: improve phy state checking") >> Reported-by: Andrew Lunn >> Signed-off-by: Heiner Kallweit >> --- >> drivers/net/phy/phy.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c >> index 189cd2048c3a..ca5e0c0f018c 100644 >> --- a/drivers/net/phy/phy.c >> +++ b/drivers/net/phy/phy.c >> @@ -762,9 +762,6 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat) >> { >> struct phy_device *phydev = phy_dat; >> >> - if (!phy_is_started(phydev)) >> - return IRQ_NONE; /* It can't be ours. */ >> - >> if (phydev->drv->did_interrupt && !phydev->drv->did_interrupt(phydev)) >> return IRQ_NONE; > > Hi Heiner > > Did you look at > Thanks for this interesting old read. > ommit 3c3070d713d798f7f9e7ee3614e49b47655d14d8 > Author: Maciej W. Rozycki > Date: Tue Oct 3 16:18:35 2006 +0100 > > [PATCH] 2.6.18: sb1250-mac: Phylib IRQ handling fixes > > This patch fixes a couple of problems discovered with interrupt handling > in the phylib core, namely: > > 1. The driver uses timer and workqueue calls, but does not include > nor . > > 2. The driver uses schedule_work() for handling interrupts, but does not > make sure any pending work scheduled thus has been completed before > driver's structures get freed from memory. This is especially > important as interrupts may keep arriving if the line is shared with > another PHY. > Since that time this have massively changed and I *think* this doesn't apply any longer. cancel_delayed_work_sync() is called always before driver structure is freed. > The solution is to ignore phy_interrupt() calls if the reported device > has already been halted and calling flush_scheduled_work() from > phy_stop_interrupts() (but guarded with current_is_keventd() in case > the function has been called through keventd from the MAC device's > close call to avoid a deadlock on the netlink lock). > > Signed-off-by: Maciej W. Rozycki > > patch-mips-2.6.18-20060920-phy-irq-16 > Signed-off-by: Jeff Garzik > > There has been a lot of change since then, so maybe this is no longer > an issue? > AFAICS, yes. I'll have a closer look at the scenario described by Russell, this however should be independent of the patch here. > Thanks > Andrew > Heiner