Skip to content

Add support for the BCM2711 - #15188

Merged
xiaoxiang781216 merged 1 commit into
apache:masterfrom
linguini1:bcm2711
Dec 17, 2024
Merged

Add support for the BCM2711#15188
xiaoxiang781216 merged 1 commit into
apache:masterfrom
linguini1:bcm2711

Conversation

@linguini1

@linguini1 linguini1 commented Dec 13, 2024

Copy link
Copy Markdown
Contributor

Summary

Closes #11586.

This PR adds semi-functional support for the BCM2711 and Raspberry Pi 4B. The working features are a Mini-UART shell which can boot into NSH. I have run OStest and confirmed it works. The latest changes also boot.

Impact

Although the feature implementation for this board is minimal right now, I hope that by releasing the work I've done so far it is easier for other community members to start adding drivers/features to the board's support on NuttX. Several people were interested in using the Pi 4B with NuttX and it's a reasonably popular development board, so having it fully supported will be an asset.

Testing

Built the documentation, ensured it looks correct.

Built the written code so far, which was tested by:

  • Connecting the RPi 4B to my computer via USB-TTL cable and playing around in the shell
  • Running test programs like OStest and getprime (succeeded)
  • Running i2ctool with some I2C devices connected to the board. I was able to dump the contents of an EEPROM but not much more, I am confused by the I2C register interface and haven't spent enough time to overcome these issues. Partial completeness is noted on the docs page.
  • Tested GPIO driver interface by performing basic operations such as echo 1 > /dev/gpio23 and checking with a multimeter that the GPIO has gone high, or connecting input to a GPIO and performing cat /dev/gpio23, etc.

OStest results log are attached
ostest-bcm2711.txt

@github-actions github-actions Bot added Area: Documentation Improvements or additions to documentation Area: Tooling Arch: arm64 Issues related to ARM64 (64-bit) architecture Board: arm64 Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. labels Dec 13, 2024
@linguini1

linguini1 commented Dec 13, 2024

Copy link
Copy Markdown
Contributor Author

It is going to take me a while to squash all these commits

@acassis

acassis commented Dec 13, 2024

Copy link
Copy Markdown
Contributor

It is going to take me a while to squash all these commits

Hi @linguini1 I suggest you to squash in a way that the mental logic you used to get this port working be preserved.

You can use squash the commits that created the headers files and the commits that fix them. Then they will appear early as an initial commit.

Preserving it will be useful to help other people to do the port to other chips.

@xiaoxiang781216 what do you think?

Comment thread arch/arm64/src/bcm2711/bcm2711_serial.c Outdated
Comment thread arch/arm64/src/bcm2711/bcm2711_serial.c Outdated
Comment thread arch/arm64/src/bcm2711/bcm2711_serial.h Outdated
Comment thread arch/arm64/src/bcm2711/bcm2711_serial.h Outdated
Comment thread arch/arm64/src/bcm2711/hardware/bcm2711_armtimer.h Outdated
Comment thread boards/arm64/bcm2711/raspberrypi-4b/src/rpi4b_gpio.c Outdated
Comment thread boards/arm64/bcm2711/raspberrypi-4b/src/rpi4b_gpio.c Outdated
Comment thread boards/arm64/bcm2711/raspberrypi-4b/src/rpi4b_gpio.c Outdated
Comment thread boards/arm64/bcm2711/raspberrypi-4b/src/rpi4b_gpio.c Outdated
Comment thread boards/arm64/bcm2711/raspberrypi-4b/src/rpi4b_gpio.c Outdated
Comment thread .gitignore Outdated
Comment thread arch/arm64/Kconfig Outdated
Comment thread arch/arm64/src/bcm2711/bcm2711_spi.c Outdated

@hartmannathan hartmannathan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't reviewed yet but I would like to express support -- this is definitely an asset to NuttX. Also, it is a good idea to make it possible for the community to help develop it.

I think it's a good idea to mention in the Docs what is known to work right now and what is missing, to help set the right expectations and guide other interested people at what they could work on to improve support.

Comment thread Documentation/platforms/arm64/bcm2711/boards/raspberrypi-4b/index.rst Outdated
Comment thread Documentation/platforms/arm64/bcm2711/boards/raspberrypi-4b/index.rst Outdated
Comment thread Documentation/platforms/arm64/bcm2711/boards/raspberrypi-4b/index.rst Outdated
Comment thread Documentation/platforms/arm64/bcm2711/boards/raspberrypi-4b/index.rst Outdated
Comment thread tools/bcm2711/Config.mk Outdated

@hartmannathan hartmannathan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't do it yet, but when it's getting close to ready for merge, it will be a good idea to squash the commits.

@acassis

acassis commented Dec 15, 2024

Copy link
Copy Markdown
Contributor

Don't do it yet, but when it's getting close to ready for merge, it will be a good idea to squash the commits.

@hartmannathan I suggest squashing it in a logic way to preserve the history, this way it could help other people doing ports in the future. For instance all commits adding the headers files or fixing it could be squashed together.

This way it will be reduced for about 10 commits or so. What do you think?

@linguini1

Copy link
Copy Markdown
Contributor Author

I suggest squashing it in a logic way to preserve the history, this way it could help other people doing ports in the future.

I also took some notes while doing this port to attempt to make it easier for the next people trying to port new boards. I don't think they're incredibly helpful but I can post them somewhere if it might help.

@hartmannathan

Copy link
Copy Markdown
Contributor

Don't do it yet, but when it's getting close to ready for merge, it will be a good idea to squash the commits.

@hartmannathan I suggest squashing it in a logic way to preserve the history, this way it could help other people doing ports in the future. For instance all commits adding the headers files or fixing it could be squashed together.

This way it will be reduced for about 10 commits or so. What do you think?

@acassis yes, this is a good idea. (I was only suggesting to reduce the 173 commits but it doesn't need to be squashed into 1.)

@hartmannathan

Copy link
Copy Markdown
Contributor

I suggest squashing it in a logic way to preserve the history, this way it could help other people doing ports in the future.

I also took some notes while doing this port to attempt to make it easier for the next people trying to port new boards. I don't think they're incredibly helpful but I can post them somewhere if it might help.

@linguini1 there is a porting guide at https://nuttx.apache.org/docs/latest/guides/port.html (Documentation/guides/port) and if you have anything helpful to add there, please do!

Another idea is to add a separate document: Porting Case Studies and document the RPi 4b port as a case study.

@linguini1
linguini1 force-pushed the bcm2711 branch 2 times, most recently from 930bf93 to 54d80e4 Compare December 15, 2024 20:12
@linguini1
linguini1 marked this pull request as ready for review December 15, 2024 20:14
@linguini1

Copy link
Copy Markdown
Contributor Author

Not too sure from reading the error logs why this build is failing. Anyone have an idea?

@hartmannathan

Copy link
Copy Markdown
Contributor

@linguini1 please squash into one patch

I was suggested to keep a few logical commits in case it helps others; is this still the case?

@linguini1 @xiaoxiang781216 I suggested him to reduce to few numbers of commits that preserve the logic process to adding this port, to be used as future reference. Nathan also agreed on that.

Yes that's correct. I replied to xiaoxiang781216 before I noticed that the 170+ commits were already squashed to 4. I think it is OK to keep it as-is. Only the defconfig issue needs to be updated and then I think it will be ready for merge.

@xiaoxiang781216

xiaoxiang781216 commented Dec 16, 2024

Copy link
Copy Markdown
Contributor

@linguini1 please squash into one patch

I was suggested to keep a few logical commits in case it helps others; is this still the case?

Yes, but your later commit fixes many errors in early commit. You should ensure each commit can be build and pass ci correctly, instead ignoring them until the last commit.

For example:

  1. "arch/bcm2711 Working Mini-UART serial driver" should only contain serial driver related change
  2. "arch/bcm2711 Working GPIO driver for the RPi4B" should only contain gpio driver related change
    ...

So, you either squash ALL commit into one patch, or split the huge change into the independent patchset. Actually, I prefer you select the second approach, but you need organize your patch clearly: one patch adds one driver and not touch other driver.

@linguini1

Copy link
Copy Markdown
Contributor Author

Yes, but your later commit fixes many errors in early commit. You should ensure each commit can be build and pass ci correctly, instead ignoring them until the last commit.

Oh I see, that makes sense. Okay, I'll squash into one commit.

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

BTW, before you update pr, you can run the check locally:

./tools/checpatch.sh -g HEAD~4...HEAD
./tools/refresh.sh --silent raspberrypi-4b/nsh

@linguini1

Copy link
Copy Markdown
Contributor Author

Still not quite sure about the failure here, the defconfig problem seems to have been resolved. Is it failing because of the generated config.txt?

@acassis

acassis commented Dec 16, 2024

Copy link
Copy Markdown
Contributor

@linguini1 the issue is happening in another board: pinephone/lcd

@lupyuen what do you think? Maybe some modification in the Kconfig reflected on pinephone?

@acassis

acassis commented Dec 16, 2024

Copy link
Copy Markdown
Contributor

@linguini1 I think that is the case, you modified a main Kconfig at arch, so it will reflect in other boards.

I suggest you to run a update for all arm64 boards:

$ ./tools/refresh.sh --silent arch:arm64

Then add these modified defconfigs in a separated commit, because they are not from your board, but affected by your modifications

Updated: Initially I said all boards, but in fact the Kconfig is inside arch/arm64, so you just need to refresh boards from arm64 arch!

@linguini1

Copy link
Copy Markdown
Contributor Author

@linguini1 the issue is happening in another board: pinephone/lcd

How did you determine that? I am still getting used to reading through the build logs, but I don't see anything unusual under the step for pinephone/lcd.

@acassis

acassis commented Dec 16, 2024

Copy link
Copy Markdown
Contributor

@linguini1 the issue is happening in another board: pinephone/lcd

How did you determine that? I am still getting used to reading through the build logs, but I don't see anything unusual under the step for pinephone/lcd.

See the message:

====================================================================================
Configuration/Tool: pinephone/lvgl
2024-12-16 17:23:15
------------------------------------------------------------------------------------
  Cleaning...
HEAD detached at pull/15188/merge
Ignored files:
  (use "git add -f <file>..." to include in what will be committed)
	config.txt

nothing to commit, working tree clean
  Configuring...
  Building NuttX...
  Normalize pinephone/lvgl
====================================================================================
Configuration/Tool: pinephone/nsh
2024-12-16 17:24:44
------------------------------------------------------------------------------------
  Cleaning...
HEAD detached at pull/15188/merge
Ignored files:
  (use "git add -f <file>..." to include in what will be committed)
	config.txt

nothing to commit, working tree clean
  Configuring...
  Building NuttX...
  Normalize pinephone/nsh
====================================================================================

Normally when you see that "Use git add -f " the defconfig is outdated.

Our CI is really bad to indicate about the issue. I suggested coloring these text to help. @lupyuen do you have some idea how to make these error more visible?

@linguini1

Copy link
Copy Markdown
Contributor Author

Normally when you see that "Use git add -f " the defconfig is outdated.

I thought that was something to do with config.txt, I see. I've now run the refresh tool against all the arm64 boards, but it appears nothing has changed. I suppose that means the defconfigs aren't the issue here?

I used the command ./tools/refresh.sh --silent --defaults arch:arm64

@acassis

acassis commented Dec 16, 2024

Copy link
Copy Markdown
Contributor

Normally when you see that "Use git add -f " the defconfig is outdated.

I thought that was something to do with config.txt, I see. I've now run the refresh tool against all the arm64 boards, but it appears nothing has changed. I suppose that means the defconfigs aren't the issue here?

I used the command ./tools/refresh.sh --silent --defaults arch:arm64

Strange, you can try another approach to confirm the defconfig has changed:

$ ./tools/configure.sh pinephone/nsh
$ make menuconfig
Don't modify anything, if it asks to have the modifications, something was different in the Kconfig now, then save it

$ make savedefconfig
Copy the generated defconfig to boards/arm64/.../pinephone/configs/nsh/

@linguini1

Copy link
Copy Markdown
Contributor Author

Strange, you can try another approach to confirm the defconfig has changed:

$ ./tools/configure.sh pinephone/nsh $ make menuconfig Don't modify anything, if it asks to have the modifications, something was different in the Kconfig now, then save it

$ make savedefconfig Copy the generated defconfig to boards/arm64/.../pinephone/configs/nsh/

Still no changes, menuconfig does not ask me to save anything. I checked for differences earlier using git status.

@lupyuen

lupyuen commented Dec 17, 2024

Copy link
Copy Markdown
Member

@linguini1 I built your branch in Docker, config.txt contains this:
https://gist.github.com/lupyuen/c5d8d1d92cd8fd09641dbd5ac62c0c8d#file-gistfile1-txt-L196-L202

$ cat nuttx/config.txt
kernel=nuttx.bin
arm_64bit=1
core_freq_min=500

Which is probably generated by this Makefile in your PR:

CONFIG_TXT = config.txt
define POSTBUILD
$(Q)echo "Generating $(CONFIG_TXT)";
$(Q)echo "kernel=nuttx.bin" > $(CONFIG_TXT);
$(Q)echo "arm_64bit=1" >> $(CONFIG_TXT);
$(Q)echo "core_freq_min=500" >> $(CONFIG_TXT);
$(if $(CONFIG_RPI4B_DEBUG_BOOT),$(Q)echo "uart_2ndstage=1" >> $(CONFIG_TXT);)
endef

Could you delete config.txt after building NuttX? I think the CI Build will complain no more after we fix this :-)

FYI Here's how I built your branch in Docker:
https://gist.github.com/lupyuen/c5d8d1d92cd8fd09641dbd5ac62c0c8d

sudo docker run \
  -it \
  --name nuttx \
  ghcr.io/apache/nuttx/apache-nuttx-ci-linux:latest \
  /bin/bash
cd
git clone https://github.com/linguini1/nuttx --branch bcm2711
git clone https://github.com/apache/nuttx-apps apps
pushd nuttx ; echo NuttX Source: https://github.com/apache/nuttx/tree/$(git rev-parse HEAD) ; popd
pushd apps  ; echo NuttX Apps: https://github.com/apache/nuttx-apps/tree/$(git rev-parse HEAD) ; popd
cd nuttx/tools/ci
./cibuild.sh -c -A -N -R testlist/arm64-01.dat 

@linguini1

Copy link
Copy Markdown
Contributor Author

Could you delete config.txt after building NuttX? I think the CI Build will complain no more after we fix this :-)

Unfortunately the config.txt is generated because it's needed for the Raspberry Pi 4B to boot, it needs to be copied onto the Pi's SD card.

If I add it to the files that are cleaned on make clean, will that make the builds pass? Or is it more NuttX idiomatic to have it be generated into the tools/bcm2711 directory? It's a required part of the build results, just like nuttx.bin.

@lupyuen

lupyuen commented Dec 17, 2024

Copy link
Copy Markdown
Member

@linguini1 Wonder if we could generate config.txt inside bootfiles.sh, since the content is static anyway?

Or we add tools/bcm2711/config.txt as a static file to this PR?

@linguini1

Copy link
Copy Markdown
Contributor Author

@linguini1 Wonder if we could generate config.txt inside bootfiles.sh, since the content is static anyway?

The content actually won't be static; it depends on the Kconfig options. For instance, right now the user can select the Kconfig option CONFIG_RPI4B_DEBUG_BOOT=y to have the Raspberry Pi's builtin bootloader show debug output on UART when booting. This will dynamically add another option to config.txt. There are also a lot of other options documented for config.txt but I have not implemented them all in NuttX yet. This is why I added generation of the file to the build.

I could generate it so that it stays in tools/bcm2711 with the other boot files? I just assumed that the root directory would be a good idea since the generated config.txt must be copied along with nuttx.bin, but I don't want to break the build.

@lupyuen

lupyuen commented Dec 17, 2024

Copy link
Copy Markdown
Member

I could generate it so that it stays in tools/bcm2711 with the other boot files?

@linguini1 Hmmm the defconfig check might still fail, if it detects new files in tools/bcm2711? We need to be careful when generating new files in POSTBUILD. (Here's an example of a complex POSTBUILD)

Maybe: We could generate the files inside /staging or /build, since .gitignore will skip them

Sorry I can't think of any good solution right now, and I think it's really good to merge this PR as-is, right now, with minimal tweaks. Wonder if we could hardcode tools/bcm2711/config.txt for now, and uncomment manually if required? (Until we fix it next release)

## Uncomment this for Debug Boot
## uart_2ndstage=1

@linguini1

Copy link
Copy Markdown
Contributor Author

Maybe: We could generate the files inside /staging or /build, since .gitignore will skip them

Will that work? I had config.txt in the root-level .gitignore file for this PR but it still seemed to be throwing off the build. If possible I'd like to come up with a solution that keeps the generation of config.txt since it will be necessary to add more features later.

It appears the other POSTBUILD example which generates files in the build directory also defines a .PHONY distclean rule inside the boards/arm64/imx9/imx93-evk/src/Makefile which deletes the generated files on make distclean. Maybe this will prevent the build from failing? From my view of the logs, it appears the build is only failing after building the rpi4b board because config.txt persists.

… BCM2711 and Raspberry Pi 4B.

Includes a Mini-UART NSH console for the 4B, tested on the 4GB RAM
model. Part of an I2C driver which can only read, boiler-plate for a SPI
driver, and a GPIO driver with limited pins. Some tools are present for
automatically fetching the boot files and creating the `config.txt` file
based on selected Kconfig options.
@lupyuen

lupyuen commented Dec 17, 2024

Copy link
Copy Markdown
Member

It appears the other POSTBUILD example which generates files in the build directory also defines a .PHONY distclean rule inside the boards/arm64/imx9/imx93-evk/src/Makefile which deletes the generated files on make distclean

Yep let's go ahead and try this. Otherwise we fallback to the plain simple tools/bcm2711/config.txt. So we can merge this PR ASAP thanks :-)

@xiaoxiang781216
xiaoxiang781216 merged commit e96fb6b into apache:master Dec 17, 2024
@linguini1
linguini1 deleted the bcm2711 branch December 17, 2024 16:20
@linguini1

Copy link
Copy Markdown
Contributor Author

Thank you everyone for guiding me through the build errors!

@acassis

acassis commented Dec 17, 2024

Copy link
Copy Markdown
Contributor

@linguini1 since you have your "logbook" with the steps you took to do the port, maybe you could submit a simple Guide to be included at https://nuttx.apache.org/docs/latest/guides/index.html

The official Porting Guide probably is outdated, so having this other guide while things are kind of fresh in your head could be useful. Maybe later it could be adapted and included in the official Porting Guide. What do you think?

@linguini1

Copy link
Copy Markdown
Contributor Author

@linguini1 since you have your "logbook" with the steps you took to do the port, maybe you could submit a simple Guide to be included at https://nuttx.apache.org/docs/latest/guides/index.html

The official Porting Guide probably is outdated, so having this other guide while things are kind of fresh in your head could be useful. Maybe later it could be adapted and included in the official Porting Guide. What do you think?

Yeah I think that's a great idea! I can add it as a case study under the porting guide like Nathan suggested? That way people porting things will stumble upon it?

In the next few days I'll clean up my logs in a way that they can be posted and open a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: arm64 Issues related to ARM64 (64-bit) architecture Area: Documentation Improvements or additions to documentation Area: Tooling Board: arm64 Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Raspberry pi 4 model B support

6 participants