LinuxCNC / linuxcnc

LinuxCNC controls CNC machines. It can drive milling machines, lathes, 3d printers, laser cutters, plasma cutters, robot arms, hexapods, and more.
http://linuxcnc.org/
GNU General Public License v2.0
1.8k stars 1.15k forks source link

`hal_pi_gpio` doesn't work on Raspberry Pi 4 on Debian Bookworm #2371

Closed SebKuzminsky closed 1 year ago

SebKuzminsky commented 1 year ago

On a Raspberry Pi 4B, running a recent Debian Bookworm image from https://github.com/SebKuzminsky/image-specs:

$  lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:  Debian GNU/Linux bookworm/sid
Release:      n/a
Codename:     bookworm

$ uname -a
Linux linuxcnc 6.1.0-3-rt-arm64 #1 SMP PREEMPT_RT Debian 6.1.8-1 (2023-01-29) aarch64 GNU/Linux

$ cat /proc/device-tree/model; echo 
Raspberry Pi 4 Model B Rev 1.2

$ halcmd loadrt hal_pi_gpio
Note: Using POSIX realtime
HAL_PI_GPIO: ERROR: board revision 7 not supported
hal_pi_gpio: rtapi_app_main: Invalid argument (-22)
<commandline>:0: waitpid failed /usr/bin/rtapi_app hal_pi_gpio
<commandline>:0: /usr/bin/rtapi_app exited without becoming ready
<commandline>:0: insmod for hal_pi_gpio failed, returned -1

This seems to be due to an out of date version of the cpuinfo.c file we got from MachineKit, who got it from raspberry-gpio-python.

We should probably fetch the new file from upstream and integrate it with our code.

We should maybe also consider writing a generic gpio driver for HAL using libgpiod for the back end.

andypugh commented 1 year ago

There seems to have been a great deal of divergence: https://sourceforge.net/p/raspberry-gpio-python/code/ci/default/tree/source/cpuinfo.c That file links to this definitive list: https://www.raspberrypi.com/documentation/computers/raspberry-pi.html#raspberry-pi-revision-codes

Maybe we need to consider why the driver cares? I think it is just so it knows which of the (currently two?) GPIO connectors we are dealing with?

The RPi docs specifically warn against doing what we are doing: https://www.raspberrypi.com/documentation/computers/raspberry-pi.html#best-practice-for-revision-code-usage

And provide sample code of a better way to do it.

SebKuzminsky commented 1 year ago

And we should maybe switch to libgpiod, which is a standard way for the kernel to expose gpios on all kinds of platforms. Don't know if it's fast enough to do realtime type stuff with...?

rodw-au commented 1 year ago

I have a PR pending for the getting Linuxcnc doc page which has a proven and tested method to install Linuxcnc on the Raspberry Pi and the Bookworm repos. I think it should be tagged 2.9-must-fix. Seb asked for it. Ref: #2496

andypugh commented 1 year ago

Having addressed the cpuinfo issue, I have now found that the hal_pi_gpio relied on /dev/gpiomem, and that isn't present with a bookworm installation. It does seem to work on Raspbian Buster, though.

Which gives more weight to the suggestion to use libgpiod

rodw-au commented 1 year ago

I've only used the Pi with Mesa and EtherCAT devices. With a bit of research, libgpiod was introduced into the kernel at Version 4.8. So this has been a sleeper since the release of Linuxcnc on Debian Buster (kernel 4.19). The libgpiod source is now hosted at kernel.org. One would think that it is quite safe to use in a RT environment given it's a kernel module. Surely the RT devs would have been screaming by now if it wasn't...

petterreinholdtsen commented 1 year ago

[Andy Pugh]

Having addressed the cpuinfo issue, I have now found that the hal_pi_gpio relied on /dev/gpiomem, and that isn't present with a bookworm installation. It does seem to work on Raspbian Buster, though.

I do not know the inner working of this, but was able to trace /dev/gpiomem to the kernel driver implemented in <URL: https://github.com/eq-3/RaspberryMatic/blob/master/linux-4.1/drivers/char/broadcom/bcm2835-gpiomem.c >. No idea if it is implemented in the default Linux kernel. :)

-- Happy hacking Petter Reinholdtsen

andypugh commented 1 year ago

Following @SebKuzminsky 's suggestion, I have written a new driver called hal_gpio which should work on all of these SBCs that support gpiod

https://github.com/LinuxCNC/linuxcnc/blob/andypugh/hal_gpio/src/hal/drivers/hal_gpio.c

Issues: . Currently all pins of a Rock64 have the name "unused" so need to support numerical referencing . It seems slow, 50µs to read 8 pins. There are "bulk" read/write options to be explored . Probably won't work with RTAI? . Need to add invert, reset, pull-up, pull down options. . No docs . Needs updates to the rtapi_app submakefile, and I don't know the right way to do that.

andypugh commented 1 year ago

Most of these issues are now addressed. It currently reads 8 GPIO lines in 16µs and writes 8 gpio lines in 8µs. (It slows down once you start to span more than one GPIO "chip")

sample HAL file:

debug 5
loadrt hal_gpio inputs=GPIO5,GPIO6,GPIO12,GPIO13,GPIO16,GPIO17,GPIO18,GPIO19 \
        outputs=GPIO20,GPIO21,GPIO22,GPIO23,GPIO24,GPIO25,GPIO26,GPIO27 \
        invert=GPIO20,GPIO27
loadrt threads name1=base-thread name2=servo-thread period1=100000 period2=1000000 fp1=0
addf hal_gpio.read base-thread
addf hal_gpio.write base-thread
loadusr halshow
start

Given that I think that I have fixed the original issue, and that there is a new driver on the way. should I close this issue?

d2inventory commented 1 year ago

The configure script checks for libgpiod-dev and if it's missing it says:

configure: WARNING: Could not find libgpiod, not building hal_gpio

But it's "built" anyway and later on it fails at the linking stage. Maybe make configure fail if libgpiod-dev is not found?

andypugh commented 1 year ago

I don't think it makes much sense for libgpiod-dev to be mandatory, it isn't any use on most PCs, for example.

What OS are you building on? I am currently puzzled by the behaviour on Bookworm. It was developed and tested on a Pi400 under Buster, and works fine. But on Bookworm it builds but then fails at load-time with "unknown symbol" for gpio_line_find()

d2inventory commented 1 year ago

A fresh install of Debian 12.

If it's not mandatory something needs to be set so it wont actually build.

For example, if there is no po4a the environment variable BUILD_DOCS_TRANSLATED is set to no which later on is checked in the submakefile for docs. Something like this needs to be done for hal_gpio if it shouldn't be built if the library is not available.

rodw-au commented 1 year ago

I was planning on building a linuxcnc PI image using the upstream repo https://github.com/pyavitz/rpi-img-builder I just have another project I want to tackle first. (But could swap it round iif this is needed urgently). I think this is the right way forward. If we do this, it will be cross compiled on a normal PC just like linuxcnc-live-build. If the Pi image is created this way, then a user will not ever need to see libgpiod-dev as it will only be in the chroot. This should be pretty straightforward as I have done it once before.

It might be worth considering some sort of switch to determine which docs should be included given the number that now exist. My 2 Gb ISO became 3.0 Gb once I forked the Linuxcnc-live-build repo...

rodw-au commented 1 year ago

Andys' fixes to GPIO and his new driver in master branch have been tested by a few users now. My Debian 12 imager is based on master branch source and includes those fixes. https://github.com/rodw-au/rpi-img-builder-lcnc/tree/linuxcnc It sees that Andy's changes can be back ported into 2.9 branch and this issue can be closed. That would allow me to change the imager to build 2.9 code.

andypugh commented 1 year ago

hal_gpio went into 2.9 last night. So, I am closing this.