nuttx icon indicating copy to clipboard operation
nuttx copied to clipboard

Published 20 hours ago • verified publisher icon apache

GPIO issues on BL602 + suggestions for RISC-V, maybe more.

Open robertlipe opened this issue 2 years ago • 20 comments

  1. Some RISC-V architectures have as many as 80 GPIOs. Defining those in KConfigs will be annoying, especially given the current awesome trend of pins being behind a MUX so they can be configured as Inputs, Outputs, or Interrupts. This is a project-wide issue and I don't really know how to fix this.

  2. For those arches, the various places defining GPIOs as a bitmask isn't going to work. Since I've discovered that almost all of the current RISC-V targets break over 1 GPIO with only ESP32 reaching 2 as a breaking point, supporting 'only' a long long's of bits isn't terrible.

  3. BOARD_NGPIOfoo is defined in the board-specific, configurable board.h. However, they're used in the boards/risc-v///src/foo_gpio.c files, which are arch-specific and not board-specific. Thus, if you want to add GPIOs, you can't do it solely in boards.h; you have to modify the OS configuration. I have added an example for BL602 on how to at least DEBUG_ASSERT on this configuration error instead of expensive chasing. The basic idea either needs to be hoisted higher into the infrastructure somewhere or repeated into a bunch of foo_gpio.c files. Of course, this patch is only relevant until an appropriate design for no.1 here has a better design applied.

  4. BL604 supports up to 23 GPIOs; BL602 up to 16. (They're probably the same wafer, just packaged differently.) There are several cases of uint8_t pin = (cfgset & GPIO_PIN_MASK) >> GPIO_PIN_SHIFT; in the code. That's just not going to work for any pin > pin8, though that case has clearly never been exercised in light of no. 3, but I made a patch for that for BL602 that probably helps. Again, this should be in a test case and probably higher level or repeated DEBUG_ASSERTSs to catch it somewhere. src/mpfs/mpfs_gpio.c is almost surely wrong and searching for "8_t.*MASK" elsewhere in the tree brings attention to code that may need inspection. Possible patch attached.

These patches are created by hand (to exclude local changes) and may need hand-fitting, but since they're all one-liners, they should be easy to repair if needed.

gpio.patch addresses the issues in BL602 for GPIO >= 8. at least mpfs needs this, too. gpio2.patch is inspiration on the bandaid for debug checking that board.h matches *gpio.c. This needs to be ported to every *gpio.c file. Ick. (And nobody is building for RISC-V with a 1989 compiler.. That code is already using designated initializers, which is from C99, which has // comments.) gpio.patch.txt gpio.patch2.txt

Neither of these tries to fix the similar existing problems (def. in ESP-32 and MPFS) elsewhere in the tree.

Edit: I misspelled 'number' in this diff. Sorry.

robertlipe avatar Mar 21 '22 02:03 robertlipe

Thanks for posting this! I wanted to use Multiple GPIO Outputs for Semtech SX1262 LoRa Transceiver on BL602, but I couldn't figure out. Hope we can work out a solution for this.

lupyuen avatar Mar 21 '22 04:03 lupyuen

This is almost certainly the reason why. You hit exactly the largest configuration that would work without crashing into this by using exactly one of each of {IN, OUT, INTR}. The second on any of those would have never been connected.

With this change, you should be able to hook up that Reset line on GPIO18. One of the changes I edited away was about identical to your case; I needed a second output.

diff --git a/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c b/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c index be0aa35228..51cc63e5b5 100644 --- a/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c +++ b/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c @@ -120,6 +120,7 @@ static struct bl602_gpio_dev_s g_gpin[BOARD_NGPIOIN]; static const uint32_t g_gpiooutputs[BOARD_NGPIOOUT] = { BOARD_GPIO_OUT1,

  • BOARD_GPIO_OUT2, };

That'll actually add an instance and dev node for /dev/gpioout2 and connect it through the indirection. Then in your board.h,add something like:

#define BOARD_GPIO_OUT2 (GPIO_OUTPUT | GPIO_PULLUP |
GPIO_FUNC_SWGPIO | GPIO_PIN18)

These patches should come very close to applying for you because I forked your nuttx tree and not master. :-)

On Sun, Mar 20, 2022 at 11:06 PM Lee Lup Yuen @.***> wrote:

Thanks for posting this! I wanted to use Multiple GPIO Outputs for Semtech SX1262 LoRa Transceiver https://lupyuen.github.io/articles/sx1262#connect-sx1262-transceiver on BL602, but I couldn't figure out. Hope we can work out a solution for this.

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-nuttx/issues/5810#issuecomment-1073464882, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD342IEIK4ESCDMNTHTLVA7YTLANCNFSM5RGMP4IQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

robertlipe avatar Mar 21 '22 05:03 robertlipe

Perhaps we should take the required BL602 GPIOs and wrap them as Device Drivers, then expose them via ioctl().

I'm taking this approach for the ST7789 Display Pins on BL602: bl602_bringup.c

int board_lcd_initialize(void) {
  ...
  //  Configure Reset Pin
  bl602_configgpio(BOARD_LCD_RST);
  //  Set Reset Pin to Low
  bl602_gpiowrite(BOARD_LCD_RST, false);

The ST7789 Pins are defined in board.h

#ifdef CONFIG_LCD_ST7789
/* ST7789 Configuration: Reset and Backlight Pins */
#define BOARD_LCD_RST (GPIO_OUTPUT | GPIO_PULLUP | GPIO_FUNC_SWGPIO | GPIO_PIN4)
#define BOARD_LCD_BL  (GPIO_OUTPUT | GPIO_PULLUP | GPIO_FUNC_SWGPIO | GPIO_PIN5)
#endif  /* CONFIG_LCD_ST7789 */

I don't think we're breaking NuttX design because that's how ESP32 handles the ST7789 pins: esp32c3_st7789.c

int board_lcd_initialize(void) {
  ...
  //  Configure Reset Pin
  esp32c3_configgpio(LCD_RST, OUTPUT);
  //  Set Reset Pin to Low
  esp32c3_gpiowrite(LCD_RST, false);

These articles explain how I created my own Device Drivers for NuttX:

lupyuen avatar Mar 23 '22 23:03 lupyuen

Or we can implement an I/O Expander Driver for BL602, if we prefer to manipulate BL602 GPIOs directly from our NuttX App...

https://github.com/apache/incubator-nuttx/blob/master/drivers/ioexpander/ioe_dummy.c

lupyuen avatar Mar 23 '22 23:03 lupyuen

I saw that in ESP32C3-C3. That's how I knew it was the only other board to support as many as TWO GPIO pins - they do that very kconfig/add the #if defined thing that I suggested in one option in one of my patches. But there are a dozen other GPIOs on the chip.

ioctl gives up any real type checking. You're not going to manipulate pins via 'echo' from user mode that way, for example. That should be a last call, IMO.

ESP's code is coming beneath the standard GPIO layer. There should probably be a generic LCD driver that knows how to do open /dev/gpiio* and do reads and writes on the appropriate GPIO pins ot flip DC and Backlight as that's such a common thing. That basic ESP32 driver would be the right shapce, but it "just" needs to open /dev/gpiowhatever and issues writes to /dev/gpio whatever instead of directly calling across kernel modules like that.

On Wed, Mar 23, 2022 at 6:36 PM Lee Lup Yuen @.***> wrote:

Or we can implement a I/O Expander Driver for BL602, if we prefer to manipulate BL602 GPIOs directly from our NuttX App...

https://github.com/apache/incubator-nuttx/blob/master/drivers/ioexpander/ioe_dummy.c

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-nuttx/issues/5810#issuecomment-1076920169, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD34GAIHZG5F4OW5BOK3VBOTHRANCNFSM5RGMP4IQ . You are receiving this because you authored the thread.Message ID: @.***>

robertlipe avatar Mar 24 '22 01:03 robertlipe

ST7789 Driver runs in Kernel Space so I don't think it can access /dev/gpio*?

There is a generic LCD Driver that wraps up the ST7789 Driver as /dev/lcd*:

https://github.com/lupyuen/incubator-nuttx/blob/st7789/drivers/lcd/lcd_dev.c#L342-L343

I suppose LVGL runs in User Mode to access /dev/lcd*? I'm still deciphering the code: lvgldemo.c

UPDATES: Yes LVGL indeed runs in User Mode to access /dev/lcd0. See lcddev.c

There's another GPIO quirk inside the SPI Driver for ESP32: To control the ST7789 Data/Command Pin, the SPI Driver flips the MISO Pin as though it was a GPIO: esp32c3_board_spi.c

int esp32c3_spi2_cmddata(FAR struct spi_dev_s *dev, uint32_t devid, bool cmd) {
      ...
      /*  This is the Data/Command control pad which determines whether the
       *  data bits are data or a command.
       */
      esp32c3_gpiowrite(CONFIG_ESP32C3_SPI2_MISOPIN, !cmd);

To implement this on BL602 I'm trying to reconfigure MISO as GPIO on the fly: bl602_spi.c

static int bl602_spi_cmddata(struct spi_dev_s *dev, uint32_t devid, bool cmd) {
  //  MISO is now configured as SPI Pin. We reconfigure MISO as GPIO Pin.
  gpio_pinset_t gpio = 
    (BOARD_SPI_MISO & GPIO_PIN_MASK)  //  Get the pin number
    | GPIO_OUTPUT | GPIO_PULLUP | GPIO_FUNC_SWGPIO;  //  Change to GPIO Output
  int ret = bl602_configgpio(gpio);
  ...
  //  Set MISO to High (data) or Low (command)
  bl602_gpiowrite(gpio, !cmd);

  //  After this the caller will transmit data or command.
  //  Then bl602_spi_select() will revert MISO back from GPIO to SPI Pin.
  //  We must revert because the SPI Bus may be used by other drivers.
  return OK;
}

I'm now testing this with a Logic Analyser. I'm hitting another problem with SPI Receive Timeout but I'll explain later:

https://github.com/lupyuen/incubator-nuttx/pull/39/files

UPDATE: The fix for SPI Receive Timeout is here: https://github.com/lupyuen/incubator-nuttx/pull/42

lupyuen avatar Mar 24 '22 02:03 lupyuen

If it helps, I got lost in the layering, too. I wasn't sure what was supposed to be in kernel/"user" space and who was supposed to be able to fondle each others' bits from any given layer in the abstraction. That's why I called in for design guidance from those above with familiarity with multiple subsystems.

Though I saw that ESP32-C3 got one case "more right" than BL602, I wasn't really convinced it was the ideal role model, either. That was the point I started chasing both ends of the candle, one up and over into well-exercised ARM boards and generic vs. chip vs board vs any given bit being remapped as a normal GPIO or a specialized SPIO-type pin - and if the inversion was done within the chip itself AND learning the other end and getting tripped up in LVGL vs. NX and a framebuffer vs. a framework and so on as those didn't seem to have crisp edges to me. Your code had the MISO swap and trunk didn't, so I wasn't sure it had been done in some other way, but that was also the time I was working with your branch and not nuttx master, so I couldn't tell if that was a design decision or just a. dropped/unneeded/unwanted PR.

I got quite confused and had to mentally context switch away from it for other reasons; I've not made it back since then. I hope to get back to it in the next couple of days.

Good luck!

On Wed, Mar 23, 2022 at 9:23 PM Lee Lup Yuen @.***> wrote:

ST7789 Driver runs in Kernel Space so I don't think it can access /dev/gpio*?

There is a generic LCD Driver that wraps up the ST7789 Driver as /dev/lcd* :

https://github.com/lupyuen/incubator-nuttx/blob/st7789/drivers/lcd/lcd_dev.c#L342-L343

I suppose LVGL runs in User Mode to access /dev/lcd*? I'm still deciphering the code.

There's another GPIO quirk inside the SPI Driver for ESP32: To control the ST7789 Data/Command Pin, the SPI Driver flips the MISO Pin as though it was a GPIO: esp32c3_board_spi.c https://github.com/lupyuen/incubator-nuttx/blob/st7789/boards/risc-v/esp32c3/esp32c3-devkit/src/esp32c3_board_spi.c#L64-L84

int esp32c3_spi2_cmddata(FAR struct spi_dev_s dev, uint32_t devid, bool cmd) { ... / This is the Data/Command control pad which determines whether the * data bits are data or a command. */ esp32c3_gpiowrite(CONFIG_ESP32C3_SPI2_MISOPIN, !cmd);

To implement this on BL602 I'm trying to reconfigure MISO as GPIO on the fly: bl602_spi.c https://github.com/lupyuen/incubator-nuttx/blob/st7789/arch/risc-v/src/bl602/bl602_spi.c#L709-L735

static int bl602_spi_cmddata(struct spi_dev_s *dev, uint32_t devid, bool cmd) { // MISO is now configured as SPI Pin. We reconfigure MISO as GPIO Pin. gpio_pinset_t gpio = (BOARD_SPI_MISO & GPIO_PIN_MASK) // Get the pin number | GPIO_OUTPUT | GPIO_PULLUP | GPIO_FUNC_SWGPIO; // Change to GPIO Output int ret = bl602_configgpio(gpio); ... // Set MISO to High (data) or Low (command) bl602_gpiowrite(gpio, !cmd);

// After this the caller will transmit data or command. // Then bl602_spi_select() will revert MISO back from GPIO to SPI Pin. // We must revert because the SPI Bus may be used by other drivers. return OK; }

I'm now testing this with a Logic Analyser. I'm hitting another problem with SPI Receive Timeout but I'll explain later:

https://github.com/lupyuen/incubator-nuttx/pull/39/files

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-nuttx/issues/5810#issuecomment-1077001344, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD3Y4BTCE5JJRNNQSFGDVBPG2BANCNFSM5RGMP4IQ . You are receiving this because you authored the thread.Message ID: @.***>

robertlipe avatar Mar 24 '22 06:03 robertlipe

Cool thanks! BL602 MISO / MOSI Swap is in the master branch here: bl602_spi.c

BTW The ST7789 Data/Command Pin has been tested OK on BL602.

MISO goes Low when we send an ST7789 Command...

st7789-logic

And MISO goes High when we send ST7789 Data...

st7789-logic2

We implemented this by reconfiguring MISO from SPI Pin to GPIO on the fly: bl602_spi.c

Just found out that bl602_spi_poll_exchange() works for SPI Transfer but bl602_spi_poll_send() doesn't send any SPI data, which affects the ST7789 Driver. I'm checking through the code now.

UPDATE: The fix for SPI Poll Send is here: https://github.com/lupyuen/incubator-nuttx/pull/42

lupyuen avatar Mar 24 '22 08:03 lupyuen

OK LVGL works on BL602 with ST7789 Display! We fixed a couple of GPIO and SPI issues...

  1. Fixed SPI Send on BL602: https://github.com/lupyuen/incubator-nuttx/pull/42

  2. Implemented SPI Cmd/Data on BL602: https://github.com/lupyuen/incubator-nuttx/pull/44

  3. BL602 uses SPI Mode 3 to talk to ST7789

Details here: https://github.com/lupyuen/st7789-nuttx

lupyuen avatar Mar 25 '22 10:03 lupyuen

@lupyuen it will be great if you can upstream your fix and improvement to mainline.

xiaoxiang781216 avatar Mar 25 '22 13:03 xiaoxiang781216

Yep I'll submit the PR soon thanks!

lupyuen avatar Mar 25 '22 13:03 lupyuen

Excellent. Thank you, Lup Yuen!. Please feel free to cherry pick patches from the OP or Xiang Xiao can take the ones he'd accept and perhaps guide us on how to do the rest.

On Fri, Mar 25, 2022 at 8:38 AM Lee Lup Yuen @.***> wrote:

Yep I'll submit the PR soon thanks!

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-nuttx/issues/5810#issuecomment-1079038122, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD34DJED44JRZAD4MP4DVBW6W3ANCNFSM5RGMP4IQ . You are receiving this because you authored the thread.Message ID: @.***>

robertlipe avatar Mar 25 '22 20:03 robertlipe

Our fix for SPI Poll Send has been merged into NuttX (thanks Xiao Xiang!)

https://github.com/apache/incubator-nuttx/pull/5869

Now doing the PR for SPI Cmd/Data...

lupyuen avatar Mar 28 '22 06:03 lupyuen

Our implementation of SPI Cmd/Data has been into NuttX (thanks Xiao Xiang!)

https://github.com/apache/incubator-nuttx/pull/5898

lupyuen avatar Mar 29 '22 04:03 lupyuen

There's a Chinese version of this thread 🤔

https://m.editcode.net/forum.php?mod=viewthread&tid=882551&mobile=2

lupyuen avatar Apr 01 '22 01:04 lupyuen

Here's my plan to implement a GPIO Expander for BL604:

https://lupyuen.github.io/articles/pinedio2#gpio-expander

lupyuen avatar Apr 10 '22 13:04 lupyuen

Documenting a bug in BL602 EVB GPIO: This call to bl602_gpio_set_intmod is incorrect: bl602evb/bl602_gpio.c

int bl602_gpio_initialize(void) {
  ...
  bl602_gpio_set_intmod(g_gpiointinputs[i], 1, GLB_GPIO_INT_TRIG_NEG_PULSE);

This passes a pinset (uint32_t) to bl602_gpio_set_intmod. But bl602_gpio_set_intmod accepts a GPIO Pin Number gpio_pin (uint8_t), not a pinset (uint32_t).

We need to convert the pinset to gpio_pin like so:

  uint8_t gpio_pin = (g_gpiointinputs[i] & GPIO_PIN_MASK) >> GPIO_PIN_SHIFT;
  bl602_gpio_set_intmod(gpio_pin, 1, GLB_GPIO_INT_TRIG_NEG_PULSE);

To catch such issues, we might implement Assertion Checks on GPIO Numbers in these functions:

Here are the Assertion Checks that I'm testing:

GPIO Pin Numbers like 28 have been changed to GPIO_PIN28 for consistency.

Also note that bl602_gpio_intmask should accept a gpio_pin (uint8_t), not uint32_t.

Here's the fix

lupyuen avatar Apr 14 '22 01:04 lupyuen

I'm a fan of arguments being 'real' data types instead of everything being ints (and remember that passing chars often means passing an int anyway) for everything. That lets the compiler enforce the contract between the parts, making this whole class of problems more obvious at compile time.

Does bl602_expander_intmask() need to inhibit interrupts/context switches between the read of BL602_GPIO_INT_MASK1 and the store to BL602_GPIO_INT_MASK1? That may suggest performing the shift outside the block just to shorten that window by a few opcodes. Maybe a irqstate_t flags = spin_lock_irqsave(NULL); ... do the thing spin_unlock_irqrestore(NULL, flags);

Actually, it looks like there's a LOT of that protection missing in the BL602 code. If you agree that's sane, please create a PR and assign it to me and I'll pencil-whip the code and make a pass at improving that.

On Wed, Apr 13, 2022 at 8:55 PM Lee Lup Yuen @.***> wrote:

Documenting a bug in BL602 EVB GPIO: This call to bl602_gpio_set_intmod is incorrect: bl602evb/bl602_gpio.c https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L597-L598

int bl602_gpio_initialize(void) { ... bl602_gpio_set_intmod(g_gpiointinputs[i], 1, GLB_GPIO_INT_TRIG_NEG_PULSE);

This passes a pinset (uint32_t) to bl602_gpio_set_intmod https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L179-L180. But bl602_gpio_set_intmod https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L179-L180 accepts a GPIO Pin Number gpio_pin (uint8_t), not a pinset (uint32_t).

We need to convert the pinset to gpio_pin like so:

uint8_t gpio_pin = (g_gpiointinputs[i] & GPIO_PIN_MASK) >> GPIO_PIN_SHIFT; bl602_gpio_set_intmod(gpio_pin, 1, GLB_GPIO_INT_TRIG_NEG_PULSE);

To catch such issues, we might implement Assertion Checks on GPIO Numbers in these functions:

Here are the Assertion Checks that I'm testing:

GPIO Pin Numbers like 28 have been changed to GPIO_PIN28 for consistency.

Also note that bl602_gpio_intmask https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L143-L169 should accept a gpio_pin (uint8_t), not uint32_t.

Here's the fix https://github.com/lupyuen/cst816s-nuttx/blob/0a517f6bbe5e393a095da3748c6c5cf18a1709c6/cst816s.c#L1259-L1291

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-nuttx/issues/5810#issuecomment-1098633538, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD336EXEHUKM5SOIQOPLVE53KLANCNFSM5RGMP4IQ . You are receiving this because you authored the thread.Message ID: @.***>

robertlipe avatar Apr 14 '22 03:04 robertlipe

AHA!

Inside bl602_expander_intmask() please consider the following modification ... unsigned bit = (1U << gpio_pin); if (intmask) { modifyreg32(BL602_GPIO_INT_MASK1, 0, bit); } else { modifyreg32(BL602_GPIO_INT_MASK1, bit, 0); ...and that should take care of the atomicity issue as modifyreg32() handles it. Anything in bl602* (or elsewhere) that calls getreg and then setreg should be treated with great suspicion.

Assign it to me to make a pass at examining that code if you agree.

On Wed, Apr 13, 2022 at 10:11 PM Robert Lipe @.***> wrote:

I'm a fan of arguments being 'real' data types instead of everything being ints (and remember that passing chars often means passing an int anyway) for everything. That lets the compiler enforce the contract between the parts, making this whole class of problems more obvious at compile time.

Does bl602_expander_intmask() need to inhibit interrupts/context switches between the read of BL602_GPIO_INT_MASK1 and the store to BL602_GPIO_INT_MASK1? That may suggest performing the shift outside the block just to shorten that window by a few opcodes. Maybe a irqstate_t flags = spin_lock_irqsave(NULL); ... do the thing spin_unlock_irqrestore(NULL, flags);

Actually, it looks like there's a LOT of that protection missing in the BL602 code. If you agree that's sane, please create a PR and assign it to me and I'll pencil-whip the code and make a pass at improving that.

On Wed, Apr 13, 2022 at 8:55 PM Lee Lup Yuen @.***> wrote:

Documenting a bug in BL602 EVB GPIO: This call to bl602_gpio_set_intmod is incorrect: bl602evb/bl602_gpio.c https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L597-L598

int bl602_gpio_initialize(void) { ... bl602_gpio_set_intmod(g_gpiointinputs[i], 1, GLB_GPIO_INT_TRIG_NEG_PULSE);

This passes a pinset (uint32_t) to bl602_gpio_set_intmod https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L179-L180. But bl602_gpio_set_intmod https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L179-L180 accepts a GPIO Pin Number gpio_pin (uint8_t), not a pinset (uint32_t).

We need to convert the pinset to gpio_pin like so:

uint8_t gpio_pin = (g_gpiointinputs[i] & GPIO_PIN_MASK) >> GPIO_PIN_SHIFT; bl602_gpio_set_intmod(gpio_pin, 1, GLB_GPIO_INT_TRIG_NEG_PULSE);

To catch such issues, we might implement Assertion Checks on GPIO Numbers in these functions:

Here are the Assertion Checks that I'm testing:

GPIO Pin Numbers like 28 have been changed to GPIO_PIN28 for consistency.

Also note that bl602_gpio_intmask https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L143-L169 should accept a gpio_pin (uint8_t), not uint32_t.

Here's the fix https://github.com/lupyuen/cst816s-nuttx/blob/0a517f6bbe5e393a095da3748c6c5cf18a1709c6/cst816s.c#L1259-L1291

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-nuttx/issues/5810#issuecomment-1098633538, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD336EXEHUKM5SOIQOPLVE53KLANCNFSM5RGMP4IQ . You are receiving this because you authored the thread.Message ID: @.***>

robertlipe avatar Apr 14 '22 03:04 robertlipe

We have just implemented a GPIO Expander that fixes the above issues for BL602 and BL604:

  • Support 23 GPIOs with any mix of GPIO Inputs / Outputs / Interrupts

  • Renumber the GPIOs as "/dev/gpio0" to "/dev/gpio22", which are simply mapped as GPIO 0 to GPIO 22

  • Allow gaps in the GPIO Numbering, skipping the GPIOs reserved for UART, I2C, SPI and PWM

  • Keep the BL602 EVB Pin Definitions (board.h), so that BL602 EVB will still build OK without GPIO Expander

  • Validate the GPIOs at startup

More details in this article...

lupyuen avatar May 02 '22 06:05 lupyuen