New Revision b03115 of Raspberry Pi not identified correctly

[Mr-Bart-Simpson]

In priciple this is the same issue as in #1270 that was fixed with #1376 again.
I have a new revision that is not correctly detected as Rasperry Pi 4 and hence the same problem again.

Hint: I've posted the information below already as a comment at #1376, but @pgrawehr asked me to open a new issue here.

Details:
When looking at /proc/cpuinfo I get the following output (cut down to the relevant part, important parts highlighted as bold text):

Hardware : BCM2711
Revision : b03115
Serial : 10000000aeb9cbea
Model : Raspberry Pi 4 Model B Rev 1.5

This matches with the data that calling the pinout tool retrieves (again cut down):

Revision : b03115
SoC : BCM2711
RAM : 2GB

The "funny" part here is, that even the offical documentation does not list this board revision (but it states, that "this list is not exhaustive"...)

A quick fix for this issue could be to extend RaspberryBoardInfo.cs, line 166
0x3111 or 0x3112 or 0x3114 or 0x3115 => Model.RaspberryPi4,

But I'd like to question, if the approach to hardcode those regularly updated/supplemented numbers really is a good way? Shouldn't there better be an alternative (read: extensible) way, that let's one influence the board detection mechanism? Maybe some kind of delegate or by providing a kind of option pattern?

As @pgrawehr mentioned, the current way is the only reliable way to detect the board type, I want to clarify the concern I've written above:
I do not question the mechanism in general. I rather question the implementation. Resolving the board's revision number into the correct board type is basically a dictionary lookup. Currently this is a hard coded switch expression and I doubt this is a good way ragrding the "moving target" of revisions that are beeing released from time to time.
Maybe there is no better way to do it at the moment, then I'm fine with it - but I wanted to set you thinking about it... In my eyes there should at least be some kind of extensibility mechanism for that.

[pgrawehr]

I agree, the situation is bad. Maybe we could do some (maybe not so stable) heurisitcs in case of an unknown ID. Or just set the default to Raspi4, because that would now mostly be the right choice.

[W1zzardTPU]

Same problem here on a 4B bought a week ago in Germany

How about matching on Firmware & 0xFFF0 for models that seem to be getting a significant number of revisions?

[joperezr]

Thanks for logging the issue @Mr-Bart-Simpson. While I agree that the detection is not perfect and of course not future proof, in theory this shouldn’t at all be a blocker for you to use the new Pi. We have several underlying drivers which have the logic of performing GPIO operations, and among them we have the RaspberryPi driver and the UnixGpioDriver. The former one is very hardware specific since it deals with registers on the Pi’s memory directly, so it is of course faster, but it is also not guaranteed to work on every Pi in the future since new revisions could change the location of the registers we use. The latter one, is a generic driver which should work on most boards. The issue here, is that when you create a GpioController and do not pass a driver yourself, we will try to guess what the best driver is for you, and this is the place we read cpuinfo file and match against raspberry pi revisions. This same logic will fallback to the generic driver in case we don’t recognize the raspberry pi correctly. Additionally, all of this only happens if you didn’t pass in a driver yourself, if you are using a raspberry pi that is not recognized and you are sure that the raspberrypi driver is compatible with it, you can always just create a controller and pass in the raspberry pi driver to force the use of it.

Due to the fact that the faster driver deals with registers in memory directly, and how easy would it be for that to break on a future revision of the board, I feel like the system we have of being explicit on the boards that are supported by this driver is the way to go, especially for the automatic selection of a driver. As said before, if you do want to have the faster driver and are willing to take the risk of the driver not working on your board, you can always override this logic if you want to. I’m open to suggestions here, but I do still think that the right solution for us is to continue to manually adding revisions to our automatic detection only after testing and ensuring that such revision works correctly with the faster driver.

[Mr-Bart-Simpson]

I can follow @joperezr's point in general. Though, I had some trouble with injecting the driver by myself because it didn't allow me to set some pins to INPUT_PULLUP - but maybe that was an issue on my side. I will probably re-check that...

On the actual topic, I have to admit, I didn't have a detailed look into the documentation until now. But after doing so (especially at the structure of New-style Revision Codes), I agree with @W1zzardTPU's proposal.
When having a closer look at e.g. the revision number I get, it disassembles as follows:

0 0 0 000 0 0 1 011 0000 0011 00010001 0100

N O Q uuu W u F MMM CCCC PPPP TTTTTTTT RRRR

0 0 0   0 0 0 1   3    0    3     0x11    4
                            |        |    |
                            |        |    |-> Revision
                            |        |
                            |        |-> Type: 4B
                            |
                            |-> Processor: BCM2711

So maybe the current way to mask with & 0xFFFF is just not good here and the last four bits should be ignored, particularly because the information encoded there isn't really used for retreiving the model . Hence masking with &0xFFF0 should do the job.

Edited for some clarification:
"isn't really used for retreiving the model" here of course is only related to new-style revision codes. It probably would be the best to distinguish between old- and new-style revision codes in the first place and doing the proposed masking only for the latter...

[W1zzardTPU]

Then you should work with the Foundation to get new revisions added in a timely manner.

Also the code could be restructured so that people can actually force a specific hardware driver without having to fork the whole repo to make code changes (which is beyond most people's abilities anyway).

because it didn't allow me to set some pins to INPUT_PULLUP

Yeah that's the reason I'm posting here, too, the UnixGpioDriver doesn't support pullups because the underlying sysfs implementation doesn't support it. Yeah, we all know that we can always use external pullup resistors.

[Mr-Bart-Simpson]

@W1zzardTPU: Somewhat offtopic, but I just had to post again here, as your answer produced a big smile on my face (hope this doesn't violate the CoC rules here)...

Also the code could be restructured so that people can actually force a specific hardware driver without having to fork the whole repo to make code changes (which is beyond most people's abilities anyway).

That's exactly what I did for now - my programming skills are high in enough to do so, but I totally agree that this cannot be the way for each and every developer, especially the unexperienced ones... And I only did it because it a) was a hobby project that I b) wanted to get ready in a timely fashion. I would never ever have done that when it comes to professional development - at least not without having a very good reason and not without having checked all posibilities.

because it didn't allow me to set some pins to INPUT_PULLUP

Yeah that's the reason I'm posting here, too, the UnixGpioDriver doesn't support pullups because the underlying sysfs implementation doesn't support it. Yeah, we all know that we can always use external pullup resistors.

I have to admit that I'm a software guy and I'm quite unexperienced when it comes to hardware/electronics, except having some basic knowledge. But I've read an article now that states it's almost always better to use external pullup resistors (in my case because it eliminates some sources of interference). I think I'll stick with that and change my circuit accordingly. Maybe this will allow me to switch back to the (unfixed) original code/nuget package.

[W1zzardTPU]

that states it's almost always better to use external pullup resistors

This might be true when you have lots of noise from nearby AC or high-frequency signals, but that's not how most people are using the Raspberry (lots of jumper cables). And I'm not buying the "but you can see the resistors, so you can be sure they are there" argument, or am I supposed to print out my code, too?

[Mr-Bart-Simpson]

This might be true when you have lots of noise from nearby AC or high-frequency signals

That's exactly the case here. I'm trying to control some 230V AC devices with some relays. So noise from the AC is kind of guranteed.

The "you can see the resistors" argument doesn't convince me neither. If this was the case, why would anyone even come up with the idea to include internal resistors in the first place?

[pgrawehr]

@W1zzardTPU

Also the code could be restructured so that people can actually force a specific hardware driver without having to fork the whole repo to make code changes (which is beyond most people's abilities anyway).

That is possible. Just use .. = new GpioController(PinNumberingScheme.Logical, new RaspberryPi3Driver()).

[W1zzardTPU]

That's the first thing I tried :) Unfortunately it won't work, the library code still calls the model detection

iot/src/System.Device.Gpio/System/Device/Gpio/Drivers/RaspberryPi3Driver.cs

Line 88 in 2db2f02

_linuxDriver = CreateInternalRaspberryPi3LinuxDriver(out RaspberryBoardInfo boardInfo);

[Mr-Bart-Simpson]

To contribute to the solution of the original problem, I had a closer look to the documentation again. But this brings up some more questions...
I've done a very naive implementation just to show what I mean. This little commit just shows what (according to my understanding!) is written in the documentation:
You either have an old-style revision code (which are in the range of 0x02 to 0x15) -or- you have a new-style revision code. They can be distinguished by applying 0x800000 as a bit mask (i.e. the "New Flag").
For new-style revision codes, applying 0xFFF0 as a bit mask and shifting 4 bits to the right directly brings us to the list of board models from the documentation.
So far so good - at least when just looking at the description of the bit layout of the new-style codes (as I read and understand it...)

BUT, when looking at the list of new-style codes in use (or the current implementation), you'll see that there are a lot of numbers that are not covered by the naive implementation as they also use the higher bits of the value.
That brings me to the question: Am I not understanding the documentation correctly? Or is it just incomplete and does not list all types?
According to the actual values one could apply 0x0FF0 as bitmask (instead of 0xFFF0) and that seems to work for all models I've checked - but I can't see any evidence for that in the documentation...

[joperezr]

That's the first thing I tried :) Unfortunately it won't work, the library code still calls the model detection

iot/src/System.Device.Gpio/System/Device/Gpio/Drivers/RaspberryPi3Driver.cs

Line 88 in 2db2f02

_linuxDriver = CreateInternalRaspberryPi3LinuxDriver(out RaspberryBoardInfo boardInfo);

I believe this is a bug then. The board detection logic was mostly there for the automatic detection of which driver to use, but we should allow you overwriting it and pass the specific driver you want. I need to check again why we call this as part of the initialization of the RaspberryPi Driver, presumably it is because we need to get some initial values in order to know where to look for specific registers, but if that's the case I think we should add a new constructor to the RaspberryPi driver that recieves these values to be set instead of reading them from an internal table in case someone wanted to pass in the RaspberryPi driver manually.

[W1zzardTPU]

I believe this is a bug then

+1, or at least a design oversight. Should be a trivial code change, just need to expose a few things, remove a bunch of internals, and done. That's what I'm doing locally to work around the issue.

This would also make it easier to inherit from the hardware driver classes, so some functionality can be overridden. i.e. replace CreateInternalRaspberryPi3LinuxDriver() with own hardcoded logic

[pgrawehr]

@joperezr I think we just couldn't agree on a good, future-proof API for hardware and platform detection. The entire stuff is internal, because it is very hardware specific, and if we want to be generic, we need to make the hardware detection extensible, maybe with some global hooks.

[krwq]

@W1zzardTPU @Mr-Bart-Simpson any of you interested in sending a PR? Would be good to also check if there are any other missing values. If PR is sent this week we can make sure it's included in the next minor release

[joperezr]

Just to clarify, I think @krwq is asking about sending a PR to include the new revision, as opposed to one that changes the current extensibility model. For the latter, we should probably open a new API Proposal with an extensibility point that would allow folks to manually pass in a specific revision and the required info in order for a new board to work.

[Mr-Bart-Simpson]

@krwq I'm not really sure what you'd like to see in such a PR? As @joperezr pointed out, it could be "only" adding the new revision code to the existing implementation. That would be just what I mentioned in my very first post here

A quick fix for this issue could be to extend RaspberryBoardInfo.cs, line 166
0x3111 or 0x3112 or 0x3114 or 0x3115 => Model.RaspberryPi4,

When writing this, I thought this to be so trivial that creating a PR would somewhat be an overkill here. But if you wish...

OR

Did you mean some kind of implementation I've done and mentioned here:

I've done a very naive implementation just to show what I mean. This little commit just shows what (according to my understanding!) is written in the documentation.

This would somewhat justify a PR - but it should be taken into account what I've mentioned in the rest of the posting there...

[krwq]

@Mr-Bart-Simpson trivial change first (add new revision numbers maping), I don't think we're at a place where we can do it in any more future proof way