Skip to content

Commit e1149b7

Browse files
committed
drivers/can: Fix close drain, write-only reader lifecycle, and STM32 RX header
Always allocate per-file can_reader on open so write-only descriptors get msgalign / ioctl state; free that context on close when it was never linked into cd_readers (fixes leak). Release the critical section before waiting for SW/HW TX drain on last close, and cap each drain loop (20×500 ms) so close() cannot block forever when bxCAN retries without ACK or dev_txempty never clears. In stm32 CAN RX interrupt, zero the full can_hdr before filling it so padding and optional fields are not stack garbage (avoids false memcmp failures vs TX path, e.g. loopback examples). Fix stm32can_vputreg debug log to print the written value (correct parameter name). Signed-off-by: Alexey Matveev <tippet@yandex.ru>
1 parent a33c555 commit e1149b7

File tree

2 files changed

+91
-24
lines changed

2 files changed

+91
-24
lines changed

arch/arm/src/stm32/stm32_can.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ static void stm32can_vputreg(uint32_t addr, uint32_t value)
365365
{
366366
/* Show the register value being written */
367367

368-
caninfo("%08" PRIx32 "->%08" PRIx32 "\n", addr, val);
368+
caninfo("%08" PRIx32 "->%08" PRIx32 "\n", addr, value);
369369

370370
/* Write the value */
371371

@@ -1533,6 +1533,14 @@ static int stm32can_rxinterrupt(struct can_dev_s *dev, int rxmb)
15331533
DEBUGASSERT(dev != NULL && dev->cd_priv != NULL);
15341534
priv = dev->cd_priv;
15351535

1536+
/* Clear full header:
1537+
* padding and optional fields (timestamp, etc.) must
1538+
* not contain stack garbage or memcmp() in apps (e.g. examples/can with
1539+
* CONFIG_CAN_LOOPBACK) will falsely fail vs. zero-initialized TX header.
1540+
*/
1541+
1542+
memset(&hdr, 0, sizeof(hdr));
1543+
15361544
/* Verify that a message is pending in the FIFO */
15371545

15381546
regval = stm32can_getreg(priv, STM32_CAN_RFR_OFFSET(rxmb));

drivers/can/can.c

Lines changed: 82 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,13 @@
8989
#define HALF_SECOND_MSEC 500
9090
#define HALF_SECOND_USEC 500000L
9191

92+
/* can_close waits for SW queue and H/W TX to drain. Without a second bus
93+
* node (no ACK) bxCAN may retry indefinitely; dev_txempty() then never
94+
* becomes true. Bound the wait so close() does not hang forever.
95+
*/
96+
97+
#define CAN_CLOSE_DRAIN_LOOPS 20u /* 20 * 500 ms = 10 s per stage */
98+
9299
/****************************************************************************
93100
* Private Function Prototypes
94101
****************************************************************************/
@@ -257,14 +264,22 @@ static int can_open(FAR struct file *filep)
257264

258265
if (ret == OK)
259266
{
267+
FAR struct can_reader_s *reader;
268+
260269
dev->cd_crefs++;
261270

262-
/* Update the reader list only if driver was open for reading */
271+
/* Per-file context (msgalign, optional ioctl FIFO). Always
272+
* allocated: write-only needs msgalign / CANIOC_* without O_RDOK.
273+
* Receive path and poll() only use readers that are also queued
274+
* on cd_readers (see below).
275+
*/
276+
277+
reader = init_can_reader(filep);
263278

264279
if ((filep->f_oflags & O_RDOK) != 0)
265280
{
266281
list_add_head(&dev->cd_readers,
267-
(FAR struct list_node *)init_can_reader(filep));
282+
(FAR struct list_node *)reader);
268283
}
269284
}
270285

@@ -305,29 +320,43 @@ static int can_close(FAR struct file *filep)
305320

306321
flags = enter_critical_section(); /* Disable interrupts */
307322

308-
list_for_every(&dev->cd_readers, node)
309323
{
310-
if (((FAR struct can_reader_s *)node) ==
311-
((FAR struct can_reader_s *)filep->f_priv))
324+
FAR struct can_reader_s *priv =
325+
(FAR struct can_reader_s *)filep->f_priv;
326+
bool onlist = false;
327+
328+
list_for_every(&dev->cd_readers, node)
312329
{
313-
FAR struct can_reader_s *reader = (FAR struct can_reader_s *)node;
314-
FAR struct can_rxfifo_s *fifo = &reader->fifo;
330+
if (((FAR struct can_reader_s *)node) == priv)
331+
{
332+
FAR struct can_reader_s *reader =
333+
(FAR struct can_reader_s *)node;
334+
FAR struct can_rxfifo_s *fifo = &reader->fifo;
315335

316-
/* Unlock the binary semaphore, waking up can_read if it
317-
* is blocked.
318-
*/
336+
/* Unlock the binary semaphore, waking up can_read if it
337+
* is blocked.
338+
*/
319339

320-
nxsem_post(&fifo->rx_sem);
340+
nxsem_post(&fifo->rx_sem);
321341

322-
/* Notify specific poll/select waiter that they can read from the
323-
* cd_recv buffer
324-
*/
342+
/* Notify specific poll/select waiter that they can read from
343+
* the cd_recv buffer
344+
*/
325345

326-
poll_notify(&reader->cd_fds, 1, POLLHUP);
327-
reader->cd_fds = NULL;
328-
list_delete(node);
329-
kmm_free(node);
330-
break;
346+
poll_notify(&reader->cd_fds, 1, POLLHUP);
347+
reader->cd_fds = NULL;
348+
list_delete(node);
349+
kmm_free(node);
350+
onlist = true;
351+
break;
352+
}
353+
}
354+
355+
/* Write-only opens use init_can_reader() but are not on cd_readers */
356+
357+
if (!onlist && priv != NULL)
358+
{
359+
kmm_free(priv);
331360
}
332361
}
333362

@@ -341,26 +370,56 @@ static int can_close(FAR struct file *filep)
341370
goto errout;
342371
}
343372

373+
/* Last client: release the critical section before draining TX.
374+
* Long sleeps with IRQs masked can prevent the TX-complete ISR
375+
* from running and stall the SW queue; the same applies when
376+
* the controller retries on NACK.
377+
*/
378+
379+
leave_critical_section(flags);
380+
344381
/* Stop accepting input */
345382

346383
dev_rxint(dev, false);
347384

348385
/* Now we wait for the sender to clear */
349386

350-
while (!TX_EMPTY(&dev->cd_sender))
351387
{
352-
nxsched_usleep(HALF_SECOND_USEC);
388+
unsigned int n = 0;
389+
390+
while (!TX_EMPTY(&dev->cd_sender) && n < CAN_CLOSE_DRAIN_LOOPS)
391+
{
392+
nxsched_usleep(HALF_SECOND_USEC);
393+
n++;
394+
}
395+
396+
if (!TX_EMPTY(&dev->cd_sender))
397+
{
398+
canerr("CAN close: SW TX queue still not empty after timeout\n");
399+
}
353400
}
354401

355402
/* And wait for the hardware sender to drain */
356403

357-
while (!dev_txempty(dev))
358404
{
359-
nxsched_usleep(HALF_SECOND_USEC);
405+
unsigned int n = 0;
406+
407+
while (!dev_txempty(dev) && n < CAN_CLOSE_DRAIN_LOOPS)
408+
{
409+
nxsched_usleep(HALF_SECOND_USEC);
410+
n++;
411+
}
412+
413+
if (!dev_txempty(dev))
414+
{
415+
canerr("CAN close: H/W TX still busy after timeout "
416+
"(no ACK / bus-off / stuck mailbox)\n");
417+
}
360418
}
361419

362420
/* Free the IRQ and disable the CAN device */
363421

422+
flags = enter_critical_section();
364423
dev_shutdown(dev); /* Disable the CAN */
365424

366425
errout:

0 commit comments

Comments
 (0)