Message ID | 1276855711-18570-2-git-send-email-roman.tereshonkov@nokia.com |
---|---|
State | New, archived |
Headers | show |
CCing Arnd to review the ioctl interface. One thing just struck me, see below. On Fri, 2010-06-18 at 13:08 +0300, Roman Tereshonkov wrote: > Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> > --- > include/mtd/mtd-abi.h | 15 +++++++++++++++ > 1 files changed, 15 insertions(+), 0 deletions(-) > > diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h > index be51ae2..c2c6b41 100644 > --- a/include/mtd/mtd-abi.h > +++ b/include/mtd/mtd-abi.h > @@ -88,6 +88,20 @@ struct otp_info { > __u32 locked; > }; > > +#define MTD_MAX_PARTITION_NAME_LEN 64 > +struct mtd_partition_user { > + __u64 size; > + __u64 offset; > + __u32 mask_flags; > + char name[MTD_MAX_PARTITION_NAME_LEN]; > + __u8 padding[128]; /* reserved for future, must be zero! */ > +}; > + > +struct mtd_partitions { > + __u32 nparts; > + struct mtd_partition_user __user *parts; > +}; Hmm, I think nowadays pointers should be passed as __u64 and compat_ioctl() should be avoided. Also please, document the ioctl a little - add some comments. > + > #define MEMGETINFO _IOR('M', 1, struct mtd_info_user) > #define MEMERASE _IOW('M', 2, struct erase_info_user) > #define MEMWRITEOOB _IOWR('M', 3, struct mtd_oob_buf) > @@ -110,6 +124,7 @@ struct otp_info { > #define MEMERASE64 _IOW('M', 20, struct erase_info_user64) > #define MEMWRITEOOB64 _IOWR('M', 21, struct mtd_oob_buf64) > #define MEMREADOOB64 _IOWR('M', 22, struct mtd_oob_buf64) > +#define MTDREPARTITION _IOW('M', 23, struct mtd_partitions) > > /* > * Obsolete legacy interface. Keep it in order not to break userspace
On Sunday 18 July 2010, Artem Bityutskiy wrote: > CCing Arnd to review the ioctl interface. > > One thing just struck me, see below. > > On Fri, 2010-06-18 at 13:08 +0300, Roman Tereshonkov wrote: > > Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> > > --- > > include/mtd/mtd-abi.h | 15 +++++++++++++++ > > 1 files changed, 15 insertions(+), 0 deletions(-) > > > > diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h > > index be51ae2..c2c6b41 100644 > > --- a/include/mtd/mtd-abi.h > > +++ b/include/mtd/mtd-abi.h > > @@ -88,6 +88,20 @@ struct otp_info { > > __u32 locked; > > }; > > > > +#define MTD_MAX_PARTITION_NAME_LEN 64 > > +struct mtd_partition_user { > > + __u64 size; > > + __u64 offset; > > + __u32 mask_flags; > > + char name[MTD_MAX_PARTITION_NAME_LEN]; > > + __u8 padding[128]; /* reserved for future, must be zero! */ > > +}; > > + > > +struct mtd_partitions { > > + __u32 nparts; > > + struct mtd_partition_user __user *parts; > > +}; > > Hmm, I think nowadays pointers should be passed as __u64 and > compat_ioctl() should be avoided. Yes, that's generally true. It would be nice if the BLKPG ioctl definition could be reused for this. It is more complicated than it should be, but not more than this suggestion, and it's an existing ioctl. Arnd
On Sun, 2010-07-18 at 20:22 +0000, Arnd Bergmann wrote: > > > +#define MTD_MAX_PARTITION_NAME_LEN 64 > > > +struct mtd_partition_user { > > > + __u64 size; > > > + __u64 offset; > > > + __u32 mask_flags; > > > + char name[MTD_MAX_PARTITION_NAME_LEN]; > > > + __u8 padding[128]; /* reserved for future, must be zero! */ > > > +}; > > > + > > > +struct mtd_partitions { > > > + __u32 nparts; > > > + struct mtd_partition_user __user *parts; > > > +}; > > > > Hmm, I think nowadays pointers should be passed as __u64 and > > compat_ioctl() should be avoided. > > Yes, that's generally true. It would be nice if the BLKPG ioctl > definition could be reused for this. It is more complicated > than it should be, but not more than this suggestion, and > it's an existing ioctl. Thanks for reply. MTD devices do not support BLKPG, do you mean we should you the same data-structures and names as block devices?
On Monday 19 July 2010, Artem Bityutskiy wrote: > On Sun, 2010-07-18 at 20:22 +0000, Arnd Bergmann wrote: > > Yes, that's generally true. It would be nice if the BLKPG ioctl > > definition could be reused for this. It is more complicated > > than it should be, but not more than this suggestion, and > > it's an existing ioctl. > > Thanks for reply. > > MTD devices do not support BLKPG, do you mean we should you the same > data-structures and names as block devices? Yes. I'm not sure if it should also be possible to actually repartition the flash using the mtdblock driver, but what I meant was to implement the BLKPG API in the mtdchar driver, with slightly adapted semantics. Arnd
On Tue, 2010-07-20 at 11:19 +0200, Arnd Bergmann wrote: > On Monday 19 July 2010, Artem Bityutskiy wrote: > > On Sun, 2010-07-18 at 20:22 +0000, Arnd Bergmann wrote: > > > Yes, that's generally true. It would be nice if the BLKPG ioctl > > > definition could be reused for this. It is more complicated > > > than it should be, but not more than this suggestion, and > > > it's an existing ioctl. > > > > Thanks for reply. > > > > MTD devices do not support BLKPG, do you mean we should you the same > > data-structures and names as block devices? > > Yes. I'm not sure if it should also be possible to actually repartition > the flash using the mtdblock driver, but what I meant was to implement > the BLKPG API in the mtdchar driver, with slightly adapted semantics. No, using mtdblock is not feasible, because of several reasons like that it is a debugging driver which does not handle bad blocks, and it works on top of mtd, has no idea about mtd internals, and all the partitions logic is inside mtd, and not available for mtdblock. But yes, using the BLKPG API sounds like a reasonable idea, thanks.
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h index be51ae2..c2c6b41 100644 --- a/include/mtd/mtd-abi.h +++ b/include/mtd/mtd-abi.h @@ -88,6 +88,20 @@ struct otp_info { __u32 locked; }; +#define MTD_MAX_PARTITION_NAME_LEN 64 +struct mtd_partition_user { + __u64 size; + __u64 offset; + __u32 mask_flags; + char name[MTD_MAX_PARTITION_NAME_LEN]; + __u8 padding[128]; /* reserved for future, must be zero! */ +}; + +struct mtd_partitions { + __u32 nparts; + struct mtd_partition_user __user *parts; +}; + #define MEMGETINFO _IOR('M', 1, struct mtd_info_user) #define MEMERASE _IOW('M', 2, struct erase_info_user) #define MEMWRITEOOB _IOWR('M', 3, struct mtd_oob_buf) @@ -110,6 +124,7 @@ struct otp_info { #define MEMERASE64 _IOW('M', 20, struct erase_info_user64) #define MEMWRITEOOB64 _IOWR('M', 21, struct mtd_oob_buf64) #define MEMREADOOB64 _IOWR('M', 22, struct mtd_oob_buf64) +#define MTDREPARTITION _IOW('M', 23, struct mtd_partitions) /* * Obsolete legacy interface. Keep it in order not to break userspace
Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> --- include/mtd/mtd-abi.h | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)