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

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

[Brian Norris]

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

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

[Brian Norris]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

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

[RFC PATCH 1/7] mtd: move partition parsers to drivers/mtd/partitions/

[Brian Norris]

For better organization.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/Makefile                       | 8 +-------
 drivers/mtd/partitions/Makefile            | 7 +++++++
 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}/ofpart.c      | 0
 drivers/mtd/{ => partitions}/redboot.c     | 0
 9 files changed, 8 insertions(+), 7 deletions(-)
 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%)
 rename drivers/mtd/{ => partitions}/ofpart.c (100%)
 rename drivers/mtd/{ => partitions}/redboot.c (100%)

diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 99bb9a1f6e16..1c0cd3b1c7c3 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -6,13 +6,7 @@
 obj-$(CONFIG_MTD)		+= mtd.o
 mtd-y				:= mtdcore.o mtdsuper.o mtdconcat.o mtdpart.o mtdchar.o
 
-obj-$(CONFIG_MTD_OF_PARTS)	+= ofpart.o
-obj-$(CONFIG_MTD_REDBOOT_PARTS) += redboot.o
-obj-$(CONFIG_MTD_CMDLINE_PARTS) += cmdlinepart.o
-obj-$(CONFIG_MTD_AFS_PARTS)	+= afs.o
-obj-$(CONFIG_MTD_AR7_PARTS)	+= ar7part.o
-obj-$(CONFIG_MTD_BCM63XX_PARTS)	+= bcm63xxpart.o
-obj-$(CONFIG_MTD_BCM47XX_PARTS)	+= bcm47xxpart.o
+obj-y				+= partitions/
 
 # 'Users' - code which presents functionality to userspace.
 obj-$(CONFIG_MTD_BLKDEVS)	+= mtd_blkdevs.o
diff --git a/drivers/mtd/partitions/Makefile b/drivers/mtd/partitions/Makefile
new file mode 100644
index 000000000000..89822f2bfa59
--- /dev/null
+++ b/drivers/mtd/partitions/Makefile
@@ -0,0 +1,7 @@
+obj-$(CONFIG_MTD_OF_PARTS)	+= ofpart.o
+obj-$(CONFIG_MTD_REDBOOT_PARTS) += redboot.o
+obj-$(CONFIG_MTD_CMDLINE_PARTS) += cmdlinepart.o
+obj-$(CONFIG_MTD_AFS_PARTS)	+= afs.o
+obj-$(CONFIG_MTD_AR7_PARTS)	+= ar7part.o
+obj-$(CONFIG_MTD_BCM63XX_PARTS)	+= bcm63xxpart.o
+obj-$(CONFIG_MTD_BCM47XX_PARTS)	+= bcm47xxpart.o
diff --git a/drivers/mtd/afs.c b/drivers/mtd/partitions/afs.c
similarity index 100%
rename from drivers/mtd/afs.c
rename to drivers/mtd/partitions/afs.c
diff --git a/drivers/mtd/ar7part.c b/drivers/mtd/partitions/ar7part.c
similarity index 100%
rename from drivers/mtd/ar7part.c
rename to drivers/mtd/partitions/ar7part.c
diff --git a/drivers/mtd/bcm47xxpart.c b/drivers/mtd/partitions/bcm47xxpart.c
similarity index 100%
rename from drivers/mtd/bcm47xxpart.c
rename to drivers/mtd/partitions/bcm47xxpart.c
diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/partitions/bcm63xxpart.c
similarity index 100%
rename from drivers/mtd/bcm63xxpart.c
rename to drivers/mtd/partitions/bcm63xxpart.c
diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/partitions/cmdlinepart.c
similarity index 100%
rename from drivers/mtd/cmdlinepart.c
rename to drivers/mtd/partitions/cmdlinepart.c
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/partitions/ofpart.c
similarity index 100%
rename from drivers/mtd/ofpart.c
rename to drivers/mtd/partitions/ofpart.c
diff --git a/drivers/mtd/redboot.c b/drivers/mtd/partitions/redboot.c
similarity index 100%
rename from drivers/mtd/redboot.c
rename to drivers/mtd/partitions/redboot.c
-- 
2.6.0.rc2.230.g3dd15c0

[RFC PATCH 1/7] mtd: move partition parsers to drivers/mtd/partitions/

[Brian Norris]

For better organization.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/Makefile                       | 8 +-------
 drivers/mtd/partitions/Makefile            | 7 +++++++
 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}/ofpart.c      | 0
 drivers/mtd/{ => partitions}/redboot.c     | 0
 9 files changed, 8 insertions(+), 7 deletions(-)
 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%)
 rename drivers/mtd/{ => partitions}/ofpart.c (100%)
 rename drivers/mtd/{ => partitions}/redboot.c (100%)

diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 99bb9a1f6e16..1c0cd3b1c7c3 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -6,13 +6,7 @@
 obj-$(CONFIG_MTD)		+= mtd.o
 mtd-y				:= mtdcore.o mtdsuper.o mtdconcat.o mtdpart.o mtdchar.o
 
-obj-$(CONFIG_MTD_OF_PARTS)	+= ofpart.o
-obj-$(CONFIG_MTD_REDBOOT_PARTS) += redboot.o
-obj-$(CONFIG_MTD_CMDLINE_PARTS) += cmdlinepart.o
-obj-$(CONFIG_MTD_AFS_PARTS)	+= afs.o
-obj-$(CONFIG_MTD_AR7_PARTS)	+= ar7part.o
-obj-$(CONFIG_MTD_BCM63XX_PARTS)	+= bcm63xxpart.o
-obj-$(CONFIG_MTD_BCM47XX_PARTS)	+= bcm47xxpart.o
+obj-y				+= partitions/
 
 # 'Users' - code which presents functionality to userspace.
 obj-$(CONFIG_MTD_BLKDEVS)	+= mtd_blkdevs.o
diff --git a/drivers/mtd/partitions/Makefile b/drivers/mtd/partitions/Makefile
new file mode 100644
index 000000000000..89822f2bfa59
--- /dev/null
+++ b/drivers/mtd/partitions/Makefile
@@ -0,0 +1,7 @@
+obj-$(CONFIG_MTD_OF_PARTS)	+= ofpart.o
+obj-$(CONFIG_MTD_REDBOOT_PARTS) += redboot.o
+obj-$(CONFIG_MTD_CMDLINE_PARTS) += cmdlinepart.o
+obj-$(CONFIG_MTD_AFS_PARTS)	+= afs.o
+obj-$(CONFIG_MTD_AR7_PARTS)	+= ar7part.o
+obj-$(CONFIG_MTD_BCM63XX_PARTS)	+= bcm63xxpart.o
+obj-$(CONFIG_MTD_BCM47XX_PARTS)	+= bcm47xxpart.o
diff --git a/drivers/mtd/afs.c b/drivers/mtd/partitions/afs.c
similarity index 100%
rename from drivers/mtd/afs.c
rename to drivers/mtd/partitions/afs.c
diff --git a/drivers/mtd/ar7part.c b/drivers/mtd/partitions/ar7part.c
similarity index 100%
rename from drivers/mtd/ar7part.c
rename to drivers/mtd/partitions/ar7part.c
diff --git a/drivers/mtd/bcm47xxpart.c b/drivers/mtd/partitions/bcm47xxpart.c
similarity index 100%
rename from drivers/mtd/bcm47xxpart.c
rename to drivers/mtd/partitions/bcm47xxpart.c
diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/partitions/bcm63xxpart.c
similarity index 100%
rename from drivers/mtd/bcm63xxpart.c
rename to drivers/mtd/partitions/bcm63xxpart.c
diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/partitions/cmdlinepart.c
similarity index 100%
rename from drivers/mtd/cmdlinepart.c
rename to drivers/mtd/partitions/cmdlinepart.c
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/partitions/ofpart.c
similarity index 100%
rename from drivers/mtd/ofpart.c
rename to drivers/mtd/partitions/ofpart.c
diff --git a/drivers/mtd/redboot.c b/drivers/mtd/partitions/redboot.c
similarity index 100%
rename from drivers/mtd/redboot.c
rename to drivers/mtd/partitions/redboot.c
-- 
2.6.0.rc2.230.g3dd15c0

[RFC PATCH 2/7] mtd: move partition parsers' Kconfig under a sub-menu

[Brian Norris]

For better organization.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/Kconfig            | 134 +----------------------------------------
 drivers/mtd/partitions/Kconfig | 131 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 134 insertions(+), 131 deletions(-)
 create mode 100644 drivers/mtd/partitions/Kconfig

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 42cc953309f1..a06e80d24499 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -23,137 +23,9 @@ config MTD_TESTS
 	  WARNING: some of the tests will ERASE entire MTD device which they
 	  test. Do not use these tests unless you really know what you do.
 
-config MTD_REDBOOT_PARTS
-	tristate "RedBoot partition table parsing"
-	---help---
-	  RedBoot is a ROM monitor and bootloader which deals with multiple
-	  'images' in flash devices by putting a table one of the erase
-	  blocks on the device, similar to a partition table, which gives
-	  the offsets, lengths and names of all the images stored in the
-	  flash.
-
-	  If you need code which can detect and parse this table, and register
-	  MTD 'partitions' corresponding to each image in the table, enable
-	  this option.
-
-	  You will still need the parsing functions to be called by the driver
-	  for your particular device. It won't happen automatically. The
-	  SA1100 map driver (CONFIG_MTD_SA1100) has an option for this, for
-	  example.
-
-if MTD_REDBOOT_PARTS
-
-config MTD_REDBOOT_DIRECTORY_BLOCK
-	int "Location of RedBoot partition table"
-	default "-1"
-	---help---
-	  This option is the Linux counterpart to the
-	  CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK RedBoot compile time
-	  option.
-
-	  The option specifies which Flash sectors holds the RedBoot
-	  partition table.  A zero or positive value gives an absolute
-	  erase block number. A negative value specifies a number of
-	  sectors before the end of the device.
-
-	  For example "2" means block number 2, "-1" means the last
-	  block and "-2" means the penultimate block.
-
-config MTD_REDBOOT_PARTS_UNALLOCATED
-	bool "Include unallocated flash regions"
-	help
-	  If you need to register each unallocated flash region as a MTD
-	  'partition', enable this option.
-
-config MTD_REDBOOT_PARTS_READONLY
-	bool "Force read-only for RedBoot system images"
-	help
-	  If you need to force read-only for 'RedBoot', 'RedBoot Config' and
-	  'FIS directory' images, enable this option.
-
-endif # MTD_REDBOOT_PARTS
-
-config MTD_CMDLINE_PARTS
-	tristate "Command line partition table parsing"
-	depends on MTD
-	---help---
-	  Allow generic configuration of the MTD partition tables via the kernel
-	  command line. Multiple flash resources are supported for hardware where
-	  different kinds of flash memory are available.
-
-	  You will still need the parsing functions to be called by the driver
-	  for your particular device. It won't happen automatically. The
-	  SA1100 map driver (CONFIG_MTD_SA1100) has an option for this, for
-	  example.
-
-	  The format for the command line is as follows:
-
-	  mtdparts=<mtddef>[;<mtddef]
-	  <mtddef>  := <mtd-id>:<partdef>[,<partdef>]
-	  <partdef> := <size>[@offset][<name>][ro]
-	  <mtd-id>  := unique id used in mapping driver/device
-	  <size>    := standard linux memsize OR "-" to denote all
-	  remaining space
-	  <name>    := (NAME)
-
-	  Due to the way Linux handles the command line, no spaces are
-	  allowed in the partition definition, including mtd id's and partition
-	  names.
-
-	  Examples:
-
-	  1 flash resource (mtd-id "sa1100"), with 1 single writable partition:
-	  mtdparts=sa1100:-
-
-	  Same flash, but 2 named partitions, the first one being read-only:
-	  mtdparts=sa1100:256k(ARMboot)ro,-(root)
-
-	  If unsure, say 'N'.
-
-config MTD_AFS_PARTS
-	tristate "ARM Firmware Suite partition parsing"
-	depends on (ARM || ARM64)
-	---help---
-	  The ARM Firmware Suite allows the user to divide flash devices into
-	  multiple 'images'. Each such image has a header containing its name
-	  and offset/size etc.
-
-	  If you need code which can detect and parse these tables, and
-	  register MTD 'partitions' corresponding to each image detected,
-	  enable this option.
-
-	  You will still need the parsing functions to be called by the driver
-	  for your particular device. It won't happen automatically. The
-	  'physmap' map driver (CONFIG_MTD_PHYSMAP) does this, for example.
-
-config MTD_OF_PARTS
-	tristate "OpenFirmware partitioning information support"
-	default y
-	depends on OF
-	help
-	  This provides a partition parsing function which derives
-	  the partition map from the children of the flash node,
-	  as described in Documentation/devicetree/bindings/mtd/partition.txt.
-
-config MTD_AR7_PARTS
-	tristate "TI AR7 partitioning support"
-	---help---
-	  TI AR7 partitioning support
-
-config MTD_BCM63XX_PARTS
-	tristate "BCM63XX CFE partitioning support"
-	depends on BCM63XX
-	select CRC32
-	help
-	  This provides partions parsing for BCM63xx devices with CFE
-	  bootloaders.
-
-config MTD_BCM47XX_PARTS
-	tristate "BCM47XX partitioning support"
-	depends on BCM47XX || ARCH_BCM_5301X
-	help
-	  This provides partitions parser for devices based on BCM47xx
-	  boards.
+menu "Partition Parsers"
+source "drivers/mtd/partitions/Kconfig"
+endmenu
 
 comment "User Modules And Translation Layers"
 
diff --git a/drivers/mtd/partitions/Kconfig b/drivers/mtd/partitions/Kconfig
new file mode 100644
index 000000000000..0827d7a8be4e
--- /dev/null
+++ b/drivers/mtd/partitions/Kconfig
@@ -0,0 +1,131 @@
+config MTD_REDBOOT_PARTS
+	tristate "RedBoot partition table parsing"
+	---help---
+	  RedBoot is a ROM monitor and bootloader which deals with multiple
+	  'images' in flash devices by putting a table one of the erase
+	  blocks on the device, similar to a partition table, which gives
+	  the offsets, lengths and names of all the images stored in the
+	  flash.
+
+	  If you need code which can detect and parse this table, and register
+	  MTD 'partitions' corresponding to each image in the table, enable
+	  this option.
+
+	  You will still need the parsing functions to be called by the driver
+	  for your particular device. It won't happen automatically. The
+	  SA1100 map driver (CONFIG_MTD_SA1100) has an option for this, for
+	  example.
+
+if MTD_REDBOOT_PARTS
+
+config MTD_REDBOOT_DIRECTORY_BLOCK
+	int "Location of RedBoot partition table"
+	default "-1"
+	---help---
+	  This option is the Linux counterpart to the
+	  CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK RedBoot compile time
+	  option.
+
+	  The option specifies which Flash sectors holds the RedBoot
+	  partition table.  A zero or positive value gives an absolute
+	  erase block number. A negative value specifies a number of
+	  sectors before the end of the device.
+
+	  For example "2" means block number 2, "-1" means the last
+	  block and "-2" means the penultimate block.
+
+config MTD_REDBOOT_PARTS_UNALLOCATED
+	bool "Include unallocated flash regions"
+	help
+	  If you need to register each unallocated flash region as a MTD
+	  'partition', enable this option.
+
+config MTD_REDBOOT_PARTS_READONLY
+	bool "Force read-only for RedBoot system images"
+	help
+	  If you need to force read-only for 'RedBoot', 'RedBoot Config' and
+	  'FIS directory' images, enable this option.
+
+endif # MTD_REDBOOT_PARTS
+
+config MTD_CMDLINE_PARTS
+	tristate "Command line partition table parsing"
+	depends on MTD
+	---help---
+	  Allow generic configuration of the MTD partition tables via the kernel
+	  command line. Multiple flash resources are supported for hardware where
+	  different kinds of flash memory are available.
+
+	  You will still need the parsing functions to be called by the driver
+	  for your particular device. It won't happen automatically. The
+	  SA1100 map driver (CONFIG_MTD_SA1100) has an option for this, for
+	  example.
+
+	  The format for the command line is as follows:
+
+	  mtdparts=<mtddef>[;<mtddef]
+	  <mtddef>  := <mtd-id>:<partdef>[,<partdef>]
+	  <partdef> := <size>[@offset][<name>][ro]
+	  <mtd-id>  := unique id used in mapping driver/device
+	  <size>    := standard linux memsize OR "-" to denote all
+	  remaining space
+	  <name>    := (NAME)
+
+	  Due to the way Linux handles the command line, no spaces are
+	  allowed in the partition definition, including mtd id's and partition
+	  names.
+
+	  Examples:
+
+	  1 flash resource (mtd-id "sa1100"), with 1 single writable partition:
+	  mtdparts=sa1100:-
+
+	  Same flash, but 2 named partitions, the first one being read-only:
+	  mtdparts=sa1100:256k(ARMboot)ro,-(root)
+
+	  If unsure, say 'N'.
+
+config MTD_AFS_PARTS
+	tristate "ARM Firmware Suite partition parsing"
+	depends on (ARM || ARM64)
+	---help---
+	  The ARM Firmware Suite allows the user to divide flash devices into
+	  multiple 'images'. Each such image has a header containing its name
+	  and offset/size etc.
+
+	  If you need code which can detect and parse these tables, and
+	  register MTD 'partitions' corresponding to each image detected,
+	  enable this option.
+
+	  You will still need the parsing functions to be called by the driver
+	  for your particular device. It won't happen automatically. The
+	  'physmap' map driver (CONFIG_MTD_PHYSMAP) does this, for example.
+
+config MTD_OF_PARTS
+	tristate "OpenFirmware partitioning information support"
+	default y
+	depends on OF
+	help
+	  This provides a partition parsing function which derives
+	  the partition map from the children of the flash node,
+	  as described in Documentation/devicetree/bindings/mtd/partition.txt.
+
+config MTD_AR7_PARTS
+	tristate "TI AR7 partitioning support"
+	---help---
+	  TI AR7 partitioning support
+
+config MTD_BCM63XX_PARTS
+	tristate "BCM63XX CFE partitioning support"
+	depends on BCM63XX
+	select CRC32
+	help
+	  This provides partions parsing for BCM63xx devices with CFE
+	  bootloaders.
+
+config MTD_BCM47XX_PARTS
+	tristate "BCM47XX partitioning support"
+	depends on BCM47XX || ARCH_BCM_5301X
+	help
+	  This provides partitions parser for devices based on BCM47xx
+	  boards.
-- 
2.6.0.rc2.230.g3dd15c0

[RFC PATCH 2/7] mtd: move partition parsers' Kconfig under a sub-menu

[Brian Norris]

For better organization.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/Kconfig            | 134 +----------------------------------------
 drivers/mtd/partitions/Kconfig | 131 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 134 insertions(+), 131 deletions(-)
 create mode 100644 drivers/mtd/partitions/Kconfig

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 42cc953309f1..a06e80d24499 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -23,137 +23,9 @@ config MTD_TESTS
 	  WARNING: some of the tests will ERASE entire MTD device which they
 	  test. Do not use these tests unless you really know what you do.
 
-config MTD_REDBOOT_PARTS
-	tristate "RedBoot partition table parsing"
-	---help---
-	  RedBoot is a ROM monitor and bootloader which deals with multiple
-	  'images' in flash devices by putting a table one of the erase
-	  blocks on the device, similar to a partition table, which gives
-	  the offsets, lengths and names of all the images stored in the
-	  flash.
-
-	  If you need code which can detect and parse this table, and register
-	  MTD 'partitions' corresponding to each image in the table, enable
-	  this option.
-
-	  You will still need the parsing functions to be called by the driver
-	  for your particular device. It won't happen automatically. The
-	  SA1100 map driver (CONFIG_MTD_SA1100) has an option for this, for
-	  example.
-
-if MTD_REDBOOT_PARTS
-
-config MTD_REDBOOT_DIRECTORY_BLOCK
-	int "Location of RedBoot partition table"
-	default "-1"
-	---help---
-	  This option is the Linux counterpart to the
-	  CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK RedBoot compile time
-	  option.
-
-	  The option specifies which Flash sectors holds the RedBoot
-	  partition table.  A zero or positive value gives an absolute
-	  erase block number. A negative value specifies a number of
-	  sectors before the end of the device.
-
-	  For example "2" means block number 2, "-1" means the last
-	  block and "-2" means the penultimate block.
-
-config MTD_REDBOOT_PARTS_UNALLOCATED
-	bool "Include unallocated flash regions"
-	help
-	  If you need to register each unallocated flash region as a MTD
-	  'partition', enable this option.
-
-config MTD_REDBOOT_PARTS_READONLY
-	bool "Force read-only for RedBoot system images"
-	help
-	  If you need to force read-only for 'RedBoot', 'RedBoot Config' and
-	  'FIS directory' images, enable this option.
-
-endif # MTD_REDBOOT_PARTS
-
-config MTD_CMDLINE_PARTS
-	tristate "Command line partition table parsing"
-	depends on MTD
-	---help---
-	  Allow generic configuration of the MTD partition tables via the kernel
-	  command line. Multiple flash resources are supported for hardware where
-	  different kinds of flash memory are available.
-
-	  You will still need the parsing functions to be called by the driver
-	  for your particular device. It won't happen automatically. The
-	  SA1100 map driver (CONFIG_MTD_SA1100) has an option for this, for
-	  example.
-
-	  The format for the command line is as follows:
-
-	  mtdparts=<mtddef>[;<mtddef]
-	  <mtddef>  := <mtd-id>:<partdef>[,<partdef>]
-	  <partdef> := <size>[@offset][<name>][ro]
-	  <mtd-id>  := unique id used in mapping driver/device
-	  <size>    := standard linux memsize OR "-" to denote all
-	  remaining space
-	  <name>    := (NAME)
-
-	  Due to the way Linux handles the command line, no spaces are
-	  allowed in the partition definition, including mtd id's and partition
-	  names.
-
-	  Examples:
-
-	  1 flash resource (mtd-id "sa1100"), with 1 single writable partition:
-	  mtdparts=sa1100:-
-
-	  Same flash, but 2 named partitions, the first one being read-only:
-	  mtdparts=sa1100:256k(ARMboot)ro,-(root)
-
-	  If unsure, say 'N'.
-
-config MTD_AFS_PARTS
-	tristate "ARM Firmware Suite partition parsing"
-	depends on (ARM || ARM64)
-	---help---
-	  The ARM Firmware Suite allows the user to divide flash devices into
-	  multiple 'images'. Each such image has a header containing its name
-	  and offset/size etc.
-
-	  If you need code which can detect and parse these tables, and
-	  register MTD 'partitions' corresponding to each image detected,
-	  enable this option.
-
-	  You will still need the parsing functions to be called by the driver
-	  for your particular device. It won't happen automatically. The
-	  'physmap' map driver (CONFIG_MTD_PHYSMAP) does this, for example.
-
-config MTD_OF_PARTS
-	tristate "OpenFirmware partitioning information support"
-	default y
-	depends on OF
-	help
-	  This provides a partition parsing function which derives
-	  the partition map from the children of the flash node,
-	  as described in Documentation/devicetree/bindings/mtd/partition.txt.
-
-config MTD_AR7_PARTS
-	tristate "TI AR7 partitioning support"
-	---help---
-	  TI AR7 partitioning support
-
-config MTD_BCM63XX_PARTS
-	tristate "BCM63XX CFE partitioning support"
-	depends on BCM63XX
-	select CRC32
-	help
-	  This provides partions parsing for BCM63xx devices with CFE
-	  bootloaders.
-
-config MTD_BCM47XX_PARTS
-	tristate "BCM47XX partitioning support"
-	depends on BCM47XX || ARCH_BCM_5301X
-	help
-	  This provides partitions parser for devices based on BCM47xx
-	  boards.
+menu "Partition Parsers"
+source "drivers/mtd/partitions/Kconfig"
+endmenu
 
 comment "User Modules And Translation Layers"
 
diff --git a/drivers/mtd/partitions/Kconfig b/drivers/mtd/partitions/Kconfig
new file mode 100644
index 000000000000..0827d7a8be4e
--- /dev/null
+++ b/drivers/mtd/partitions/Kconfig
@@ -0,0 +1,131 @@
+config MTD_REDBOOT_PARTS
+	tristate "RedBoot partition table parsing"
+	---help---
+	  RedBoot is a ROM monitor and bootloader which deals with multiple
+	  'images' in flash devices by putting a table one of the erase
+	  blocks on the device, similar to a partition table, which gives
+	  the offsets, lengths and names of all the images stored in the
+	  flash.
+
+	  If you need code which can detect and parse this table, and register
+	  MTD 'partitions' corresponding to each image in the table, enable
+	  this option.
+
+	  You will still need the parsing functions to be called by the driver
+	  for your particular device. It won't happen automatically. The
+	  SA1100 map driver (CONFIG_MTD_SA1100) has an option for this, for
+	  example.
+
+if MTD_REDBOOT_PARTS
+
+config MTD_REDBOOT_DIRECTORY_BLOCK
+	int "Location of RedBoot partition table"
+	default "-1"
+	---help---
+	  This option is the Linux counterpart to the
+	  CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK RedBoot compile time
+	  option.
+
+	  The option specifies which Flash sectors holds the RedBoot
+	  partition table.  A zero or positive value gives an absolute
+	  erase block number. A negative value specifies a number of
+	  sectors before the end of the device.
+
+	  For example "2" means block number 2, "-1" means the last
+	  block and "-2" means the penultimate block.
+
+config MTD_REDBOOT_PARTS_UNALLOCATED
+	bool "Include unallocated flash regions"
+	help
+	  If you need to register each unallocated flash region as a MTD
+	  'partition', enable this option.
+
+config MTD_REDBOOT_PARTS_READONLY
+	bool "Force read-only for RedBoot system images"
+	help
+	  If you need to force read-only for 'RedBoot', 'RedBoot Config' and
+	  'FIS directory' images, enable this option.
+
+endif # MTD_REDBOOT_PARTS
+
+config MTD_CMDLINE_PARTS
+	tristate "Command line partition table parsing"
+	depends on MTD
+	---help---
+	  Allow generic configuration of the MTD partition tables via the kernel
+	  command line. Multiple flash resources are supported for hardware where
+	  different kinds of flash memory are available.
+
+	  You will still need the parsing functions to be called by the driver
+	  for your particular device. It won't happen automatically. The
+	  SA1100 map driver (CONFIG_MTD_SA1100) has an option for this, for
+	  example.
+
+	  The format for the command line is as follows:
+
+	  mtdparts=<mtddef>[;<mtddef]
+	  <mtddef>  := <mtd-id>:<partdef>[,<partdef>]
+	  <partdef> := <size>[@offset][<name>][ro]
+	  <mtd-id>  := unique id used in mapping driver/device
+	  <size>    := standard linux memsize OR "-" to denote all
+	  remaining space
+	  <name>    := (NAME)
+
+	  Due to the way Linux handles the command line, no spaces are
+	  allowed in the partition definition, including mtd id's and partition
+	  names.
+
+	  Examples:
+
+	  1 flash resource (mtd-id "sa1100"), with 1 single writable partition:
+	  mtdparts=sa1100:-
+
+	  Same flash, but 2 named partitions, the first one being read-only:
+	  mtdparts=sa1100:256k(ARMboot)ro,-(root)
+
+	  If unsure, say 'N'.
+
+config MTD_AFS_PARTS
+	tristate "ARM Firmware Suite partition parsing"
+	depends on (ARM || ARM64)
+	---help---
+	  The ARM Firmware Suite allows the user to divide flash devices into
+	  multiple 'images'. Each such image has a header containing its name
+	  and offset/size etc.
+
+	  If you need code which can detect and parse these tables, and
+	  register MTD 'partitions' corresponding to each image detected,
+	  enable this option.
+
+	  You will still need the parsing functions to be called by the driver
+	  for your particular device. It won't happen automatically. The
+	  'physmap' map driver (CONFIG_MTD_PHYSMAP) does this, for example.
+
+config MTD_OF_PARTS
+	tristate "OpenFirmware partitioning information support"
+	default y
+	depends on OF
+	help
+	  This provides a partition parsing function which derives
+	  the partition map from the children of the flash node,
+	  as described in Documentation/devicetree/bindings/mtd/partition.txt.
+
+config MTD_AR7_PARTS
+	tristate "TI AR7 partitioning support"
+	---help---
+	  TI AR7 partitioning support
+
+config MTD_BCM63XX_PARTS
+	tristate "BCM63XX CFE partitioning support"
+	depends on BCM63XX
+	select CRC32
+	help
+	  This provides partions parsing for BCM63xx devices with CFE
+	  bootloaders.
+
+config MTD_BCM47XX_PARTS
+	tristate "BCM47XX partitioning support"
+	depends on BCM47XX || ARCH_BCM_5301X
+	help
+	  This provides partitions parser for devices based on BCM47xx
+	  boards.
-- 
2.6.0.rc2.230.g3dd15c0

[RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Brian Norris]

The platform description (such as the type of partition formats used on
a given flash) should be done independently of the flash driver in use.
However, we can't reasonably have *all* partition parsers run on all
flash (until they find a match), so let's overload the 'partitions'
subnode to support specifying which format(s) to try in the device tree.

Start by supporting Google's (Chrome OS) FMAP structure.

There have been others interested in extending devicetree support to
other parsers, like the bcm47xxpart parser:

  http://patchwork.ozlabs.org/patch/475986/

and the AFS (ARM Flash Structure?) parser:

  http://patchwork.ozlabs.org/patch/537827/

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 .../devicetree/bindings/mtd/partition.txt          | 75 ++++++++++++++++++++--
 1 file changed, 69 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 28ae56f5c972..1bf9a7243993 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -1,29 +1,56 @@
-Representing flash partitions in devicetree
+Flash partitions in device tree
+===============================
 
-Partitions can be represented by sub-nodes of an mtd device. This can be used
+Flash devices can be partitioned into one or more functional ranges (e.g.,
+"boot code", "nvram", and "kernel") in at least two distinct ways:
+
+ (A) a fixed flash layout at production time or
+ (B) with an on-flash partition table, such as RedBoot, that describes the
+     geometry and naming/purpose of each functional region
+
+The former typically requires an operating system to learn about the
+partitioning from some kind of metadata provided by the bootloader/firmware.
+Such partitions can be described using the method in "Section A: Fixed Partitions".
+
+The latter is somewhat analogous to partition tables used on block devices
+(e.g., MBR or GPT), except that there is less standardization for flash
+devices, and it is not always safe or efficient to attempt to search for all of
+them on every flash device in the system, 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.
+
+To assist system software in locating these partition tables, we provide a
+binding to describe which partition format(s) may be used on a given flash,
+found below in "Section B: On-Flash Partition Tables".
+
+
+Section A: Fixed Partitions
+---------------------------
+
+Partitions can be represented by sub-nodes of a flash device. This can be used
 on platforms which have strong conventions about which portions of a flash are
 used for what purposes, but which don't use an on-flash partition table such
 as RedBoot.
 
-The partition table should be a subnode of the mtd node and should be named
+The partition table should be a subnode of the flash node and should be named
 'partitions'. This node should have the following property:
 - compatible : (required) must be "partitions"
 Partitions are then defined in subnodes of the partitions node.
 
-For backwards compatibility partitions as direct subnodes of the mtd device are
+For backwards compatibility partitions as direct subnodes of the flash device are
 supported. This use is discouraged.
 NOTE: also for backwards compatibility, direct subnodes that have a compatible
 string are not considered partitions, as they may be used for other bindings.
 
 #address-cells & #size-cells must both be present in the partitions subnode of the
-mtd device. There are two valid values for both:
+flash device. There are two valid values for both:
 <1>: for partitions that require a single 32-bit cell to represent their
      size/address (aka the value is below 4 GiB)
 <2>: for partitions that require two 32-bit cells to represent their
      size/address (aka the value is 4 GiB or greater).
 
 Required properties:
-- reg : The partition's offset and size within the mtd bank.
+- reg : The partition's offset and size within the flash
 
 Optional properties:
 - label : The label / name for this partition.  If omitted, the label is taken
@@ -89,3 +116,39 @@ flash@2 {
 		};
 	};
 };
+
+
+Section B: On-Flash Partition Tables
+------------------------------------
+
+System designers use a variety of on-flash data structures to describe the
+layout of the flash. Because it's not always optimal for system software to
+scan for every sort of data structure that might be used, one can specify which
+structure(s) might be used on a given flash using the 'partitions' subnode of
+the flash node.
+
+Node name: partitions
+Properties:
+ - compatible: (required) used to define which partition format(s) may be in
+   use on this flash may contain one or more of the strings defined in the
+   following subsections
+
+
+Google's FMAP (Flash MAP)
+#########################
+
+Often used to describe the boot flash on Chrome OS devices. Specified here:
+
+  https://github.com/dhendrix/flashmap/blob/wiki/FmapSpec.md
+
+Properties:
+- compatible: (required) must include "google,fmap"
+
+
+Examples:
+
+flash@0 {
+	partitions {
+		compatible = "google,fmap";
+	};
+};
-- 
2.6.0.rc2.230.g3dd15c0

[RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Brian Norris]

The platform description (such as the type of partition formats used on
a given flash) should be done independently of the flash driver in use.
However, we can't reasonably have *all* partition parsers run on all
flash (until they find a match), so let's overload the 'partitions'
subnode to support specifying which format(s) to try in the device tree.

Start by supporting Google's (Chrome OS) FMAP structure.

There have been others interested in extending devicetree support to
other parsers, like the bcm47xxpart parser:

  http://patchwork.ozlabs.org/patch/475986/

and the AFS (ARM Flash Structure?) parser:

  http://patchwork.ozlabs.org/patch/537827/

Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/mtd/partition.txt          | 75 ++++++++++++++++++++--
 1 file changed, 69 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 28ae56f5c972..1bf9a7243993 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -1,29 +1,56 @@
-Representing flash partitions in devicetree
+Flash partitions in device tree
+===============================
 
-Partitions can be represented by sub-nodes of an mtd device. This can be used
+Flash devices can be partitioned into one or more functional ranges (e.g.,
+"boot code", "nvram", and "kernel") in at least two distinct ways:
+
+ (A) a fixed flash layout at production time or
+ (B) with an on-flash partition table, such as RedBoot, that describes the
+     geometry and naming/purpose of each functional region
+
+The former typically requires an operating system to learn about the
+partitioning from some kind of metadata provided by the bootloader/firmware.
+Such partitions can be described using the method in "Section A: Fixed Partitions".
+
+The latter is somewhat analogous to partition tables used on block devices
+(e.g., MBR or GPT), except that there is less standardization for flash
+devices, and it is not always safe or efficient to attempt to search for all of
+them on every flash device in the system, 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.
+
+To assist system software in locating these partition tables, we provide a
+binding to describe which partition format(s) may be used on a given flash,
+found below in "Section B: On-Flash Partition Tables".
+
+
+Section A: Fixed Partitions
+---------------------------
+
+Partitions can be represented by sub-nodes of a flash device. This can be used
 on platforms which have strong conventions about which portions of a flash are
 used for what purposes, but which don't use an on-flash partition table such
 as RedBoot.
 
-The partition table should be a subnode of the mtd node and should be named
+The partition table should be a subnode of the flash node and should be named
 'partitions'. This node should have the following property:
 - compatible : (required) must be "partitions"
 Partitions are then defined in subnodes of the partitions node.
 
-For backwards compatibility partitions as direct subnodes of the mtd device are
+For backwards compatibility partitions as direct subnodes of the flash device are
 supported. This use is discouraged.
 NOTE: also for backwards compatibility, direct subnodes that have a compatible
 string are not considered partitions, as they may be used for other bindings.
 
 #address-cells & #size-cells must both be present in the partitions subnode of the
-mtd device. There are two valid values for both:
+flash device. There are two valid values for both:
 <1>: for partitions that require a single 32-bit cell to represent their
      size/address (aka the value is below 4 GiB)
 <2>: for partitions that require two 32-bit cells to represent their
      size/address (aka the value is 4 GiB or greater).
 
 Required properties:
-- reg : The partition's offset and size within the mtd bank.
+- reg : The partition's offset and size within the flash
 
 Optional properties:
 - label : The label / name for this partition.  If omitted, the label is taken
@@ -89,3 +116,39 @@ flash@2 {
 		};
 	};
 };
+
+
+Section B: On-Flash Partition Tables
+------------------------------------
+
+System designers use a variety of on-flash data structures to describe the
+layout of the flash. Because it's not always optimal for system software to
+scan for every sort of data structure that might be used, one can specify which
+structure(s) might be used on a given flash using the 'partitions' subnode of
+the flash node.
+
+Node name: partitions
+Properties:
+ - compatible: (required) used to define which partition format(s) may be in
+   use on this flash may contain one or more of the strings defined in the
+   following subsections
+
+
+Google's FMAP (Flash MAP)
+#########################
+
+Often used to describe the boot flash on Chrome OS devices. Specified here:
+
+  https://github.com/dhendrix/flashmap/blob/wiki/FmapSpec.md
+
+Properties:
+- compatible: (required) must include "google,fmap"
+
+
+Examples:
+
+flash@0 {
+	partitions {
+		compatible = "google,fmap";
+	};
+};
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Jonas Gorski]

On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> The platform description (such as the type of partition formats used on
> a given flash) should be done independently of the flash driver in use.
> However, we can't reasonably have *all* partition parsers run on all
> flash (until they find a match), so let's overload the 'partitions'
> subnode to support specifying which format(s) to try in the device tree.
>
> Start by supporting Google's (Chrome OS) FMAP structure.
>
> There have been others interested in extending devicetree support to
> other parsers, like the bcm47xxpart parser:
>
>   http://patchwork.ozlabs.org/patch/475986/
>
> and the AFS (ARM Flash Structure?) parser:
>
>   http://patchwork.ozlabs.org/patch/537827/
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  .../devicetree/bindings/mtd/partition.txt          | 75 ++++++++++++++++++++--
>  1 file changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 28ae56f5c972..1bf9a7243993 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,29 +1,56 @@
> -Representing flash partitions in devicetree
> +Flash partitions in device tree
> +===============================
>
> -Partitions can be represented by sub-nodes of an mtd device. This can be used
> +Flash devices can be partitioned into one or more functional ranges (e.g.,
> +"boot code", "nvram", and "kernel") in at least two distinct ways:
> +
> + (A) a fixed flash layout at production time or
> + (B) with an on-flash partition table, such as RedBoot, that describes the
> +     geometry and naming/purpose of each functional region
> +
> +The former typically requires an operating system to learn about the
> +partitioning from some kind of metadata provided by the bootloader/firmware.
> +Such partitions can be described using the method in "Section A: Fixed Partitions".
> +
> +The latter is somewhat analogous to partition tables used on block devices
> +(e.g., MBR or GPT), except that there is less standardization for flash
> +devices, and it is not always safe or efficient to attempt to search for all of
> +them on every flash device in the system, 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.
> +
> +To assist system software in locating these partition tables, we provide a
> +binding to describe which partition format(s) may be used on a given flash,
> +found below in "Section B: On-Flash Partition Tables".
> +
> +
> +Section A: Fixed Partitions
> +---------------------------
> +
> +Partitions can be represented by sub-nodes of a flash device. This can be used
>  on platforms which have strong conventions about which portions of a flash are
>  used for what purposes, but which don't use an on-flash partition table such
>  as RedBoot.
>
> -The partition table should be a subnode of the mtd node and should be named
> +The partition table should be a subnode of the flash node and should be named
>  'partitions'. This node should have the following property:
>  - compatible : (required) must be "partitions"
>  Partitions are then defined in subnodes of the partitions node.
>
> -For backwards compatibility partitions as direct subnodes of the mtd device are
> +For backwards compatibility partitions as direct subnodes of the flash device are
>  supported. This use is discouraged.
>  NOTE: also for backwards compatibility, direct subnodes that have a compatible
>  string are not considered partitions, as they may be used for other bindings.
>
>  #address-cells & #size-cells must both be present in the partitions subnode of the
> -mtd device. There are two valid values for both:
> +flash device. There are two valid values for both:
>  <1>: for partitions that require a single 32-bit cell to represent their
>       size/address (aka the value is below 4 GiB)
>  <2>: for partitions that require two 32-bit cells to represent their
>       size/address (aka the value is 4 GiB or greater).
>
>  Required properties:
> -- reg : The partition's offset and size within the mtd bank.
> +- reg : The partition's offset and size within the flash
>
>  Optional properties:
>  - label : The label / name for this partition.  If omitted, the label is taken
> @@ -89,3 +116,39 @@ flash@2 {
>                 };
>         };
>  };
> +
> +
> +Section B: On-Flash Partition Tables
> +------------------------------------
> +
> +System designers use a variety of on-flash data structures to describe the
> +layout of the flash. Because it's not always optimal for system software to
> +scan for every sort of data structure that might be used, one can specify which
> +structure(s) might be used on a given flash using the 'partitions' subnode of
> +the flash node.
> +
> +Node name: partitions
> +Properties:
> + - compatible: (required) used to define which partition format(s) may be in
> +   use on this flash may contain one or more of the strings defined in the
> +   following subsections
> +
> +
> +Google's FMAP (Flash MAP)
> +#########################
> +
> +Often used to describe the boot flash on Chrome OS devices. Specified here:
> +
> +  https://github.com/dhendrix/flashmap/blob/wiki/FmapSpec.md
> +
> +Properties:
> +- compatible: (required) must include "google,fmap"
> +
> +
> +Examples:
> +
> +flash@0 {
> +       partitions {
> +               compatible = "google,fmap";
> +       };
> +};

I wonder if this wouldn't be better served in a separate binding doc
with its compatible name as the filename, like we do with
driver^Whardware blocks, especially if we want to add more parsers.


Jonas

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Jonas Gorski]

On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> The platform description (such as the type of partition formats used on
> a given flash) should be done independently of the flash driver in use.
> However, we can't reasonably have *all* partition parsers run on all
> flash (until they find a match), so let's overload the 'partitions'
> subnode to support specifying which format(s) to try in the device tree.
>
> Start by supporting Google's (Chrome OS) FMAP structure.
>
> There have been others interested in extending devicetree support to
> other parsers, like the bcm47xxpart parser:
>
>   http://patchwork.ozlabs.org/patch/475986/
>
> and the AFS (ARM Flash Structure?) parser:
>
>   http://patchwork.ozlabs.org/patch/537827/
>
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/mtd/partition.txt          | 75 ++++++++++++++++++++--
>  1 file changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 28ae56f5c972..1bf9a7243993 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,29 +1,56 @@
> -Representing flash partitions in devicetree
> +Flash partitions in device tree
> +===============================
>
> -Partitions can be represented by sub-nodes of an mtd device. This can be used
> +Flash devices can be partitioned into one or more functional ranges (e.g.,
> +"boot code", "nvram", and "kernel") in at least two distinct ways:
> +
> + (A) a fixed flash layout at production time or
> + (B) with an on-flash partition table, such as RedBoot, that describes the
> +     geometry and naming/purpose of each functional region
> +
> +The former typically requires an operating system to learn about the
> +partitioning from some kind of metadata provided by the bootloader/firmware.
> +Such partitions can be described using the method in "Section A: Fixed Partitions".
> +
> +The latter is somewhat analogous to partition tables used on block devices
> +(e.g., MBR or GPT), except that there is less standardization for flash
> +devices, and it is not always safe or efficient to attempt to search for all of
> +them on every flash device in the system, 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.
> +
> +To assist system software in locating these partition tables, we provide a
> +binding to describe which partition format(s) may be used on a given flash,
> +found below in "Section B: On-Flash Partition Tables".
> +
> +
> +Section A: Fixed Partitions
> +---------------------------
> +
> +Partitions can be represented by sub-nodes of a flash device. This can be used
>  on platforms which have strong conventions about which portions of a flash are
>  used for what purposes, but which don't use an on-flash partition table such
>  as RedBoot.
>
> -The partition table should be a subnode of the mtd node and should be named
> +The partition table should be a subnode of the flash node and should be named
>  'partitions'. This node should have the following property:
>  - compatible : (required) must be "partitions"
>  Partitions are then defined in subnodes of the partitions node.
>
> -For backwards compatibility partitions as direct subnodes of the mtd device are
> +For backwards compatibility partitions as direct subnodes of the flash device are
>  supported. This use is discouraged.
>  NOTE: also for backwards compatibility, direct subnodes that have a compatible
>  string are not considered partitions, as they may be used for other bindings.
>
>  #address-cells & #size-cells must both be present in the partitions subnode of the
> -mtd device. There are two valid values for both:
> +flash device. There are two valid values for both:
>  <1>: for partitions that require a single 32-bit cell to represent their
>       size/address (aka the value is below 4 GiB)
>  <2>: for partitions that require two 32-bit cells to represent their
>       size/address (aka the value is 4 GiB or greater).
>
>  Required properties:
> -- reg : The partition's offset and size within the mtd bank.
> +- reg : The partition's offset and size within the flash
>
>  Optional properties:
>  - label : The label / name for this partition.  If omitted, the label is taken
> @@ -89,3 +116,39 @@ flash@2 {
>                 };
>         };
>  };
> +
> +
> +Section B: On-Flash Partition Tables
> +------------------------------------
> +
> +System designers use a variety of on-flash data structures to describe the
> +layout of the flash. Because it's not always optimal for system software to
> +scan for every sort of data structure that might be used, one can specify which
> +structure(s) might be used on a given flash using the 'partitions' subnode of
> +the flash node.
> +
> +Node name: partitions
> +Properties:
> + - compatible: (required) used to define which partition format(s) may be in
> +   use on this flash may contain one or more of the strings defined in the
> +   following subsections
> +
> +
> +Google's FMAP (Flash MAP)
> +#########################
> +
> +Often used to describe the boot flash on Chrome OS devices. Specified here:
> +
> +  https://github.com/dhendrix/flashmap/blob/wiki/FmapSpec.md
> +
> +Properties:
> +- compatible: (required) must include "google,fmap"
> +
> +
> +Examples:
> +
> +flash@0 {
> +       partitions {
> +               compatible = "google,fmap";
> +       };
> +};

I wonder if this wouldn't be better served in a separate binding doc
with its compatible name as the filename, like we do with
driver^Whardware blocks, especially if we want to add more parsers.


Jonas

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Michal Suchanek]

On 5 December 2015 at 12:39, Jonas Gorski <jogo@openwrt.org> wrote:
> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> <computersforpeace@gmail.com> wrote:

>> +
>> +Examples:
>> +
>> +flash@0 {
>> +       partitions {
>> +               compatible = "google,fmap";
>> +       };
>> +};
>
> I wonder if this wouldn't be better served in a separate binding doc
> with its compatible name as the filename, like we do with
> driver^Whardware blocks, especially if we want to add more parsers.


I find that *very* counter productive for bindings that go to the same
node. You have a description of a node, and then suddenly there you
have another file with another description of the same node. Totally
awesome.

Also how do you plan to write partitioning schemes with parameters
like with non-zero offset of the partition table.

Thanks

Michal

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Michal Suchanek]

On 5 December 2015 at 12:39, Jonas Gorski <jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

>> +
>> +Examples:
>> +
>> +flash@0 {
>> +       partitions {
>> +               compatible = "google,fmap";
>> +       };
>> +};
>
> I wonder if this wouldn't be better served in a separate binding doc
> with its compatible name as the filename, like we do with
> driver^Whardware blocks, especially if we want to add more parsers.


I find that *very* counter productive for bindings that go to the same
node. You have a description of a node, and then suddenly there you
have another file with another description of the same node. Totally
awesome.

Also how do you plan to write partitioning schemes with parameters
like with non-zero offset of the partition table.

Thanks

Michal

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]

On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
> On 5 December 2015 at 12:39, Jonas Gorski <jogo@openwrt.org> wrote:
> > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> > <computersforpeace@gmail.com> wrote:
> 
> >> +
> >> +Examples:
> >> +
> >> +flash@0 {
> >> +       partitions {
> >> +               compatible = "google,fmap";
> >> +       };
> >> +};
> >
> > I wonder if this wouldn't be better served in a separate binding doc
> > with its compatible name as the filename, like we do with
> > driver^Whardware blocks, especially if we want to add more parsers.
> 
> 
> I find that *very* counter productive for bindings that go to the same
> node. You have a description of a node, and then suddenly there you
> have another file with another description of the same node. Totally
> awesome.

I can't actually work out from that if you're agreeing with the
original post or the first reply.

> Also how do you plan to write partitioning schemes with parameters
> like with non-zero offset of the partition table.

Presumably with properties in the patitions node.  Not seeing the
problem here.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1404 bytes --]

On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
> On 5 December 2015 at 12:39, Jonas Gorski <jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
> > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> > <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> >> +
> >> +Examples:
> >> +
> >> +flash@0 {
> >> +       partitions {
> >> +               compatible = "google,fmap";
> >> +       };
> >> +};
> >
> > I wonder if this wouldn't be better served in a separate binding doc
> > with its compatible name as the filename, like we do with
> > driver^Whardware blocks, especially if we want to add more parsers.
> 
> 
> I find that *very* counter productive for bindings that go to the same
> node. You have a description of a node, and then suddenly there you
> have another file with another description of the same node. Totally
> awesome.

I can't actually work out from that if you're agreeing with the
original post or the first reply.

> Also how do you plan to write partitioning schemes with parameters
> like with non-zero offset of the partition table.

Presumably with properties in the patitions node.  Not seeing the
problem here.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Brian Norris]

On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
> On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
> > On 5 December 2015 at 12:39, Jonas Gorski <jogo@openwrt.org> wrote:
> > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> > > <computersforpeace@gmail.com> wrote:
> > 
> > >> +
> > >> +Examples:
> > >> +
> > >> +flash@0 {
> > >> +       partitions {
> > >> +               compatible = "google,fmap";
> > >> +       };
> > >> +};
> > >
> > > I wonder if this wouldn't be better served in a separate binding doc
> > > with its compatible name as the filename, like we do with
> > > driver^Whardware blocks, especially if we want to add more parsers.
> > 
> > 
> > I find that *very* counter productive for bindings that go to the same
> > node. You have a description of a node, and then suddenly there you
> > have another file with another description of the same node. Totally
> > awesome.
> 
> I can't actually work out from that if you're agreeing with the
> original post or the first reply.

Perhaps I'm biased, but I think he was agreeing with the first reply.
(Particularly, "I find that *very* counter productive" uses the word
"that" to refer to "separate binding doc[s]".)

> > Also how do you plan to write partitioning schemes with parameters
> > like with non-zero offset of the partition table.

If you are directing this question at me: I don't have a specific plan
for it. MTD parsers don't currently take external input for this; many
scan the whole device, but some might also have conventions built into
the parser itself too, so this just gets hooked based on "compatible".
But if the need arose, I would hope we could work out a common binding.

> Presumably with properties in the patitions node.  Not seeing the
> problem here.

I believe Michal is bringing up the (important, IMO) point that if
distinct partition types are being described in the same node, then any
use of additional properties *must* be closely coordinated. We can't
have two parsers "foo" and "bar" defining conflicting uses of the same
property in the same node, like this:

	partitions {
		compatible = "foo", "bar";
		property-baz = ...; // e.g., reg = <...>;
	};

where if "foo" is not found, we fall back to "bar". But what if "foo"
and "bar" use "property-baz" differently?

Having everything in one doc would help ensure that the entire
"partitions" binding is considered as a whole when extending it, in my
(and an in my interpretation of Michal's) opinion.

Brian

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Brian Norris]

On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
> On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
> > On 5 December 2015 at 12:39, Jonas Gorski <jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
> > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> > > <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > 
> > >> +
> > >> +Examples:
> > >> +
> > >> +flash@0 {
> > >> +       partitions {
> > >> +               compatible = "google,fmap";
> > >> +       };
> > >> +};
> > >
> > > I wonder if this wouldn't be better served in a separate binding doc
> > > with its compatible name as the filename, like we do with
> > > driver^Whardware blocks, especially if we want to add more parsers.
> > 
> > 
> > I find that *very* counter productive for bindings that go to the same
> > node. You have a description of a node, and then suddenly there you
> > have another file with another description of the same node. Totally
> > awesome.
> 
> I can't actually work out from that if you're agreeing with the
> original post or the first reply.

Perhaps I'm biased, but I think he was agreeing with the first reply.
(Particularly, "I find that *very* counter productive" uses the word
"that" to refer to "separate binding doc[s]".)

> > Also how do you plan to write partitioning schemes with parameters
> > like with non-zero offset of the partition table.

If you are directing this question at me: I don't have a specific plan
for it. MTD parsers don't currently take external input for this; many
scan the whole device, but some might also have conventions built into
the parser itself too, so this just gets hooked based on "compatible".
But if the need arose, I would hope we could work out a common binding.

> Presumably with properties in the patitions node.  Not seeing the
> problem here.

I believe Michal is bringing up the (important, IMO) point that if
distinct partition types are being described in the same node, then any
use of additional properties *must* be closely coordinated. We can't
have two parsers "foo" and "bar" defining conflicting uses of the same
property in the same node, like this:

	partitions {
		compatible = "foo", "bar";
		property-baz = ...; // e.g., reg = <...>;
	};

where if "foo" is not found, we fall back to "bar". But what if "foo"
and "bar" use "property-baz" differently?

Having everything in one doc would help ensure that the entire
"partitions" binding is considered as a whole when extending it, in my
(and an in my interpretation of Michal's) opinion.

Brian

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Michal Suchanek]

Hello,

On 10 December 2015 at 21:43, Brian Norris <computersforpeace@gmail.com> wrote:
> On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
>> On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
>> > On 5 December 2015 at 12:39, Jonas Gorski <jogo@openwrt.org> wrote:
>> > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>> > > <computersforpeace@gmail.com> wrote:
>> >
>> > >> +
>> > >> +Examples:
>> > >> +
>> > >> +flash@0 {
>> > >> +       partitions {
>> > >> +               compatible = "google,fmap";
>> > >> +       };
>> > >> +};
>> > >
>> > > I wonder if this wouldn't be better served in a separate binding doc
>> > > with its compatible name as the filename, like we do with
>> > > driver^Whardware blocks, especially if we want to add more parsers.
>> >
>> >
>> > I find that *very* counter productive for bindings that go to the same
>> > node. You have a description of a node, and then suddenly there you
>> > have another file with another description of the same node. Totally
>> > awesome.
>>
>> I can't actually work out from that if you're agreeing with the
>> original post or the first reply.
>
> Perhaps I'm biased, but I think he was agreeing with the first reply.
> (Particularly, "I find that *very* counter productive" uses the word
> "that" to refer to "separate binding doc[s]".)
>
>
> I believe Michal is bringing up the (important, IMO) point that if
> distinct partition types are being described in the same node, then any
> use of additional properties *must* be closely coordinated. We can't
> have two parsers "foo" and "bar" defining conflicting uses of the same
> property in the same node, like this:
>

I have seen some MFD bindings which are described in multiple files
with no crossreference whatsoever.

If on-flash partition table bindings are going to be the same then
people are not going to find there are even some on-flash partition
table bindings (because the document describing the in-DT bindings is
complete and exhaustive, right). Can't even imagine coordination.

When you have to grep the tree for docs anyway what's even the point
of the Documentation directory?

You can just grep the whole tree.

Thanks

Michal

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Michal Suchanek]

Hello,

On 10 December 2015 at 21:43, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
>> On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
>> > On 5 December 2015 at 12:39, Jonas Gorski <jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
>> > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>> > > <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >
>> > >> +
>> > >> +Examples:
>> > >> +
>> > >> +flash@0 {
>> > >> +       partitions {
>> > >> +               compatible = "google,fmap";
>> > >> +       };
>> > >> +};
>> > >
>> > > I wonder if this wouldn't be better served in a separate binding doc
>> > > with its compatible name as the filename, like we do with
>> > > driver^Whardware blocks, especially if we want to add more parsers.
>> >
>> >
>> > I find that *very* counter productive for bindings that go to the same
>> > node. You have a description of a node, and then suddenly there you
>> > have another file with another description of the same node. Totally
>> > awesome.
>>
>> I can't actually work out from that if you're agreeing with the
>> original post or the first reply.
>
> Perhaps I'm biased, but I think he was agreeing with the first reply.
> (Particularly, "I find that *very* counter productive" uses the word
> "that" to refer to "separate binding doc[s]".)
>
>
> I believe Michal is bringing up the (important, IMO) point that if
> distinct partition types are being described in the same node, then any
> use of additional properties *must* be closely coordinated. We can't
> have two parsers "foo" and "bar" defining conflicting uses of the same
> property in the same node, like this:
>

I have seen some MFD bindings which are described in multiple files
with no crossreference whatsoever.

If on-flash partition table bindings are going to be the same then
people are not going to find there are even some on-flash partition
table bindings (because the document describing the in-DT bindings is
complete and exhaustive, right). Can't even imagine coordination.

When you have to grep the tree for docs anyway what's even the point
of the Documentation directory?

You can just grep the whole tree.

Thanks

Michal

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3317 bytes --]

On Thu, Dec 10, 2015 at 12:43:24PM -0800, Brian Norris wrote:
> On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
> > On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
> > > On 5 December 2015 at 12:39, Jonas Gorski <jogo@openwrt.org> wrote:
> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> > > > <computersforpeace@gmail.com> wrote:
> > > 
> > > >> +
> > > >> +Examples:
> > > >> +
> > > >> +flash@0 {
> > > >> +       partitions {
> > > >> +               compatible = "google,fmap";
> > > >> +       };
> > > >> +};
> > > >
> > > > I wonder if this wouldn't be better served in a separate binding doc
> > > > with its compatible name as the filename, like we do with
> > > > driver^Whardware blocks, especially if we want to add more parsers.
> > > 
> > > 
> > > I find that *very* counter productive for bindings that go to the same
> > > node. You have a description of a node, and then suddenly there you
> > > have another file with another description of the same node. Totally
> > > awesome.
> > 
> > I can't actually work out from that if you're agreeing with the
> > original post or the first reply.
> 
> Perhaps I'm biased, but I think he was agreeing with the first reply.
> (Particularly, "I find that *very* counter productive" uses the word
> "that" to refer to "separate binding doc[s]".)
> 
> > > Also how do you plan to write partitioning schemes with parameters
> > > like with non-zero offset of the partition table.
> 
> If you are directing this question at me: I don't have a specific plan
> for it. MTD parsers don't currently take external input for this; many
> scan the whole device, but some might also have conventions built into
> the parser itself too, so this just gets hooked based on "compatible".
> But if the need arose, I would hope we could work out a common binding.
> 
> > Presumably with properties in the patitions node.  Not seeing the
> > problem here.
> 
> I believe Michal is bringing up the (important, IMO) point that if
> distinct partition types are being described in the same node, then any
> use of additional properties *must* be closely coordinated. We can't
> have two parsers "foo" and "bar" defining conflicting uses of the same
> property in the same node, like this:
> 
> 	partitions {
> 		compatible = "foo", "bar";
> 		property-baz = ...; // e.g., reg = <...>;
> 	};
> 
> where if "foo" is not found, we fall back to "bar". But what if "foo"
> and "bar" use "property-baz" differently?

Ah.. that is an excellent point, and leads me to realise that using
compatible in this way is wrong.  The whole point of compatible is
that the node is, well, compatible with *all* the things in the list,
and therefore the things in the list are compatible with each other.

Using it for a list of entirely different things to attempt in order
is not correct.


> Having everything in one doc would help ensure that the entire
> "partitions" binding is considered as a whole when extending it, in my
> (and an in my interpretation of Michal's) opinion.
> 
> Brian

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Geert Uytterhoeven]

On Sat, Dec 12, 2015 at 6:51 AM, David Gibson
<david@gibson.dropbear.id.au> wrote:
> On Thu, Dec 10, 2015 at 12:43:24PM -0800, Brian Norris wrote:
>> On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
>> > On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
>> > > On 5 December 2015 at 12:39, Jonas Gorski <jogo@openwrt.org> wrote:
>> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>> > > > <computersforpeace@gmail.com> wrote:
>> > >
>> > > >> +
>> > > >> +Examples:
>> > > >> +
>> > > >> +flash@0 {
>> > > >> +       partitions {
>> > > >> +               compatible = "google,fmap";
>> > > >> +       };
>> > > >> +};
>> > > >
>> > > > I wonder if this wouldn't be better served in a separate binding doc
>> > > > with its compatible name as the filename, like we do with
>> > > > driver^Whardware blocks, especially if we want to add more parsers.
>> > >
>> > >
>> > > I find that *very* counter productive for bindings that go to the same
>> > > node. You have a description of a node, and then suddenly there you
>> > > have another file with another description of the same node. Totally
>> > > awesome.
>> >
>> > I can't actually work out from that if you're agreeing with the
>> > original post or the first reply.
>>
>> Perhaps I'm biased, but I think he was agreeing with the first reply.
>> (Particularly, "I find that *very* counter productive" uses the word
>> "that" to refer to "separate binding doc[s]".)
>>
>> > > Also how do you plan to write partitioning schemes with parameters
>> > > like with non-zero offset of the partition table.
>>
>> If you are directing this question at me: I don't have a specific plan
>> for it. MTD parsers don't currently take external input for this; many
>> scan the whole device, but some might also have conventions built into
>> the parser itself too, so this just gets hooked based on "compatible".
>> But if the need arose, I would hope we could work out a common binding.
>>
>> > Presumably with properties in the patitions node.  Not seeing the
>> > problem here.
>>
>> I believe Michal is bringing up the (important, IMO) point that if
>> distinct partition types are being described in the same node, then any
>> use of additional properties *must* be closely coordinated. We can't
>> have two parsers "foo" and "bar" defining conflicting uses of the same
>> property in the same node, like this:
>>
>>       partitions {
>>               compatible = "foo", "bar";
>>               property-baz = ...; // e.g., reg = <...>;
>>       };
>>
>> where if "foo" is not found, we fall back to "bar". But what if "foo"
>> and "bar" use "property-baz" differently?
>
> Ah.. that is an excellent point, and leads me to realise that using
> compatible in this way is wrong.  The whole point of compatible is
> that the node is, well, compatible with *all* the things in the list,
> and therefore the things in the list are compatible with each other.
>
> Using it for a list of entirely different things to attempt in order
> is not correct.

Isn't the idea behind a partition table that all partition information is
stored on the device in a well-known format, so you don't need additional
properties?

If the only property needed is the partition table offset, it can be encoded
in the unit-address, and the "reg" property:

        partitions {

                partition-table@xxxx {
                        reg = <0xxxx ...>;
                        ...
                };

                ...
        };

If you do need additional properties, you'll have to add separate partition
table nodes.
Where? Outside the "partitions" subnode?
What with multiple partition tables
  - some needing properties (outside "partitions"),
  - others not (outside/inside "partitions"),
  - others needing the offset (inside "partitions"?

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

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Geert Uytterhoeven]

On Sat, Dec 12, 2015 at 6:51 AM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Thu, Dec 10, 2015 at 12:43:24PM -0800, Brian Norris wrote:
>> On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
>> > On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
>> > > On 5 December 2015 at 12:39, Jonas Gorski <jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
>> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>> > > > <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > >
>> > > >> +
>> > > >> +Examples:
>> > > >> +
>> > > >> +flash@0 {
>> > > >> +       partitions {
>> > > >> +               compatible = "google,fmap";
>> > > >> +       };
>> > > >> +};
>> > > >
>> > > > I wonder if this wouldn't be better served in a separate binding doc
>> > > > with its compatible name as the filename, like we do with
>> > > > driver^Whardware blocks, especially if we want to add more parsers.
>> > >
>> > >
>> > > I find that *very* counter productive for bindings that go to the same
>> > > node. You have a description of a node, and then suddenly there you
>> > > have another file with another description of the same node. Totally
>> > > awesome.
>> >
>> > I can't actually work out from that if you're agreeing with the
>> > original post or the first reply.
>>
>> Perhaps I'm biased, but I think he was agreeing with the first reply.
>> (Particularly, "I find that *very* counter productive" uses the word
>> "that" to refer to "separate binding doc[s]".)
>>
>> > > Also how do you plan to write partitioning schemes with parameters
>> > > like with non-zero offset of the partition table.
>>
>> If you are directing this question at me: I don't have a specific plan
>> for it. MTD parsers don't currently take external input for this; many
>> scan the whole device, but some might also have conventions built into
>> the parser itself too, so this just gets hooked based on "compatible".
>> But if the need arose, I would hope we could work out a common binding.
>>
>> > Presumably with properties in the patitions node.  Not seeing the
>> > problem here.
>>
>> I believe Michal is bringing up the (important, IMO) point that if
>> distinct partition types are being described in the same node, then any
>> use of additional properties *must* be closely coordinated. We can't
>> have two parsers "foo" and "bar" defining conflicting uses of the same
>> property in the same node, like this:
>>
>>       partitions {
>>               compatible = "foo", "bar";
>>               property-baz = ...; // e.g., reg = <...>;
>>       };
>>
>> where if "foo" is not found, we fall back to "bar". But what if "foo"
>> and "bar" use "property-baz" differently?
>
> Ah.. that is an excellent point, and leads me to realise that using
> compatible in this way is wrong.  The whole point of compatible is
> that the node is, well, compatible with *all* the things in the list,
> and therefore the things in the list are compatible with each other.
>
> Using it for a list of entirely different things to attempt in order
> is not correct.

Isn't the idea behind a partition table that all partition information is
stored on the device in a well-known format, so you don't need additional
properties?

If the only property needed is the partition table offset, it can be encoded
in the unit-address, and the "reg" property:

        partitions {

                partition-table@xxxx {
                        reg = <0xxxx ...>;
                        ...
                };

                ...
        };

If you do need additional properties, you'll have to add separate partition
table nodes.
Where? Outside the "partitions" subnode?
What with multiple partition tables
  - some needing properties (outside "partitions"),
  - others not (outside/inside "partitions"),
  - others needing the offset (inside "partitions"?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Michal Suchanek]

On 14 December 2015 at 11:22, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sat, Dec 12, 2015 at 6:51 AM, David Gibson
> <david@gibson.dropbear.id.au> wrote:
>> On Thu, Dec 10, 2015 at 12:43:24PM -0800, Brian Norris wrote:
>>> On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
>>> > On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
>>> > > On 5 December 2015 at 12:39, Jonas Gorski <jogo@openwrt.org> wrote:
>>> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>>> > > > <computersforpeace@gmail.com> wrote:
>>> > >
>>> > > >> +
>>> > > >> +Examples:
>>> > > >> +
>>> > > >> +flash@0 {
>>> > > >> +       partitions {
>>> > > >> +               compatible = "google,fmap";
>>> > > >> +       };
>>> > > >> +};
>>> > > >
>>> > > > I wonder if this wouldn't be better served in a separate binding doc
>>> > > > with its compatible name as the filename, like we do with
>>> > > > driver^Whardware blocks, especially if we want to add more parsers.
>>> > >
>>> > >
>>> > > I find that *very* counter productive for bindings that go to the same
>>> > > node. You have a description of a node, and then suddenly there you
>>> > > have another file with another description of the same node. Totally
>>> > > awesome.
>>> >
>>> > I can't actually work out from that if you're agreeing with the
>>> > original post or the first reply.
>>>
>>> Perhaps I'm biased, but I think he was agreeing with the first reply.
>>> (Particularly, "I find that *very* counter productive" uses the word
>>> "that" to refer to "separate binding doc[s]".)
>>>
>>> > > Also how do you plan to write partitioning schemes with parameters
>>> > > like with non-zero offset of the partition table.
>>>
>>> If you are directing this question at me: I don't have a specific plan
>>> for it. MTD parsers don't currently take external input for this; many
>>> scan the whole device, but some might also have conventions built into
>>> the parser itself too, so this just gets hooked based on "compatible".
>>> But if the need arose, I would hope we could work out a common binding.
>>>
>>> > Presumably with properties in the patitions node.  Not seeing the
>>> > problem here.
>>>
>>> I believe Michal is bringing up the (important, IMO) point that if
>>> distinct partition types are being described in the same node, then any
>>> use of additional properties *must* be closely coordinated. We can't
>>> have two parsers "foo" and "bar" defining conflicting uses of the same
>>> property in the same node, like this:
>>>
>>>       partitions {
>>>               compatible = "foo", "bar";
>>>               property-baz = ...; // e.g., reg = <...>;
>>>       };
>>>
>>> where if "foo" is not found, we fall back to "bar". But what if "foo"
>>> and "bar" use "property-baz" differently?
>>
>> Ah.. that is an excellent point, and leads me to realise that using
>> compatible in this way is wrong.  The whole point of compatible is
>> that the node is, well, compatible with *all* the things in the list,
>> and therefore the things in the list are compatible with each other.
>>
>> Using it for a list of entirely different things to attempt in order
>> is not correct.
>
> Isn't the idea behind a partition table that all partition information is
> stored on the device in a well-known format, so you don't need additional
> properties?
>
> If the only property needed is the partition table offset, it can be encoded
> in the unit-address, and the "reg" property:
>
>         partitions {
>
>                 partition-table@xxxx {
>                         reg = <0xxxx ...>;
>                         ...
>                 };
>
>                 ...
>         };
>
> If you do need additional properties, you'll have to add separate partition
> table nodes.
> Where? Outside the "partitions" subnode?
> What with multiple partition tables
>   - some needing properties (outside "partitions"),
>   - others not (outside/inside "partitions"),
>   - others needing the offset (inside "partitions"?

Hello,

IIRC there is RedBoot on-flash partition format support in kernel
which has the partition table offset as compile time configuration
parameter. Moving this to DT would make unified kernel for boards with
different RedBoot table offsets possible.

I don't use any board with RedBoot so I would not know if there is
other issue preventing such unified kernel.

It might be an actual use case for both the binding and the extra
argument with current partitioning scheme, though.

Thanks

Michal

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Michal Suchanek]

On 14 December 2015 at 11:22, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Sat, Dec 12, 2015 at 6:51 AM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>> On Thu, Dec 10, 2015 at 12:43:24PM -0800, Brian Norris wrote:
>>> On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
>>> > On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
>>> > > On 5 December 2015 at 12:39, Jonas Gorski <jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
>>> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>>> > > > <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> > >
>>> > > >> +
>>> > > >> +Examples:
>>> > > >> +
>>> > > >> +flash@0 {
>>> > > >> +       partitions {
>>> > > >> +               compatible = "google,fmap";
>>> > > >> +       };
>>> > > >> +};
>>> > > >
>>> > > > I wonder if this wouldn't be better served in a separate binding doc
>>> > > > with its compatible name as the filename, like we do with
>>> > > > driver^Whardware blocks, especially if we want to add more parsers.
>>> > >
>>> > >
>>> > > I find that *very* counter productive for bindings that go to the same
>>> > > node. You have a description of a node, and then suddenly there you
>>> > > have another file with another description of the same node. Totally
>>> > > awesome.
>>> >
>>> > I can't actually work out from that if you're agreeing with the
>>> > original post or the first reply.
>>>
>>> Perhaps I'm biased, but I think he was agreeing with the first reply.
>>> (Particularly, "I find that *very* counter productive" uses the word
>>> "that" to refer to "separate binding doc[s]".)
>>>
>>> > > Also how do you plan to write partitioning schemes with parameters
>>> > > like with non-zero offset of the partition table.
>>>
>>> If you are directing this question at me: I don't have a specific plan
>>> for it. MTD parsers don't currently take external input for this; many
>>> scan the whole device, but some might also have conventions built into
>>> the parser itself too, so this just gets hooked based on "compatible".
>>> But if the need arose, I would hope we could work out a common binding.
>>>
>>> > Presumably with properties in the patitions node.  Not seeing the
>>> > problem here.
>>>
>>> I believe Michal is bringing up the (important, IMO) point that if
>>> distinct partition types are being described in the same node, then any
>>> use of additional properties *must* be closely coordinated. We can't
>>> have two parsers "foo" and "bar" defining conflicting uses of the same
>>> property in the same node, like this:
>>>
>>>       partitions {
>>>               compatible = "foo", "bar";
>>>               property-baz = ...; // e.g., reg = <...>;
>>>       };
>>>
>>> where if "foo" is not found, we fall back to "bar". But what if "foo"
>>> and "bar" use "property-baz" differently?
>>
>> Ah.. that is an excellent point, and leads me to realise that using
>> compatible in this way is wrong.  The whole point of compatible is
>> that the node is, well, compatible with *all* the things in the list,
>> and therefore the things in the list are compatible with each other.
>>
>> Using it for a list of entirely different things to attempt in order
>> is not correct.
>
> Isn't the idea behind a partition table that all partition information is
> stored on the device in a well-known format, so you don't need additional
> properties?
>
> If the only property needed is the partition table offset, it can be encoded
> in the unit-address, and the "reg" property:
>
>         partitions {
>
>                 partition-table@xxxx {
>                         reg = <0xxxx ...>;
>                         ...
>                 };
>
>                 ...
>         };
>
> If you do need additional properties, you'll have to add separate partition
> table nodes.
> Where? Outside the "partitions" subnode?
> What with multiple partition tables
>   - some needing properties (outside "partitions"),
>   - others not (outside/inside "partitions"),
>   - others needing the offset (inside "partitions"?

Hello,

IIRC there is RedBoot on-flash partition format support in kernel
which has the partition table offset as compile time configuration
parameter. Moving this to DT would make unified kernel for boards with
different RedBoot table offsets possible.

I don't use any board with RedBoot so I would not know if there is
other issue preventing such unified kernel.

It might be an actual use case for both the binding and the extra
argument with current partitioning scheme, though.

Thanks

Michal

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4306 bytes --]

On Mon, Dec 14, 2015 at 11:22:46AM +0100, Geert Uytterhoeven wrote:
> On Sat, Dec 12, 2015 at 6:51 AM, David Gibson
> <david@gibson.dropbear.id.au> wrote:
> > On Thu, Dec 10, 2015 at 12:43:24PM -0800, Brian Norris wrote:
> >> On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
> >> > On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
> >> > > On 5 December 2015 at 12:39, Jonas Gorski <jogo@openwrt.org> wrote:
> >> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> >> > > > <computersforpeace@gmail.com> wrote:
> >> > >
> >> > > >> +
> >> > > >> +Examples:
> >> > > >> +
> >> > > >> +flash@0 {
> >> > > >> +       partitions {
> >> > > >> +               compatible = "google,fmap";
> >> > > >> +       };
> >> > > >> +};
> >> > > >
> >> > > > I wonder if this wouldn't be better served in a separate binding doc
> >> > > > with its compatible name as the filename, like we do with
> >> > > > driver^Whardware blocks, especially if we want to add more parsers.
> >> > >
> >> > >
> >> > > I find that *very* counter productive for bindings that go to the same
> >> > > node. You have a description of a node, and then suddenly there you
> >> > > have another file with another description of the same node. Totally
> >> > > awesome.
> >> >
> >> > I can't actually work out from that if you're agreeing with the
> >> > original post or the first reply.
> >>
> >> Perhaps I'm biased, but I think he was agreeing with the first reply.
> >> (Particularly, "I find that *very* counter productive" uses the word
> >> "that" to refer to "separate binding doc[s]".)
> >>
> >> > > Also how do you plan to write partitioning schemes with parameters
> >> > > like with non-zero offset of the partition table.
> >>
> >> If you are directing this question at me: I don't have a specific plan
> >> for it. MTD parsers don't currently take external input for this; many
> >> scan the whole device, but some might also have conventions built into
> >> the parser itself too, so this just gets hooked based on "compatible".
> >> But if the need arose, I would hope we could work out a common binding.
> >>
> >> > Presumably with properties in the patitions node.  Not seeing the
> >> > problem here.
> >>
> >> I believe Michal is bringing up the (important, IMO) point that if
> >> distinct partition types are being described in the same node, then any
> >> use of additional properties *must* be closely coordinated. We can't
> >> have two parsers "foo" and "bar" defining conflicting uses of the same
> >> property in the same node, like this:
> >>
> >>       partitions {
> >>               compatible = "foo", "bar";
> >>               property-baz = ...; // e.g., reg = <...>;
> >>       };
> >>
> >> where if "foo" is not found, we fall back to "bar". But what if "foo"
> >> and "bar" use "property-baz" differently?
> >
> > Ah.. that is an excellent point, and leads me to realise that using
> > compatible in this way is wrong.  The whole point of compatible is
> > that the node is, well, compatible with *all* the things in the list,
> > and therefore the things in the list are compatible with each other.
> >
> > Using it for a list of entirely different things to attempt in order
> > is not correct.
> 
> Isn't the idea behind a partition table that all partition information is
> stored on the device in a well-known format, so you don't need additional
> properties?

I guess that's the idea, but I wouldn't like to count on it.

And more importantly, it's still abusing the 'compatible' property.  A
node is supposed to be compatible with *everything* in 'compatible',
not just one of the things listed there.

> If the only property needed is the partition table offset, it can be encoded
> in the unit-address, and the "reg" property:
> 
>         partitions {
> 
>                 partition-table@xxxx {
>                         reg = <0xxxx ...>;
>                         ...
>                 };
> 
>                 ...
>         };

Urgh.. and that's abusing the unit address.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4392 bytes --]

On Mon, Dec 14, 2015 at 11:22:46AM +0100, Geert Uytterhoeven wrote:
> On Sat, Dec 12, 2015 at 6:51 AM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Thu, Dec 10, 2015 at 12:43:24PM -0800, Brian Norris wrote:
> >> On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
> >> > On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
> >> > > On 5 December 2015 at 12:39, Jonas Gorski <jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
> >> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> >> > > > <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> > >
> >> > > >> +
> >> > > >> +Examples:
> >> > > >> +
> >> > > >> +flash@0 {
> >> > > >> +       partitions {
> >> > > >> +               compatible = "google,fmap";
> >> > > >> +       };
> >> > > >> +};
> >> > > >
> >> > > > I wonder if this wouldn't be better served in a separate binding doc
> >> > > > with its compatible name as the filename, like we do with
> >> > > > driver^Whardware blocks, especially if we want to add more parsers.
> >> > >
> >> > >
> >> > > I find that *very* counter productive for bindings that go to the same
> >> > > node. You have a description of a node, and then suddenly there you
> >> > > have another file with another description of the same node. Totally
> >> > > awesome.
> >> >
> >> > I can't actually work out from that if you're agreeing with the
> >> > original post or the first reply.
> >>
> >> Perhaps I'm biased, but I think he was agreeing with the first reply.
> >> (Particularly, "I find that *very* counter productive" uses the word
> >> "that" to refer to "separate binding doc[s]".)
> >>
> >> > > Also how do you plan to write partitioning schemes with parameters
> >> > > like with non-zero offset of the partition table.
> >>
> >> If you are directing this question at me: I don't have a specific plan
> >> for it. MTD parsers don't currently take external input for this; many
> >> scan the whole device, but some might also have conventions built into
> >> the parser itself too, so this just gets hooked based on "compatible".
> >> But if the need arose, I would hope we could work out a common binding.
> >>
> >> > Presumably with properties in the patitions node.  Not seeing the
> >> > problem here.
> >>
> >> I believe Michal is bringing up the (important, IMO) point that if
> >> distinct partition types are being described in the same node, then any
> >> use of additional properties *must* be closely coordinated. We can't
> >> have two parsers "foo" and "bar" defining conflicting uses of the same
> >> property in the same node, like this:
> >>
> >>       partitions {
> >>               compatible = "foo", "bar";
> >>               property-baz = ...; // e.g., reg = <...>;
> >>       };
> >>
> >> where if "foo" is not found, we fall back to "bar". But what if "foo"
> >> and "bar" use "property-baz" differently?
> >
> > Ah.. that is an excellent point, and leads me to realise that using
> > compatible in this way is wrong.  The whole point of compatible is
> > that the node is, well, compatible with *all* the things in the list,
> > and therefore the things in the list are compatible with each other.
> >
> > Using it for a list of entirely different things to attempt in order
> > is not correct.
> 
> Isn't the idea behind a partition table that all partition information is
> stored on the device in a well-known format, so you don't need additional
> properties?

I guess that's the idea, but I wouldn't like to count on it.

And more importantly, it's still abusing the 'compatible' property.  A
node is supposed to be compatible with *everything* in 'compatible',
not just one of the things listed there.

> If the only property needed is the partition table offset, it can be encoded
> in the unit-address, and the "reg" property:
> 
>         partitions {
> 
>                 partition-table@xxxx {
>                         reg = <0xxxx ...>;
>                         ...
>                 };
> 
>                 ...
>         };

Urgh.. and that's abusing the unit address.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Geert Uytterhoeven]

Hi David,

On Tue, Dec 15, 2015 at 7:00 AM, David Gibson
<david@gibson.dropbear.id.au> wrote:
>> If the only property needed is the partition table offset, it can be encoded
>> in the unit-address, and the "reg" property:
>>
>>         partitions {
>>
>>                 partition-table@xxxx {
>>                         reg = <0xxxx ...>;
>>                         ...
>>                 };
>>
>>                 ...
>>         };
>
> Urgh.. and that's abusing the unit address.

Why? The partition is part of the FLASH. In this respect, it doesn't differ
from other hardcoded partitions using the same DT syntax.
It would just have a compatible value indicating it's a partition table.

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

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Geert Uytterhoeven]

Hi David,

On Tue, Dec 15, 2015 at 7:00 AM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>> If the only property needed is the partition table offset, it can be encoded
>> in the unit-address, and the "reg" property:
>>
>>         partitions {
>>
>>                 partition-table@xxxx {
>>                         reg = <0xxxx ...>;
>>                         ...
>>                 };
>>
>>                 ...
>>         };
>
> Urgh.. and that's abusing the unit address.

Why? The partition is part of the FLASH. In this respect, it doesn't differ
from other hardcoded partitions using the same DT syntax.
It would just have a compatible value indicating it's a partition table.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1330 bytes --]

On Tue, Dec 15, 2015 at 11:03:48AM +0100, Geert Uytterhoeven wrote:
> Hi David,
> 
> On Tue, Dec 15, 2015 at 7:00 AM, David Gibson
> <david@gibson.dropbear.id.au> wrote:
> >> If the only property needed is the partition table offset, it can be encoded
> >> in the unit-address, and the "reg" property:
> >>
> >>         partitions {
> >>
> >>                 partition-table@xxxx {
> >>                         reg = <0xxxx ...>;
> >>                         ...
> >>                 };
> >>
> >>                 ...
> >>         };
> >
> > Urgh.. and that's abusing the unit address.
> 
> Why? The partition is part of the FLASH. In this respect, it doesn't differ
> from other hardcoded partitions using the same DT syntax.
> It would just have a compatible value indicating it's a partition
> table.

Ah.. yes, fair enough.  I'd forgotten that the encoding of explicit
partitions in the device tree already established the address space
here as being the flash blocks.  So, no, it's not an abuse of unit
address.

Doesn't help for partition table types which require scanning the
device, of course.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Rob Herring]

On Fri, Dec 04, 2015 at 09:19:19PM -0800, Brian Norris wrote:
> The platform description (such as the type of partition formats used on
> a given flash) should be done independently of the flash driver in use.
> However, we can't reasonably have *all* partition parsers run on all
> flash (until they find a match), so let's overload the 'partitions'
> subnode to support specifying which format(s) to try in the device tree.
> 
> Start by supporting Google's (Chrome OS) FMAP structure.
> 
> There have been others interested in extending devicetree support to
> other parsers, like the bcm47xxpart parser:
> 
>   http://patchwork.ozlabs.org/patch/475986/
> 
> and the AFS (ARM Flash Structure?) parser:
> 
>   http://patchwork.ozlabs.org/patch/537827/
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Looks good to me. If you had lots of partitions, I'd agree with comments 
that they should be separate files, but I doubt we'll have many 10s of 
them. Plus all we really need here is a list of compatible strings. 

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  .../devicetree/bindings/mtd/partition.txt          | 75 ++++++++++++++++++++--
>  1 file changed, 69 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 28ae56f5c972..1bf9a7243993 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,29 +1,56 @@
> -Representing flash partitions in devicetree
> +Flash partitions in device tree
> +===============================
>  
> -Partitions can be represented by sub-nodes of an mtd device. This can be used
> +Flash devices can be partitioned into one or more functional ranges (e.g.,
> +"boot code", "nvram", and "kernel") in at least two distinct ways:
> +
> + (A) a fixed flash layout at production time or
> + (B) with an on-flash partition table, such as RedBoot, that describes the
> +     geometry and naming/purpose of each functional region
> +
> +The former typically requires an operating system to learn about the
> +partitioning from some kind of metadata provided by the bootloader/firmware.
> +Such partitions can be described using the method in "Section A: Fixed Partitions".
> +
> +The latter is somewhat analogous to partition tables used on block devices
> +(e.g., MBR or GPT), except that there is less standardization for flash
> +devices, and it is not always safe or efficient to attempt to search for all of
> +them on every flash device in the system, 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.
> +
> +To assist system software in locating these partition tables, we provide a
> +binding to describe which partition format(s) may be used on a given flash,
> +found below in "Section B: On-Flash Partition Tables".
> +
> +
> +Section A: Fixed Partitions
> +---------------------------
> +
> +Partitions can be represented by sub-nodes of a flash device. This can be used
>  on platforms which have strong conventions about which portions of a flash are
>  used for what purposes, but which don't use an on-flash partition table such
>  as RedBoot.
>  
> -The partition table should be a subnode of the mtd node and should be named
> +The partition table should be a subnode of the flash node and should be named
>  'partitions'. This node should have the following property:
>  - compatible : (required) must be "partitions"
>  Partitions are then defined in subnodes of the partitions node.
>  
> -For backwards compatibility partitions as direct subnodes of the mtd device are
> +For backwards compatibility partitions as direct subnodes of the flash device are
>  supported. This use is discouraged.
>  NOTE: also for backwards compatibility, direct subnodes that have a compatible
>  string are not considered partitions, as they may be used for other bindings.
>  
>  #address-cells & #size-cells must both be present in the partitions subnode of the
> -mtd device. There are two valid values for both:
> +flash device. There are two valid values for both:
>  <1>: for partitions that require a single 32-bit cell to represent their
>       size/address (aka the value is below 4 GiB)
>  <2>: for partitions that require two 32-bit cells to represent their
>       size/address (aka the value is 4 GiB or greater).
>  
>  Required properties:
> -- reg : The partition's offset and size within the mtd bank.
> +- reg : The partition's offset and size within the flash
>  
>  Optional properties:
>  - label : The label / name for this partition.  If omitted, the label is taken
> @@ -89,3 +116,39 @@ flash@2 {
>  		};
>  	};
>  };
> +
> +
> +Section B: On-Flash Partition Tables
> +------------------------------------
> +
> +System designers use a variety of on-flash data structures to describe the
> +layout of the flash. Because it's not always optimal for system software to
> +scan for every sort of data structure that might be used, one can specify which
> +structure(s) might be used on a given flash using the 'partitions' subnode of
> +the flash node.
> +
> +Node name: partitions
> +Properties:
> + - compatible: (required) used to define which partition format(s) may be in
> +   use on this flash may contain one or more of the strings defined in the
> +   following subsections
> +
> +
> +Google's FMAP (Flash MAP)
> +#########################
> +
> +Often used to describe the boot flash on Chrome OS devices. Specified here:
> +
> +  https://github.com/dhendrix/flashmap/blob/wiki/FmapSpec.md
> +
> +Properties:
> +- compatible: (required) must include "google,fmap"
> +
> +
> +Examples:
> +
> +flash@0 {
> +	partitions {
> +		compatible = "google,fmap";
> +	};
> +};
> -- 
> 2.6.0.rc2.230.g3dd15c0
> 

Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

[Rob Herring]

On Fri, Dec 04, 2015 at 09:19:19PM -0800, Brian Norris wrote:
> The platform description (such as the type of partition formats used on
> a given flash) should be done independently of the flash driver in use.
> However, we can't reasonably have *all* partition parsers run on all
> flash (until they find a match), so let's overload the 'partitions'
> subnode to support specifying which format(s) to try in the device tree.
> 
> Start by supporting Google's (Chrome OS) FMAP structure.
> 
> There have been others interested in extending devicetree support to
> other parsers, like the bcm47xxpart parser:
> 
>   http://patchwork.ozlabs.org/patch/475986/
> 
> and the AFS (ARM Flash Structure?) parser:
> 
>   http://patchwork.ozlabs.org/patch/537827/
> 
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Looks good to me. If you had lots of partitions, I'd agree with comments 
that they should be separate files, but I doubt we'll have many 10s of 
them. Plus all we really need here is a list of compatible strings. 

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> ---
>  .../devicetree/bindings/mtd/partition.txt          | 75 ++++++++++++++++++++--
>  1 file changed, 69 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 28ae56f5c972..1bf9a7243993 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,29 +1,56 @@
> -Representing flash partitions in devicetree
> +Flash partitions in device tree
> +===============================
>  
> -Partitions can be represented by sub-nodes of an mtd device. This can be used
> +Flash devices can be partitioned into one or more functional ranges (e.g.,
> +"boot code", "nvram", and "kernel") in at least two distinct ways:
> +
> + (A) a fixed flash layout at production time or
> + (B) with an on-flash partition table, such as RedBoot, that describes the
> +     geometry and naming/purpose of each functional region
> +
> +The former typically requires an operating system to learn about the
> +partitioning from some kind of metadata provided by the bootloader/firmware.
> +Such partitions can be described using the method in "Section A: Fixed Partitions".
> +
> +The latter is somewhat analogous to partition tables used on block devices
> +(e.g., MBR or GPT), except that there is less standardization for flash
> +devices, and it is not always safe or efficient to attempt to search for all of
> +them on every flash device in the system, 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.
> +
> +To assist system software in locating these partition tables, we provide a
> +binding to describe which partition format(s) may be used on a given flash,
> +found below in "Section B: On-Flash Partition Tables".
> +
> +
> +Section A: Fixed Partitions
> +---------------------------
> +
> +Partitions can be represented by sub-nodes of a flash device. This can be used
>  on platforms which have strong conventions about which portions of a flash are
>  used for what purposes, but which don't use an on-flash partition table such
>  as RedBoot.
>  
> -The partition table should be a subnode of the mtd node and should be named
> +The partition table should be a subnode of the flash node and should be named
>  'partitions'. This node should have the following property:
>  - compatible : (required) must be "partitions"
>  Partitions are then defined in subnodes of the partitions node.
>  
> -For backwards compatibility partitions as direct subnodes of the mtd device are
> +For backwards compatibility partitions as direct subnodes of the flash device are
>  supported. This use is discouraged.
>  NOTE: also for backwards compatibility, direct subnodes that have a compatible
>  string are not considered partitions, as they may be used for other bindings.
>  
>  #address-cells & #size-cells must both be present in the partitions subnode of the
> -mtd device. There are two valid values for both:
> +flash device. There are two valid values for both:
>  <1>: for partitions that require a single 32-bit cell to represent their
>       size/address (aka the value is below 4 GiB)
>  <2>: for partitions that require two 32-bit cells to represent their
>       size/address (aka the value is 4 GiB or greater).
>  
>  Required properties:
> -- reg : The partition's offset and size within the mtd bank.
> +- reg : The partition's offset and size within the flash
>  
>  Optional properties:
>  - label : The label / name for this partition.  If omitted, the label is taken
> @@ -89,3 +116,39 @@ flash@2 {
>  		};
>  	};
>  };
> +
> +
> +Section B: On-Flash Partition Tables
> +------------------------------------
> +
> +System designers use a variety of on-flash data structures to describe the
> +layout of the flash. Because it's not always optimal for system software to
> +scan for every sort of data structure that might be used, one can specify which
> +structure(s) might be used on a given flash using the 'partitions' subnode of
> +the flash node.
> +
> +Node name: partitions
> +Properties:
> + - compatible: (required) used to define which partition format(s) may be in
> +   use on this flash may contain one or more of the strings defined in the
> +   following subsections
> +
> +
> +Google's FMAP (Flash MAP)
> +#########################
> +
> +Often used to describe the boot flash on Chrome OS devices. Specified here:
> +
> +  https://github.com/dhendrix/flashmap/blob/wiki/FmapSpec.md
> +
> +Properties:
> +- compatible: (required) must include "google,fmap"
> +
> +
> +Examples:
> +
> +flash@0 {
> +	partitions {
> +		compatible = "google,fmap";
> +	};
> +};
> -- 
> 2.6.0.rc2.230.g3dd15c0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Brian Norris]

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

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

[Brian Norris]

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

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

[Rob Herring]

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;
> +}

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

[Rob Herring]

On Fri, Dec 4, 2015 at 11:19 PM, Brian Norris
<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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;
> +}

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

[Brian Norris]

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

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

[Brian Norris]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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

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

[Rob Herring]

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

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

[Rob Herring]

On Mon, Dec 7, 2015 at 12:13 PM, Brian Norris
<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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

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

[Brian Norris]

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

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

[Brian Norris]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 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

[RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching

[Brian Norris]

Partition parsers can now provide an of_match_table to enable
flash<-->parser matching via device tree.

TODO: Doesn't yet work when parser is built as module. I can't just use
request_module() and friends, since OF matches don't tell me the name of
the driver/module. Maybe I can report uevents?

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

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index b3100742ddf6..91eb2df0bf1e 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -29,6 +29,7 @@
 #include <linux/kmod.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
+#include <linux/of_mtd.h>
 #include <linux/err.h>
 #include <linux/kconfig.h>
 
@@ -728,6 +729,25 @@ static struct mtd_part_parser *mtd_part_parser_get_by_name(const char *name)
 	return __mtd_part_parser_get_by_name(name);
 }
 
+static struct mtd_part_parser *mtd_part_parser_get_by_of(struct mtd_info *mtd)
+{
+	struct mtd_part_parser *p, *ret = NULL;
+
+	spin_lock(&part_parser_lock);
+
+	list_for_each_entry(p, &part_parsers, list) {
+		if (of_mtd_match_mtd_parser(mtd, p) &&
+				try_module_get(p->owner)) {
+			ret = p;
+			break;
+		}
+	}
+
+	spin_unlock(&part_parser_lock);
+
+	return ret;
+}
+
 static inline void mtd_part_parser_put(const struct mtd_part_parser *p)
 {
 	module_put(p->owner);
@@ -847,6 +867,18 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 		if (ret < 0 && !err)
 			err = ret;
 	}
+
+	parser = mtd_part_parser_get_by_of(master);
+	if (!parser)
+		return err;
+
+	ret = mtd_part_do_parse(parser, master, pparts, data);
+	if (ret > 0)
+		return 0;
+	mtd_part_parser_put(parser);
+	if (ret < 0 && !err)
+		err = ret;
+
 	return err;
 }
 
-- 
2.6.0.rc2.230.g3dd15c0

[RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching

[Brian Norris]

Partition parsers can now provide an of_match_table to enable
flash<-->parser matching via device tree.

TODO: Doesn't yet work when parser is built as module. I can't just use
request_module() and friends, since OF matches don't tell me the name of
the driver/module. Maybe I can report uevents?

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

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index b3100742ddf6..91eb2df0bf1e 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -29,6 +29,7 @@
 #include <linux/kmod.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
+#include <linux/of_mtd.h>
 #include <linux/err.h>
 #include <linux/kconfig.h>
 
@@ -728,6 +729,25 @@ static struct mtd_part_parser *mtd_part_parser_get_by_name(const char *name)
 	return __mtd_part_parser_get_by_name(name);
 }
 
+static struct mtd_part_parser *mtd_part_parser_get_by_of(struct mtd_info *mtd)
+{
+	struct mtd_part_parser *p, *ret = NULL;
+
+	spin_lock(&part_parser_lock);
+
+	list_for_each_entry(p, &part_parsers, list) {
+		if (of_mtd_match_mtd_parser(mtd, p) &&
+				try_module_get(p->owner)) {
+			ret = p;
+			break;
+		}
+	}
+
+	spin_unlock(&part_parser_lock);
+
+	return ret;
+}
+
 static inline void mtd_part_parser_put(const struct mtd_part_parser *p)
 {
 	module_put(p->owner);
@@ -847,6 +867,18 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 		if (ret < 0 && !err)
 			err = ret;
 	}
+
+	parser = mtd_part_parser_get_by_of(master);
+	if (!parser)
+		return err;
+
+	ret = mtd_part_do_parse(parser, master, pparts, data);
+	if (ret > 0)
+		return 0;
+	mtd_part_parser_put(parser);
+	if (ret < 0 && !err)
+		err = ret;
+
 	return err;
 }
 
-- 
2.6.0.rc2.230.g3dd15c0

[RFC PATCH 7/7] mtd: partitions: add Google's FMAP partition parser

[Brian Norris]

Cc: David Hendricks <dhendrix@chromium.org>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/partitions/Kconfig       |   7 ++
 drivers/mtd/partitions/Makefile      |   1 +
 drivers/mtd/partitions/google_fmap.c | 226 +++++++++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 drivers/mtd/partitions/google_fmap.c

diff --git a/drivers/mtd/partitions/Kconfig b/drivers/mtd/partitions/Kconfig
index 0827d7a8be4e..98783f1d3a36 100644
--- a/drivers/mtd/partitions/Kconfig
+++ b/drivers/mtd/partitions/Kconfig
@@ -129,3 +129,10 @@ config MTD_BCM47XX_PARTS
 	help
 	  This provides partitions parser for devices based on BCM47xx
 	  boards.
+
+config MTD_GOOGLE_FMAP_PARTS
+	tristate "Google's Flash Map (FMAP) partition support"
+	help
+	  This provides partition parsing for Google's flash map layout, used
+	  primarily on the boot flash of Chrome OS hardware (e.g., Chromebooks
+	  and Chromeboxes).
diff --git a/drivers/mtd/partitions/Makefile b/drivers/mtd/partitions/Makefile
index 89822f2bfa59..ab398c7f4d01 100644
--- a/drivers/mtd/partitions/Makefile
+++ b/drivers/mtd/partitions/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_MTD_AFS_PARTS)	+= afs.o
 obj-$(CONFIG_MTD_AR7_PARTS)	+= ar7part.o
 obj-$(CONFIG_MTD_BCM63XX_PARTS)	+= bcm63xxpart.o
 obj-$(CONFIG_MTD_BCM47XX_PARTS)	+= bcm47xxpart.o
+obj-$(CONFIG_MTD_GOOGLE_FMAP_PARTS) += google_fmap.o
diff --git a/drivers/mtd/partitions/google_fmap.c b/drivers/mtd/partitions/google_fmap.c
new file mode 100644
index 000000000000..abd10eb65c84
--- /dev/null
+++ b/drivers/mtd/partitions/google_fmap.c
@@ -0,0 +1,226 @@
+/*
+ * Parse Google FMAP partitions
+ *
+ * Author: Brian Norris <briannorris@chromium.org>
+ *
+ * Copyright © 2015 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * See:
+ *   https://github.com/dhendrix/flashmap/blob/wiki/FmapSpec.md
+ *
+ * Notes:
+ *   - scans only at block boundaries; this is not guaranteed for FMAP (the
+ *     Chrome OS tools do a kind of stride search, of decreasing size), but
+ *     seems like a decent start
+ *   - at worst, scans (beginning of) every block on an unformatted flash
+ *   - only validates the "__FMAP__" signature, just like the Chrome OS tools;
+ *     however, this seems (theoretically) easy to produce false matches
+ *   - major/minor version numbers are currently unused
+ */
+
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
+#include <linux/compiler.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/vmalloc.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+
+static const char fmap_signature[] = "__FMAP__";
+
+struct fmap_layout {
+	uint8_t signature[8];		/* "__FMAP__" (0x5F5F50414D465F5F) */
+	uint8_t ver_major;		/* major version number of this structure */
+	uint8_t ver_minor;		/* minor version of this structure */
+	__le64 base;			/* physical memory-mapped address of the flash chip */
+	__le32 size;			/* size of the flash chip in bytes */
+	uint8_t name[32];		/* descriptive name of this flash device, 0 terminated */
+	__le16 nareas;			/* number of areas described by areas[] below */
+	struct fmap_area {
+		__le32 offset;		/* offset of this area in the flash device */
+		__le32 size;		/* size of this area in bytes */
+		uint8_t name[32];	/* descriptive name of this area, 0 terminated */
+		__le16 flags;		/* flags for this area */
+	} __packed areas[0];
+} __packed;
+
+/* mtd_read() helper */
+static int fmap_mtd_read(struct mtd_info *mtd, loff_t offset, size_t len,
+			 void *buf)
+{
+	size_t retlen;
+	int ret;
+
+	ret = mtd_read(mtd, offset, len, &retlen, buf);
+	if (ret)
+		return ret;
+	if (retlen != len)
+		return -EIO;
+	return 0;
+}
+
+/* Return 0 on no match, non-zero on match */
+static inline int fmap_check_signature(struct fmap_layout *fmap)
+{
+	return !strncmp(fmap->signature, fmap_signature,
+			sizeof(fmap->signature));
+}
+
+static int fmap_parse_block(struct mtd_info *master,
+			    const struct mtd_partition **pparts,
+			    struct fmap_layout *fmap, size_t maxlen)
+{
+	struct mtd_partition *parts;
+	char *names;
+	int nparts;
+	int ret, i, namelen = 0;
+
+	if (!fmap_check_signature(fmap))
+		return 0;
+
+	nparts = le16_to_cpu(fmap->nareas);
+
+	if (!nparts) {
+		pr_err("found FMAP, but no FMAP areas; skipping\n");
+		return -EINVAL;
+	}
+
+	/* pre-process names */
+	for (i = 0; i < nparts; i++) {
+		struct fmap_area *area = &fmap->areas[i];
+
+		/* Terminate */
+		area->name[sizeof(area->name) - 1] = '\0';
+
+		namelen += strlen(area->name);
+	}
+
+	/* partitions + name strings + string terminators */
+	parts = kzalloc(sizeof(*parts) * nparts + namelen + nparts, GFP_KERNEL);
+	if (!parts) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	names = (char *)(parts + nparts);
+
+	for (i = 0; i < nparts; i++) {
+		struct mtd_partition *part = &parts[i];
+		struct fmap_area *area = &fmap->areas[i];
+
+		strcpy(names, area->name);
+		part->name = names;
+		names += strlen(names) + 1;
+
+		part->offset = le32_to_cpu(area->offset);
+		part->size = le32_to_cpu(area->size);
+	}
+
+	*pparts = parts;
+	return nparts;
+
+err:
+	kfree(parts);
+	return ret;
+}
+
+static int fmap_peek_header(struct mtd_info *master, loff_t offs, void *buf)
+{
+	struct fmap_layout *fmap;
+	int ret;
+
+	fmap = buf;
+
+	ret = fmap_mtd_read(master, offs, sizeof(*fmap), fmap);
+	if (ret)
+		return ret;
+
+	return fmap_check_signature(fmap);
+}
+
+static int fmap_parse_partitions(struct mtd_info *master,
+				 const struct mtd_partition **pparts,
+				 struct mtd_part_parser_data *data)
+{
+	void *buf;
+	struct fmap_layout *fmap;
+	int ret = 0;
+	size_t len;
+	loff_t offset = 0;
+
+	buf = vmalloc(master->erasesize);
+	if (!buf)
+		return -ENOMEM;
+
+	for (offset = 0; offset < master->size; offset += master->erasesize) {
+		if (mtd_block_isbad(master, offset))
+			continue;
+
+		ret = fmap_peek_header(master, offset, buf);
+		if (ret < 0)
+			break;
+		if (!ret)
+			/* No match; don't read the rest */
+			continue;
+
+		len = master->erasesize;
+		ret = fmap_mtd_read(master, offset, len, buf);
+		if (ret)
+			break;
+
+		fmap = buf;
+
+		ret = fmap_parse_block(master, pparts, fmap, len);
+		if (ret < 0)
+			break;
+		if (ret) /* success */
+			break;
+	}
+
+	vfree(buf);
+
+	if (ret < 0) {
+		pr_err("error %d while searching for fmap\n", ret);
+		return ret;
+	} else if (ret) {
+		pr_info("found fmap @ offset %#llx\n", (u64)offset);
+		return ret;
+	} else {
+		pr_debug("no fmap found\n");
+		return 0;
+	}
+}
+
+static const struct of_device_id fmap_match[] = {
+	{ .compatible = "google,fmap" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fmap_match);
+
+static struct mtd_part_parser fmap_parser = {
+	.parse_fn = fmap_parse_partitions,
+	.name = "fmap",
+	.of_match_table = fmap_match,
+};
+module_mtd_part_parser(fmap_parser);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Brian Norris");
+MODULE_DESCRIPTION("Parsing code for Chrome OS FMAP tables");
+/* MTD parsers request the module by parser name */
+MODULE_ALIAS("fmap");
-- 
2.6.0.rc2.230.g3dd15c0

[RFC PATCH 7/7] mtd: partitions: add Google's FMAP partition parser

[Brian Norris]

Cc: David Hendricks <dhendrix@chromium.org>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/partitions/Kconfig       |   7 ++
 drivers/mtd/partitions/Makefile      |   1 +
 drivers/mtd/partitions/google_fmap.c | 226 +++++++++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 drivers/mtd/partitions/google_fmap.c

diff --git a/drivers/mtd/partitions/Kconfig b/drivers/mtd/partitions/Kconfig
index 0827d7a8be4e..98783f1d3a36 100644
--- a/drivers/mtd/partitions/Kconfig
+++ b/drivers/mtd/partitions/Kconfig
@@ -129,3 +129,10 @@ config MTD_BCM47XX_PARTS
 	help
 	  This provides partitions parser for devices based on BCM47xx
 	  boards.
+
+config MTD_GOOGLE_FMAP_PARTS
+	tristate "Google's Flash Map (FMAP) partition support"
+	help
+	  This provides partition parsing for Google's flash map layout, used
+	  primarily on the boot flash of Chrome OS hardware (e.g., Chromebooks
+	  and Chromeboxes).
diff --git a/drivers/mtd/partitions/Makefile b/drivers/mtd/partitions/Makefile
index 89822f2bfa59..ab398c7f4d01 100644
--- a/drivers/mtd/partitions/Makefile
+++ b/drivers/mtd/partitions/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_MTD_AFS_PARTS)	+= afs.o
 obj-$(CONFIG_MTD_AR7_PARTS)	+= ar7part.o
 obj-$(CONFIG_MTD_BCM63XX_PARTS)	+= bcm63xxpart.o
 obj-$(CONFIG_MTD_BCM47XX_PARTS)	+= bcm47xxpart.o
+obj-$(CONFIG_MTD_GOOGLE_FMAP_PARTS) += google_fmap.o
diff --git a/drivers/mtd/partitions/google_fmap.c b/drivers/mtd/partitions/google_fmap.c
new file mode 100644
index 000000000000..abd10eb65c84
--- /dev/null
+++ b/drivers/mtd/partitions/google_fmap.c
@@ -0,0 +1,226 @@
+/*
+ * Parse Google FMAP partitions
+ *
+ * Author: Brian Norris <briannorris@chromium.org>
+ *
+ * Copyright © 2015 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * See:
+ *   https://github.com/dhendrix/flashmap/blob/wiki/FmapSpec.md
+ *
+ * Notes:
+ *   - scans only at block boundaries; this is not guaranteed for FMAP (the
+ *     Chrome OS tools do a kind of stride search, of decreasing size), but
+ *     seems like a decent start
+ *   - at worst, scans (beginning of) every block on an unformatted flash
+ *   - only validates the "__FMAP__" signature, just like the Chrome OS tools;
+ *     however, this seems (theoretically) easy to produce false matches
+ *   - major/minor version numbers are currently unused
+ */
+
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
+#include <linux/compiler.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/vmalloc.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+
+static const char fmap_signature[] = "__FMAP__";
+
+struct fmap_layout {
+	uint8_t signature[8];		/* "__FMAP__" (0x5F5F50414D465F5F) */
+	uint8_t ver_major;		/* major version number of this structure */
+	uint8_t ver_minor;		/* minor version of this structure */
+	__le64 base;			/* physical memory-mapped address of the flash chip */
+	__le32 size;			/* size of the flash chip in bytes */
+	uint8_t name[32];		/* descriptive name of this flash device, 0 terminated */
+	__le16 nareas;			/* number of areas described by areas[] below */
+	struct fmap_area {
+		__le32 offset;		/* offset of this area in the flash device */
+		__le32 size;		/* size of this area in bytes */
+		uint8_t name[32];	/* descriptive name of this area, 0 terminated */
+		__le16 flags;		/* flags for this area */
+	} __packed areas[0];
+} __packed;
+
+/* mtd_read() helper */
+static int fmap_mtd_read(struct mtd_info *mtd, loff_t offset, size_t len,
+			 void *buf)
+{
+	size_t retlen;
+	int ret;
+
+	ret = mtd_read(mtd, offset, len, &retlen, buf);
+	if (ret)
+		return ret;
+	if (retlen != len)
+		return -EIO;
+	return 0;
+}
+
+/* Return 0 on no match, non-zero on match */
+static inline int fmap_check_signature(struct fmap_layout *fmap)
+{
+	return !strncmp(fmap->signature, fmap_signature,
+			sizeof(fmap->signature));
+}
+
+static int fmap_parse_block(struct mtd_info *master,
+			    const struct mtd_partition **pparts,
+			    struct fmap_layout *fmap, size_t maxlen)
+{
+	struct mtd_partition *parts;
+	char *names;
+	int nparts;
+	int ret, i, namelen = 0;
+
+	if (!fmap_check_signature(fmap))
+		return 0;
+
+	nparts = le16_to_cpu(fmap->nareas);
+
+	if (!nparts) {
+		pr_err("found FMAP, but no FMAP areas; skipping\n");
+		return -EINVAL;
+	}
+
+	/* pre-process names */
+	for (i = 0; i < nparts; i++) {
+		struct fmap_area *area = &fmap->areas[i];
+
+		/* Terminate */
+		area->name[sizeof(area->name) - 1] = '\0';
+
+		namelen += strlen(area->name);
+	}
+
+	/* partitions + name strings + string terminators */
+	parts = kzalloc(sizeof(*parts) * nparts + namelen + nparts, GFP_KERNEL);
+	if (!parts) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	names = (char *)(parts + nparts);
+
+	for (i = 0; i < nparts; i++) {
+		struct mtd_partition *part = &parts[i];
+		struct fmap_area *area = &fmap->areas[i];
+
+		strcpy(names, area->name);
+		part->name = names;
+		names += strlen(names) + 1;
+
+		part->offset = le32_to_cpu(area->offset);
+		part->size = le32_to_cpu(area->size);
+	}
+
+	*pparts = parts;
+	return nparts;
+
+err:
+	kfree(parts);
+	return ret;
+}
+
+static int fmap_peek_header(struct mtd_info *master, loff_t offs, void *buf)
+{
+	struct fmap_layout *fmap;
+	int ret;
+
+	fmap = buf;
+
+	ret = fmap_mtd_read(master, offs, sizeof(*fmap), fmap);
+	if (ret)
+		return ret;
+
+	return fmap_check_signature(fmap);
+}
+
+static int fmap_parse_partitions(struct mtd_info *master,
+				 const struct mtd_partition **pparts,
+				 struct mtd_part_parser_data *data)
+{
+	void *buf;
+	struct fmap_layout *fmap;
+	int ret = 0;
+	size_t len;
+	loff_t offset = 0;
+
+	buf = vmalloc(master->erasesize);
+	if (!buf)
+		return -ENOMEM;
+
+	for (offset = 0; offset < master->size; offset += master->erasesize) {
+		if (mtd_block_isbad(master, offset))
+			continue;
+
+		ret = fmap_peek_header(master, offset, buf);
+		if (ret < 0)
+			break;
+		if (!ret)
+			/* No match; don't read the rest */
+			continue;
+
+		len = master->erasesize;
+		ret = fmap_mtd_read(master, offset, len, buf);
+		if (ret)
+			break;
+
+		fmap = buf;
+
+		ret = fmap_parse_block(master, pparts, fmap, len);
+		if (ret < 0)
+			break;
+		if (ret) /* success */
+			break;
+	}
+
+	vfree(buf);
+
+	if (ret < 0) {
+		pr_err("error %d while searching for fmap\n", ret);
+		return ret;
+	} else if (ret) {
+		pr_info("found fmap @ offset %#llx\n", (u64)offset);
+		return ret;
+	} else {
+		pr_debug("no fmap found\n");
+		return 0;
+	}
+}
+
+static const struct of_device_id fmap_match[] = {
+	{ .compatible = "google,fmap" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fmap_match);
+
+static struct mtd_part_parser fmap_parser = {
+	.parse_fn = fmap_parse_partitions,
+	.name = "fmap",
+	.of_match_table = fmap_match,
+};
+module_mtd_part_parser(fmap_parser);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Brian Norris");
+MODULE_DESCRIPTION("Parsing code for Chrome OS FMAP tables");
+/* MTD parsers request the module by parser name */
+MODULE_ALIAS("fmap");
-- 
2.6.0.rc2.230.g3dd15c0

[RFC PATCH 7/7] mtd: partitions: add Google's FMAP partition parser

[Brian Norris]

Cc: David Hendricks <dhendrix@chromium.org>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/partitions/Kconfig       |   7 ++
 drivers/mtd/partitions/Makefile      |   1 +
 drivers/mtd/partitions/google_fmap.c | 226 +++++++++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 drivers/mtd/partitions/google_fmap.c

diff --git a/drivers/mtd/partitions/Kconfig b/drivers/mtd/partitions/Kconfig
index 0827d7a8be4e..98783f1d3a36 100644
--- a/drivers/mtd/partitions/Kconfig
+++ b/drivers/mtd/partitions/Kconfig
@@ -129,3 +129,10 @@ config MTD_BCM47XX_PARTS
 	help
 	  This provides partitions parser for devices based on BCM47xx
 	  boards.
+
+config MTD_GOOGLE_FMAP_PARTS
+	tristate "Google's Flash Map (FMAP) partition support"
+	help
+	  This provides partition parsing for Google's flash map layout, used
+	  primarily on the boot flash of Chrome OS hardware (e.g., Chromebooks
+	  and Chromeboxes).
diff --git a/drivers/mtd/partitions/Makefile b/drivers/mtd/partitions/Makefile
index 89822f2bfa59..ab398c7f4d01 100644
--- a/drivers/mtd/partitions/Makefile
+++ b/drivers/mtd/partitions/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_MTD_AFS_PARTS)	+= afs.o
 obj-$(CONFIG_MTD_AR7_PARTS)	+= ar7part.o
 obj-$(CONFIG_MTD_BCM63XX_PARTS)	+= bcm63xxpart.o
 obj-$(CONFIG_MTD_BCM47XX_PARTS)	+= bcm47xxpart.o
+obj-$(CONFIG_MTD_GOOGLE_FMAP_PARTS) += google_fmap.o
diff --git a/drivers/mtd/partitions/google_fmap.c b/drivers/mtd/partitions/google_fmap.c
new file mode 100644
index 000000000000..abd10eb65c84
--- /dev/null
+++ b/drivers/mtd/partitions/google_fmap.c
@@ -0,0 +1,226 @@
+/*
+ * Parse Google FMAP partitions
+ *
+ * Author: Brian Norris <briannorris@chromium.org>
+ *
+ * Copyright © 2015 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * See:
+ *   https://github.com/dhendrix/flashmap/blob/wiki/FmapSpec.md
+ *
+ * Notes:
+ *   - scans only at block boundaries; this is not guaranteed for FMAP (the
+ *     Chrome OS tools do a kind of stride search, of decreasing size), but
+ *     seems like a decent start
+ *   - at worst, scans (beginning of) every block on an unformatted flash
+ *   - only validates the "__FMAP__" signature, just like the Chrome OS tools;
+ *     however, this seems (theoretically) easy to produce false matches
+ *   - major/minor version numbers are currently unused
+ */
+
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
+#include <linux/compiler.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/vmalloc.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+
+static const char fmap_signature[] = "__FMAP__";
+
+struct fmap_layout {
+	uint8_t signature[8];		/* "__FMAP__" (0x5F5F50414D465F5F) */
+	uint8_t ver_major;		/* major version number of this structure */
+	uint8_t ver_minor;		/* minor version of this structure */
+	__le64 base;			/* physical memory-mapped address of the flash chip */
+	__le32 size;			/* size of the flash chip in bytes */
+	uint8_t name[32];		/* descriptive name of this flash device, 0 terminated */
+	__le16 nareas;			/* number of areas described by areas[] below */
+	struct fmap_area {
+		__le32 offset;		/* offset of this area in the flash device */
+		__le32 size;		/* size of this area in bytes */
+		uint8_t name[32];	/* descriptive name of this area, 0 terminated */
+		__le16 flags;		/* flags for this area */
+	} __packed areas[0];
+} __packed;
+
+/* mtd_read() helper */
+static int fmap_mtd_read(struct mtd_info *mtd, loff_t offset, size_t len,
+			 void *buf)
+{
+	size_t retlen;
+	int ret;
+
+	ret = mtd_read(mtd, offset, len, &retlen, buf);
+	if (ret)
+		return ret;
+	if (retlen != len)
+		return -EIO;
+	return 0;
+}
+
+/* Return 0 on no match, non-zero on match */
+static inline int fmap_check_signature(struct fmap_layout *fmap)
+{
+	return !strncmp(fmap->signature, fmap_signature,
+			sizeof(fmap->signature));
+}
+
+static int fmap_parse_block(struct mtd_info *master,
+			    const struct mtd_partition **pparts,
+			    struct fmap_layout *fmap, size_t maxlen)
+{
+	struct mtd_partition *parts;
+	char *names;
+	int nparts;
+	int ret, i, namelen = 0;
+
+	if (!fmap_check_signature(fmap))
+		return 0;
+
+	nparts = le16_to_cpu(fmap->nareas);
+
+	if (!nparts) {
+		pr_err("found FMAP, but no FMAP areas; skipping\n");
+		return -EINVAL;
+	}
+
+	/* pre-process names */
+	for (i = 0; i < nparts; i++) {
+		struct fmap_area *area = &fmap->areas[i];
+
+		/* Terminate */
+		area->name[sizeof(area->name) - 1] = '\0';
+
+		namelen += strlen(area->name);
+	}
+
+	/* partitions + name strings + string terminators */
+	parts = kzalloc(sizeof(*parts) * nparts + namelen + nparts, GFP_KERNEL);
+	if (!parts) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	names = (char *)(parts + nparts);
+
+	for (i = 0; i < nparts; i++) {
+		struct mtd_partition *part = &parts[i];
+		struct fmap_area *area = &fmap->areas[i];
+
+		strcpy(names, area->name);
+		part->name = names;
+		names += strlen(names) + 1;
+
+		part->offset = le32_to_cpu(area->offset);
+		part->size = le32_to_cpu(area->size);
+	}
+
+	*pparts = parts;
+	return nparts;
+
+err:
+	kfree(parts);
+	return ret;
+}
+
+static int fmap_peek_header(struct mtd_info *master, loff_t offs, void *buf)
+{
+	struct fmap_layout *fmap;
+	int ret;
+
+	fmap = buf;
+
+	ret = fmap_mtd_read(master, offs, sizeof(*fmap), fmap);
+	if (ret)
+		return ret;
+
+	return fmap_check_signature(fmap);
+}
+
+static int fmap_parse_partitions(struct mtd_info *master,
+				 const struct mtd_partition **pparts,
+				 struct mtd_part_parser_data *data)
+{
+	void *buf;
+	struct fmap_layout *fmap;
+	int ret = 0;
+	size_t len;
+	loff_t offset = 0;
+
+	buf = vmalloc(master->erasesize);
+	if (!buf)
+		return -ENOMEM;
+
+	for (offset = 0; offset < master->size; offset += master->erasesize) {
+		if (mtd_block_isbad(master, offset))
+			continue;
+
+		ret = fmap_peek_header(master, offset, buf);
+		if (ret < 0)
+			break;
+		if (!ret)
+			/* No match; don't read the rest */
+			continue;
+
+		len = master->erasesize;
+		ret = fmap_mtd_read(master, offset, len, buf);
+		if (ret)
+			break;
+
+		fmap = buf;
+
+		ret = fmap_parse_block(master, pparts, fmap, len);
+		if (ret < 0)
+			break;
+		if (ret) /* success */
+			break;
+	}
+
+	vfree(buf);
+
+	if (ret < 0) {
+		pr_err("error %d while searching for fmap\n", ret);
+		return ret;
+	} else if (ret) {
+		pr_info("found fmap @ offset %#llx\n", (u64)offset);
+		return ret;
+	} else {
+		pr_debug("no fmap found\n");
+		return 0;
+	}
+}
+
+static const struct of_device_id fmap_match[] = {
+	{ .compatible = "google,fmap" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fmap_match);
+
+static struct mtd_part_parser fmap_parser = {
+	.parse_fn = fmap_parse_partitions,
+	.name = "fmap",
+	.of_match_table = fmap_match,
+};
+module_mtd_part_parser(fmap_parser);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Brian Norris");
+MODULE_DESCRIPTION("Parsing code for Chrome OS FMAP tables");
+/* MTD parsers request the module by parser name */
+MODULE_ALIAS("fmap");
-- 
2.6.0.rc2.230.g3dd15c0

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

[Geert Uytterhoeven]

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

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

[Geert Uytterhoeven]

On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Michal Suchanek]

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

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

[Michal Suchanek]

On 5 December 2015 at 11:15, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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

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

[Brian Norris]

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

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

[Brian Norris]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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

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

[Geert Uytterhoeven]

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

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

[Geert Uytterhoeven]

Hi Brian,

On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Michal Suchanek]

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

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

[Michal Suchanek]

On 11 December 2015 at 09:44, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> Hi Brian,
>
> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
> <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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

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

[Geert Uytterhoeven]

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

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

[Geert Uytterhoeven]

Hi Michal,

On Fri, Dec 11, 2015 at 4:34 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 11 December 2015 at 09:44, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
>> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
>> <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Michal Suchanek]

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

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

[Michal Suchanek]

On 11 December 2015 at 17:00, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> Hi Michal,
>
> On Fri, Dec 11, 2015 at 4:34 PM, Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 11 December 2015 at 09:44, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
>>> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
>>> <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Brian Norris]

On Fri, Dec 11, 2015 at 09:44:37AM +0100, Geert Uytterhoeven wrote:
> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> > 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.

OK. But these flash are often used for the boot firmware, no? And then,
does the boot code have to be provided at one end of the flash (e.g.,
bottom)? If so, then something like MBR or GPT will likely not apply,
since they reserve the first (and sometimes last) blocks of the medium.

Brian

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

[Brian Norris]

On Fri, Dec 11, 2015 at 09:44:37AM +0100, Geert Uytterhoeven wrote:
> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
> <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > 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.

OK. But these flash are often used for the boot firmware, no? And then,
does the boot code have to be provided at one end of the flash (e.g.,
bottom)? If so, then something like MBR or GPT will likely not apply,
since they reserve the first (and sometimes last) blocks of the medium.

Brian

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

[Geert Uytterhoeven]

Hi Brian,

On Sat, Dec 12, 2015 at 2:33 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Fri, Dec 11, 2015 at 09:44:37AM +0100, Geert Uytterhoeven wrote:
>> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
>> <computersforpeace@gmail.com> wrote:
>> > 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.
>
> OK. But these flash are often used for the boot firmware, no? And then,
> does the boot code have to be provided at one end of the flash (e.g.,
> bottom)? If so, then something like MBR or GPT will likely not apply,
> since they reserve the first (and sometimes last) blocks of the medium.

If the boot firmware is in the first few blocks, Amiga RDB or BSD disklabel
come to the rescue...

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

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

[Geert Uytterhoeven]

Hi Brian,

On Sat, Dec 12, 2015 at 2:33 AM, Brian Norris
<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Dec 11, 2015 at 09:44:37AM +0100, Geert Uytterhoeven wrote:
>> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
>> <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > 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.
>
> OK. But these flash are often used for the boot firmware, no? And then,
> does the boot code have to be provided at one end of the flash (e.g.,
> bottom)? If so, then something like MBR or GPT will likely not apply,
> since they reserve the first (and sometimes last) blocks of the medium.

If the boot firmware is in the first few blocks, Amiga RDB or BSD disklabel
come to the rescue...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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

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

[Jonas Gorski]

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

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

[Brian Norris]

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

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

[Brian Norris]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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