Inconsistent of PICO_RP2350B

[sfe-SparkFro]

The Pico SDK has mandated a standard of

#define PICO_RP2350A 1

to denote boards with the RP2350A, and

#define PICO_RP2350A 0

to denote boards with the RP2350B. In the early days of the RP2350B, some boards "incorrectly" used

#define PICO_RP2350B 1

to denote boards with the RP2350B. This was highlighted in raspberrypi/pico-sdk#2286 and fixed in raspberrypi/pico-sdk#2290, although there is ongoing discussion about this in raspberrypi/pico-sdk#2364.

While I don't personally agree with that decision, it seems to be the direct Raspberry Pi is going (assuming the ongoing discussion doesn't change anything). So in #2847, I set #define PICO_RP2350A 0 for a board with an RP2350B to do it "correctly". However I did not catch that this Arduino port actually makes use of

#ifdef PICO_RP2350B

in these locations, so some features don't work on the higher GPIO pins like they should.

So, the question is whether this Arduino port should match Raspberry Pi's convention of #define PICO_RP2350A 0 or continue with #define PICO_RP2350B 1 to denote boards with the RP2350B.

I personally think it should match whatever the Pico SDK does for consistency, but just want to get the conversation started so the appropriate updates can be made so all boards with the RP2350B work correctly.

[earlephilhower]

I think when I looked at it the first time (I'm just an end user, so I got to see the SDK only after it was released) I couldn't be sure exactly how to differentiate the different chip types. It seemed suboptimal to me as well to define what the chip explicitly is not (#define PICO_RP2350A 0) vs. positively identifying it as what it was (hence #define PICO_RP2350B 1).

I'm fine matching the SDK since this is basically a sed operation on the repo.

To separate between the different chips today I think we need to implement the following logic here:

#if defined(PICO_RP2040)
// <this is a RP2040 board, no matter if PICO_RP2040==1 or ==0, mere definition of value means RP2040>
#elif defined(PICO_RP2350A) && (!PICO_RP2350A)
// <this is an RP2350A>
#elif defined(PICO_RP2350A) && (PICO_RP2350A)
// <this is a RP2350B>
#else
#error Unable to identify chip
#endif

And for things where we just need the chip model and don't care A or B

#if defined(PICO_RP2040)
// This is an RP2040
#elif defined(PICO_RP2350A)
// This is an RP2350A or B
#else
#error Undefined chip
#endif

And for sanity an assert like the following would be belt-and-braces

#if defined(PICO_RP2040) && defined(PICO_RP2350A)
#error Chip multiply specified
#endif

See anything missing/wrong? I'm about to leave to walk the Camino de Santiago for a couple weeks so won't be able to actually implement the change until I get back, but I'm fine doing it...

(FWIW, for the libpico SDK library build we force a RP2350B board= setting because otherwise the SDK actually drops the code for the upper GPIOs.)

[sfe-SparkFro]

It seemed suboptimal to me as well to define what the chip explicitly is not (#define PICO_RP2350A 0) vs. positively identifying it as what it was (hence #define PICO_RP2350B 1).

I totally agree, in addition to the arguments brought up in raspberrypi/pico-sdk#2364. Maybe Raspberry Pi will revert that decision? No clue, will have to wait and see on that.

To separate between the different chips today I think we need to implement the following logic

That sounds to me like a reasonable compromise solution for now!

[earlephilhower]

Looks like the SDK bug has been moved up to the next minor release, 2.1.2. I'll wait for that to land and then normalize things here.

[earlephilhower]

Looks like they decided to stick with PICO_RP2350A == 0 means "RP2350B" and OTW it's a "RP2350" per the patch they linked to: raspberrypi/pico-sdk#2233. So I'll try and organize accordingly and remove the PICO_RP2350B define and usage and go to the ifdef+value check method where it's needed.