mirror of
https://github.com/apache/nuttx.git
synced 2025-12-08 10:55:51 +08:00
drivers/can: correctly unblock only one read at the time
The previous version of the code was posting the semaphore every time the value was less or equal to the zero. In case there is read already waiting on the semaphore the value will be -1. The first frame will post the value to 0 and thus unblocking the waiting read. The read will take frames out of FIFO. During that time if next frame is received it will be placed to the FIFO but because rx_sem has value 0 at that time it will be posted to value 1. Once read finishes the execution it will check if there is something still in the FIFO and posts semaphore back (expecting that the value is 0) thus increasing value to 2. Thus we end up with read being unblocked once more triggering 'RX FIFO sem posted but FIFO is empty.' error branch and returning zero (signaling EOF?). The intuitive fix is to check the value of the semaphore before posting it in the read's tail. This could have race condition case when interrupt is delivered between nxsem_get_value and nxsem_post and can_receive is called. This fix instead uses the same condition to detect if semaphore must be posted as read does. Thus we check if previously the FIFO was empty. We post the semaphore on in case FIFO was initially empty, otherwise we expect semaphore to be fully managed by can_read. Signed-off-by: Karel Kočí <cynerd@email.cz>
This commit is contained in:
@@ -1170,6 +1170,7 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr,
|
||||
int ret = -ENOMEM;
|
||||
int i;
|
||||
int sval;
|
||||
bool was_empty;
|
||||
|
||||
caninfo("ID: %" PRId32 " DLC: %d\n", (uint32_t)hdr->ch_id, hdr->ch_dlc);
|
||||
|
||||
@@ -1234,6 +1235,7 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr,
|
||||
{
|
||||
FAR struct can_reader_s *reader = (FAR struct can_reader_s *)node;
|
||||
fifo = &reader->fifo;
|
||||
was_empty = fifo->rx_head == fifo->rx_tail;
|
||||
|
||||
nexttail = fifo->rx_tail + 1;
|
||||
if (nexttail >= CONFIG_CAN_RXFIFOSIZE)
|
||||
@@ -1270,22 +1272,12 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr,
|
||||
|
||||
fifo->rx_tail = nexttail;
|
||||
|
||||
if (nxsem_get_value(&fifo->rx_sem, &sval) < 0)
|
||||
{
|
||||
#ifdef CONFIG_CAN_ERRORS
|
||||
/* Report unspecified error */
|
||||
|
||||
fifo->rx_error |= CAN_ERROR5_UNSPEC;
|
||||
#endif
|
||||
continue;
|
||||
}
|
||||
|
||||
/* Unlock the binary semaphore, waking up can_read if it is
|
||||
* blocked. If can_read were not blocked, we would not be
|
||||
* executing this because interrupts would be disabled.
|
||||
*/
|
||||
|
||||
if (sval <= 0)
|
||||
if (was_empty)
|
||||
{
|
||||
nxsem_post(&fifo->rx_sem);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user