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/08/30 08:51:15 UTC
[GitHub] [incubator-nuttx] Ouss4 opened a new pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Ouss4 opened a new pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437
## Summary
Don't fake a TX interrupt when interrupts are not suppressed. We have already enabled interrupts, we can just wait for the next TX interrupt.
## Impact
STM32 Serial driver
## Testing
olimex-stm32-p407:nsh
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-909286990
@Ouss4
I suspect ( I need to diagram it out) the case is for a resume of TX after a -HS in the case the user is not writing any more data (acknowledged protocol, sizeof(remote RX buffer) < sizeof(local TX buffer). The first user write loads and triggers the successive TX and TXE until the -HS case. TXE does not enqueue again. So the call is made to kick off the restart.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-908164642
@davids5 I've tried this when I was working on https://github.com/apache/incubator-nuttx/pull/4403#issuecomment-905019719, can you please check?
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-910189575
Thanks for these details @davids5.
I didn't attach an LA to look at what's going on with HW flow control enabled either (my stuff are kind of a mess right now), I might do that later. However I tried to find when this was introduced and try to find any hint in the commit message or the change itself. It came as part of https://github.com/apache/incubator-nuttx/commit/4494689a2220501f74404e68b3eb72c112c9a888#diff-867b440e179e185b86ca1b3423ad19b5174ec9345a02380733d51f437a40db58R691-R700
Since everything is folded, let me copy the relevant snippet:
```C
#ifndef CONFIG_SUPPRESS_SERIAL_INTS
// priv->im |= ;
up_serialout(priv, stm32_USART_IM_OFFSET, priv->im);
/* Fake a TX interrupt here by just calling uart_xmitchars() with
* interrupts disabled (note that the may recurse and that priv->im
* may be altered.
*/
uart_xmitchars(dev);
#endif
```
My guess is since setting the interrupt bits was commented out, faking it was necessary.
This was completed in https://github.com/apache/incubator-nuttx/commit/a7942e1bbe734f9bd7a4b737a899fa880ef66a88, but (again my guess) faking the interrupt part was not removed (i.e. adding an `"#else` was forgotten). At that point flow control was not implemented in STM32.
I don't know if later this part was deemed necessary when flow control was added.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 merged pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-917213046
@Ouss4 I am sorry for the delay. I had to clear my bench and my mind. :) I have proved that the ISR will happen (It is a level)
and the TXIE enable after the HS going true will cause the ISR. So the PR is correct. I tested this on the F4 and F7. I have not tested it on the H7, but I expect it will be the same.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-908754668
@davids5 Thanks for the comment. I tested (albeit very quickly) with hardware flow control and I didn't see any issue. Actually I don't see how this recursive call re-enables hardware flow control.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-909286990
@Ouss4
I suspect ( I need to diagram it out) the case is for a resume of TX after a -HS in the case the user is not writing any more data (acknowledged protocol, sizeof(remote RX buffer) < sizeof(local TX buffer). The first user write loads and triggers the successive TX and TXE until the -HS case. TXE does not enqueue again. So the call is made to kick off the restart.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-908644969
@Ouss4 - I want to say that the call needs to happen to re-enable flow control. Did you test with HW flow control?
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-908644969
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-917235600
@davids5 thanks for the tests, much appreciated. I too had a hectic week, so couldn't do anything else to confirm.
The PR isn't a draft anymore.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-908754668
@davids5 Thanks for the comment. I tested (albeit very quickly) with hardware flow control and I didn't see any issue. Actually I don't see how this recursive call re-enables hardware flow control.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #4437: stm32_serial.c: Don't fake a TX interrupt when interrupts are not suppressed
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4437:
URL: https://github.com/apache/incubator-nuttx/pull/4437#issuecomment-910822711
> Thanks for these details @davids5.
> I didn't attach an LA to look at what's going on with HW flow control enabled either (my stuff are kind of a mess right now), I might do that later. However I tried to find when this was introduced and try to find any hint in the commit message or the change itself. It came as part of [4494689#diff-867b440e179e185b86ca1b3423ad19b5174ec9345a02380733d51f437a40db58R691-R700](https://github.com/apache/incubator-nuttx/commit/4494689a2220501f74404e68b3eb72c112c9a888#diff-867b440e179e185b86ca1b3423ad19b5174ec9345a02380733d51f437a40db58R691-R700)
>
> Since everything is folded, let me copy the relevant snippet:
>
> ```c
> #ifndef CONFIG_SUPPRESS_SERIAL_INTS
> // priv->im |= ;
> up_serialout(priv, stm32_USART_IM_OFFSET, priv->im);
>
> /* Fake a TX interrupt here by just calling uart_xmitchars() with
> * interrupts disabled (note that the may recurse and that priv->im
> * may be altered.
> */
>
> uart_xmitchars(dev);
> #endif
> ```
>
> My guess is since setting the interrupt bits was commented out, faking it was necessary.
> This was completed in [a7942e1](https://github.com/apache/incubator-nuttx/commit/a7942e1bbe734f9bd7a4b737a899fa880ef66a88), but (again my guess) faking the interrupt part was not removed (i.e. adding an `"#else` was forgotten). At that point flow control was not implemented in STM32.
> I don't know if later this part was deemed necessary when flow control was added.
I do not remember :(
I want to test it, I just could not get to it today. I think it is a simple as determining if the IRQ is a level or an edge.
HS CTS__________--------------
TXE_______________---------------
TXEI set before_------------- > the TXE edge or level will be seen for sure.
TXEI___________________------------- This the case I question. If the enable changes after the HS allows the DR->TXS will the IRQ get seen. If so the fake is not needed.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org