From patchwork Sun Mar 3 15:03:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wolfram Sang X-Patchwork-Id: 1050867 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.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=linux-i2c-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=sang-engineering.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 44C5y774Hqz9s1B for ; Mon, 4 Mar 2019 02:03:27 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726352AbfCCPDY (ORCPT ); Sun, 3 Mar 2019 10:03:24 -0500 Received: from sauhun.de ([88.99.104.3]:45684 "EHLO pokefinder.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726331AbfCCPDY (ORCPT ); Sun, 3 Mar 2019 10:03:24 -0500 Received: from localhost (p54B334AE.dip0.t-ipconnect.de [84.179.52.174]) by pokefinder.org (Postfix) with ESMTPSA id BED254B60A6; Sun, 3 Mar 2019 16:03:21 +0100 (CET) From: Wolfram Sang To: linux-i2c@vger.kernel.org Cc: linux-renesas-soc@vger.kernel.org, Hiromitsu Yamasaki , Wolfram Sang Subject: [PATCH 1/2] i2c: rcar: fix concurrency issue related to ICDMAER Date: Sun, 3 Mar 2019 16:03:13 +0100 Message-Id: <20190303150314.6528-2-wsa+renesas@sang-engineering.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20190303150314.6528-1-wsa+renesas@sang-engineering.com> References: <20190303150314.6528-1-wsa+renesas@sang-engineering.com> Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org From: Hiromitsu Yamasaki This patch fixes the problem that an interrupt may set up a new I2C message and the DMA callback overwrites this setup. By disabling the DMA Enable Register(ICDMAER), rcar_i2c_dma_unmap() enables interrupts for register settings (such as Master Control Register(ICMCR)) and advances the I2C transfer sequence. If an interrupt occurs immediately after ICDMAER is disabled, the callback handler later continues and overwrites the previous settings from the interrupt. So, disable ICDMAER at the end of the callback to ensure other interrupts are masked until then. Note that this driver needs to work lock-free because there are IP cores with a HW race condition which prevent us from using a spinlock in the interrupt handler. Reproduction test: 1. Add udelay(1) after disabling ICDMAER. (It is expected to generate an interrupt of rcar_i2c_irq()) void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv) { ... rcar_i2c_write(priv, ICDMAER, 0); udelay(1); <-- Add this line ... priv->dma_direction = DMA_NONE; } 2. Execute DMA transfer. Performs DMA transfer of read or write. In the sample, write was used. $ i2cset -y -f 4 0x6a 0x01 0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 i 3. A log message of BUG_ON() will be displayed. Signed-off-by: Hiromitsu Yamasaki Signed-off-by: Wolfram Sang Reviewed-by: Simon Horman --- drivers/i2c/busses/i2c-rcar.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c index dd52a068b140..6410896f717b 100644 --- a/drivers/i2c/busses/i2c-rcar.c +++ b/drivers/i2c/busses/i2c-rcar.c @@ -363,9 +363,6 @@ static void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv) struct dma_chan *chan = priv->dma_direction == DMA_FROM_DEVICE ? priv->dma_rx : priv->dma_tx; - /* Disable DMA Master Received/Transmitted */ - rcar_i2c_write(priv, ICDMAER, 0); - dma_unmap_single(chan->device->dev, sg_dma_address(&priv->sg), sg_dma_len(&priv->sg), priv->dma_direction); @@ -375,6 +372,9 @@ static void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv) priv->flags |= ID_P_NO_RXDMA; priv->dma_direction = DMA_NONE; + + /* Disable DMA Master Received/Transmitted */ + rcar_i2c_write(priv, ICDMAER, 0); } static void rcar_i2c_cleanup_dma(struct rcar_i2c_priv *priv) From patchwork Sun Mar 3 15:03:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wolfram Sang X-Patchwork-Id: 1050866 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.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=linux-i2c-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=sang-engineering.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 44C5y72KfZz9s4V for ; Mon, 4 Mar 2019 02:03:27 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726217AbfCCPDY (ORCPT ); Sun, 3 Mar 2019 10:03:24 -0500 Received: from sauhun.de ([88.99.104.3]:45696 "EHLO pokefinder.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726324AbfCCPDY (ORCPT ); Sun, 3 Mar 2019 10:03:24 -0500 Received: from localhost (p54B334AE.dip0.t-ipconnect.de [84.179.52.174]) by pokefinder.org (Postfix) with ESMTPSA id 5D4B54A0BD4; Sun, 3 Mar 2019 16:03:22 +0100 (CET) From: Wolfram Sang To: linux-i2c@vger.kernel.org Cc: linux-renesas-soc@vger.kernel.org, Wolfram Sang Subject: [PATCH 2/2] i2c: rcar: explain the lockless design Date: Sun, 3 Mar 2019 16:03:14 +0100 Message-Id: <20190303150314.6528-3-wsa+renesas@sang-engineering.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20190303150314.6528-1-wsa+renesas@sang-engineering.com> References: <20190303150314.6528-1-wsa+renesas@sang-engineering.com> Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org To make sure people can understand the lockless design of this driver without the need to dive into git history, add a comment giving an overview of the situation. Signed-off-by: Wolfram Sang Reviewed-by: Simon Horman --- drivers/i2c/busses/i2c-rcar.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c index 6410896f717b..3ce74edcd70c 100644 --- a/drivers/i2c/busses/i2c-rcar.c +++ b/drivers/i2c/busses/i2c-rcar.c @@ -611,6 +611,15 @@ static bool rcar_i2c_slave_irq(struct rcar_i2c_priv *priv) return true; } +/* + * This driver has a lock-free design because there are IP cores (at least + * R-Car Gen2) which have an inherent race condition in their hardware design. + * There, we need to clear RCAR_BUS_MASK_DATA bits as soon as possible after + * the interrupt was generated, otherwise an unwanted repeated message gets + * generated. It turned out that taking a spinlock at the beginning of the ISR + * was already causing repeated messages. Thus, this driver was converted to + * the now lockless behaviour. Please keep this in mind when hacking the driver. + */ static irqreturn_t rcar_i2c_irq(int irq, void *ptr) { struct rcar_i2c_priv *priv = ptr;