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=-15.8 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,SPF_HELO_NONE,SPF_PASS 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 35898C04FF3 for ; Mon, 24 May 2021 12:54:01 +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 87E8D613AD for ; Mon, 24 May 2021 12:53:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 87E8D613AD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=posteo.net 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 36BCB82E91; Mon, 24 May 2021 14:53:57 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=posteo.net 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; secure) header.d=posteo.net header.i=@posteo.net header.b="CfM6MIjS"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id BB8D282E95; Mon, 24 May 2021 14:53:55 +0200 (CEST) Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 7C39A82D58 for ; Mon, 24 May 2021 14:53:52 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=posteo.net Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=daniil.stas@posteo.net Received: from submission (posteo.de [89.146.220.130]) by mout02.posteo.de (Postfix) with ESMTPS id 14CAC2400E5 for ; Mon, 24 May 2021 14:53:52 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1621860832; bh=PR8EO/1RRAiOzoIinCkSwsZ4OHTD9vM+iq/XXOasi50=; h=Date:From:To:Cc:Subject:From; b=CfM6MIjSr/j78UNpE83MZk7qYFFbwrVCzGta7/RDWUvfw6PXOnUaSuNbP2DvzwtYm 4cXAW2ydNRAlbBlyAdUQlV99iGnDgoE4hG8Cg0FsSEszxfEQRV3ANCVzGqf92f2TBh XQCcO7lRXeJy4U05ErXRFrjDrStS1HCjqrB7ymy7W+YGWGLH7bvhzHdeJzumvcDVk5 qyxhEJbywcso1DE+gG/IRZvbVja10AxnBLU7uR4sTG3l1++3H5FdStq/6UrzG1Ijy2 IWfw+OADhSuQknOiHTAJ7s546RLpd0lmWZHMNtxoD/jqiI46vLO2SRKsUp5VK0zf1Z HWGmqlyuLlPIw== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4FpcbM2qhnz9rxV; Mon, 24 May 2021 14:53:51 +0200 (CEST) Date: Mon, 24 May 2021 12:53:30 +0000 From: Daniil Stas To: Patrice CHOTARD Cc: , Patrick Delaunay Subject: Re: [PATCH] spi: stm32_qspi: Fix short data write operation Message-ID: <20210524155330.47e8d96c@ux550ve> In-Reply-To: <4e531f04-4228-05e6-6bdb-32c29becb38f@foss.st.com> References: <20210523222449.1495352-1-daniil.stas@posteo.net> <20210523222449.1495352-2-daniil.stas@posteo.net> <4e531f04-4228-05e6-6bdb-32c29becb38f@foss.st.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 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