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

Use gpiochip0 for the user-facing GPIOs on Pi 5 #6144

Merged
merged 4 commits into from
Aug 5, 2024

Conversation

pelwell
Copy link
Contributor

@pelwell pelwell commented May 2, 2024

Pi 5 contains multiple GPIO controllers; running gpiodetect shows that there are five. Prior to this patch set, they appear like this:

gpiochip0 [gpio-brcmstb@107d508500] (32 lines)
gpiochip1 [gpio-brcmstb@107d508520] (4 lines)
gpiochip2 [gpio-brcmstb@107d517c00] (17 lines)
gpiochip3 [gpio-brcmstb@107d517c20] (6 lines)
gpiochip4 [pinctrl-rp1] (54 lines)

Notice how pinctrl-rp1 - the driver for the GPIOs that appear on the 40-pin header - is shown as gpiochip4. This is not user-friendly, requiring careful handling when trying to locate a particular GPIO via libgpiod.

This patch set adds the ability to override the gpiochip numbers using DT aliases, just as is done for I2C, SPI etc. It also adds a gpiochip0 alias to pinctrl-rp1. changing the gpiochip numbering to this:

gpiochip0 [pinctrl-rp1] (54 lines)
gpiochip1 [gpio-brcmstb@107d508500] (32 lines)
gpiochip2 [gpio-brcmstb@107d508520] (4 lines)
gpiochip3 [gpio-brcmstb@107d517c00] (17 lines)
gpiochip4 [gpio-brcmstb@107d517c20] (6 lines)

P.S. There's now an additional commit that sets the base of the SoC gpiochips to 10, for two reasons:

  1. it makes it more clear that these are system resources, like i2c10 and spi10, and
  2. there will no longer be a gpiochip4, preventing any existing code that chooses gpiochip4 on a Pi 5 from changing the wrong GPIOs accidentally.

With this commit, the final picture is:

gpiochip0 [pinctrl-rp1] (54 lines)
gpiochip10 [gpio-brcmstb@107d508500] (32 lines)
gpiochip11 [gpio-brcmstb@107d508520] (4 lines)
gpiochip12 [gpio-brcmstb@107d517c00] (17 lines)
gpiochip13 [gpio-brcmstb@107d517c20] (6 lines)

@6by9
Copy link
Contributor

6by9 commented May 2, 2024

Are we not risking breaking existing users who have hard coded gpiochip4 into their library?
I'm looking at https://github.com/gpiozero/gpiozero/blob/master/gpiozero/pins/lgpio.py#L66 for example.

@waveform80 would probably have a better knowledge of the current assumptions made.

@pelwell
Copy link
Contributor Author

pelwell commented May 2, 2024

Heads up, @waveform80. I see that rpi-lgpio selects gpiochip0 by default on BCM2712. Would you be happy making it fall back to gpiochip0 if gpiochip4 is not found? I'd like to think the main gpiolib patch would be acceptable upstream, and I think it lowers the barrier-to-entry a bit more for people switching from sysfs.

@pelwell pelwell marked this pull request as draft May 2, 2024 17:16
@waveform80
Copy link

Yes, I'm afraid gpiozero and rpi-lgpio both assume gpiochip4 at the moment (which is almost certainly terrible laziness on my part, but the latter in particular has been a bit of a rush-job).

I did something slightly more respectable for our GPIO udev rules, filtering on the driver, and ideally I think something similar should work for the two libraries, by querying sysfs (e.g. /sys/bus/gpio/devices/gpiochip4/of_node/compatible). Something like:

  • If a gpiochip is specified explicitly, use it. If it fails, die with exception (don't second-guess explicit requests)
  • If a gpiochip is not specified explicitly, find first gpiochip with a driver {rpi1-gpio, bcm2835-gpio, bcm2711-gpio} and try that (some nuance there if multiple devices with such a driver exist, and we can't access the first one)
  • If that fails, fall back to gpiochip0
  • If that fails, die with exception

I'll see what I can do about pushing a fix out to the libraries in the next few days.

waveform80 added a commit to waveform80/rpi-lgpio that referenced this pull request May 11, 2024
There are moves afoot [1] to change the gpiochip device on the Pi 5 from
gpiochip4 to gpiochip0. The current gpiochip discovery in rpi-lgpio is
crude, to say the least. This commit refines the method to discovering
the chip based on the GPIO driver in use (which should work on all Pi
boards with all kernels).

Closes: #10

[1]: raspberrypi/linux#6144
@pelwell
Copy link
Contributor Author

pelwell commented Jul 17, 2024

The issue of discovering the user-facing GPIOs, the subject of this PR, came up again today. How are people feeling about the possibility of this being merged in the near future?

In the same way that other subsystems support the setting of device
id numbers from Device Tree aliases, allow gpiochip numbers to be
derived from "gpiochip<n>" aliases.

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
Add a gpiochip0 aliase pointing to the rp1 GPIO node, making it appear
as gpiochip0.

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
Make the BCM2712's onboard GPIOs start at gpiochip10, marking them out
as system resources and preventing accidental use by existing Pi 5
code.

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
@waveform80
Copy link

Let me double-check I've got things in place. I definitely have in rpi-lgpio but I think I forgot to release a version of gpiozero with this in place. I'll have a look this evening!

@pelwell
Copy link
Contributor Author

pelwell commented Aug 2, 2024

If there are no objections, this will be merged next week.

@pelwell pelwell marked this pull request as ready for review August 2, 2024 10:34
@pelwell pelwell merged commit 70c640c into raspberrypi:rpi-6.6.y Aug 5, 2024
12 checks passed
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Aug 5, 2024
@ragazenta
Copy link

Sorry if I ask in the wrong place, when will these changes uploaded to APT repository? I hope it will soon. Thanks.

@popcornmix
Copy link
Collaborator

We don't have a specific date planned. In "some" weeks time is my guess.

@HiassofT
Copy link
Contributor

quick heads up: gpiozero using the lgpio pin factory (recommended default on RPiOS) is currently broken due to this change, I've created a gpiozero issue gpiozero/gpiozero#1166

@L4wnmower
Copy link

Yesternights RPi kernel bump from 6.6.31 to 6.6.47 effectively broke gpiozero. Python PWM-control is currently dead-in-the-water

@pelwell
Copy link
Contributor Author

pelwell commented Sep 9, 2024

So 4 months' notice was not enough?

@L4wnmower
Copy link

Sadly

@L4wnmower
Copy link

I'm glad they did!
From things I've read, I thought that project was faltering. Those good reflexes are a healthy sign

@mstroh76
Copy link

mstroh76 commented Sep 10, 2024

can’t an alias be created from gpiochip4 to gpiochip0? That would solve all the problems, making both gpiochip0 and gpiochip4 functional,

@pelwell
Copy link
Contributor Author

pelwell commented Sep 11, 2024

can’t an alias be created from gpiochip4 to gpiochip0?

Not without adding some potentially large hack to the kernel code. The client-side fix is much easier.

@mstroh76
Copy link

I understand that it is a large hack, but I would like to point out that a client-side fix means that:

  • All internet blogs/guides
  • YouTube tutorials
  • Books
  • libgpiod guides and programs
  • GPIO libraries

need to implement these changes.

Furthermore, there is the problem that if the client is changed, one cannot assume that Kernel 6.6.47 is installed. The current Pi OS image (July 4th, 2024) still contains the old kernel. With older kernel versions, gpiochip0 can be opened, but this is the wrong chip. To be sure, a check of the chip label must be performed.

For me, however, this topic is now closed.

@lurch
Copy link
Contributor

lurch commented Sep 11, 2024

I suspect that most internet blogs / YouTube tutorials / etc. are simply using a 3rd-party library (e.g. GpioZero) rather than making low-level libgpiod calls directly; so once GpioZero is fixed to work with this kernel change, all that tutorial content will be totally fine again!

With older kernel versions, gpiochip0 can be opened, but this is the wrong chip. To be sure, a check of the chip label must be performed.

Yes, that's exactly what waveform80/rpi-lgpio@bb7ff0e does, and I assume the eventual GpioZero fix will be similar.

@pelwell
Copy link
Contributor Author

pelwell commented Sep 11, 2024

It turns out that we may be able to put a workaround into RPiOS - watch this space.

@pelwell
Copy link
Contributor Author

pelwell commented Sep 12, 2024

sudo apt update and sudo apt full-upgrade should get you a working gpiozero. It's a belt-and-braces (suspenders, for non Brits) double fix:

  1. The version of gpiozero installed includes an unofficial patch for the problem.
  2. There's a udev rule that creates a symbolic link from gpipchip4 to gpiochip0. This has the minor unwanted side-effect that gpiochip0 shows up twice when running gpioinfo and gpiodetect, but it should fix the problem for apps and libraries other than gpiozero.

@lgeitner
Copy link

Meteo-pi on a Raspberry Pi 5 still does not work with the above changes!

pi@cumulusmxkitchen:~ $ gpiodetect
gpiochip0 [pinctrl-rp1] (54 lines)
gpiochip10 [gpio-brcmstb@107d508500] (32 lines)
gpiochip11 [gpio-brcmstb@107d508520] (4 lines)
gpiochip12 [gpio-brcmstb@107d517c00] (17 lines)
gpiochip13 [gpio-brcmstb@107d517c20] (6 lines)
gpiochip0 [pinctrl-rp1] (54 lines)

@pelwell
Copy link
Contributor Author

pelwell commented Sep 13, 2024

That looks like a commercial product - have you reported the problem to the manufacturer?

@lgeitner
Copy link

I have reported the problem to both the developer of cumulusmx https://cumulus.hosiene.co.uk/viewtopic.php?t=22529 and have sent an email to the manufacturer of the meteo-pi.

@L4wnmower
Copy link

sudo apt update and sudo apt full-upgrade should get you a working gpiozero. It's a belt-and-braces (suspenders, for non Brits) double fix:

  1. The version of gpiozero installed includes an unofficial patch for the problem.
  2. There's a udev rule that creates a symbolic link from gpipchip4 to gpiochip0. This has the minor unwanted side-effect that gpiochip0 shows up twice when running gpioinfo and gpiodetect, but it should fix the problem for apps and libraries other than gpiozero.

After upgrading, gpiozero's PWM is back in working order. Well Done!

@lgeitner
Copy link

I use the below dtparam and dtoverlay in my config.txt

Is it possible whoever created the below dtparam for uart0 and uart0_console hard coded a reference to gpiochip4?
Is it possible whoever created the below dtoverlay for uart0-pi5 hard coded a reference to gpiochip4?

Can the dtparam and dtoverlay reference the udev rule that was created?
Would that explain why the udev rule doesn't seem to work?

Here is what i have in my config.txt

/boot/firmware/config.txt

dtparam=uart0=on
dtparam=uart0_console=on
dtoverlay=uart0-pi5

Thanks,
Lance

@lgeitner
Copy link

That looks like a commercial product - have you reported the problem to the manufacturer?

I have contacted the manufacturer of the meteo-pi

Here is the response from the manufacturer:

MeteoPi is just connectivity tool. It provides wires from Console's UART TX and RX, to RP UART RX and TX on PIN 8(GPIO14), and 10(GPIO15).
If you want to use MeteoPi with it, you have to do something to enable UART on Pins 8(GPIO14) and 10(GPIO15), because there are connected wires.

I enable uart on Pins 8(GPIO14) and 10(GPIO15) with the following lines in the config.txt as mentioned above.

dtparam=uart0=on
dtparam=uart0_console=on
dtoverlay=uart0-pi5

I contacted the developer of Cumulusmx and asked if there where any hard coded references to gpiochip4 in the cumulusmx software and he said no.

Thanks,
Lance

@pelwell
Copy link
Contributor Author

pelwell commented Sep 16, 2024

It doesn't sound as though MeteoPi is using GPIOs - just a UART.

To enable UART0 on GPIOs 14 & 15 (pins 8 & 10) on a Pi 5, all you need is:

dtparam=uart0=on

Enabling the console on that UART may interfere with however MeteoPi is using it, but if you do want to be able to log in over that UART then you should use:

dtparam=uart0_console=on

You don't need to keep dtparam=uart0=on because that is a part of the uart0_console parameter, but there is no harm in doing so.

@lgeitner
Copy link

the meteopi uses the gpio on the Raspberry PI 5. It uses the Uart0 on GPIO 14(pins 8) and GPIO 15(pin 10).

uart0_console is a dtparam that enables uart0, maps it to GPIOs 14 & 15, and makes it the "console" UART (i.e. serial0 points to it).

please see this topic for details.
https://forums.raspberrypi.com/viewtopic.php?t=359132&start=25

Unfortunately, this is a major breaking change for me!
Is there someplace I can report this issue to?

Thanks,
Lance

@pelwell
Copy link
Contributor Author

pelwell commented Sep 16, 2024

the meteopi uses the gpio on the Raspberry PI 5. It uses the Uart0 on GPIO 14(pins 8) and GPIO 15(pin 10).

Using two pins for UART does not amount to using GPIO in the strictest sense. GPIO standards for General Purpose Input/Output, and it allows software to set or read the level on a pin.

uart0_console is a dtparam that enables uart0, maps it to GPIOs 14 & 15, and makes it the "console" UART (i.e. serial0 points to it).

I know - I created uart0_console, and the whole dtoverlay/dtparam mechanism.

Is there someplace I can report this issue to?

Here, and you have done, but arguing with the people trying to help is not a good way to go about it.

@lgeitner
Copy link

I am very grateful for the help you have been giving me.

Would there be any log files I could generate and send to you that may help to diagnose the reason the meteopi no longer works?

Thanks,
Lance

@pelwell
Copy link
Contributor Author

pelwell commented Sep 16, 2024

Let's first establish whether it might have been this change that caused your problem:

  1. Run sudo rpi-update bfaaf1b9, reboot, and test Meteo-Pi. If you are right, it should fail.
  2. Run sudo rpi-update 2806f395, reboot, and test Meteo-Pi. If you are right, it should work.

@lgeitner
Copy link

when i ran sudo rpi-update bfaaf1b9, i receive an error invalid hash given.

pi@cumulusmxkitchen:~ $ sudo rpi-update bfaaf1b
*** Raspberry Pi firmware updater by Hexxeh, enhanced by AndrewS and Dom
*** Performing self-update
*** Relaunching after update
*** Raspberry Pi firmware updater by Hexxeh, enhanced by AndrewS and Dom
FW_REV:
BOOTLOADER_REV:d05f05c94fd7a1916942e56d7f486c142ae5661e
*** Invalid hash given
pi@cumulusmxkitchen:~ $

@pelwell
Copy link
Contributor Author

pelwell commented Sep 16, 2024

Sorry - there are two parallel repos, and those hashes must be from the wrong one.

This should give a non-working system: sudo rpi-update 39f724da

And this should work: sudo rpi-update 44a287fe

@lgeitner
Copy link

I ran sudo rpi-update 39f724da
then sudo reboot
the meteopi did not work!
below is the output from uname-a

pi@cumulusmxkitchen:~ $ uname -a
Linux cumulusmxkitchen 6.6.45-v8-16k+ #1791 SMP PREEMPT Tue Aug 13 12:52:29 BST 2024 aarch64 GNU/Linux
pi@cumulusmxkitchen:~ $

I then ran sudo rpi-update 44a287fe
then sudo reboot
the meteopi did not work!
below is the output from uname-a

pi@cumulusmxkitchen:~ $ uname -a
Linux cumulusmxkitchen 6.6.44-v8-16k+ #1789 SMP PREEMPT Mon Aug 5 15:24:18 BST 2024 aarch64 GNU/Linux

I then ran sudo rpi-update 0ccee17 to revert the kernel to 6.6.31
then sudo reboot
the meteo pi did work!
below is the output from uname-a

pi@CumulusMX:~ $ uname -a
Linux CumulusMX 6.6.31+rpt-rpi-2712 #1 SMP PREEMPT Debian 1:6.6.31-1+rpt1 (2024-05-29) aarch64 GNU/Linux

what would the command be to update the kernel to 6.6.42?

I would like to see if the meteopi works with kernel 6.6.42

@pelwell
Copy link
Contributor Author

pelwell commented Sep 16, 2024

On this page you will find a list of kernels that you can install. The 7/8-digit hexadecimal number on each line is the value to pass to rpi-update. (Each build is actually represented by a much longer hex number, but any unique prefix should work).

@lgeitner
Copy link

I ran sudo rpi-update 1969fd7 then sudo reboot
The meteopi works fine!
uname -a shows the following:

pi@cumulusmxkitchen:~ $ uname -a
Linux cumulusmxkitchen 6.6.42-v8-16k+ #1786 SMP PREEMPT Thu Jul 25 14:11:18 BST 2024 aarch64 GNU/Linux
pi@cumulusmxkitchen:~ $

The issue with the meteopi not working occurs with kernel 6.6.44! How can I see what changes were made with this kernel version?

Thank You.

@pelwell
Copy link
Contributor Author

pelwell commented Sep 17, 2024

Clicking on any of the releases on that page takes you to another page that shows the contents (strictly, the changes introduced by that release). The file git_hash contains the full 40-digit hash that represents the state of the kernel source tree. The page can be slow to load from GitHub, so I can save you some time by saying that the relevant hashes are:
"kernel: Bump to 6.6.42" (1969fd7): bfbd468
"kernel: Bump to 6.6.44" (44a287f): 2806f39

Because GitHub recognises those hashes it has transformed them into links to the source code commits.

There are over 600 commits between those two points (git log --oneline 2806f39528464 | head -614), but if we limit it to only changes peculiar to our downstream tree, there are only 25 (git log --oneline --first-parent 2806f39528464 | head -25):

2806f39528464 Revert "mfd: Partial revert of Add rp1 driver"
7c66928a39e1a Merge remote-tracking branch 'stable/linux-6.6.y' into rpi-6.6.y
d0ea54e1c941a mfd: Partial revert of Add rp1 driver
216df57950849 DTS: bcm2712: enable SD slot CQE by default on Pi 5
3c319a466a1c7 dtoverlays: Document display_[width|height] on hd44780-lcd overlay
e94e761305fa2 dtoverlays: Add overlay for HD44780 via I2C PCF8574 backpack
a4bf61fad9fe1 fixup! pinctrl: bcm2712 pinctrl/pinconf driver
16d0ee22d2c0b hwmon: (adt7410) Add DT compatible strings
05e3687c6c973 overlays: i2c-rtc: Correct bq32000 property name
e9294823cf020 spi: dw: Clamp the minimum clock speed
199e611183de0 spi: dw: Fix non-DMA transmit-only transfers
70636ad110715 ARM: dts: bcm2712: Fix invalid polling-delay-passive setting
485d11cfa7df2 drm/vc4: Disable the 2pixel/clock odd timings workaround for interlaced
062434ab3be76 dts: rp1: restrict i2s burst lengths to 4
b6b4260fa546d sound/soc: dwc-i2s: choose FIFO thresholds based on DMA burst constraints
5112fd8cce4f1 DT: bindings: add a dma-maxburst property to snps,designware-i2s
f3cb675102a2a tty/serial: pl011: restrict RX burst FIFO threshold
1c4c90982462e Revert "spi: dw-dma: Get the last DMA scoop out of the FIFO"
ce56098eb4dc2 drivers: dw-axi-dmac: make more sensible choices about memory accesses
4c1a665b465fa dts: rp1: hobble DMA AXI burst lengths
3af7822df36e3 spi: dw: don't immediately kill DMA transfers if an error occurs
cd9084ceb606a spi: dw: Save bandwidth with the TMOD_RO feature
6014649de765a spi: dw: Save bandwidth with the TMOD_TO feature
31b9871b8895d staging: bcm2835-codec: Add support for H264 level 5.0 and 5.1
bfbd468e81def Merge remote-tracking branch 'stable/linux-6.6.y' into rpi-6.6.y

Further ignoring the Merge commits (the first because it is a placeholder for the upstream changes from that period, and the second because it is the known good state).

Looking at that list, a few things stand out - the various RP1 DMA changes, and a UART-driver change: f3cb675

Step one will be to create a PR with all of those downstream changes reverted (I hope that doesn't get messy), to give you another build to try. And this deserves a separate issue since we've established that it broke before the gpiochip0 change.

pelwell added a commit to pelwell/linux that referenced this pull request Sep 17, 2024
This reverts commit f3cb675.

Data loss has been observed on the Pi's PL011 UARTs since f3cb675 was
merged. The loss appears to be on reception, not transmission.

Revert the commit as a workaroud and potential fix.

Link: raspberrypi#6365
Link: raspberrypi#6144 (comment)
Link: https://forums.raspberrypi.com/viewtopic.php?t=376532

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants