golemparts / rppal

A Rust library that provides access to the Raspberry Pi's GPIO, I2C, PWM, SPI and UART peripherals.
MIT License
1.2k stars 94 forks source link

i2c address on mainline kernels #80

Open steev opened 3 years ago

steev commented 3 years ago

On a mainline kernel (5.10) using an rpi4, the i2c address ends up being i2c-3 and not i2c-0 or i2c-1. This causes errors like Invalid slave address: 117 or Invalid slave address: 50 - I did a clone locally and changed the line that sets the address to 1 to be 3 and things work here. I reported this downstream to the PiSugar PowerManager repository at https://github.com/PiSugar/pisugar-power-manager-rs/issues/24 - I'm no rust expert and I'm not sure the best way to go about properly fixing this aside from forking the repository.

steev commented 3 years ago

This could also be a kernel bug, I haven't checked with the mailing list at all to find out.

steev commented 3 years ago

This also appears to be Pi4 specific as the pi3 only shows i2c-{0,1,2}

steev commented 3 years ago
diff --git a/src/i2c.rs b/src/i2c.rs
index b6783ed..5e6b60d 100644
--- a/src/i2c.rs
+++ b/src/i2c.rs
@@ -254,6 +254,7 @@ impl I2c {
     pub fn new() -> Result<I2c> {
         match DeviceInfo::new()?.model() {
             Model::RaspberryPiBRev1 => I2c::with_bus(0),
+            Model::RaspberryPi4B => I2c::with_bus(3),
             _ => I2c::with_bus(1),
         }
     }

Is the patch I'm currently using; this isn't likely correct, and should probably do some detection of if on a mainline vs raspbian kernel, but as far as I know, there isn't an "easy" way to do that. I was thinking something along the lines of (pseudo code) if /proc/device-tree/name == "" then mainline kernel else raspbian - but I'm not sure if name is empty on raspbian like it is on mainline.

golemparts commented 3 years ago

Thanks for reporting this issue. I'm not sure why you would see i2c-3 on those pins, other than because of a configuration issue in /boot/config.txt or a bug related to the mainline kernel. GPIO2 and GPIO3, bound to physical pins 3 and 5, should be set to SDA1/SCL1 on the Raspberry Pi 4, although they do have an alternate function to use them for SDA3/SCL3.

I'll have to take a deeper look and figure out what's causing it. You could try to manually assign i2c1 and i2c3 through /boot/config.txt, although I don't know if that takes precedence over whatever is causing i2c3 to be assigned to those pins.

steev commented 3 years ago

Yeah, I'm not entirely sure how to set it in config.txt with the mainline kernel, or the interaction between the mainline kernel and it. e.g. i don't have dtparam=i2c_arm=on in there, just have i2c-dev listed in /etc/modules

The kernel command line [ 0.000000] Kernel command line: dma.dmachans=0x37f5 bcm2709.boardrev=0xd03114 bcm2709.serial=0xce36f90d bcm2709.uart_clock=48000000 bcm2709.disk_led_gpio=42 bcm2709.disk_led_active_low=0 smsc95xx.macaddr=DC:A6:32:B0:07:C1 vc_mem.mem_base=0x3ec00000 vc_mem.mem_size=0x40000000 console=tty0 console=ttyS1,115200 root=/dev/mmcblk1p2 rw fsck.repair=yes net.ifnames=0 rootwait iomem=relaxed quiet splash

And like my other comment says, it works fine on the Pi3 (same card) - so it's definitely specific to the Pi4 on mainline.

golemparts commented 3 years ago

Considering i2c3 is a valid I2C bus for those pins, even though it's not the one that should be set by default, I'll add a check for it for the Raspberry Pi 4 when i2c1 isn't available.

golemparts commented 3 years ago

This has been fixed in the current master branch, and will be part of the upcoming 0.12.0 release.

golemparts commented 3 years ago

Version 0.12.0 is now live. Once PiSugar PowerManager switches over to using 0.12.0, that should fix your issue.

steev commented 3 years ago

Thanks! In looking closer, it seems the Raspbian kernel deletes an i2c node, and mainline kernels do not do so, on the Pi4, so it could be that that is why. I'm trying to free up some time to test if that actually works on a mainline kernel or not, to force it back to i2c-1.

steev commented 3 years ago

I tested the deleting the i2c node and that didn't work - I also bumped rppal locally, and that didn't work either - so i cloned rppal and swapped the logic, testing bus 3 first, and falling back to 1 if it fails, and that works. Could it be because i2c-1 also exists on the rpi4 with mainline kernel?

[ decker ~/git/pisugar-power-manager-rs] # i2cdetect -y 3
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:                         -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- 32 -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- 75 -- --
[ decker ~/git/pisugar-power-manager-rs] # i2cdetect -y 1
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:                         08 09 0a 0b 0c 0d 0e 0f
10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30: -- -- -- -- -- -- -- -- 38 39 3a 3b 3c 3d 3e 3f
40: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
60: 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e 6f
70: 70 71 72 73 74 75 76 77
[ decker ~/git/pisugar-power-manager-rs] #
golemparts commented 3 years ago

Could it be because i2c-1 also exists on the rpi4 with mainline kernel?

On the PI 4B, i2c1 can be bound to either the default pins, or to the (internal) GPIO lines 44 and 45, which is likely happening in your case. Unfortunately I can't switch the checks, because someone could theoretically have set up i2c3 on GPIO 4 and 5, and i2c3 on GPIO 2 and 3, so that would enable the wrong I2C bus by default.

This sounds like something that needs to be changed in the mainline kernel, since it doesn't really make sense to have a different I2C bus bound to GPIO2 and GPIO3 by default. I could add a check like you mentioned, although I'm not quite sure how to check if we're running a mainline kernel or Raspberry Pi OS/Raspbian. /proc/device-tree/name is empty on there as well.

I'll re-open this issue for now until we can get this resolved.

golemparts commented 3 years ago

Actually, could you show me the output of hostnamectl, specifically the Operating System line?

golemparts commented 3 years ago

And while we're at it, lsb_release -a as well?

steev commented 3 years ago

lsb_release -a:

lsb_release -a
No LSB modules are available.
Distributor ID: Kali
Description:    Kali GNU/Linux Rolling
Release:        2021.1
Codename:       kali-rolling

Note: this is NOT a common setup - this is done by me using the same scripts that Debian uses to create theirs, just pointed at our (I'm a Kali dev) repos instead of debian's, and using our 5.10 kernel instead of the usual raspberry pi foundation's kernel. But it would be the same as a debian setup from https://wiki.debian.org/RaspberryPiImages

golemparts commented 3 years ago

Let's try a different approach, as basing the selection on which kernel is used could lead to problems down the road when this configuration issue is fixed.

Instead, I'd like to see if I can check the mode the relevant pins (GPIO2 and GPIO3 for physical pins 3 and 5) are set to. Normally this should be easy enough, but it relies on the GPIO peripheral being configured properly by default, which I have my doubts about considering these I2C issues.

I'm not sure how experienced you are in using Rust, so to keep things simple, and since you already have rppal cloned locally, can you run the gpio_status example with cargo run --example gpio_status and paste the output? If that works, we'll know the GPIO peripheral can be used, and if the mode for GPIO2/3 is set to either ALT0 (I2C1) or ALT5 (I2C3) so we can use that to select the correct I2C bus.

steev commented 3 years ago

I only know enough rust to be dangerous. Not enough to write my own app, but enough to poke at others :D

That said... here is the output:

[ decker ~/git/rppal] # cargo run --example gpio_status
  Downloaded simple-signal v1.1.1
  Downloaded 1 crate (3.2 KB) in 1.21s
   Compiling libc v0.2.93
   Compiling lazy_static v1.4.0
   Compiling simple-signal v1.1.1
   Compiling rppal v0.12.0 (/root/git/rppal)
    Finished dev [unoptimized + debuginfo] target(s) in 25.23s
     Running `target/debug/examples/gpio_status`
+------+-------+---+---------+---+-------+------+
| GPIO | Mode  | L |   Pin   | L | Mode  | GPIO |
+------+-------+---+----+----+---+-------+------+
|      | 3.3 V |   |  1 |  2 |   | 5 V   |      |
|    2 | ALT0  | 1 |  3 |  4 |   | 5 V   |      |
|    3 | ALT0  | 1 |  5 |  6 |   | GND   |      |
|    4 | IN    | 1 |  7 |  8 | 1 | ALT5  |   14 |
|      | GND   |   |  9 | 10 | 1 | ALT5  |   15 |
|   17 | IN    | 0 | 11 | 12 | 0 | IN    |   18 |
|   27 | IN    | 0 | 13 | 14 |   | GND   |      |
|   22 | IN    | 0 | 15 | 16 | 0 | IN    |   23 |
|      | 3.3 V |   | 17 | 18 | 0 | IN    |   24 |
|   10 | IN    | 0 | 19 | 20 |   | GND   |      |
|    9 | IN    | 0 | 21 | 22 | 0 | IN    |   25 |
|   11 | IN    | 0 | 23 | 24 | 1 | IN    |    8 |
|      | GND   |   | 25 | 26 | 1 | IN    |    7 |
|    0 | ALT0  | 1 | 27 | 28 | 1 | ALT0  |    1 |
|    5 | IN    | 1 | 29 | 30 |   | GND   |      |
|    6 | IN    | 1 | 31 | 32 | 0 | IN    |   12 |
|   13 | IN    | 0 | 33 | 34 |   | GND   |      |
|   19 | IN    | 0 | 35 | 36 | 0 | IN    |   16 |
|   26 | IN    | 0 | 37 | 38 | 0 | IN    |   20 |
|      | GND   |   | 39 | 40 | 0 | IN    |   21 |
+------+-------+---+----+----+---+-------+------+
golemparts commented 3 years ago

Thanks for testing that. While it's great that works, the results are odd to say the least. Did you run that on the Pi where I2C3 has to be selected? Because that output indicates GPIO2 and 3 are configured as ALT0, which would mean I2C1 is active on those pins, so needing to use /dev/i2c-3 in linux makes no sense. That would mean the I2C buses are configured properly by default after all, but the i2c device numbering in linux is wrong, which I would definitely consider a bug.

If that's the case, I'm afraid there's not much I can do with rppal. I can't rely on checking if you're on the mainline kernel, since this bug might be fixed in the future which would result in rppal returning the wrong bus. I can't rely on checking which I2C bus is active on those pins, because linux is using an incorrect numbering scheme in your case. I think your best bet is to get this addressed in the mainline kernel, and until that's fixed, fork PiSugar PowerManager and instead of using the new method to select a bus, use with_bus instead and hardcode it to use 3.

steev commented 3 years ago

This is indeed on a Pi4 8GB, with the pisugar 2 pro connected to it, using the debian/kali 5.10 kernel. I'll definitely send a mail to the i2c/rpi kernel mailing lists to figure out what is going on. I'm definitely not an i2c expert by any stretch of the imagination, and I'm not sure what is going on either.

Based on a look at https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/drivers/i2c/busses/i2c-bcm2835.c - the i2c driver hasn't been touched since September of 2020, so it has been broken for a long while. I'll be using the output of the example in my mail to the mailing list as well, so thank you for that!

steev commented 3 years ago

For what it's worth, I ran the example on a different rpi 4 8GB (with the raspberrypi foundation 5.10 kernel) and the output is slightly different:

+------+-------+---+---------+---+-------+------+
| GPIO | Mode  | L |   Pin   | L | Mode  | GPIO |
+------+-------+---+----+----+---+-------+------+
|      | 3.3 V |   |  1 |  2 |   | 5 V   |      |
|    2 | ALT0  | 1 |  3 |  4 |   | 5 V   |      |
|    3 | ALT0  | 1 |  5 |  6 |   | GND   |      |
|    4 | IN    | 0 |  7 |  8 | 1 | IN    |   14 |
|      | GND   |   |  9 | 10 | 1 | IN    |   15 |
|   17 | IN    | 0 | 11 | 12 | 0 | ALT0  |   18 |
|   27 | IN    | 0 | 13 | 14 |   | GND   |      |
|   22 | IN    | 0 | 15 | 16 | 1 | IN    |   23 |
|      | 3.3 V |   | 17 | 18 | 0 | IN    |   24 |
|   10 | ALT0  | 0 | 19 | 20 |   | GND   |      |
|    9 | ALT0  | 0 | 21 | 22 | 0 | IN    |   25 |
|   11 | ALT0  | 0 | 23 | 24 | 1 | OUT   |    8 |
|      | GND   |   | 25 | 26 | 1 | OUT   |    7 |
|    0 | IN    | 1 | 27 | 28 | 1 | IN    |    1 |
|    5 | IN    | 1 | 29 | 30 |   | GND   |      |
|    6 | IN    | 1 | 31 | 32 | 0 | IN    |   12 |
|   13 | IN    | 0 | 33 | 34 |   | GND   |      |
|   19 | ALT0  | 0 | 35 | 36 | 0 | IN    |   16 |
|   26 | IN    | 0 | 37 | 38 | 0 | ALT0  |   20 |
|      | GND   |   | 39 | 40 | 0 | ALT0  |   21 |
+------+-------+---+----+----+---+-------+------+
golemparts commented 3 years ago

The differences on the other pins can be explained as either Raspberry Pi OS having a different default configuration where some peripherals are enabled by default, or changes made manually through either raspi-config or /boot/config.txt.

If you're curious what those alternate functions do, check out chapter 5.3 in https://datasheets.raspberrypi.org/bcm2711/bcm2711-peripherals.pdf