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=-8.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,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 420F0C2F421 for ; Mon, 21 Jan 2019 16:40:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0468521721 for ; Mon, 21 Jan 2019 16:40:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b="mDtBBlaV" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730189AbfAUQkl (ORCPT ); Mon, 21 Jan 2019 11:40:41 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:50553 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729075AbfAUQkk (ORCPT ); Mon, 21 Jan 2019 11:40: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; bh=R1AyCmEL1qhLwjWoGpN0yWafyyY+tT8HxupSCMHXhAo=; b=mDtBBlaV5P6ocizfNP4DBFLdiQfyUY0EP8sTa9TqQpDbV8pv0XWmfVCZL4RdUEt4nKzsr9/eSUaMY6k+F1iu608hPAMHGwX+PzJuuIMaKzHRzHiPGnNZpTsaqDU5JCt7qMTzU5pGQD2NWpLwtnd87/qIAKR7Qnyw88/2IrmPE78=; Received: from andrew by vps0.lunn.ch with local (Exim 4.84_2) (envelope-from ) id 1glccm-0002pd-Hl; Mon, 21 Jan 2019 17:40:36 +0100 Date: Mon, 21 Jan 2019 17:40:36 +0100 From: Andrew Lunn To: Heiner Kallweit Cc: Florian Fainelli , David Miller , "netdev@vger.kernel.org" Subject: Re: [PATCH net-next v2 2/4] net: phy: warn if phy_start is called from invalid state Message-ID: <20190121164036.GF8620@lunn.ch> References: <36a3f6a9-7f4c-7d51-05aa-71e847132102@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 On Sun, Jan 20, 2019 at 10:02:13AM +0100, Heiner Kallweit wrote: > phy_start() should be called from states PHY_READY or PHY_HALTED only. > Check for this to detect misbehaving drivers. Also the state machine > should be started only when being called from one of the valid states. > > Signed-off-by: Heiner Kallweit > --- > drivers/net/phy/phy.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index 3df6aadc5..fd928979b 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -861,9 +861,16 @@ void phy_start(struct phy_device *phydev) > > mutex_lock(&phydev->lock); > > + if (phydev->state != PHY_READY && phydev->state != PHY_HALTED) { > + WARN(1, "called from state %s\n", > + phy_state_to_str(phydev->state)); > + goto out; > + } Hi Heiner Warning is good. But jumping to out i'm not so sure about. Drivers which are 'broken' work well enough that users don't know they are broken. But jumping to out is going to really break them. It seems better to have the kernel only warn for one cycle so we find out about such drivers and fix them, and later add the goto out. Andrew