MTres19 commented on a change in pull request #2641:
URL: https://github.com/apache/incubator-nuttx/pull/2641#discussion_r774280528
##########
File path: include/nuttx/can/can.h
##########
@@ -491,7 +491,13 @@ begin_packed_struct struct can_msg_s
struct can_rxfifo_s
{
- sem_t rx_sem; /* Counting semaphore */
+ /* Binary semaphore. Indicates whether FIFO is available for reading
+ * AND not empty. Only take this sem inside a critical section to guarantee
+ * exclusive access to both the semaphore and the head/tail FIFO indices.
+ */
+
+ sem_t rx_sem;
Review comment:
No; `rx_sem` is not used as a lock because the ISR (which calls
`can_receive()`) can edit the FIFO whenever it wants as long as interrupts are
enabled. `rx_sem` exists to allow `can_receive()` to wake up a reader thread,
but the reader thread then has exclusive access only because it uses a critical
section, not because it took the semaphore.
See also my reply above.
Actually, `rx_sem` was a binary semaphore before too, but "0" and "1" didn't
have any defined meaning because it was initialized to 1 and then immediately
decremented in a loop in `can_read()` and `can_poll()`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
