Subject: Re: [RFC PATCH 20/27] mtd: nand: Let software ECC engines be retrieved from the NAND core

From: Boris Brezillon <[email-masked]>
Date: Fri, 22 Feb 2019 15:29:57 +0100

On Thu, 21 Feb 2019 13:57:59 +0100
Miquel Raynal <[email-masked]> wrote:

> Before making use of the ECC engines, we must retrieve them. Add the
> boilerplate for the ones already available: software engines (Hamming
> and BCH).
> 
> Signed-off-by: Miquel Raynal <[email-masked]>
> ---
>  drivers/mtd/nand/ecc/engine.c              | 14 ++++++++++++++
>  include/linux/mtd/nand-sw-bch-engine.h     |  3 +++
>  include/linux/mtd/nand-sw-hamming-engine.h |  3 +++
>  include/linux/mtd/nand.h                   |  3 +++
>  4 files changed, 23 insertions(+)
> 
> diff --git a/drivers/mtd/nand/ecc/engine.c b/drivers/mtd/nand/ecc/engine.c
> index 7dd3f9772698..318dbb2d56a2 100644
> --- a/drivers/mtd/nand/ecc/engine.c
> +++ b/drivers/mtd/nand/ecc/engine.c
> @@ -286,6 +286,20 @@ bool nand_ecc_correction_is_enough(struct nand_device *nand)
>  	return corr >= ds_corr && conf->strength >= reqs->strength;
>  }
>  
> +struct nand_ecc_engine *nand_ecc_sw_get_engine(struct nand_device *nand)

What if you want to instantiate SW ECC with a custom layout? Can't we
instead have a function that create a new engine dynamically?

struct nand_ecc_engine *
nand_ecc_create_sw_engine(struct nand_device* nand,
			  enum nand_ecc_algo algo,
			  struct mtd_ooblayout *layout);

> +{
> +	switch (nand->ecc.user_conf.algo) {

Note that the conf is supposed to be passed afterwards, when the ctx is
created, so you should check nand->ecc.user_conf directly here.

> +	case NAND_ECC_HAMMING:
> +		return ecc_sw_hamming_get_engine();
> +	case NAND_ECC_BCH:
> +		return ecc_sw_bch_get_engine();
> +	default:
> +		break;
> +	}
> +
> +	return NULL;
> +}
> +
>  MODULE_LICENSE("GPL v2");
>  MODULE_AUTHOR("Miquel Raynal <[email-masked]>");
>  MODULE_DESCRIPTION("Generic ECC engine");
> diff --git a/include/linux/mtd/nand-sw-bch-engine.h b/include/linux/mtd/nand-sw-bch-engine.h
> index e406aa53ec4e..38cdad117fea 100644
> --- a/include/linux/mtd/nand-sw-bch-engine.h
> +++ b/include/linux/mtd/nand-sw-bch-engine.h
> @@ -11,6 +11,9 @@
>  #include <linux/mtd/nand.h>
>  #include <linux/bch.h>
>  
> +/* Needed for cross inclusion with nand.h */
> +struct nand_device;
> +
>  /**
>   * struct ecc_sw_bch_conf - private software BCH ECC engine structure
>   * @reqooblen: Save the actual user OOB length requested before overwriting it
> diff --git a/include/linux/mtd/nand-sw-hamming-engine.h b/include/linux/mtd/nand-sw-hamming-engine.h
> index 8df36d189482..51c5a2acee42 100644
> --- a/include/linux/mtd/nand-sw-hamming-engine.h
> +++ b/include/linux/mtd/nand-sw-hamming-engine.h
> @@ -12,6 +12,9 @@
>  
>  #include <linux/mtd/nand.h>
>  
> +/* Needed for cross inclusion with nand.h */
> +struct nand_device;
> +
>  /**
>   * struct ecc_sw_hamming_conf - private software Hamming ECC engine structure
>   * @reqooblen: Save the actual user OOB length requested before overwriting it
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 4482eb2bbfd4..3abe113e4f06 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -11,6 +11,8 @@
>  #define __LINUX_MTD_NAND_H
>  
>  #include <linux/mtd/mtd.h>
> +#include <linux/mtd/nand-sw-hamming-engine.h>
> +#include <linux/mtd/nand-sw-bch-engine.h>
>  
>  struct nand_device;
>  
> @@ -253,6 +255,7 @@ struct nand_ecc_engine {
>  };
>  
>  void nand_ecc_read_user_conf(struct nand_device *nand);
> +struct nand_ecc_engine *nand_ecc_sw_get_engine(struct nand_device *nand);
>  int nand_ecc_init_ctx(struct nand_device *nand);
>  void nand_ecc_cleanup_ctx(struct nand_device *nand);
>  int nand_ecc_prepare_io_req(struct nand_device *nand,

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel