From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-03.galae.net (smtpout-03.galae.net [185.246.85.4]) (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 5430F3624C8 for ; Thu, 13 Aug 2026 08:19:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.246.85.4 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786609162; cv=none; b=uBvcE/NnxIam1RWZkYEatXCk/HQzHMNbfGO6/YSq9jDu9Zx6mCSlcd4bCwmmP3T+uWLFQK2wqXR341APqBNP/pt1FRvwKT0NklgVTriqHIcQO0oGMk5Qh9oPlBtB60/CAt2d/6EDJ5WzZsj+kmolBzAvQaw02AS9+xU1cMnovlM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786609162; c=relaxed/simple; bh=p/pyOdIQRSG3ZNRknIZ3Bn3/CnfFaRSPbmvBB6aAOps=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=rr378qfreopa9cQhq0+pCHo7l0lc0pyguZVPRWNHYIeMedYawu6avdzgfxklgOuTi5WLJaztlcyR2g3Yu9JuV2K/8D1p7XITmr3AKnxxZ27HOZT/y5ISSfmeHh3HHazirG7cfX/o33ALhSrDNphya9HVcfOycixvDTCxhd8OxwQ= 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=x96JQ2jP; arc=none smtp.client-ip=185.246.85.4 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="x96JQ2jP" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-03.galae.net (Postfix) with ESMTPS id DDDFC4E411E0; Thu, 13 Aug 2026 08:19:11 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id A8895602B8; Thu, 13 Aug 2026 08:19:11 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id E88FD11C4DAD8; Thu, 13 Aug 2026 10:19:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1786609151; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=d5IPq6WZnZycQbNu50OcaWDugoY4LFNngT4j7PMNmZY=; b=x96JQ2jPuwrMz+XZybqlmBbYo9gLlUq+/1XRkjOepnkDvfd3PjOxQ12qGi4eQ6QDQyKPYh iuDygC0oiy+pGp1exI6ktWQ1SGxmI625Xrt3tc3qJbDVaUBixWiSmmR9arzfdy6FtVgPte votGlfgPLNLlWb9ZJEEkmyiL88MplxXqM9ZlGGg8SnMoayHtO3EN3Z3SY1BPISp6J4mRJX rNgGdwnAU7HjmpDy08QuNPIIFssQvX+ZEU2ETBDfSYnzNDuxQVUmPRYneWMjeNZ4nTwLa2 Q6vSkAmJbWWJaWPeoTk9PGBzyhVLkBZw5I8Ry5DjuYdY0vmadt6xhtAzlTiE8w== From: Miquel Raynal To: Dominique Martinet Cc: Richard Weinberger , Md Sadre Alam , Vignesh Raghavendra , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, Daisuke Mizobuchi Subject: Re: [PATCH RFC v2] mtd: spinand: winbond: add support for W25N04LW In-Reply-To: <20260812-w25n04lw-v2-1-deee97602fc4@atmark-techno.com> (Dominique Martinet's message of "Wed, 12 Aug 2026 04:14:02 +0000") References: <20260812-w25n04lw-v2-1-deee97602fc4@atmark-techno.com> User-Agent: mu4e 1.12.12; emacs 30.2 Date: Thu, 13 Aug 2026 10:19:08 +0200 Message-ID: <87mruql9fn.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-Last-TLS-Session-Version: TLSv1.3 Hi Dominique, A bit more feedback... On 12/08/2026 at 04:14:02 GMT, Dominique Martinet wrote: > Add support for Winbond W25N04LW 4Gbit SPI-NAND: >> 1.8V 4G-bit >> Raw serial SLC NAND flash memory >> Dual/Quad SPI with 104MHz >> buffer read, continuous read & sequential read > > Highlights copied from datasheet: > =E2=80=93 Page size: 4,352 Bytes (4096 + 256 Bytes) > =E2=80=93 Block size: 64 pages (256K + 16K Bytes) > - Built-in 8-Bit ECC for memory array > =E2=80=93 ECC status bits indicate ECC results > =E2=80=93 Bad Block Management and LUT access > =E2=80=93 Software and Hardware Write-Protect > =E2=80=93 Power Supply Lock-Down and OTP protection > =E2=80=93 Unique ID and Parameter page > =E2=80=93 Ten 4KB OTP pages per die > =E2=80=93 Read Level Setting for Read Retry > > The 4096+256 byte page layout is as follow: > - 4096 bytes ECC protected "main memory array", subdivided in 8 > 512 bytes sectors > - 128 bytes spare array, subdivided in 8 16 bytes sectors as follow: > - 2 bytes bad block marker (no ECC protection) > - 2 bytes "user data II" (no ECC protection) > - 12 bytes "user data I" (ECC protected) > - 128 bytes parity array, subdivided in 8 16 bytes sectors: > - 13 bytes ECC for main data and user data > - 3 unused bytes > > This commit does not support most advanced features such as continuous > read, LUT management and OTP pages, and only provides basic usage. > > OOB layout was defined to only show ECC-protected "user data I", leaving > "user data II" unavailable. Fine, there is not strong rule, and anyway this is only useful for non UBI users (jffs2, typically). > Signed-off-by: Daisuke Mizobuchi > Signed-off-by: Dominique Martinet > --- > Changes in v2: > - Link to v1: https://patch.msgid.link/ansYUoNKJyDQ+6f2@hu-mdalam-blr.qua= lcomm.com > - fixed oob layout > - fixed NAND_MEMORG oob size > > I've tested the NAND with various mtd test utils (mtd_oobtest, > mtd_pagetest, mtd_subpagetest, mtd_nandbiterrs and some mtd-utils > userspace counterparts) with no problem, but I'm not sure what to do > with the ooblayout free() informations: as written in the commit > message, this chip has 2+12 bytes of user data per sector where the > first two are not ECC protected and the later 12 are. > > I believe user should make the choice of which oob bytes they want to > use, but afaik we can't say "this is not protected", so we can't let > them choose and must either say both are free or only provide one like I > did here. Indeed. > As an extra data point, this "user area I and II" distinction is the same > in W25N04KW (same 2+12), but w25n02kv_ooblayout_free() use there returns > the whole 14 bytes as a single chunk, so I explicitly made a different > choice here. > (I believe that's not something that can be changed easily, so we should > discuss this before merging) If it's the same layout, I would go for keeping the existing one and avoid the proliferation of these helpers. > That aside: > - Alam, would you like your name somewhere in the commit? I didn't keep > anything from your commit because I already had one, but happy to add > a Co-developed-by or something > - I kept NAND_ECCREQ(8, 512) like W25N04KW but the datasheet says it's > based on 8-bits/544-bytes ECC, so I should set it as (8, 544)? No, we are protecting 512 bytes of data. 544 is the area including bytes used for the ECC operation itself and we are not interested by those in this field. > (since it's protecting 512+12 bytes for up to 8 bits corruption) > In practice ECC is done by hardware and looks like it is correctly > reported (e.g. manually overwriting a couple of bits with nandwrite > and checking with nanddump properly corrects and reports number of > corrected bits), but might as well get this right, even if as far as I > understand nothing actually uses the ECC oob data to double-check > hardware status? No we do not. If nandbiterrs pass, normally it's all fine. > - I'll shamefully admit I do not understand the read / write / > update_cache_variants() I copied from KW, > Alam had the same but it'd be great to confirm using the same > callbacks makes sense? There are various operations available for I/Os. Depending on the spi controller capability and the routing at the board level, you might want to use single, dual, quad, dtr or not transfers. And depending on the speed you may need extra dummy cycles. All of this is captured in this array. > - (I wrote about OTP in the commit message but have no plan of > implementing it at this point as we have no use for it, I still > intend to look at continuous read after some other work) Fine! Thanks, Miqu=C3=A8l