Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1284461 > unrolled thread

[RFC PATCH 0/7] mtd: partitions: add of_match_table support

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 0/7] mtd: partitions: add of_match_table support Brian Norris <computersforpeace@gmail.com> - 2015-12-05 06:30 +0100
    [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers Brian Norris <computersforpeace@gmail.com> - 2015-12-05 06:30 +0100
      Re: [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and  of_mtd_match_mtd_parser() helpers Rob Herring <robh+dt@kernel.org> - 2015-12-07 03:50 +0100
        Re: [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and  of_mtd_match_mtd_parser() helpers Brian Norris <computersforpeace@gmail.com> - 2015-12-07 19:20 +0100
          Re: [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and  of_mtd_match_mtd_parser() helpers Rob Herring <robh+dt@kernel.org> - 2015-12-07 20:10 +0100
    [RFC PATCH 5/7] mtd: partitions: factor out "match by name" handling Brian Norris <computersforpeace@gmail.com> - 2015-12-05 06:30 +0100
    Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support Geert Uytterhoeven <geert@linux-m68k.org> - 2015-12-05 11:20 +0100
      Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support Michal Suchanek <hramrach@gmail.com> - 2015-12-05 19:10 +0100
      Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support Brian Norris <computersforpeace@gmail.com> - 2015-12-10 22:00 +0100
        Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support Geert Uytterhoeven <geert@linux-m68k.org> - 2015-12-11 09:50 +0100
          Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support Michal Suchanek <hramrach@gmail.com> - 2015-12-11 16:40 +0100
            Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support Geert Uytterhoeven <geert@linux-m68k.org> - 2015-12-11 17:10 +0100
              Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support Michal Suchanek <hramrach@gmail.com> - 2015-12-11 17:20 +0100
    Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support Jonas Gorski <jogo@openwrt.org> - 2015-12-05 12:40 +0100
      Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support Brian Norris <computersforpeace@gmail.com> - 2015-12-10 22:10 +0100

#1284461 — [RFC PATCH 0/7] mtd: partitions: add of_match_table support

Hi,

There have been several discussions [1] about adding a device tree binding for
associating flash devices with the partition parser(s) that are used on the
flash. There are a few reasons:

 (1) drivers shouldn't have to be encoding platform knowledge by listing what
     parsers might be used on a given system (this is the currently all that's
     supported)
 (2) we can't just scan for all supported parsers (like the block system does), since
     there is a wide diversity of "formats" (no standardization), and it is not
     always safe or efficient to attempt to do so, particularly since many of
     them allow their data structures to be placed anywhere on the flash, and
     so require scanning the entire flash device to find them.

So instead, let's support a new binding so that a device tree can specify what
partition formats might be used. This seems like a reasonable choice (even
though it's not strictly a hardware description) because the flash layout /
partitioning is often very closely tied with the bootloader/firmware, at
production time.

Also, as an example first-use of this mechanism, I support Google's FMAP flash
structure, used on Chrome OS devices.

Note that this is an RFC, mainly for the reason noted in patch 6 ("RFC: mtd:
partitions: enable of_match_table matching"): the of_match_table support won't
yet autoload a partition parser that is built as a module. I'm not quite sure
if there's a lot of value in supporting MTD parsers as modules (block partition
support can't be), but that is supported for "by-name" parser lookups in MTD
already, so I don't feel like dropping that feature yet. Tips or thoughts are
particularly welcome on this aspect!

Also note that there's an existing undocumented binding for a
"linux,part-probe" property, but it is only usable on the physmap_of.c driver
at the moment, and it is IMO not a good binding. I posted my thoughts on that
previously here [2], and since no one else cared to make a better one...I did
it myself.

I'd love it if we could kill the unreviewed binding off in favor of something
more like this...

Currently based on v2 of "mtd: partitions: support cleanup callback for
parsers":

  http://lkml.kernel.org/g/1449271518-118900-1-git-send-email-computersforpeace@gmail.com

and this series
("mtd: ofpart: don't complain about missing 'partitions' node too loudly" and
"doc: dt: mtd: partitions: add compatible property to "partitions" node"):

  http://lkml.kernel.org/g/1449194529-145705-1-git-send-email-computersforpeace@gmail.com

Both of which should hopefully be merged soon.

The current total of this work is stashed here for now:

  git fetch git://git.infradead.org/users/norris/linux-mtd.git partition-of-match

I may rewrite this branch if I post future revisions of these patch sets, FYI.

I look forward to your reviews.

Regards,
Brian

[1] Trying to extend "linux,part-probe":
    http://patchwork.ozlabs.org/patch/475988/
    For bcm47xxpart:
    http://patchwork.ozlabs.org/patch/475986/
    For AFS:
    http://patchwork.ozlabs.org/patch/537827/

[2] "mtd: document linux-specific partition parser DT binding"
    http://lists.infradead.org/pipermail/linux-mtd/2015-October/062773.html

Brian Norris (7):
  mtd: move partition parsers to drivers/mtd/partitions/
  mtd: move partition parsers' Kconfig under a sub-menu
  doc: dt: mtd: partition: add on-flash format binding
  mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
  mtd: partitions: factor out "match by name" handling
  RFC: mtd: partitions: enable of_match_table matching
  mtd: partitions: add Google's FMAP partition parser

 .../devicetree/bindings/mtd/partition.txt          |  75 ++++++-
 drivers/mtd/Kconfig                                | 134 +-----------
 drivers/mtd/Makefile                               |   8 +-
 drivers/mtd/mtdpart.c                              |  99 +++++++--
 drivers/mtd/partitions/Kconfig                     | 138 +++++++++++++
 drivers/mtd/partitions/Makefile                    |   8 +
 drivers/mtd/{ => partitions}/afs.c                 |   0
 drivers/mtd/{ => partitions}/ar7part.c             |   0
 drivers/mtd/{ => partitions}/bcm47xxpart.c         |   0
 drivers/mtd/{ => partitions}/bcm63xxpart.c         |   0
 drivers/mtd/{ => partitions}/cmdlinepart.c         |   0
 drivers/mtd/partitions/google_fmap.c               | 226 +++++++++++++++++++++
 drivers/mtd/{ => partitions}/ofpart.c              |   0
 drivers/mtd/{ => partitions}/redboot.c             |   0
 drivers/of/of_mtd.c                                |  33 +++
 include/linux/mtd/partitions.h                     |   2 +
 include/linux/of_mtd.h                             |  13 ++
 17 files changed, 577 insertions(+), 159 deletions(-)
 create mode 100644 drivers/mtd/partitions/Kconfig
 create mode 100644 drivers/mtd/partitions/Makefile
 rename drivers/mtd/{ => partitions}/afs.c (100%)
 rename drivers/mtd/{ => partitions}/ar7part.c (100%)
 rename drivers/mtd/{ => partitions}/bcm47xxpart.c (100%)
 rename drivers/mtd/{ => partitions}/bcm63xxpart.c (100%)
 rename drivers/mtd/{ => partitions}/cmdlinepart.c (100%)
 create mode 100644 drivers/mtd/partitions/google_fmap.c
 rename drivers/mtd/{ => partitions}/ofpart.c (100%)
 rename drivers/mtd/{ => partitions}/redboot.c (100%)

-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1284462 — [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers

Like the corresponding OF-based device/driver matching infrascture,
let's begin to support a mtd/partition-parser matching infrastructure.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/of/of_mtd.c            | 33 +++++++++++++++++++++++++++++++++
 include/linux/mtd/partitions.h |  2 ++
 include/linux/of_mtd.h         | 13 +++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
index b7361ed70537..169d7500af5d 100644
--- a/drivers/of/of_mtd.c
+++ b/drivers/of/of_mtd.c
@@ -9,6 +9,7 @@
 #include <linux/kernel.h>
 #include <linux/of_mtd.h>
 #include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
 #include <linux/export.h>
 
 /**
@@ -117,3 +118,35 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)
 	return of_property_read_bool(np, "nand-on-flash-bbt");
 }
 EXPORT_SYMBOL_GPL(of_get_nand_on_flash_bbt);
+
+static const struct of_device_id *of_match_mtd_parser(
+		struct mtd_part_parser *parser, struct device_node *np)
+{
+	if (!parser || !np)
+		return NULL;
+
+	return of_match_node(parser->of_match_table, np);
+}
+
+static struct device_node *mtd_get_partitions_of_node(struct mtd_info *master)
+{
+	struct device_node *np = mtd_get_of_node(master);
+
+	if (!np)
+		return NULL;
+
+	return of_get_child_by_name(np, "partitions");
+}
+
+bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+			     struct mtd_part_parser *parser)
+{
+	struct device_node *np = mtd_get_partitions_of_node(mtd);
+	bool ret;
+
+	ret = of_match_mtd_parser(parser, np) != NULL;
+	of_node_put(np);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(of_mtd_match_mtd_parser);
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 70736e1e6c8f..2e68ef561a40 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -51,6 +51,7 @@ struct mtd_partition {
 
 struct mtd_info;
 struct device_node;
+struct of_device_id;
 
 /**
  * struct mtd_part_parser_data - used to pass data to MTD partition parsers.
@@ -69,6 +70,7 @@ struct mtd_part_parser {
 	struct list_head list;
 	struct module *owner;
 	const char *name;
+	const struct of_device_id *of_match_table;
 	int (*parse_fn)(struct mtd_info *, const struct mtd_partition **,
 			struct mtd_part_parser_data *);
 	void (*cleanup)(const struct mtd_partition *pparts, int nr_parts);
diff --git a/include/linux/of_mtd.h b/include/linux/of_mtd.h
index e266caa36402..781362d0be0c 100644
--- a/include/linux/of_mtd.h
+++ b/include/linux/of_mtd.h
@@ -9,6 +9,10 @@
 #ifndef __LINUX_OF_MTD_H
 #define __LINUX_OF_MTD_H
 
+#include <linux/mtd/mtd.h>
+
+struct mtd_part_parser;
+
 #ifdef CONFIG_OF_MTD
 
 #include <linux/of.h>
@@ -18,6 +22,9 @@ int of_get_nand_ecc_strength(struct device_node *np);
 int of_get_nand_bus_width(struct device_node *np);
 bool of_get_nand_on_flash_bbt(struct device_node *np);
 
+bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+			     struct mtd_part_parser *parser);
+
 #else /* CONFIG_OF_MTD */
 
 static inline int of_get_nand_ecc_mode(struct device_node *np)
@@ -45,6 +52,12 @@ static inline bool of_get_nand_on_flash_bbt(struct device_node *np)
 	return false;
 }
 
+static inline bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+					   struct mtd_part_parser *parser)
+{
+	return false;
+}
+
 #endif /* CONFIG_OF_MTD */
 
 #endif /* __LINUX_OF_MTD_H */
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1284973 — Re: [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers

On Fri, Dec 4, 2015 at 11:19 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> Like the corresponding OF-based device/driver matching infrascture,

typo.

> let's begin to support a mtd/partition-parser matching infrastructure.
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  drivers/of/of_mtd.c            | 33 +++++++++++++++++++++++++++++++++

BTW, this file should be moved to drivers/mtd/ at some point.

>  include/linux/mtd/partitions.h |  2 ++
>  include/linux/of_mtd.h         | 13 +++++++++++++
>  3 files changed, 48 insertions(+)
>
> diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
> index b7361ed70537..169d7500af5d 100644
> --- a/drivers/of/of_mtd.c
> +++ b/drivers/of/of_mtd.c
> @@ -9,6 +9,7 @@
>  #include <linux/kernel.h>
>  #include <linux/of_mtd.h>
>  #include <linux/mtd/nand.h>
> +#include <linux/mtd/partitions.h>
>  #include <linux/export.h>
>
>  /**
> @@ -117,3 +118,35 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)
>         return of_property_read_bool(np, "nand-on-flash-bbt");
>  }
>  EXPORT_SYMBOL_GPL(of_get_nand_on_flash_bbt);
> +
> +static const struct of_device_id *of_match_mtd_parser(

This function name and the only caller's function name are very
similar. Why not just move this function inline.

> +               struct mtd_part_parser *parser, struct device_node *np)
> +{
> +       if (!parser || !np)
> +               return NULL;
> +
> +       return of_match_node(parser->of_match_table, np);
> +}
> +
> +static struct device_node *mtd_get_partitions_of_node(struct mtd_info *master)
> +{
> +       struct device_node *np = mtd_get_of_node(master);
> +
> +       if (!np)
> +               return NULL;
> +
> +       return of_get_child_by_name(np, "partitions");
> +}
> +
> +bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
> +                            struct mtd_part_parser *parser)
> +{
> +       struct device_node *np = mtd_get_partitions_of_node(mtd);
> +       bool ret;
> +
> +       ret = of_match_mtd_parser(parser, np) != NULL;
> +       of_node_put(np);
> +
> +       return ret;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1285837 — Re: [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers

On Sun, Dec 06, 2015 at 08:45:40PM -0600, Rob Herring wrote:
> On Fri, Dec 4, 2015 at 11:19 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> >  drivers/of/of_mtd.c            | 33 +++++++++++++++++++++++++++++++++
> 
> BTW, this file should be moved to drivers/mtd/ at some point.

How about s/at some point/now/ ? I can send a separate patch. It also
seems like these should just get linked into the 'mtd' module (when
CONFIG_OF=y) instead of having a tiny module for just a few functions.

Why did files like this get placed here anyway? Is there a reason that
there are things like of_net and of_pci here too?

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1285863 — Re: [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers

On Mon, Dec 7, 2015 at 12:13 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Sun, Dec 06, 2015 at 08:45:40PM -0600, Rob Herring wrote:
>> On Fri, Dec 4, 2015 at 11:19 PM, Brian Norris
>> <computersforpeace@gmail.com> wrote:
>> >  drivers/of/of_mtd.c            | 33 +++++++++++++++++++++++++++++++++
>>
>> BTW, this file should be moved to drivers/mtd/ at some point.
>
> How about s/at some point/now/ ?

Great.

> I can send a separate patch. It also
> seems like these should just get linked into the 'mtd' module (when
> CONFIG_OF=y) instead of having a tiny module for just a few functions.

Agreed.

> Why did files like this get placed here anyway? Is there a reason that
> there are things like of_net and of_pci here too?

Things started out here, but as the number of subsystems and
associated bindings grew it became evident that binding specific
things should go with the subsystems. I also have a secret goal to
eliminate drivers/of. Don't tell anyone.

of_net should probably move, just no one has had the itch to do it.
PCI is a bit special and somewhat tied into the rest of the core.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1284463 — [RFC PATCH 5/7] mtd: partitions: factor out "match by name" handling

This code structure is going to be imitated for a match-by-device-node
implementation, so let's factor out a few functions to make this easier.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/mtdpart.c | 67 +++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 52 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 10bf304027dd..b3100742ddf6 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -687,23 +687,47 @@ int add_mtd_partitions(struct mtd_info *master,
 static DEFINE_SPINLOCK(part_parser_lock);
 static LIST_HEAD(part_parsers);
 
-static struct mtd_part_parser *mtd_part_parser_get(const char *name)
+static bool mtd_part_parser_match_name(struct mtd_part_parser *p,
+				       const char *name)
+{
+	return !strcmp(p->name, name);
+}
+
+static struct mtd_part_parser *__mtd_part_parser_get_by_name(const char *name)
 {
 	struct mtd_part_parser *p, *ret = NULL;
 
 	spin_lock(&part_parser_lock);
 
-	list_for_each_entry(p, &part_parsers, list)
-		if (!strcmp(p->name, name) && try_module_get(p->owner)) {
+	list_for_each_entry(p, &part_parsers, list) {
+		if (mtd_part_parser_match_name(p, name) &&
+				try_module_get(p->owner)) {
 			ret = p;
 			break;
 		}
+	}
 
 	spin_unlock(&part_parser_lock);
 
 	return ret;
 }
 
+static struct mtd_part_parser *mtd_part_parser_get_by_name(const char *name)
+{
+	struct mtd_part_parser *p;
+
+	/* Get parser, if already loaded */
+	p = __mtd_part_parser_get_by_name(name);
+	if (p)
+		return p;
+
+	if (request_module("%s", name))
+		return NULL;
+
+	/* Try again */
+	return __mtd_part_parser_get_by_name(name);
+}
+
 static inline void mtd_part_parser_put(const struct mtd_part_parser *p)
 {
 	module_put(p->owner);
@@ -752,6 +776,27 @@ static const char * const default_mtd_part_types[] = {
 	NULL
 };
 
+static int mtd_part_do_parse(struct mtd_part_parser *parser,
+			     struct mtd_info *master,
+			     struct mtd_partitions *pparts,
+			     struct mtd_part_parser_data *data)
+{
+	int ret;
+
+	ret = (*parser->parse_fn)(master, &pparts->parts, data);
+	pr_debug("%s: parser %s: %i\n", master->name, parser->name, ret);
+	if (ret <= 0)
+		return ret;
+
+	pr_notice("%d %s partitions found on MTD device %s\n",
+		  ret, parser->name, master->name);
+
+	pparts->nr_parts = ret;
+	pparts->parser = parser;
+
+	return ret;
+}
+
 /**
  * parse_mtd_partitions - parse MTD partitions
  * @master: the master partition (describes whole MTD device)
@@ -785,23 +830,15 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 
 	for ( ; *types; types++) {
 		pr_debug("%s: parsing partitions %s\n", master->name, *types);
-		parser = mtd_part_parser_get(*types);
-		if (!parser && !request_module("%s", *types))
-			parser = mtd_part_parser_get(*types);
+		parser = mtd_part_parser_get_by_name(*types);
 		pr_debug("%s: got parser %s\n", master->name,
 			 parser ? parser->name : NULL);
 		if (!parser)
 			continue;
-		ret = (*parser->parse_fn)(master, &pparts->parts, data);
-		pr_debug("%s: parser %s: %i\n",
-			 master->name, parser->name, ret);
-		if (ret > 0) {
-			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",
-			       ret, parser->name, master->name);
-			pparts->nr_parts = ret;
-			pparts->parser = parser;
+		ret = mtd_part_do_parse(parser, master, pparts, data);
+		/* Found partitions! */
+		if (ret > 0)
 			return 0;
-		}
 		mtd_part_parser_put(parser);
 		/*
 		 * Stash the first error we see; only report it if no parser
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1284512

On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> There have been several discussions [1] about adding a device tree binding for
> associating flash devices with the partition parser(s) that are used on the
> flash. There are a few reasons:
>
>  (1) drivers shouldn't have to be encoding platform knowledge by listing what
>      parsers might be used on a given system (this is the currently all that's
>      supported)
>  (2) we can't just scan for all supported parsers (like the block system does), since
>      there is a wide diversity of "formats" (no standardization), and it is not
>      always safe or efficient to attempt to do so, particularly since many of
>      them allow their data structures to be placed anywhere on the flash, and
>      so require scanning the entire flash device to find them.

I read the second reason, but would it be useful to (partially) merge
block/partitions/ and drivers/mtd/partitions/, so I can use e.g. msdos
partitions
on an mtd device??

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1284618

On 5 December 2015 at 11:15, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> <computersforpeace@gmail.com> wrote:
>> There have been several discussions [1] about adding a device tree binding for
>> associating flash devices with the partition parser(s) that are used on the
>> flash. There are a few reasons:
>>
>>  (1) drivers shouldn't have to be encoding platform knowledge by listing what
>>      parsers might be used on a given system (this is the currently all that's
>>      supported)
>>  (2) we can't just scan for all supported parsers (like the block system does), since
>>      there is a wide diversity of "formats" (no standardization), and it is not
>>      always safe or efficient to attempt to do so, particularly since many of
>>      them allow their data structures to be placed anywhere on the flash, and
>>      so require scanning the entire flash device to find them.
>
> I read the second reason, but would it be useful to (partially) merge
> block/partitions/ and drivers/mtd/partitions/, so I can use e.g. msdos
> partitions
> on an mtd device??
>

Using msdos partition on a MTD device does not sound terribly useful.
On what kind of non-block device would you expect to find one?

On the other hand, using FMAP on a block device might work an even
might have some uses.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288927

On Sat, Dec 05, 2015 at 11:15:54AM +0100, Geert Uytterhoeven wrote:
> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> > There have been several discussions [1] about adding a device tree binding for
> > associating flash devices with the partition parser(s) that are used on the
> > flash. There are a few reasons:
> >
> >  (1) drivers shouldn't have to be encoding platform knowledge by listing what
> >      parsers might be used on a given system (this is the currently all that's
> >      supported)
> >  (2) we can't just scan for all supported parsers (like the block system does), since
> >      there is a wide diversity of "formats" (no standardization), and it is not
> >      always safe or efficient to attempt to do so, particularly since many of
> >      them allow their data structures to be placed anywhere on the flash, and
> >      so require scanning the entire flash device to find them.
> 
> I read the second reason, but would it be useful to (partially) merge
> block/partitions/ and drivers/mtd/partitions/, so I can use e.g. msdos
> partitions
> on an mtd device??

I kinda agree with Michal: is there a good use case?

Really, MTD partitioning is not a highly-scalable design. Particularly,
it's not typically that well-suited to large (read: unreliable) NAND
flash, where fixing partitions at the raw flash level mostly serves to
restrict UBI's ability to wear-level across the device. For that sort of
case, it's best if people are using UBI volumes on a (mostly?)
unpartitioned MTD, instead of using MTD partitions as the main
separation mechanism. Also, most partition designs (either MTD or block)
aren't very robust against bitflips, read disturb, etc.

IOW, I wouldn't expect MBR or GPT to work well on large raw NAND flash,
and so I don't plan to do that sort of work myself. If you can provide
some better argument for it, and some nice maintainable code to go with
it, then of course it could be considered :)

Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1289330

Hi Brian,

On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Sat, Dec 05, 2015 at 11:15:54AM +0100, Geert Uytterhoeven wrote:
>> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>> <computersforpeace@gmail.com> wrote:
>> > There have been several discussions [1] about adding a device tree binding for
>> > associating flash devices with the partition parser(s) that are used on the
>> > flash. There are a few reasons:
>> >
>> >  (1) drivers shouldn't have to be encoding platform knowledge by listing what
>> >      parsers might be used on a given system (this is the currently all that's
>> >      supported)
>> >  (2) we can't just scan for all supported parsers (like the block system does), since
>> >      there is a wide diversity of "formats" (no standardization), and it is not
>> >      always safe or efficient to attempt to do so, particularly since many of
>> >      them allow their data structures to be placed anywhere on the flash, and
>> >      so require scanning the entire flash device to find them.
>>
>> I read the second reason, but would it be useful to (partially) merge
>> block/partitions/ and drivers/mtd/partitions/, so I can use e.g. msdos
>> partitions
>> on an mtd device??
>
> I kinda agree with Michal: is there a good use case?

I don't have an immediate use case.
Just looking at it from a high-level viewpoint.

> Really, MTD partitioning is not a highly-scalable design. Particularly,
> it's not typically that well-suited to large (read: unreliable) NAND
> flash, where fixing partitions at the raw flash level mostly serves to
> restrict UBI's ability to wear-level across the device. For that sort of
> case, it's best if people are using UBI volumes on a (mostly?)
> unpartitioned MTD, instead of using MTD partitions as the main
> separation mechanism. Also, most partition designs (either MTD or block)
> aren't very robust against bitflips, read disturb, etc.
>
> IOW, I wouldn't expect MBR or GPT to work well on large raw NAND flash,
> and so I don't plan to do that sort of work myself. If you can provide
> some better argument for it, and some nice maintainable code to go with
> it, then of course it could be considered :)

There's also NOR FLASH (e.g. SPI-NOR), which is what most boards I'm
working on have.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1289690

On 11 December 2015 at 09:44, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Brian,
>
> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
>> On Sat, Dec 05, 2015 at 11:15:54AM +0100, Geert Uytterhoeven wrote:
>>> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>>> <computersforpeace@gmail.com> wrote:
>>> > There have been several discussions [1] about adding a device tree binding for
>>> > associating flash devices with the partition parser(s) that are used on the
>>> > flash. There are a few reasons:
>>> >
>>> >  (1) drivers shouldn't have to be encoding platform knowledge by listing what
>>> >      parsers might be used on a given system (this is the currently all that's
>>> >      supported)
>>> >  (2) we can't just scan for all supported parsers (like the block system does), since
>>> >      there is a wide diversity of "formats" (no standardization), and it is not
>>> >      always safe or efficient to attempt to do so, particularly since many of
>>> >      them allow their data structures to be placed anywhere on the flash, and
>>> >      so require scanning the entire flash device to find them.
>>>
>>> I read the second reason, but would it be useful to (partially) merge
>>> block/partitions/ and drivers/mtd/partitions/, so I can use e.g. msdos
>>> partitions
>>> on an mtd device??
>>
>> I kinda agree with Michal: is there a good use case?
>
> I don't have an immediate use case.
> Just looking at it from a high-level viewpoint.
>
>> Really, MTD partitioning is not a highly-scalable design. Particularly,
>> it's not typically that well-suited to large (read: unreliable) NAND
>> flash, where fixing partitions at the raw flash level mostly serves to
>> restrict UBI's ability to wear-level across the device. For that sort of
>> case, it's best if people are using UBI volumes on a (mostly?)
>> unpartitioned MTD, instead of using MTD partitions as the main
>> separation mechanism. Also, most partition designs (either MTD or block)
>> aren't very robust against bitflips, read disturb, etc.
>>
>> IOW, I wouldn't expect MBR or GPT to work well on large raw NAND flash,
>> and so I don't plan to do that sort of work myself. If you can provide
>> some better argument for it, and some nice maintainable code to go with
>> it, then of course it could be considered :)
>
> There's also NOR FLASH (e.g. SPI-NOR), which is what most boards I'm
> working on have.
>

Yes, you can dump the content of a NOR flash to a file, attach a loop
device to it, and if block devices had the ability to use flash
partitioning access the different partitions.

Maybe it would be more useful to use some kind of mtdloop, though.
There might even be one already. I never needed it.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1289703

Hi Michal,

On Fri, Dec 11, 2015 at 4:34 PM, Michal Suchanek <hramrach@gmail.com> wrote:
> On 11 December 2015 at 09:44, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
>> <computersforpeace@gmail.com> wrote:
>>> On Sat, Dec 05, 2015 at 11:15:54AM +0100, Geert Uytterhoeven wrote:
>>>> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>>>> <computersforpeace@gmail.com> wrote:
>>>> > There have been several discussions [1] about adding a device tree binding for
>>>> > associating flash devices with the partition parser(s) that are used on the
>>>> > flash. There are a few reasons:
>>>> >
>>>> >  (1) drivers shouldn't have to be encoding platform knowledge by listing what
>>>> >      parsers might be used on a given system (this is the currently all that's
>>>> >      supported)
>>>> >  (2) we can't just scan for all supported parsers (like the block system does), since
>>>> >      there is a wide diversity of "formats" (no standardization), and it is not
>>>> >      always safe or efficient to attempt to do so, particularly since many of
>>>> >      them allow their data structures to be placed anywhere on the flash, and
>>>> >      so require scanning the entire flash device to find them.
>>>>
>>>> I read the second reason, but would it be useful to (partially) merge
>>>> block/partitions/ and drivers/mtd/partitions/, so I can use e.g. msdos
>>>> partitions
>>>> on an mtd device??
>>>
>>> I kinda agree with Michal: is there a good use case?
>>
>> I don't have an immediate use case.
>> Just looking at it from a high-level viewpoint.
>>
>>> Really, MTD partitioning is not a highly-scalable design. Particularly,
>>> it's not typically that well-suited to large (read: unreliable) NAND
>>> flash, where fixing partitions at the raw flash level mostly serves to
>>> restrict UBI's ability to wear-level across the device. For that sort of
>>> case, it's best if people are using UBI volumes on a (mostly?)
>>> unpartitioned MTD, instead of using MTD partitions as the main
>>> separation mechanism. Also, most partition designs (either MTD or block)
>>> aren't very robust against bitflips, read disturb, etc.
>>>
>>> IOW, I wouldn't expect MBR or GPT to work well on large raw NAND flash,
>>> and so I don't plan to do that sort of work myself. If you can provide
>>> some better argument for it, and some nice maintainable code to go with
>>> it, then of course it could be considered :)
>>
>> There's also NOR FLASH (e.g. SPI-NOR), which is what most boards I'm
>> working on have.
>>
>
> Yes, you can dump the content of a NOR flash to a file, attach a loop
> device to it, and if block devices had the ability to use flash
> partitioning access the different partitions.
>
> Maybe it would be more useful to use some kind of mtdloop, though.
> There might even be one already. I never needed it.

That's the inverse, which looks like a solid use case to me ;-)
E.g. for investigation or virtualization.

You can do this already in userspace with kpartx, though.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1289709

On 11 December 2015 at 17:00, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Michal,
>
> On Fri, Dec 11, 2015 at 4:34 PM, Michal Suchanek <hramrach@gmail.com> wrote:
>> On 11 December 2015 at 09:44, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
>>> <computersforpeace@gmail.com> wrote:
>>>> On Sat, Dec 05, 2015 at 11:15:54AM +0100, Geert Uytterhoeven wrote:
>>>>> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>>>>> <computersforpeace@gmail.com> wrote:
>>>>> > There have been several discussions [1] about adding a device tree binding for
>>>>> > associating flash devices with the partition parser(s) that are used on the
>>>>> > flash. There are a few reasons:
>>>>> >
>>>>> >  (1) drivers shouldn't have to be encoding platform knowledge by listing what
>>>>> >      parsers might be used on a given system (this is the currently all that's
>>>>> >      supported)
>>>>> >  (2) we can't just scan for all supported parsers (like the block system does), since
>>>>> >      there is a wide diversity of "formats" (no standardization), and it is not
>>>>> >      always safe or efficient to attempt to do so, particularly since many of
>>>>> >      them allow their data structures to be placed anywhere on the flash, and
>>>>> >      so require scanning the entire flash device to find them.
>>>>>
>>>>> I read the second reason, but would it be useful to (partially) merge
>>>>> block/partitions/ and drivers/mtd/partitions/, so I can use e.g. msdos
>>>>> partitions
>>>>> on an mtd device??
>>>>
>>>> I kinda agree with Michal: is there a good use case?
>>>
>>> I don't have an immediate use case.
>>> Just looking at it from a high-level viewpoint.
>>>
>>>> Really, MTD partitioning is not a highly-scalable design. Particularly,
>>>> it's not typically that well-suited to large (read: unreliable) NAND
>>>> flash, where fixing partitions at the raw flash level mostly serves to
>>>> restrict UBI's ability to wear-level across the device. For that sort of
>>>> case, it's best if people are using UBI volumes on a (mostly?)
>>>> unpartitioned MTD, instead of using MTD partitions as the main
>>>> separation mechanism. Also, most partition designs (either MTD or block)
>>>> aren't very robust against bitflips, read disturb, etc.
>>>>
>>>> IOW, I wouldn't expect MBR or GPT to work well on large raw NAND flash,
>>>> and so I don't plan to do that sort of work myself. If you can provide
>>>> some better argument for it, and some nice maintainable code to go with
>>>> it, then of course it could be considered :)
>>>
>>> There's also NOR FLASH (e.g. SPI-NOR), which is what most boards I'm
>>> working on have.
>>>
>>
>> Yes, you can dump the content of a NOR flash to a file, attach a loop
>> device to it, and if block devices had the ability to use flash
>> partitioning access the different partitions.
>>
>> Maybe it would be more useful to use some kind of mtdloop, though.
>> There might even be one already. I never needed it.
>
> That's the inverse, which looks like a solid use case to me ;-)
> E.g. for investigation or virtualization.
>
> You can do this already in userspace with kpartx, though.
>

What kind of partitioning does kpartx support? I do not see that
documented anywhere.

Anyway, it seems there is no mtdblock so in case of non-ecc (or
hidden-ecc) flashes you could use losetup -P and block2mtd on each
partition to look at the dumped flash content or even prepare flash
images. So long as the kernel supports the partitioning scheme used on
the flash.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1284533

Hi,

On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> Hi,
>
> There have been several discussions [1] about adding a device tree binding for
> associating flash devices with the partition parser(s) that are used on the
> flash. There are a few reasons:
>
>  (1) drivers shouldn't have to be encoding platform knowledge by listing what
>      parsers might be used on a given system (this is the currently all that's
>      supported)
>  (2) we can't just scan for all supported parsers (like the block system does), since
>      there is a wide diversity of "formats" (no standardization), and it is not
>      always safe or efficient to attempt to do so, particularly since many of
>      them allow their data structures to be placed anywhere on the flash, and
>      so require scanning the entire flash device to find them.
>
> So instead, let's support a new binding so that a device tree can specify what
> partition formats might be used. This seems like a reasonable choice (even
> though it's not strictly a hardware description) because the flash layout /
> partitioning is often very closely tied with the bootloader/firmware, at
> production time.

On a first glance this looks good to me, and looks easily extensible
for application of non-complete partition parsers.

E.g. for the "brcm,bcm6345-imagetag" we would want to actually do something like

partitions {
    ....

    partition@0 {
        reg = <0x0 0x10000>;
        label = "cfe";
        read-only;
    };

    partition@10000 {
        reg = <0x10000 0x3d0000>;
        label = "firmware";
        compatible = "brcm,bcm6345-imagetag";
    };

    partition@3e0000 {
        reg = <0x3e0000 0x10000>;
        label = "art";
        read-only;
    };

   partition@3f0000 {
        reg = <0x3f0000 0x10000>;
        label = "nvram";
        read-only;
    };
};

as the image tag can only specify the offsets and sizes of the rootfs
and kernel parts, but not of any other parts.

>
> Also, as an example first-use of this mechanism, I support Google's FMAP flash
> structure, used on Chrome OS devices.
>
> Note that this is an RFC, mainly for the reason noted in patch 6 ("RFC: mtd:
> partitions: enable of_match_table matching"): the of_match_table support won't
> yet autoload a partition parser that is built as a module. I'm not quite sure
> if there's a lot of value in supporting MTD parsers as modules (block partition
> support can't be), but that is supported for "by-name" parser lookups in MTD
> already, so I don't feel like dropping that feature yet. Tips or thoughts are
> particularly welcome on this aspect!

I would assume a lot of the cases these would be a chicken-egg
problem, you need the parser to be able to find and mount the rootfs,
but you you need mount the rootfs to load the parser.

> Also note that there's an existing undocumented binding for a
> "linux,part-probe" property, but it is only usable on the physmap_of.c driver
> at the moment, and it is IMO not a good binding. I posted my thoughts on that
> previously here [2], and since no one else cared to make a better one...I did
> it myself.
>
> I'd love it if we could kill the unreviewed binding off in favor of something
> more like this...

I agree fully that this is a bad binding, as it exposes internal names
that aren't supposed to be fixed.


Jonas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288930

On Sat, Dec 05, 2015 at 12:35:42PM +0100, Jonas Gorski wrote:
> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> > Hi,
> >
> > There have been several discussions [1] about adding a device tree binding for
> > associating flash devices with the partition parser(s) that are used on the
> > flash. There are a few reasons:
> >
> >  (1) drivers shouldn't have to be encoding platform knowledge by listing what
> >      parsers might be used on a given system (this is the currently all that's
> >      supported)
> >  (2) we can't just scan for all supported parsers (like the block system does), since
> >      there is a wide diversity of "formats" (no standardization), and it is not
> >      always safe or efficient to attempt to do so, particularly since many of
> >      them allow their data structures to be placed anywhere on the flash, and
> >      so require scanning the entire flash device to find them.
> >
> > So instead, let's support a new binding so that a device tree can specify what
> > partition formats might be used. This seems like a reasonable choice (even
> > though it's not strictly a hardware description) because the flash layout /
> > partitioning is often very closely tied with the bootloader/firmware, at
> > production time.
> 
> On a first glance this looks good to me, and looks easily extensible
> for application of non-complete partition parsers.
> 
> E.g. for the "brcm,bcm6345-imagetag" we would want to actually do something like
> 
> partitions {
>     ....
> 
>     partition@0 {
>         reg = <0x0 0x10000>;
>         label = "cfe";
>         read-only;
>     };
> 
>     partition@10000 {
>         reg = <0x10000 0x3d0000>;
>         label = "firmware";
>         compatible = "brcm,bcm6345-imagetag";
>     };
> 
>     partition@3e0000 {
>         reg = <0x3e0000 0x10000>;
>         label = "art";
>         read-only;
>     };
> 
>    partition@3f0000 {
>         reg = <0x3f0000 0x10000>;
>         label = "nvram";
>         read-only;
>     };
> };
> 
> as the image tag can only specify the offsets and sizes of the rootfs
> and kernel parts, but not of any other parts.

I had your (and others') prior attempts and suggestions in mind when
planning this, and I agree that the binding looks extendible to cases
like that. I haven't yet worked out what a good MTD infrastructure for
that would look like, so I stuck with defining and implementing only
what I know use :)

> > Also, as an example first-use of this mechanism, I support Google's FMAP flash
> > structure, used on Chrome OS devices.
> >
> > Note that this is an RFC, mainly for the reason noted in patch 6 ("RFC: mtd:
> > partitions: enable of_match_table matching"): the of_match_table support won't
> > yet autoload a partition parser that is built as a module. I'm not quite sure
> > if there's a lot of value in supporting MTD parsers as modules (block partition
> > support can't be), but that is supported for "by-name" parser lookups in MTD
> > already, so I don't feel like dropping that feature yet. Tips or thoughts are
> > particularly welcome on this aspect!
> 
> I would assume a lot of the cases these would be a chicken-egg
> problem, you need the parser to be able to find and mount the rootfs,
> but you you need mount the rootfs to load the parser.

Not necessarily. One of my current use cases has a boot SPI NOR flash +
an eMMC rootfs. Modules can be loaded from eMMC.

BTW, I'm realizing that if partition parsers are forced to built-in
only, then we'd have to do the same for the MTD core (or at least, the
MTD core that handles partitioning). Not sure if that's a desirable
trade-off. (Again, block support is 'bool' in Kconfig, if we're trying
to compare.)

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web