From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gw2.atmark-techno.com (gw2.atmark-techno.com [35.74.137.57]) (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 56A2930F924 for ; Thu, 13 Aug 2026 09:30:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=35.74.137.57 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786613450; cv=none; b=koWFeKuNgKbf+r+DAm4sCUeL62lR4GcfeEI/hIXUFm9ZfROfPm0SM9tGDpogXCwa6Bp23cnjxMWoxVsd6yxY4irAdmTaAGl4RdZH6JtoTjltgSvk5ksWbhUvgYaix6sjgke4YN0qXtOyYklv4NMqv2Z44QYlN82qLf9vEjQeMig= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786613450; c=relaxed/simple; bh=8VNeESPcQFehuUcPO+mlISItg3X/NhtgmV3aZuGO204=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=DMCghQU3itIw1ks1g9p51ZxybrUbNJeVcyLu+JXauX1tFQ4yh5tO1YCY0A29tnHHLMi1BSG7ePhvi1sO00z/pr4zFQFxCn8N34v+J0//JMnFrmwQ3m+r2BgvhYGIy5IpT2J/XOGRgOe+yUqHs/+lAibGAZReypzMpcbwOUfI8/g= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=atmark-techno.com; spf=pass smtp.mailfrom=atmark-techno.com; dkim=pass (2048-bit key) header.d=atmark-techno.com header.i=@atmark-techno.com header.b=x4rS8hU4; dkim=pass (2048-bit key) header.d=atmark-techno.com header.i=@atmark-techno.com header.b=Sfu6fwK2; arc=none smtp.client-ip=35.74.137.57 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=atmark-techno.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=atmark-techno.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=atmark-techno.com header.i=@atmark-techno.com header.b="x4rS8hU4"; dkim=pass (2048-bit key) header.d=atmark-techno.com header.i=@atmark-techno.com header.b="Sfu6fwK2" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=atmark-techno.com; s=gw2_bookworm; t=1786613442; bh=8VNeESPcQFehuUcPO+mlISItg3X/NhtgmV3aZuGO204=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=x4rS8hU4dCWk8iStn8qSbvCEQBjy2a1xatJdWIARU2+vwWx6OxfNSxPQQQA3+mveI tT1VlRNuIGo1XA4VDuOAubXaG8VG/XWn8vUUrOC8Cj3LKBHnCGmZ50s12XhQ/2IbVd LzYjOy6w3o8AYIlHyGXDR+kQhM41Vkung58og5Yk1Cw4Bjy+QbpV5Hq6m3foPyWZNK r0X2HfnnmGQmeqJUhnhfAOlRUz7JGf3ZS91q4vqH0Ync3byz3SXePNS/ZPQHcw8v2L lGxehGJr5vz7LpXSXi1q63L/RQQiA8BS84RhUY4dASzxLiQV3iusUFaJh69Es7BfkI f4reVpGpSniaw== Received: from gw2.atmark-techno.com (localhost [127.0.0.1]) by gw2.atmark-techno.com (Postfix) with ESMTP id 8BB9E36C for ; Thu, 13 Aug 2026 18:30:42 +0900 (JST) Authentication-Results: gw2.atmark-techno.com; dkim=pass (2048-bit key; unprotected) header.d=atmark-techno.com header.i=@atmark-techno.com header.a=rsa-sha256 header.s=google header.b=Sfu6fwK2; dkim-atps=neutral Received: from mail-pl1-f200.google.com (mail-pl1-f200.google.com [209.85.214.200]) by gw2.atmark-techno.com (Postfix) with ESMTPS id EAE8C356 for ; Thu, 13 Aug 2026 18:30:41 +0900 (JST) Received: by mail-pl1-f200.google.com with SMTP id d9443c01a7336-2cfc52ddc55so28600745ad.3 for ; Thu, 13 Aug 2026 02:30:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=atmark-techno.com; s=google; t=1786613441; x=1787218241; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=RBHWByBuLBDZsbwfIWc5rJuZ8chbs4+pLuEKTz2S0fc=; b=Sfu6fwK2RgKz9u7iRpjkN2dLSwsC52DOKSko3Akqa4eh2o2RbLzn+B3FmGE3Sk4/n9 I+7SS38zcqzqXW0MmAuH2Oq5uJZHxibgbnBen8A3GXMcu/8EkhUqnm5a2dHWylyljdBD W98lq6U55K6/cKn5LzItdQDCJk8wCh+HW4w/6xoorb+p3cAB6R2R8NTxEjQYmWTt3cSC kk2up+5V3su1vNpczIHi6aaNlIRw9VgpUkH4WQ6J4JPNSTQNKF29S4/UKTKjNjXXCxdX 483RKcrBhP/PksM8OPJC2NR07iJRQZbW/u+NtW8N/9TQauRI9OeaMyPmZBQk5I4rzA5Z bMcA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786613441; x=1787218241; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=RBHWByBuLBDZsbwfIWc5rJuZ8chbs4+pLuEKTz2S0fc=; b=KmV18u8f+XkrYKmUWNWGQd6HBpreLk8pSxkc4FlyCWdMF+XDErxG4LcyitN7loLBpQ zba5rrWMMxFI11gxR7q/5Hg2SdF3FOdX3+mS0ljfpI/wYQ0oVsIcE71ZlnHihy7d5s2M UQXz+bKrFzL31kZtr/Rs03iHWub0HIr31cAY1/2snmXfkm5z4eBDEVSKQYXCqKEHg1n2 WlYUhWonpsfYwkziOt+56jreweyhTeI9EE1a6i8MS8miTpr1NiaGWgNxgkF1HYvX6HQk 1KQ9hMp6AcUJYwMGt5rnMF5jX5Ey1MK4HHfHL0zVaabq7YIn7jRKJwXVu0rNS0+xVVn+ WlPw== X-Forwarded-Encrypted: i=1; AHgh+RopNftHlxq7iy7T4ZEDv0AWoHpUdZFpYwyBzZSffpNOMd4Af4Oc2y7BfI4+1sGUD7m7xYc1843CMQAygEQ=@vger.kernel.org X-Gm-Message-State: AOJu0YyaLpzi9C22+UqA5t/s0hIs+H3UR4xKPW66WqNOJZhwiBCYbd1Q P+8dN/kiPb2saZpHEXF6bu1kW+vwQdmaKHpiF/S+AoLgmMk6jJwmeUn+0mPg0G6bAlfKYG4DFv6 XozBKT2oCygdu0VjwWYeoTKtQldbPUgkim61hrwlXvO6E+CptBgtQShqxSIzXNGZeqIc= X-Gm-Gg: AR+sD13PCxO0K3I7sebKh1fLX1VOoqbOKqLXnivlBGcRl16nhunfNCQl2T8Qq8KBAl9 mGCHsAOLEFBhCXqZbpVTMb1rYAPh4mWrl7hSeVBkmQZb+OXz5dugS4YxQ/jp8jLuKTn2Sml9vSI Rrvc1G67W907RsI38krqmV1AwbgS4tBh27z+M36A5/7UGcoCrQLan3nYvEGX+5ifopaKjKZ4OD6 qCgzBVqgCvd3IiqDvFty5Q9YgGhBz2/87sirQwTjqOpNwlLPHR7iliMfgh2HOnJq8Db54hs68ne J2jIBnK13ZcR8V6fkEUjhYYjAEUAl5fvx7vxAc3ubzmuDDwW90jKRvcXKiSo4jh2zHSkQYFJIOa JovvOWPAPodzQdx+8H6SEYCHrKvAut97GN8evwc/E5fHG6ndz X-Received: by 2002:a17:902:cf09:b0:2cf:9347:f445 with SMTP id d9443c01a7336-2d37d5810abmr50497055ad.10.1786613440781; Thu, 13 Aug 2026 02:30:40 -0700 (PDT) X-Received: by 2002:a17:902:cf09:b0:2cf:9347:f445 with SMTP id d9443c01a7336-2d37d5810abmr50496405ad.10.1786613440177; Thu, 13 Aug 2026 02:30:40 -0700 (PDT) Received: from localhost (sodcd-04p2-40.ppp11.odn.ad.jp. [203.139.65.40]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-2d37c49ce11sm6472065ad.67.2026.08.13.02.30.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Aug 2026 02:30:39 -0700 (PDT) Date: Thu, 13 Aug 2026 18:30:28 +0900 From: Dominique Martinet To: Miquel Raynal 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 Message-ID: References: <20260812-w25n04lw-v2-1-deee97602fc4@atmark-techno.com> <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-Disposition: inline In-Reply-To: <87mruql9fn.fsf@bootlin.com> 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", leaving > > "user data II" unavailable. > > 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 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. 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? ----(untested) static int w25n02kv_ooblayout_ecc(struct mtd_info *mtd, int section, struct mtd_oob_region *region) { /* 4 sections for oobsize=128, 8 for oobsize=256 */ if (section >= mtd->oobsize / 32) return -ERANGE; region->offset = (mtd->oobsize / 2) + (16 * section); region->length = 13; return 0; } static int w25n02kv_ooblayout_free(struct mtd_info *mtd, int section, struct mtd_oob_region *region) { if (section >= mtd->oobsize / 32) return -ERANGE; region->offset = (16 * section) + 2; region->length = 14; return 0; } ---- ( 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 == 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 ) (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)? > > 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. > > - 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. 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? the datasheet doesn't seem to handle any command with a *D so I'll ignore the S/D...) Ah, okay, the dummy delay is in byte and looking at the datasheet I need one byte worth for 0x6b (1S_1S_4S, 8 cycles at 1 line) and two bytes worth for 0xeb (1S_4S_4S, 4 cycles at 4 lines) so this makes sense, I'll take some time to check the other ops before submitting v3. (And I guess that's where I'll need to add stuff for continuous read too, it doesn't look like I can reuse the cont_read_cache_dual_quad_dtr_variants as the ops you added don't seem to exist here, if I understood it right I *think* I need to set some state register then with the same commands keeping CS down/clock ticking should keep providing data...? Well; I'll look it up some more too before bothering you again) Thank you again, -- Dominique