[U-Boot] [PATCH] NAND: Allow per-buffer allocation

Marek Vasut marek.vasut at gmail.com
Wed Aug 10 01:15:19 CEST 2011


On Wednesday, August 10, 2011 12:37:29 AM Scott Wood wrote:
> On 08/09/2011 04:54 PM, Marek Vasut wrote:
> > Don't allocate NAND buffers as one block, but allocate them separately.
> > This allows systems where DMA to buffers happen to allocate these
> > buffers properly aligned.
> > 
> > Signed-off-by: Marek Vasut <marek.vasut at gmail.com>
> 
> That second sentence is hard to parse -- I think you mean something
> like, "This accommodates drivers which DMA to the buffers and have
> alignment constraints."

Yes, something like that. Sorry, it's 1.14 PM here.

> 
> Will a similar change be needed in Linux?

I'm not sure how much in sync we are with linux here. It'd be worth looking at.

> 
> >  int nand_scan_tail(struct mtd_info *mtd)
> >  {
> > 
> > -	int i;
> > +	int i, bufsize;
> > +	uint8_t *buf;
> > 
> >  	struct nand_chip *chip = mtd->priv;
> > 
> > -	if (!(chip->options & NAND_OWN_BUFFERS))
> > -		chip->buffers = kmalloc(sizeof(*chip->buffers), GFP_KERNEL);
> > -	if (!chip->buffers)
> > +	if (!(chip->options & NAND_OWN_BUFFERS)) {
> > +		chip->buffers = malloc(sizeof(struct nand_buffers));
> > +		if (!chip->buffers)
> > +			return -ENOMEM;
> 
> Why does the struct itself need to be dynamically allocated?

That was in the NOTE: ... to avoid breaking drivers. We can have that changed, 
but that'd be much more intrussive.
> 
> > +
> > +		bufsize = NAND_MAX_PAGESIZE + (3 * NAND_MAX_OOBSIZE);
> > +		buf = malloc(bufsize);
> > +
> > +		chip->buffers->buffer = (struct nand_buffers *)buf;
> > +		chip->buffers->ecccalc = buf;
> > +		chip->buffers->ecccode = buf + NAND_MAX_OOBSIZE;
> > +		chip->buffers->databuf = buf + (2 * NAND_MAX_OOBSIZE);
> > +	}
> > +
> > +	if (!chip->buffers->buffer)
> > 
> >  		return -ENOMEM;
> 
> What does "buffer" mean now?  What would a driver that supplies its own
> completely separate ecccalc/ecccode/databuf buffers put in "buffer"?

Maybe that condition should go also into the if() statement above. What do you 
think ?

> 
> -Scott


More information about the U-Boot mailing list