From patchwork Wed Aug 5 07:02:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alain Volmat X-Patchwork-Id: 11701455 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 B499214DD for ; Wed, 5 Aug 2020 07:07:40 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3DF4122B45 for ; Wed, 5 Aug 2020 07:07:40 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="26OeHj3w"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=st.com header.i=@st.com header.b="PnXKrIQ+" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3DF4122B45 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=st.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Message-ID:Date: Subject:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=yKaK6gKk3ny7pmTcl2JvZa6EnfLJPo8iYO7xuzlzW1w=; b=26OeHj3wCmkJEINLylck8Rdbf NwyyuP3g5YItyXjpexMA36c0NMCu+IFEYRcrxUjedntgQioz+3phSNe4uuQxxTl2cwsoxH4T28l1x iJrcf58QLKbnr03ykZO9sxDp5wsbjN/Yu/fb/sAAaJ4Lg5gPkbkvrtNwfmmGmj/ExSG3jPn6xm46y OiSsvPUIVrh4QhFhqNVjMMXJgvsF78oGUt1Z4bGovc2trbFy7TMNpC98p+ffvRk9Iypf6YWGA0zSk 35ThZ5l18M+t/cNI076Gcoqt9p2/EYT5+NjVFHaFOtm+9VMAm8JjZM/OdEvuiarpIlhIq8hPfTc/4 sKTsSjqFQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k3DUA-0003DK-GL; Wed, 05 Aug 2020 07:05:14 +0000 Received: from mx08-00178001.pphosted.com ([91.207.212.93] helo=mx07-00178001.pphosted.com) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k3DTM-0002sd-F1 for linux-arm-kernel@lists.infradead.org; Wed, 05 Aug 2020 07:04:30 +0000 Received: from pps.filterd (m0046660.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 07572Sl7030166; Wed, 5 Aug 2020 09:04:18 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=st.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-type; s=STMicroelectronics; bh=3MRwNXcFNhvxFNa+Aw+KEYhccyIrQvOmuOQRumq6L9k=; b=PnXKrIQ+Db1H1eqxLrVPJO8WkfcMnLS7fnI3+9rbda+VxEZnMujP9JLqf7bq9lIKVXX7 5JPR65l5Cx9sJgxiRBB/DJuraZpVsS1JrMBImu+TghD7QTj7DErO0PLkX+2fGwRk6g9b nBlkVvhu3Q1Xaod8Tr9y46oeiIka6nh3DnT3UA14BLngEwJLlLnAZC4ZljLIZr1Oo0iH uN3Ce2vy7vrIRseO7IXUVQ96D814H98WV0B8eWkO4rB0AJcJZj943l1HcndiKt+zoXX7 7q65Az/l89WyuVNSPRC2hCm9F7WsOBG+ra3ZQr/+vinHFaFMSz60D/A8JuPMa5st3fm/ Og== Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com with ESMTP id 32n6hyq7tq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 05 Aug 2020 09:04:18 +0200 Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id C08B0100038; Wed, 5 Aug 2020 09:04:17 +0200 (CEST) Received: from Webmail-eu.st.com (sfhdag3node2.st.com [10.75.127.8]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id B472F2A4D8E; Wed, 5 Aug 2020 09:04:17 +0200 (CEST) Received: from localhost (10.75.127.50) by SFHDAG3NODE2.st.com (10.75.127.8) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Wed, 5 Aug 2020 09:04:17 +0200 From: Alain Volmat To: , Subject: [PATCH 10/18] spi: stm32: wait for completion in transfer_one() Date: Wed, 5 Aug 2020 09:02:05 +0200 Message-ID: <1596610933-32599-11-git-send-email-alain.volmat@st.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1596610933-32599-1-git-send-email-alain.volmat@st.com> References: <1596610933-32599-1-git-send-email-alain.volmat@st.com> MIME-Version: 1.0 X-Originating-IP: [10.75.127.50] X-ClientProxiedBy: SFHDAG7NODE2.st.com (10.75.127.20) To SFHDAG3NODE2.st.com (10.75.127.8) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235, 18.0.687 definitions=2020-08-05_04:2020-08-03, 2020-08-05 signatures=0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200805_030425_060803_15179D93 X-CRM114-Status: GOOD ( 28.63 ) X-Spam-Score: -0.9 (/) X-Spam-Report: SpamAssassin version 3.4.4 on merlin.infradead.org summary: Content analysis details: (-0.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at https://www.dnswl.org/, low trust [91.207.212.93 listed in list.dnswl.org] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 SPF_PASS SPF: sender matches SPF record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alexandre.torgue@st.com, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, alain.volmat@st.com, mcoquelin.stm32@gmail.com, fabrice.gasnier@st.com, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org From: Amelie Delaunay In irq mode, when cpu is under heavy load and spi speed is set too high, the irq handler is not fast enough to feed the spi FIFOs. This does not compromises the data tranferred; the spi clock is temporarily stopped, the transfer takes longer time and the real speed is much lower than expected. The caller of transfer_one(), spi_transfer_one_message(), waits for the end of current transfer. It sets a timeouts with quite a generous tolerance of +100% + 100ms, but this can still expire. We could make transfer_one() blocking till the end of the transfer and bypass the wait/timeout mechanism in spi_transfer_one_message(). But if for some reason, we never get either an error (OVR, SUSP) event or end of transfer (EOT) event, xfer_completion will never "complete". That's why a timeout is useful here to avoid a hang. Timeout delay is deducted from the transfer length, the real speed and the optional delay we can add between each data frames. Timeout delay is doubled compared to the theorical transfer duration. While doing it to address irq mode only, take benefit of the new code structure and wait also in dma mode so an eventual error can be returned to the framework. Signed-off-by: Antonio Borneo Signed-off-by: Amelie Delaunay Signed-off-by: Alain Volmat --- drivers/spi/spi-stm32.c | 87 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 25 deletions(-) diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c index a931c821c280..7e3894455331 100644 --- a/drivers/spi/spi-stm32.c +++ b/drivers/spi/spi-stm32.c @@ -240,7 +240,7 @@ struct stm32_spi_cfg { int (*set_mode)(struct stm32_spi *spi, unsigned int comm_type); void (*set_data_idleness)(struct stm32_spi *spi, u32 length); int (*set_number_of_data)(struct stm32_spi *spi, u32 length); - void (*transfer_one_dma_start)(struct stm32_spi *spi); + int (*transfer_one_dma_start)(struct stm32_spi *spi); void (*dma_rx_cb)(void *data); void (*dma_tx_cb)(void *data); int (*transfer_one_irq)(struct stm32_spi *spi); @@ -276,6 +276,8 @@ struct stm32_spi_cfg { * @dma_tx: dma channel for TX transfer * @dma_rx: dma channel for RX transfer * @phys_addr: SPI registers physical base address + * @xfer_completion: completion to wait for end of transfer + * @xfer_status: current transfer status */ struct stm32_spi { struct device *dev; @@ -303,6 +305,8 @@ struct stm32_spi { struct dma_chan *dma_tx; struct dma_chan *dma_rx; dma_addr_t phys_addr; + struct completion xfer_completion; + int xfer_status; }; static const struct stm32_spi_regspec stm32f4_spi_regspec = { @@ -863,8 +867,8 @@ static irqreturn_t stm32f4_spi_irq_thread(int irq, void *dev_id) struct spi_master *master = dev_id; struct stm32_spi *spi = spi_master_get_devdata(master); - spi_finalize_current_transfer(master); stm32f4_spi_disable(spi); + complete(&spi->xfer_completion); return IRQ_HANDLED; } @@ -920,13 +924,16 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id) * If communication is suspended while using DMA, it means * that something went wrong, so stop the current transfer */ - if (spi->cur_usedma) + if (spi->cur_usedma) { + spi->xfer_status = -EIO; end = true; + } ifcr |= STM32H7_SPI_SR_SUSP; } if (mask & STM32H7_SPI_SR_OVR) { dev_err(spi->dev, "Overrun: RX data lost\n"); + spi->xfer_status = -EIO; end = true; ifcr |= STM32H7_SPI_SR_OVR; } @@ -955,7 +962,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id) if (end) { stm32h7_spi_disable(spi); - spi_finalize_current_transfer(master); + complete(&spi->xfer_completion); } return IRQ_HANDLED; @@ -1026,8 +1033,8 @@ static void stm32f4_spi_dma_tx_cb(void *data) struct stm32_spi *spi = data; if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX) { - spi_finalize_current_transfer(spi->master); stm32f4_spi_disable(spi); + complete(&spi->xfer_completion); } } @@ -1041,8 +1048,8 @@ static void stm32f4_spi_dma_rx_cb(void *data) { struct stm32_spi *spi = data; - spi_finalize_current_transfer(spi->master); stm32f4_spi_disable(spi); + complete(&spi->xfer_completion); } /** @@ -1124,9 +1131,6 @@ static void stm32_spi_dma_config(struct stm32_spi *spi, * stm32f4_spi_transfer_one_irq - transfer a single spi_transfer using * interrupts * @spi: pointer to the spi controller data structure - * - * It must returns 0 if the transfer is finished or 1 if the transfer is still - * in progress. */ static int stm32f4_spi_transfer_one_irq(struct stm32_spi *spi) { @@ -1160,16 +1164,13 @@ static int stm32f4_spi_transfer_one_irq(struct stm32_spi *spi) spin_unlock_irqrestore(&spi->lock, flags); - return 1; + return 0; } /** * stm32h7_spi_transfer_one_irq - transfer a single spi_transfer using * interrupts * @spi: pointer to the spi controller data structure - * - * It must returns 0 if the transfer is finished or 1 if the transfer is still - * in progress. */ static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi) { @@ -1202,7 +1203,7 @@ static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi) spin_unlock_irqrestore(&spi->lock, flags); - return 1; + return 0; } /** @@ -1210,8 +1211,12 @@ static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi) * transfer using DMA * @spi: pointer to the spi controller data structure */ -static void stm32f4_spi_transfer_one_dma_start(struct stm32_spi *spi) +static int stm32f4_spi_transfer_one_dma_start(struct stm32_spi *spi) { + unsigned long flags; + + spin_lock_irqsave(&spi->lock, flags); + /* In DMA mode end of transfer is handled by DMA TX or RX callback. */ if (spi->cur_comm == SPI_SIMPLEX_RX || spi->cur_comm == SPI_3WIRE_RX || spi->cur_comm == SPI_FULL_DUPLEX) { @@ -1224,6 +1229,10 @@ static void stm32f4_spi_transfer_one_dma_start(struct stm32_spi *spi) } stm32_spi_enable(spi); + + spin_unlock_irqrestore(&spi->lock, flags); + + return 0; } /** @@ -1231,8 +1240,12 @@ static void stm32f4_spi_transfer_one_dma_start(struct stm32_spi *spi) * transfer using DMA * @spi: pointer to the spi controller data structure */ -static void stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi) +static int stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi) { + unsigned long flags; + + spin_lock_irqsave(&spi->lock, flags); + /* Enable the interrupts relative to the end of transfer */ stm32_spi_set_bits(spi, STM32H7_SPI_IER, STM32H7_SPI_IER_EOTIE | STM32H7_SPI_IER_TXTFIE | @@ -1241,15 +1254,16 @@ static void stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi) stm32_spi_enable(spi); stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_CSTART); + + spin_unlock_irqrestore(&spi->lock, flags); + + return 0; } /** * stm32_spi_transfer_one_dma - transfer a single spi_transfer using DMA * @spi: pointer to the spi controller data structure * @xfer: pointer to the spi_transfer structure - * - * It must returns 0 if the transfer is finished or 1 if the transfer is still - * in progress. */ static int stm32_spi_transfer_one_dma(struct stm32_spi *spi, struct spi_transfer *xfer) @@ -1325,12 +1339,9 @@ static int stm32_spi_transfer_one_dma(struct stm32_spi *spi, stm32_spi_set_bits(spi, spi->cfg->regs->dma_tx_en.reg, spi->cfg->regs->dma_tx_en.mask); } - - spi->cfg->transfer_one_dma_start(spi); - spin_unlock_irqrestore(&spi->lock, flags); - return 1; + return spi->cfg->transfer_one_dma_start(spi); dma_submit_error: if (spi->dma_rx) @@ -1640,6 +1651,7 @@ static int stm32_spi_transfer_one(struct spi_master *master, struct spi_transfer *transfer) { struct stm32_spi *spi = spi_master_get_devdata(master); + u32 xfer_time, midi_delay_ns; int ret; spi->tx_buf = transfer->tx_buf; @@ -1656,10 +1668,34 @@ static int stm32_spi_transfer_one(struct spi_master *master, return ret; } + reinit_completion(&spi->xfer_completion); + spi->xfer_status = 0; + if (spi->cur_usedma) - return stm32_spi_transfer_one_dma(spi, transfer); + ret = stm32_spi_transfer_one_dma(spi, transfer); else - return spi->cfg->transfer_one_irq(spi); + ret = spi->cfg->transfer_one_irq(spi); + + if (ret) + return ret; + + /* Wait for transfer to complete */ + xfer_time = spi->cur_xferlen * 8 * MSEC_PER_SEC / spi->cur_speed; + midi_delay_ns = spi->cur_xferlen * 8 / spi->cur_bpw * spi->cur_midi; + xfer_time += DIV_ROUND_UP(midi_delay_ns, NSEC_PER_MSEC); + xfer_time = max(2 * xfer_time, 100U); + + ret = wait_for_completion_timeout(&spi->xfer_completion, + (msecs_to_jiffies(xfer_time))); + if (!ret) { + dev_err(spi->dev, "SPI transfer timeout (%u ms)\n", xfer_time); + spi->xfer_status = -ETIMEDOUT; + spi->cfg->disable(spi); + } + + spi_finalize_current_transfer(master); + + return spi->xfer_status; } /** @@ -1810,6 +1846,7 @@ static int stm32_spi_probe(struct platform_device *pdev) spi->dev = &pdev->dev; spi->master = master; spin_lock_init(&spi->lock); + init_completion(&spi->xfer_completion); spi->cfg = (const struct stm32_spi_cfg *) of_match_device(pdev->dev.driver->of_match_table,