CAN driver: Removing call to can_xmit() from can_txready() in a previous commit was a mistake. This commit restores the call to can_xmit(), but does the call in a safer environment on the work thread.

This commit is contained in:
Gregory Nutt 2015-11-14 10:33:02 -06:00
parent 4a7fb2cbc1
commit 81ab270a96
3 changed files with 146 additions and 17 deletions

View File

@ -128,6 +128,7 @@ config CAN_NPENDINGRTR
config CAN_TXREADY
bool "can_txready interface"
default n
select SCHED_WORKQUEUE
---help---
This selection enables the can_txready() interface. This interface
is needed only for CAN hardware that supports queing of outgoing
@ -173,6 +174,22 @@ config CAN_TXREADY
no longer full. can_txready() will then awaken the
can_write() logic and the hang condition is avoided.
choice
prompt "TX Ready Work Queue"
default CAN_TXREADY_LOPRI if SCHED_LPWORK
default CAN_TXREADY_HIPRI if !SCHED_LPWORK
depends on CAN_TXREADY
config CAN_TXREADY_LOPRI
bool "Low-priority work queue"
select SCHED_LPWORK
config CAN_TXREADY_HIPRI
bool "High-priority work queue"
select SCHED_HPWORK
endchoice # TX Ready Work Queue
config CAN_LOOPBACK
bool "CAN loopback mode"
default n

View File

@ -54,6 +54,10 @@
#include <nuttx/arch.h>
#include <nuttx/can.h>
#ifdef CONFIG_CAN_TXREADY
# include <nuttx/wqueue.h>
#endif
#include <arch/irq.h>
#ifdef CONFIG_CAN
@ -61,6 +65,37 @@
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
/* Configuration ************************************************************/
#ifdef CONFIG_CAN_TXREADY
# if !defined(CONFIG_SCHED_WORKQUEUE)
# error Work queue support required in this configuration
# undef CONFIG_CAN_TXREADY
# undef CONFIG_CAN_TXREADY_LOPRI
# undef CONFIG_CAN_TXREADY_HIPRI
# elif defined(CONFIG_CAN_TXREADY_LOPRI)
# undef CONFIG_CAN_TXREADY_HIPRI
# ifdef CONFIG_SCHED_LPWORK
# define CANWORK LPWORK
# else
# error Low priority work queue support required in this configuration
# undef CONFIG_CAN_TXREADY
# undef CONFIG_CAN_TXREADY_LOPRI
# endif
# elif defined(CONFIG_CAN_TXREADY_HIPRI)
# ifdef CONFIG_SCHED_HPWORK
# define CANWORK HPWORK
# else
# error High priority work queue support required in this configuration
# undef CONFIG_CAN_TXREADY
# undef CONFIG_CAN_TXREADY_HIPRI
# endif
# else
# error No work queue selection
# undef CONFIG_CAN_TXREADY
# endif
#endif
/* Debug ********************************************************************/
/* Non-standard debug that may be enabled just for testing CAN */
@ -95,6 +130,9 @@ static uint8_t can_dlc2bytes(uint8_t dlc);
#if 0 /* Not used */
static uint8_t can_bytes2dlc(uint8_t nbytes);
#endif
#ifdef CONFIG_CAN_TXREADY
static void can_txready_work(FAR void *arg);
#endif
/* Character driver methods */
@ -246,6 +284,50 @@ static uint8_t can_bytes2dlc(FAR struct sam_can_s *priv, uint8_t nbytes)
}
#endif
/****************************************************************************
* Name: can_txready_work
*
* Description:
* This function performs deferred processing from can_txready. See the
* discription of can_txready below for additionla information.
*
****************************************************************************/
#ifdef CONFIG_CAN_TXREADY
static void can_txready_work(FAR void *arg)
{
FAR struct can_dev_s *dev = (FAR struct can_dev_s *)arg;
irqstate_t flags;
canllvdbg("xmit head: %d queue: %d tail: %d\n",
dev->cd_xmit.tx_head, dev->cd_xmit.tx_queue,
dev->cd_xmit.tx_tail);
/* Verify that the xmit FIFO is not empty. The following operations must
* be performed with interrupt disabled.
*/
flags = irqsave();
if (dev->cd_xmit.tx_head != dev->cd_xmit.tx_tail)
{
/* Send the next message in the FIFO. */
(void)can_xmit(dev);
/* Are there any threads waiting for space in the TX FIFO? */
if (dev->cd_ntxwaiters > 0)
{
/* Yes.. Inform them that new xmit space is available */
(void)sem_post(&dev->cd_xmit.tx_sem);
}
}
irqrestore(flags);
}
#endif
/****************************************************************************
* Name: can_open
*
@ -532,12 +614,12 @@ static int can_xmit(FAR struct can_dev_s *dev)
{
DEBUGASSERT(dev->cd_xmit.tx_queue == dev->cd_xmit.tx_head);
#ifndef CONFIG_CAN_TXREADY
/* We can disable CAN TX interrupts -- unless there is a H/W FIFO. In
* that case, TX interrupts must stay enabled until the H/W FIFO is
* fully emptied.
*/
#ifndef CONFIG_CAN_TXREADY
dev_txint(dev, false);
#endif
return -EIO;
@ -1036,9 +1118,11 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr,
* 2b. H/W TX FIFO (CONFIG_CAN_TXREADY=y) and S/W TX FIFO full
*
* In this case, the thread calling can_write() is blocked waiting for
* space in the S/W TX FIFO.
* space in the S/W TX FIFO. can_txdone() will be called, indirectly,
* from can_txready_work() running on the thread of the work queue.
*
* CAN interrupt -> can_txready() -> can_xmit() -> dev_send() -> can_txdone()
* CAN interrupt -> can_txready() -> Schedule can_txready_work()
* can_txready_work() -> can_xmit() -> dev_send() -> can_txdone()
*
* The call dev_send() should not fail in this case and the subsequent
* call to can_txdone() will make space in the S/W TX FIFO and will
@ -1179,34 +1263,49 @@ int can_txready(FAR struct can_dev_s *dev)
if (dev->cd_xmit.tx_head != dev->cd_xmit.tx_tail)
{
/* Are there any threads waiting for space in the xmit FIFO? */
/* Is work already scheduled? */
if (dev->cd_ntxwaiters > 0)
if (work_available(&dev->cd_work))
{
/* Inform one waiter that space is now available in the S/W
* TX FIFO.
/* Yes... schedule to perform can_txready() work on the worker
* thread. Although data structures are protected by disabling
* interrupts, the can_xmit() operations may involve semaphore
* operations and, hence, should not be done at the interrupt
* level.
*/
ret = sem_post(&dev->cd_xmit.tx_sem);
ret = work_queue(CANWORK, &dev->cd_work, can_txready_work, dev, 0);
}
else
{
ret = -EBUSY;
}
}
#if 0 /* REVISIT */
/* REVISIT: Does the fact that the S/W FIFO is empty also mean that the
* H/W FIFO is also empty? If we really want this to work this way, then
* we would probably need and additional parameter to tell us if the H/W
* FIFO is empty.
*/
else
{
/* There should not be any threads waiting for space in the S/W TX
* FIFO is it is empty.
*
* REVISIT: Assertion can fire in certain race conditions, i.e, when
* all waiters have been awakened but have not yet had a chance to
* decrement cd_ntxwaiters.
*/
//DEBUGASSERT(dev->cd_ntxwaiters == 0);
#if 0 /* REVISIT */
/* When the H/W FIFO has been emptied, we can disable further TX
* interrupts.
*
* REVISIT: The fact that the S/W FIFO is empty does not mean that
* the H/W FIFO is also empty. If we really want this to work this
* way, then we would probably need and additional parameter to tell
* us if the H/W FIFO is empty.
*/
dev_txint(dev, false);
}
#endif
}
return ret;
}

View File

@ -51,6 +51,10 @@
#include <nuttx/fs/fs.h>
#include <nuttx/fs/ioctl.h>
#ifdef CONFIG_CAN_TXREADY
# include <nuttx/wqueue.h>
#endif
#ifdef CONFIG_CAN
/************************************************************************************
@ -70,6 +74,12 @@
* CONFIG_CAN_LOOPBACK - A CAN driver may or may not support a loopback
* mode for testing. If the driver does support loopback mode, the setting
* will enable it. (If the driver does not, this setting will have no effect).
* CONFIG_CAN_TXREADY - Add support for the can_txready() callback. This is needed
* only for CAN hardware the supports an separate H/W TX message FIFO. The call
* back is needed to keep the S/W FIFO and the H/W FIFO in sync. Work queue
* support is needed for this feature.
* CONFIG_CAN_TXREADY_HIPRI or CONFIG_CAN_TXREADY_LOPRI - Selects which work queue
* will be used for the can_txready() processing.
*/
/* Default configuration settings that may be overridden in the NuttX configuration
@ -405,6 +415,9 @@ struct can_dev_s
sem_t cd_closesem; /* Locks out new opens while close is in progress */
struct can_txfifo_s cd_xmit; /* Describes transmit FIFO */
struct can_rxfifo_s cd_recv; /* Describes receive FIFO */
#ifdef CONFIG_CAN_TXREADY
struct work_s cd_work; /* Use to manage can_txready() work */
#endif
/* List of pending RTR requests */
struct can_rtrwait_s cd_rtr[CONFIG_CAN_NPENDINGRTR];
FAR const struct can_ops_s *cd_ops; /* Arch-specific operations */