Received: by 2002:a25:8b91:0:0:0:0:0 with SMTP id j17csp3082922ybl; Sun, 12 Jan 2020 09:19:44 -0800 (PST) X-Google-Smtp-Source: APXvYqxNThTRg7g+/eSAg57c3cd2hWGss1aLBjV7wBN+8L162sLgp2b9p718cnB+zmuif/ke1Db0 X-Received: by 2002:a9d:c42:: with SMTP id 60mr10031047otr.182.1578849584086; Sun, 12 Jan 2020 09:19:44 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1578849584; cv=none; d=google.com; s=arc-20160816; b=hVQOmlLaj80/wPnnp1/pQzSzSsUaMaN1N/GBeWBU1pvYOQ44IkItH4Q0CtegFiarIV AtU2zOd2l7crtYE5E65H3JVopw4lviY6lc0vHdTgH/Wx4Pph1pkGQnbHiO0ocF8RWQeH S8JvxUcZjCXSVIWfKiJztZw0gv27Yix5BxefuD+tgRo0uo0VSu7SVT5Ci1RI2+pGRzb9 qf9GopWqOGN3Wo94sikhKeZ0GOTBUTKwDxu3XRKsU4CWtjmKCelbLJnEK//5cJhT+l9x JD0Vc593/o0ZssDax1bgDnVl1QCSykRDVzTTetusdg75MUGjyxQOt/f7YsJ3LA19zwzh MCpw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=R9OJk1oEYttgvht3VdngYW5fu/JjHOhoNUqTnPVMWnU=; b=kOSk95Vpa0271tuqlFrWmefDN/C7ZceA0EGjdMtH1ECMqS8+CMJnOTz+AzR4XP1vU2 KQg39mc1Ay8ZlJX03+Fu/4IfTe3zS2bvPZU7Gync48OpkBJZ6ZH1pRc7j1dcq3c8IBiU Fmm3vPwxfcRsJwDNK1tXxsBlziMMqoDVUcqnfhFG36y1Np3j7JjU8oGanIRLmxCHdRZK AEqLo0LFL04wV/VGhWyT0WL/xUoA+FycJORMnCP3/E8GmgmLYOGVNCflRwdk5istwBQA UlcFTslB/y4lF3BB03qb0hXYr3a15AGKRo5C25Z35xOvQ8vsCDDjNDUPM3RgHkMkWeaw ZP8Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=a2yd1c8J; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id x3si4518115oie.146.2020.01.12.09.19.32; Sun, 12 Jan 2020 09:19:44 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=a2yd1c8J; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1733227AbgALRSK (ORCPT + 99 others); Sun, 12 Jan 2020 12:18:10 -0500 Received: from mail-lj1-f195.google.com ([209.85.208.195]:46099 "EHLO mail-lj1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1733121AbgALRRq (ORCPT ); Sun, 12 Jan 2020 12:17:46 -0500 Received: by mail-lj1-f195.google.com with SMTP id m26so7397764ljc.13; Sun, 12 Jan 2020 09:17:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=R9OJk1oEYttgvht3VdngYW5fu/JjHOhoNUqTnPVMWnU=; b=a2yd1c8JpTdD0L1YOZRevlo1k9bl8vO7wygGJqnk8dhasnQnk7v8vfPkR/b2U+YcvH bh0ZbLUWGnMPyi0yE7onH/EnkHWPGzVD1f1duKuZGac+n7TkGEG/REf9WxaXGulPyID7 dZ0yLHLxZgmHvMdCaio7OxTQ8wIRxP9yliBXIpEqwtCTYY64xzGD6FL222ul0L7Q/ocp 9spydsV9GLQOs8+z68UbnlRLKu+ZpLrJErBDff14vqRYTYmmEZA+lZrvEmEe6D+qaLlc Hj/w/McySX4gocvNegDVFrz8kHaoCFvdZ9O9P8WYesUjhPfdH/Umj0kIeVln52r45fLQ bXjw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=R9OJk1oEYttgvht3VdngYW5fu/JjHOhoNUqTnPVMWnU=; b=ENaK4jv3I+lH4jVc1W75CtGbn9CLrjLPzCJdn0gtATuDc4kWuAPy/999TtqRzXm7Jt ACPwPvlvpjC7rN++KfWlrFNdIfx4D/in/c7HHxPFzwdnlpU3mjyw4/tJgH+KbmvZN5rW XTKI/yL81mY3Ff65gigqufB0kI7OemgV5kxSTAg83XwXTvTZGLLlrf7MPocVzJMl57bO aFEr9kqqcsajhJ+W7xifXVFjwj6YUn5UwECNGExPyIm+S6jllPGRl3ij7Jn2eMRa+3Wc WxXzvEnQfbqeiViGJ/7En9CDfLt/EfcvfI2E6I+koVlPZHTcOWsI7UKvwr6HbwRITSJJ M8hg== X-Gm-Message-State: APjAAAULKOrO6Xp3OwSUgVtBLomCDgGncRIg6a9onAaPviQDTfqUgjIn O7kjnOXWwpgCfHgETDETwZfLv3ks X-Received: by 2002:a2e:5357:: with SMTP id t23mr8514219ljd.227.1578849462208; Sun, 12 Jan 2020 09:17:42 -0800 (PST) Received: from localhost.localdomain (79-139-233-37.dynamic.spd-mgts.ru. [79.139.233.37]) by smtp.gmail.com with ESMTPSA id i13sm4506628ljg.89.2020.01.12.09.17.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 12 Jan 2020 09:17:41 -0800 (PST) From: Dmitry Osipenko To: Thierry Reding , Jonathan Hunter , Laxman Dewangan , Mikko Perttunen , Wolfram Sang Cc: linux-i2c@vger.kernel.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 3/8] i2c: tegra: Prevent interrupt triggering after transfer timeout Date: Sun, 12 Jan 2020 20:14:25 +0300 Message-Id: <20200112171430.27219-4-digetx@gmail.com> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20200112171430.27219-1-digetx@gmail.com> References: <20200112171430.27219-1-digetx@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Potentially it is possible that interrupt may fire after transfer timeout. That may not end up well for the next transfer because interrupt handling may race with hardware resetting. This is very unlikely to happen in practice, but anyway let's prevent the potential problem by enabling interrupt only at the moments when it is actually necessary to get some interrupt event. Tested-by: Thierry Reding Signed-off-by: Dmitry Osipenko --- drivers/i2c/busses/i2c-tegra.c | 70 +++++++++++++++++----------------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 61339c665ebd..882b283e0ed7 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -230,7 +231,6 @@ struct tegra_i2c_hw_feature { * @base_phys: physical base address of the I2C controller * @cont_id: I2C controller ID, used for packet header * @irq: IRQ number of transfer complete interrupt - * @irq_disabled: used to track whether or not the interrupt is enabled * @is_dvc: identifies the DVC I2C controller, has a different register layout * @msg_complete: transfer completion notifier * @msg_err: error code for completed message @@ -240,7 +240,6 @@ struct tegra_i2c_hw_feature { * @bus_clk_rate: current I2C bus clock rate * @clk_divisor_non_hs_mode: clock divider for non-high-speed modes * @is_multimaster_mode: track if I2C controller is in multi-master mode - * @xfer_lock: lock to serialize transfer submission and processing * @tx_dma_chan: DMA transmit channel * @rx_dma_chan: DMA receive channel * @dma_phys: handle to DMA resources @@ -260,7 +259,6 @@ struct tegra_i2c_dev { phys_addr_t base_phys; int cont_id; int irq; - bool irq_disabled; int is_dvc; struct completion msg_complete; int msg_err; @@ -270,8 +268,6 @@ struct tegra_i2c_dev { u32 bus_clk_rate; u16 clk_divisor_non_hs_mode; bool is_multimaster_mode; - /* xfer_lock: lock to serialize transfer submission and processing */ - spinlock_t xfer_lock; struct dma_chan *tx_dma_chan; struct dma_chan *rx_dma_chan; dma_addr_t dma_phys; @@ -790,11 +786,6 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev, bool clk_reinit) if (err) return err; - if (i2c_dev->irq_disabled) { - i2c_dev->irq_disabled = false; - enable_irq(i2c_dev->irq); - } - return 0; } @@ -825,18 +816,12 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id) status = i2c_readl(i2c_dev, I2C_INT_STATUS); - spin_lock(&i2c_dev->xfer_lock); if (status == 0) { dev_warn(i2c_dev->dev, "irq status 0 %08x %08x %08x\n", i2c_readl(i2c_dev, I2C_PACKET_TRANSFER_STATUS), i2c_readl(i2c_dev, I2C_STATUS), i2c_readl(i2c_dev, I2C_CNFG)); i2c_dev->msg_err |= I2C_ERR_UNKNOWN_INTERRUPT; - - if (!i2c_dev->irq_disabled) { - disable_irq_nosync(i2c_dev->irq); - i2c_dev->irq_disabled = true; - } goto err; } @@ -925,7 +910,6 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id) complete(&i2c_dev->msg_complete); done: - spin_unlock(&i2c_dev->xfer_lock); return IRQ_HANDLED; } @@ -999,6 +983,30 @@ static void tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev, i2c_writel(i2c_dev, val, reg); } +static unsigned long +tegra_i2c_wait_completion_timeout(struct tegra_i2c_dev *i2c_dev, + struct completion *complete, + unsigned int timeout_ms) +{ + unsigned long ret; + + enable_irq(i2c_dev->irq); + ret = wait_for_completion_timeout(complete, + msecs_to_jiffies(timeout_ms)); + disable_irq(i2c_dev->irq); + + /* + * There is a chance that completion may happen after IRQ + * synchronization, which is done by disable_irq(). + */ + if (ret == 0 && completion_done(complete)) { + dev_warn(i2c_dev->dev, "completion done after timeout\n"); + ret = 1; + } + + return ret; +} + static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap) { struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap); @@ -1020,8 +1028,8 @@ static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap) i2c_writel(i2c_dev, reg, I2C_BUS_CLEAR_CNFG); tegra_i2c_unmask_irq(i2c_dev, I2C_INT_BUS_CLR_DONE); - time_left = wait_for_completion_timeout(&i2c_dev->msg_complete, - msecs_to_jiffies(50)); + time_left = tegra_i2c_wait_completion_timeout( + i2c_dev, &i2c_dev->msg_complete, 50); if (time_left == 0) { dev_err(i2c_dev->dev, "timed out for bus clear\n"); return -ETIMEDOUT; @@ -1044,7 +1052,6 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, u32 packet_header; u32 int_mask; unsigned long time_left; - unsigned long flags; size_t xfer_size; u32 *buffer = NULL; int err = 0; @@ -1075,7 +1082,6 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, */ xfer_time += DIV_ROUND_CLOSEST(((xfer_size * 9) + 2) * MSEC_PER_SEC, i2c_dev->bus_clk_rate); - spin_lock_irqsave(&i2c_dev->xfer_lock, flags); int_mask = I2C_INT_NO_ACK | I2C_INT_ARBITRATION_LOST; tegra_i2c_unmask_irq(i2c_dev, int_mask); @@ -1090,7 +1096,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, dev_err(i2c_dev->dev, "starting RX DMA failed, err %d\n", err); - goto unlock; + return err; } } else { @@ -1149,7 +1155,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, dev_err(i2c_dev->dev, "starting TX DMA failed, err %d\n", err); - goto unlock; + return err; } } else { tegra_i2c_fill_tx_fifo(i2c_dev); @@ -1169,15 +1175,10 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, dev_dbg(i2c_dev->dev, "unmasked irq: %02x\n", i2c_readl(i2c_dev, I2C_INT_MASK)); -unlock: - spin_unlock_irqrestore(&i2c_dev->xfer_lock, flags); - if (dma) { - if (err) - return err; + time_left = tegra_i2c_wait_completion_timeout( + i2c_dev, &i2c_dev->dma_complete, xfer_time); - time_left = wait_for_completion_timeout(&i2c_dev->dma_complete, - msecs_to_jiffies(xfer_time)); if (time_left == 0) { dev_err(i2c_dev->dev, "DMA transfer timeout\n"); dmaengine_terminate_sync(i2c_dev->msg_read ? @@ -1202,13 +1203,13 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, i2c_dev->tx_dma_chan); } - time_left = wait_for_completion_timeout(&i2c_dev->msg_complete, - msecs_to_jiffies(xfer_time)); + time_left = tegra_i2c_wait_completion_timeout( + i2c_dev, &i2c_dev->msg_complete, xfer_time); + tegra_i2c_mask_irq(i2c_dev, int_mask); if (time_left == 0) { dev_err(i2c_dev->dev, "i2c transfer timed out\n"); - tegra_i2c_init(i2c_dev, true); return -ETIMEDOUT; } @@ -1568,7 +1569,6 @@ static int tegra_i2c_probe(struct platform_device *pdev) I2C_PACKET_HEADER_SIZE; init_completion(&i2c_dev->msg_complete); init_completion(&i2c_dev->dma_complete); - spin_lock_init(&i2c_dev->xfer_lock); if (!i2c_dev->hw->has_single_clk_source) { fast_clk = devm_clk_get(&pdev->dev, "fast-clk"); @@ -1644,6 +1644,8 @@ static int tegra_i2c_probe(struct platform_device *pdev) goto release_dma; } + irq_set_status_flags(i2c_dev->irq, IRQ_NOAUTOEN); + ret = devm_request_irq(&pdev->dev, i2c_dev->irq, tegra_i2c_isr, 0, dev_name(&pdev->dev), i2c_dev); if (ret) { -- 2.24.0