On Thu, Dec 11, 2008 at 07:57:18PM +0530, Rohit Hagargundgi wrote:
This patch adds support for MLC OneNAND and Flex-OneNAND devices.
Patch does not apply to u-boot-nand-flash/next (the master branch is for 2008.12 bugfixes only).
It's looking a lot better (though I'm not familiar enough with this hardware to comment on the functional bits), though while you're rebasing, I have some style nits:
- unsigned boundary, blk, die = 0;
"unsigned int"
+inline int flexonenand_region(struct mtd_info *mtd, loff_t addr)
Compiler should be able to figure out inlines by itself.
+{
- int i;
- for (i = 0; i < mtd->numeraseregions &&
addr >= mtd->eraseregions[i].offset; i++);
Align the continuation line with "i = 0", making it distinct from the if-body.
Move the addr >= mtd->eraseregions[i].offset test into the if-body, for readability.
- i--;
- return i;
+}
Just return i - 1.
ret = ret ? onenand_recover_lsb(mtd, from, ret) : ret;
if (ret) ret = onenand_recover_lsb(mtd, from, ret);
The ?: usually decreases readability rather than increases it; it's best to avoid unless using it would eliminate duplication or a temporary variable.
/* Block lock scheme */
- for (block = start; block < end; block++) {
- for (block = start; block < end + 1; block++) {
block <= end
locked = bdry >> FLEXONENAND_PI_UNLOCK_SHIFT;locked = (locked == 0x3) ? 0 : 1;
if ((bdry >> FLEXONENAND_PI_UNLOCK_SHIFT) == 3) locked = 0; else locked = 1;
- eraseshift = this->erase_shift - 1;
- mtd->numeraseregions = this->dies << 1;
Only one blank line.
- mtd->erasesize = 1 << (this->erase_shift);
Unnecessary parens.
-Scott