[PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

Broadcom based home router devices use partitions which have to be
discovered in a specific non-standard way. As there is no partition
table this commit adds and describes a new custom binding.

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
 .../devicetree/bindings/mtd/partition.txt          | 46 +++++++++++++++++++++-
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 36f3b769a626..b201d497b618 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -14,8 +14,6 @@ method is used for a given flash device. To describe the method there should be
 a subnode of the flash device that is named 'partitions'. It must have a
 'compatible' property, which is used to identify the method to use.
 
-We currently only document a binding for fixed layouts.
-
 
 Fixed Partitions
 ================
@@ -109,3 +107,47 @@ flash@2 {
 		};
 	};
 };
+
+
+Broadcom BCM47xx Partitions
+===========================
+
+Broadcom is one of hardware manufacturers providing SoCs (BCM47xx) used in
+home routers. Their BCM947xx boards using CFE bootloader have several partitions
+without any on-flash partition table. On some devices their sizes and/or
+meanings can also vary so fixed partitioning can't be used.
+
+Discovering partitions on these devices is possible thanks to having a special
+header and/or magic signature at the beginning of each of them. They are also
+block aligned which is important for determinig a size.
+
+Most of partitions use ASCII text based magic for determining a type. More
+complex partitions (like TRX with its HDR0 magic) may include extra header
+containing some details, including a length.
+
+A list of supported partitions includes:
+1) Bootloader with Broadcom's CFE (Common Firmware Environment)
+2) NVRAM with configuration/calibration data
+3) Device manufacturer's data with some default values (e.g. SSIDs)
+4) TRX firmware container which can hold up to 4 subpartitions
+5) Backup TRX firmware used after failed upgrade
+
+As mentioned earlier, role of some partitions may depend on extra configuration.
+For example both: main firmware and backup firmware use the same TRX format with
+the same header. To distinguish currently used firmware a CFE's environment
+variable "bootpartition" is used.
+
+
+Devices using Broadcom partitions described above should should have flash node
+with a subnode named "partitions" using following properties:
+
+Required properties:
+- compatible : (required) must be "brcm,bcm947xx-cfe-partitions"
+
+Example:
+
+flash@0 {
+	partitions {
+		compatible = "brcm,bcm947xx-cfe-partitions";
+	};
+};
-- 
2.11.0

--
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

[PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[Rafał Miłecki]

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

Broadcom based home router devices use partitions which have to be
discovered in a specific non-standard way. As there is no partition
table this commit adds and describes a new custom binding.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/mtd/partition.txt          | 46 +++++++++++++++++++++-
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 36f3b769a626..b201d497b618 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -14,8 +14,6 @@ method is used for a given flash device. To describe the method there should be
 a subnode of the flash device that is named 'partitions'. It must have a
 'compatible' property, which is used to identify the method to use.
 
-We currently only document a binding for fixed layouts.
-
 
 Fixed Partitions
 ================
@@ -109,3 +107,47 @@ flash@2 {
 		};
 	};
 };
+
+
+Broadcom BCM47xx Partitions
+===========================
+
+Broadcom is one of hardware manufacturers providing SoCs (BCM47xx) used in
+home routers. Their BCM947xx boards using CFE bootloader have several partitions
+without any on-flash partition table. On some devices their sizes and/or
+meanings can also vary so fixed partitioning can't be used.
+
+Discovering partitions on these devices is possible thanks to having a special
+header and/or magic signature at the beginning of each of them. They are also
+block aligned which is important for determinig a size.
+
+Most of partitions use ASCII text based magic for determining a type. More
+complex partitions (like TRX with its HDR0 magic) may include extra header
+containing some details, including a length.
+
+A list of supported partitions includes:
+1) Bootloader with Broadcom's CFE (Common Firmware Environment)
+2) NVRAM with configuration/calibration data
+3) Device manufacturer's data with some default values (e.g. SSIDs)
+4) TRX firmware container which can hold up to 4 subpartitions
+5) Backup TRX firmware used after failed upgrade
+
+As mentioned earlier, role of some partitions may depend on extra configuration.
+For example both: main firmware and backup firmware use the same TRX format with
+the same header. To distinguish currently used firmware a CFE's environment
+variable "bootpartition" is used.
+
+
+Devices using Broadcom partitions described above should should have flash node
+with a subnode named "partitions" using following properties:
+
+Required properties:
+- compatible : (required) must be "brcm,bcm947xx-cfe-partitions"
+
+Example:
+
+flash@0 {
+	partitions {
+		compatible = "brcm,bcm947xx-cfe-partitions";
+	};
+};
-- 
2.11.0

[PATCH 2/2] mtd: bcm47xxpart: add of_match_table with a new DT binding

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

This allows using bcm47xxpart parser to support DT describing partitions
on a flash using "brcm,bcm947xx-cfe-partitions" compatible property. It
means this parser doesn't have to be explicitly selected by a flash
driver anymore. It can be used e.g. together with a generic m25p80 /
spi-nor if device is just properly described.

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
 drivers/mtd/bcm47xxpart.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/bcm47xxpart.c b/drivers/mtd/bcm47xxpart.c
index fe2581d9d882..4bc2ad842eee 100644
--- a/drivers/mtd/bcm47xxpart.c
+++ b/drivers/mtd/bcm47xxpart.c
@@ -290,9 +290,16 @@ static int bcm47xxpart_parse(struct mtd_info *master,
 	return curr_part;
 };
 
+static const struct of_device_id bcm47xxpart_of_match_table[] = {
+	{ .compatible = "brcm,bcm947xx-cfe-partitions" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, bcm47xxpart_of_match_table);
+
 static struct mtd_part_parser bcm47xxpart_mtd_parser = {
 	.parse_fn = bcm47xxpart_parse,
 	.name = "bcm47xxpart",
+	.of_match_table = bcm47xxpart_of_match_table,
 };
 module_mtd_part_parser(bcm47xxpart_mtd_parser);
 
-- 
2.11.0

--
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

[PATCH 2/2] mtd: bcm47xxpart: add of_match_table with a new DT binding

[Rafał Miłecki]

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

This allows using bcm47xxpart parser to support DT describing partitions
on a flash using "brcm,bcm947xx-cfe-partitions" compatible property. It
means this parser doesn't have to be explicitly selected by a flash
driver anymore. It can be used e.g. together with a generic m25p80 /
spi-nor if device is just properly described.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/mtd/bcm47xxpart.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/bcm47xxpart.c b/drivers/mtd/bcm47xxpart.c
index fe2581d9d882..4bc2ad842eee 100644
--- a/drivers/mtd/bcm47xxpart.c
+++ b/drivers/mtd/bcm47xxpart.c
@@ -290,9 +290,16 @@ static int bcm47xxpart_parse(struct mtd_info *master,
 	return curr_part;
 };
 
+static const struct of_device_id bcm47xxpart_of_match_table[] = {
+	{ .compatible = "brcm,bcm947xx-cfe-partitions" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, bcm47xxpart_of_match_table);
+
 static struct mtd_part_parser bcm47xxpart_mtd_parser = {
 	.parse_fn = bcm47xxpart_parse,
 	.name = "bcm47xxpart",
+	.of_match_table = bcm47xxpart_of_match_table,
 };
 module_mtd_part_parser(bcm47xxpart_mtd_parser);
 
-- 
2.11.0

Re: [PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[Rob Herring]

On Fri, Jan 12, 2018 at 10:33:53AM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> 
> Broadcom based home router devices use partitions which have to be
> discovered in a specific non-standard way. As there is no partition
> table this commit adds and describes a new custom binding.
> 
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
>  .../devicetree/bindings/mtd/partition.txt          | 46 +++++++++++++++++++++-
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 36f3b769a626..b201d497b618 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -14,8 +14,6 @@ method is used for a given flash device. To describe the method there should be
>  a subnode of the flash device that is named 'partitions'. It must have a
>  'compatible' property, which is used to identify the method to use.
>  
> -We currently only document a binding for fixed layouts.
> -
>  
>  Fixed Partitions
>  ================
> @@ -109,3 +107,47 @@ flash@2 {
>  		};
>  	};
>  };
> +
> +
> +Broadcom BCM47xx Partitions
> +===========================
> +
> +Broadcom is one of hardware manufacturers providing SoCs (BCM47xx) used in
> +home routers. Their BCM947xx boards using CFE bootloader have several partitions
> +without any on-flash partition table. On some devices their sizes and/or
> +meanings can also vary so fixed partitioning can't be used.
> +
> +Discovering partitions on these devices is possible thanks to having a special
> +header and/or magic signature at the beginning of each of them. They are also
> +block aligned which is important for determinig a size.
> +
> +Most of partitions use ASCII text based magic for determining a type. More
> +complex partitions (like TRX with its HDR0 magic) may include extra header
> +containing some details, including a length.
> +
> +A list of supported partitions includes:
> +1) Bootloader with Broadcom's CFE (Common Firmware Environment)
> +2) NVRAM with configuration/calibration data
> +3) Device manufacturer's data with some default values (e.g. SSIDs)
> +4) TRX firmware container which can hold up to 4 subpartitions
> +5) Backup TRX firmware used after failed upgrade
> +
> +As mentioned earlier, role of some partitions may depend on extra configuration.
> +For example both: main firmware and backup firmware use the same TRX format with
> +the same header. To distinguish currently used firmware a CFE's environment
> +variable "bootpartition" is used.
> +
> +
> +Devices using Broadcom partitions described above should should have flash node
> +with a subnode named "partitions" using following properties:
> +
> +Required properties:
> +- compatible : (required) must be "brcm,bcm947xx-cfe-partitions"

So you can't discover you have CFE partitions, but you can discover 
everything else like where partitions start? 

Why doesn't the kernel just probe for the partition table?

Rob
--
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: [PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[Rob Herring]

On Fri, Jan 12, 2018 at 10:33:53AM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Broadcom based home router devices use partitions which have to be
> discovered in a specific non-standard way. As there is no partition
> table this commit adds and describes a new custom binding.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/mtd/partition.txt          | 46 +++++++++++++++++++++-
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 36f3b769a626..b201d497b618 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -14,8 +14,6 @@ method is used for a given flash device. To describe the method there should be
>  a subnode of the flash device that is named 'partitions'. It must have a
>  'compatible' property, which is used to identify the method to use.
>  
> -We currently only document a binding for fixed layouts.
> -
>  
>  Fixed Partitions
>  ================
> @@ -109,3 +107,47 @@ flash@2 {
>  		};
>  	};
>  };
> +
> +
> +Broadcom BCM47xx Partitions
> +===========================
> +
> +Broadcom is one of hardware manufacturers providing SoCs (BCM47xx) used in
> +home routers. Their BCM947xx boards using CFE bootloader have several partitions
> +without any on-flash partition table. On some devices their sizes and/or
> +meanings can also vary so fixed partitioning can't be used.
> +
> +Discovering partitions on these devices is possible thanks to having a special
> +header and/or magic signature at the beginning of each of them. They are also
> +block aligned which is important for determinig a size.
> +
> +Most of partitions use ASCII text based magic for determining a type. More
> +complex partitions (like TRX with its HDR0 magic) may include extra header
> +containing some details, including a length.
> +
> +A list of supported partitions includes:
> +1) Bootloader with Broadcom's CFE (Common Firmware Environment)
> +2) NVRAM with configuration/calibration data
> +3) Device manufacturer's data with some default values (e.g. SSIDs)
> +4) TRX firmware container which can hold up to 4 subpartitions
> +5) Backup TRX firmware used after failed upgrade
> +
> +As mentioned earlier, role of some partitions may depend on extra configuration.
> +For example both: main firmware and backup firmware use the same TRX format with
> +the same header. To distinguish currently used firmware a CFE's environment
> +variable "bootpartition" is used.
> +
> +
> +Devices using Broadcom partitions described above should should have flash node
> +with a subnode named "partitions" using following properties:
> +
> +Required properties:
> +- compatible : (required) must be "brcm,bcm947xx-cfe-partitions"

So you can't discover you have CFE partitions, but you can discover 
everything else like where partitions start? 

Why doesn't the kernel just probe for the partition table?

Rob

Re: [PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[Rafał Miłecki]

Hi Rob & sorry for the late reply. My earlier accepted patchset adding
of_match_table matching was reported to cause a regression. It had to be
dropped and I had to focus on fixing that first.

On 01/19/2018 10:44 PM, Rob Herring wrote:
> On Fri, Jan 12, 2018 at 10:33:53AM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Broadcom based home router devices use partitions which have to be
>> discovered in a specific non-standard way. As there is no partition
>> table this commit adds and describes a new custom binding.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>  .../devicetree/bindings/mtd/partition.txt          | 46 +++++++++++++++++++++-
>>  1 file changed, 44 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
>> index 36f3b769a626..b201d497b618 100644
>> --- a/Documentation/devicetree/bindings/mtd/partition.txt
>> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
>> @@ -14,8 +14,6 @@ method is used for a given flash device. To describe the method there should be
>>  a subnode of the flash device that is named 'partitions'. It must have a
>>  'compatible' property, which is used to identify the method to use.
>>
>> -We currently only document a binding for fixed layouts.
>> -
>>
>>  Fixed Partitions
>>  ================
>> @@ -109,3 +107,47 @@ flash@2 {
>>  		};
>>  	};
>>  };
>> +
>> +
>> +Broadcom BCM47xx Partitions
>> +===========================
>> +
>> +Broadcom is one of hardware manufacturers providing SoCs (BCM47xx) used in
>> +home routers. Their BCM947xx boards using CFE bootloader have several partitions
>> +without any on-flash partition table. On some devices their sizes and/or
>> +meanings can also vary so fixed partitioning can't be used.
>> +
>> +Discovering partitions on these devices is possible thanks to having a special
>> +header and/or magic signature at the beginning of each of them. They are also
>> +block aligned which is important for determinig a size.
>> +
>> +Most of partitions use ASCII text based magic for determining a type. More
>> +complex partitions (like TRX with its HDR0 magic) may include extra header
>> +containing some details, including a length.
>> +
>> +A list of supported partitions includes:
>> +1) Bootloader with Broadcom's CFE (Common Firmware Environment)
>> +2) NVRAM with configuration/calibration data
>> +3) Device manufacturer's data with some default values (e.g. SSIDs)
>> +4) TRX firmware container which can hold up to 4 subpartitions
>> +5) Backup TRX firmware used after failed upgrade
>> +
>> +As mentioned earlier, role of some partitions may depend on extra configuration.
>> +For example both: main firmware and backup firmware use the same TRX format with
>> +the same header. To distinguish currently used firmware a CFE's environment
>> +variable "bootpartition" is used.
>> +
>> +
>> +Devices using Broadcom partitions described above should should have flash node
>> +with a subnode named "partitions" using following properties:
>> +
>> +Required properties:
>> +- compatible : (required) must be "brcm,bcm947xx-cfe-partitions"
>
> So you can't discover you have CFE partitions, but you can discover
> everything else like where partitions start?

I could discover there is a CFE bootloader at the flash beginning and
assume there are CFE-like partitions. It doesn't sound safe however and
I believe the point of using DT + compatible properties is to avoid such
/guessing/. There are too many cases, devices, "formats" to just blindly
try every possible parser. That's why we agreed to use "compatible"
strings.

This was also described by Boris in his patchset 2+ years ago:
[RFC PATCH 0/7] mtd: partitions: add of_match_table support
http://lists.infradead.org/pipermail/linux-mtd/2015-December/064076.html

 >  (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 also believe this is solution you acked back then:
[RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
http://lists.infradead.org/pipermail/linux-mtd/2015-December/064100.html

 > 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.
 > >
 > > (...)
 >
 > 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 at kernel.org>

So I guess the answer is:
1) I really don't want to blindly look for CFE-like partitions. Too much
    risk of false positives.
2) I don't want parser to be blindly called & use extra magic checks to
    make sure I'm dealing with device using CFE bootloader.
3) Once I know I'm dealing with device using CFE bootloader I can find
    partitions on the flash.


> Why doesn't the kernel just probe for the partition table?

If by a "partition table" you mean a single block with data using a
clear structure: it doesn't exist. That's the whole point with crappy
Broadcom devices.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[Rafał Miłecki]

Hi Rob & sorry for the late reply. My earlier accepted patchset adding
of_match_table matching was reported to cause a regression. It had to be
dropped and I had to focus on fixing that first.

On 01/19/2018 10:44 PM, Rob Herring wrote:
> On Fri, Jan 12, 2018 at 10:33:53AM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Broadcom based home router devices use partitions which have to be
>> discovered in a specific non-standard way. As there is no partition
>> table this commit adds and describes a new custom binding.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>  .../devicetree/bindings/mtd/partition.txt          | 46 +++++++++++++++++++++-
>>  1 file changed, 44 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
>> index 36f3b769a626..b201d497b618 100644
>> --- a/Documentation/devicetree/bindings/mtd/partition.txt
>> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
>> @@ -14,8 +14,6 @@ method is used for a given flash device. To describe the method there should be
>>  a subnode of the flash device that is named 'partitions'. It must have a
>>  'compatible' property, which is used to identify the method to use.
>>
>> -We currently only document a binding for fixed layouts.
>> -
>>
>>  Fixed Partitions
>>  ================
>> @@ -109,3 +107,47 @@ flash@2 {
>>  		};
>>  	};
>>  };
>> +
>> +
>> +Broadcom BCM47xx Partitions
>> +===========================
>> +
>> +Broadcom is one of hardware manufacturers providing SoCs (BCM47xx) used in
>> +home routers. Their BCM947xx boards using CFE bootloader have several partitions
>> +without any on-flash partition table. On some devices their sizes and/or
>> +meanings can also vary so fixed partitioning can't be used.
>> +
>> +Discovering partitions on these devices is possible thanks to having a special
>> +header and/or magic signature at the beginning of each of them. They are also
>> +block aligned which is important for determinig a size.
>> +
>> +Most of partitions use ASCII text based magic for determining a type. More
>> +complex partitions (like TRX with its HDR0 magic) may include extra header
>> +containing some details, including a length.
>> +
>> +A list of supported partitions includes:
>> +1) Bootloader with Broadcom's CFE (Common Firmware Environment)
>> +2) NVRAM with configuration/calibration data
>> +3) Device manufacturer's data with some default values (e.g. SSIDs)
>> +4) TRX firmware container which can hold up to 4 subpartitions
>> +5) Backup TRX firmware used after failed upgrade
>> +
>> +As mentioned earlier, role of some partitions may depend on extra configuration.
>> +For example both: main firmware and backup firmware use the same TRX format with
>> +the same header. To distinguish currently used firmware a CFE's environment
>> +variable "bootpartition" is used.
>> +
>> +
>> +Devices using Broadcom partitions described above should should have flash node
>> +with a subnode named "partitions" using following properties:
>> +
>> +Required properties:
>> +- compatible : (required) must be "brcm,bcm947xx-cfe-partitions"
>
> So you can't discover you have CFE partitions, but you can discover
> everything else like where partitions start?

I could discover there is a CFE bootloader at the flash beginning and
assume there are CFE-like partitions. It doesn't sound safe however and
I believe the point of using DT + compatible properties is to avoid such
/guessing/. There are too many cases, devices, "formats" to just blindly
try every possible parser. That's why we agreed to use "compatible"
strings.

This was also described by Boris in his patchset 2+ years ago:
[RFC PATCH 0/7] mtd: partitions: add of_match_table support
http://lists.infradead.org/pipermail/linux-mtd/2015-December/064076.html

 >  (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 also believe this is solution you acked back then:
[RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
http://lists.infradead.org/pipermail/linux-mtd/2015-December/064100.html

 > 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.
 > >
 > > (...)
 >
 > 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 at kernel.org>

So I guess the answer is:
1) I really don't want to blindly look for CFE-like partitions. Too much
    risk of false positives.
2) I don't want parser to be blindly called & use extra magic checks to
    make sure I'm dealing with device using CFE bootloader.
3) Once I know I'm dealing with device using CFE bootloader I can find
    partitions on the flash.


> Why doesn't the kernel just probe for the partition table?

If by a "partition table" you mean a single block with data using a
clear structure: it doesn't exist. That's the whole point with crappy
Broadcom devices.

Re: [PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[Rafał Miłecki]

Hi Rob,

On 29 March 2018 at 08:23, Rafał Miłecki <zajec5@gmail.com> wrote:
> Hi Rob & sorry for the late reply. My earlier accepted patchset adding
> of_match_table matching was reported to cause a regression. It had to be
> dropped and I had to focus on fixing that first.
>
>
> On 01/19/2018 10:44 PM, Rob Herring wrote:
>>
>> On Fri, Jan 12, 2018 at 10:33:53AM +0100, Rafał Miłecki wrote:
>>>
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Broadcom based home router devices use partitions which have to be
>>> discovered in a specific non-standard way. As there is no partition
>>> table this commit adds and describes a new custom binding.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>>  .../devicetree/bindings/mtd/partition.txt          | 46
>>> +++++++++++++++++++++-
>>>  1 file changed, 44 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt
>>> b/Documentation/devicetree/bindings/mtd/partition.txt
>>> index 36f3b769a626..b201d497b618 100644
>>> --- a/Documentation/devicetree/bindings/mtd/partition.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
>>> @@ -14,8 +14,6 @@ method is used for a given flash device. To describe
>>> the method there should be
>>>  a subnode of the flash device that is named 'partitions'. It must have a
>>>  'compatible' property, which is used to identify the method to use.
>>>
>>> -We currently only document a binding for fixed layouts.
>>> -
>>>
>>>  Fixed Partitions
>>>  ================
>>> @@ -109,3 +107,47 @@ flash@2 {
>>>                 };
>>>         };
>>>  };
>>> +
>>> +
>>> +Broadcom BCM47xx Partitions
>>> +===========================
>>> +
>>> +Broadcom is one of hardware manufacturers providing SoCs (BCM47xx) used
>>> in
>>> +home routers. Their BCM947xx boards using CFE bootloader have several
>>> partitions
>>> +without any on-flash partition table. On some devices their sizes and/or
>>> +meanings can also vary so fixed partitioning can't be used.
>>> +
>>> +Discovering partitions on these devices is possible thanks to having a
>>> special
>>> +header and/or magic signature at the beginning of each of them. They are
>>> also
>>> +block aligned which is important for determinig a size.
>>> +
>>> +Most of partitions use ASCII text based magic for determining a type.
>>> More
>>> +complex partitions (like TRX with its HDR0 magic) may include extra
>>> header
>>> +containing some details, including a length.
>>> +
>>> +A list of supported partitions includes:
>>> +1) Bootloader with Broadcom's CFE (Common Firmware Environment)
>>> +2) NVRAM with configuration/calibration data
>>> +3) Device manufacturer's data with some default values (e.g. SSIDs)
>>> +4) TRX firmware container which can hold up to 4 subpartitions
>>> +5) Backup TRX firmware used after failed upgrade
>>> +
>>> +As mentioned earlier, role of some partitions may depend on extra
>>> configuration.
>>> +For example both: main firmware and backup firmware use the same TRX
>>> format with
>>> +the same header. To distinguish currently used firmware a CFE's
>>> environment
>>> +variable "bootpartition" is used.
>>> +
>>> +
>>> +Devices using Broadcom partitions described above should should have
>>> flash node
>>> +with a subnode named "partitions" using following properties:
>>> +
>>> +Required properties:
>>> +- compatible : (required) must be "brcm,bcm947xx-cfe-partitions"
>>
>>
>> So you can't discover you have CFE partitions, but you can discover
>> everything else like where partitions start?
>
>
> I could discover there is a CFE bootloader at the flash beginning and
> assume there are CFE-like partitions. It doesn't sound safe however and
> I believe the point of using DT + compatible properties is to avoid such
> /guessing/. There are too many cases, devices, "formats" to just blindly
> try every possible parser. That's why we agreed to use "compatible"
> strings.
>
> This was also described by Boris in his patchset 2+ years ago:
> [RFC PATCH 0/7] mtd: partitions: add of_match_table support
> http://lists.infradead.org/pipermail/linux-mtd/2015-December/064076.html
>
>>  (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 also believe this is solution you acked back then:
> [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
> http://lists.infradead.org/pipermail/linux-mtd/2015-December/064100.html
>
>> 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.
>> >
>> > (...)
>>
>> 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 at kernel.org>
>
> So I guess the answer is:
> 1) I really don't want to blindly look for CFE-like partitions. Too much
>    risk of false positives.
> 2) I don't want parser to be blindly called & use extra magic checks to
>    make sure I'm dealing with device using CFE bootloader.
> 3) Once I know I'm dealing with device using CFE bootloader I can find
>    partitions on the flash.
>
>
>> Why doesn't the kernel just probe for the partition table?
>
>
> If by a "partition table" you mean a single block with data using a
> clear structure: it doesn't exist. That's the whole point with crappy
> Broadcom devices.

Would you find a moment to review that patch again, after extra
explanation I provided, please?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[Rafał Miłecki]

Hi Rob,

On 29 March 2018 at 08:23, Rafał Miłecki <zajec5@gmail.com> wrote:
> Hi Rob & sorry for the late reply. My earlier accepted patchset adding
> of_match_table matching was reported to cause a regression. It had to be
> dropped and I had to focus on fixing that first.
>
>
> On 01/19/2018 10:44 PM, Rob Herring wrote:
>>
>> On Fri, Jan 12, 2018 at 10:33:53AM +0100, Rafał Miłecki wrote:
>>>
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Broadcom based home router devices use partitions which have to be
>>> discovered in a specific non-standard way. As there is no partition
>>> table this commit adds and describes a new custom binding.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>>  .../devicetree/bindings/mtd/partition.txt          | 46
>>> +++++++++++++++++++++-
>>>  1 file changed, 44 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt
>>> b/Documentation/devicetree/bindings/mtd/partition.txt
>>> index 36f3b769a626..b201d497b618 100644
>>> --- a/Documentation/devicetree/bindings/mtd/partition.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
>>> @@ -14,8 +14,6 @@ method is used for a given flash device. To describe
>>> the method there should be
>>>  a subnode of the flash device that is named 'partitions'. It must have a
>>>  'compatible' property, which is used to identify the method to use.
>>>
>>> -We currently only document a binding for fixed layouts.
>>> -
>>>
>>>  Fixed Partitions
>>>  ================
>>> @@ -109,3 +107,47 @@ flash@2 {
>>>                 };
>>>         };
>>>  };
>>> +
>>> +
>>> +Broadcom BCM47xx Partitions
>>> +===========================
>>> +
>>> +Broadcom is one of hardware manufacturers providing SoCs (BCM47xx) used
>>> in
>>> +home routers. Their BCM947xx boards using CFE bootloader have several
>>> partitions
>>> +without any on-flash partition table. On some devices their sizes and/or
>>> +meanings can also vary so fixed partitioning can't be used.
>>> +
>>> +Discovering partitions on these devices is possible thanks to having a
>>> special
>>> +header and/or magic signature at the beginning of each of them. They are
>>> also
>>> +block aligned which is important for determinig a size.
>>> +
>>> +Most of partitions use ASCII text based magic for determining a type.
>>> More
>>> +complex partitions (like TRX with its HDR0 magic) may include extra
>>> header
>>> +containing some details, including a length.
>>> +
>>> +A list of supported partitions includes:
>>> +1) Bootloader with Broadcom's CFE (Common Firmware Environment)
>>> +2) NVRAM with configuration/calibration data
>>> +3) Device manufacturer's data with some default values (e.g. SSIDs)
>>> +4) TRX firmware container which can hold up to 4 subpartitions
>>> +5) Backup TRX firmware used after failed upgrade
>>> +
>>> +As mentioned earlier, role of some partitions may depend on extra
>>> configuration.
>>> +For example both: main firmware and backup firmware use the same TRX
>>> format with
>>> +the same header. To distinguish currently used firmware a CFE's
>>> environment
>>> +variable "bootpartition" is used.
>>> +
>>> +
>>> +Devices using Broadcom partitions described above should should have
>>> flash node
>>> +with a subnode named "partitions" using following properties:
>>> +
>>> +Required properties:
>>> +- compatible : (required) must be "brcm,bcm947xx-cfe-partitions"
>>
>>
>> So you can't discover you have CFE partitions, but you can discover
>> everything else like where partitions start?
>
>
> I could discover there is a CFE bootloader at the flash beginning and
> assume there are CFE-like partitions. It doesn't sound safe however and
> I believe the point of using DT + compatible properties is to avoid such
> /guessing/. There are too many cases, devices, "formats" to just blindly
> try every possible parser. That's why we agreed to use "compatible"
> strings.
>
> This was also described by Boris in his patchset 2+ years ago:
> [RFC PATCH 0/7] mtd: partitions: add of_match_table support
> http://lists.infradead.org/pipermail/linux-mtd/2015-December/064076.html
>
>>  (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 also believe this is solution you acked back then:
> [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
> http://lists.infradead.org/pipermail/linux-mtd/2015-December/064100.html
>
>> 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.
>> >
>> > (...)
>>
>> 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 at kernel.org>
>
> So I guess the answer is:
> 1) I really don't want to blindly look for CFE-like partitions. Too much
>    risk of false positives.
> 2) I don't want parser to be blindly called & use extra magic checks to
>    make sure I'm dealing with device using CFE bootloader.
> 3) Once I know I'm dealing with device using CFE bootloader I can find
>    partitions on the flash.
>
>
>> Why doesn't the kernel just probe for the partition table?
>
>
> If by a "partition table" you mean a single block with data using a
> clear structure: it doesn't exist. That's the whole point with crappy
> Broadcom devices.

Would you find a moment to review that patch again, after extra
explanation I provided, please?

Re: [PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[Rob Herring]

On Sat, May 05, 2018 at 11:47:31PM +0200, Rafał Miłecki wrote:
> Hi Rob,
> 
> On 29 March 2018 at 08:23, Rafał Miłecki <zajec5@gmail.com> wrote:
> > Hi Rob & sorry for the late reply. My earlier accepted patchset adding
> > of_match_table matching was reported to cause a regression. It had to be
> > dropped and I had to focus on fixing that first.
> >
> >
> > On 01/19/2018 10:44 PM, Rob Herring wrote:
> >>
> >> On Fri, Jan 12, 2018 at 10:33:53AM +0100, Rafał Miłecki wrote:
> >>>
> >>> From: Rafał Miłecki <rafal@milecki.pl>
> >>>
> >>> Broadcom based home router devices use partitions which have to be
> >>> discovered in a specific non-standard way. As there is no partition
> >>> table this commit adds and describes a new custom binding.
> >>>
> >>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> >>> ---
> >>>  .../devicetree/bindings/mtd/partition.txt          | 46
> >>> +++++++++++++++++++++-
> >>>  1 file changed, 44 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt
> >>> b/Documentation/devicetree/bindings/mtd/partition.txt
> >>> index 36f3b769a626..b201d497b618 100644
> >>> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> >>> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> >>> @@ -14,8 +14,6 @@ method is used for a given flash device. To describe
> >>> the method there should be
> >>>  a subnode of the flash device that is named 'partitions'. It must have a
> >>>  'compatible' property, which is used to identify the method to use.
> >>>
> >>> -We currently only document a binding for fixed layouts.
> >>> -
> >>>
> >>>  Fixed Partitions
> >>>  ================
> >>> @@ -109,3 +107,47 @@ flash@2 {
> >>>                 };
> >>>         };
> >>>  };
> >>> +
> >>> +
> >>> +Broadcom BCM47xx Partitions
> >>> +===========================
> >>> +
> >>> +Broadcom is one of hardware manufacturers providing SoCs (BCM47xx) used
> >>> in
> >>> +home routers. Their BCM947xx boards using CFE bootloader have several
> >>> partitions
> >>> +without any on-flash partition table. On some devices their sizes and/or
> >>> +meanings can also vary so fixed partitioning can't be used.
> >>> +
> >>> +Discovering partitions on these devices is possible thanks to having a
> >>> special
> >>> +header and/or magic signature at the beginning of each of them. They are
> >>> also
> >>> +block aligned which is important for determinig a size.
> >>> +
> >>> +Most of partitions use ASCII text based magic for determining a type.
> >>> More
> >>> +complex partitions (like TRX with its HDR0 magic) may include extra
> >>> header
> >>> +containing some details, including a length.
> >>> +
> >>> +A list of supported partitions includes:
> >>> +1) Bootloader with Broadcom's CFE (Common Firmware Environment)
> >>> +2) NVRAM with configuration/calibration data
> >>> +3) Device manufacturer's data with some default values (e.g. SSIDs)
> >>> +4) TRX firmware container which can hold up to 4 subpartitions
> >>> +5) Backup TRX firmware used after failed upgrade
> >>> +
> >>> +As mentioned earlier, role of some partitions may depend on extra
> >>> configuration.
> >>> +For example both: main firmware and backup firmware use the same TRX
> >>> format with
> >>> +the same header. To distinguish currently used firmware a CFE's
> >>> environment
> >>> +variable "bootpartition" is used.
> >>> +
> >>> +
> >>> +Devices using Broadcom partitions described above should should have
> >>> flash node
> >>> +with a subnode named "partitions" using following properties:
> >>> +
> >>> +Required properties:
> >>> +- compatible : (required) must be "brcm,bcm947xx-cfe-partitions"
> >>
> >>
> >> So you can't discover you have CFE partitions, but you can discover
> >> everything else like where partitions start?
> >
> >
> > I could discover there is a CFE bootloader at the flash beginning and
> > assume there are CFE-like partitions. It doesn't sound safe however and
> > I believe the point of using DT + compatible properties is to avoid such
> > /guessing/. There are too many cases, devices, "formats" to just blindly
> > try every possible parser. That's why we agreed to use "compatible"
> > strings.
> >
> > This was also described by Boris in his patchset 2+ years ago:
> > [RFC PATCH 0/7] mtd: partitions: add of_match_table support
> > http://lists.infradead.org/pipermail/linux-mtd/2015-December/064076.html
> >
> >>  (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 also believe this is solution you acked back then:
> > [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
> > http://lists.infradead.org/pipermail/linux-mtd/2015-December/064100.html
> >
> >> 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.
> >> >
> >> > (...)
> >>
> >> 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 at kernel.org>
> >
> > So I guess the answer is:
> > 1) I really don't want to blindly look for CFE-like partitions. Too much
> >    risk of false positives.
> > 2) I don't want parser to be blindly called & use extra magic checks to
> >    make sure I'm dealing with device using CFE bootloader.
> > 3) Once I know I'm dealing with device using CFE bootloader I can find
> >    partitions on the flash.
> >
> >
> >> Why doesn't the kernel just probe for the partition table?
> >
> >
> > If by a "partition table" you mean a single block with data using a
> > clear structure: it doesn't exist. That's the whole point with crappy
> > Broadcom devices.
> 
> Would you find a moment to review that patch again, after extra
> explanation I provided, please?

I guess this is fine. Can you resend as I don't have the original patch. 

I think it should be a separate file though as partition.txt would 
become very long if every vendor partitioning was added there.

Rob

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/2] dt-bindings: mtd: document Broadcom's BCM47xx partitions

[Rob Herring]

On Sat, May 05, 2018 at 11:47:31PM +0200, Rafał Miłecki wrote:
> Hi Rob,
> 
> On 29 March 2018 at 08:23, Rafał Miłecki <zajec5@gmail.com> wrote:
> > Hi Rob & sorry for the late reply. My earlier accepted patchset adding
> > of_match_table matching was reported to cause a regression. It had to be
> > dropped and I had to focus on fixing that first.
> >
> >
> > On 01/19/2018 10:44 PM, Rob Herring wrote:
> >>
> >> On Fri, Jan 12, 2018 at 10:33:53AM +0100, Rafał Miłecki wrote:
> >>>
> >>> From: Rafał Miłecki <rafal@milecki.pl>
> >>>
> >>> Broadcom based home router devices use partitions which have to be
> >>> discovered in a specific non-standard way. As there is no partition
> >>> table this commit adds and describes a new custom binding.
> >>>
> >>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> >>> ---
> >>>  .../devicetree/bindings/mtd/partition.txt          | 46
> >>> +++++++++++++++++++++-
> >>>  1 file changed, 44 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt
> >>> b/Documentation/devicetree/bindings/mtd/partition.txt
> >>> index 36f3b769a626..b201d497b618 100644
> >>> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> >>> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> >>> @@ -14,8 +14,6 @@ method is used for a given flash device. To describe
> >>> the method there should be
> >>>  a subnode of the flash device that is named 'partitions'. It must have a
> >>>  'compatible' property, which is used to identify the method to use.
> >>>
> >>> -We currently only document a binding for fixed layouts.
> >>> -
> >>>
> >>>  Fixed Partitions
> >>>  ================
> >>> @@ -109,3 +107,47 @@ flash@2 {
> >>>                 };
> >>>         };
> >>>  };
> >>> +
> >>> +
> >>> +Broadcom BCM47xx Partitions
> >>> +===========================
> >>> +
> >>> +Broadcom is one of hardware manufacturers providing SoCs (BCM47xx) used
> >>> in
> >>> +home routers. Their BCM947xx boards using CFE bootloader have several
> >>> partitions
> >>> +without any on-flash partition table. On some devices their sizes and/or
> >>> +meanings can also vary so fixed partitioning can't be used.
> >>> +
> >>> +Discovering partitions on these devices is possible thanks to having a
> >>> special
> >>> +header and/or magic signature at the beginning of each of them. They are
> >>> also
> >>> +block aligned which is important for determinig a size.
> >>> +
> >>> +Most of partitions use ASCII text based magic for determining a type.
> >>> More
> >>> +complex partitions (like TRX with its HDR0 magic) may include extra
> >>> header
> >>> +containing some details, including a length.
> >>> +
> >>> +A list of supported partitions includes:
> >>> +1) Bootloader with Broadcom's CFE (Common Firmware Environment)
> >>> +2) NVRAM with configuration/calibration data
> >>> +3) Device manufacturer's data with some default values (e.g. SSIDs)
> >>> +4) TRX firmware container which can hold up to 4 subpartitions
> >>> +5) Backup TRX firmware used after failed upgrade
> >>> +
> >>> +As mentioned earlier, role of some partitions may depend on extra
> >>> configuration.
> >>> +For example both: main firmware and backup firmware use the same TRX
> >>> format with
> >>> +the same header. To distinguish currently used firmware a CFE's
> >>> environment
> >>> +variable "bootpartition" is used.
> >>> +
> >>> +
> >>> +Devices using Broadcom partitions described above should should have
> >>> flash node
> >>> +with a subnode named "partitions" using following properties:
> >>> +
> >>> +Required properties:
> >>> +- compatible : (required) must be "brcm,bcm947xx-cfe-partitions"
> >>
> >>
> >> So you can't discover you have CFE partitions, but you can discover
> >> everything else like where partitions start?
> >
> >
> > I could discover there is a CFE bootloader at the flash beginning and
> > assume there are CFE-like partitions. It doesn't sound safe however and
> > I believe the point of using DT + compatible properties is to avoid such
> > /guessing/. There are too many cases, devices, "formats" to just blindly
> > try every possible parser. That's why we agreed to use "compatible"
> > strings.
> >
> > This was also described by Boris in his patchset 2+ years ago:
> > [RFC PATCH 0/7] mtd: partitions: add of_match_table support
> > http://lists.infradead.org/pipermail/linux-mtd/2015-December/064076.html
> >
> >>  (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 also believe this is solution you acked back then:
> > [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
> > http://lists.infradead.org/pipermail/linux-mtd/2015-December/064100.html
> >
> >> 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.
> >> >
> >> > (...)
> >>
> >> 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 at kernel.org>
> >
> > So I guess the answer is:
> > 1) I really don't want to blindly look for CFE-like partitions. Too much
> >    risk of false positives.
> > 2) I don't want parser to be blindly called & use extra magic checks to
> >    make sure I'm dealing with device using CFE bootloader.
> > 3) Once I know I'm dealing with device using CFE bootloader I can find
> >    partitions on the flash.
> >
> >
> >> Why doesn't the kernel just probe for the partition table?
> >
> >
> > If by a "partition table" you mean a single block with data using a
> > clear structure: it doesn't exist. That's the whole point with crappy
> > Broadcom devices.
> 
> Would you find a moment to review that patch again, after extra
> explanation I provided, please?

I guess this is fine. Can you resend as I don't have the original patch. 

I think it should be a separate file though as partition.txt would 
become very long if every vendor partitioning was added there.

Rob