lupyuen commented on issue #5810:
URL: 
https://github.com/apache/incubator-nuttx/issues/5810#issuecomment-1098633538

   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)
   
   ```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`](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:
   
   ```c
     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:
   -  
[`bl602_gpio_intmask`](https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L143-L169)
   -  
[`bl602_gpio_set_intmod`](https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L171-L211)
   -  
[`bl602_gpio_get_intstatus`](https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L213-L233)
   -  
[`bl602_gpio_intclear`](https://github.com/apache/incubator-nuttx/blob/master/boards/risc-v/bl602/bl602evb/src/bl602_gpio.c#L235-L253)
   
   Here are the Assertion Checks that I'm testing:
   -  
[`bl602_gpio_intmask`](https://github.com/lupyuen/cst816s-nuttx/blob/0a517f6bbe5e393a095da3748c6c5cf18a1709c6/cst816s.c#L1259-L1291)
   -  
[`bl602_gpio_set_intmod`](https://github.com/lupyuen/cst816s-nuttx/blob/0a517f6bbe5e393a095da3748c6c5cf18a1709c6/cst816s.c#L1293-L1340)
   -  
[`bl602_gpio_get_intstatus`](https://github.com/lupyuen/cst816s-nuttx/blob/0a517f6bbe5e393a095da3748c6c5cf18a1709c6/cst816s.c#L1342-L1368)
   -  
[`bl602_gpio_intclear`](https://github.com/lupyuen/cst816s-nuttx/blob/0a517f6bbe5e393a095da3748c6c5cf18a1709c6/cst816s.c#L1370-L1394)
   
   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)
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

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

Reply via email to