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 2021/04/29 13:09:41 UTC

[GitHub] [incubator-nuttx] julianoes opened a new pull request #3633: drivers/serial: fix Rx interrupt enable for cdcacm

julianoes opened a new pull request #3633:
URL: https://github.com/apache/incubator-nuttx/pull/3633

The debugging and work leading to this pull request was done together with @davids5.

## Summary

This is addressing an issue where Rx interrupts were not restored for cdcacm after the buffer had been filled (in a burst).
The Rx interrupts were only restored after the [watchdog](https://github.com/apache/incubator-nuttx/blob/940c5b69c36fd1210ada473a5665e4c83420be77/drivers/usbdev/cdcacm.c#L734-L735) timeout of 200ms fired.

This happened because `CONFIG_SERIAL_RXDMA` was set, however, for the cdcacm driver this does not actually matter as it is relying on `uart_enablerxint` to be called via the ops table.

This patch makes sure that `uart_enablerxint` and `uart_disablerxint` are always called, independent of CONFIG_SERIAL_RXDMA, relying on the ops table to be correct for the case where it should be only a no-op.

## Impact

- Before applying this patch we need to check all archs/platforms whether they have the `uart_enablerxint` and `uart_disablerxint` correctly set (or not set) in the ops table.
- Also, the location of `enter_critical_section` and `leave_critical_section` need to be reviewed as I'm not quite sure of how it should exactly be, for the "normal" and DMA case.

## Testing

Testing so far was done on a stm32f4 and stm32f7 running PX4 by sending a burst of serial data and basically disabling the cdcacm watchdog by making the [timeout](https://github.com/apache/incubator-nuttx/blob/940c5b69c36fd1210ada473a5665e4c83420be77/drivers/usbdev/cdcacm.c#L65) very big.

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

[GitHub] [incubator-nuttx] julianoes commented on pull request #3633: drivers/serial: fix Rx interrupt enable for cdcacm

Posted by GitBox <gi...@apache.org>.

julianoes commented on pull request #3633:
URL: https://github.com/apache/incubator-nuttx/pull/3633#issuecomment-834087639


   Thank you @davids5 and @xiaoxiang781216 for your help.


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

[GitHub] [incubator-nuttx] davids5 edited a comment on pull request #3633: drivers/serial: fix Rx interrupt enable for cdcacm

Posted by GitBox <gi...@apache.org>.

davids5 edited a comment on pull request #3633:
URL: https://github.com/apache/incubator-nuttx/pull/3633#issuecomment-833395039


   @xiaoxiang781216 - would you please have a look at the changes here. The issues is that DMA is per port by TX or RX. So all 4 permutations (NO DMA, RX & TX DMA, RX Only DMA or TX Only DMA) can be enabled on different ports in the low level drivers by port.
   
   So U[S]ART1 may have RX & TX DMA and U[S]ART2 may have NO DMA. But CONFIG_SERIAL_RXDMA will be lit and then the proper functions are not called on the ports not using DMA.
   
   


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

[GitHub] [incubator-nuttx] davids5 edited a comment on pull request #3633: drivers/serial: fix Rx interrupt enable for cdcacm

Posted by GitBox <gi...@apache.org>.

davids5 edited a comment on pull request #3633:
URL: https://github.com/apache/incubator-nuttx/pull/3633#issuecomment-833395039


   @xiaoxiang781216 - would you please have a look at the changes here. The issues is that DMA is per port by TX or RX. So all 4 permutations (NO DMA, RX & TX DMA, RX Only DMA or TX Only DMA) can be enabled on different ports in the low level drivers by port.
   
   So U[S]ART1 may have RX & TX DMA and U[S]ART2 may have NO DMA. and CONFIG_SERIAL_RXDMA will be lit and then the proper functions are not called on the ports not using DMA.
   
   


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

[GitHub] [incubator-nuttx] davids5 commented on pull request #3633: drivers/serial: fix Rx interrupt enable for cdcacm

Posted by GitBox <gi...@apache.org>.

davids5 commented on pull request #3633:
URL: https://github.com/apache/incubator-nuttx/pull/3633#issuecomment-833492357


   @xiaoxiang781216 - Thank you - please merge.


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

[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #3633: drivers/serial: fix Rx interrupt enable for cdcacm

Posted by GitBox <gi...@apache.org>.

xiaoxiang781216 merged pull request #3633:
URL: https://github.com/apache/incubator-nuttx/pull/3633


   


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

[GitHub] [incubator-nuttx] davids5 commented on pull request #3633: drivers/serial: fix Rx interrupt enable for cdcacm

Posted by GitBox <gi...@apache.org>.

davids5 commented on pull request #3633:
URL: https://github.com/apache/incubator-nuttx/pull/3633#issuecomment-833395039


   @xiaoxiang781216 - would you please have a look at the changes here. The issues is that DMA is per port by TX or RX. So all 4 permutations (NO DMA, RX & TX DMA, RX Only DMA or TX Only DMA) can be enabled on different ports in the low level drivers by port.
   
   So U[S]ART1 may have RX & TX DMA and U[S]ARTmay have NO DMA. and CONFIG_SERIAL_RXDMA will be lit and then the proper functions are not called on the ports not using DMA.
   
   


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