[RFC][PATCH] Add NAND lock/unlock routines

[Vimal Singh]

On Mon, Dec 7, 2009 at 4:58 PM, Artem Bityutskiy <dedekind1 at gmail.com> wrote:
> On Mon, 2009-12-07 at 16:36 +0530, Vimal Singh wrote:
>> On Mon, Dec 7, 2009 at 2:59 PM, Artem Bityutskiy <dedekind1 at gmail.com> wrote:
>> > On Mon, 2009-12-07 at 12:18 +0530, Vimal Singh wrote:
>> >> On Fri, Dec 4, 2009 at 2:08 PM, Artem Bityutskiy <dedekind1 at gmail.com> wrote:
>> >> > Hi, some cosmetic comments:
>> >> >
>> >> > On Wed, 2009-12-02 at 19:54 +0530, Vimal Singh wrote:
>> >> >> I am not sure how useful it will be, but still here is a patch for review.
>> >> >> -vimal
>> >> >>
>> >> >> From: Vimal Singh <vimalsingh at ti.com>
>> >> >> Date: Tue, 24 Nov 2009 18:26:43 +0530
>> >> >> Subject: [PATCH] Add NAND lock/unlock routines
>> >> >>
>> >> >> At least 'Micron' NAND parts have lock/unlock feature.
>> >> >> Adding routines for this.
>> >> >>
>> >> >> Signed-off-by: Vimal Singh <vimalsingh at ti.com>
>> >> >> ---
>> >> >>  drivers/mtd/nand/nand_base.c |  217 +++++++++++++++++++++++++++++++++++++++++-
>> >> >>  include/linux/mtd/nand.h     |    6 +
>> >> >>  2 files changed, 221 insertions(+), 2 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> >> >> index 2957cc7..e447c24 100644
>> >> >> --- a/drivers/mtd/nand/nand_base.c
>> >> >> +++ b/drivers/mtd/nand/nand_base.c
>> >> >> @@ -757,6 +757,218 @@ static int nand_wait(struct mtd_info *mtd,
>> >> >> struct nand_chip *chip)
>> >> >>  }
>> >> >>
>> >> >>  /**
>> >> >> + * __nand_unlock - [REPLACABLE] unlocks specified locked blockes
>> >> >> + *
>> >> >> + * @param mtd - mtd info
>> >> >> + * @param ofs - offset to start unlock from
>> >> >> + * @param len - length to unlock
>> >> >> + * @invert -  when = 0, unlock the range of blocks within the lower and
>> >> >> + *                      upper boundary address
>> >> >> + *            whne = 1, unlock the range of blocks outside the boundaries
>> >> >> + *                      of the lower and upper boundary address
>> >> >> + *
>> >> >> + * @return - unlock status
>> >> >> + */
>> >> >> +static int __nand_unlock(struct mtd_info *mtd, loff_t ofs,
>> >> >> +                                     uint64_t len, int invert)
>> >> >> +{
>> >> >> +     int ret = 0;
>> >> >> +     int status, page;
>> >> >> +     struct nand_chip *chip = mtd->priv;
>> >> >> +
>> >> >> +     DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
>> >> >> +                     __func__, (unsigned long long)ofs, len);
>> >> >> +
>> >> >> +     /* Submit address of first page to unlock */
>> >> >> +     page = (int)(ofs >> chip->page_shift);
>> >> >
>> >> > The compiler will automatically cast the result to int I believe.
>> >>
>> >> I just copied this line from erase functions.
>> >> I believe its better to cast here as otherwise we may see compiler warnings.
>> >
>> > Good point. Could you please create a validation checking helper instead
>> > of duplicating code?
>>
>> IMHO that should be done in a separate patch.
>
> Right, you can first send this as a separate patch, and then the rest as
> a follow up one.

when I went back on validation checking's I notice:

-nand_read: does validation for access to past end of the device
-nand_do_read_oob: does it for access to past oob and device.
-nand_read_oob: does for access to past end of the device

-nand_write: does it for access to past end of the device
-nand_do_write_ops: does it for page alignment
-nand_do_write_oob: does it for access to past oob and device and page
alignment
-panic_nand_write: does it for access to past end of the device

-nand_erase_nand: does it for access to past end of the device and
block alignment 'lock/unlock' routines are doing same validations as
'nand_erase_nand' does.

There is no consistancy in validation checks other than between
'erase' and 'lock/unlock'.
Now since currently only 'erase' function does those validations. We
can have patch for separate validation functions only after
'lock/unlock' patch.

Any comment or suggestions?

-- 
Regards,
Vimal Singh