Skip to content
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

Portenta H7: Add spi and wire objects to overlay - Plus smaller GPIO pin list and PinNames #82

Closed
wants to merge 12 commits into from

Conversation

KurtE
Copy link

@KurtE KurtE commented Mar 8, 2025

This maybe resolves #68
And includes at least a partial implementation of the stuff I mentioned in the last few postings on that issue.

This is an alternative PR to #71

This is a combination PR from stuff that @mjs513 and I have done to start to flesh out the H7 variant.

There is now code/overlay/PWM and ...

This branch also added some WIP support for some different cameras. I believe it is currently configured in the
overlay and config file for the GC2145

Where this PR defers from the #71 is that the GPIO pin list has been reduced to the first 26 items plus any others
that are exported on the two High Density connectors that connect the board to the shields. In particular only those
that the exposed to the user on one or more plug in pins. Like all of them on the breakout board. Plus I then also
removed any of these which maps to the same GPIO port/pin as the ones in the first 26.

In addition, I then added a PinNames.h file to the Portenta H7_M7 variant, which has most of the entries that the same logical file had on the MBED version, but I stripped out all of those which had a | something like | ALT0 or | |DUAL_PAD.

So far I have added only a small subset of overloaded APIs that use these, like: pinMode, digitalWrite, plus the mapping functions
that map PinNameToindex and digitalPinToPinName. Soon will add a few more, like digitalRead.

As I mentioned in the issue, currently I am using a simple table I added to the overlay, that I can use to find the addresses
of the different objects like GPIOA, GPIOB...

		port-pin-gpios =	<&gpioa 0 0>,
					<&gpiob 0 0>,
					<&gpioc 0 0>,
					<&gpiod 0 0>,
					<&gpioe 0 0>,
					<&gpiof 0 0>,
					<&gpiog 0 0>,
					<&gpioh 0 0>,
					<&gpioi 0 0>,
					<&gpioj 0 0>,
					<&gpiok 0 0>;

Currently I have the overloaded function definitions at the end of the PinNames.h file in the variant.
Not sure what is the right way for Overloads specific to a subset of variants?

Likewise, I have all of that code currently in the variant in the PinNames.cpp. Which currently has it duplicating the GPIO table that is in zephyrCommon.cpp as that table is defined as static.

I started off with a simple test sketch which included tabs with the PinNames.h and .cpp files before I added them to this
PR branch.

#include <Arduino.h>
#include "PinNames.h"
void setup() {
  Serial.begin(115200);
  while (!Serial && millis() < 5000) {}
  if (Serial) {
    Serial.print("Red Pin: ");
    Serial.println(PinNameToIndex(LED_RED));
    Serial.print("Green Pin: ");
    Serial.println(PinNameToIndex(LED_GREEN));
    Serial.print("Blue Pin: ");
    Serial.println(PinNameToIndex(LED_BLUE));

    for (uint8_t i = 0; i < 26; i++) {
      PinName pn = digitalPinToPinName(i);
      if (pn != NC) {
        Serial.print("\t");
        Serial.print(i);
        Serial.print(" = P");
        Serial.write('A' + ((uint8_t)pn >> 4));
        Serial.write('_');
        Serial.println((uint8_t)pn & 0xf);
      }
    }
  }
  pinMode(LED_RED, OUTPUT);
  pinMode(LED_BLUE, OUTPUT);
  pinMode(LED_GREEN, OUTPUT);
}

uint8_t loop_count = 0;
void loop() {
  loop_count = (loop_count + 1) & 0x7;
  digitalWrite(LED_RED, (loop_count & 1) ? HIGH : LOW);
  digitalWrite(LED_BLUE, (loop_count & 2) ? HIGH : LOW);
  digitalWrite(LED_GREEN, (loop_count & 4) ? HIGH : LOW);
  delay(250);
}

The include of PinNames.h in this sketch is not necessary using this fork as I added the include in variants.h and the local tab has a check for the same #define so in this case it does nothing. I did add an extra define in the sketch one, such that the
sketches .cpp file will see this other define and not include any additional code.

As another test, I updated my version of my ILI9341 driver for Zephyr and if it sees that the Pin names file was included, it
add alternate constructors, which allows me to pass in PinNames for the CS, DC, and reset pins. If you pass in pin numbers it will convert those to PinNames. And I tried it out on my test sketch that tries to display a camera image onto the display.

#include <ILI9341_GIGA_zephyr.h>
#include "camera.h"

Camera cam;

#ifdef ARDUINO_PORTENTA_H7
#ifdef ZEPHYR_PINNAMES_H
#define TFT_DC PC_6
#define TFT_RST PC_7
#define TFT_CS PG_7
#else
#define TFT_DC 5
#define TFT_RST 4
#define TFT_CS 3
#endif
ILI9341_GIGA_n tft(&SPI, TFT_CS, TFT_DC, TFT_RST);
#else
#define TFT_DC 9
#define TFT_RST 8
#define TFT_CS 10
ILI9341_GIGA_n tft(&SPI1, TFT_CS, TFT_DC, TFT_RST);
#endif

void fatal_error(const char *msg) {
  Serial.println(msg);
  pinMode(LED_BUILTIN, OUTPUT);
  while (1) {
    digitalWrite(LED_BUILTIN, HIGH);
    delay(100);
    digitalWrite(LED_BUILTIN, LOW);
    delay(100);
  }
}



void setup() {
  pinMode(LED_BUILTIN, OUTPUT);
  Serial.begin(115200);
  while (!Serial && millis() < 5000) {}

  // put your setup code here, to run once:
  Serial.println("\n*** start display camera image on ILI9341 ***");
  tft.begin();
  tft.setRotation(1);
  tft.fillScreen(ILI9341_BLACK);
  delay(500);
  tft.fillScreen(ILI9341_RED);
  delay(500);
  tft.fillScreen(ILI9341_GREEN);
  delay(500);
  tft.fillScreen(ILI9341_BLUE);
  delay(500);
  tft.fillScreen(ILI9341_BLACK);
  delay(500);

  if (!cam.begin(320, 240, CAMERA_RGB565)) {
    fatal_error("Camera begin failed");
  }
  cam.setVerticalFlip(false);
  cam.setHorizontalMirror(false);
}

void loop() {
  // put your main code here, to run repeatedly:
  FrameBuffer fb;
  if (cam.grabFrame(fb)) {
    digitalWrite(LED_BUILTIN, !digitalRead(LED_BUILTIN));
    //tft.writeRect(0, 0, 320, 240, (const uint16_t*)fb.getBuffer());
    //tft.writeSubImageRectBytesReversed(0, 0, 320, 240, 0, 0, 320, 240, (const uint16_t*)fb.getBuffer());
    tft.writeSubImageRect(0, 0, 320, 240, 0, 0, 320, 240, (const uint16_t*)fb.getBuffer());
    cam.releaseFrame(fb);
  }
}

The test sketches as well as the updated ILI9341 library are up in my github project:
https://github.com/KurtE/Arduino_GIGA-stuff

I probably will not add much more to this branch, until we have some feedback on if this is the going in an
appropriate direction.

Kurt

KurtE added 8 commits March 6, 2025 08:26
This is a partial fix for: arduino#68

the spis line was not in the 	zephyr,user {
section of the ovleray file.
I added it with one entry: spis = <&spi2>;

Updated: the GPIO table, that only the LEDS should be defined with GPIO_ACTIVE_LOW.
Added in the other two Wire objects to match the MBED version.
i2cs = <&i2c3>, <&i2c1>, <&i2c4>;

I checked it out with simple wire scanner sketch that checks Wire, Wire1 and Wire2.
And then tried adding QWIIC Button first to the I2C0 section of breakout board and
it was found on Wire.

Moved it to I2C1 section and found with Wire1 and then moved to I2C2 section and found on Wire2
resolves: arduino#74

needed to add: CONFIG_MEMC=y
to have the SDRAM enabled and as such not fault if you do anything with the SDRAM library.
Also enabled CONFIG_DMA=y
As not sure if that was needed but is in the GIGA .conf file

Update arduino_portenta_h7_m7.overlay

Add dma sections to remove the build warnings
@mjs513 and I added/modified the device tree for this board, with the tables to support PWM (analogWrite) and analogRead on some of the Analog pins.

Note: there are some issues with A4/A5 in that those pins are also on the same pins that support SPI.
And when SPI is enabled, the GPIOC pin configuration is setup by the device tree to be MODER to be set for Alternat function and their AF set to 5.  The analogRead code does not does not update these tables and as
such analogRead fails.

We found that if y ou disable SPI then they work better.

I currently left the SPI enabled.  Probably need to add something that when analogRead is called that it updates the GPIO registers to make it work.

I am ecurrently experimenting with a hack in my test sketch that appears to change the MODER of those pins back to 3, but so far that does not appear to be sufficent:
```
 static const struct gpio_dt_spec arduino_pins[] = {DT_FOREACH_PROP_ELEM_SEP(
   DT_PATH(zephyr_user), digital_pin_gpios, GPIO_DT_SPEC_GET_BY_IDX, (, ))};

  void pinModeAnalog(pin_size_t pinNumber) {
     gpio_pin_configure_dt(&arduino_pins[pinNumber],
                           GPIO_INPUT /*| GPIO_ACTIVE_HIGH */ | GPIO_MODE_ANALOG);
 }
```

On the analogWrites, this code has optionally in place that allows duplicate pins in the device tree GPIO list,  by changing how I read in the PWM table from the Tree, instead of summing all of the matches in the GPIO tree, it instead saves the GPIO Port and pin number and uses them for the compare.

Again this depends on if we wish for the Pin numbers for the functions like digitalRead/digitalWrite to match those used by the MBED version.   I still have the last 3 truncated K5-K7 which are used by the LEDs and fault
but depending on directions, can find other solutions.

enable spi2
Note: the overload function definitions are in the PinNames.h
PinNames.cpp - duplicates the pin table, as the main pin table is defined static within zephyrCommon.cpp
Not sure if to move some up to there but sort of specific to Portenta H7 maybe X8, maybe GIGA...

Also I sort of still have a lot of the MBED names within the pin names, Probably should be weeded out

Maybe some place should define breakout board specific namings?
Added a few more override functions.

Brings up questions, with the analog ones.
More on the PR
@KurtE
Copy link
Author

KurtE commented Mar 8, 2025

Quick Update:
Added: digitalRead(PinName) - which appears to be working.
I tested it on D2 - PJ_11

Added some real default stuff for analogRead and analogWrite.
The analogWrite just maps to pin number and calls the pin number version.

so far ditto for analogRead - but I believe this shows, that we need to add items to the
PinName table like: PC_3C - to handle the Analog pins that are using the Pure Analog...

@KurtE
Copy link
Author

KurtE commented Mar 9, 2025

Quick update: I added in the Pin names like: PA_0C for analog on the complementary pins.

    // Lets add the Analog only pins, will name them like the schematic, but number like  
    PA_0C      = 0X00 | DUAL_PAD,
    PA_1C      = 0X01 | DUAL_PAD,
    PC_2C      = 0X02 | DUAL_PAD,
    PC_3C      = 0X03 | DUAL_PAD,

and the override readAnalog is sort of a hack. Could be better if some of the other tables and the like are exposed:
Ooops I see I need to remove some debug stuff:

int analogRead(PinName pinNumber) {
  // Not sure what to do here, does anyone do something like:
  // analogRead(PA_0c); or analogRead(PC2_ALT0)?
  // first pass only support ones on pins.
  if (pinNumber & DUAL_PAD) {
    switch (pinNumber & 0xf) {
    case 0: Serial.print("<A0>"); return analogRead(A0);
    case 1: Serial.print("<A1>"); return analogRead(A1);
    case 2: Serial.print("<A2>"); return analogRead(A2);
    case 3: Serial.print("<A3>"); return analogRead(A3);
    default: return -1;
    }

  }
  int pin_index = PinNameToIndex(pinNumber);
  if (pin_index != -1) {
    Serial.write('<'); Serial.print(pin_index); Serial.write('>');
    return analogRead(pin_index);
  }
  return -1;
}


void analogWrite(PinName pinNumber, int value) {
  // first pass only support ones on pins.
  if (pinNumber & DUAL_PAD) return;
  int pin_index = PinNameToIndex(pinNumber);
  if (pin_index != -1) {
    analogWrite(pin_index, value);
  }
}

However I ran into a problem with some of the other pins, and debugging, this found same issue in doing a normal Analog
read like: analogRead(A4);
Which does not work.

Why? The PC_2 pin is duplicated on the Portenta H7.
That is it is on pin D10 (10) and also on A4 (19) and the code that defines A4
finds both of these and sums their index: 10+19 = 29.
And same problem for A5: PC_3 D8(8) and A5(d20) and 20+8= 28
And the Analog read tries to look at the pin table entries 29 and 28, to find what to sue for analogRead.
Probably need to differentiate some how for the later onss, like MBED has them as ALT0...

KurtE added 3 commits March 9, 2025 16:45
Remove debug Serial.print
This fixes issue where the
constants A4 and A5 were computed as the sum of two pin numbers
as PC_2 and PC_3 pins are duplicated on these boards.

Note: accessing those pins directly by index like 18, 19 will probably not work
KurtE added a commit to KurtE/ArduinoCore-zephyr that referenced this pull request Mar 23, 2025
This is sort of a hand merge of PR arduino#82 which supersedes arduino#71

Adds in the SPI, Wire, Some Analog, some PWM, access to all of the pins.
This has a reduced full pin table, but also introduced the use of pin names
using slightly stripped down table that was used for MBED version
@KurtE KurtE mentioned this pull request Mar 23, 2025
KurtE added a commit to KurtE/ArduinoCore-zephyr that referenced this pull request Mar 27, 2025
This is a replacement for PR: arduino#71 and arduino#82.

All of the earlier commits were squashed into one.  Then this was converted a few times during the arduino#85 pr time
frame as things kept changing and moving around.   It has now been updated to the released .3 version.

I started off adding in the whole pin table as defined by the MBED version, which actually contained duplicate defines.  I later reduced
this set such that it now longer matches the MBED version, but does still include all of the pins that have external pins on some of the
breakout boards. As for compatibility, most of the documentation for these show the PIN names and not numbers, so I imported the
MBED Pin name table and have the start of allowing several different operations to be done, like pinMode, digitalWrite.

We defined the additional SPI ports and Wire ports.

We defined an initial setup for Analog pins.  Have similar hack to GIGA version for pure Analog.  Added additional hacks for duplicated
pins.  That is two of the analog Pins are the exact same pin as some other digital pins...

Added some PWM support.

Also added WIP: camera support.

Co-Authored-By: Mike S <[email protected]>
@KurtE
Copy link
Author

KurtE commented Mar 27, 2025

replaced by #87

Converted to new layout and updated code with .3 release

@KurtE KurtE closed this Mar 27, 2025
KurtE added a commit to KurtE/ArduinoCore-zephyr that referenced this pull request Mar 31, 2025
This is a replacement for PR: arduino#71 and arduino#82.

All of the earlier commits were squashed into one.  Then this was converted a few times during the arduino#85 pr time
frame as things kept changing and moving around.   It has now been updated to the released .3 version.

I started off adding in the whole pin table as defined by the MBED version, which actually contained duplicate defines.  I later reduced
this set such that it now longer matches the MBED version, but does still include all of the pins that have external pins on some of the
breakout boards. As for compatibility, most of the documentation for these show the PIN names and not numbers, so I imported the
MBED Pin name table and have the start of allowing several different operations to be done, like pinMode, digitalWrite.

We defined the additional SPI ports and Wire ports.

We defined an initial setup for Analog pins.  Have similar hack to GIGA version for pure Analog.  Added additional hacks for duplicated
pins.  That is two of the analog Pins are the exact same pin as some other digital pins...

Added some PWM support.

Also added WIP: camera support.

Co-Authored-By: Mike S <[email protected]>
KurtE added a commit to KurtE/ArduinoCore-zephyr that referenced this pull request Apr 1, 2025
This is a replacement for PR: arduino#71 and arduino#82.

All of the earlier commits were squashed into one.  Then this was converted a few times during the arduino#85 pr time
frame as things kept changing and moving around.   It has now been updated to the released .3 version.

I started off adding in the whole pin table as defined by the MBED version, which actually contained duplicate defines.  I later reduced
this set such that it now longer matches the MBED version, but does still include all of the pins that have external pins on some of the
breakout boards. As for compatibility, most of the documentation for these show the PIN names and not numbers, so I imported the
MBED Pin name table and have the start of allowing several different operations to be done, like pinMode, digitalWrite.

We defined the additional SPI ports and Wire ports.

We defined an initial setup for Analog pins.  Have similar hack to GIGA version for pure Analog.  Added additional hacks for duplicated
pins.  That is two of the analog Pins are the exact same pin as some other digital pins...

Added some PWM support.

Also added WIP: camera support.

Co-Authored-By: Mike S <[email protected]>
KurtE added a commit to KurtE/ArduinoCore-zephyr that referenced this pull request Apr 3, 2025
This is a replacement for PR: arduino#71 and arduino#82.

All of the earlier commits were squashed into one.  Then this was converted a few times during the arduino#85 pr time
frame as things kept changing and moving around.   It has now been updated to the released .3 version.

I started off adding in the whole pin table as defined by the MBED version, which actually contained duplicate defines.  I later reduced
this set such that it now longer matches the MBED version, but does still include all of the pins that have external pins on some of the
breakout boards. As for compatibility, most of the documentation for these show the PIN names and not numbers, so I imported the
MBED Pin name table and have the start of allowing several different operations to be done, like pinMode, digitalWrite.

We defined the additional SPI ports and Wire ports.

We defined an initial setup for Analog pins.  Have similar hack to GIGA version for pure Analog.  Added additional hacks for duplicated
pins.  That is two of the analog Pins are the exact same pin as some other digital pins...

Added some PWM support.

Also added WIP: camera support.

Co-Authored-By: Mike S <[email protected]>
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

Successfully merging this pull request may close these issues.

Portenta H7: Pin Table - How complete should it be? Compatible with MBED version?
1 participant