mtd: document linux-specific partition parser DT binding

The Linux code in drivers/mtd/maps/physmap_of.c will optionally look for this binding for hints on a partition type to look for in the MTD. It was added in commit 9d5da3a9b849
"mtd: extend physmap_of to let the device tree specify the parition probe" but no corresponding device tree binding was added. Fix this.

---
 Documentation/devicetree/bindings/mtd/mtd-physmap.txt | 3 +++
 1 file changed, 3 insertions(+)

index 4a0a48bf4ecb..0dee084651da 100644
@@ -26,6 +26,9 @@ file systems on embedded devices.
  - linux,mtd-name: allow to specify the mtd name for retro capability with
    physmap-flash drivers as boot loader pass the mtd partition via the old
    device name physmap-flash.
+ - linux,part-probe: a flash partition type to look for, using the
+   Linux-internal partition naming scheme, e.g. "afs" for the ARM
+   Flash footers.
  - use-advanced-sector-protection: boolean to enable support for the
    advanced sector protection (Spansion: PPB - Persistent Protection
    Bits) locking.

2.4.3

+ others

Hi Linus,

Are you trying to use this binding, or is this just purely a mechanical documentation issue? I ask, because it seems that binding never really got reviewed at all, and others have recently tried to extend support for it generically [1], but a few objections came up [2][3].

Unfortunately I/we dropped the ball a bit on that thread, but we'd ideally like to address those concerns in a new binding that is supported for all MTDs, and deprecate the old one. The new one would probably not directly use the parser name as used by Linux, but define some list of compatible strings that fit DT conventions better. Also, I don't want people including things like "cmdlinepart" in DT, but it should be available as an override if necessary. IOW, DT shouldn't supersede the kernel command line.

That's not to say we can't document the old one, but I'm curious if there are real users. I'd also like to encourage new users to avoid the old one if we can make that feasible.

    http://thread.gmane.org/gmane.linux.drivers.devicetree/122479/focus=122788

I wrote it before ARM started down the DT direction and all these formalities were developed.

We continue to use it here, there is just no way to autodetect flash partition layouts. I'm not fussed about having to change to something else in future.

The reason the original patch exposed the cmdline parser is because that is how the internal mechanisms work - but realistically, the cmdline parser is a hack to get around the lack of DT partition description. If the DT can describe the partitions it should supersede and obsolete the old command line hack. IMHO

That is why it is prefixed with linux, the review of the part parser name is the responsibility of the MTD crew, not the DT folks.

bcm47xxpart seems like a terrible name for a partition scheme. Really, almost any scheme can be used on any SOC, naming a partition parser after a SOC family makes very little sense to me..

Jason

Hi Jason,

Good to know.

Right. It'd be lovely if things were more standardized, and we could just build all parsers in for all MTDs, like how block devices do it, but sadly that's probably not feasible, given the quirks of various parsers, and how they often require scanning a large portion of the device :(

OK, good to know. Recent developments have put maintainers in a tough position sometimes, since as soon as there's a DT binding used in the wild, some people like to claim that it is ABI and must never change. This ties the hands when wanting to deprecate/improve/replace features. In reality, there are few (my guess: a negligible near-zero-percent) DT users that actually have DTs locked into some kind of firmware in a way that they (1) can never update their DT and (2) expect to be able to run mainline kernels, so this all just becomes a lot of fuss over nothing.

Right. And that's why it's best these days not to directly expose internal mechanisms via DT.

Hmm, I don't know. I wouldn't expect people should really be using cmdlineparts as a production solution, but I'd consider it more of a debugging/development option -- if I want to override the DT (which is sometimes a bit harder to change) or the on-flash layout (e.g., RedBoot), then I can fiddle with the command line. So I wouldn't quite qualify it as a hack (though many might use it in a hacky way) nor would I suggest that the DT should override it.

OK. That works for now. But I don't want to extend this binding to any other drivers, if I can help it. And I don't want to encourage it (lest it *really* becomes ABI).

I don't think it would be too hard to fix this to not be so Linux-specific. I think any partition layouts that people would want to specify in DT can be reasonably well-defined to not consider them Linux-specific. Some probably didn't even have a Linux target in mind when they were developed. We'd just need to give them better names and a short description and/or pointers to documentation. e.g. [1][2].

I believe it was defined purely by Broadcom, for use with their firmwares mostly seen on that SoC family. And in the spirit of DT naming, IP often gets named after the first SoC that implements it, even if later revs aren't the same SoC (or SoC family) but are still compatible. So I can see the name being reasonable, even if not perfect.

Any better nominations for names?

Brian

[1] http://infocenter.arm.com/help/topic/com.arm.doc.dui0102g/DUI0102F_afs1_4_rg.pdf
[2] https://sourceware.org/redboot/

We don't have cmdline overrides for very much (if any?) of DT.

The DT is supposed to describe the hardware/boot environment, if overriding the partition layout is really common, then is the DT binding really describing the hardware at all?

The partition layout is already very border line to support in DT, it is really very close to a software configuration which is strongly discouraged from DT files.

I justify it as the boot loader communicating the partition scheme it understands to the OS, so the OS can have a hope of setting up the flash in a way the bootloader understands.

I've never used the cmdline option, so maybe I don't see the use case, but it seems like a really weird desire to change the partitioning away from what the bootloader supports.

bcrm-part-table-43 ?

Jason

I am using it in this devicetree patch:
http://marc.info/?l=linux-arm-kernel&m=144492610417605&w=2

And this devicetree patch:
http://marc.info/?l=linux-arm-kernel&m=144490455308758&w=2

Otherwise the AFS partition type will not be scanned.

The other option is to add "afs" to this list in
drivers/mtd/maps/physmap_of.c:

index 3e614e9119d5..6233473a55d6 100644
@@ -110,7 +110,7 @@ static struct mtd_info *obsolete_probe(struct

platform_device *dev,
default is use. These take precedence over other device tree information. */
static const char * const part_probe_types_def[] = {
- "cmdlinepart", "RedBoot", "ofpart", "ofoldpart", NULL };
+ "cmdlinepart", "RedBoot", "ofpart", "ofoldpart", "afs", NULL };

I can send a patch like this if you prefer?

This is not for cmdlinepart, as AFS is an actual on-flash format.

I'm happy to do either patch, or define a new binding if you prefer, like simply:

partition-type = <string>;

and then we define this as "arm,arm-flash-structure" or something, and parse that to "afs" internally in physmap_of.c.

Ah it does. If we wanna go with this patch I'll fix it.

OK that sounds like a case for "arm,arm-flash-structure" for this specific binding.

Yours,
Linus Walleij

Hi Linus,

Hmm, not really. While I'd love to have a good common set of reasonably generic partition parsers that we could always run on all MTDs (or in this case, all physmap_of MTDs) similar to how the block subsystem does it, I don't think that is really workable here, as there is too much variation in parsers and mamy of them actually scan the whole flash (or at least, the headers of each block). This can get very expensive, and potentially leaves room for false matches, especially when you start layering on all the other potential parsers others might want to use.

IOW, this solution is not scalable IMO. In previous discussions, I think we've agreed that partition parsing must be informed by platform knowledge.

Right. I was just mentioning that this binding:

(1) allows people to specify nonsensical things like "ofpart" and "cmdlinepart" in their DT and
(2) allows people to specify DTs that *don't* allow developer overrides like cmdlinepart (e.g., the partition layout is corrupted, so we want to boot a recovery kernel with a predetermined 'mtdparts=...' parameter)

Neither is directly related to your use of AFS, but it's just part of my thoughts on codifying the use of this binding.

I would prefer a new binding. And personally, I'd like to get something like this extended to all MTDs (there are other users asking for similar things), but I haven't made the time to write the patch :(

Seems OK to me. Again, I'd prefer this not live in physmap_of.c, but if that's the expedient solution, I could probably take it from there.

I'd rather not take this patch if we can safely pivot to an improved binding, but I'm not sure if that violates the DT policy party line :)

Right.

Brian