raspberrypi / firmware

This repository contains pre-compiled binaries of the current Raspberry Pi kernel and modules, userspace libraries, and bootloader/GPU firmware.
5.14k stars 1.68k forks source link

Default device tree is not 100% upstream compatible #943

Closed agraf closed 6 years ago

agraf commented 6 years ago

There are a few differences that render devices unusable with an upstream kernel when using it with the firmware provided device tree:

With those bits fixed, I see little reason to use upstream kernel device trees to run upstream kernels and would like to switch over to such a model for openSUSE.

pelwell commented 6 years ago

Downstream device tree is closer to upstream than it's ever been, but it is different and is likely to remain so. We won't be switching to dwc2 as long as dwc-otg gives significantly better performance. Similarly we won't be enabling the vc4 driver by default until we decide that it is suitable for most users.

agraf commented 6 years ago

Yes, the SDHCI compatible surely is easy to fix though :).

As for the other 2 bits, I thought you had a way of determining whether a booted kernel is upstream or downstream (with default on upstream iirc)? In that case, wouldn't it make sense to just apply the vc4 and dwc2 overlays by default for upstream kernels?

pelwell commented 6 years ago

Yes, the SDHCI compatible surely is easy to fix though :).

Yes, and I will do that.

As for the other 2 bits, I thought you had a way of determining whether a booted kernel is upstream or downstream (with default on upstream iirc)?

Yes we do, but upstream folks didn't like it because it involved running a downstream script (scripts/mkimage) on the built kernel that decompresses it, looks for certain magic symbols and strings, and adds a small trailer with a few flags indicating its properties. If you run that now on your upstream kernel it will be marked as upstream, and the firmware will load the equivalent upstream .dtb instead.

We have considered adding a config option to say "I'm using upstream kernels", but nothing has happened yet. Another option would be to have a new kernel name format to indicate that it is upstream; the firmware could either look for the presence of such a kernel first (I'm not so keen because we already have a few variants to consider, but it wouldn't be hard), or it could do some pattern matching on the kernel=mumble.img line to allow it to load the appropriate .dtb.

agraf commented 6 years ago

Where would it fetch the upstream dtb files from? I only see "downstream" dtbs shipped in the boot directory.

Also, I don't think having 2 separate device trees is a good path forward, because they're bound to diverge in naming one way or another. So overlays won't easily apply on them. Wouldn't it be much easier to convert one to the other (like downstream -> upstream) using overlays? Then the maintenance burden should be heavily reduced.

agraf commented 6 years ago

As for the "downstream script", I agree that it feels wrong to run a script that indicates "I'm upstream compatible". What does make a lot of sense however is a script that runs on the downstream kernel, indicating "I'm not upstream compatible, use non-standard bindings please".

The only difference is that the default would be upstream bindings.

pelwell commented 6 years ago

Submit an upstream overlay pull request and I'll look on it favourably.

agraf commented 6 years ago

I can't do the plumbing of detecting what file to apply when, but I'll happily provide a PR with an overlay that moves the downstream DT to something upstream compatible :).

pelwell commented 6 years ago

There were enough users of the downstream kernel in custom distributions, buildroots etc. who objected to the script that we deprecated it. Unless we adopt a new naming convention for upstream kernels then I don't think any kind of auto-detection is going to be feasible.

agraf commented 6 years ago

I don't know if a naming convention is the right way to go - if you go by name you may as well just introduce a new upstream=1 option.

Apart from existing device tree overlays (vc4, dwc2) I only need to overlay the AUX interrupt parent and SDHCI compatible to make the DT work fully with upstream (on rpi3). You mentioned you will fix the SDHCI compatible, so that means we're only left with the AUX interrupt parent.

I guess it makes sense to have 3 overlays for upstream kernels then?

pelwell commented 6 years ago

The upstream compatible string is now in the rpi-4.14.y and rpi-4.15.y branches.

An AUX overlay would be very niche since it is enabled by default downstream, but it would be so small I'm happy to merge one.

agraf commented 6 years ago

With all these bits in place, we should be able to use the upstream tag from your script to automatically apply the dwc2, vc4 and upstream-aux-interrupt overlays when detected, right?

With that in place, I could automatically have U-Boot emit the upstream tag on u-boot.bin and move it to a model where it sets CONFIG_OF_BOARD which means it no longer uses its own device tree, but instead the one from firmware.

pelwell commented 6 years ago

But if you have the upstream flag in the trailer the firmware will load the upstream dtb so you don't need the overlay.

agraf commented 6 years ago

Where would the firmware load the upstream dtb from? I don't see any delivered in the boot/ directory.

The whole point of this exercise is that I don't want to be in the business of maintaining device trees. It's something board vendors (you in this case) are supposed to handle.

pelwell commented 6 years ago

Presumably from the same place you will be getting the upstream kernel (also not supplied in /boot). Being upstream the dtbs are maintained by the Linux devs, and built from the kernel source tree.

agraf commented 6 years ago

I would claim the platform is stable enough at this point that we don't need to distinguish between upstream and downstream DTs that much anymore. They are almost identical at this point.

The major difference is that the default configuration in the upstream Linux kernel provided DT is different (no spidev, different led config, etc). And that's something the overlays potentially depend on. So if I end up using the upstream DT as base, your "downstream overlays" will not always work as expected. That's bad.

Instead, I would by far prefer if we could just maintain a single device tree base that can be tweaked with a few minor bits (the 3 overlays I mentioned here) between the different users of DT.

After all, keep in mind that there are many more users of the DT than the RPi Linux tree and the upstream Linux tree. There are also BSDs, U-Boot, self build OSs, etc etc out there. They need something to base against - and that's a "board device tree" with upstream Linux acknowledged bindings.

To see the bigger picture, take a look at the following presentation at FOSDEM: https://fosdem.org/2018/schedule/event/hwenablement_simplifying_soc_enablement_in_linux/

Can we at least agree on something where the RPi firmware will use the downstream DT + dwc2 + vc4 + upstream-aux-interrupts automatically if it does not find an "upstream DT" file, but a kernel that is tagged as upstream? And that going forward, inherently incompatible changes to the downstream DT get overlay-reversed with the same mechanism?

Then people can start to work against a stable base that "just works" in most cases.

pelwell commented 6 years ago

I'm reluctant to add a dependency on an arbitrary list of overlays, since the list may grow in the future. I do, however, have a utility that can merge the source of a number of overlays into a single source which can then be included in the downstream tree with a defined name - "upstream-overlay.dts", for example. This makes it easy to maintain and extend the overlay as circumstances change without requiring firmware changes.

agraf commented 6 years ago

So we would just generate upstream-overlay.dts as part of the overlay builds in your downstream Linux tree? That would work too, yes.

pelwell commented 6 years ago

The ovmerge utility is now available in the "utils" repo. You would run it like this (from within arch/arm/boot/dts/overlays):

ovmerge -c vc4-kms-v3d-overlay.dts dwc2-overlay.dts upstream-aux-interrupt-overlay.dts > upstream-overlay.dts

The -c option adds a comment (// redo: ovmerge -c ...) that makes it easy to "redo" the command should the sources change:

ovmerge -r upstream-overlay.dts > new.dts
mv new.dts upstream-overlay.dts
pelwell commented 6 years ago

But I thought to check that file back into the repo where it can be treated just like any other overlay.

agraf commented 6 years ago

Working on an image based on this approach, I also stumbled over the fact that the downstream device tree overrides the "compatible" property of the root node. It is now missing the "raspberrypi,3-model-b" compatible for the RPi3b for example.

Please add those back as well so that our matching logic for the wifi logic works again :).

pelwell commented 6 years ago

It isn't that we are overwriting it, it is that we aren't including it:

$ ovmerge -i bcm2710-rpi-3-b.dts
bcm2710-rpi-3-b.dts
    bcm2710.dtsi
        bcm2837.dtsi
            bcm283x.dtsi
        bcm270x.dtsi
        bcm2708-rpi.dtsi
    bcm283x-rpi-smsc9514.dtsi

The quick fix is to add it to the downstream dts, but using more of the upstream dts may be preferable.

pelwell commented 6 years ago

The quick fix is completed, at least for .dts files that are only used by one model.

pelwell commented 6 years ago

Tonight's firmware release includes an update to the DTB location logic. The changes are:

  1. The loader now knows about the wider selection of DTB names used upstream.
  2. When an upstream kernel is detected the loader will try upstream names first, then fall back to downstream names and apply the upstream overlay.
  3. In addition to the kernel trailer, a new config setting - upstream_kernel - can be used to force or prevent the loading of upstream DTBs (with fallback); it takes priority over any trailer.

I think that completes the work required by this issue.

stapelberg commented 6 years ago

With commit 7fdcd00e00a42a1c91e8bd6f5eb8352fe9358557, I can’t get my Raspberry Pi 3 to boot anymore. I think the change discussed in this issue is the culprit, but it’s hard to tell for sure without the source :).

I’m using a boot partition containing:

I tried explicitly setting upstream_kernel=0 or upstream_kernel=1 in config.txt. I also tried removing device_tree=rpi-3-b.dtb from config.txt. None of these changes seemed to have any effect.

What do I need to change to make things work again?

Let me know if you need more details to reproduce the issue. I could provide a working and non-working SD card image if that helps.

Thanks!

pelwell commented 6 years ago

Try backing off the dtb to the previous version.

stapelberg commented 6 years ago

I didn’t change the dtb at all, only bootcode.bin, fixup.dat and start.elf.

I’m using https://github.com/gokrazy/kernel/blob/master/rpi-3-b.dtb both in the working and non-working case.

pelwell commented 6 years ago

The significant change in that firmware release was to do with the naming and location of dtb files. One thing I didn't test was whether a manual device_tree setting still worked, and I don't have access to the codebase right now.

Can you make a copy of your .dtb and call it bcm2710-rpi-3-b.dtb, and try again?

stapelberg commented 6 years ago

Renaming the dtb to bcm2710-rpi-3-b.dtb makes it work indeed, thanks!

Could you push a fix to the firmware please?

pelwell commented 6 years ago

A fix will be in the next release.

pelwell commented 6 years ago

The patch to restore the device_tree=... support is in the current rpi-update firmware.

stapelberg commented 6 years ago

Confirmed working, thank you!

pelwell commented 6 years ago

Thanks for the timely report.

afaerber commented 6 years ago

According to @agraf the B+ is now showing the same compatible string as the B and only changing the model string? Can you please define a distinct compatible binding for the B+, too?

pelwell commented 6 years ago

Here you go: https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts

It's almost as though we had them sitting waiting for launch day.

afaerber commented 6 years ago

@pelwell Perfect. Does that need an update of the .dtb files on the FAT partition (does it fall back to b if b-plus is not present?) or just of the firmware blobs?

pelwell commented 6 years ago

It is better to use the dedicated 3-b-plus.dtb, but the current firmware knows enough to be able to construct one based on the 3-b.dtb.

afaerber commented 6 years ago

SLES is not using the .dtb files itself, so my comment/request was in fact about the default device tree we get from your firmware. Yes, it constructs something, but apparently does not yet allow for a safe distinction between B and B+.

pelwell commented 6 years ago

SLES is not using the .dtb files itself, so my comment/request was in fact about the default device tree we get from your firmware.

I think you've misunderstood. The .dtb that SLES gets "from the firmware" for a 3B is created by loading bcm2710-rpi-3-b.dtb and tweaking it a bit (optionally applying the upstream overlay). For a 3B+ the firmware will try to load bcm2710-rpi-3-b-plus.dtb, but if it doesn't exist it will load bcm2710-rpi-3-b.dtb and make a few more significant changes before the tweaking and upstream overlay application.

afaerber commented 6 years ago

Misunderstanding indeed, it read to me as if we were supposed to "use" the .dtb by loading it ourselves. If it is rather today's firmware that loads it if available, that'll cover our use case. Thanks!

dezgeg commented 6 years ago

The .dtb that SLES gets "from the firmware" for a 3B is created by loading bcm2710-rpi-3-b.dtb and tweaking it a bit.

Is there some documentation on what tweaks the firmware is doing to the DTB loaded from the SD card? It would be nice to be able to use U-Boot (which does know how to do some general things like updating the MAC address of ethernet nodes but nothing RPi-specific) to boot the RPi foundation kernel.

(Incidentally, I'm asking because on the ARMv6 RPi, this has worked me all the way until FW+kernel 4.9.59 tagged 1.20171029 but stopped working when I upgraded FW+kernel to 4.14.30 tagged 1.20180328. I can open a separate issue for that when I get time to try some more debugging.)

agraf commented 6 years ago

U-Boot actually knows how to update the MAC address :). Also by default, no DT modification is happening, so you should be able to just pass the DT along as long as CONFIG_OF_BOARD is set. Keep in mind that by default that won't give you U-Boot output on HDMI, as that requires the vc4 dt overlay.

Either way, it's most likely a separate issue to this one, so please open a new one (and CC me if you need input from me).

dezgeg commented 6 years ago

@agraf I was actually asking what device tree modifications the Raspberry Pi firmware is doing to the device tree, not what U-Boot does to it. But I figured out the problem is unrelated to that, instead it's just that the load address for the device tree in U-Boot (fdt_addr_r=0x00000100) is too small to fit the downstream device tree since the kernel decompressor apparently uses memory starting at 0x4000 (I thought it was only at 0x8000...). I will try to send U-Boot patches.

pelwell commented 6 years ago

But U-Boot is capable of using the DT loaded by the firmware to a location of its choice, using a script something like this:

fatload mmc 0:1 ${kernel_addr_r} kernel7.img
setenv bootargs earlyprintk dwc_otg.lpm_enable=0 console=serial0,115200 root=/dev/mmcblk0p2 rootfstype=ext4 rootwait
bootz ${kernel_addr_r} - ${fdt_addr}

Only the use of ${ftd_addr} is required - the rest of the script is just provided by way of an example.

dezgeg commented 6 years ago

Yes, but our distro (https://nixos.org/) has this feature where GRUB / systemd-boot / U-Boot gives a boot menu where the user can roll back to previous system configuration and by extension to an older kernel version. So only after the user has made their selection in the boot menu (which I've implemented with the extlinux.conf feature of U-Boot) it's known what kernel is to be used, and .dtbs are always loaded from the matching zImage (as the user might have switched from the downstream kernel to upstream kernel or vice versa).

agraf commented 6 years ago

In that case please just adjust $fdt_addr_r in U-Boot, yes. You basically want the last hunk from this patch: https://github.com/openSUSE/u-boot/commit/4e3a7960c98360ef3673753034ce840e7216828b