nand locking

nand locking

[Timofei V. Bondarenko]

Hi.

Is there a straight way to lock/unlock nand pages?

The nand_scan() does "mtd_info->lock = NULL;" unconditionally,
though the nand chip is capable of locking.

Regards.
   	Timofei.

Re: nand locking

[Thomas Gleixner]

On Wed, 2005-04-27 at 11:34 +0400, Timofei V. Bondarenko wrote:
>
> Is there a straight way to lock/unlock nand pages?
> 
> The nand_scan() does "mtd_info->lock = NULL;" unconditionally,
> though the nand chip is capable of locking.

I'm not aware of page locking features in NAND chips. The only locking
I'm aware of is the write protection pin, which protects the whole chip.

You talk about "the nand chip". Which chip is it exactly and what makes
you believe that it has a per page locking feature ?

tglx

Re: nand locking

[Timofei V. Bondarenko]

Thomas Gleixner wrote:
> On Wed, 2005-04-27 at 11:34 +0400, Timofei V. Bondarenko wrote:
> 
>>Is there a straight way to lock/unlock nand pages?
>>
> 
> You talk about "the nand chip". Which chip is it exactly and what makes
> you believe that it has a per page locking feature ?

Samsung k9f5608u0c-d

http://www.samsung.com/Products/Semiconductor/Flash/NAND/256Mbit/K9F5608U0C/K9F5608U0C.htm

http://www.samsung.com/Products/Semiconductor/Flash/NAND/256Mbit/K9F5608U0C/ds_k9f5608u0c_rev32.pdf

See the 'Block Lock Mode' chapter.

Lock block: 2Ah
Unlock Block: 23h - start block - 24h - end block
Lock tight: 2Ch
Block Lock Status Read: 7Ah - block address

I'm really using that chip and could provide some code.
The chip is locked at power on.
So i'm checking the locking status at startup and then unlocking it
(using range of pages).

It would be nice to unlock the rw partitions only.
But it's weird a bit:
the mtd_partition may be incomplete before add_mtd_partitions() done.
After add_mtd_partitions() my startap code has no handle to actual 
partition info.
The mtd_partition.mtdp seems only choice, though it prevents partitions 
from registering...
So, could couple of lines in add_mtd_partitions() do that work?

BTW. Can I post patches to this list?
I've already sent one as attacment, but that message gets bounced
"awaits moderator approval".

Regards.
	Timofei.

Re: nand locking

[Thomas Gleixner]

On Wed, 2005-04-27 at 13:09 +0400, Timofei V. Bondarenko wrote:
> Thomas Gleixner wrote:
> See the 'Block Lock Mode' chapter.
> 
> Lock block: 2Ah
> Unlock Block: 23h - start block - 24h - end block
> Lock tight: 2Ch
> Block Lock Status Read: 7Ah - block address

Ah, I see. Yes I remember that I have seen that before, but I was
puzzled by your per page locking question :)

> It would be nice to unlock the rw partitions only.
> But it's weird a bit:
> the mtd_partition may be incomplete before add_mtd_partitions() done.
> After add_mtd_partitions() my startap code has no handle to actual 
> partition info.
> The mtd_partition.mtdp seems only choice, though it prevents partitions 
> from registering...
> So, could couple of lines in add_mtd_partitions() do that work?

I'm not quite understanding, what your problem is.

Lock/unlock are general features and should be added to nand_base.c.
I guess the chips which have those functions can be identified and an
appropriate option flag set in the ID list, which then lets nand_scan
decide whether to fill in the lock/unlock function pointers or not.

> BTW. Can I post patches to this list?

Yes

> I've already sent one as attacment, but that message gets bounced
> "awaits moderator approval".

Thats list policy. Attachments have to be approved

tglx

Re: nand locking

[Timofei V. Bondarenko]

Thomas Gleixner wrote:
>>It would be nice to unlock the rw partitions only.
>>But it's weird a bit:
>>the mtd_partition may be incomplete before add_mtd_partitions() done.
>>After add_mtd_partitions() my startap code has no handle to actual 
>>partition info.
>>The mtd_partition.mtdp seems only choice, though it prevents partitions 
>>from registering...
>>So, could couple of lines in add_mtd_partitions() do that work?
> 
> 
> I'm not quite understanding, what your problem is.
> 
> Lock/unlock are general features and should be added to nand_base.c.
> I guess the chips which have those functions can be identified and an
> appropriate option flag set in the ID list, which then lets nand_scan
> decide whether to fill in the lock/unlock function pointers or not.

An easy problem: I'm looking for the simplest way to protect
read-only mtd partitions by hardware lock.
The current nand_base seems lack of this feature.
Should i hack the nand_base? Or is there a correct workaround?

>>I've already sent one as attacment, but that message gets bounced
>>"awaits moderator approval".
> 
> Thats list policy. Attachments have to be approved

It awaits since 18 Apr - longer than a week... is it ok?
Are there other ways to submit a patch? (a small bugfix to cmdlinepart).
AFAIK, placing diffs into message text is not welcomed too.

Thanks.
	Timofei.

Re: nand locking

[Artem B. Bityuckiy]

Timofei V. Bondarenko wrote:
  > It awaits since 18 Apr - longer than a week... is it ok?
> Are there other ways to submit a patch? (a small bugfix to cmdlinepart).
> AFAIK, placing diffs into message text is not welcomed too.

Insert the patch to the mail body then.

-- 
Best Regards,
Artem B. Bityuckiy,
St.-Petersburg, Russia.

Re: nand locking

[Thomas Gleixner]

On Wed, 2005-04-27 at 14:27 +0400, Timofei V. Bondarenko wrote:
> > Lock/unlock are general features and should be added to nand_base.c.
> > I guess the chips which have those functions can be identified and an
> > appropriate option flag set in the ID list, which then lets nand_scan
> > decide whether to fill in the lock/unlock function pointers or not.
> 
> An easy problem: I'm looking for the simplest way to protect
> read-only mtd partitions by hardware lock.
> The current nand_base seems lack of this feature.
> Should i hack the nand_base? Or is there a correct workaround?

As I said above, it is functionality which should be integrated cleanly
into nand_base.c. It should be automatically available for those chips
which support it.

There is no workaround. Its missing and must be added. I'm happy if you
can provide a patch.

> It awaits since 18 Apr - longer than a week... is it ok?

David Woodhouse was travelling last week

> Are there other ways to submit a patch? (a small bugfix to cmdlinepart).
> AFAIK, placing diffs into message text is not welcomed too.

It's a correct way to do it, when your mail client can handle the
whitespace (tabs) correct and does no line truncating at 80, which is
requested for the mail text itself.

tglx

Re: nand locking

[Timofei V. Bondarenko]

Thomas Gleixner wrote:
> 
> There is no workaround. Its missing and must be added. I'm happy if you
> can provide a patch.

Unfortunately, I have no sufficient expirience with nand devices to make
such patch generic enough.


	Timofei.

Re: nand locking

[Todd Poynor]

Timofei V. Bondarenko wrote:

> It would be nice to unlock the rw partitions only.
> But it's weird a bit:
> the mtd_partition may be incomplete before add_mtd_partitions() done.
> After add_mtd_partitions() my startap code has no handle to actual 
> partition info.
> The mtd_partition.mtdp seems only choice, though it prevents partitions 
> from registering...
> So, could couple of lines in add_mtd_partitions() do that work?

If I understand correctly, it may be the same issue I ran into on Intel 
NOR flash: the chip driver (which may have the knowledge of whether the 
particular chips are locked or lockable) gets to see the "whole device" 
struct mtd_info at probe time, but doesn't see the "partition" structs 
registered later via add_mtd_partitions(), which have the per-partition 
r/w flags.

I tried adding a callback to the chip driver at each device/partition 
add (see first patch attempt in recent "Fixup Intel flash that powers up 
locked" thread).  But I believe that was part of what was (justifiably) 
deemed messy about the patch.

So far the general verdict on these flashes that power up locked seems 
to be either leave all blocks locked or unlock all blocks (either in the 
bootloader or at flash probe time), and let custom code in userspace 
lock/unlock what is needed.  I do like the idea of the kernel rectifying 
lock status with the map flags myself, though...

-- 
Todd

Re: nand locking

[Timofei V. Bondarenko]

Todd Poynor wrote:
> Timofei V. Bondarenko wrote:
> 
>> It would be nice to unlock the rw partitions only.
>> After add_mtd_partitions() my startap code has no handle to actual 
>> partition info.
>> The mtd_partition.mtdp seems only choice, though it prevents 
>> partitions from registering...
>> So, could couple of lines in add_mtd_partitions() do that work?
> 
> If I understand correctly, it may be the same issue I ran into on Intel 
> NOR flash: the chip driver (which may have the knowledge of whether the 
> particular chips are locked or lockable) gets to see the "whole device" 
> struct mtd_info at probe time, but doesn't see the "partition" structs 
> registered later via add_mtd_partitions(), which have the per-partition 
> r/w flags.

Yes, exactly.

> I tried adding a callback to the chip driver at each device/partition 
> add (see first patch attempt in recent "Fixup Intel flash that powers up 
> locked" thread).  But I believe that was part of what was (justifiably) 
> deemed messy about the patch.

Do you mean 'mtd_info::notify_add' ?
I've mentioned the 'mtd_partition::mtdp' - it could be an alternative
(a bit complicated though). The register_mtd_user() also may help...

> So far the general verdict on these flashes that power up locked seems 
> to be either leave all blocks locked or unlock all blocks (either in the 
> bootloader or at flash probe time), and let custom code in userspace 
> lock/unlock what is needed.  I do like the idea of the kernel rectifying 
> lock status with the map flags myself, though...

I'm not complaining about.

--
	Timofei

Re: nand locking

[Todd Poynor]

Timofei V. Bondarenko wrote:
> Todd Poynor wrote:
...
>> I tried adding a callback to the chip driver at each device/partition 
>> add...
> 
> 
> Do you mean 'mtd_info::notify_add' ?

Yes.

> I've mentioned the 'mtd_partition::mtdp' - it could be an alternative
> (a bit complicated though). The register_mtd_user() also may help...

I originally tried the register_mtd_user() route, but in the case of 
partitions it is passed the "slave" struct mtd_info, while only the 
"master" whole-device struct has been filled in with chip-driver-private 
info (and only mtdpart.c knows how to map the slave back to the master), 
so that would need some ugly tweaks as well...

-- 
Todd