From patchwork Mon Aug 23 21:41:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Vasut X-Patchwork-Id: 501652 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=-15.9 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, UNWANTED_LANGUAGE_BODY, USER_AGENT_GIT 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 35EBBC4338F for ; Mon, 23 Aug 2021 21:42:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1CC69613D2 for ; Mon, 23 Aug 2021 21:42:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232956AbhHWVnB (ORCPT ); Mon, 23 Aug 2021 17:43:01 -0400 Received: from phobos.denx.de ([85.214.62.61]:40760 "EHLO phobos.denx.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232503AbhHWVnA (ORCPT ); Mon, 23 Aug 2021 17:43:00 -0400 Received: from tr.lan (ip-89-176-112-137.net.upcbroadband.cz [89.176.112.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id E7ACA81FC6; Mon, 23 Aug 2021 23:42:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1629754936; bh=GU37FIeuFBtxTf9ROaPkhRsxalMDB2svF77w7psfyL0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=gvSel/JXZ4i/xqWEGggRxEpIsi/JneAt85N0q8suAfg54805QaOb8pJsiNPK/aKcr YlnMbxBcsvB/axzygX6hWzs0N1Sq/CmUrZO7z/NVYKMOowoBhT5SmLGiwy/DfQXvpp gB9UZfgrSm+KpKdzKoUfBg2DPiSkyO7HQLbRaD3+Ye0om7SAgReI+9ID2OWQT6nWfo Rwjvk7tAvfEoVlNZICSD35WG5dLCwFUlBI4/FCsONvdtL9MCY9A3ZrC53URKLELQvA RW3/eznDUyjJK5LMiAdb4RLKlaSuP5iYpusicM1Z62g2nj4ITdt07HdcrMlBz3Gwpo O2RPqD/UVV60w== From: Marek Vasut To: linux-i2c@vger.kernel.org Cc: Marek Vasut , Michal Simek , Shubhrajyoti Datta , Wolfram Sang Subject: [PATCH v2 1/6] i2c: xiic: Fix broken locking on tx_msg Date: Mon, 23 Aug 2021 23:41:40 +0200 Message-Id: <20210823214145.295104-2-marex@denx.de> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210823214145.295104-1-marex@denx.de> References: <20210823214145.295104-1-marex@denx.de> MIME-Version: 1.0 X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org The tx_msg is set from multiple places, sometimes without locking, which fall apart on any SMP system. Only ever access tx_msg inside the driver mutex. Signed-off-by: Marek Vasut Cc: Michal Simek Cc: Shubhrajyoti Datta Cc: Wolfram Sang --- V2: Initialize RX message pointer in xiic_start_xfer() too --- drivers/i2c/busses/i2c-xiic.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index bb93db98404ef..50320dd32eea9 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -170,7 +170,7 @@ struct xiic_i2c { #define xiic_tx_space(i2c) ((i2c)->tx_msg->len - (i2c)->tx_pos) #define xiic_rx_space(i2c) ((i2c)->rx_msg->len - (i2c)->rx_pos) -static int xiic_start_xfer(struct xiic_i2c *i2c); +static int xiic_start_xfer(struct xiic_i2c *i2c, struct i2c_msg *msgs, int num); static void __xiic_start_xfer(struct xiic_i2c *i2c); /* @@ -684,15 +684,25 @@ static void __xiic_start_xfer(struct xiic_i2c *i2c) } -static int xiic_start_xfer(struct xiic_i2c *i2c) +static int xiic_start_xfer(struct xiic_i2c *i2c, struct i2c_msg *msgs, int num) { int ret; + mutex_lock(&i2c->lock); + ret = xiic_busy(i2c); + if (ret) + goto out; + + i2c->tx_msg = msgs; + i2c->rx_msg = NULL; + i2c->nmsgs = num; + ret = xiic_reinit(i2c); if (!ret) __xiic_start_xfer(i2c); +out: mutex_unlock(&i2c->lock); return ret; @@ -710,14 +720,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) if (err < 0) return err; - err = xiic_busy(i2c); - if (err) - goto out; - - i2c->tx_msg = msgs; - i2c->nmsgs = num; - - err = xiic_start_xfer(i2c); + err = xiic_start_xfer(i2c, msgs, num); if (err < 0) { dev_err(adap->dev.parent, "Error xiic_start_xfer\n"); goto out; @@ -725,9 +728,11 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) if (wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) || (i2c->state == STATE_DONE), HZ)) { + mutex_lock(&i2c->lock); err = (i2c->state == STATE_DONE) ? num : -EIO; goto out; } else { + mutex_lock(&i2c->lock); i2c->tx_msg = NULL; i2c->rx_msg = NULL; i2c->nmsgs = 0; @@ -735,6 +740,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) goto out; } out: + mutex_unlock(&i2c->lock); pm_runtime_mark_last_busy(i2c->dev); pm_runtime_put_autosuspend(i2c->dev); return err; From patchwork Mon Aug 23 21:41:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Vasut X-Patchwork-Id: 501650 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=-18.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT 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 7A13FC4320E for ; Mon, 23 Aug 2021 21:42:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 604A0613D0 for ; Mon, 23 Aug 2021 21:42:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232963AbhHWVnC (ORCPT ); Mon, 23 Aug 2021 17:43:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45658 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232954AbhHWVnC (ORCPT ); Mon, 23 Aug 2021 17:43:02 -0400 Received: from phobos.denx.de (phobos.denx.de [IPv6:2a01:238:438b:c500:173d:9f52:ddab:ee01]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C82A3C06175F for ; Mon, 23 Aug 2021 14:42:18 -0700 (PDT) Received: from tr.lan (ip-89-176-112-137.net.upcbroadband.cz [89.176.112.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 19CD582C2C; Mon, 23 Aug 2021 23:42:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1629754937; bh=Iozl/1Vu3QETYuwoTOyVg9yzpnCMEX0kBJLxqlhhOr0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WC3j7EnZqNb1jHPAZDMah88Y0RhQIVGzqkJU2fxkpPpVOeNX3+0u8llAJWkCQlUpz +WG3iJFe+S5a1NY+IIIOv2MBpEEbYGJ/0QSl4l0v0YcMlOCjw0zYtBPSIrG7YtuD3/ py+5ptg9qHWcALxzDoJm6kdxD7qaLseN+GIXREoGuo9TYfHTmq9UMNJSS3LZWkZfKO CatsMl1cOTsbVoVTzPlEE+gLsOaAU4iaBFdgKyMAnIrPJJeZKtNkDy7qve6FqfJBnQ ESfd0oH7I4fHjuZXCz7bXFH1MQhOgor7k/Q04KHEO4a1n244dFQT1w32a0HWxOiPjv wzLaEow5aM2EQ== From: Marek Vasut To: linux-i2c@vger.kernel.org Cc: Marek Vasut , Michal Simek , Shubhrajyoti Datta , Wolfram Sang Subject: [PATCH v2 4/6] i2c: xiic: Switch from waitqueue to completion Date: Mon, 23 Aug 2021 23:41:43 +0200 Message-Id: <20210823214145.295104-5-marex@denx.de> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210823214145.295104-1-marex@denx.de> References: <20210823214145.295104-1-marex@denx.de> MIME-Version: 1.0 X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org There will never be threads queueing up in the xiic_xmit(), use completion synchronization primitive to wait for the interrupt handler thread to complete instead as it is much better fit and there is no need to overload it for this purpose. Signed-off-by: Marek Vasut Cc: Michal Simek Cc: Shubhrajyoti Datta Cc: Wolfram Sang --- V2: Add separate timeout for the transfer completion, since on SMP zynqmp it can happen that the interrupt handler thread is preempted for longer than a second (measurements indicate below 2 seconds most of the time). So add a separate timeout and make it long to give this a chance. --- drivers/i2c/busses/i2c-xiic.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index aecdeec579977..f7277629923ff 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -23,7 +23,7 @@ #include #include #include -#include +#include #include #include #include @@ -48,7 +48,7 @@ enum xiic_endian { * struct xiic_i2c - Internal representation of the XIIC I2C bus * @dev: Pointer to device structure * @base: Memory base of the HW registers - * @wait: Wait queue for callers + * @completion: Completion for callers * @adap: Kernel adapter representation * @tx_msg: Messages from above to be sent * @lock: Mutual exclusion @@ -64,7 +64,7 @@ enum xiic_endian { struct xiic_i2c { struct device *dev; void __iomem *base; - wait_queue_head_t wait; + struct completion completion; struct i2c_adapter adap; struct i2c_msg *tx_msg; struct mutex lock; @@ -160,6 +160,9 @@ struct xiic_i2c { #define XIIC_PM_TIMEOUT 1000 /* ms */ /* timeout waiting for the controller to respond */ #define XIIC_I2C_TIMEOUT (msecs_to_jiffies(1000)) +/* timeout waiting for the controller finish transfers */ +#define XIIC_XFER_TIMEOUT (msecs_to_jiffies(10000)) + /* * The following constant is used for the device global interrupt enable * register, to enable all interrupts for the device, this is the only bit @@ -367,7 +370,7 @@ static void xiic_wakeup(struct xiic_i2c *i2c, int code) i2c->rx_msg = NULL; i2c->nmsgs = 0; i2c->state = code; - wake_up(&i2c->wait); + complete(&i2c->completion); } static irqreturn_t xiic_process(int irq, void *dev_id) @@ -689,6 +692,7 @@ static int xiic_start_xfer(struct xiic_i2c *i2c, struct i2c_msg *msgs, int num) i2c->tx_msg = msgs; i2c->rx_msg = NULL; i2c->nmsgs = num; + init_completion(&i2c->completion); ret = xiic_reinit(i2c); if (!ret) @@ -715,23 +719,23 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) err = xiic_start_xfer(i2c, msgs, num); if (err < 0) { dev_err(adap->dev.parent, "Error xiic_start_xfer\n"); - goto out; + return err; } - if (wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) || - (i2c->state == STATE_DONE), HZ)) { - mutex_lock(&i2c->lock); - err = (i2c->state == STATE_DONE) ? num : -EIO; - goto out; - } else { - mutex_lock(&i2c->lock); + err = wait_for_completion_timeout(&i2c->completion, XIIC_XFER_TIMEOUT); + mutex_lock(&i2c->lock); + if (err == 0) { /* Timeout */ i2c->tx_msg = NULL; i2c->rx_msg = NULL; i2c->nmsgs = 0; err = -ETIMEDOUT; - goto out; + } else if (err < 0) { /* Completion error */ + i2c->tx_msg = NULL; + i2c->rx_msg = NULL; + i2c->nmsgs = 0; + } else { + err = (i2c->state == STATE_DONE) ? num : -EIO; } -out: mutex_unlock(&i2c->lock); pm_runtime_mark_last_busy(i2c->dev); pm_runtime_put_autosuspend(i2c->dev); @@ -793,7 +797,6 @@ static int xiic_i2c_probe(struct platform_device *pdev) i2c->adap.dev.of_node = pdev->dev.of_node; mutex_init(&i2c->lock); - init_waitqueue_head(&i2c->wait); i2c->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(i2c->clk)) From patchwork Mon Aug 23 21:41:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Vasut X-Patchwork-Id: 501649 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=-18.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT 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 1E07DC43216 for ; Mon, 23 Aug 2021 21:42:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F0774613CD for ; Mon, 23 Aug 2021 21:42:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232981AbhHWVnD (ORCPT ); Mon, 23 Aug 2021 17:43:03 -0400 Received: from phobos.denx.de ([85.214.62.61]:40868 "EHLO phobos.denx.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232970AbhHWVnC (ORCPT ); Mon, 23 Aug 2021 17:43:02 -0400 Received: from tr.lan (ip-89-176-112-137.net.upcbroadband.cz [89.176.112.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 77F4882D3C; Mon, 23 Aug 2021 23:42:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1629754937; bh=DXpoiNJN9yfkPKq5NcMQBP72L7KIlxhNlrAJAYLhDsY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=n44W0uYrZD7Zz9tqBajSCJzLV0bD1wOzvqGybnX8izGFXk0FYdxgDflyteshRWvA4 MJU/IZ7SKH4P6WSE4SrjjtVbR283MmCK0F8b904qVjc5xedKXkZlPlkwIKjoxLgarX NgRSp9o/G033bhcJbDCs9eO/VlmWMUIVkNyKZUK8gX4YZ6X79LKYD8YGnwfRhuspYE fEI98/HQSMmdTByBNsrnojzmcidSwJYJ+2vojoUJ649Y7LkR7sTYR1afyRbRg6pfO6 0Gi23qwfYlq1bGXJCUlA5sgCjVMtnkxAKQfzMNYww+ecV57xxLriCAqzNnq+ua/HMo lVZjyFkc/l5cw== From: Marek Vasut To: linux-i2c@vger.kernel.org Cc: Marek Vasut , Michal Simek , Shubhrajyoti Datta , Wolfram Sang Subject: [PATCH v2 5/6] i2c: xiic: Only ever transfer single message Date: Mon, 23 Aug 2021 23:41:44 +0200 Message-Id: <20210823214145.295104-6-marex@denx.de> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210823214145.295104-1-marex@denx.de> References: <20210823214145.295104-1-marex@denx.de> MIME-Version: 1.0 X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org Transferring multiple messages via XIIC suffers from strange interaction between the interrupt status/enable register flags. These flags are being reused in the hardware to indicate different things for read and write transfer, and doing multiple transactions becomes horribly complex. Just send a single transaction and reload the controller with another message once the transaction is done in the interrupt handler thread. Signed-off-by: Marek Vasut Cc: Michal Simek Cc: Shubhrajyoti Datta Cc: Wolfram Sang --- V2: No change --- drivers/i2c/busses/i2c-xiic.c | 44 ++++++++--------------------------- 1 file changed, 10 insertions(+), 34 deletions(-) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index f7277629923ff..6cd7830fe4898 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -609,8 +609,6 @@ static void xiic_start_send(struct xiic_i2c *i2c) { struct i2c_msg *msg = i2c->tx_msg; - xiic_irq_clr(i2c, XIIC_INTR_TX_ERROR_MASK); - dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d", __func__, msg, msg->len); dev_dbg(i2c->adap.dev.parent, "%s entry, ISR: 0x%x, CR: 0x%x\n", @@ -628,16 +626,17 @@ static void xiic_start_send(struct xiic_i2c *i2c) xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data); } - xiic_fill_tx_fifo(i2c); - /* Clear any pending Tx empty, Tx Error and then enable them. */ xiic_irq_clr_en(i2c, XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_ERROR_MASK | - XIIC_INTR_BNB_MASK); + XIIC_INTR_BNB_MASK | + ((i2c->nmsgs > 1 || xiic_tx_space(i2c)) ? + XIIC_INTR_TX_HALF_MASK : 0)); + + xiic_fill_tx_fifo(i2c); } static void __xiic_start_xfer(struct xiic_i2c *i2c) { - int first = 1; int fifo_space = xiic_tx_fifo_space(i2c); dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, fifos space: %d\n", __func__, i2c->tx_msg, fifo_space); @@ -648,35 +647,12 @@ static void __xiic_start_xfer(struct xiic_i2c *i2c) i2c->rx_pos = 0; i2c->tx_pos = 0; i2c->state = STATE_START; - while ((fifo_space >= 2) && (first || (i2c->nmsgs > 1))) { - if (!first) { - i2c->nmsgs--; - i2c->tx_msg++; - i2c->tx_pos = 0; - } else - first = 0; - - if (i2c->tx_msg->flags & I2C_M_RD) { - /* we dont date putting several reads in the FIFO */ - xiic_start_recv(i2c); - return; - } else { - xiic_start_send(i2c); - if (xiic_tx_space(i2c) != 0) { - /* the message could not be completely sent */ - break; - } - } - - fifo_space = xiic_tx_fifo_space(i2c); + if (i2c->tx_msg->flags & I2C_M_RD) { + /* we dont date putting several reads in the FIFO */ + xiic_start_recv(i2c); + } else { + xiic_start_send(i2c); } - - /* there are more messages or the current one could not be completely - * put into the FIFO, also enable the half empty interrupt - */ - if (i2c->nmsgs > 1 || xiic_tx_space(i2c)) - xiic_irq_clr_en(i2c, XIIC_INTR_TX_HALF_MASK); - } static int xiic_start_xfer(struct xiic_i2c *i2c, struct i2c_msg *msgs, int num)