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

GpioController is not correctly identifying Raspberry Pi after latest Raspbian kernel updates #1145

Closed
WoutStandaert opened this issue Jul 30, 2020 · 19 comments · Fixed by #1200
Labels
bug Something isn't working

Comments

@WoutStandaert
Copy link

Describe the bug

After running apt-get upgrade on a Raspberry Pi 4B, I couldn't set pinmodes to pullup or pulldown anymore. Instead, I was getting an error: Unhandled exception. System.InvalidOperationException: The pin does not support the selected mode.

After checking the System.Device.Gpio project I found that GpioController is trying to know if it's running on a Raspberry Pi via /proc/cpuinfo. There's a field in there called Hardware and it returns the processor type. Apparently in older Raspbian kernels it was returning the wrong value, since even on a Raspberry Pi 4 it would return "BCM2835". This is the value that GpioController is comparing against.

Since the kernel update however it seems to be returning the correct value, which for a Raspberry Pi 4 is "BCM2711".

In fact, BCM2835 is only correct for the first Raspberry Pi, the other models are listed here: https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2711/README.md

Current workaround is to manually specify a GpioDriver and pass it to the GpioController constructor:

GpioDriver driver = new RaspberryPi3Driver();
Controller = new GpioController(PinNumberingScheme.Logical, driver);

Steps to reproduce

Update Raspbian on Raspberry Pi 4b to latest kernel via:

sudo apt-get update
sudo apt-get upgrade

and reboot the Pi. Then the following code will fail:

Controller = new GpioController();
Controller.OpenPin(26, PinMode.InputPullUp);

Expected behavior

The pin would be opened and the internal pullup resistor turned on.

Actual behavior

Exception is raised: System.InvalidOperationException: The pin does not support the selected mode.

Versions used
Target framework: netcoreapp3.1
System.Device.Gpio version: 1.1.0-prerelease.20276.1

Running self-contained on Raspberry Pi 4b, with kernel version 5.4.51-v7l+ (before the update it was 4.19.118-v7l+)

@WoutStandaert WoutStandaert added the bug Something isn't working label Jul 30, 2020
@WoutStandaert
Copy link
Author

According to the Raspberry Pi foundation the Hardware field shouldn't be used to determine CPU type, go figure :)

https://www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md

@pgrawehr
Copy link
Contributor

pgrawehr commented Aug 1, 2020

This looks more like failing to determine whether its a Pi at all. Note that there's also a test to differentiate a Pi3 from a Pi4 (but that would unlikely cause the given exception).

@WoutStandaert
Copy link
Author

You might be right, but I don't have a Pi3 on hand to check if the model number is also updated there.

@WoutStandaert WoutStandaert changed the title GpioController is not correctly identifying Raspberry Pi 4 after latest Raspbian kernel updates GpioController is not correctly identifying Raspberry Pi after latest Raspbian kernel updates Aug 2, 2020
@krwq
Copy link
Member

krwq commented Aug 6, 2020

@WoutStandaert CI is using PI3s, you might be able to write a check against that. At least the page above gave a better solution to check for model which we might use instead

@pgrawehr
Copy link
Contributor

pgrawehr commented Aug 6, 2020

@krwq: Maybe #766 or #1034 already contain the correct solution. Please have a look especially at the later one.

@krwq
Copy link
Member

krwq commented Aug 6, 2020

@pgrawehr for #766 it seems to be reading hardware line which is not recommended. For #1034 I'd prefer to wait with any design conversations and new APIs until we ship .NET 5... I'm currently a bit occupied with adding nullability annotations...

@pgrawehr
Copy link
Contributor

pgrawehr commented Aug 6, 2020

Is .net core 5 still scheduled for September?

@joperezr
Copy link
Member

Is .net core 5 still scheduled for September?

If you mean GA, then it is actually scheduled for November, not September.

@pgrawehr
Copy link
Contributor

Then we should definitelly find a solution for this before that. @joperezr Can you have a look at #1034, please?

@joperezr
Copy link
Member

Yup, we will fix this definitely before that. I'm pretty sure we know that the problem is related to the new way to PInvoke to WinRT packages which we have to update before shipping. I'll take care of that in the next few days.

@joperezr
Copy link
Member

I just did a bit more digging here and found that actually looks like it is good that our detection logic is not picking the RaspberryPi3Driver, since that one works directly with the registers and those addresses have changed now according to the Datasheets. RPi3 uses BCM2835 which has the base address for GPIO at 0x7E20_0000 whereas Rpi4 uses BCM2711 which has the base address shifted to 0x7E21_5000. I need to test whether or not the logic we have on the Rpi Linux driver with both /dev/gpiomem and to get the base address when using /dev/mem still works in these cases.

@pgrawehr
Copy link
Contributor

Two observations from my side:

  • The RaspberryPi3Driver works fine on the Pi4. I've always used that one and it has never had any problems (except for the pull up/pull down stuff, but that's fixed since long). So the register address detection works.
  • When (due to the bug in our detection routine) the LibgpiodDriver is used instead of the Raspi driver, everything should be fine as well, since with the current build, that one also supports the pulls.

@krwq
Copy link
Member

krwq commented Sep 15, 2020

@joperezr any progress here? what are the next steps? Is only detection broken?

@pgrawehr
Copy link
Contributor

Yea, I think only the detection is broken (or using an approach that no longer works, because it had always been wrong in the OS). I do think we should fix it correctly and do something like #1034.We should have some generic hardware detection implementation or at least abstractions for it.

@krwq
Copy link
Member

krwq commented Sep 15, 2020

I agree but for now I propose to fix just this specific issue while we figure out long term approach for any board. I'm getting more crisper idea of what the next steps should be here but I don't think we should start with auto detection logic

@pgrawehr
Copy link
Contributor

A straight forward patch would be quite easy to do with the type information (see link above). I think we should at least fix the CM3 support in the same run as well. That would however require the new driver to be added to SDG.

@krwq
Copy link
Member

krwq commented Sep 18, 2020

@pgrawehr I'm fine with that PR but I do not want any new APIs in SDG as part of it, I'd prefer we've split SDG into abstraction and implementation. We're currently brainstorming about how to align board abstraction with other repos and we already have some idea what we could possibly do

@pgrawehr
Copy link
Contributor

Yea, having some abstraction layer is certainly a good idea (especially some plug-and-play approach), but that proposal there seems like a long way to go and is likely to much to be done for the next release. So we could update that PR with hiding the implementation? (Note however that I'm not the owner of it).

@krwq
Copy link
Member

krwq commented Sep 24, 2020

I'm fine with hiding the implementation details (no new public APIs as part of the fix)

@krwq krwq closed this as completed Sep 24, 2020
@krwq krwq reopened this Sep 24, 2020
@krwq krwq closed this as completed in #1200 Oct 2, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Nov 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants