[PATCH 1/4] mtd: Add sysfs attr to expose ECC stats
Ezequiel Garcia
ezequiel.garcia at free-electrons.com
Tue Apr 1 04:13:59 PDT 2014
Hello Pekon,
On Mar 27, Gupta, Pekon wrote:
> >
> >+static ssize_t mtd_ecc_stats_show(struct device *dev,
> >+ struct device_attribute *attr, char *buf)
> >+{
> >+ struct mtd_info *mtd = dev_get_drvdata(dev);
> >+ struct mtd_ecc_stats *ecc_stats = &mtd->ecc_stats;
> >+
> >+ return snprintf(buf, PAGE_SIZE, "%8u %8u %8u %8u\n", ecc_stats->corrected,
> >+ ecc_stats->failed,
> >+ ecc_stats->badblocks,
> >+ ecc_stats->bbtblocks);
> Though I would have liked to see each field of ecc_stats as a separate sysfs entry
I tried to match the ecc_stats structure exactly in the sysfs entry. Keep
in mind creating/keeping a sysfs file has its cost. Since it's trivial
to get one of the fields with any text parsing tool I didn't think having
a file per stat was worth it.
> But, I don't know what it impacts to keep it different from /sys/block/<device>/stat
> Do you know any user-space tools which utilize these, and can be re-used on
> UBI-block kind of layer, if we keep this compatibility ?
>
It won't impact as they are two completely different things. The ecc_stats
output is only "vaguely inspired" in block stats; but these are two completely
different stats. It's not about tool reusability, but about consistency.
> Also, How about printing what these numbers mean ?
> I hope this will still keep it machine readable.
Well, this is not a debugfs entry, so I'm not sure we want to add such debug
information. Anyone can take a look at the code and see what ecc_stats mean.
On the other side, I don't have a strong opinion on this.
>
> Also please update "Documentation/ABI/testing/sysfs-class-mtd"
> with details about 'ecc_stats'
>
OK, I will.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
More information about the linux-mtd
mailing list