Re: [PATCH v2 15/20] mtd: spinand: Create direct mapping descriptors for ECC operations

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

Hi Boris,

boris.brezillon@collabora.com wrote on Fri, 26 Nov 2021 15:47:06 +0100:

> On Fri, 26 Nov 2021 15:42:23 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > > > @@ -866,6 +872,24 @@ static int spinand_create_dirmap(struct spinand_device *spinand,
> > > >  
> > > >  	spinand->dirmaps[plane].rdesc = desc;
> > > >  
> > > > +	info.op_tmpl = *spinand->op_templates.update_cache;
> > > > +	info.op_tmpl.ecc_en = true;
> > > > +	desc = devm_spi_mem_dirmap_create(&spinand->spimem->spi->dev,
> > > > +					  spinand->spimem, &info);
> > > > +	if (IS_ERR(desc))
> > > > +		return PTR_ERR(desc);
> > > > +
> > > > +	spinand->dirmaps[plane].wdesc_ecc = desc;
> > > > +
> > > > +	info.op_tmpl = *spinand->op_templates.read_cache;
> > > > +	info.op_tmpl.ecc_en = true;
> > > > +	desc = devm_spi_mem_dirmap_create(&spinand->spimem->spi->dev,
> > > > +					  spinand->spimem, &info);
> > > > +	if (IS_ERR(desc))
> > > > +		return PTR_ERR(desc);
> > > > +
> > > > +	spinand->dirmaps[plane].rdesc_ecc = desc;
> > > > +      
> > > 
> > > Direct mappings are not free (they might reserve a piece of MMIO
> > > address space depending on the spi-mem controller implementation), so
> > > I'd recommend creating those mapping only when strictly needed, that
> > > is, when dealing with a pipelined ECC.    
> > 
> > That's what I tried to do in the first place, but unfortunately the
> > direct mappings are instantiated at the time the SPI controller gets
> > probed, which happens before the NAND flash being initialized and its
> > ECC engine being picked. Hence, at this point, we don't really know what
> > engine will be used with this ECC engine. I finally gave up and ended
> > up creating 4 dirmaps (per lun). I know it's not optimal and in most
> > cases consume more address space than needed but couldn't find a
> > better solution yet :/  
> 
> Why do you have to create the ECC mappings that early? Can't you just
> create them after the ECC initialization is done? AFAICT, they won't be
> used before that anyway.

Yeah that's right, maybe we don't need them that early. I'll move
things around and try that out.

Thanks!
Miquèl