[PATCH] mtd: export mtd->writebufsize attribute over sysfs

[PATCH] mtd: export mtd->writebufsize attribute over sysfs

[Anatolij Gustschin]

Also allow to change mtd->writebufsize for MTD ram test
device dynamicaly.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/mtd/mtdcore.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index b177e75..282ad41 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -230,12 +230,40 @@ static ssize_t mtd_name_show(struct device *dev,
 }
 static DEVICE_ATTR(name, S_IRUGO, mtd_name_show, NULL);
 
+static ssize_t mtd_writebufsize_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct mtd_info *mtd = dev_to_mtd(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%lu\n",
+			(unsigned long)mtd->writebufsize);
+}
+
+static ssize_t mtd_writebufsize_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t size)
+{
+	struct mtd_info *mtd = dev_to_mtd(dev);
+	unsigned long value;
+	int ret;
+
+	ret = strict_strtoul(buf, 0, &value);
+	if (ret < 0)
+		return ret;
+
+	mtd->writebufsize = value;
+	return size;
+}
+static DEVICE_ATTR(writebufsize, S_IRUGO, mtd_writebufsize_show,
+		   mtd_writebufsize_store);
+
 static struct attribute *mtd_attrs[] = {
 	&dev_attr_type.attr,
 	&dev_attr_flags.attr,
 	&dev_attr_size.attr,
 	&dev_attr_erasesize.attr,
 	&dev_attr_writesize.attr,
+	&dev_attr_writebufsize.attr,
 	&dev_attr_subpagesize.attr,
 	&dev_attr_oobsize.attr,
 	&dev_attr_numeraseregions.attr,
@@ -258,6 +286,28 @@ static struct device_type mtd_devtype = {
 	.release	= mtd_release,
 };
 
+/*
+ * Make attributes of the mtdram test device writable.
+ * Useful for testing different UBIFS images.
+ */
+static inline void mtd_fixup_attr(struct mtd_info *mtd)
+{
+	struct attribute *attr = mtd_attrs[0];
+	int i = 0;
+
+	if (mtd->type != MTD_RAM)
+		return;
+
+	for (i = 0; mtd_attrs[i]; i++) {
+		attr = mtd_attrs[i];
+		if (!strcmp(attr->name, "writebufsize")) {
+			sysfs_chmod_file(&mtd->dev.kobj, attr,
+					 S_IRUGO | S_IWUSR | S_IWGRP);
+			break;
+		}
+	}
+}
+
 /**
  *	add_mtd_device - register an MTD device
  *	@mtd: pointer to new MTD device info structure
@@ -339,6 +389,8 @@ int add_mtd_device(struct mtd_info *mtd)
 						MTD_DEVT(i) + 1,
 						NULL, "mtd%dro", i);
 
+			mtd_fixup_attr(mtd);
+
 			DEBUG(0, "mtd: Giving out device %d to %s\n",i, mtd->name);
 			/* No need to get a refcount on the module containing
 			   the notifier, since we hold the mtd_table_mutex */
-- 
1.7.4

Re: [PATCH] mtd: export mtd->writebufsize attribute over sysfs

[Artem Bityutskiy]

On Fri, 2011-02-11 at 12:46 +0100, Anatolij Gustschin wrote:
> +static ssize_t mtd_writebufsize_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t size)
> +{
> +	struct mtd_info *mtd = dev_to_mtd(dev);
> +	unsigned long value;
> +	int ret;
> +
> +	ret = strict_strtoul(buf, 0, &value);
> +	if (ret < 0)
> +		return ret;
> +
> +	mtd->writebufsize = value;
> +	return size;
> +}
> +static DEVICE_ATTR(writebufsize, S_IRUGO, mtd_writebufsize_show,
> +		   mtd_writebufsize_store);


I think writebufsize should be read-only. This is characteristic of the
flash and should not be changed. OK, if the chip allows to change it, it
could be done, but this is not what the patch seems to be about.

I mean, writebufsize is like writesize. Why writesize sysfs attribut is
R/O but writebufsize should be RW?

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

Re: [PATCH] mtd: export mtd->writebufsize attribute over sysfs

[Anatolij Gustschin]

On Fri, 11 Feb 2011 17:09:54 +0200
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Fri, 2011-02-11 at 12:46 +0100, Anatolij Gustschin wrote:
> > +static ssize_t mtd_writebufsize_store(struct device *dev,
> > +				struct device_attribute *attr,
> > +				const char *buf, size_t size)
> > +{
> > +	struct mtd_info *mtd = dev_to_mtd(dev);
> > +	unsigned long value;
> > +	int ret;
> > +
> > +	ret = strict_strtoul(buf, 0, &value);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	mtd->writebufsize = value;
> > +	return size;
> > +}
> > +static DEVICE_ATTR(writebufsize, S_IRUGO, mtd_writebufsize_show,
> > +		   mtd_writebufsize_store);
> 
> 
> I think writebufsize should be read-only. This is characteristic of the
> flash and should not be changed. OK, if the chip allows to change it, it
> could be done, but this is not what the patch seems to be about.

Yes, it is read-only by default.

> I mean, writebufsize is like writesize. Why writesize sysfs attribut is
> R/O but writebufsize should be RW?

This attribute is R/O when the appropriate file is created. For
mtd-ram test device it will be set RW by the fixup routine. I used
this to set the writebufsize to different values when working with
different UBIFS images. If I can set it dynamically, I do not need
to recompile the mtdram module or the kernel.

Anatolij

Re: [PATCH] mtd: export mtd->writebufsize attribute over sysfs

[Artem Bityutskiy]

On Fri, 2011-02-11 at 16:35 +0100, Anatolij Gustschin wrote:
> On Fri, 11 Feb 2011 17:09:54 +0200
> Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 
> > On Fri, 2011-02-11 at 12:46 +0100, Anatolij Gustschin wrote:
> > > +static ssize_t mtd_writebufsize_store(struct device *dev,
> > > +				struct device_attribute *attr,
> > > +				const char *buf, size_t size)
> > > +{
> > > +	struct mtd_info *mtd = dev_to_mtd(dev);
> > > +	unsigned long value;
> > > +	int ret;
> > > +
> > > +	ret = strict_strtoul(buf, 0, &value);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	mtd->writebufsize = value;
> > > +	return size;
> > > +}
> > > +static DEVICE_ATTR(writebufsize, S_IRUGO, mtd_writebufsize_show,
> > > +		   mtd_writebufsize_store);
> > 
> > 
> > I think writebufsize should be read-only. This is characteristic of the
> > flash and should not be changed. OK, if the chip allows to change it, it
> > could be done, but this is not what the patch seems to be about.
> 
> Yes, it is read-only by default.
> 
> > I mean, writebufsize is like writesize. Why writesize sysfs attribut is
> > R/O but writebufsize should be RW?
> 
> This attribute is R/O when the appropriate file is created. For
> mtd-ram test device it will be set RW by the fixup routine. I used
> this to set the writebufsize to different values when working with
> different UBIFS images. If I can set it dynamically, I do not need
> to recompile the mtdram module or the kernel.

But this is bad architecture, you are putting policy into the kernel and
hard-coding device-type specific checks with things like:

+                       sysfs_chmod_file(&mtd->dev.kobj, attr,
+                                        S_IRUGO | S_IWUSR | S_IWGRP);

and

+       if (mtd->type != MTD_RAM)
+               return;

If you are doing something mtdram-specific, make it mtdram-specific. How
about this: add the "writebufsize" mtdram module parameter, then you
will have a /sys/modules/mtdram/parameters/writebufsize file, which you
then can change run-time.

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