From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B29791ADC60 for ; Tue, 14 Jan 2025 11:07:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.197 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736852874; cv=none; b=lFPXG8TjSY5joDvGIhNZvzXcnT9VJdb1k6oI0JJoL8UbNhWLtrv9MOyPr5L/XHroXK33OQVUhlfIqgnFq5GewO82gM0tk2Rb+UKhyGI4yHgY8Z4eCVxjbhw7VGtTAJyheIxmq1RuvL7SXv7T/1Q0pwXBzre4MgzFJfhcVK0y3dY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736852874; c=relaxed/simple; bh=0Be/isEI03pMPs1O/aSTVtClOYCCbYYqrz3FSUlMmhU=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=FlrNaZbCOthK2tXP/hW5RrRFrkkrc8gPoJXvISzkSnXcR/jWTQSXWsuV5tHMfTLirhHnP1nMsTc4eCEkqjqmIIAM2MeNqB2rkvdXU/4fiXwg9SEwUxZgEnk4QcXRKPNMgBOCk8P9nUnmx1HX5Teglw1NMUeFGnwoLUTQz4Qh5mg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=pXL4uiQi; arc=none smtp.client-ip=217.70.183.197 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="pXL4uiQi" Received: by mail.gandi.net (Postfix) with ESMTPSA id 94BA81C0002; Tue, 14 Jan 2025 11:07:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1736852864; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Z91mxdq404TCduefYxC4gpxwfyjslXNMsT+OaaAh4GE=; b=pXL4uiQi3hi0x359JlQRQ67GpaIvtlIEYjHPr417136pCStyr2odiJqgzecE0bnWcCURhN D3M+PpoaIpJK5MCiBIoXr14GHDPYGYakXHu5SsS0g2GxNfZNrXO/1G36Gl67HvdMdS3Qr3 983MQKigyjF01U0/stZjPjZQu7lbDV0CiaifL/hdWrCmq5sg81UJcl6oKHGYhmC92xLCbJ axRTdxv/FCZrnf+OsUA3SL5DAqAgt4VnhtBl+XKFhxNGYVdNuLdA8R2DDCz2wUxb3D/R54 xHG+6hYmRbF6c4RBKazrs0QGGoAzR4jW3DFX5MUxqOKK3k8yMi8E8xn42Rpzzg== From: Miquel Raynal To: Pratyush Yadav Cc: Tudor Ambarus , Michael Walle , Richard Weinberger , Vignesh Raghavendra , Thomas Petazzoni , Steam Lin , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] mtd: spi-nor: winbond: Add support for w25q01jv In-Reply-To: (Pratyush Yadav's message of "Mon, 13 Jan 2025 14:08:43 +0000") References: <20241224-winbond-6-12-rc1-nor-volatile-bit-v1-0-f7c4dff66182@bootlin.com> <20241224-winbond-6-12-rc1-nor-volatile-bit-v1-1-f7c4dff66182@bootlin.com> <871pxp798c.fsf@bootlin.com> User-Agent: mu4e 1.12.7; emacs 29.4 Date: Tue, 14 Jan 2025 12:07:42 +0100 Message-ID: <87a5btslfl.fsf@bootlin.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-GND-Sasl: miquel.raynal@bootlin.com Hello Pratyush, >> Winbond chips (maybe this is a shared capability?) accepts another >> command, "Write Enable for Volatile Status Register (50h)", which >> specifically change the status register bits to use the volatile method. >> >> Hence, if the only situation we want to solve is the status register >> access, then we may just enable this command (this is the third solution >> I tried to explain in the commit log), but if we think there are other >> racy situations, this approach is not complete and we must fallback to >> one of the approaches listed above. > > I am not quite sure how you fix the write-enable-being-racy bug with > your patch. If you look at the code, spi_nor_write_enable() only calls > the write enable command (06h), and does not call > spi_nor_wait_till_ready() after that. After the write enable, it > immediately executes the program or erase operation. So you never > actually wait for all dies to be ready after a write enable. I will double check but my understanding is that the *status register* write is racy, not the spi_nor_write_enable(). > You can see an example in spi_nor_write(). It does: > > spi_nor_write_enable() -> spi_nor_write_data() -> > spi_nor_wait_till_ready() What is racy is: act on all dies then check the status of a single die. > Do you have a consistent reproducer for the race? If so, does the patch > actually somehow make the race go away? If so, I would be curious to > know why. Not with Linux, it is a problem that has been (consistently) observed using an rtos. It's been analysed so we know what the issue is and we want to make sure this cannot happen using Linux. Thanks, Miqu=C3=A8l