From: "Uwe Kleine-König" <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
To: David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>,
Jon Loeliger <loeliger-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Yves-Alexis Perez
<corsac-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>,
Maxime Ripard <mripard-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: Re: ftdoverlay overwrites phandle
Date: Mon, 4 Jul 2022 22:04:50 +0200 [thread overview]
Message-ID: <20220704200450.kd2oipq6lhnokabv@pengutronix.de> (raw)
In-Reply-To: <20220629205925.v56wxrvib33tgu65-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 5021 bytes --]
Hello,
[Cc += Maxime who seems to have done some overlay work in dtc]
On Wed, Jun 29, 2022 at 10:59:27PM +0200, Uwe Kleine-König wrote:
> I want to apply an overlay to a device tree that is compiled without -@
> because that's how dtbs are shipped by the Debian kernel package.
> I'm using dtc 1.6.1-1 as provided by Debian.
>
> The machine dtb is bcm2836-rpi-2-b.dtb[1]. The source for the dtbo looks
> as follows:
>
> uwe@taurus:~/tmp$ cat dht11-overlay.dts
> /*
> * Overlay for the DHT11/21/22 humidity/temperature sensor modules.
> */
> /dts-v1/;
> /plugin/;
>
> / {
> compatible = "brcm,bcm2708";
>
> fragment@0 {
> target-path = "/";
>
> __overlay__ {
> dht11: dht11@0 {
> compatible = "dht11";
> pinctrl-names = "default";
> pinctrl-0 = <&dht11_pins>;
> gpios = <&gpio 4 0>;
> status = "okay";
> };
> };
> };
>
> fragment@1 {
> target-path = "/soc/";
> __overlay__ {
> gpio: gpio@7e200000 {
> dht11_pins: dht11_pins {
> brcm,pins = <4>;
> brcm,function = <0>; // in
> brcm,pull = <0>; // off
> };
> };
> };
> };
>
> __overrides__ {
> gpiopin = <&dht11_pins>,"brcm,pins:0",
> <&dht11>,"gpios:4";
> };
> };
>
> which is more or less the overlay with the same name provided by the
> RaspberryPi Foundation[2].
>
> I can compile and apply the overlay just fine (well there are warnings,
> but ...):
>
> uwe@taurus:~/tmp$ dtc -I dts -O dtb dht11-overlay.dts > dht11-overlay.dtbo
> dht11-overlay.dts:14.19-20.6: Warning (unit_address_vs_reg): /fragment@0/__overlay__/dht11@0: node has a unit name, but no reg or ranges property
> dht11-overlay.dts:27.24-33.6: Warning (unit_address_vs_reg): /fragment@1/__overlay__/gpio@7e200000: node has a unit name, but no reg or ranges property
> dht11-overlay.dts:14.19-20.6: Warning (gpios_property): /fragment@0/__overlay__/dht11@0: Missing property '#gpio-cells' in node /fragment@1/__overlay__/gpio@7e200000 or bad phandle (referred from gpios[0])
> uwe@taurus:~/tmp$ fdtoverlay -i bcm2836-rpi-2-b.dtb -o bcm2836-rpi-2-b+dht11.dtb dht11-overlay.dtbo
>
> However there is a problem: The original bcm2836-rpi-2-b.dtb has:
>
> uwe@taurus:~/tmp$ fdtdump bcm2836-rpi-2-b.dtb
> ...
> gpio@7e200000 {
> ....
> phandle = <0x00000006>;
> ....
> hdmi@7e902000 {
> ...
> hpd-gpios = <0x00000006 0x0000002e 0x00000001>;
> ...
>
> the patched dtb has:
>
> ...
> dht11@0 {
> phandle = <0x0000001d>;
> status = "okay";
> gpios = <0x0000001c 0x00000004 0x00000000>;
> pinctrl-0 = <0x0000001b>;
> pinctrl-names = "default";
> compatible = "dht11";
> };
> ...
> gpio@7e200000 {
> phandle = <0x0000001c>;
> ....
> hdmi@7e902000 {
> ...
> hpd-gpios = <0x00000006 0x0000002e 0x00000001>;
> ...
>
> So the gpios property of the dht11 node points to the gpio node as
> expected, however the hpd-gpios property of the hdmi node is broken
> because the phandle of gpio@7e200000 changed.
>
> I think it should be possible to not overwrite the phandle of
> gpio@7e200000 and just reuse the value 6.
I thought a bit more about that and with the given design of fdtoverlay
this is hard because there are two types of (local) references: One
where you just have to add $delta (which is the only case that is
currently supported) and the other where the already existing number
from the dtb is reused.
An alternative would be to use a new node (say) __base_symbols__ that is
used to lookup symbols in the base dtb. E.g.:
/ {
compatible = "brcm,bcm2708";
fragment@0 {
target-path = "/";
__overlay__ {
dht11 {
compatible = "dht11";
pinctrl-names = "default";
pinctrl-0 = <0x00000001>;
gpios = <0xffffffff 0x00000004 0x00000000>;
status = "okay";
phandle = <0x00000002>;
};
};
};
__fixups__ {
gpio = "/fragment@0/__overlay__/dht11:gpios:0";
};
__base_symbols__ {
gpio = "/soc/gpio@7e200000";
};
};
then if the dtb that is supposed to be patched doesn't have gpio in its
__symbols__ node, this is looked up using __base_symbols__ in the dtbo.
Does this sound sensible? The downside compared to the first possibility
is that we have to extend the overlay device tree format.
A workaround I just noticed is: I could create two overlays and apply
them im a row. The first adds a __symbols__ node to the dtb and the
second uses the newly created node to properly fixup the local
reference.
What do you think?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2022-07-04 20:04 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-29 20:59 ftdoverlay overwrites phandle Uwe Kleine-König
[not found] ` <20220629205925.v56wxrvib33tgu65-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2022-07-04 20:04 ` Uwe Kleine-König [this message]
2022-07-12 15:53 ` Rob Herring
[not found] ` <CAL_JsqJbU6vUhxRodTSFmPjPUAvw3-qcu=usdPp9Ym43CM+t9A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-08-01 12:22 ` Uwe Kleine-König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220704200450.kd2oipq6lhnokabv@pengutronix.de \
--to=u.kleine-koenig-bicnvbalz9megne8c9+irq@public.gmane.org \
--cc=corsac-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org \
--cc=david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org \
--cc=devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=loeliger-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=mripard-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).