[PATCH] [OneNAND] OTP support re-implementation 1/1

Amul Kumar Saha amul.saha at samsung.com
Thu Sep 3 01:51:58 EDT 2009


Hi Artem,

>> +if MTD_ONENAND_OTP
>> +
>> +config ONENAND_OTP_AREA
>> + bool "OTP area ONLY"
>> + depends on MTD_ONENAND_OTP
>> + select ON_OTP_AREA
>> +
>> +config ONENAND_OTP_BLOCK0
>> + bool "Block[0] ONLY"
>> + depends on MTD_ONENAND_OTP&&  !ONENAND_OTP_AREA
>> + select ON_OTP_BLOCK0
>> +
>> +config ONENAND_OTP_AREA_BLOCK0
>> + bool "BOTH OTP area AND Block[0]"
>> + depends on MTD_ONENAND_OTP&&  !ONENAND_OTP_AREA&&  !ONENAND_OTP_BLOCK0
>> + select ON_OTP_AREA_BLOCK0
>> +
>> +endif #MTD_ONENAND_OTP
>
> If there were 10 OTP blocks, would you add 10 options?
> I mean, are these switches really needed? Can we remove them?

There is just one OTP block.
Three options are provided for the three known combinations of 1st Block and the OTP Block.
The option to choose one should be provided to the user.


>> @@ -13,6 +13,10 @@
>>    * Flex-OneNAND support
>>    * Copyright (C) Samsung Electronics, 2008
>>    *
>> + * Amul Kumar Saha<amul.saha at samsung.com>
>> + * OTP support
>> + * Copyright (C) SAMSUNG Electronics, 2009
>
> If this all is (C) Samsung already, do you really need to add one more
> (C) line?

Accepted

>> + struct onenand_chip *this = mtd->priv;
>> + int value, block, page;
>> +
>> + /* Address translation */
>> + switch (cmd) {
>> + case ONENAND_CMD_OTP_ACCESS:
>> + block = (int) (addr>>  this->erase_shift);
>
> Why do you need the (int) cast there? How about cleaning up
>>> (missing space) ?

I can do that, but (int) cast has been followed throughout onenand_base.c file
Just adhering to that.

>> + page = -1;
>> + break;
>> +
>> + default:
>> + block = (int) (addr>>  this->erase_shift);
>> + page = (int) (addr>>  this->page_shift);
>
> Ditto. And there are many places where you did not have put spaces properly.

I can't see any in the patch I sent.

>>   written -= prevlen;
>> - printk(KERN_ERR "onenand_write_ops_nolock: write filaed %d\n", ret);
>> + printk(KERN_ERR "onenand_write_ops_nolock: \
>> + write failed %d\n", ret);
>>   break;
>
> Please, fix printk's. Do not use "\".
>

Accepted

With Regards,
Amul Kumar Saha 





More information about the linux-mtd mailing list