Loading ...
Sorry, an error occurred while loading the content.
 

Re: UART drivers vs. CONFIG_SUPPRESS_SERIAL_INTS

Expand Messages
  • Gregory N
    Hi, Mike, ... It is not implemented as you describe. CONFIG_SUPPRESS_SERIAL_INTS enables a hack that I used in the past during some board bring-up only to
    Message 1 of 13 , Apr 1, 2012
      Hi, Mike,

      > The intent of the design seems to be that CONFIG_SUPPRESS_SERIAL_INTS will cause serial interrupts not to be generated, and "the console will poll", but I haven't been able to work out how that's implemented.

      It is not implemented as you describe. CONFIG_SUPPRESS_SERIAL_INTS enables a hack that I used in the past during some board bring-up only to avoid interrupts before I am able to handle them. It is not a "feature" this is typically used or supported.

      This would normally be defined in arch/arm/src/common/up_internal.h. Note the comments before the definitions:

      /* Bring-up debug configurations. These are here (vs defconfig)
      * because these should only be controlled during low level
      * board bring-up and not part of normal platform configuration.
      */

      #undef CONFIG_SUPPRESS_INTERRUPTS /* DEFINED: Do not enable interrupts */
      #undef CONFIG_SUPPRESS_TIMER_INTS /* DEFINED: No timer */
      #undef CONFIG_SUPPRESS_SERIAL_INTS /* DEFINED: Console will poll */
      #undef CONFIG_SUPPRESS_UART_CONFIG /* DEFINED: Do not reconfig UART */
      #undef CONFIG_DUMP_ON_EXIT /* DEFINED: Dump task state on exit */

      I probably should remove most of those.

      > On the receive side (which is where I am currently focussing), I would assume this means that something would call uart_rxavailable() followed by uart_receive() at the designated interval. I just can't find where this is done.
      >
      > Is this still an intended part of the architecture, or is it obsolete?

      These are macros defined in includes/nuttx/serial.h:

      /* vtable access helpers */

      #define uart_setup(dev) dev->ops->setup(dev)
      #define uart_shutdown(dev) dev->ops->shutdown(dev)
      #define uart_attach(dev) dev->ops->attach(dev)
      #define uart_detach(dev) dev->ops->detach(dev)
      #define uart_enabletxint(dev) dev->ops->txint(dev, true)
      #define uart_disabletxint(dev) dev->ops->txint(dev, false)
      #define uart_enablerxint(dev) dev->ops->rxint(dev, true)
      #define uart_disablerxint(dev) dev->ops->rxint(dev, false)
      #define uart_rxavailable(dev) dev->ops->rxavailable(dev)
      #define uart_txready(dev) dev->ops->txready(dev)
      #define uart_txempty(dev) dev->ops->txempty(dev)
      #define uart_send(dev,ch) dev->ops->send(dev,ch)
      #define uart_receive(dev,s) dev->ops->receive(dev,s)

      There was a post some time back similar to yours. Is the problem in this thread the same as the one you are seeing: http://tech.groups.yahoo.com/group/nuttx/message/1004

      Greg
    • Michael Smith
      ... Got it, thanks. This actually simplifies my fix and leads to another question; The current push-pull architecture for serial receive means that the
      Message 2 of 13 , Apr 1, 2012

        On Apr 1, 2012, at 6:04 AM, Gregory N wrote:

         

        > The intent of the design seems to be that CONFIG_SUPPRESS_SERIAL_INTS will cause serial interrupts not to be generated, and "the console will poll", but I haven't been able to work out how that's implemented.

        It is not implemented as you describe. CONFIG_SUPPRESS_SERIAL_INTS enables a hack that I used in the past during some board bring-up only to avoid interrupts before I am able to handle them. It is not a "feature" this is typically used or supported.

        Got it, thanks.  This actually simplifies my fix and leads to another question;

        The current push-pull architecture for serial receive means that the interrupt flow for the STM32 UART with a single-byte holding register looks like this:

        interrupt handler, dispatch
        up_interrupt_common
        uart_recvchars
        uart_rxavailable
        uart_receive
        uart_rxavailable

        It would seem to be a useful optimization to have a common UART routine that allowed the driver to up call with a single, successfully received byte; i.e. the common flow would replace the call to uart_recvchars with a single uart_recv(byte) call that could accept or discard the byte at will.

        There was a post some time back similar to yours. Is the problem in this thread the same as the one you are seeing: http://tech.groups.yahoo.com/group/nuttx/message/1004


        I haven't looked at the LPC serial driver, but it's quite possible.  In the STM32 driver there's a race in up_restoreusartint where the interrupt mask and the cached enable bits are not changed in sync, allowing an interrupt to occur while the mask claims it's disabled.  Combined with the (arguably redundant) checks in up_interrupt_common this can lead to fatal interrupt livelock.

         = Mike
      • Gregory N
        ... The current serial driver is designed to work well as a console with dribs and drabs of data are exchanged. ( dribs and drabs? I can t believe I said
        Message 3 of 13 , Apr 1, 2012
          > It would seem to be a useful optimization to have a common UART routine that allowed the driver to up call with a single, successfully received byte; i.e. the common flow would replace the call to uart_recvchars with a single uart_recv(byte) call that could accept or discard the byte at will.

          The current serial driver is designed to work well as a console with dribs and drabs of data are exchanged. ("dribs and drabs?" I can't believe I said that). It actually works pretty well that way.

          I think what you really need is a custom serial driver, perhaps using DMA, ping pong buffers, and hardware handshaking. Some design that is optimized for high data rate transfers vs. dribs and drabs.

          > ... http://tech.groups.yahoo.com/group/nuttx/message/1004

          As I recall that was my bottom line advice in this case as well.

          Greg
        • Michael Smith
          ... With the stock STM32 UART driver set up as a console, I can cut and paste a single-line command into the console @115200bps and lock the system up maybe
          Message 4 of 13 , Apr 1, 2012

            On Apr 1, 2012, at 3:23 PM, Gregory N wrote:

             

            > It would seem to be a useful optimization to have a common UART routine that allowed the driver to up call with a single, successfully received byte; i.e. the common flow would replace the call to uart_recvchars with a single uart_recv(byte) call that could accept or discard the byte at will.

            The current serial driver is designed to work well as a console with dribs and drabs of data are exchanged. ("dribs and drabs?" I can't believe I said that). It actually works pretty well that way.

            I think what you really need is a custom serial driver, perhaps using DMA, ping pong buffers, and hardware handshaking. Some design that is optimized for high data rate transfers vs. dribs and drabs.

            > ... http://tech.groups.yahoo.com/group/nuttx/message/1004

            As I recall that was my bottom line advice in this case as well.

            With the stock STM32 UART driver set up as a console, I can cut and paste a single-line command into the console @115200bps and lock the system up maybe one time in ten on the STM32F4.  On a 24MHz STM32F100 I can do it every second time.  It's not working well as a console.

            I understand the design goal, but the current architecture actually looks like it's trying to be substantially more complex; why else the convoluted callback scheme for draining the FIFO, for example?

            Separately, yes, I can see that there's a need for a design that supports higher data rates and lower byte latency given the lack of a FIFO on the STM32 UART.  I'm pretty sure that it can fit within the current architecture, but I'll reserve final commentary until I can give you code.

             = Mike
          • Gregory N
            ... Is this is because of overrun in the serial driver because it is not servicing the RX input at a high enough rate? That would explain the difference
            Message 5 of 13 , Apr 1, 2012
              > With the stock STM32 UART driver set up as a console, I can cut and paste a single-line command into the console @115200bps and lock the system up maybe one time in ten on the STM32F4. On a 24MHz STM32F100 I can do it every second time. It's not working well as a console.

              Is this is because of overrun in the serial driver because it is not servicing the RX input at a high enough rate? That would explain the difference between behavior on the 168MHz and 24Mhz machines.

              Greg
            • Gregory N
              Hi, again, ... And... if that is the case, I bet that the culprit is the logic in drivers/serial/serial.c. Look at the function uart_read(); it keeps the RX
              Message 6 of 13 , Apr 1, 2012
                Hi, again,

                > > With the stock STM32 UART driver set up as a console, I can cut and paste a single-line command into the console @115200bps and lock the system up maybe one time in ten on the STM32F4. On a 24MHz STM32F100 I can do it every second time. It's not working well as a console.
                >
                > Is this is because of overrun in the serial driver because it is not servicing the RX input at a high enough rate? That would explain the difference between behavior on the 168MHz and 24Mhz machines.

                And... if that is the case, I bet that the culprit is the logic in drivers/serial/serial.c. Look at the function uart_read(); it keeps the RX interrupts disabled while it removes data from the circular buffer.

                Now, there are some operations on the circular buffer that do require protection. But the "bottom half" is not permitted to process incoming characters while UART RX interrupts are disabled and they are disabled throughout the entire read operation (unless we run out of data).

                In addition to the other changes that you have in the STM32 "lower half" driver, consider something like the following to give the RX interrupt handling a little more opportunity:

                static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen)
                {
                ...
                - uart_disablerxint(dev);
                while (recvd < buflen)
                {
                + uart_disablerxint(dev);
                ...
                + uart_enablerxint(dev);
                }

                - uart_enablerxint(dev);
                ...
                }

                This would still provide the same protection, but would greatly reduce the amount of time the RX interrupts are disabled. (If you are really careful you should even be able implement one side of a FIFO without disabling the other side at all.).

                If this works for you, let me know and I can incorporate this.

                Greg
              • Michael Smith
                ... This is exactly what I was proposing in my earlier comment off-list when I was talking about the size of the FIFO index value. If you re amenable to
                Message 7 of 13 , Apr 1, 2012

                  On Apr 1, 2012, at 5:18 PM, Gregory N wrote:

                   

                  In addition to the other changes that you have in the STM32 "lower half" driver, consider something like the following to give the RX interrupt handling a little more opportunity:

                  This is exactly what I was proposing in my earlier comment off-list when I was talking about the size of the FIFO index value. 

                  If you're amenable to considering changes here (per your other comment about changes in the top half) then I would be more than happy to prototype something and test it in our setup.

                  This would still provide the same protection, but would greatly reduce the amount of time the RX interrupts are disabled. (If you are really careful you should even be able implement one side of a FIFO without disabling the other side at all.).

                  For a strict producer/consumer ringbuffer, as long as updates are atomic there's no need for locking as long as you're careful with your order of operations.  The only case where I can see that lock being required is on an 8-bit micro where the FIFO is more than 256 bytes large.

                  If that is not a supported configuration, then removing that lock should help in a number of cases.

                   = Mike

                • Gregory N
                  Mike, I just checked in the change to drivers/serial/serial.c that I recommended in the previous email. I also checked in a subset of your STM32 serial driver
                  Message 8 of 13 , Apr 1, 2012
                    Mike,

                    I just checked in the change to drivers/serial/serial.c that I recommended in the previous email. I also checked in a subset of your STM32 serial driver changes.

                    The only testing that I did here was on an F1 to assure that I still have a working serial console. There could be other issues. Let me know if there are problems or if you have other ideas.

                    Greg
                  • Michael Smith
                    Greg, Thanks; these should get us a long way as they stand, and I ll definitely let you know where we end up w.r.t. performance changes. If anyone else is
                    Message 9 of 13 , Apr 1, 2012
                      Greg,

                      Thanks; these should get us a long way as they stand, and I'll definitely let you know where we end up w.r.t. performance changes.

                      If anyone else is looking at serial throughput on STM32 or elsewhere, I'm interested to hear your experiences.

                       = Mike

                      On Apr 1, 2012, at 6:04 PM, Gregory N wrote:

                       

                      Mike,

                      I just checked in the change to drivers/serial/serial.c that I recommended in the previous email. I also checked in a subset of your STM32 serial driver changes.

                      The only testing that I did here was on an F1 to assure that I still have a working serial console. There could be other issues. Let me know if there are problems or if you have other ideas.

                      Greg


                    • Gregory N
                      Mike, ... I don t think that the problem is, strictly speaking, a throughput problem. Isn t the issue that the tiny STM32 FIFOs are being filled and overrun
                      Message 10 of 13 , Apr 2, 2012
                        Mike,

                        > If anyone else is looking at serial throughput on STM32 or elsewhere, I'm interested to hear your experiences.

                        I don't think that the problem is, strictly speaking, a throughput problem. Isn't the issue that the tiny STM32 FIFOs are being filled and overrun because data reception is being held off.

                        So data throughput could be great, but short bursts of high rate data could result in data overruns if the "upper half" driver is not accepting data at that moment.

                        BTW: You might want to look how I handle this in the USB serial driver. That driver uses the same upper half serial driver. USB serial has the issue that you really cannot effectively disable Rx interrupts at all. (Well, you could, but you would have to start NAKing USB packets so it would require some levels of control through the USB architecture that don't exist now).

                        The USB serial driver keeps a shadow copy of the ring buffer tail pointer. When the upper half "disables RX interrupts", the lower half simply switches to the shadow copy of the ring buffer tail pointer. When Rx interrupts are re-enabled, it goes back to the real ring buffer tail pointer. So the lower half continues to receive and buffer serial data, even when RX interrupts are disabled but the result is invisible to the upper half driver.

                        This is why the drivers/serial/serialirq.c file exists. It is the break-off portion of the serial driver that gets replaced when using the USB serial driver.

                        You have a similar problem in the STM32 because it clearly has insufficient UART FIFO depth. Without deep FIFOs or without DMA, you may not be able to disable RX interrupts at all. So, if all else fails, you can also consider the USB serial driver solution (see drivers/usbdev/cdcacm.c).

                        Greg
                      • Gregory N
                        Mike, ... I have implemented the logic to access the ring buffer without disabling Rx interrupts and checked this in (with so far only superficial testing).
                        Message 11 of 13 , Apr 18, 2012
                          Mike,

                          With regard to this older discussion about disabling Rx interrupts in the serial driver when extracting data from the interrupt-driven ring buffer (drivers/serial/serial.c):

                          > > ... (If you are really careful you should even be able implement one side of a FIFO without disabling the other side at all.).
                          > >
                          > For a strict producer/consumer ringbuffer, as long as updates are atomic there's no need for locking as long as you're careful with your order of operations. The only case where I can see that lock being required is on an 8-bit micro where the FIFO is more than 256 bytes large.
                          >
                          > If that is not a supported configuration, then removing that lock should help in a number of cases.

                          I have implemented the logic to access the ring buffer without disabling Rx interrupts and checked this in (with so far only superficial testing). This should give you a little better Rx throughput.

                          Your comment about issues with non-atomic 8-bit accesses to fetch the full 16-bit index values gave me some pause. But in all of the 8-bit micros that I am aware of, 16-bit loads and stores are performed using atomic 16-bit data accesses. So while this could be a problem with some hypothetical 8-bit architecture (or with a very bad compiler) I don't think it is a real world case that we need to concern ourselves with.

                          Correct me if I am wrong. If anyone knows of an 8-bit micro that will not support atomic 16-bit accesses, then I will need to revisit this yet again.

                          Greg
                        • Michael Smith
                          ... It s only an issue if the serial buffer is larger than 256 bytes, which is a pretty rare combination. There are a number of 8-bit architectures that would
                          Message 12 of 13 , Apr 18, 2012

                            On Apr 18, 2012, at 10:24 AM, Gregory N wrote:

                             

                            Your comment about issues with non-atomic 8-bit accesses to fetch the full 16-bit index values gave me some pause. But in all of the 8-bit micros that I am aware of, 16-bit loads and stores are performed using atomic 16-bit data accesses. So while this could be a problem with some hypothetical 8-bit architecture (or with a very bad compiler) I don't think it is a real world case that we need to concern ourselves with.

                            Correct me if I am wrong. If anyone knows of an 8-bit micro that will not support atomic 16-bit accesses, then I will need to revisit this yet again.

                            It's only an issue if the serial buffer is larger than 256 bytes, which is a pretty rare combination.  There are a number of 8-bit architectures that would have trouble with this, but in all honestly that would be the least of their worries when it comes to running NuttX.

                            Thanks for taking a look at this; I have some other serial-related issues to look at so I'll have a chance to get you some feedback on this soon.

                             = Mike

                          Your message has been successfully submitted and would be delivered to recipients shortly.