From mboxrd@z Thu Jan 1 00:00:00 1970 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=-17.3 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,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 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 EF62BC2B9F8 for ; Tue, 25 May 2021 16:02:46 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2A0FD61400 for ; Tue, 25 May 2021 16:02:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2A0FD61400 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=foss.st.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 8FC1682EC5; Tue, 25 May 2021 18:02:43 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=foss.st.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=foss.st.com header.i=@foss.st.com header.b="cOvrd5CP"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 462F382EDB; Tue, 25 May 2021 18:02:41 +0200 (CEST) Received: from mx07-00178001.pphosted.com (mx08-00178001.pphosted.com [91.207.212.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 328E182EB5 for ; Tue, 25 May 2021 18:02:38 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=foss.st.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=patrice.chotard@foss.st.com Received: from pps.filterd (m0046661.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 14PFvtRU012816; Tue, 25 May 2021 18:02:37 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foss.st.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=selector1; bh=6hlG0M6sxos0iZE88FgI4QE/gYA2to1Noi33Nqc3tO8=; b=cOvrd5CPYGVxNPVVxs3YmPSFdUp1fwExiYqzkazpXLPZiVtrkEoxgV0oTB3IW2el74b0 B9Wt8EAU87rp41W8Ukz8tqAVtXxejBtwDz88wJQct3CeCs5EzCPNV+MZTvn7DKDrhch3 bHmC7OvpWhqh05+5FDJK5Sh2PZeAThVfy6vKijXNbO9534dhnBYLZ0dme41uJxOJhBrA 2zUi+ey2HCg2bksR8cJIHEt6nrnZ1vb51wwCQijn/YZLoBQ3kIF5yiRBrIQIEtXTaTWE xUWsXUW+nwnLxY3JyfUxq87zvyeRTWu4NfwscU40PB+hWapKc/GGOrNhTSflyfNgZOvl Tw== Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com with ESMTP id 38ra6u0f6a-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 25 May 2021 18:02:37 +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 DB53710002A; Tue, 25 May 2021 18:02:36 +0200 (CEST) Received: from Webmail-eu.st.com (sfhdag2node3.st.com [10.75.127.6]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id AD5A7236566; Tue, 25 May 2021 18:02:36 +0200 (CEST) Received: from lmecxl0573.lme.st.com (10.75.127.45) by SFHDAG2NODE3.st.com (10.75.127.6) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Tue, 25 May 2021 18:02:36 +0200 Subject: Re: [PATCH] spi: stm32_qspi: Fix short data write operation To: Daniil Stas CC: , Patrick Delaunay References: <20210523222449.1495352-1-daniil.stas@posteo.net> <20210523222449.1495352-2-daniil.stas@posteo.net> <4e531f04-4228-05e6-6bdb-32c29becb38f@foss.st.com> <20210524155330.47e8d96c@ux550ve> From: Patrice CHOTARD Message-ID: <026719e7-bbb8-7fd8-bcf6-75a7be76f304@foss.st.com> Date: Tue, 25 May 2021 18:02:35 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20210524155330.47e8d96c@ux550ve> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.75.127.45] X-ClientProxiedBy: SFHDAG1NODE1.st.com (10.75.127.1) To SFHDAG2NODE3.st.com (10.75.127.6) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391, 18.0.761 definitions=2021-05-25_07:2021-05-25, 2021-05-25 signatures=0 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.102.4 at phobos.denx.de X-Virus-Status: Clean Hi Daniil On 5/24/21 2:53 PM, Daniil Stas wrote: > On Mon, 24 May 2021 09:40:05 +0200 > Patrice CHOTARD wrote: > >> Hi Daniil >> >> On 5/24/21 12:24 AM, Daniil Stas wrote: >>> TCF flag only means that all data was sent to FIFO. To check if the >>> data was sent out of FIFO we should also wait for the BUSY flag to >>> be cleared. Otherwise there is a race condition which can lead to >>> inability to write short (one byte long) data. >>> >>> Signed-off-by: Daniil Stas >>> Cc: Patrick Delaunay >>> Cc: Patrice Chotard >>> --- >>> drivers/spi/stm32_qspi.c | 29 +++++++++++++++-------------- >>> 1 file changed, 15 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/spi/stm32_qspi.c b/drivers/spi/stm32_qspi.c >>> index 4acc9047b9..8f4aabc3d1 100644 >>> --- a/drivers/spi/stm32_qspi.c >>> +++ b/drivers/spi/stm32_qspi.c >>> @@ -148,23 +148,24 @@ static int _stm32_qspi_wait_cmd(struct >>> stm32_qspi_priv *priv, const struct spi_mem_op *op) >>> { >>> u32 sr; >>> - int ret; >>> - >>> - if (!op->data.nbytes) >>> - return _stm32_qspi_wait_for_not_busy(priv); >>> + int ret = 0; >>> >>> - ret = readl_poll_timeout(&priv->regs->sr, sr, >>> - sr & STM32_QSPI_SR_TCF, >>> - STM32_QSPI_CMD_TIMEOUT_US); >>> - if (ret) { >>> - log_err("cmd timeout (stat:%#x)\n", sr); >>> - } else if (readl(&priv->regs->sr) & STM32_QSPI_SR_TEF) { >>> - log_err("transfer error (stat:%#x)\n", sr); >>> - ret = -EIO; >>> + if (op->data.nbytes) { >>> + ret = readl_poll_timeout(&priv->regs->sr, sr, >>> + sr & STM32_QSPI_SR_TCF, >>> + >>> STM32_QSPI_CMD_TIMEOUT_US); >>> + if (ret) { >>> + log_err("cmd timeout (stat:%#x)\n", sr); >>> + } else if (readl(&priv->regs->sr) & >>> STM32_QSPI_SR_TEF) { >>> + log_err("transfer error (stat:%#x)\n", sr); >>> + ret = -EIO; >>> + } >>> + /* clear flags */ >>> + writel(STM32_QSPI_FCR_CTCF | STM32_QSPI_FCR_CTEF, >>> &priv->regs->fcr); } >>> >>> - /* clear flags */ >>> - writel(STM32_QSPI_FCR_CTCF | STM32_QSPI_FCR_CTEF, >>> &priv->regs->fcr); >>> + if (!ret) >>> + ret = _stm32_qspi_wait_for_not_busy(priv); >>> >>> return ret; >>> } >>> >> >> Have you got a simple test to reproduce the described race condition ? >> >> Thanks >> Patrice > > Hi, Patrice > > I found this issue on an stm32mp153 based board. > To reproduce it you need to set qspi peripheral clock to a low > value (for example 24 MHz). > Then you can test it in the u-boot console: > > STM32MP> clk dump > Clocks: > ... > - CK_PER : 24 MHz > ... > - QSPI(10) => parent CK_PER(30) > ... > > STM32MP> sf probe > SF: Detected w25q32jv with page size 256 Bytes, erase size 64 KiB, total 4 MiB > STM32MP> sf erase 0x00300000 +1 > SF: 65536 bytes @ 0x300000 Erased: OK > STM32MP> sf read 0xc4100000 0x300000 10 > device 0 offset 0x300000, size 0x10 > SF: 16 bytes @ 0x300000 Read: OK > STM32MP> md.b 0xc4100000 > c4100000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > ... > STM32MP> mw.b 0xc4200000 55 > STM32MP> sf write 0xc4200000 0x00300000 1 > device 0 offset 0x300000, size 0x1 > SF: 1 bytes @ 0x300000 Written: OK > STM32MP> sf read 0xc4100000 0x00300000 10 > device 0 offset 0x300000, size 0x10 > SF: 16 bytes @ 0x300000 Read: OK > STM32MP> md.b 0xc4100000 > c4100000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > ... > > > With my patch applied the last command result would be: > STM32MP> md.b 0xc4100000 > c4100000: 55 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff U............... > > Thanks, > Daniil > Thanks for the detailed informations, i also reproduced this issue on a stm32mp157c-ev1 board. Reviewed-by: Patrice Chotard Patrice