From e57a5755cc8fdcc5a4b48f41cafce53dfc0d77f2 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sun, 6 Aug 2017 16:56:02 -0600 Subject: [PATCH] TX timeout must also be cancelled on a TX error. TX timeout should check if we are waiting for a TXto complete. TX timeout was too short (short than the maximum number of retries). --- configs/b-l475e-iot01a/README.txt | 14 ++-- drivers/bch/Make.defs | 6 +- .../wireless/spirit/drivers/spirit_netdev.c | 65 ++++++++++++------- 3 files changed, 50 insertions(+), 35 deletions(-) diff --git a/configs/b-l475e-iot01a/README.txt b/configs/b-l475e-iot01a/README.txt index dc3c0cfd06..788cf1e1a5 100644 --- a/configs/b-l475e-iot01a/README.txt +++ b/configs/b-l475e-iot01a/README.txt @@ -523,15 +523,10 @@ Configuration sub-directories length to 84 and that did NOT eliminate the RX FIFO error. At the end of the TCP test, the "nsh> ifconfig" command shows that - there were two TX timeouts. Perhaps this is related? The TX timeout - is set to 5 seconds, so this could be a serious performance issue. - - So for now I have to live with the RX FIFO error. I have observed - only a single RX FIFO error and it occurs at the same place in the - the TCP test (when the data is turned around and sent back to the - client). the UDP test and Telnet work perfectly. The RX FIFO error - is handled perfectly and, since it is TCP, there is no loss of data - and all tests pass. That is as good as I can do for now. + there were two TX timeouts. Perhaps this is related? I found that + the TX timeout was not being cancelled. It must be canceled on each + TX completed or TX error. This DID eliminate the RX FIFO error, but + now the test hangs and does not complete. Another Errata: "Using the STack packet format and no CRC field, the reading from RX FIFO to the last received byte, is not possible. ..." @@ -542,4 +537,3 @@ Configuration sub-directories Reducing the FIFO to 94 bytes fixed the problem with the 2 byte CRC but did not resolve that occasional RX FIFO error. - diff --git a/drivers/bch/Make.defs b/drivers/bch/Make.defs index 78dfbff30e..18225c5593 100644 --- a/drivers/bch/Make.defs +++ b/drivers/bch/Make.defs @@ -38,9 +38,9 @@ ifneq ($(CONFIG_DISABLE_MOUNTPOINT),y) # Include BCH driver -CSRCS += bchlib_setup.c bchlib_teardown.c bchlib_read.c bchlib_write.c \ - bchlib_cache.c bchlib_sem.c bchdev_register.c bchdev_unregister.c \ - bchdev_driver.c +CSRCS += bchlib_setup.c bchlib_teardown.c bchlib_read.c bchlib_write.c +CSRCS += bchlib_cache.c bchlib_sem.c bchdev_register.c bchdev_unregister.c +CSRCS += bchdev_driver.c # Include BCH driver build support diff --git a/drivers/wireless/spirit/drivers/spirit_netdev.c b/drivers/wireless/spirit/drivers/spirit_netdev.c index 4038af736f..bde0d22ffc 100644 --- a/drivers/wireless/spirit/drivers/spirit_netdev.c +++ b/drivers/wireless/spirit/drivers/spirit_netdev.c @@ -232,14 +232,20 @@ #define SPIRIT_WDDELAY (1*CLK_TCK) -/* TX timeout = 5 seconds */ +/* Maximum number of retries (10) */ -#define SPIRIT_TXTIMEOUT (5*CLK_TCK) +#define SPIRIT_MAX_RETX PKT_N_RETX_10 -/* RX timeout = 1.5 seconds */ +/* RX timeout = 1.5 seconds. Transmitter will wait this amount timer for + * an ACK from the receiver (per transmission). + */ #define SPIRIT_RXTIMEOUT 1500.0 +/* Failsafe TX timeout = MAX_RETX * RXTIMEOUT + 1 = 16 seconds */ + +#define SPIRIT_TXTIMEOUT (16*CLK_TCK) + /**************************************************************************** * Private Types ****************************************************************************/ @@ -1081,10 +1087,16 @@ static void spirit_interrupt_work(FAR void *arg) DEBUGVERIFY(spirit_command(spirit, COMMAND_FLUSHTXFIFO)); irqstatus.IRQ_TX_DATA_SENT = 0; - /* Revert the sending state */ + /* Were we in the sending state? */ if (priv->state == DRIVER_STATE_SENDING) { + /* Yes.. Cancel the TX timeout */ + + wd_cancel(priv->txtimeout); + + /* Revert the sending state */ + priv->state = DRIVER_STATE_IDLE; } @@ -1106,7 +1118,7 @@ static void spirit_interrupt_work(FAR void *arg) { wlinfo("Data sent\n"); - /* Disable the TX timeout */ + /* Cancel the TX timeout */ wd_cancel(priv->txtimeout); @@ -1407,10 +1419,10 @@ static void spirit_interrupt_work(FAR void *arg) if (irqstatus.IRQ_RX_DATA_DISC != 0) { - wlinfo("Data discarded: Node addr=%02x RX dest addr=%02x\n", + wlwarn("WARNING: Data discarded: Node addr=%02x RX dest addr=%02x\n", spirit_pktcommon_get_nodeaddress(spirit), spirit_pktcommon_get_rxdestaddr(spirit)); - wlinfo(" CRC error=%u RX timeout=%u\n", + wlwarn(" CRC error=%u RX timeout=%u\n", irqstatus.IRQ_CRC_ERROR, irqstatus.IRQ_RX_TIMEOUT); /* Flush the RX FIFO and revert the receiving state */ @@ -1500,29 +1512,38 @@ static void spirit_txtimeout_work(FAR void *arg) { FAR struct spirit_driver_s *priv = (FAR struct spirit_driver_s *)arg; - /* Lock the network and serialize driver operations if necessary. - * NOTE: Serialization is only required in the case where the driver work - * is performed on an LP worker thread and where more than one LP worker - * thread has been configured. + wlwarn("WARNING: TX Timeout. state=%u\n", priv->state); + + /* Are we in the sending state? If not, then this must be a spurious + * timout. */ - net_lock(); + if (priv->state == DRIVER_STATE_SENDING) + { + /* Lock the network and serialize driver operations if necessary. + * NOTE: Serialization is only required in the case where the driver + * work is performed on an LP worker thread and where more than one LP + * worker thread has been configured. + */ - /* Increment statistics and dump debug info */ + net_lock(); - NETDEV_TXTIMEOUTS(&priv->radio.r_dev); + /* Increment statistics and dump debug info */ - /* Then reset the hardware */ + NETDEV_TXTIMEOUTS(&priv->radio.r_dev); - spirit_ifdown(&priv->radio.r_dev); - spirit_ifup(&priv->radio.r_dev); + /* Then reset the hardware */ - /* Then schedule to poll the network for new XMIT data on the LP worker - * thread. - */ + spirit_ifdown(&priv->radio.r_dev); + spirit_ifup(&priv->radio.r_dev); - work_queue(LPWORK, &priv->pollwork, spirit_txpoll_work, priv, 0); - net_unlock(); + /* Then schedule to poll the network for new XMIT data on the LP + * worker thread. + */ + + work_queue(LPWORK, &priv->pollwork, spirit_txpoll_work, priv, 0); + net_unlock(); + } } /****************************************************************************