Re: [PATCH 0/5] Add support for resolving path references in overlays

[Ayush Singh <ayush@beagleboard.org>]

On 11/16/24 20:30, Ayush Singh wrote:

> dts allows references both in integer context:
> 	foo = <&bar>;
> in which case it resolves to a phandle, but also in string/bytestring
> context:
> 	foo = &bar;
> In which case it resolves to a path.
>
> Runtime overlays, only support the former, but not the latter. The
> following patch attempts to solve this asymmetry.
>
> Additionally, `__symbols__` does not support phandles, which
> makes overlays modifying `__symbols__` rather limiting. More context
> regarding this patch can be found here[0].
>
> Implementation
> **************
>
> Overlay
> =======
>
> Properties to path references in overlays are left empty in the compiled
> binary blob. Only `__fixups__` entry is generated for such properties.
> This makes it simple to distinguish between phandles and paths when
> applying the overlay (and also makes overlays small).
>
> Application
> ===========
>
> I have divided the overlay application into 2 stages.
>
> 1. Overlay prepare (`fdt_overlay_prepare`)
>
> This step prepares the overlay for merging with the base device tree. In
> At this stage, the base device tree is passed as read-only and only the
> overlay is modified and resized.
>
> Additionally, since any resizing will invalidate the offsets and
> property references, I am creating a read-only copy of the table while
> resolving the path references.
>
> 2. Overlay application (`fdt_overlay_apply`)
>
> Performs the actual merging to base tree. The overlay is read-only at this
> stage.
>
> Limitations
> ***********
>
> 1. Local Path references
>
> Currently, this patch series only implements path references to base
> devicetree. This means local path references are still broken. I am
> working on adding support for that using `__local_fixups__` but it is
> not ready yet.
>
> 2. Breaking change for utilities
>
> This is a breaking change for any utilities using `fdt_overlay_apply`
> since I have moved some of it's functionality to `fdt_overlay_prepare`.
> Not really sure how important this is.
>
> If it is not desirable, I do have 2 ways to avoid it.
>
>    a. Just expand both overlay and base device tree when `FDT_ERR_NOSPACE`
>       is returned. A bit wasteful, but probably not a big deal.
>
>    b. Add new error variant `FDT_ERR_OVERLAY_NOSPACE`.
>
> Alternatives
> ************
>
> Some alternative approaches that were considered:
>
> 1. Using aliases.
>
> Currently, it is not possible to update aliases in device tree overlays.
> I sent a patch a few weeks ago to add this support [1]. However, as was
> outlined by Rob, this can break existing drivers that used the unused
> indexes for devices not present in the aliases list.
>
> 2. Add support for phandles in `__symbols__`
>
> This has been discussed in the following patch series [2]. However,
> since there is no way to distinguish between strings and phandles in
> devicetree (everything is bytestring), the type guessing is awkward.
>
> [0]: https://lore.kernel.org/devicetree-compiler/44bfc9b3-8282-4cc7-8d9a-7292cac663ef@ti.com/T/#mf0f6ae4db0848f725ec6e2fb625291fa0d4eec71
> [1]: https://lore.kernel.org/all/20241110-of-alias-v2-0-16da9844a93e@beagleboard.org/T/#t
> [2]: https://lore.kernel.org/devicetree-compiler/44bfc9b3-8282-4cc7-8d9a-7292cac663ef@ti.com/T/#mbbc181b0ef394b85b76b2024d7e209ebe70f7003
>
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
>
> ---
> Ayush Singh (5):
>        dtc: Allow path fixups in overlays
>        libfdt: Add namelen variants for setprop
>        fdtoverlay: Implement resolving path references
>        tests: Fix overlay tests
>        tests: Add path tests for overlay
>
>   checks.c                                |   6 +-
>   fdtoverlay.c                            |  34 +++++--
>   libfdt/fdt_overlay.c                    | 174 +++++++++++++++++++-------------
>   libfdt/fdt_rw.c                         |  19 ++++
>   libfdt/libfdt.h                         |  93 ++++++++++++++++-
>   libfdt/version.lds                      |   1 +
>   livetree.c                              |  19 +++-
>   tests/overlay.c                         |   1 +
>   tests/overlay_bad_fixup.c               |   2 +-
>   tests/overlay_overlay.dts               |  11 ++
>   tests/overlay_overlay_manual_fixups.dts |  26 ++++-
>   tests/overlay_overlay_nosugar.dts       |  19 ++++
>   util.h                                  |   1 +
>   13 files changed, 321 insertions(+), 85 deletions(-)
> ---
> base-commit: 2d10aa2afe35527728db30b35ec491ecb6959e5c
> change-id: 20241114-overlay-path-d9980477f76a
>
> Best regards,


cc David Gibson <david@gibson.dropbear.id.au>


Sorry for missing the maintainer of dtc and fdt in the initial patches.


Ayush Singh