boards/arm/rp2040: Fix code style and error checking by acassis · Pull Request #16869 · apache/nuttx

acassis · 2025-08-19T23:12:06Z

Summary

This patch tries to follow the coding style of NuttX and avoids nesting functions, handling the error return.

Impact

Improvement

Testing

build only

This patch tries to follow the coding style of NuttX and avoids nesting functions, handling the error return. Signed-off-by: Alan C. Assis <acassis@gmail.com>

acassis · 2025-08-19T23:23:31Z

@nmaggioni please test if you temperature sensor still working. I just tested compiling it!

cederom

Thank you @acassis :-)

I had to switch pico-sdk from 2.2.0 to 2.1.0 as the latest one seems to have changed headers and does not build :D
No build errors, but I have no sensor to test on real hardware sorry.

% uname -a
FreeBSD hexagon 14.2-RELEASE-p1 FreeBSD 14.2-RELEASE-p1 GENERIC amd64

% pwd
/XXX/embedded/rpi/pico-sdk.git
% git branch
* (HEAD detached at 2.1.0)

% ./tools/configure.sh -B raspberrypi-pico:tmp112
(..)
% /usr/bin/time -h gmake -j24
Create version.h
LN: platform/board to /XXX/nuttx-apps.git/platform/dummy
Register: dd
Register: nsh
Register: sh
Register: hello
Register: getprime
Register: ostest
/usr/local/gcc-arm-embedded-14.2.rel1/bin/../lib/gcc/arm-none-eabi/14.2.1/../../../../arm-none-eabi/bin/ld: warning: rp2040_boot_stage2.elf has a LOAD segment with RWX permissions
CPP:  /XXX/nuttx.git/boards/arm/rp2040/raspberrypi-pico/scripts/raspberrypi-pico-flash.ld-> /XXXLD: nuttx
Memory region         Used Size  Region Size  %age Used
           flash:        160 KB         2 MB      7.81%
            sram:        8884 B       264 KB      3.29%
Generating: nuttx.uf2
Done.
        6,81s real              30,48s user             14,54s sys

% picotool load nuttx.uf2
Loading into Flash:   [==============================]  100%
% picotool reboot
The device was rebooted into application mode.
% cu -l /dev/cuaU0 -s 115200
Connected
NuttShell (NSH) NuttX-10.4.0
nsh>
nsh> uname -a
NuttX 10.4.0 2e07dc5956 Aug 20 2025 01:50:52 arm raspberrypi-pico
nsh> ls /dev
/dev:
 console
 null
 temp0
 ttyACM0
 zero

cederom · 2025-08-20T00:17:16Z

CI build error :-(

linguini1 · 2025-08-20T00:32:43Z

boards/arm/rp2040/common/src/rp2040_common_bringup.c

+  else
    {
-      syslog(LOG_ERR, "Failed to register MCP3008 device driver: %d\n", ret);
+      struct adc_dev_s *mcp3008 = mcp3008_initialize(spi);


I'd honestly fully remove the MCP3008 stuff

this actually may be a good idea to remove external ic specific code from common rp2040 initialization code.. users may want to add it to their application :-)

nmaggioni · 2025-08-20T11:00:08Z

@acassis Thanks for having taken care of this, I've retested my hw and the sensor still works fine.

@cederom I'm on it, SDK 2.2.0 is misbehaving on clean pulls for me too. Sorry if I didn't catch that earlier, I may have been tricked by some cache somewhere.

simbit18 · 2025-08-20T11:08:46Z

There are errors in job Linux (arm-06)

https://github.com/apache/nuttx/actions/runs/17084057983/job/48447096636#logs`

====================================================================================
Configuration/Tool: raspberrypi-pico-w/nsh-flash,CONFIG_ARM_TOOLCHAIN_GNU_EABI
2025-08-20 00:22:32
------------------------------------------------------------------------------------
  Cleaning...
  Configuring...
  Disabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
  Enabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
  Building NuttX...
In file included from /tools/pico-sdk/src/common/pico_base_headers/include/pico.h:64,
                 from /tools/pico-sdk/src/rp2040/pico_platform/include/pico/asm_helper.S:7,
                 from /tools/pico-sdk/src/rp2040/boot_stage2/boot2_w25q080.S:29:
/tools/pico-sdk/src/rp2040/pico_platform/include/pico/platform.h:26:10: fatal error: pico/platform/common.h: No such file or directory
   26 | #include "pico/platform/common.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [chip/boot2/Make.defs:56: rp2040_boot_stage2.elf] Error 1
make[2]: Target 'makedepfile' not remade because of errors.
make[1]: *** [Makefile:261: .depend] Error 2
make[1]: Target 'depend' not remade because of errors.
make: *** [tools/Unix.mk:660: pass2dep] Error 2
make: Target 'all' not remade because of errors.
/github/workspace/sources/nuttx/tools/testbuild.sh: line 385: /github/workspace/sources/nuttx/../nuttx/nuttx.manifest: No such file or directory
  [1/1] Normalize raspberrypi-pico-w/nsh-flash
====================================================================================

cederom · 2025-08-20T11:44:31Z

This CI build comes from pico-sdk version 2.2.0. I had exactly the same problem in local build, so had to revert pico-sdk to version 2.1.0.
tools/ci: Include fetching pico-sdk in the CI environment setup #16862 adds local fetch of pico-sdk 2.2.0 to CI to help with builds, but 2.2.0 breaks self-compatibility in a minor release :D :D :D
thus my push for avoiding and clearly marking breaking changes :-)
external SDKs are soooo sweet :-) this is why we avoid it here in NuttX :-)
although this sdk issue is not related with this specific PR, rather a result, we should fix that probably before merging this PR, and use this PR as testbench to verify build? :-)

nmaggioni · 2025-08-20T11:58:16Z

@cederom Care to try #16876? The fix was simple enough but, as you said, the upstream did not mark it as a breaking change (...?).

cederom · 2025-08-20T12:03:50Z

@cederom Care to try #16876? The fix was simple enough but, as you said, the upstream did not mark it as a breaking change (...?).

Im on it, thanks! :-)

Looking at https://github.com/raspberrypi/pico-sdk/releases/tag/2.2.0 there is only one mention of breaking change for pico_encrypt_binary, no mention on headers relocation / change :-P

nmaggioni · 2025-08-20T12:11:56Z

no mention on headers relocation / change :-P

In their defense, I suppose not using the SDK in its entirety is outside their scope - even if they should care about that IMO.

Anyway, my bad for having trusted their changelog and not having retested everything on a clean slate :)
Fortunately there's CI for that, though.

hartmannathan · 2025-08-20T16:07:55Z

boards/arm/rp2040/common/src/rp2040_common_bringup.c


-  ret = mcp9600_register(rp2040_i2cbus_initialize(0), 0x60, 1, 2, 3);
-  if (ret < 0)
+  i2c = rp2040_i2cbus_initialize(0);


What happens if CONFIG_SENSORS_SHT4X and i2c is already initialized?

hartmannathan · 2025-08-20T16:08:28Z

boards/arm/rp2040/common/src/rp2040_common_bringup.c

-  ret = ms56xx_register(rp2040_i2cbus_initialize(0), 0, MS56XX_ADDR0,
-                        MS56XX_MODEL_MS5611);
-  if (ret < 0)
+  i2c = rp2040_i2cbus_initialize(0);


Ditto, what happens if CONFIG_SENSORS_SHT4X and/or CONFIG_SENSORS_MCP9600 and i2c is already initialized?

hartmannathan · 2025-08-20T16:09:24Z

boards/arm/rp2040/common/src/rp2040_common_bringup.c

-  ret = board_tmp112_initialize(rp2040_i2cbus_initialize(0), 0,
-                                TMP112_ADDR_1);
-  if (ret < 0)
+  i2c = rp2040_i2cbus_initialize(0);


Ditto, CONFIG_SENSORS_SHT4X and/or CONFIG_SENSORS_MCP9600 and/or CONFIG_SENSORS_MS56XX already initialized i2c?

For all of these, it doesn't matter. The rp2040_i2cbus_initialize() function (and all xxxx_i2cbus_initialize() functions first check if the bus has already been initialized. If it has, it just returns the initialized i2cdev struct without re-performing the init.

cederom · 2025-08-20T16:12:58Z

As #16876 is merged lets see how this CI builds now :-)

hartmannathan

I recommend a different git log message. (The current one makes it seem like the change is nxstyle fixes, but the actual change is error handling in board bringup.)

Something like:

boards/arm/rp2040: Improve error handling in board bringup

rp2040_common_bringup: In case of failure to initialize SPI and/or I2C
buses, do not try to initialize devices that depend on these buses:
MCP3008 (SPI), RP2040, SHT4X, MCP9600, MS56XX (I2C).

(Edit: Wrap the proposed log message at 72 columns.)

xiaoxiang781216 · 2025-08-25T12:42:02Z

please fix the conflict @acassis

linguini1 · 2025-10-31T13:54:34Z

@acassis any updates on this PR? It can get merged once the conflict is fixed and I think it would be good.

hartmannathan · 2025-11-02T15:41:58Z

@acassis any updates on this PR? It can get merged once the conflict is fixed and I think it would be good.

Hi,

Please see my comment above regarding the commit log. I think a better log message is (copying from my earlier comment):

boards/arm/rp2040: Improve error handling in board bringup

rp2040_common_bringup: In case of failure to initialize SPI and/or I2C
buses, do not try to initialize devices that depend on these buses:
MCP3008 (SPI), RP2040, SHT4X, MCP9600, MS56XX (I2C).

Thanks!

github-actions bot added Board: arm Size: S The size of the change in this PR is small labels Aug 19, 2025

boards/arm/rp2040: Fix code style and error checking

2e07dc5

This patch tries to follow the coding style of NuttX and avoids nesting functions, handling the error return. Signed-off-by: Alan C. Assis <acassis@gmail.com>

acassis force-pushed the fix_analog branch from 417560b to 2e07dc5 Compare August 19, 2025 23:17

acassis requested review from cederom, hartmannathan, jerpelea, linguini1 and xiaoxiang781216 August 19, 2025 23:17

cederom approved these changes Aug 19, 2025

View reviewed changes

linguini1 approved these changes Aug 20, 2025

View reviewed changes

jerpelea approved these changes Aug 20, 2025

View reviewed changes

xiaoxiang781216 approved these changes Aug 20, 2025

View reviewed changes

linguini1 approved these changes Aug 20, 2025

View reviewed changes

simbit18 approved these changes Aug 20, 2025

View reviewed changes

hartmannathan reviewed Aug 20, 2025

View reviewed changes

jerpelea approved these changes Aug 26, 2025

View reviewed changes

nmaggioni mentioned this pull request Sep 2, 2025

drivers/power/supply/husb238: Add support for HUSB238 USB-PD Sink Controller #16963

Draft

Conversation

acassis commented Aug 19, 2025

Summary

Impact

Testing

Uh oh!

acassis commented Aug 19, 2025

Uh oh!

cederom left a comment

Choose a reason for hiding this comment

Uh oh!

cederom commented Aug 20, 2025

Uh oh!

linguini1 Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

cederom Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

nmaggioni commented Aug 20, 2025

Uh oh!

simbit18 commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cederom commented Aug 20, 2025

Uh oh!

nmaggioni commented Aug 20, 2025

Uh oh!

cederom commented Aug 20, 2025

Uh oh!

nmaggioni commented Aug 20, 2025

Uh oh!

hartmannathan Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

hartmannathan Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

hartmannathan Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

linguini1 Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

cederom commented Aug 20, 2025

Uh oh!

hartmannathan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiaoxiang781216 commented Aug 25, 2025

Uh oh!

linguini1 commented Oct 31, 2025

Uh oh!

hartmannathan commented Nov 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

simbit18 commented Aug 20, 2025 •

edited

Loading

hartmannathan left a comment •

edited

Loading