Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump rpi-rf-mod dt overlay to latest version #2921

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

jens-maus
Copy link
Contributor

This PR bumps the rpi-rf-mod dt overlay package to the latest 1.13.0 version available. This new version integrates some fixes including one that changes the default stdout-path dtb definition to an empty string so that u-boot won't try to pickup the reserved serial port for the RPI-RF-MOD Homematic RF module once the rpi-rf-mod.dtbo overlay is used in the config.txt file of RaspberryPi installations (fixes #2919).

Copy link
Member

@sairon sairon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though I don't know why the problem was triggered only by the recent kernel update, it happens because you are completely overriding compatible of uart0 in rpi-rf-mod.dtbo to a reduced set of compatible strings, instead of adding the one you need. The behavior you're seeing (and which I was able to reproduce on my RPi 4 too) is the same as described in the description of this patch which is applied to the RPi device tree (with the exception that now it crashes ALSO if enable_uart=1).

So IMHO the correct way to address the problem would be to adjust compatible in fragment@3 in RPi and Yellow DTBOs to:

compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell", "pivccu,pl011";

With this patch my RPi is booting fine but I can't check pivccu function.

Also there was no need to override stdout-path for other boards than RPi, as this issue seems to be exclusive to PL011 on RPi SoCs only.

@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@home-assistant home-assistant bot marked this pull request as draft November 16, 2023 09:12
@jens-maus
Copy link
Contributor Author

@sairon

So IMHO the correct way to address the problem would be to adjust compatible in fragment@3 in RPi and Yellow DTBOs to:

compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell", "pivccu,pl011";

With this patch my RPi is booting fine but I can't check pivccu function.

That's exactly the issue here. With this compatible line the RaspberryPi boots up, yes. But it will then prefer to load/use the standard bcm2835-pl011 serial driver rather than preferring the special generic_raw_uart serial driver which is required for appropriate low-latency serial communication with the HomeMatic RF modules like RPI-RF-MOD or HM-MOD-RPI-PCB.

I also tried to put the pivccu-pl011 at the very beginning of the compatible line, but also in that case the standard kernel pl011 serial driver will then still be used. So the compatible line have to contain only pivccu,pl011 for the correct serial driver being automatically selected during bootup.

Also there was no need to override stdout-path for other boards than RPi, as this issue seems to be exclusive to PL011 on RPi SoCs only.

Indeed. It is really strange that only u-boot on RaspberryPi requires either a compatible line with brcm,bcm2835-pl011 or arm,pl011 mentioned while for the other hardware platforms this is not the case and u-boot boots up correctly. And as the generic_raw_uart driver requires the compatible line to contain only pivccu,pl011 the only currently working workaround for getting u-boot to boot correctly is wiping stdout-path as this seems to actually trigger the u-boot issue here. And that's what this PR is actually introducing.

So either we keep that stdout-path wiping or we come up with an u-boot patch making sure u-boot will not be confused if the compatible line does not contain any reference to brcm,bcm2835-pl011 or arm,pl011

@jens-maus
Copy link
Contributor Author

@sairon
Please note that I was able to compile a u-boot version that does not seem to crash if the following items are added to the uboot config file:

# CONFIG_SERIAL_PRESENT is not set
# CONFIG_BCM283X_MU_SERIAL is not set
# CONFIG_BCM283X_PL011_SERIAL is not set

So perhaps adding those config options so that u-boot won't try to pickup any serial device could help instead of modifying the rpi-rf-mod.dtbo to wipe stdout-path?

@jens-maus
Copy link
Contributor Author

Ok, I have throughoutly tested disabling the BCM2835 serial in u-boot alltogether and for my main project (RaspberryMatic) this commit allowed to let u-boot continue to boot even thought the stdout-path of the dtbo is not in place:

jens-maus/RaspberryMatic@1e8c11a

So please advice if I should resubmit a different patch for modifying the uboot.config in HAos for the RaspberryPis so that there the BCM2835 serial interaction is also completely disabled. This perhaps then also would allow us to remove the other patch (https://github.com/home-assistant/operating-system/blob/dev/buildroot-external/board/raspberrypi/patches/linux/0001-ARM-dts-bcm283x-add-compatible-picked-up-by-U-Boot.patch)

@sairon
Copy link
Member

sairon commented Nov 19, 2023

Disabling serial in U-Boot could be a solution if we can accept it won't be available for debugging purposes anymore. Although we have bootdelay set to -2 (boot immediately without prompt) AFAIK it is still shown e.g. on an early boot failure (like boot disk error). @agners what do you think?

@jens-maus
Copy link
Contributor Author

Disabling serial in U-Boot could be a solution if we can accept it won't be available for debugging purposes anymore.
Although we have bootdelay set to -2 (boot immediately without prompt) AFAIK it is still shown e.g. on an early boot failure (like boot disk error).

In RaspberryMatic U-Boot outputs its debugging on the HDMI port. And for hard debugging reasons we could actually easily re-enable it. Perhaps changing the U-Boot config in HAos to output its boot phase on the HDMI could also be an option?

Thus, in the current situation this clearly looks like U-Boot bug that the bcm2835 serial init brings down u-boot altogether if stdout-path is set and the compatible string does not point to the bcm2835 serial device but to another device. This IMHO should not bring down U-Boot at all, but in case of the RaspberryPi / bcm2835 it obviously does and disabling bcm2835 serial support altgether seems to be a valid procedure to fix that issue.

So question remains: Should I resubmit a different PR that updates rpi-rf-mod to the latest version in RM (without stdout-path patching) and then submit another PR to disable the serial BCM2835 driver in U-Boot altogether?

@sairon
Copy link
Member

sairon commented Nov 20, 2023

Just realized my previous suggestion was wrong - to keep both U-Boot and piVCCU working, the compatible should be set to "brcm,bcm2835-pl011", "pivccu,pl011" - that way U-Boot should pick the first one and Linux the second one (because the former is not defined in Linux).

Anyway, I still keep in mind it's rather a hacky solution, just looking for some middle grounds now.

@jens-maus
Copy link
Contributor Author

Just realized my previous suggestion was wrong - to keep both U-Boot and piVCCU working, the compatible should be set to "brcm,bcm2835-pl011", "pivccu,pl011" - that way U-Boot should pick the first one and Linux the second one (because the former is not defined in Linux).

Thx. could try that as well again, but I still think the cleaniest solution would be to either fix U-Boot to not crash or disable the bcm2835 serial support in U-Boot altogether.

Anyway, I still keep in mind it's rather a hacky solution, just looking for some middle grounds now.

Why is it so important to keep serial support in U-Boot? There are also potential benefits of disabling serial support in U-boot actually. For exampe, some HATs (RPI-RF-MOD is also a HAT) which are actually utilizing the UART might get confused by the serial output/input U-Boot produces during startup. Thus, if no deep debugging is supported I think the serial support should be disabled (at least muted). And if debugging is required HAos could be recompiled with serial support enabled in U-boot to get it working again.

@agners
Copy link
Member

agners commented Nov 20, 2023

Disabling serial in U-Boot could be a solution if we can accept it won't be available for debugging purposes anymore. Although we have bootdelay set to -2 (boot immediately without prompt) AFAIK it is still shown e.g. on an early boot failure (like boot disk error). @agners what do you think?

In general, I think removing that compatible work around we have today would be nice! Ideally if possible we should find a solution which doesn't involve disabling drivers and rendering serial console unusable.

From what I understand the problem is the selection of the serial console port by U-Boot (as by my last findings, see https://lists.denx.de/pipermail/u-boot/2020-November/432502.html).

In RaspberryMatic U-Boot outputs its debugging on the HDMI port. And for hard debugging reasons we could actually easily re-enable it. Perhaps changing the U-Boot config in HAos to output its boot phase on the HDMI could also be an option?

I thought we've (re)enabled HDMI a while ago in U-Boot. Definitely remember we did something in that area, maybe we enabled and had to disable it again due to breakages of some setups 🤔

@agners agners marked this pull request as ready for review November 21, 2023 08:47
@home-assistant home-assistant bot requested a review from sairon November 21, 2023 08:47
@agners
Copy link
Member

agners commented Nov 21, 2023

@jens-maus I've discussed this issue with @sairon today. We agreed that disabling the driver in U-Boot is not the preferred solution for us. Especially on Yellow this is the only method to really see the U-Boot output.

I think that the suggestion made in #2921 (comment) doesn't work, as you already set the compatible string to pivccu,pl011. I really wonder why U-Boot misbehaves here at all.

For now, let's merge this as it serves as a work around. We'd like to release 11.2 soon.

I'll leave #2919 open so we can discuss future solutions there.

@agners agners merged commit 5891250 into home-assistant:dev Nov 21, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

11.2dev (rpi): u-boot crashes with miniuart-bt and rpi-rf-mod dtbo enabled with latest stable_2023XXXX kernels
3 participants