From patchwork Mon Feb 4 18:37:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: John David Anglin X-Patchwork-Id: 1036147 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=bell.net Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 43tbzJ6SRDz9s4V for ; Tue, 5 Feb 2019 05:37:16 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729290AbfBDShP (ORCPT ); Mon, 4 Feb 2019 13:37:15 -0500 Received: from belmont80srvr.owm.bell.net ([184.150.200.80]:49623 "EHLO mtlfep02.bell.net" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727917AbfBDShO (ORCPT ); Mon, 4 Feb 2019 13:37:14 -0500 Received: from bell.net mtlfep02 184.150.200.30 by mtlfep02.bell.net with ESMTP id <20190204183713.XJNO27401.mtlfep02.bell.net@mtlspm02.bell.net> for ; Mon, 4 Feb 2019 13:37:13 -0500 Received: from [192.168.0.138] (really [64.231.92.241]) by mtlspm02.bell.net with ESMTP id <20190204183713.LKNV26679.mtlspm02.bell.net@[192.168.0.138]>; Mon, 4 Feb 2019 13:37:13 -0500 From: John David Anglin To: Andrew Lunn Cc: Russell King , Vivien Didelot , Florian Fainelli , netdev@vger.kernel.org References: <49eec816-9238-c893-0860-602aa8965515@bell.net> <20190122202834.GB12052@lunn.ch> <09fce2e0-9dd2-ea30-7e7d-72f04eede68b@bell.net> <20190122223649.GD3634@lunn.ch> <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> Openpgp: preference=signencrypt Subject: [PATCH] net: phylink: dsa: mv88e6xxx: Revise irq setup ordering Message-ID: <53b49df8-53ed-704f-9197-230b18d83090@bell.net> Date: Mon, 4 Feb 2019 13:37:13 -0500 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-Cloudmark-Analysis: v=2.2 cv=O6bWhF1W c=1 sm=0 tr=0 a=jJmVOGxBrKxWvcjqaG+oVg==:17 a=IkcTkHD0fZMA:10 a=CFTnQlWoA9kA:10 a=FBHGMhGWAAAA:8 a=urtSqo2nV6wmkLt8OgcA:9 a=QEXdDO2ut3YA:10 a=9gvnlMMaQFpL9xblJ6ne:22 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org This change fixes a race condition in the setup of hardware irqs and the code enabling PHY link detection. This 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 control1, 2) setup the hardware irq, and 3) perform the remaining common setup. This in fact simplifies the setup and allows some unnecessary code to be removed. In order to prevent races in mv88e6xxx_g1_irq_thread_work(), we clear and restore the interrupt enables in the normal path just prior to exit.      mutex_lock(&chip->reg_lock); @@ -277,6 +277,14 @@ static irqreturn_t mv88e6xxx_g1_irq_thread_work(struct mv88e6xxx_chip *chip)              ++nhandled;          }      } + +    mutex_lock(&chip->reg_lock); +    mv88e6xxx_g1_read(chip, MV88E6XXX_G1_CTL1, ®); +    mask = ~GENMASK(chip->g1_irq.nirqs, 0); +    mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, (reg & mask)); +    mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, reg); +    mutex_unlock(&chip->reg_lock); +  out:      return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE);  } @@ -374,10 +382,30 @@ static void mv88e6xxx_g1_irq_free(struct mv88e6xxx_chip *chip)      mutex_unlock(&chip->reg_lock);  }   +static int mv88e6xxx_g1_irq_setup_masks(struct mv88e6xxx_chip *chip) +{ +    int err; +    u16 reg; + +    /* The INTn output must be high when hardware interrupts are setup. +       The EEPROM done interrupt enable is set on reset, so clear all +       interrupt enable bits to ensure INTn is not driven low */ +    err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_CTL1, ®); +    if (err) +        return err; +    reg &= ~GENMASK(chip->info->g1_irqs, 0); +    err = mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, reg); +    if (err) +        return err; + +    /* Reading the interrupt status clears (most of) them */ +    err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_STS, ®); +    return err; +} +  static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)  { -    int err, irq, virq; -    u16 reg, mask; +    int irq;        chip->g1_irq.nirqs = chip->info->g1_irqs;      chip->g1_irq.domain = irq_domain_add_simple( @@ -392,43 +420,14 @@ static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)      chip->g1_irq.chip = mv88e6xxx_g1_irq_chip;      chip->g1_irq.masked = ~0;   -    err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_CTL1, &mask); -    if (err) -        goto out_mapping; - -    mask &= ~GENMASK(chip->g1_irq.nirqs, 0); - -    err = mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, mask); -    if (err) -        goto out_disable; - -    /* Reading the interrupt status clears (most of) them */ -    err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_STS, ®); -    if (err) -        goto out_disable; -      return 0; - -out_disable: -    mask &= ~GENMASK(chip->g1_irq.nirqs, 0); -    mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, mask); - -out_mapping: -    for (irq = 0; irq < 16; irq++) { -        virq = irq_find_mapping(chip->g1_irq.domain, irq); -        irq_dispose_mapping(virq); -    } - -    irq_domain_remove(chip->g1_irq.domain); - -    return err;  }    static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)  {      int err;   -    err = mv88e6xxx_g1_irq_setup_common(chip); +    err = mv88e6xxx_g1_irq_setup_masks(chip);      if (err)          return err;   @@ -437,9 +436,9 @@ static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)                     IRQF_ONESHOT | IRQF_SHARED,                     dev_name(chip->dev), chip);      if (err) -        mv88e6xxx_g1_irq_free_common(chip); +        return err;   -    return err; +    return mv88e6xxx_g1_irq_setup_common(chip);  }    static void mv88e6xxx_irq_poll(struct kthread_work *work) @@ -457,6 +456,10 @@ static int mv88e6xxx_irq_poll_setup(struct mv88e6xxx_chip *chip)  {      int err;   +    err = mv88e6xxx_g1_irq_setup_masks(chip); +    if (err) +        return err; +      err = mv88e6xxx_g1_irq_setup_common(chip);      if (err)          return err; Signed-off-by: John David Anglin diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index b2a0e59b6252..0dcbc25c1b4b 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -260,7 +260,7 @@ static irqreturn_t mv88e6xxx_g1_irq_thread_work(struct mv88e6xxx_chip *chip)      unsigned int nhandled = 0;      unsigned int sub_irq;      unsigned int n; -    u16 reg; +    u16 mask, reg;      int err;