From patchwork Thu Jun 18 15:06:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11612483 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9A26E14DD for ; Thu, 18 Jun 2020 15:11:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8142A2071A for ; Thu, 18 Jun 2020 15:11:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="HC73/dQn" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731387AbgFRPLQ (ORCPT ); Thu, 18 Jun 2020 11:11:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33354 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731254AbgFRPKu (ORCPT ); Thu, 18 Jun 2020 11:10:50 -0400 Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6EFA3C0613ED for ; Thu, 18 Jun 2020 08:10:48 -0700 (PDT) Received: by mail-pf1-x441.google.com with SMTP id z63so2928380pfb.1 for ; Thu, 18 Jun 2020 08:10:48 -0700 (PDT) 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=t+eo8u2tvjQcobTsfwzlNHUA5Mu/KPf2pgVwBrin5d8=; b=HC73/dQnkC9Dn5aSVy4RnCYBA5ijhBmCJTKj2PsOBV/VghxkBBjfVyTFAGfWwIbAps 0iYX0Bw2m+c18/djLCXhIcY6yGIFs5wJ3D/NZ9LshATV8SBeATCxim6b4ZeCdAJD/bpd s+HD0VcUYngCx818XJNFW6jugOMX4hlLdHHgw= 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=t+eo8u2tvjQcobTsfwzlNHUA5Mu/KPf2pgVwBrin5d8=; b=HFFNk+6CSeFuoO6CNToUcbAUCgb4gkJTDu9qHVhDSrgNCV2m6wKIS3DA2Y1bPxykYI Lt15CqybDzI1xYtTGt28O6dfe7kNAxsRuAJfhH6/Q4glpOvkNhoxwAE8SUqtHMmeEcaA Pc0HNfgURlVcKqroKe3a4/CvLnBmCi4gnWaQ7iVB3QSSdsm32oU4y5KBFlmPlqnZzXgY zc6atGWLAuE7+twllC7GRrMPtYsBbG1xiUrCDHOi8x6c7JtVGvWjr1p2pZgTV2hf5Hvw FfBcgXORsdp6dpgNpLfTKspGH/XNFT08n+j6ZoKb862XiXiZCVCecQcjivY0aHQmm/bl YFUQ== X-Gm-Message-State: AOAM5308EPGCYi8mtiML6DFDYW7kSWiHQIYsye/aokUoHDucSUjpSQSt Cg2VY/gkTHZyrPDK1btRp7ulzQ== X-Google-Smtp-Source: ABdhPJwH4R+xbXw4ZAQeCHxEFnPqtmhSc85aG0KbO3UK44AAzlOvfA5p3syDpRaqGxQ1lUZzcDfIyg== X-Received: by 2002:a63:5d54:: with SMTP id o20mr3517250pgm.253.1592493047938; Thu, 18 Jun 2020 08:10:47 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id b14sm3171510pft.23.2020.06.18.08.10.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jun 2020 08:10:47 -0700 (PDT) From: Douglas Anderson To: Mark Brown Cc: swboyd@chromium.org, Alok Chauhan , skakit@codeaurora.org, Douglas Anderson , Andy Gross , Bjorn Andersson , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org Subject: [PATCH v4 1/5] spi: spi-geni-qcom: No need for irqsave variant of spinlock calls Date: Thu, 18 Jun 2020 08:06:22 -0700 Message-Id: <20200618080459.v4.1.Ic50cccdf27d42420a63485082f8b5bf86ed1a2b6@changeid> X-Mailer: git-send-email 2.27.0.290.gba653c62da-goog In-Reply-To: <20200618150626.237027-1-dianders@chromium.org> References: <20200618150626.237027-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org The driver locks its locks in two places. In the first usage of the lock the function doing the locking already has a sleeping call and thus we know we can't be called from interrupt context. That means we can use the "spin_lock_irq" variant of the function. In the second usage of the lock the function is the interrupt handler and we know interrupt handlers are called with interrupts disabled. That means we can use the "spin_lock" variant of the function. This patch is expected to be a no-op and is just a cleanup / slight optimization. Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd --- Changes in v4: None Changes in v3: - ("spi: spi-geni-qcom: No need for irqsave variant...") new for v3 Changes in v2: None drivers/spi/spi-geni-qcom.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index c3972424af71..c7d2c7e45b3f 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -122,23 +122,23 @@ static void handle_fifo_timeout(struct spi_master *spi, struct spi_message *msg) { struct spi_geni_master *mas = spi_master_get_devdata(spi); - unsigned long time_left, flags; + unsigned long time_left; struct geni_se *se = &mas->se; - spin_lock_irqsave(&mas->lock, flags); + spin_lock_irq(&mas->lock); reinit_completion(&mas->xfer_done); mas->cur_mcmd = CMD_CANCEL; geni_se_cancel_m_cmd(se); writel(0, se->base + SE_GENI_TX_WATERMARK_REG); - spin_unlock_irqrestore(&mas->lock, flags); + spin_unlock_irq(&mas->lock); time_left = wait_for_completion_timeout(&mas->xfer_done, HZ); if (time_left) return; - spin_lock_irqsave(&mas->lock, flags); + spin_lock_irq(&mas->lock); reinit_completion(&mas->xfer_done); geni_se_abort_m_cmd(se); - spin_unlock_irqrestore(&mas->lock, flags); + spin_unlock_irq(&mas->lock); time_left = wait_for_completion_timeout(&mas->xfer_done, HZ); if (!time_left) dev_err(mas->dev, "Failed to cancel/abort m_cmd\n"); @@ -477,12 +477,11 @@ static irqreturn_t geni_spi_isr(int irq, void *data) struct spi_geni_master *mas = spi_master_get_devdata(spi); struct geni_se *se = &mas->se; u32 m_irq; - unsigned long flags; if (mas->cur_mcmd == CMD_NONE) return IRQ_NONE; - spin_lock_irqsave(&mas->lock, flags); + spin_lock(&mas->lock); m_irq = readl(se->base + SE_GENI_M_IRQ_STATUS); if ((m_irq & M_RX_FIFO_WATERMARK_EN) || (m_irq & M_RX_FIFO_LAST_EN)) @@ -524,7 +523,7 @@ static irqreturn_t geni_spi_isr(int irq, void *data) } writel(m_irq, se->base + SE_GENI_M_IRQ_CLEAR); - spin_unlock_irqrestore(&mas->lock, flags); + spin_unlock(&mas->lock); return IRQ_HANDLED; } From patchwork Thu Jun 18 15:06:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11612465 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6EEBA14DD for ; Thu, 18 Jun 2020 15:10:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 50C5C206C3 for ; Thu, 18 Jun 2020 15:10:56 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="A0dJZO/E" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731405AbgFRPKy (ORCPT ); Thu, 18 Jun 2020 11:10:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33366 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731391AbgFRPKu (ORCPT ); Thu, 18 Jun 2020 11:10:50 -0400 Received: from mail-pf1-x443.google.com (mail-pf1-x443.google.com [IPv6:2607:f8b0:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7462FC0613EF for ; Thu, 18 Jun 2020 08:10:50 -0700 (PDT) Received: by mail-pf1-x443.google.com with SMTP id j1so2926339pfe.4 for ; Thu, 18 Jun 2020 08:10:50 -0700 (PDT) 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=mqrLxsOH23jMmWDCAvD7Zv1dq0cfbx+EXnsz1QwYjMc=; b=A0dJZO/ESj54vdLdSebDiEifYFwpT8y9FYfiJ68gcGnncK0ZwSwc2Fp9L8OPb4zYWK bFhk9bA8NDMtJ+ia5MsjEl0ziE+CSA67JfvtFpFqCtRMDU4O4ySMjskZy7Yx1jrl8JI1 ua1bzHIZRI3BbrQm4RrtrYX1UuyuPolxSqg6A= 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=mqrLxsOH23jMmWDCAvD7Zv1dq0cfbx+EXnsz1QwYjMc=; b=AQWxKLy1LIWwSv37lNNOxcWZj8z5Xr6k7NTQex/Gq6kPzNem0Mqb/aiEyKVFwngr03 mqbUqwAwrtnWAV2h8CM2mL8uXoLspyz8sLN5FO5CDEK+QrMVANVH6sFUgGXhvZdKCpOR D16A12o1/lA03f2HhaKDDQ+xXsCWCInhs9ich2MSezArdQOHkkQMeHt5+x1z73kJK0bx DUrj0P7bUjj0R/HzsqxMNWDRml5zOTYiNGc1sBm5Whlu2sFOgyagI+iNYPvtLtT6ujhF kgkJQTwGysvsTFpUc+xtMKDYu4WJbAya3upyFCWiamyFFI84YrYp7CUEddUzScxeAMVX jcUg== X-Gm-Message-State: AOAM532397GnyZHuKOtUaHr21B1TQXpqzT6MniEhxFzAE6AgcBEokNsp 5USc3MqAci1qK5qZrcOGHztM1MPseZk= X-Google-Smtp-Source: ABdhPJxTAr3ojc6iO/37TWVVPwyClnlGU0LJ1oS7TjCQ42cOm3S3Sf7uNaBPLZG9lCJygBt9d+a58A== X-Received: by 2002:a63:80c8:: with SMTP id j191mr2254438pgd.38.1592493049531; Thu, 18 Jun 2020 08:10:49 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id b14sm3171510pft.23.2020.06.18.08.10.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jun 2020 08:10:49 -0700 (PDT) From: Douglas Anderson To: Mark Brown Cc: swboyd@chromium.org, Alok Chauhan , skakit@codeaurora.org, Douglas Anderson , 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 v4 2/5] spi: spi-geni-qcom: Mo' betta locking Date: Thu, 18 Jun 2020 08:06:23 -0700 Message-Id: <20200618080459.v4.2.I752ebdcfd5e8bf0de06d66e767b8974932b3620e@changeid> X-Mailer: git-send-email 2.27.0.290.gba653c62da-goog In-Reply-To: <20200618150626.237027-1-dianders@chromium.org> References: <20200618150626.237027-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org If you added a bit of a delay (like a trace_printk) into the ISR for the spi-geni-qcom driver, you would suddenly start seeing some errors spit out. The problem was that, though the ISR itself held a lock, other parts of the driver didn't always grab the lock. One example race was this: CPU0 CPU1 ---- ---- spi_geni_set_cs() mas->cur_mcmd = CMD_CS; geni_se_setup_m_cmd(...) wait_for_completion_timeout(&xfer_done); geni_spi_isr() complete(&xfer_done); pm_runtime_put(mas->dev); ... // back to SPI core spi_geni_transfer_one() setup_fifo_xfer() mas->cur_mcmd = CMD_XFER; mas->cur_cmd = CMD_NONE; // bad! return IRQ_HANDLED; Let's fix this. Before we start messing with hardware, we'll grab the lock to make sure that the IRQ handler from some previous command has really finished. We don't need to hold the lock unless we're in a state where more interrupts can come in, but we at least need to make sure the previous IRQ is done. This lock is used exclusively to prevent the IRQ handler and non-IRQ from stomping on each other. The SPI core handles all other mutual exclusion. As part of this, we change the way that the IRQ handler detects spurious interrupts. Previously we checked for our state variable being set to IRQ_NONE, but that was done outside the spinlock. We could move it into the spinlock, but instead let's just change it to look for the lack of any IRQ status bits being set. This can be done outside the lock--the hardware certainly isn't grabbing or looking at the spinlock when it updates its status register. It's possible that this will fix real (but very rare) errors seen in the field that look like: irq ...: nobody cared (try booting with the "irqpoll" option) NOTE: an alternate strategy considered here was to always make the complete() / spi_finalize_current_transfer() the very last thing in our IRQ handler. With such a change you could consider that we could be "lockless". In that case, though, we'd have to be very careful w/ memory barriers so we made sure we didn't have any bugs with weakly ordered memory. Using spinlocks makes the driver much easier to understand. 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 v4: - Drop 'controller' in comment. - Use Stephen's diagram to explain the race better. Changes in v3: - Split out some lock cleanup to previous patch. - Don't need to read IRQ status register inside spinlock. - Don't check for state CMD_NONE; later patch is removing state var. - Don't hold the lock for all of setup_fifo_xfer(). - Comment about why it's safe to Ack interrupts at the end. - Subject/desc changed since race is definitely there. Changes in v2: - Detect true spurious interrupt. - Still return IRQ_NONE for state machine mismatch, but print warn. drivers/spi/spi-geni-qcom.c | 45 ++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index c7d2c7e45b3f..7d022ccb1b6c 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -151,16 +151,18 @@ static void spi_geni_set_cs(struct spi_device *slv, bool set_flag) struct geni_se *se = &mas->se; unsigned long time_left; - reinit_completion(&mas->xfer_done); pm_runtime_get_sync(mas->dev); if (!(slv->mode & SPI_CS_HIGH)) set_flag = !set_flag; + spin_lock_irq(&mas->lock); + reinit_completion(&mas->xfer_done); mas->cur_mcmd = CMD_CS; if (set_flag) geni_se_setup_m_cmd(se, SPI_CS_ASSERT, 0); else geni_se_setup_m_cmd(se, SPI_CS_DEASSERT, 0); + spin_unlock_irq(&mas->lock); time_left = wait_for_completion_timeout(&mas->xfer_done, HZ); if (!time_left) @@ -307,6 +309,21 @@ static void setup_fifo_xfer(struct spi_transfer *xfer, u32 spi_tx_cfg, len; struct geni_se *se = &mas->se; + /* + * Ensure that our interrupt handler isn't still running from some + * prior command before we start messing with the hardware behind + * its back. We don't need to _keep_ the lock here since we're only + * worried about racing with out interrupt handler. The SPI core + * already handles making sure that we're not trying to do two + * transfers at once or setting a chip select and doing a transfer + * concurrently. + * + * NOTE: we actually _can't_ hold the lock here because possibly we + * might call clk_set_rate() which needs to be able to sleep. + */ + spin_lock_irq(&mas->lock); + spin_unlock_irq(&mas->lock); + spi_tx_cfg = readl(se->base + SE_SPI_TRANS_CFG); if (xfer->bits_per_word != mas->cur_bits_per_word) { spi_setup_word_len(mas, mode, xfer->bits_per_word); @@ -367,6 +384,12 @@ static void setup_fifo_xfer(struct spi_transfer *xfer, } writel(spi_tx_cfg, se->base + SE_SPI_TRANS_CFG); mas->cur_mcmd = CMD_XFER; + + /* + * Lock around right before we start the transfer since our + * interrupt could come in at any time now. + */ + spin_lock_irq(&mas->lock); geni_se_setup_m_cmd(se, m_cmd, FRAGMENTATION); /* @@ -376,6 +399,7 @@ static void setup_fifo_xfer(struct spi_transfer *xfer, */ if (m_cmd & SPI_TX_ONLY) writel(mas->tx_wm, se->base + SE_GENI_TX_WATERMARK_REG); + spin_unlock_irq(&mas->lock); } static int spi_geni_transfer_one(struct spi_master *spi, @@ -478,11 +502,11 @@ static irqreturn_t geni_spi_isr(int irq, void *data) struct geni_se *se = &mas->se; u32 m_irq; - if (mas->cur_mcmd == CMD_NONE) + m_irq = readl(se->base + SE_GENI_M_IRQ_STATUS); + if (!m_irq) return IRQ_NONE; spin_lock(&mas->lock); - m_irq = readl(se->base + SE_GENI_M_IRQ_STATUS); if ((m_irq & M_RX_FIFO_WATERMARK_EN) || (m_irq & M_RX_FIFO_LAST_EN)) geni_spi_handle_rx(mas); @@ -522,8 +546,23 @@ static irqreturn_t geni_spi_isr(int irq, void *data) complete(&mas->xfer_done); } + /* + * It's safe or a good idea to Ack all of our our interrupts at the + * end of the function. Specifically: + * - M_CMD_DONE_EN / M_RX_FIFO_LAST_EN: Edge triggered interrupts and + * clearing Acks. Clearing at the end relies on nobody else having + * started a new transfer yet or else we could be clearing _their_ + * done bit, but everyone grabs the spinlock before starting a new + * transfer. + * - M_RX_FIFO_WATERMARK_EN / M_TX_FIFO_WATERMARK_EN: These appear + * to be "latched level" interrupts so it's important to clear them + * _after_ you've handled the condition and always safe to do so + * since they'll re-assert if they're still happening. + */ writel(m_irq, se->base + SE_GENI_M_IRQ_CLEAR); + spin_unlock(&mas->lock); + return IRQ_HANDLED; } From patchwork Thu Jun 18 15:06:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11612475 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 29D96159A for ; Thu, 18 Jun 2020 15:11:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0FDCE206C3 for ; Thu, 18 Jun 2020 15:11:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="GDnU2+Ca" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731410AbgFRPK5 (ORCPT ); Thu, 18 Jun 2020 11:10:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33370 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731180AbgFRPKv (ORCPT ); Thu, 18 Jun 2020 11:10:51 -0400 Received: from mail-pg1-x541.google.com (mail-pg1-x541.google.com [IPv6:2607:f8b0:4864:20::541]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8647AC0613F0 for ; Thu, 18 Jun 2020 08:10:51 -0700 (PDT) Received: by mail-pg1-x541.google.com with SMTP id e9so3023186pgo.9 for ; Thu, 18 Jun 2020 08:10:51 -0700 (PDT) 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=EgswRRarbAngjEcfcdO7vzU1X69Dnu5N0WEEYS8N17E=; b=GDnU2+CaEI6iO1FX18aBPuHJlLAXyHMvQn8RC61SQb2CvOFmgQWPfbcual4cPk934X sA0sjnBzrS6IZu2mx3Yj26le/et18tojNvJpgNQHujueWEvey7C4obNY7U2CIl4myHAz mcjeD1MBLt/CARfSmNysfkIDQLJS4fUXzJr6w= 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=EgswRRarbAngjEcfcdO7vzU1X69Dnu5N0WEEYS8N17E=; b=RY78I8yhAfgsW3/2xVIb54c9NWrl7XqLeitgXvB/2aW8u1PMIO6bVVPzBTaRx/fx+n 72IcpXKR0PiX1I95UYAkVtEm6sJ19fUKEezRt11zSWv8T2TzAML29heIU/NUmicff/Ib VssoDvnvODaraCUMUwTuoIF8/0FbeTI+qYESfUdeBHa8objZUjxekAuL8MfvtYN/OOz8 JNT+CkI41ozCfmG5B5F/D1Vk7/fMvog/K1gT7M0tOmxZfEcIpRUudaLLFU5uAcHvHknL nkbbM2OBCuKMP+8huQ1s1LTSbsdOI5y7Pcv9EkxLUfElJM3KrSV1lq2Dk4ybWMOhOhQ0 zKPg== X-Gm-Message-State: AOAM532pZLtACs2BGp7OuyoxBYBxsexdI8ogXsjFvPg/1TGkPyJbUmUs 10CqdYBv9HKMcv/DuSZ38xiJcw== X-Google-Smtp-Source: ABdhPJxWP5h6GqAJBOvQkYlENccfoLscwWKOsJ5bOnlei9cHfBy1KvAoZrC0RykDVJAKLwA2KbbfAw== X-Received: by 2002:a63:a558:: with SMTP id r24mr3845460pgu.70.1592493051091; Thu, 18 Jun 2020 08:10:51 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id b14sm3171510pft.23.2020.06.18.08.10.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jun 2020 08:10:50 -0700 (PDT) From: Douglas Anderson To: Mark Brown Cc: swboyd@chromium.org, Alok Chauhan , skakit@codeaurora.org, Douglas Anderson , Andy Gross , Bjorn Andersson , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org Subject: [PATCH v4 3/5] spi: spi-geni-qcom: Check for error IRQs Date: Thu, 18 Jun 2020 08:06:24 -0700 Message-Id: <20200618080459.v4.3.Id8bebdbdb4d2ed9468634343a7e6207d6cffff8a@changeid> X-Mailer: git-send-email 2.27.0.290.gba653c62da-goog In-Reply-To: <20200618150626.237027-1-dianders@chromium.org> References: <20200618150626.237027-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org From reading the #defines it seems like we should shout if we ever see one of these error bits. Let's do so. This doesn't do anything functional except print a yell in the log if the error bits are seen. Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd --- Changes in v4: None Changes in v3: - ("spi: spi-geni-qcom: Check for error IRQs") new in v3. Changes in v2: None drivers/spi/spi-geni-qcom.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 7d022ccb1b6c..11f36d237c77 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -506,6 +506,11 @@ static irqreturn_t geni_spi_isr(int irq, void *data) if (!m_irq) return IRQ_NONE; + if (m_irq & (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN | + M_RX_FIFO_RD_ERR_EN | M_RX_FIFO_WR_ERR_EN | + M_TX_FIFO_RD_ERR_EN | M_TX_FIFO_WR_ERR_EN)) + dev_warn(mas->dev, "Unexpected IRQ err status %#010x\n", m_irq); + spin_lock(&mas->lock); if ((m_irq & M_RX_FIFO_WATERMARK_EN) || (m_irq & M_RX_FIFO_LAST_EN)) From patchwork Thu Jun 18 15:06:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11612467 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 79FBA14DD for ; Thu, 18 Jun 2020 15:10:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5CE4F2071A for ; Thu, 18 Jun 2020 15:10:57 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="HfoZ5lRO" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728788AbgFRPK4 (ORCPT ); Thu, 18 Jun 2020 11:10:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33362 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731396AbgFRPKw (ORCPT ); Thu, 18 Jun 2020 11:10:52 -0400 Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE8AAC06174E for ; Thu, 18 Jun 2020 08:10:52 -0700 (PDT) Received: by mail-pf1-x441.google.com with SMTP id j1so2926408pfe.4 for ; Thu, 18 Jun 2020 08:10:52 -0700 (PDT) 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=bVe6UCUtrVR7cafEV9bt9G1zk5uDkvg9No0xHEHinP8=; b=HfoZ5lRONe1F61xrxP9U4L/d9eEWRtxvGR1cJncfL/woxzmvANr4v7yU5w9uV+0w7P XcjIgNgXmghUX+eAEQaMHmAjrIUFdQM2LLEcvBGYYsI1BUw4VKM5Q0VLNRn6y7Vq7/9j CkxlFgDNBB8iW6/bhIasfNViYUjlV+/a8geOA= 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=bVe6UCUtrVR7cafEV9bt9G1zk5uDkvg9No0xHEHinP8=; b=tOGwgwwXkRdUWGopM3fYu1AcUs1kvd1smMnVLT+74ouFd2J/vcec/RHmdkf6FvGrPI /Koqs+vKxHRG45uTTU+CiyjNEh+7XCXUEGvWpI+rnTaudbsCY24xt9W3pN/1ZHVS2cFr YHxE5CsucURgVYwlSru5ITLdVVf+pMtmglgq+mdfYPFx5RpnNcwa4G//pUNpOAguXEMU eBR84euZtZ0aBTvUXWPuW8Vg08MHx0o+5m3OaEsxoqkknYTaodkheldplZobuCPAs63+ isSV0JZmYzUnd7HI9FQnrMeJoaWHo9PC0d1y9eTM7AfzQoWxIArMV61S70WGD2U7veBr Ooww== X-Gm-Message-State: AOAM532RcT0ILHseqzvHUdCxo0HWFmFGH//cSImpFp6102nRoNpyjOY/ K+aKtkXTheS2S3hTfmm22SpTmQ== X-Google-Smtp-Source: ABdhPJxeb3obYpA8AnWN2pNjegngMD5qnPGIspUqIiUGT3QjemzJUaNiSOGvh5KCaQsVt6QmO1Xtyw== X-Received: by 2002:a62:3103:: with SMTP id x3mr3897100pfx.130.1592493052313; Thu, 18 Jun 2020 08:10:52 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id b14sm3171510pft.23.2020.06.18.08.10.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jun 2020 08:10:51 -0700 (PDT) From: Douglas Anderson To: Mark Brown Cc: swboyd@chromium.org, Alok Chauhan , skakit@codeaurora.org, Douglas Anderson , 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 v4 4/5] spi: spi-geni-qcom: Actually use our FIFO Date: Thu, 18 Jun 2020 08:06:25 -0700 Message-Id: <20200618080459.v4.4.I988281f7c6ee0ed00325559bfce7539f403da69e@changeid> X-Mailer: git-send-email 2.27.0.290.gba653c62da-goog In-Reply-To: <20200618150626.237027-1-dianders@chromium.org> References: <20200618150626.237027-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org The geni hardware has a FIFO that can hold up to 64 bytes (it has 16 entries that can hold 4 bytes each), at least on the two SoCs I tested (sdm845 and sc7180). We configured our RX Watermark to 0, which basically meant we got an interrupt as soon as the first 4 bytes showed up in the FIFO. Tracing the IRQ handler showed that we often only read 4 or 8 bytes per IRQ handler. I tried setting the RX Watermark to "fifo size - 2" but that just got me a bunch of overrun errors reported. Setting it to "fifo size - 3" seemed to work great, though. This made me worried that we'd start getting overruns if we had long interrupt latency, but that doesn't appear to be the case and delays inserted in the IRQ handler while using "fifo size - 3" didn't cause any errors. Presumably there is some interaction with the poorly-documented RFR (ready for receive) level means that "fifo size - 3" is the max. We are the SPI master, so it makes sense that there would be no problems with overruns, the master should just stop clocking. Despite "fifo size - 3" working, I chose "fifo size / 2" (8 entries = 32 bytes) which gives us a little extra time to get to the interrupt handler and should reduce dead time on the SPI wires. With this setting, I often saw the IRQ handler handle 40 bytes but sometimes up to 56 if we had bad interrupt latency. Testing by running "flashrom -p ec -r" on a Chromebook saw interrupts from the SPI driver cut roughly in half. Time was roughly the same. Fixes: 561de45f72bd ("spi: spi-geni-qcom: Add SPI driver support for GENI based QUP") Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd --- Technically this is an improvement, not a fix. ...but it should be such a decrease in interrupts that I've tagged it as a Fix nonetheless as per suggestion during review. Changes in v4: None Changes in v3: - ("spi: spi-geni-qcom: Actually use our FIFO") new in v3. Changes in v2: None drivers/spi/spi-geni-qcom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 11f36d237c77..5b8ca8b93b06 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -285,7 +285,7 @@ static int spi_geni_init(struct spi_geni_master *mas) * Hardware programming guide suggests to configure * RX FIFO RFR level to fifo_depth-2. */ - geni_se_init(se, 0x0, mas->tx_fifo_depth - 2); + geni_se_init(se, mas->tx_fifo_depth / 2, mas->tx_fifo_depth - 2); /* Transmit an entire FIFO worth of data per IRQ */ mas->tx_wm = 1; ver = geni_se_get_qup_hw_version(se); From patchwork Thu Jun 18 15:06:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11612477 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1BB56138C for ; Thu, 18 Jun 2020 15:11:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F25692071A for ; Thu, 18 Jun 2020 15:11:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="dH8VWtr5" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731421AbgFRPLB (ORCPT ); Thu, 18 Jun 2020 11:11:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33364 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731401AbgFRPKy (ORCPT ); Thu, 18 Jun 2020 11:10:54 -0400 Received: from mail-pg1-x543.google.com (mail-pg1-x543.google.com [IPv6:2607:f8b0:4864:20::543]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 59B53C0613EE for ; Thu, 18 Jun 2020 08:10:54 -0700 (PDT) Received: by mail-pg1-x543.google.com with SMTP id e9so3023247pgo.9 for ; Thu, 18 Jun 2020 08:10:54 -0700 (PDT) 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=G4WpNdW6vNPqyWYwEyXbMN3MKHk3EbcVItClv2f3EZU=; b=dH8VWtr5H0+lYfILsmT6ScmwmCOKzTfPUZe3jKBFZg9TNjr9hSU5sdTGWtB3RJncaN AtYBH9KOmXVWOPwZyAGiz0tshcbNFz8/Unp+P9oWAvA3rvsm0Fy6NaPdExkJJ2QJuWSx k/x6DPcz0WPccx0UG8h2uWoZjOLIK1p7jkBFY= 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=G4WpNdW6vNPqyWYwEyXbMN3MKHk3EbcVItClv2f3EZU=; b=iOJUypZZ0luwuIerfdPFyVf5wJm9x+zRXTRdCV7Ak43xlNn4Hvy+oSEzqTq8TY0UCn HUxcAxp2HgT9HDb6ZcospD7Skg9Lcwc9rEbrH5UrLd/A5c8/q4bWCoi4KBpcPYe5jBSn ZDL0q5Z1YfbrgtTconKbrw0GXcLZ39wkph0k+LqCBbtxdtmGRLk4ydrGFbgilUSn+DVc ryaHPphNGj7v2DgXN1/Vh6qNjiQqC5moRuPrBKFjUV109Et0Mh/PaJtmnBZrW9yFpOSf hHK3PgeH1SrEbztXfK1eEnQGyeIQTauVgL9lFR4ltH+nFsvqXFlvljPtYbVfl/j2NWbu 536Q== X-Gm-Message-State: AOAM5307yru+Du7mTKqhRzL1p4q7LfT2+A9BQ3exE3Naj4Q9MDRbwAif 1VPD0Rm8hbxFiXSiGTUEJsidnw== X-Google-Smtp-Source: ABdhPJytq+tfcAyMoofwFs/+vA00AkGYTDF9W8ylxgUb2Mg8XNqsOtezXIuPvb60CeLZvUVmgSKhGg== X-Received: by 2002:a63:1910:: with SMTP id z16mr3591357pgl.50.1592493053836; Thu, 18 Jun 2020 08:10:53 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id b14sm3171510pft.23.2020.06.18.08.10.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jun 2020 08:10:53 -0700 (PDT) From: Douglas Anderson To: Mark Brown Cc: swboyd@chromium.org, Alok Chauhan , skakit@codeaurora.org, Douglas Anderson , Andy Gross , Bjorn Andersson , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org Subject: [PATCH v4 5/5] spi: spi-geni-qcom: Don't keep a local state variable Date: Thu, 18 Jun 2020 08:06:26 -0700 Message-Id: <20200618080459.v4.5.Ib1e6855405fc9c99916ab7c7dee84d73a8bf3d68@changeid> X-Mailer: git-send-email 2.27.0.290.gba653c62da-goog In-Reply-To: <20200618150626.237027-1-dianders@chromium.org> References: <20200618150626.237027-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org The variable "cur_mcmd" kept track of our current state (idle, xfer, cs, cancel). We don't really need it, so get rid of it. Instead: * Use separate condition variables for "chip select done", "cancel done", and "abort done". This is important so that if a "done" comes through (perhaps some previous interrupt finally came through) it can't confuse the cancel/abort function. * Use the "done" interrupt only for when a chip select or transfer is done and we can tell the difference by looking at whether "cur_xfer" is NULL. This is mostly a no-op change. However, it is possible it could fix an issue where a super delayed interrupt for a cancel command could have confused our waiting for an abort command. Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd --- Changes in v4: None Changes in v3: - ("spi: spi-geni-qcom: Don't keep a local state variable") new in v3. Changes in v2: None drivers/spi/spi-geni-qcom.c | 55 ++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 5b8ca8b93b06..0c534d151370 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -63,13 +63,6 @@ #define TIMESTAMP_AFTER BIT(3) #define POST_CMD_DELAY BIT(4) -enum spi_m_cmd_opcode { - CMD_NONE, - CMD_XFER, - CMD_CS, - CMD_CANCEL, -}; - struct spi_geni_master { struct geni_se se; struct device *dev; @@ -81,10 +74,11 @@ struct spi_geni_master { unsigned int tx_rem_bytes; unsigned int rx_rem_bytes; const struct spi_transfer *cur_xfer; - struct completion xfer_done; + struct completion cs_done; + struct completion cancel_done; + struct completion abort_done; unsigned int oversampling; spinlock_t lock; - enum spi_m_cmd_opcode cur_mcmd; int irq; }; @@ -126,20 +120,23 @@ static void handle_fifo_timeout(struct spi_master *spi, struct geni_se *se = &mas->se; spin_lock_irq(&mas->lock); - reinit_completion(&mas->xfer_done); - mas->cur_mcmd = CMD_CANCEL; - geni_se_cancel_m_cmd(se); + reinit_completion(&mas->cancel_done); writel(0, se->base + SE_GENI_TX_WATERMARK_REG); + mas->cur_xfer = NULL; + mas->tx_rem_bytes = mas->rx_rem_bytes = 0; + geni_se_cancel_m_cmd(se); spin_unlock_irq(&mas->lock); - time_left = wait_for_completion_timeout(&mas->xfer_done, HZ); + + time_left = wait_for_completion_timeout(&mas->cancel_done, HZ); if (time_left) return; spin_lock_irq(&mas->lock); - reinit_completion(&mas->xfer_done); + reinit_completion(&mas->abort_done); geni_se_abort_m_cmd(se); spin_unlock_irq(&mas->lock); - time_left = wait_for_completion_timeout(&mas->xfer_done, HZ); + + time_left = wait_for_completion_timeout(&mas->abort_done, HZ); if (!time_left) dev_err(mas->dev, "Failed to cancel/abort m_cmd\n"); } @@ -156,15 +153,14 @@ static void spi_geni_set_cs(struct spi_device *slv, bool set_flag) set_flag = !set_flag; spin_lock_irq(&mas->lock); - reinit_completion(&mas->xfer_done); - mas->cur_mcmd = CMD_CS; + reinit_completion(&mas->cs_done); if (set_flag) geni_se_setup_m_cmd(se, SPI_CS_ASSERT, 0); else geni_se_setup_m_cmd(se, SPI_CS_DEASSERT, 0); spin_unlock_irq(&mas->lock); - time_left = wait_for_completion_timeout(&mas->xfer_done, HZ); + time_left = wait_for_completion_timeout(&mas->cs_done, HZ); if (!time_left) handle_fifo_timeout(spi, NULL); @@ -383,7 +379,6 @@ static void setup_fifo_xfer(struct spi_transfer *xfer, mas->rx_rem_bytes = xfer->len; } writel(spi_tx_cfg, se->base + SE_SPI_TRANS_CFG); - mas->cur_mcmd = CMD_XFER; /* * Lock around right before we start the transfer since our @@ -520,11 +515,13 @@ static irqreturn_t geni_spi_isr(int irq, void *data) geni_spi_handle_tx(mas); if (m_irq & M_CMD_DONE_EN) { - if (mas->cur_mcmd == CMD_XFER) + if (mas->cur_xfer) { spi_finalize_current_transfer(spi); - else if (mas->cur_mcmd == CMD_CS) - complete(&mas->xfer_done); - mas->cur_mcmd = CMD_NONE; + mas->cur_xfer = NULL; + } else { + complete(&mas->cs_done); + } + /* * If this happens, then a CMD_DONE came before all the Tx * buffer bytes were sent out. This is unusual, log this @@ -546,10 +543,10 @@ static irqreturn_t geni_spi_isr(int irq, void *data) mas->rx_rem_bytes, mas->cur_bits_per_word); } - if ((m_irq & M_CMD_CANCEL_EN) || (m_irq & M_CMD_ABORT_EN)) { - mas->cur_mcmd = CMD_NONE; - complete(&mas->xfer_done); - } + if (m_irq & M_CMD_CANCEL_EN) + complete(&mas->cancel_done); + if (m_irq & M_CMD_ABORT_EN) + complete(&mas->abort_done); /* * It's safe or a good idea to Ack all of our our interrupts at the @@ -617,7 +614,9 @@ static int spi_geni_probe(struct platform_device *pdev) spi->handle_err = handle_fifo_timeout; spi->set_cs = spi_geni_set_cs; - init_completion(&mas->xfer_done); + init_completion(&mas->cs_done); + init_completion(&mas->cancel_done); + init_completion(&mas->abort_done); spin_lock_init(&mas->lock); pm_runtime_enable(dev); From patchwork Thu Jun 18 23:39:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 11613063 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 19BDC913 for ; Thu, 18 Jun 2020 23:40:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 00A7B20888 for ; Thu, 18 Jun 2020 23:40:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="dY+RVgnM" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727950AbgFRXkE (ORCPT ); Thu, 18 Jun 2020 19:40:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55396 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727922AbgFRXkC (ORCPT ); Thu, 18 Jun 2020 19:40:02 -0400 Received: from mail-pl1-x643.google.com (mail-pl1-x643.google.com [IPv6:2607:f8b0:4864:20::643]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3C422C0613EF for ; Thu, 18 Jun 2020 16:40:01 -0700 (PDT) Received: by mail-pl1-x643.google.com with SMTP id v24so3154977plo.6 for ; Thu, 18 Jun 2020 16:40:01 -0700 (PDT) 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=rcHTOjx1Q346P9ZsaYW4owKJanYsDyPidI2shLjtOJ0=; b=dY+RVgnMbwlyOGl4P//IOxVJvx4Qsckqq5/7RKUlK3B0HNqReZ1iIEAYd/Gdz4XzoZ JUOMUSv3Yr6q9tPxAX+XhmV2u8vtcVcwEflJYgP+lEujrAkAgl82N8OhC6huceMDZDQq ytrLGOoiUvTIDJ8xyGuEVcRJxs7f0s/eDwhw8= 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=rcHTOjx1Q346P9ZsaYW4owKJanYsDyPidI2shLjtOJ0=; b=o1lKR+7M75KA63tCP7mAH3gxRQT//TBgD3NOFUV9H/H2qQE5QTqBFx3jGJpfKLIXyL wt029gglzmHCeyazTbXnIpoLIUj+/zRr6CG9KMv/cx2HHvDFNB+HShSCM9P/lzZ8OCL8 40/Cy1Zr9CZFETjc+1adXjmkXsX9I2ViK2NGm6G4U4rXFy5MF/p+5PFDfl7Jg2IOpquW tWv/LYUkj8KPNJz0+lc9JI5pFPwK8EubaLYhzW5HVlKJ6EUwzzLcW+B89AtJjb0sL14a d1kCmj6P8kXe55CXwz5Rc8UWSVWn7mKYzXS++CMfE7yr3hbI1E6T2CFQeV0Hy+PlFfNt excg== X-Gm-Message-State: AOAM531ZwYxsv5YK5GaA80kvV2K7Uhv51VygbQFFHNWX7UlgxL5g0rFF ha05H8bxHP+UpdAtT4xdIRwuzHoScEg= X-Google-Smtp-Source: ABdhPJyi4TrKrINHRr4ksSg7boVpVbJGS2fQzBBNhqgUlekddu3vS2EjX4ceIVFsaPIJfbmrwC/oOQ== X-Received: by 2002:a17:90a:930f:: with SMTP id p15mr765529pjo.85.1592523600729; Thu, 18 Jun 2020 16:40:00 -0700 (PDT) Received: from smtp.gmail.com ([2620:15c:202:1:fa53:7765:582b:82b9]) by smtp.gmail.com with ESMTPSA id r202sm3876993pfr.185.2020.06.18.16.39.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jun 2020 16:40:00 -0700 (PDT) From: Stephen Boyd To: Mark Brown Cc: linux-kernel@vger.kernel.org, Alok Chauhan , linux-arm-msm@vger.kernel.org, linux-spi@vger.kernel.org, Douglas Anderson Subject: [PATCH 6/5] spi: spi-geni-qcom: Simplify setup_fifo_xfer() Date: Thu, 18 Jun 2020 16:39:58 -0700 Message-Id: <20200618233959.160032-1-swboyd@chromium.org> X-Mailer: git-send-email 2.27.0.111.gc72c7da667-goog In-Reply-To: <20200618150626.237027-1-dianders@chromium.org> References: <20200618150626.237027-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org The definition of SPI_FULL_DUPLEX (3) is really SPI_TX_ONLY (1) ORed with SPI_RX_ONLY (2). Let's drop the define and simplify the code here a bit by collapsing the setting of 'm_cmd' into conditions that are the same. This is a non-functional change, just cleanup to consolidate code. Cc: Douglas Anderson Signed-off-by: Stephen Boyd Reviewed-by: Douglas Anderson --- drivers/spi/spi-geni-qcom.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 636c3da15db0..670f83793aa4 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -51,7 +51,6 @@ /* M_CMD OP codes for SPI */ #define SPI_TX_ONLY 1 #define SPI_RX_ONLY 2 -#define SPI_FULL_DUPLEX 3 #define SPI_TX_RX 7 #define SPI_CS_ASSERT 8 #define SPI_CS_DEASSERT 9 @@ -357,12 +356,6 @@ static void setup_fifo_xfer(struct spi_transfer *xfer, mas->tx_rem_bytes = 0; mas->rx_rem_bytes = 0; - if (xfer->tx_buf && xfer->rx_buf) - m_cmd = SPI_FULL_DUPLEX; - else if (xfer->tx_buf) - m_cmd = SPI_TX_ONLY; - else if (xfer->rx_buf) - m_cmd = SPI_RX_ONLY; spi_tx_cfg &= ~CS_TOGGLE; @@ -373,12 +366,14 @@ static void setup_fifo_xfer(struct spi_transfer *xfer, len &= TRANS_LEN_MSK; mas->cur_xfer = xfer; - if (m_cmd & SPI_TX_ONLY) { + if (xfer->tx_buf) { + m_cmd |= SPI_TX_ONLY; mas->tx_rem_bytes = xfer->len; writel(len, se->base + SE_SPI_TX_TRANS_LEN); } - if (m_cmd & SPI_RX_ONLY) { + if (xfer->rx_buf) { + m_cmd |= SPI_RX_ONLY; writel(len, se->base + SE_SPI_RX_TRANS_LEN); mas->rx_rem_bytes = xfer->len; } From patchwork Thu Jun 18 23:39:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 11613065 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id EF59C913 for ; Thu, 18 Jun 2020 23:40:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D709420888 for ; Thu, 18 Jun 2020 23:40:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="JGQdHi4c" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728086AbgFRXkM (ORCPT ); Thu, 18 Jun 2020 19:40:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55406 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726001AbgFRXkD (ORCPT ); Thu, 18 Jun 2020 19:40:03 -0400 Received: from mail-pl1-x644.google.com (mail-pl1-x644.google.com [IPv6:2607:f8b0:4864:20::644]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4E292C061795 for ; Thu, 18 Jun 2020 16:40:02 -0700 (PDT) Received: by mail-pl1-x644.google.com with SMTP id n2so3140732pld.13 for ; Thu, 18 Jun 2020 16:40:02 -0700 (PDT) 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=0LwNUM9r2K766Bpc8uhRggwPkvLcxyldoWgAyqBsLHs=; b=JGQdHi4cAk9ynSlkc2MLZrVw9vuu0aa67R3mMbMXCKleaglDUWIgiBE9LgBxuCJX9/ Q0AHLynQe4sKoPSZhrnps5qRe+CywpQ30+7AFC10rSRq/IajqSOZdLYKEDIR3Al6AAxc T9Sdas8h9nx2DwLg2KFwPelACgyIMiUDXlWw8= 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=0LwNUM9r2K766Bpc8uhRggwPkvLcxyldoWgAyqBsLHs=; b=e/xUoZpqZclxNPFylxnb5r7faUJEztOvzygqdzNNL5vWLbBzkwtkb5d4gKuCiL+98P +m+ycXhZb79yS31/vL9smTvWAjtMcvDOOim0CEvgbA6zZYteYNcjqB2S+9V+wb6qaPHw fIZqs9qcggGFtGDun0DKCgiGX8PecC0RsgZiygehQp9OgTKvAeIx/fqZ7aAvqCZPHTc/ IiwSw0LQa8IFe+zOLp60LmhPhe1bONlBKL5mc0tZW9+uRfsj4kWRkRpnRVvnq1BUJr2W gFsLZ0BP9s5FGCR/+b9jMD9SdEIldfM0BoAYZSnQWyip8pgC/GIbwT7dK0A7lkMz7Ti2 8+OQ== X-Gm-Message-State: AOAM532x7lmyZOMyaiyflrDYozbyorcX+nfiXa3XNhbq1TrXDpfd7JYc iKr5S2r3/rUWdAsX8JJ1/psU5Q== X-Google-Smtp-Source: ABdhPJwQ/hJoSBJMHMkuQtqXhSeDW1z3BUpC+AR+YhDRj5//RUP55YF1vFN5nLf+YeDoKlM4/qVEcg== X-Received: by 2002:a17:90a:7c4e:: with SMTP id e14mr732821pjl.175.1592523601795; Thu, 18 Jun 2020 16:40:01 -0700 (PDT) Received: from smtp.gmail.com ([2620:15c:202:1:fa53:7765:582b:82b9]) by smtp.gmail.com with ESMTPSA id r202sm3876993pfr.185.2020.06.18.16.40.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jun 2020 16:40:01 -0700 (PDT) From: Stephen Boyd To: Mark Brown Cc: linux-kernel@vger.kernel.org, Alok Chauhan , linux-arm-msm@vger.kernel.org, linux-spi@vger.kernel.org, Douglas Anderson Subject: [PATCH 7/5] spi: spi-geni-qcom: Don't set {tx,rx}_rem_bytes unnecessarily Date: Thu, 18 Jun 2020 16:39:59 -0700 Message-Id: <20200618233959.160032-2-swboyd@chromium.org> X-Mailer: git-send-email 2.27.0.111.gc72c7da667-goog In-Reply-To: <20200618150626.237027-1-dianders@chromium.org> References: <20200618150626.237027-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org We only need to test for these counters being non-zero when we see the end of a transfer. If we're doing a CS change then they will already be zero. This implies that we don't need to set these to 0 if we're cancelling an in flight transfer too, because we only care to test these counters when the 'DONE' bit is set in the hardware and we've set them to non-zero for a transfer. This is a non-functional change, just cleanup to consolidate code. Cc: Douglas Anderson Signed-off-by: Stephen Boyd Reviewed-by: Douglas Anderson --- drivers/spi/spi-geni-qcom.c | 42 ++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 670f83793aa4..828cfc988a3f 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -126,7 +126,6 @@ static void handle_fifo_timeout(struct spi_master *spi, * came in while cancelling. */ mas->cur_xfer = NULL; - mas->tx_rem_bytes = mas->rx_rem_bytes = 0; geni_se_cancel_m_cmd(se); spin_unlock_irq(&mas->lock); @@ -517,29 +516,30 @@ static irqreturn_t geni_spi_isr(int irq, void *data) if (mas->cur_xfer) { spi_finalize_current_transfer(spi); mas->cur_xfer = NULL; + /* + * If this happens, then a CMD_DONE came before all the + * Tx buffer bytes were sent out. This is unusual, log + * this condition and disable the WM interrupt to + * prevent the system from stalling due an interrupt + * storm. + * + * If this happens when all Rx bytes haven't been + * received, log the condition. The only known time + * this can happen is if bits_per_word != 8 and some + * registers that expect xfer lengths in num spi_words + * weren't written correctly. + */ + if (mas->tx_rem_bytes) { + writel(0, se->base + SE_GENI_TX_WATERMARK_REG); + dev_err(mas->dev, "Premature done. tx_rem = %d bpw%d\n", + mas->tx_rem_bytes, mas->cur_bits_per_word); + } + if (mas->rx_rem_bytes) + dev_err(mas->dev, "Premature done. rx_rem = %d bpw%d\n", + mas->rx_rem_bytes, mas->cur_bits_per_word); } else { complete(&mas->cs_done); } - - /* - * If this happens, then a CMD_DONE came before all the Tx - * buffer bytes were sent out. This is unusual, log this - * condition and disable the WM interrupt to prevent the - * system from stalling due an interrupt storm. - * If this happens when all Rx bytes haven't been received, log - * the condition. - * The only known time this can happen is if bits_per_word != 8 - * and some registers that expect xfer lengths in num spi_words - * weren't written correctly. - */ - if (mas->tx_rem_bytes) { - writel(0, se->base + SE_GENI_TX_WATERMARK_REG); - dev_err(mas->dev, "Premature done. tx_rem = %d bpw%d\n", - mas->tx_rem_bytes, mas->cur_bits_per_word); - } - if (mas->rx_rem_bytes) - dev_err(mas->dev, "Premature done. rx_rem = %d bpw%d\n", - mas->rx_rem_bytes, mas->cur_bits_per_word); } if (m_irq & M_CMD_CANCEL_EN)