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 E8BAC411FA2 for ; Thu, 13 Aug 2026 10:44:42 +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=1786617887; cv=none; b=EBMCHHsUW+ViSI+lbUVeqtQdMrtl8b9pje4rkcDWYa6dwpJqkE69n9MiKPYrQTxOnIyyXnSButBBheuHV6fUtwlUUyoE/lUNQvFahiHEi0CpzLNs2TZUMAjlXdLd+RMSX8rlL52kWfLSFRAiYagRwXfIWQY/fN5/6ga0VMqcFdU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786617887; c=relaxed/simple; bh=q8f9EQubBbGI+hSwvhPDypVoXykSzoklQHr+X5mG7ww=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=d4GJOaFR2ZaV2/ehF1fgcJiDEqBWMiOpDNmlGfhECu31PLxLSIvzFvQIF1eqvnxdB9vLuA12zdF/BNd/uLj/dqHNKfsf2A9+EFCW3H1wko9W5qP2WqrePJs0haiFWarcFtWBYSGfatIgphgMeUuOiKJp/U46dc8elO/pNApAJMM= 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=bMwGV/bY; 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="bMwGV/bY" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-03.galae.net (Postfix) with ESMTPS id 2C7CE4E411EC; Thu, 13 Aug 2026 10:44:41 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id E6332602B8; Thu, 13 Aug 2026 10:44:40 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id BB95F11C4DD77; Thu, 13 Aug 2026 12:44:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1786617876; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=JttpmspCHapwDXzhlD1M7SP5FQ7R1H7xGCIvIw9IXQ0=; b=bMwGV/bYalyy5/e0q5CjDErG2pnW7P2fMkwY1kHpWSDlV0jJCXeGlBYR2yI1VLbbtLiGwu +1ug/+h5b76aY5yibDCIVKfOZkSlHHhxaS8JrN/WEtyafhp3mkDDIqrbfIZzxuqI7gXsR8 BoZEWoOFNuE00vh8vy/VxlltZWCk7KzHsbnEsncMJOIzjVZO3ftG4KVYsOJkqPB4658qrN zHxSo1GjzyvwDLnsZqTdlAf1RCEl/QEnJSJiJBmSFWIaX79pgecO0dn7S/e7tM7vtACAbi JceU+uyfAhKOoTDTXII1fe674+/0k5h5Cf7lmozWlN/YzKi67WmSX7XsWTJauA== 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: (Dominique Martinet's message of "Thu, 13 Aug 2026 18:30:28 +0900") References: <20260812-w25n04lw-v2-1-deee97602fc4@atmark-techno.com> <87mruql9fn.fsf@bootlin.com> User-Agent: mu4e 1.12.12; emacs 30.2 Date: Thu, 13 Aug 2026 12:44:32 +0200 Message-ID: <875x1el2pb.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 On 13/08/2026 at 18:30:28 +09, Dominique Martinet wrote: > Thanks for the feedback! > > Miquel Raynal wrote on Thu, Aug 13, 2026 at 10:19:08AM +0200: >> > OOB layout was defined to only show ECC-protected "user data I", leavi= ng >> > "user data II" unavailable. >>=20 >> Fine, there is not strong rule, and anyway this is only useful for non >> UBI users (jffs2, typically). > > Yeah, I was expecting UBI to use it too but I looked it up after posting > and saw it doesn't, so I agree it doesn't matter much for us either way. > >> > As an extra data point, this "user area I and II" distinction is the s= ame >> > in W25N04KW (same 2+12), but w25n02kv_ooblayout_free() use there retur= ns >> > 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 shou= ld >> > discuss this before merging) >>=20 >> If it's the same layout, I would go for keeping the existing one and >> avoid the proliferation of these helpers. > > The layout is similar (each section has 2 bytes for BB marker / 2 bytes > user data II / 12 bytes user data I), but the number of sections per > erase block is different (went from 4 to 8) > For ECC it's the same (4 to 8 sections, and offset changed from 64 to > 128 accordingly because there were more data sections before) > > I see similar helpers in other drivers (e.g. alliancememory.c) use > mtd->oobsize; so if you prefer I can reuse the existing > w25n02kv_ooblayout* helpers based on this? Yes, that would be ideal. > ( > Technically could go as far as checking the ecc strength[1] and also > merge in w25n01kv_ooblayout_ecc (variable ecc length/offset step), > then if we're greedy also handle oobsize =3D=3D 64 to replace > w25m02gv_ooblayout_ecc/w25m02gv_ooblayout_free, but that's starting to > make the functions more complex than I'm confident in... :) > [1] mtd_to_nanddev(mtd)->eccreq.strength > ) Up to you :) > (And in that case I'll happily give up on the non-ECC distinction and > keep the current behavior that exposes the non-protected bits) > >> > 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)? >>=20 >> 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. > > There's also the 12 bytes in "user data I", but I agree it's probably > fine to ignore here. > nandbiterrs passes, so I'll leave it as is. https://elixir.bootlin.com/linux/v7.1.8/source/include/linux/mtd/nand.h#L187 It is not very clear, but this macro is about the main data ("bytes per ste= p"). >> > - 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? >>=20 >> 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. > > Ok, so I ought to check that e.g. > SPINAND_PAGE_READ_FROM_CACHE_1S_4S_4S_OP still needs 2 dummy op > delay? > (I couldn't find the _1S_4S_4S_ meaning explained anywhere, but looking > at the commands used the number is the number of lines used for the > command instruction itself, the column address and the data itself? Yes. Dummy width is the same as address width BTW. > the datasheet doesn't seem to handle any command with a *D so I'll > ignore the S/D...) SDR vs DTR. You chip may or may not support it. Good luck! Miqu=C3=A8l