[U-Boot] [PATCH] NAND: Allow per-buffer allocation
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@gmail.com --- drivers/mtd/nand/nand_base.c | 27 +++++++++++++++++++++------ include/linux/mtd/nand.h | 7 ++++--- 2 files changed, 25 insertions(+), 9 deletions(-)
NOTE: This less intrusive approach should avoid breaking older drivers (this is the reason for malloc()ing chip->buffers).
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 1a95a91..be8469c 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -2749,12 +2749,25 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips, */ 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; + + 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;
/* Set the internal oob buffer location, just after the page data */ @@ -2996,6 +3009,8 @@ void nand_release(struct mtd_info *mtd)
/* Free bad block table memory */ kfree(chip->bbt); - if (!(chip->options & NAND_OWN_BUFFERS)) - kfree(chip->buffers); + if (!(chip->options & NAND_OWN_BUFFERS)) { + free(chip->buffers->buffer); + free(chip->buffers); + } } diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 987a2ec..c3449a9 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -370,9 +370,10 @@ struct nand_ecc_ctrl { * consecutive order. */ struct nand_buffers { - uint8_t ecccalc[NAND_MAX_OOBSIZE]; - uint8_t ecccode[NAND_MAX_OOBSIZE]; - uint8_t databuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE]; + uint8_t *buffer; + uint8_t *ecccalc; + uint8_t *ecccode; + uint8_t *databuf; };
/**
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@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."
Will a similar change be needed in Linux?
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?
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"?
-Scott
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@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
participants (2)
-
Marek Vasut -
Scott Wood