dgibson / dtc

Device Tree Compiler
235 stars 134 forks source link

allow using labels with fdtget/fdtput #89

Closed Villemoes closed 1 month ago

Villemoes commented 1 year ago

It can be convenient and more readable to use &label syntax with fdtget and fdtput, rather than having to know the full path. This small series adds that support by hooking into fdt_path_offset_namelen() in the same place where we already allow the first component to be an alias. Of course this only works for blobs that have been built with -@, but it shouldn't affect existing use cases.

Villemoes commented 1 year ago

Any comments?

The CI errors don't seem to be related to these changes, so I don't know if I'm supposed to do anything about them.

dgibson commented 1 year ago

Sorry, I'm finding it very difficult these days to find even minimal time to maintain dtc.

The concept for these patches looks good, I've put in a review for some of the details.

Villemoes commented 1 year ago

Branch updated. I think this should address the comments, but I may have been too verbose in the fdt_path_offset documentation update.

dgibson commented 1 year ago

On Fri, Apr 28, 2023 at 01:15:23AM -0700, Villemoes wrote:

@Villemoes commented on this pull request.

  • if (!can_assume(VALID_INPUT) && *p != '/')
  • return -FDT_ERR_BADVALUE;

I'm not 100% sure, but I believe it's also valid for aliases to reference other aliases. I couldn't quickly find absolute confirmation in IEEE1275, but I think this was valid and used in practice in the old OF days. For example:

/ {
        aliases {
                sa1 = "/foo/bar";
                sa2 = "sa1/baz";
        };
}

Well, if that's really the case, then of course my sanity check patch should be removed.

Ah, but https://www.devicetree.org/specifications/ is quite clear:

Eh.. not necessarily.

Each property of the ``/aliases`` node defines an alias. The property name
specifies the alias name. The property value specifies the full path to
a node in the devicetree.

and

An alias value is a device path and is encoded as a string. The value
represents the full path to a node, but the path does not need to refer
to a leaf node.

So, both of these look to be paraphrased from IEEE1275. The ambiguity, such as it is, is in the meaning of "full path". 1275 uses the term "device-path" in the equivalent place, and I think that term may allow the first component to be an alias, rather than '/', though I'm having trouble pinning down something which says that one way or another. I do see "device-path" used in other contexts of 1275 where I'm pretty certain aliases are allowed (e.g. the value of the 'bootpath' property of /chosen).

That text seems to be essentially unchanged since it first appeared in that git repo.

As you'd expect for something more or less inherited from 1275.

-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT the other | way around! http://www.ozlabs.org/~dgibson

dgibson commented 1 year ago

I've merged the first 4 patches. I need to think a bit more about the remaining two.

Villemoes commented 1 month ago

I have rebased the last two patches on top of current main. Can they be considered for inclusion?