Skip to content

Commit 34b3dde

Browse files
committed
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).
Cap each drain loop (20×500 ms) so close() cannot block forever when bxCAN retries without ACK or dev_txempty never clears. 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 34b3dde

2 files changed

Lines changed: 63 additions & 17 deletions

File tree

arch/arm/src/stm32/stm32_can.c

Lines changed: 1 addition & 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

drivers/can/can.c

Lines changed: 62 additions & 16 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

@@ -287,11 +302,14 @@ static int can_open(FAR struct file *filep)
287302

288303
static int can_close(FAR struct file *filep)
289304
{
290-
FAR struct inode *inode = filep->f_inode;
291-
FAR struct can_dev_s *dev = inode->i_private;
292-
irqstate_t flags;
293-
FAR struct list_node *node;
294-
int ret;
305+
FAR struct inode *inode = filep->f_inode;
306+
FAR struct can_dev_s *dev = inode->i_private;
307+
irqstate_t flags;
308+
FAR struct list_node *node;
309+
FAR struct can_reader_s *priv;
310+
bool onlist;
311+
int ret;
312+
unsigned int n;
295313

296314
#ifdef CONFIG_DEBUG_CAN_INFO
297315
caninfo("ocount: %u\n", dev->cd_crefs);
@@ -305,12 +323,15 @@ static int can_close(FAR struct file *filep)
305323

306324
flags = enter_critical_section(); /* Disable interrupts */
307325

326+
priv = (FAR struct can_reader_s *)filep->f_priv;
327+
onlist = false;
328+
308329
list_for_every(&dev->cd_readers, node)
309330
{
310-
if (((FAR struct can_reader_s *)node) ==
311-
((FAR struct can_reader_s *)filep->f_priv))
331+
if (((FAR struct can_reader_s *)node) == priv)
312332
{
313-
FAR struct can_reader_s *reader = (FAR struct can_reader_s *)node;
333+
FAR struct can_reader_s *reader =
334+
(FAR struct can_reader_s *)node;
314335
FAR struct can_rxfifo_s *fifo = &reader->fifo;
315336

316337
/* Unlock the binary semaphore, waking up can_read if it
@@ -319,18 +340,26 @@ static int can_close(FAR struct file *filep)
319340

320341
nxsem_post(&fifo->rx_sem);
321342

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

326347
poll_notify(&reader->cd_fds, 1, POLLHUP);
327348
reader->cd_fds = NULL;
328349
list_delete(node);
329350
kmm_free(node);
351+
onlist = true;
330352
break;
331353
}
332354
}
333355

356+
/* Write-only opens use init_can_reader() but are not on cd_readers */
357+
358+
if (!onlist && priv != NULL)
359+
{
360+
kmm_free(priv);
361+
}
362+
334363
filep->f_priv = NULL;
335364
dev->cd_crefs--;
336365

@@ -347,16 +376,33 @@ static int can_close(FAR struct file *filep)
347376

348377
/* Now we wait for the sender to clear */
349378

350-
while (!TX_EMPTY(&dev->cd_sender))
351379
{
352-
nxsched_usleep(HALF_SECOND_USEC);
380+
for (n = 0;
381+
!TX_EMPTY(&dev->cd_sender) && n < CAN_CLOSE_DRAIN_LOOPS;
382+
n++)
383+
{
384+
nxsched_usleep(HALF_SECOND_USEC);
385+
}
386+
387+
if (!TX_EMPTY(&dev->cd_sender))
388+
{
389+
canerr("CAN close: SW TX queue still not empty after timeout\n");
390+
}
353391
}
354392

355393
/* And wait for the hardware sender to drain */
356394

357-
while (!dev_txempty(dev))
358395
{
359-
nxsched_usleep(HALF_SECOND_USEC);
396+
for (n = 0; !dev_txempty(dev) && n < CAN_CLOSE_DRAIN_LOOPS; n++)
397+
{
398+
nxsched_usleep(HALF_SECOND_USEC);
399+
}
400+
401+
if (!dev_txempty(dev))
402+
{
403+
canerr("CAN close: H/W TX still busy after timeout "
404+
"(no ACK / bus-off / stuck mailbox)\n");
405+
}
360406
}
361407

362408
/* Free the IRQ and disable the CAN device */

0 commit comments

Comments
 (0)