On Tuesday 19 August 2014 07:42 AM, Brian Norris wrote:
On Wed, Aug 06, 2014 at 02:32:18AM +0530, pekon@xxxxxxxxxxx wrote:
On Tuesday 05 August 2014 07:53 PM, Lee Jones wrote:
On Thu, 03 Jul 2014, Gupta, Pekon wrote:
+ /* Load last page of block */
+ offs = (loff_t)block << chip->phys_erase_shift;
+ offs += mtd->erasesize - mtd->writesize;
+ if (bch_read_page(nandi, offs, buf) < 0) {
*Important*: You shouldn't call internal functions directly here..
Instead use chip->_read() OR mtd-read() that will:
I'm not sure exactly what you're saying in the above line (chip->_read()
is not a function, and and mtd-read() is syntactically incorrect),
but...
You most certainly do *not* want to call mtd->_read() directly (or any
of the callbacks prefixed with underscores). That is one of the main
purposes of:
commit 3c3c10bba1e4ccb75b41442e45c1a072f6cded19
Author: Artem Bityutskiy <artem.bityutskiy@xxxxxxxxxxxxxxx>
Date: Mon Jan 30 14:58:32 2012 +0200
mtd: add leading underscore to all mtd functions
Makes sense. Thanks for pointing this.
[...]
I think somewhere in earlier comments, Brian also supported
to use high-level function like mtd_read(). Also, nand_bbt.c
itself uses 'mtd_read(), mtd_read_oob() and mtd_write_oob()
at many places. So I'll let Brain decide here.
But having low-level function will add redundant code for
re-checking and aligning the addresses boundaries to block
and page/sector sizes.
Not all checks are redundant. It's redundant to have the direct
descendant doing the same checks that the parent does, like:
mtd_read() (checks boundaries)
|_ mtd->_read() (e.g., nand_read())
So nand_read() and nand_do_read_ops() don't need the checking.
But for a BBT implementation, it can help to make sure that its
page/block/etc. arithmetic fits the right bounds when it ends up
deciding to scan a block.
Now, it still may be easy to prove that the checks are
redundant/unnecessary for correctly-written code, but if the layering
makes sense, then it still may be a better choice to simply use the
high-level, self-checking API than to try to dig deeper. For instance,
I'm pretty sure UBI does some checks to make sure it's not reading off
the end of an MTD, but it still calls mtd_read() because it's The Right
Thing (TM).
Brian
Agree.. re-using mtd_*() API for non-critical paths is justified.
with regards, pekon
------------------------
Powered by BigRock.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
