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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BF369C02180 for ; Wed, 15 Jan 2025 14:03:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References :In-Reply-To:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=+NQteiPbF6vZXuEXR6W+NkIGOUiv8jirtYJMcfrCWbM=; b=mqlLVnT4gvD+E8 0B2zAJvQ9I/mBdWG+bUJo+Qbm8VAM+Rw+qyGK4xFdBSEbn8AdvpmlXlX3aw08yuojBzwL7zAY55Qp sADAK0Y2UniUIeLwiW3qL0XKNDLVyoyjkNLxRv/KRK/3Yv5A/BmSz4b68sPCE6z5p9ds3Fk/jBtRK V3VobBd2aCXy5IAHMhJur1jGtxSbRCOeQEq0yT+wm43xtiHpI39XlHGxPRg42j4pR8g9FNXfysmiY trefo3vItWMnP+mLP53DouZ0hcil4iun6Tl3ALsepdxwcAxqlpu62iPOSyrzMGi96nQ7HNq2+8dQu QF5InnhhXtho4a4thghg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tY3zU-0000000C4Ig-2fiu; Wed, 15 Jan 2025 14:03:28 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tY3zS-0000000C4IJ-3QKA for linux-mtd@lists.infradead.org; Wed, 15 Jan 2025 14:03:27 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 5BF265C5B30; Wed, 15 Jan 2025 14:02:45 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 654CBC4CEDF; Wed, 15 Jan 2025 14:03:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736949805; bh=b4hQ6NunCke1/CWWrRyXZHunsy0DEoKHZi66BbKVvvY=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=tLaURzo178D75fShPt+NuymZ6HvqxR1pM+axBDgFdInpo0PBvBknTJdmy/UGP6VSy 8aZ4ESBtrFb/OIPbT9tes1AOi60zyqtLy+/WxterkOVumzKk4Q3zWxYShTmpEFVl2W Jxc7o8ZER+nqFeSe/uv59+CZMiX+vHFmb43qthMprJ463wcwXiOrR6CgC6vMnIoqjk 9qsrKU2b7YlL00A7DCD4w+Utm7eV00Pr3WwF39Gmpx2emef3k7E1/bUeVUpRtt3zfz vhLdyEXjr1hWoZwCsrdSxOBqrNG8C0oCx7oZZLlZ23p1rvgEblOT6vYAWRzYh8v8A8 GfGtgn7UdZAVA== From: Pratyush Yadav To: Miquel Raynal Cc: Pratyush Yadav , 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: <87a5btslfl.fsf@bootlin.com> (Miquel Raynal's message of "Tue, 14 Jan 2025 12:07:42 +0100") 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> <87a5btslfl.fsf@bootlin.com> Date: Wed, 15 Jan 2025 14:03:23 +0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250115_060326_938765_D2C2764E X-CRM114-Status: GOOD ( 30.64 ) X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org T24gVHVlLCBKYW4gMTQgMjAyNSwgTWlxdWVsIFJheW5hbCB3cm90ZToKCj4gSGVsbG8gUHJhdHl1 c2gsCj4KPj4+IFdpbmJvbmQgY2hpcHMgKG1heWJlIHRoaXMgaXMgYSBzaGFyZWQgY2FwYWJpbGl0 eT8pIGFjY2VwdHMgYW5vdGhlcgo+Pj4gY29tbWFuZCwgIldyaXRlIEVuYWJsZSBmb3IgVm9sYXRp bGUgU3RhdHVzIFJlZ2lzdGVyICg1MGgpIiwgd2hpY2gKPj4+IHNwZWNpZmljYWxseSBjaGFuZ2Ug dGhlIHN0YXR1cyByZWdpc3RlciBiaXRzIHRvIHVzZSB0aGUgdm9sYXRpbGUgbWV0aG9kLgo+Pj4K Pj4+IEhlbmNlLCBpZiB0aGUgb25seSBzaXR1YXRpb24gd2Ugd2FudCB0byBzb2x2ZSBpcyB0aGUg c3RhdHVzIHJlZ2lzdGVyCj4+PiBhY2Nlc3MsIHRoZW4gd2UgbWF5IGp1c3QgZW5hYmxlIHRoaXMg Y29tbWFuZCAodGhpcyBpcyB0aGUgdGhpcmQgc29sdXRpb24KPj4+IEkgdHJpZWQgdG8gZXhwbGFp biBpbiB0aGUgY29tbWl0IGxvZyksIGJ1dCBpZiB3ZSB0aGluayB0aGVyZSBhcmUgb3RoZXIKPj4+ IHJhY3kgc2l0dWF0aW9ucywgdGhpcyBhcHByb2FjaCBpcyBub3QgY29tcGxldGUgYW5kIHdlIG11 c3QgZmFsbGJhY2sgdG8KPj4+IG9uZSBvZiB0aGUgYXBwcm9hY2hlcyBsaXN0ZWQgYWJvdmUuCj4+ Cj4+IEkgYW0gbm90IHF1aXRlIHN1cmUgaG93IHlvdSBmaXggdGhlIHdyaXRlLWVuYWJsZS1iZWlu Zy1yYWN5IGJ1ZyB3aXRoCj4+IHlvdXIgcGF0Y2guIElmIHlvdSBsb29rIGF0IHRoZSBjb2RlLCBz cGlfbm9yX3dyaXRlX2VuYWJsZSgpIG9ubHkgY2FsbHMKPj4gdGhlIHdyaXRlIGVuYWJsZSBjb21t YW5kICgwNmgpLCBhbmQgZG9lcyBub3QgY2FsbAo+PiBzcGlfbm9yX3dhaXRfdGlsbF9yZWFkeSgp IGFmdGVyIHRoYXQuIEFmdGVyIHRoZSB3cml0ZSBlbmFibGUsIGl0Cj4+IGltbWVkaWF0ZWx5IGV4 ZWN1dGVzIHRoZSBwcm9ncmFtIG9yIGVyYXNlIG9wZXJhdGlvbi4gU28geW91IG5ldmVyCj4+IGFj dHVhbGx5IHdhaXQgZm9yIGFsbCBkaWVzIHRvIGJlIHJlYWR5IGFmdGVyIGEgd3JpdGUgZW5hYmxl Lgo+Cj4gSSB3aWxsIGRvdWJsZSBjaGVjayBidXQgbXkgdW5kZXJzdGFuZGluZyBpcyB0aGF0IHRo ZSAqc3RhdHVzIHJlZ2lzdGVyKgo+IHdyaXRlIGlzIHJhY3ksIG5vdCB0aGUgc3BpX25vcl93cml0 ZV9lbmFibGUoKS4KCk9rYXksIEkgYW0gY29uZnVzZWQgYmVjYXVzZSB5b3Ugc2FpZCBlYXJsaWVy IHRoYXQ6Cgo+IFRoZSBidWcgdGhhdCBoYXMgYmVlbiBleHBlcmllbmNlZCBmb2xsb3dlZCB0aGlz IHNlcXVlbmNlOgo+IC0gc2VuZCB0aGUgd3JpdGUgZW5hYmxlIGNvbW1hbmQgKG5vbi12b2xhdGls ZSkKPiAtIHdhaXQgZm9yIHRoZSByZWFkeS9idXN5IGJpdCwgaWUuIHdhaXQgZm9yIHRoZSBXRUwg Yml0IHRvIGJlIHNldAo+ICAgYmVjYXVzZSBpdCBpcyBub24tdm9sYXRpbGUgd3JpdGUKPiAtIGFj dGl2ZSBkaWUgaXMgcmVhZHksIChidXQgaWRsZSBkaWUgaXMgbm90ISkKPiAtIGVudGVyIDQtYnl0 ZSBhZGRyZXNzIG1vZGUsIG9ubHkgdGhlIGRpZSB0aGF0IGlzIHJlYWR5IHByb2Nlc3NlcyB0aGUK PiAgIGNvbW1hbmQuCgpXaGljaCBzYXlzIHRoZSBXRUwgYml0IGJlaW5nIHNldCBpdHNlbGYgaXMg cmFjeS4gV2hhdCBJIHVuZGVyc3RhbmQgZnJvbQp0aGF0IGlzIG9uZSBkaWUgaXMgcmVhZHkgdG8g dGFrZSB3cml0ZXMgYW5kIHRoZSBvdGhlciBpcyBub3QuIE5vdyB3aGVuCnlvdSB0cnkgdG8gd3Jp dGUgdGhlIFNSIHRvIGVuYWJsZSA0QiBtb2RlLCBpdCB3b3VsZCBvbmx5IHdvcmsgb24gdGhlIGRp ZQp0aGF0IGdvdCB0aGUgV0VMIHNldC4gVGhlIG90aGVyIG9uZSBpZ25vcmVzIGl0IGFuZCBzdGF5 cyBpbiAzQiBtb2RlLiBEbwpJIHVuZGVyc3RhbmQgdGhpcyBjb3JyZWN0bHk/IFRvIGZpeCB0aGlz IHlvdSBuZWVkIHRvIHdhaXQgYWZ0ZXIgdGhlCndyaXRlIGVuYWJsZSwgYmVmb3JlIHlvdSBpbml0 aWF0ZSB0aGUgd3JpdGUgU1Igb3BlcmF0aW9uLgoKPgo+PiBZb3UgY2FuIHNlZSBhbiBleGFtcGxl IGluIHNwaV9ub3Jfd3JpdGUoKS4gSXQgZG9lczoKPj4KPj4gICAgIHNwaV9ub3Jfd3JpdGVfZW5h YmxlKCkgLT4gc3BpX25vcl93cml0ZV9kYXRhKCkgLT4KPj4gICAgIHNwaV9ub3Jfd2FpdF90aWxs X3JlYWR5KCkKPgo+IFdoYXQgaXMgcmFjeSBpczogYWN0IG9uIGFsbCBkaWVzIHRoZW4gY2hlY2sg dGhlIHN0YXR1cyBvZiBhIHNpbmdsZSBkaWUuCgpZb3VyIHBhdGNoIGZpeGVzIGFsbCBzdWNoIG9w ZXJhdGlvbnMsIGV4Y2VwdCB3cml0ZSBlbmFibGUgSUlVQy4gRm9yCm9wZXJhdGlvbnMgc3VjaCBh cyB3cml0ZSBTUiAob3IgYW55IG90aGVyIHJlZ2lzdGVyKSBvciBjaGlwIGVyYXNlLCB3ZQp3b3Vs ZCBjYWxsIHNwaV9ub3Jfd2FpdF90aWxsX3JlYWR5KCksIGFuZCB5b3VyIHBhdGNoIHdvdWxkIG1h a2Ugc3VyZSBhbGwKZGllcyBhcmUgcmVhZHkuCgpCdXQgd2hlbiB3cml0ZSBlbmFibGUgaXRzZWxm IGlzIHJhY3ksIHRoZW4gd2Ugd291bGQgbmVlZCB0byBhZGQgYSB3YWl0CmFmdGVyIHRoZSB3cml0 ZSBlbmFibGUsIHdoaWNoIHlvdXIgcGF0Y2ggZG9lcyBub3QgZG8uIEkgYW0gYSBiaXQKY29uZnVz ZWQgcmlnaHQgbm93IHdoZXRoZXIgdGhhdCBpcyBhbiBhY3R1YWwgcHJvYmxlbSBvciBJIGp1c3Qg bWlzcmVhZAp5b3VyIG1lc3NhZ2UuIElmIHdyaXRlIGVuYWJsZSBpdHNlbGYgaXNuJ3QgcmFjeSwg dGhlbiB0aGUgdjMgc2VyaWVzCnNob3VsZCBiZSBnb29kIHRvIGdvLgoKPgo+PiBEbyB5b3UgaGF2 ZSBhIGNvbnNpc3RlbnQgcmVwcm9kdWNlciBmb3IgdGhlIHJhY2U/IElmIHNvLCBkb2VzIHRoZSBw YXRjaAo+PiBhY3R1YWxseSBzb21laG93IG1ha2UgdGhlIHJhY2UgZ28gYXdheT8gSWYgc28sIEkg d291bGQgYmUgY3VyaW91cyB0bwo+PiBrbm93IHdoeS4KPgo+IE5vdCB3aXRoIExpbnV4LCBpdCBp cyBhIHByb2JsZW0gdGhhdCBoYXMgYmVlbiAoY29uc2lzdGVudGx5KSBvYnNlcnZlZAo+IHVzaW5n IGFuIHJ0b3MuIEl0J3MgYmVlbiBhbmFseXNlZCBzbyB3ZSBrbm93IHdoYXQgdGhlIGlzc3VlIGlz IGFuZCB3ZQo+IHdhbnQgdG8gbWFrZSBzdXJlIHRoaXMgY2Fubm90IGhhcHBlbiB1c2luZyBMaW51 eC4KPgo+IFRoYW5rcywKPiBNaXF1w6hsCgotLSAKUmVnYXJkcywKUHJhdHl1c2ggWWFkYXYKCl9f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpMaW51 eCBNVEQgZGlzY3Vzc2lvbiBtYWlsaW5nIGxpc3QKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcv bWFpbG1hbi9saXN0aW5mby9saW51eC1tdGQvCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 4A70B2BAF7 for ; Wed, 15 Jan 2025 14:03:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736949809; cv=none; b=DwqRciSNmCvHep5vct2YpVv9K2Yb5/sQXqC/M1C3+ajiLShGnVIdDmEedz8Vk9uQTwqSR3yxawYgbAfQaMVCT1tffHBAud2pNlDoJjsijfNpX3cHKvxHR3PRUx4x/NcrroCq8JhgSiNm7FvFAL6Q0fKjSfy/214455npxECMbls= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736949809; c=relaxed/simple; bh=b4hQ6NunCke1/CWWrRyXZHunsy0DEoKHZi66BbKVvvY=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=BUkT30tbyGWDVKjNNgBp2MZ9iGuj6QLByQt4YbuzuQK40XZbmAgrDy251M01ioxrKRY0Zza085NDnezm+bCI8uUiihHn4HJKBDvq09Gfu9WzlyNU5BkxW7SeBb+vQkuUcM7Z43+yxVWYN6kx1OSHJBMzKjuhkoCxRIQMazHHq0k= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=tLaURzo1; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="tLaURzo1" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 654CBC4CEDF; Wed, 15 Jan 2025 14:03:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736949805; bh=b4hQ6NunCke1/CWWrRyXZHunsy0DEoKHZi66BbKVvvY=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=tLaURzo178D75fShPt+NuymZ6HvqxR1pM+axBDgFdInpo0PBvBknTJdmy/UGP6VSy 8aZ4ESBtrFb/OIPbT9tes1AOi60zyqtLy+/WxterkOVumzKk4Q3zWxYShTmpEFVl2W Jxc7o8ZER+nqFeSe/uv59+CZMiX+vHFmb43qthMprJ463wcwXiOrR6CgC6vMnIoqjk 9qsrKU2b7YlL00A7DCD4w+Utm7eV00Pr3WwF39Gmpx2emef3k7E1/bUeVUpRtt3zfz vhLdyEXjr1hWoZwCsrdSxOBqrNG8C0oCx7oZZLlZ23p1rvgEblOT6vYAWRzYh8v8A8 GfGtgn7UdZAVA== From: Pratyush Yadav To: Miquel Raynal Cc: Pratyush Yadav , 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: <87a5btslfl.fsf@bootlin.com> (Miquel Raynal's message of "Tue, 14 Jan 2025 12:07:42 +0100") 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> <87a5btslfl.fsf@bootlin.com> Date: Wed, 15 Jan 2025 14:03:23 +0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) 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 On Tue, Jan 14 2025, Miquel Raynal wrote: > 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(). Okay, I am confused because you said earlier that: > The bug that has been experienced followed this sequence: > - send the write enable command (non-volatile) > - wait for the ready/busy bit, ie. wait for the WEL bit to be set > because it is non-volatile write > - active die is ready, (but idle die is not!) > - enter 4-byte address mode, only the die that is ready processes the > command. Which says the WEL bit being set itself is racy. What I understand from that is one die is ready to take writes and the other is not. Now when you try to write the SR to enable 4B mode, it would only work on the die that got the WEL set. The other one ignores it and stays in 3B mode. Do I understand this correctly? To fix this you need to wait after the write enable, before you initiate the write SR operation. > >> 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. Your patch fixes all such operations, except write enable IIUC. For operations such as write SR (or any other register) or chip erase, we would call spi_nor_wait_till_ready(), and your patch would make sure all dies are ready. But when write enable itself is racy, then we would need to add a wait after the write enable, which your patch does not do. I am a bit confused right now whether that is an actual problem or I just misread your message. If write enable itself isn't racy, then the v3 series should be good to go. > >> 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 --=20 Regards, Pratyush Yadav