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 EE33AC282C4 for ; Mon, 4 Feb 2019 23:14:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A8B972081B for ; Mon, 4 Feb 2019 23:14:16 +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="iFsYUbn8" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726533AbfBDXOP (ORCPT ); Mon, 4 Feb 2019 18:14:15 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:38681 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726086AbfBDXOP (ORCPT ); Mon, 4 Feb 2019 18:14:15 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Transfer-Encoding:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: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=235quo8qS5l6drrcpITX4eNtgdwsbsaGo4xu+3VTu0Q=; b=iFsYUbn8+puxC6x2XPrRiaSKGL hsm8okwPg6yWojZNJnGvgUjX/UvvOZ7LbIPaz1iy8ceCJ0tgPnG7sBJmqxNvMlnFgUG74ccUv4iuE tQEqkqfw0/tsaAklNl6iYJlmVxwKOlR3/ozhV7uonwqY6NGD6Xtcvzu7GxstAluv04to=; Received: from andrew by vps0.lunn.ch with local (Exim 4.89) (envelope-from ) id 1gqnRK-0001xA-HC; Tue, 05 Feb 2019 00:14:10 +0100 Date: Tue, 5 Feb 2019 00:14:10 +0100 From: Andrew Lunn To: John David Anglin Cc: Russell King , Vivien Didelot , Florian Fainelli , netdev@vger.kernel.org Subject: Re: [PATCH v2] net: dsa: mv88e6xxx: Revise irq setup ordering Message-ID: <20190204231410.GG3397@lunn.ch> References: <20190123002240.GF3634@lunn.ch> <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> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <824d011b-3692-69c3-5e2c-58e950a80abf@bell.net> 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 Mon, Feb 04, 2019 at 04:59:13PM -0500, John David Anglin wrote: > This change fixes a race condition in the setup of hardware irqs and the > code enabling PHY link detection in the mv88e6xxx driver. > > This race was observed on the espressobin board where the GPIO interrupt > controller only supports edge interrupts.  If the INTn output pin goes low > before the GPIO interrupt is enabled, PHY link interrupts are not detected. > > With this change, we > 1) force INTn high by clearing all interrupt enables in global 1 control 1, > 2) setup the hardware irq, and then > 3) perform the remaining common setup. > > This simplifies the setup and allows some unnecessary code to be removed. Hi Dave I took a closer look now. I don't actually see why the current code is wrong. mv88e6xxx_g1_irq_setup() calls mv88e6xxx_g1_irq_setup_common() and then registers the interrupt handler. mv88e6xxx_g1_irq_setup_common() does what you want, it masks all interrupts in the hardware and clears any pending interrupts which can be cleared. The change you made is actually dangerous. As soon as you request the interrupt, it is live, it can fire, and call mv88e6xxx_g1_irq_thread_work(). That needs the irq domain. But the change you made defers the creating of the domain until after the interrupt is registered. So we can de-refernece a NULL pointer in the interrupt handler. Andrew