[PATCH v3 0/3] Improve MTD dynamic partitioning support

[PATCH v3 0/3] Improve MTD dynamic partitioning support

[Dan Ehrenberg]

The goal of this patchset is to make mtd dynamic partitioning work
better.
- First, a new config option is added to allow the master device to
  coexist with partitions, which allows partitions to be created or
  removed based on the master even if other partitions exist.
- Second, to identify partitions and improve visibility, 
- Third, remove overlap checks for partitions, which had a bug
  in them and don't seem necessary since static partitions don't
  have overlap checks.

This patchset has gone through a few iterations, initially based on
using one partition as a stand-in for the master, then adding the
master coexisting, and now in this iteration, bringing back the patch
to remove overlap checks, as testing discovered that the overlap
checks were buggy. This version also makes some style improvements.

Dan Ehrenberg (3):
  mtd: part: Create the master device node when partitioned
  mtd: part: Add sysfs variable for offset of partition
  mtd: part: Remove partition overlap checks

 drivers/mtd/Kconfig   | 13 +++++++++++
 drivers/mtd/mtdcore.c | 52 ++++++++++++++++++++++++++++-------------
 drivers/mtd/mtdpart.c | 64 ++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 92 insertions(+), 37 deletions(-)

-- 
2.2.0.rc0.207.ga3a616c

[PATCH v3 1/3] mtd: part: Create the master device node when partitioned

[Dan Ehrenberg]

For many use cases, it helps to have a device node for the entire
MTD device as well as device nodes for the individual partitions.
For example, this allows querying the entire device's properties.
A common idiom is to create an additional partition which spans
over the whole device.

This patch makes a config option, CONFIG_MTD_PARTITIONED_MASTER,
which makes the master partition present even when the device is
partitioned. This isn't turned on by default since it presents
a backwards-incompatible device numbering.

The patch also makes the parent of a partition device be the master,
if the config flag is set, now that the master is a full device.

Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
---
v3: Refactored the mtd_device_parse_register function to make it a bit cleaner

 drivers/mtd/Kconfig   | 13 +++++++++++++
 drivers/mtd/mtdcore.c | 52 +++++++++++++++++++++++++++++++++++----------------
 drivers/mtd/mtdpart.c | 18 +++++++++++++-----
 3 files changed, 62 insertions(+), 21 deletions(-)

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 71fea89..a03ad29 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -309,6 +309,19 @@ config MTD_SWAP
 	  The driver provides wear leveling by storing erase counter into the
 	  OOB.
 
+config MTD_PARTITIONED_MASTER
+	bool "Retain master device when partitioned"
+	default n
+	depends on MTD
+	help
+	  For historical reasons, by default, either a master is present or
+	  several partitions are present, but not both. The concern was that
+	  data listed in multiple partitions was dangerous; however, SCSI does
+	  this and it is frequently useful for applications. This config option
+	  leaves the master in even if the device is partitioned. It also makes
+	  the parent of the partition device be the master device, rather than
+	  what lies behind the master.
+
 source "drivers/mtd/chips/Kconfig"
 
 source "drivers/mtd/maps/Kconfig"
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 11883bd..d172195 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -38,6 +38,7 @@
 #include <linux/gfp.h>
 #include <linux/slab.h>
 #include <linux/reboot.h>
+#include <linux/kconfig.h>
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
@@ -501,6 +502,29 @@ out_error:
 	return ret;
 }
 
+static int mtd_add_device_partitions(struct mtd_info *mtd,
+				     struct mtd_partition *real_parts,
+				     int nbparts)
+{
+	int ret;
+
+	if (nbparts == 0 || IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER)) {
+		ret = add_mtd_device(mtd);
+		if (ret == 1)
+			return -ENODEV;
+	}
+
+	if (nbparts > 0) {
+		ret = add_mtd_partitions(mtd, real_parts, nbparts);
+		if (ret && IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
+			del_mtd_device(mtd);
+		return ret;
+	}
+
+	return 0;
+}
+
+
 /**
  * mtd_device_parse_register - parse partitions and register an MTD device.
  *
@@ -523,7 +547,8 @@ out_error:
  *   found this functions tries to fallback to information specified in
  *   @parts/@nr_parts.
  * * If any partitioning info was found, this function registers the found
- *   partitions.
+ *   partitions. If the MTD_PARTITIONED_MASTER option is set, then the device
+ *   as a whole is registered first.
  * * If no partitions were found this function just registers the MTD device
  *   @mtd and exits.
  *
@@ -534,27 +559,21 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
 			      const struct mtd_partition *parts,
 			      int nr_parts)
 {
-	int err;
-	struct mtd_partition *real_parts;
+	int ret;
+	struct mtd_partition *real_parts = NULL;
 
-	err = parse_mtd_partitions(mtd, types, &real_parts, parser_data);
-	if (err <= 0 && nr_parts && parts) {
+	ret = parse_mtd_partitions(mtd, types, &real_parts, parser_data);
+	if (ret <= 0 && nr_parts && parts) {
 		real_parts = kmemdup(parts, sizeof(*parts) * nr_parts,
 				     GFP_KERNEL);
 		if (!real_parts)
-			err = -ENOMEM;
+			ret = -ENOMEM;
 		else
-			err = nr_parts;
+			ret = nr_parts;
 	}
 
-	if (err > 0) {
-		err = add_mtd_partitions(mtd, real_parts, err);
-		kfree(real_parts);
-	} else if (err == 0) {
-		err = add_mtd_device(mtd);
-		if (err == 1)
-			err = -ENODEV;
-	}
+	if (ret >= 0)
+		ret = mtd_add_device_partitions(mtd, real_parts, ret);
 
 	/*
 	 * FIXME: some drivers unfortunately call this function more than once.
@@ -569,7 +588,8 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
 		register_reboot_notifier(&mtd->reboot_notifier);
 	}
 
-	return err;
+	kfree(real_parts);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(mtd_device_parse_register);
 
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index e779de3..a19ec5a 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -30,6 +30,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/err.h>
+#include <linux/kconfig.h>
 
 #include "mtdcore.h"
 
@@ -379,10 +380,17 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
 	slave->mtd.name = name;
 	slave->mtd.owner = master->owner;
 
-	/* NOTE:  we don't arrange MTDs as a tree; it'd be error-prone
-	 * to have the same data be in two different partitions.
+	/* NOTE: Historically, we didn't arrange MTDs as a tree out of
+	 * concern for showing the same data in multiple partitions.
+	 * However, it is very useful to have the master node present,
+	 * so the MTD_PARTITIONED_MASTER option allows that. The master
+	 * will have device nodes etc only if this is set, so make the
+	 * parent conditional on that option. Note, this is a way to
+	 * distinguish between the master and the partition in sysfs.
 	 */
-	slave->mtd.dev.parent = master->dev.parent;
+	slave->mtd.dev.parent = IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) ?
+				&master->dev :
+				master->dev.parent;
 
 	slave->mtd._read = part_read;
 	slave->mtd._write = part_write;
@@ -631,8 +639,8 @@ EXPORT_SYMBOL_GPL(mtd_del_partition);
  * and registers slave MTD objects which are bound to the master according to
  * the partition definitions.
  *
- * We don't register the master, or expect the caller to have done so,
- * for reasons of data integrity.
+ * For historical reasons, this function's caller only registers the master
+ * if the MTD_PARTITIONED_MASTER config option is set.
  */
 
 int add_mtd_partitions(struct mtd_info *master,
-- 
2.2.0.rc0.207.ga3a616c

[PATCH v3 2/3] mtd: part: Add sysfs variable for offset of partition

[Dan Ehrenberg]

This patch makes a sysfs variable called 'offset' on each partition
which contains the offset in bytes from the beginning of the master
device that the partition starts.

Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
---
 drivers/mtd/mtdpart.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index a19ec5a..b8ed202 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -554,6 +554,30 @@ out_register:
 	return slave;
 }
 
+static ssize_t mtd_partition_offset_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct mtd_info *mtd = dev_get_drvdata(dev);
+	struct mtd_part *part = PART(mtd);
+	return snprintf(buf, PAGE_SIZE, "%lld\n", part->offset);
+}
+
+static DEVICE_ATTR(offset, S_IRUGO, mtd_partition_offset_show, NULL);
+
+const static struct attribute *mtd_partition_attrs[] = {
+	&dev_attr_offset.attr,
+	NULL
+};
+
+int mtd_add_partition_attrs(struct mtd_part *new)
+{
+	int ret = sysfs_create_files(&new->mtd.dev.kobj, mtd_partition_attrs);
+	if (ret)
+		printk(KERN_WARNING
+		       "mtd: failed to create partition attrs, err=%d\n", ret);
+	return ret;
+}
+
 int mtd_add_partition(struct mtd_info *master, const char *name,
 		      long long offset, long long length)
 {
@@ -603,6 +627,8 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
 
 	add_mtd_device(&new->mtd);
 
+	mtd_add_partition_attrs(new);
+
 	return ret;
 err_inv:
 	mutex_unlock(&mtd_partitions_mutex);
@@ -620,6 +646,8 @@ int mtd_del_partition(struct mtd_info *master, int partno)
 	list_for_each_entry_safe(slave, next, &mtd_partitions, list)
 		if ((slave->master == master) &&
 		    (slave->mtd.index == partno)) {
+			sysfs_remove_files(&slave->mtd.dev.kobj,
+					   mtd_partition_attrs);
 			ret = del_mtd_device(&slave->mtd);
 			if (ret < 0)
 				break;
@@ -663,6 +691,7 @@ int add_mtd_partitions(struct mtd_info *master,
 		mutex_unlock(&mtd_partitions_mutex);
 
 		add_mtd_device(&slave->mtd);
+		mtd_add_partition_attrs(slave);
 
 		cur_offset = slave->offset + slave->mtd.size;
 	}
-- 
2.2.0.rc0.207.ga3a616c

[PATCH v3 3/3] mtd: part: Remove partition overlap checks

[Dan Ehrenberg]

This patch makes MTD dynamic partitioning more flexible by removing
overlap checks for dynamic partitions. I don't see any  particular
reason why overlapping dynamic partitions should be prohibited while
static partitions are allowed to overlap freely.

The checks previously had an off-by-one error, where 'end' should be
one less than what it is currently set at, and adding partitions out of
increasing order will fail. Disabling the checks resolves this issue.

Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
---
 drivers/mtd/mtdpart.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index b8ed202..f746a68 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -582,7 +582,7 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
 		      long long offset, long long length)
 {
 	struct mtd_partition part;
-	struct mtd_part *p, *new;
+	struct mtd_part *new;
 	uint64_t start, end;
 	int ret = 0;
 
@@ -611,17 +611,6 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
 	end = offset + length;
 
 	mutex_lock(&mtd_partitions_mutex);
-	list_for_each_entry(p, &mtd_partitions, list)
-		if (p->master == master) {
-			if ((start >= p->offset) &&
-			    (start < (p->offset + p->mtd.size)))
-				goto err_inv;
-
-			if ((end >= p->offset) &&
-			    (end < (p->offset + p->mtd.size)))
-				goto err_inv;
-		}
-
 	list_add(&new->list, &mtd_partitions);
 	mutex_unlock(&mtd_partitions_mutex);
 
@@ -630,10 +619,6 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
 	mtd_add_partition_attrs(new);
 
 	return ret;
-err_inv:
-	mutex_unlock(&mtd_partitions_mutex);
-	free_partition(new);
-	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(mtd_add_partition);
 
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH v3 2/3] mtd: part: Add sysfs variable for offset of partition

[Brian Norris]

On Fri, Mar 27, 2015 at 12:24:16PM -0700, Dan Ehrenberg wrote:
> This patch makes a sysfs variable called 'offset' on each partition
> which contains the offset in bytes from the beginning of the master
> device that the partition starts.
> 
> Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
> ---
>  drivers/mtd/mtdpart.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index a19ec5a..b8ed202 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -554,6 +554,30 @@ out_register:
>  	return slave;
>  }
>  
> +static ssize_t mtd_partition_offset_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct mtd_info *mtd = dev_get_drvdata(dev);
> +	struct mtd_part *part = PART(mtd);
> +	return snprintf(buf, PAGE_SIZE, "%lld\n", part->offset);
> +}
> +
> +static DEVICE_ATTR(offset, S_IRUGO, mtd_partition_offset_show, NULL);

You'll want #include <linux/device.h>, just to be explicit.

And if you're adding to the sysfs ABI, please update
Documentation/ABI/testing/sysfs-class-mtd

> +
> +const static struct attribute *mtd_partition_attrs[] = {

gcc complains:

  drivers/mtd/mtdpart.c:568:1: warning: 'static' is not at beginning of declaration [-Wold-style-declaration]

> +	&dev_attr_offset.attr,
> +	NULL
> +};
> +
> +int mtd_add_partition_attrs(struct mtd_part *new)

gcc and sprase complain:

  drivers/mtd/mtdpart.c:573:5: warning: no previous prototype for 'mtd_add_partition_attrs' [-Wmissing-prototypes]
  drivers/mtd/mtdpart.c:573:5: warning: symbol 'mtd_add_partition_attrs' was not declared. Should it be static? [sparse]

> +{
> +	int ret = sysfs_create_files(&new->mtd.dev.kobj, mtd_partition_attrs);
> +	if (ret)
> +		printk(KERN_WARNING
> +		       "mtd: failed to create partition attrs, err=%d\n", ret);
> +	return ret;
> +}
> +
>  int mtd_add_partition(struct mtd_info *master, const char *name,
>  		      long long offset, long long length)
>  {
> @@ -603,6 +627,8 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
>  
>  	add_mtd_device(&new->mtd);
>  
> +	mtd_add_partition_attrs(new);

Hmm, so you're adding the partition attribute only to dynamically
created partitions? I think we'll want it for all partitions. I know
the naming isn't completely straightforward, but some partitions are
added through add_mtd_device(), without calling mtd_add_partition().

> +
>  	return ret;
>  err_inv:
>  	mutex_unlock(&mtd_partitions_mutex);
> @@ -620,6 +646,8 @@ int mtd_del_partition(struct mtd_info *master, int partno)
>  	list_for_each_entry_safe(slave, next, &mtd_partitions, list)
>  		if ((slave->master == master) &&
>  		    (slave->mtd.index == partno)) {
> +			sysfs_remove_files(&slave->mtd.dev.kobj,
> +					   mtd_partition_attrs);
>  			ret = del_mtd_device(&slave->mtd);
>  			if (ret < 0)
>  				break;
> @@ -663,6 +691,7 @@ int add_mtd_partitions(struct mtd_info *master,
>  		mutex_unlock(&mtd_partitions_mutex);
>  
>  		add_mtd_device(&slave->mtd);
> +		mtd_add_partition_attrs(slave);
>  
>  		cur_offset = slave->offset + slave->mtd.size;
>  	}

Brian

Re: [PATCH v3 2/3] mtd: part: Add sysfs variable for offset of partition

[Daniel Ehrenberg]

On Fri, Mar 27, 2015 at 5:53 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Fri, Mar 27, 2015 at 12:24:16PM -0700, Dan Ehrenberg wrote:
>> This patch makes a sysfs variable called 'offset' on each partition
>> which contains the offset in bytes from the beginning of the master
>> device that the partition starts.
>>
>> Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
>> ---
>>  drivers/mtd/mtdpart.c | 29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>> index a19ec5a..b8ed202 100644
>> --- a/drivers/mtd/mtdpart.c
>> +++ b/drivers/mtd/mtdpart.c
>> @@ -554,6 +554,30 @@ out_register:
>>       return slave;
>>  }
>>
>> +static ssize_t mtd_partition_offset_show(struct device *dev,
>> +             struct device_attribute *attr, char *buf)
>> +{
>> +     struct mtd_info *mtd = dev_get_drvdata(dev);
>> +     struct mtd_part *part = PART(mtd);
>> +     return snprintf(buf, PAGE_SIZE, "%lld\n", part->offset);
>> +}
>> +
>> +static DEVICE_ATTR(offset, S_IRUGO, mtd_partition_offset_show, NULL);
>
> You'll want #include <linux/device.h>, just to be explicit.
>
> And if you're adding to the sysfs ABI, please update
> Documentation/ABI/testing/sysfs-class-mtd
>
>> +
>> +const static struct attribute *mtd_partition_attrs[] = {
>
> gcc complains:
>
>   drivers/mtd/mtdpart.c:568:1: warning: 'static' is not at beginning of declaration [-Wold-style-declaration]
>
>> +     &dev_attr_offset.attr,
>> +     NULL
>> +};
>> +
>> +int mtd_add_partition_attrs(struct mtd_part *new)
>
> gcc and sprase complain:
>
>   drivers/mtd/mtdpart.c:573:5: warning: no previous prototype for 'mtd_add_partition_attrs' [-Wmissing-prototypes]
>   drivers/mtd/mtdpart.c:573:5: warning: symbol 'mtd_add_partition_attrs' was not declared. Should it be static? [sparse]

Thanks, not sure why these warnings haven't been showing up for me
(while other warnings do, e.g., if I make an unused variable). I'll
fix this and the docs in a new patch.
>
>> +{
>> +     int ret = sysfs_create_files(&new->mtd.dev.kobj, mtd_partition_attrs);
>> +     if (ret)
>> +             printk(KERN_WARNING
>> +                    "mtd: failed to create partition attrs, err=%d\n", ret);
>> +     return ret;
>> +}
>> +
>>  int mtd_add_partition(struct mtd_info *master, const char *name,
>>                     long long offset, long long length)
>>  {
>> @@ -603,6 +627,8 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
>>
>>       add_mtd_device(&new->mtd);
>>
>> +     mtd_add_partition_attrs(new);
>
> Hmm, so you're adding the partition attribute only to dynamically
> created partitions? I think we'll want it for all partitions. I know
> the naming isn't completely straightforward, but some partitions are
> added through add_mtd_device(), without calling mtd_add_partition().

It's also called in add_mtd_partitions, as you can see just below. I
tested this with static partitions and the attribute was visible.
>
>> +
>>       return ret;
>>  err_inv:
>>       mutex_unlock(&mtd_partitions_mutex);
>> @@ -620,6 +646,8 @@ int mtd_del_partition(struct mtd_info *master, int partno)
>>       list_for_each_entry_safe(slave, next, &mtd_partitions, list)
>>               if ((slave->master == master) &&
>>                   (slave->mtd.index == partno)) {
>> +                     sysfs_remove_files(&slave->mtd.dev.kobj,
>> +                                        mtd_partition_attrs);
>>                       ret = del_mtd_device(&slave->mtd);
>>                       if (ret < 0)
>>                               break;
>> @@ -663,6 +691,7 @@ int add_mtd_partitions(struct mtd_info *master,
>>               mutex_unlock(&mtd_partitions_mutex);
>>
>>               add_mtd_device(&slave->mtd);
>> +             mtd_add_partition_attrs(slave);
>>
>>               cur_offset = slave->offset + slave->mtd.size;
>>       }
>
> Brian

Re: [PATCH v3 2/3] mtd: part: Add sysfs variable for offset of partition

[Brian Norris]

On Mon, Mar 30, 2015 at 01:32:21PM -0700, Daniel Ehrenberg wrote:
> On Fri, Mar 27, 2015 at 5:53 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> > On Fri, Mar 27, 2015 at 12:24:16PM -0700, Dan Ehrenberg wrote:
> >> This patch makes a sysfs variable called 'offset' on each partition
> >> which contains the offset in bytes from the beginning of the master
> >> device that the partition starts.
> >>
> >> Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
> >> ---
> >>  drivers/mtd/mtdpart.c | 29 +++++++++++++++++++++++++++++
> >>  1 file changed, 29 insertions(+)
> >>
> >> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> >> index a19ec5a..b8ed202 100644
> >> --- a/drivers/mtd/mtdpart.c
> >> +++ b/drivers/mtd/mtdpart.c
> >> @@ -554,6 +554,30 @@ out_register:
> >>       return slave;
> >>  }
> >>
> >> +static ssize_t mtd_partition_offset_show(struct device *dev,
> >> +             struct device_attribute *attr, char *buf)
> >> +{
> >> +     struct mtd_info *mtd = dev_get_drvdata(dev);
> >> +     struct mtd_part *part = PART(mtd);
> >> +     return snprintf(buf, PAGE_SIZE, "%lld\n", part->offset);
> >> +}
> >> +
> >> +static DEVICE_ATTR(offset, S_IRUGO, mtd_partition_offset_show, NULL);
> >
> > You'll want #include <linux/device.h>, just to be explicit.
> >
> > And if you're adding to the sysfs ABI, please update
> > Documentation/ABI/testing/sysfs-class-mtd
> >
> >> +
> >> +const static struct attribute *mtd_partition_attrs[] = {
> >
> > gcc complains:
> >
> >   drivers/mtd/mtdpart.c:568:1: warning: 'static' is not at beginning of declaration [-Wold-style-declaration]
> >
> >> +     &dev_attr_offset.attr,
> >> +     NULL
> >> +};
> >> +
> >> +int mtd_add_partition_attrs(struct mtd_part *new)
> >
> > gcc and sprase complain:
> >
> >   drivers/mtd/mtdpart.c:573:5: warning: no previous prototype for 'mtd_add_partition_attrs' [-Wmissing-prototypes]
> >   drivers/mtd/mtdpart.c:573:5: warning: symbol 'mtd_add_partition_attrs' was not declared. Should it be static? [sparse]
> 
> Thanks, not sure why these warnings haven't been showing up for me
> (while other warnings do, e.g., if I make an unused variable). I'll
> fix this and the docs in a new patch.

sparse is not run by default, and some of the gcc warnings might only be
available on newer gcc's and/or when building with extra warnings
enabled.

To build with extra warnings, you can use 'make W=1' or 'make W=2'. To
build with sparse checking you can use 'make C=1' or 'make C=2'. See
Makefile for details.

> >> +{
> >> +     int ret = sysfs_create_files(&new->mtd.dev.kobj, mtd_partition_attrs);
> >> +     if (ret)
> >> +             printk(KERN_WARNING
> >> +                    "mtd: failed to create partition attrs, err=%d\n", ret);
> >> +     return ret;
> >> +}
> >> +
> >>  int mtd_add_partition(struct mtd_info *master, const char *name,
> >>                     long long offset, long long length)
> >>  {
> >> @@ -603,6 +627,8 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
> >>
> >>       add_mtd_device(&new->mtd);
> >>
> >> +     mtd_add_partition_attrs(new);
> >
> > Hmm, so you're adding the partition attribute only to dynamically
> > created partitions? I think we'll want it for all partitions. I know
> > the naming isn't completely straightforward, but some partitions are
> > added through add_mtd_device(), without calling mtd_add_partition().
> 
> It's also called in add_mtd_partitions, as you can see just below. I
> tested this with static partitions and the attribute was visible.

Sorry, must have overlooked this.

> >
> >> +
> >>       return ret;
> >>  err_inv:
> >>       mutex_unlock(&mtd_partitions_mutex);
> >> @@ -620,6 +646,8 @@ int mtd_del_partition(struct mtd_info *master, int partno)
> >>       list_for_each_entry_safe(slave, next, &mtd_partitions, list)
> >>               if ((slave->master == master) &&
> >>                   (slave->mtd.index == partno)) {
> >> +                     sysfs_remove_files(&slave->mtd.dev.kobj,
> >> +                                        mtd_partition_attrs);
> >>                       ret = del_mtd_device(&slave->mtd);
> >>                       if (ret < 0)
> >>                               break;
> >> @@ -663,6 +691,7 @@ int add_mtd_partitions(struct mtd_info *master,
> >>               mutex_unlock(&mtd_partitions_mutex);
> >>
> >>               add_mtd_device(&slave->mtd);
> >> +             mtd_add_partition_attrs(slave);
> >>
> >>               cur_offset = slave->offset + slave->mtd.size;

Brian