Re: [PATCH v10 2/2] dt-bindings: mtd: fixed-partition: Add binman compatibles
On Tue, 26 Mar 2024 14:06:45 -0600, Simon Glass wrote: > Add two compatibles for binman entries, as a starting point for the > schema. > > Note that, after discussion on v2, we decided to keep the existing > meaning of label so as not to require changes to existing userspace > software when moving to use binman nodes to specify the firmware > layout. > > Note also that, after discussion on v6, we decided to use the same > 'fixed-partition' schema for the binman features, so this version > adds a new 'binman.yaml' file providing the new compatibles to the > existing partition.yaml binding. > > Signed-off-by: Simon Glass > --- > > Changes in v10: > - Drop binman,entry since it is likely not necessary > - Put the description back > > Changes in v8: > - Switch the patch ordering so the partition change comes first > > Changes in v7: > - Adjust MAINTAINERS entry > - Put compatible strings into the 'fixed-partition' binding > > Changes in v5: > - Add mention of why 'binman' is the vendor > - Drop 'select: false' > - Tidy up the compatible setings > - Use 'tfa-bl31' instead of 'atf-bl31' > > Changes in v4: > - Correct selection of multiple compatible strings > > Changes in v3: > - Drop fixed-partitions from the example > - Use compatible instead of label > > Changes in v2: > - Use plain partition@xxx for the node name > > .../bindings/mtd/partitions/binman.yaml | 53 +++ > .../bindings/mtd/partitions/partition.yaml| 21 > MAINTAINERS | 5 ++ > 3 files changed, 79 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman.yaml > Reviewed-by: Rob Herring
Re: [PATCH v9 2/2] arm64: boot: Support Flat Image Tree
On Wed, Jan 31, 2024 at 8:09 PM Masahiro Yamada wrote: > > On Thu, Feb 1, 2024 at 7:03 AM Rob Herring wrote: > > > > On Tue, Jan 30, 2024 at 3:16 AM Masahiro Yamada > > wrote: > > > > > > On Fri, Jan 26, 2024 at 1:04 AM Simon Glass wrote: > > > > > > > > Hi, > > > > > > > > On Wed, 17 Jan 2024 at 06:14, Simon Glass wrote: > > > > > > > > > > Hi Masahiro, Tom, > > > > > > > > > > On Tue, 9 Jan 2024 at 07:33, Tom Rini wrote: > > > > > > > > > > > > On Tue, Jan 09, 2024 at 11:01:42PM +0900, Masahiro Yamada wrote: > > > > > > > Hi Simon, > > > > > > > > > > > > > > > > > > > > > On Wed, Jan 3, 2024 at 8:47 AM Simon Glass > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Masahiro, > > > > > > > > > > > > > > > > On Wed, Dec 13, 2023 at 5:14 AM Will Deacon > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Fri, Dec 01, 2023 at 08:54:42PM -0700, Simon Glass wrote: > > > > > > > > > > Add a script which produces a Flat Image Tree (FIT), a > > > > > > > > > > single file > > > > > > > > > > containing the built kernel and associated devicetree files. > > > > > > > > > > Compression defaults to gzip which gives a good balance of > > > > > > > > > > size and > > > > > > > > > > performance. > > > > > > > > > > > > > > > > > > > > The files compress from about 86MB to 24MB using this > > > > > > > > > > approach. > > > > > > > > > > > > > > > > > > > > The FIT can be used by bootloaders which support it, such > > > > > > > > > > as U-Boot > > > > > > > > > > and Linuxboot. It permits automatic selection of the correct > > > > > > > > > > devicetree, matching the compatible string of the running > > > > > > > > > > board with > > > > > > > > > > the closest compatible string in the FIT. There is no need > > > > > > > > > > for > > > > > > > > > > filenames or other workarounds. > > > > > > > > > > > > > > > > > > > > Add a 'make image.fit' build target for arm64, as well. Use > > > > > > > > > > FIT_COMPRESSION to select a different algorithm. > > > > > > > > > > > > > > > > > > > > The FIT can be examined using 'dumpimage -l'. > > > > > > > > > > > > > > > > > > > > This features requires pylibfdt (use 'pip install libfdt'). > > > > > > > > > > It also > > > > > > > > > > requires compression utilities for the algorithm being > > > > > > > > > > used. Supported > > > > > > > > > > compression options are the same as the Image.xxx files. > > > > > > > > > > For now there > > > > > > > > > > is no way to change the compression other than by editing > > > > > > > > > > the rule for > > > > > > > > > > $(obj)/image.fit > > > > > > > > > > > > > > > > > > > > While FIT supports a ramdisk / initrd, no attempt is made > > > > > > > > > > to support > > > > > > > > > > this here, since it must be built separately from the Linux > > > > > > > > > > build. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > Changes in v9: > > > > > > > > > > - Move the compression control into Makefile.lib > > > > > > > > > > > > > > > > > > > > Changes in v8: > > > > > > > > &g
Re: [PATCH v9 2/2] arm64: boot: Support Flat Image Tree
On Tue, Jan 30, 2024 at 3:16 AM Masahiro Yamada wrote: > > On Fri, Jan 26, 2024 at 1:04 AM Simon Glass wrote: > > > > Hi, > > > > On Wed, 17 Jan 2024 at 06:14, Simon Glass wrote: > > > > > > Hi Masahiro, Tom, > > > > > > On Tue, 9 Jan 2024 at 07:33, Tom Rini wrote: > > > > > > > > On Tue, Jan 09, 2024 at 11:01:42PM +0900, Masahiro Yamada wrote: > > > > > Hi Simon, > > > > > > > > > > > > > > > On Wed, Jan 3, 2024 at 8:47 AM Simon Glass wrote: > > > > > > > > > > > > Hi Masahiro, > > > > > > > > > > > > On Wed, Dec 13, 2023 at 5:14 AM Will Deacon wrote: > > > > > > > > > > > > > > On Fri, Dec 01, 2023 at 08:54:42PM -0700, Simon Glass wrote: > > > > > > > > Add a script which produces a Flat Image Tree (FIT), a single > > > > > > > > file > > > > > > > > containing the built kernel and associated devicetree files. > > > > > > > > Compression defaults to gzip which gives a good balance of size > > > > > > > > and > > > > > > > > performance. > > > > > > > > > > > > > > > > The files compress from about 86MB to 24MB using this approach. > > > > > > > > > > > > > > > > The FIT can be used by bootloaders which support it, such as > > > > > > > > U-Boot > > > > > > > > and Linuxboot. It permits automatic selection of the correct > > > > > > > > devicetree, matching the compatible string of the running board > > > > > > > > with > > > > > > > > the closest compatible string in the FIT. There is no need for > > > > > > > > filenames or other workarounds. > > > > > > > > > > > > > > > > Add a 'make image.fit' build target for arm64, as well. Use > > > > > > > > FIT_COMPRESSION to select a different algorithm. > > > > > > > > > > > > > > > > The FIT can be examined using 'dumpimage -l'. > > > > > > > > > > > > > > > > This features requires pylibfdt (use 'pip install libfdt'). It > > > > > > > > also > > > > > > > > requires compression utilities for the algorithm being used. > > > > > > > > Supported > > > > > > > > compression options are the same as the Image.xxx files. For > > > > > > > > now there > > > > > > > > is no way to change the compression other than by editing the > > > > > > > > rule for > > > > > > > > $(obj)/image.fit > > > > > > > > > > > > > > > > While FIT supports a ramdisk / initrd, no attempt is made to > > > > > > > > support > > > > > > > > this here, since it must be built separately from the Linux > > > > > > > > build. > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > --- > > > > > > > > > > > > > > > > Changes in v9: > > > > > > > > - Move the compression control into Makefile.lib > > > > > > > > > > > > > > > > Changes in v8: > > > > > > > > - Drop compatible string in FDT node > > > > > > > > - Correct sorting of MAINTAINERS to before ARM64 PORT > > > > > > > > - Turn compress part of the make_fit.py comment in to a sentence > > > > > > > > - Add two blank lines before parse_args() and setup_fit() > > > > > > > > - Use 'image.fit: dtbs' instead of BUILD_DTBS var > > > > > > > > - Use '$( > > > > > > > - Add 'mkimage' details Documentation/process/changes.rst > > > > > > > > - Allow changing the compression used > > > > > > > > - Tweak cover letter since there is only one clean-up patch > > > > > > > > > > > > > > > > Changes in v7: > > > > > > > > - Add Image as a dependency of image.fit > > > > > > > > - Drop kbuild tag > > > > > > > > - Add dependency on dtbs > > > > > > > > - Drop unnecessary path separator for dtbs > > > > > > > > - Rebase to -next > > > > > > > > > > > > > > > > Changes in v5: > > > > > > > > - Drop patch previously applied > > > > > > > > - Correct compression rule which was broken in v4 > > > > > > > > > > > > > > > > Changes in v4: > > > > > > > > - Use single quotes for UIMAGE_NAME > > > > > > > > > > > > > > > > Changes in v3: > > > > > > > > - Drop temporary file image.itk > > > > > > > > - Drop patch 'Use double quotes for image name' > > > > > > > > - Drop double quotes in use of UIMAGE_NAME > > > > > > > > - Drop unnecessary CONFIG_EFI_ZBOOT condition for help > > > > > > > > - Avoid hard-coding "arm64" for the DT architecture > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > - Drop patch previously applied > > > > > > > > - Add .gitignore file > > > > > > > > - Move fit rule to Makefile.lib using an intermediate file > > > > > > > > - Drop dependency on CONFIG_EFI_ZBOOT > > > > > > > > - Pick up .dtb files separately from the kernel > > > > > > > > - Correct pylint too-many-args warning for write_kernel() > > > > > > > > - Include the kernel image in the file count > > > > > > > > - Add a pointer to the FIT spec and mention of its wide > > > > > > > > industry usage > > > > > > > > - Mention the kernel version in the FIT description > > > > > > > > > > > > > > > > Documentation/process/changes.rst | 9 + > > > > > > > > MAINTAINERS | 7 + > > > > > > > > arch/arm64/Makefile | 7 +- > > > > > > > > arch/arm64/boot/.gitignore| 1 + > > > > > > > >
Re: [PATCH v4 00/11] An effort to bring DT bindings compliance within U-Boot
On Mon, Jan 22, 2024 at 6:59 PM Andre Przywara wrote: > > On Mon, 22 Jan 2024 11:49:59 -0500 > Tom Rini wrote: > > Hi Tom, > > > On Mon, Jan 22, 2024 at 11:45:15AM +, Andre Przywara wrote: > > > On Wed, 10 Jan 2024 16:05:36 +0530 > > > Sumit Garg wrote: > > > > > > Hi, > > > > > > I certainly welcome this more automatic synchronisation of the DTs, > > > however have one comment about the upcoming sync process: > > > > > > > ... > > > > However, Linux kernel DT maintainers proposed [2] for U-Boot to rather > > > > use devicetree-rebasing repo [3] which is a forked copy from Linux > > > > kernel for DT source files as well as bindings. It is tagged at every > > > > Linux kernel major release or intermideate release candidates. So here I > > > > have tried to reuse that to bring DT bingings compliance as well as a > > > > standard way to maintain a regular sync of DT source files with Linux > > > > kernel. > > > > > > > > In order to maintain devicetree files sync, U-Boot will maintains a Git > > > > subtree for devicetee-rebasing repo as `dts/upstream` sub-directory. > > > > U-Boot will regularly sync `dts/upstream/` subtree whenever the next > > > > window > > > > opens with the next available kernel major release. > > > > > > I hope this doesn't need to stay that restricted? Can we either sync more > > > often, or at least on the kernel's -rc1, and not only on a "full" release? > > > > > > The reason I ask is that we have a chicken-egg problem here: without a DT > > > merged into the kernel tree, no U-Boot board support can be merged. And > > > without U-Boot support, we cannot boot a kernel, at least not in the > > > canonical way. > > > > > > Since the U-Boot and kernel merge windows are not in phase, for sunxi I > > > try > > > to sync the kernel DTs either as early as possible (-rc1, sometimes even > > > before, when the maintainers have merged them into their tree), or > > > sometimes "out of season", if a board defconfig patch is coming up. > > > > > > Otherwise new board support, which typically has a very low regression > > > risk > > > for the rest of the code base, would need to be delayed until the next > > > release. In the worst case the U-Boot merge windows opens one week before > > > a kernel release, then new boards need to wait three months? > > > > Would it be to bad to bring in board X with OF_UPSTREAM=n for one U-Boot > > release, and then with the next one switch to OF_UPSTREAM=y (and delete > > the dts from arch/) for the next release, when we would have gotten back > > in sync? > > Ah, I didn't look into the actual patches, but if this provision is > there, that sounds surely acceptable. It might still be good to sync > earlier than the .0 kernel release: if it appears in Linus' tree, it > had already seen a good share of review and testing. And with the > U-Boot releases being always further away than the next kernel release, > we could pull fixes still in time. So we could pick the latest -rc (or > .0 release, whichever is more recent) when the U-Boot merge window > opens? That should be mostly fine IMO, but there are the occasional "oops, let's change/fix the binding before it's released". Couldn't you pull in the latest rc in the merge window, but only if the .0 release will happen before the next u-boot release. And then update from rc to .0 before the u-boot release. Also, from a quick look at the dts changes during rc releases, they tend to come in the later rc releases. Probably that's just the latency of going to sub-arch maintainer->SoC->Linus. Rob
Re: [PATCH v6 1/3] dt-bindings: mtd: partitions: Add binman compatible
On Thu, Jan 4, 2024 at 3:54 PM Simon Glass wrote: > > Hi Rob, > > On Thu, Dec 14, 2023 at 2:09 PM Simon Glass wrote: > > > > Hi Rob, > > > > On Thu, 14 Dec 2023 at 10:27, Rob Herring wrote: > > > > > > On Fri, Dec 08, 2023 at 03:58:10PM -0700, Simon Glass wrote: > > > > Hi Rob, > > > > > > > > On Fri, 8 Dec 2023 at 14:56, Rob Herring wrote: > > > > > > > > > > On Fri, Dec 8, 2023 at 11:47 AM Simon Glass wrote: > > > > > > > > > > > > Hi Rob, > > > > > > > > > > > > On Fri, 8 Dec 2023 at 08:00, Rob Herring wrote: > > > > > > > > > > > > > > On Thu, Nov 16, 2023 at 10:28:50AM -0700, Simon Glass wrote: > > > > > > > > Add a compatible string for binman, so we can extend > > > > > > > > fixed-partitions > > > > > > > > in various ways. > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > --- > > > > > > > > > > > > > > > > (no changes since v5) > > > > > > > > > > > > > > > > Changes in v5: > > > > > > > > - Add #address/size-cells and parternProperties > > > > > > > > - Drop $ref to fixed-partitions.yaml > > > > > > > > - Drop 'select: false' > > > > > > > > > > > > > > > > Changes in v4: > > > > > > > > - Change subject line > > > > > > > > > > > > > > > > Changes in v3: > > > > > > > > - Drop fixed-partition additional compatible string > > > > > > > > - Drop fixed-partitions from the example > > > > > > > > - Mention use of compatible instead of label > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > - Drop mention of 'enhanced features' in fixed-partitions.yaml > > > > > > > > - Mention Binman input and output properties > > > > > > > > - Use plain partition@xxx for the node name > > > > > > > > > > > > > > > > .../bindings/mtd/partitions/binman.yaml | 68 > > > > > > > > +++ > > > > > > > > .../bindings/mtd/partitions/partitions.yaml | 1 + > > > > > > > > MAINTAINERS | 5 ++ > > > > > > > > 3 files changed, 74 insertions(+) > > > > > > > > create mode 100644 > > > > > > > > Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > > > > > > > > > > > > > > diff --git > > > > > > > > a/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > > > > > > b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > > > > > > new file mode 100644 > > > > > > > > index ..329217550a98 > > > > > > > > --- /dev/null > > > > > > > > +++ > > > > > > > > b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > > > > > > @@ -0,0 +1,68 @@ > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > > > > > > +# Copyright 2023 Google LLC > > > > > > > > + > > > > > > > > +%YAML 1.2 > > > > > > > > +--- > > > > > > > > +$id: http://devicetree.org/schemas/mtd/partitions/binman.yaml# > > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > > + > > > > > > > > +title: Binman firmware layout > > > > > > > > + > > > > > > > > +maintainers: > > > > > > > > + - Simon Glass > > > > > > > > + > > > > > > > > +description: | > > > > > > > > + The binman node provides a layout for firmware, used when > > > > > > > > packaging firmware > > > > > > > > + from multiple projects. It is based on fixed-partitions, > > > > > > > >
Re: [PATCH V3 1/6] dt-bindings: nvmem: layouts: add U-Boot environment variables layout
On Thu, Jan 04, 2024 at 10:10:13AM +0100, Rafał Miłecki wrote:
> On 4.01.2024 08:58, Miquel Raynal wrote:
> > r...@kernel.org wrote on Wed, 3 Jan 2024 17:11:29 -0700:
> > > On Thu, Dec 21, 2023 at 06:34:16PM +0100, Rafał Miłecki wrote:
> > > > From: Rafał Miłecki
> > > >
> > > > U-Boot env data is a way of storing firmware variables. It's a format
> > > > that can be used of top of various storage devices. Its binding should
> > > > be an NVMEM layout instead of a standalone device.
> > > >
> > > > This patch adds layout binding which allows using it on top of MTD NVMEM
> > > > device as well as any other. At the same time it deprecates the old
> > > > combined binding.
> > >
> > > I don't understand the issue. From a DT perspective, there isn't. A
> > > partition is not a device, but is describing the layout of storage
> > > already.
> >
> > Actually I think what Rafał wants to do goes in the right direction but
> > I also understand from a binding perspective it may be a little
> > confusing, even more if we consider "NVMEM" a Linux specific concept.
> >
> > There is today a "u-boot env" NVMEM *device* description which
> > almost sits at the same level as eg. an eeprom device. We cannot
> > compare "an eeprom device" and "a u-boot environment" of course. But
> > that's truly what is currently described.
> >
> > * Current situation
> >
> > Flash device -> U-Boot env layout -> NVMEM cells
Isn't it?:
Flash device -> fixed-partitions -> U-Boot env layout -> NVMEM cells
> >
> > * Improved situation
> >
> > Any storage device -> NVMEM -> U-Boot env layout -> NVMEM cells
Why is this better? We don't need a container to say 'this is NVMEM
stuff' or 'this is MTD stuff'. 'U-Boot env layout' can tell us 'this is
NVMEM stuff' or whatever the kernel decides in the future.
> >
> > The latter is of course the most relevant description as we expect
> > storage devices to expose a storage-agnostic interface (NVMEM in
> > this case) which can then be parsed (by NVMEM layouts) in a storage
> > agnostic way.
> >
> > In the current case, the current U-Boot env binding tells people to
> > declare the env layout on top of a flash device (only). The current
> > description also expects a partition node which is typical to flash
> > devices. Whereas what we should have described in the first place is a
> > layout that applies on any kind of NVMEM device.
> >
> > Bonus point: We've been working the last couple years on clarifying
> > bindings, especially with mtd partitions (with the partitions{}
> > container) and NVMEM layouts (with the nvmem-layout{} container).
> > The switch proposed in this patch makes use of the latter, of course.
>
> Thanks Miquèl for filling bits I missed in commit description. Despite
> years in Linux/DT I still struggle with more complex designs
> documentation.
>
>
> As per Rob's comment I think I see his point and a possible design
> confusion. If you look from a pure DT perspective then "partitions" and
> "nvmem-layout" serve a very similar purpose. They describe device's data
> content structure. For fixed structures we have very similar
> "fixed-partitions" and "fixed-cells".
>
> If we were to design those bindings today I'm wondering if we couldn't
> have s/partitions/layout/ and s/nvmem-layout/layout/.
Why!? It is just a name, and we can't get rid of the old names. We don't
need 2 names.
> Rob: other than having different bindings for MTD vs. NVMEM layouts I
> think they overall design makes sense. A single device may have content
> structurized on more than 1 level:
> 1. You may have fixed layout at top level (multiple partitions)
> 2. Single partitions may have their own layouts (like U-Boot env data)
Sure. Partitions is for 1 and Layouts is for 2.
> Maybe ideally above should look more like:
>
> flash@0 {
> compatible = "";
>
> layout {
> compatible = "fixed-layout";
Why does 'partitions' and 'fixed-partitions' not work here?
> #address-cells = <1>;
> #size-cells = <1>;
>
> partition@0 {
> reg = <0x0 0x4>;
> label = "u-boot";
> };
>
> partition@4 {
> reg = <0x4 0x1>;
> label = "u-boot-env";
>
> layout {
> compatible = "u-boot,env-layout";
> };
> };
>
> partition@5 {
> reg = <0x5 0x10>;
> label = "u-boot";
> };
> };
> };
>
> but I can clearly see a use for nested "layout"s. As I said maybe we
> just shouldn't be so open in calling those MTD or NVMEM devices as that
> is kind of Linux specific.
The overall structure should be agnostic to the subsystem. Specific
compatibles like 'u-boot,env' can be tied to a subsystem.
Maybe some things need to be both MTD and NVMEM. MTD to
Re: [PATCH V3 1/6] dt-bindings: nvmem: layouts: add U-Boot environment variables layout
On Thu, Dec 21, 2023 at 06:34:16PM +0100, Rafał Miłecki wrote: > From: Rafał Miłecki > > U-Boot env data is a way of storing firmware variables. It's a format > that can be used of top of various storage devices. Its binding should > be an NVMEM layout instead of a standalone device. > > This patch adds layout binding which allows using it on top of MTD NVMEM > device as well as any other. At the same time it deprecates the old > combined binding. I don't understand the issue. From a DT perspective, there isn't. A partition is not a device, but is describing the layout of storage already. Rob
Re: [PATCH v3 5/8] doc: devicetree: Updates for devicetree-rebasing subtree
On Thu, Dec 28, 2023 at 4:59 AM Sumit Garg wrote: > > Encourage SoC/board maintainers to migrate to using devicetree-rebasing > subtree and maintain a regular sync with Linux kernel devicetree files > and bindings. > > Along with that add documentation regarding how to run DT bindings > schema checks. > > Signed-off-by: Sumit Garg > --- > > Changes in v3: > -- > - Replace CONFIG_* with Kconfig options > > Changes in v2: > -- > - s/U-boot/U-Boot/ > > doc/develop/devicetree/control.rst | 131 + > 1 file changed, 96 insertions(+), 35 deletions(-) > > diff --git a/doc/develop/devicetree/control.rst > b/doc/develop/devicetree/control.rst > index cbb65c9b177..65180d33e8d 100644 > --- a/doc/develop/devicetree/control.rst > +++ b/doc/develop/devicetree/control.rst > @@ -1,5 +1,6 @@ > .. SPDX-License-Identifier: GPL-2.0+ > .. sectionauthor:: Copyright 2011 The Chromium OS Authors > +.. Copyright 2023 Linaro Ltd. > > Devicetree Control in U-Boot > > @@ -22,14 +23,13 @@ for three reasons: >hierarchical format > - It is fairly efficient to read incrementally > > -The arch//dts directories contains a Makefile for building the > devicetree > -blob and embedding it in the U-Boot image. This is useful since it allows > -U-Boot to configure itself according to what it finds there. If you have > -a number of similar boards with different peripherals, you can describe > -the features of each board in the devicetree file, and have a single > -generic source base. > +The U-Boot Makefile infrastructure allows for building the devicetree blob > +and embedding it in the U-Boot image. This is useful since it allows U-Boot > +to configure itself according to what it finds there. If you have a number > +of similar boards with different peripherals, you can describe the features > +of each board in the devicetree file, and have a single generic source base. > > -To enable this feature, add CONFIG_OF_CONTROL to your board config file. > +To enable this feature, select `OF_CONTROL` via Kconfig. > > > What is a Flattened Devicetree? > @@ -68,8 +68,21 @@ a binary file. U-Boot adds its own `fdtgrep` for creating > subsets of the file. > Where do I get a devicetree file for my board? > -- > > -You may find that the Linux kernel has a suitable file. Look in the > -kernel source in arch//boot/dts. > +Linux kernel Git repository has been the place where devicetree files along > +with devicetree bindings are stored and maintained. There is > devicetee-rebasing > +(dtrepo_) which maintains a forked copy of devicetree files along with > bindings > +at every Linux kernel major release or intermideate release candidates. Typo. > + > +In order to maintain devicetree files sync, U-Boot maintains a Git subtree > for > +devicetee-rebasing repo as `devicetee-rebasing/` sub-directory. It is > regularly > +kept updated with every new kernel major release via subtree pull as > follows:: I would suggest dropping "-rebasing" in the u-boot tree. (I wish we had in the original repo). I don't think it's relevant. We're not likely to regenerate the tree, but any clue what 'git subtree pull' would do in this case? It could happen if we switched to git-filter-repo. > + > +git subtree pull --prefix devicetree-rebasing \ > + > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git > \ > + --squash I'd put this in a script to run. Documentation tends to be not quite correct. A script could also be smarter and figure out . > + > +You may find that the `devicetee-rebasing/` sub-directory has a suitable > +devicetree file for your board. Look in `devicetree-rebasing/src//`.
Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages
On Fri, Dec 22, 2023 at 5:48 AM Ard Biesheuvel wrote: > > On Thu, 21 Dec 2023 at 17:50, Chiu, Chasel wrote: > > > > > > Hi Ard, > > > > Please see my reply below inline and let me know your thoughts. > > > > Thanks, > > Chasel > > > > > > > -Original Message- > > > From: Ard Biesheuvel > > > Sent: Thursday, December 21, 2023 6:31 AM > > > To: Chiu, Chasel > > > Cc: Simon Glass ; devicet...@vger.kernel.org; Mark > > > Rutland > > > ; Rob Herring ; Tan, Lean Sheng > > > ; lkml ; Dhaval > > > Sharma ; Brune, Maximilian > > > ; Yunhui Cui ; > > > Dong, Guo ; Tom Rini ; ron minnich > > > ; Guo, Gua ; linux- > > > a...@vger.kernel.org; U-Boot Mailing List > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory > > > usages > > > > > > On Tue, 28 Nov 2023 at 21:31, Chiu, Chasel wrote: > > > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > From: Ard Biesheuvel > > > > > Sent: Tuesday, November 28, 2023 10:08 AM > > > > > To: Chiu, Chasel > > > > > Cc: Simon Glass ; devicet...@vger.kernel.org; Mark > > > > > Rutland ; Rob Herring ; Tan, > > > > > Lean Sheng ; lkml > > > > > ; Dhaval Sharma ; > > > > > Brune, Maximilian ; Yunhui Cui > > > > > ; Dong, Guo ; Tom Rini > > > > > ; ron minnich ; Guo, Gua > > > > > ; linux- a...@vger.kernel.org; U-Boot Mailing > > > > > List > > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory > > > > > usages > > > > > > > > > > You are referring to a 2000 line patch so it is not 100% clear where > > > > > to look tbh. > > > > > > > > > > > > > > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel > > > > > wrote: > > > > > > > > > > > > > > > > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line > > > > > > 268 is for > > > > > related example code. > > > > > > > > > > > > > > > > That refers to a 'memory-allocation' node, right? How does that > > > > > relate to the 'reserved-memory' node? > > > > > > > > > > And crucially, how does this clarify in which way "runtime-code" and > > > > > "runtime- data" reservations are being used? > > > > > > > > > > Since the very beginning of this discussion, I have been asking > > > > > repeatedly for examples that describe the wider context in which these > > > reservations are used. > > > > > The "runtime" into runtime-code and runtime-data means that these > > > > > regions have a special significance to the operating system, not > > > > > just to the next bootloader stage. So I want to understand exactly > > > > > why it is necessary to describe these regions in a way where the > > > > > operating system might be expected to interpret this information and > > > > > act > > > upon it. > > > > > > > > > > > > > > > > > I think runtime code and data today are mainly for supporting UEFI > > > > runtime > > > services - some BIOS functions for OS to utilize, OS may follow below > > > ACPI spec to > > > treat them as reserved range: > > > > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html# > > > > uefi-memory-types-and-mapping-to-acpi-address-range-types > > > > > > > > Like I mentioned earlier, that PR is still in early phase and has not > > > > reflected all > > > the required changes yet, but the idea is to build > > > gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes. > > > > UEFI generic Payload has DxeMain integrated, however Memory Types are > > > platform-specific, for example, some platforms may need bigger runtime > > > memory > > > for their implementation, that's why we want such FDT reserved-memory > > > node to > > > tell DxeMain. > > > > > > > > > > > The Payload flow will be like this: > > > > Payload creates built-in default MemoryTypes table -> > > > >
Re: [PATCH 1/4] dt-bindings: nvmem: layouts: add U-Boot environment variables layout
On Mon, 18 Dec 2023 14:37:19 +0100, Rafał Miłecki wrote: > From: Rafał Miłecki > > U-Boot env data is a way of storing firmware variables. It's a format > that can be used of top of various storage devices. Its binding should > be an NVMEM layout instead of a standalone device. > > This patch adds layout binding which allows using it on top of MTD NVMEM > device as well as any other. At the same time it deprecates the old > combined binding. > > Signed-off-by: Rafał Miłecki > --- > .../bindings/nvmem/layouts/u-boot,env.yaml| 55 +++ > .../devicetree/bindings/nvmem/u-boot,env.yaml | 6 ++ > 2 files changed, 61 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/nvmem/u-boot,env.example.dtb: partition@4: 'ethaddr', 'reg' do not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/nvmem/layouts/u-boot,env.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/nvmem/u-boot,env.example.dtb: partition-u-boot-env: 'ethaddr' does not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/nvmem/layouts/u-boot,env.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231218133722.16150-1-zaj...@gmail.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH 0/8] An effort to bring DT bindings compliance within U-boot
On Thu, Dec 14, 2023 at 12:23 PM Tom Rini wrote: > > On Thu, Dec 14, 2023 at 03:53:11PM +0100, neil.armstr...@linaro.org wrote: > > Hi, > > > > On 14/12/2023 14:50, Sumit Garg wrote: > > > Prerquisite > > > > s/Prerquisite/Prerequisite/ > > > > > --- > > > > > > This patch series requires devicetree-rebasing git repo to be added as a > > > subtree to the main U-boot repo via: > > > > > > $ git subtree add --prefix devicetree-rebasing \ > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git > > > \ > > >v6.6-dts --squash > > > > So I think the big question is: when should the subtree be updated ? > > > > Because as we discussed in the previous GH pull request, if a bindings > > changes > > was made in the upstream Linux DT, then the subtree update should wait until > > the u-boot support is merged before updating. This could cause a lot of > > frustration. > > > > And this could cause a lot of regressions, even more if both Linux and > > U-boot are > > not maintained by the same people. > > I think some of the important questions to ask are, how often / likely > are the breakages to occur? It seems like these days it's either: > - U-Boot had an early version of the binding and we already state we > don't support backwards compatibility here. It should be on the > maintainer to be proactive in this case. > - It's a "the DT was wrong about the hardware, sorry not sorry it's an > incompatible DTS change now". This too is hopefully the kind of thing > that at least board maintainers will be more actively aware of needing > to deal with in U-Boot, if it's really a problem. A common issue in the kernel is with forward compatibility when platforms add new resources from a new provider. Then the kernel expects a driver for the provider and waits for the dependency. Of course, older kernels don't have that provider driver and so the dependency is never met. Not sure if u-boot will have similar issues? At least you should/could know if the provider driver exists or not. (The kernel doesn't because modules.) Rob
Re: [PATCH v6 1/3] dt-bindings: mtd: partitions: Add binman compatible
On Fri, Dec 08, 2023 at 03:58:10PM -0700, Simon Glass wrote: > Hi Rob, > > On Fri, 8 Dec 2023 at 14:56, Rob Herring wrote: > > > > On Fri, Dec 8, 2023 at 11:47 AM Simon Glass wrote: > > > > > > Hi Rob, > > > > > > On Fri, 8 Dec 2023 at 08:00, Rob Herring wrote: > > > > > > > > On Thu, Nov 16, 2023 at 10:28:50AM -0700, Simon Glass wrote: > > > > > Add a compatible string for binman, so we can extend fixed-partitions > > > > > in various ways. > > > > > > > > > > Signed-off-by: Simon Glass > > > > > --- > > > > > > > > > > (no changes since v5) > > > > > > > > > > Changes in v5: > > > > > - Add #address/size-cells and parternProperties > > > > > - Drop $ref to fixed-partitions.yaml > > > > > - Drop 'select: false' > > > > > > > > > > Changes in v4: > > > > > - Change subject line > > > > > > > > > > Changes in v3: > > > > > - Drop fixed-partition additional compatible string > > > > > - Drop fixed-partitions from the example > > > > > - Mention use of compatible instead of label > > > > > > > > > > Changes in v2: > > > > > - Drop mention of 'enhanced features' in fixed-partitions.yaml > > > > > - Mention Binman input and output properties > > > > > - Use plain partition@xxx for the node name > > > > > > > > > > .../bindings/mtd/partitions/binman.yaml | 68 > > > > > +++ > > > > > .../bindings/mtd/partitions/partitions.yaml | 1 + > > > > > MAINTAINERS | 5 ++ > > > > > 3 files changed, 74 insertions(+) > > > > > create mode 100644 > > > > > Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > > > b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > > > new file mode 100644 > > > > > index ..329217550a98 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > > > @@ -0,0 +1,68 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > > > +# Copyright 2023 Google LLC > > > > > + > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: http://devicetree.org/schemas/mtd/partitions/binman.yaml# > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > + > > > > > +title: Binman firmware layout > > > > > + > > > > > +maintainers: > > > > > + - Simon Glass > > > > > + > > > > > +description: | > > > > > + The binman node provides a layout for firmware, used when > > > > > packaging firmware > > > > > + from multiple projects. It is based on fixed-partitions, with some > > > > > + extensions, but uses 'compatible' to indicate the contents of the > > > > > node, to > > > > > + avoid perturbing or confusing existing installations which use > > > > > 'label' for a > > > > > + particular purpose. > > > > > + > > > > > + Binman supports properties used as inputs to the > > > > > firmware-packaging process, > > > > > + such as those which control alignment of partitions. This binding > > > > > addresses > > > > > + these 'input' properties. For example, it is common for the 'reg' > > > > > property > > > > > + (an 'output' property) to be set by Binman, based on the alignment > > > > > requested > > > > > + in the input. > > > > > + > > > > > + Once processing is complete, input properties have mostly served > > > > > their > > > > > + purpose, at least until the firmware is repacked later, e.g. due > > > > > to a > > > > > + firmware update. The 'fixed-partitions' binding should provide > > > > > enough > > > > > + information to read the firmware at runtime, including > > > > > decompression if > > > > > + needed. > > > > > > > > How is this going to work exactly? binman reads these nodes and then > > > > writes out 'fixed-partitions' nodes. But then you've lost the binman > > > > specifc parts needed for repacking. > > > > > > No, they are the same node. I do want the extra information to stick > > > around. So long as it is compatible with fixed-partition as well, this > > > should work OK. > > > > How can it be both? The partitions node compatible can be either > > 'fixed-partitions' or 'binman'. > > Can we not allow it to be both? I have tried to adjust things in > response to feedback but perhaps the feedback was leading me down the > wrong path? Sure, but then the schema has to and that means extending fixed-partitions. Rob
Re: [PATCH 00/21] Qualcomm generic board support
On Mon, Dec 11, 2023 at 11:47 PM Sumit Garg wrote: > > Hi Tom, > > On Sun, 10 Dec 2023 at 03:33, Tom Rini wrote: > > > > On Mon, Dec 04, 2023 at 11:02:57AM +0530, Sumit Garg wrote: > > > > [snip] > > > But currently u-boot doesn't have a proper way to validate those DTS > > > against DT bindings (maintained in Linux kernel). Although there are > > > Devicetree schema tools available here [2], there isn't a versioned > > > release package of DT bindings which one should use to validate DTS > > > files. > > > > I will have more / other things to say but I want to chime in here. That > > U-Boot cannot validate the DTS files is a bug, not a feature. I would > > very much appreciate if someone(s) with time and skills to do so would > > re-sync us with the kernel Kbuild again so that we can both stay in sync > > again and have the validation targets / functionality available here > > too. > > > > Agree, the Kbuild changes to add dtbs_check was the first thing I > implemented after importing devicetree-rebasing repo in u-boot (see > commit [1] for details). Usage with PR [2] included: > > While building any u-boot target, just add make target: "dtbs_check" > and you will see the DT schema checks being performed. Steps: > > $ make _defconfig > $ make -j`nproc` dtbs_check > > Currently there are a lot of incompatibility warnings I have seen for > the platforms I built. This shows how much difficult it has been to > keep DT in sync with upstream DT bindings. The versions in the Linux tree generally still have lots of warnings too. It's a mountain of warnings. The warnings get amplified when there are N boards for 1 SoC. Some platforms are more active than others to get rid of them. Newer platforms like Apple are warning free (or nearly so). If you want an overview of the state of platforms, I have a CI job[1] doing just that. Look at platform-warnings.log in the artifacts. It does some deduplicating of the warnings. Linux-next and Linus' master are built daily. Fixing the warnings alone will not solve potential incompatibility issues. The schemas can and do change (and in turn the dts files). We try to catch that in review, but the rule is that the platform must be okay with the ABI change (recent mistake, early stages, limited users, etc.) and the commit message must spell out 'this is an ABI change'. That's all manual and not easily identifiable. I'm working on a tool[2] to compare versions of the schema to identify some ABI changes. Though it is limited by thinking of what schema changes are ABI changes and my ability to write python code to parse those cases. Right now it looks for new required properties, removed properties, and changed number of entries. I'm interested in any ideas for other checks. Rob [1] https://gitlab.com/robherring/linux-dt/-/jobs [2] https://github.com/robherring/dt-schema/tree/abi-check
Re: [PATCH v6 1/3] dt-bindings: mtd: partitions: Add binman compatible
On Fri, Dec 8, 2023 at 11:47 AM Simon Glass wrote: > > Hi Rob, > > On Fri, 8 Dec 2023 at 08:00, Rob Herring wrote: > > > > On Thu, Nov 16, 2023 at 10:28:50AM -0700, Simon Glass wrote: > > > Add a compatible string for binman, so we can extend fixed-partitions > > > in various ways. > > > > > > Signed-off-by: Simon Glass > > > --- > > > > > > (no changes since v5) > > > > > > Changes in v5: > > > - Add #address/size-cells and parternProperties > > > - Drop $ref to fixed-partitions.yaml > > > - Drop 'select: false' > > > > > > Changes in v4: > > > - Change subject line > > > > > > Changes in v3: > > > - Drop fixed-partition additional compatible string > > > - Drop fixed-partitions from the example > > > - Mention use of compatible instead of label > > > > > > Changes in v2: > > > - Drop mention of 'enhanced features' in fixed-partitions.yaml > > > - Mention Binman input and output properties > > > - Use plain partition@xxx for the node name > > > > > > .../bindings/mtd/partitions/binman.yaml | 68 +++ > > > .../bindings/mtd/partitions/partitions.yaml | 1 + > > > MAINTAINERS | 5 ++ > > > 3 files changed, 74 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > new file mode 100644 > > > index ..329217550a98 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > > @@ -0,0 +1,68 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +# Copyright 2023 Google LLC > > > + > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/mtd/partitions/binman.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Binman firmware layout > > > + > > > +maintainers: > > > + - Simon Glass > > > + > > > +description: | > > > + The binman node provides a layout for firmware, used when packaging > > > firmware > > > + from multiple projects. It is based on fixed-partitions, with some > > > + extensions, but uses 'compatible' to indicate the contents of the > > > node, to > > > + avoid perturbing or confusing existing installations which use 'label' > > > for a > > > + particular purpose. > > > + > > > + Binman supports properties used as inputs to the firmware-packaging > > > process, > > > + such as those which control alignment of partitions. This binding > > > addresses > > > + these 'input' properties. For example, it is common for the 'reg' > > > property > > > + (an 'output' property) to be set by Binman, based on the alignment > > > requested > > > + in the input. > > > + > > > + Once processing is complete, input properties have mostly served their > > > + purpose, at least until the firmware is repacked later, e.g. due to a > > > + firmware update. The 'fixed-partitions' binding should provide enough > > > + information to read the firmware at runtime, including decompression if > > > + needed. > > > > How is this going to work exactly? binman reads these nodes and then > > writes out 'fixed-partitions' nodes. But then you've lost the binman > > specifc parts needed for repacking. > > No, they are the same node. I do want the extra information to stick > around. So long as it is compatible with fixed-partition as well, this > should work OK. How can it be both? The partitions node compatible can be either 'fixed-partitions' or 'binman'. In the partition nodes, 'align' for example is allowed for a binman partition but not a fixed-partition. Note that the schema may not actually warn on extra properties ATM because there are some issues with the schema structure. Since there can be nested partittions, that complicates matters. It's been on my todo list to fix. Rob
Re: [PATCH v6 1/3] dt-bindings: mtd: partitions: Add binman compatible
On Thu, Nov 16, 2023 at 10:28:50AM -0700, Simon Glass wrote: > Add a compatible string for binman, so we can extend fixed-partitions > in various ways. > > Signed-off-by: Simon Glass > --- > > (no changes since v5) > > Changes in v5: > - Add #address/size-cells and parternProperties > - Drop $ref to fixed-partitions.yaml > - Drop 'select: false' > > Changes in v4: > - Change subject line > > Changes in v3: > - Drop fixed-partition additional compatible string > - Drop fixed-partitions from the example > - Mention use of compatible instead of label > > Changes in v2: > - Drop mention of 'enhanced features' in fixed-partitions.yaml > - Mention Binman input and output properties > - Use plain partition@xxx for the node name > > .../bindings/mtd/partitions/binman.yaml | 68 +++ > .../bindings/mtd/partitions/partitions.yaml | 1 + > MAINTAINERS | 5 ++ > 3 files changed, 74 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman.yaml > > diff --git a/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > new file mode 100644 > index ..329217550a98 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > @@ -0,0 +1,68 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2023 Google LLC > + > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mtd/partitions/binman.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Binman firmware layout > + > +maintainers: > + - Simon Glass > + > +description: | > + The binman node provides a layout for firmware, used when packaging > firmware > + from multiple projects. It is based on fixed-partitions, with some > + extensions, but uses 'compatible' to indicate the contents of the node, to > + avoid perturbing or confusing existing installations which use 'label' for > a > + particular purpose. > + > + Binman supports properties used as inputs to the firmware-packaging > process, > + such as those which control alignment of partitions. This binding addresses > + these 'input' properties. For example, it is common for the 'reg' property > + (an 'output' property) to be set by Binman, based on the alignment > requested > + in the input. > + > + Once processing is complete, input properties have mostly served their > + purpose, at least until the firmware is repacked later, e.g. due to a > + firmware update. The 'fixed-partitions' binding should provide enough > + information to read the firmware at runtime, including decompression if > + needed. How is this going to work exactly? binman reads these nodes and then writes out 'fixed-partitions' nodes. But then you've lost the binman specifc parts needed for repacking. Rob
Re: [PATCH 00/21] Qualcomm generic board support
On Thu, Dec 7, 2023 at 2:08 AM ff wrote: > > > > > Le 6 déc. 2023 à 21:42, Rob Herring a écrit : > > > > On Tue, Dec 5, 2023 at 11:05 PM Sumit Garg wrote: > >> > >>> On Tue, 5 Dec 2023 at 15:39, Krzysztof Kozlowski > >>> wrote: > >>> > >>> On 05/12/2023 10:45, Sumit Garg wrote: > >>>> + U-boot custodians list > >>>> > >>>> On Tue, 5 Dec 2023 at 12:58, Krzysztof Kozlowski > >>>> wrote: > >>>>> > >>>>> On 05/12/2023 08:13, Sumit Garg wrote: > >>>>>>>> @DT bindings maintainers, > >>>>>>>> > >>>>>>>> Given the ease of maintenance of DT bindings within Linux kernel > >>>>>>>> source tree, I don't have a specific objection there. But can we ease > >>>>>>>> DTS testing for firmware/bootloader projects by providing a versioned > >>>>>>>> release package for DT bindings? Or if someone else has a better idea > >>>>>>>> here please feel free to chime in. > >>>>>>> > >>>>>>> This doesn't work for you?: > >>>>>>> > >>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/ > >>>>>> > >>>>>> Thanks, this is certainly a good step which I wasn't aware of. Further > >>>>>> simplification can be done to decouple devicetree source files from DT > >>>>>> bindings. > >>>>> > >>>>> Why? > >>>> > >>>> I suppose you are already aware that Linux DTS files are a subset of > >>>> what could be supported by devicetree schemas. There can be > >>>> firmware/bootloader specific properties (one example being [1]) which > >>>> Linux kernel can simply ignore. Will you be willing to add all of > >>>> those DT properties to Linux DTS files and maintain them? > >>> > >>> We already added them and we already maintain them. DTS describes the > >>> hardware, not the OS-subset of the hardware. > >> > >> Let look at some numbers if your statement is justified or not for the > >> example I gave: > >> > >> u-boot$ git grep -nr bootph-* arch/arm* | wc -l > >> 4079 > >> > >> linux$ git grep -nr bootph-* arch/arm* | wc -l > >> 267 > > > > I have no control over whether anyone has submitted the other 3812 > > instances. > > > >> It looks like there is always going to be a catch up game regarding DT > >> properties which either Linux kernel or u-boot or any other > >> firmware/bootloader project don't care about. > > > > As long as dts files in u-boot are manually sync'ed, yes. That is the > > problem and it doesn't matter if we have a standalone repository or > > not. > > > > If you want to move in that direction, start automating what u-boot > > imports. You need to do that for bindings if you want to run > > validation, so why not dts files too? > > > >>>> However, DT bindings are something which should be common, the > >>>> hardware description of a device should be universal. IMO, splitting > >>> > >>> Both DT bindings and DTS should be common. I don't see the difference. > >> > >> If we really care about DTS to be common then the contribution model > >> has to change where there is a single repo hosting DT bindings and > >> DTS. All other projects whether it is Linux kernel or u-boot or any > >> other OS/firmware/bootloader are just consuming DTS files from that > >> single repo. > > > > Really, only the kernel and u-boot matter. No, I don't mean I don't > > care about other projects, but those are the 2 with the widest h/w > > support by far and which have a major effort to sync copies of dts > > files in both projects. The rest are just noise in terms of this > > problem. > > > >> I suppose this is something that Linux DT maintainers > >> have objected to in the past for ease of maintenance. I am not sure if > >> you folks are willing to change that stance. > > > > The issue is no one steps up to help maintain such a repository while > > there is lots of review and maintainer work on what goes into the > > kernel tree. I'm happy to direct my binding review attention to > > wherever the majority of the bindings go.
Re: [PATCH 00/21] Qualcomm generic board support
On Tue, Dec 5, 2023 at 11:05 PM Sumit Garg wrote: > > On Tue, 5 Dec 2023 at 15:39, Krzysztof Kozlowski > wrote: > > > > On 05/12/2023 10:45, Sumit Garg wrote: > > > + U-boot custodians list > > > > > > On Tue, 5 Dec 2023 at 12:58, Krzysztof Kozlowski > > > wrote: > > >> > > >> On 05/12/2023 08:13, Sumit Garg wrote: > > > @DT bindings maintainers, > > > > > > Given the ease of maintenance of DT bindings within Linux kernel > > > source tree, I don't have a specific objection there. But can we ease > > > DTS testing for firmware/bootloader projects by providing a versioned > > > release package for DT bindings? Or if someone else has a better idea > > > here please feel free to chime in. > > > > This doesn't work for you?: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/ > > >>> > > >>> Thanks, this is certainly a good step which I wasn't aware of. Further > > >>> simplification can be done to decouple devicetree source files from DT > > >>> bindings. > > >> > > >> Why? > > > > > > I suppose you are already aware that Linux DTS files are a subset of > > > what could be supported by devicetree schemas. There can be > > > firmware/bootloader specific properties (one example being [1]) which > > > Linux kernel can simply ignore. Will you be willing to add all of > > > those DT properties to Linux DTS files and maintain them? > > > > We already added them and we already maintain them. DTS describes the > > hardware, not the OS-subset of the hardware. > > Let look at some numbers if your statement is justified or not for the > example I gave: > > u-boot$ git grep -nr bootph-* arch/arm* | wc -l > 4079 > > linux$ git grep -nr bootph-* arch/arm* | wc -l > 267 I have no control over whether anyone has submitted the other 3812 instances. > It looks like there is always going to be a catch up game regarding DT > properties which either Linux kernel or u-boot or any other > firmware/bootloader project don't care about. As long as dts files in u-boot are manually sync'ed, yes. That is the problem and it doesn't matter if we have a standalone repository or not. If you want to move in that direction, start automating what u-boot imports. You need to do that for bindings if you want to run validation, so why not dts files too? > > > However, DT bindings are something which should be common, the > > > hardware description of a device should be universal. IMO, splitting > > > > Both DT bindings and DTS should be common. I don't see the difference. > > If we really care about DTS to be common then the contribution model > has to change where there is a single repo hosting DT bindings and > DTS. All other projects whether it is Linux kernel or u-boot or any > other OS/firmware/bootloader are just consuming DTS files from that > single repo. Really, only the kernel and u-boot matter. No, I don't mean I don't care about other projects, but those are the 2 with the widest h/w support by far and which have a major effort to sync copies of dts files in both projects. The rest are just noise in terms of this problem. > I suppose this is something that Linux DT maintainers > have objected to in the past for ease of maintenance. I am not sure if > you folks are willing to change that stance. The issue is no one steps up to help maintain such a repository while there is lots of review and maintainer work on what goes into the kernel tree. I'm happy to direct my binding review attention to wherever the majority of the bindings go. But the work on the DTS side is mostly SoC tree maintainers and sub-maintainers. Assume for a minute we have this standalone repo. What happens next? We start with an empty repo and then merge and move platforms 1 by 1? How many years will that take? What do we do with platforms no one is interested in moving? Or do we start with devicetree-rebasing instead (That was the plan at one time) and sync u-boot back to that? All the work needed to get u-boot and kernel dts files in sync has virtually no dependency on a standalone repo. If the dts files aren't already kept in sync, someone has to figure the differences and eliminate them. Maybe some platforms are in good shape, but it is still a manual process. Fix that part, because a standalone repo does nothing for you until you do. > > > DT bindings alone would ease the compliance process for u-boot drivers > > > in quite similar manner to Linux drivers. There's no compliance of drivers really beyond checking if compatible strings used by drivers have a schema. Why is extracting the bindings out a problem? SystemReady has no issue doing just that for its compliance test. Rob
Re: [PATCH 00/21] Qualcomm generic board support
On Sun, Dec 3, 2023 at 11:33 PM Sumit Garg wrote: > > + Linux kernel DT bindings maintainers, EBBR ML > > On Thu, 30 Nov 2023 at 20:05, Tom Rini wrote: > > > > On Thu, Nov 30, 2023 at 01:02:25PM +0530, Sumit Garg wrote: > > > On Wed, 29 Nov 2023 at 22:06, Neil Armstrong > > > wrote: > > > > > > > > On 29/11/2023 16:34, Caleb Connolly wrote: > > > > > > > > > > > > > > > On 23/11/2023 07:04, Sumit Garg wrote: > > > > >> On Wed, 22 Nov 2023 at 21:34, Caleb Connolly > > > > >> wrote: > > > > >>> > > > > >>> > > > > >>> > > > > >>> On 22/11/2023 14:27, Tom Rini wrote: > > > > On Wed, Nov 22, 2023 at 07:44:09PM +0530, Sumit Garg wrote: > > > > > On Wed, 22 Nov 2023 at 19:31, Tom Rini wrote: > > > > >> > > > > >> On Wed, Nov 22, 2023 at 11:51:29AM +0530, Sumit Garg wrote: > > > > >>> Hi Caleb, > > > > >>> > > > > >>> On Tue, 21 Nov 2023 at 22:39, Caleb Connolly > > > > >>> wrote: > > > > >> [snip] > > > > == DT loading == > > > > > > > > Previously, boards used the FDT blob embedded into U-Boot (via > > > > OF_SEPARATE). However, most Qualcomm boards run U-Boot as a > > > > secondary > > > > bootloader, so we can instead rely on the first-stage > > > > bootloader to > > > > populate some useful FDT properties for us (notably the > > > > /memory node and > > > > KASLR seed) and fetch the DTB that it provides. Combined with > > > > the memory > > > > map changes above, this let's us entirely avoid configuring > > > > the memory > > > > map explicitly. > > > > >>> > > > > >>> Since with this change, we don't need to embed FDT blob in the > > > > >>> u-boot > > > > >>> binary, so I was thinking if we really need to import DTs from > > > > >>> Linux > > > > >>> for different platforms and then play a catchup game? > > > > >>> > > > > >>> For now, yes. > > > > >> > > > > >> But why? Is there any value added by larger u-boot specific DT (most > > > > >> of the nodes being unused by u-boot) than what currently u-boot > > > > >> supports? The more important part is to get alignment with Linux DT > > > > >> bindings. If you need to have memory/reserved-memory nodes in u-boot > > > > >> DT for generalization purposes then you should import those > > > > >> particular > > > > >> nodes only. > > > > > > > > > > I've been thinking about and hacking on this for the last week or so, > > > > > sorry for the delayed reply here. > > > > > > > > > > The value is in preventing any of the existing bindings from > > > > > regressing, > > > > > > That is actually best addressed in Linux by checking the DTS against > > > yaml DT bindings. We don't have that testing available in u-boot and > > > only depend on careful reviews. > > > > I would absolutely love for someone to make another attempt at updating > > our kbuild infrastucture so that we can run the validation targets. > > > > Given that EBBR requires [1] the platform (firmware/bootloader) and > not OS to supply the devicetree, it becomes evident that > firmware/bootloaders import DTS from Linux kernel (where it is > maintained). > > But currently u-boot doesn't have a proper way to validate those DTS > against DT bindings (maintained in Linux kernel). Although there are > Devicetree schema tools available here [2], there isn't a versioned > release package of DT bindings which one should use to validate DTS > files. > > @DT bindings maintainers, > > Given the ease of maintenance of DT bindings within Linux kernel > source tree, I don't have a specific objection there. But can we ease > DTS testing for firmware/bootloader projects by providing a versioned > release package for DT bindings? Or if someone else has a better idea > here please feel free to chime in. This doesn't work for you?: https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/ Note that I would like to revamp this repo to use some modern CI job, use git_filter_repo python module rather than git-filter-branch, and move to devicetree.org GH, but if projects aren't relying on this repo, I'm not motivated to do that work. Rob
Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages
On Wed, Nov 8, 2023 at 5:38 AM Ard Biesheuvel wrote: > > On Tue, 7 Nov 2023 at 19:07, Rob Herring wrote: > > > > > > All of this: > > > > > > On Mon, 16 Oct 2023 at 15:54, Simon Glass wrote: > > > > > > > > It is not specific to EDK2. Imagine this boot sequence: > > > > > > > > - Platform Init (U-Boot) starts up > > > > - U-Boot uses its platform knowledge to sets some ACPI tables and put > > > > various things in memory > > > > - U-Boot sets up some runtime code and data for the OS > > > > - U-Boot jumps to the Tianocore payload ** > > > > - Payload (Tianocore) wants to know where the ACPI tables are, for > > > > example > > > > - Tianocore needs to provide boot services to the OS, so needs to know > > > > the memory map, etc. > > > > > > > > ** At this point we want to use DT to pass the required information. > > > > > > > > Of course, Platform Init could be coreboot or Tianocore or some > > > > strange private binary. Payload could be U-Boot or something else. > > > > That is the point of this effort, to build interoperability. > > > > [...] > > > > > > Perhaps the problem here is that Linux has tied itself up in knots > > > > with its EFI stuff and DT fixups and what-not. But this is not that. > > > > It is a simple handoff between two pieces of firmware, Platform Init > > > > and Payload. It has nothing to do with the OS. With Tianocore they are > > > > typically combined, but with this usage they are split, and we can > > > > swap out one project for another on either side of the DT interface. > > > > Is perhaps the clearest description of the problem you want to solve. > > It's clearly related to EFI though not the interface to the OS. IIRC, > > "platform init" and "payload" are terms in the UEFI spec, right? > > No they are not. This is from the universal payload specification that > is being drafted here > > https://universalpayload.github.io/spec/index.html > > but the UEFI specification does not use this terminology. Then I'm confused as to what this is: https://uefi.org/specs/PI/1.8/index.html Rob
Re: [PATCH v3 0/2] rng: Provide a RNG based on the RISC-V Zkr ISA extension
On Tue, Nov 7, 2023 at 1:30 PM Tom Rini wrote:
>
> On Tue, Nov 07, 2023 at 01:10:44AM +, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 6 Nov 2023 at 13:46, Tom Rini wrote:
> > >
> > > On Mon, Nov 06, 2023 at 01:38:39PM -0700, Simon Glass wrote:
> > > > Hi Andre,
> > > >
> > > > On Mon, 6 Nov 2023 at 10:26, Andre Przywara
> > > > wrote:
> > > > >
> > > > > On Sat, 4 Nov 2023 19:45:06 +
> > > > > Simon Glass wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > > On Sat, 4 Nov 2023 at 17:13, Andre Przywara
> > > > > > wrote:
> > > > > > >
> > > > > > > On Fri, 3 Nov 2023 13:38:58 -0600
> > > > > > > Simon Glass wrote:
> > > > > > >
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > > Hi Heinrich,
> > > > > > > >
> > > > > > > > On Wed, 1 Nov 2023 at 14:20, Heinrich Schuchardt
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On 11/1/23 19:05, Andre Przywara wrote:
> > > > > > > > > > On Tue, 31 Oct 2023 14:55:50 +0200
> > > > > > > > > > Heinrich Schuchardt
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Heinrich,
> > > > > > > > > >
> > > > > > > > > >> The Zkr ISA extension (ratified Nov 2021) introduced the
> > > > > > > > > >> seed CSR. It
> > > > > > > > > >> provides an interface to a physical entropy source.
> > > > > > > > > >>
> > > > > > > > > >> A RNG driver based on the seed CSR is provided. It depends
> > > > > > > > > >> on
> > > > > > > > > >> mseccfg.sseed being set in the SBI firmware.
> > > > > > > > > >
> > > > > > > > > > As you might have seen, I added a similar driver for the
> > > > > > > > > > respective Arm
> > > > > > > > > > functionality:
> > > > > > > > > > https://lore.kernel.org/u-boot/20230830113230.3925868-1-andre.przyw...@arm.com/
> > > > > > > > > >
> > > > > > > > > > And I see that you seem to use the same mechanism to probe
> > > > > > > > > > and init the
> > > > > > > > > > driver: U_BOOT_DRVINFO and fail in probe() if the feature
> > > > > > > > > > is not
> > > > > > > > > > implemented.
> > > > > > > > > > One downside of this approach is that the driver is always
> > > > > > > > > > loaded (and
> > > > > > > > > > visible in the DM tree), even with the feature not being
> > > > > > > > > > available.
> > > > > > > > > > That doesn't seem too much of a problem on the first
> > > > > > > > > > glance, but it
> > > > > > > > > > occupies a device number, and any subsequent other DM_RNG
> > > > > > > > > > devices
> > > > > > > > > > (like virtio-rng) typically get higher device numbers. So
> > > > > > > > > > without
> > > > > > > > > > the feature, but with virtio-rng, I get:
> > > > > > > > > > VExpress64# rng 0
> > > > > > > > > > No RNG device
> > > > > > > >
> > > > > > > > Why do we get this? If the device is not there, the bind()
> > > > > > > > function
> > > > > > > > can return -ENODEV
> > > > > > > >
> > > > > > > > I see this in U-Boot:
> > > > > > > >
> > > > > > > > U_BOOT_DRVINFO(cpu_arm_rndr) = {
> > > > > > > >
> > > > > > > > We should not use this.
> > > > > > >
> > > > > > > Agreed.
> > > > > > >
> > > > > > > > Use the devicetree.
> > > > > > >
> > > > > > > No, this is definitely not something for the DT, at least not on
> > > > > > > ARM.
> > > > > > > It's perfectly discoverable via the architected CPU ID registers.
> > > > > > > Similar to PCI and USB devices, which we don't probe via the DT
> > > > > > > as well.
> > > > > > >
> > > > > > > It's arguably not proper "driver" material per se, as I've argued
> > > > > > > before, but
> > > > > > > it's the simplest solution and fits in nicely otherwise.
> > > > > > >
> > > > > > > I was wondering if it might be something for UCLASS_CPU,
> > > > > > > something like
> > > > > > > a "CPU feature bus": to let devices register on one on the many
> > > > > > > CPU
> > > > > > > features (instead of compatible strings), then only bind() those
> > > > > > > drivers it the respective bit is set.
> > > > > > >
> > > > > > > Does that make sense? Would that be doable without boiling the
> > > > > > > ocean?
> > > > > > > As I don't know if we see many users apart from this.
> > > > > >
> > > > > > I have seen this so many times, where people want to avoid putting
> > > > > > things in the DT and then are surprised that everything is
> > > > > > difficult,
> > > > > > broken and confusing. Why not just follow the rules? It is not just
> > > > > > about whether we can avoid it, etc. It is about how devices fit
> > > > > > together cohesively in the system, and how U-Boot operates.
> > > > >
> > > > > A devicetree is only for peripherals *that cannot be located by
> > > > > probing*.
> > > >
> > > > I have to stop you there. It absolutely is not limited to that.
> > >
> > > It is limited to that if we're going to keep using the device trees that
> > > Linux uses. Full stop. There's not really wiggle room there either.
> >
> > That is really the problem, I agree.
>
> And we need to accept that, and what is/isn't something that we can
>
Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages
On Tue, Oct 31, 2023 at 10:56 AM Simon Glass wrote: > > Hi Rob, > > On Mon, 16 Oct 2023 at 15:54, Simon Glass wrote: > > > > Hi Rob, > > > > On Mon, 16 Oct 2023 at 10:50, Rob Herring wrote: > > > > > > On Fri, Oct 13, 2023 at 4:09 PM Simon Glass wrote: > > > > > > > > Hi Rob, > > > > > > > > On Fri, 13 Oct 2023 at 13:42, Rob Herring wrote: > > > > > > > > > > On Fri, Oct 6, 2023 at 7:03 PM Simon Glass wrote: > > > > > > > > > > > > Hi Ard, > > > > > > > > > > > > On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel wrote: > > > > > > > > > > > > > > On Fri, 6 Oct 2023 at 20:17, Simon Glass > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Ard, > > > > > > > > > > > > > > > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Rob, > > > > > > > > > > > > > > > > > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > It is common to split firmware into 'Platform Init', > > > > > > > > > > > which does the > > > > > > > > > > > initial hardware setup and a "Payload" which selects the > > > > > > > > > > > OS to be booted. > > > > > > > > > > > Thus an handover interface is required between these two > > > > > > > > > > > pieces. > > > > > > > > > > > > > > > > > > > > > > Where UEFI boot-time services are not available, but UEFI > > > > > > > > > > > firmware is > > > > > > > > > > > present on either side of this interface, information > > > > > > > > > > > about memory usage > > > > > > > > > > > and attributes must be presented to the "Payload" in some > > > > > > > > > > > form. > > > > > > > > > > > > > > > > > > > > > > This aims to provide an small schema addition for the > > > > > > > > > > > memory mapping > > > > > > > > > > > needed to keep these two pieces working together well. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > Changes in v7: > > > > > > > > > > > - Rename acpi-reclaim to acpi > > > > > > > > > > > - Drop individual mention of when memory can be reclaimed > > > > > > > > > > > - Rewrite the item descriptions > > > > > > > > > > > - Add back the UEFI text (with trepidation) > > > > > > > > > > > > > > > > > > > > I am again checking on this series. Can it be applied, > > > > > > > > > > please? > > > > > > > > > > > > > > > > > > > > > > > > > > > > Apologies for the delay in response. I have been away. > > > > > > > > > > > > > > > > OK, I hope you had a nice trip. > > > > > > > > > > > > > > > > > > > > > > Thanks, it was wonderful! > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Changes in v6: > > > > > > > > > > > - Drop mention of UEFI > > > > > > > > > > > - Use compatible strings instead of n
Re: [PATCH v3 1/1] efi_loader: expose the device-tree file name
On Sun, Oct 22, 2023 at 4:32 PM Heinrich Schuchardt wrote: > > On 10/22/23 19:08, Tom Rini wrote: > > On Sun, Oct 22, 2023 at 06:34:08PM +0200, Heinrich Schuchardt wrote: > >> On 10/22/23 17:55, Tom Rini wrote: > >>> On Sun, Oct 22, 2023 at 10:47:33AM +0200, Heinrich Schuchardt wrote: > >>> > Forward and backward compatibility of Linux kernel device-trees is > sometimes missing. One solution approach is to load a kernel specific > device-tree. This can either be done via a U-Boot scripts (like the one > generated by Debian package flash-kernel or by a boot loader like GRUB. > The boot loader approach currently requires to know the device-tree name > before first boot which makes it unusable for generic images. > > Expose the device-tree file name as EFI variable FdtFile. > This will allow bootloaders to load a kernel specific device-tree. > > The variable will not be exposed on ACPI based systems or if the > environment variable fdtfile is not defined. > > Signed-off-by: Heinrich Schuchardt > --- > v3: > Add documentation > v2: > Use a unique GUID to enable future U-Boot independent > standardization. > Do not try to add the variable on ACPI based systems. > --- > doc/develop/uefi/uefi.rst | 17 + > include/efi_loader.h | 5 + > lib/efi_loader/efi_setup.c | 30 ++ > 3 files changed, 52 insertions(+) > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst > index fb16ac743a..a81d09ae81 100644 > --- a/doc/develop/uefi/uefi.rst > +++ b/doc/develop/uefi/uefi.rst > @@ -916,6 +916,23 @@ So our final format of the FilePathList[] is:: > Loaded image - end node (0xff) - VenMedia - initrd_1 - [end node > (0x01) - initrd_n ...] - end node (0xff) > +EFI variable FdtFile > + > + > +Ideally U-Boot would always expose a device-tree that can be used for > booting > +any operating systems. Unfortunately operating systems like Linux > sometimes > +break forward and backward compatibility. In this case there is a need > to load > +an operating system version specific device-tree. > + > +U-Boot has an environment variable fdtfile identifying the device-tree > file to > +load. The content of this variable is exposed as EFI variable Fdtfile, > vendor > +GUID d45dde69-3bd6-40e0-90d5-6b606aa57730. It contains the device-tree > path > +name as a NUL terminated ASCII string. > + > +On 32bit ARM this is currently only a file name, e.g. > 'imx6dl-wandboard.dtb'. > +On other architectures the file name is preceded by the vendor > directory, e.g. > +'rockchip/rk3326-odroid-go2.dtb'. > >>> > >>> The Linux Kernel has split 32bit ARM up by directory now, too. > >> > >> Since Linux v6.5. That is why I wrote "currently". Once we migrate the > >> values of $fdtfile in U-Boot we may want to change > >> distro_efi_try_bootflow_files() to search both with and without vendor > >> directory. > >> > >> Are there already plans for that migration? > > > > Right, v6.5 is out and has this change and v6.6 will be out soon enough, > > so the documentation we're adding here and now should be worded such > > that doesn't get stuck on these specifics. > > Should I add a sentence: > > Linux v6.5 has added vendor directories on 32bit ARM and U-Boot is > expected to follow suit. Note that while this is correct, the install (make dtbs_install) still uses a flat directory. That was to not break existing users. It is currently not a visible config option. Perhaps it should be if there's a desire to move to having sub-directories for the install? Rob
Re: [PATCH v5 2/3] dt-bindings: mtd: binman-partition: Add binman compatibles
On Wed, 25 Oct 2023 10:39:17 +1300, Simon Glass wrote:
> Add two compatible for binman entries, as a starting point for the
> schema.
>
> Note that, after discussion on v2, we decided to keep the existing
> meaning of label so as not to require changes to existing userspace
> software when moving to use binman nodes to specify the firmware
> layout.
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v5:
> - Add mention of why 'binman' is the vendor
> - Drop 'select: false'
> - Tidy up the compatible setings
> - Use 'tfa-bl31' instead of 'atf-bl31'
>
> Changes in v4:
> - Correct selection of multiple compatible strings
>
> Changes in v3:
> - Drop fixed-partitions from the example
> - Use compatible instead of label
>
> Changes in v2:
> - Use plain partition@xxx for the node name
>
> .../mtd/partitions/binman-partition.yaml | 50 +++
> 1 file changed, 50 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml:
properties:compatible: [{'enum': ['binman,entry', 'u-boot', 'tfa-bl31']}] is
not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml:
properties:compatible: [{'enum': ['binman,entry', 'u-boot', 'tfa-bl31']}] is
not of type 'object', 'boolean'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
Documentation/devicetree/bindings/mtd/partitions/binman-partition.example.dtb:
/example-0/partitions/partition@10: failed to match any schema with
compatible: ['u-boot']
Documentation/devicetree/bindings/mtd/partitions/binman-partition.example.dtb:
/example-0/partitions/partition@20: failed to match any schema with
compatible: ['tfa-bl31']
doc reference errors (make refcheckdocs):
See
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231024213940.3590507-2-...@chromium.org
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH v4 3/3] dt-bindings: mtd: binman-partitions: Add alignment properties
On Mon, Oct 09, 2023 at 04:04:15PM -0600, Simon Glass wrote:
> Add three properties for controlling alignment of partitions, aka
> 'entries' in binman.
>
> For now there is no explicit mention of hierarchy, so a 'section' is
> just the 'binman' node.
>
> These new properties are inputs to the packaging process, but are also
> needed if the firmware is repacked, to ensure that alignment
> constraints are not violated. Therefore they are provided as part of
> the schema.
>
> Signed-off-by: Simon Glass
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Fix 'a' typo in commit message
>
> .../mtd/partitions/binman-partition.yaml | 39 +++
> 1 file changed, 39 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> index 35a320359ec1..8e8a3b6d4d14 100644
> --- a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> +++ b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> @@ -28,6 +28,42 @@ properties:
>- const: u-boot # u-boot.bin from U-Boot project
>- const: atf-bl31 # bl31.bin or bl31.elf from TF-A project
>
> + align:
> +$ref: /schemas/types.yaml#/definitions/uint32
> +description:
> + This sets the alignment of the entry. The entry offset is adjusted
> + so that the entry starts on an aligned boundary within the containing
> + section or image. For example ‘align = <16>’ means that the entry will
> + start on a 16-byte boundary. This may mean that padding is added before
Only your example defines that alignment is in bytes.
> + the entry. The padding is part of the containing section but is not
> + included in the entry, meaning that an empty space may be created
> before
> + the entry starts. Alignment should be a power of 2. If ‘align’ is not
> + provided, no alignment is performed.
Would be nice to have some constraints. Unfortunately, no way to say
'power of 2' in json-schema (we could add something possibly), so the
only way is:
enum: [ 2, 4, 8, 16, 32, 64, ... ]
Kind of verbose if we add all 31 possibilities...
Could also do this:
minium: 2
maximum: 0x8000
multipleOf: 2
> +
> + align-size:
> +$ref: /schemas/types.yaml#/definitions/uint32
> +description:
> + This sets the alignment of the entry size. For example, to ensure
> + that the size of an entry is a multiple of 64 bytes, set this to 64.
> + While this does not affect the contents of the entry within binman
> + itself (the padding is performed only when its parent section is
> + assembled), the end result is that the entry ends with the padding
> + bytes, so may grow. If ‘align-size’ is not provided, no alignment is
> + performed.
> +
> + align-end:
> +$ref: /schemas/types.yaml#/definitions/uint32
> +description:
> + This sets the alignment of the end of an entry with respect to the
> + containing section. Some entries require that they end on an alignment
> + boundary, regardless of where they start. This does not move the start
> + of the entry, so the contents of the entry will still start at the
> + beginning. But there may be padding at the end. While this does not
> + affect the contents of the entry within binman itself (the padding is
> + performed only when its parent section is assembled), the end result is
> + that the entry ends with the padding bytes, so may grow. If ‘align-end’
> + is not provided, no alignment is performed.
> +
> additionalProperties: false
>
> examples:
> @@ -40,10 +76,13 @@ examples:
> partition@10 {
> compatible = "u-boot";
> reg = <0x10 0xf0>;
> +align-size = <0x1000>;
> +align-end = <0x1>;
> };
>
> partition@20 {
> compatible = "atf-bl31";
> reg = <0x20 0x10>;
> +align = <0x4000>;
> };
> };
> --
> 2.42.0.609.gbb76f46606-goog
>
Re: [PATCH v4 2/3] dt-bindings: mtd: binman-partition: Add binman compatibles
On Mon, Oct 09, 2023 at 04:04:14PM -0600, Simon Glass wrote:
> Add two compatible for binman entries, as a starting point for the
> schema.
>
> Note that, after discussion on v2, we decided to keep the existing
> meaning of label so as not to require changes to existing userspace
> software when moving to use binman nodes to specify the firmware
> layout.
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v4:
> - Correct selection of multiple compatible strings
>
> Changes in v3:
> - Drop fixed-partitions from the example
> - Use compatible instead of label
>
> Changes in v2:
> - Use plain partition@xxx for the node name
>
> .../mtd/partitions/binman-partition.yaml | 49 +++
> 1 file changed, 49 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> new file mode 100644
> index ..35a320359ec1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Google LLC
> +
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/partitions/binman-partition.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Binman partition
> +
> +maintainers:
> + - Simon Glass
> +
> +select: false
So this schema is never used. 'select: false' is only useful if
something else if referencing the schema.
> +
> +description: |
> + This corresponds to a binman 'entry'. It is a single partition which holds
> + data of a defined type.
> +
> +allOf:
> + - $ref: /schemas/mtd/partitions/partition.yaml#
> +
> +properties:
> + compatible:
> +oneOf:
> + - const: binman,entry # generic binman entry
'binman' is not a vendor. You could add it if you think that's useful.
Probably not with only 1 case...
> + - items:
> + - const: u-boot # u-boot.bin from U-Boot project
> + - const: atf-bl31 # bl31.bin or bl31.elf from TF-A project
Probably should use the new 'tfa' rather than old 'atf'. Is this the
only binary for TFA? The naming seems inconsistent in that every image
goes in (or can go in) a bl?? section. Why does TFA have it but u-boot
doesn't? Perhaps BL?? is orthogonal to defining what is in each
partition. Perhaps someone more familar with all this than I am can
comment.
Once you actually test this, you'll find you are specifying:
compatible = "u-boot", "atf-bl31";
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +partitions {
> +compatible = "binman";
> +#address-cells = <1>;
> +#size-cells = <1>;
> +
> +partition@10 {
> +compatible = "u-boot";
> +reg = <0x10 0xf0>;
> +};
> +
> +partition@20 {
> +compatible = "atf-bl31";
> +reg = <0x20 0x10>;
> +};
> +};
> --
> 2.42.0.609.gbb76f46606-goog
>
Re: [PATCH v4 1/3] dt-bindings: mtd: partitions: Add binman compatible
On Mon, Oct 09, 2023 at 04:04:13PM -0600, Simon Glass wrote:
> Add a compatible string for binman, so we can extend fixed-partitions
> in various ways.
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v4:
> - Change subject line
>
> Changes in v3:
> - Drop fixed-partition additional compatible string
> - Drop fixed-partitions from the example
> - Mention use of compatible instead of label
>
> Changes in v2:
> - Drop mention of 'enhanced features' in fixed-partitions.yaml
> - Mention Binman input and output properties
> - Use plain partition@xxx for the node name
>
> .../bindings/mtd/partitions/binman.yaml | 63 +++
> .../bindings/mtd/partitions/partitions.yaml | 1 +
> MAINTAINERS | 5 ++
> 3 files changed, 69 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/mtd/partitions/binman.yaml
>
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/binman.yaml
> b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml
> new file mode 100644
> index ..7d6c8bd738f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Google LLC
> +
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/partitions/binman.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Binman firmware layout
> +
> +maintainers:
> + - Simon Glass
> +
> +select: false
> +
> +description: |
> + The binman node provides a layout for firmware, used when packaging
> firmware
> + from multiple projects. It is based on fixed-partitions, with some
> + extensions, but uses 'compatible' to indicate the contents of the node, to
> + avoid perturbing or confusing existing installations which use 'label' for
> a
> + particular purpose.
> +
> + Binman supports properties used as inputs to the firmware-packaging
> process,
> + such as those which control alignment of partitions. This binding addresses
> + these 'input' properties. For example, it is common for the 'reg' property
> + (an 'output' property) to be set by Binman, based on the alignment
> requested
> + in the input.
> +
> + Once processing is complete, input properties have mostly served their
> + purpose, at least until the firmware is repacked later, e.g. due to a
> + firmware update. The 'fixed-partitions' binding should provide enough
> + information to read the firmware at runtime, including decompression if
> + needed.
> +
> + Documentation for Binman is available at:
> +
> + https://u-boot.readthedocs.io/en/latest/develop/package/binman.html
> +
> + with the current image-description format at:
> +
> +
> https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#image-description-format
> +
> +allOf:
> + - $ref: /schemas/mtd/partitions/fixed-partitions.yaml#
> +
> +properties:
> + compatible:
> +const: binman
This couldn't possibly pass validation. Including fixed-partitions.yaml
says the compatible must be one thing and this says something else.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +partitions {
> +compatible = "binman";
> +#address-cells = <1>;
> +#size-cells = <1>;
> +
> +partition@10 {
> +label = "u-boot";
> +reg = <0x10 0xf0>;
> +};
> +};
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
> b/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
> index 1dda2c80747b..849fd15d085c 100644
> --- a/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
> +++ b/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
> @@ -15,6 +15,7 @@ maintainers:
>
> oneOf:
>- $ref: arm,arm-firmware-suite.yaml
> + - $ref: binman.yaml
>- $ref: brcm,bcm4908-partitions.yaml
>- $ref: brcm,bcm947xx-cfe-partitions.yaml
>- $ref: fixed-partitions.yaml
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c934244acc31..ebc8158fe67d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3536,6 +3536,11 @@ F: Documentation/filesystems/bfs.rst
> F: fs/bfs/
> F: include/uapi/linux/bfs_fs.h
>
> +BINMAN
> +M: Simon Glass
> +S: Supported
> +F: Documentation/devicetree/bindings/mtd/partitions/binman*
> +
> BITMAP API
> M: Yury Norov
> R: Andy Shevchenko
> --
> 2.42.0.609.gbb76f46606-goog
>
Re: [PATCH v7 1/2] schemas: memory: Add ECC properties
On Tue, Sep 26, 2023 at 01:42:38PM -0600, Simon Glass wrote: > Some memories provide ECC detection and/or correction. For software which > wants to check memory, it is helpful to see which regions provide this > feature. > > Add this as a property of the /memory nodes, since it presumably follows > the hardware-level memory system. > > Signed-off-by: Simon Glass > --- > > Changes in v7: > - Drop unnecessary | > - Add a blank line between properties > > Changes in v6: > - Use a number of bits instead of a string property > - Fix inidcates typo > > Changes in v5: > - Redo to make this property specific to ECC > - Provide properties both for detection and correction > > Changes in v3: > - Add new patch to update the /memory nodes > > dtschema/schemas/memory.yaml | 13 + > 1 file changed, 13 insertions(+) Applied this patch. Rob
Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages
On Fri, Oct 13, 2023 at 4:09 PM Simon Glass wrote: > > Hi Rob, > > On Fri, 13 Oct 2023 at 13:42, Rob Herring wrote: > > > > On Fri, Oct 6, 2023 at 7:03 PM Simon Glass wrote: > > > > > > Hi Ard, > > > > > > On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel wrote: > > > > > > > > On Fri, 6 Oct 2023 at 20:17, Simon Glass wrote: > > > > > > > > > > Hi Ard, > > > > > > > > > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel wrote: > > > > > > > > > > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass wrote: > > > > > > > > > > > > > > Hi Rob, > > > > > > > > > > > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass > > > > > > > wrote: > > > > > > > > > > > > > > > > It is common to split firmware into 'Platform Init', which does > > > > > > > > the > > > > > > > > initial hardware setup and a "Payload" which selects the OS to > > > > > > > > be booted. > > > > > > > > Thus an handover interface is required between these two pieces. > > > > > > > > > > > > > > > > Where UEFI boot-time services are not available, but UEFI > > > > > > > > firmware is > > > > > > > > present on either side of this interface, information about > > > > > > > > memory usage > > > > > > > > and attributes must be presented to the "Payload" in some form. > > > > > > > > > > > > > > > > This aims to provide an small schema addition for the memory > > > > > > > > mapping > > > > > > > > needed to keep these two pieces working together well. > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > --- > > > > > > > > > > > > > > > > Changes in v7: > > > > > > > > - Rename acpi-reclaim to acpi > > > > > > > > - Drop individual mention of when memory can be reclaimed > > > > > > > > - Rewrite the item descriptions > > > > > > > > - Add back the UEFI text (with trepidation) > > > > > > > > > > > > > > I am again checking on this series. Can it be applied, please? > > > > > > > > > > > > > > > > > > > Apologies for the delay in response. I have been away. > > > > > > > > > > OK, I hope you had a nice trip. > > > > > > > > > > > > > Thanks, it was wonderful! > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Changes in v6: > > > > > > > > - Drop mention of UEFI > > > > > > > > - Use compatible strings instead of node names > > > > > > > > > > > > > > > > Changes in v5: > > > > > > > > - Drop the memory-map node (should have done that in v4) > > > > > > > > - Tidy up schema a bit > > > > > > > > > > > > > > > > Changes in v4: > > > > > > > > - Make use of the reserved-memory node instead of creating a > > > > > > > > new one > > > > > > > > > > > > > > > > Changes in v3: > > > > > > > > - Reword commit message again > > > > > > > > - cc a lot more people, from the FFI patch > > > > > > > > - Split out the attributes into the /memory nodes > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > - Reword commit message > > > > > > > > > > > > > > > > .../reserved-memory/common-reserved.yaml | 71 > > > > > > > > +++ > > > > > > > > 1 file changed, 71 insertions(+) > > > > > > > > create mode 100644 > > > > > > > > dtschema/schemas/reserved-memory/common-reserved.yaml > > > > > > > > > > > > > > > > diff --git > >
Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages
On Fri, Oct 6, 2023 at 7:03 PM Simon Glass wrote: > > Hi Ard, > > On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel wrote: > > > > On Fri, 6 Oct 2023 at 20:17, Simon Glass wrote: > > > > > > Hi Ard, > > > > > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel wrote: > > > > > > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass wrote: > > > > > > > > > > Hi Rob, > > > > > > > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass wrote: > > > > > > > > > > > > It is common to split firmware into 'Platform Init', which does the > > > > > > initial hardware setup and a "Payload" which selects the OS to be > > > > > > booted. > > > > > > Thus an handover interface is required between these two pieces. > > > > > > > > > > > > Where UEFI boot-time services are not available, but UEFI firmware > > > > > > is > > > > > > present on either side of this interface, information about memory > > > > > > usage > > > > > > and attributes must be presented to the "Payload" in some form. > > > > > > > > > > > > This aims to provide an small schema addition for the memory mapping > > > > > > needed to keep these two pieces working together well. > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > --- > > > > > > > > > > > > Changes in v7: > > > > > > - Rename acpi-reclaim to acpi > > > > > > - Drop individual mention of when memory can be reclaimed > > > > > > - Rewrite the item descriptions > > > > > > - Add back the UEFI text (with trepidation) > > > > > > > > > > I am again checking on this series. Can it be applied, please? > > > > > > > > > > > > > Apologies for the delay in response. I have been away. > > > > > > OK, I hope you had a nice trip. > > > > > > > Thanks, it was wonderful! > > > > > > > > > > > > > > > > > > > > > > > Changes in v6: > > > > > > - Drop mention of UEFI > > > > > > - Use compatible strings instead of node names > > > > > > > > > > > > Changes in v5: > > > > > > - Drop the memory-map node (should have done that in v4) > > > > > > - Tidy up schema a bit > > > > > > > > > > > > Changes in v4: > > > > > > - Make use of the reserved-memory node instead of creating a new one > > > > > > > > > > > > Changes in v3: > > > > > > - Reword commit message again > > > > > > - cc a lot more people, from the FFI patch > > > > > > - Split out the attributes into the /memory nodes > > > > > > > > > > > > Changes in v2: > > > > > > - Reword commit message > > > > > > > > > > > > .../reserved-memory/common-reserved.yaml | 71 > > > > > > +++ > > > > > > 1 file changed, 71 insertions(+) > > > > > > create mode 100644 > > > > > > dtschema/schemas/reserved-memory/common-reserved.yaml > > > > > > > > > > > > diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml > > > > > > b/dtschema/schemas/reserved-memory/common-reserved.yaml > > > > > > new file mode 100644 > > > > > > index 000..f7fbdfd > > > > > > --- /dev/null > > > > > > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml > > > > > > @@ -0,0 +1,71 @@ > > > > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: > > > > > > http://devicetree.org/schemas/reserved-memory/common-reserved.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: Common memory reservations > > > > > > + > > > > > > +description: | > > > > > > + Specifies that the reserved memory region can be used for the > > > > > > purpose > > > > > > + indicated by its compatible string. > > > > > > + > > > > > > + Clients may reuse this reserved memory if they understand what > > > > > > it is for, > > > > > > + subject to the notes below. > > > > > > + > > > > > > +maintainers: > > > > > > + - Simon Glass > > > > > > + > > > > > > +allOf: > > > > > > + - $ref: reserved-memory.yaml > > > > > > + > > > > > > +properties: > > > > > > + compatible: > > > > > > +description: | > > > > > > + This describes some common memory reservations, with the > > > > > > compatible > > > > > > + string indicating what it is used for: > > > > > > + > > > > > > + acpi: Advanced Configuration and Power Interface (ACPI) > > > > > > tables > > > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This > > > > > > is reserved by > > > > > > + the firmware for its use and is required to be saved > > > > > > and restored > > > > > > + across an NVS sleep > > > > > > + boot-code: Contains code used for booting which is not > > > > > > needed by the OS > > > > > > + boot-code: Contains data used for booting which is not > > > > > > needed by the OS > > > > > > + runtime-code: Contains code used for interacting with the > > > > > > system when > > > > > > + running the OS > > > > > > + runtime-data: Contains data used for interacting with the > > > > > > system when > > > > > > + running the OS > > > > > > + > > > > > > +enum: > >
Re: [PATCH v3 2/3] dt-bindings: mtd: binman-partition: Add binman compatibles
On Mon, Oct 09, 2023 at 04:02:40PM -0600, Simon Glass wrote:
> Hi Rob,
>
> On Mon, 9 Oct 2023 at 15:18, Rob Herring wrote:
> >
> >
> > On Mon, 09 Oct 2023 14:10:00 -0600, Simon Glass wrote:
> > > Add two compatible for binman entries, as a starting point for the
> > > schema.
> > >
> > > Note that, after discussion on v2, we decided to keep the existing
> > > meaning of label so as not to require changes to existing userspace
> > > software when moving to use binman nodes to specify the firmware
> > > layout.
> > >
> > > Signed-off-by: Simon Glass
> > > ---
> > >
> > > Changes in v3:
> > > - Drop fixed-partitions from the example
> > > - Use compatible instead of label
> > >
> > > Changes in v2:
> > > - Use plain partition@xxx for the node name
> > >
> > > .../mtd/partitions/binman-partition.yaml | 48 +++
> > > 1 file changed, 48 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> > >
> >
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml:
> > properties:compatible:items: {'enum': ['u-boot', 'atf-bl31']} is not of
> > type 'array'
> > from schema $id:
> > http://devicetree.org/meta-schemas/string-array.yaml#
> >
> > doc reference errors (make refcheckdocs):
> >
> > See
> > https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231009201005.1964794-2-...@chromium.org
> >
> > The base for the series is generally the latest rc1. A different dependency
> > should be noted in *this* patch.
> >
> > If you already ran 'make dt_binding_check' and didn't see the above
> > error(s), then make sure 'yamllint' is installed and dt-schema is up to
> > date:
> >
> > pip3 install dtschema --upgrade
> >
> > Please check and re-submit after running the above command yourself. Note
> > that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> > your schema. However, it must be unset to test all examples with your
> > schema.
> >
>
> Oh dear, I didn't notice that output but I see it now. Could the check
> return a non-zero exit code if something goes wrong?
No, because things go wrong too often and then it breaks for everyone.
It's better now, but only because of the above reports and 3
maintainers.
Also, it is not fatal. The schemas are checked against the DT
meta-schema, but are used for validation if they pass just draft2019-09
meta-schema. That allows new DT meta-schema checks to not start
excluding previously used schema.
Rob
Re: [PATCH v4] dt-bindings: mtd: fixed-partitions: Add compression property
On Wed, 27 Sep 2023 12:05:43 -0600, Simon Glass wrote: > Sometimes the contents of a partition are compressed. Add a property to > express this and define the algorithm used. > > Signed-off-by: Simon Glass > --- > > Changes in v4: > - Add an example > > Changes in v3: > - Just add a compression property for now > > Changes in v2: > - Use "binman" for compatible instead of "u-boot,binman" > - Significantly rework the patch > - Use make dt_binding_check DT_SCHEMA_FILES=Documentation/../partitions > > .../mtd/partitions/fixed-partitions.yaml | 19 +++++++ > 1 file changed, 19 insertions(+) > Reviewed-by: Rob Herring
Re: [PATCH v3 2/3] dt-bindings: mtd: binman-partition: Add binman compatibles
On Mon, 09 Oct 2023 14:10:00 -0600, Simon Glass wrote:
> Add two compatible for binman entries, as a starting point for the
> schema.
>
> Note that, after discussion on v2, we decided to keep the existing
> meaning of label so as not to require changes to existing userspace
> software when moving to use binman nodes to specify the firmware
> layout.
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v3:
> - Drop fixed-partitions from the example
> - Use compatible instead of label
>
> Changes in v2:
> - Use plain partition@xxx for the node name
>
> .../mtd/partitions/binman-partition.yaml | 48 +++
> 1 file changed, 48 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml:
properties:compatible:items: {'enum': ['u-boot', 'atf-bl31']} is not of type
'array'
from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
doc reference errors (make refcheckdocs):
See
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231009201005.1964794-2-...@chromium.org
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH v2 1/3] dt-bindings: mtd: fixed-partitions: Add binman compatible
On Fri, Oct 06, 2023 at 10:37:41AM +0200, Michael Walle wrote: > Hi, > > > > I'm still not sure why that compatible is needed. Also I'd need to > > > change > > > the label which might break user space apps looking for that specific > > > name. > > > > > > Also, our board might have u-boot/spl or u-boot/spl/bl31/bl32, right > > > now > > > that's something which depends on an u-boot configuration variable, > > > which > > > then enables or disables binman nodes in the -u-boot.dtsi. So in linux > > > we only have that "bootloader" partition, but there might be either > > > u-boot+spl or u-boot+spl+bl31+bl32. > > > > > > Honestly, I'm really not sure this should go into a device tree. > > > > I think we might be getting a bit ahead of ourselves here. I thought > > that the decision was that the label should indicate the contents. > > If you have multiple things in a partition then it would become a > > 'section' in Binman's terminology. Either the label programmatically > > describes what is inside or it doesn't. We can't have it both ways. > > What do you suggest? > > As Rob pointed out earlier, it's just a user-facing string. I'm a bit > reluctant to use it programatically. In general, yes, but the partition stuff has long (and still) uses label. As long as the values the tools understand are documented (which we don't normally do for label), I don't care so much. That's my opinion as long as this is shared with fixed-partitions. If it is not and there's little reason to use label, then absolutely, I think 'compatible' makes more sense. > Taking my example again, the string "bootloader" is sufficient for a > user. He doesn't care if it's u-boot with spl or u-boot with tfa, or > even coreboot. It just says, "in this partition is the bootloader". > If you have an "bootloader" image you can flash it there. These days, there's generally not just 1 bootloader in the boot flow. Maybe there's 1 image, maybe not. Being more specific is hardly ever a bad thing. Only when the number of specific things becomes multiple 10s or 100s of them does it become a problem. > If it has a label "u-boot" and I want to switch to coreboot, will > it have to change to "coreboot"? I really don't think this is practical, > you are really putting software configuration into the device tree. On the input side (to binman), yes it is config, but on the output side (to the running system) we are saying what's there. > > At present it seems you have the image described in two places - one > > is the binman node and the other is the partitions node. I would like > > to unify these. > > And I'm not sure that will work for all the corner cases :/ > > If you keep the binman section seperate from the flash partition > definition you don't have any of these problems, although there is > some redundancy: > - you only have compatible = "binman", "fixed-partition", no further >compatibles are required > - you don't have any conflicts with the current partition descriptions > - you could even use the labels, because binman is the (only?) user > > But of course you need to find a place where to put your node. And remove it. We don't need 2 sources of truth in the DTB. Rob
Re: [PATCH] dt-bindings: mtd: Add a schema for binman
On Tue, Sep 26, 2023 at 2:48 AM Miquel Raynal wrote: > > Hello, > > > > > > > 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". > > > > Well so long as we don't add any binman things in here, you are right. > > > > But the eventual goal is parity with current Binman functionality, > > which writes the entire (augmented) description to the DT, allowing > > tools to rebuild / repack / replace pieces later, maintaining the same > > alignment constraints, etc. I am assuming that properties like 'align > > = <16>' would not fit with fixed-partitions. > > I am personally not bothered by this kind of properties. But if we plan > on adding too much properties, I will advise to indeed use another name > than fixed-partitions (or add the "binman" secondary compatible) > otherwise it's gonna be hard to support in the code while still > restraining as much as we can the other partition schema. Agreed. It's a trade off. I think we need enough to understand the problem (not just presented with a solution), agree on the general solution/direction, and then discuss specific additions. > > But if we don't preserve > > these properties then Binman cannot do repacking reliably. Perhaps for > > now I could put the augmented DT in its own section somewhere, but I > > am just not sure if that will work in a real system. E.g. with VBE the > > goal is to use the DT to figure out how to access the firmware, update > > it, etc. VBE? > > Is it not possible to have my own node with whatever things Binman > > needs in it (subject to review of course)? i.e. could we discuss how > > to encode it, but argue less about whether things are needed? I > > kind-of feel I know what is needed, since I wrote the tool. What we don't need is the same information in 2 places for the DTB used at runtime. If the binman node is removed, do whatever you want. If you want to keep it at runtime, then it's got to extend what we already have. I don't think anyone is disagreeing about whether specific information is needed or not. > > > > 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. > > > > OK so we are agreed that we are going with 'compatible'. > > Still strongly disagree here. Miquel is right. I was confused here. "label" is still pretty much used for what the image is. Though we do have "u-boot,env" for both it seems. My position on "label" stands. To the extent we have images for common components, I think we should standardize the names. Certainly if tools rely on the names, then they should be documented. > My understanding is that a compatible carries how the content is > organized, and how this maybe specific (like you have in-band meta data > data that needs to be parsed in a specific way or in your case > additional specific properties in the DT which give more context about > how the data is stored). But the real content of the partition, ie. if > it contains a firmware, the kernel or some user data does not belong to > the compatible. > > I.e: > - The first byte of my partition gives the compression algorithm: > -> compatible = "compressed-partition-foo"; > or > -> compatible = "fixed-partitions" + compression-algorithm = "foo"; > - The partition contains a picture of my dog: > -> label = "my dog is beautiful" > but certainly not > -> compatible = "my-dog"; IMO, compatible in this case should convey "JPEG image" or similar. > I don't see why, for the binman schema, we could not constrain the > labels? Yes, but those should follow what we already have. "u-boot" for example rather than
Re: [PATCH] dt-bindings: mtd: Add a schema for binman
On Mon, Sep 25, 2023 at 11:25 AM Simon Glass wrote: > > Hi Miquel, > > On Mon, 25 Sept 2023 at 09:24, Miquel Raynal > 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' >
Re: [PATCH v2] dt-bindings: mtd: Add a schema for binman
On Fri, 22 Sep 2023 13:34:15 -0600, Simon Glass wrote: > Binman[1] is a tool for creating firmware images. It allows you to > combine various binaries and place them in an output file. > > Binman uses a DT schema to describe an image, in enough detail that > it can be automatically built from component parts, disassembled, > replaced, listed, etc. > > Images are typically stored in flash, which is why this binding is > targeted at mtd. Previous discussion is at [2] [3]. > > This is only a starting point, an attempt to align on the best way to > add this to the schema. > > [1] https://u-boot.readthedocs.io/en/stable/develop/package/binman.html > [2] https://lore.kernel.org/u-boot/20230821180220.2724080-3-...@chromium.org/ > [3] https://www.spinics.net/lists/devicetree/msg626149.html > > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Use "binman" for compatible instead of "u-boot,binman" > - Significantly rework the patch > - Use make dt_binding_check DT_SCHEMA_FILES=Documentation/../partitions > > .../bindings/mtd/partitions/binman.yaml | 59 ++ > .../mtd/partitions/binman/atf-bl31.yaml | 43 + > .../bindings/mtd/partitions/binman/entry.yaml | 62 +++ > .../mtd/partitions/binman/u-boot.yaml | 43 + > .../bindings/mtd/partitions/partitions.yaml | 1 + > MAINTAINERS | 5 ++ > 6 files changed, 213 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman.yaml > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman/atf-bl31.yaml > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman/entry.yaml > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman/u-boot.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/mtd/partitions/binman/atf-bl31.example.dtb: /example-0/binman/partition@0: failed to match any schema with compatible: ['binman,entry'] Documentation/devicetree/bindings/mtd/partitions/binman/entry.example.dtb: /example-0/binman/partition@0: failed to match any schema with compatible: ['binman,entry'] Documentation/devicetree/bindings/mtd/partitions/binman/entry.example.dtb: /example-0/binman/partition@10: failed to match any schema with compatible: ['binman,entry'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230922193417.1697379-1-...@chromium.org The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH] dt-bindings: mtd: Add a schema for binman
On Fri, Sep 22, 2023 at 1:12 PM Simon Glass wrote: > > Hi Rob, > > On Fri, 22 Sept 2023 at 11:46, Rob Herring wrote: > > > > On Fri, Sep 22, 2023 at 11:01:18AM -0600, Simon Glass wrote: > > > Hi Rob, > > > > > > On Fri, 22 Sept 2023 at 10:00, Rob Herring wrote: > > > > > > > > On Thu, Sep 21, 2023 at 1:45 PM Simon Glass wrote: > > > > > > > > > > Binman[1] is a tool for creating firmware images. It allows you to > > > > > combine various binaries and place them in an output file. > > > > > > > > > > Binman uses a DT schema to describe an image, in enough detail that > > > > > it can be automatically built from component parts, disassembled, > > > > > replaced, listed, etc. > > > > > > > > > > Images are typically stored in flash, which is why this binding is > > > > > targeted at mtd. Previous discussion is at [2] [3]. > > > > > > > > > > [1] > > > > > https://u-boot.readthedocs.io/en/stable/develop/package/binman.html > > > > > [2] > > > > > https://lore.kernel.org/u-boot/20230821180220.2724080-3-...@chromium.org/ > > > > > [3] https://www.spinics.net/lists/devicetree/msg626149.html > > > > > > > > You missed: > > > > > > > > https://github.com/devicetree-org/dt-schema/pull/110 > > > > > > > > where I said: We certainly shouldn't duplicate the existing partitions > > > > bindings. What's missing from them (I assume we're mostly talking > > > > about "fixed-partitions" which has been around forever I think (before > > > > me))? > > > > > > > > To repeat, unless there is some reason binman partitions conflict with > > > > fixed-partitions, you need to start there and extend it. From what's > > > > posted here, it neither conflicts nor needs extending. > > > > > > I think at this point I am just hopelessly confused. Have you taken a > > > look at the binman schema? [1] > > > > Why do I need to? That's used for some tool and has nothing to do with a > > device's DTB. However, I thought somewhere in this discussion you showed > > it under a flash device node. > > Yes, that is the intent (under a flash node). > > > Then I care because then it overlaps with > > what we already have for partitions. If I misunderstood that, then just > > put your schema with your tool. Only users of the tool should care about > > the tool's schema. > > OK. I believe that binman will fit into both camps, since its input is > not necessarily fully formed. E.g. if you don't specify the offset of > an entry, then it will be packed automatically. But the output is > fully formed, in that Binman now knows the offset so can write it to > the DT. I suppose it could take its own format as input and then write out something different for the "on the device" format (i.e. fixed-partitions). At least for the dynamic offsets, we may need something allowed for binman input, but not allowed on device. In general, there is support for partitions without addresses/offsets, but only for partitions that have some other way to figure that out (on disk partition info). There's also the image filename which doesn't really belong in the on device partitions. So maybe the input and output schemas should be separate. > > > I saw this file, which seems to extend a partition. > > > > > > Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml > > > > IIRC, that's a different type where partition locations are stored in > > the flash, so we don't need location and size in DT. > > OK. > > > > > > > > > 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. > 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. We already have some compatibles in use. We should reuse them if possible. Not sure about TF-A though. Rob
Re: [PATCH] dt-bindings: mtd: Add a schema for binman
On Fri, Sep 22, 2023 at 11:01:18AM -0600, Simon Glass wrote: > Hi Rob, > > On Fri, 22 Sept 2023 at 10:00, Rob Herring wrote: > > > > On Thu, Sep 21, 2023 at 1:45 PM Simon Glass wrote: > > > > > > Binman[1] is a tool for creating firmware images. It allows you to > > > combine various binaries and place them in an output file. > > > > > > Binman uses a DT schema to describe an image, in enough detail that > > > it can be automatically built from component parts, disassembled, > > > replaced, listed, etc. > > > > > > Images are typically stored in flash, which is why this binding is > > > targeted at mtd. Previous discussion is at [2] [3]. > > > > > > [1] https://u-boot.readthedocs.io/en/stable/develop/package/binman.html > > > [2] > > > https://lore.kernel.org/u-boot/20230821180220.2724080-3-...@chromium.org/ > > > [3] https://www.spinics.net/lists/devicetree/msg626149.html > > > > You missed: > > > > https://github.com/devicetree-org/dt-schema/pull/110 > > > > where I said: We certainly shouldn't duplicate the existing partitions > > bindings. What's missing from them (I assume we're mostly talking > > about "fixed-partitions" which has been around forever I think (before > > me))? > > > > To repeat, unless there is some reason binman partitions conflict with > > fixed-partitions, you need to start there and extend it. From what's > > posted here, it neither conflicts nor needs extending. > > I think at this point I am just hopelessly confused. Have you taken a > look at the binman schema? [1] Why do I need to? That's used for some tool and has nothing to do with a device's DTB. However, I thought somewhere in this discussion you showed it under a flash device node. Then I care because then it overlaps with what we already have for partitions. If I misunderstood that, then just put your schema with your tool. Only users of the tool should care about the tool's schema. > > I saw this file, which seems to extend a partition. > > Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml IIRC, that's a different type where partition locations are stored in the flash, so we don't need location and size in DT. > > 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. Top-level, meaning the root of the DT? That sound like just something for the tool, so I don't care, but it doesn't belong in the DTB. > > Re extending, what is the minimum I can do? Are you looking for > something like a "compress" property that indicates that the entry is > compressed? > > I'm really just a bit lost. Me too. Rob
Re: [PATCH] dt-bindings: mtd: Add a schema for binman
On Thu, Sep 21, 2023 at 1:45 PM Simon Glass wrote: > > Binman[1] is a tool for creating firmware images. It allows you to > combine various binaries and place them in an output file. > > Binman uses a DT schema to describe an image, in enough detail that > it can be automatically built from component parts, disassembled, > replaced, listed, etc. > > Images are typically stored in flash, which is why this binding is > targeted at mtd. Previous discussion is at [2] [3]. > > [1] https://u-boot.readthedocs.io/en/stable/develop/package/binman.html > [2] https://lore.kernel.org/u-boot/20230821180220.2724080-3-...@chromium.org/ > [3] https://www.spinics.net/lists/devicetree/msg626149.html You missed: https://github.com/devicetree-org/dt-schema/pull/110 where I said: We certainly shouldn't duplicate the existing partitions bindings. What's missing from them (I assume we're mostly talking about "fixed-partitions" which has been around forever I think (before me))? To repeat, unless there is some reason binman partitions conflict with fixed-partitions, you need to start there and extend it. From what's posted here, it neither conflicts nor needs extending. I did a bit more research. "fixed-partitions" as a compatible has "only" been around since 2015. Prior to that, it was implicit with just partition nodes with addresses (i.e. reg) and that dates back to 2007. Looks like u-boot only supports the newer form and since 2021. Rob
Re: [PATCH v6 1/2] schemas: Add some common reserved-memory usages
On Thu, Sep 21, 2023 at 9:38 AM Simon Glass wrote: > > Hi Rob, > > On Thu, 21 Sept 2023 at 08:25, Rob Herring wrote: > > > > On Thu, Sep 7, 2023 at 4:40 PM Simon Glass wrote: > > > > > > It is common to split firmware into 'Platform Init', which does the > > > initial hardware setup and a "Payload" which selects the OS to be booted. > > > Thus an handover interface is required between these two pieces. > > > > > > This aims to provide an small schema addition for the memory mapping > > > needed to keep these two pieces working together well. > > > > > > Signed-off-by: Simon Glass > > > --- > > > > > > Changes in v6: > > > - Drop mention of UEFI > > > - Use compatible strings instead of node names > > > > > > Changes in v5: > > > - Drop the memory-map node (should have done that in v4) > > > - Tidy up schema a bit > > > > > > Changes in v4: > > > - Make use of the reserved-memory node instead of creating a new one > > > > > > Changes in v3: > > > - Reword commit message again > > > - cc a lot more people, from the FFI patch > > > - Split out the attributes into the /memory nodes > > > > > > Changes in v2: > > > - Reword commit message > > > > > > .../reserved-memory/common-reserved.yaml | 71 +++ > > > 1 file changed, 71 insertions(+) > > > create mode 100644 dtschema/schemas/reserved-memory/common-reserved.yaml > > > > > > diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml > > > b/dtschema/schemas/reserved-memory/common-reserved.yaml > > > new file mode 100644 > > > index 000..4889f59 > > > --- /dev/null > > > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml > > > @@ -0,0 +1,71 @@ > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/reserved-memory/common-reserved.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Common memory reservations > > > + > > > +description: | > > > + Specifies that the reserved memory region can be used for the purpose > > > + indicated by its compatible string. > > > + > > > + Clients may reuse this reserved memory if they understand what it is > > > for, > > > + subject to the notes below. > > > + > > > +maintainers: > > > + - Simon Glass > > > + > > > +allOf: > > > + - $ref: reserved-memory.yaml > > > + > > > +properties: > > > + compatible: > > > +description: | > > > + This describes some common memory reservations: > > > + > > > + acpi-reclaim: Contains ACPI tables; memory may be reclaimed > > > when the > > > + tables are no-longer needed > > > > I think you are mixing 2 things with the name here. What the memory > > contains and what to do with it. You don't need the latter. The > > consumer of the region will know what to do with it if anything based > > on knowing what is in the region. For example, The DTB passed to the > > OS is typically in a reserved region (probably still /mem-reserve/ > > though). The DTB may remain there forever or the OS could copy it > > somewhere else and free the reserved region. The Linux kernel does > > both depending on the arch. (Of course there is no "dtb" compatible > > because we have to pass the location of the dtb to even find the > > reserved regions in the first place.) > > > > So the question here is whether just "acpi" (or "acpi-tables"?) would > > be explicit enough? > > Yes acpi-tables would be enough. > > > > > > + acpi-nvs: Contains ACPI Non-volatile-storage data; memory may be > > > + reclaimed when the tables are no-longer needed > > > > No need to say anything about reclaiming. > > OK...so what about all that discussion about being able to reclaim the > memory if you know what it is for? Where should that be written? Or is > it somewhere else already? Reclaiming is a policy of the consumer of the data. It belongs in the documentation of the consumer if you are going to document it. The global policy is you can't use reserved regions and you can't reclaim them if you don't know what they are. If you want to add something to that effect in reserved-mem
Re: [PATCH v6 1/2] schemas: Add some common reserved-memory usages
On Thu, Sep 7, 2023 at 4:40 PM Simon Glass wrote: > > It is common to split firmware into 'Platform Init', which does the > initial hardware setup and a "Payload" which selects the OS to be booted. > Thus an handover interface is required between these two pieces. > > This aims to provide an small schema addition for the memory mapping > needed to keep these two pieces working together well. > > Signed-off-by: Simon Glass > --- > > Changes in v6: > - Drop mention of UEFI > - Use compatible strings instead of node names > > Changes in v5: > - Drop the memory-map node (should have done that in v4) > - Tidy up schema a bit > > Changes in v4: > - Make use of the reserved-memory node instead of creating a new one > > Changes in v3: > - Reword commit message again > - cc a lot more people, from the FFI patch > - Split out the attributes into the /memory nodes > > Changes in v2: > - Reword commit message > > .../reserved-memory/common-reserved.yaml | 71 +++ > 1 file changed, 71 insertions(+) > create mode 100644 dtschema/schemas/reserved-memory/common-reserved.yaml > > diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml > b/dtschema/schemas/reserved-memory/common-reserved.yaml > new file mode 100644 > index 000..4889f59 > --- /dev/null > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml > @@ -0,0 +1,71 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/reserved-memory/common-reserved.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Common memory reservations > + > +description: | > + Specifies that the reserved memory region can be used for the purpose > + indicated by its compatible string. > + > + Clients may reuse this reserved memory if they understand what it is for, > + subject to the notes below. > + > +maintainers: > + - Simon Glass > + > +allOf: > + - $ref: reserved-memory.yaml > + > +properties: > + compatible: > +description: | > + This describes some common memory reservations: > + > + acpi-reclaim: Contains ACPI tables; memory may be reclaimed when the > + tables are no-longer needed I think you are mixing 2 things with the name here. What the memory contains and what to do with it. You don't need the latter. The consumer of the region will know what to do with it if anything based on knowing what is in the region. For example, The DTB passed to the OS is typically in a reserved region (probably still /mem-reserve/ though). The DTB may remain there forever or the OS could copy it somewhere else and free the reserved region. The Linux kernel does both depending on the arch. (Of course there is no "dtb" compatible because we have to pass the location of the dtb to even find the reserved regions in the first place.) So the question here is whether just "acpi" (or "acpi-tables"?) would be explicit enough? > + acpi-nvs: Contains ACPI Non-volatile-storage data; memory may be > + reclaimed when the tables are no-longer needed No need to say anything about reclaiming. I know some ACPIisms (e.g. DSDT), but I don't know what NVS or "Non-volatile-storage data" is in an ACPI context. > + boot-code: Contains code used for booting; memory may be reclaimed > by > + the OS when it is running > + boot-code: Contains data used for booting; memory may be reclaimed > by boot-data? > + the OS when it is running I thought these were for stages before we get to OS? > + runtime-code: Contains code used for interacting with the system > when > + running; memory may be reclaimed if this code is not called > + runtime-data: Contains data used for interacting with the system > when > + running; memory may be reclaimed if the runtime code is not used "boot" vs. "runtime" seem too vague. However, if these mean EFI boot time services and runtime services, then I understand exactly what they are. In that case dropping 'uefi,' was a mistake. But EFI has its own way to define these regions, right? Rob
Re: [RFC PATCH 0/5] Allow for removal of DT nodes and properties
On Tue, Sep 19, 2023 at 3:26 PM Simon Glass wrote: > > Hi Rob, > > On Mon, 18 Sept 2023 at 11:00, Rob Herring wrote: > > > > On Thu, Sep 14, 2023 at 5:42 PM Simon Glass wrote: > > > > > > Hi Rob, > > > > > > On Wed, 13 Sept 2023 at 16:39, Rob Herring wrote: > > > > > > > > On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis > > > > wrote: > > > > > > > > > > > From: Jassi Brar > > > > > > Date: Fri, 8 Sep 2023 09:37:12 -0500 > > > > > > > > > > > > Hi Simon, > > > > > > > > > > > > On Thu, Sep 7, 2023 at 3:08 PM Simon Glass > > > > > > wrote: > > > > > > > On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas > > > > > > > > > > > > > > > > > > I beg to differ. Devicetree is more than just hardware and > > > > > > > > > > always has > > > > > > > > > > been. See, for example the /chosen and /options nodes. > > > > > > > > > > > > > > > > > > There are exceptions... > > > > > > > > > > > > > > > > > > > > > > > > > We've been this over and over again and frankly it gets a bit > > > > > > > > annoying. > > > > > > > > It's called *DEVICE* tree for a reason. As Rob pointed out > > > > > > > > there are > > > > > > > > exceptions, but those made a lot of sense. Having arbitrary > > > > > > > > internal ABI > > > > > > > > stuff of various projects in the schema just defeats the > > > > > > > > definition of a > > > > > > > > spec. > > > > > > > > > > > > > > Our efforts should not just be about internal ABI, but working to > > > > > > > provide a consistent configuration system for all firmware > > > > > > > elements. > > > > > > > > > > > > > Sure, programmatically we can pass any data/info via DT, however it > > > > > > is > > > > > > only meant to map hardware features onto data structures. > > > > > > > > > > > > devicetree.org landing page > > > > > > "The devicetree is a data structure for describing hardware." > > > > > > > > > > > > devicetree-specification-v0.3.pdf Chapter-2 Line-1 > > > > > >"DTSpec specifies a construct called a devicetree to describe > > > > > > system hardware." > > > > > > > > > > But it doesn't say that it describes *only* hardware. And it does > > > > > describe more than just hardware. There is /chosen to specify > > > > > firmware configuration and there are several examples of device tree > > > > > bindings that describe non-discoverable firmware interfaces in the > > > > > Linux tree. And it has been that way since the days of IEEE 1275. > > > > > There are also bindings describing things like the firmware partition > > > > > layout. > > > > > > > > Yes. The exact title for 1275[1] is: IEEE Standard for Boot > > > > (Initialization Configuration) > > > > Firmware: Core Requirements and Practices > > > > > > > > I see "configuration" in there. However, in the OF case, it's > > > > generally how firmware configured the hardware and what it discovered. > > > > That's a little different than telling the OS how to configure the > > > > hardware which is what we do with FDT. > > > > > > For the /options node it says "The properties of this node are the > > > system’s configuration variables." > > > > > > Then there is section 7.4.4 which has a large list of options which > > > don't seem to be so narrowly defined. > > > > > > In any case, this is not quite the point, which IMO is that we need DT > > > to support firmware use cases, whether or not a 29-year-old spec > > > thought of it or not. In fact it is amazing to me how forward-looking > > > Open Firmware was. > > > > > > > Then there's stuff that's how > > > > to configure Linux which we try to reject. > > > > > > Fair
Re: [RFC PATCH 0/5] Allow for removal of DT nodes and properties
On Thu, Sep 14, 2023 at 5:42 PM Simon Glass wrote: > > Hi Rob, > > On Wed, 13 Sept 2023 at 16:39, Rob Herring wrote: > > > > On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis > > wrote: > > > > > > > From: Jassi Brar > > > > Date: Fri, 8 Sep 2023 09:37:12 -0500 > > > > > > > > Hi Simon, > > > > > > > > On Thu, Sep 7, 2023 at 3:08 PM Simon Glass wrote: > > > > > On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas > > > > > > > > > > > > > > I beg to differ. Devicetree is more than just hardware and > > > > > > > > always has > > > > > > > > been. See, for example the /chosen and /options nodes. > > > > > > > > > > > > > > There are exceptions... > > > > > > > > > > > > > > > > > > > We've been this over and over again and frankly it gets a bit > > > > > > annoying. > > > > > > It's called *DEVICE* tree for a reason. As Rob pointed out there > > > > > > are > > > > > > exceptions, but those made a lot of sense. Having arbitrary > > > > > > internal ABI > > > > > > stuff of various projects in the schema just defeats the definition > > > > > > of a > > > > > > spec. > > > > > > > > > > Our efforts should not just be about internal ABI, but working to > > > > > provide a consistent configuration system for all firmware elements. > > > > > > > > > Sure, programmatically we can pass any data/info via DT, however it is > > > > only meant to map hardware features onto data structures. > > > > > > > > devicetree.org landing page > > > > "The devicetree is a data structure for describing hardware." > > > > > > > > devicetree-specification-v0.3.pdf Chapter-2 Line-1 > > > >"DTSpec specifies a construct called a devicetree to describe > > > > system hardware." > > > > > > But it doesn't say that it describes *only* hardware. And it does > > > describe more than just hardware. There is /chosen to specify > > > firmware configuration and there are several examples of device tree > > > bindings that describe non-discoverable firmware interfaces in the > > > Linux tree. And it has been that way since the days of IEEE 1275. > > > There are also bindings describing things like the firmware partition > > > layout. > > > > Yes. The exact title for 1275[1] is: IEEE Standard for Boot > > (Initialization Configuration) > > Firmware: Core Requirements and Practices > > > > I see "configuration" in there. However, in the OF case, it's > > generally how firmware configured the hardware and what it discovered. > > That's a little different than telling the OS how to configure the > > hardware which is what we do with FDT. > > For the /options node it says "The properties of this node are the > system’s configuration variables." > > Then there is section 7.4.4 which has a large list of options which > don't seem to be so narrowly defined. > > In any case, this is not quite the point, which IMO is that we need DT > to support firmware use cases, whether or not a 29-year-old spec > thought of it or not. In fact it is amazing to me how forward-looking > Open Firmware was. > > > Then there's stuff that's how > > to configure Linux which we try to reject. > > Fair enough: Linux has user-space for that. Yep, the question I usually ask is who needs to configure something and what determines the config. Changing things with sysfs is much easier than changing the DT provided by firmware. > > > > Once we get into configuration of the OS/client (including u-boot), > > making that an ABI is a lot harder and if we use DT for that, I don't > > think it should be an ABI. > > > > > > If we want to digress from the spec, we need the majority of > > > > developers to switch sides :) which is unlikely to happen and rightly > > > > so, imho. > > > > > > It isn't even clear what the spec is. There is the document you > > > reference above, there are the yaml files at > > > https://github.com/devicetree-org/dt-schema and then there are > > > additional yaml files in the Linux tree. As far as I know it isn't > > > written down anywhere that those are the only places where device tre
Re: [RFC PATCH 0/5] Allow for removal of DT nodes and properties
On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis wrote: > > > From: Jassi Brar > > Date: Fri, 8 Sep 2023 09:37:12 -0500 > > > > Hi Simon, > > > > On Thu, Sep 7, 2023 at 3:08 PM Simon Glass wrote: > > > On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas > > > > > > > > > > I beg to differ. Devicetree is more than just hardware and always > > > > > > has > > > > > > been. See, for example the /chosen and /options nodes. > > > > > > > > > > There are exceptions... > > > > > > > > > > > > > We've been this over and over again and frankly it gets a bit annoying. > > > > It's called *DEVICE* tree for a reason. As Rob pointed out there are > > > > exceptions, but those made a lot of sense. Having arbitrary internal > > > > ABI > > > > stuff of various projects in the schema just defeats the definition of a > > > > spec. > > > > > > Our efforts should not just be about internal ABI, but working to > > > provide a consistent configuration system for all firmware elements. > > > > > Sure, programmatically we can pass any data/info via DT, however it is > > only meant to map hardware features onto data structures. > > > > devicetree.org landing page > > "The devicetree is a data structure for describing hardware." > > > > devicetree-specification-v0.3.pdf Chapter-2 Line-1 > >"DTSpec specifies a construct called a devicetree to describe > > system hardware." > > But it doesn't say that it describes *only* hardware. And it does > describe more than just hardware. There is /chosen to specify > firmware configuration and there are several examples of device tree > bindings that describe non-discoverable firmware interfaces in the > Linux tree. And it has been that way since the days of IEEE 1275. > There are also bindings describing things like the firmware partition > layout. Yes. The exact title for 1275[1] is: IEEE Standard for Boot (Initialization Configuration) Firmware: Core Requirements and Practices I see "configuration" in there. However, in the OF case, it's generally how firmware configured the hardware and what it discovered. That's a little different than telling the OS how to configure the hardware which is what we do with FDT. Then there's stuff that's how to configure Linux which we try to reject. Once we get into configuration of the OS/client (including u-boot), making that an ABI is a lot harder and if we use DT for that, I don't think it should be an ABI. > > If we want to digress from the spec, we need the majority of > > developers to switch sides :) which is unlikely to happen and rightly > > so, imho. > > It isn't even clear what the spec is. There is the document you > reference above, there are the yaml files at > https://github.com/devicetree-org/dt-schema and then there are > additional yaml files in the Linux tree. As far as I know it isn't > written down anywhere that those are the only places where device tree > bindings can live. There's not any restriction. My intention with dtschema schemas is to only have "spec level" schemas. (Stuff we'd add to DTSpec (but don't because no one wants to write specs).) Hardware specific stuff lives somewhere else. That happens to be the Linux tree because that is where all the h/w support is (nothing else is close (by far)). Last I checked, we've got ~3700 schemas and ~1500 unconverted bindings. > Anyway, let's face it, there is some consensus among developers that > what Simon has done in U-Boot is pushing the use of devicetree beyond > the point where a significant fraction of developers thinks it makes > sense. And I think I agree with that. But you can't beat him with > the spec to make your point. > > Now the devicetree is cleverly constructed such that it is possible to > define additional bindings without the risk of conflicting with > bindings developed by other parties. In particular if U-Boot is > augmenting a device tree with properties that are prefixed with > "u-boot," this isn't going to hurt an operating system that uses such > an augmented device tree. Until someone has some great idea to start using them. If the OS doesn't need something, then why pass it in the first place? What purpose does it serve? It's an invitation for someone somewhere to start using them. The downside of keeping the nodes is it creates an ABI where there doesn't need to be one necessarily. > The real problem is that some folks developed a certification program > that allegedly requires schema verification and now propose adding > code to U-Boot that doesn't really solve any problem. My proposed > solution would be to change said certification program to allow > firmware to augment the device tree with properties and nodes with > compatibles that are in the namespace controlled by the firmware. I don't think we should decide what to do here based on said certification program. It can adapt to what's decided. Probably, the /option nodes will be stripped out or ignored for certification. I accepted u-boot options node schema into
Re: [PATCH v6 2/2] schemas: memory: Add ECC properties
On Thu, Sep 7, 2023 at 4:40 PM Simon Glass wrote: > > Some memories provide ECC detection and/or correction. For software which > wants to check memory, it is helpful to see which regions provide this > feature. > > Add this as a property of the /memory nodes, since it presumably follows > the hardware-level memory system. > > Signed-off-by: Simon Glass > --- > > Changes in v6: > - Use a number of bits instead of a string property > - Fix inidcates typo > > Changes in v5: > - Redo to make this property specific to ECC > - Provide properties both for detection and correction > > Changes in v3: > - Add new patch to update the /memory nodes > > dtschema/schemas/memory.yaml | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/dtschema/schemas/memory.yaml b/dtschema/schemas/memory.yaml > index 1d74410..1a48b1c 100644 > --- a/dtschema/schemas/memory.yaml > +++ b/dtschema/schemas/memory.yaml > @@ -31,10 +31,21 @@ patternProperties: > >numa-node-id: > $ref: types.yaml#/definitions/uint32 > -description: > +description: | Why? '|' is not needed for any of these. >For the purpose of identification, each NUMA node is associated > with >a unique token known as a node id. > - blank line between properties. I can fix these up when applying. > + ecc-detection-bits: > +default: 0 > +description: | > + If present, this indicates the number of bits of memory error which > + can be detected and reported by the Error-Correction Code (ECC) > memory > + subsystem (typically 0, 1 or 2). > + ecc-correction-bits: > +default: 0 > +description: | > + If present, this indicates the number of bits of memory error which > + can be corrected by the Error-Correction Code (ECC) memory > subsystem > + (typically 0, 1 or 2). > > required: >- device_type > -- > 2.42.0.283.g2d96d420d3-goog >
Re: [PATCH v5 4/4] memory: Add ECC properties
On Wed, Aug 30, 2023 at 6:18 PM Simon Glass wrote: > > Some memories provide ECC detection and/or correction. For software which > wants to check memory, it is helpful to see which regions provide this > feature. > > Add this as a property of the /memory nodes, since it presumably follows > the hardware-level memory system. > > Signed-off-by: Simon Glass > --- > > Changes in v5: > - Redo to make this property specific to ECC > - Provide properties both for detection and correction > > Changes in v3: > - Add new patch to update the /memory nodes > > dtschema/schemas/memory.yaml | 30 ++ > 1 file changed, 30 insertions(+) > > diff --git a/dtschema/schemas/memory.yaml b/dtschema/schemas/memory.yaml > index 1d74410..944aa9f 100644 > --- a/dtschema/schemas/memory.yaml > +++ b/dtschema/schemas/memory.yaml > @@ -34,7 +34,37 @@ patternProperties: > description: >For the purpose of identification, each NUMA node is associated > with >a unique token known as a node id. > + ecc-detection: > +$ref: /schemas/types.yaml#/definitions/string > +enum: > + - none > + - single-bit > + - multi-bit > +description: | > + If present, this inidcates the type of memory errors which can be typo > + detected and reported by the Error-Correction Code (ECC) memory > + subsystem: > > +none - No error detection is possible > +single-bit - Detects and reports single-bit ECC errors > +multi-bit - Detects and reports multiple-bit ECC errors I don't think 'multi' is specific enough. Perhaps this should be an int instead with how many bits. (And '-bits' is a standard unit suffix so a type isn't needed) > + > + If not present, this is equivalent to 'none'. Can be expressed as schema: default: none Though if that's the default why have it as a value? (It's fine though) > + ecc-correction: > +$ref: /schemas/types.yaml#/definitions/string > +enum: > + - none > + - single-bit > + - multi-bit > +description: | > + If present, this inidcates the type of memory errors which can be typo > + corrected by the Error-Correction Code (ECC) memory subsystem: > + > +none - No error correction is possible > +single-bit - Corrects single-bit ECC errors > +multi-bit - Corrects multiple-bit ECC errors > + > + If not present, this is equivalent to 'none'. One issue is with 2 properties nonsensical combinations are allowed. Not really any way to handle that in the schema though. Rob
Re: [PATCH v5 1/4] Add reserved-memory
On Wed, Aug 30, 2023 at 6:18 PM Simon Glass wrote: > > Bring in this file from Linux v6.5 > > Signed-off-by: Simon Glass > --- > > (no changes since v4) > > Changes in v4: > - New patch > > .../reserved-memory/reserved-memory.yaml | 181 ++ > 1 file changed, 181 insertions(+) > create mode 100644 dtschema/schemas/reserved-memory/reserved-memory.yaml I've applied this one and patch 2. Rob
Re: [PATCH v5 3/4] schemas: Add some common reserved-memory usages
On Thu, Sep 7, 2023 at 8:56 AM Simon Glass wrote: > > Hi Ard, > > On Thu, 7 Sept 2023 at 07:31, Ard Biesheuvel wrote: > > > > On Wed, 6 Sept 2023 at 18:50, Simon Glass wrote: > > > > > > Hi Ard, > > > > > > On Wed, Sep 6, 2023, 10:09 Ard Biesheuvel wrote: > > >> > > >> On Wed, 6 Sept 2023 at 16:54, Simon Glass wrote: > > >> > > > >> > Hi Rob, Ard, > > >> > > > >> > On Wed, 6 Sept 2023 at 08:34, Rob Herring wrote: > > >> > > > > >> > > On Tue, Sep 5, 2023 at 4:44 PM Ard Biesheuvel > > >> > > wrote: > > >> > > > > > >> > > > On Thu, 31 Aug 2023 at 01:18, Simon Glass > > >> > > > wrote: > > >> > > > > > > >> > > > > The Devicetree specification skips over handling of a logical > > >> > > > > view of > > >> > > > > the memory map, pointing users to the UEFI specification. > > >> > > > > > > >> > > > > It is common to split firmware into 'Platform Init', which does > > >> > > > > the > > >> > > > > initial hardware setup and a "Payload" which selects the OS to > > >> > > > > be booted. > > >> > > > > Thus an handover interface is required between these two pieces. > > >> > > > > > > >> > > > > Where UEFI boot-time services are not available, but UEFI > > >> > > > > firmware is > > >> > > > > present on either side of this interface, information about > > >> > > > > memory usage > > >> > > > > and attributes must be presented to the "Payload" in some form. > > >> > > > > > > >> > > > > > >> > > > I don't think the UEFI references are needed or helpful here. > > >> > > > > > >> > > > > This aims to provide an small schema addition for this mapping. > > >> > > > > > > >> > > > > For now, no attempt is made to create an exhaustive binding, so > > >> > > > > there are > > >> > > > > some example types listed. More can be added later. > > >> > > > > > > >> > > > > The compatible string is not included, since the node name is > > >> > > > > enough to > > >> > > > > indicate the purpose of a node, as per the existing > > >> > > > > reserved-memory > > >> > > > > schema. > > >> > > > > >> > > Node names reflect the 'class', but not what's specifically in the > > >> > > node. So really, all reserved-memory nodes should have the same name, > > >> > > but that ship already sailed for existing users. 'compatible' is the > > >> > > right thing here. As to what the node name should be, well, we > > >> > > haven't > > >> > > defined that. I think we just used 'memory' on some platforms. > > >> > > > >> > OK > > >> > > > >> > > > > >> > > > > This binding does not include a binding for the memory > > >> > > > > 'attribute' > > >> > > > > property, defined by EFI_BOOT_SERVICES.GetMemoryMap(). It may be > > >> > > > > useful > > >> > > > > to have that as well, but perhaps not as a bit mask. > > >> > > > > > > >> > > > > Signed-off-by: Simon Glass > > >> > > > > --- > > >> > > > > > > >> > > > > Changes in v5: > > >> > > > > - Drop the memory-map node (should have done that in v4) > > >> > > > > - Tidy up schema a bit > > >> > > > > > > >> > > > > Changes in v4: > > >> > > > > - Make use of the reserved-memory node instead of creating a new > > >> > > > > one > > >> > > > > > > >> > > > > Changes in v3: > > >> > > > > - Reword commit message again > > >> > > > > - cc a lot more people, from the FFI patch > > >> >
Re: [PATCH v5 3/4] schemas: Add some common reserved-memory usages
On Tue, Sep 5, 2023 at 4:44 PM Ard Biesheuvel wrote: > > On Thu, 31 Aug 2023 at 01:18, Simon Glass wrote: > > > > The Devicetree specification skips over handling of a logical view of > > the memory map, pointing users to the UEFI specification. > > > > It is common to split firmware into 'Platform Init', which does the > > initial hardware setup and a "Payload" which selects the OS to be booted. > > Thus an handover interface is required between these two pieces. > > > > Where UEFI boot-time services are not available, but UEFI firmware is > > present on either side of this interface, information about memory usage > > and attributes must be presented to the "Payload" in some form. > > > > I don't think the UEFI references are needed or helpful here. > > > This aims to provide an small schema addition for this mapping. > > > > For now, no attempt is made to create an exhaustive binding, so there are > > some example types listed. More can be added later. > > > > The compatible string is not included, since the node name is enough to > > indicate the purpose of a node, as per the existing reserved-memory > > schema. Node names reflect the 'class', but not what's specifically in the node. So really, all reserved-memory nodes should have the same name, but that ship already sailed for existing users. 'compatible' is the right thing here. As to what the node name should be, well, we haven't defined that. I think we just used 'memory' on some platforms. > > This binding does not include a binding for the memory 'attribute' > > property, defined by EFI_BOOT_SERVICES.GetMemoryMap(). It may be useful > > to have that as well, but perhaps not as a bit mask. > > > > Signed-off-by: Simon Glass > > --- > > > > Changes in v5: > > - Drop the memory-map node (should have done that in v4) > > - Tidy up schema a bit > > > > Changes in v4: > > - Make use of the reserved-memory node instead of creating a new one > > > > Changes in v3: > > - Reword commit message again > > - cc a lot more people, from the FFI patch > > - Split out the attributes into the /memory nodes > > > > Changes in v2: > > - Reword commit message > > > > .../reserved-memory/common-reserved.yaml | 53 +++ > > 1 file changed, 53 insertions(+) > > create mode 100644 dtschema/schemas/reserved-memory/common-reserved.yaml > > > > diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml > > b/dtschema/schemas/reserved-memory/common-reserved.yaml > > new file mode 100644 > > index 000..d1b466b > > --- /dev/null > > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml > > @@ -0,0 +1,53 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/reserved-memory/common-reserved.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Common memory reservations > > + > > +description: | > > + Specifies that the reserved memory region can be used for the purpose > > + indicated by its node name. > > + > > + Clients may reuse this reserved memory if they understand what it is for. > > + > > +maintainers: > > + - Simon Glass > > + > > +allOf: > > + - $ref: reserved-memory.yaml > > + > > +properties: > > + $nodename: > > +enum: > > + - acpi-reclaim > > + - acpi-nvs > > + - boot-code > > + - boot-data > > + - runtime-code > > + - runtime-data > > + > > These types are used by firmware to describe the nature of certain > memory regions to the OS. Boot code and data can be discarded, as well > as ACPI reclaim after its contents have been consumed. Runtime code > and data need to be mapped for runtime features to work. > > When one firmware phase communicates the purpose of a certain memory > reservation to another, it is typically not limited to whether its > needs to be preserved and when it needs to be mapped (and with which > attributes). I'd expect a memory reservation appearing under this node > to have a clearly defined purpose, and the subsequent phases need to > be able to discover this information. > > For example, a communication buffer for secure<->non-secure > communication or a page with spin tables used by PSCI. None of the > proposed labels are appropriate for this, and I'd much rather have a > compatible string or some other property that clarifies the nature in > a more suitable way. Note that 'no-map' already exists to indicate > that the CPU should not map this memory unless it does so for the > specific purpose that the reservation was made for. I agree. I think compatible is the better approach. Some property like 'discard' may not be sufficient information if the OS needs to consume the region first and then discard it. Better to state exactly what's there and then the OS can imply the rest. Rob
Re: [RFC PATCH 0/5] Allow for removal of DT nodes and properties
On Mon, Aug 28, 2023 at 04:09:29PM -0600, Simon Glass wrote: > Hi Peter, > > On Mon, 28 Aug 2023 at 14:29, Peter Robinson wrote: > > > > On Mon, Aug 28, 2023 at 6:54 PM Simon Glass wrote: > > > > > > Hi Peter, > > > > > > On Mon, 28 Aug 2023 at 10:37, Peter Robinson wrote: > > > > > > > > On Mon, Aug 28, 2023 at 5:20 PM Simon Glass wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu > > > > > wrote: > > > > > > > > > > > > > > > > > > Provide a way for removing certain devicetree nodes and/or > > > > > > properties > > > > > > from the devicetree. This is needed to purge certain nodes and > > > > > > properties which may be relevant only in U-Boot. Such nodes and > > > > > > properties are then removed from the devicetree before it is passed > > > > > > to > > > > > > the kernel. This ensures that the devicetree passed to the OS does > > > > > > not > > > > > > contain any non-compliant nodes and properties. > > > > > > > > > > > > The removal of the nodes and properties is being done through an > > > > > > EVT_FT_FIXUP handler. I am not sure if the removal code needs to be > > > > > > behind any Kconfig symbol. > > > > > > > > > > > > I have only build tested this on sandbox, and tested on qemu arm64 > > > > > > virt platform. This being a RFC, I have not put this through a CI > > > > > > run. > > > > > > > > > > > > Sughosh Ganu (5): > > > > > > dt: Provide a way to remove non-compliant nodes and properties > > > > > > fwu: Add the fwu-mdata node for removal from devicetree > > > > > > capsule: Add the capsule-key property for removal from devicetree > > > > > > bootefi: Call the EVT_FT_FIXUP event handler > > > > > > doc: Add a document for non-compliant DT node/property removal > > > > > > > > > > > > cmd/bootefi.c | 18 + > > > > > > .../devicetree/dt_non_compliant_purge.rst | 64 > > > > > > drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ > > > > > > include/dt-structs.h | 11 +++ > > > > > > lib/Makefile | 1 + > > > > > > lib/dt_purge.c| 73 > > > > > > +++ > > > > > > lib/efi_loader/efi_capsule.c | 7 ++ > > > > > > 7 files changed, 179 insertions(+) > > > > > > create mode 100644 > > > > > > doc/develop/devicetree/dt_non_compliant_purge.rst > > > > > > create mode 100644 lib/dt_purge.c > > > > > > > > > > What is the point of removing them? Instead, we should make sure that > > > > > we upstream the bindings and encourage SoC vendors to sync them. If we > > > > > remove them, no one will bother and U-Boot just becomes a dumping > > > > > ground. > > > > > > > > Well things like the binman entries in DT are U-Boot specific and not > > > > useful for HW related descriptions or for Linux or another OS being > > > > able to deal with HW so arguably we're already a dumping ground to > > > > some degree for not defining hardware. > > > > > > I have started the process to upstream the binman bindings. > > > > I don't think they should be in DT at all, they don't describe > > anything to do with hardware, or generally even the runtime of a > > device, they don't even describe the boot/runtime state of the > > firmware, they describe build time, so I don't see what that has to do > > with device tree? Can you explain that? To me those sorts of things > > should live in a build time style config file. For the record, I tend to agree. > I beg to differ. Devicetree is more than just hardware and always has > been. See, for example the /chosen and /options nodes. There are exceptions... > We need run-time configuration here, since we cannot know at build > time what we will be asked to do by a previous firmware phase. Really, I don't want to have to care about the binman binding. If it is u-boot specific, then it should stay in u-boot. I took /options/u-boot/, but now I'm starting to have second thoughts on that being in dtschema if it is going to be continually and frequently extended. Validating it in SR does little. If a vendor is abusing /options/u-boot/ in some way they could just as easily remove the node in their u-boot fork to pass. SR is certainly not going to require the node be there. On A/B updates, that really doesn't seem like a u-boot specific problem to me. No one wants A/B updates in EDK2 or anything else? Rob
Re: [PATCH v4 4/4] memory: Add ECC property
On Tue, Aug 29, 2023 at 2:18 PM Simon Glass wrote: > > Some memories provides ECC correction. For software which wants to check > memory, it is helpful to see which regions provide this feature. > > Add this as a property of the /memory nodes, since it presumably follows > the hardware-level memory system. > > Signed-off-by: Simon Glass > --- > > (no changes since v3) > > Changes in v3: > - Add new patch to update the /memory nodes > > dtschema/schemas/memory.yaml | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/dtschema/schemas/memory.yaml b/dtschema/schemas/memory.yaml > index 1d74410..981af04 100644 > --- a/dtschema/schemas/memory.yaml > +++ b/dtschema/schemas/memory.yaml > @@ -34,7 +34,14 @@ patternProperties: > description: >For the purpose of identification, each NUMA node is associated > with >a unique token known as a node id. > - > + attr: Kind of vague. > +$ref: /schemas/types.yaml#/definitions/string-array > +description: | > + Attributes possessed by this memory region: > + > +"single-bit-ecc" - supports single-bit ECC > +"multi-bit-ecc" - supports multiple-bit ECC "supports" means corrects or reports? Most h/w supports both, but only reports multi-bit errors. > +"no-ecc" - non-ECC memory Don't define values in free form text. This form is difficult to validate especially when non-ECC related attr's are added to the mix as we can't really define which combinations are valid. For example how do we prevent: attr = "single-bit-ecc", "multi-bit-ecc"; Or maybe that's valid? If so, how would we express that? Why do we need "no-ecc"? Is that the same as no "attr" property? I think it's better if we have 'ecc-type' or something? Or generally, a property per class/type of attribute. Rob
Re: [PATCH v4 2/4] Bring in other reserved-memory files
On Tue, Aug 29, 2023 at 2:18 PM Simon Glass wrote: > > Add those files from v6.5 which are already converted to yaml. > > Signed-off-by: Simon Glass > --- > > Changes in v4: > - New patch > > .../schemas/reserved-memory/framebuffer.yaml | 52 +++ > .../reserved-memory/google,open-dice.yaml | 46 ++ > .../reserved-memory/memory-region.yaml| 40 + > .../nvidia,tegra210-emc-table.yaml| 31 > .../nvidia,tegra264-bpmp-shmem.yaml | 47 ++ > dtschema/schemas/reserved-memory/phram.yaml | 47 ++ > .../schemas/reserved-memory/qcom,cmd-db.yaml | 46 ++ > .../reserved-memory/qcom,rmtfs-mem.yaml | 55 +++ > dtschema/schemas/reserved-memory/ramoops.yaml | 144 ++ > .../reserved-memory/shared-dma-pool.yaml | 97 > 10 files changed, 605 insertions(+) I don't think I want all of these in dtschema. Certainly not vendor specific ones. Generally, if it's not something we'd put in the spec, then I don't want it in dtschema. The u-boot stuff so far has been an exception. Probably a good rule is if it has a compatible, then it doesn't go into dtschema. Though there's some exceptions already. > create mode 100644 dtschema/schemas/reserved-memory/framebuffer.yaml Maybe > create mode 100644 dtschema/schemas/reserved-memory/google,open-dice.yaml No > create mode 100644 dtschema/schemas/reserved-memory/memory-region.yaml Yes > create mode 100644 > dtschema/schemas/reserved-memory/nvidia,tegra210-emc-table.yaml > create mode 100644 > dtschema/schemas/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml No > create mode 100644 dtschema/schemas/reserved-memory/phram.yaml Depends on mtd.yaml in the kernel, so no. > create mode 100644 dtschema/schemas/reserved-memory/qcom,cmd-db.yaml > create mode 100644 dtschema/schemas/reserved-memory/qcom,rmtfs-mem.yaml No. > create mode 100644 dtschema/schemas/reserved-memory/ramoops.yaml Probably not. > create mode 100644 dtschema/schemas/reserved-memory/shared-dma-pool.yaml Maybe. Rob
Re: [PATCH v2] schemas: Add a schema for memory map
On Tue, Aug 22, 2023 at 3:34 PM Simon Glass wrote:
>
> Hi Rob,
>
> On Tue, 22 Aug 2023 at 12:53, Rob Herring wrote:
> >
> > On Mon, Aug 21, 2023 at 2:48 PM Simon Glass wrote:
> > >
> > > The Devicespec specification skips over handling of a logical view of
> > > the memory map, pointing users to the UEFI specification.
> >
> > It's more that the DT spec defines what is not used with UEFI. If UEFI
> > covers more than the DT Spec defined, then we should look at that.
> >
> > I would look some into (IBM) PowerPC for any prior art in this area.
> > Unfortunately, not publicly documented other than any users.
>
> OK, but I'm not sure what you are looking for here. The DT (as
> currently specified) is an incomplete description of memory, for
> EFI-type firmware.
I thought this was for non-EFI based systems. Confused.
> I recall the ePAPR thing, but not much else. Any
> pointers?
ePAPR is the source of DT Spec. That was mainly FSL PPC, not IBM PPC.
There's something called SPAPR, but no public spec. Otherwise, it's
looking at arch/powerpc in the kernel.
> > > It is common to split firmware into 'Platform Init', which does the
> > > initial hardware setup and a "Payload" which selects the OS to be booted.
> > > Thus an handover interface is required between these two pieces.
> > >
> > > Where UEFI boot-time services are not available, but UEFI firmware is
> > > present on either side of this interface, information about memory usage
> > > and attributes must be presented to the "Payload" in some form.
> > >
> > > This aims to provide an initial schema for this mapping.
> > >
> > > Note that this is separate from the existing /memory and /reserved-memory
> > > nodes, since it is mostly concerned with what the memory is used for. It
> > > may cover only a small fraction of available memory, although it could be
> > > used to signal which area of memory has ECC.
> > >
> > > For now, no attempt is made to create an exhaustive binding, so there are
> > > some example types lists. This can be completed once this has passed
> > > initial review.
> >
> > I don't have much interest in picking this up unless there's some
> > wider agreement. From the previously referenced discussion[1], it
> > didn't seem like there was. But none of those folk are Cc'ed here.
>
> Yes, Ron pointed me to that...although it isn't quite the same thing.
> That is implementing a way to pass SMBIOS and ACPI tables through to
> Linux, right? But it does mention memory types, so I'll send a new
> version with those people on cc, in case they don't look at linux-acpi
> much.
Both are defining regions of memory to pass from one stage to the
next. Isn't that the same thing?
> But note, this is for *firmware* (on both sides of the interface).
> Whether it is useful for Linux is another question. But in any case,
> we should avoid having things in the DT which Linux cannot validate /
> parse.
Perhaps it is easiest if firmware removes its private stuff. You can
put whatever you want into a DT and I don't care if it's not an ABI
between the components. You may still want to document things and have
a schema for other reasons.
> > > ---
> > >
> > > Changes in v2:
> > > - Reword commit message
> > >
> > > dtschema/schemas/memory-map.yaml | 51
> > > 1 file changed, 51 insertions(+)
> > > create mode 100644 dtschema/schemas/memory-map.yaml
> > >
> > > diff --git a/dtschema/schemas/memory-map.yaml
> > > b/dtschema/schemas/memory-map.yaml
> > > new file mode 100644
> > > index 000..97e531e
> > > --- /dev/null
> > > +++ b/dtschema/schemas/memory-map.yaml
> > > @@ -0,0 +1,51 @@
> > > +# SPDX-License-Identifier: BSD-2-Clause
> > > +# Copyright 2023 Google LLC
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/memory-map.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: /memory-map nodes
> > > +description: |
> > > + Common properties always required in /memory-map nodes. These nodes are
> > > + intended to resolve the nonchalant clause 3.4.1 ("/memory node and
> > > UEFI")
> > > + in the Devicetree Specification.
> > > +
> > > +maintainers:
> > > + - Simon Glass
> > > +
> > > +properties:
> > > + $nodename:
> > > +const: '/'
> >
> > This goes in the root node?
>
>
Re: [PATCH v2] schemas: Add a schema for memory map
On Mon, Aug 21, 2023 at 2:48 PM Simon Glass wrote:
>
> The Devicespec specification skips over handling of a logical view of
> the memory map, pointing users to the UEFI specification.
It's more that the DT spec defines what is not used with UEFI. If UEFI
covers more than the DT Spec defined, then we should look at that.
I would look some into (IBM) PowerPC for any prior art in this area.
Unfortunately, not publicly documented other than any users.
> It is common to split firmware into 'Platform Init', which does the
> initial hardware setup and a "Payload" which selects the OS to be booted.
> Thus an handover interface is required between these two pieces.
>
> Where UEFI boot-time services are not available, but UEFI firmware is
> present on either side of this interface, information about memory usage
> and attributes must be presented to the "Payload" in some form.
>
> This aims to provide an initial schema for this mapping.
>
> Note that this is separate from the existing /memory and /reserved-memory
> nodes, since it is mostly concerned with what the memory is used for. It
> may cover only a small fraction of available memory, although it could be
> used to signal which area of memory has ECC.
>
> For now, no attempt is made to create an exhaustive binding, so there are
> some example types lists. This can be completed once this has passed
> initial review.
I don't have much interest in picking this up unless there's some
wider agreement. From the previously referenced discussion[1], it
didn't seem like there was. But none of those folk are Cc'ed here.
> ---
>
> Changes in v2:
> - Reword commit message
>
> dtschema/schemas/memory-map.yaml | 51
> 1 file changed, 51 insertions(+)
> create mode 100644 dtschema/schemas/memory-map.yaml
>
> diff --git a/dtschema/schemas/memory-map.yaml
> b/dtschema/schemas/memory-map.yaml
> new file mode 100644
> index 000..97e531e
> --- /dev/null
> +++ b/dtschema/schemas/memory-map.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: BSD-2-Clause
> +# Copyright 2023 Google LLC
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-map.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: /memory-map nodes
> +description: |
> + Common properties always required in /memory-map nodes. These nodes are
> + intended to resolve the nonchalant clause 3.4.1 ("/memory node and UEFI")
> + in the Devicetree Specification.
> +
> +maintainers:
> + - Simon Glass
> +
> +properties:
> + $nodename:
> +const: '/'
This goes in the root node?
> + usage:
> +$ref: /schemas/types.yaml#/definitions/string
> +description: |
> + Describes the usage of the memory region, e.g.:
> +
> +"acpi-reclaim", "acpi-nvs", "bootcode", "bootdata", "bootdata",
> +"runtime-code", "runtime-data"
Can't these be covered by reserved-memory? The client is free to
reclaim any regions if it knows what they are.
> + attr:
> +$ref: /schemas/types.yaml#/definitions/string-array
> +description: |
> + Attributes possessed by this memory region:
> +
> +"single-bit-ecc" - supports single-bit ECC
> +"multi-bit-ecc" - supports multiple-bit ECC
> +"no-ecc" - non-ECC memory
Isn't this pretty much a property of a memory region as a whole. IOW,
couldn't it just go into /memory node(s)?
> +
> +patternProperties:
> + "^([a-z][a-z0-9\\-]+@[0-9a-f]+)?$":
> +type: object
> +additionalProperties: false
> +
> +properties:
> + reg:
> +minItems: 1
> +maxItems: 1024
> +
> +required:
> + - reg
> +
> +additionalProperties: true
> +
> +...
> --
> 2.42.0.rc1.204.g551eb34607-goog
[1]
https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyun...@bytedance.com/
Re: [PATCH 1/2] schemas: Add firmware node schema
On Tue, Jul 11, 2023 at 3:18 PM Simon Glass wrote: > > Add a motivation and purpose for this new proposed node. > > Signed-off-by: Simon Glass > --- > > dtschema/schemas/firmware.yaml | 83 ++ > 1 file changed, 83 insertions(+) > create mode 100644 dtschema/schemas/firmware.yaml > > diff --git a/dtschema/schemas/firmware.yaml b/dtschema/schemas/firmware.yaml > new file mode 100644 > index 000..4439a70 > --- /dev/null > +++ b/dtschema/schemas/firmware.yaml > @@ -0,0 +1,83 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-clause > +# Copyright 2023 Google LLC > +# > + > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/firmware.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: /firmware Node > + > +maintainers: > + - Simon Glass > + > +description: | > + The '/firmware' node does not represent a real device, but serves as a > place > + for recording information about the main firmware used on the device, such > as > + a map of its contents. This is used by the Operating System (OS), user > space > + programs and possibly other firmware components. Data in the '/firmware' > node > + does not itself represent the hardware. > + > + Properties in this node should be common to (and used by) at least two > + firmware projects, such as U-Boot and TF-A. Project-specific subnodes can > be > + used for properties which are specific to a single project. > + > + Purpose of '/firmware' node > + --- > + > + Firmware has traditionally been fairly opaque to the OS, with the OS taking > + no interest in its contents, version, layout or how it might be updated. > This > + is less than ideal, since firmware is an important part of the system and > + visibility into its operation is every bit as important as visbility into > the > + OS and user-space programs within the system. > + > + The traditional approach has been to let firmware deal with firmware, and > the > + OS deal with everything else. Updating firmware has been handled by > firmware. > + For example, the UEFI spec defines a way for the OS to post a 'capsule' > which > + is discovered next time the system boots, permitting firmware updates. But > + firmware updates in firmware are highly problematic. They require a reboot > + and a sometimes-lengthy wait with a strange-looking interface unfamiliar > + to most users. It seems better to make the update as transparent as > possible > + to the user. As an example of that, ChromeOS has full knowledge of the > + firmware version and layout, updates it in the background from user space > and > + instantly selects the new firmware when the user reboots or logs out. Perhaps if OS based firmware updates are useful, then UEFI should gain that capability rather than inventing some way to do it with DT. Seems like a worthy goal, just needs wider review IMO. > + A common objection to considering the system holistically is that some > parts > + of the system are inaccessible to the OS, such as a secure enclave. However > + this does not preclude providing visibility into what is present in that > + enclave. Firmware-version information is still useful. Firmware updates are > + still needed and can still be initiated from user space. > + > + Another objection is that firmware should provide an interface to the OS, > + while keeping its structure private. This thinking is largely driven by > + extrapolating from how firmware has been handled in the 'BIOS' days. It's also the case that the OS may not have direct access to the h/w needed. > + It should be considered a degenerate case rather than the norm. As > complexity > + increases, it creates an artificial boundary between two pieces of the > whole. > + Mechanisms then need to be invented to cross this unnecessary chasm. An > + example of this is Intel's Dynamic Platform and Thermal Framework (DPTF), > + which consists of user-space, OS and firmware components all working > towards > + a shared goal. We need a standard description of these cross-system pieces. > + > + In order to 'teach the OS about firmware', we need a place to put this > + information. That is the purpose of this node. > + > + In an Open Source world the entire model of firmware needs to adjust to be > + more open, more visible and managed just like any other part of the system. > + The major goal is to standardise how firmware is presented to the OS and > user > + space, so that common utilities can be used to manage the entire system, > + including the firmware. For example, fwupd can look in this node for > + information on how to update the firmware, similar to how VBE works. [1] > + It is likely that other purposes will come to light over time. It's good we're documenting /firmware, but your use seems different to what's already in place. Generally, /firmware has been for providers which are implemented by firmware and are not on any
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
ity with old dtbs, existing > parsers may not be removable - which may greatly simplify > dt parsing code. In Linux, for example, checking for whether a hart > supports an extension becomes as simple as: > of_property_match_string(node, "riscv,isa-extensions", "zicbom") > > vendor extensions > = > > Compared to riscv,isa, this proposed scheme promotes vendor extensions, > oft touted as the strength of RISC-V, to first-class citizens. > At present, extensions are defined as meaning what the RISC-V ISA > specifications say they do. There is no realistic way of using that > interface to provide cross-platform definitions for what vendor > extensions mean. Vendor extensions may also have even less consistency > than RVI do in terms of versioning, or no care about backwards > compatibility. > The new property allows us to assign explicit meanings on a per vendor > extension basis, backed up by a description of their meanings. > > fin > === > > Create a new file to store the extension meanings and a new > riscv,isa-base property to replace the aspect of riscv,isa that is > not represented by the new property - the base ISA implemented by a hart. > > As a starting point, add properties for extensions currently used in > Linux. > > Finally, mark riscv,isa as deprecated, as removing support for it in > existing programs would be an ABI break. > > CC: Palmer Dabbelt > CC: Paul Walmsley > CC: Rob Herring > CC: Krzysztof Kozlowski > CC: Alistair Francis > CC: Andrew Jones > CC: Anup Patel > CC: Atish Patra > CC: Jessica Clarke > CC: Rick Chen > CC: Leo > CC: Oleksii > CC: linux-ri...@lists.infradead.org > CC: qemu-ri...@nongnu.org > CC: u-boot@lists.denx.de > CC: devicet...@vger.kernel.org > CC: linux-ker...@vger.kernel.org > Reviewed-by: Palmer Dabbelt > Acked-by: Palmer Dabbelt > Signed-off-by: Conor Dooley > --- > Changes in v3: > - Per Rob's suggestion, switch to an array of strings. Cuts down on the > size, compared to booleans. It has a standard mechanism for parsing > (you need to parse arrays of strings for compatibles). It still allows > for having a limited set of explicitly defined properties - so the > advantages over a free-form string still apply. > - Pick up Palmer's Ack and Review (although I expect that he will be the > one to apply this). > --- > .../devicetree/bindings/riscv/cpus.yaml | 43 ++- > .../devicetree/bindings/riscv/extensions.yaml | 245 ++ > 2 files changed, 265 insertions(+), 23 deletions(-) > create mode 100644 Documentation/devicetree/bindings/riscv/extensions.yaml > Reviewed-by: Rob Herring
Re: [PATCH v1] dt-bindings: riscv: deprecate riscv,isa
On Mon, Jun 12, 2023 at 3:23 PM Conor Dooley wrote:
>
> Rob,
> Before I press on with more versions...
>
> On Thu, Jun 08, 2023 at 08:30:28PM +0100, Conor Dooley wrote:
> > On Thu, Jun 08, 2023 at 01:15:37PM -0600, Rob Herring wrote:
> > > On Tue, May 30, 2023 at 03:12:12PM +0100, Conor Dooley wrote:
> > > > On Thu, May 18, 2023 at 10:42:34PM +0100, Conor Dooley wrote:
> > > > > On Thu, May 18, 2023 at 02:30:53PM -0400, Sean Anderson wrote:
> > > >
> > > > > >
> > > > > > Why not just have something like
> > > > > >
> > > > > > mycpu {
> > > > > > ...
> > > > > > riscv,isa {
> > > > > > i;
> > > > > > m;
> > > > > > a;
> > > > > > zicsr;
> > > > > > ...
> > >
> > > I prefer property names be globally unique. The tools are geared towards
> > > that too. That's largely a symptom of having 0 type information in the
> > > DT.
> > >
> > > For example if you had an extension called 'reg', it would be a problem.
> >
> > Per the current ISA rules, that'd not be valid. But then again, I do
> > have trust issues & it's not like "reg" is the only property name in DT
> > land.
>
> ...you say "prefer" here. Is that a NAK, or a "you keep the pieces"?
Don't do the above node.
Rob
Re: [PATCH v2] dt-bindings: riscv: deprecate riscv,isa
On Thu, Jun 8, 2023 at 12:05 PM Conor Dooley wrote:
>
> On Thu, Jun 08, 2023 at 11:49:08AM -0600, Rob Herring wrote:
> > On Thu, 08 Jun 2023 17:54:05 +0100, Conor Dooley wrote:
>
> > > As a result of implementing Sean's suggestion, I believe I need to add
> > > riscv,isa-extensions as an exception to the rules preventing vendor
> > > properties being of object type, otherwise dt_binding_check is less than
> > > happy with me.
>
> > dtschema/dtc warnings/errors:
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/riscv/extensions.yaml:
> > properties:riscv,isa-extensions: 'oneOf' conditional failed, one must be
> > fixed:
> > Additional properties are not allowed ('additionalProperties',
> > 'properties' were unexpected)
> > hint: A vendor boolean property can use "type: boolean"
> >
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/riscv/extensions.yaml:
> > properties:riscv,isa-extensions: 'oneOf' conditional failed, one must be
> > fixed:
> > 'enum' is a required property
> > 'const' is a required property
> > hint: A vendor string property with exact values has an
> > implicit type
> > from schema $id:
> > http://devicetree.org/meta-schemas/vendor-props.yaml#
> > Additional properties are not allowed ('additionalProperties',
> > 'properties', 'type' were unexpected)
> > hint: A vendor string property with exact values has an
> > implicit type
> >
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/riscv/extensions.yaml:
> > properties:riscv,isa-extensions: 'oneOf' conditional failed, one must be
> > fixed:
> > '$ref' is a required property
> > 'allOf' is a required property
> > hint: A vendor property needs a $ref to types.yaml
> > from schema $id:
> > http://devicetree.org/meta-schemas/vendor-props.yaml#
> > 'boolean' was expected
> > hint: A vendor boolean property can use "type: boolean"
> > hint: Vendor specific properties must have a type and description
> > unless they have a defined, common suffix.
> > from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>
> Yeah, expected. I think I need an exception in vendor-props for this to
> pass the checks.
Nope, vendor prefixes don't go in node names. That's not explicit
anywhere, but goes against using generic node names.
Also, note that looking at the DT spec, there's already prior art here
with PPC. See "power-isa-*". Though that appears in .dts files, but no
kernel code. Maybe you already saw that as your v1 is similar. There's
not really much advantage to align with it, but also not much reason
to deviate.
Rob
Re: [PATCH v1] dt-bindings: riscv: deprecate riscv,isa
On Tue, May 30, 2023 at 03:12:12PM +0100, Conor Dooley wrote:
> On Thu, May 18, 2023 at 10:42:34PM +0100, Conor Dooley wrote:
> > On Thu, May 18, 2023 at 02:30:53PM -0400, Sean Anderson wrote:
>
> > >
> > > Why not just have something like
> > >
> > > mycpu {
> > > ...
> > > riscv,isa {
> > > i;
> > > m;
> > > a;
> > > zicsr;
> > > ...
I prefer property names be globally unique. The tools are geared towards
that too. That's largely a symptom of having 0 type information in the
DT.
For example if you had an extension called 'reg', it would be a problem.
> > > };
> > > };
> >
> > Naming of the node aside (perhaps that could be riscv,isa-extensions)
> > there's not something hitting me immediately as to why that is a no-no.
> > If the size is a concern, this would certainly be more efficient & not
> > like the probing would be anything other than trivial more difficult
> > what I have in my proposal.
>
> Having started messing around with this, one of the main advantages, to
> me, of this approach is proper validation.
> cpus.yaml has additionalProperties: true in it, which would have had to
> be sorted out, or worked around, but creating a child-node with the
> properties in it allows setting additionalProperties: false.
That's an issue on my radar to fix. I started that for the Arm cpus.yaml
a while back. Sadly it involves adding all the misc properties vendors
added. It's not a lot, but still better to get in front of that for
Risc-V.
> > Rob's AFK at the moment, and I was hoping that he would take a look at
> > the idea, so I won't respin til he is back, but I'll give this a go in
> > the interim.
>
> Mechanically, the conversion of the patch isn't difficult, but I'll still
> wait for Rob to come back before sending a v2. But that v2 will more
> than likely implement your suggestion.
I haven't read the whole thread, but the initial proposal looks okay to
me.
Another way you could do this is a list of strings:
riscv,isa-ext = "i", "m", "zicsr";
I think we have a helper to test is a string in the list.
Rob
Re: [PATCH v2] dt-bindings: riscv: deprecate riscv,isa
sions
> =
>
> Compared to riscv,isa, this proposed scheme promotes vendor extensions,
> oft touted as the strength of RISC-V, to first-class citizens.
> At present, extensions are defined as meaning what the RISC-V ISA
> specifications say they do. There is no realistic way of using that
> interface to provide cross-platform definitions for what vendor
> extensions mean. Vendor extensions may also have even less consistency
> than RVI do in terms of versioning, or no care about backwards
> compatibility.
> A boolean property allows us to assign explicit meanings on a per vendor
> extension basis, backed up by a description of their meanings.
>
> fin
> ===
>
> Create a new file to store the extension meanings, each of which are
> boolean children of a riscv,isa-extensions node and a new
> riscv,isa-base property to replace the aspect of riscv,isa that is
> not represented by booleans - the base ISA implemented by a hart.
> Originally I proposed properties in the cpu node, rather than as a child
> of the cpu node, but some concerns were raised about the size of the dtb
> for systems with dozens of cpus & dozens of extensions. Using a child
> node, and dropping the "riscv,isa-extension-" prefix saves the guts of
> 20 bytes per extension, per hart, and hopefully placates the size
> conscious.
>
> As a starting point, add properties for extensions currently used in
> Linux.
>
> Finally, mark riscv,isa as deprecated, as removing it is an ABI break.
>
> CC: Palmer Dabbelt
> CC: Paul Walmsley
> CC: Rob Herring
> CC: Krzysztof Kozlowski
> CC: Alistair Francis
> CC: Andrew Jones
> CC: Anup Patel
> CC: Atish Patra
> CC: Jessica Clarke
> CC: Rick Chen
> CC: Leo
> CC: Oleksii
> CC: linux-ri...@lists.infradead.org
> CC: qemu-ri...@nongnu.org
> CC: u-boot@lists.denx.de
> CC: devicet...@vger.kernel.org
> CC: linux-ker...@vger.kernel.org
> Signed-off-by: Conor Dooley
> ---
> Changes in v2:
> - Use Sean's suggestion of a child node to calm fears of bloat
> - Fixup a rake of wording etc issues that Drew pointed out
>
> As a result of implementing Sean's suggestion, I believe I need to add
> riscv,isa-extensions as an exception to the rules preventing vendor
> properties being of object type, otherwise dt_binding_check is less than
> happy with me.
>
> I've tried to CC a few folks here that would care about this, but I am
> sure there are more. I'll go cross-post it to sw-dev, if it allows me to
> post there...
> ---
> .../devicetree/bindings/riscv/cpus.yaml | 57 ++--
> .../devicetree/bindings/riscv/extensions.yaml | 278 ++
> 2 files changed, 313 insertions(+), 22 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/riscv/extensions.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/riscv/extensions.yaml:
properties:riscv,isa-extensions: 'oneOf' conditional failed, one must be fixed:
Additional properties are not allowed ('additionalProperties',
'properties' were unexpected)
hint: A vendor boolean property can use "type: boolean"
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/riscv/extensions.yaml:
properties:riscv,isa-extensions: 'oneOf' conditional failed, one must be fixed:
'enum' is a required property
'const' is a required property
hint: A vendor string property with exact values has an
implicit type
from schema $id:
http://devicetree.org/meta-schemas/vendor-props.yaml#
Additional properties are not allowed ('additionalProperties',
'properties', 'type' were unexpected)
hint: A vendor string property with exact values has an
implicit type
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/riscv/extensions.yaml:
properties:riscv,isa-extensions: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id:
http://devicetree.org/meta-schemas/vendor-props.yaml#
'boolean' was expected
hint: A vendor boolean property can use "type: boolean"
hint: Vendor specific properties must have a type and description
unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
doc reference errors (make refcheckdocs):
See
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230608-sitting-bath-31eddc03c5a5@spud
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH] dt/bindings: fwu-mdata-mtd: drop changes outside FWU
On Thu, May 4, 2023 at 11:42 AM Jassi Brar wrote: > > On Thu, 4 May 2023 at 11:31, Ilias Apalodimas > wrote: > > > > Replying to both Jassi and Tom here since it makes more sense, > > > > On Thu, 4 May 2023 at 19:19, Tom Rini wrote: > > > > > > On Thu, May 04, 2023 at 10:39:06AM -0500, Jassi Brar wrote: > > > > On Thu, 4 May 2023 at 10:19, Rob Herring wrote: > > > > > On Thu, May 4, 2023 at 9:01 AM Jassi Brar > > > > > wrote: > > > > > > > > > > > I may be wrong, but I see having fwu properties contained within > > > > > > the > > > > > > fwu node is cleaner than having them embedded into existing bindings > > > > > > (potentially different classes in future). So I moved to the current > > > > > > design. > > > > > > > > > > Having all the information related to a device/node in one place is > > > > > cleaner IMO. > > > > > > > > > > As I said, if u-boot wants private interfaces between the DT and > > > > > itself, then fine, but that should remain private and be stripped by > > > > > u-boot. A separate node would certainly be easier for doing that. > > > > > > > > > Seems we are on the same page(?). Current implementation does exactly > > > > that -- we have a separate fwu node containing all the properties it > > > > needs. > > > > I think Rob is saying the exact opposite. The way I see it we either > > - Keep the bindings as an internal u-boot ABI, in which case the > > current format is fine, but we need to strip it from the DT before > > handing it over to the OS. > > > I interpreted "Having all the information related to a device/node in > one place is cleaner IMO." to suggest this approach. The one place being where there is already partition information in the partition nodes. This binding adds a 2nd location with pointers to the 1st location. > Though the stripping part remains tbd. Where should that be done in u-boot? Yes. > > - Alternatively, if we want to submit it upstream, we need to change > > where that data lives and ideally have them under existing partition > > bindings. > > > Here we may have to add to bindings of various subsystems (as support > widens) and still have to strip the property before handover to the > kernel. right? > > > Both would work, with the latter offering a bit more standardization > > if another bootloader tries to implement something similar. > > > > > > > > Well, isn't part of why we're here is that this isn't strictly a U-Boot > > > only thing? My question is can, and then is, this also being used in > > > other projects yet? > > > > I am not aware of any other projects currently using it. > > > Maybe I am overlooking something, but the kernel should not have > anything to do with it. Um, LinuxBoot, kexec? > EDK2 may use the node as such and BL1/2 would > have the meta-data location info hardcoded into them (?) Certainly not hard to imagine others needing this. Rob
Re: [PATCH] dt/bindings: fwu-mdata-mtd: drop changes outside FWU
On Thu, May 4, 2023 at 9:01 AM Jassi Brar wrote: > > On Thu, 4 May 2023 at 07:08, Ilias Apalodimas > wrote: > > [...] > > > > > > > I'm assuming it's per partition type rather than storage medium (e.g. > > > > > SATA, USB, SD, NAND, SPI-NOR)? GPT, 'fixed-partitions', other DT > > > > > partition bindings, etc. If so, then I'm really wondering why it's a > > > > > standalone tree rather than integrated into those existing partition > > > > > bindings. > > > > > > > > I think it's per medium, unless I misunderstood something. Sughosh? > > > > > > The bindings would be required as per the metadata access mechanism. > > > So for example, the MTD bindings would suffice for all the storage > > > mediums which would have MTD based partitioning schemes. Same for all > > > GPT partitioned devices. Again, as for the need for a separate node > > > with the compatible property, it is as per the driver model design. > > > For the other properties, we can indeed have them integrated with the > > > corresponding partition bindings. > > > > Ok, Rob is correct then it really is per partition type. > > > Originally the fwu related properties were embedded into existing nodes. > > As Sughosh already explained, we need a compatible string, and hence a > node, for the u-boot driver core to call probe(). DT's purpose is not OS driver instantiation. DT cannot cater to a client's evolving driver structure. > I may be wrong, but I see having fwu properties contained within the > fwu node is cleaner than having them embedded into existing bindings > (potentially different classes in future). So I moved to the current > design. Having all the information related to a device/node in one place is cleaner IMO. As I said, if u-boot wants private interfaces between the DT and itself, then fine, but that should remain private and be stripped by u-boot. A separate node would certainly be easier for doing that. Rob
Re: [PATCH] dt/bindings: fwu-mdata-mtd: drop changes outside FWU
On Wed, May 3, 2023 at 9:37 AM Ilias Apalodimas wrote: > > Hi Krzysztof, > > On Tue, Apr 11, 2023 at 07:38:11AM +0200, Krzysztof Kozlowski wrote: > > On 11/04/2023 01:21, jaswinder.si...@linaro.org wrote: > > > From: Jassi Brar > > > > > > Any requirement of FWU should not require changes to bindings > > > of other subsystems. For example, for mtd-backed storage we > > > can do without requiring 'fixed-partitions' children to also > > > carry 'uuid', a property which is non-standard and not in the > > > bindings. > > > > > > There exists no code yet, so we can change the fwu-mtd bindings > > > to contain all properties within the fwu-mdata node. > > > > > > Signed-off-by: Jassi Brar > > > --- > > > > > > Hi Rob, Hi Krzysztof, > > > > > > I was suggested, and I agree, it would be a good idea to get your > > > blessings > > > for the location and meta-data (fwu-mdata) bindings for the FWU. > > > > > > The FWU images can be located in GPT partitions or MTD backed storage. > > > The basic bindings for fwu-mdata has already been merged in uboot > > > (ideally they > > > too should have had your review). Now I am trying to fully support MTD > > > backed > > > storage and hence looking for your review. The proposed bindings are > > > totally > > > self-contained and don't require changes to any other subsystem. > > > > > > Thanks. > > > > I think we do not review U-Boot bindings usually, except these put in > > the Linux kernel. There were few targeting U-Boot specifically (e.g. > > Documentation/devicetree/bindings/mtd/partitions/u-boot.yaml and > > Documentation/devicetree/bindings/nvmem/u-boot,env.yaml) so if you want > > our blessing, the bindings should be done in Linux kernel repo. > > > > I am pretty sure that reviewing other project bindings would be too much > > of work for me. > > Sure that makes sense. But an answer here of whether the bindings make > sense to the DT maintainers or not would help to move forward. I have lots of comments on the bindings, but I'm missing what is the problem to solve. Yes, it's 'A/B firmware updates', but what configuration data do you need, why and when do you need it. IOW, give me enough information to understand why a binding is designed the way it is and be able to propose alternatives. That's easy enough to deduce for the GPT case. It's just needing to know which device has the f/w update GPT? That's easily solved with an alias, a boolean property in the device, or property with the disk GUID. All of those options are much more simple than a whole node and compatible. > These bindings are trying to define a standardized interface for A/B > *firmware* > updates [0] which is not what traditionally goes into a device tree. OTOH we > already have some U-Boot specific bindings as you already mentioned. As we > move forward we need to be very precise on what is allowed or not on the DT > since it's now tested and verified on SystemReady certifications. IOW if > we add those binding in U-Boot only, we would need to strip them before > handing the DT to linux, otherwise certification would fail. If you do > think that having them in the kernel repo makes sense, it would help > standardizing other boot loaders (at least it would standardize where that > metadata lives) if they want to implement something similar. I think it is fine if u-boot has things in DT which aren't an ABI, so private and bundled, but I agree those should be stripped before passing. To put it another way, if it's not an ABI nor shared amongst projects, I don't think we need a schema nor to care outside of u-boot. However, it doesn't seem to me that needs for A/B updates would be unique to u-boot. > Just keep in mind we would need a schema per storage medium. IOW this tries > to > standardize devices which keep the firmware binary in an mtd. There's also > another biding which describes firmware files on a GPT [1]. I'm assuming it's per partition type rather than storage medium (e.g. SATA, USB, SD, NAND, SPI-NOR)? GPT, 'fixed-partitions', other DT partition bindings, etc. If so, then I'm really wondering why it's a standalone tree rather than integrated into those existing partition bindings. Rob
Re: [PATCH] schemas: Add schema for firmware logs
+boot-architecture On Mon, Feb 6, 2023 at 3:25 PM Simon Glass wrote: > > Hi Rob, > > On Mon, 6 Feb 2023 at 10:15, Rob Herring wrote: > > > > On Sat, Feb 4, 2023 at 6:04 AM Simon Glass wrote: > > > > > > Hi Peter, > > > > > > On Sat, 4 Feb 2023 at 02:36, Peter Robinson wrote: > > > > > > > > Hi Simon, > > > > > > > > Does it make sense to devise something that is compatible with the > > > > kernel's pstore [1] mechanism? > > > > > > Possibly...can you please be a little more specific? > > > > Peter is talking about the same thing I suggested on IRC. > > > > pstore == ramoops > > Oh, I only looked at the DT binding as I thought that was what you > were talking about on irc. The binding is called ramoops as it's for the RAM backend for pstore. My suggestion was either using/extending ramoops or following its design as a reserved memory region. All you would need to extend the ramoops binding is a new property to define the size of your data. > For pstore, isn't the point that Linux wants to save stuff to allow > debugging or collection on reboot? What does that have to do with > console logs from firmware? That seems like a different thing. Or are > you suggesting that we add a pstore driver into U-Boot? It is quite a > lot of code, including compression, etc. It might be easier for Linux > to write the data into pstore when it starts up? Originally ramoops was just what you described. It has grown to multiple backends and types of records (hence the rename to pstore). If you just add a new subsection within the pstore region, then I think the existing kernel infrastructure will support reading it from userspace. Maybe new types have to be explicitly supported, IDK. U-boot being able to read pstore wouldn't be a terrible feature to have anyways if your boot crashes before anything else is up to get the output. Note I'd guess the ram backend doesn't do compression as supporting slightly corrupted ram is a feature which wouldn't work. I think any new DT binding is premature and pstore/ramoops was just a suggestion to consider. This needs wider consideration of how to handle all the various (boot) firmware logs. I've added the boot-architecture list for a bit more visibility. Rob
Re: [PATCH] schemas: Add schema for firmware logs
On Sat, Feb 4, 2023 at 6:04 AM Simon Glass wrote: > > Hi Peter, > > On Sat, 4 Feb 2023 at 02:36, Peter Robinson wrote: > > > > Hi Simon, > > > > Does it make sense to devise something that is compatible with the > > kernel's pstore [1] mechanism? > > Possibly...can you please be a little more specific? Peter is talking about the same thing I suggested on IRC. pstore == ramoops Rob
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Tue, Jan 24, 2023 at 9:56 AM Abdellatif El Khlifi wrote: > > On Mon, Jan 23, 2023 at 09:32:28AM -0700, Simon Glass wrote: > > Hi Rob, > > > > On Mon, 23 Jan 2023 at 08:13, Rob Herring wrote: > > > > > > On Fri, Jan 20, 2023 at 4:04 PM Simon Glass wrote: > > > > > > > > Hi Rob, > > > > > > > > On Thu, 19 Jan 2023 at 11:11, Rob Herring wrote: > > > > > > > > > > On Thu, Jan 19, 2023 at 10:41 AM Simon Glass > > > > > wrote: > > > > > > > > > > > > Hi Abdellatif, > > > > > > > > > > > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > > > > > > wrote: > > > > > > > > > > > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > > > > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > > > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I guess the problem comes down to, can we have one > > > > > > > > > > discovery method that > > > > > > > > > > everyone shares, or do we have to let everyone invent a new > > > > > > > > > > discovery > > > > > > > > > > method every time? > > > > > > > > > > > > > > > > > > > > > > > > > > > No one needs to invent any discovery method every time if the > > > > > > > > > firmware > > > > > > > > > specification > > > > > > > > > provides one and as Rob mentioned many times in the thread, > > > > > > > > > all new firmware > > > > > > > > > specification must provide one and we are trying to make sure > > > > > > > > > that is > > > > > > > > > the case with all new > > > > > > > > > specs from Arm. > > > > > > > > > > > > > > > > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > > > > > > everyone else I'm unintentionally forgetting) could just > > > > > > > > > > discover these > > > > > > > > > > things via device tree. > > > > > > > > > > > > > > > > > > > > > > > > > > > I leave that to the individual projects to decide and agree > > > > > > > > > but > > > > > > > > > fundamentally if > > > > > > > > > the specification provides a way to discover, not sure why we > > > > > > > > > are even > > > > > > > > > discussing > > > > > > > > > an alternative method here. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > > > > > > the discovery. > > > > > > > > > > > > > > > > > > > > > > > > > > > For what reason ? I can understand if there is no discovery > > > > > > > > > mechanism but > > > > > > > > > that's not the > > > > > > > > > case in $subject. > > > > > > > > > > > > > > > > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > > > > > > we could probe their device tree and see they've > > > > > > > > > > implemented the same > > > > > > > > > > concept, but a little differently, but still have the > > > > > > > > > > discovery portion > > > > > > > > > > be in the device tree. To which it sounds like your answer > > > > > > > > > > is "not in > > > > > > > > > > the device tree". > > > > > > > > > > > > > > > > > > >
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Fri, Jan 20, 2023 at 4:04 PM Simon Glass wrote: > > Hi Rob, > > On Thu, 19 Jan 2023 at 11:11, Rob Herring wrote: > > > > On Thu, Jan 19, 2023 at 10:41 AM Simon Glass wrote: > > > > > > Hi Abdellatif, > > > > > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > > > wrote: > > > > > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > I guess the problem comes down to, can we have one discovery > > > > > > > method that > > > > > > > everyone shares, or do we have to let everyone invent a new > > > > > > > discovery > > > > > > > method every time? > > > > > > > > > > > > > > > > > > No one needs to invent any discovery method every time if the > > > > > > firmware > > > > > > specification > > > > > > provides one and as Rob mentioned many times in the thread, all new > > > > > > firmware > > > > > > specification must provide one and we are trying to make sure that > > > > > > is > > > > > > the case with all new > > > > > > specs from Arm. > > > > > > > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > > > everyone else I'm unintentionally forgetting) could just discover > > > > > > > these > > > > > > > things via device tree. > > > > > > > > > > > > > > > > > > I leave that to the individual projects to decide and agree but > > > > > > fundamentally if > > > > > > the specification provides a way to discover, not sure why we are > > > > > > even > > > > > > discussing > > > > > > an alternative method here. > > > > > > > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > > > the discovery. > > > > > > > > > > > > > > > > > > For what reason ? I can understand if there is no discovery > > > > > > mechanism but > > > > > > that's not the > > > > > > case in $subject. > > > > > > > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > > > we could probe their device tree and see they've implemented the > > > > > > > same > > > > > > > concept, but a little differently, but still have the discovery > > > > > > > portion > > > > > > > be in the device tree. To which it sounds like your answer is > > > > > > > "not in > > > > > > > the device tree". > > > > > > > > > > > > > > > > > > > I see U-boot seem to have made a decision to create DT node for > > > > > > each and > > > > > > everything > > > > > > that needs to be added to DM which seems bit unfortunate but I don't > > > > > > understand the > > > > > > history/motive/background for it but I respect the decision if it is > > > > > > already made. > > > > > > > > > > > > These firmware interfaces are standard on all Arm platforms and can > > > > > > be > > > > > > discovered > > > > > > based on PSCI/SMCCC. Not using the same and use DT node needs > > > > > > unnecessary > > > > > > addition of DT nodes for all the f/w i/f on all the platforms that > > > > > > need the > > > > > > support when > > > > > > one can be just discovered. > > > > > > > > > > > > Sorry for the sudden appearance on this thread, I was avoiding > > > > > > getting into > > > > > > this but thought > > > > > > I will at least express my opinion and also the way the firmware > > > > > > specifi
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 10:41 AM Simon Glass wrote: > > Hi Abdellatif, > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > wrote: > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini wrote: > > > > > > > > > > > > > > I guess the problem comes down to, can we have one discovery method > > > > > that > > > > > everyone shares, or do we have to let everyone invent a new discovery > > > > > method every time? > > > > > > > > > > > > No one needs to invent any discovery method every time if the firmware > > > > specification > > > > provides one and as Rob mentioned many times in the thread, all new > > > > firmware > > > > specification must provide one and we are trying to make sure that is > > > > the case with all new > > > > specs from Arm. > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > everyone else I'm unintentionally forgetting) could just discover > > > > > these > > > > > things via device tree. > > > > > > > > > > > > I leave that to the individual projects to decide and agree but > > > > fundamentally if > > > > the specification provides a way to discover, not sure why we are even > > > > discussing > > > > an alternative method here. > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > the discovery. > > > > > > > > > > > > For what reason ? I can understand if there is no discovery mechanism > > > > but > > > > that's not the > > > > case in $subject. > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > we could probe their device tree and see they've implemented the same > > > > > concept, but a little differently, but still have the discovery > > > > > portion > > > > > be in the device tree. To which it sounds like your answer is "not in > > > > > the device tree". > > > > > > > > > > > > > I see U-boot seem to have made a decision to create DT node for each and > > > > everything > > > > that needs to be added to DM which seems bit unfortunate but I don't > > > > understand the > > > > history/motive/background for it but I respect the decision if it is > > > > already made. > > > > > > > > These firmware interfaces are standard on all Arm platforms and can be > > > > discovered > > > > based on PSCI/SMCCC. Not using the same and use DT node needs > > > > unnecessary > > > > addition of DT nodes for all the f/w i/f on all the platforms that need > > > > the > > > > support when > > > > one can be just discovered. > > > > > > > > Sorry for the sudden appearance on this thread, I was avoiding getting > > > > into > > > > this but thought > > > > I will at least express my opinion and also the way the firmware > > > > specifications from Arm is > > > > expected to be evolved from now on. With that I will leave it to you and > > > > other U-boot > > > > maintainers and the community in general to decide the right course in > > > > this > > > > case. > > > > > > To be clear, if the position is that "this is what everyone else will > > > use, really" then yes, we'll follow this in U-Boot. > > > > Hi Simon, Tom, > > > > The FF-A transport is a SW bus and is not associated to any HW peripheral or > > undiscoverable base address. > > > > There is only 1 way of discovering the FF-A bus and it's through the FF-A SW > > interfaces. The FF-A spec [1] describes this in details. > > Can you add a DT node for the 'FF-A SW interfaces' and attach some > sort of top-level driver to that? Perhaps simple-bus, or your own > thing? You don't need to add compatible strings for subnodes (devices > that are discoverable within that). We already have that. It's just called 'arm,psci'. FF-A is not the top-level thing. SMCCC is. That's unfortunately called PSCI in DT because SMCCC grew out of PSCI. Evolution is ugly... It's like this: SMCCC +--PSCI +--TRNG +--FF-A +--SCMI (sometimes) +--OP-TEE +--...Whatever Arm comes up with next... Rob
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Tue, Jan 17, 2023 at 8:04 AM Tom Rini wrote:
>
> On Mon, Jan 16, 2023 at 01:23:53PM +, Abdellatif El Khlifi wrote:
> > On Fri, Jan 13, 2023 at 11:00:28AM -0700, Simon Glass wrote:
> > > Hi Abdellatif,
> > >
> > > On Fri, 13 Jan 2023 at 03:44, Abdellatif El Khlifi
> > > wrote:
> > > >
> > > > On Thu, Jan 12, 2023 at 04:43:32PM -0700, Simon Glass wrote:
> > > > > Hi Rob,
> > > > >
> > > > > On Wed, 11 Jan 2023 at 19:10, Rob Herring wrote:
> > > > > >
> > > > > > On Mon, Dec 19, 2022 at 1:21 PM Simon Glass
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Abdellatif,
> > > > > > >
> > > > > > > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > > > > > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Rob,
> > > > > > > > > >
> > > > > > > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring
> > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass
> > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Abdellatif,
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > should be called 'priov' and should beHi
> > > > > > > > > > > > > > Abdellatif,
> > > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > [..]
> > > > > > > > > > > >
> > > > > > > > > > > > > > > +/**
> > > > > > > > > > > > > > > + * ffa_device_get - create, bind and probe the
> > > > > > > > > > > > > > > arm_ffa device
> > > > > > > > > > > > > > > + * @pdev: the address of a device pointer (to be
> > > > > > > > > > > > > > > filled when the arm_ffa bus device is created
> > > > > > > > > > > > > > > + * successfully)
> > > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > > > > > > > + * created, bound to this driver, probed and
> > > > > > > > > > > > > > > ready to use.
> > > > > > > > > > > > > > > + * Arm FF-A transport is implemented through a
> > > > > > > > > > > > > > > single U-Boot
> > > > > > > > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > > + * Return:
> > > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > > > > > > > + */
> > > > > > > > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > > > > > > > +{
> > > > > > > > > > > > > > > + int ret;
> > > > >
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 12, 2023 at 5:43 PM Simon Glass wrote:
>
> Hi Rob,
>
> On Wed, 11 Jan 2023 at 19:10, Rob Herring wrote:
> >
> > On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote:
> > >
> > > Hi Abdellatif,
> > >
> > > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> > > wrote:
> > > >
> > > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote:
> > > > > >
> > > > > > Hi Rob,
> > > > > >
> > > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> > > > > > >
> > > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Abdellatif,
> > > > > > > >
> > > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > > > > >
> > > > > > > >
> > > > > > > > [..]
> > > > > > > >
> > > > > > > > > > > +/**
> > > > > > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa
> > > > > > > > > > > device
> > > > > > > > > > > + * @pdev: the address of a device pointer (to be filled
> > > > > > > > > > > when the arm_ffa bus device is created
> > > > > > > > > > > + * successfully)
> > > > > > > > > > > + *
> > > > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > > > + * created, bound to this driver, probed and ready to
> > > > > > > > > > > use.
> > > > > > > > > > > + * Arm FF-A transport is implemented through a single
> > > > > > > > > > > U-Boot
> > > > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > > > + *
> > > > > > > > > > > + * Return:
> > > > > > > > > > > + *
> > > > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > > > + */
> > > > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > > > +{
> > > > > > > > > > > + int ret;
> > > > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > > > +
> > > > > > > > > > > + ret = device_bind(dm_root(),
> > > > > > > > > > > DM_DRIVER_GET(arm_ffa), FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > > > > > > > + );
> > > > > > > > > >
> > > > > > > > > > Please add a DT binding. Even if only temporary, we need
> > > > > > > > > > something for this.
> > > > > > > > >
> > > > > > > > > Thanks for the feedback. I'm happy to address all the
> > > > > > > > > comments.
> > > > > > > > >
> > > > > > > > > Regarding DT binding and FF-A discovery. We agreed with
> > > > > > > > > Linaro and Rob Herring
> > > > > > > > > about the following:
> > > > > > > > >
> > > > > > > > > - DT is only for what we failed to make discoverable. For
> > > > > > > > > hardware, we're stuck
> > > > > > > > > with it. We shouldn't repeat that for software interfaces.
> > > > > > > > > This approach is
> > > > > > > > > already applied in the FF-A kernel driver which comes with
> > > > > > > > > no DT support and
> > > > &g
Re: [PATCH V2 5/6] dt-bindings: nvmem: u-boot,env: add MAC's #nvmem-cell-cells
On Wed, 11 Jan 2023 08:31:01 +0100, Rafał Miłecki wrote: > From: Rafał Miłecki > > U-Boot's "ethaddr" environment variable is very often used to store > *base* MAC address. It's used as a base for calculating addresses for > multiple interfaces. It's done by adding proper values. Actual offsets > are picked by manufacturers and vary across devices. > > Signed-off-by: Rafał Miłecki > --- > .../devicetree/bindings/nvmem/layouts/u-boot,env.yaml | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > Reviewed-by: Rob Herring
Re: [PATCH V2 3/6] dt-bindings: nvmem: convert U-Boot env vars to NVMEM layout
On Wed, 11 Jan 2023 08:30:59 +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki
>
> U-Boot environment variables can be found of various underlaying storage
> entities. This binding should be defined as a layout on top on NVMEM
> device not a NVMEM device itself.
>
> Signed-off-by: Rafał Miłecki
> ---
> .../bindings/nvmem/layouts/nvmem-layout.yaml | 1 +
> .../nvmem/{ => layouts}/u-boot,env.yaml | 29 ++-
> MAINTAINERS | 2 +-
> 3 files changed, 17 insertions(+), 15 deletions(-)
> rename Documentation/devicetree/bindings/nvmem/{ => layouts}/u-boot,env.yaml
> (77%)
>
Reviewed-by: Rob Herring
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote:
>
> Hi Abdellatif,
>
> On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> wrote:
> >
> > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> > > > >
> > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wrote:
> > > > > >
> > > > > > Hi Abdellatif,
> > > > > >
> > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > wrote:
> > > > > > >
> > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > > >
> > > > > >
> > > > > > [..]
> > > > > >
> > > > > > > > > +/**
> > > > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > > > > > > > + * @pdev: the address of a device pointer (to be filled when
> > > > > > > > > the arm_ffa bus device is created
> > > > > > > > > + * successfully)
> > > > > > > > > + *
> > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > + * created, bound to this driver, probed and ready to use.
> > > > > > > > > + * Arm FF-A transport is implemented through a single U-Boot
> > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > + *
> > > > > > > > > + * Return:
> > > > > > > > > + *
> > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > + */
> > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > +{
> > > > > > > > > + int ret;
> > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > +
> > > > > > > > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
> > > > > > > > > FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > > > > > + );
> > > > > > > >
> > > > > > > > Please add a DT binding. Even if only temporary, we need
> > > > > > > > something for this.
> > > > > > >
> > > > > > > Thanks for the feedback. I'm happy to address all the comments.
> > > > > > >
> > > > > > > Regarding DT binding and FF-A discovery. We agreed with Linaro
> > > > > > > and Rob Herring
> > > > > > > about the following:
> > > > > > >
> > > > > > > - DT is only for what we failed to make discoverable. For
> > > > > > > hardware, we're stuck
> > > > > > > with it. We shouldn't repeat that for software interfaces. This
> > > > > > > approach is
> > > > > > > already applied in the FF-A kernel driver which comes with no
> > > > > > > DT support and
> > > > > > > discovers the bus with bus_register() API [1].
> > > > > >
> > > > > > This may be the UEFI view, but it is not how U-Boot works. This is
> > > > > > not something we are 'stuck' with. It is how we define what is
> > > > > > present on a device. This is how the PCI bus works in U-Boot. It is
> > > > > > best practice in U-Boot to use the device tree to make this things
> > > > > > visible and configurable. Unlike with Linux there is no other way
> > > > > > to provide configuration needed by these devices.
> > > > >
> > > > > Where do you get UEFI out of this?
> > > >
> > > > I assume it was UEFI as there was no discussion about this in U-Boot.
> > > > Which firmware project was consulted about this?
> > > >
> > > > >
> > > > > It is the discoverability of hardware that is fixed
Re: [PATCH v5] schemas: Add schema for U-Boot driver model 'phase tags'
On Thu, Nov 10, 2022 at 10:59 AM Simon Glass wrote:
>
> U-Boot has some particular challenges with device tree and devices:
>
> - U-Boot has multiple build phases, such as a Secondary Program Loader
> (SPL) phase which typically runs in a pre-SDRAM environment where code
> and data space are limited. In particular, there may not be enough
> space for the full device tree blob. U-Boot uses various automated
> techniques to reduce the size from perhaps 40KB to 3KB. It is not
> always possible to handle these tags entirely at build time, since
> U-Boot proper must have the full device tree, even though we do not
> want it to process all nodes until after relocation.
> - Some U-Boot phases needs to run before the clocks are properly set up,
> where the CPU may be running very slowly. Therefore it is important to
> bind only those devices which are actually needed in that phase
> - U-Boot uses lazy initialisation for its devices, with 'bind' and
> 'probe' being separate steps. Even if a device is bound, it is not
> actually probed until it is used. This is necessary to keep the boot
> time reasonable, e.g. to under a second
>
> The phases of U-Boot in order are: TPL, VPL, SPL, U-Boot (first
> pre-relocation, then post-relocation). ALl but the last two are optional.
>
> For the above reasons, U-Boot only includes the full device tree in the
> final 'U-Boot proper' build. Even then, before relocation U-Boot only
> processes nodes which are marked as being needed.
>
> For this to work, U-Boot's driver model[1] provides a way to mark device
> tree nodes as applicable for a particular phase. This works by adding a
> tag to the node, e.g.:
>
>cru: clock-controller@ff76 {
> phase-all;
> compatible = "rockchip,rk3399-cru";
> reg = <0x0 0xff76 0x0 0x1000>;
> rockchip,grf = <>;
> #clock-cells = <1>;
> #reset-cells = <1>;
> ...
>};
>
> Here the "phase-all" tag indicates that the node must be present in all
> phases, since the clock driver is required.
>
> There has been discussion over the years about whether this could be done
> in a property instead, e.g.
>
>options {
> phase-all = <> <_a> ...;
> ...
>};
>
> Some problems with this:
>
> - we need to be able to merge several such tags from different .dtsi files
> since many boards have their own specific requirements
> - it is hard to find and cross-reference the affected nodes
> - it is more error-prone
> - it requires significant tool rework in U-Boot, including fdtgrep and
> the build system
> - is harder (slower, more code) to process since it involves scanning
> another node/property to find out what to do with a particular node
> - we don't want to add phandle arguments to the above since we are
> referring, e.g., to the clock device as a whole, not a paricular clock
> - the of-platdata feature[2], which converts device tree to C for even
> more constrained environments, would need to become aware of the
> /options node
>
> There is also the question about whether this needs to be U-Boot-specific,
> or whether the tags could be generic. From what I can tell, U-Boot is the
> only bootloader which seriously attempts to use a runtime device tree in
> all cases. For this version, an attempt is made to name the phases in a
> generic manner.
>
> It should also be noted that the approach provided here has stood the test
> of time, used in U-Boot for 8 years so far.
I guess if no one else has any input at all on alternatives to a
boolean per phase, then we should just stick with that as that's the
easiest.
However, I do still think parent nodes need to be implicitly included
rather than explicitly. Otherwise, the result can be invalid DTs
(primarily with 'reg' and addressing)
> So add the schema for this. This will allow a major class of schema
> exceptions to be dropped from the U-Boot source tree.
>
> This being sent to the mailing list since it might attract more review.
> A PR will be sent when this has had some review. That is why the file
> path is set up for https://github.com/devicetree-org/dt-schema rather
> than the Linux kernel.
>
> [1] https://u-boot.readthedocs.io/en/latest/develop/driver-model/index.html
> [2] https://u-boot.readthedocs.io/en/latest/develop/driver-model/of-plat.html
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v5:
> - Fix instructions to run test
> - Update binding title
> - Use 'phase-' instead of 'phase,'
>
> Changes in v4:
> - Drop some unnecessary context from the commit message
> - Explain why parent nodes do not automatically inherit their children's
> tags
> - Rename the tags to use a phase,xxx format, explaining each one
>
> Changes in v3:
> - Fix an incorrect schema path in $id
>
> Changes in v2:
> - Expand docs to include a description of each tag
> - Fix some typos and unclear wording
>
> dtschema/lib.py | 5 +++
> dtschema/schemas/phase.yaml | 74 +
>
Re: [PATCH] Revert "time: add weak annotation to timer_read_counter declaration"
On Wed, Jan 4, 2023 at 6:09 PM Harald Seiler wrote:
>
> This reverts commit 65ba7add0d609bbd035b8d42fafdaf428ac24751.
>
> A weak extern is a nasty sight to behold: If the symbol is never
> defined, on ARM, the linker will replace the function call with a NOP.
> This behavior isn't well documented but there are at least some hints
> to it [1].
>
> When timer_read_counter() is not defined, this obviously does the wrong
> thing here and it does so silently. The consequence is that a board
> without timer_read_counter() will sleep for random amounts and generally
> have erratic get_ticks() values.
>
> Drop the __weak annotation of the extern so a linker error is raised
> when timer_read_counter() is not defined. This is okay, the original
> reason for the reverted change - breaking the sandbox build - no longer
> applies.
>
> Final sidenote: This was the only weak extern in the entire tree at
> this time as far as I can tell. I guess we should avoid introduction of
> them again as they are obviously a very big footgun.
>
> [1]:
> https://stackoverflow.com/questions/31203402/gcc-behavior-for-unresolved-weak-functions
>
> Fixes: 65ba7add0d60 ("time: add weak annotation to timer_read_counter
> declaration")
I don't agree that this is a fix to the above commit. Applying it at
any point after commit 65ba7add0d60 would not work in all cases. It
may not be needed any more, but that is probably due to the addition
of the timer class 2 years later, not that the commit was wrong/broken
at the time.
Rob
Re: [PATCH 1/3] dt-bindings: nvmem: u-boot,env: add MAC's #nvmem-cell-cells
On Thu, Jan 05, 2023 at 06:10:36PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki
>
> U-Boot's "ethaddr" environment variable is very often used to store
> *base* MAC address. It's used as a base for calculating addresses for
> multiple interfaces. It's done by adding proper values. Actual offsets
> are picked by manufacturers and vary across devices.
>
> Signed-off-by: Rafał Miłecki
> ---
> Documentation/devicetree/bindings/nvmem/u-boot,env.yaml | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> index cbc5c69fd405..1c139bd689ea 100644
> --- a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> @@ -50,7 +50,11 @@ properties:
>
>ethaddr:
> type: object
> -description: Ethernet interface's MAC address
> +description:
> + Ethernet interfaces base MAC address. The first argument is an offset.
The 2nd sentence belongs in the '#nvmem-cell-cells' description.
> +properties:
> + "#nvmem-cell-cells":
> +const: 1
>
> additionalProperties: false
>
> @@ -72,6 +76,7 @@ examples:
> reg = <0x4 0x1>;
>
> mac: ethaddr {
> +#nvmem-cell-cells = <1>;
> };
> };
> };
> --
> 2.34.1
>
>
Re: [PATCH v5 3/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export
On Sat, Dec 24, 2022 at 6:04 AM Paul Barker wrote:
>
> On 20/12/2022 15:55, Rob Herring wrote:
> > On Wed, Nov 23, 2022 at 05:50:06PM +, Paul Barker wrote:
> >> Add properties to the Authenta SPI flash device node to enable access by
> >> a UEFI application using a fixed GUID.
> >>
> >> Signed-off-by: Paul Barker
> >> ---
> >> arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi | 13 ++---
> >> arch/arm/dts/am335x-sancloud-bbe-lite.dts | 2 +-
> >> 2 files changed, 11 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi
> >> b/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi
> >> index 01c105ebb383..6c4ff67f9a4b 100644
> >> --- a/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi
> >> +++ b/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi
> >> @@ -38,7 +38,14 @@
> >>
> >> {
> >> u-boot,dm-pre-reloc;
> >> -channel@0 {
> >> -u-boot,dm-pre-reloc;
> >> -};
> >> +};
> >> +
> >> +_flash {
> >> +u-boot,dm-pre-reloc;
> >> +
> >> +u-boot,uefi-spi-vendor = "micron";
> >> +u-boot,uefi-spi-part-number = "mt25ql128abb";
> >
> > Looks like a compatible string. Yet, the flash node compatible string,
> > micron,spi-authenta, is not documented (though in use for spidev). So
> > use a compatible string for the flash that is specific to the flash
> > model. I assume there is some reason the specific model is needed?
>
> For context, the UEFI Platform Initialization (PI) spec defines
> EFI_SPI_PART, EFI_SPI_PERIPHERAL and EFI_SPI_IO_PROTOCOL structures.
> I'm referencing v1.7 Errata A. See https://uefi.org/specifications for
> downloads.
>
> The EFI_SPI_PART structure has "Vendor" and "PartNumber" fields. We need
> something to put in those fields and the device tree is the best place
> to store the data. These properties are in the `-u-boot.dtsi` file so
> they won't be submitted to the Linux kernel.
Can't you just split the `micron,mt25ql128abb` compatible string to
populate those?
> The `micron,spi-authenta` compatible string is unfortunate in my
> opinion. I plan to replace this with two entries, `micron,mt25ql128abb`
> and `jedec,spi-nor`, once we've got MTD support for the Authenta parts
> merged into the mainline kernel. That work is currently on hold, I'll
> be working with Micron in the new year to unblock it. We could submit
> a change to the compatible property in the meantime if you think it's
> worth getting a temporary improvement in before the MTD changes are
> done.
No need to wait on driver changes to add compatible strings. Anyone
that says otherwise is wrong.
Looking at the datasheet, maybe `micron,mt25ql128abb` is too specific.
Perhaps 'abb' or just the last 'b' can be dropped? It's a trade off of
lots of compatible strings vs. whether you might need to distinguish
the parts (only before you do jedec discovery).
>
> >> +/* GUID in UEFI format: 77126730-a4ca-4386-b341-881fe18e7f7d */
> >> +u-boot,uefi-spi-io-guid = [30 67 12 77 ca a4 86 43
> >> + b3 41 88 1f e1 8e 7f 7d];
> >
> > We need to define first in the DT spec the format for GUIDs. I don't
> > think there are any existing cases though some have been proposed. IMO,
> > I would make this a string instead. The byte array is not that
> > readable with its little endian order. A GUID as a string is readily
> > identifiable as a GUID.
>
> I see there is a `uuid_str_to_bin()` function in u-boot so I could make
> this a string property and convert it to a binary GUID at runtime. This
> would make the dts more readable so I'll make the change for v6 of this
> series.
>
> > Why is this u-boot specific? Another UEFI implementation doesn't need
> > the GUID?
>
> Each EFI_SPI_IO_PROTOCOL instance needs its own GUID so that it can
> be located at runtime. These GUIDs are not pre-defined by the spec,
> instead an arbitrary per-device GUID is used and is stored in the
> SpiPeripheralDriverGuid field of the relevant EFI_SPI_PERIPHERAL
> instance. We could randomly generate these GUIDs at runtime since a UEFI
> application can walk the tree of SPI busses and peripherals, looking
> for a match in the Vendor and PartNumber fields defined above, to get
> the appropriate GUID. However, storing a known value in the device tree
> allows a UEFI application to just lookup the GUID without needing to
> walk the SPI tree.
Okay, then I guess my next question is why is it SPI IO protocol
specific? I'd assume the UEFI spec would use this for any h/w
protocol.
Generating GUIDs at runtime seems like the right way though. We simply
can't populate DT with every client's method of identifying a given
instance. That simply doesn't scale.
Rob
Re: [PATCH v5 3/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export
On Wed, Nov 23, 2022 at 05:50:06PM +, Paul Barker wrote:
> Add properties to the Authenta SPI flash device node to enable access by
> a UEFI application using a fixed GUID.
>
> Signed-off-by: Paul Barker
> ---
> arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi | 13 ++---
> arch/arm/dts/am335x-sancloud-bbe-lite.dts | 2 +-
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi
> b/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi
> index 01c105ebb383..6c4ff67f9a4b 100644
> --- a/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi
> +++ b/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi
> @@ -38,7 +38,14 @@
>
> {
> u-boot,dm-pre-reloc;
> - channel@0 {
> - u-boot,dm-pre-reloc;
> - };
> +};
> +
> +_flash {
> + u-boot,dm-pre-reloc;
> +
> + u-boot,uefi-spi-vendor = "micron";
> + u-boot,uefi-spi-part-number = "mt25ql128abb";
Looks like a compatible string. Yet, the flash node compatible string,
micron,spi-authenta, is not documented (though in use for spidev). So
use a compatible string for the flash that is specific to the flash
model. I assume there is some reason the specific model is needed?
> + /* GUID in UEFI format: 77126730-a4ca-4386-b341-881fe18e7f7d */
> + u-boot,uefi-spi-io-guid = [30 67 12 77 ca a4 86 43
> +b3 41 88 1f e1 8e 7f 7d];
We need to define first in the DT spec the format for GUIDs. I don't
think there are any existing cases though some have been proposed. IMO,
I would make this a string instead. The byte array is not that
readable with its little endian order. A GUID as a string is readily
identifiable as a GUID.
Why is this u-boot specific? Another UEFI implementation doesn't need
the GUID?
Rob
[PATCH 1/2] dts: synquacer: Drop unused and undocumented SPI properties
'active_clk_edges' and 'chipselect_num' SPI controller properties are
unused in u-boot and Linux. They are also not documented in the binding.
So drop them.
Signed-off-by: Rob Herring
---
arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
b/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
index 6b85c05458d4..9f9837b33bef 100644
--- a/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
@@ -20,8 +20,6 @@
#address-cells = <1>;
#size-cells = <0>;
status = "okay";
- active_clk_edges;
- chipselect_num = <1>;
flash@0 {
#address-cells = <1>;
--
2.35.1
[PATCH 2/2] dts: synquacer: Drop unused and undocumented GPIO 'base' property
The 'base' GPIO controller property is unused in u-boot and Linux. It is
also not documented in the binding. So drop it.
Signed-off-by: Rob Herring
---
arch/arm/dts/synquacer-sc2a11.dtsi | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/dts/synquacer-sc2a11.dtsi
b/arch/arm/dts/synquacer-sc2a11.dtsi
index 0dd2969b5e3c..7ba1cd1bee70 100644
--- a/arch/arm/dts/synquacer-sc2a11.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11.dtsi
@@ -497,7 +497,6 @@
gpio-controller;
#gpio-cells = <2>;
clocks = <_apb>;
-base = <0>;
};
exiu: interrupt-controller@510c {
--
2.35.1
[PATCH 3/4] dts: synquacer: Fix "arm, armv7-timer-mem" node address sizes
The "arm,armv7-timer-mem" schema defines the address sizes for child
nodes to be 32-bit as there's no need for 64-bit offsets and sizes of
the child 'frame' nodes.
Signed-off-by: Rob Herring
---
arch/arm/dts/synquacer-sc2a11.dtsi | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/synquacer-sc2a11.dtsi
b/arch/arm/dts/synquacer-sc2a11.dtsi
index 0e1bc164549f..049afcb0af8a 100644
--- a/arch/arm/dts/synquacer-sc2a11.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11.dtsi
@@ -364,13 +364,13 @@
timer@2a81 {
compatible = "arm,armv7-timer-mem";
reg = <0x0 0x2a81 0x0 0x1>;
-#address-cells = <2>;
-#size-cells = <2>;
-ranges;
-frame@2a83 {
+#address-cells = <1>;
+#size-cells = <1>;
+ranges = <0x0 0x0 0x2a81 0x3>;
+frame@2 {
frame-number = <0>;
interrupts = ;
-reg = <0x0 0x2a83 0x0 0x1>;
+reg = <0x2 0x1>;
};
};
--
b4 0.11.0-dev
[PATCH 4/4] dts: synquacer: Fix idle-states 'entry-method' value
The correct value for 'entry-method' in the idle-states binding is 'psci',
not 'arm,psci'. It hasn't mattered because it isn't used by the OS.
Signed-off-by: Rob Herring
---
arch/arm/dts/synquacer-sc2a11.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/synquacer-sc2a11.dtsi
b/arch/arm/dts/synquacer-sc2a11.dtsi
index 049afcb0af8a..0dd2969b5e3c 100644
--- a/arch/arm/dts/synquacer-sc2a11.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11.dtsi
@@ -309,7 +309,7 @@
};
idle-states {
-entry-method = "arm,psci";
+entry-method = "psci";
CPU_SLEEP_0: cpu-sleep-0 {
compatible = "arm,idle-state";
--
b4 0.11.0-dev
[PATCH 1/4] dts: synquacer: Drop CPU 'arm,armv8' compatibles
'arm,armv8' compatible is for software models only. so drop it from cpu
nodes.
Signed-off-by: Rob Herring
---
arch/arm/dts/synquacer-sc2a11.dtsi | 48 +++---
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/arch/arm/dts/synquacer-sc2a11.dtsi
b/arch/arm/dts/synquacer-sc2a11.dtsi
index 1fe7d214b9c9..1f8cd9d25780 100644
--- a/arch/arm/dts/synquacer-sc2a11.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11.dtsi
@@ -41,168 +41,168 @@
CPU0: cpu@0 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x0>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU1: cpu@1 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x1>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU2: cpu@100 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x100>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU3: cpu@101 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x101>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU4: cpu@200 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x200>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU5: cpu@201 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x201>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU6: cpu@300 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x300>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU7: cpu@301 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x301>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU8: cpu@400 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x400>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU9: cpu@401 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x401>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU10: cpu@500 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x500>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU11: cpu@501 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x501>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU12: cpu@600 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x600>;
enable-method = "psci";
cpu-idle-state
[PATCH 2/4] dts: synquacer: Use generic node names
DT node names should follow generic names defined in the DT spec. These
are also now checked by dtschema tools.
Signed-off-by: Rob Herring
---
arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi | 2 +-
arch/arm/dts/synquacer-sc2a11-developerbox.dts | 2 +-
arch/arm/dts/synquacer-sc2a11.dtsi | 10 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
b/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
index 7a56116d6f12..6b85c05458d4 100644
--- a/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
@@ -23,7 +23,7 @@
active_clk_edges;
chipselect_num = <1>;
- spi-flash@0 {
+ flash@0 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "jedec,spi-nor";
diff --git a/arch/arm/dts/synquacer-sc2a11-developerbox.dts
b/arch/arm/dts/synquacer-sc2a11-developerbox.dts
index 42b6cbbb8251..c8087b99a781 100644
--- a/arch/arm/dts/synquacer-sc2a11-developerbox.dts
+++ b/arch/arm/dts/synquacer-sc2a11-developerbox.dts
@@ -18,7 +18,7 @@
compatible = "gpio-keys";
interrupt-parent = <>;
-power {
+power-button {
label = "Power Button";
linux,code = ;
interrupts = ;
diff --git a/arch/arm/dts/synquacer-sc2a11.dtsi
b/arch/arm/dts/synquacer-sc2a11.dtsi
index 1f8cd9d25780..0e1bc164549f 100644
--- a/arch/arm/dts/synquacer-sc2a11.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11.dtsi
@@ -344,7 +344,7 @@
interrupt-controller;
interrupts = ;
-its: gic-its@3002 {
+its: msi-controller@3002 {
compatible = "arm,gic-v3-its";
reg = <0x0 0x3002 0x0 0x2>;
#msi-cells = <1>;
@@ -361,7 +361,7 @@
; // HYP
};
-mmio-timer@2a81 {
+timer@2a81 {
compatible = "arm,armv7-timer-mem";
reg = <0x0 0x2a81 0x0 0x1>;
#address-cells = <2>;
@@ -398,7 +398,7 @@
clock-output-names = "apb_pclk";
};
-soc_uart0: uart@2a40 {
+soc_uart0: serial@2a40 {
compatible = "arm,pl011", "arm,primecell";
reg = <0x0 0x2a40 0x0 0x1000>;
interrupts = ;
@@ -406,7 +406,7 @@
clock-names = "uartclk", "apb_pclk";
};
-fuart: uart@5104 {
+fuart: serial@5104 {
compatible = "snps,dw-apb-uart";
reg = <0x0 0x5104 0x0 0x1000>;
interrupts = ;
@@ -523,7 +523,7 @@
clock-output-names = "sd_sd4clk";
};
-sdhci: sdhci@5230 {
+sdhci: mmc@5230 {
compatible = "socionext,synquacer-sdhci", "fujitsu,mb86s70-sdhci-3.0";
reg = <0 0x5230 0x0 0x1000>;
interrupts = ,
--
b4 0.11.0-dev
[PATCH 0/4] Synquacer DT schema fixes
This is a series of DT fixes for the Synquacer. These issues were found running the dtschema tools. I don't have a board, but Ilias has tested the changes for me. Thanks! Signed-off-by: Rob Herring --- Rob Herring (4): dts: synquacer: Drop CPU 'arm,armv8' compatibles dts: synquacer: Use generic node names dts: synquacer: Fix "arm,armv7-timer-mem" node address sizes dts: synquacer: Fix idle-states 'entry-method' value .../dts/synquacer-sc2a11-developerbox-u-boot.dtsi | 2 +- arch/arm/dts/synquacer-sc2a11-developerbox.dts | 2 +- arch/arm/dts/synquacer-sc2a11.dtsi | 70 +++--- 3 files changed, 37 insertions(+), 37 deletions(-) --- base-commit: bebb393b340295edb9ba50a996fc0510cd1b6ac0 change-id: 20221206-synquacer-dts-521500f88a1d Best regards, -- Rob Herring
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote:
>
> Hi Rob,
>
> On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> >
> > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wrote:
> > >
> > > Hi Abdellatif,
> > >
> > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > wrote:
> > > >
> > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > should be called 'priov' and should beHi Abdellatif,
> > > > >
> > >
> > > [..]
> > >
> > > > > > +/**
> > > > > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > > > > + * @pdev: the address of a device pointer (to be filled when the
> > > > > > arm_ffa bus device is created
> > > > > > + * successfully)
> > > > > > + *
> > > > > > + * This function makes sure the arm_ffa device is
> > > > > > + * created, bound to this driver, probed and ready to use.
> > > > > > + * Arm FF-A transport is implemented through a single U-Boot
> > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > + *
> > > > > > + * Return:
> > > > > > + *
> > > > > > + * 0 on success. Otherwise, failure
> > > > > > + */
> > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > +{
> > > > > > + int ret;
> > > > > > + struct udevice *dev = NULL;
> > > > > > +
> > > > > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
> > > > > > FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > > + );
> > > > >
> > > > > Please add a DT binding. Even if only temporary, we need something
> > > > > for this.
> > > >
> > > > Thanks for the feedback. I'm happy to address all the comments.
> > > >
> > > > Regarding DT binding and FF-A discovery. We agreed with Linaro and Rob
> > > > Herring
> > > > about the following:
> > > >
> > > > - DT is only for what we failed to make discoverable. For hardware,
> > > > we're stuck
> > > > with it. We shouldn't repeat that for software interfaces. This
> > > > approach is
> > > > already applied in the FF-A kernel driver which comes with no DT
> > > > support and
> > > > discovers the bus with bus_register() API [1].
> > >
> > > This may be the UEFI view, but it is not how U-Boot works. This is not
> > > something we are 'stuck' with. It is how we define what is present on a
> > > device. This is how the PCI bus works in U-Boot. It is best practice in
> > > U-Boot to use the device tree to make this things visible and
> > > configurable. Unlike with Linux there is no other way to provide
> > > configuration needed by these devices.
> >
> > Where do you get UEFI out of this?
>
> I assume it was UEFI as there was no discussion about this in U-Boot.
> Which firmware project was consulted about this?
>
> >
> > It is the discoverability of hardware that is fixed (and we are stuck
> > with). We can't change hardware. The disoverability may be PCI
> > VID/PID, USB device descriptors, or nothing. We only use DT when those
> > are not sufficient. For a software interface, there is no reason to
> > make them non-discoverable as the interface can be fixed (at least for
> > new things like FF-A).
>
> Here I am talking about the controller itself, the top-level node in
> the device tree. For PCI this is a device tree node and it should be
> the same here. So I am not saying that the devices on the bus need to
> be in the device tree (that can be optional, but may be useful in some
> situations where it is status and known).
Sure, the PCI host bridges are not discoverable, have a bunch of
resources, and do need to be in DT. The downstream devices only do if
they have extra resources such as when a device is soldered down on a
board rather than a standard slot.
> We need something like:
>
> ff-a {
> compatible = "something";
> };
>
> I don't know what mechanism is actually used to communicate with it,
> but that will be enough to get the top-level driver started.
There's discovery of FF-A itself and then discovery of FF-A features
(e.g. partitions). Both of those are discoverable without DT. The
first is don
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wrote:
>
> Hi Abdellatif,
>
> On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> wrote:
> >
> > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > should be called 'priov' and should beHi Abdellatif,
> > >
>
> [..]
>
> > > > +/**
> > > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > > + * @pdev: the address of a device pointer (to be filled when the
> > > > arm_ffa bus device is created
> > > > + * successfully)
> > > > + *
> > > > + * This function makes sure the arm_ffa device is
> > > > + * created, bound to this driver, probed and ready to use.
> > > > + * Arm FF-A transport is implemented through a single U-Boot
> > > > + * device managing the FF-A bus (arm_ffa).
> > > > + *
> > > > + * Return:
> > > > + *
> > > > + * 0 on success. Otherwise, failure
> > > > + */
> > > > +int ffa_device_get(struct udevice **pdev)
> > > > +{
> > > > + int ret;
> > > > + struct udevice *dev = NULL;
> > > > +
> > > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
> > > > FFA_DRV_NAME, NULL, ofnode_null(),
> > > > + );
> > >
> > > Please add a DT binding. Even if only temporary, we need something for
> > > this.
> >
> > Thanks for the feedback. I'm happy to address all the comments.
> >
> > Regarding DT binding and FF-A discovery. We agreed with Linaro and Rob
> > Herring
> > about the following:
> >
> > - DT is only for what we failed to make discoverable. For hardware, we're
> > stuck
> > with it. We shouldn't repeat that for software interfaces. This approach
> > is
> > already applied in the FF-A kernel driver which comes with no DT support
> > and
> > discovers the bus with bus_register() API [1].
>
> This may be the UEFI view, but it is not how U-Boot works. This is not
> something we are 'stuck' with. It is how we define what is present on a
> device. This is how the PCI bus works in U-Boot. It is best practice in
> U-Boot to use the device tree to make this things visible and configurable.
> Unlike with Linux there is no other way to provide configuration needed by
> these devices.
Where do you get UEFI out of this?
It is the discoverability of hardware that is fixed (and we are stuck
with). We can't change hardware. The disoverability may be PCI
VID/PID, USB device descriptors, or nothing. We only use DT when those
are not sufficient. For a software interface, there is no reason to
make them non-discoverable as the interface can be fixed (at least for
new things like FF-A).
Rob
Re: [PATCH v4] schemas: Add schema for U-Boot driver model 'phase tags'
+Ilias, Bill and Joakim
On Sat, Nov 12, 2022 at 9:21 AM Simon Glass wrote:
>
> Hi Rob,
>
> (unfortunately I have a filter on this list due to the insane traffic
> and am not sure how to let these emails through, so I just saw this)
https://lore.kernel.org/linux-devicetree/?q=a%3Asjg%40chromium.org
And 'lei' can make that search a persistent mailbox.
>
> On Thu, 10 Nov 2022 at 11:30, Rob Herring wrote:
> >
> > On Thu, Nov 10, 2022 at 10:59 AM Simon Glass wrote:
> > >
> > > Hi Rob,
> > >
> > > On Tue, 8 Nov 2022 at 10:19, Rob Herring wrote:
> > > >
> > > > On Tue, Nov 1, 2022 at 10:13 PM Simon Glass wrote:
> > > > >
> > > > > U-Boot has some particular challenges with device tree and devices:
> > > > >
> > > > > - U-Boot has multiple build phases, such as a Secondary Program Loader
> > > > > (SPL) phase which typically runs in a pre-SDRAM environment where
> > > > > code
> > > > > and data space are limited. In particular, there may not be enough
> > > > > space for the full device tree blob. U-Boot uses various automated
> > > > > techniques to reduce the size from perhaps 40KB to 3KB. It is not
> > > > > always possible to handle these tags entirely at build time, since
> > > > > U-Boot proper must have the full device tree, even though we do not
> > > > > want it to process all nodes until after relocation.
> > > > > - Some U-Boot phases needs to run before the clocks are properly set
> > > > > up,
> > > > > where the CPU may be running very slowly. Therefore it is important
> > > > > to
> > > > > bind only those devices which are actually needed in that phase
> > > > > - U-Boot uses lazy initialisation for its devices, with 'bind' and
> > > > > 'probe' being separate steps. Even if a device is bound, it is not
> > > > > actually probed until it is used. This is necessary to keep the boot
> > > > > time reasonable, e.g. to under a second
> > > > >
> > > > > The phases of U-Boot in order are: TPL, VPL, SPL, U-Boot (first
> > > > > pre-relocation, then post-relocation). ALl but the last two are
> > > > > optional.
> > > > >
> > > > > For the above reasons, U-Boot only includes the full device tree in
> > > > > the
> > > > > final 'U-Boot proper' build. Even then, before relocation U-Boot only
> > > > > processes nodes which are marked as being needed.
> > > > >
> > > > > For this to work, U-Boot's driver model[1] provides a way to mark
> > > > > device
> > > > > tree nodes as applicable for a particular phase. This works by adding
> > > > > a
> > > > > tag to the node, e.g.:
> > > > >
> > > > >cru: clock-controller@ff76 {
> > > > > phase,all;
> > > > > compatible = "rockchip,rk3399-cru";
> > > > > reg = <0x0 0xff76 0x0 0x1000>;
> > > > > rockchip,grf = <>;
> > > > > #clock-cells = <1>;
> > > > > #reset-cells = <1>;
> > > > > ...
> > > > >};
> > > > >
> > > > > Here the "phase,all" tag indicates that the node must be present in
> > > > > all
> > > > > phases, since the clock driver is required.
> > > > >
> > > > > There has been discussion over the years about whether this could be
> > > > > done
> > > > > in a property instead, e.g.
> > > > >
> > > > >options {
> > > > > phase,all = <> <_a> ...;
> > > > > ...
> > > > >};
> > > > >
> > > > > Some problems with this:
> > > > >
> > > > > - we need to be able to merge several such tags from different .dtsi
> > > > > files
> > > > > since many boards have their own specific requirements
> > > > > - it is hard to find and cross-reference the affected nodes
> > > > > - it is more error-prone
> > > > > - it requires significant tool rework in U-Boot, including fdtgrep and
> > > > > the build system
> > > > > - is harder (slower, more code
Re: [PATCH v4] schemas: Add schema for U-Boot driver model 'phase tags'
On Thu, Nov 10, 2022 at 10:59 AM Simon Glass wrote:
>
> Hi Rob,
>
> On Tue, 8 Nov 2022 at 10:19, Rob Herring wrote:
> >
> > On Tue, Nov 1, 2022 at 10:13 PM Simon Glass wrote:
> > >
> > > U-Boot has some particular challenges with device tree and devices:
> > >
> > > - U-Boot has multiple build phases, such as a Secondary Program Loader
> > > (SPL) phase which typically runs in a pre-SDRAM environment where code
> > > and data space are limited. In particular, there may not be enough
> > > space for the full device tree blob. U-Boot uses various automated
> > > techniques to reduce the size from perhaps 40KB to 3KB. It is not
> > > always possible to handle these tags entirely at build time, since
> > > U-Boot proper must have the full device tree, even though we do not
> > > want it to process all nodes until after relocation.
> > > - Some U-Boot phases needs to run before the clocks are properly set up,
> > > where the CPU may be running very slowly. Therefore it is important to
> > > bind only those devices which are actually needed in that phase
> > > - U-Boot uses lazy initialisation for its devices, with 'bind' and
> > > 'probe' being separate steps. Even if a device is bound, it is not
> > > actually probed until it is used. This is necessary to keep the boot
> > > time reasonable, e.g. to under a second
> > >
> > > The phases of U-Boot in order are: TPL, VPL, SPL, U-Boot (first
> > > pre-relocation, then post-relocation). ALl but the last two are optional.
> > >
> > > For the above reasons, U-Boot only includes the full device tree in the
> > > final 'U-Boot proper' build. Even then, before relocation U-Boot only
> > > processes nodes which are marked as being needed.
> > >
> > > For this to work, U-Boot's driver model[1] provides a way to mark device
> > > tree nodes as applicable for a particular phase. This works by adding a
> > > tag to the node, e.g.:
> > >
> > >cru: clock-controller@ff76 {
> > > phase,all;
> > > compatible = "rockchip,rk3399-cru";
> > > reg = <0x0 0xff76 0x0 0x1000>;
> > > rockchip,grf = <>;
> > > #clock-cells = <1>;
> > > #reset-cells = <1>;
> > > ...
> > >};
> > >
> > > Here the "phase,all" tag indicates that the node must be present in all
> > > phases, since the clock driver is required.
> > >
> > > There has been discussion over the years about whether this could be done
> > > in a property instead, e.g.
> > >
> > >options {
> > > phase,all = <> <_a> ...;
> > > ...
> > >};
> > >
> > > Some problems with this:
> > >
> > > - we need to be able to merge several such tags from different .dtsi files
> > > since many boards have their own specific requirements
> > > - it is hard to find and cross-reference the affected nodes
> > > - it is more error-prone
> > > - it requires significant tool rework in U-Boot, including fdtgrep and
> > > the build system
> > > - is harder (slower, more code) to process since it involves scanning
> > > another node/property to find out what to do with a particular node
> > > - we don't want to add phandle arguments to the above since we are
> > > referring, e.g., to the clock device as a whole, not a paricular clock
> > > - the of-platdata feature[2], which converts device tree to C for even
> > > more constrained environments, would need to become aware of the
> > > /options node
> > >
> > > There is also the question about whether this needs to be U-Boot-specific,
> > > or whether the tags could be generic. From what I can tell, U-Boot is the
> > > only bootloader which seriously attempts to use a runtime device tree in
> > > all cases. For this version, an attempt is made to name the phases in a
> > > generic manner.
> > >
> > > It should also be noted that the approach provided here has stood the test
> > > of time, used in U-Boot for 8 years so far.
> > >
> > > So add the schema for this. This will allow a major class of schema
> > > exceptions to be dropped from the U-Boot source tree.
> > >
> > > This being sent to the mailing list since it might attract more review.
> > > A PR will be sent when this has had some review. Th
Re: [PATCH v4] schemas: Add schema for U-Boot driver model 'phase tags'
On Tue, Nov 1, 2022 at 10:13 PM Simon Glass wrote:
>
> U-Boot has some particular challenges with device tree and devices:
>
> - U-Boot has multiple build phases, such as a Secondary Program Loader
> (SPL) phase which typically runs in a pre-SDRAM environment where code
> and data space are limited. In particular, there may not be enough
> space for the full device tree blob. U-Boot uses various automated
> techniques to reduce the size from perhaps 40KB to 3KB. It is not
> always possible to handle these tags entirely at build time, since
> U-Boot proper must have the full device tree, even though we do not
> want it to process all nodes until after relocation.
> - Some U-Boot phases needs to run before the clocks are properly set up,
> where the CPU may be running very slowly. Therefore it is important to
> bind only those devices which are actually needed in that phase
> - U-Boot uses lazy initialisation for its devices, with 'bind' and
> 'probe' being separate steps. Even if a device is bound, it is not
> actually probed until it is used. This is necessary to keep the boot
> time reasonable, e.g. to under a second
>
> The phases of U-Boot in order are: TPL, VPL, SPL, U-Boot (first
> pre-relocation, then post-relocation). ALl but the last two are optional.
>
> For the above reasons, U-Boot only includes the full device tree in the
> final 'U-Boot proper' build. Even then, before relocation U-Boot only
> processes nodes which are marked as being needed.
>
> For this to work, U-Boot's driver model[1] provides a way to mark device
> tree nodes as applicable for a particular phase. This works by adding a
> tag to the node, e.g.:
>
>cru: clock-controller@ff76 {
> phase,all;
> compatible = "rockchip,rk3399-cru";
> reg = <0x0 0xff76 0x0 0x1000>;
> rockchip,grf = <>;
> #clock-cells = <1>;
> #reset-cells = <1>;
> ...
>};
>
> Here the "phase,all" tag indicates that the node must be present in all
> phases, since the clock driver is required.
>
> There has been discussion over the years about whether this could be done
> in a property instead, e.g.
>
>options {
> phase,all = <> <_a> ...;
> ...
>};
>
> Some problems with this:
>
> - we need to be able to merge several such tags from different .dtsi files
> since many boards have their own specific requirements
> - it is hard to find and cross-reference the affected nodes
> - it is more error-prone
> - it requires significant tool rework in U-Boot, including fdtgrep and
> the build system
> - is harder (slower, more code) to process since it involves scanning
> another node/property to find out what to do with a particular node
> - we don't want to add phandle arguments to the above since we are
> referring, e.g., to the clock device as a whole, not a paricular clock
> - the of-platdata feature[2], which converts device tree to C for even
> more constrained environments, would need to become aware of the
> /options node
>
> There is also the question about whether this needs to be U-Boot-specific,
> or whether the tags could be generic. From what I can tell, U-Boot is the
> only bootloader which seriously attempts to use a runtime device tree in
> all cases. For this version, an attempt is made to name the phases in a
> generic manner.
>
> It should also be noted that the approach provided here has stood the test
> of time, used in U-Boot for 8 years so far.
>
> So add the schema for this. This will allow a major class of schema
> exceptions to be dropped from the U-Boot source tree.
>
> This being sent to the mailing list since it might attract more review.
> A PR will be sent when this has had some review. That is why the file
> path is set up for https://github.com/devicetree-org/dt-schema rather
> than the Linux kernel.
>
> [1] https://u-boot.readthedocs.io/en/latest/develop/driver-model/index.html
> [2] https://u-boot.readthedocs.io/en/latest/develop/driver-model/of-plat.html
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v4:
> - Drop some unnecessary context from the commit message
> - Explain why parent nodes do not automatically inherit their children's
> tags
> - Rename the tags to use a phase,xxx format, explaining each one
>
> Changes in v3:
> - Fix an incorrect schema path in $id
>
> Changes in v2:
> - Expand docs to include a description of each tag
> - Fix some typos and unclear wording
>
> dtschema/lib.py | 5 +++
> dtschema/schemas/phase.yaml | 73 +
> test/phases.dts | 26 +
> 3 files changed, 104 insertions(+)
> create mode 100644 dtschema/schemas/phase.yaml
> create mode 100644 test/phases.dts
>
> diff --git a/dtschema/lib.py b/dtschema/lib.py
> index 3b6c937..9a2fafa 100644
> --- a/dtschema/lib.py
> +++ b/dtschema/lib.py
> @@ -514,6 +514,11 @@ def fixup_node_props(schema):
> schema['properties'].setdefault('status', True)
>
Re: [PATCH V3 2/2] dt-bindings: nvmem: u-boot,env: add Broadcom's variant binding
On Tue, 18 Oct 2022 17:42:02 +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki
>
> Broadcom uses U-Boot for a lot of their bcmbca familiy chipsets. U-Boot
> stores its configuration in an environment data block.
>
> Such blocks are usually stored on flash as a separated partition at
> hardcoded address. Broadcom however decided to:
> 1. Store env data block inside U-Boot partition
> 2. Avoid sticking to hardcoded offsets
> 3. Use custom header with "uEnv" magic and env data length
>
> Example (length 0x4000):
> $ hexdump -n 32 -C -s 0x4 /dev/mtdblock0
> 0004 76 6e 45 75 00 40 00 00 34 89 7a 82 49 4d 41 47 |vn...@..4.z.imag|
> 00040010 45 3d 4e 41 4e 44 3a 31 4d 2c 31 30 32 34 4d 00 |E=NAND:1M,1024M.|
> (0x4 offset is unit specific and can change)
>
> Starting with the commit 118f3fbe517f4 ("dt-bindings: mtd: partitions:
> support label/name only partition") DT can describe partitions matching
> them by a name (without specifying actual address). With that feature
> and this binding change it's possible to:
> 1. Specify DT node for Broadcom's U-Boot env data subpartition
> 2. Add nodes for specific environment data variables
> 3. Reference them as NVMEM cells
>
> This binding is unlikely to help Broadcom's U-Boot. U-Boot SPL needs to
> find environment data early (before it accesses DTB) and it does that by
> looking for an "uEnv" magic. Dirty way.
>
> This binding can however be used by operating systems. It allows
> describing cleanly U-Boot, its env data and variables. It tells
> operating system about Broadcom-specific env data so it can parse it.
>
> Signed-off-by: Rafał Miłecki
> ---
> V2: Work on better commit body & add example
> V3: Avoid duplicated "mac" label
> ---
> .../devicetree/bindings/nvmem/u-boot,env.yaml | 21 +++
> 1 file changed, 21 insertions(+)
>
Applied, thanks!
Re: [PATCH V3 1/2] dt-bindings: mtd: partitions: u-boot: allow dynamic subpartitions
On Tue, 18 Oct 2022 17:42:01 +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki
>
> U-Boot partition may contain subpartitions. For example Broadcom
> includes environment data block in the middle of its U-Boot partition.
>
> This allows describing Broadcom's U-Boot env data and will allow
> referencing its NVMEM cell in the future.
>
> Reg: 118f3fbe517f4 ("dt-bindings: mtd: partitions: support label/name only
> partition")
> Ref: dd638202dfb65 ("dt-bindings: mtd: partitions: add additional example for
> qcom,smem-part")
> Signed-off-by: Rafał Miłecki
> ---
> V3: Add this patch as pre-requirement for u-boot,env.yaml & brcm,env
> ---
> .../devicetree/bindings/mtd/partitions/u-boot.yaml | 7 +++
> 1 file changed, 7 insertions(+)
>
Applied, thanks!
Re: [PATCH V2] dt-bindings: nvmem: u-boot, env: add Broadcom's variant binding
On Tue, Oct 18, 2022 at 9:10 AM Conor Dooley wrote:
>
> On Tue, Oct 18, 2022 at 03:58:38PM +0200, Rafał Miłecki wrote:
> > On 18.10.2022 12:19, Conor Dooley wrote:
> > > On Fri, Oct 14, 2022 at 04:09:40PM -0500, Rob Herring wrote:
> > > > On Fri, 30 Sep 2022 18:36:31 +0200, Rafał Miłecki wrote:
> > > > > From: Rafał Miłecki
> > > > >
> > > > > Broadcom uses U-Boot for a lot of their bcmbca familiy chipsets.
> > > > > U-Boot
> > > > > stores its configuration in an environment data block.
> > > > >
> > > > > Such blocks are usually stored on flash as a separated partition at
> > > > > hardcoded address. Broadcom however decided to:
> > > > > 1. Store env data block inside U-Boot partition
> > > > > 2. Avoid sticking to hardcoded offsets
> > > > > 3. Use custom header with "uEnv" magic and env data length
> > > > >
> > > > > Example (length 0x4000):
> > > > > $ hexdump -n 32 -C -s 0x4 /dev/mtdblock0
> > > > > 0004 76 6e 45 75 00 40 00 00 34 89 7a 82 49 4d 41 47
> > > > > |vn...@..4.z.imag|
> > > > > 00040010 45 3d 4e 41 4e 44 3a 31 4d 2c 31 30 32 34 4d 00
> > > > > |E=NAND:1M,1024M.|
> > > > > (0x4 offset is unit specific and can change)
> > > > >
> > > > > Starting with the commit 118f3fbe517f4 ("dt-bindings: mtd: partitions:
> > > > > support label/name only partition") DT can describe partitions
> > > > > matching
> > > > > them by a name (without specifying actual address). With that feature
> > > > > and this binding change it's possible to:
> > > > > 1. Specify DT node for Broadcom's U-Boot env data subpartition
> > > > > 2. Add nodes for specific environment data variables
> > > > > 3. Reference them as NVMEM cells
> > > > >
> > > > > This binding is unlikely to help Broadcom's U-Boot. U-Boot SPL needs
> > > > > to
> > > > > find environment data early (before it accesses DTB) and it does that
> > > > > by
> > > > > looking for an "uEnv" magic. Dirty way.
> > > > >
> > > > > This binding can however be used by operating systems. It allows
> > > > > describing cleanly U-Boot, its env data and variables. It tells
> > > > > operating system about Broadcom-specific env data so it can parse it.
> > > > >
> > > > > Signed-off-by: Rafał Miłecki
> > > > > ---
> > > > > V2: Work on better commit body & add example
> > > > > ---
> > > > > .../devicetree/bindings/nvmem/u-boot,env.yaml | 21
> > > > > +++
> > > > > 1 file changed, 21 insertions(+)
> > > > >
> > > >
> > > > Applied, thanks!
> > >
> > > Hey Rob,
> > > Maybe my tooling is out of date or w/e but this is breaking
> > > dt_binding_check for me.
> > >
> > > I applied the below to fix the build, which I was about to send, before
> > > realising that you'd applied it and wondered if I was missing something.
> >
> > Thanks for catching that and submitting a fix!
>
> No worries.
>
> >
> > I guess I didn't run dt_binding_check this time or I did it before
> > adding an example. Sorry for that!
>
> BTW, subsequent to sending the fix I double checked my dt_binding_check
> logs and I saw:
> Documentation/devicetree/bindings/nvmem/u-boot,env.example.dtb: partition@0:
> Unevaluated properties are not allowed ('partition-u-boot-env' was unexpected)
> From schema:
> Documentation/devicetree/bindings/mtd/partitions/u-boot.yaml
>
> I think unevaluated property detection got better in v2022.08 of
> dt-schema so that is probably worth fixing too. I'll leave that one up
> to you ;)
Sigh. The simple fix is add 'partition-u-boot-env' to u-boot.yaml. But
now I have no idea if that is really complete as this Broadcom stuff
is coming in bit by bit. So I've now just dropped everything. Please
resend with it all fixed.
Rob
Re: [PATCH V2] dt-bindings: nvmem: u-boot, env: add Broadcom's variant binding
On Tue, Oct 18, 2022 at 5:19 AM Conor Dooley wrote:
>
> On Fri, Oct 14, 2022 at 04:09:40PM -0500, Rob Herring wrote:
> > On Fri, 30 Sep 2022 18:36:31 +0200, Rafał Miłecki wrote:
> > > From: Rafał Miłecki
> > >
> > > Broadcom uses U-Boot for a lot of their bcmbca familiy chipsets. U-Boot
> > > stores its configuration in an environment data block.
> > >
> > > Such blocks are usually stored on flash as a separated partition at
> > > hardcoded address. Broadcom however decided to:
> > > 1. Store env data block inside U-Boot partition
> > > 2. Avoid sticking to hardcoded offsets
> > > 3. Use custom header with "uEnv" magic and env data length
> > >
> > > Example (length 0x4000):
> > > $ hexdump -n 32 -C -s 0x4 /dev/mtdblock0
> > > 0004 76 6e 45 75 00 40 00 00 34 89 7a 82 49 4d 41 47
> > > |vn...@..4.z.imag|
> > > 00040010 45 3d 4e 41 4e 44 3a 31 4d 2c 31 30 32 34 4d 00
> > > |E=NAND:1M,1024M.|
> > > (0x4 offset is unit specific and can change)
> > >
> > > Starting with the commit 118f3fbe517f4 ("dt-bindings: mtd: partitions:
> > > support label/name only partition") DT can describe partitions matching
> > > them by a name (without specifying actual address). With that feature
> > > and this binding change it's possible to:
> > > 1. Specify DT node for Broadcom's U-Boot env data subpartition
> > > 2. Add nodes for specific environment data variables
> > > 3. Reference them as NVMEM cells
> > >
> > > This binding is unlikely to help Broadcom's U-Boot. U-Boot SPL needs to
> > > find environment data early (before it accesses DTB) and it does that by
> > > looking for an "uEnv" magic. Dirty way.
> > >
> > > This binding can however be used by operating systems. It allows
> > > describing cleanly U-Boot, its env data and variables. It tells
> > > operating system about Broadcom-specific env data so it can parse it.
> > >
> > > Signed-off-by: Rafał Miłecki
> > > ---
> > > V2: Work on better commit body & add example
> > > ---
> > > .../devicetree/bindings/nvmem/u-boot,env.yaml | 21 +++
> > > 1 file changed, 21 insertions(+)
> > >
> >
> > Applied, thanks!
>
> Hey Rob,
> Maybe my tooling is out of date or w/e but this is breaking
> dt_binding_check for me.
>
> I applied the below to fix the build, which I was about to send, before
> realising that you'd applied it and wondered if I was missing something.
Indeed, it is broken. I've applied your fix. Thanks.
Rob
Re: [PATCH V2] dt-bindings: nvmem: u-boot,env: add Broadcom's variant binding
On Fri, 30 Sep 2022 18:36:31 +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki
>
> Broadcom uses U-Boot for a lot of their bcmbca familiy chipsets. U-Boot
> stores its configuration in an environment data block.
>
> Such blocks are usually stored on flash as a separated partition at
> hardcoded address. Broadcom however decided to:
> 1. Store env data block inside U-Boot partition
> 2. Avoid sticking to hardcoded offsets
> 3. Use custom header with "uEnv" magic and env data length
>
> Example (length 0x4000):
> $ hexdump -n 32 -C -s 0x4 /dev/mtdblock0
> 0004 76 6e 45 75 00 40 00 00 34 89 7a 82 49 4d 41 47 |vn...@..4.z.imag|
> 00040010 45 3d 4e 41 4e 44 3a 31 4d 2c 31 30 32 34 4d 00 |E=NAND:1M,1024M.|
> (0x4 offset is unit specific and can change)
>
> Starting with the commit 118f3fbe517f4 ("dt-bindings: mtd: partitions:
> support label/name only partition") DT can describe partitions matching
> them by a name (without specifying actual address). With that feature
> and this binding change it's possible to:
> 1. Specify DT node for Broadcom's U-Boot env data subpartition
> 2. Add nodes for specific environment data variables
> 3. Reference them as NVMEM cells
>
> This binding is unlikely to help Broadcom's U-Boot. U-Boot SPL needs to
> find environment data early (before it accesses DTB) and it does that by
> looking for an "uEnv" magic. Dirty way.
>
> This binding can however be used by operating systems. It allows
> describing cleanly U-Boot, its env data and variables. It tells
> operating system about Broadcom-specific env data so it can parse it.
>
> Signed-off-by: Rafał Miłecki
> ---
> V2: Work on better commit body & add example
> ---
> .../devicetree/bindings/nvmem/u-boot,env.yaml | 21 +++
> 1 file changed, 21 insertions(+)
>
Applied, thanks!
Re: [PATCH v3] schemas: Add schema for U-Boot driver model 'phase tags'
On Tue, Oct 4, 2022 at 6:22 PM Simon Glass wrote:
>
> Until recently it has not been possible to add any U-Boot-specific
> properties to the device tree schema. Now that it is somewhat separated
> from Linux and people are feeling the enormous pain of the bifurcated
> schema, it seems like a good time to add this and other U-Boot-specific
> bindings.
It's been possible provided there was agreement on the properties.
There just wasn't in this case.
What's the pain point precisely? I can think of several. Syncing dts
files from Linux tree, running schema validation on a dtb from u-boot,
or ...?
> U-Boot has some particular challenges with device tree and devices which
> are not faced with Linux:
>
> - U-Boot has multiple build phases, such as a Secondary Program Loader
> (SPL) phase which typically runs in a pre-SDRAM environment where code
> and data space are limited. In particular, there may not be enough
> space for the full device tree blob. U-Boot uses various automated
> techniques to reduce the size from perhaps 40KB to 3KB.
As this is a build time thing, would this be better handled as a
source annotation rather than properties? We already have a 'delete if
unreferenced' tag for similar reasons. We could add something more
flexible like '/tag spl, tpl/' and dtc could have command line options
to keep or delete nodes based on tag names. (Added David G for his
thoughts on this)
Yes, there is also a runtime need for a portion of this, but I think
it's 2 different problems AIUI.
> - Some U-Boot phases needs to run before the clocks are properly set up,
> where the CPU may be running very slowly. Therefore it is important to
> bind only those devices which are actually needed in that phase
> - Unlike Linux or UEFI, U-Boot uses lazy initialisation for its devices,
> with 'bind' and 'probe' being separate steps. Even if a device is bound,
> it is not actually probed until it is used. This is necessary to keep
> the boot time reasonable, e.g. to under a second
Linux could do this now if we wanted. There's a full dependency graph.
Once you have that, it's just an implementation decision whether you
probe top down or bottom up. We have this graph because Linux specific
probing hint properties in DT was rejected. (Not saying u-boot needs
to go implement a dependency graph, but rather u-boot is not unique
here and there's more than one way to solve it.)
> The phases of U-Boot in order are: TPL, VPL, SPL, U-Boot (first
> pre-relocation, then post-relocation). ALl but the last two are optional.
>
> For the above reasons, U-Boot only includes the full device tree in the
> final 'U-Boot proper' build. Even then, before relocation U-Boot only
> processes nodes which are marked as being needed.
>
> For this to work, U-Boot's driver model[1] provides a way to mark device
> tree nodes as applicable for a particular phase. This works by adding a
> tag to the node, e.g.:
>
>cru: clock-controller@ff76 {
> u-boot,dm-all;
> compatible = "rockchip,rk3399-cru";
> reg = <0x0 0xff76 0x0 0x1000>;
> rockchip,grf = <>;
> #clock-cells = <1>;
> #reset-cells = <1>;
> ...
>};
>
> Here the "u-boot,dm-all" tag indicates that the node must be present in
> all phases, since the clock driver is required
>
> There has been discussion over the years about whether this could be done
> in a property instead, e.g.
>
>options {
> u-boot,dm-all = <> <_a> ...;
> ...
>};
>
> Some problems with this:
>
> - we need to be able to merge several such tags from different .dtsi files
> since many boards have their own specific requirements
A source annotation to append/prepend properties could solve that.
> - it is hard to find and cross-reference the affected nodes
> - it is more error-prone
> - it requires significant tool rework in U-Boot, including fdtgrep and
> the build system
> - is harder (slower, more code) to process since it involves scanning
> another node/property to find out what to do with a particular node
> - we don't want to add phandle arguments to the above since we are
> referring, e.g., to the clock device as a whole, not a paricular clock
> - the of-platdata feature[2], which converts device tree to C for even
> more constrained environments, would need to become aware of the
> /options node
>
> There is also the question about whether this needs to be U-Boot-specific,
> or whether the tags could be generic. From what I can tell, U-Boot is the
> only bootloader which seriously attempts to use a runtime device tree in
> all cases. We could perhaps adopt U-Boot's naming for the phases and drop
> the "u-boot," prefix, but that might cause confusion.
I am concerned about the u-boot adding your own properties, and then
the next consumer wanting to add something similar. Even dropping
'u-boot,' the properties seem pretty u-boot specific. Of them, SPL
seems like the least specific as that's a pre-DRAM state. VPL/TPL
Re: [PATCH] dt-bindings: nvmem: u-boot,env: add basic NVMEM cells
On Sun, 03 Jul 2022 10:48:43 +0200, Rafał Miłecki wrote: > From: Rafał Miłecki > > U-Boot doesn't have cells at hardcoded addresses. They are stored in > internal format. It's still important to define relevant cells in DT so > NVMEM consumers can reference them. > > Update binding to allow including basic cells as NVMEM device subnodes. > > Signed-off-by: Rafał Miłecki > --- > .../devicetree/bindings/nvmem/u-boot,env.yaml | 13 + > 1 file changed, 13 insertions(+) > Sorry, I guess I was thinking this depended on or interacted with one of the other nvmem series. Applied, thanks! Rob
Re: ethernet dt aliases implications in U-Boot and Linux
On Tue, Aug 9, 2022 at 4:39 PM Tim Harvey wrote:
>
> On Tue, Aug 9, 2022 at 2:31 PM Pali Rohár wrote:
> >
> > On Tuesday 09 August 2022 16:48:23 Sean Anderson wrote:
> > > On 8/8/22 5:45 PM, Michal Suchánek wrote:
> > > > On Mon, Aug 08, 2022 at 02:38:35PM -0700, Stephen Hemminger wrote:
> > > >> On Mon, 8 Aug 2022 23:09:45 +0200
> > > >> Michal Suchánek wrote:
> > > >>
> > > >> > On Mon, Aug 08, 2022 at 03:57:55PM -0400, Sean Anderson wrote:
> > > >> > > Hi Tim,
> > > >> > >
> > > >> > > On 8/8/22 3:18 PM, Tim Harvey wrote:
> > > >> > > > Greetings,
> > > >> > > >
> > > >> > > > I'm trying to understand if there is any implication of
> > > >> > > > 'ethernet'
> > > >> > > > aliases in Linux such as:
> > > >> > > > aliases {
> > > >> > > > ethernet0 =
> > > >> > > > ethernet1 =
> > > >> > > > ethernet2 =
> > > >> > > > ethernet3 =
> > > >> > > > ethernet4 =
> > > >> > > > ethernet5 =
> > > >> > > > ethernet6 =
> > > >> > > > };
> > > >> > > >
> > > >> > > > I know U-Boot boards that use device-tree will use these aliases
> > > >> > > > to
> > > >> > > > name the devices in U-Boot such that the device with alias
> > > >> > > > 'ethernet0'
> > > >> > > > becomes eth0 and alias 'ethernet1' becomes eth1 but for Linux it
> > > >> > > > appears that the naming of network devices that are embedded (ie
> > > >> > > > SoC)
> > > >> > > > vs enumerated (ie pci/usb) are always based on device
> > > >> > > > registration
> > > >> > > > order which for static drivers depends on Makefile linking order
> > > >> > > > and
> > > >> > > > has nothing to do with device-tree.
> > > >> > > >
> > > >> > > > Is there currently any way to control network device naming in
> > > >> > > > Linux
> > > >> > > > other than udev?
Ah, the topic that will never die.
> > > >> > >
> > > >> > > You can also use systemd-networkd et al. (but that is the same
> > > >> > > kind of mechanism)
> > > >> > >
> > > >> > > > Does Linux use the ethernet aliases for anything at all?
> > > >> > >
> > > >> > > No :l
> > > >> >
> > > >> > Maybe it's a great opportunity for porting biosdevname to DT based
> > > >> > platforms ;-)
> > > >>
> > > >> Sorry, biosdevname was wrong way to do things.
> > > >> Did you look at the internals, it was dumpster diving as root into
> > > >> BIOS.
> > > >
> > > > When it's BIOS what defines the names then you have to read them from
> > > > the BIOS. Recently it was updated to use some sysfs file or whatver.
> > > > It's not like you would use any of that code with DT, anyway.
> > > >
> > > >> Systemd-networkd does things in much more supportable manner using
> > > >> existing
> > > >> sysfs API's.
> > > >
> > > > Which is a dumpster of systemd code, no thanks.
> > > >
> > > > I want my device naming independent of the init system, especially if
> > > > it's systemd.
> > >
> > > Well, there's always nameif...
> > >
> > > That said, I have made [1] for people using systemd-networkd.
> > >
> > > --Sean
> > >
> > > [1] https://github.com/systemd/systemd/pull/24265
> >
> > Hello!
> >
> > In some cases "label" DT property can be used also as interface name.
> > For example this property is already used by DSA kernel driver.
> >
> > I created very simple script which renames all interfaces in system to
> > their "label" DT property (if there is any defined).
> >
> > #!/bin/sh
> > for iface in `ls /sys/class/net/`; do
> > for of_node in of_node device/of_node; do
> > if test -e /sys/class/net/$iface/$of_node/; then
> > label=`cat /sys/class/net/$iface/$of_node/label
> > 2>/dev/null`
> > if test -n "$label" && test "$label" != "$iface";
> > then
> > echo "Renaming net interface $iface to
> > $label..."
> > up=$((`cat /sys/class/net/$iface/flags
> > 2>/dev/null || echo 1` & 0x1))
> > if test "$up" != "0"; then
> > ip link set dev $iface down
> > fi
> > ip link set dev $iface name "$label" &&
> > iface=$label
> > if test "$up" != "0"; then
> > ip link set dev $iface up
> > fi
> > fi
> > break
> > fi
> > done
> > done
> >
> > Maybe it would be better first to use "label" and then use ethernet alias?
>
> I've been wondering the same as well which made me wonder what the
> history of the 'aliases' node is and why its not used in most cases in
> Linux. I know for the SOC's I work with we've always defined aliases
> for ethernet, gpio, serial, spi, i2c, mmc etc. Where
> did this practice come from and why are we putting that in Linux dts
> files
Re: [PATCH 1/2] dt-bindings: mtd: partitions: add binding for U-Boot bootloader
On Mon, 11 Jul 2022 17:30:40 +0200, Rafał Miłecki wrote: > From: Rafał Miłecki > > Right now there is no (known) real reason for a custom binding for > standard U-Boot partitions. Broadcom's U-Boot however requires extra > handling - looking for environment variables subblocks. This commit adds > Broadcom specific binding. > > Signed-off-by: Rafał Miłecki > --- > .../bindings/mtd/partitions/u-boot.yaml | 49 +++ > 1 file changed, 49 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/u-boot.yaml > Reviewed-by: Rob Herring
Re: [PATCH v6 3/7] tpm: Add the RNG child device
On Wed, Jul 13, 2022 at 9:28 AM Simon Glass wrote:
>
> Hi Rob,
>
> On Tue, 12 Jul 2022 at 08:11, Rob Herring wrote:
> >
> > On Tue, Jul 12, 2022 at 5:04 AM Simon Glass wrote:
> > >
> > > Hi Ilias,
> > >
> > > On Fri, 8 Jul 2022 at 02:24, Ilias Apalodimas
> > > wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > [...]
> > > >
> > > > > > +
> > > > > > UCLASS_DRIVER(tpm) = {
> > > > > > - .id = UCLASS_TPM,
> > > > > > - .name = "tpm",
> > > > > > - .flags = DM_UC_FLAG_SEQ_ALIAS,
> > > > > > + .id = UCLASS_TPM,
> > > > > > + .name = "tpm",
> > > > > > + .flags = DM_UC_FLAG_SEQ_ALIAS,
> > > > > > #if CONFIG_IS_ENABLED(OF_REAL)
> > > > > > - .post_bind = dm_scan_fdt_dev,
> > > > > > + .post_bind = dm_scan_fdt_dev,
> > > > > > #endif
> > > > > > + .post_probe = tpm_uclass_post_probe,
> > > > > > .per_device_auto= sizeof(struct tpm_chip_priv),
> > > > > > };
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > > >
> > > > > The driver needs a compatible string so it can be in the device tree.
> > > >
> > > > Why? I've tried to hint this on the previous iteration of the patch.
> > > > The RNG here is not a *device*. The TPM is the device and you are
> > > > guaranteed to have an RNG. The way to get a random number is send a
> > > > special command to the TPM. So all that we should do here is leverage
> > > > the fact that the TPM is already in the device tree.
> > > >
> > > > And fwiw we should stick to try to stick to what the DT spec defines
> > > > as much as possible. I personally don't see this as a special usecase
> > > > were deviating from the spec is justified.
> > >
> > > This is not a deviation from a spec. What spec? Also, I don't want to
> > > get into another discussion about what a device is. We can disagree on
> > > that if you like.
> > >
> > > One reason is that U-Boot generally requires compatible strings, e.g.
> > > with of-platdata. But also we can refer to the rand device from
> > > elsewhere in the tree. I know that Linux does lots of ad-hoc device
> > > creation and doesn't really have the concept of a uclass consistently
> > > applied, but this is U-Boot.
> >
> > You are letting client/OS details define your binding. Doing so
> > doesn't result in OS agnostic bindings. Sure, it would be nice if DT
> > nodes and drivers were always a nice clean 1:1 ratio, but h/w is messy
> > sometimes and DT is not an abstraction layer. The general guidance on
> > whether there are child nodes for sub-blocks is do they have their own
> > resources in DT or are the sub-blocks reusable on multiple devices.
> > Neither of those are the case here.
> >
> > Besides, we already have TPM device bindings defined. Requiring
> > binding changes when adding a new client/OS feature is not good
> > practice either.
>
> I'm not sure what to do with this, but in general the practice of
> implied subnodes is not friendly to U-Boot. Yet it seems like a common
> feature of the bindings at present, for example GPIOs.
It's perfectly normal for a single device to be multiple providers. A
common example is clock AND reset blocks. They are both a clock and
reset provider. So you could have either:
crm {
compatible = "foo,soc-crm";
clock-controller {
compatible = "foo,soc-crm-clocks";
#clock-cells = <1>;
};
reset-controller {
compatible = "foo,soc-crm-resets";
#reset-cells = <1>;
};
};
or:
crm {
compatible = "foo,soc-crm";
#clock-cells = <1>;
#reset-cells = <1>;
};
Which one's right? Well, depends on the h/w really. If clock and reset
controls are interleaved bits or registers, then the first case really
doesn't make sense.
In any case, this ship has sailed. There are tons of the latter case
already, so either you have to figure out support 1 DT node to N
drivers or you're going to be carrying a bunch of your own u-boot
bindings.
> The device tree is how U-Boot determines which random-number generator
> to use for a particular function. For example, for VBE, if we need to
> generate some random numbers we'd like to specify which device creates
> them. It can't just be 'use the TPM if you find one'.
Why not?
> I'm not sure how that works in Linux?
Not sure. I think Linux's answer is use all of the RNG sources and
none of them directly.
Rob
