Re: [PATCH] schemas: Add schema for U-Boot driver model 'phase tags'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Heinrich,

On Thu, 29 Sept 2022 at 15:00, Heinrich Schuchardt <xypron.glpk@xxxxxx> wrote:
>
> On 9/29/22 22:11, Simon Glass wrote:
> > +U-Boot Custodians too
> >
> > On Thu, 29 Sept 2022 at 13:58, Simon Glass <sjg@xxxxxxxxxxxx> 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.
> >>
> >> 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.
> >> - 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
> >>
> >> 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@ff760000 {
> >>        u-boot,dm-all;
> >>        compatible = "rockchip,rk3399-cru";
> >>        reg = <0x0 0xff760000 0x0 0x1000>;
> >>        rockchip,grf = <&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 = <&cru> <&gpio_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. We could perhaps adopt U-Boot's naming for the phases and drop
> >> the "u-boot," prefix, but that might cause confusion.
> >>
> >> 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 <sjg@xxxxxxxxxxxx>
> >> ---
> >>
> >>   dtschema/schemas/u-boot.yaml | 48 ++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 48 insertions(+)
> >>   create mode 100644 dtschema/schemas/u-boot.yaml
> >>
> >> diff --git a/dtschema/schemas/u-boot.yaml b/dtschema/schemas/u-boot.yaml
>
> I guess we will more files to describe device-tree additions by U-Boot.
> I would suggest to use a folder schemas/u-boot/ with a file xpl-select.yaml.

That's OK with me, but I'd like to get some guidance from Rob. Can we
put these things in a subdir?

I have a test failure on master and this patch produces some extra
errors so I cannot really try it out properly.

BTW I have a series for U-Boot to tidy things up, but I want to get
some eyes on this patch (the v2 one I just sent) first.

>
> >> new file mode 100644
> >> index 0000000..6c5c820
> >> --- /dev/null
> >> +++ b/dtschema/schemas/u-boot.yaml
> >> @@ -0,0 +1,48 @@
> >> +# SPDX-License-Identifier: BSD-2-Clause
> >> +# Copyright 2022 Google LLC
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/serial.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Bindings required by U-Boot driver model
> >> +
> >> +maintainers:
> >> +  - Simon Glass <sjg@xxxxxxxxxxxx>
> >> +
> >> +patternProperties:
> >> +  "^u-boot,dm-(tpl|vpl|spl|all|pre-reloc)$":
>
> Please, describe the meaning of each suffix individually.
> Especially the meaning of 'all' and 'pre-reloc' remains unclear in the
> text below.

OK done in v2

>
> >> +    type: boolean
> >> +    description: |
> >> +      The device tree is often quite large (e.g. 40KB) and cannot fit into xPL
> >> +      phases like SPL, TPL. Even with U-Boot proper, we normally only want to
> >> +      init a small subset of devices before relocation.
> >> +
> >> +      U-Boot supports adding tags to device tree nodes to allow them to be
> >> +      marked according to the U-Boot phases where they should be included.
> >> +
> >> +      Without any tags, nodes are included only in U-Boot proper after
> >> +      relocation. Any untagged nodes are dropped from xPL and are ignored before
> >> +      relocation in U-Boot proper.
> >> +
> >> +      The xPL builds use fdtgrep drop any nodes which are not needed for that
> >> +      build. For example, TPL will drop any nodes which are not marked with
> >> +      u-boot,dm-tpl or u-boot,dm-all tags.
> >> +
> >> +      Note that xPL builds also drop the tags, since they have server their
>
> %s/server/served/
>
> >> +      purpose by that point. So when looking at xPL device tree files you will
> >> +      not see these tags. The means, for example, that ofnode_pre_reloc()
>
> %s/The/This/
>
> >> +      returns true always in xPL builds. This is done to save space.
>
> %s/returns true always/always returns true/
>
> >> +
> >> +      Multiple tags can be used in the same node.
> >> +
> >> +      One complication is that tags apply only to the node they are added to,
> >> +      not to any parents. This means that you often need to add the same tag to
> >> +      parent nodes so that any properties needed by the parent driver are
> >> +      included. Without that, the parent node may have no properties, or may not
> >> +      be bound before relocation (meaning that its child will not be either).
>
> %s/either/bound either/
>
> Best regards
>
> Heinrich
>
> >> +
> >> +      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.
> >> +
> >> +additionalProperties: true
> >> --
> >> 2.38.0.rc1.362.ged0d419d3c-goog
> >>
>

Regards,
Simon



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux