[RESEND,1/4] dt-bindings: mtd: make partitions doc a bit more generic

diff mbox

From: Brian Norris <computersforpeace@gmail.com>

Currently the only documented partitioning is "fixed-partitions" but
there are more methods in use that we may want to support in the future.
Mention them and make it clear Fixed Partitions are just a single case.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/mtd/partition.txt          | 28 +++++++++++++++++-----
 1 file changed, 22 insertions(+), 6 deletions(-)

+ others

Hi Rafał,

Thanks for drudging up my old work. This is something that's been stuck
too far down in my stack of TODOs that it essentially timed out...

On Thu, Apr 20, 2017 at 03:57:28PM +0200, Rafał Miłecki wrote:
> From: Brian Norris <computersforpeace@gmail.com>
> 
> Currently the only documented partitioning is "fixed-partitions" but
> there are more methods in use that we may want to support in the future.
> Mention them and make it clear Fixed Partitions are just a single case.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

I mentioned the missing changelog on IRC, but to fill in the gaps:
essentially you've dropped the partition parser and associated bindings
I had in my series (for the 'Google FMAP' format). That's fine, but I
just wanted to note it.

And with that, I think you've not quite nailed the purpose of my
original patch. This now seems to suggest there are other potential
bindings here, but then you leave the reader hanging. One note to that
effect below. If that's the only objection, then I can make the
additions myself when applying.

>  .../devicetree/bindings/mtd/partition.txt          | 28 +++++++++++++++++-----
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 81a224da63be..27593adc45c2 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,29 +1,45 @@
> -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", "kernel").
> +
> +Different devices may be partitioned in a different ways. Some may use a fixed
> +flash layout set at production time. Some may use on-flash table that describes
> +the geometry and naming/purpose of each functional region. It is also possible
> +to see these methods mixed.
> +
> +To assist system software in locating partitions, we provide a binding to
> +describe which method is used for a given flash.


We've suggested above that there may be "different ways" (fixed vs.
on-flash tables) to partition, but then we still only describe one way.
Maybe we can just paste something like this as a caveat before moving
on?

"We currently only document a binding for fixed layouts."

We can delete that in the presumed follow-up that proposes the
"on-flash" parser bindings. Sound OK?

Brian

> +
> +
> +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 "fixed-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
> -- 
> 2.11.0
>

From: Rafał Miłecki <rafal@milecki.pl>

My recent work on adding wide support for linux,part-probe was reviewed and
kind of Nack-ed, but fortunately I was pointed to the old (!) patchset from
Brian doing similar thing in a cleaner way.

This patchset picks the important changes from Brian, cleans them and rebases.

Original patches:
(not picked) [RFC PATCH 1/7] mtd: move partition parsers to drivers/mtd/partitions/
(not picked) [RFC PATCH 2/7] mtd: move partition parsers' Kconfig under a sub-menu
(partially)  [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
(picked)     [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
(partially)  [RFC PATCH 5/7] mtd: partitions: factor out "match by name" handling
(picked)     [RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching
(not picked) [RFC PATCH 7/7] mtd: partitions: add Google's FMAP partition parser

At this point this simply adds a full support for "fixed-partitions" binding.
It should also make adding new bindings (like Google's FMAP) easier in the
future.

I've successfully tested this with bcm47xxsflash driver on Tenda AC9 device. I
used following DT node to get "ofpart" driver parse & register my partitions.

&bcma-sflash {
	partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

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

		firmware@40000 {
			label = "firmware";
			reg = <0x40000 0x7f0000>;
		};
	};
};

V2: Modify patch 1/4
    Include list of original patches in 0/4
    Include changelog in every patch
    Add Brian's tags (Acked/Reviewed/Tested)

Brian Norris (3):
  dt-bindings: mtd: make partitions doc a bit more generic
  mtd: partitions: factor out code calling parser
  mtd: partitions: add of_match_table parser matching

Rafał Miłecki (1):
  mtd: ofpart: add of_match_table with "fixed-partitions"

 .../devicetree/bindings/mtd/partition.txt          | 30 ++++++--
 drivers/mtd/mtdpart.c                              | 80 +++++++++++++++++++---
 drivers/mtd/ofpart.c                               |  7 ++
 include/linux/mtd/partitions.h                     |  1 +
 4 files changed, 103 insertions(+), 15 deletions(-)