[ANNOUNCE] [PATCH] [MTD] Flex-OneNAND MTD Driver available.

[Amit Kumar Sharma]

Hi Adrian


> Rohit wrote:
>> Adrian Hunter wrote:
>>>> + this->command(mtd, FLEXONENAND_CMD_PI_ACCESS, die, 
>>>> 0);
>>>> + this->wait(mtd, FL_SYNCING);
>>> Why is the error return not checked?
>>>
>>>> +
>>>> + this->command(mtd, ONENAND_CMD_READ, die, 0);
>>>> + ret = this->wait(mtd, FL_READING);
>>> Why is the error return not checked?
>>>
>>>> +
>>>> + bdry = this->read_word(this->base + ONENAND_DATARAM);
>>>> + locked = bdry >> FLEXONENAND_PI_UNLOCK_SHIFT;
>>>> + locked = (locked == 0x3) ? 0 : 1;
>>>> + this->boundary[die] = bdry & FLEXONENAND_PI_MASK;
>>>> + this->boundary_locked[die] = locked;
>>>> + this->command(mtd, ONENAND_CMD_RESET, 0, 0);
>>>> + ret = this->wait(mtd, FL_RESETING);
>>> Why is the error return not checked?
>>
>> As per datasheet PI_ACCESS, PI_UPDATE, PI_READ and RESET 
>> always succeed.
>> PI_ERASE and PI_WRITE can fail. Added error check for 
>> PI_ERASE.
>
> So if the device is behaving correctly and the driver has 
> no bugs and there
> are no other related hardware errors, then 'ret' will 
> always be zero.  Well,
> I would check it anyway, but that's just me.
>
>>>> + if (FLEXONENAND(this)) {
>>>> + int boundary[] =
>>>> + {FLEXONENAND_DIE0_BOUNDARY, 
>>>> FLEXONENAND_DIE1_BOUNDARY};
>>>> + int lock[] =
>>>> + {FLEXONENAND_DIE0_ISLOCKED, 
>>>> FLEXONENAND_DIE1_ISLOCKED};
>>>> + unsigned die;
>>>> +
>>>> + flexonenand_get_size(mtd);
>>>> +
>>>> + /* Change the device boundaries if required */
>>>> + for (die = 0; die < this->dies; die++)
>>>> + if ((!this->boundary_locked[die]) &&
>>>> +    (boundary[die] >= 0) &&
>>>> +    (boundary[die] != this->boundary[die]))
>>>> + flexonenand_set_boundary(mtd, die,
>>>> + boundary[die], lock[die]);
>>> I am not sure how you intend FLEXONENAND_DIEx_BOUNDARY 
>>> and FLEXONENAND_DIEx_ISLOCKED
>>> to be used.  Obviously this code presently does nothing. 
>>> If you expect the values
>>> to be configured for the device, then shouldn't they be 
>>> config options?
>>
>> Ok. Added config option for boundary and lock settings.
>> Configuration of boundary / lock is optional. Default 
>> value in config is -1
>> which will not change existing boundary of Flex-OneNAND.
>>
>>> Also, ideally there should be a way for the platform 
>>> driver to override them.
>>>
>>
>> Do you mean runtime configuration of boundary. Then we 
>> need to add some ioctl.
>>
>
> No I mean a onenand driver (e.g. omap2.c) that reads the 
> values from platform data
> and wants to set them.  It is not important.  It can be 
> done later by whomever needs it.
>
>> diff --git a/include/linux/mtd/onenand.h 
>> b/include/linux/mtd/onenand.h
>> index 9aa2a91..d23eeef 100644
>> --- a/include/linux/mtd/onenand.h
>> +++ b/include/linux/mtd/onenand.h
>> @@ -17,8 +17,23 @@
>>  #include <linux/mtd/onenand_regs.h>
>>  #include <linux/mtd/bbm.h>
>>  +#define MAX_DIES 2
>>  #define MAX_BUFFERRAM 2
>>  +#define FLEXONENAND_DIE0_BOUNDARY 
>> CONFIG_MTD_FLEXONENAND_DIE0_BOUNDARY
>> +#define FLEXONENAND_DIE1_BOUNDARY 
>> CONFIG_MTD_FLEXONENAND_DIE1_BOUNDARY
>> +
>
> CONFIG_MTD_FLEXONENAND_DIEx_BOUNDARY are not necessarily 
> defined i.e. you
> need "#if defined ..." there as well.
>
> I briefly tested the driver with an old 'non-Flex' OneNAND 
> and it seemd to work
> fine with no apparent effect on performance.  So I have no 
> more issues :-)

we will share iozone result on flexOneNand but during 
testing of  FlexOneNand on kernel 2.6.22 performance results 
are far better then 2.6.27 still we donot know the reasons.
we checked performance for OneNand on same kernel veriosn 
but OneNand also has down performance with 2.6.27 kernel 
version.still a question for us?

>
 Thanks
Amit