Message ID | 20230515094440.3552094-4-AVKrasnov@sberdevices.ru |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp6796083vqo; Mon, 15 May 2023 02:58:45 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ66qEMeIE7bDTK6Kh2Vcuvxofuxp8aCFGq70lMBYbDWf0pv2p+vbzsH0t/egrPUwJSCRNnf X-Received: by 2002:a05:6a20:12c8:b0:105:2199:7b93 with SMTP id v8-20020a056a2012c800b0010521997b93mr8170804pzg.50.1684144725310; Mon, 15 May 2023 02:58:45 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1684144725; cv=none; d=google.com; s=arc-20160816; b=mBQGQ7Y4xF41GXVbrxALsaXPALqhpdntY30Dw4X25s2s9vDHmCr+K95i0X23eZhPSw uLVAhDPfIxE4RaMUmQt/CgPcVB4vH3LFJjhFaaMv7GDQT7BCPDRbdPyckStRB/46uLF1 MDLY8tEcze5rJXkMLImq2qmQaqtVijqYJtZmhAGA6r97riDf3Rq0grSA4H6oYd8ZYLa7 0wwA+SbAsRqM/CEUAPKKru88M0+EzQHmW4NWl4I73WXEsjXUZRRaGRUMXN98qP5hn2Iw E4Mie8Ob9zWZC7goXRsS2rw7RP+yoghd7nv2nizEF80zHkw6lqPrDymPcRzMA90CpaNw xKww== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=Vz83jCy0spHA1ZRural7ddOQ1iT++4tUqhV7Ue23lk4=; b=j2MX+qwdNzY9L1sspde0NIFHrQ+2m8YpKEaz5gTvc5CjvuJGPAI7XztCIVLAe2cUZD SSrQlYaQfFIruYeUJ0IWTeIXUTTA40lgpn4TVyoBkRAtt1+0S8zmmCfJvzdA3ni/hFvi LKsFt9HztrTIDTvYIAPRFcLFMhNxds4ITaYhtAXQkB9RDjqevaR/EEsJ2+TcNwXmJQTi QMhtqoc1wUwGAPgqtM5dCrFhwH2+86rbtcUrhMpf2sovc0YaC4YdUO2WfvThguI1DAdB BbxSisiNwDWL3RvzKhwd7appI5RX7scXTJUvEaRgh5qvlK8WGEvvknb8qedjEvGqyN+n p8sA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sberdevices.ru header.s=mail header.b=qcn0IQfH; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=sberdevices.ru Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id q23-20020a638c57000000b0051b54dccffbsi15443621pgn.859.2023.05.15.02.58.32; Mon, 15 May 2023 02:58:45 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@sberdevices.ru header.s=mail header.b=qcn0IQfH; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=sberdevices.ru Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239215AbjEOJuK (ORCPT <rfc822;peekingduck44@gmail.com> + 99 others); Mon, 15 May 2023 05:50:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53910 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237900AbjEOJtt (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 15 May 2023 05:49:49 -0400 Received: from mx.sberdevices.ru (mx.sberdevices.ru [45.89.227.171]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 418991AD for <linux-kernel@vger.kernel.org>; Mon, 15 May 2023 02:49:48 -0700 (PDT) Received: from s-lin-edge02.sberdevices.ru (localhost [127.0.0.1]) by mx.sberdevices.ru (Postfix) with ESMTP id A1E125FD20; Mon, 15 May 2023 12:49:46 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sberdevices.ru; s=mail; t=1684144186; bh=Vz83jCy0spHA1ZRural7ddOQ1iT++4tUqhV7Ue23lk4=; h=From:To:Subject:Date:Message-ID:MIME-Version:Content-Type; b=qcn0IQfHKcGxRJrBonclWUzFGa4XbN3UU6+FFP2AsksiDymQ6qNyJELtmB+wp36eq PGVeFzgUnZww1VpOhjHQc58e5YFXfAN5i8YvvR4CIUctnyZ1aY4dVrn4aGRuy0NQfd m12CbKfAW7nSCh8ufHe4Hct5q1juezSISYESH0Nbt8QI243gw1FewU8wrF5fZ+YVAi lTjuvjuIfvhtsNSDqqR8z+GT+Qb4/2WO/sIIfYliAHUQ9QPFWwmd5Dz09Bm1II3kbW nDBgUSxU1tD1iqMAjGKSpGg4GvssvSm0RyiHy2X3nvvkVeqZqUnoQSXqFXVO08SkuM +nqn2DBNfqqKQ== Received: from S-MS-EXCH01.sberdevices.ru (S-MS-EXCH01.sberdevices.ru [172.16.1.4]) by mx.sberdevices.ru (Postfix) with ESMTP; Mon, 15 May 2023 12:49:46 +0300 (MSK) From: Arseniy Krasnov <AVKrasnov@sberdevices.ru> To: Liang Yang <liang.yang@amlogic.com>, Miquel Raynal <miquel.raynal@bootlin.com>, Richard Weinberger <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, Neil Armstrong <neil.armstrong@linaro.org>, Kevin Hilman <khilman@baylibre.com>, Jerome Brunet <jbrunet@baylibre.com>, Martin Blumenstingl <martin.blumenstingl@googlemail.com>, Jianxin Pan <jianxin.pan@amlogic.com>, Yixun Lan <yixun.lan@amlogic.com> CC: <oxffffaa@gmail.com>, <kernel@sberdevices.ru>, Arseniy Krasnov <AVKrasnov@sberdevices.ru>, <linux-mtd@lists.infradead.org>, <linux-arm-kernel@lists.infradead.org>, <linux-amlogic@lists.infradead.org>, <linux-kernel@vger.kernel.org> Subject: [PATCH v4 3/5] mtd: rawnand: meson: always read whole OOB bytes Date: Mon, 15 May 2023 12:44:37 +0300 Message-ID: <20230515094440.3552094-4-AVKrasnov@sberdevices.ru> X-Mailer: git-send-email 2.35.0 In-Reply-To: <20230515094440.3552094-1-AVKrasnov@sberdevices.ru> References: <20230515094440.3552094-1-AVKrasnov@sberdevices.ru> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [172.16.1.6] X-ClientProxiedBy: S-MS-EXCH01.sberdevices.ru (172.16.1.4) To S-MS-EXCH01.sberdevices.ru (172.16.1.4) X-KSMG-Rule-ID: 4 X-KSMG-Message-Action: clean X-KSMG-AntiSpam-Status: not scanned, disabled by settings X-KSMG-AntiSpam-Interceptor-Info: not scanned X-KSMG-AntiPhishing: not scanned, disabled by settings X-KSMG-AntiVirus: Kaspersky Secure Mail Gateway, version 1.1.2.30, bases: 2023/05/15 04:03:00 #21308474 X-KSMG-AntiVirus-Status: Clean, skipped X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765953739634620744?= X-GMAIL-MSGID: =?utf-8?q?1765953739634620744?= |
Series |
refactoring and fix for Meson NAND
|
|
Commit Message
Arseniy Krasnov
May 15, 2023, 9:44 a.m. UTC
This changes size of read access to OOB area by reading all bytes of
OOB (free bytes + ECC engine bytes).
Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
---
drivers/mtd/nand/raw/meson_nand.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
Comments
Hi Arseniy, AVKrasnov@sberdevices.ru wrote on Mon, 15 May 2023 12:44:37 +0300: > This changes size of read access to OOB area by reading all bytes of > OOB (free bytes + ECC engine bytes). This is normally up to the user (user in your case == jffs2). The controller driver should expose a number of user accessible bytes and then when users want the OOB area, they should access it entirely. On top of that read, they can extract (or "write only") the user bytes. > Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> > --- > drivers/mtd/nand/raw/meson_nand.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c > index 8526a6b87720..a31106c943d7 100644 > --- a/drivers/mtd/nand/raw/meson_nand.c > +++ b/drivers/mtd/nand/raw/meson_nand.c > @@ -755,6 +755,30 @@ static int __meson_nfc_read_oob(struct nand_chip *nand, int page, > u32 oob_bytes; > u32 page_size; > int ret; > + int i; > + > + /* Read ECC codes and user bytes. */ > + for (i = 0; i < nand->ecc.steps; i++) { > + u32 ecc_offs = nand->ecc.size * (i + 1) + > + NFC_OOB_PER_ECC(nand) * i; > + > + ret = nand_read_page_op(nand, page, 0, NULL, 0); > + if (ret) > + return ret; > + > + /* Use temporary buffer, because 'nand_change_read_column_op()' > + * seems work with some alignment, so we can't read data to > + * 'oob_buf' directly. DMA? > + */ > + ret = nand_change_read_column_op(nand, ecc_offs, meson_chip->oob_buf, > + NFC_OOB_PER_ECC(nand), false); > + if (ret) > + return ret; > + > + memcpy(oob_buf + i * NFC_OOB_PER_ECC(nand), > + meson_chip->oob_buf, > + NFC_OOB_PER_ECC(nand)); > + } > > oob_bytes = meson_nfc_get_oob_bytes(nand); > Thanks, Miquèl
On 22.05.2023 18:38, Miquel Raynal wrote: > Hi Arseniy, > > AVKrasnov@sberdevices.ru wrote on Mon, 15 May 2023 12:44:37 +0300: > >> This changes size of read access to OOB area by reading all bytes of >> OOB (free bytes + ECC engine bytes). > > This is normally up to the user (user in your case == jffs2). The > controller driver should expose a number of user accessible bytes and > then when users want the OOB area, they should access it entirely. On > top of that read, they can extract (or "write only") the user bytes. Sorry, I didn't get it. If driver exposes N bytes of user accessible bytes, I must always return whole OOB yes? E.g. N + rest of OOB > >> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >> --- >> drivers/mtd/nand/raw/meson_nand.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c >> index 8526a6b87720..a31106c943d7 100644 >> --- a/drivers/mtd/nand/raw/meson_nand.c >> +++ b/drivers/mtd/nand/raw/meson_nand.c >> @@ -755,6 +755,30 @@ static int __meson_nfc_read_oob(struct nand_chip *nand, int page, >> u32 oob_bytes; >> u32 page_size; >> int ret; >> + int i; >> + >> + /* Read ECC codes and user bytes. */ >> + for (i = 0; i < nand->ecc.steps; i++) { >> + u32 ecc_offs = nand->ecc.size * (i + 1) + >> + NFC_OOB_PER_ECC(nand) * i; >> + >> + ret = nand_read_page_op(nand, page, 0, NULL, 0); >> + if (ret) >> + return ret; >> + >> + /* Use temporary buffer, because 'nand_change_read_column_op()' >> + * seems work with some alignment, so we can't read data to >> + * 'oob_buf' directly. > > DMA? Yes I guess, this address passed to exec_op code and used as DMA. > >> + */ >> + ret = nand_change_read_column_op(nand, ecc_offs, meson_chip->oob_buf, >> + NFC_OOB_PER_ECC(nand), false); >> + if (ret) >> + return ret; >> + >> + memcpy(oob_buf + i * NFC_OOB_PER_ECC(nand), >> + meson_chip->oob_buf, >> + NFC_OOB_PER_ECC(nand)); >> + } >> >> oob_bytes = meson_nfc_get_oob_bytes(nand); >> > > > Thanks, > Miquèl Thanks, Arseniy
Hi Arseniy, avkrasnov@sberdevices.ru wrote on Tue, 23 May 2023 20:27:35 +0300: > On 22.05.2023 18:38, Miquel Raynal wrote: > > Hi Arseniy, > > > > AVKrasnov@sberdevices.ru wrote on Mon, 15 May 2023 12:44:37 +0300: > > > >> This changes size of read access to OOB area by reading all bytes of > >> OOB (free bytes + ECC engine bytes). > > > > This is normally up to the user (user in your case == jffs2). The > > controller driver should expose a number of user accessible bytes and > > then when users want the OOB area, they should access it entirely. On > > top of that read, they can extract (or "write only") the user bytes. > > Sorry, I didn't get it. If driver exposes N bytes of user accessible bytes, > I must always return whole OOB yes? E.g. N + rest of OOB Yes. At the NAND controller level, you get asked for either a page of data (sometimes a subpage, but whatever), and/or the oob area. You need to provide what is requested, no more, no less. The upper layers will trim down what's uneeded and extract the bytes they want. > >> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> > >> --- > >> drivers/mtd/nand/raw/meson_nand.c | 24 ++++++++++++++++++++++++ > >> 1 file changed, 24 insertions(+) > >> > >> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c > >> index 8526a6b87720..a31106c943d7 100644 > >> --- a/drivers/mtd/nand/raw/meson_nand.c > >> +++ b/drivers/mtd/nand/raw/meson_nand.c > >> @@ -755,6 +755,30 @@ static int __meson_nfc_read_oob(struct nand_chip *nand, int page, > >> u32 oob_bytes; > >> u32 page_size; > >> int ret; > >> + int i; > >> + > >> + /* Read ECC codes and user bytes. */ > >> + for (i = 0; i < nand->ecc.steps; i++) { > >> + u32 ecc_offs = nand->ecc.size * (i + 1) + > >> + NFC_OOB_PER_ECC(nand) * i; > >> + > >> + ret = nand_read_page_op(nand, page, 0, NULL, 0); > >> + if (ret) > >> + return ret; > >> + > >> + /* Use temporary buffer, because 'nand_change_read_column_op()' > >> + * seems work with some alignment, so we can't read data to > >> + * 'oob_buf' directly. > > > > DMA? > > Yes I guess, this address passed to exec_op code and used as DMA. If your controller uses DMA on exec_op accesses, then yes. Exec_op reads/writes are usually small enough (or not time sensitive at all if they are bigger) so it's not required to use DMA there. Anyhow, oob_buf is suitable for DMA purposes, so I'm a bit surprised you need a bounce buffer, if that's the only reason. Maybe you need a bounce buffer to reorganize the data. That would be a much better explanation. > >> + */ > >> + ret = nand_change_read_column_op(nand, ecc_offs, meson_chip->oob_buf, > >> + NFC_OOB_PER_ECC(nand), false); > >> + if (ret) > >> + return ret; > >> + > >> + memcpy(oob_buf + i * NFC_OOB_PER_ECC(nand), > >> + meson_chip->oob_buf, > >> + NFC_OOB_PER_ECC(nand)); > >> + } > >> > >> oob_bytes = meson_nfc_get_oob_bytes(nand); > >> > > > > > > Thanks, > > Miquèl > > Thanks, Arseniy Thanks, Miquèl
On 26.05.2023 20:09, Miquel Raynal wrote: > Hi Arseniy, > > avkrasnov@sberdevices.ru wrote on Tue, 23 May 2023 20:27:35 +0300: > >> On 22.05.2023 18:38, Miquel Raynal wrote: >>> Hi Arseniy, >>> >>> AVKrasnov@sberdevices.ru wrote on Mon, 15 May 2023 12:44:37 +0300: >>> >>>> This changes size of read access to OOB area by reading all bytes of >>>> OOB (free bytes + ECC engine bytes). >>> >>> This is normally up to the user (user in your case == jffs2). The >>> controller driver should expose a number of user accessible bytes and >>> then when users want the OOB area, they should access it entirely. On >>> top of that read, they can extract (or "write only") the user bytes. >> >> Sorry, I didn't get it. If driver exposes N bytes of user accessible bytes, >> I must always return whole OOB yes? E.g. N + rest of OOB > > Yes. At the NAND controller level, you get asked for either a page of > data (sometimes a subpage, but whatever), and/or the oob area. You need > to provide what is requested, no more, no less. The upper layers will > trim down what's uneeded and extract the bytes they want. I see, so in this case I think this patch could be merged to the patch which changes OOB layout be moving it out of ECC area? Because driver MUST return all bytes of OOB area. > >>>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >>>> --- >>>> drivers/mtd/nand/raw/meson_nand.c | 24 ++++++++++++++++++++++++ >>>> 1 file changed, 24 insertions(+) >>>> >>>> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c >>>> index 8526a6b87720..a31106c943d7 100644 >>>> --- a/drivers/mtd/nand/raw/meson_nand.c >>>> +++ b/drivers/mtd/nand/raw/meson_nand.c >>>> @@ -755,6 +755,30 @@ static int __meson_nfc_read_oob(struct nand_chip *nand, int page, >>>> u32 oob_bytes; >>>> u32 page_size; >>>> int ret; >>>> + int i; >>>> + >>>> + /* Read ECC codes and user bytes. */ >>>> + for (i = 0; i < nand->ecc.steps; i++) { >>>> + u32 ecc_offs = nand->ecc.size * (i + 1) + >>>> + NFC_OOB_PER_ECC(nand) * i; >>>> + >>>> + ret = nand_read_page_op(nand, page, 0, NULL, 0); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* Use temporary buffer, because 'nand_change_read_column_op()' >>>> + * seems work with some alignment, so we can't read data to >>>> + * 'oob_buf' directly. >>> >>> DMA? >> >> Yes I guess, this address passed to exec_op code and used as DMA. > > If your controller uses DMA on exec_op accesses, then yes. Exec_op > reads/writes are usually small enough (or not time sensitive at all if > they are bigger) so it's not required to use DMA there. Anyhow, oob_buf > is suitable for DMA purposes, so I'm a bit surprised you need a bounce > buffer, if that's the only reason. Maybe you need a bounce buffer to > reorganize the data. That would be a much better explanation. Yes! I remove this temporary buffer, seems my mistake! Without it everything works good, I'll remove it from the next version! Thanks, Arseniy > >>>> + */ >>>> + ret = nand_change_read_column_op(nand, ecc_offs, meson_chip->oob_buf, >>>> + NFC_OOB_PER_ECC(nand), false); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + memcpy(oob_buf + i * NFC_OOB_PER_ECC(nand), >>>> + meson_chip->oob_buf, >>>> + NFC_OOB_PER_ECC(nand)); >>>> + } >>>> >>>> oob_bytes = meson_nfc_get_oob_bytes(nand); >>>> >>> >>> >>> Thanks, >>> Miquèl >> >> Thanks, Arseniy > > > Thanks, > Miquèl
diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c index 8526a6b87720..a31106c943d7 100644 --- a/drivers/mtd/nand/raw/meson_nand.c +++ b/drivers/mtd/nand/raw/meson_nand.c @@ -755,6 +755,30 @@ static int __meson_nfc_read_oob(struct nand_chip *nand, int page, u32 oob_bytes; u32 page_size; int ret; + int i; + + /* Read ECC codes and user bytes. */ + for (i = 0; i < nand->ecc.steps; i++) { + u32 ecc_offs = nand->ecc.size * (i + 1) + + NFC_OOB_PER_ECC(nand) * i; + + ret = nand_read_page_op(nand, page, 0, NULL, 0); + if (ret) + return ret; + + /* Use temporary buffer, because 'nand_change_read_column_op()' + * seems work with some alignment, so we can't read data to + * 'oob_buf' directly. + */ + ret = nand_change_read_column_op(nand, ecc_offs, meson_chip->oob_buf, + NFC_OOB_PER_ECC(nand), false); + if (ret) + return ret; + + memcpy(oob_buf + i * NFC_OOB_PER_ECC(nand), + meson_chip->oob_buf, + NFC_OOB_PER_ECC(nand)); + } oob_bytes = meson_nfc_get_oob_bytes(nand);