From patchwork Thu Dec 17 22:29:11 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 345298 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,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, 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 2A537C4361B for ; Thu, 17 Dec 2020 22:30:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 009D02376F for ; Thu, 17 Dec 2020 22:30:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732056AbgLQWaQ (ORCPT ); Thu, 17 Dec 2020 17:30:16 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54102 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732026AbgLQWaQ (ORCPT ); Thu, 17 Dec 2020 17:30:16 -0500 Received: from mail-pg1-x530.google.com (mail-pg1-x530.google.com [IPv6:2607:f8b0:4864:20::530]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E3EB7C0617A7 for ; Thu, 17 Dec 2020 14:29:35 -0800 (PST) Received: by mail-pg1-x530.google.com with SMTP id c22so46423pgg.13 for ; Thu, 17 Dec 2020 14:29:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=v7xCKWj505XOK8pmaVepIcbXuQ+piHeDyFLhHQR5U2Q=; b=MUIEHvh95AUP6V1mzbTcu2YJqrE/3xPIlBC0YJsbKW3ewh99f5g1nMnI0mXabiEg/B UroJpxJF6AzkjbS43C706tVZONxmo4K8NmNtdSn1RIU0XCnO0qFM4j4bY2tcd8IJt4z3 WF6i2QGXLrhaI3qashWV5StE1S7SDu8OKFN0M= 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:mime-version :content-transfer-encoding; bh=v7xCKWj505XOK8pmaVepIcbXuQ+piHeDyFLhHQR5U2Q=; b=CegQ/N+I4ZbHT2qOrJApF3Nrv/9xFjdLPC1E3mQTZOY0hN6PQxcMVrKNw6uTn6vEY/ YjTFCn1MVvrDkHrBhnAAGDLknUoK7iCevdPBW0eZOrTzPKwn4u+4YUcXhNzIE3nM3DZx 9GVtaOMP6+fCo2YdGxt5jx+IVN2AnDy+uiuApn7p1nk+00oJGyl1HmzcUuLhYTwVjMSe woaLZi7WBvYbtYugVEpf1YKggaBVDBW7cvpPd53+0whD+jH34N3tmn/RE3H2nHiHTbuY /Ko+ADqf3tydb31k6AGHiVpO1o0nc9Kga/MLy2Hue5q+wjOmEYnj7/ag2xfv0V52eAw0 9ElQ== X-Gm-Message-State: AOAM532Vm2rz4xqf5OxAzZaBqYpZBPMsjPJ4xhOuW0hrh/2R+am/3MQd K3RHE+9ug3lX6leh5o6ZrlYNeA== X-Google-Smtp-Source: ABdhPJz6Dtc4cRRUFyl0u5B3TKJYZN6oBuUE1DwLdrGi6ti46ehE1k4YtBfuwM8sp2oD3Q2patXwcg== X-Received: by 2002:aa7:93b0:0:b029:19d:e625:2062 with SMTP id x16-20020aa793b00000b029019de6252062mr1320109pff.47.1608244175337; Thu, 17 Dec 2020 14:29:35 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:42b0:34ff:fe3d:58e6]) by smtp.gmail.com with ESMTPSA id gm18sm5805850pjb.55.2020.12.17.14.29.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Dec 2020 14:29:34 -0800 (PST) From: Douglas Anderson To: Mark Brown Cc: msavaliy@qti.qualcomm.com, Stephen Boyd , akashast@codeaurora.org, Roja Rani Yarubandi , Douglas Anderson , Alok Chauhan , Andy Gross , Bjorn Andersson , Dilip Kota , Girish Mahadevan , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org Subject: [PATCH v3 1/4] spi: spi-geni-qcom: Fix geni_spi_isr() NULL dereference in timeout case Date: Thu, 17 Dec 2020 14:29:11 -0800 Message-Id: <20201217142842.v3.1.I99ee04f0cb823415df59bd4f550d6ff5756e43d6@changeid> X-Mailer: git-send-email 2.29.2.684.gfbc64c5ab5-goog MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org In commit 7ba9bdcb91f6 ("spi: spi-geni-qcom: Don't keep a local state variable") we changed handle_fifo_timeout() so that we set "mas->cur_xfer" to NULL to make absolutely sure that we don't mess with the buffers from the previous transfer in the timeout case. Unfortunately, this caused the IRQ handler to dereference NULL in some cases. One case: CPU0 CPU1 ---- ---- setup_fifo_xfer() geni_se_setup_m_cmd() ... handle_fifo_timeout() spin_lock_irq(mas->lock) mas->cur_xfer = NULL geni_se_cancel_m_cmd() spin_unlock_irq(mas->lock) geni_spi_isr() spin_lock(mas->lock) if (m_irq & M_RX_FIFO_WATERMARK_EN) geni_spi_handle_rx() mas->cur_xfer NULL dereference! tl;dr: Seriously delayed interrupts for RX/TX can lead to timeout handling setting mas->cur_xfer to NULL. Let's check for the NULL transfer in the TX and RX cases and reset the watermark or clear out the fifo respectively to put the hardware back into a sane state. NOTE: things still could get confused if we get timeouts all the way through handle_fifo_timeout() and then start a new transfer because interrupts from the old transfer / cancel / abort could still be pending. A future patch will help this corner case. Fixes: 561de45f72bd ("spi: spi-geni-qcom: Add SPI driver support for GENI based QUP") Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd Reviewed-by: Stephen Boyd Reviewed-by: Stephen Boyd --- Changes in v3: - (ptr == NULL) => (!ptr), take 2. - while loop => for loop Changes in v2: - (ptr == NULL) => (!ptr). - Addressed loop nits in geni_spi_handle_rx(). - Commit message rewording from Stephen. drivers/spi/spi-geni-qcom.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 25810a7eef10..6939c6cabe91 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -354,6 +354,12 @@ static bool geni_spi_handle_tx(struct spi_geni_master *mas) unsigned int bytes_per_fifo_word = geni_byte_per_fifo_word(mas); unsigned int i = 0; + /* Stop the watermark IRQ if nothing to send */ + if (!mas->cur_xfer) { + writel(0, se->base + SE_GENI_TX_WATERMARK_REG); + return false; + } + max_bytes = (mas->tx_fifo_depth - mas->tx_wm) * bytes_per_fifo_word; if (mas->tx_rem_bytes < max_bytes) max_bytes = mas->tx_rem_bytes; @@ -396,6 +402,14 @@ static void geni_spi_handle_rx(struct spi_geni_master *mas) if (rx_last_byte_valid && rx_last_byte_valid < 4) rx_bytes -= bytes_per_fifo_word - rx_last_byte_valid; } + + /* Clear out the FIFO and bail if nowhere to put it */ + if (!mas->cur_xfer) { + for (i = 0; i < DIV_ROUND_UP(rx_bytes, bytes_per_fifo_word); i++) + readl(se->base + SE_GENI_RX_FIFOn); + return; + } + if (mas->rx_rem_bytes < rx_bytes) rx_bytes = mas->rx_rem_bytes; From patchwork Thu Dec 17 22:29:12 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 345845 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,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable 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 BDD1FC2D0E4 for ; Thu, 17 Dec 2020 22:30:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8C09D2376F for ; Thu, 17 Dec 2020 22:30:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732108AbgLQWaS (ORCPT ); Thu, 17 Dec 2020 17:30:18 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54112 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732100AbgLQWaR (ORCPT ); Thu, 17 Dec 2020 17:30:17 -0500 Received: from mail-pf1-x429.google.com (mail-pf1-x429.google.com [IPv6:2607:f8b0:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5EA50C061282 for ; Thu, 17 Dec 2020 14:29:37 -0800 (PST) Received: by mail-pf1-x429.google.com with SMTP id w6so314293pfu.1 for ; Thu, 17 Dec 2020 14:29:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=CBG392X/fqmC2FSSXnmbfDW/za8PCEWVRQmenaYl98s=; b=bxppXRLfzJvnGAGc3fb6OnIE3ujPsvT95MFOws56MX/JbWAlK5gzLVxiZNFCFaoPra nxX6xV731je5sR5ERkvOu+PfYaYkFH+m/pDhjX5uINXic1eTb+Q/X1ySdvlzpnaRv0Su bnu2f2U5pNb0ZPqxh27TQqg+1Hs2gsKSk1SK0= 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=CBG392X/fqmC2FSSXnmbfDW/za8PCEWVRQmenaYl98s=; b=Povr9+L7DlhVwJQTAODjgnFJJssNgBa79LRF1sJPWRRoK1HR+mq8qruTZnbeRMU6xb E9PFavrVVndIBhh/0wy8p8Hj+bkOsTr8H+8GW1iUIArHG1on+7ai0Ffb8VDpoW5k0b8M P8Va3oo+NuE3cZWaKy6l8DJJilhHHOs14v7R9EsRfMPuCEW9QQZJa7zODLTQBLJ5Kga6 nxKRJsgGgg6YRuKMcM+YIFFZ9o/YmapYVEaDU8Ku7xV+r0ikN0bHCfPIWPzB9aN+0Oo5 eztQAHK/BKxjFfzoqFgIK2Z8ccs6yJYkJ59L6afC6QTyK2YXA6981YW7VVeVdjaDl+sc MhSA== X-Gm-Message-State: AOAM5307+TY/lhUVJKF2h8vM7l7VH2aI9PcjobaWRaFCTyhtlRAMiLXf CdvyTBFZqzbGr2caa/CAXv2MJQ== X-Google-Smtp-Source: ABdhPJwFi9/LiWzp30bFETjaBqAuGxtXVQg8FuhDxbCnWrm4vKWfvuxAT/6hNRSeijaYpd23S3WT9g== X-Received: by 2002:a63:1521:: with SMTP id v33mr1375689pgl.30.1608244176903; Thu, 17 Dec 2020 14:29:36 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:42b0:34ff:fe3d:58e6]) by smtp.gmail.com with ESMTPSA id gm18sm5805850pjb.55.2020.12.17.14.29.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Dec 2020 14:29:36 -0800 (PST) From: Douglas Anderson To: Mark Brown Cc: msavaliy@qti.qualcomm.com, Stephen Boyd , akashast@codeaurora.org, Roja Rani Yarubandi , Douglas Anderson , Alok Chauhan , Andy Gross , Bjorn Andersson , Dilip Kota , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org Subject: [PATCH v3 2/4] spi: spi-geni-qcom: Fail new xfers if xfer/cancel/abort pending Date: Thu, 17 Dec 2020 14:29:12 -0800 Message-Id: <20201217142842.v3.2.Ibade998ed587e070388b4bf58801f1107a40eb53@changeid> X-Mailer: git-send-email 2.29.2.684.gfbc64c5ab5-goog In-Reply-To: <20201217142842.v3.1.I99ee04f0cb823415df59bd4f550d6ff5756e43d6@changeid> References: <20201217142842.v3.1.I99ee04f0cb823415df59bd4f550d6ff5756e43d6@changeid> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org If we got a timeout when trying to send an abort command then it means that we just got 3 timeouts in a row: 1. The original timeout that caused handle_fifo_timeout() to be called. 2. A one second timeout waiting for the cancel command to finish. 3. A one second timeout waiting for the abort command to finish. SPI is clocked by the controller, so nothing (aside from a hardware fault or a totally broken sequencer) should be causing the actual commands to fail in hardware. However, even though the hardware itself is not expected to fail (and it'd be hard to predict how we should handle things if it did), it's easy to hit the timeout case by simply blocking our interrupt handler from running for a long period of time. Obviously the system is in pretty bad shape if a interrupt handler is blocked for > 2 seconds, but there are certainly bugs (even bugs in other unrelated drivers) that can make this happen. Let's make things a bit more robust against this case. If we fail to abort we'll set a flag and then we'll block all future transfers until we have no more interrupts pending. Fixes: 561de45f72bd ("spi: spi-geni-qcom: Add SPI driver support for GENI based QUP") Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd --- Changes in v3: - spin the lock in spi_geni_is_abort_still_pending() if abort pending. Changes in v2: - Make this just about the failed abort. drivers/spi/spi-geni-qcom.c | 59 +++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 6939c6cabe91..cf3db40ae5ba 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -83,6 +83,7 @@ struct spi_geni_master { spinlock_t lock; int irq; bool cs_flag; + bool abort_failed; }; static int get_spi_clk_cfg(unsigned int speed_hz, @@ -141,8 +142,49 @@ static void handle_fifo_timeout(struct spi_master *spi, spin_unlock_irq(&mas->lock); time_left = wait_for_completion_timeout(&mas->abort_done, HZ); - if (!time_left) + if (!time_left) { dev_err(mas->dev, "Failed to cancel/abort m_cmd\n"); + + /* + * No need for a lock since SPI core has a lock and we never + * access this from an interrupt. + */ + mas->abort_failed = true; + } +} + +static bool spi_geni_is_abort_still_pending(struct spi_geni_master *mas) +{ + struct geni_se *se = &mas->se; + u32 m_irq, m_irq_en; + + if (!mas->abort_failed) + return false; + + /* + * The only known case where a transfer times out and then a cancel + * times out then an abort times out is if something is blocking our + * interrupt handler from running. Avoid starting any new transfers + * until that sorts itself out. + */ + spin_lock_irq(&mas->lock); + m_irq = readl(se->base + SE_GENI_M_IRQ_STATUS); + m_irq_en = readl(se->base + SE_GENI_M_IRQ_EN); + spin_unlock_irq(&mas->lock); + + if (m_irq & m_irq_en) { + dev_err(mas->dev, "Interrupts pending after abort: %#010x\n", + m_irq & m_irq_en); + return true; + } + + /* + * If we're here the problem resolved itself so no need to check more + * on future transfers. + */ + mas->abort_failed = false; + + return false; } static void spi_geni_set_cs(struct spi_device *slv, bool set_flag) @@ -158,9 +200,15 @@ static void spi_geni_set_cs(struct spi_device *slv, bool set_flag) if (set_flag == mas->cs_flag) return; + pm_runtime_get_sync(mas->dev); + + if (spi_geni_is_abort_still_pending(mas)) { + dev_err(mas->dev, "Can't set chip select\n"); + goto exit; + } + mas->cs_flag = set_flag; - pm_runtime_get_sync(mas->dev); spin_lock_irq(&mas->lock); reinit_completion(&mas->cs_done); if (set_flag) @@ -173,6 +221,7 @@ static void spi_geni_set_cs(struct spi_device *slv, bool set_flag) if (!time_left) handle_fifo_timeout(spi, NULL); +exit: pm_runtime_put(mas->dev); } @@ -280,6 +329,9 @@ static int spi_geni_prepare_message(struct spi_master *spi, int ret; struct spi_geni_master *mas = spi_master_get_devdata(spi); + if (spi_geni_is_abort_still_pending(mas)) + return -EBUSY; + ret = setup_fifo_params(spi_msg->spi, spi); if (ret) dev_err(mas->dev, "Couldn't select mode %d\n", ret); @@ -509,6 +561,9 @@ static int spi_geni_transfer_one(struct spi_master *spi, { struct spi_geni_master *mas = spi_master_get_devdata(spi); + if (spi_geni_is_abort_still_pending(mas)) + return -EBUSY; + /* Terminate and return success for 0 byte length transfer */ if (!xfer->len) return 0; From patchwork Thu Dec 17 22:29:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 345297 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,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable 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 4B2E3C4361B for ; Thu, 17 Dec 2020 22:31:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 003D22376F for ; Thu, 17 Dec 2020 22:31:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732129AbgLQWa4 (ORCPT ); Thu, 17 Dec 2020 17:30:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54212 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730778AbgLQWa4 (ORCPT ); Thu, 17 Dec 2020 17:30:56 -0500 Received: from mail-pg1-x52c.google.com (mail-pg1-x52c.google.com [IPv6:2607:f8b0:4864:20::52c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DA414C061248 for ; Thu, 17 Dec 2020 14:29:38 -0800 (PST) Received: by mail-pg1-x52c.google.com with SMTP id e2so71476pgi.5 for ; Thu, 17 Dec 2020 14:29:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=0vo+nW0dH46P2KHLc68abpdc4ZQATo0Haicb01r3gGc=; b=dbCU0DJ2thR9h4Iz7w1YVlFi+HfxzId5hJDFxlm3vZnAzRN4afmLNoXle64azsK4sg XbtjvivnBA8YgZchJk+neKH7GPHzVcuu/P02D4TEJFR9kqh+pt+StIQr9H6CfHjIykP6 DNwkmHOCIbIB/9vTRoIFtHqF22NYbFmtJ7rvI= 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=0vo+nW0dH46P2KHLc68abpdc4ZQATo0Haicb01r3gGc=; b=pmkrjq3CYdLTyVKb5fntrp93ssayoyeZf94s/eDBKd8b9RHwykKgWmXABCD2r8Lgqm A2vfhBM+UvcVKrNovO4uxw8BUidNVW/15szAf+fUkGwpsCZ+b4d/ZCLZZTlGpgZNFy0z o+T14VcaeepchwRNWdHkkmvlHUQW7NLKhqA9rscLD4mFNd8hJUHeLc3URbJzzcZkMbAo fCcB/YpT27g+Ug67KqX70v2zjTojbrcUWEwaOw+wg2x4VytRi/OA6J2XEyxhL+1fQyV5 raRPnOsFnA2anh+WNBd+EXTK7t4dQleB1Enqb7jbWTugg/m7OLlfR214zFh4R2Jp/Zf6 VTwA== X-Gm-Message-State: AOAM533j3BOQlJXHiIDTZAA3g8Mei9iZZX7d1qkyEXVSD/WMexvQ1Dkf Efb3poyZrszEOvdFfenMbeOKPA== X-Google-Smtp-Source: ABdhPJwu2sL7WtP1xSDUnnnfQFJiRPkWMbpq8CPaTFt/Z+ZvMZIuOdvbcWPtbycwcQe24LffmO4WxA== X-Received: by 2002:a63:5845:: with SMTP id i5mr1291424pgm.355.1608244178430; Thu, 17 Dec 2020 14:29:38 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:42b0:34ff:fe3d:58e6]) by smtp.gmail.com with ESMTPSA id gm18sm5805850pjb.55.2020.12.17.14.29.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Dec 2020 14:29:37 -0800 (PST) From: Douglas Anderson To: Mark Brown Cc: msavaliy@qti.qualcomm.com, Stephen Boyd , akashast@codeaurora.org, Roja Rani Yarubandi , Douglas Anderson , Andy Gross , Bjorn Andersson , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org Subject: [PATCH v3 3/4] spi: spi-geni-qcom: Don't try to set CS if an xfer is pending Date: Thu, 17 Dec 2020 14:29:13 -0800 Message-Id: <20201217142842.v3.3.I07afdedcc49655c5d26880f8df9170aac5792378@changeid> X-Mailer: git-send-email 2.29.2.684.gfbc64c5ab5-goog In-Reply-To: <20201217142842.v3.1.I99ee04f0cb823415df59bd4f550d6ff5756e43d6@changeid> References: <20201217142842.v3.1.I99ee04f0cb823415df59bd4f550d6ff5756e43d6@changeid> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org If we get a timeout sending then this happens: spi_transfer_one_message() ->transfer_one() AKA spi_geni_transfer_one() setup_fifo_xfer() mas->cur_xfer = non-NULL spi_transfer_wait() => TIMES OUT if (msg->status != -EINPROGRESS) goto out if (ret != 0 ...) spi_set_cs() ->set_cs AKA spi_geni_set_cs() # mas->cur_xfer is non-NULL The above happens _before_ the SPI core calls ->handle_err() AKA handle_fifo_timeout(). Unfortunately that won't work so well on geni. If we got a timeout transferring then it's likely that our interrupt handler is blocked, but we need that same interrupt handler to run and the command channel to be unblocked in order to adjust the chip select. Trying to set the chip select doesn't crash us but ends up confusing our state machine and leads to messages like: Premature done. rx_rem = 32 bpw8 Let's just drop the chip select request in this case. We can detect the case because cur_xfer is non-NULL--it would have been set to NULL in the interrupt handler if the previous transfer had finished. Sure, we might leave the chip select in the wrong state but it's likely it was going to fail anyway and this avoids getting the driver even more confused about what it's doing. The SPI core in general assumes that setting chip select is a simple operation that doesn't fail. Yet another reason to just reconfigure the chip select line as GPIOs. Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd --- Changes in v3: - xfter => xfer in error message. - More obvious in commit message how this happens Changes in v2: - ("spi: spi-geni-qcom: Don't try to set CS if an xfer is pending") new for v2. drivers/spi/spi-geni-qcom.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index cf3db40ae5ba..b3ba092db489 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -207,9 +207,14 @@ static void spi_geni_set_cs(struct spi_device *slv, bool set_flag) goto exit; } - mas->cs_flag = set_flag; - spin_lock_irq(&mas->lock); + if (mas->cur_xfer) { + dev_err(mas->dev, "Can't set CS when prev xfer running\n"); + spin_unlock_irq(&mas->lock); + goto exit; + } + + mas->cs_flag = set_flag; reinit_completion(&mas->cs_done); if (set_flag) geni_se_setup_m_cmd(se, SPI_CS_ASSERT, 0); From patchwork Thu Dec 17 22:29:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 345844 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,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable 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 657DEC3526E for ; Thu, 17 Dec 2020 22:31:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3E95E238A0 for ; Thu, 17 Dec 2020 22:31:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732058AbgLQWbK (ORCPT ); Thu, 17 Dec 2020 17:31:10 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54218 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732126AbgLQWa4 (ORCPT ); Thu, 17 Dec 2020 17:30:56 -0500 Received: from mail-pg1-x52f.google.com (mail-pg1-x52f.google.com [IPv6:2607:f8b0:4864:20::52f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 85B1EC0611CA for ; Thu, 17 Dec 2020 14:29:40 -0800 (PST) Received: by mail-pg1-x52f.google.com with SMTP id i7so61447pgc.8 for ; Thu, 17 Dec 2020 14:29:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=sWkiWzk6Rso22yNG+cnGpuPXvotVryC+O2+4me1Qo6c=; b=ElbqSAKABhF+sBG9f/NXWoC5A3GLpmmFLgEKHC/ljsaQ0mqLK+bKaIZb5pKKbzznO+ uNXIW5SRSl/YV2W0YOt8zDCqT8ZhE0Hy07HV/rWRf5vK68PN5W6jOIUVf4CGY3fSS4fs 9qx4vynHQFlxaz1Ga7ugGbnHY7SrTvEOMwuRk= 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=sWkiWzk6Rso22yNG+cnGpuPXvotVryC+O2+4me1Qo6c=; b=Soztj6BDh3tOIgYPGsFWMDghURdo7j1dd4RD1fCe2slzynpdNa1eVf0gS5pbWqyK7K TQUBSbBuy0bpP+sKGJrl3J+49CcXK8JieFa11e5FBvvtqMw2HxK9oR0W39/2yvQPzmGv AQYbmB0oSe07wh5GNP2HITNC2s4k+Fsid2OLvqYdQClXMS5KBLcObrktrxjLiTAXq2qQ Qllz3wzBx+Rl1GhJbVE3X7lACcLfosUBusOmL1asLDIBFWSHN0KoU4qKiTiwSd5tmU1d rs/x5zajApYROZmobBQlzhN7J4e0/eViON+8u2wzxcLBQLSpj1BrmPW55K/Cvc25Dk4N o9VA== X-Gm-Message-State: AOAM533c/Dm+8FNZf5UX3YxCSgLxegU5NBPfLpNkUwR+D5UkNqNrxWVG zTUFw3SiqHgbFI9VH+b7enyevg== X-Google-Smtp-Source: ABdhPJy4bycT77LtZjm+hgxVOqL/vnjb7rOjBzQ6uEr8599KO9Wl4MBrwV7m7A8mi9p0ChEeI87r5g== X-Received: by 2002:a62:32c3:0:b029:1a1:c2f2:d771 with SMTP id y186-20020a6232c30000b02901a1c2f2d771mr1364793pfy.29.1608244180129; Thu, 17 Dec 2020 14:29:40 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:42b0:34ff:fe3d:58e6]) by smtp.gmail.com with ESMTPSA id gm18sm5805850pjb.55.2020.12.17.14.29.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Dec 2020 14:29:39 -0800 (PST) From: Douglas Anderson To: Mark Brown Cc: msavaliy@qti.qualcomm.com, Stephen Boyd , akashast@codeaurora.org, Roja Rani Yarubandi , Douglas Anderson , Andy Gross , Bjorn Andersson , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org Subject: [PATCH v3 4/4] spi: spi-geni-qcom: Print an error when we timeout setting the CS Date: Thu, 17 Dec 2020 14:29:14 -0800 Message-Id: <20201217142842.v3.4.I666b37646de9652cef438ac7c2c6c2053367fc6b@changeid> X-Mailer: git-send-email 2.29.2.684.gfbc64c5ab5-goog In-Reply-To: <20201217142842.v3.1.I99ee04f0cb823415df59bd4f550d6ff5756e43d6@changeid> References: <20201217142842.v3.1.I99ee04f0cb823415df59bd4f550d6ff5756e43d6@changeid> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org If we're using geni to manage the chip select line (don't do it--use a GPIO!) and we happen to get a timeout waiting for the chip select command to be completed, no errors are printed even though things might not be in the best shape. Let's add a print. Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd --- (no changes since v2) Changes in v2: - ("spi: spi-geni-qcom: Print an error when we timeout setting the CS") new for v2 drivers/spi/spi-geni-qcom.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index b3ba092db489..5472d895a9e0 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -223,8 +223,10 @@ static void spi_geni_set_cs(struct spi_device *slv, bool set_flag) spin_unlock_irq(&mas->lock); time_left = wait_for_completion_timeout(&mas->cs_done, HZ); - if (!time_left) + if (!time_left) { + dev_warn(mas->dev, "Timeout setting chip select\n"); handle_fifo_timeout(spi, NULL); + } exit: pm_runtime_put(mas->dev);