[PATCH] MTD ioctl interface rework to get rid of MEMGETOOBSEL/MEMSETOOBSEL

[PATCH] MTD ioctl interface rework to get rid of MEMGETOOBSEL/MEMSETOOBSEL

[Vitaly Wool]

Hi,

this patch gets rid of MEMGETOOBSEL/MEMSETOOBSEL ioctls and introduces a new one (MEMGETOOBAVAIL).
As page layout is moving towards being an internal NAND layer knowledge, there's no need to let the user lavel apps change or read it. On the other hand, user level apps will need to know how many bytes long a free OOB area is for this particular flash type.
The patch for MTD utilities will be ready shortly.

Vitaly

diff -uNr linux-2.6.10.orig/drivers/mtd/mtdchar.c linux-2.6.10.nand/drivers/mtd/mtdchar.c
--- linux-2.6.10.orig/drivers/mtd/mtdchar.c	2005-11-24 15:58:37.000000000 +0300
+++ linux-2.6.10.nand/drivers/mtd/mtdchar.c	2005-11-28 16:40:40.000000000 +0300
@@ -520,16 +520,9 @@
 		break;
 	}
 
-	case MEMSETOOBSEL:
+	case MEMGETOOBAVAIL:
 	{
-		if (copy_from_user(&mtd->oobinfo, argp, sizeof(struct nand_oobinfo)))
-			return -EFAULT;
-		break;
-	}
-
-	case MEMGETOOBSEL:
-	{
-		if (copy_to_user(argp, &(mtd->oobinfo), sizeof(struct nand_oobinfo)))
+		if (copy_to_user(argp, &(mtd->oobavail), sizeof(mtd->oobavail)))
 			return -EFAULT;
 		break;
 	}
diff -uNr linux-2.6.10.orig/include/mtd/mtd-abi.h linux-2.6.10.nand/include/mtd/mtd-abi.h
--- linux-2.6.10.orig/include/mtd/mtd-abi.h	2005-11-24 15:58:49.000000000 +0300
+++ linux-2.6.10.nand/include/mtd/mtd-abi.h	2005-11-28 17:03:12.000000000 +0300
@@ -102,8 +102,7 @@
 #define MEMUNLOCK               _IOW('M', 6, struct erase_info_user)
 #define MEMGETREGIONCOUNT	_IOR('M', 7, int)
 #define MEMGETREGIONINFO	_IOWR('M', 8, struct region_info_user)
-#define MEMSETOOBSEL		_IOW('M', 9, struct nand_oobinfo)
-#define MEMGETOOBSEL		_IOR('M', 10, struct nand_oobinfo)
+#define MEMGETOOBAVAIL		_IOR('M', 10, uint32_t)
 #define MEMGETBADBLOCK		_IOW('M', 11, loff_t)
 #define MEMSETBADBLOCK		_IOW('M', 12, loff_t)
 #define OTPSELECT		_IOR('M', 13, int)

Re: [PATCH] MTD ioctl interface rework to get rid of MEMGETOOBSEL/MEMSETOOBSEL

[Jörn Engel]

On Tue, 29 November 2005 18:06:59 +0300, Vitaly Wool wrote:
> diff -uNr linux-2.6.10.orig/include/mtd/mtd-abi.h linux-2.6.10.nand/include/mtd/mtd-abi.h
> --- linux-2.6.10.orig/include/mtd/mtd-abi.h	2005-11-24 15:58:49.000000000 +0300
> +++ linux-2.6.10.nand/include/mtd/mtd-abi.h	2005-11-28 17:03:12.000000000 +0300
> @@ -102,8 +102,7 @@
>  #define MEMUNLOCK               _IOW('M', 6, struct erase_info_user)
>  #define MEMGETREGIONCOUNT	_IOR('M', 7, int)
>  #define MEMGETREGIONINFO	_IOWR('M', 8, struct region_info_user)
> -#define MEMSETOOBSEL		_IOW('M', 9, struct nand_oobinfo)
> -#define MEMGETOOBSEL		_IOR('M', 10, struct nand_oobinfo)
> +#define MEMGETOOBAVAIL		_IOR('M', 10, uint32_t)
>  #define MEMGETBADBLOCK		_IOW('M', 11, loff_t)
>  #define MEMSETBADBLOCK		_IOW('M', 12, loff_t)
>  #define OTPSELECT		_IOR('M', 13, int)

You really don't want to reuse an old ioctl().  Even removing the code
to handle an old number breaks binary compatibility.  But changing the
behaviour will result in silent breakage.

Jörn

-- 
When in doubt, punt.  When somebody actually complains, go back and fix it...
The 90% solution is a good thing.
-- Rob Landley

Re: [PATCH] MTD ioctl interface rework to get rid of MEMGETOOBSEL/MEMSETOOBSEL

[Vitaly Wool]

Agreed, thanks.
However, I think we should return error for MEMSETOOBSEL/MEMGETOOBSEL 
for userland apps to learn they can't change oobinfo any more.

Vitaly

JЖrn Engel wrote:

>On Tue, 29 November 2005 18:06:59 +0300, Vitaly Wool wrote:
>  
>
>>diff -uNr linux-2.6.10.orig/include/mtd/mtd-abi.h linux-2.6.10.nand/include/mtd/mtd-abi.h
>>--- linux-2.6.10.orig/include/mtd/mtd-abi.h	2005-11-24 15:58:49.000000000 +0300
>>+++ linux-2.6.10.nand/include/mtd/mtd-abi.h	2005-11-28 17:03:12.000000000 +0300
>>@@ -102,8 +102,7 @@
>> #define MEMUNLOCK               _IOW('M', 6, struct erase_info_user)
>> #define MEMGETREGIONCOUNT	_IOR('M', 7, int)
>> #define MEMGETREGIONINFO	_IOWR('M', 8, struct region_info_user)
>>-#define MEMSETOOBSEL		_IOW('M', 9, struct nand_oobinfo)
>>-#define MEMGETOOBSEL		_IOR('M', 10, struct nand_oobinfo)
>>+#define MEMGETOOBAVAIL		_IOR('M', 10, uint32_t)
>> #define MEMGETBADBLOCK		_IOW('M', 11, loff_t)
>> #define MEMSETBADBLOCK		_IOW('M', 12, loff_t)
>> #define OTPSELECT		_IOR('M', 13, int)
>>    
>>
>
>You really don't want to reuse an old ioctl().  Even removing the code
>to handle an old number breaks binary compatibility.  But changing the
>behaviour will result in silent breakage.
>
>JЖrn
>
>  
>

Re: [PATCH] MTD ioctl interface rework to get rid of MEMGETOOBSEL/MEMSETOOBSEL

[Jörn Engel]

On Tue, 29 November 2005 18:39:40 +0300, Vitaly Wool wrote:
> 
> However, I think we should return error for MEMSETOOBSEL/MEMGETOOBSEL 
> for userland apps to learn they can't change oobinfo any more.

No objections from my side.

Jörn

-- 
When people work hard for you for a pat on the back, you've got
to give them that pat.
-- Robert Heinlein