Re: [RFC,v3 2/5] mtd: ecc: realize Mediatek HW ECC driver

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Xiangsheng,

xiangsheng.hou@mediatek.com wrote on Fri, 22 Oct 2021 10:40:18 +0800:

> The v3 driver realize Mediatek HW ECC engine with pipelined
> case.

v3 driver? I guess you are talking about the hardware?

I don't think 'realize' makes much sense here. Perhaps the title could
be:
"mtd: ecc: mtk: Convert to the ECC infrastructure

> 
> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> ---
>  drivers/mtd/nand/core.c     |  10 +-
>  drivers/mtd/nand/ecc.c      |  88 +++++++
>  drivers/mtd/nand/mtk_ecc.c  | 488 ++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/mtk_ecc.h |  38 +++
>  include/linux/mtd/nand.h    |  11 +
>  5 files changed, 632 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
> index 5e13a03d2b32..b228b4d13b7a 100644
> --- a/drivers/mtd/nand/core.c
> +++ b/drivers/mtd/nand/core.c
> @@ -232,7 +232,9 @@ static int nanddev_get_ecc_engine(struct nand_device *nand)
>  		nand->ecc.engine = nand_ecc_get_on_die_hw_engine(nand);
>  		break;
>  	case NAND_ECC_ENGINE_TYPE_ON_HOST:
> -		pr_err("On-host hardware ECC engines not supported yet\n");
> +		nand->ecc.engine = nand_ecc_get_on_host_hw_engine(nand);
> +		if (PTR_ERR(nand->ecc.engine) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;

Please base your series on top of my previous work (even if not 100%
stable yet) to avoid leaking core changes here. They already exist, no
need to duplicate them.

>  		break;
>  	default:
>  		pr_err("Missing ECC engine type\n");
> @@ -252,7 +254,7 @@ static int nanddev_put_ecc_engine(struct nand_device *nand)
>  {
>  	switch (nand->ecc.ctx.conf.engine_type) {
>  	case NAND_ECC_ENGINE_TYPE_ON_HOST:
> -		pr_err("On-host hardware ECC engines not supported yet\n");
> +		nand_ecc_put_on_host_hw_engine(nand);
>  		break;
>  	case NAND_ECC_ENGINE_TYPE_NONE:
>  	case NAND_ECC_ENGINE_TYPE_SOFT:
> @@ -297,7 +299,9 @@ int nanddev_ecc_engine_init(struct nand_device *nand)
>  	/* Look for the ECC engine to use */
>  	ret = nanddev_get_ecc_engine(nand);
>  	if (ret) {
> -		pr_err("No ECC engine found\n");
> +		if (ret != -EPROBE_DEFER)
> +			pr_err("No ECC engine found\n");
> +
>  		return ret;
>  	}
>  
> diff --git a/drivers/mtd/nand/ecc.c b/drivers/mtd/nand/ecc.c
> index 6c43dfda01d4..55d6946da9c3 100644
> --- a/drivers/mtd/nand/ecc.c
> +++ b/drivers/mtd/nand/ecc.c
> @@ -96,7 +96,12 @@
>  #include <linux/module.h>
>  #include <linux/mtd/nand.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
>  
> +static LIST_HEAD(on_host_hw_engines);
> +static DEFINE_MUTEX(on_host_hw_engines_mutex);
>  /**
>   * nand_ecc_init_ctx - Init the ECC engine context
>   * @nand: the NAND device
> @@ -611,6 +616,89 @@ struct nand_ecc_engine *nand_ecc_get_on_die_hw_engine(struct nand_device *nand)
>  }
>  EXPORT_SYMBOL(nand_ecc_get_on_die_hw_engine);
>  
> +int nand_ecc_register_on_host_hw_engine(struct nand_ecc_engine *engine)
> +{
> +	struct nand_ecc_engine *item;
> +
> +	if (!engine)
> +		return -ENOTSUPP;
> +
> +	/* Prevent multiple registerations of one engine */
> +	list_for_each_entry(item, &on_host_hw_engines, node)
> +		if (item == engine)
> +			return 0;
> +
> +	mutex_lock(&on_host_hw_engines_mutex);
> +	list_add_tail(&engine->node, &on_host_hw_engines);
> +	mutex_unlock(&on_host_hw_engines_mutex);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(nand_ecc_register_on_host_hw_engine);
> +
> +int nand_ecc_unregister_on_host_hw_engine(struct nand_ecc_engine *engine)
> +{
> +	if (!engine)
> +		return -ENOTSUPP;
> +
> +	mutex_lock(&on_host_hw_engines_mutex);
> +	list_del(&engine->node);
> +	mutex_unlock(&on_host_hw_engines_mutex);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(nand_ecc_unregister_on_host_hw_engine);
> +
> +struct nand_ecc_engine *nand_ecc_match_on_host_hw_engine(struct device *dev)
> +{
> +	struct nand_ecc_engine *item;
> +
> +	list_for_each_entry(item, &on_host_hw_engines, node)
> +		if (item->dev == dev)
> +			return item;
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL(nand_ecc_match_on_host_hw_engine);
> +
> +struct nand_ecc_engine *nand_ecc_get_on_host_hw_engine(struct nand_device *nand)
> +{
> +	struct nand_ecc_engine *engine = NULL;
> +	struct device *dev = &nand->mtd.dev;
> +	struct platform_device *pdev;
> +	struct device_node *np;
> +
> +	if (list_empty(&on_host_hw_engines))
> +		return NULL;
> +
> +	/* Check for an explicit nand-ecc-engine property */
> +	np = of_parse_phandle(dev->of_node, "nand-ecc-engine", 0);
> +	if (np) {
> +		pdev = of_find_device_by_node(np);
> +		if (!pdev)
> +			return ERR_PTR(-EPROBE_DEFER);
> +
> +		engine = nand_ecc_match_on_host_hw_engine(&pdev->dev);
> +		platform_device_put(pdev);
> +		of_node_put(np);
> +
> +		if (!engine)
> +			return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	if (engine)
> +		get_device(engine->dev);
> +
> +	return engine;
> +}
> +EXPORT_SYMBOL(nand_ecc_get_on_host_hw_engine);
> +
> +void nand_ecc_put_on_host_hw_engine(struct nand_device *nand)
> +{
> +	put_device(nand->ecc.engine->dev);
> +}
> +EXPORT_SYMBOL(nand_ecc_put_on_host_hw_engine);
> +
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
>  MODULE_DESCRIPTION("Generic ECC engine");
> diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> index ce0f8b491e5d..e0c971d6a443 100644
> --- a/drivers/mtd/nand/mtk_ecc.c
> +++ b/drivers/mtd/nand/mtk_ecc.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/mutex.h>
>  
> +#include <linux/mtd/nand.h>
>  #include <linux/mtd/mtk_ecc.h>
>  
>  #define ECC_IDLE_MASK		BIT(0)
> @@ -41,6 +42,9 @@
>  #define ECC_IDLE_REG(op)	((op) == ECC_ENCODE ? ECC_ENCIDLE : ECC_DECIDLE)
>  #define ECC_CTL_REG(op)		((op) == ECC_ENCODE ? ECC_ENCCON : ECC_DECCON)
>  
> +#define ECC_FDM_MAX_SIZE 8
> +#define ECC_FDM_MIN_SIZE 1
> +
>  struct mtk_ecc_caps {
>  	u32 err_mask;
>  	const u8 *ecc_strength;
> @@ -79,6 +83,10 @@ static const u8 ecc_strength_mt7622[] = {
>  	4, 6, 8, 10, 12, 14, 16
>  };
>  
> +static const u8 ecc_strength_mt7986[] = {
> +	4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24
> +};
> +
>  enum mtk_ecc_regs {
>  	ECC_ENCPAR00,
>  	ECC_ENCIRQ_EN,
> @@ -115,6 +123,15 @@ static int mt7622_ecc_regs[] = {
>  	[ECC_DECIRQ_STA] =      0x144,
>  };
>  
> +static int mt7986_ecc_regs[] = {
> +	[ECC_ENCPAR00] =        0x300,
> +	[ECC_ENCIRQ_EN] =       0x80,
> +	[ECC_ENCIRQ_STA] =      0x84,
> +	[ECC_DECDONE] =         0x124,
> +	[ECC_DECIRQ_EN] =       0x200,
> +	[ECC_DECIRQ_STA] =      0x204,
> +};
> +
>  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
>  				     enum mtk_ecc_operation op)
>  {
> @@ -447,6 +464,464 @@ unsigned int mtk_ecc_get_parity_bits(struct mtk_ecc *ecc)
>  }
>  EXPORT_SYMBOL(mtk_ecc_get_parity_bits);
>  
> +static inline int data_off(struct nand_device *nand, int i)

Please always prefix all your helpers with mtk_ecc_

> +{
> +	int eccsize = nand->ecc.ctx.conf.step_size;
> +
> +	return i * eccsize;
> +}
> +
> +static inline int fdm_off(struct nand_device *nand, int i)

What is fdm?

> +{
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +	int poi;
> +
> +	if (i < eng->bad_mark.sec)

sec does not mean anything to me

> +		poi = (i + 1) * eng->fdm_size;
> +	else if (i == eng->bad_mark.sec)
> +		poi = 0;
> +	else
> +		poi = i * eng->fdm_size;
> +
> +	return poi;
> +}
> +
> +static inline int mtk_ecc_data_len(struct nand_device *nand)
> +{
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +	int eccsize = nand->ecc.ctx.conf.step_size;
> +	int eccbytes = eng->oob_ecc;
> +
> +	return eccsize + eng->fdm_size + eccbytes;
> +}
> +
> +static inline u8 *mtk_ecc_sec_ptr(struct nand_device *nand,  int i)
> +{
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +
> +	return eng->page_buf + i * mtk_ecc_data_len(nand);
> +}
> +
> +static inline u8 *mtk_ecc_fdm_ptr(struct nand_device *nand, int i)
> +{
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +	int eccsize = nand->ecc.ctx.conf.step_size;
> +
> +	return eng->page_buf + i * mtk_ecc_data_len(nand) + eccsize;
> +}
> +
> +static void mtk_ecc_no_bad_mark_swap(struct nand_device *a, u8 *b,
> +							u8 *c)
> +{
> +	/* nop */
> +}
> +
> +static void mtk_ecc_bad_mark_swap(struct nand_device *nand, u8 *databuf,
> +							u8 *oobbuf)
> +{
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +	int step_size = nand->ecc.ctx.conf.step_size;
> +	u32 bad_pos = eng->bad_mark.pos;
> +
> +	bad_pos += eng->bad_mark.sec * step_size;
> +
> +	swap(oobbuf[0], databuf[bad_pos]);

please change "bad" or "bad mark" by "bbm" which is the name used
everywhere else in this subsystem.

> +}
> +
> +static void mtk_ecc_set_bad_mark_ctl(struct mtk_ecc_bad_mark_ctl *bm_ctl,
> +				     struct mtd_info *mtd)
> +{
> +	struct nand_device *nand = mtd_to_nanddev(mtd);
> +
> +	if (mtd->writesize == 512) {
> +		bm_ctl->bm_swap = mtk_ecc_no_bad_mark_swap;
> +	} else {
> +		bm_ctl->bm_swap = mtk_ecc_bad_mark_swap;
> +		bm_ctl->sec = mtd->writesize / mtk_ecc_data_len(nand);
> +		bm_ctl->pos = mtd->writesize % mtk_ecc_data_len(nand);

sec? pos? what does this mean?

> +	}
> +}
> +
> +static int mtk_ecc_ooblayout_free(struct mtd_info *mtd, int section,
> +				  struct mtd_oob_region *oob_region)
> +{
> +	struct nand_device *nand = mtd_to_nanddev(mtd);
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +	struct nand_ecc_props *conf = &nand->ecc.ctx.conf;
> +	u32 eccsteps, bbm_bytes = 0;
> +
> +	eccsteps = mtd->writesize / conf->step_size;
> +
> +	if (section >= eccsteps)
> +		return -ERANGE;
> +
> +	/* reserve 1 byte for bad mark only for section 0 */
> +	if (section == 0)
> +		bbm_bytes = 1;
> +
> +	oob_region->length = eng->fdm_size - bbm_bytes;
> +	oob_region->offset = section * eng->fdm_size + bbm_bytes;
> +
> +	return 0;
> +}
> +
> +static int mtk_ecc_ooblayout_ecc(struct mtd_info *mtd, int section,
> +				 struct mtd_oob_region *oob_region)
> +{
> +	struct nand_device *nand = mtd_to_nanddev(mtd);
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +
> +	if (section)
> +		return -ERANGE;
> +
> +	oob_region->offset = eng->fdm_size * eng->nsteps;
> +	oob_region->length = mtd->oobsize - oob_region->offset;
> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops mtk_ecc_ooblayout_ops = {
> +	.free = mtk_ecc_ooblayout_free,
> +	.ecc = mtk_ecc_ooblayout_ecc,
> +};
> +
> +const struct mtd_ooblayout_ops *mtk_ecc_get_ooblayout(void)
> +{
> +	return &mtk_ecc_ooblayout_ops;
> +}
> +
> +static struct device *mtk_ecc_get_engine_dev(struct device *dev)
> +{
> +	struct platform_device *eccpdev;
> +	struct device_node *np;
> +
> +	/*
> +	 * The device node is only the host controller,
> +	 * not the actual ECC engine when pipelined case.
> +	 */
> +	np = of_parse_phandle(dev->of_node, "nand-ecc-engine", 0);
> +	if (!np)
> +		return NULL;
> +
> +	eccpdev = of_find_device_by_node(np);
> +	if (!eccpdev) {
> +		of_node_put(np);
> +		return NULL;
> +	}
> +
> +	platform_device_put(eccpdev);
> +	of_node_put(np);
> +
> +	return &eccpdev->dev;
> +}
> +
> +/*
> + * mtk_ecc_data_format() - Covert data between ecc format and data/oob buf

Convert

> + *
> + * Mediatek HW ECC engine organize data/oob free/oob ecc by sector,
> + * the data format for one page as bellow:
> + * ||          sector 0         ||          sector 1         || ...
> + * || data |   fdm    | oob ecc || data ||   fdm   | oob ecc || ...
> + *
> + * Terefore, it`s necessary to covert data when read/write in MTD_OPS_RAW.

it is ... convert ... reading/writing in raw mode.

> + * These data include bad mark, sector data, fdm data and oob ecc.
> + */
> +static void mtk_ecc_data_format(struct nand_device *nand,
> +			u8 *databuf, u8 *oobbuf, bool write)
> +{
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +	int step_size = nand->ecc.ctx.conf.step_size;
> +	int i;
> +
> +	if (write) {
> +		for (i = 0; i < eng->nsteps; i++) {
> +			if (i == eng->bad_mark.sec)
> +				eng->bad_mark.bm_swap(nand,
> +						databuf, oobbuf);
> +			memcpy(mtk_ecc_sec_ptr(nand, i),
> +				   databuf + data_off(nand, i), step_size);
> +
> +			memcpy(mtk_ecc_fdm_ptr(nand, i),
> +				   oobbuf + fdm_off(nand, i),
> +				   eng->fdm_size);
> +
> +			memcpy(mtk_ecc_fdm_ptr(nand, i) + eng->fdm_size,
> +				   oobbuf + eng->fdm_size * eng->nsteps +
> +				   i * eng->oob_ecc,
> +				   eng->oob_ecc);
> +
> +			/* swap back when write */
> +			if (i == eng->bad_mark.sec)
> +				eng->bad_mark.bm_swap(nand,
> +						databuf, oobbuf);
> +		}
> +	} else {
> +		for (i = 0; i < eng->nsteps; i++) {
> +			memcpy(databuf + data_off(nand, i),
> +				   mtk_ecc_sec_ptr(nand, i), step_size);
> +
> +			memcpy(oobbuf + fdm_off(nand, i),
> +				   mtk_ecc_sec_ptr(nand, i) + step_size,
> +				   eng->fdm_size);
> +
> +			memcpy(oobbuf + eng->fdm_size * eng->nsteps +
> +				   i * eng->oob_ecc,
> +				   mtk_ecc_sec_ptr(nand, i) + step_size
> +				   + eng->fdm_size,
> +				   eng->oob_ecc);
> +
> +			if (i == eng->bad_mark.sec)
> +				eng->bad_mark.bm_swap(nand,
> +						databuf, oobbuf);
> +		}
> +	}
> +}
> +
> +static void mtk_ecc_fdm_shift(struct nand_device *nand,
> +				u8 *dst_buf, u8 *src_buf)
> +{
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +	u8 *poi;
> +	int i;
> +
> +	for (i = 0; i < eng->nsteps; i++) {
> +		if (i < eng->bad_mark.sec)
> +			poi = src_buf + (i + 1) * eng->fdm_size;
> +		else if (i == eng->bad_mark.sec)
> +			poi = src_buf;
> +		else
> +			poi = src_buf + i * eng->fdm_size;
> +
> +		memcpy(dst_buf + i * eng->fdm_size, poi, eng->fdm_size);
> +	}
> +}
> +
> +int mtk_ecc_prepare_io_req_pipelined(struct nand_device *nand,
> +					  struct nand_page_io_req *req)
> +{
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +	struct mtd_info *mtd = nanddev_to_mtd(nand);
> +	u8 *buf = eng->page_buf;
> +
> +	nand_ecc_tweak_req(&eng->req_ctx, req);
> +
> +	if (req->mode == MTD_OPS_RAW) {
> +		if (req->type == NAND_PAGE_WRITE) {
> +			/* change data/oob buf to MTK HW ECC data format */
> +			mtk_ecc_data_format(nand, req->databuf.in,
> +					req->oobbuf.in, true);
> +			req->databuf.out = buf;
> +			req->oobbuf.out = buf + nand->memorg.pagesize;
> +		}
> +	} else {
> +		eng->ecc_cfg.op = ECC_DECODE;
> +		if (req->type == NAND_PAGE_WRITE) {
> +			memset(eng->oob_buf, 0xff, nand->memorg.oobsize);
> +			mtd_ooblayout_set_databytes(mtd, req->oobbuf.out,
> +							eng->oob_buf,
> +							req->ooboffs,
> +							mtd->oobavail);
> +			eng->bad_mark.bm_swap(nand,
> +						req->databuf.in, eng->oob_buf);
> +			mtk_ecc_fdm_shift(nand, req->oobbuf.in,
> +						eng->oob_buf);
> +
> +			eng->ecc_cfg.op = ECC_ENCODE;
> +		}
> +
> +		eng->ecc_cfg.mode = ECC_NFI_MODE;
> +		eng->ecc_cfg.sectors = eng->nsteps;
> +		return mtk_ecc_enable(eng->ecc, &eng->ecc_cfg);
> +	}
> +
> +	return 0;
> +}
> +
> +int mtk_ecc_finish_io_req_pipelined(struct nand_device *nand,
> +					  struct nand_page_io_req *req)
> +{
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +	struct mtd_info *mtd = nanddev_to_mtd(nand);
> +	struct mtk_ecc_stats stats;
> +	u8 *buf = eng->page_buf;
> +	int ret;
> +
> +	if (req->type == NAND_PAGE_WRITE) {
> +		if (req->mode != MTD_OPS_RAW) {
> +			mtk_ecc_disable(eng->ecc);
> +			mtk_ecc_fdm_shift(nand, eng->oob_buf,
> +					req->oobbuf.in);
> +			eng->bad_mark.bm_swap(nand,
> +					req->databuf.in, eng->oob_buf);
> +		}
> +		nand_ecc_restore_req(&eng->req_ctx, req);
> +
> +		return 0;
> +	}
> +
> +	if (req->mode == MTD_OPS_RAW) {
> +		memcpy(buf, req->databuf.in,
> +			   nand->memorg.pagesize);
> +		memcpy(buf + nand->memorg.pagesize,
> +			   req->oobbuf.in, nand->memorg.oobsize);
> +
> +		/* change MTK HW ECC data format to data/oob buf */
> +		mtk_ecc_data_format(nand, req->databuf.in,
> +				req->oobbuf.in, false);
> +		nand_ecc_restore_req(&eng->req_ctx, req);
> +
> +		return 0;
> +	}
> +
> +	ret = mtk_ecc_wait_done(eng->ecc, ECC_DECODE);
> +	if (ret)
> +		return -ETIMEDOUT;

 You should go to an error path and disable the engine.

> +
> +	if (eng->read_empty) {
> +		memset(req->databuf.in, 0xff, nand->memorg.pagesize);
> +		memset(req->oobbuf.in, 0xff, nand->memorg.oobsize);
> +
> +		ret = 0;
> +	} else {
> +		mtk_ecc_get_stats(eng->ecc, &stats, eng->nsteps);
> +		mtd->ecc_stats.corrected += stats.corrected;
> +		mtd->ecc_stats.failed += stats.failed;
> +
> +		/*
> +		 * Return -EBADMSG when exit uncorrect ecc error.
> +		 * Otherwise, return the bitflips.
> +		 */
> +		if (stats.failed)
> +			ret = -EBADMSG;
> +		else
> +			ret = stats.bitflips;
> +
> +		mtk_ecc_fdm_shift(nand, eng->oob_buf, req->oobbuf.in);
> +		eng->bad_mark.bm_swap(nand,
> +					req->databuf.in, eng->oob_buf);
> +		mtd_ooblayout_get_databytes(mtd, req->oobbuf.in,
> +			eng->oob_buf,
> +			0, mtd->oobavail);

Alignment looks wrong (please check the entire driver).

> +	}
> +
> +	mtk_ecc_disable(eng->ecc);
> +	nand_ecc_restore_req(&eng->req_ctx, req);
> +
> +	return ret;
> +}
> +
> +int mtk_ecc_init_ctx_pipelined(struct nand_device *nand)
> +{
> +	struct nand_ecc_props *conf = &nand->ecc.ctx.conf;
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +	struct mtd_info *mtd = nanddev_to_mtd(nand);
> +	struct device *dev;
> +	int free, ret;
> +
> +	/*
> +	 * In the case of a pipelined engine, the device registering the ECC
> +	 * engine is not the actual ECC engine device but the host controller.
> +	 */
> +	dev = mtk_ecc_get_engine_dev(nand->ecc.engine->dev);
> +	if (!dev) {
> +		ret = -EINVAL;
> +		goto free_engine;
> +	}
> +
> +	eng->ecc = dev_get_drvdata(dev);
> +
> +	/* calculate oob free bytes except ecc parity data */

Please use upper case for acronyms: OOB, ECC, NAND, MTD, etc

> +	free = (conf->strength * mtk_ecc_get_parity_bits(eng->ecc)
> +		+ 7) >> 3;
> +	free = eng->oob_per_sector - free;
> +
> +	/*
> +	 * enhance ecc strength if oob left is bigger than max FDM size
> +	 * or reduce ecc strength if oob size is not enough for ecc
> +	 * parity data.
> +	 */
> +	if (free > ECC_FDM_MAX_SIZE)
> +		eng->oob_ecc = eng->oob_per_sector - ECC_FDM_MAX_SIZE;
> +	else if (free < 0)
> +		eng->oob_ecc = eng->oob_per_sector - ECC_FDM_MIN_SIZE;
> +
> +	/* calculate and adjust ecc strenth based on oob ecc bytes */
> +	conf->strength = (eng->oob_ecc << 3) /
> +				 mtk_ecc_get_parity_bits(eng->ecc);
> +	mtk_ecc_adjust_strength(eng->ecc, &conf->strength);
> +
> +	eng->oob_ecc = DIV_ROUND_UP(conf->strength *
> +				 mtk_ecc_get_parity_bits(eng->ecc), 8);
> +
> +	eng->fdm_size = eng->oob_per_sector - eng->oob_ecc;
> +	if (eng->fdm_size > ECC_FDM_MAX_SIZE)
> +		eng->fdm_size = ECC_FDM_MAX_SIZE;
> +
> +	eng->fdm_ecc_size = ECC_FDM_MIN_SIZE;
> +
> +	eng->oob_ecc = eng->oob_per_sector - eng->fdm_size;
> +
> +	if (!mtd->ooblayout)
> +		mtd_set_ooblayout(mtd, mtk_ecc_get_ooblayout());
> +
> +	ret = nand_ecc_init_req_tweaking(&eng->req_ctx, nand);
> +	if (ret)
> +		goto free_engine;
> +
> +	eng->oob_buf = kzalloc(mtd->oobsize, GFP_KERNEL);
> +	eng->page_buf =
> +		kzalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
> +	if (!eng->oob_buf || !eng->page_buf) {
> +		ret = -ENOMEM;
> +		goto free_bufs;
> +	}
> +
> +	mtk_ecc_set_bad_mark_ctl(&eng->bad_mark, mtd);
> +	eng->ecc_cfg.strength = conf->strength;
> +	eng->ecc_cfg.len = conf->step_size + eng->fdm_ecc_size;
> +
> +	return 0;
> +
> +free_bufs:
> +	nand_ecc_cleanup_req_tweaking(&eng->req_ctx);
> +	kfree(eng->oob_buf);
> +	kfree(eng->page_buf);
> +free_engine:
> +	kfree(eng);
> +
> +	return ret;
> +}
> +
> +void mtk_ecc_cleanup_ctx_pipelined(struct nand_device *nand)
> +{
> +	struct mtk_ecc_engine *eng = nand->ecc.ctx.priv;
> +
> +	if (eng) {
> +		nand_ecc_cleanup_req_tweaking(&eng->req_ctx);
> +		kfree(eng->ecc);
> +		kfree(eng->oob_buf);
> +		kfree(eng->page_buf);
> +		kfree(eng);
> +	}
> +}
> +
> +/*
> + * The on-host mtk HW ECC engine work at pipelined situation,
> + * will be registered by the drivers that wrap it.
> + */
> +static struct nand_ecc_engine_ops mtk_ecc_engine_pipelined_ops = {
> +	.init_ctx = mtk_ecc_init_ctx_pipelined,
> +	.cleanup_ctx = mtk_ecc_cleanup_ctx_pipelined,
> +	.prepare_io_req = mtk_ecc_prepare_io_req_pipelined,
> +	.finish_io_req = mtk_ecc_finish_io_req_pipelined,
> +};
> +
> +struct nand_ecc_engine_ops *mtk_ecc_get_pipelined_ops(void)
> +{
> +	return &mtk_ecc_engine_pipelined_ops;
> +}
> +EXPORT_SYMBOL(mtk_ecc_get_pipelined_ops);
> +
>  static const struct mtk_ecc_caps mtk_ecc_caps_mt2701 = {
>  	.err_mask = 0x3f,
>  	.ecc_strength = ecc_strength_mt2701,
> @@ -477,6 +952,16 @@ static const struct mtk_ecc_caps mtk_ecc_caps_mt7622 = {
>  	.pg_irq_sel = 0,
>  };
>  
> +static const struct mtk_ecc_caps mtk_ecc_caps_mt7986 = {
> +	.err_mask = 0x1f,
> +	.ecc_strength = ecc_strength_mt7986,
> +	.ecc_regs = mt7986_ecc_regs,
> +	.num_ecc_strength = 11,
> +	.ecc_mode_shift = 5,
> +	.parity_bits = 14,
> +	.pg_irq_sel = 1,
> +};
> +
>  static const struct of_device_id mtk_ecc_dt_match[] = {
>  	{
>  		.compatible = "mediatek,mt2701-ecc",
> @@ -487,6 +972,9 @@ static const struct of_device_id mtk_ecc_dt_match[] = {
>  	}, {
>  		.compatible = "mediatek,mt7622-ecc",
>  		.data = &mtk_ecc_caps_mt7622,
> +	}, {
> +		.compatible = "mediatek,mt7986-ecc",
> +		.data = &mtk_ecc_caps_mt7986,

Here you do something unrelated, please split.

>  	},
>  	{},
>  };
> diff --git a/include/linux/mtd/mtk_ecc.h b/include/linux/mtd/mtk_ecc.h
> index 0e48c36e6ca0..a286183d16c4 100644
> --- a/include/linux/mtd/mtk_ecc.h
> +++ b/include/linux/mtd/mtk_ecc.h
> @@ -33,6 +33,31 @@ struct mtk_ecc_config {
>  	u32 len;
>  };
>  
> +struct mtk_ecc_bad_mark_ctl {
> +	void (*bm_swap)(struct nand_device *, u8 *databuf, u8* oobbuf);
> +	u32 sec;
> +	u32 pos;
> +};
> +
> +struct mtk_ecc_engine {
> +	struct nand_ecc_req_tweak_ctx req_ctx;
> +
> +	u32 oob_per_sector;
> +	u32 oob_ecc;
> +	u32 fdm_size;
> +	u32 fdm_ecc_size;
> +
> +	bool read_empty;
> +	u32 nsteps;
> +
> +	u8 *page_buf;
> +	u8 *oob_buf;
> +
> +	struct mtk_ecc *ecc;
> +	struct mtk_ecc_config ecc_cfg;
> +	struct mtk_ecc_bad_mark_ctl bad_mark;
> +};
> +
>  int mtk_ecc_encode(struct mtk_ecc *, struct mtk_ecc_config *, u8 *, u32);
>  void mtk_ecc_get_stats(struct mtk_ecc *, struct mtk_ecc_stats *, int);
>  int mtk_ecc_wait_done(struct mtk_ecc *, enum mtk_ecc_operation);
> @@ -44,4 +69,17 @@ unsigned int mtk_ecc_get_parity_bits(struct mtk_ecc *ecc);
>  struct mtk_ecc *of_mtk_ecc_get(struct device_node *);
>  void mtk_ecc_release(struct mtk_ecc *);
>  
> +#if IS_ENABLED(CONFIG_MTD_NAND_ECC_MTK)
> +
> +struct nand_ecc_engine_ops *mtk_ecc_get_pipelined_ops(void);
> +
> +#else /* !CONFIG_MTD_NAND_ECC_MTK */
> +
> +struct nand_ecc_engine_ops *mtk_ecc_get_pipelined_ops(void)
> +{
> +	return NULL;
> +}
> +
> +#endif /* CONFIG_MTD_NAND_ECC_MTK */
> +
>  #endif
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 32fc7edf65b3..5ffd3e359515 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -265,10 +265,16 @@ struct nand_ecc_engine_ops {
>  
>  /**
>   * struct nand_ecc_engine - ECC engine abstraction for NAND devices
> + * @dev: Host device
> + * @node: Private field for registration time
>   * @ops: ECC engine operations
> + * @priv: Private data
>   */
>  struct nand_ecc_engine {
> +	struct device *dev;
> +	struct list_head node;
>  	struct nand_ecc_engine_ops *ops;
> +	void *priv;
>  };
>  
>  void of_get_nand_ecc_user_config(struct nand_device *nand);
> @@ -279,8 +285,13 @@ int nand_ecc_prepare_io_req(struct nand_device *nand,
>  int nand_ecc_finish_io_req(struct nand_device *nand,
>  			   struct nand_page_io_req *req);
>  bool nand_ecc_is_strong_enough(struct nand_device *nand);
> +int nand_ecc_register_on_host_hw_engine(struct nand_ecc_engine *engine);
> +int nand_ecc_unregister_on_host_hw_engine(struct nand_ecc_engine *engine);
>  struct nand_ecc_engine *nand_ecc_get_sw_engine(struct nand_device *nand);
>  struct nand_ecc_engine *nand_ecc_get_on_die_hw_engine(struct nand_device *nand);
> +struct nand_ecc_engine *nand_ecc_get_on_host_hw_engine(struct nand_device *nand);
> +struct nand_ecc_engine *nand_ecc_match_on_host_hw_engine(struct device *dev);
> +void nand_ecc_put_on_host_hw_engine(struct nand_device *nand);
>  
>  #if IS_ENABLED(CONFIG_MTD_NAND_ECC_SW_HAMMING)
>  struct nand_ecc_engine *nand_ecc_sw_hamming_get_engine(void);


Thanks,
Miquèl

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek