RPi-Distro / raspberrypi-sys-mods

A collection of Raspberry Pi-sourced system configuration files and associated scripts
102 stars 37 forks source link

Serial port symlinks broken in bookworm. #84

Open metachip opened 1 year ago

metachip commented 1 year ago

There appears to be a bug in the assignment of the primary and secondary serial ports in bookworm.

It looks like a problem in:

raspberrypi-sys-mods: /etc/udev/rules.d/99-com.rules

When bluetooth is disabled in config.txt as:

dtoverlay=disable-bt

I would expect that

/dev/serial0 -> ttyAMA0 /dev/serial1 -> ttyS0

and this is what bullseye does, correctly, but instead with bookworm we see:

/dev/serial1 -> ttyAMA0

and /dev/serial0 is not symlinked at all.

This is contrary to the advice here:

https://www.raspberrypi.com/documentation/computers/configuration.html#primary-and-secondary-uart

and looks like a bug in bookworm :)

XECDesign commented 1 year ago

@pelwell, is this something that has been or will be fixed with newer firmware/kernel or just expected behaviour?

pelwell commented 1 year ago

It's definitely not expected behaviour, and it may have been fixed with the latest firmware (coming to an apt upgrade soon) - knowing which Pi it was would help.

pelwell commented 1 year ago

Reproduced on a Pi 400.

metachip commented 1 year ago

Serial Device Alias Fault Diagnosis

The problem is the line:

readlink /sys/class/tty/ttyS[0-9]*/device/of_node | sed 's/base/:/' | cut -d: -f2

in

/etc/udev/rules.d/99-com.rules

On Pi5 it works.

On Pi4 it does not.

The problem is the format of the descriptor for ttyS0 is different.

On Pi5:

$ echo cat /proc/device-tree/model Raspberry Pi 5 Model B Rev 1.0 $ tail -2 /boot/firmware/config.txt enable_uart=1 dtoverlay=disable-bt $ readlink /sys/class/tty/ttyS0/device/of_node ../../../../firmware/devicetree/base/soc/serial@7d50c000

On Pi4:

$ echo cat /proc/device-tree/model Raspberry Pi 4 Model B Rev 1.4 $ tail -2 /boot/firmware/config.txt enable_uart=1 dtoverlay=disable-bt $ readlink /sys/class/tty/ttyS0/device/of_node $ ls /sys/class/tty/ttyS0/device/ driver driver_override modalias power subsystem tty uevent

Bottom line:

/etc/udev/rules.d/99-com.rules

requires modification to work on models other than Pi5. The test on of_node fails on Pi4 because it does not exist.

pelwell commented 1 year ago

The real problem is that the use of the bluetooth alias to determine which UART is which should be conditional on the existence of the console alias, not vice-versa:

diff --git a/etc.armhf/udev/rules.d/99-com.rules b/etc.armhf/udev/rules.d/99-com.rules
index 1931d3d..7ad5f38 100644
--- a/etc.armhf/udev/rules.d/99-com.rules
+++ b/etc.armhf/udev/rules.d/99-com.rules
@@ -17,11 +17,11 @@ SUBSYSTEM=="pwm", ACTION!="remove", PROGRAM="/bin/sh -c 'chgrp -R gpio /sys%p &&
 KERNEL=="ttyAMA[0-9]*|ttyS[0-9]*", PROGRAM="/bin/sh -c '\
         ALIASES=/proc/device-tree/aliases; \
         TTYNODE=$$(readlink /sys/class/tty/%k/device/of_node | sed 's/base/:/' | cut -d: -f2); \
-        if [ -e $$ALIASES/bluetooth ] && [ $$TTYNODE/bluetooth = $$(strings $$ALIASES/bluetooth) ]; then \
-            echo 1; \
-        elif [ -e $$ALIASES/console ]; then \
+        if [ -e $$ALIASES/console ]; then \
             if [ $$TTYNODE = $$(strings $$ALIASES/console) ]; then \
-                echo 0;\
+                echo 0; \
+            elif [ -e $$ALIASES/bluetooth ] && [ $$TTYNODE/bluetooth = $$(strings $$ALIASES/bluetooth) ]; then \
+                echo 1; \
             else \
                 exit 1; \
             fi \

Doing so with dtoverlay=disable-bt results in:

/dev/serial0 -> ttyAMA0

which is arguably more correct than:

/dev/serial0 -> ttyAMA0
/dev/serial1 -> ttyS0

since /dev/ttyS0 doesn't actually work (try it - the device node is disabled, and yet somehow the driver is creating a /dev/ entry for it, perhaps because of 8250.nr_uarts=1 on the command line).

pelwell commented 1 year ago

There's now a PR with this change: #85