Re: [RFC,v4,4/5] mtd: spinand: Move set/get OOB databytes to each ECC engines

[xiangsheng.hou <xiangsheng.hou@mediatek.com>]

Hi Miquel,

On Tue, 2021-12-14 at 12:41 +0100, Miquel Raynal wrote:
> 
> > 
> > @@ -299,22 +303,42 @@ static int
> > nand_ecc_sw_bch_prepare_io_req(struct nand_device *nand,
> >  	int total = nand->ecc.ctx.total;
> >  	u8 *ecccalc = engine_conf->calc_buf;
> >  	const u8 *data;
> > -	int i;
> > +	int i, ret = 0;
> 
> int i, ret;
> 
> >  
> >  	/* Nothing to do for a raw operation */
> >  	if (req->mode == MTD_OPS_RAW)
> >  		return 0;
> >  
> > -	/* This engine does not provide BBM/free OOB bytes protection
> > */
> > -	if (!req->datalen)
> > -		return 0;
> > -
> 
> Why? Please drop this removal here and below, it has nothing to do
> with
> the current fix, right?
> 

For req->datalen == 0 and req->ooblen !=0 when write operation in
OPS_AUTO mode, it`s also need to set OOB databytes. So I remove this to
the back.

However, for OPS_RAW and OPS_PLACE_OOB mode, it`s unnecessary.

I will try to fix this.

> >  	nand_ecc_tweak_req(&engine_conf->req_ctx, req);
> >  
> >  	/* No more preparation for page read */
> >  	if (req->type == NAND_PAGE_READ)
> >  		return 0;
> >  
> > +	if (req->ooblen) {
> > +		memset(engine_conf->oob_buf, 0xff,
> > +		       nanddev_per_page_oobsize(nand));
> 
> I think this is only needed in the AUTO case.
> 
> > +
> > +		if (req->mode == MTD_OPS_AUTO_OOB) {
> > +			ret = mtd_ooblayout_set_databytes(mtd, req-
> > >oobbuf.out,
> > +							  engine_conf-
> > >oob_buf,
> > +							  req->ooboffs,
> > +							  mtd-
> > >oobavail);
> > +			if (ret)
> > +				return ret;
> > +		} else {
> > +			memcpy(engine_conf->oob_buf + req->ooboffs,
> > +			       req->oobbuf.out, req->ooblen);
> > +		}
> > +
> > +		engine_conf->src_oob_buf = (void *)req->oobbuf.out;
> > +		req->oobbuf.out = engine_conf->oob_buf;
> > +	}
> > +
> > +	/* This engine does not provide BBM/free OOB bytes protection
> > */
> > +	if (!req->datalen)
> > +		return 0;
> > +
> 
> I believe we can do something simpler:
> 
> if (req->ooblen && req->mode == AUTO) {
> 	memcpy(oobbuf, req->oobbuf.out...
> 	mtd_ooblayout_set_databytes(using oobbuf)
> }
> 
> But I believe this should be done before tweaking the request so that
> you can avoid messing with src_oob_buf, right?

Yes, you are right. For OPS_AUTO mode, the maximal request OOB len is
mtd->oobavail which smaller than nand->memorg.oobsize.

Therefore, this will use bounce oob buffer when tweaking the request.

> 
> Same applies to the two other engines.

I will.

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