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 X-Spam-Level: X-Spam-Status: No, score=-11.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 56421C2D0A8 for ; Mon, 28 Sep 2020 14:55:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 19BC22100A for ; Mon, 28 Sep 2020 14:55:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726613AbgI1Oze convert rfc822-to-8bit (ORCPT ); Mon, 28 Sep 2020 10:55:34 -0400 Received: from relay7-d.mail.gandi.net ([217.70.183.200]:59859 "EHLO relay7-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726424AbgI1Oze (ORCPT ); Mon, 28 Sep 2020 10:55:34 -0400 X-Originating-IP: 91.224.148.103 Received: from xps13 (unknown [91.224.148.103]) (Authenticated sender: miquel.raynal@bootlin.com) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 6B4C520005; Mon, 28 Sep 2020 14:55:30 +0000 (UTC) Date: Mon, 28 Sep 2020 16:55:28 +0200 From: Miquel Raynal To: Thirumalesha N Cc: Richard Weinberger , Vignesh Raghavendra , Shivamurthy Shastri , Boris Brezillon , Chuanhong Guo , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names Message-ID: <20200928165528.54e5db6e@xps13> In-Reply-To: References: <20200913161533.10655-1-nthirumalesha7@gmail.com> <20200913161533.10655-2-nthirumalesha7@gmail.com> <20200915101321.1afa5033@xps13> Organization: Bootlin X-Mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Thirumalesha, Thirumalesha N wrote on Sun, 20 Sep 2020 22:38:53 +0800: > Hi Miquèl, > > I adopted these changes as per the Boris Brezillon comments > > On Tue, Sep 15, 2020 at 4:13 PM Miquel Raynal > wrote: > > > Hi Thirumalesha, > > > > Thirumalesha Narasimhappa wrote on Mon, 14 > > Sep 2020 00:15:32 +0800: > > > > > Rename the oob structure and read/write/update function names to > > > a generic names > > > > > > Signed-off-by: Thirumalesha Narasimhappa > > > --- > > > drivers/mtd/nand/spi/micron.c | 80 +++++++++++++++++------------------ > > > 1 file changed, 40 insertions(+), 40 deletions(-) > > > > > > diff --git a/drivers/mtd/nand/spi/micron.c > > b/drivers/mtd/nand/spi/micron.c > > > index 5d370cfcdaaa..d1b1073d1a55 100644 > > > --- a/drivers/mtd/nand/spi/micron.c > > > +++ b/drivers/mtd/nand/spi/micron.c > > > @@ -28,7 +28,7 @@ > > > > > > #define MICRON_SELECT_DIE(x) ((x) << 6) > > > > > > -static SPINAND_OP_VARIANTS(read_cache_variants, > > > +static SPINAND_OP_VARIANTS(quadio_read_cache_variants, > > > SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0), > > > SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0), > > > SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0), > > > @@ -36,11 +36,11 @@ static SPINAND_OP_VARIANTS(read_cache_variants, > > > SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0), > > > SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0)); > > > > > > -static SPINAND_OP_VARIANTS(write_cache_variants, > > > +static SPINAND_OP_VARIANTS(x4_write_cache_variants, > > > > Why quadio_read and x4_write? I don't get the logic. > > > > IMHO, quadio_read is derived from SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP & > x4_write is derived from SPINAND_PROG_LOAD_X4 > > Boris, Please correct me, if my understanding is wrong I see. Then please split this change, one updating the helpers with an "x4" prefix when relevant and another one for 8 vs. grouped. > > > > > SPINAND_PROG_LOAD_X4(true, 0, NULL, 0), > > > SPINAND_PROG_LOAD(true, 0, NULL, 0)); > > > > > > -static SPINAND_OP_VARIANTS(update_cache_variants, > > > +static SPINAND_OP_VARIANTS(x4_update_cache_variants, > > > SPINAND_PROG_LOAD_X4(false, 0, NULL, 0), > > > SPINAND_PROG_LOAD(false, 0, NULL, 0)); > > > > > > @@ -69,7 +69,7 @@ static int micron_8_ooblayout_free(struct mtd_info > > *mtd, int section, > > > return 0; > > > } > > > > > > -static const struct mtd_ooblayout_ops micron_8_ooblayout = { > > > +static const struct mtd_ooblayout_ops micron_grouped_ooblayout = { > > > > Is this necessary? What does "grouped" means? Should we rename all > > functions with _8_ in it to make sense? > > > > IMHO, grouped means, ecc bytes are at continuous address, where as > interleaved means ecc bytes splitted into multiple addresses I don't like the name. Interleaved means that there are OOB bytes stored in the data section, which is not the case here. The way OOB bytes are organized do not seem relevant to me, I think i prefer the "_4_/_8_" naming,even if it's not very explicit. Also please be consistent, do not mix "_4_" and "interleaved" (see the other patch). Any feedback from Micron to validate these changes will be appreciated. Thanks, Miquèl