| Message ID | 1297424789-4144-1-git-send-email-agust@denx.de |
|---|---|
| State | New, archived |
| Headers | show |
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?
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
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.
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 */
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(-)