Compatibility: digitalPinToPort and digitalPinToBitMask

Maybe a better name would be how can libraries do faster IO. I see this a lot in a lot of libraries, especially a lot of ones who manipulate the SPI CS pin, as well as others, like DC pin on many displays, and in cases of non-standard pins, the MISO, MOSI, and SCK pins.

Also, not sure if best to continue on the other compatibility thread I started or to split off each type of issue.

There are a lot of libraries and user code out there that do things like:

  xp_port =  portOutputRegister(digitalPinToPort(_xp));
  yp_port =  portOutputRegister(digitalPinToPort(_yp));
  xm_port =  portOutputRegister(digitalPinToPort(_xm));
  ym_port =  portOutputRegister(digitalPinToPort(_ym));
  
  xp_pin = digitalPinToBitMask(_xp);
  yp_pin = digitalPinToBitMask(_yp);
  xm_pin = digitalPinToBitMask(_xm);
  ym_pin = digitalPinToBitMask(_ym);

...
  *xp_port |= xp_pin;
  *xm_port &= ~xm_pin;

Which will not work with this board. In this case of this library, iit may not fail as they have the code under #if defined (USE_FAST_PINIO)

But many don't and the error message is rather cryptic, as you do have a digitalPinToPort, but it does not return something that is expected.
Which is these lines like: xp_port = portOutputRegister(digitalPinToPort(_xp));
The calling code is expecting that xp_port will something like: volatile uint8_t *
or in some case uint32_t *...

Now luckily several of these libraries will hopefully function as they don't know anything about these boards and will fall back to digitalWrite calls.

Even if you were able to have these functions/macros work, it may not necessarily be a good thing. As if it actually retuns a pointer to the Ports output register, then the changes could work, but the operations are not atomic. And something not good could happen like, you read in the current state, update, and write out, and in between your read and your write an ISR happens that also changes the state of another pin on the same IO port, when your write completes it just tromped on the update that happened in the ISR. Even if you are in an ISR a higher priority ISR could trample on it.

Options:
Punt: obviously the easiest and hopefully more of the developers of libraries will handle it in a default case. Assuming of course that library owner actually is still active...

Make the macros compatible -
I mentioned the main issue with this above.

This approach works for Teensy 3.x, as they are either Arm Cortex M3 or M4 boards that implement bit-banding. This is where there is another region of memory where each bit of a certain region of memory is mapped to whole words in another region. So you can for example update bit 11 or Port 1 and this would be done as an atomic operation. (Port names are actually different but...)

However - I found out on a different thread. That supporting bit-banding is optional. And it appears like these boards do not support it. Which I tried to confirm yesterday, with an example sketch:

#define GPIO_BITBAND_ADDR(reg, bit) (((uint32_t)&(reg) - 0x40000000) * 32 + (bit) * 4 + 0x42000000)
volatile uint32_t *p_led_bit = (volatile uint32_t *)GPIO_BITBAND_ADDR(R_PORT1->PODR, 11);

void setup() {
  // put your setup code here, to run once:
  pinMode(13, OUTPUT);

  while (!Serial && millis() < 5000) ;
  Serial.begin(115200);
  Serial.println((uint32_t)&R_PORT1->PODR, HEX);
  Serial.println((uint32_t)p_led_bit, HEX);
}


uint8_t counter = 0;
void loop() {
  // put your main code here, to run repeatedly:
  counter++;
#if 0
  uint8_t portb_val = PINB;
  if (counter & 1) portb_val |= (1 << 5);
  else portb_val &= ~(1 << 5);
  PORTB = portb_val;
#elif 0
  uint16_t  d1 = R_PORT1->PODR;
  if (counter & 1) d1 |= (1 << 11);
  else d1 &= ~(1 << 11);
  R_PORT1->PODR = d1;
#else
  if (counter & 1) *p_led_bit = 1;
  else  *p_led_bit = 0;

#endif  
  delay(250);
}

Which did not work. I tried a few other variations of it. This was on the MINIMA where the
LED is on Port 1 pin 11, for WIFI I believe pin 2

Setup to easily use SET and CLEAR registers
There are other boards with this similar issue about how best to do atomic operations.
This includes Teensy 4.x boards, and guessing SAMD boards.

So for example, the Adafruit SPITFT code in GFX library, is setup that if it is one of these types of boards, it sets a define HAS_PORT_SET_CLR

In the case of the Teensy there are calls then for:

  dcPinMask = digitalPinToBitMask(dc);
  dcPortSet = portSetRegister(dc);
  dcPortClr = portClearRegister(dc);

Personally, I like this last approach.

Does this make sense? Thoughts?

If something like that was implemented again something to go into the cheat sheet.

3 Likes

Please do not open a multiple thread about tge same thing.

Moderator - Please feel free to close and/or delete this topic.

There appears to be very little interest in such things. I will do my best to resist reporting other compatibility issues such as this, or suggestions on possible ways to address them.

Thanks

I am devastated that there is no interest to make those macros compatible. Especially the Librarys that are dependant on such macros for some Uno-Fitting-Shields which will have no compatibility and probably will never see updates to support the R4. My only choice will be to try writing my own display library which will probably never work :smiley: I had high hopes when i bought the R4 but it was a big letdown for me

At least nobody could suggest a solution right now :frowning:

I can imagine some optimizations on digitalRead/Write() with constant pin numbers at compile time, but not for lower levels.

Check out this code.

You would do it yourself and than make a PR to Uno R4 core. I think the maintainers will be happy to accept you improvements.

I believe that there is probably Arduino community interest in solving issues such as this. It just may be this is the wrong venue for doing it.

What I was hoping for is to come up with a recommended approach, with potentially some new helper functions, that for example one could create an issue or better yet a PR to different libraries. For example, to Adafruit GFX library (SPITFT), that says for the new board, do the following... I know that is probably being naive, although have successfully done so in the past with other boards.

As I mentioned in the first post, the approach used for the UNO R1-3 may not be appropriate for R4, as the operation to do the writes is not atomic. Although that was true of AVR boards as well.

I did propose a strawman possible approach, with the SET and CLEAR registers.

Currently in Arduino.h we have:

#define digitalPinToPort(P)		      (digitalPinToBspPin(P) >> 8)
#define digitalPinToBitMask(P)      (1 << (digitalPinToBspPin(P) & 0xFF))
#define portOutputRegister(port)    &(((R_PORT0_Type *)IOPORT_PRV_PORT_ADDRESS(port))->PODR)
#define portInputRegister(port)     &(((R_PORT0_Type *)IOPORT_PRV_PORT_ADDRESS(port))->PIDR)
#define portModeRegister(port)      &(((R_PORT0_Type *)IOPORT_PRV_PORT_ADDRESS(port))->PDR)

Suggest adding:

#define portSetRegister(port)     &(((R_PORT0_Type *)IOPORT_PRV_PORT_ADDRESS(port))->POSR)
#define portClearRegister(port)      &(((R_PORT0_Type *)IOPORT_PRV_PORT_ADDRESS(port))->PORR)

Then for suggestions for SPITFT or the like, would suggest using an approach similar to what is done for Teensy T4.x or others that use a SET/CLEAR operation.

Note: Their library is a lot more complex than most, so would probably actually start off with a simpler one, to see what pieces we may not have covered.

You are right, someone should try one or more approaches and see what works. And see what pieces of the solution, should be migrated into the core releases, what parts should go into documentation. LIkeise they should also potentially communicate the possible solutions with some of the major players, like Adafruit, Sparkfun, etc, and get their feedback.

As for getting things PRs merged in, that is always hit or miss. Especially so, if you have not signed their agreement.

If you are not willing to sign the contributor license agreement then it should always be a miss because the Arduino company has a strict policy that this is required for all code contributions to official Arduino software projects.

@KurtE that topic is really important in that opinion. We really need to find a good way to extend the Arduino standard API with functions for fast IO operations, that can be interoperable across different boards. So, if some great suggestion or contribution in that direction comes from the community we're all ears! :slight_smile:
We also have a place for discussions and proposals about the standard API which is the GitHub - arduino/language: This repository serves a central entry point to discussions concerning the Arduino Language. repository.