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.5 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 D1D09C48BDF for ; Fri, 18 Jun 2021 08:01:21 +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 530A561139 for ; Fri, 18 Jun 2021 08:01:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 530A561139 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 6212182BF1; Fri, 18 Jun 2021 10:01:19 +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="M99e5M4x"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9B03182BF1; Fri, 18 Jun 2021 10:01:17 +0200 (CEST) Received: from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com [185.132.182.106]) (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 1C31182BF5 for ; Fri, 18 Jun 2021 10:01:14 +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=prvs=080390e63f=patrice.chotard@foss.st.com Received: from pps.filterd (m0241204.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 15I80cvc007300; Fri, 18 Jun 2021 10:01:13 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foss.st.com; h=subject : from : to : cc : references : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=selector1; bh=LWFnHkU3jYWQKzhFK9SpwJful3nV8PVCowwDy2zhjSU=; b=M99e5M4xJRJPnaDcpOLRp01mTTslHJ46viRzrMqS/290jnBZVqJwVrttq3QCd97hA+ku pcX0GBSRtTH6kCSBZq5wsDv0Xo52jgen2QKRWM/TqbJWXat0ZLP7iH/oXMjXEzrmeen2 QoUMdhP9eXUt7RRx3kELUu88xqt5DMeFjWdPHX8RqoKS4UJCF6vu9frM6BoC8lqQ1+he nbNUhyijmvDm4QMcqtdC+cF0iOv+z9KVY52Vc2r+lBUv+VcKcARa633AGRMCaiuca3nc Fv1bMy1lDyTRB2ZBfXgZ9LQw17VKWhFBA6t6ywOf+GLRBaoLb8xswkXr1/3Oko8b07qV 5A== Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com with ESMTP id 398hn39sr4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 18 Jun 2021 10:01:13 +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 1E75310002A; Fri, 18 Jun 2021 10:01:13 +0200 (CEST) Received: from Webmail-eu.st.com (sfhdag2node3.st.com [10.75.127.6]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 13989214D27; Fri, 18 Jun 2021 10:01:13 +0200 (CEST) Received: from lmecxl0573.lme.st.com (10.75.127.51) by SFHDAG2NODE3.st.com (10.75.127.6) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Fri, 18 Jun 2021 10:01:12 +0200 Subject: Re: [PATCH] spi: stm32_qspi: Fix short data write operation From: Patrice CHOTARD 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> <026719e7-bbb8-7fd8-bcf6-75a7be76f304@foss.st.com> Message-ID: <9c850021-d11a-9e28-6da6-44cf82c6e78e@foss.st.com> Date: Fri, 18 Jun 2021 10:01:12 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <026719e7-bbb8-7fd8-bcf6-75a7be76f304@foss.st.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.75.127.51] X-ClientProxiedBy: SFHDAG1NODE2.st.com (10.75.127.2) To SFHDAG2NODE3.st.com (10.75.127.6) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391, 18.0.790 definitions=2021-06-17_17:2021-06-15, 2021-06-17 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.103.2 at phobos.denx.de X-Virus-Status: Clean On 5/25/21 6:02 PM, Patrice CHOTARD wrote: > 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 > > Applied on u-boot-stm32/next Thanks