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 0DAE6C4332F for ; Thu, 14 Apr 2022 15:07:26 +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:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=0nLz34Zj25ZHSLs1fPM0s3+WoeTvGKsJiTzXfw2MmSI=; b=sWh35xbDaSB+yI uZutkVc1XezYqLKnUNrMUoZZF7lFiSxiS9vRTobxTkOjXusVr5e+GQPY/mPL+X3+VKD/iy0I/1F1D jvNVSrfixKg1uyS9cKZVWl7C8LlzWzfwfSW6AI0FMr2Do1oSq+igciU5y7dQU+UC8pBUNt02+DFmz 9SeTo9Za1aZvy3tZOdNBT3yKMSErrDd7yykeYjBq+lpeBzOPDip1hE8RNiMrqfIJ1LMvKgX0K1z3F jBS/9VqO1QyW+c1f6VTuptLQKCqlET6vVU6CtAu8T9IOoTook4yMtxAx8aizDWWskwru7fwVWeTm/ nBV2XO5O7nkPdApHDtUg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nf13g-0069hW-OT; Thu, 14 Apr 2022 15:06:56 +0000 Received: from bhuna.collabora.co.uk ([46.235.227.227]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nf13e-0069f4-3L for linux-mtd@lists.infradead.org; Thu, 14 Apr 2022 15:06:55 +0000 Received: from localhost (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 218D21F47B07; Thu, 14 Apr 2022 16:06:48 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1649948808; bh=UpSLyopnlq5YAkIaxL8rC/z8gQiFGUjISBXjuDV87YA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=b0pwOTmW+kxJoLFq5v97T8FjTuaAb8wrqPxesw6cwS+6nTA86EsvdR+Chu26nEWCy Xf3qKjda/0QTX/bFoVGdrjagui6XsSTSQ1L9HwCXh74Rm5w5x0DiRSjHVb0JxxNPNQ bqWf+vIKAbX/hVQfy7h2oTN2Iy0IZKRAebF+sd41nSxF4QV0psAsLqbKE2uWE8/WlE qcVEUlR3nJwD01Nq3MxUmpUKrKWr2rLCJUC36537UvTZAn/x0R6pehthyq6/P2TqtC xv6GBdR7477RO7wlrat+EGIYskHD8T4Xr1PPPiqCP99itD0GSaDanjiPUTpFkJP+5h qVJgqOvlkKTAA== Date: Thu, 14 Apr 2022 17:06:44 +0200 From: Boris Brezillon To: Chuanhong Guo Cc: linux-mtd@lists.infradead.org, Miquel Raynal , Richard Weinberger , Vignesh Raghavendra , Patrice Chotard , Christophe Kerello , Mark Brown , Daniel Palmer , linux-kernel@vger.kernel.org (open list) Subject: Re: [PATCH 1/2] mtd: spinand: add support for detection with param page Message-ID: <20220414170644.4a53484f@collabora.com> In-Reply-To: <20220414143426.723168-2-gch981213@gmail.com> References: <20220414143426.723168-1-gch981213@gmail.com> <20220414143426.723168-2-gch981213@gmail.com> Organization: Collabora X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.31; x86_64-redhat-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220414_080654_399937_8910C50B X-CRM114-Status: GOOD ( 23.76 ) 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="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org On Thu, 14 Apr 2022 22:34:25 +0800 Chuanhong Guo wrote: > SPI-NAND detection using chip ID isn't always reliable. > Here are two known cases: > 1. ESMT uses JEDEC ID from other vendors. This may collapse with future > chips. Really?! So the manufacturer id matching is not even possible? > 2. Winbond W25N01KV uses the exact same JEDEC ID as W25N01GV while > having completely different chip parameters. > > Recent SPI-NANDs have a parameter page which is stored in the same > format as raw NAND ONFI data. There are strings for chip manufacturer > and chip model. Chip ECC requirement and memory organization are > available too. > This patch adds support for detecting SPI-NANDs with the parameter page > after ID matching failed. In this detection, memory organization and > ECC requirements are taken from the parameter page, and other SPI-NAND > specific parameters are obtained by matching chip model string with > a separated table. Oh come on! Looks like they never learn from their mistakes... > > It's common for vendors to release a series of SPI-NANDs with the same > SPI-NAND parameters in different voltages and/or capacities. The chip > table defined in this patch supports multiple model strings in one > entry, and multiple chip models can be covered using only one entry. That's really disappointing. And I guess the ONFI-page read commands are not even standard, and each vendor will come with its own sequence to read it. What's bothering me the most is the fact that ONFI parameter pages are not even designed for SPI NANDs. They have a bunch of parameters that don't really apply to SPI NANDs, and we're still lacking SPI-specific info, like the read/write/erase command variants, the maximum SPI bus width (AKA SPI modes)... To sum-up, if we have to support reading the ONFI parameter page, I'd rather keep it vendor-private (but that's assuming the vendor ID detection works, and you seem to imply ESMT cheats on that too), and use it only as a way to distinguish between 2 chip variants. > > Signed-off-by: Chuanhong Guo > --- > > I'm not brave enough to touch raw nand code without testing, so > sanitize_string, onfi_crc16 and nand_bit_wise_majority are > duplicated from raw/nand_onfi.c into spi/onfi.c > > drivers/mtd/nand/spi/Makefile | 2 +- > drivers/mtd/nand/spi/core.c | 23 +-- > drivers/mtd/nand/spi/onfi.c | 296 ++++++++++++++++++++++++++++++++++ Please, no. Try to move the common code to drivers/mtd/nand/onfi.c instead of duplicating it. AFAICT, the only thing that's spinand specific is spinand_onfi_read() and the last part of spinand_onfi_detect(). I'm sure we can find a way to expose generic onfi_parsing() helpers. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D8B5DC352A7 for ; Thu, 14 Apr 2022 15:45:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241245AbiDNPnT (ORCPT ); Thu, 14 Apr 2022 11:43:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57494 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1354587AbiDNPSX (ORCPT ); Thu, 14 Apr 2022 11:18:23 -0400 Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4EBD7ECB3B for ; Thu, 14 Apr 2022 08:06:50 -0700 (PDT) Received: from localhost (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 218D21F47B07; Thu, 14 Apr 2022 16:06:48 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1649948808; bh=UpSLyopnlq5YAkIaxL8rC/z8gQiFGUjISBXjuDV87YA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=b0pwOTmW+kxJoLFq5v97T8FjTuaAb8wrqPxesw6cwS+6nTA86EsvdR+Chu26nEWCy Xf3qKjda/0QTX/bFoVGdrjagui6XsSTSQ1L9HwCXh74Rm5w5x0DiRSjHVb0JxxNPNQ bqWf+vIKAbX/hVQfy7h2oTN2Iy0IZKRAebF+sd41nSxF4QV0psAsLqbKE2uWE8/WlE qcVEUlR3nJwD01Nq3MxUmpUKrKWr2rLCJUC36537UvTZAn/x0R6pehthyq6/P2TqtC xv6GBdR7477RO7wlrat+EGIYskHD8T4Xr1PPPiqCP99itD0GSaDanjiPUTpFkJP+5h qVJgqOvlkKTAA== Date: Thu, 14 Apr 2022 17:06:44 +0200 From: Boris Brezillon To: Chuanhong Guo Cc: linux-mtd@lists.infradead.org, Miquel Raynal , Richard Weinberger , Vignesh Raghavendra , Patrice Chotard , Christophe Kerello , Mark Brown , Daniel Palmer , linux-kernel@vger.kernel.org (open list) Subject: Re: [PATCH 1/2] mtd: spinand: add support for detection with param page Message-ID: <20220414170644.4a53484f@collabora.com> In-Reply-To: <20220414143426.723168-2-gch981213@gmail.com> References: <20220414143426.723168-1-gch981213@gmail.com> <20220414143426.723168-2-gch981213@gmail.com> Organization: Collabora X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.31; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 14 Apr 2022 22:34:25 +0800 Chuanhong Guo wrote: > SPI-NAND detection using chip ID isn't always reliable. > Here are two known cases: > 1. ESMT uses JEDEC ID from other vendors. This may collapse with future > chips. Really?! So the manufacturer id matching is not even possible? > 2. Winbond W25N01KV uses the exact same JEDEC ID as W25N01GV while > having completely different chip parameters. > > Recent SPI-NANDs have a parameter page which is stored in the same > format as raw NAND ONFI data. There are strings for chip manufacturer > and chip model. Chip ECC requirement and memory organization are > available too. > This patch adds support for detecting SPI-NANDs with the parameter page > after ID matching failed. In this detection, memory organization and > ECC requirements are taken from the parameter page, and other SPI-NAND > specific parameters are obtained by matching chip model string with > a separated table. Oh come on! Looks like they never learn from their mistakes... > > It's common for vendors to release a series of SPI-NANDs with the same > SPI-NAND parameters in different voltages and/or capacities. The chip > table defined in this patch supports multiple model strings in one > entry, and multiple chip models can be covered using only one entry. That's really disappointing. And I guess the ONFI-page read commands are not even standard, and each vendor will come with its own sequence to read it. What's bothering me the most is the fact that ONFI parameter pages are not even designed for SPI NANDs. They have a bunch of parameters that don't really apply to SPI NANDs, and we're still lacking SPI-specific info, like the read/write/erase command variants, the maximum SPI bus width (AKA SPI modes)... To sum-up, if we have to support reading the ONFI parameter page, I'd rather keep it vendor-private (but that's assuming the vendor ID detection works, and you seem to imply ESMT cheats on that too), and use it only as a way to distinguish between 2 chip variants. > > Signed-off-by: Chuanhong Guo > --- > > I'm not brave enough to touch raw nand code without testing, so > sanitize_string, onfi_crc16 and nand_bit_wise_majority are > duplicated from raw/nand_onfi.c into spi/onfi.c > > drivers/mtd/nand/spi/Makefile | 2 +- > drivers/mtd/nand/spi/core.c | 23 +-- > drivers/mtd/nand/spi/onfi.c | 296 ++++++++++++++++++++++++++++++++++ Please, no. Try to move the common code to drivers/mtd/nand/onfi.c instead of duplicating it. AFAICT, the only thing that's spinand specific is spinand_onfi_read() and the last part of spinand_onfi_detect(). I'm sure we can find a way to expose generic onfi_parsing() helpers.