[PATCH 00/10] new partitioning helper

[PATCH 00/10] new partitioning helper

[Uwe Kleine-König]

Hello,

this is a follow up on my patch series "Introduce devfs_append_partition
and a few users" implementing the (a bit cryptic) suggestion by
Jean-Christophe PLAGNIOL-VILLARD to use an array of structs to specify
the partitions to be created.

It makes creating partitions use more lines of code, but in return the
new call is better understandable without checking its prototype and it
supports creating partitions that start directly at the end of the
previous one without the need to explicitly calculate its offset.

This also includes a few fixes and cleanups.

Best regards
Uwe

Uwe Kleine-König (10):
  devfs: partitioning: add missing free in error path
  devfs_add_partition: make flags parameter unsigned
  Documentation: fix example call to devfs_add_partition
  mtd/nand: constify filename parameter
  devfs: partitioning: add new helper devfs_create_partitions
  ARM: a9m2410: convert to devfs_create_partitions
  ARM: freescale-mx35-3-stack: convert to devfs_create_partitions
  ARM: pca100: convert to devfs_create_partitions
  ARM: pcm038: convert to devfs_create_partitions
  ARM: sama5d3xek: convert to devfs_create_partitions

 Documentation/porting.txt                       |  2 +-
 arch/arm/boards/a9m2410/a9m2410.c               | 23 ++++--
 arch/arm/boards/freescale-mx35-3-stack/3stack.c | 36 +++++++--
 arch/arm/boards/pcm038/pcm038.c                 | 41 ++++++++---
 arch/arm/boards/phycard-i.MX27/pca100.c         | 23 ++++--
 arch/arm/boards/sama5d3xek/init.c               | 38 ++++++++--
 drivers/mtd/nand/nand-bb.c                      |  2 +-
 fs/devfs-core.c                                 | 97 +++++++++++++++++++++----
 include/driver.h                                | 41 ++++++++++-
 include/nand.h                                  |  4 +-
 10 files changed, 246 insertions(+), 61 deletions(-)

-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 01/10] devfs: partitioning: add missing free in error path

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 fs/devfs-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 44f0169e6324..5a120c64879a 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -292,6 +292,7 @@ struct cdev *devfs_add_partition(const char *devname, loff_t offset, loff_t size
 		new->mtd = mtd_add_partition(cdev->mtd, offset, size, flags, name);
 		if (IS_ERR(new->mtd)) {
 			int ret = PTR_ERR(new->mtd);
+			free(new->partname);
 			free(new);
 			return ERR_PTR(ret);
 		}
-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 02/10] devfs_add_partition: make flags parameter unsigned

[Uwe Kleine-König]

The value is only used to assign to a (*struct cdev)->flags which is an
unsigned int and it is passed as fourth parameter of mtd_add_partition which
is an unsigned long.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 fs/devfs-core.c  | 2 +-
 include/driver.h | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 5a120c64879a..f92a07c43d3e 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -261,7 +261,7 @@ int devfs_remove(struct cdev *cdev)
 }
 
 struct cdev *devfs_add_partition(const char *devname, loff_t offset, loff_t size,
-		int flags, const char *name)
+		unsigned int flags, const char *name)
 {
 	struct cdev *cdev, *new;
 
diff --git a/include/driver.h b/include/driver.h
index bbe789b51ead..33b82c3e969b 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -482,13 +482,13 @@ ssize_t cdev_write(struct cdev *cdev, const void *buf, size_t count, loff_t offs
 int cdev_ioctl(struct cdev *cdev, int cmd, void *buf);
 int cdev_erase(struct cdev *cdev, size_t count, loff_t offset);
 
-#define DEVFS_PARTITION_FIXED		(1 << 0)
-#define DEVFS_PARTITION_READONLY	(1 << 1)
+#define DEVFS_PARTITION_FIXED		(1U << 0)
+#define DEVFS_PARTITION_READONLY	(1U << 1)
 #define DEVFS_IS_PARTITION		(1 << 2)
 #define DEVFS_IS_CHARACTER_DEV		(1 << 3)
 
-struct cdev *devfs_add_partition(const char *devname, loff_t offset, loff_t size,
-		int flags, const char *name);
+struct cdev *devfs_add_partition(const char *devname, loff_t offset,
+		loff_t size, unsigned int flags, const char *name);
 int devfs_del_partition(const char *name);
 
 #define DRV_OF_COMPAT(compat) \
-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 03/10] Documentation: fix example call to devfs_add_partition

[Uwe Kleine-König]

The flags parameter was skipped in the example. Add it as
DEVFS_PARTITION_FIXED.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 Documentation/porting.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/porting.txt b/Documentation/porting.txt
index a350e5e5fe38..bd807a318607 100644
--- a/Documentation/porting.txt
+++ b/Documentation/porting.txt
@@ -50,7 +50,7 @@ extra-y += barebox.lds
   is not ported yet.
 
 - Call devfs_add_partition() to add an environment partition for your device:
-  devfs_add_partition("nor0", 0x40000, 0x20000, "env0");
+  devfs_add_partition("nor0", 0x40000, 0x20000, DEVFS_PARTITION_FIXED, "env0");
   This will add an area starting at 0x40000 of size 0x20000 of the device
   nor0 as env0.
 
-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 04/10] mtd/nand: constify filename parameter

[Uwe Kleine-König]

The string pointed to isn't modified, so declare it as const.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mtd/nand/nand-bb.c | 2 +-
 include/nand.h             | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/nand-bb.c b/drivers/mtd/nand/nand-bb.c
index 0292f2f426c0..f387ef687a9b 100644
--- a/drivers/mtd/nand/nand-bb.c
+++ b/drivers/mtd/nand/nand-bb.c
@@ -261,7 +261,7 @@ static LIST_HEAD(bb_list);
  * @param[in] name Partition name (can be obtained with devinfo command)
  * @return The device representing the new partition.
  */
-int dev_add_bb_dev(char *path, const char *name)
+int dev_add_bb_dev(const char *path, const char *name)
 {
 	struct nand_bb *bb;
 	int ret = -ENOMEM;
diff --git a/include/nand.h b/include/nand.h
index b1762dfa4591..a0e77cc8cde0 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -5,10 +5,10 @@
 struct nand_bb;
 
 #ifdef CONFIG_NAND
-int dev_add_bb_dev(char *filename, const char *name);
+int dev_add_bb_dev(const char *filename, const char *name);
 int dev_remove_bb_dev(const char *name);
 #else
-static inline int dev_add_bb_dev(char *filename, const char *name) {
+static inline int dev_add_bb_dev(const char *filename, const char *name) {
 	return 0;
 }
 static inline int dev_remove_bb_dev(const char *name)
-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 05/10] devfs: partitioning: add new helper devfs_create_partitions

[Uwe Kleine-König]

Compared to devfs_add_partition which adds a single partition
devfs_create_partitions creates several partitions at once. One nice
benefit is that this simplifies appending partitions because the start
of the latter partition doesn't need to be specified explicitly.
Also dev_add_bb_dev() is called by the new helper if the bbname is
specified for a partition.

Note that adding partitions is also more flexible now (also via
devfs_add_partition) because negative values for offset and size now
have a proper meaning instead of creating broken partitions.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 fs/devfs-core.c  | 96 ++++++++++++++++++++++++++++++++++++++++++++++----------
 include/driver.h | 33 +++++++++++++++++++
 2 files changed, 113 insertions(+), 16 deletions(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index f92a07c43d3e..237e13a93172 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -22,6 +22,7 @@
 #include <errno.h>
 #include <malloc.h>
 #include <ioctl.h>
+#include <nand.h>
 #include <linux/err.h>
 #include <linux/mtd/mtd.h>
 
@@ -260,32 +261,52 @@ int devfs_remove(struct cdev *cdev)
 	return 0;
 }
 
-struct cdev *devfs_add_partition(const char *devname, loff_t offset, loff_t size,
-		unsigned int flags, const char *name)
+static struct cdev *__devfs_add_partition(struct cdev *cdev,
+		const struct devfs_partition *partinfo, loff_t *end)
 {
-	struct cdev *cdev, *new;
+	loff_t offset, size;
+	static struct cdev *new;
 
-	cdev = cdev_by_name(name);
-	if (cdev)
+	if (cdev_by_name(partinfo->name))
 		return ERR_PTR(-EEXIST);
 
-	cdev = cdev_by_name(devname);
-	if (!cdev)
-		return ERR_PTR(-ENOENT);
-
-	if (offset + size > cdev->size)
+	if (partinfo->offset > 0)
+		offset = partinfo->offset;
+	else if (partinfo->offset == 0)
+		/* append to previous partition */
+		offset = *end;
+	else
+		/* relative to end of cdev */
+		offset = cdev->size + partinfo->offset;
+
+	if (partinfo->size > 0)
+		size = partinfo->size;
+	else
+		size = cdev->size + partinfo->size - offset;
+
+	if (offset >= 0 && offset < *end)
+		pr_debug("partition %s not after previous partition\n",
+				partinfo->name);
+
+	*end = offset + size;
+
+	if (offset < 0 || *end > cdev->size) {
+		pr_warn("partition %s not completely inside device %s\n",
+				partinfo->name, cdev->name);
 		return ERR_PTR(-EINVAL);
+	}
 
-	new = xzalloc(sizeof (*new));
-	new->name = strdup(name);
-	if (!strncmp(devname, name, strlen(devname)))
-		new->partname = xstrdup(name + strlen(devname) + 1);
+	new = xzalloc(sizeof(*new));
+	new->name = strdup(partinfo->name);
+	if (!strncmp(cdev->name, partinfo->name, strlen(cdev->name)))
+		new->partname = xstrdup(partinfo->name + strlen(cdev->name) + 1);
 	new->ops = cdev->ops;
 	new->priv = cdev->priv;
 	new->size = size;
-	new->offset = offset + cdev->offset;
+	new->offset = cdev->offset + offset;
+
 	new->dev = cdev->dev;
-	new->flags = flags | DEVFS_IS_PARTITION;
+	new->flags = partinfo->flags | DEVFS_IS_PARTITION;
 
 #ifdef CONFIG_PARTITION_NEED_MTD
 	if (cdev->mtd) {
@@ -304,6 +325,25 @@ struct cdev *devfs_add_partition(const char *devname, loff_t offset, loff_t size
 	return new;
 }
 
+struct cdev *devfs_add_partition(const char *devname, loff_t offset,
+		loff_t size, unsigned int flags, const char *name)
+{
+	struct cdev *cdev;
+	loff_t end = 0;
+	const struct devfs_partition partinfo = {
+		.offset = offset,
+		.size = size,
+		.flags = flags,
+		.name = name,
+	};
+
+	cdev = cdev_by_name(devname);
+	if (!cdev)
+		return ERR_PTR(-ENOENT);
+
+	return __devfs_add_partition(cdev, &partinfo, &end);
+}
+
 int devfs_del_partition(const char *name)
 {
 	struct cdev *cdev;
@@ -333,3 +373,27 @@ int devfs_del_partition(const char *name)
 
 	return 0;
 }
+
+int devfs_create_partitions(const char *devname,
+		const struct devfs_partition partinfo[])
+{
+	loff_t offset = 0;
+	struct cdev *cdev;
+
+	cdev = cdev_by_name(devname);
+	if (!cdev)
+		return -ENOENT;
+
+	for (; partinfo->name; ++partinfo) {
+		struct cdev *new;
+
+		new = __devfs_add_partition(cdev, partinfo, &offset);
+		if (IS_ERR(new))
+			return PTR_ERR(new);
+
+		if (partinfo->bbname)
+			dev_add_bb_dev(partinfo->name, partinfo->bbname);
+	}
+
+	return 0;
+}
diff --git a/include/driver.h b/include/driver.h
index 33b82c3e969b..31bdecf6d82e 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -491,6 +491,39 @@ struct cdev *devfs_add_partition(const char *devname, loff_t offset,
 		loff_t size, unsigned int flags, const char *name);
 int devfs_del_partition(const char *name);
 
+#define DEVFS_PARTITION_APPEND		0
+
+/**
+ * struct devfs_partition - defines parameters for a single partition
+ * @offset: start of partition
+ * 	a negative offset requests to start the partition relative to the
+ * 	device's end. DEVFS_PARTITION_APPEND (i.e. 0) means start directly at
+ * 	the end of the previous partition.
+ * @size: size of partition
+ * 	a non-positive value requests to use a size that keeps -size free space
+ * 	after the current partition. A special case of this is passing 0, which
+ * 	means "until end of device".
+ * @flags: flags passed to devfs_add_partition
+ * @name: name passed to devfs_add_partition
+ * @bbname: if non-NULL also dev_add_bb_dev() is called for the partition during
+ * 	devfs_create_partitions().
+ */
+struct devfs_partition {
+	loff_t offset;
+	loff_t size;
+	unsigned int flags;
+	const char *name;
+	const char *bbname;
+};
+/**
+ * devfs_create_partitions - create a set of partitions for a device
+ * @devname: name of the device to partition
+ * @partinfo: array of partition parameters
+ * 	The array is processed until an entry with .name = NULL is found.
+ */
+int devfs_create_partitions(const char *devname,
+		const struct devfs_partition partinfo[]);
+
 #define DRV_OF_COMPAT(compat) \
 	IS_ENABLED(CONFIG_OFDEVICE) ? (compat) : NULL
 
-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 06/10] ARM: a9m2410: convert to devfs_create_partitions

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/arm/boards/a9m2410/a9m2410.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boards/a9m2410/a9m2410.c b/arch/arm/boards/a9m2410/a9m2410.c
index b2b6c87117a3..8d528cf60378 100644
--- a/arch/arm/boards/a9m2410/a9m2410.c
+++ b/arch/arm/boards/a9m2410/a9m2410.c
@@ -117,13 +117,24 @@ static int a9m2410_devices_init(void)
 			16, IORESOURCE_MEM, NULL);
 
 #ifdef CONFIG_NAND
-	/* ----------- add some vital partitions -------- */
-	devfs_add_partition("nand0", 0x00000, 0x40000, DEVFS_PARTITION_FIXED, "self_raw");
-	dev_add_bb_dev("self_raw", "self0");
-
-	devfs_add_partition("nand0", 0x40000, 0x20000, DEVFS_PARTITION_FIXED, "env_raw");
-	dev_add_bb_dev("env_raw", "env0");
+	devfs_create_partitions("nand0", (struct devfs_partition[]){
+			{
+				.offset = 0,
+				.size = 0x40000,
+				.flags = DEVFS_PARTITION_FIXED,
+				.name = "self_raw",
+				.bbname = "self0",
+			}, {
+				.offset = DEVFS_PARTITION_APPEND,
+				.size = 0x20000,
+				.flags = DEVFS_PARTITION_FIXED,
+				.name = "env_raw",
+				.bbname = "env0",
+			}, {
+				/* sentinel (detected by .name = NULL) */
+			}});
 #endif
+
 	armlinux_set_architecture(MACH_TYPE_A9M2410);
 
 	return 0;
-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 06/10] ARM: a9m2410: convert to devfs_create_partitions

[Sebastian Hesselbarth]

On 02/25/2014 11:51 PM, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>   arch/arm/boards/a9m2410/a9m2410.c | 23 +++++++++++++++++------
>   1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/boards/a9m2410/a9m2410.c b/arch/arm/boards/a9m2410/a9m2410.c
> index b2b6c87117a3..8d528cf60378 100644
> --- a/arch/arm/boards/a9m2410/a9m2410.c
> +++ b/arch/arm/boards/a9m2410/a9m2410.c
> @@ -117,13 +117,24 @@ static int a9m2410_devices_init(void)
>   			16, IORESOURCE_MEM, NULL);
>
>   #ifdef CONFIG_NAND
> -	/* ----------- add some vital partitions -------- */
> -	devfs_add_partition("nand0", 0x00000, 0x40000, DEVFS_PARTITION_FIXED, "self_raw");
> -	dev_add_bb_dev("self_raw", "self0");
> -
> -	devfs_add_partition("nand0", 0x40000, 0x20000, DEVFS_PARTITION_FIXED, "env_raw");
> -	dev_add_bb_dev("env_raw", "env0");
> +	devfs_create_partitions("nand0", (struct devfs_partition[]){

nit: It would be even more readable, if you move the struct
devfs_partition[] out of a9m2410_device_init() and reference
it here instead, i.e.

static struct devfs_partition a9m2410_nand_partitions[] = {
	...
	{ }
};

...

static int a9m2410_devices_init(void)
{
...
	#ifdef CONFIG_NAND
	devfs_create_partitions("nand0", a9m2410_nand_partitions);
	#endif
...

in here and the following patches.

Sebastian

> +			{
> +				.offset = 0,
> +				.size = 0x40000,
> +				.flags = DEVFS_PARTITION_FIXED,
> +				.name = "self_raw",
> +				.bbname = "self0",
> +			}, {
> +				.offset = DEVFS_PARTITION_APPEND,
> +				.size = 0x20000,
> +				.flags = DEVFS_PARTITION_FIXED,
> +				.name = "env_raw",
> +				.bbname = "env0",
> +			}, {
> +				/* sentinel (detected by .name = NULL) */
> +			}});
>   #endif
> +
>   	armlinux_set_architecture(MACH_TYPE_A9M2410);
>
>   	return 0;
>


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 06/10] ARM: a9m2410: convert to devfs_create_partitions

[Uwe Kleine-König]

On Wed, Feb 26, 2014 at 12:25:20AM +0100, Sebastian Hesselbarth wrote:
> On 02/25/2014 11:51 PM, Uwe Kleine-König wrote:
> >Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >---
> >  arch/arm/boards/a9m2410/a9m2410.c | 23 +++++++++++++++++------
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> >
> >diff --git a/arch/arm/boards/a9m2410/a9m2410.c b/arch/arm/boards/a9m2410/a9m2410.c
> >index b2b6c87117a3..8d528cf60378 100644
> >--- a/arch/arm/boards/a9m2410/a9m2410.c
> >+++ b/arch/arm/boards/a9m2410/a9m2410.c
> >@@ -117,13 +117,24 @@ static int a9m2410_devices_init(void)
> >  			16, IORESOURCE_MEM, NULL);
> >
> >  #ifdef CONFIG_NAND
> >-	/* ----------- add some vital partitions -------- */
> >-	devfs_add_partition("nand0", 0x00000, 0x40000, DEVFS_PARTITION_FIXED, "self_raw");
> >-	dev_add_bb_dev("self_raw", "self0");
> >-
> >-	devfs_add_partition("nand0", 0x40000, 0x20000, DEVFS_PARTITION_FIXED, "env_raw");
> >-	dev_add_bb_dev("env_raw", "env0");
> >+	devfs_create_partitions("nand0", (struct devfs_partition[]){
> 
> nit: It would be even more readable, if you move the struct
> devfs_partition[] out of a9m2410_device_init() and reference
> it here instead, i.e.
> 
> static struct devfs_partition a9m2410_nand_partitions[] = {
> 	...
> 	{ }
> };
nit: I'd add "const" here. And you'd need something to not let the
compiler generate a "a9m2410_nand_partitions not used" warning if
CONFIG_NAND is disabled. Another (related) upside of using compound
literals as I did is that "nand0" and the respecitve partition array is
to be found at the same place.

But having said that I don't really care how the boards are converted.
If you want your approach, fine for me. (Ideally send a patch yourself,
you can get my Ack then :-)

Best regards
Uwe

> ...
> 
> static int a9m2410_devices_init(void)
> {
> ...
> 	#ifdef CONFIG_NAND
> 	devfs_create_partitions("nand0", a9m2410_nand_partitions);
> 	#endif
> ...
> 
> in here and the following patches.
> 
> Sebastian
> 
> >+			{
> >+				.offset = 0,
> >+				.size = 0x40000,
> >+				.flags = DEVFS_PARTITION_FIXED,
> >+				.name = "self_raw",
> >+				.bbname = "self0",
> >+			}, {
> >+				.offset = DEVFS_PARTITION_APPEND,
> >+				.size = 0x20000,
> >+				.flags = DEVFS_PARTITION_FIXED,
> >+				.name = "env_raw",
> >+				.bbname = "env0",
> >+			}, {
> >+				/* sentinel (detected by .name = NULL) */
> >+			}});
> >  #endif
> >+
> >  	armlinux_set_architecture(MACH_TYPE_A9M2410);
> >
> >  	return 0;
> >
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 06/10] ARM: a9m2410: convert to devfs_create_partitions

[Sebastian Hesselbarth]

On 02/26/14 16:55, Uwe Kleine-König wrote:
> On Wed, Feb 26, 2014 at 12:25:20AM +0100, Sebastian Hesselbarth wrote:
>> On 02/25/2014 11:51 PM, Uwe Kleine-König wrote:
>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>> ---
>>>   arch/arm/boards/a9m2410/a9m2410.c | 23 +++++++++++++++++------
>>>   1 file changed, 17 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/arm/boards/a9m2410/a9m2410.c b/arch/arm/boards/a9m2410/a9m2410.c
>>> index b2b6c87117a3..8d528cf60378 100644
>>> --- a/arch/arm/boards/a9m2410/a9m2410.c
>>> +++ b/arch/arm/boards/a9m2410/a9m2410.c
>>> @@ -117,13 +117,24 @@ static int a9m2410_devices_init(void)
>>>   			16, IORESOURCE_MEM, NULL);
>>>
>>>   #ifdef CONFIG_NAND
>>> -	/* ----------- add some vital partitions -------- */
>>> -	devfs_add_partition("nand0", 0x00000, 0x40000, DEVFS_PARTITION_FIXED, "self_raw");
>>> -	dev_add_bb_dev("self_raw", "self0");
>>> -
>>> -	devfs_add_partition("nand0", 0x40000, 0x20000, DEVFS_PARTITION_FIXED, "env_raw");
>>> -	dev_add_bb_dev("env_raw", "env0");
>>> +	devfs_create_partitions("nand0", (struct devfs_partition[]){
>>
>> nit: It would be even more readable, if you move the struct
>> devfs_partition[] out of a9m2410_device_init() and reference
>> it here instead, i.e.
>>
>> static struct devfs_partition a9m2410_nand_partitions[] = {
>> 	...
>> 	{ }
>> };
> nit: I'd add "const" here. And you'd need something to not let the
> compiler generate a "a9m2410_nand_partitions not used" warning if

static const __maybe_unused devfs_partition a9m2410_nand_partitions[] ..

> CONFIG_NAND is disabled. Another (related) upside of using compound
> literals as I did is that "nand0" and the respecitve partition array is
> to be found at the same place.

You talk about "readability" in the cover letter. From that point of
view, moving it outside the code section, makes it _more_ readable, not
less.

> But having said that I don't really care how the boards are converted.
> If you want your approach, fine for me. (Ideally send a patch yourself,
> you can get my Ack then :-)

I don't care about the board conversion. It's a review, take it or leave
it. You seem to care how the boards are converted, as you've sent a
patch.

Sebastian

>> ...
>>
>> static int a9m2410_devices_init(void)
>> {
>> ...
>> 	#ifdef CONFIG_NAND
>> 	devfs_create_partitions("nand0", a9m2410_nand_partitions);
>> 	#endif
>> ...
>>
>> in here and the following patches.
>>
>> Sebastian
>>
>>> +			{
>>> +				.offset = 0,
>>> +				.size = 0x40000,
>>> +				.flags = DEVFS_PARTITION_FIXED,
>>> +				.name = "self_raw",
>>> +				.bbname = "self0",
>>> +			}, {
>>> +				.offset = DEVFS_PARTITION_APPEND,
>>> +				.size = 0x20000,
>>> +				.flags = DEVFS_PARTITION_FIXED,
>>> +				.name = "env_raw",
>>> +				.bbname = "env0",
>>> +			}, {
>>> +				/* sentinel (detected by .name = NULL) */
>>> +			}});
>>>   #endif
>>> +
>>>   	armlinux_set_architecture(MACH_TYPE_A9M2410);
>>>
>>>   	return 0;
>>>
>>
>>
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
>


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 06/10] ARM: a9m2410: convert to devfs_create_partitions

[Sascha Hauer]

On Wed, Feb 26, 2014 at 12:25:20AM +0100, Sebastian Hesselbarth wrote:
> On 02/25/2014 11:51 PM, Uwe Kleine-König wrote:
> >Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >---
> >  arch/arm/boards/a9m2410/a9m2410.c | 23 +++++++++++++++++------
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> >
> >diff --git a/arch/arm/boards/a9m2410/a9m2410.c b/arch/arm/boards/a9m2410/a9m2410.c
> >index b2b6c87117a3..8d528cf60378 100644
> >--- a/arch/arm/boards/a9m2410/a9m2410.c
> >+++ b/arch/arm/boards/a9m2410/a9m2410.c
> >@@ -117,13 +117,24 @@ static int a9m2410_devices_init(void)
> >  			16, IORESOURCE_MEM, NULL);
> >
> >  #ifdef CONFIG_NAND
> >-	/* ----------- add some vital partitions -------- */
> >-	devfs_add_partition("nand0", 0x00000, 0x40000, DEVFS_PARTITION_FIXED, "self_raw");
> >-	dev_add_bb_dev("self_raw", "self0");
> >-
> >-	devfs_add_partition("nand0", 0x40000, 0x20000, DEVFS_PARTITION_FIXED, "env_raw");
> >-	dev_add_bb_dev("env_raw", "env0");
> >+	devfs_create_partitions("nand0", (struct devfs_partition[]){
> 
> nit: It would be even more readable, if you move the struct
> devfs_partition[] out of a9m2410_device_init() and reference
> it here instead, i.e.
> 
> static struct devfs_partition a9m2410_nand_partitions[] = {
> 	...
> 	{ }
> };
> 
> ...
> 
> static int a9m2410_devices_init(void)
> {
> ...
> 	#ifdef CONFIG_NAND
> 	devfs_create_partitions("nand0", a9m2410_nand_partitions);
> 	#endif
> ...
> 
> in here and the following patches.

+1

In this particular case the __maybe_unused can also be avoided with

	if(IS_ENABLED(CONFIG_NAND))
		devfs_create_partitions("nand0", a9m2410_nand_partitions);

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 07/10] ARM: freescale-mx35-3-stack: convert to devfs_create_partitions

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/arm/boards/freescale-mx35-3-stack/3stack.c | 36 ++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boards/freescale-mx35-3-stack/3stack.c b/arch/arm/boards/freescale-mx35-3-stack/3stack.c
index dbd1c7adcb10..c82ce61b5a00 100644
--- a/arch/arm/boards/freescale-mx35-3-stack/3stack.c
+++ b/arch/arm/boards/freescale-mx35-3-stack/3stack.c
@@ -151,15 +151,39 @@ static int f3s_devices_init(void)
 
 	switch ((reg >> 25) & 0x3) {
 	case 0x01:		/* NAND is the source */
-		devfs_add_partition("nand0", 0x00000, 0x40000, DEVFS_PARTITION_FIXED, "self_raw");
-		dev_add_bb_dev("self_raw", "self0");
-		devfs_add_partition("nand0", 0x40000, 0x80000, DEVFS_PARTITION_FIXED, "env_raw");
-		dev_add_bb_dev("env_raw", "env0");
+		devfs_create_partitions("nand0", (struct devfs_partition[]){
+				{
+					.offset = 0,
+					.size = 0x40000,
+					.flags = DEVFS_PARTITION_FIXED,
+					.name = "self_raw",
+					.bbname = "self0",
+				}, {
+					.offset = DEVFS_PARTITION_APPEND,
+					.size = 0x80000,
+					.flags = DEVFS_PARTITION_FIXED,
+					.name = "env_raw",
+					.bbname = "env0",
+				}, {
+					/* sentinel */
+				}});
 		break;
 
 	case 0x00:		/* NOR is the source */
-		devfs_add_partition("nor0", 0x00000, 0x40000, DEVFS_PARTITION_FIXED, "self0");
-		devfs_add_partition("nor0", 0x40000, 0x80000, DEVFS_PARTITION_FIXED, "env0");
+		devfs_create_partitions("nor0", (struct devfs_partition[]){
+				{
+					.offset = 0,
+					.size = 0x40000,
+					.flags = DEVFS_PARTITION_FIXED,
+					.name = "self0",
+				}, {
+					.offset = DEVFS_PARTITION_APPEND,
+					.size = 0x80000,
+					.flags = DEVFS_PARTITION_FIXED,
+					.name = "env0",
+				}, {
+					/* sentinel */
+				}});
 		protect_file("/dev/env0", 1);
 		break;
 	}
-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 08/10] ARM: pca100: convert to devfs_create_partitions

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/arm/boards/phycard-i.MX27/pca100.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boards/phycard-i.MX27/pca100.c b/arch/arm/boards/phycard-i.MX27/pca100.c
index 2ff1b793291c..66ad0c772b96 100644
--- a/arch/arm/boards/phycard-i.MX27/pca100.c
+++ b/arch/arm/boards/phycard-i.MX27/pca100.c
@@ -178,7 +178,6 @@ static void pca100_usb_init(void)
 static int pca100_devices_init(void)
 {
 	int i;
-	struct device_d *nand;
 
 	unsigned int mode[] = {
 		PD0_AIN_FEC_TXD0,
@@ -286,12 +285,22 @@ static int pca100_devices_init(void)
 	pca100_usb_register();
 #endif
 
-	nand = get_device_by_name("nand0");
-	devfs_add_partition("nand0", 0x00000, 0x40000, DEVFS_PARTITION_FIXED, "self_raw");
-	dev_add_bb_dev("self_raw", "self0");
-
-	devfs_add_partition("nand0", 0x40000, 0x20000, DEVFS_PARTITION_FIXED, "env_raw");
-	dev_add_bb_dev("env_raw", "env0");
+	devfs_create_partitions("nand0", (struct devfs_partition[]){
+			{
+				.offset = 0x00000,
+				.size = 0x40000,
+				.flags = DEVFS_PARTITION_FIXED,
+				.name = "self_raw",
+				.bbname = "self0",
+			}, {
+				.offset = DEVFS_PARTITION_APPEND, /* 0x40000 */
+				.size = 0x20000,
+				.flags = DEVFS_PARTITION_FIXED,
+				.name = "env_raw",
+				.bbname = "env0",
+			}, {
+				/* sentinel */
+			}});
 
 	armlinux_set_architecture(2149);
 
-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 09/10] ARM: pcm038: convert to devfs_create_partitions

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/arm/boards/pcm038/pcm038.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boards/pcm038/pcm038.c b/arch/arm/boards/pcm038/pcm038.c
index 1733261f7d04..a5171cc920ce 100644
--- a/arch/arm/boards/pcm038/pcm038.c
+++ b/arch/arm/boards/pcm038/pcm038.c
@@ -336,19 +336,40 @@ static int pcm038_devices_init(void)
 
 	switch (bootsource_get()) {
 	case BOOTSOURCE_NAND:
-		devfs_add_partition("nand0", 0, SZ_512K,
-				    DEVFS_PARTITION_FIXED, "self_raw");
-		dev_add_bb_dev("self_raw", "self0");
-		devfs_add_partition("nand0", SZ_512K, SZ_128K,
-				    DEVFS_PARTITION_FIXED, "env_raw");
-		dev_add_bb_dev("env_raw", "env0");
+		devfs_create_partitions("nand0", (struct devfs_partition[]){
+				{
+					.offset = 0,
+					.size = SZ_512K,
+					.flags = DEVFS_PARTITION_FIXED,
+					.name = "self_raw",
+					.bbname = "self0",
+				}, {
+					.offset = DEVFS_PARTITION_APPEND,
+					.size = SZ_128K,
+					.flags = DEVFS_PARTITION_FIXED,
+					.name = "env_raw",
+					.bbname = "env0",
+				}, {
+					/* sentinel */
+				}});
+
 		envdev = "NAND";
 		break;
 	default:
-		devfs_add_partition("nor0", 0, SZ_512K,
-				    DEVFS_PARTITION_FIXED, "self0");
-		devfs_add_partition("nor0", SZ_512K, SZ_128K,
-				    DEVFS_PARTITION_FIXED, "env0");
+		devfs_create_partitions("nor0", (struct devfs_partition[]){
+				{
+					.offset = 0,
+					.size = SZ_512K,
+					.flags = DEVFS_PARTITION_FIXED,
+					.name = "self0",
+				}, {
+					.offset = DEVFS_PARTITION_APPEND,
+					.size = SZ_128K,
+					.flags = DEVFS_PARTITION_FIXED,
+					.name = "env0",
+				}, {
+					/* sentinel */
+				}});
 		protect_file("/dev/env0", 1);
 		envdev = "NOR";
 	}
-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 10/10] ARM: sama5d3xek: convert to devfs_create_partitions

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/arm/boards/sama5d3xek/init.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boards/sama5d3xek/init.c b/arch/arm/boards/sama5d3xek/init.c
index 4f866aa6f85d..5459bff2df5e 100644
--- a/arch/arm/boards/sama5d3xek/init.c
+++ b/arch/arm/boards/sama5d3xek/init.c
@@ -411,14 +411,36 @@ static int at91sama5d3xek_devices_init(void)
 	ek_add_device_mci();
 	ek_add_device_lcdc();
 
-	devfs_add_partition("nand0", 0x00000, SZ_256K, DEVFS_PARTITION_FIXED, "at91bootstrap_raw");
-	dev_add_bb_dev("at91bootstrap_raw", "at91bootstrap");
-	devfs_add_partition("nand0", SZ_256K, SZ_256K + SZ_128K, DEVFS_PARTITION_FIXED, "self_raw");
-	dev_add_bb_dev("self_raw", "self0");
-	devfs_add_partition("nand0", SZ_512K + SZ_256K, SZ_256K, DEVFS_PARTITION_FIXED, "env_raw");
-	dev_add_bb_dev("env_raw", "env0");
-	devfs_add_partition("nand0", SZ_1M, SZ_256K, DEVFS_PARTITION_FIXED, "env_raw1");
-	dev_add_bb_dev("env_raw1", "env1");
+	devfs_create_partitions("nand0", (struct devfs_partition[]){
+			{
+				.offset = 0x00000,
+				.size = SZ_256K,
+				.flags = DEVFS_PARTITION_FIXED,
+				.name = "at91bootstrap_raw",
+				.bbname = "at91bootstrap",
+			}, {
+				.offset = DEVFS_PARTITION_APPEND, /* 256 KiB */
+				.size = SZ_256K + SZ_128K,
+				.flags = DEVFS_PARTITION_FIXED,
+				.name = "self_raw",
+				.bbname = "self0",
+			},
+				/* hole of 128 KiB */
+			{
+				.offset = SZ_512K + SZ_256K,
+				.size = SZ_256K,
+				.flags = DEVFS_PARTITION_FIXED,
+				.name = "env_raw",
+				.bbname = "env0",
+			}, {
+				.offset = DEVFS_PARTITION_APPEND, /* 1 MiB */
+				.size = SZ_256K,
+				.flags = DEVFS_PARTITION_FIXED,
+				.name = "env_raw1",
+				.bbname = "env1",
+			}, {
+				/* sentinel */
+			}});
 
 	return 0;
 }
-- 
1.8.5.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox