On Mon, Sep 25, 2023 at 11:25 AM Simon Glass <sjg@xxxxxxxxxxxx> wrote: > > Hi Miquel, > > On Mon, 25 Sept 2023 at 09:24, Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > > > Hi Simon, > > > > > > > > > > > > > I was assuming that I should create a top-level compatible = "binman" > > > > > > > > > > > node, with subnodes like compatible = "binman,bl31-atf", for example. > > > > > > > > > > > I should use the compatible string to indicate the contents, right? > > > > > > > > > > > > > > > > > > > > Yes for subnodes, and we already have some somewhat standard ones for > > > > > > > > > > "u-boot" and "u-boot-env". Though historically, "label" was used. > > > > > > > > > > > > > > > > > > Binman has common properties for all entries, including "compress" > > > > > > > > > which sets the compression algorithm. > > > > > > > > > > > > > > > > I see no issue with adding that. It seems useful and something missing > > > > > > > > in the existing partition schemas. > > > > > > > > > > > > > > OK I sent a patch with that. > > > > > > > > > > > > > > > > > > > > > > > > So perhaps I should start by defining a new binman,bl31-atf which has > > > > > > > > > common properties from an "binman,entry" definition? > > > > > > > > > > > > > > > > I don't understand the binman prefix. The contents are ATF (or TF-A > > > > > > > > now). Who wrote it to the flash image is not relevant. > > > > > > > > > > > > > > Are you suggesting just "atf-bl31", or "arm,atf-bl31" ? Or should we > > > > > > > change it to "tfa-bl31"? > > > > > > > > > > > > I don't really understand the relationship with TF-A here. Can't we > > > > > > just have a kind of fixed-partitions with additional properties like > > > > > > the compression? > > > > > > > > > > Binman needs to know what to put in there, which is the purpose of the > > > > > compatible string. > > > > > > > > But "what" should be put inside the partition is part of the input > > > > argument, not the output. You said (maybe I got this wrong) that the > > > > schema would apply to the output of binman. If you want to let user > > > > know what's inside, maybe it is worth adding a label, but otherwise I > > > > don't like the idea of a compatible for that, which for me would mean: > > > > "here is how to handle that specific portion of the flash/here is how > > > > the flash is organized". > > > > > > But I thought that the compatible string was for that purpose? See for > > > example "brcm,bcm4908-firmware" and "brcm,bcm963xx-imagetag" and > > > "linksys,ns-firmware". > > > > These three examples apparently need specific handling, the partitions > > contain meta-data that a parser needs to check or something like that. > > And finally it looks like partition names are set depending on the > > content that was discovered, so yes, the partition name is likely the > > good location to tell users/OSes what's inside. > > > > > > > > Also, I still don't understand the purpose of this schema. So binman > > > > > > generates an image, you want to flash this image and you would like the > > > > > > tool to generate the corresponding (partition) DT snippet automatically. > > > > > > Do I get this right? I don't get why you would need new compatibles for > > > > > > that. > > > > > > > > > > It is actually the other way around. The schema tells Binman how to > > > > > build the image (what goes in there and where). Then outputs an > > > > > updated DT which describes where everything ended up, for use by other > > > > > tools, e.g. firmware update. It is a closed loop in that sense. See > > > > > the references for more information. > > > > > > > > Maybe I fail to see why you would want these description to be > > > > introduced here, if they are not useful to the OS. > > > > > > Well I was asked to send them to Linux since they apparently don't > > > belong in dt-schema. That is not what I said. I said fixed-partitions should be extended. I prefer they are extended in-place before moving them rather than the other way around. > > > These are firmware bindings, as indicated, but I > > > took them out of the /firmware node since that is for a different > > > purpose. Rob suggested that partitions was a good place. We have fwupd > > > using DT to hold the firmware-update information, so I expect it will > > > move to use these bindings too. > > > > I would definitely use fixed partitions as that's what you need then: > > registering where everything starts and ends. If you have "in-band" > > meta data you might require a compatible, but I don't think you > > do, in this case you should probably carry the content through a label > > (which will become the partition name) and we can discuss additional > > properties if needed. > > I believe I am going to need a compatible string at the 'partitions' > level to indicate that this is the binman scheme. But we can leave > that until later. Perhaps: compatible = "binman", "fixed-partitions"; Though I don't understand why binman couldn't just understand what "fixed-partitions" means rather than "binman". > So you are suggesting 'label' for the contents. Rob suggested > 'compatible' [1], so what should I do? "label" is for consumption by humans, not tools/software. Compatible values are documented, label values are not. Though the partition stuff started out using label long ago and it's evolved to preferring compatible. > With this schema, would every node be called 'partition@...' or is > there flexibility to use other names? The preference is to use generic names. Do you mean without a unit-address or different from "partition"? The need for the input side of binman to have dynamic addresses seems like the biggest issue. That's allowed in other cases with "partition-N" or "partition-foo" IIRC. I don't think we want to allow that for "fixed-partitions" at least in the DTB (i.e. the output side of binman). Rob
