[PATCH] Add sysfs attribute for mtd OOB available size

[PATCH] Add sysfs attribute for mtd OOB available size

[Xiaolei Li]

This patch adds one new sysfs node named oobavail to expose mtd OOB
available size.

Changes relative to:
--------------------

tree    : https://github.com/bbrezillon/linux-0day
branch  : mtd/next
commit  :
	'commit 097ccca726ff ("mtd: nand: Fix some function
	 description mismatches in core.c")'

Xiaolei Li (1):
  mtd: Add sysfs attribute for mtd OOB available size

 Documentation/ABI/testing/sysfs-class-mtd |  8 ++++++++
 drivers/mtd/mtdcore.c                     | 10 ++++++++++
 2 files changed, 18 insertions(+)

-- 
1.9.1

[PATCH] mtd: Add sysfs attribute for mtd OOB available size

[Xiaolei Li]

Expose mtd OOB available size by sysfs file. Then users can get available
OOB size by accessing /sys/class/mtd/mtdX/oobavail.

Signed-off-by: Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 Documentation/ABI/testing/sysfs-class-mtd |  8 ++++++++
 drivers/mtd/mtdcore.c                     | 10 ++++++++++
 2 files changed, 18 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-mtd b/Documentation/ABI/testing/sysfs-class-mtd
index f34e5923..3bc7c0a 100644
--- a/Documentation/ABI/testing/sysfs-class-mtd
+++ b/Documentation/ABI/testing/sysfs-class-mtd
@@ -232,3 +232,11 @@ Description:
 		of the parent (another partition or a flash device) in bytes.
 		This attribute is absent on flash devices, so it can be used
 		to distinguish them from partitions.
+
+What:		/sys/class/mtd/mtdX/oobavail
+Date:		April 2018
+KernelVersion:	4.16
+Contact:	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
+Description:
+		Number of bytes available for a client to place data into
+		the out of band area.
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 807d17d..99d9352 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -210,6 +210,15 @@ static ssize_t mtd_oobsize_show(struct device *dev,
 }
 static DEVICE_ATTR(oobsize, S_IRUGO, mtd_oobsize_show, NULL);
 
+static ssize_t mtd_oobavail_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct mtd_info *mtd = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%lu\n", (unsigned long)mtd->oobavail);
+}
+static DEVICE_ATTR(oobavail, S_IRUGO, mtd_oobavail_show, NULL);
+
 static ssize_t mtd_numeraseregions_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
@@ -327,6 +336,7 @@ static ssize_t mtd_bbtblocks_show(struct device *dev,
 	&dev_attr_writesize.attr,
 	&dev_attr_subpagesize.attr,
 	&dev_attr_oobsize.attr,
+	&dev_attr_oobavail.attr,
 	&dev_attr_numeraseregions.attr,
 	&dev_attr_name.attr,
 	&dev_attr_ecc_strength.attr,
-- 
1.9.1

Re: [PATCH] mtd: Add sysfs attribute for mtd OOB available size

[Boris Brezillon]

Hi Xiaolei,

On Mon, 2 Apr 2018 16:20:10 +0800
Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:

> Expose mtd OOB available size by sysfs file. Then users can get available
> OOB size by accessing /sys/class/mtd/mtdX/oobavail.

May I ask why you need to expose that? I'm not against exposing new
things through sysfs, but since this is part of the ABI, I'd like to be
sure we actually need it.

Regards,

Boris

> 
> Signed-off-by: Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/ABI/testing/sysfs-class-mtd |  8 ++++++++
>  drivers/mtd/mtdcore.c                     | 10 ++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-mtd b/Documentation/ABI/testing/sysfs-class-mtd
> index f34e5923..3bc7c0a 100644
> --- a/Documentation/ABI/testing/sysfs-class-mtd
> +++ b/Documentation/ABI/testing/sysfs-class-mtd
> @@ -232,3 +232,11 @@ Description:
>  		of the parent (another partition or a flash device) in bytes.
>  		This attribute is absent on flash devices, so it can be used
>  		to distinguish them from partitions.
> +
> +What:		/sys/class/mtd/mtdX/oobavail
> +Date:		April 2018
> +KernelVersion:	4.16
> +Contact:	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> +Description:
> +		Number of bytes available for a client to place data into
> +		the out of band area.
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 807d17d..99d9352 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -210,6 +210,15 @@ static ssize_t mtd_oobsize_show(struct device *dev,
>  }
>  static DEVICE_ATTR(oobsize, S_IRUGO, mtd_oobsize_show, NULL);
>  
> +static ssize_t mtd_oobavail_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct mtd_info *mtd = dev_get_drvdata(dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%lu\n", (unsigned long)mtd->oobavail);
> +}
> +static DEVICE_ATTR(oobavail, S_IRUGO, mtd_oobavail_show, NULL);
> +
>  static ssize_t mtd_numeraseregions_show(struct device *dev,
>  		struct device_attribute *attr, char *buf)
>  {
> @@ -327,6 +336,7 @@ static ssize_t mtd_bbtblocks_show(struct device *dev,
>  	&dev_attr_writesize.attr,
>  	&dev_attr_subpagesize.attr,
>  	&dev_attr_oobsize.attr,
> +	&dev_attr_oobavail.attr,
>  	&dev_attr_numeraseregions.attr,
>  	&dev_attr_name.attr,
>  	&dev_attr_ecc_strength.attr,



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] mtd: Add sysfs attribute for mtd OOB available size

[xiaolei li]

Hello Boris,

On Mon, 2018-04-02 at 14:15 +0200, Boris Brezillon wrote:
> Hi Xiaolei,
> 
> On Mon, 2 Apr 2018 16:20:10 +0800
> Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> 
> > Expose mtd OOB available size by sysfs file. Then users can get available
> > OOB size by accessing /sys/class/mtd/mtdX/oobavail.
> 
> May I ask why you need to expose that? I'm not against exposing new
> things through sysfs, but since this is part of the ABI, I'd like to be
> sure we actually need it.
> 
That is user-space can write OOB data through ioctl MEMWRITE now.
If OOB operation mode is MTD_OPS_AUTO_OOB, user should know how many
OOB available bytes it can use. But I didn't find a way to get it. (If
there is already a method, please kindly let me know, thanks.)

One problem I know is to do Jffs2 type flash erase using MTD user-space
tool flash_erase. flash_erase tool will program "cleanmarker" into OOB
free area, but it can not get OOB available size.
Please refer this commit:
http://git.infradead.org/mtd-utils.git/commit/d7e86124d55bbcee1b82c68b82389ebcda588076

Thanks!
Xiaolei

> Regards,
> 
> Boris
> 
> > 
> > Signed-off-by: Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  Documentation/ABI/testing/sysfs-class-mtd |  8 ++++++++
> >  drivers/mtd/mtdcore.c                     | 10 ++++++++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-class-mtd b/Documentation/ABI/testing/sysfs-class-mtd
> > index f34e5923..3bc7c0a 100644
> > --- a/Documentation/ABI/testing/sysfs-class-mtd
> > +++ b/Documentation/ABI/testing/sysfs-class-mtd
> > @@ -232,3 +232,11 @@ Description:
> >  		of the parent (another partition or a flash device) in bytes.
> >  		This attribute is absent on flash devices, so it can be used
> >  		to distinguish them from partitions.
> > +
> > +What:		/sys/class/mtd/mtdX/oobavail
> > +Date:		April 2018
> > +KernelVersion:	4.16
> > +Contact:	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> > +Description:
> > +		Number of bytes available for a client to place data into
> > +		the out of band area.
> > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> > index 807d17d..99d9352 100644
> > --- a/drivers/mtd/mtdcore.c
> > +++ b/drivers/mtd/mtdcore.c
> > @@ -210,6 +210,15 @@ static ssize_t mtd_oobsize_show(struct device *dev,
> >  }
> >  static DEVICE_ATTR(oobsize, S_IRUGO, mtd_oobsize_show, NULL);
> >  
> > +static ssize_t mtd_oobavail_show(struct device *dev,
> > +				 struct device_attribute *attr, char *buf)
> > +{
> > +	struct mtd_info *mtd = dev_get_drvdata(dev);
> > +
> > +	return snprintf(buf, PAGE_SIZE, "%lu\n", (unsigned long)mtd->oobavail);
> > +}
> > +static DEVICE_ATTR(oobavail, S_IRUGO, mtd_oobavail_show, NULL);
> > +
> >  static ssize_t mtd_numeraseregions_show(struct device *dev,
> >  		struct device_attribute *attr, char *buf)
> >  {
> > @@ -327,6 +336,7 @@ static ssize_t mtd_bbtblocks_show(struct device *dev,
> >  	&dev_attr_writesize.attr,
> >  	&dev_attr_subpagesize.attr,
> >  	&dev_attr_oobsize.attr,
> > +	&dev_attr_oobavail.attr,
> >  	&dev_attr_numeraseregions.attr,
> >  	&dev_attr_name.attr,
> >  	&dev_attr_ecc_strength.attr,
> 
> 
> 

Re: [PATCH] mtd: Add sysfs attribute for mtd OOB available size

[Boris Brezillon]

On Tue, 3 Apr 2018 10:46:58 +0800
xiaolei li <xiaolei.li@mediatek.com> wrote:

> Hello Boris,
> 
> On Mon, 2018-04-02 at 14:15 +0200, Boris Brezillon wrote:
> > Hi Xiaolei,
> > 
> > On Mon, 2 Apr 2018 16:20:10 +0800
> > Xiaolei Li <xiaolei.li@mediatek.com> wrote:
> >   
> > > Expose mtd OOB available size by sysfs file. Then users can get available
> > > OOB size by accessing /sys/class/mtd/mtdX/oobavail.  
> > 
> > May I ask why you need to expose that? I'm not against exposing new
> > things through sysfs, but since this is part of the ABI, I'd like to be
> > sure we actually need it.
> >   
> That is user-space can write OOB data through ioctl MEMWRITE now.
> If OOB operation mode is MTD_OPS_AUTO_OOB, user should know how many
> OOB available bytes it can use. But I didn't find a way to get it. (If
> there is already a method, please kindly let me know, thanks.)

Nope. We have the ECCGETLAYOUT ioctl, but it's not reliable because of
the MTD_MAX_OOBFREE_ENTRIES limitation.

> 
> One problem I know is to do Jffs2 type flash erase using MTD user-space
> tool flash_erase. flash_erase tool will program "cleanmarker" into OOB
> free area, but it can not get OOB available size.

That's a good reason to expose this property indeed. Do you plan to fix
mtd-utils to use the sysfs value when it's available? Also, do we need
to expose the ECC/free layout as done with the
ECCGETLAYOUT/MEMGETOOBSEL ioctls?


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: Add sysfs attribute for mtd OOB available size

[xiaolei li]

On Tue, 2018-04-03 at 10:43 +0200, Boris Brezillon wrote:
> On Tue, 3 Apr 2018 10:46:58 +0800
> xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> 
> > Hello Boris,
> > 
> > On Mon, 2018-04-02 at 14:15 +0200, Boris Brezillon wrote:
> > > Hi Xiaolei,
> > > 
> > > On Mon, 2 Apr 2018 16:20:10 +0800
> > > Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > >   
> > > > Expose mtd OOB available size by sysfs file. Then users can get available
> > > > OOB size by accessing /sys/class/mtd/mtdX/oobavail.  
> > > 
> > > May I ask why you need to expose that? I'm not against exposing new
> > > things through sysfs, but since this is part of the ABI, I'd like to be
> > > sure we actually need it.
> > >   
> > That is user-space can write OOB data through ioctl MEMWRITE now.
> > If OOB operation mode is MTD_OPS_AUTO_OOB, user should know how many
> > OOB available bytes it can use. But I didn't find a way to get it. (If
> > there is already a method, please kindly let me know, thanks.)
> 
> Nope. We have the ECCGETLAYOUT ioctl, but it's not reliable because of
> the MTD_MAX_OOBFREE_ENTRIES limitation.
> 
> > 
> > One problem I know is to do Jffs2 type flash erase using MTD user-space
> > tool flash_erase. flash_erase tool will program "cleanmarker" into OOB
> > free area, but it can not get OOB available size.
> 
> That's a good reason to expose this property indeed. Do you plan to fix
> mtd-utils to use the sysfs value when it's available? Also, do we need
> to expose the ECC/free layout as done with the
> ECCGETLAYOUT/MEMGETOOBSEL ioctls?
> 
Yes. I plan to fix mtd-utils problem if this change is accepted.

struct nand_oobinfo and struct nand_ecclayout_user in mtd-abi.h are
obsoleted, and user can not get oob layout by ioctl method now.
sysfs seems a method to fetch oob layout. But I don't know which
user-space case will use oob layout now. Maybe expose them in the future
if they are really needed?

Thanks,
Xiaolei

Re: [PATCH] mtd: Add sysfs attribute for mtd OOB available size

[Boris Brezillon]

On Tue, 3 Apr 2018 17:40:11 +0800
xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:

> On Tue, 2018-04-03 at 10:43 +0200, Boris Brezillon wrote:
> > On Tue, 3 Apr 2018 10:46:58 +0800
> > xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >   
> > > Hello Boris,
> > > 
> > > On Mon, 2018-04-02 at 14:15 +0200, Boris Brezillon wrote:  
> > > > Hi Xiaolei,
> > > > 
> > > > On Mon, 2 Apr 2018 16:20:10 +0800
> > > > Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > > >     
> > > > > Expose mtd OOB available size by sysfs file. Then users can get available
> > > > > OOB size by accessing /sys/class/mtd/mtdX/oobavail.    
> > > > 
> > > > May I ask why you need to expose that? I'm not against exposing new
> > > > things through sysfs, but since this is part of the ABI, I'd like to be
> > > > sure we actually need it.
> > > >     
> > > That is user-space can write OOB data through ioctl MEMWRITE now.
> > > If OOB operation mode is MTD_OPS_AUTO_OOB, user should know how many
> > > OOB available bytes it can use. But I didn't find a way to get it. (If
> > > there is already a method, please kindly let me know, thanks.)  
> > 
> > Nope. We have the ECCGETLAYOUT ioctl, but it's not reliable because of
> > the MTD_MAX_OOBFREE_ENTRIES limitation.
> >   
> > > 
> > > One problem I know is to do Jffs2 type flash erase using MTD user-space
> > > tool flash_erase. flash_erase tool will program "cleanmarker" into OOB
> > > free area, but it can not get OOB available size.  
> > 
> > That's a good reason to expose this property indeed. Do you plan to fix
> > mtd-utils to use the sysfs value when it's available? Also, do we need
> > to expose the ECC/free layout as done with the
> > ECCGETLAYOUT/MEMGETOOBSEL ioctls?
> >   
> Yes. I plan to fix mtd-utils problem if this change is accepted.
> 
> struct nand_oobinfo and struct nand_ecclayout_user in mtd-abi.h are
> obsoleted, and user can not get oob layout by ioctl method now.
> sysfs seems a method to fetch oob layout. But I don't know which
> user-space case will use oob layout now. Maybe expose them in the future
> if they are really needed?

If the layout is not needed yet, let's not expose it. BTW, are you
really using JFFS2 on modern NAND chips? Sounds like a bad idea to me.

Re: [PATCH] mtd: Add sysfs attribute for mtd OOB available size

[xiaolei li]

On Tue, 2018-04-03 at 12:14 +0200, Boris Brezillon wrote:
> On Tue, 3 Apr 2018 17:40:11 +0800
> xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> 
> > On Tue, 2018-04-03 at 10:43 +0200, Boris Brezillon wrote:
> > > On Tue, 3 Apr 2018 10:46:58 +0800
> > > xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > >   
> > > > Hello Boris,
> > > > 
> > > > On Mon, 2018-04-02 at 14:15 +0200, Boris Brezillon wrote:  
> > > > > Hi Xiaolei,
> > > > > 
> > > > > On Mon, 2 Apr 2018 16:20:10 +0800
> > > > > Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > > > >     
> > > > > > Expose mtd OOB available size by sysfs file. Then users can get available
> > > > > > OOB size by accessing /sys/class/mtd/mtdX/oobavail.    
> > > > > 
> > > > > May I ask why you need to expose that? I'm not against exposing new
> > > > > things through sysfs, but since this is part of the ABI, I'd like to be
> > > > > sure we actually need it.
> > > > >     
> > > > That is user-space can write OOB data through ioctl MEMWRITE now.
> > > > If OOB operation mode is MTD_OPS_AUTO_OOB, user should know how many
> > > > OOB available bytes it can use. But I didn't find a way to get it. (If
> > > > there is already a method, please kindly let me know, thanks.)  
> > > 
> > > Nope. We have the ECCGETLAYOUT ioctl, but it's not reliable because of
> > > the MTD_MAX_OOBFREE_ENTRIES limitation.
> > >   
> > > > 
> > > > One problem I know is to do Jffs2 type flash erase using MTD user-space
> > > > tool flash_erase. flash_erase tool will program "cleanmarker" into OOB
> > > > free area, but it can not get OOB available size.  
> > > 
> > > That's a good reason to expose this property indeed. Do you plan to fix
> > > mtd-utils to use the sysfs value when it's available? Also, do we need
> > > to expose the ECC/free layout as done with the
> > > ECCGETLAYOUT/MEMGETOOBSEL ioctls?
> > >   
> > Yes. I plan to fix mtd-utils problem if this change is accepted.
> > 
> > struct nand_oobinfo and struct nand_ecclayout_user in mtd-abi.h are
> > obsoleted, and user can not get oob layout by ioctl method now.
> > sysfs seems a method to fetch oob layout. But I don't know which
> > user-space case will use oob layout now. Maybe expose them in the future
> > if they are really needed?
> 
> If the layout is not needed yet, let's not expose it. BTW, are you
> really using JFFS2 on modern NAND chips? Sounds like a bad idea to me.
Yes. I tested Micron SLC NAND with 4K page size and 224 OOB size these
days. Jffs2 runs good on this NAND.

Re: [PATCH] mtd: Add sysfs attribute for mtd OOB available size

[Boris Brezillon]

On Tue, 3 Apr 2018 19:01:03 +0800
xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:

> On Tue, 2018-04-03 at 12:14 +0200, Boris Brezillon wrote:
> > On Tue, 3 Apr 2018 17:40:11 +0800
> > xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >   
> > > On Tue, 2018-04-03 at 10:43 +0200, Boris Brezillon wrote:  
> > > > On Tue, 3 Apr 2018 10:46:58 +0800
> > > > xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > > >     
> > > > > Hello Boris,
> > > > > 
> > > > > On Mon, 2018-04-02 at 14:15 +0200, Boris Brezillon wrote:    
> > > > > > Hi Xiaolei,
> > > > > > 
> > > > > > On Mon, 2 Apr 2018 16:20:10 +0800
> > > > > > Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > > > > >       
> > > > > > > Expose mtd OOB available size by sysfs file. Then users can get available
> > > > > > > OOB size by accessing /sys/class/mtd/mtdX/oobavail.      
> > > > > > 
> > > > > > May I ask why you need to expose that? I'm not against exposing new
> > > > > > things through sysfs, but since this is part of the ABI, I'd like to be
> > > > > > sure we actually need it.
> > > > > >       
> > > > > That is user-space can write OOB data through ioctl MEMWRITE now.
> > > > > If OOB operation mode is MTD_OPS_AUTO_OOB, user should know how many
> > > > > OOB available bytes it can use. But I didn't find a way to get it. (If
> > > > > there is already a method, please kindly let me know, thanks.)    
> > > > 
> > > > Nope. We have the ECCGETLAYOUT ioctl, but it's not reliable because of
> > > > the MTD_MAX_OOBFREE_ENTRIES limitation.
> > > >     
> > > > > 
> > > > > One problem I know is to do Jffs2 type flash erase using MTD user-space
> > > > > tool flash_erase. flash_erase tool will program "cleanmarker" into OOB
> > > > > free area, but it can not get OOB available size.    
> > > > 
> > > > That's a good reason to expose this property indeed. Do you plan to fix
> > > > mtd-utils to use the sysfs value when it's available? Also, do we need
> > > > to expose the ECC/free layout as done with the
> > > > ECCGETLAYOUT/MEMGETOOBSEL ioctls?
> > > >     
> > > Yes. I plan to fix mtd-utils problem if this change is accepted.
> > > 
> > > struct nand_oobinfo and struct nand_ecclayout_user in mtd-abi.h are
> > > obsoleted, and user can not get oob layout by ioctl method now.
> > > sysfs seems a method to fetch oob layout. But I don't know which
> > > user-space case will use oob layout now. Maybe expose them in the future
> > > if they are really needed?  
> > 
> > If the layout is not needed yet, let's not expose it. BTW, are you
> > really using JFFS2 on modern NAND chips? Sounds like a bad idea to me.  
> Yes. I tested Micron SLC NAND with 4K page size and 224 OOB size these
> days. Jffs2 runs good on this NAND.

Yes, it should work, but I'm not sure JFFS2 scales well on >128MB
NANDs, and people tend to use UBI on such devices nowadays (this + the
fact that JFFS2 is not actively maintained).

Anyway, I guess your change is acceptable, I'd just like to see
patches for mtd-utils using this sysfs entry now ;-).

Regards,

Boris

Re: [PATCH] mtd: Add sysfs attribute for mtd OOB available size

[xiaolei li]

On Wed, 2018-04-04 at 22:05 +0200, Boris Brezillon wrote:
> On Tue, 3 Apr 2018 19:01:03 +0800
> xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> 
> > On Tue, 2018-04-03 at 12:14 +0200, Boris Brezillon wrote:
> > > On Tue, 3 Apr 2018 17:40:11 +0800
> > > xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > >   
> > > > On Tue, 2018-04-03 at 10:43 +0200, Boris Brezillon wrote:  
> > > > > On Tue, 3 Apr 2018 10:46:58 +0800
> > > > > xiaolei li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > > > >     
> > > > > > Hello Boris,
> > > > > > 
> > > > > > On Mon, 2018-04-02 at 14:15 +0200, Boris Brezillon wrote:    
> > > > > > > Hi Xiaolei,
> > > > > > > 
> > > > > > > On Mon, 2 Apr 2018 16:20:10 +0800
> > > > > > > Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > > > > > >       
> > > > > > > > Expose mtd OOB available size by sysfs file. Then users can get available
> > > > > > > > OOB size by accessing /sys/class/mtd/mtdX/oobavail.      
> > > > > > > 
> > > > > > > May I ask why you need to expose that? I'm not against exposing new
> > > > > > > things through sysfs, but since this is part of the ABI, I'd like to be
> > > > > > > sure we actually need it.
> > > > > > >       
> > > > > > That is user-space can write OOB data through ioctl MEMWRITE now.
> > > > > > If OOB operation mode is MTD_OPS_AUTO_OOB, user should know how many
> > > > > > OOB available bytes it can use. But I didn't find a way to get it. (If
> > > > > > there is already a method, please kindly let me know, thanks.)    
> > > > > 
> > > > > Nope. We have the ECCGETLAYOUT ioctl, but it's not reliable because of
> > > > > the MTD_MAX_OOBFREE_ENTRIES limitation.
> > > > >     
> > > > > > 
> > > > > > One problem I know is to do Jffs2 type flash erase using MTD user-space
> > > > > > tool flash_erase. flash_erase tool will program "cleanmarker" into OOB
> > > > > > free area, but it can not get OOB available size.    
> > > > > 
> > > > > That's a good reason to expose this property indeed. Do you plan to fix
> > > > > mtd-utils to use the sysfs value when it's available? Also, do we need
> > > > > to expose the ECC/free layout as done with the
> > > > > ECCGETLAYOUT/MEMGETOOBSEL ioctls?
> > > > >     
> > > > Yes. I plan to fix mtd-utils problem if this change is accepted.
> > > > 
> > > > struct nand_oobinfo and struct nand_ecclayout_user in mtd-abi.h are
> > > > obsoleted, and user can not get oob layout by ioctl method now.
> > > > sysfs seems a method to fetch oob layout. But I don't know which
> > > > user-space case will use oob layout now. Maybe expose them in the future
> > > > if they are really needed?  
> > > 
> > > If the layout is not needed yet, let's not expose it. BTW, are you
> > > really using JFFS2 on modern NAND chips? Sounds like a bad idea to me.  
> > Yes. I tested Micron SLC NAND with 4K page size and 224 OOB size these
> > days. Jffs2 runs good on this NAND.
> 
> Yes, it should work, but I'm not sure JFFS2 scales well on >128MB
> NANDs, and people tend to use UBI on such devices nowadays (this + the
> fact that JFFS2 is not actively maintained).
Yes.
> 
> Anyway, I guess your change is acceptable, I'd just like to see
> patches for mtd-utils using this sysfs entry now ;-).
OK. Thanks. I will send mtd-utils patches ASAP.
> 
> Regards,
> 
> Boris

Re: [PATCH] mtd: Add sysfs attribute for mtd OOB available size

[Boris Brezillon]

On Mon, 2 Apr 2018 16:20:10 +0800
Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:

> Expose mtd OOB available size by sysfs file. Then users can get available
> OOB size by accessing /sys/class/mtd/mtdX/oobavail.
> 
> Signed-off-by: Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Applied.

Thanks,

Boris

> ---
>  Documentation/ABI/testing/sysfs-class-mtd |  8 ++++++++
>  drivers/mtd/mtdcore.c                     | 10 ++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-mtd b/Documentation/ABI/testing/sysfs-class-mtd
> index f34e5923..3bc7c0a 100644
> --- a/Documentation/ABI/testing/sysfs-class-mtd
> +++ b/Documentation/ABI/testing/sysfs-class-mtd
> @@ -232,3 +232,11 @@ Description:
>  		of the parent (another partition or a flash device) in bytes.
>  		This attribute is absent on flash devices, so it can be used
>  		to distinguish them from partitions.
> +
> +What:		/sys/class/mtd/mtdX/oobavail
> +Date:		April 2018
> +KernelVersion:	4.16
> +Contact:	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> +Description:
> +		Number of bytes available for a client to place data into
> +		the out of band area.
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 807d17d..99d9352 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -210,6 +210,15 @@ static ssize_t mtd_oobsize_show(struct device *dev,
>  }
>  static DEVICE_ATTR(oobsize, S_IRUGO, mtd_oobsize_show, NULL);
>  
> +static ssize_t mtd_oobavail_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct mtd_info *mtd = dev_get_drvdata(dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%lu\n", (unsigned long)mtd->oobavail);
> +}
> +static DEVICE_ATTR(oobavail, S_IRUGO, mtd_oobavail_show, NULL);
> +
>  static ssize_t mtd_numeraseregions_show(struct device *dev,
>  		struct device_attribute *attr, char *buf)
>  {
> @@ -327,6 +336,7 @@ static ssize_t mtd_bbtblocks_show(struct device *dev,
>  	&dev_attr_writesize.attr,
>  	&dev_attr_subpagesize.attr,
>  	&dev_attr_oobsize.attr,
> +	&dev_attr_oobavail.attr,
>  	&dev_attr_numeraseregions.attr,
>  	&dev_attr_name.attr,
>  	&dev_attr_ecc_strength.attr,