[RFC] Support for > 2Gib MTD

[RFC] Support for > 2Gib MTD

[Bruce_Leonard]

So I'm back to work on this.  My last attempt was shot down in flames 
because I changed the kernel-user space API.  No worries on that.  Got a 
new plan.  Gonna' float it out there BEFORE I put a ton of work into it 
this time.

Here's a rough idea of what I'm thinking.  In .../include/mtd/mtd-abi.h 
add new 64 bit structs like this:

+struct erase_info_user_64 {
+       uint64_t start;
+       uint64_t length;
+}

+struct mtd_info_user_64 {
+       ...
+       uint32_t flags;
+       uint64_t size;
+       ...
+}

These new structs would be identical to the old ones execept for the types 
on a few fields.  To indicate that the new 64 bit interface is to be used 
I would introduce a new flag for 'struct mtd_info_user_XX->flags', 
something like this:

#define MTD_LARGE_SIZE 0x4000

In .../include/linux/mtd.h I would add new fields as follows:

struct erase_info {
        ...
        u_int32 fail_addr;
+       u_int64_t addr_64;
+       u_int64_t len_64;
+       u_int64_t fail_addr_64;
        u_long time;
        ...
}

struct mtd_info {
        ...
        u_int32_t size;
+       u_int64_t size_64;
        u_int32_t erasesize;
        ...
}

Plus a few new IOCTLs for getting info to/from user space if people want 
to use it.

Comments?

Bruce

Re: [RFC] Support for > 2Gib MTD

[Artem Bityutskiy]

Hi,

On Fri, 2008-10-03 at 13:36 -0700, Bruce_Leonard@selinc.com wrote:
> Here's a rough idea of what I'm thinking.  In .../include/mtd/mtd-abi.h 
> add new 64 bit structs like this:
> 
> +struct erase_info_user_64 {
> +       uint64_t start;
> +       uint64_t length;
> +}
> 
> +struct mtd_info_user_64 {
> +       ...
> +       uint32_t flags;
> +       uint64_t size;
> +       ...
> +}
> 
> These new structs would be identical to the old ones execept for the types 
> on a few fields.

Please, add something like
uint8_t padding[128]; /* Reserved for future, should be zeroed */
to the new data structures.

>   To indicate that the new 64 bit interface is to be used 
> I would introduce a new flag for 'struct mtd_info_user_XX->flags', 
> something like this:
> 
> #define MTD_LARGE_SIZE 0x4000

No need for this. Because you'll change size of data structures and
their layout, this is not going to be compatible anyway.

> In .../include/linux/mtd.h I would add new fields as follows:
> 
> struct erase_info {
>         ...
>         u_int32 fail_addr;
> +       u_int64_t addr_64;
> +       u_int64_t len_64;
> +       u_int64_t fail_addr_64;
>         u_long time;
>         ...
> }
> 
> struct mtd_info {
>         ...
>         u_int32_t size;
> +       u_int64_t size_64;
>         u_int32_t erasesize;
>         ...
> }

This is what I would avoid doing. I would try to make size 64-bit and
amend all MTD users if needed.

> Plus a few new IOCTLs for getting info to/from user space if people want 
> to use it.

I do not mind ioctl for this, but dwmw2 expressed a desire to do this
via sysfs instead of ioctl...

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [RFC] Support for > 2Gib MTD

[Adrian Hunter]

ext Artem Bityutskiy wrote:
> Hi,
> 
> On Fri, 2008-10-03 at 13:36 -0700, Bruce_Leonard@selinc.com wrote:
>> Here's a rough idea of what I'm thinking.  In .../include/mtd/mtd-abi.h 
>> add new 64 bit structs like this:
>>
>> +struct erase_info_user_64 {
>> +       uint64_t start;
>> +       uint64_t length;
>> +}
>>
>> +struct mtd_info_user_64 {
>> +       ...
>> +       uint32_t flags;
>> +       uint64_t size;
>> +       ...
>> +}
>>
>> These new structs would be identical to the old ones execept for the types 
>> on a few fields.
> 
> Please, add something like
> uint8_t padding[128]; /* Reserved for future, should be zeroed */
> to the new data structures.
> 
>>   To indicate that the new 64 bit interface is to be used 
>> I would introduce a new flag for 'struct mtd_info_user_XX->flags', 
>> something like this:
>>
>> #define MTD_LARGE_SIZE 0x4000
> 
> No need for this. Because you'll change size of data structures and
> their layout, this is not going to be compatible anyway.
> 
>> In .../include/linux/mtd.h I would add new fields as follows:
>>
>> struct erase_info {
>>         ...
>>         u_int32 fail_addr;
>> +       u_int64_t addr_64;
>> +       u_int64_t len_64;
>> +       u_int64_t fail_addr_64;
>>         u_long time;
>>         ...
>> }
>>
>> struct mtd_info {
>>         ...
>>         u_int32_t size;
>> +       u_int64_t size_64;
>>         u_int32_t erasesize;
>>         ...
>> }
> 
> This is what I would avoid doing. I would try to make size 64-bit and
> amend all MTD users if needed.

Yes, why not just change the size to 64-bits and fix the code that assumes
32-bits?  I have unfinished patches that take that approach - they seem
to work for NAND - at least nandsim works.

>> Plus a few new IOCTLs for getting info to/from user space if people want 
>> to use it.
> 
> I do not mind ioctl for this, but dwmw2 expressed a desire to do this
> via sysfs instead of ioctl...

It would be simpler, more consistent, and easier to understand just to
create a set of 64-bit IOCTLs that are otherwise the same as their 32-bit
counterparts.

Re: [RFC] Support for > 2Gib MTD

[Amit Kumar Sharma]

Hi Folks

We should consider 4KB page size also that why i think 
padding of dummy bits will help for such issues.
because we have faced problem in abi.h file because current 
mtd-abi.h structure donot consider 4KB Nand flash.


Rgs
Amit
----- Original Message ----- 
From: "Artem Bityutskiy" <dedekind@infradead.org>
To: <Bruce_Leonard@selinc.com>
Cc: <linux-mtd@lists.infradead.org>
Sent: Monday, October 06, 2008 10:54 AM
Subject: Re: [RFC] Support for > 2Gib MTD


> Hi,
>
> On Fri, 2008-10-03 at 13:36 -0700, 
> Bruce_Leonard@selinc.com wrote:
>> Here's a rough idea of what I'm thinking.  In 
>> .../include/mtd/mtd-abi.h
>> add new 64 bit structs like this:
>>
>> +struct erase_info_user_64 {
>> +       uint64_t start;
>> +       uint64_t length;
>> +}
>>
>> +struct mtd_info_user_64 {
>> +       ...
>> +       uint32_t flags;
>> +       uint64_t size;
>> +       ...
>> +}
>>
>> These new structs would be identical to the old ones 
>> execept for the types
>> on a few fields.
>
> Please, add something like
> uint8_t padding[128]; /* Reserved for future, should be 
> zeroed */
> to the new data structures.
>
>>   To indicate that the new 64 bit interface is to be used
>> I would introduce a new flag for 'struct 
>> mtd_info_user_XX->flags',
>> something like this:
>>
>> #define MTD_LARGE_SIZE 0x4000
>
> No need for this. Because you'll change size of data 
> structures and
> their layout, this is not going to be compatible anyway.
>
>> In .../include/linux/mtd.h I would add new fields as 
>> follows:
>>
>> struct erase_info {
>>         ...
>>         u_int32 fail_addr;
>> +       u_int64_t addr_64;
>> +       u_int64_t len_64;
>> +       u_int64_t fail_addr_64;
>>         u_long time;
>>         ...
>> }
>>
>> struct mtd_info {
>>         ...
>>         u_int32_t size;
>> +       u_int64_t size_64;
>>         u_int32_t erasesize;
>>         ...
>> }
>
> This is what I would avoid doing. I would try to make size 
> 64-bit and
> amend all MTD users if needed.
>
>> Plus a few new IOCTLs for getting info to/from user space 
>> if people want
>> to use it.
>
> I do not mind ioctl for this, but dwmw2 expressed a desire 
> to do this
> via sysfs instead of ioctl...
>
> -- 
> Best regards,
> Artem Bityutskiy (???????? ?????)
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

Re: [RFC] Support for > 2Gib MTD

[Artem Bityutskiy]

On Mon, 2008-10-06 at 14:32 +0530, Amit Kumar Sharma wrote:
> Hi Folks
> 
> We should consider 4KB page size also that why i think 
> padding of dummy bits will help for such issues.
> because we have faced problem in abi.h file because current 
> mtd-abi.h structure donot consider 4KB Nand flash.

I'm sorry for telling this, but could you please not top
post? This is standard good tone practice in public mailing
list. We discussed several things, and why do you make the
reader (me) guess what you are talking about and what exactly
you comment?

Please, write your post _below_ particular text you refer to.

Also, it is good tone to remove irrelevant stuff when you
reply. Just see how other people do this. E.g, see how I
replied to Burce's e-mail.

Apologies for teaching you and thanks in advance.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [RFC] Support for > 2Gib MTD

[Bruce_Leonard]

Hi Artem,

Sorry for the lateness of this, had a couple of fires to put out in 
manufacturing this week :).

Artem Bityutskiy <dedekind@infradead.org> wrote on 10/05/2008 10:24:48 PM:

> Hi,
> 
> On Fri, 2008-10-03 at 13:36 -0700, Bruce_Leonard@selinc.com wrote:
> > Here's a rough idea of what I'm thinking.  In 
.../include/mtd/mtd-abi.h 
> > add new 64 bit structs like this:
> > 
> > +struct erase_info_user_64 {
> > +       uint64_t start;
> > +       uint64_t length;
> > +}
> > 
> > +struct mtd_info_user_64 {
> > +       ...
> > +       uint32_t flags;
> > +       uint64_t size;
> > +       ...
> > +}
> > 
> > These new structs would be identical to the old ones execept for the 
types 
> > on a few fields.
> 
> Please, add something like
> uint8_t padding[128]; /* Reserved for future, should be zeroed */
> to the new data structures.
>

Is this for future expandibility?
 
> >   To indicate that the new 64 bit interface is to be used 
> > I would introduce a new flag for 'struct mtd_info_user_XX->flags', 
> > something like this:
> > 
> > #define MTD_LARGE_SIZE 0x4000
> 
> No need for this. Because you'll change size of data structures and
> their layout, this is not going to be compatible anyway.
> 

How then would you indicate to user space that they need to use the new 
64-bit structures and IOCTLs?  How does using another bit in an existing 
'flags' field of a structure change the size and layout of the structure?

Bruce

Re: [RFC] Support for > 2Gib MTD

[Artem Bityutskiy]

On Thu, 2008-10-09 at 11:49 -0700, Bruce_Leonard@selinc.com wrote:
> > Please, add something like
> > uint8_t padding[128]; /* Reserved for future, should be zeroed */
> > to the new data structures.
> >
> 
> Is this for future expandibility?

Sure.

> > >   To indicate that the new 64 bit interface is to be used 
> > > I would introduce a new flag for 'struct mtd_info_user_XX->flags', 
> > > something like this:
> > > 
> > > #define MTD_LARGE_SIZE 0x4000
> > 
> > No need for this. Because you'll change size of data structures and
> > their layout, this is not going to be compatible anyway.
> >
> How then would you indicate to user space that they need to use the new 
> 64-bit structures and IOCTLs?  How does using another bit in an existing 
> 'flags' field of a structure change the size and layout of the structure?

If you have different data structure, you cannot reuse old ioctl. You
have to define a new ioctl for the 64-bit interface. Old code will use
old ioctl which you do not change. New code will use new ioctl. Once we
have new ioctls, the old one may be declared as obsolete. This is my
understanding of how things go.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)