You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/09/28 03:03:58 UTC

[GitHub] [incubator-nuttx] btashton commented on pull request #1904: xtensa/ESP32: Fixed the type of cpuint variables in esp32_emac.c esp32_i2c.c esp32_spi.c esp32_spi_slave.c

btashton commented on pull request #1904:
URL: https://github.com/apache/incubator-nuttx/pull/1904#issuecomment-699743404


   We should move this to be `int` instead of `int8` since it is being used to optionally hold the negative error status from the allocation function which returns type `int`.   This is also in line with the other arch.  Alternatively we assign to a temporary variable prior to assigning to the struct.
   
   Also I think this change is needed in some of the other drivers:
   
   ```
   xtensa/src/esp32/esp32_serial.c:  priv->cpuint = esp32_alloc_levelint(1);
   xtensa/src/esp32/esp32_spi_slave.c:  priv->cpuint = esp32_alloc_levelint(1);
   xtensa/src/esp32/esp32_gpio.c:  g_gpio_cpuint = esp32_alloc_levelint(1);
   xtensa/src/esp32/esp32_i2c.c:  priv->cpuint = esp32_alloc_levelint(1);
   xtensa/src/esp32/esp32_cpuint.c: * Name:  esp32_alloc_levelint
   xtensa/src/esp32/esp32_cpuint.c:int esp32_alloc_levelint(int priority)
   xtensa/src/esp32/esp32_emac.c:  priv->cpuint = esp32_alloc_levelint(1);
   xtensa/src/esp32/esp32_irq.c:  cpuint = esp32_alloc_levelint(1);
   xtensa/src/esp32/esp32_spi.c:      priv->cpuint = esp32_alloc_levelint(1);
   xtensa/src/esp32/esp32_cpustart.c:  cpuint = esp32_alloc_levelint(1);
   ```
   
   `esp32_detach` also assigns to 0xff which seems like a poor chose. It would probably be better to make that < 0.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org