Re: [PATCH 1/8] mtd: nand: move ONFI related functions to onfi.h

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: shiva.linuxworks@gmail.com
Cc: Richard Weinberger <richard@nod.at>,
	David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Marek Vasut <marek.vasut@gmail.com>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Boris Brezillon <bbrezillon@kernel.org>,
	Marcel Ziswiler <marcel.ziswiler@toradex.com>,
	Frieder Schrempf <frieder.schrempf@kontron.de>,
	Shivamurthy Shastri <sshivamurthy@micron.com>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	Jeff Kletsky <git-commits@allycomm.com>,
	Chuanhong Guo <gch981213@gmail.com>,
	liaoweixiong <liaoweixiong@allwinnertech.com>
Subject: Re: [PATCH 1/8] mtd: nand: move ONFI related functions to onfi.h
Date: Wed, 7 Aug 2019 10:34:37 +0200	[thread overview]
Message-ID: <20190807103437.36abb59b@xps13> (raw)
In-Reply-To: <20190722055621.23526-2-sshivamurthy@micron.com>

Hi Shiva,

shiva.linuxworks@gmail.com wrote on Mon, 22 Jul 2019 07:56:14 +0200:

> From: Shivamurthy Shastri <sshivamurthy@micron.com>
> 
> These functions will be used by both raw NAND and SPI NAND, which
> supports ONFI like standards.

This is not exactly what you do. Why not:

mtd: nand: export ONFI related functions to onfi.h

These functions can be used by all flavors of NAND chips (raw, SPI)
which may all follow ONFI standards. Export the related functions in
the onfi.h generic file.

> 
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> ---
>  drivers/mtd/nand/raw/internals.h | 1 -
>  include/linux/mtd/onfi.h         | 9 +++++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index cba6fe7dd8c4..ed323087d884 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -140,7 +140,6 @@ void nand_legacy_adjust_cmdfunc(struct nand_chip *chip);
>  int nand_legacy_check_hooks(struct nand_chip *chip);
>  
>  /* ONFI functions */
> -u16 onfi_crc16(u16 crc, u8 const *p, size_t len);
>  int nand_onfi_detect(struct nand_chip *chip);
>  
>  /* JEDEC functions */
> diff --git a/include/linux/mtd/onfi.h b/include/linux/mtd/onfi.h
> index 339ac798568e..2c8a05a02bb0 100644
> --- a/include/linux/mtd/onfi.h
> +++ b/include/linux/mtd/onfi.h
> @@ -10,6 +10,7 @@
>  #ifndef __LINUX_MTD_ONFI_H
>  #define __LINUX_MTD_ONFI_H
>  
> +#include <linux/mtd/nand.h>

This should be removed, or at least not added at this moment.

>  #include <linux/types.h>
>  
>  /* ONFI version bits */
> @@ -175,4 +176,12 @@ struct onfi_params {
>  	u8 vendor[88];
>  };
>  
> +/* ONFI functions */
> +u16 onfi_crc16(u16 crc, u8 const *p, size_t len);
> +void nand_bit_wise_majority(const void **srcbufs,
> +			    unsigned int nsrcbufs,
> +			    void *dstbuf,
> +			    unsigned int bufsize);

Don't export this function while you don't use it from elsewhere.

> +void sanitize_string(u8 *s, size_t len);

This one is used by jedec code and has no onfi-related logic, so you
may want to export it (only when you will use it) in another header
like linux/mtd/nand.h

> +
>  #endif /* __LINUX_MTD_ONFI_H */

Thanks,
Miquèl