Skip to content

/proc/cpuinfo reporting BCM2835 on BRANCH=next rpi-update #705

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

Closed
joolswills opened this issue Dec 30, 2016 · 37 comments
Closed

/proc/cpuinfo reporting BCM2835 on BRANCH=next rpi-update #705

joolswills opened this issue Dec 30, 2016 · 37 comments

Comments

@joolswills
Copy link

I had a user report this over at the retropie bugtracker

RetroPie/RetroPie-Setup#1809

That when using BRANCH=next rpi-update on a rpi3 /proc/cpuinfo returns Hardware : BCM2835 rather than Hardware : BCM2709 as it is currently.

Is this intentional and will this change also for the rpi zero/1?

@joolswills joolswills changed the title /proc/cpuinfo reporting BCM2709 on BRANCH=next rpi-update /proc/cpuinfo reporting BCM2835 on BRANCH=next rpi-update Dec 30, 2016
@clivem
Copy link

clivem commented Dec 30, 2016

raspberrypi/linux#1725

@joolswills
Copy link
Author

Thanks - so reading that thread I'm not sure if it's planned to be changed or not - It will break out existing platform detection on RetroPie and I know of a few other projects which rely on this for detecting the system.

@clivem
Copy link

clivem commented Dec 30, 2016

It will break out existing platform detection on RetroPie and I know of a few other projects which rely on this for detecting the system

Yes, I pointed that out. But @pelwell seemed to think that userspace tools should have to change to follow the kernel change.....

@pelwell
Copy link
Contributor

pelwell commented Dec 30, 2016

It is an unavoidable side effect of using more upstream code, which is our continued aim.

@joolswills
Copy link
Author

joolswills commented Dec 30, 2016

I don't think this is a wise choice - even if it's easier for the kernel development (I'm certain you will get lots of reports once this is the main kernel). However if we have to change our code, please could you let us know another reliable way to detect exactly which board we are running on etc?

@joolswills
Copy link
Author

joolswills commented Dec 30, 2016

I currently have

    case "$(sed -n '/^Hardware/s/^.*: \(.*\)/\1/p' < /proc/cpuinfo)" in
        BCM2708)
            __platform="rpi1"
            ;;
        BCM2709)
            local revision="$(sed -n '/^Revision/s/^.*: \(.*\)/\1/p' < /proc/cpuinfo)"
            if [[ "$revision" == "a02082" || "$revision" == "a22082" ]]; then
                    __platform="rpi3"
            else
                __platform="rpi2"
            fi
            ;;
    esac

@joolswills
Copy link
Author

Just an idea of how much code relies on this - https://github.com/search?q=cpuinfo+BCM2709&type=Code&utf8=%E2%9C%93

@pelwell
Copy link
Contributor

pelwell commented Dec 30, 2016

/proc/device-tree/machine contains (and will always contain) a human-readable version of the board version. The Revision code of /proc/cpuinfo contains enough information to work out which board it is - the meaning of the bits is published, even if it is slightly complicated by the old/new format.

Essentially the logic is:

  • If the "new format" bit (0x00800000) is set then the model number is in bits 12-15 (ish - from memory).
  • Otherwise it is a Pi1.

@joolswills
Copy link
Author

joolswills commented Dec 30, 2016

It's going to be a pain having to handle the old and new methods (or switch over), and looking at how much software will have to be changed, I really hope you will reconsider this. I will look at using the Revision code though in the meantime. Thanks.

@MilhouseVH
Copy link

@pelwell
Copy link
Contributor

pelwell commented Dec 30, 2016

I don't think using the Revision value correctly is any more arduous than using both Hardware and Revision.

@joolswills
Copy link
Author

Depends which language - in a Makefile it certainly is. But certainly extracting bits from a value is more involved imho.

My last argument against the change (Then I shall be quiet!) :)

Looking at the code search results, there will be hundreds of problematic programs which will not work when the switch happens. 610 shell code matches, 285 makefiles, 267 python files,467 C files etc.

Even if some authors update their code, there will be a period when a lot of stuff will be broken. Then you have unmaintained applications which will need to rely on other people to "fix them". You are going to get a lot of people complaining about this (as are application authors).

I don't think it's a good idea to break hundreds of pieces of code by changing something that has been in place for so many years.

@popcornmix
Copy link
Contributor

And parsing the revision code fully means:

  • It won't break when a Pi3 rev 1.3, 1.4 1.5 etc appear
  • It will do the right thing on 2837 Compute Module
  • It will work with users running the upstream kernel currently

@pelwell
Copy link
Contributor

pelwell commented Dec 30, 2016

Are you aware of any Pi-published decription of the Hardware field? The fact that people have chosen to use this field out of apparent convenience is unfortunate.

@pelwell
Copy link
Contributor

pelwell commented Dec 30, 2016

I would suggest that we could Device Tree properties with the different fields broken out, but that won't help on all the old devices out there; proper Revision parsing will work on all devices.

@joolswills
Copy link
Author

Sure, and I see the argument for using the field, and yes it is unfortunate. But I guess developers went for what they thought was the simplest option. We are still at the point the change will break things. But ok - I will fix up retropie to use the revision code, and I hope other projects will do so to. I still think this is going to cause a lot of user complaints - if you don't mind about that - that's fine.

@pelwell
Copy link
Contributor

pelwell commented Dec 30, 2016

I still think this is going to cause a lot of user complaints - if you don't mind about that - that's fine.

Maintaining backwards compatibility is something we take seriously, but on this occasion I can live with the outcome; the fact that the Hardware field was pretty meaningless on a Pi3 makes it easier.

@popcornmix
Copy link
Contributor

I still think this is going to cause a lot of user complaints - if you don't mind about that - that's fine.

It is being introduced gradually. Currently it is just users on BRANCH=next rpi-update.
It will appear on master rpi-update in the near future. Getting into a standard raspbian image is probably a couple of months off.
Hopefully there will be testers (who know rpi-update could break things) who will report issues with applications that suffer from this change, and those applications can be fixed up before it is needed in default raspbian images.

@clivem
Copy link

clivem commented Dec 30, 2016

but on this occasion I can live with the outcome

I have my popcorn at the ready. ;)

The thing is, and I kind of agree with Phil, that they should stick to their guns over this, for the reasons stated.... But there is going to be a cacophony of squawking over this from users, who do not understand why things do not "just work" anymore, once it hits master, followed by the RPi devs telling those users that they need to have a word with the userspace and distribution devs about fixing their software, a bunch of pissed-off userspace devs getting earache from users, like it is their fault for relying on that undocumented hardware field..... Me thinks the effects of this change are going to cause more consternation and gnashing of teeth than anyone anticipates right now. I hope to be proven wrong. If not I'll sit back with popcorn in hand and watch with a grin as this one plays out......

@joolswills
Copy link
Author

joolswills commented Dec 30, 2016

I hope so - I just don't want to be the one that has to fix up everyones code (RetroPie ships with a lot of 3rd party apps that may be affected). I have some bash code to use the revision now for retropie at least

                local revision="$(sed -n '/^Revision/s/^.*: \(.*\)/\1/p' < /proc/cpuinfo)"
                if [[ 0x$revision -lt 16 ]]; then
                    __platform="rpi1"
                else
                    local cpu=$(((0x$revision >> 12) & 15))
                    case $cpu in
                        0)
                            __platform="rpi1"
                            ;;
                        1)
                            __platform="rpi2"
                            ;;
                        2)
                            __platform="rpi3"
                            ;;
                    esac
                fi

@joolswills
Copy link
Author

@clivem Unfortunately I think you are right.

@popcornmix
Copy link
Contributor

@joolswills I don't think your bash code does the right thing for revisions 0x10-0x15 (B+ and A+).
I think the else clause should be predicated on bit 23 being set.

@joolswills
Copy link
Author

@popcornmix Not sure I understand - if [[ 0x$revision -lt 16 ]]; then should pick up the older revision correctly no ? Can you give me a revision number you think it will fail on ?

@joolswills
Copy link
Author

aah hex 0x15.. sorry. I misread the docs.

@clivem
Copy link

clivem commented Dec 30, 2016

I think the most important thing, is to make sure Gordon has changed the wiringPi code, and distributions are shipping it before a kernel including this change becomes the "default". I gave him a heads-up but I don't know if it has resulted in him changing his platform detection code. It's wiringPi I suspect, that will cause the most grief, if it does what it currently does and just bombs out with that Hardware field change.

@popcornmix
Copy link
Contributor

I believe revisions 0x10, 0x11, 0x12, 0x13, 0x14, 0x15 will fall into the else clause rather than be handled by the if clause. Now, I think it works by luck as bits 12-15 are zero, but it's best to not rely on that.

@joolswills
Copy link
Author

                if [[ 0x$rev -le 0x0015 ]]; then

should be ok though no ? or are you saying it could still fail because the other bits could be non zero on an older board ?

@joolswills
Copy link
Author

joolswills commented Dec 30, 2016

I actually had 0x0015 in before and somehow "optimised" it to lt 16 :). my brain forgot it was hex.

@popcornmix
Copy link
Contributor

popcornmix commented Dec 30, 2016

It will work but it's not following the definition of the bitfield. In theory a 0x234 could exist that we haven't told you about yet (but it shouldn't be decoded using the bitfield description as bit 23 is not set).
I'd suggest:

if [[ $(((0x$rev >> 23) & 1)) -eq 0 ]]; then

is actually implementing what we have specified to decode the field.

@pelwell
Copy link
Contributor

pelwell commented Dec 30, 2016

I also warned Gordon, and he said it was in hand. Looking at http://git.drogon.net/?p=wiringPi;a=commitdiff;h=b1dfc186efe327aa1d59de43ef631a2fa24e7c95 suggests the updated code is ready.

@joolswills
Copy link
Author

joolswills commented Dec 30, 2016

@popcornmix I'll change it as you have suggested. Thanks. :)

@popcornmix
Copy link
Contributor

Actually looking at the firmware revision decoding boards exist up to 0x1b (same board, different manufacturer), so it's best not to hard code a restriction when no restriction is necessary.

We have specified that bit 23 being set means decode using bitfields.
Bit 23 not set means you have to use a look up table. Fortunately all options in this table are Pi 1 variants, so in your case you don't need to know the exact details.

@joolswills
Copy link
Author

@popcornmix thanks - makes sense.

@Ruffio
Copy link

Ruffio commented Jan 14, 2017

@joolswills can this issue be closed?

@dlech
Copy link

dlech commented Oct 26, 2017

We have specified that bit 23 being set means decode using bitfields.

Is there documentation on these bitfields somewhere? In particular, I would like to know how to tell the difference between Zero models and 2/3 models.

@pelwell
Copy link
Contributor

pelwell commented Oct 26, 2017

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

@JamesH65
Copy link
Contributor

OK, I think this can be closed. The expected inundation of complaints didn't really happen. Yes, we have a few, but then again, too few to mention. We did what we had to do, and saw it through without exemption.

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

No branches or pull requests

8 participants